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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <68720b68-430c-bc69-f531-cc9884ade331@intel.com>
Date:   Wed, 2 Nov 2016 10:39:33 +0200
From:   Adrian Hunter <adrian.hunter@...el.com>
To:     Zach Brown <zach.brown@...com>, ulf.hansson@...aro.org
Cc:     robh+dt@...nel.org, mark.rutland@....com,
        linux-mmc@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] mmc: sdhci: Use sdhci-caps-mask and sdhci-caps to
 change the caps read during __sdhci_read_caps

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@...com>

Apart from minor style issue:

Acked-by: Adrian Hunter <adrian.hunter@...el.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);
>  
>