| Message ID | 1430214020-16816-2-git-send-email-lars@metafoo.de |
|---|---|
| State | Needs Review / ACK, archived |
| Headers | show |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 1 errors, 0 warnings, 0 lines checked |
| robh/patch-applied | success |
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 devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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;
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(+)