On 31 December 2014 at 11:54, Thomas Petazzoni
<thomas.petazzoni@xxxxxxxxxxxxxxxxxx> wrote:
> In commit 5491ce3f79ee ("mmc: sdhci-pxav3: add support for the Armada
> 38x SDHCI controller"), the sdhci-pxav3 driver was extended to include
> support for the SDHCI controller found in the Armada 38x
> processor. This mainly involved adding some MBus window related
> configuration.
>
> However, this configuration is currently done too early in ->probe():
> it is done before clocks are enabled, while this configuration
> involves touching the registers of the controller, which will hang the
> SoC if the clock is disabled. It wasn't noticed until now because the
> bootloader typically leaves gatable clocks enabled, but in situations
> where we have a deferred probe (due to a CD GPIO that cannot be taken,
> for example), then the probe will be re-tried later, after a clock
> disable has been done in the exit path of the failed probe attempt of
> the device. This second probe() will hang the system due to the clock
> being disabled.
>
> This can for example be produced on Armada 385 GP, which has a CD GPIO
> connected to an I2C PCA9555. If the driver for the PCA9555 is not
> compiled into the kernel, then we will have the following sequence of
> events:
>
> 1. The SDHCI probes
> 2. It does the MBus configuration (which works, because the clock is
> left enabled by the bootloader)
> 3. It enables the clock
> 4. It tries to get the CD GPIO, which fails due to the driver being
> missing, so -EPROBE_DEFER is returned.
> 5. Before returning -EPROBE_DEFER, the driver cleans up what was
> done, which includes disabling the clock.
> 6. Later on, the SDHCI probe is tried again.
> 7. It does the MBus configuration, which hangs because the clock is
> no longer enabled.
>
> This commit does the obvious fix of doing the MBus configuration after
> the clock has been enabled by the driver.
>
> Fixes: 5491ce3f79ee ("mmc: sdhci-pxav3: add support for the Armada 38x SDHCI controller")
> Cc: <stable@xxxxxxxxxxxxxxx> # v3.15+
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@xxxxxxxxxxxxxxxxxx>
Thanks! Applied for fixes.
Kind regards
Uffe
> ---
> drivers/mmc/host/sdhci-pxav3.c | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-pxav3.c b/drivers/mmc/host/sdhci-pxav3.c
> index 4523887..ca3424e 100644
> --- a/drivers/mmc/host/sdhci-pxav3.c
> +++ b/drivers/mmc/host/sdhci-pxav3.c
> @@ -300,13 +300,6 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
> if (IS_ERR(host))
> return PTR_ERR(host);
>
> - if (of_device_is_compatible(np, "marvell,armada-380-sdhci")) {
> - ret = mv_conf_mbus_windows(pdev, mv_mbus_dram_info());
> - if (ret < 0)
> - goto err_mbus_win;
> - }
> -
> -
> pltfm_host = sdhci_priv(host);
> pltfm_host->priv = pxa;
>
> @@ -325,6 +318,12 @@ static int sdhci_pxav3_probe(struct platform_device *pdev)
> if (!IS_ERR(pxa->clk_core))
> clk_prepare_enable(pxa->clk_core);
>
> + if (of_device_is_compatible(np, "marvell,armada-380-sdhci")) {
> + ret = mv_conf_mbus_windows(pdev, mv_mbus_dram_info());
> + if (ret < 0)
> + goto err_mbus_win;
> + }
> +
> /* enable 1/8V DDR capable */
> host->mmc->caps |= MMC_CAP_1_8V_DDR;
>
> @@ -396,11 +395,11 @@ err_add_host:
> pm_runtime_disable(&pdev->dev);
> err_of_parse:
> err_cd_req:
> +err_mbus_win:
> clk_disable_unprepare(pxa->clk_io);
> if (!IS_ERR(pxa->clk_core))
> clk_disable_unprepare(pxa->clk_core);
> err_clk_get:
> -err_mbus_win:
> sdhci_pltfm_free(pdev);
> return ret;
> }
> --
> 2.1.0
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
