* [RFC v2 0/2] Ignore capability registers when it comes to speeds and use DT binding instead. @ 2016-10-21 21:35 Zach Brown 2016-10-21 21:35 ` [RFC v2 1/2] mmc: sdhci: Add device tree property sdhci-cap-speed-modes-broken Zach Brown 2016-10-21 21:35 ` [RFC v2 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 0 siblings, 2 replies; 7+ messages in thread From: Zach Brown @ 2016-10-21 21:35 UTC (permalink / raw) To: ulf.hansson Cc: adrian.hunter, robh+dt, mark.rutland, linux-mmc, devicetree, linux-kernel, zach.brown 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[1] set I sent up. [1] https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1251944.html v2: * Removed separate OF parsing function, relying on mmc_of_parse to be called before read_caps * Moved check of sdhci-cap-speed-modes-broken into sdhci_read_caps. * Added SDHCI_SUPPORT_HS400 to list of bits to clear in sdhci cap1, not sure that's all of the them now, but I think so. Zach Brown (2): mmc: sdhci: 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 | 10 ++++++++++ 2 files changed, 13 insertions(+) -- 2.7.4 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [RFC v2 1/2] mmc: sdhci: Add device tree property sdhci-cap-speed-modes-broken 2016-10-21 21:35 [RFC v2 0/2] Ignore capability registers when it comes to speeds and use DT binding instead Zach Brown @ 2016-10-21 21:35 ` Zach Brown 2016-10-21 21:35 ` [RFC v2 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 1 sibling, 0 replies; 7+ messages in thread From: Zach Brown @ 2016-10-21 21:35 UTC (permalink / raw) To: ulf.hansson Cc: adrian.hunter, robh+dt, mark.rutland, linux-mmc, devicetree, linux-kernel, zach.brown 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [RFC v2 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. 2016-10-21 21:35 [RFC v2 0/2] Ignore capability registers when it comes to speeds and use DT binding instead Zach Brown 2016-10-21 21:35 ` [RFC v2 1/2] mmc: sdhci: Add device tree property sdhci-cap-speed-modes-broken Zach Brown @ 2016-10-21 21:35 ` Zach Brown 2016-10-24 7:34 ` Adrian Hunter 1 sibling, 1 reply; 7+ messages in thread From: Zach Brown @ 2016-10-21 21:35 UTC (permalink / raw) To: ulf.hansson Cc: adrian.hunter, robh+dt, mark.rutland, linux-mmc, devicetree, linux-kernel, zach.brown 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. Signed-off-by: Zach Brown <zach.brown@ni.com> --- drivers/mmc/host/sdhci.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 1e25b01..59c62d3 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> @@ -3013,10 +3014,19 @@ void __sdhci_read_caps(struct sdhci_host *host, u16 *ver, u32 *caps, u32 *caps1) host->caps = caps ? *caps : sdhci_readl(host, SDHCI_CAPABILITIES); + if (of_property_read_bool(mmc_dev(host->mmc)->of_node, + "sdhci-cap-speed-modes-broken")) + host->caps &= ~SDHCI_CAN_DO_HISPD; + if (host->version < SDHCI_SPEC_300) return; host->caps1 = caps1 ? *caps1 : sdhci_readl(host, SDHCI_CAPABILITIES_1); + + if (of_property_read_bool(mmc_dev(host->mmc)->of_node, + "sdhci-cap-speed-modes-broken")) + host->caps1 &= ~(SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_SDR104 | + SDHCI_SUPPORT_DDR50 | SDHCI_SUPPORT_HS400); } EXPORT_SYMBOL_GPL(__sdhci_read_caps); -- 2.7.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [RFC v2 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. 2016-10-21 21:35 ` [RFC v2 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 @ 2016-10-24 7:34 ` Adrian Hunter 2016-10-24 15:48 ` Zach Brown 0 siblings, 1 reply; 7+ messages in thread From: Adrian Hunter @ 2016-10-24 7:34 UTC (permalink / raw) To: Zach Brown, ulf.hansson Cc: robh+dt, mark.rutland, linux-mmc, devicetree, linux-kernel On 22/10/16 00:35, Zach Brown 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. > > Signed-off-by: Zach Brown <zach.brown@ni.com> > --- > drivers/mmc/host/sdhci.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index 1e25b01..59c62d3 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> > > @@ -3013,10 +3014,19 @@ void __sdhci_read_caps(struct sdhci_host *host, u16 *ver, u32 *caps, u32 *caps1) > > host->caps = caps ? *caps : sdhci_readl(host, SDHCI_CAPABILITIES); > > + if (of_property_read_bool(mmc_dev(host->mmc)->of_node, > + "sdhci-cap-speed-modes-broken")) It rather begs the question: if you are going to do something sdhci specific, why not just read the whole of the caps register from DT? > + host->caps &= ~SDHCI_CAN_DO_HISPD; > + > if (host->version < SDHCI_SPEC_300) > return; > > host->caps1 = caps1 ? *caps1 : sdhci_readl(host, SDHCI_CAPABILITIES_1); > + > + if (of_property_read_bool(mmc_dev(host->mmc)->of_node, > + "sdhci-cap-speed-modes-broken")) Why read it twice? > + host->caps1 &= ~(SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_SDR104 | > + SDHCI_SUPPORT_DDR50 | SDHCI_SUPPORT_HS400); > } > EXPORT_SYMBOL_GPL(__sdhci_read_caps); > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC v2 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. 2016-10-24 7:34 ` Adrian Hunter @ 2016-10-24 15:48 ` Zach Brown 2016-10-25 11:30 ` Adrian Hunter 0 siblings, 1 reply; 7+ messages in thread From: Zach Brown @ 2016-10-24 15:48 UTC (permalink / raw) To: Adrian Hunter Cc: ulf.hansson, robh+dt, mark.rutland, linux-mmc, devicetree, linux-kernel On Mon, Oct 24, 2016 at 10:34:46AM +0300, Adrian Hunter wrote: > On 22/10/16 00:35, Zach Brown 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. > > > > Signed-off-by: Zach Brown <zach.brown@ni.com> > > --- > > drivers/mmc/host/sdhci.c | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > > index 1e25b01..59c62d3 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> > > > > @@ -3013,10 +3014,19 @@ void __sdhci_read_caps(struct sdhci_host *host, u16 *ver, u32 *caps, u32 *caps1) > > > > host->caps = caps ? *caps : sdhci_readl(host, SDHCI_CAPABILITIES); > > > > + if (of_property_read_bool(mmc_dev(host->mmc)->of_node, > > + "sdhci-cap-speed-modes-broken")) > > It rather begs the question: if you are going to do something sdhci > specific, why not just read the whole of the caps register from DT? > Throwing out the whole of the caps register seems like overkill. Also there are some things set by the caps that are not available in the DT. For example, SDHCI_CAN_64BIT is set by the cap register and is used in sdhci_setup_host to set host->flags SDHCI_USE_64_BIT_DMA. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC v2 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. 2016-10-24 15:48 ` Zach Brown @ 2016-10-25 11:30 ` Adrian Hunter 2016-10-26 22:28 ` Zach Brown 0 siblings, 1 reply; 7+ messages in thread From: Adrian Hunter @ 2016-10-25 11:30 UTC (permalink / raw) To: Zach Brown Cc: ulf.hansson, robh+dt, mark.rutland, linux-mmc, devicetree, linux-kernel On 24/10/16 18:48, Zach Brown wrote: > On Mon, Oct 24, 2016 at 10:34:46AM +0300, Adrian Hunter wrote: >> On 22/10/16 00:35, Zach Brown 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. >>> >>> Signed-off-by: Zach Brown <zach.brown@ni.com> >>> --- >>> drivers/mmc/host/sdhci.c | 10 ++++++++++ >>> 1 file changed, 10 insertions(+) >>> >>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c >>> index 1e25b01..59c62d3 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> >>> >>> @@ -3013,10 +3014,19 @@ void __sdhci_read_caps(struct sdhci_host *host, u16 *ver, u32 *caps, u32 *caps1) >>> >>> host->caps = caps ? *caps : sdhci_readl(host, SDHCI_CAPABILITIES); >>> >>> + if (of_property_read_bool(mmc_dev(host->mmc)->of_node, >>> + "sdhci-cap-speed-modes-broken")) >> >> It rather begs the question: if you are going to do something sdhci >> specific, why not just read the whole of the caps register from DT? >> > > Throwing out the whole of the caps register seems like overkill. Also What about 2 values: one for the 64-bit caps register and one for a 64-bit mask of bits to override. That way you can select which bits to override and what to override them to. > there are some things set by the caps that are not available in the DT. > For example, SDHCI_CAN_64BIT is set by the cap register and is used in > sdhci_setup_host to set host->flags SDHCI_USE_64_BIT_DMA. Not sure what you mean. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC v2 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. 2016-10-25 11:30 ` Adrian Hunter @ 2016-10-26 22:28 ` Zach Brown 0 siblings, 0 replies; 7+ messages in thread From: Zach Brown @ 2016-10-26 22:28 UTC (permalink / raw) To: Adrian Hunter Cc: ulf.hansson, robh+dt, mark.rutland, linux-mmc, devicetree, linux-kernel On Tue, Oct 25, 2016 at 02:30:25PM +0300, Adrian Hunter wrote: > On 24/10/16 18:48, Zach Brown wrote: > > On Mon, Oct 24, 2016 at 10:34:46AM +0300, Adrian Hunter wrote: > >> On 22/10/16 00:35, Zach Brown 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. > >>> > >>> Signed-off-by: Zach Brown <zach.brown@ni.com> > >>> --- > >>> drivers/mmc/host/sdhci.c | 10 ++++++++++ > >>> 1 file changed, 10 insertions(+) > >>> > >>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > >>> index 1e25b01..59c62d3 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> > >>> > >>> @@ -3013,10 +3014,19 @@ void __sdhci_read_caps(struct sdhci_host *host, u16 *ver, u32 *caps, u32 *caps1) > >>> > >>> host->caps = caps ? *caps : sdhci_readl(host, SDHCI_CAPABILITIES); > >>> > >>> + if (of_property_read_bool(mmc_dev(host->mmc)->of_node, > >>> + "sdhci-cap-speed-modes-broken")) > >> > >> It rather begs the question: if you are going to do something sdhci > >> specific, why not just read the whole of the caps register from DT? > >> > > > > Throwing out the whole of the caps register seems like overkill. Also > > What about 2 values: one for the 64-bit caps register and one for a 64-bit > mask of bits to override. That way you can select which bits to override > and what to override them to. > I like this idea. I sent a RFC of it. > > there are some things set by the caps that are not available in the DT. > > For example, SDHCI_CAN_64BIT is set by the cap register and is used in > > sdhci_setup_host to set host->flags SDHCI_USE_64_BIT_DMA. > > Not sure what you mean. > Your suggestion addresses my concern. So it's not an issue. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-10-26 22:28 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-10-21 21:35 [RFC v2 0/2] Ignore capability registers when it comes to speeds and use DT binding instead Zach Brown 2016-10-21 21:35 ` [RFC v2 1/2] mmc: sdhci: Add device tree property sdhci-cap-speed-modes-broken Zach Brown 2016-10-21 21:35 ` [RFC v2 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 2016-10-24 7:34 ` Adrian Hunter 2016-10-24 15:48 ` Zach Brown 2016-10-25 11:30 ` Adrian Hunter 2016-10-26 22:28 ` Zach Brown
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).