On Thu, 21 Mar 2019 at 07:39, Kishon Vijay Abraham I <kishon@xxxxxx> wrote:
>
> Hi Thomas,
>
> On 20/03/19 2:26 AM, Thomas Petazzoni wrote:
> > Hello Sebastian,
> >
> > On Tue, 19 Mar 2019 18:27:37 +0100
> > Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx> wrote:
> >
> >> Since commit 39ee32ce48675 ("mmc: sdhci-omap: drop ->get_ro()
> >> implementation") my
> >> |Model: TI AM5728 IDK
> >> |Board: AM572x IDK REV 1.3B
> >>
> >> does not boot from MMC anymore:
> >
> > Thanks for the feedback. It's really weird. Before my change, the
> > sdhci-omap driver was setting:
> >
> > host->mmc_host_ops.get_ro = mmc_gpio_get_ro;
> >
> > But thanks to another change I did, the core is now doing:
> >
> > else if (host->ops->get_ro)
> > is_readonly = host->ops->get_ro(host);
> > + else if (mmc_can_gpio_ro(host->mmc))
> > + is_readonly = mmc_gpio_get_ro(host->mmc);
> > else
> > is_readonly = !(sdhci_readl(host, SDHCI_PRESENT_STATE)
> > & SDHCI_WRITE_PROTECT);
> >
> > So basically, if there's no host->ops->get_ro, it calls
> > mmc_gpio_get_ro(), which should do the exact same thing.
> >
> > The only difference that I can see is that it only calls
> > mmc_gpio_get_ro() if mmc_can_gpio_ro() returns true. But looking at the
> > code of mmc_can_gpio_ro(), I don't see how it can be borked, and
> > mmc_gpio_get_ro() be working.
> > So there is something odd going on here. Do you have the time to add
> > some traces in sdhci_check_ro(), to see what is the return value of
> > mmc_can_gpio_ro(), whether we call mmc_gpio_get_ro(), and what does it
> > return ?
> >
> > This would help debugging his issue. Thanks a lot!
>
> I debugged this issue and realized not all the platforms using sdhci-omap will
> have wp-gpios populated (with micro SD slot) making it use incorrect value from
> SDHCI_PRESENT_STATE register. The following patch fixes it. I can post it as a
> separate patch if required.
>
> Thanks
> Kishon
>
> 8<-------------------------------------------
>
> From 37c69ac189a8ec7f0ca91f7cbcdbc9456ce22599 Mon Sep 17 00:00:00 2001
> From: Kishon Vijay Abraham I <kishon@xxxxxx>
> Date: Thu, 21 Mar 2019 11:45:44 +0530
> Subject: [PATCH] mmc: sdhci-omap: Set caps2 to indicate no physical write
> protect pin
>
> After commit 6d5cd068ee59fba ("mmc: sdhci: use WP GPIO in
> sdhci_check_ro()") and commit 39ee32ce486756f ("mmc: sdhci-omap: drop
> ->get_ro() implementation"), sdhci-omap relied on SDHCI_PRESENT_STATE
> to check if the card is read-only, if wp-gpios is not populated
> in device tree. However SDHCI_PRESENT_STATE in sdhci-omap does not have
> correct read-only state.
>
> sdhci-omap can be used by platforms with both micro SD slot and standard
> SD slot with physical write protect pin (using GPIO). Set caps2 to
> MMC_CAP2_NO_WRITE_PROTECT based on if wp-gpios property is populated or
> not.
>
> This fix is required since existing device-tree node doesn't have
> "disable-wp" property and to preserve old-dt compatibility.
>
> Fixes: commit 6d5cd068ee59fba ("mmc: sdhci: use WP GPIO in
> sdhci_check_ro()")
> Fixes: commit 39ee32ce486756f ("mmc: sdhci-omap: drop ->get_ro()
> implementation")
> Signed-off-by: Kishon Vijay Abraham I <kishon@xxxxxx>
Applied for fixes, thanks!
Kind regards
Uffe
> ---
> drivers/mmc/host/sdhci-omap.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
> index b1a66ca3821a..5bbed477c9b1 100644
> --- a/drivers/mmc/host/sdhci-omap.c
> +++ b/drivers/mmc/host/sdhci-omap.c
> @@ -1056,6 +1056,9 @@ static int sdhci_omap_probe(struct platform_device *pdev)
> mmc->f_max = 48000000;
> }
>
> + if (!mmc_can_gpio_ro(mmc))
> + mmc->caps2 |= MMC_CAP2_NO_WRITE_PROTECT;
> +
> pltfm_host->clk = devm_clk_get(dev, "fck");
> if (IS_ERR(pltfm_host->clk)) {
> ret = PTR_ERR(pltfm_host->clk);
> --
> 2.17.1
