Re: [PATCH 1/5] mmc: sdhci-brcmstb: "mmc1: Internal clock never stabilised." seen on 72113

[Adrian Hunter <adrian.hunter@intel.com>]

On 21/04/22 21:27, Kamal Dasu wrote:
> From: Al Cooper <alcooperx@gmail.com>
> 
> The problem is in the .shutdown callback that was added to the
> sdhci-iproc and sdhci-brcmstb drivers to save power in S5. The
> shutdown callback will just call the sdhci_pltfm_suspend() function
> to suspend the lower level driver and then stop the sdhci system
> clock. The problem is that in some cases there can be a worker
> thread in the "system_freezable_wq" work queue that is scanning
> for a device every second. In normal system suspend, this queue
> is suspended before the driver suspend is called. In shutdown the
> queue is not suspended and the thread my run after we stop the
> sdhci clock in the shutdown callback which will cause the "clock
> never stabilised" error. The solution will be to have the shutdown
> callback cancel the worker thread before calling suspend (and
> stopping the sdhci clock).
> 
> NOTE: This is only happening on systems with the Legacy RPi SDIO
> core because that's the only controller that doesn't have the
> presence signal and needs to use a worker thread to do a 1 second
> poll loop.
> 
> Fixes: 5b191dcba719 ("mmc: sdhci-brcmstb: Fix mmc timeout errors on S5 suspend")
> Signed-off-by: Al Cooper <alcooperx@gmail.com>
> Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
> ---
>  drivers/mmc/host/sdhci-brcmstb.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-brcmstb.c b/drivers/mmc/host/sdhci-brcmstb.c
> index f24623aac2db..11037cd14cfa 100644
> --- a/drivers/mmc/host/sdhci-brcmstb.c
> +++ b/drivers/mmc/host/sdhci-brcmstb.c
> @@ -313,6 +313,10 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
>  
>  static void sdhci_brcmstb_shutdown(struct platform_device *pdev)
>  {
> +	struct sdhci_host *host = platform_get_drvdata(pdev);
> +
> +	/* Cancel possible rescan worker thread */
> +	cancel_delayed_work_sync(&host->mmc->detect);
>  	sdhci_pltfm_suspend(&pdev->dev);
>  }
>  

I think we fixed that already with the commit below:


commit 66c915d09b942fb3b2b0cb2f56562180901fba17
Author: Ulf Hansson <ulf.hansson@linaro.org>
Date:   Fri Dec 3 15:15:54 2021 +0100

    mmc: core: Disable card detect during shutdown
    
    It's seems prone to problems by allowing card detect and its corresponding
    mmc_rescan() work to run, during platform shutdown. For example, we may end
    up turning off the power while initializing a card, which potentially could
    damage it.
    
    To avoid this scenario, let's add ->shutdown_pre() callback for the mmc host
    class device and then turn of the card detect from there.
    
    Reported-by: Al Cooper <alcooperx@gmail.com>
    Suggested-by: Adrian Hunter <adrian.hunter@intel.com>
    Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
    Cc: stable@vger.kernel.org
    Link: https://lore.kernel.org/r/20211203141555.105351-1-ulf.hansson@linaro.org