| Message ID | 84f084eee198fb340631c739df6c05550a1a995a.1590144247.git.michal.simek@xilinx.com |
|---|---|
| State | Deferred |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | mmc: zynqmp_sdhci: Add support for Tap delay | expand |
Hi On 5/22/20 7:44 PM, Michal Simek wrote: > From: Ashok Reddy Soma <ashok.reddy.soma@xilinx.com> > > This reverts commit 942b5fc03218d1c94468fc658e7dec65dabcc830. > > This is partial revert of the above commit. > > mmc_of_parse() is reading no-1-8-v from device tree and if set, > it is clearing the UHS speed capabilities of cfg->host_caps. > cfg->host_caps &= ~(UHS_CAPS | MMC_MODE_HS200 | > MMC_MODE_HS400 | MMC_MODE_HS400_ES); > > This is still missing to clear UHS speeds like SDHCI_SUPPORT_SDR104, > SDHCI_SUPPORT_SDR50 and SDHCI_SUPPORT_DDR50. > > Even if we clear the flags SDHCI_SUPPORT_XXX in mmc_of_parse(), > these speed flags are getting set again in cfg->host_caps in > sdhci_setup_cfg(). > > The reason for this is, SDHCI_SUPPORT_XXX flags are cleared > only if controller is not capable of supporting MMC_VDD_165_195 volts. > > if (caps & SDHCI_CAN_VDD_180) > cfg->voltages |= MMC_VDD_165_195; > > if (!(cfg->voltages & MMC_VDD_165_195)) > caps_1 &= ~(SDHCI_SUPPORT_SDR104 | SDHCI_SUPPORT_SDR50 | > SDHCI_SUPPORT_DDR50); I don't have a target to test with zynq_sdhci. If there is no effect no-1-8-v property, i think that it needs to fix main problem. I will check about this. > > It means "no-1-8-v", which is read from DT is not coming in to effect. > So it is better we keep the host quirks(SDHCI_QUIRK_NO_1_8_V) to > clear UHS speeds based on no-1-8-v from device tree. > > Hence revert the functionality related to no-1-8-v only, rest is fine > in the patch. Anyway, I don't have any objection about reverting it. > > Signed-off-by: Ashok Reddy Soma <ashok.reddy.soma@xilinx.com> > Signed-off-by: Michal Simek <michal.simek@xilinx.com> > --- > > drivers/mmc/sdhci.c | 3 ++- > drivers/mmc/zynq_sdhci.c | 5 +++++ > include/sdhci.h | 1 + > 3 files changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c > index 92cc8434af24..099a007984a8 100644 > --- a/drivers/mmc/sdhci.c > +++ b/drivers/mmc/sdhci.c > @@ -844,7 +844,8 @@ int sdhci_setup_cfg(struct mmc_config *cfg, struct sdhci_host *host, > cfg->host_caps &= ~MMC_MODE_HS_52MHz; > } > > - if (!(cfg->voltages & MMC_VDD_165_195)) > + if (!(cfg->voltages & MMC_VDD_165_195) || > + (host->quirks & SDHCI_QUIRK_NO_1_8_V)) > caps_1 &= ~(SDHCI_SUPPORT_SDR104 | SDHCI_SUPPORT_SDR50 | > SDHCI_SUPPORT_DDR50); > > diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c > index 43b9f215229a..94c69cf1c1bd 100644 > --- a/drivers/mmc/zynq_sdhci.c > +++ b/drivers/mmc/zynq_sdhci.c > @@ -28,6 +28,7 @@ struct arasan_sdhci_priv { > struct sdhci_host *host; > u8 deviceid; > u8 bank; > + u8 no_1p8; > }; > > #if defined(CONFIG_ARCH_ZYNQMP) > @@ -236,6 +237,9 @@ static int arasan_sdhci_probe(struct udevice *dev) > host->quirks |= SDHCI_QUIRK_BROKEN_HISPD_MODE; > #endif > > + if (priv->no_1p8) > + host->quirks |= SDHCI_QUIRK_NO_1_8_V; > + > plat->cfg.f_max = CONFIG_ZYNQ_SDHCI_MAX_FREQ; > > ret = mmc_of_parse(dev, &plat->cfg); > @@ -277,6 +281,7 @@ static int arasan_sdhci_ofdata_to_platdata(struct udevice *dev) > > priv->deviceid = dev_read_u32_default(dev, "xlnx,device_id", -1); > priv->bank = dev_read_u32_default(dev, "xlnx,mio_bank", -1); > + priv->no_1p8 = dev_read_bool(dev, "no-1-8-v"); > > return 0; > } > diff --git a/include/sdhci.h b/include/sdhci.h > index 94fc3ed56ace..ef84bebcb4aa 100644 > --- a/include/sdhci.h > +++ b/include/sdhci.h > @@ -244,6 +244,7 @@ > #define SDHCI_QUIRK_BROKEN_HISPD_MODE BIT(5) > #define SDHCI_QUIRK_WAIT_SEND_CMD (1 << 6) > #define SDHCI_QUIRK_USE_WIDE8 (1 << 8) > +#define SDHCI_QUIRK_NO_1_8_V (1 << 9) > > /* to make gcc happy */ > struct sdhci_host; >
On 27. 05. 20 8:44, Jaehoon Chung wrote: > Hi > > On 5/22/20 7:44 PM, Michal Simek wrote: >> From: Ashok Reddy Soma <ashok.reddy.soma@xilinx.com> >> >> This reverts commit 942b5fc03218d1c94468fc658e7dec65dabcc830. >> >> This is partial revert of the above commit. >> >> mmc_of_parse() is reading no-1-8-v from device tree and if set, >> it is clearing the UHS speed capabilities of cfg->host_caps. >> cfg->host_caps &= ~(UHS_CAPS | MMC_MODE_HS200 | >> MMC_MODE_HS400 | MMC_MODE_HS400_ES); >> >> This is still missing to clear UHS speeds like SDHCI_SUPPORT_SDR104, >> SDHCI_SUPPORT_SDR50 and SDHCI_SUPPORT_DDR50. >> >> Even if we clear the flags SDHCI_SUPPORT_XXX in mmc_of_parse(), >> these speed flags are getting set again in cfg->host_caps in >> sdhci_setup_cfg(). >> >> The reason for this is, SDHCI_SUPPORT_XXX flags are cleared >> only if controller is not capable of supporting MMC_VDD_165_195 volts. >> >> if (caps & SDHCI_CAN_VDD_180) >> cfg->voltages |= MMC_VDD_165_195; >> >> if (!(cfg->voltages & MMC_VDD_165_195)) >> caps_1 &= ~(SDHCI_SUPPORT_SDR104 | SDHCI_SUPPORT_SDR50 | >> SDHCI_SUPPORT_DDR50); > > I don't have a target to test with zynq_sdhci. > If there is no effect no-1-8-v property, i think that it needs to fix main problem. > I will check about this. Have you had any time to take a look at it? Thanks, Michal
Hi, On 6/8/20 10:16 PM, Michal Simek wrote: > On 27. 05. 20 8:44, Jaehoon Chung wrote: >> Hi >> >> On 5/22/20 7:44 PM, Michal Simek wrote: >>> From: Ashok Reddy Soma <ashok.reddy.soma@xilinx.com> >>> >>> This reverts commit 942b5fc03218d1c94468fc658e7dec65dabcc830. >>> >>> This is partial revert of the above commit. >>> >>> mmc_of_parse() is reading no-1-8-v from device tree and if set, >>> it is clearing the UHS speed capabilities of cfg->host_caps. >>> cfg->host_caps &= ~(UHS_CAPS | MMC_MODE_HS200 | >>> MMC_MODE_HS400 | MMC_MODE_HS400_ES); >>> >>> This is still missing to clear UHS speeds like SDHCI_SUPPORT_SDR104, >>> SDHCI_SUPPORT_SDR50 and SDHCI_SUPPORT_DDR50. >>> >>> Even if we clear the flags SDHCI_SUPPORT_XXX in mmc_of_parse(), >>> these speed flags are getting set again in cfg->host_caps in >>> sdhci_setup_cfg(). >>> >>> The reason for this is, SDHCI_SUPPORT_XXX flags are cleared >>> only if controller is not capable of supporting MMC_VDD_165_195 volts. >>> >>> if (caps & SDHCI_CAN_VDD_180) >>> cfg->voltages |= MMC_VDD_165_195; >>> >>> if (!(cfg->voltages & MMC_VDD_165_195)) >>> caps_1 &= ~(SDHCI_SUPPORT_SDR104 | SDHCI_SUPPORT_SDR50 | >>> SDHCI_SUPPORT_DDR50); >> >> I don't have a target to test with zynq_sdhci. >> If there is no effect no-1-8-v property, i think that it needs to fix main problem. >> I will check about this. > > Have you had any time to take a look at it? Sorry for late. I think it was similar issue on my environment. I will share result within this week. If it's urgent, then after merging i will find solution to fix it. Best Regards, Jaehoon Chung > > Thanks, > Michal >
diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index 92cc8434af24..099a007984a8 100644 --- a/drivers/mmc/sdhci.c +++ b/drivers/mmc/sdhci.c @@ -844,7 +844,8 @@ int sdhci_setup_cfg(struct mmc_config *cfg, struct sdhci_host *host, cfg->host_caps &= ~MMC_MODE_HS_52MHz; } - if (!(cfg->voltages & MMC_VDD_165_195)) + if (!(cfg->voltages & MMC_VDD_165_195) || + (host->quirks & SDHCI_QUIRK_NO_1_8_V)) caps_1 &= ~(SDHCI_SUPPORT_SDR104 | SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_DDR50); diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c index 43b9f215229a..94c69cf1c1bd 100644 --- a/drivers/mmc/zynq_sdhci.c +++ b/drivers/mmc/zynq_sdhci.c @@ -28,6 +28,7 @@ struct arasan_sdhci_priv { struct sdhci_host *host; u8 deviceid; u8 bank; + u8 no_1p8; }; #if defined(CONFIG_ARCH_ZYNQMP) @@ -236,6 +237,9 @@ static int arasan_sdhci_probe(struct udevice *dev) host->quirks |= SDHCI_QUIRK_BROKEN_HISPD_MODE; #endif + if (priv->no_1p8) + host->quirks |= SDHCI_QUIRK_NO_1_8_V; + plat->cfg.f_max = CONFIG_ZYNQ_SDHCI_MAX_FREQ; ret = mmc_of_parse(dev, &plat->cfg); @@ -277,6 +281,7 @@ static int arasan_sdhci_ofdata_to_platdata(struct udevice *dev) priv->deviceid = dev_read_u32_default(dev, "xlnx,device_id", -1); priv->bank = dev_read_u32_default(dev, "xlnx,mio_bank", -1); + priv->no_1p8 = dev_read_bool(dev, "no-1-8-v"); return 0; } diff --git a/include/sdhci.h b/include/sdhci.h index 94fc3ed56ace..ef84bebcb4aa 100644 --- a/include/sdhci.h +++ b/include/sdhci.h @@ -244,6 +244,7 @@ #define SDHCI_QUIRK_BROKEN_HISPD_MODE BIT(5) #define SDHCI_QUIRK_WAIT_SEND_CMD (1 << 6) #define SDHCI_QUIRK_USE_WIDE8 (1 << 8) +#define SDHCI_QUIRK_NO_1_8_V (1 << 9) /* to make gcc happy */ struct sdhci_host;