| Message ID | 20171214130941.26666-1-kishon@ti.com |
|---|---|
| Headers | show |
| Series | mmc: sdhci-omap: Add UHS/HS200 mode support | expand |
Kishon, On Thu, Dec 14, 2017 at 2:09 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote: > DRA74x EVM Rev H EVM comes with revision 2.0 silicon. However, earlier > versions of EVM can come with either revision 1.1 or revision 1.0 of > silicon. > > The device-tree file is written to support rev 2.0 of silicon. > pdata-quirks are used to then override the settings needed for > PG 1.1 silicon. > > PG 1.1 silicon has limitations w.r.t frequencies at which MMC1/2/3 > can operate as well as different IOdelay numbers. > > Add support in sdhci-omap driver to get platform data if available > (added using pdata quirks) and override the data (max-frequency and > iodelay data) obtained from device tree. > > Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> <snip> > --- /dev/null > +++ b/include/linux/platform_data/sdhci-omap.h > @@ -0,0 +1,35 @@ > +/** > + * SDHCI Controller Platform Data for TI's OMAP SoCs > + * > + * Copyright (C) 2017 Texas Instruments > + * Author: Kishon Vijay Abraham I <kishon@ti.com> > + * > + * This program is free software: you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 of > + * the License as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. If not, see <http://www.gnu.org/licenses/>. > + */ Could you use the new SPDX tags instead of this fine and long boilerplate? See Thomas doc for details [1] [1] https://lkml.org/lkml/2017/12/4/934 Thanks! PS: if you could spread the word out in your team too, this would be much welcomed!
Hi, * Kishon Vijay Abraham I <kishon@ti.com> [171214 13:13]: > The data manual of J6/J6 Eco recommends to set different IODELAY values > depending on the mode in which the MMC/SD is enumerated in order to > ensure IO timings are met. > > Add support to set the IODELAY values depending on the various MMC > modes using the pinctrl APIs. ... > --- a/drivers/mmc/host/sdhci-omap.c > +++ b/drivers/mmc/host/sdhci-omap.c > @@ -105,6 +109,20 @@ struct sdhci_omap_host { > struct sdhci_host *host; > u8 bus_mode; > u8 power_mode; > + u8 timing; > + u8 flags; > + > + struct pinctrl *pinctrl; > + struct pinctrl_state *pinctrl_state; > + struct pinctrl_state *default_pinctrl_state; > + struct pinctrl_state *sdr104_pinctrl_state; > + struct pinctrl_state *hs200_1_8v_pinctrl_state; > + struct pinctrl_state *ddr50_pinctrl_state; > + struct pinctrl_state *sdr50_pinctrl_state; > + struct pinctrl_state *sdr25_pinctrl_state; > + struct pinctrl_state *sdr12_pinctrl_state; > + struct pinctrl_state *hs_pinctrl_state; > + struct pinctrl_state *ddr_1_8v_pinctrl_state; > }; You can make the pinctrl code more generic by allocating an array of states and have just: struct pinctrl_state **pinctrl_state; Then access it with omap_host->pinctrl_state[MMC_TIMING_MMC_HS200] and so on. This way the code gets simplified and you can do a generic function to initialize things and call it from a for loop etc. Just remember that pinctrl use can be optional as the pins can be set up in the bootloader alone. Then you can just continue with the default iodelay state like we are currently doing. Regards, Tony
On Thu, Dec 14, 2017 at 06:39:39PM +0530, Kishon Vijay Abraham I wrote: > Add binding for the TI's sdhci-omap controller present in K2G. > > Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> > --- > Documentation/devicetree/bindings/mmc/sdhci-omap.txt | 2 ++ > 1 file changed, 2 insertions(+) Reviewed-by: Rob Herring <robh@kernel.org>
On 14/12/17 15:09, Kishon Vijay Abraham I wrote: > Updating 'power_mode' in sdhci_omap_init_74_clocks results in > 'power_mode' never updated to MMC_POWER_OFF during card > removal. This results in initialization sequence not sent to the > card during re-insertion. > Fix it here by adding sdhci_omap_set_power_mode to update power_mode. > This function can also be used later to perform operations that > are specific to a power mode (e.g, disable tuning during power off). > > Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> Acked-by: Adrian Hunter <adrian.hunter@intel.com> > --- > drivers/mmc/host/sdhci-omap.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c > index 628bfe9a3d17..96985786cadf 100644 > --- a/drivers/mmc/host/sdhci-omap.c > +++ b/drivers/mmc/host/sdhci-omap.c > @@ -244,6 +244,12 @@ static int sdhci_omap_start_signal_voltage_switch(struct mmc_host *mmc, > return 0; > } > > +static void sdhci_omap_set_power_mode(struct sdhci_omap_host *omap_host, > + u8 power_mode) > +{ > + omap_host->power_mode = power_mode; > +} > + > static void sdhci_omap_set_bus_mode(struct sdhci_omap_host *omap_host, > unsigned int mode) > { > @@ -273,6 +279,7 @@ static void sdhci_omap_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) > > sdhci_omap_set_bus_mode(omap_host, ios->bus_mode); > sdhci_set_ios(mmc, ios); > + sdhci_omap_set_power_mode(omap_host, ios->power_mode); > } > > static u16 sdhci_omap_calc_divisor(struct sdhci_pltfm_host *host, > @@ -401,8 +408,6 @@ static void sdhci_omap_init_74_clocks(struct sdhci_host *host, u8 power_mode) > sdhci_omap_writel(omap_host, SDHCI_OMAP_STAT, INT_CC_EN); > > enable_irq(host->irq); > - > - omap_host->power_mode = power_mode; > } > > static struct sdhci_ops sdhci_omap_ops = { > @@ -504,6 +509,7 @@ static int sdhci_omap_probe(struct platform_device *pdev) > omap_host->host = host; > omap_host->base = host->ioaddr; > omap_host->dev = dev; > + omap_host->power_mode = MMC_POWER_UNDEFINED; > host->ioaddr += offset; > > mmc = host->mmc; >
On 14/12/17 15:09, Kishon Vijay Abraham I wrote: > UHS-1 DDR50 and MMC DDR52 mode require DDR bit to be > set in the configuration register (MMCHS_CON). Add > sdhci-omap specific set_uhs_signaling ops to set > this bit. Also while setting the UHSMS bit, clock should be > disabled. > > Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> Apart from 1 minor comment below: Acked-by: Adrian Hunter <adrian.hunter@intel.com> > --- > drivers/mmc/host/sdhci-omap.c | 26 +++++++++++++++++++++++++- > 1 file changed, 25 insertions(+), 1 deletion(-) > > diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c > index defe4eac020d..8f7239e2edc2 100644 > --- a/drivers/mmc/host/sdhci-omap.c > +++ b/drivers/mmc/host/sdhci-omap.c > @@ -31,6 +31,7 @@ > #define SDHCI_OMAP_CON 0x12c > #define CON_DW8 BIT(5) > #define CON_DMA_MASTER BIT(20) > +#define CON_DDR BIT(19) > #define CON_CLKEXTFREE BIT(16) > #define CON_PADEN BIT(15) > #define CON_INIT BIT(1) > @@ -93,6 +94,9 @@ struct sdhci_omap_host { > u8 power_mode; > }; > > +static void sdhci_omap_start_clock(struct sdhci_omap_host *omap_host); > +static void sdhci_omap_stop_clock(struct sdhci_omap_host *omap_host); These forward declarations aren't needed are they. > + > static inline u32 sdhci_omap_readl(struct sdhci_omap_host *host, > unsigned int offset) > { > @@ -471,6 +475,26 @@ static void sdhci_omap_init_74_clocks(struct sdhci_host *host, u8 power_mode) > enable_irq(host->irq); > } > > +static void sdhci_omap_set_uhs_signaling(struct sdhci_host *host, > + unsigned int timing) > +{ > + u32 reg; > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > + struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host); > + > + sdhci_omap_stop_clock(omap_host); > + > + reg = sdhci_omap_readl(omap_host, SDHCI_OMAP_CON); > + if (timing == MMC_TIMING_UHS_DDR50 || timing == MMC_TIMING_MMC_DDR52) > + reg |= CON_DDR; > + else > + reg &= ~CON_DDR; > + sdhci_omap_writel(omap_host, SDHCI_OMAP_CON, reg); > + > + sdhci_set_uhs_signaling(host, timing); > + sdhci_omap_start_clock(omap_host); > +} > + > static struct sdhci_ops sdhci_omap_ops = { > .set_clock = sdhci_omap_set_clock, > .set_power = sdhci_omap_set_power, > @@ -480,7 +504,7 @@ static struct sdhci_ops sdhci_omap_ops = { > .set_bus_width = sdhci_omap_set_bus_width, > .platform_send_init_74_clocks = sdhci_omap_init_74_clocks, > .reset = sdhci_reset, > - .set_uhs_signaling = sdhci_set_uhs_signaling, > + .set_uhs_signaling = sdhci_omap_set_uhs_signaling, > }; > > static int sdhci_omap_set_capabilities(struct sdhci_omap_host *omap_host) >
On 14/12/17 15:09, Kishon Vijay Abraham I wrote: > Remove SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk as gpio card detection > is supported in sdhci-omap. SDHCI_QUIRK_BROKEN_CARD_DETECTION is for native card detection not gpio card detection. > > Add SDHCI_QUIRK2_PRESET_VALUE_BROKEN quirk as setting preset values loads > incorrect CLKD values (for UHS modes). > > Remove SDHCI_QUIRK2_NO_1_8_V quirk as sdhci-omap now supports UHS modes. > > Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> > --- > drivers/mmc/host/sdhci-omap.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c > index 594e41200d8a..6dee275b2e57 100644 > --- a/drivers/mmc/host/sdhci-omap.c > +++ b/drivers/mmc/host/sdhci-omap.c > @@ -755,13 +755,12 @@ static int sdhci_omap_set_capabilities(struct sdhci_omap_host *omap_host) > } > > static const struct sdhci_pltfm_data sdhci_omap_pdata = { > - .quirks = SDHCI_QUIRK_BROKEN_CARD_DETECTION | > - SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK | > + .quirks = SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK | > SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN | > SDHCI_QUIRK_NO_HISPD_BIT | > SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC, > - .quirks2 = SDHCI_QUIRK2_NO_1_8_V | > - SDHCI_QUIRK2_ACMD23_BROKEN | > + .quirks2 = SDHCI_QUIRK2_ACMD23_BROKEN | > + SDHCI_QUIRK2_PRESET_VALUE_BROKEN | > SDHCI_QUIRK2_RSP_136_HAS_CRC, > .ops = &sdhci_omap_ops, > }; >
On 14/12/17 15:09, Kishon Vijay Abraham I wrote: > DRA74x EVM Rev H EVM comes with revision 2.0 silicon. However, earlier > versions of EVM can come with either revision 1.1 or revision 1.0 of > silicon. > > The device-tree file is written to support rev 2.0 of silicon. > pdata-quirks are used to then override the settings needed for > PG 1.1 silicon. > > PG 1.1 silicon has limitations w.r.t frequencies at which MMC1/2/3 > can operate as well as different IOdelay numbers. > > Add support in sdhci-omap driver to get platform data if available > (added using pdata quirks) and override the data (max-frequency and > iodelay data) obtained from device tree. > > Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> > --- > drivers/mmc/host/sdhci-omap.c | 17 ++++++++++++++++ > include/linux/platform_data/sdhci-omap.h | 35 ++++++++++++++++++++++++++++++++ > 2 files changed, 52 insertions(+) > create mode 100644 include/linux/platform_data/sdhci-omap.h > > diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c > index 6dee275b2e57..cddc3ad1331f 100644 > --- a/drivers/mmc/host/sdhci-omap.c > +++ b/drivers/mmc/host/sdhci-omap.c > @@ -22,6 +22,7 @@ > #include <linux/module.h> > #include <linux/of.h> > #include <linux/of_device.h> > +#include <linux/platform_data/sdhci-omap.h> > #include <linux/platform_device.h> > #include <linux/pm_runtime.h> > #include <linux/regulator/consumer.h> > @@ -102,6 +103,7 @@ struct sdhci_omap_data { > }; > > struct sdhci_omap_host { > + char *version; > void __iomem *base; > struct device *dev; > struct regulator *pbias; > @@ -781,11 +783,18 @@ static struct pinctrl_state > u32 *caps, u32 capmask) > { > struct device *dev = omap_host->dev; > + char *version = omap_host->version; > struct pinctrl_state *pinctrl_state = ERR_PTR(-ENODEV); > + char str[20]; > > if (!(*caps & capmask)) > goto ret; > > + if (version) { > + sprintf(str, "%s-%s", mode, version); snprintf please > + pinctrl_state = pinctrl_lookup_state(omap_host->pinctrl, str); Doesn't look like this 'pinctrl_state' is used? > + } > + > pinctrl_state = pinctrl_lookup_state(omap_host->pinctrl, mode); > if (IS_ERR(pinctrl_state)) { > dev_err(dev, "no pinctrl state for %s mode", mode); > @@ -879,6 +888,7 @@ static int sdhci_omap_probe(struct platform_device *pdev) > struct mmc_host *mmc; > const struct of_device_id *match; > struct sdhci_omap_data *data; > + struct sdhci_omap_platform_data *platform_data; > > match = of_match_device(omap_sdhci_match, dev); > if (!match) > @@ -913,6 +923,13 @@ static int sdhci_omap_probe(struct platform_device *pdev) > if (ret) > goto err_pltfm_free; > > + platform_data = dev_get_platdata(dev); > + if (platform_data) { > + omap_host->version = platform_data->version; > + if (platform_data->max_freq) > + mmc->f_max = platform_data->max_freq; > + } > + > pltfm_host->clk = devm_clk_get(dev, "fck"); > if (IS_ERR(pltfm_host->clk)) { > ret = PTR_ERR(pltfm_host->clk); > diff --git a/include/linux/platform_data/sdhci-omap.h b/include/linux/platform_data/sdhci-omap.h > new file mode 100644 > index 000000000000..a46e1240956a > --- /dev/null > +++ b/include/linux/platform_data/sdhci-omap.h > @@ -0,0 +1,35 @@ > +/** > + * SDHCI Controller Platform Data for TI's OMAP SoCs > + * > + * Copyright (C) 2017 Texas Instruments > + * Author: Kishon Vijay Abraham I <kishon@ti.com> > + * > + * This program is free software: you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 of > + * the License as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. If not, see <http://www.gnu.org/licenses/>. > + */ > +#ifndef __SDHCI_OMAP_PDATA_H__ > +#define __SDHCI_OMAP_PDATA_H__ > + > +struct sdhci_omap_platform_data { > + const char *name; > + > + /* > + * set if your board has components or wiring that limits the > + * maximum frequency on the MMC bus > + */ > + unsigned int max_freq; > + > + /* string specifying a particular variant of hardware */ > + char *version; > +}; > + > +#endif >
On 14/12/17 15:09, Kishon Vijay Abraham I wrote: > Add support for the new compatible added specifically to support > k2g's MMC/SD controller. > > Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> Acked-by: Adrian Hunter <adrian.hunter@intel.com> > --- > drivers/mmc/host/sdhci-omap.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c > index cddc3ad1331f..5e81e29383d9 100644 > --- a/drivers/mmc/host/sdhci-omap.c > +++ b/drivers/mmc/host/sdhci-omap.c > @@ -767,6 +767,10 @@ static const struct sdhci_pltfm_data sdhci_omap_pdata = { > .ops = &sdhci_omap_ops, > }; > > +static const struct sdhci_omap_data k2g_data = { > + .offset = 0x200, > +}; > + > static const struct sdhci_omap_data dra7_data = { > .offset = 0x200, > .flags = SDHCI_OMAP_REQUIRE_IODELAY, > @@ -774,6 +778,7 @@ static const struct sdhci_omap_data dra7_data = { > > static const struct of_device_id omap_sdhci_match[] = { > { .compatible = "ti,dra7-sdhci", .data = &dra7_data }, > + { .compatible = "ti,k2g-sdhci", .data = &k2g_data }, > {}, > }; > MODULE_DEVICE_TABLE(of, omap_sdhci_match); > @@ -882,6 +887,7 @@ static int sdhci_omap_probe(struct platform_device *pdev) > int ret; > u32 offset; > struct device *dev = &pdev->dev; > + struct device_node *node = dev->of_node; > struct sdhci_host *host; > struct sdhci_pltfm_host *pltfm_host; > struct sdhci_omap_host *omap_host; > @@ -908,6 +914,9 @@ static int sdhci_omap_probe(struct platform_device *pdev) > return PTR_ERR(host); > } > > + if (of_device_is_compatible(node, "ti,k2g-sdhci")) > + host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V; > + > pltfm_host = sdhci_priv(host); > omap_host = sdhci_pltfm_priv(pltfm_host); > omap_host->host = host; >
Add UHS/HS200 mode support in sdhci-omap. The programming sequence for voltage switching, tuning is followed from AM572x TRM http://www.ti.com/lit/ug/spruhz6i/spruhz6i.pdf (Similar to all AM57x/DRA7x SoCs). The patch series also implements workaround for errata published in http://www.ti.com/lit/er/sprz429k/sprz429k.pdf. While most of this series is specific to sdhci-omap, it also patches sdhci to use software timer when the requested timeout is greater than hardware capablility. This re-uses the SW data timer already implemented in sdhci while disabling the HW timeout (so that spurious timeout is not observed). The patch for sdhci.c is based on an earlier patch that was done specific to omap_hsmmc.c (https://patchwork.kernel.org/patch/9791449/) It also includes a pdata-quirk patch since both pdata-quirks and sdhci-omap uses struct sdhci_omap_platform_data. The dt patches enabling UHS/HS200 will be follow this patch series. This series is created on git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/mmc.git next Kishon Vijay Abraham I (12): mmc: sdhci-omap: Update 'power_mode' outside sdhci_omap_init_74_clocks mmc: sdhci-omap: Add card_busy host ops mmc: sdhci-omap: Add custom set_uhs_signaling sdhci_host ops mmc: sdhci-omap: Add tuning support mmc: sdhci-omap: Workaround for Errata i802 mmc: sdhci_omap: Add support to set IODELAY values mmc: sdhci_omap: Fix sdhci-omap quirks mmc: sdhci-omap: Add support to override f_max and iodelay from pdata mmc: sdhci: Use software timer when timeout greater than hardware capablility dt-bindings: sdhci-omap: Add K2G specific binding mmc: sdhci-omap: Add support for MMC/SD controller in k2g SoC ARM: OMAP2+: Use sdhci-omap specific pdata-quirks for MMC/SD on DRA74x EVM .../devicetree/bindings/mmc/sdhci-omap.txt | 2 + arch/arm/mach-omap2/pdata-quirks.c | 34 +- drivers/mmc/host/sdhci-omap.c | 446 ++++++++++++++++++++- drivers/mmc/host/sdhci.c | 41 +- drivers/mmc/host/sdhci.h | 11 + include/linux/platform_data/sdhci-omap.h | 35 ++ 6 files changed, 544 insertions(+), 25 deletions(-) create mode 100644 include/linux/platform_data/sdhci-omap.h