Hi Thomas,
On 21/03/19 1:18 PM, Thomas Petazzoni wrote:
> 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 ?
yes.
>
>> + 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
Right. This is true for all OMAP platforms with micro SD slot.
>
> (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
Right. This is true for OMAP platforms with standard SD slot. They have
wp-gpios dt property populated.
>
> (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.
This is not supported in OMAP platforms.
>
> 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.
That is correct. For write protect, we always use GPIO.
Thanks
Kishon
