| Message ID | 1477085715-30213-3-git-send-email-zach.brown@ni.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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); > > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" 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/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);
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(+)