diff mbox

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

Message ID 1430214020-16816-2-git-send-email-lars@metafoo.de (mailing list archive)
State New, archived
Headers show

Commit Message

Lars-Peter Clausen April 28, 2015, 9:40 a.m. UTC
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@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(+)

Comments

Ulf Hansson May 5, 2015, 8:36 a.m. UTC | #1
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
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lars-Peter Clausen May 5, 2015, 9:04 a.m. UTC | #2
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 linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ulf Hansson May 5, 2015, 10:15 a.m. UTC | #3
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
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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;