[PATCH 0/2] mmc: sdhci: Fix sdhci caps register bits with corrections provided by dt

[PATCH 0/2] mmc: sdhci: Fix sdhci caps register bits with corrections provided by dt

[Zach Brown]

For various reasons the sdhci caps register can be incorrect. This patch set
introduces a general way to correct the bits when they are read to accurately
reflect the capabilties of the controller/board combo.

The first patch creates sdhci-caps and sdhci-caps-mask dt properties that
combined represent the correction to the sdhci caps register.

The second patch uses the new dt properties to correct the caps from the
register as they read during __sdhci_read_caps.

Changes from RFC:
 * /s/registers/register
 * Moved sdhci dt properties into new documentation file sdhci.txt

Zach Brown (2):
  mmc: sdhci: dt: Add device tree properties sdhci-caps and
    sdhci-caps-mask
  mmc: sdhci: Use sdhci-caps-mask and sdhci-caps to change the caps read
        during __sdhci_read_caps

 Documentation/devicetree/bindings/mmc/sdhci.txt | 14 ++++++++++++++
 drivers/mmc/host/sdhci.c                        | 24 ++++++++++++++++++++++--
 2 files changed, 36 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mmc/sdhci.txt

-- 
2.7.4

[PATCH 1/2] mmc: sdhci: dt: Add device tree properties sdhci-caps and sdhci-caps-mask

[Zach Brown]

On some systems the sdhci capabilty register is incorrect for one
reason or another.

The sdhci-caps-mask property specifies which bits in the register
are incorrect and should be turned off before using sdhci-caps to turn
on bits.

The sdhci-caps property specifies which bits should be turned on.

Signed-off-by: Zach Brown <zach.brown@ni.com>
---
 Documentation/devicetree/bindings/mmc/sdhci.txt | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/sdhci.txt

diff --git a/Documentation/devicetree/bindings/mmc/sdhci.txt b/Documentation/devicetree/bindings/mmc/sdhci.txt
new file mode 100644
index 0000000..0f4f1bb
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/sdhci.txt
@@ -0,0 +1,14 @@
+The properties specific for SD host controllers. For properties shared by MMC
+host controllers refer to the mmc[1] bindings.
+
+  [1] Documentation/devicetree/bindings/mmc/mmc.txt
+
+Optional properties:
+- sdhci-caps-mask: The sdhci capabilities register is incorrect. This 64bit
+  property corresponds to the bits in the sdhci capabilty register. If the bit
+  is on in the mask then the bit is incorrect in the register and should be
+  turned off, before applying sdhci-caps.
+- sdhci-caps: The sdhci capabilities register is incorrect. This 64bit
+  property corresponds to the bits in the sdhci capability register. If the
+  bit is on in the property then the bit should be turned on.
+
-- 
2.7.4

[PATCH 2/2] mmc: sdhci: Use sdhci-caps-mask and sdhci-caps to change the caps read during __sdhci_read_caps

[Zach Brown]

The sdhci capabilities register can be incorrect. The sdhci-caps-mask
and sdhci-caps dt properties specify which bits of the register are
incorrect and what their values should be. This patch makes the sdhci
driver use those properties to correct the caps during
__sdhci_read_caps.

During __sdhci_read_caps
Use the sdhci-caps-mask property to turn off the incorrect bits of the
sdhci register after reading them.
Use the sdhci-caps to turn on bits after using sdhci-caps-mask to turn
off the incorrect ones.

Signed-off-by: Zach Brown <zach.brown@ni.com>
---
 drivers/mmc/host/sdhci.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 1e25b01..d5feae4 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -22,6 +22,7 @@
 #include <linux/scatterlist.h>
 #include <linux/regulator/consumer.h>
 #include <linux/pm_runtime.h>
+#include <linux/of.h>
 
 #include <linux/leds.h>
 
@@ -2991,6 +2992,8 @@ static int sdhci_set_dma_mask(struct sdhci_host *host)
 void __sdhci_read_caps(struct sdhci_host *host, u16 *ver, u32 *caps, u32 *caps1)
 {
 	u16 v;
+	u64 dt_caps_mask = 0;
+	u64 dt_caps = 0;
 
 	if (host->read_caps)
 		return;
@@ -3005,18 +3008,35 @@ void __sdhci_read_caps(struct sdhci_host *host, u16 *ver, u32 *caps, u32 *caps1)
 
 	sdhci_do_reset(host, SDHCI_RESET_ALL);
 
+	of_property_read_u64(mmc_dev(host->mmc)->of_node,
+			     "sdhci-caps-mask", &dt_caps_mask);
+	of_property_read_u64(mmc_dev(host->mmc)->of_node,
+			     "sdhci-caps", &dt_caps);
+
 	v = ver ? *ver : sdhci_readw(host, SDHCI_HOST_VERSION);
 	host->version = (v & SDHCI_SPEC_VER_MASK) >> SDHCI_SPEC_VER_SHIFT;
 
 	if (host->quirks & SDHCI_QUIRK_MISSING_CAPS)
 		return;
 
-	host->caps = caps ? *caps : sdhci_readl(host, SDHCI_CAPABILITIES);
+	if (caps)
+		host->caps = *caps;
+	else {
+		host->caps = sdhci_readl(host, SDHCI_CAPABILITIES);
+		host->caps &= ~lower_32_bits(dt_caps_mask);
+		host->caps |= lower_32_bits(dt_caps);
+	}
 
 	if (host->version < SDHCI_SPEC_300)
 		return;
 
-	host->caps1 = caps1 ? *caps1 : sdhci_readl(host, SDHCI_CAPABILITIES_1);
+	if (caps1)
+		host->caps1 = *caps1;
+	else {
+		host->caps1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
+		host->caps1 &= ~upper_32_bits(dt_caps_mask);
+		host->caps1 |= upper_32_bits(dt_caps);
+	}
 }
 EXPORT_SYMBOL_GPL(__sdhci_read_caps);
 
