| Message ID | 20230405-pl180-busydetect-fix-v2-0-eeb10323b546@linaro.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | Fix busydetect on Ux500 PL180/MMCI | expand |
On 4/9/23 00:00, Linus Walleij wrote: > This series fixes a pretty serious problem in the MMCI > busy detect handling, discovered only after going up and > down a ladder of refactorings. > > The code is written expecting the Ux500 busy detect > to fire two interrupts: one at the start of the busy > signalling and one at the end of the busy signalling. > > The root cause of the problem seen on some devices > is that only the first IRQ arrives, and then the device > hangs, waiting perpetually for the next IRQ to arrive. > > This is eventually solved by adding a timeout using > a delayed work that fire after 10 ms if the busy detect > has not stopped at this point. (Other delay spans can > be suggested.) This is the last patch in the series. > > I included the rewrite of the entire busy detect logic > to use a state machine as this makes it way easier to > debug and will print messages about other error > conditions as well. > > The problem affects especially the Skomer > (Samsung GT-I9070) and Kyle (Samsung SGH-I407). > > It is fine to just apply this for the -next kernel, > despite it fixes the busy detect that has been broken > for these devices for a while, we can think about > backporting a simpler version of the timeout for > stable kernels if we want. > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > Changes in v2: > - Drop pointless patch nr 1 > - Unconditionally intialize some state variables > - Use a less fragile method to look for busy status when > using busy detect, should fix Yann's problem Hi Linus, Thanks for the update, it is now OK on STM32MP1. For the series, you can add my: Reviewed-by: Yann Gautier <yann.gautier@foss.st.com> Best regards, Yann > - Link to v1: https://lore.kernel.org/r/20230405-pl180-busydetect-fix-v1-0-28ac19a74e5e@linaro.org > > --- > Linus Walleij (12): > mmc: mmci: Clear busy_status when starting command > mmc: mmci: Unwind big if() clause > mmc: mmci: Stash status while waiting for busy > mmc: mmci: Break out error check in busy detect > mmc: mmci: Make busy complete state machine explicit > mmc: mmci: Retry the busy start condition > mmc: mmci: Use state machine state as exit condition > mmc: mmci: Use a switch statement machine > mmc: mmci: Break out a helper function > mmc: mmci: mmci_card_busy() from state machine > mmc: mmci: Drop end IRQ from Ux500 busydetect > mmc: mmci: Add busydetect timeout > > drivers/mmc/host/mmci.c | 166 ++++++++++++++++++++++++++++-------- > drivers/mmc/host/mmci.h | 17 ++++ > drivers/mmc/host/mmci_stm32_sdmmc.c | 6 +- > 3 files changed, 151 insertions(+), 38 deletions(-) > --- > base-commit: fe15c26ee26efa11741a7b632e9f23b01aca4cc6 > change-id: 20230405-pl180-busydetect-fix-66a0360d398a > > Best regards,
This series fixes a pretty serious problem in the MMCI busy detect handling, discovered only after going up and down a ladder of refactorings. The code is written expecting the Ux500 busy detect to fire two interrupts: one at the start of the busy signalling and one at the end of the busy signalling. The root cause of the problem seen on some devices is that only the first IRQ arrives, and then the device hangs, waiting perpetually for the next IRQ to arrive. This is eventually solved by adding a timeout using a delayed work that fire after 10 ms if the busy detect has not stopped at this point. (Other delay spans can be suggested.) This is the last patch in the series. I included the rewrite of the entire busy detect logic to use a state machine as this makes it way easier to debug and will print messages about other error conditions as well. The problem affects especially the Skomer (Samsung GT-I9070) and Kyle (Samsung SGH-I407). It is fine to just apply this for the -next kernel, despite it fixes the busy detect that has been broken for these devices for a while, we can think about backporting a simpler version of the timeout for stable kernels if we want. Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- Changes in v2: - Drop pointless patch nr 1 - Unconditionally intialize some state variables - Use a less fragile method to look for busy status when using busy detect, should fix Yann's problem - Link to v1: https://lore.kernel.org/r/20230405-pl180-busydetect-fix-v1-0-28ac19a74e5e@linaro.org --- Linus Walleij (12): mmc: mmci: Clear busy_status when starting command mmc: mmci: Unwind big if() clause mmc: mmci: Stash status while waiting for busy mmc: mmci: Break out error check in busy detect mmc: mmci: Make busy complete state machine explicit mmc: mmci: Retry the busy start condition mmc: mmci: Use state machine state as exit condition mmc: mmci: Use a switch statement machine mmc: mmci: Break out a helper function mmc: mmci: mmci_card_busy() from state machine mmc: mmci: Drop end IRQ from Ux500 busydetect mmc: mmci: Add busydetect timeout drivers/mmc/host/mmci.c | 166 ++++++++++++++++++++++++++++-------- drivers/mmc/host/mmci.h | 17 ++++ drivers/mmc/host/mmci_stm32_sdmmc.c | 6 +- 3 files changed, 151 insertions(+), 38 deletions(-) --- base-commit: fe15c26ee26efa11741a7b632e9f23b01aca4cc6 change-id: 20230405-pl180-busydetect-fix-66a0360d398a Best regards,