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>
---
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
