Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1610651 > unrolled thread
| Started by | Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
|---|---|
| First post | 2017-03-28 14:40 +0200 |
| Last post | 2017-04-06 16:30 +0200 |
| Articles | 4 — 3 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH 4.4 42/76] mmc: sdhci: Do not disable interrupts while waiting for clock Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-03-28 14:40 +0200
Re: [PATCH 4.4 42/76] mmc: sdhci: Do not disable interrupts while waiting for clock Ben Hutchings <ben.hutchings@codethink.co.uk> - 2017-04-04 19:00 +0200
Re: [PATCH 4.4 42/76] mmc: sdhci: Do not disable interrupts while waiting for clock Ludovic Desroches <ludovic.desroches@microchip.com> - 2017-04-06 14:20 +0200
Re: [PATCH 4.4 42/76] mmc: sdhci: Do not disable interrupts while waiting for clock Ben Hutchings <ben.hutchings@codethink.co.uk> - 2017-04-06 16:30 +0200
| From | Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
|---|---|
| Date | 2017-03-28 14:40 +0200 |
| Subject | [PATCH 4.4 42/76] mmc: sdhci: Do not disable interrupts while waiting for clock |
| Message-ID | <tpYzM-7we-21@gated-at.bofh.it> |
4.4-stable review patch. If anyone has any objections, please let me know. ------------------ From: Adrian Hunter <adrian.hunter@intel.com> commit e2ebfb2142acefecc2496e71360f50d25726040b upstream. Disabling interrupts for even a millisecond can cause problems for some devices. That can happen when sdhci changes clock frequency because it waits for the clock to become stable under a spin lock. The spin lock is not necessary here. Anything that is racing with changes to the I/O state is already broken. The mmc core already provides synchronization via "claiming" the host. Although the spin lock probably should be removed from the code paths that lead to this point, such a patch would touch too much code to be suitable for stable trees. Consequently, for this patch, just drop the spin lock while waiting. Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> Tested-by: Ludovic Desroches <ludovic.desroches@microchip.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> --- drivers/mmc/host/sdhci.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -1274,7 +1274,9 @@ clock_set: return; } timeout--; - mdelay(1); + spin_unlock_irq(&host->lock); + usleep_range(900, 1100); + spin_lock_irq(&host->lock); } clk |= SDHCI_CLOCK_CARD_EN;
[toc] | [next] | [standalone]
| From | Ben Hutchings <ben.hutchings@codethink.co.uk> |
|---|---|
| Date | 2017-04-04 19:00 +0200 |
| Subject | Re: [PATCH 4.4 42/76] mmc: sdhci: Do not disable interrupts while waiting for clock |
| Message-ID | <tszYd-7uN-3@gated-at.bofh.it> |
| In reply to | #1610651 |
On Tue, 2017-03-28 at 14:30 +0200, Greg Kroah-Hartman wrote: > 4.4-stable review patch. If anyone has any objections, please let me know. > > ------------------ > > From: Adrian Hunter <adrian.hunter@intel.com> > > commit e2ebfb2142acefecc2496e71360f50d25726040b upstream. > > Disabling interrupts for even a millisecond can cause problems for some > devices. That can happen when sdhci changes clock frequency because it > waits for the clock to become stable under a spin lock. > > The spin lock is not necessary here. Anything that is racing with changes > to the I/O state is already broken. The mmc core already provides > synchronization via "claiming" the host. [...] In mainline, drivers/mmc/host/sdhci-of-at91.c has a slightly different version of this code that seems to have the same issue. In 4.4 there's another (conditional) mdelay(1) further up this function that seems to be related to that hardware, and probably ought to have an unlock/lock around it. Ben. -- Ben Hutchings Software Developer, Codethink Ltd.
[toc] | [prev] | [next] | [standalone]
| From | Ludovic Desroches <ludovic.desroches@microchip.com> |
|---|---|
| Date | 2017-04-06 14:20 +0200 |
| Subject | Re: [PATCH 4.4 42/76] mmc: sdhci: Do not disable interrupts while waiting for clock |
| Message-ID | <tteyl-8ob-19@gated-at.bofh.it> |
| In reply to | #1616207 |
On Tue, Apr 04, 2017 at 05:50:50PM +0100, Ben Hutchings wrote: > On Tue, 2017-03-28 at 14:30 +0200, Greg Kroah-Hartman wrote: > > 4.4-stable review patch. If anyone has any objections, please let me know. > > > > ------------------ > > > > From: Adrian Hunter <adrian.hunter@intel.com> > > > > commit e2ebfb2142acefecc2496e71360f50d25726040b upstream. > > > > Disabling interrupts for even a millisecond can cause problems for some > > devices. That can happen when sdhci changes clock frequency because it > > waits for the clock to become stable under a spin lock. > > > > The spin lock is not necessary here. Anything that is racing with changes > > to the I/O state is already broken. The mmc core already provides > > synchronization via "claiming" the host. > [...] > > In mainline, drivers/mmc/host/sdhci-of-at91.c has a slightly different > version of this code that seems to have the same issue. In 4.4 there's > another (conditional) mdelay(1) further up this function that seems to > be related to that hardware, and probably ought to have an unlock/lock > around it. Right, how do you want to proceed? Do you want me to send a patch on top of it to manage this extra mdelay? Regards Ludovic
[toc] | [prev] | [next] | [standalone]
| From | Ben Hutchings <ben.hutchings@codethink.co.uk> |
|---|---|
| Date | 2017-04-06 16:30 +0200 |
| Subject | Re: [PATCH 4.4 42/76] mmc: sdhci: Do not disable interrupts while waiting for clock |
| Message-ID | <ttgA9-1jL-5@gated-at.bofh.it> |
| In reply to | #1617916 |
On Thu, 2017-04-06 at 14:12 +0200, Ludovic Desroches wrote: > On Tue, Apr 04, 2017 at 05:50:50PM +0100, Ben Hutchings wrote: > > On Tue, 2017-03-28 at 14:30 +0200, Greg Kroah-Hartman wrote: > > > 4.4-stable review patch. If anyone has any objections, please let me know. > > > > > > ------------------ > > > > > > From: Adrian Hunter <adrian.hunter@intel.com> > > > > > > commit e2ebfb2142acefecc2496e71360f50d25726040b upstream. > > > > > > Disabling interrupts for even a millisecond can cause problems for some > > > devices. That can happen when sdhci changes clock frequency because it > > > waits for the clock to become stable under a spin lock. > > > > > > The spin lock is not necessary here. Anything that is racing with changes > > > to the I/O state is already broken. The mmc core already provides > > > synchronization via "claiming" the host. > > [...] > > > > In mainline, drivers/mmc/host/sdhci-of-at91.c has a slightly different > > version of this code that seems to have the same issue. In 4.4 there's > > another (conditional) mdelay(1) further up this function that seems to > > be related to that hardware, and probably ought to have an unlock/lock > > around it. > > Right, how do you want to proceed? Do you want me to send a patch on top > of it to manage this extra mdelay? This change doesn't appear to break anything; I'm just saying that it's an incomplete fix. The other case where there's a delay with IRQs disabled should be fixed with an additional patch. Ben. -- Ben Hutchings Software Developer, Codethink Ltd.
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web