diff mbox series

[v4] mmc: zynq: parse dt when probing

Message ID 20200414053212.5588-1-Benedikt.Grassl@rohde-schwarz.com
State Accepted
Commit 942b5fc03218d1c94468fc658e7dec65dabcc830
Headers show
Series [v4] mmc: zynq: parse dt when probing | expand

Commit Message

Benedikt Grassl April 14, 2020, 5:32 a.m. UTC
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 at 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(-)

Comments

Michal Simek April 14, 2020, 6:30 a.m. UTC | #1
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 at 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
Benedikt Grassl April 15, 2020, 12:05 p.m. UTC | #2
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
Michal Simek April 15, 2020, 2:19 p.m. UTC | #3
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
diff mbox series

Patch

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;