Hello Kishon,
First of all, thanks for debugging this.
On Thu, 21 Mar 2019 12:08:33 +0530
Kishon Vijay Abraham I <kishon@xxxxxx> wrote:
> 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.
I'm still a bit confused as to how it could work before my change.
Before by change, the .get_ro callback in sdhci-omap was set to
mmc_gpio_get_ro(). When there is no wp-gpio, this function returns
-ENOSYS. So when mmc_sd_setup_card() calls mmc_sd_get_ro(), it should
then print a warning:
int ro = mmc_sd_get_ro(host);
if (ro < 0) {
pr_warn("%s: host does not support reading read-only switch, assuming write-enable\n",
mmc_hostname(host));
} else if (ro > 0) {
mmc_card_set_readonly(card);
}
Do you see this warning message before my change ?
> + if (!mmc_can_gpio_ro(mmc))
> + mmc->caps2 |= MMC_CAP2_NO_WRITE_PROTECT;
So if you do this, it means that the SDHCI WP signal is never used on
OMAP platforms ? Indeed, if I understand correctly, there are three
possibilities:
(1) No WP signal at all
(2) WP signal routed to a GPIO. In this case, the kernel needs to read
the state of the GPIO to know the WP status of the MMC device
(3) WP signal routed to a dedicated signal of the SDHCI controller. In
this case, the kernel needs to read the SDHCI_PRESENT_STATE
register of the SDHCI controller.
With your change, (3) is no longer possible, because as soon as there's
no GPIO, you say there's no write protect information available at all.
Could you clarify this ?
Thanks!
Thomas
--
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