-- 
2.7.4

Re: [PATCH 1/2] mmc: sdhci: dt: Add device tree properties sdhci-caps and sdhci-caps-mask

[Rob Herring]

On Fri, Oct 28, 2016 at 10:56:10AM -0500, Zach Brown wrote:
> On some systems the sdhci capabilty register is incorrect for one
> reason or another.
> 
> The sdhci-caps-mask property specifies which bits in the register
> are incorrect and should be turned off before using sdhci-caps to turn
> on bits.
> 
> The sdhci-caps property specifies which bits should be turned on.
> 
> Signed-off-by: Zach Brown <zach.brown@ni.com>
> ---
>  Documentation/devicetree/bindings/mmc/sdhci.txt | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mmc/sdhci.txt

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH 2/2] mmc: sdhci: Use sdhci-caps-mask and sdhci-caps to change the caps read during __sdhci_read_caps

[Adrian Hunter]

On 28/10/16 18:56, Zach Brown wrote:
> The sdhci capabilities register can be incorrect. The sdhci-caps-mask
> and sdhci-caps dt properties specify which bits of the register are
> incorrect and what their values should be. This patch makes the sdhci
> driver use those properties to correct the caps during
> __sdhci_read_caps.
> 
> During __sdhci_read_caps
> Use the sdhci-caps-mask property to turn off the incorrect bits of the
> sdhci register after reading them.
> Use the sdhci-caps to turn on bits after using sdhci-caps-mask to turn
> off the incorrect ones.
> 
> Signed-off-by: Zach Brown <zach.brown@ni.com>

Apart from minor style issue:

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci.c | 24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 1e25b01..d5feae4 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -22,6 +22,7 @@
>  #include <linux/scatterlist.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/of.h>
>  
>  #include <linux/leds.h>
>  
> @@ -2991,6 +2992,8 @@ static int sdhci_set_dma_mask(struct sdhci_host *host)
>  void __sdhci_read_caps(struct sdhci_host *host, u16 *ver, u32 *caps, u32 *caps1)
>  {
>  	u16 v;
> +	u64 dt_caps_mask = 0;
> +	u64 dt_caps = 0;
>  
>  	if (host->read_caps)
>  		return;
> @@ -3005,18 +3008,35 @@ void __sdhci_read_caps(struct sdhci_host *host, u16 *ver, u32 *caps, u32 *caps1)
>  
>  	sdhci_do_reset(host, SDHCI_RESET_ALL);
>  
> +	of_property_read_u64(mmc_dev(host->mmc)->of_node,
> +			     "sdhci-caps-mask", &dt_caps_mask);
> +	of_property_read_u64(mmc_dev(host->mmc)->of_node,
> +			     "sdhci-caps", &dt_caps);
> +
>  	v = ver ? *ver : sdhci_readw(host, SDHCI_HOST_VERSION);
>  	host->version = (v & SDHCI_SPEC_VER_MASK) >> SDHCI_SPEC_VER_SHIFT;
>  
>  	if (host->quirks & SDHCI_QUIRK_MISSING_CAPS)
>  		return;
>  
> -	host->caps = caps ? *caps : sdhci_readl(host, SDHCI_CAPABILITIES);
> +	if (caps)
> +		host->caps = *caps;
> +	else {

All branches of 'if' should have {}.
i.e. refer results of ./scripts/checkpatch.pl --strict

> +		host->caps = sdhci_readl(host, SDHCI_CAPABILITIES);
> +		host->caps &= ~lower_32_bits(dt_caps_mask);
> +		host->caps |= lower_32_bits(dt_caps);
> +	}
>  
>  	if (host->version < SDHCI_SPEC_300)
>  		return;
>  
> -	host->caps1 = caps1 ? *caps1 : sdhci_readl(host, SDHCI_CAPABILITIES_1);
> +	if (caps1)
> +		host->caps1 = *caps1;
> +	else {

Ditto

> +		host->caps1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
> +		host->caps1 &= ~upper_32_bits(dt_caps_mask);
> +		host->caps1 |= upper_32_bits(dt_caps);
> +	}
>  }
>  EXPORT_SYMBOL_GPL(__sdhci_read_caps);
>  
>