[PATCH V2] mmc: zynq: parse dt when probing

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;

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

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

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;

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

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;

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

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

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;

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

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@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@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

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@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@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
participants (3)
-
Benedikt Grassl
-
Jaehoon Chung
-
Michal Simek