Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1503349 > unrolled thread

[RFC 0/2] Ignore capability registers when it comes to speeds and use DT binding instead.

Started byZach Brown <zach.brown@ni.com>
First post2016-10-18 23:10 +0200
Last post2016-10-19 16:40 +0200
Articles 5 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [RFC 0/2] Ignore capability registers when it comes to speeds and use DT binding instead. Zach Brown <zach.brown@ni.com> - 2016-10-18 23:10 +0200
    [RFC 1/2] mmc: sdhci: dt: Add device tree property sdhci-cap-speed-modes-broken Zach Brown <zach.brown@ni.com> - 2016-10-18 23:10 +0200
      Re: [RFC 1/2] mmc: sdhci: dt: Add device tree property  sdhci-cap-speed-modes-broken Rob Herring <robh@kernel.org> - 2016-10-26 23:10 +0200
    [RFC 2/2] mmc: sdhci: Ignore capability register when it comes to speeds and use DT binding instead when sdhci-cap-speed-modes-broken is set. Zach Brown <zach.brown@ni.com> - 2016-10-18 23:10 +0200
      Re: [RFC 2/2] mmc: sdhci: Ignore capability register when it comes to  speeds and use DT binding instead when sdhci-cap-speed-modes-broken is set. Ulf Hansson <ulf.hansson@linaro.org> - 2016-10-19 16:40 +0200

#1503349 — [RFC 0/2] Ignore capability registers when it comes to speeds and use DT binding instead.

FromZach Brown <zach.brown@ni.com>
Date2016-10-18 23:10 +0200
Subject[RFC 0/2] Ignore capability registers when it comes to speeds and use DT binding instead.
Message-ID<stJO2-7sS-5@gated-at.bofh.it>
The first patch add documentation about a new devicetree property
sdhci-cap-speed-modes-broken.

The second patch makes the sdhci use the DT binding instead of the caps
register for determining which speed modes are supported by the controller.

This RFC is an alternative to another patch set I sent up.
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1251944.html

Zach Brown (2):
  mmc: sdhci: dt: Add device tree property sdhci-cap-speed-modes-broken
  mmc: sdhci: Ignore capability register when it comes to speeds and use
    DT     binding instead when sdhci-cap-speed-modes-broken is set.

 Documentation/devicetree/bindings/mmc/mmc.txt |  3 +++
 drivers/mmc/host/sdhci.c                      | 31 +++++++++++++++++++++++++++
 2 files changed, 34 insertions(+)

--
2.7.4

[toc] | [next] | [standalone]


#1503352 — [RFC 1/2] mmc: sdhci: dt: Add device tree property sdhci-cap-speed-modes-broken

FromZach Brown <zach.brown@ni.com>
Date2016-10-18 23:10 +0200
Subject[RFC 1/2] mmc: sdhci: dt: Add device tree property sdhci-cap-speed-modes-broken
Message-ID<stJO2-7sS-23@gated-at.bofh.it>
In reply to#1503349
On some systems the sdhci capabilty registers are incorrect for one
reason or another.

The sdhci-cap-speed-modes-broken property will let the driver know that
the sdhci capability registers should not be relied on for speed modes.
Instead the driver should check the mmc generic DT bindings.

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

diff --git a/Documentation/devicetree/bindings/mmc/mmc.txt b/Documentation/devicetree/bindings/mmc/mmc.txt
index 8a37782..671d6c0 100644
--- a/Documentation/devicetree/bindings/mmc/mmc.txt
+++ b/Documentation/devicetree/bindings/mmc/mmc.txt
@@ -52,6 +52,9 @@ Optional properties:
 - no-sdio: controller is limited to send sdio cmd during initialization
 - no-sd: controller is limited to send sd cmd during initialization
 - no-mmc: controller is limited to send mmc cmd during initialization
