All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] mmc: fsl_esdhc_imx: remove redundant cmd11 related code.
@ 2021-03-22 10:55 haibo.chen at nxp.com
  2021-03-22 10:55 ` [PATCH 2/2] mmc: fsl_esdhc_imx: add extra delay for IO voltage switch if necessary haibo.chen at nxp.com
  2021-04-09 11:24 ` [PATCH 1/2] mmc: fsl_esdhc_imx: remove redundant cmd11 related code sbabic at denx.de
  0 siblings, 2 replies; 6+ messages in thread
From: haibo.chen at nxp.com @ 2021-03-22 10:55 UTC (permalink / raw)
  To: u-boot

From: Haibo Chen <haibo.chen@nxp.com>

Common code already handle the voltage switch sequence based on spec,
so remove the redundant voltage switch code.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/mmc/fsl_esdhc_imx.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
index 637537d262..722f33c68c 100644
--- a/drivers/mmc/fsl_esdhc_imx.c
+++ b/drivers/mmc/fsl_esdhc_imx.c
@@ -521,15 +521,6 @@ static int esdhc_send_cmd_common(struct fsl_esdhc_priv *priv, struct mmc *mmc,
 		goto out;
 	}
 
-	/* Switch voltage to 1.8V if CMD11 succeeded */
-	if (cmd->cmdidx == SD_CMD_SWITCH_UHS18V) {
-		esdhc_setbits32(&regs->vendorspec, ESDHC_VENDORSPEC_VSELECT);
-
-		printf("Run CMD11 1.8V switch\n");
-		/* Sleep for 5 ms - max time for card to switch to 1.8V */
-		udelay(5000);
-	}
-
 	/* Workaround for ESDHC errata ENGcm03648 */
 	if (!data && (cmd->resp_type & MMC_RSP_BUSY)) {
 		int timeout = 50000;
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] mmc: fsl_esdhc_imx: add extra delay for IO voltage switch if necessary
  2021-03-22 10:55 [PATCH 1/2] mmc: fsl_esdhc_imx: remove redundant cmd11 related code haibo.chen at nxp.com
@ 2021-03-22 10:55 ` haibo.chen at nxp.com
  2021-03-22 12:04   ` Fabio Estevam
  2021-04-09 11:24   ` sbabic at denx.de
  2021-04-09 11:24 ` [PATCH 1/2] mmc: fsl_esdhc_imx: remove redundant cmd11 related code sbabic at denx.de
  1 sibling, 2 replies; 6+ messages in thread
From: haibo.chen at nxp.com @ 2021-03-22 10:55 UTC (permalink / raw)
  To: u-boot

From: Haibo Chen <haibo.chen@nxp.com>

Some board like imx8mm-evkb, IO voltage switch from 3.3v to 1.8v need
around 18ms, common code only delay 10ms, so need to delay extra 8ms.
Otherwise voltage switch will timeout when wait for data0 line.

This IO voltage switch time depends on board design, depend on the
PMIC and capacitance. imx8mm-evkb board use PCA9450(PMIC) and 10uF
capacitance.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 arch/arm/dts/imx8mm-evk-u-boot.dtsi |  1 +
 drivers/mmc/fsl_esdhc_imx.c         | 12 ++++++++++++
 2 files changed, 13 insertions(+)

diff --git a/arch/arm/dts/imx8mm-evk-u-boot.dtsi b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
index e843a5648e..bccf4fe947 100644
--- a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
+++ b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
@@ -102,6 +102,7 @@
 	u-boot,dm-spl;
 	sd-uhs-sdr104;
 	sd-uhs-ddr50;
+	fsl,signal-voltage-switch-extra-delay-ms = <8>;
 };
 
 &usdhc3 {
diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
index 722f33c68c..ef2b71e6b7 100644
--- a/drivers/mmc/fsl_esdhc_imx.c
+++ b/drivers/mmc/fsl_esdhc_imx.c
@@ -146,6 +146,7 @@ struct esdhc_soc_data {
  * @start_tuning_tap: the start point for tuning in tuning_ctrl register
  * @strobe_dll_delay_target: settings in strobe_dllctrl
  * @signal_voltage: indicating the current voltage
+ * @signal_voltage_switch_extra_delay_ms: extra delay for IO voltage switch
  * @cd_gpio: gpio for card detection
  * @wp_gpio: gpio for write protection
  */
@@ -170,6 +171,7 @@ struct fsl_esdhc_priv {
 	u32 tuning_start_tap;
 	u32 strobe_dll_delay_target;
 	u32 signal_voltage;
+	u32 signal_voltage_switch_extra_delay_ms;
 #if CONFIG_IS_ENABLED(DM_REGULATOR)
 	struct udevice *vqmmc_dev;
 	struct udevice *vmmc_dev;
@@ -836,6 +838,14 @@ static int esdhc_set_voltage(struct mmc *mmc)
 		}
 #endif
 		esdhc_setbits32(&regs->vendorspec, ESDHC_VENDORSPEC_VSELECT);
+		/*
+		 * some board like imx8mm-evk need about 18ms to switch
+		 * the IO voltage from 3.3v to 1.8v, common code only
+		 * delay 10ms, so need to delay extra time to make sure
+		 * the IO voltage change to 1.8v.
+		 */
+		if (priv->signal_voltage_switch_extra_delay_ms)
+			mdelay(priv->signal_voltage_switch_extra_delay_ms);
 		if (esdhc_read32(&regs->vendorspec) & ESDHC_VENDORSPEC_VSELECT)
 			return 0;
 
@@ -1450,6 +1460,8 @@ static int fsl_esdhc_of_to_plat(struct udevice *dev)
 	val = fdtdec_get_int(fdt, node, "fsl,strobe-dll-delay-target",
 			     ESDHC_STROBE_DLL_CTRL_SLV_DLY_TARGET_DEFAULT);
 	priv->strobe_dll_delay_target = val;
+	val = fdtdec_get_int(fdt, node, "fsl,signal-voltage-switch-extra-delay-ms", 0);
+	priv->signal_voltage_switch_extra_delay_ms = val;
 
 	if (dev_read_bool(dev, "broken-cd"))
 		priv->broken_cd = 1;
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] mmc: fsl_esdhc_imx: add extra delay for IO voltage switch if necessary
  2021-03-22 10:55 ` [PATCH 2/2] mmc: fsl_esdhc_imx: add extra delay for IO voltage switch if necessary haibo.chen at nxp.com
@ 2021-03-22 12:04   ` Fabio Estevam
  2021-03-22 12:48     ` Sean Anderson
  2021-04-09 11:24   ` sbabic at denx.de
  1 sibling, 1 reply; 6+ messages in thread
From: Fabio Estevam @ 2021-03-22 12:04 UTC (permalink / raw)
  To: u-boot

Hi Haibo,

On Mon, Mar 22, 2021 at 8:09 AM <haibo.chen@nxp.com> wrote:

> --- a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> +++ b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> @@ -102,6 +102,7 @@
>         u-boot,dm-spl;
>         sd-uhs-sdr104;
>         sd-uhs-ddr50;
> +       fsl,signal-voltage-switch-extra-delay-ms = <8>;

How does the Linux kernel deal with that?

I don't see such property in the mainline kernel or in the NXP tree.

I am concerned that we add a U-Boot specific here. Also, the fact that
common code waits 10ms is a driver implementation detail, so not happy
that such information needs to be passed in the U-Boot dts.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] mmc: fsl_esdhc_imx: add extra delay for IO voltage switch if necessary
  2021-03-22 12:04   ` Fabio Estevam
@ 2021-03-22 12:48     ` Sean Anderson
  0 siblings, 0 replies; 6+ messages in thread
