diff mbox series

[V2] mmc: zynq: parse dt when probing

Message ID 20200407141532.20731-1-Benedikt.Grassl@rohde-schwarz.com
State New
Headers show
Series [V2] mmc: zynq: parse dt when probing | expand

Commit Message

Benedikt Grassl April 7, 2020, 2:15 p.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

Benedikt Grassl April 7, 2020, 2:21 p.m. UTC | #1
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
Michal Simek April 8, 2020, 6:50 a.m. UTC | #2
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 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..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
Jaehoon Chung April 8, 2020, 10:11 p.m. UTC | #3
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 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..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;
>
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..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;