Re: [PACTH v3] mmc: sdhci: Do not allow tuning procedure to be interrupted
On 18/08/16 01:12, Christopher Freeman wrote: > On 08-17 01:31 PM, Robert Foss wrote: >> >> >> On 2016-08-17 06:47 AM, Adrian Hunter wrote: >>> On 17/08/16 00:25, robert.f...@collabora.com wrote: From: Christopher Freemanwait_event_interruptible_timeout() will return early if the blocked process receives a signal, causing the driver to abort the tuning procedure and possibly leaving the controller in a bad state. Since the tuning command is expected to complete quickly (<50ms) and we've set a timeout, use wait_event_timeout() instead. Signed-off-by: Christopher Freeman Tested-by: Robert Foss Signed-off-by: Robert Foss Reviewed-by: Benson Leung >>> >>> The mmc block queues are kernel threads which I would expect ignore signals, >>> so I am curious how you hit this? >> >> The issue was discovered on (tegra2?) hardware that is sensitive to >> being interrupted during tuning and having the controller left in a >> sensitive state. >> >> @Christopher Freeman: Maybe you can provide us with some additional details? >> > > It was found with Tegra 210. The signalling was an issue because tuning was > kicked off from an ioctl to the wifi device on the controller. > > FWIW, this issue was particular to the wifi driver (bcmdhd) and the android > tree. It in part depends on the way the wifi driver is able to reset the > sdio device via a routine that's not present in mainline: sdio_reset_comm. > I believe the wifi driver would power on the wifi chip and trigger tuning in > the aforementioned ioctl. Process that sent the ioctl was some network or > wifi > manager service on Android. > > Let me know if you would like any more details. Thanks for the explanation. > >>> >>> In any case: >>> >>> Acked-by: Adrian Hunter >>> >
Re: [PACTH v3] mmc: sdhci: Do not allow tuning procedure to be interrupted
On 18/08/16 01:12, Christopher Freeman wrote: > On 08-17 01:31 PM, Robert Foss wrote: >> >> >> On 2016-08-17 06:47 AM, Adrian Hunter wrote: >>> On 17/08/16 00:25, robert.f...@collabora.com wrote: From: Christopher Freeman wait_event_interruptible_timeout() will return early if the blocked process receives a signal, causing the driver to abort the tuning procedure and possibly leaving the controller in a bad state. Since the tuning command is expected to complete quickly (<50ms) and we've set a timeout, use wait_event_timeout() instead. Signed-off-by: Christopher Freeman Tested-by: Robert Foss Signed-off-by: Robert Foss Reviewed-by: Benson Leung >>> >>> The mmc block queues are kernel threads which I would expect ignore signals, >>> so I am curious how you hit this? >> >> The issue was discovered on (tegra2?) hardware that is sensitive to >> being interrupted during tuning and having the controller left in a >> sensitive state. >> >> @Christopher Freeman: Maybe you can provide us with some additional details? >> > > It was found with Tegra 210. The signalling was an issue because tuning was > kicked off from an ioctl to the wifi device on the controller. > > FWIW, this issue was particular to the wifi driver (bcmdhd) and the android > tree. It in part depends on the way the wifi driver is able to reset the > sdio device via a routine that's not present in mainline: sdio_reset_comm. > I believe the wifi driver would power on the wifi chip and trigger tuning in > the aforementioned ioctl. Process that sent the ioctl was some network or > wifi > manager service on Android. > > Let me know if you would like any more details. Thanks for the explanation. > >>> >>> In any case: >>> >>> Acked-by: Adrian Hunter >>> >
Re: [PACTH v3] mmc: sdhci: Do not allow tuning procedure to be interrupted
On 08-17 01:31 PM, Robert Foss wrote: > > > On 2016-08-17 06:47 AM, Adrian Hunter wrote: > >On 17/08/16 00:25, robert.f...@collabora.com wrote: > >>From: Christopher Freeman> >> > >>wait_event_interruptible_timeout() will return early if the blocked > >>process receives a signal, causing the driver to abort the tuning > >>procedure and possibly leaving the controller in a bad state. Since the > >>tuning command is expected to complete quickly (<50ms) and we've set a > >>timeout, use wait_event_timeout() instead. > >> > >>Signed-off-by: Christopher Freeman > >>Tested-by: Robert Foss > >>Signed-off-by: Robert Foss > >>Reviewed-by: Benson Leung > > > >The mmc block queues are kernel threads which I would expect ignore signals, > >so I am curious how you hit this? > > The issue was discovered on (tegra2?) hardware that is sensitive to > being interrupted during tuning and having the controller left in a > sensitive state. > > @Christopher Freeman: Maybe you can provide us with some additional details? > It was found with Tegra 210. The signalling was an issue because tuning was kicked off from an ioctl to the wifi device on the controller. FWIW, this issue was particular to the wifi driver (bcmdhd) and the android tree. It in part depends on the way the wifi driver is able to reset the sdio device via a routine that's not present in mainline: sdio_reset_comm. I believe the wifi driver would power on the wifi chip and trigger tuning in the aforementioned ioctl. Process that sent the ioctl was some network or wifi manager service on Android. Let me know if you would like any more details. > > > >In any case: > > > >Acked-by: Adrian Hunter > >
Re: [PACTH v3] mmc: sdhci: Do not allow tuning procedure to be interrupted
On 08-17 01:31 PM, Robert Foss wrote: > > > On 2016-08-17 06:47 AM, Adrian Hunter wrote: > >On 17/08/16 00:25, robert.f...@collabora.com wrote: > >>From: Christopher Freeman > >> > >>wait_event_interruptible_timeout() will return early if the blocked > >>process receives a signal, causing the driver to abort the tuning > >>procedure and possibly leaving the controller in a bad state. Since the > >>tuning command is expected to complete quickly (<50ms) and we've set a > >>timeout, use wait_event_timeout() instead. > >> > >>Signed-off-by: Christopher Freeman > >>Tested-by: Robert Foss > >>Signed-off-by: Robert Foss > >>Reviewed-by: Benson Leung > > > >The mmc block queues are kernel threads which I would expect ignore signals, > >so I am curious how you hit this? > > The issue was discovered on (tegra2?) hardware that is sensitive to > being interrupted during tuning and having the controller left in a > sensitive state. > > @Christopher Freeman: Maybe you can provide us with some additional details? > It was found with Tegra 210. The signalling was an issue because tuning was kicked off from an ioctl to the wifi device on the controller. FWIW, this issue was particular to the wifi driver (bcmdhd) and the android tree. It in part depends on the way the wifi driver is able to reset the sdio device via a routine that's not present in mainline: sdio_reset_comm. I believe the wifi driver would power on the wifi chip and trigger tuning in the aforementioned ioctl. Process that sent the ioctl was some network or wifi manager service on Android. Let me know if you would like any more details. > > > >In any case: > > > >Acked-by: Adrian Hunter > >
Re: [PACTH v3] mmc: sdhci: Do not allow tuning procedure to be interrupted
On 2016-08-17 06:47 AM, Adrian Hunter wrote: On 17/08/16 00:25, robert.f...@collabora.com wrote: From: Christopher Freemanwait_event_interruptible_timeout() will return early if the blocked process receives a signal, causing the driver to abort the tuning procedure and possibly leaving the controller in a bad state. Since the tuning command is expected to complete quickly (<50ms) and we've set a timeout, use wait_event_timeout() instead. Signed-off-by: Christopher Freeman Tested-by: Robert Foss Signed-off-by: Robert Foss Reviewed-by: Benson Leung The mmc block queues are kernel threads which I would expect ignore signals, so I am curious how you hit this? The issue was discovered on (tegra2?) hardware that is sensitive to being interrupted during tuning and having the controller left in a sensitive state. @Christopher Freeman: Maybe you can provide us with some additional details? In any case: Acked-by: Adrian Hunter
Re: [PACTH v3] mmc: sdhci: Do not allow tuning procedure to be interrupted
On 2016-08-17 06:47 AM, Adrian Hunter wrote: On 17/08/16 00:25, robert.f...@collabora.com wrote: From: Christopher Freeman wait_event_interruptible_timeout() will return early if the blocked process receives a signal, causing the driver to abort the tuning procedure and possibly leaving the controller in a bad state. Since the tuning command is expected to complete quickly (<50ms) and we've set a timeout, use wait_event_timeout() instead. Signed-off-by: Christopher Freeman Tested-by: Robert Foss Signed-off-by: Robert Foss Reviewed-by: Benson Leung The mmc block queues are kernel threads which I would expect ignore signals, so I am curious how you hit this? The issue was discovered on (tegra2?) hardware that is sensitive to being interrupted during tuning and having the controller left in a sensitive state. @Christopher Freeman: Maybe you can provide us with some additional details? In any case: Acked-by: Adrian Hunter
Re: [PACTH v3] mmc: sdhci: Do not allow tuning procedure to be interrupted
On 17/08/16 00:25, robert.f...@collabora.com wrote: > From: Christopher Freeman> > wait_event_interruptible_timeout() will return early if the blocked > process receives a signal, causing the driver to abort the tuning > procedure and possibly leaving the controller in a bad state. Since the > tuning command is expected to complete quickly (<50ms) and we've set a > timeout, use wait_event_timeout() instead. > > Signed-off-by: Christopher Freeman > Tested-by: Robert Foss > Signed-off-by: Robert Foss > Reviewed-by: Benson Leung The mmc block queues are kernel threads which I would expect ignore signals, so I am curious how you hit this? In any case: Acked-by: Adrian Hunter > --- > drivers/mmc/host/sdhci.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index 0e3d7c0..9e80203 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -1960,7 +1960,7 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, > u32 opcode) > > spin_unlock_irqrestore(>lock, flags); > /* Wait for Buffer Read Ready interrupt */ > - wait_event_interruptible_timeout(host->buf_ready_int, > + wait_event_timeout(host->buf_ready_int, > (host->tuning_done == 1), > msecs_to_jiffies(50)); > spin_lock_irqsave(>lock, flags); >
Re: [PACTH v3] mmc: sdhci: Do not allow tuning procedure to be interrupted
On 17/08/16 00:25, robert.f...@collabora.com wrote: > From: Christopher Freeman > > wait_event_interruptible_timeout() will return early if the blocked > process receives a signal, causing the driver to abort the tuning > procedure and possibly leaving the controller in a bad state. Since the > tuning command is expected to complete quickly (<50ms) and we've set a > timeout, use wait_event_timeout() instead. > > Signed-off-by: Christopher Freeman > Tested-by: Robert Foss > Signed-off-by: Robert Foss > Reviewed-by: Benson Leung The mmc block queues are kernel threads which I would expect ignore signals, so I am curious how you hit this? In any case: Acked-by: Adrian Hunter > --- > drivers/mmc/host/sdhci.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index 0e3d7c0..9e80203 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -1960,7 +1960,7 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, > u32 opcode) > > spin_unlock_irqrestore(>lock, flags); > /* Wait for Buffer Read Ready interrupt */ > - wait_event_interruptible_timeout(host->buf_ready_int, > + wait_event_timeout(host->buf_ready_int, > (host->tuning_done == 1), > msecs_to_jiffies(50)); > spin_lock_irqsave(>lock, flags); >
[PACTH v3] mmc: sdhci: Do not allow tuning procedure to be interrupted
From: Christopher Freemanwait_event_interruptible_timeout() will return early if the blocked process receives a signal, causing the driver to abort the tuning procedure and possibly leaving the controller in a bad state. Since the tuning command is expected to complete quickly (<50ms) and we've set a timeout, use wait_event_timeout() instead. Signed-off-by: Christopher Freeman Tested-by: Robert Foss Signed-off-by: Robert Foss Reviewed-by: Benson Leung --- drivers/mmc/host/sdhci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 0e3d7c0..9e80203 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -1960,7 +1960,7 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode) spin_unlock_irqrestore(>lock, flags); /* Wait for Buffer Read Ready interrupt */ - wait_event_interruptible_timeout(host->buf_ready_int, + wait_event_timeout(host->buf_ready_int, (host->tuning_done == 1), msecs_to_jiffies(50)); spin_lock_irqsave(>lock, flags); -- 2.7.4
[PACTH v3] mmc: sdhci: Do not allow tuning procedure to be interrupted
From: Christopher Freeman wait_event_interruptible_timeout() will return early if the blocked process receives a signal, causing the driver to abort the tuning procedure and possibly leaving the controller in a bad state. Since the tuning command is expected to complete quickly (<50ms) and we've set a timeout, use wait_event_timeout() instead. Signed-off-by: Christopher Freeman Tested-by: Robert Foss Signed-off-by: Robert Foss Reviewed-by: Benson Leung --- drivers/mmc/host/sdhci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 0e3d7c0..9e80203 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -1960,7 +1960,7 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode) spin_unlock_irqrestore(>lock, flags); /* Wait for Buffer Read Ready interrupt */ - wait_event_interruptible_timeout(host->buf_ready_int, + wait_event_timeout(host->buf_ready_int, (host->tuning_done == 1), msecs_to_jiffies(50)); spin_lock_irqsave(>lock, flags); -- 2.7.4