* mmc: zynq: parse dt when probing
2020-04-07 14:15 ` [PATCH V2] mmc: zynq: parse dt when probing Benedikt Grassl
@ 2020-04-07 14:21 ` Benedikt Grassl
2020-04-08 6:50 ` [PATCH V2] " Michal Simek
2020-04-08 22:11 ` Jaehoon Chung
2 siblings, 0 replies; 12+ messages in thread
From: Benedikt Grassl @ 2020-04-07 14:21 UTC (permalink / raw)
To: u-boot
Hi Michal,
I did not revert b8e25ef16a58 ("mmc: sdhci: Read capabilities register1 and update host caps") since without those changes, the UHS caps would not disabled if you forgot to add the "no-1-8-v" property.
I think relying on the voltage configuration instead is worth keeping.
Best Regards,
Ben
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH V2] mmc: zynq: parse dt when probing
2020-04-07 14:15 ` [PATCH V2] mmc: zynq: parse dt when probing Benedikt Grassl
2020-04-07 14:21 ` Benedikt Grassl
@ 2020-04-08 6:50 ` Michal Simek
2020-04-08 22:11 ` Jaehoon Chung
2 siblings, 0 replies; 12+ messages in thread
From: Michal Simek @ 2020-04-08 6:50 UTC (permalink / raw)
To: u-boot
On 07. 04. 20 16:15, Benedikt Grassl wrote:
> Currently, the entry "bus-width = <8>" in the ZynqMP's sdhci nodes
> is not evaluated. This results in the bus width staying at its default
> value (4 bit in HS200 mode).
> Fix this by calling mmc_of_parse. This function also checks for the
> "no-1-8-v" and "max-frequency" entries. Remove the handling of those
> nodes from this driver.
>
> Signed-off-by: Benedikt Grassl <Benedikt.Grassl@rohde-schwarz.com>
> ---
> drivers/mmc/sdhci.c | 3 +--
> drivers/mmc/zynq_sdhci.c | 15 ++++++---------
> include/sdhci.h | 1 -
> 3 files changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> index 520c9f9feb..372dc0a820 100644
> --- a/drivers/mmc/sdhci.c
> +++ b/drivers/mmc/sdhci.c
> @@ -839,8 +839,7 @@ 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) ||
> - (host->quirks & SDHCI_QUIRK_NO_1_8_V))
> + if (!(cfg->voltages & MMC_VDD_165_195))
> 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 da3ff53da1..cfa61af265 100644
> --- a/drivers/mmc/zynq_sdhci.c
> +++ b/drivers/mmc/zynq_sdhci.c
> @@ -22,14 +22,12 @@ DECLARE_GLOBAL_DATA_PTR;
> struct arasan_sdhci_plat {
> struct mmc_config cfg;
> struct mmc mmc;
> - unsigned int f_max;
> };
>
> struct arasan_sdhci_priv {
> struct sdhci_host *host;
> u8 deviceid;
> u8 bank;
> - u8 no_1p8;
> };
>
> #if defined(CONFIG_ARCH_ZYNQMP)
> @@ -238,8 +236,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;
> + ret = mmc_of_parse(dev, &plat->cfg);
> + if (ret)
> + return ret;
>
> host->max_clk = clock;
>
> @@ -247,10 +246,12 @@ static int arasan_sdhci_probe(struct udevice *dev)
> host->mmc->dev = dev;
> host->mmc->priv = host;
>
> - ret = sdhci_setup_cfg(&plat->cfg, host, plat->f_max,
> + ret = sdhci_setup_cfg(&plat->cfg, host,
> + CONFIG_ZYNQ_SDHCI_MAX_FREQ,
> CONFIG_ZYNQ_SDHCI_MIN_FREQ);
> if (ret)
> return ret;
> +
> upriv->mmc = host->mmc;
>
> return sdhci_probe(dev);
> @@ -258,7 +259,6 @@ static int arasan_sdhci_probe(struct udevice *dev)
>
> static int arasan_sdhci_ofdata_to_platdata(struct udevice *dev)
> {
> - struct arasan_sdhci_plat *plat = dev_get_platdata(dev);
> struct arasan_sdhci_priv *priv = dev_get_priv(dev);
>
> priv->host = calloc(1, sizeof(struct sdhci_host));
> @@ -277,10 +277,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");
>
> - plat->f_max = dev_read_u32_default(dev, "max-frequency",
> - CONFIG_ZYNQ_SDHCI_MAX_FREQ);
> return 0;
> }
>
> diff --git a/include/sdhci.h b/include/sdhci.h
> index aa4378fd57..0ef8c2ed62 100644
> --- a/include/sdhci.h
> +++ b/include/sdhci.h
> @@ -243,7 +243,6 @@
> #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;
>
Driver has broken platdata/privdata usage but that's something what it
is not introduced by this patch that's why applied.
Thanks,
Michal
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH V2] mmc: zynq: parse dt when probing
2020-04-07 14:15 ` [PATCH V2] mmc: zynq: parse dt when probing Benedikt Grassl
2020-04-07 14:21 ` Benedikt Grassl
2020-04-08 6:50 ` [PATCH V2] " Michal Simek
@ 2020-04-08 22:11 ` Jaehoon Chung
2020-04-09 7:26 ` Michal Simek
2 siblings, 1 reply; 12+ messages in thread
From: Jaehoon Chung @ 2020-04-08 22:11 UTC (permalink / raw)
To: u-boot
On 4/7/20 11:15 PM, Benedikt Grassl wrote:
> Currently, the entry "bus-width = <8>" in the ZynqMP's sdhci nodes
> is not evaluated. This results in the bus width staying at its default
> value (4 bit in HS200 mode).
> Fix this by calling mmc_of_parse. This function also checks for the
> "no-1-8-v" and "max-frequency" entries. Remove the handling of those
> nodes from this driver.
>
> Signed-off-by: Benedikt Grassl <Benedikt.Grassl@rohde-schwarz.com>
> ---
> drivers/mmc/sdhci.c | 3 +--
> drivers/mmc/zynq_sdhci.c | 15 ++++++---------
> include/sdhci.h | 1 -
> 3 files changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> index 520c9f9feb..372dc0a820 100644
> --- a/drivers/mmc/sdhci.c
> +++ b/drivers/mmc/sdhci.c
> @@ -839,8 +839,7 @@ 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) ||
> - (host->quirks & SDHCI_QUIRK_NO_1_8_V))
> + if (!(cfg->voltages & MMC_VDD_165_195))
> 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 da3ff53da1..cfa61af265 100644
> --- a/drivers/mmc/zynq_sdhci.c
> +++ b/drivers/mmc/zynq_sdhci.c
> @@ -22,14 +22,12 @@ DECLARE_GLOBAL_DATA_PTR;
> struct arasan_sdhci_plat {
> struct mmc_config cfg;
> struct mmc mmc;
> - unsigned int f_max;
> };
>
> struct arasan_sdhci_priv {
> struct sdhci_host *host;
> u8 deviceid;
> u8 bank;
> - u8 no_1p8;
> };
>
> #if defined(CONFIG_ARCH_ZYNQMP)
> @@ -238,8 +236,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;
> + ret = mmc_of_parse(dev, &plat->cfg);
> + if (ret)
> + return ret;
>
> host->max_clk = clock;
>
> @@ -247,10 +246,12 @@ static int arasan_sdhci_probe(struct udevice *dev)
> host->mmc->dev = dev;
> host->mmc->priv = host;
>
> - ret = sdhci_setup_cfg(&plat->cfg, host, plat->f_max,
> + ret = sdhci_setup_cfg(&plat->cfg, host,
> + CONFIG_ZYNQ_SDHCI_MAX_FREQ,
> CONFIG_ZYNQ_SDHCI_MIN_FREQ);
you have removed the parsing "max-frequency" from below code, because it's parsed from mmc_of_parse().
But It's passing to CONFIG_ZYNQ_SDHCI_MAX_FREQ again. Is it correct?
Could you check one more?
Best Regards,
Jaehoon Chung
> if (ret)
> return ret;
> +
> upriv->mmc = host->mmc;
>
> return sdhci_probe(dev);
> @@ -258,7 +259,6 @@ static int arasan_sdhci_probe(struct udevice *dev)
>
> static int arasan_sdhci_ofdata_to_platdata(struct udevice *dev)
> {
> - struct arasan_sdhci_plat *plat = dev_get_platdata(dev);
> struct arasan_sdhci_priv *priv = dev_get_priv(dev);
>
> priv->host = calloc(1, sizeof(struct sdhci_host));
> @@ -277,10 +277,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");
>
> - plat->f_max = dev_read_u32_default(dev, "max-frequency",
> - CONFIG_ZYNQ_SDHCI_MAX_FREQ);
> return 0;
> }
>
> diff --git a/include/sdhci.h b/include/sdhci.h
> index aa4378fd57..0ef8c2ed62 100644
> --- a/include/sdhci.h
> +++ b/include/sdhci.h
> @@ -243,7 +243,6 @@
> #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;
>
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH V2] mmc: zynq: parse dt when probing
2020-04-08 22:11 ` Jaehoon Chung
@ 2020-04-09 7:26 ` Michal Simek
2020-04-11 9:03 ` [PATCH v3] " Benedikt Grassl
0 siblings, 1 reply; 12+ messages in thread
From: Michal Simek @ 2020-04-09 7:26 UTC (permalink / raw)
To: u-boot
On 09. 04. 20 0:11, Jaehoon Chung wrote:
> On 4/7/20 11:15 PM, Benedikt Grassl wrote:
>> Currently, the entry "bus-width = <8>" in the ZynqMP's sdhci nodes
>> is not evaluated. This results in the bus width staying at its default
>> value (4 bit in HS200 mode).
>> Fix this by calling mmc_of_parse. This function also checks for the
>> "no-1-8-v" and "max-frequency" entries. Remove the handling of those
>> nodes from this driver.
>>
>> Signed-off-by: Benedikt Grassl <Benedikt.Grassl@rohde-schwarz.com>
>> ---
>> drivers/mmc/sdhci.c | 3 +--
>> drivers/mmc/zynq_sdhci.c | 15 ++++++---------
>> include/sdhci.h | 1 -
>> 3 files changed, 7 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
>> index 520c9f9feb..372dc0a820 100644
>> --- a/drivers/mmc/sdhci.c
>> +++ b/drivers/mmc/sdhci.c
>> @@ -839,8 +839,7 @@ 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) ||
>> - (host->quirks & SDHCI_QUIRK_NO_1_8_V))
>> + if (!(cfg->voltages & MMC_VDD_165_195))
>> 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 da3ff53da1..cfa61af265 100644
>> --- a/drivers/mmc/zynq_sdhci.c
>> +++ b/drivers/mmc/zynq_sdhci.c
>> @@ -22,14 +22,12 @@ DECLARE_GLOBAL_DATA_PTR;
>> struct arasan_sdhci_plat {
>> struct mmc_config cfg;
>> struct mmc mmc;
>> - unsigned int f_max;
>> };
>>
>> struct arasan_sdhci_priv {
>> struct sdhci_host *host;
>> u8 deviceid;
>> u8 bank;
>> - u8 no_1p8;
>> };
>>
>> #if defined(CONFIG_ARCH_ZYNQMP)
>> @@ -238,8 +236,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;
>> + ret = mmc_of_parse(dev, &plat->cfg);
>> + if (ret)
>> + return ret;
>>
>> host->max_clk = clock;
>>
>> @@ -247,10 +246,12 @@ static int arasan_sdhci_probe(struct udevice *dev)
>> host->mmc->dev = dev;
>> host->mmc->priv = host;
>>
>> - ret = sdhci_setup_cfg(&plat->cfg, host, plat->f_max,
>> + ret = sdhci_setup_cfg(&plat->cfg, host,
>> + CONFIG_ZYNQ_SDHCI_MAX_FREQ,
>> CONFIG_ZYNQ_SDHCI_MIN_FREQ);
>
> you have removed the parsing "max-frequency" from below code, because it's parsed from mmc_of_parse().
> But It's passing to CONFIG_ZYNQ_SDHCI_MAX_FREQ again. Is it correct?
> Could you check one more?
You are right. I have checked it myself.
This should be the right flow.
diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c
index f856f46ba6a2..5ce627bee985 100644
--- a/drivers/mmc/zynq_sdhci.c
+++ b/drivers/mmc/zynq_sdhci.c
@@ -386,6 +386,8 @@ static int arasan_sdhci_probe(struct udevice *dev)
host->quirks |= SDHCI_QUIRK_BROKEN_HISPD_MODE;
#endif
+ plat->cfg.f_max = CONFIG_ZYNQ_SDHCI_MAX_FREQ;
+
ret = mmc_of_parse(dev, &plat->cfg);
if (ret)
return ret;
@@ -396,8 +398,7 @@ static int arasan_sdhci_probe(struct udevice *dev)
host->mmc->dev = dev;
host->mmc->priv = host;
- ret = sdhci_setup_cfg(&plat->cfg, host,
- CONFIG_ZYNQ_SDHCI_MAX_FREQ,
+ ret = sdhci_setup_cfg(&plat->cfg, host, plat->cfg.f_max,
CONFIG_ZYNQ_SDHCI_MIN_FREQ);
if (ret)
return ret;
Interesting is that I have checked jz_mmc.c where f_max is rewritten
after mmc_of_parse.
Thanks,
Michal
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v3] mmc: zynq: parse dt when probing
2020-04-09 7:26 ` Michal Simek
@ 2020-04-11 9:03 ` Benedikt Grassl
2020-04-13 8:15 ` Michal Simek
0 siblings, 1 reply; 12+ messages in thread
From: Benedikt Grassl @ 2020-04-11 9:03 UTC (permalink / raw)
To: u-boot
Currently, the entry "bus-width = <8>" in the ZynqMP's sdhci nodes
is not evaluated. This results in the bus width staying at its default
value (4 bit in HS200 mode).
Fix this by calling mmc_of_parse. This function also checks for the
"no-1-8-v" and "max-frequency" entries. Remove the handling of those
nodes from this driver.
Signed-off-by: Benedikt Grassl <Benedikt.Grassl@rohde-schwarz.com>
---
drivers/mmc/sdhci.c | 3 +--
drivers/mmc/zynq_sdhci.c | 16 +++++++---------
include/sdhci.h | 1 -
3 files changed, 8 insertions(+), 12 deletions(-)
diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index 520c9f9feb..372dc0a820 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -839,8 +839,7 @@ 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) ||
- (host->quirks & SDHCI_QUIRK_NO_1_8_V))
+ if (!(cfg->voltages & MMC_VDD_165_195))
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 da3ff53da1..d0a43d9539 100644
--- a/drivers/mmc/zynq_sdhci.c
+++ b/drivers/mmc/zynq_sdhci.c
@@ -22,14 +22,12 @@ DECLARE_GLOBAL_DATA_PTR;
struct arasan_sdhci_plat {
struct mmc_config cfg;
struct mmc mmc;
- unsigned int f_max;
};
struct arasan_sdhci_priv {
struct sdhci_host *host;
u8 deviceid;
u8 bank;
- u8 no_1p8;
};
#if defined(CONFIG_ARCH_ZYNQMP)
@@ -238,8 +236,11 @@ 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);
+ if (ret)
+ return ret;
host->max_clk = clock;
@@ -247,10 +248,11 @@ static int arasan_sdhci_probe(struct udevice *dev)
host->mmc->dev = dev;
host->mmc->priv = host;
- ret = sdhci_setup_cfg(&plat->cfg, host, plat->f_max,
+ ret = sdhci_setup_cfg(&plat->cfg, host, plat->cfg.f_max,
CONFIG_ZYNQ_SDHCI_MIN_FREQ);
if (ret)
return ret;
+
upriv->mmc = host->mmc;
return sdhci_probe(dev);
@@ -258,7 +260,6 @@ static int arasan_sdhci_probe(struct udevice *dev)
static int arasan_sdhci_ofdata_to_platdata(struct udevice *dev)
{
- struct arasan_sdhci_plat *plat = dev_get_platdata(dev);
struct arasan_sdhci_priv *priv = dev_get_priv(dev);
priv->host = calloc(1, sizeof(struct sdhci_host));
@@ -277,10 +278,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");
- plat->f_max = dev_read_u32_default(dev, "max-frequency",
- CONFIG_ZYNQ_SDHCI_MAX_FREQ);
return 0;
}
diff --git a/include/sdhci.h b/include/sdhci.h
index aa4378fd57..0ef8c2ed62 100644
--- a/include/sdhci.h
+++ b/include/sdhci.h
@@ -243,7 +243,6 @@
#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;
--
2.17.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v3] mmc: zynq: parse dt when probing
2020-04-11 9:03 ` [PATCH v3] " Benedikt Grassl
@ 2020-04-13 8:15 ` Michal Simek
2020-04-13 21:47 ` Jaehoon Chung
0 siblings, 1 reply; 12+ messages in thread
From: Michal Simek @ 2020-04-13 8:15 UTC (permalink / raw)
To: u-boot
On 11. 04. 20 11:03, Benedikt Grassl wrote:
> Currently, the entry "bus-width = <8>" in the ZynqMP's sdhci nodes
> is not evaluated. This results in the bus width staying at its default
> value (4 bit in HS200 mode).
> Fix this by calling mmc_of_parse. This function also checks for the
> "no-1-8-v" and "max-frequency" entries. Remove the handling of those
> nodes from this driver.
>
> Signed-off-by: Benedikt Grassl <Benedikt.Grassl@rohde-schwarz.com>
> ---
> drivers/mmc/sdhci.c | 3 +--
> drivers/mmc/zynq_sdhci.c | 16 +++++++---------
> include/sdhci.h | 1 -
> 3 files changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> index 520c9f9feb..372dc0a820 100644
> --- a/drivers/mmc/sdhci.c
> +++ b/drivers/mmc/sdhci.c
> @@ -839,8 +839,7 @@ 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) ||
> - (host->quirks & SDHCI_QUIRK_NO_1_8_V))
> + if (!(cfg->voltages & MMC_VDD_165_195))
> 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 da3ff53da1..d0a43d9539 100644
> --- a/drivers/mmc/zynq_sdhci.c
> +++ b/drivers/mmc/zynq_sdhci.c
> @@ -22,14 +22,12 @@ DECLARE_GLOBAL_DATA_PTR;
> struct arasan_sdhci_plat {
> struct mmc_config cfg;
> struct mmc mmc;
> - unsigned int f_max;
> };
>
> struct arasan_sdhci_priv {
> struct sdhci_host *host;
> u8 deviceid;
> u8 bank;
> - u8 no_1p8;
> };
>
> #if defined(CONFIG_ARCH_ZYNQMP)
> @@ -238,8 +236,11 @@ 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);
> + if (ret)
> + return ret;
>
> host->max_clk = clock;
>
> @@ -247,10 +248,11 @@ static int arasan_sdhci_probe(struct udevice *dev)
> host->mmc->dev = dev;
> host->mmc->priv = host;
>
> - ret = sdhci_setup_cfg(&plat->cfg, host, plat->f_max,
> + ret = sdhci_setup_cfg(&plat->cfg, host, plat->cfg.f_max,
> CONFIG_ZYNQ_SDHCI_MIN_FREQ);
> if (ret)
> return ret;
> +
> upriv->mmc = host->mmc;
>
> return sdhci_probe(dev);
> @@ -258,7 +260,6 @@ static int arasan_sdhci_probe(struct udevice *dev)
>
> static int arasan_sdhci_ofdata_to_platdata(struct udevice *dev)
> {
> - struct arasan_sdhci_plat *plat = dev_get_platdata(dev);
> struct arasan_sdhci_priv *priv = dev_get_priv(dev);
>
> priv->host = calloc(1, sizeof(struct sdhci_host));
> @@ -277,10 +278,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");
>
> - plat->f_max = dev_read_u32_default(dev, "max-frequency",
> - CONFIG_ZYNQ_SDHCI_MAX_FREQ);
> return 0;
> }
>
> diff --git a/include/sdhci.h b/include/sdhci.h
> index aa4378fd57..0ef8c2ed62 100644
> --- a/include/sdhci.h
> +++ b/include/sdhci.h
> @@ -243,7 +243,6 @@
> #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;
>
Looks good. Will apply.
M
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH v3] mmc: zynq: parse dt when probing
2020-04-13 8:15 ` Michal Simek
@ 2020-04-13 21:47 ` Jaehoon Chung
2020-04-14 5:32 ` [PATCH v4] " Benedikt Grassl
0 siblings, 1 reply; 12+ messages in thread
From: Jaehoon Chung @ 2020-04-13 21:47 UTC (permalink / raw)
To: u-boot
On 4/13/20 5:15 PM, Michal Simek wrote:
> On 11. 04. 20 11:03, Benedikt Grassl wrote:
>> Currently, the entry "bus-width = <8>" in the ZynqMP's sdhci nodes
>> is not evaluated. This results in the bus width staying at its default
>> value (4 bit in HS200 mode).
>> Fix this by calling mmc_of_parse. This function also checks for the
>> "no-1-8-v" and "max-frequency" entries. Remove the handling of those
>> nodes from this driver.
>>
>> Signed-off-by: Benedikt Grassl <Benedikt.Grassl@rohde-schwarz.com>
Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>
Added just minor comment.
>> ---
>> drivers/mmc/sdhci.c | 3 +--
>> drivers/mmc/zynq_sdhci.c | 16 +++++++---------
>> include/sdhci.h | 1 -
>> 3 files changed, 8 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
>> index 520c9f9feb..372dc0a820 100644
>> --- a/drivers/mmc/sdhci.c
>> +++ b/drivers/mmc/sdhci.c
>> @@ -839,8 +839,7 @@ 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) ||
>> - (host->quirks & SDHCI_QUIRK_NO_1_8_V))
>> + if (!(cfg->voltages & MMC_VDD_165_195))
>> 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 da3ff53da1..d0a43d9539 100644
>> --- a/drivers/mmc/zynq_sdhci.c
>> +++ b/drivers/mmc/zynq_sdhci.c
>> @@ -22,14 +22,12 @@ DECLARE_GLOBAL_DATA_PTR;
>> struct arasan_sdhci_plat {
>> struct mmc_config cfg;
>> struct mmc mmc;
>> - unsigned int f_max;
>> };
>>
>> struct arasan_sdhci_priv {
>> struct sdhci_host *host;
>> u8 deviceid;
>> u8 bank;
>> - u8 no_1p8;
>> };
>>
>> #if defined(CONFIG_ARCH_ZYNQMP)
>> @@ -238,8 +236,11 @@ 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);
>> + if (ret)
>> + return ret;
>>
>> host->max_clk = clock;
>>
>> @@ -247,10 +248,11 @@ static int arasan_sdhci_probe(struct udevice *dev)
>> host->mmc->dev = dev;
>> host->mmc->priv = host;
>>
>> - ret = sdhci_setup_cfg(&plat->cfg, host, plat->f_max,
>> + ret = sdhci_setup_cfg(&plat->cfg, host, plat->cfg.f_max,
>> CONFIG_ZYNQ_SDHCI_MIN_FREQ);
>> if (ret)
>> return ret;
>> +
Unnecessary white space.
>> upriv->mmc = host->mmc;
>>
>> return sdhci_probe(dev);
>> @@ -258,7 +260,6 @@ static int arasan_sdhci_probe(struct udevice *dev)
>>
>> static int arasan_sdhci_ofdata_to_platdata(struct udevice *dev)
>> {
>> - struct arasan_sdhci_plat *plat = dev_get_platdata(dev);
>> struct arasan_sdhci_priv *priv = dev_get_priv(dev);
>>
>> priv->host = calloc(1, sizeof(struct sdhci_host));
>> @@ -277,10 +278,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");
>>
>> - plat->f_max = dev_read_u32_default(dev, "max-frequency",
>> - CONFIG_ZYNQ_SDHCI_MAX_FREQ);
>> return 0;
>> }
>>
>> diff --git a/include/sdhci.h b/include/sdhci.h
>> index aa4378fd57..0ef8c2ed62 100644
>> --- a/include/sdhci.h
>> +++ b/include/sdhci.h
>> @@ -243,7 +243,6 @@
>> #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;
>>
>
> Looks good. Will apply.
>
> M
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH v4] mmc: zynq: parse dt when probing
2020-04-13 21:47 ` Jaehoon Chung
@ 2020-04-14 5:32 ` Benedikt Grassl
2020-04-14 6:30 ` Michal Simek
2020-04-15 12:05 ` Benedikt Grassl
0 siblings, 2 replies; 12+ messages in thread
From: Benedikt Grassl @ 2020-04-14 5:32 UTC (permalink / raw)
To: u-boot
Currently, the entry "bus-width = <8>" in the ZynqMP's sdhci nodes
is not evaluated. This results in the bus width staying at its default
value (4 bit in HS200 mode).
Fix this by calling mmc_of_parse. This function also checks for the
"no-1-8-v" and "max-frequency" entries. Remove the handling of those
nodes from this driver.
Signed-off-by: Benedikt Grassl <Benedikt.Grassl@rohde-schwarz.com>
---
drivers/mmc/sdhci.c | 3 +--
drivers/mmc/zynq_sdhci.c | 15 ++++++---------
include/sdhci.h | 1 -
3 files changed, 7 insertions(+), 12 deletions(-)
diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
index 520c9f9feb..372dc0a820 100644
--- a/drivers/mmc/sdhci.c
+++ b/drivers/mmc/sdhci.c
@@ -839,8 +839,7 @@ 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) ||
- (host->quirks & SDHCI_QUIRK_NO_1_8_V))
+ if (!(cfg->voltages & MMC_VDD_165_195))
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 da3ff53da1..18925d01fa 100644
--- a/drivers/mmc/zynq_sdhci.c
+++ b/drivers/mmc/zynq_sdhci.c
@@ -22,14 +22,12 @@ DECLARE_GLOBAL_DATA_PTR;
struct arasan_sdhci_plat {
struct mmc_config cfg;
struct mmc mmc;
- unsigned int f_max;
};
struct arasan_sdhci_priv {
struct sdhci_host *host;
u8 deviceid;
u8 bank;
- u8 no_1p8;
};
#if defined(CONFIG_ARCH_ZYNQMP)
@@ -238,8 +236,11 @@ 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);
+ if (ret)
+ return ret;
host->max_clk = clock;
@@ -247,7 +248,7 @@ static int arasan_sdhci_probe(struct udevice *dev)
host->mmc->dev = dev;
host->mmc->priv = host;
- ret = sdhci_setup_cfg(&plat->cfg, host, plat->f_max,
+ ret = sdhci_setup_cfg(&plat->cfg, host, plat->cfg.f_max,
CONFIG_ZYNQ_SDHCI_MIN_FREQ);
if (ret)
return ret;
@@ -258,7 +259,6 @@ static int arasan_sdhci_probe(struct udevice *dev)
static int arasan_sdhci_ofdata_to_platdata(struct udevice *dev)
{
- struct arasan_sdhci_plat *plat = dev_get_platdata(dev);
struct arasan_sdhci_priv *priv = dev_get_priv(dev);
priv->host = calloc(1, sizeof(struct sdhci_host));
@@ -277,10 +277,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");
- plat->f_max = dev_read_u32_default(dev, "max-frequency",
- CONFIG_ZYNQ_SDHCI_MAX_FREQ);
return 0;
}
diff --git a/include/sdhci.h b/include/sdhci.h
index aa4378fd57..0ef8c2ed62 100644
--- a/include/sdhci.h
+++ b/include/sdhci.h
@@ -243,7 +243,6 @@
#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;
--
2.17.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v4] mmc: zynq: parse dt when probing
2020-04-14 5:32 ` [PATCH v4] " Benedikt Grassl
@ 2020-04-14 6:30 ` Michal Simek
2020-04-15 12:05 ` Benedikt Grassl
1 sibling, 0 replies; 12+ messages in thread
From: Michal Simek @ 2020-04-14 6:30 UTC (permalink / raw)
To: u-boot
On 14. 04. 20 7:32, Benedikt Grassl wrote:
> Currently, the entry "bus-width = <8>" in the ZynqMP's sdhci nodes
> is not evaluated. This results in the bus width staying at its default
> value (4 bit in HS200 mode).
> Fix this by calling mmc_of_parse. This function also checks for the
> "no-1-8-v" and "max-frequency" entries. Remove the handling of those
> nodes from this driver.
>
> Signed-off-by: Benedikt Grassl <Benedikt.Grassl@rohde-schwarz.com>
> ---
> drivers/mmc/sdhci.c | 3 +--
> drivers/mmc/zynq_sdhci.c | 15 ++++++---------
> include/sdhci.h | 1 -
> 3 files changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
> index 520c9f9feb..372dc0a820 100644
> --- a/drivers/mmc/sdhci.c
> +++ b/drivers/mmc/sdhci.c
> @@ -839,8 +839,7 @@ 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) ||
> - (host->quirks & SDHCI_QUIRK_NO_1_8_V))
> + if (!(cfg->voltages & MMC_VDD_165_195))
> 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 da3ff53da1..18925d01fa 100644
> --- a/drivers/mmc/zynq_sdhci.c
> +++ b/drivers/mmc/zynq_sdhci.c
> @@ -22,14 +22,12 @@ DECLARE_GLOBAL_DATA_PTR;
> struct arasan_sdhci_plat {
> struct mmc_config cfg;
> struct mmc mmc;
> - unsigned int f_max;
> };
>
> struct arasan_sdhci_priv {
> struct sdhci_host *host;
> u8 deviceid;
> u8 bank;
> - u8 no_1p8;
> };
>
> #if defined(CONFIG_ARCH_ZYNQMP)
> @@ -238,8 +236,11 @@ 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);
> + if (ret)
> + return ret;
>
> host->max_clk = clock;
>
> @@ -247,7 +248,7 @@ static int arasan_sdhci_probe(struct udevice *dev)
> host->mmc->dev = dev;
> host->mmc->priv = host;
>
> - ret = sdhci_setup_cfg(&plat->cfg, host, plat->f_max,
> + ret = sdhci_setup_cfg(&plat->cfg, host, plat->cfg.f_max,
> CONFIG_ZYNQ_SDHCI_MIN_FREQ);
> if (ret)
> return ret;
> @@ -258,7 +259,6 @@ static int arasan_sdhci_probe(struct udevice *dev)
>
> static int arasan_sdhci_ofdata_to_platdata(struct udevice *dev)
> {
> - struct arasan_sdhci_plat *plat = dev_get_platdata(dev);
> struct arasan_sdhci_priv *priv = dev_get_priv(dev);
>
> priv->host = calloc(1, sizeof(struct sdhci_host));
> @@ -277,10 +277,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");
>
> - plat->f_max = dev_read_u32_default(dev, "max-frequency",
> - CONFIG_ZYNQ_SDHCI_MAX_FREQ);
> return 0;
> }
>
> diff --git a/include/sdhci.h b/include/sdhci.h
> index aa4378fd57..0ef8c2ed62 100644
> --- a/include/sdhci.h
> +++ b/include/sdhci.h
> @@ -243,7 +243,6 @@
> #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;
>
Applied.
M
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH v4] mmc: zynq: parse dt when probing
2020-04-14 5:32 ` [PATCH v4] " Benedikt Grassl
2020-04-14 6:30 ` Michal Simek
@ 2020-04-15 12:05 ` Benedikt Grassl
2020-04-15 14:19 ` Michal Simek
1 sibling, 1 reply; 12+ messages in thread
From: Benedikt Grassl @ 2020-04-15 12:05 UTC (permalink / raw)
To: u-boot
Hi Michal,
I just tried to check if the read performance acutally increased when
using an 8-bit data bus. As a quick test, I use the fatload command to
read a chunk of 1 GByte from an eMMC flash (generated with dd from
Linux). However, I don't see any difference at all:
mmc info
Device: mmc at ff160000
Manufacturer ID: 9d
OEM: 101
Name: IS016
Bus Speed: 200000000
Mode: HS200 (200MHz)
Rd Block Len: 512
MMC version 5.0
High Capacity: Yes
Capacity: 14.6 GiB
Bus Width: 4-bit
(...)
fatload mmc 0 ${loadimage} test.img
1070579712 bytes read in 70458 ms (14.5 MiB/s)
With an 8-bit data bus:
mmcinfo
Device: mmc at ff160000
Manufacturer ID: 9d
OEM: 101
Name: IS016
Bus Speed: 200000000
Mode: HS200 (200MHz)
Rd Block Len: 512
MMC version 5.0
High Capacity: Yes
Capacity: 14.6 GiB
Bus Width: 8-bit
(...)
fatload mmc 0 ${loadimage} test.img
1070579712 bytes read in 70551 ms (14.5 MiB/s)
I wonder if the fatload command is the bottleneck here? Unfortunately
I'm working from home right now and don't have an oscilloscope at hand
to verify in hardware. Do you have any inputs on that?
Thanks,
Benedikt
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH v4] mmc: zynq: parse dt when probing
2020-04-15 12:05 ` Benedikt Grassl
@ 2020-04-15 14:19 ` Michal Simek
0 siblings, 0 replies; 12+ messages in thread
From: Michal Simek @ 2020-04-15 14:19 UTC (permalink / raw)
To: u-boot
Hi,
On 15. 04. 20 14:05, Benedikt Grassl wrote:
> Hi Michal,
>
> I just tried to check if the read performance acutally increased when
> using an 8-bit data bus. As a quick test, I use the fatload command to
> read a chunk of 1 GByte from an eMMC flash (generated with dd from
> Linux). However, I don't see any difference at all:
>
> mmc info
> Device: mmc at ff160000
> Manufacturer ID: 9d
> OEM: 101
> Name: IS016
> Bus Speed: 200000000
> Mode: HS200 (200MHz)
> Rd Block Len: 512
> MMC version 5.0
> High Capacity: Yes
> Capacity: 14.6 GiB
> Bus Width: 4-bit
> (...)
>
> fatload mmc 0 ${loadimage} test.img
> 1070579712 bytes read in 70458 ms (14.5 MiB/s)
>
> With an 8-bit data bus:
>
> mmcinfo
> Device: mmc at ff160000
> Manufacturer ID: 9d
> OEM: 101
> Name: IS016
> Bus Speed: 200000000
> Mode: HS200 (200MHz)
> Rd Block Len: 512
> MMC version 5.0
> High Capacity: Yes
> Capacity: 14.6 GiB
> Bus Width: 8-bit
> (...)
>
> fatload mmc 0 ${loadimage} test.img
> 1070579712 bytes read in 70551 ms (14.5 MiB/s)
>
> I wonder if the fatload command is the bottleneck here? Unfortunately
> I'm working from home right now and don't have an oscilloscope at hand
> to verify in hardware. Do you have any inputs on that?
you can do raw write to avoid fat FS which is not optimal.
Thanks,
Michal
^ permalink raw reply [flat|nested] 12+ messages in thread