+- sdhci-cap-speed-modes-broken: One or more of the bits in the sdhci
+  capabilities registers representing speed modes are incorrect. All the bits
+  representing speed modes should be ignored.
 
 *NOTE* on CD and WP polarity. To use common for all SD/MMC host controllers line
 polarity properties, we have to fix the meaning of the "normal" and "inverted"
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]


#1509823 — Re: [RFC 1/2] mmc: sdhci: dt: Add device tree property sdhci-cap-speed-modes-broken

FromRob Herring <robh@kernel.org>
Date2016-10-26 23:10 +0200
SubjectRe: [RFC 1/2] mmc: sdhci: dt: Add device tree property sdhci-cap-speed-modes-broken
Message-ID<swDCp-Ng-7@gated-at.bofh.it>
In reply to#1503352
On Tue, Oct 18, 2016 at 04:05:35PM -0500, Zach Brown wrote:
> On some systems the sdhci capabilty registers are incorrect for one
> reason or another.
> 
> The sdhci-cap-speed-modes-broken property will let the driver know that
> the sdhci capability registers should not be relied on for speed modes.
> Instead the driver should check the mmc generic DT bindings.
> 
> Signed-off-by: Zach Brown <zach.brown@ni.com>
> ---
>  Documentation/devicetree/bindings/mmc/mmc.txt | 3 +++
>  1 file changed, 3 insertions(+)

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

[toc] | [prev] | [next] | [standalone]


#1503354 — [RFC 2/2] mmc: sdhci: Ignore capability register when it comes to speeds and use DT binding instead when sdhci-cap-speed-modes-broken is set.

FromZach Brown <zach.brown@ni.com>
Date2016-10-18 23:10 +0200
Subject[RFC 2/2] mmc: sdhci: Ignore capability register when it comes to speeds and use DT binding instead when sdhci-cap-speed-modes-broken is set.
Message-ID<stJO2-7sS-17@gated-at.bofh.it>
In reply to#1503349
When the sdhci-cap-speed-modes-broken DT property is set, the driver
will ignore the bits of the capability registers that correspond to
speed modes and will read the of properties of the device to determine
which speeds are supported.

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

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 1e25b01..100649a 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>
 
@@ -3020,6 +3021,32 @@ void __sdhci_read_caps(struct sdhci_host *host, u16 *ver, u32 *caps, u32 *caps1)
 }
 EXPORT_SYMBOL_GPL(__sdhci_read_caps);
 
+void sdhci_get_speed_caps_from_of(struct sdhci_host *host)
+{
+	struct mmc_host *mmc = host->mmc;
+
+	host->caps &= ~SDHCI_CAN_DO_HISPD;
+
+	if (of_property_read_bool(mmc_dev(mmc)->of_node, "cap-sd-highspeed"))
+		host->caps |= SDHCI_CAN_DO_HISPD;
+
+	if (host->version < SDHCI_SPEC_300)
+		return;
+
+	host->caps1 &= ~(SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_SDR104 |
+			 SDHCI_SUPPORT_DDR50);
+
+	if (of_property_read_bool(mmc_dev(mmc)->of_node, "sd-uhs-sdr50"))
+		host->caps1 |= SDHCI_SUPPORT_SDR50;
+
+	if (of_property_read_bool(mmc_dev(mmc)->of_node, "sd-uhs-sdr104"))
+		host->caps1 |= SDHCI_SUPPORT_SDR104;
+
+	if (of_property_read_bool(mmc_dev(mmc)->of_node, "sd-uhs-ddr50"))
+		host->caps1 |= SDHCI_SUPPORT_DDR50;
+
+}
+
 int sdhci_setup_host(struct sdhci_host *host)
 {
 	struct mmc_host *mmc;
@@ -3046,6 +3073,10 @@ int sdhci_setup_host(struct sdhci_host *host)
 		return ret;
 
 	sdhci_read_caps(host);
+	if (of_property_read_bool(mmc_dev(mmc)->of_node,
+				  "sdhci-cap-speed-modes-broken"))
+		sdhci_get_speed_caps_from_of(host);
+
 
 	override_timeout_clk = host->timeout_clk;
 
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]


