diff mbox series

ARM: dts: imx8mq-evk: Remove u-boot,off-on-delay-us property

Message ID 20210222174047.464227-1-festevam@gmail.com
State Accepted
Commit 5d42ea5535348c1e65abedfdf1241471f66d002b
Delegated to: Stefano Babic
Headers show
Series ARM: dts: imx8mq-evk: Remove u-boot,off-on-delay-us property | expand

Commit Message

Fabio Estevam Feb. 22, 2021, 5:40 p.m. UTC
Commit 247bbeb74c18 ("ARM: dts: imx8m: increase off-on delay on the SD Vcc
regulator") caused the imx8mq-evk board to not be able to store the
environment variables in the SD card.

Remove the u-boot,off-on-delay-us property to fix the regression.

Signed-off-by: Fabio Estevam <festevam@gmail.com>
---
 arch/arm/dts/imx8mq-evk-u-boot.dtsi | 4 ----
 1 file changed, 4 deletions(-)

Comments

ZHIZHIKIN Andrey Feb. 22, 2021, 6:21 p.m. UTC | #1
Hello Fabio,

Thanks for taking care of this!

I believe this fix still applies to mx8m mini series, as without it - some SD Cards are failed to be recognized.

> -----Original Message-----
> From: Fabio Estevam <festevam@gmail.com>
> Sent: Monday, February 22, 2021 6:41 PM
> To: sbabic@denx.de
> Cc: u-boot@lists.denx.de; trini <trini@konsulko.com>; lorusak@gmail.com;
> ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com>;
> peng.fan@nxp.com; Fabio Estevam <festevam@gmail.com>
> Subject: [PATCH] ARM: dts: imx8mq-evk: Remove u-boot,off-on-delay-us
> property
> 
> 
> Commit 247bbeb74c18 ("ARM: dts: imx8m: increase off-on delay on the SD Vcc
> regulator") caused the imx8mq-evk board to not be able to store the
> environment variables in the SD card.
> 
> Remove the u-boot,off-on-delay-us property to fix the regression.
> 
> Signed-off-by: Fabio Estevam <festevam@gmail.com>
> ---
>  arch/arm/dts/imx8mq-evk-u-boot.dtsi | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/arch/arm/dts/imx8mq-evk-u-boot.dtsi b/arch/arm/dts/imx8mq-evk-
> u-boot.dtsi
> index 44af66372712..2cfc12b7e0a4 100644
> --- a/arch/arm/dts/imx8mq-evk-u-boot.dtsi
> +++ b/arch/arm/dts/imx8mq-evk-u-boot.dtsi
> @@ -1,9 +1,5 @@
>  // SPDX-License-Identifier: (GPL-2.0 OR MIT)
> 
> -&reg_usdhc2_vmmc {
> -       u-boot,off-on-delay-us = <20000>;
> -};
> -
>  &usdhc1 {
>         mmc-hs400-1_8v;
>  };
> --
> 2.25.1

Reviewed-by: Andrey Zhizhikin <andrey.zhizhikin@leica-geosystems.com>
Peng Fan Feb. 23, 2021, 1:53 a.m. UTC | #2
Hi Fabio,

> Subject: [PATCH] ARM: dts: imx8mq-evk: Remove u-boot,off-on-delay-us
> property
> 
> Commit 247bbeb74c18 ("ARM: dts: imx8m: increase off-on delay on the SD
> Vcc
> regulator") caused the imx8mq-evk board to not be able to store the
> environment variables in the SD card.
> 
> Remove the u-boot,off-on-delay-us property to fix the regression.

It is board dependent, some board may has a big capacitance,
the off-on-delay is to give enough time for the capacitance
to leak its voltage.

Regards,
Peng. 

> 
> Signed-off-by: Fabio Estevam <festevam@gmail.com>
> ---
>  arch/arm/dts/imx8mq-evk-u-boot.dtsi | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/arch/arm/dts/imx8mq-evk-u-boot.dtsi
> b/arch/arm/dts/imx8mq-evk-u-boot.dtsi
> index 44af66372712..2cfc12b7e0a4 100644
> --- a/arch/arm/dts/imx8mq-evk-u-boot.dtsi
> +++ b/arch/arm/dts/imx8mq-evk-u-boot.dtsi
> @@ -1,9 +1,5 @@
>  // SPDX-License-Identifier: (GPL-2.0 OR MIT)
> 
> -&reg_usdhc2_vmmc {
> -	u-boot,off-on-delay-us = <20000>;
> -};
> -
>  &usdhc1 {
>  	mmc-hs400-1_8v;
>  };
> --
> 2.25.1
Fabio Estevam Feb. 23, 2021, 2:01 a.m. UTC | #3
Hi Peng,

On Mon, Feb 22, 2021 at 10:53 PM Peng Fan <peng.fan@nxp.com> wrote:

> It is board dependent, some board may has a big capacitance,
> the off-on-delay is to give enough time for the capacitance
> to leak its voltage.

Ok, but not clear from your comment if you agree with my change or not.

Please clarify.
Peng Fan Feb. 23, 2021, 2:04 a.m. UTC | #4
> Subject: Re: [PATCH] ARM: dts: imx8mq-evk: Remove u-boot,off-on-delay-us
> property
> 
> Hi Peng,
> 
> On Mon, Feb 22, 2021 at 10:53 PM Peng Fan <peng.fan@nxp.com> wrote:
> 
> > It is board dependent, some board may has a big capacitance, the
> > off-on-delay is to give enough time for the capacitance to leak its
> > voltage.
> 
> Ok, but not clear from your comment if you agree with my change or not.

I think remove it is not correct, the exact time should be measured using
oscilloscope, removing it might make sd3.0 not work. But anyway 20000 seems
too large a value.

Regards,
Peng.

> 
> Please clarify.
Fabio Estevam Feb. 23, 2021, 2:11 a.m. UTC | #5
On Mon, Feb 22, 2021 at 11:04 PM Peng Fan <peng.fan@nxp.com> wrote:

> I think remove it is not correct, the exact time should be measured using
> oscilloscope, removing it might make sd3.0 not work. But anyway 20000 seems
> too large a value.

Do you plan to submit a fix for this then? If you don't have the
bandwidth for working on this, then we could go with my proposal.

I would like to be able to boot a kernel from the SD card in the
2021.04 release and fix the regression.

Also, why don't we have such property in the imx8mq-evk.dts in the kernel?

I don't think U-Boot dts should deviate from the Linux devicetree in
this aspect.
Peng Fan (OSS) Feb. 23, 2021, 2:14 a.m. UTC | #6
> -----Original Message-----
> From: Fabio Estevam [mailto:festevam@gmail.com]
> Sent: 2021年2月23日 10:11
> To: Peng Fan <peng.fan@nxp.com>
> Cc: sbabic@denx.de; u-boot@lists.denx.de; trini@konsulko.com;
> lorusak@gmail.com; andrey.zhizhikin@leica-geosystems.com
> Subject: Re: [PATCH] ARM: dts: imx8mq-evk: Remove u-boot,off-on-delay-us
> property
> 
> On Mon, Feb 22, 2021 at 11:04 PM Peng Fan <peng.fan@nxp.com> wrote:
> 
> > I think remove it is not correct, the exact time should be measured
> > using oscilloscope, removing it might make sd3.0 not work. But anyway
> > 20000 seems too large a value.
> 
> Do you plan to submit a fix for this then? If you don't have the bandwidth for
> working on this, then we could go with my proposal.
> 
> I would like to be able to boot a kernel from the SD card in the
> 2021.04 release and fix the regression.
> 
> Also, why don't we have such property in the imx8mq-evk.dts in the kernel?
> 
> I don't think U-Boot dts should deviate from the Linux devicetree in this
> aspect.

See
https://source.codeaurora.org/external/imx/linux-imx/tree/arch/arm64/boot/dts/freescale/imx8mq-evk.dts?h=imx_5.4.70_2.3.0#n100

NXP vendor tree has off-on-delay-us.

Regards
Peng.
Fabio Estevam Feb. 23, 2021, 11:03 a.m. UTC | #7
On Mon, Feb 22, 2021 at 11:14 PM Peng Fan (OSS) <peng.fan@oss.nxp.com> wrote:

> See
> https://source.codeaurora.org/external/imx/linux-imx/tree/arch/arm64/boot/dts/freescale/imx8mq-evk.dts?h=imx_5.4.70_2.3.0#n100
>
> NXP vendor tree has off-on-delay-us.

Ok, but are you able to determine the appropriate off-on-delay-us for
imx8mq-evk in U-Boot to avoid the existing regression?
Stefano Babic March 1, 2021, 11:54 a.m. UTC | #8
> Commit 247bbeb74c18 ("ARM: dts: imx8m: increase off-on delay on the SD Vcc
> regulator") caused the imx8mq-evk board to not be able to store the
> environment variables in the SD card.
> Remove the u-boot,off-on-delay-us property to fix the regression.
> Signed-off-by: Fabio Estevam <festevam@gmail.com>
> Reviewed-by: Andrey Zhizhikin <andrey.zhizhikin@leica-geosystems.com>
Applied to u-boot-imx, master, thanks !

Best regards,
Stefano Babic
diff mbox series

Patch

diff --git a/arch/arm/dts/imx8mq-evk-u-boot.dtsi b/arch/arm/dts/imx8mq-evk-u-boot.dtsi
index 44af66372712..2cfc12b7e0a4 100644
--- a/arch/arm/dts/imx8mq-evk-u-boot.dtsi
+++ b/arch/arm/dts/imx8mq-evk-u-boot.dtsi
@@ -1,9 +1,5 @@ 
 // SPDX-License-Identifier: (GPL-2.0 OR MIT)
 
-&reg_usdhc2_vmmc {
-	u-boot,off-on-delay-us = <20000>;
-};
-
 &usdhc1 {
 	mmc-hs400-1_8v;
 };