From: Sean Anderson @ 2021-03-22 12:48 UTC (permalink / raw)
  To: u-boot


On 3/22/21 8:04 AM, Fabio Estevam wrote:
> Hi Haibo,
> 
> On Mon, Mar 22, 2021 at 8:09 AM <haibo.chen@nxp.com> wrote:
> 
>> --- a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
>> +++ b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
>> @@ -102,6 +102,7 @@
>>          u-boot,dm-spl;
>>          sd-uhs-sdr104;
>>          sd-uhs-ddr50;
>> +       fsl,signal-voltage-switch-extra-delay-ms = <8>;
> 
> How does the Linux kernel deal with that?
> 
> I don't see such property in the mainline kernel or in the NXP tree.
> 
> I am concerned that we add a U-Boot specific here. Also, the fact that
> common code waits 10ms is a driver implementation detail, so not happy
> that such information needs to be passed in the U-Boot dts.
> 

Typically they wait in the regulator. E.g. regulator-ramp-delay or
regulator-settling-time-us. There should be no need to wait in the MMC
driver.

--Sean

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] mmc: fsl_esdhc_imx: remove redundant cmd11 related code.
  2021-03-22 10:55 [PATCH 1/2] mmc: fsl_esdhc_imx: remove redundant cmd11 related code haibo.chen at nxp.com
  2021-03-22 10:55 ` [PATCH 2/2] mmc: fsl_esdhc_imx: add extra delay for IO voltage switch if necessary haibo.chen at nxp.com
@ 2021-04-09 11:24 ` sbabic at denx.de
  1 sibling, 0 replies; 6+ messages in thread
From: sbabic at denx.de @ 2021-04-09 11:24 UTC (permalink / raw)
  To: u-boot

> From: Haibo Chen <haibo.chen@nxp.com>
> Common code already handle the voltage switch sequence based on spec,
> so remove the redundant voltage switch code.
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
Applied to u-boot-imx, master, thanks !

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] mmc: fsl_esdhc_imx: add extra delay for IO voltage switch if necessary
  2021-03-22 10:55 ` [PATCH 2/2] mmc: fsl_esdhc_imx: add extra delay for IO voltage switch if necessary haibo.chen at nxp.com
  2021-03-22 12:04   ` Fabio Estevam
@ 2021-04-09 11:24   ` sbabic at denx.de
  1 sibling, 0 replies; 6+ messages in thread
From: sbabic at denx.de @ 2021-04-09 11:24 UTC (permalink / raw)
  To: u-boot

> From: Haibo Chen <haibo.chen@nxp.com>
> Some board like imx8mm-evkb, IO voltage switch from 3.3v to 1.8v need
> around 18ms, common code only delay 10ms, so need to delay extra 8ms.
> Otherwise voltage switch will timeout when wait for data0 line.
> This IO voltage switch time depends on board design, depend on the
> PMIC and capacitance. imx8mm-evkb board use PCA9450(PMIC) and 10uF
> capacitance.
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
Applied to u-boot-imx, master, thanks !

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-04-09 11:24 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-22 10:55 [PATCH 1/2] mmc: fsl_esdhc_imx: remove redundant cmd11 related code haibo.chen at nxp.com
2021-03-22 10:55 ` [PATCH 2/2] mmc: fsl_esdhc_imx: add extra delay for IO voltage switch if necessary haibo.chen at nxp.com
2021-03-22 12:04   ` Fabio Estevam
2021-03-22 12:48     ` Sean Anderson
2021-04-09 11:24   ` sbabic at denx.de
2021-04-09 11:24 ` [PATCH 1/2] mmc: fsl_esdhc_imx: remove redundant cmd11 related code sbabic at denx.de

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.