#1503729 — Re: [RFC 2/2] mmc: sdhci: Ignore capability register when it comes to speeds and use DT binding instead when sdhci-cap-speed-modes-broken is set.

FromUlf Hansson <ulf.hansson@linaro.org>
Date2016-10-19 16:40 +0200
SubjectRe: [RFC 2/2] mmc: sdhci: Ignore capability register when it comes to speeds and use DT binding instead when sdhci-cap-speed-modes-broken is set.
Message-ID<su0cb-2v4-115@gated-at.bofh.it>
In reply to#1503354
On 18 October 2016 at 23:05, Zach Brown <zach.brown@ni.com> wrote:
> When the sdhci-cap-speed-modes-broken DT property is set, the driver
> will ignore the bits of the capability registers that correspond to
> speed modes and will read the of properties of the device to determine
> which speeds are supported.

To me this seems like a great idea. Yeah, I proposed it so I guess
that's why. :-)

Anyway, it's Adrian call to decide how he want to go with this. He
might consider the other option [1] to be better.

Some more comments below.

>
> Signed-off-by: Zach Brown <zach.brown@ni.com>
> ---
>  drivers/mmc/host/sdhci.c | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 1e25b01..100649a 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>
>
> @@ -3020,6 +3021,32 @@ void __sdhci_read_caps(struct sdhci_host *host, u16 *ver, u32 *caps, u32 *caps1)
>  }
>  EXPORT_SYMBOL_GPL(__sdhci_read_caps);
>
> +void sdhci_get_speed_caps_from_of(struct sdhci_host *host)
> +{
> +       struct mmc_host *mmc = host->mmc;
> +
> +       host->caps &= ~SDHCI_CAN_DO_HISPD;
> +
> +       if (of_property_read_bool(mmc_dev(mmc)->of_node, "cap-sd-highspeed"))
> +               host->caps |= SDHCI_CAN_DO_HISPD;
> +
> +       if (host->version < SDHCI_SPEC_300)
> +               return;
> +
> +       host->caps1 &= ~(SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_SDR104 |
> +                        SDHCI_SUPPORT_DDR50);
> +
> +       if (of_property_read_bool(mmc_dev(mmc)->of_node, "sd-uhs-sdr50"))
> +               host->caps1 |= SDHCI_SUPPORT_SDR50;
> +
> +       if (of_property_read_bool(mmc_dev(mmc)->of_node, "sd-uhs-sdr104"))
> +               host->caps1 |= SDHCI_SUPPORT_SDR104;
> +
> +       if (of_property_read_bool(mmc_dev(mmc)->of_node, "sd-uhs-ddr50"))
> +               host->caps1 |= SDHCI_SUPPORT_DDR50;
> +

I don't think you need a separate OF parsing function. Instead the
SDHCI variant drivers may call mmc_of_parse() to parse the generic mmc
OF properties and then read the SDHCI caps registers (in some way or
the other).

As reading the SDHCI caps registers is done in __sdhci_read_caps(),
you could instead check for the OF property
"sdhci-cap-speed-modes-broken" in there,  and if it's set, clear the
related bits. I think that's all you need.

Note, the above code considers only SD speed modes, I assume we should
include eMMC speed modes to be broken as well!?

> +}
> +
>  int sdhci_setup_host(struct sdhci_host *host)
>  {
>         struct mmc_host *mmc;
> @@ -3046,6 +3073,10 @@ int sdhci_setup_host(struct sdhci_host *host)
>                 return ret;
>
>         sdhci_read_caps(host);
> +       if (of_property_read_bool(mmc_dev(mmc)->of_node,
> +                                 "sdhci-cap-speed-modes-broken"))
> +               sdhci_get_speed_caps_from_of(host);
> +
>
>         override_timeout_clk = host->timeout_clk;
>
> --
> 2.7.4
>

Kind regards
Uffe

[1]
http://www.spinics.net/lists/devicetree/msg146401.html

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web