[PATCH 1/2] mmc: sdhci: Add quirk to disable write-protect detection

[PATCH 1/2] mmc: sdhci: Add quirk to disable write-protect detection

[Lars-Peter Clausen]

It is not uncommon to see systems where there is no physical write-protect
signal (e.g. when using eMMC or microSD cards). Some SDHCI controllers may
have bogus data in the SDHCI_WRITE_PROTECT of the SDHCI_PRESENT_STATE
register on such systems. Add a new quirk which disables reading of the
write-protect status when set.

Signed-off-by: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
---
 drivers/mmc/host/sdhci.c | 3 +++
 drivers/mmc/host/sdhci.h | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c80287a..7448757 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1719,6 +1719,9 @@ static int sdhci_get_ro(struct mmc_host *mmc)
 	struct sdhci_host *host = mmc_priv(mmc);
 	int ret;
 
+	if (host->quirks2 & SDHCI_QUIRK2_DISABLE_WRITE_PROTECT)
+		return 0;
+
 	sdhci_runtime_pm_get(host);
 	ret = sdhci_do_get_ro(host);
 	sdhci_runtime_pm_put(host);
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index e639b7f..62615e1 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -409,6 +409,8 @@ struct sdhci_host {
 #define SDHCI_QUIRK2_SUPPORT_SINGLE			(1<<13)
 /* Controller broken with using ACMD23 */
 #define SDHCI_QUIRK2_ACMD23_BROKEN			(1<<14)
+/* Write protect signal is not wired, data in SDHCI_PRESENT_STATE is bogus */
+#define SDHCI_QUIRK2_DISABLE_WRITE_PROTECT		(1<<15)
 
 	int irq;		/* Device IRQ */
 	void __iomem *ioaddr;	/* Mapped address */
-- 
1.8.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] mmc: sdhci: DT: Allow to specify that no write protect signal is present

[Lars-Peter Clausen]

Allow to specify in the device-tree that no physical write-protect signal
is connected to a particular instance of a SDHCI controller. This allows a
driver to ignore potential bogus values of the SDHCI_WRITE_PROTECT bit in
the SDHCI_PRESENT_STATE register if it is aware that no write-protect
signal is connected.

The name for the new property is 'disable-wp' and was chosen based on the
property with the same function from the Synopsys designware mobile storage
host controller DT bindings specification.

Signed-off-by: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Cc: Ian Campbell <ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>
Cc: Kumar Gala <galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
---
 Documentation/devicetree/bindings/mmc/mmc.txt | 1 +
 drivers/mmc/host/sdhci-pltfm.c                | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
index 438899e..b068549 100644
--- a/Documentation/devicetree/bindings/mmc/mmc.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc.txt
@@ -21,6 +21,7 @@ Optional properties:
   below for the case, when a GPIO is used for the CD line
 - wp-inverted: when present, polarity on the WP line is inverted. See the note
   below for the case, when a GPIO is used for the WP line
+- disable-wp: When set no physical WP line is present
 - max-frequency: maximum operating clock frequency
 - no-1-8-v: when present, denotes that 1.8v card voltage is not supported on
   this system, even if the controller claims it is.
diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index a207f5a..aa4d343 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -86,6 +86,9 @@ void sdhci_get_of_property(struct platform_device *pdev)
 	if (sdhci_of_wp_inverted(np))
 		host->quirks |= SDHCI_QUIRK_INVERTED_WRITE_PROTECT;
 
+	if (of_property_read_bool(np, "disable-wp"))
+		host->quirks2 |= SDHCI_QUIRK2_DISABLE_WRITE_PROTECT;
+
 	if (of_get_property(np, "broken-cd", NULL))
 		host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
 
-- 
1.8.0

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] mmc: sdhci: DT: Allow to specify that no write protect signal is present

[Ulf Hansson]

On 28 April 2015 at 11:40, Lars-Peter Clausen <lars@metafoo.de> wrote:
> Allow to specify in the device-tree that no physical write-protect signal
> is connected to a particular instance of a SDHCI controller. This allows a
> driver to ignore potential bogus values of the SDHCI_WRITE_PROTECT bit in
> the SDHCI_PRESENT_STATE register if it is aware that no write-protect
> signal is connected.
>
> The name for the new property is 'disable-wp' and was chosen based on the
> property with the same function from the Synopsys designware mobile storage
> host controller DT bindings specification.

Re-using the "disable-wp" name is okay. Though, I would like this to
be treated as a common MMC DT binding and not something specific for
sdhci/dw_mmc.

Therefore, I suggest to add a corresponding MMC_CAP* bit, which should
be assigned from mmc_of_parse().

>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: Kumar Gala <galak@codeaurora.org>
> ---
>  Documentation/devicetree/bindings/mmc/mmc.txt | 1 +
>  drivers/mmc/host/sdhci-pltfm.c                | 3 +++
>  2 files changed, 4 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
> index 438899e..b068549 100644
> --- a/Documentation/devicetree/bindings/mmc/mmc.txt
> +++ b/Documentation/devicetree/bindings/mmc/mmc.txt
> @@ -21,6 +21,7 @@ Optional properties:
>    below for the case, when a GPIO is used for the CD line
>  - wp-inverted: when present, polarity on the WP line is inverted. See the note
>    below for the case, when a GPIO is used for the WP line
> +- disable-wp: When set no physical WP line is present

