| Message ID | 20210107221509.6597-1-alcooperx@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mmc: sdhci-brcmstb: Fix mmc timeout errors on S5 suspend | expand |
On 1/7/21 2:15 PM, Al Cooper wrote: > Commit e7b5d63a82fe ("mmc: sdhci-brcmstb: Add shutdown callback") > that added a shutdown callback to the diver, is causing "mmc timeout" > errors on S5 suspend. The problem was that the "remove" was queuing > additional MMC commands after the "shutdown" and these caused > timeouts as the MMC queues were cleaned up for "remove". The > shutdown callback will be changed to calling sdhci-pltfm_suspend > which should get better power savings because the clocks will be > shutdown. > > Fixes: e7b5d63a82fe ("mmc: sdhci-brcmstb: Add shutdown callback") > Signed-off-by: Al Cooper <alcooperx@gmail.com> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
On 8/01/21 12:15 am, Al Cooper wrote: > Commit e7b5d63a82fe ("mmc: sdhci-brcmstb: Add shutdown callback") > that added a shutdown callback to the diver, is causing "mmc timeout" > errors on S5 suspend. The problem was that the "remove" was queuing > additional MMC commands after the "shutdown" and these caused > timeouts as the MMC queues were cleaned up for "remove". The > shutdown callback will be changed to calling sdhci-pltfm_suspend > which should get better power savings because the clocks will be > shutdown. > > Fixes: e7b5d63a82fe ("mmc: sdhci-brcmstb: Add shutdown callback") > Signed-off-by: Al Cooper <alcooperx@gmail.com> Acked-by: Adrian Hunter <adrian.hunter@intel.com> > --- > drivers/mmc/host/sdhci-brcmstb.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/drivers/mmc/host/sdhci-brcmstb.c b/drivers/mmc/host/sdhci-brcmstb.c > index bbf3496f4495..f9780c65ebe9 100644 > --- a/drivers/mmc/host/sdhci-brcmstb.c > +++ b/drivers/mmc/host/sdhci-brcmstb.c > @@ -314,11 +314,7 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev) > > static void sdhci_brcmstb_shutdown(struct platform_device *pdev) > { > - int ret; > - > - ret = sdhci_pltfm_unregister(pdev); > - if (ret) > - dev_err(&pdev->dev, "failed to shutdown\n"); > + sdhci_pltfm_suspend(&pdev->dev); > } > > MODULE_DEVICE_TABLE(of, sdhci_brcm_of_match); >
On Thu, 7 Jan 2021 at 23:15, Al Cooper <alcooperx@gmail.com> wrote: > > Commit e7b5d63a82fe ("mmc: sdhci-brcmstb: Add shutdown callback") > that added a shutdown callback to the diver, is causing "mmc timeout" > errors on S5 suspend. The problem was that the "remove" was queuing > additional MMC commands after the "shutdown" and these caused > timeouts as the MMC queues were cleaned up for "remove". The > shutdown callback will be changed to calling sdhci-pltfm_suspend > which should get better power savings because the clocks will be > shutdown. > > Fixes: e7b5d63a82fe ("mmc: sdhci-brcmstb: Add shutdown callback") > Signed-off-by: Al Cooper <alcooperx@gmail.com> Applied for fixes and by adding a stable tag, thanks! Kind regards Uffe > --- > drivers/mmc/host/sdhci-brcmstb.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/drivers/mmc/host/sdhci-brcmstb.c b/drivers/mmc/host/sdhci-brcmstb.c > index bbf3496f4495..f9780c65ebe9 100644 > --- a/drivers/mmc/host/sdhci-brcmstb.c > +++ b/drivers/mmc/host/sdhci-brcmstb.c > @@ -314,11 +314,7 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev) > > static void sdhci_brcmstb_shutdown(struct platform_device *pdev) > { > - int ret; > - > - ret = sdhci_pltfm_unregister(pdev); > - if (ret) > - dev_err(&pdev->dev, "failed to shutdown\n"); > + sdhci_pltfm_suspend(&pdev->dev); > } > > MODULE_DEVICE_TABLE(of, sdhci_brcm_of_match); > -- > 2.17.1 >
On 07/01/2021 23:15, Al Cooper wrote: > Commit e7b5d63a82fe ("mmc: sdhci-brcmstb: Add shutdown callback") > that added a shutdown callback to the diver, is causing "mmc timeout" > errors on S5 suspend. The problem was that the "remove" was queuing > additional MMC commands after the "shutdown" and these caused > timeouts as the MMC queues were cleaned up for "remove". The > shutdown callback will be changed to calling sdhci-pltfm_suspend > which should get better power savings because the clocks will be > shutdown. > > Fixes: e7b5d63a82fe ("mmc: sdhci-brcmstb: Add shutdown callback") > Signed-off-by: Al Cooper <alcooperx@gmail.com> > --- > drivers/mmc/host/sdhci-brcmstb.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/drivers/mmc/host/sdhci-brcmstb.c b/drivers/mmc/host/sdhci-brcmstb.c > index bbf3496f4495..f9780c65ebe9 100644 > --- a/drivers/mmc/host/sdhci-brcmstb.c > +++ b/drivers/mmc/host/sdhci-brcmstb.c > @@ -314,11 +314,7 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev) > > static void sdhci_brcmstb_shutdown(struct platform_device *pdev) > { > - int ret; > - > - ret = sdhci_pltfm_unregister(pdev); > - if (ret) > - dev_err(&pdev->dev, "failed to shutdown\n"); > + sdhci_pltfm_suspend(&pdev->dev); > } > > MODULE_DEVICE_TABLE(of, sdhci_brcm_of_match); > Good morning, Unfortunately this patch will cause link failures when CONFIG_PM_SLEEP is not set in the kernel config, as in sdhci-pltfm.c, the implementation of sdhci_pltfm_suspend() is in between #ifdef CONFIG_PM_SLEEP: /opt/toolchains/aarch64-musl-1.1.21-gcc-8.3.0-binutils-2.32-gdb-7.12.1-2/bin/aarch64-linux-musl-ld: drivers/mmc/host/sdhci-brcmstb.o: in function `sdhci_brcmstb_shutdown': sdhci-brcmstb.c:(.text+0x16c): undefined reference to `sdhci_pltfm_suspend' sdhci-brcmstb.c:(.text+0x16c): relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol `sdhci_pltfm_suspend' I'm not sure if definiting sdhci_pltfm_suspend() empty stubs in sdhci-pltfm.h when CONFIG_PM_SLEEP is not set would be prefered over adding #ifdef CONFIG_PM_SLEEP in sdhci-brcmstb.c, but I can send a patch for either solution. Regards,
diff --git a/drivers/mmc/host/sdhci-brcmstb.c b/drivers/mmc/host/sdhci-brcmstb.c index bbf3496f4495..f9780c65ebe9 100644 --- a/drivers/mmc/host/sdhci-brcmstb.c +++ b/drivers/mmc/host/sdhci-brcmstb.c @@ -314,11 +314,7 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev) static void sdhci_brcmstb_shutdown(struct platform_device *pdev) { - int ret; - - ret = sdhci_pltfm_unregister(pdev); - if (ret) - dev_err(&pdev->dev, "failed to shutdown\n"); + sdhci_pltfm_suspend(&pdev->dev); } MODULE_DEVICE_TABLE(of, sdhci_brcm_of_match);
Commit e7b5d63a82fe ("mmc: sdhci-brcmstb: Add shutdown callback") that added a shutdown callback to the diver, is causing "mmc timeout" errors on S5 suspend. The problem was that the "remove" was queuing additional MMC commands after the "shutdown" and these caused timeouts as the MMC queues were cleaned up for "remove". The shutdown callback will be changed to calling sdhci-pltfm_suspend which should get better power savings because the clocks will be shutdown. Fixes: e7b5d63a82fe ("mmc: sdhci-brcmstb: Add shutdown callback") Signed-off-by: Al Cooper <alcooperx@gmail.com> --- drivers/mmc/host/sdhci-brcmstb.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)