This requires some additional explanation, I believe. I assume this
property is only relevant for those controllers that have internal
logic to handle WP!?

For those that uses GPIO, this can't ever be a correct configuration. Right?

>  - max-frequency: maximum operating clock frequency
>  - no-1-8-v: when present, denotes that 1.8v card voltage is not supported on
>    this system, even if the controller claims it is.
> diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
> index a207f5a..aa4d343 100644
> --- a/drivers/mmc/host/sdhci-pltfm.c
> +++ b/drivers/mmc/host/sdhci-pltfm.c
> @@ -86,6 +86,9 @@ void sdhci_get_of_property(struct platform_device *pdev)
>         if (sdhci_of_wp_inverted(np))
>                 host->quirks |= SDHCI_QUIRK_INVERTED_WRITE_PROTECT;
>
> +       if (of_property_read_bool(np, "disable-wp"))
> +               host->quirks2 |= SDHCI_QUIRK2_DISABLE_WRITE_PROTECT;
> +
>         if (of_get_property(np, "broken-cd", NULL))
>                 host->quirks |= SDHCI_QUIRK_BROKEN_CARD_DETECTION;
>
> --
> 1.8.0
>

Kind regards
Uffe

Re: [PATCH 2/2] mmc: sdhci: DT: Allow to specify that no write protect signal is present

[Lars-Peter Clausen]

On 05/05/2015 10:36 AM, Ulf Hansson wrote:
[...]
>> diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
>> index 438899e..b068549 100644
>> --- a/Documentation/devicetree/bindings/mmc/mmc.txt
>> +++ b/Documentation/devicetree/bindings/mmc/mmc.txt
>> @@ -21,6 +21,7 @@ Optional properties:
>>     below for the case, when a GPIO is used for the CD line
>>   - wp-inverted: when present, polarity on the WP line is inverted. See the note
>>     below for the case, when a GPIO is used for the WP line
>> +- disable-wp: When set no physical WP line is present
>
> This requires some additional explanation, I believe. I assume this
> property is only relevant for those controllers that have internal
> logic to handle WP!?

Yes, currently drivers for controllers that have a built-in WP detection 
logic typically assume that if no WP GPIO is specified the built-in WP logic 
should be used. But for some of them the value returned by the internal 
detection logic will be bogus if there is no WP signal connected. So we need 
a way to specify that that there is no WP signal and the internal detection 
logic shouldn't be used either.

>
> For those that uses GPIO, this can't ever be a correct configuration. Right?

It wouldn't make much sense to use it with a GPIO based detection logic, 
since you simply don't specify the wp-gpio property if it is not present. 
One the other hand it wouldn't cause any harm to specify it in addition to 
not specifying the wp-gpios property.

How about:

- disable-wp: When set no physical WP line is present. This property should 
only be specified when the controller has a dedicated write-protect 
detection logic. If a GPIO is used for write-protect detection logic is used 
it is sufficient to not specify wp-gpios property in the absence of a WP line.

[...]

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] mmc: sdhci: DT: Allow to specify that no write protect signal is present

[Ulf Hansson]

On 5 May 2015 at 11:04, Lars-Peter Clausen <lars@metafoo.de> wrote:
> On 05/05/2015 10:36 AM, Ulf Hansson wrote:
> [...]
>>>
>>> diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt
>>> b/Documentation/devicetree/bindings/mmc/mmc.txt
>>> index 438899e..b068549 100644
>>> --- a/Documentation/devicetree/bindings/mmc/mmc.txt
>>> +++ b/Documentation/devicetree/bindings/mmc/mmc.txt
>>> @@ -21,6 +21,7 @@ Optional properties:
>>>     below for the case, when a GPIO is used for the CD line
>>>   - wp-inverted: when present, polarity on the WP line is inverted. See
>>> the note
>>>     below for the case, when a GPIO is used for the WP line
>>> +- disable-wp: When set no physical WP line is present
>>
>>
>> This requires some additional explanation, I believe. I assume this
>> property is only relevant for those controllers that have internal
>> logic to handle WP!?
>
>
> Yes, currently drivers for controllers that have a built-in WP detection
> logic typically assume that if no WP GPIO is specified the built-in WP logic
> should be used. But for some of them the value returned by the internal
> detection logic will be bogus if there is no WP signal connected. So we need
> a way to specify that that there is no WP signal and the internal detection
> logic shouldn't be used either.
>
>>
>> For those that uses GPIO, this can't ever be a correct configuration.
>> Right?
>
>
> It wouldn't make much sense to use it with a GPIO based detection logic,
> since you simply don't specify the wp-gpio property if it is not present.
> One the other hand it wouldn't cause any harm to specify it in addition to
> not specifying the wp-gpios property.
>
> How about:
>
> - disable-wp: When set no physical WP line is present. This property should
> only be specified when the controller has a dedicated write-protect
> detection logic. If a GPIO is used for write-protect detection logic is used

Remove the last "is used".

> it is sufficient to not specify wp-gpios property in the absence of a WP
> line.

Looks good!

Kind regards
Uffe