[PATCH 1/2] ARM: dts: imx8m: increase off-on delay on the SD Vcc regulator

[Peter Robinson]

On Wed, Dec 2, 2020 at 10:08 AM ZHIZHIKIN Andrey
<andrey.zhizhikin at leica-geosystems.com> wrote:
>
> Hello Peter,
>
> > -----Original Message-----
> > From: Peter Robinson <pbrobinson at gmail.com>
> > Sent: Wednesday, December 2, 2020 9:45 AM
> > To: ZHIZHIKIN Andrey <andrey.zhizhikin at leica-geosystems.com>
> > Cc: u-boot at lists.denx.de; sbabic at denx.de
> > Subject: Re: [PATCH 1/2] ARM: dts: imx8m: increase off-on delay on the SD Vcc
> > regulator
> >
> >
> > On Tue, Dec 1, 2020 at 1:31 PM Andrey Zhizhikin <andrey.zhizhikin at leica-
> > geosystems.com> wrote:
> > >
> > > Some SD Card controller and power circuitry has increased capacitance,
> > > which keeps the internal logic remains powered after regulator is
> > > switch off. This is generally the case when card is switched to SD104
> > > mode, where a power cycle should be performed. In case if the card
> > > internal logic remains powered, it causes a subsequent failure of mode
> > > transition, effectively leading to failed enumeration.
> > >
> > > Introduce a delay of 20 msec in order to provide a possibility for
> > > internal card circuitry to drain voltages and perform a power cycle
> > > correctly.
> > >
> > > Similar fix is done in commit c49d0ac38a76 ("ARM: dts: rmobile:
> > > Increase off-on delay on the SD Vcc regulator") targeted Renesas SOCs.
> > >
> > > Signed-off-by: Andrey Zhizhikin
> > > <andrey.zhizhikin at leica-geosystems.com>
> > > Cc: Stefano Babic <sbabic at denx.de>
> > > ---
> > >  arch/arm/dts/imx8mm-beacon-kit-u-boot.dtsi | 4 ++++
> > >  arch/arm/dts/imx8mm-evk-u-boot.dtsi        | 4 ++++
> > >  arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi   | 4 ++++
> > >  arch/arm/dts/imx8mp-evk-u-boot.dtsi        | 4 ++++
> > >  arch/arm/dts/imx8mq-evk.dts                | 1 +
> > >  arch/arm/dts/imx8mq-phanbell.dts           | 1 +
> > >  6 files changed, 18 insertions(+)
> > >
> > > diff --git a/arch/arm/dts/imx8mm-beacon-kit-u-boot.dtsi
> > > b/arch/arm/dts/imx8mm-beacon-kit-u-boot.dtsi
> > > index fc1aebb2fe..6d80a529ae 100644
> > > --- a/arch/arm/dts/imx8mm-beacon-kit-u-boot.dtsi
> > > +++ b/arch/arm/dts/imx8mm-beacon-kit-u-boot.dtsi
> > > @@ -37,6 +37,10 @@
> > >         /delete-property/ assigned-clock-rates;  };
> > >
> > > +&reg_usdhc2_vmmc {
> > > +       u-boot,off-on-delay-us = <20000>; };
> > > +
> > >  &fec1 {
> > >         phy-reset-gpios = <&gpio4 22 GPIO_ACTIVE_LOW>;  }; diff --git
> > > a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> > > b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> > > index b5c12105a9..9f77d3c6ff 100644
> > > --- a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> > > +++ b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> > > @@ -46,6 +46,10 @@
> > >         u-boot,dm-spl;
> > >  };
> > >
> > > +&reg_usdhc2_vmmc {
> > > +       u-boot,off-on-delay-us = <20000>; };
> > > +
> > >  &pinctrl_reg_usdhc2_vmmc {
> > >         u-boot,dm-spl;
> > >  };
> > > diff --git a/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi
> > > b/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi
> > > index 4419679d4c..98b0b9891b 100644
> > > --- a/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi
> > > +++ b/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi
> > > @@ -47,6 +47,10 @@
> > >         u-boot,dm-spl;
> > >  };
> > >
> > > +&reg_usdhc2_vmmc {
> > > +       u-boot,off-on-delay-us = <20000>; };
> > > +
> > >  &pinctrl_uart2 {
> > >         u-boot,dm-spl;
> > >  };
> > > diff --git a/arch/arm/dts/imx8mp-evk-u-boot.dtsi
> > > b/arch/arm/dts/imx8mp-evk-u-boot.dtsi
> > > index 24a93ac2d6..2452e9175c 100644
> > > --- a/arch/arm/dts/imx8mp-evk-u-boot.dtsi
> > > +++ b/arch/arm/dts/imx8mp-evk-u-boot.dtsi
> > > @@ -48,6 +48,10 @@
> > >         u-boot,dm-spl;
> > >  };
> > >
> > > +&reg_usdhc2_vmmc {
> > > +       u-boot,off-on-delay-us = <20000>; };
> > > +
> > >  &reg_usdhc2_vmmc {
> > >         u-boot,dm-spl;
> > >  };
> > > diff --git a/arch/arm/dts/imx8mq-evk.dts b/arch/arm/dts/imx8mq-evk.dts
> > > index 55294ba9c8..9663683f69 100644
> > > --- a/arch/arm/dts/imx8mq-evk.dts
> > > +++ b/arch/arm/dts/imx8mq-evk.dts
> > > @@ -39,6 +39,7 @@
> > >                 regulator-max-microvolt = <3300000>;
> > >                 gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>;
> > >                 enable-active-high;
> > > +               u-boot,off-on-delay-us = <20000>;
> >
> > These should be going in the -u-boot.dtsi like the devices above. Why the two
> > different ways of doing it?
>
> imx8mq-evk-u-boot.dtsi is not provided in the source tree. Do you suggest I
> should create one and move those bindings there?

Yes, often the dts files are synced from Linux for various devices,
anything U-Boot specific should be in a -u-boot.dtsi so it's either
not lost or easily found what the deltra there is.

> > Also if all devices based on the imx8m[mnpq ] need
> > this change it might be better putting it in the imx8m[mnpq]-u-boot.dtsi so any
> > new boards that get added, like the recently posted imx8mm IoT Gate
> > automatically benefit from this fix.
>
> This might depend on the layout of the particular board. Some boards might include
> additional HW components to aid in solving sd card interface power cycle issue.
>
> Introducing an additional delay for those boards which had this resolved in the layout
> might not be advisable, therefore I believe that target board vendors can decide for
> themselves if this fix is applicable to them.
>
> Nevertheless, it is a good point: I was planning to propose Compulab to have a look at
> this patch as well to identify if this applies to their new IoT Gateway device as I saw
> patches from them yesterday on the list. Was just waiting for a general feedback on the
> patch itself.
>
> >
> > >         };
> > >
> > >         buck2_reg: regulator-buck2 {
> > > diff --git a/arch/arm/dts/imx8mq-phanbell.dts
> > > b/arch/arm/dts/imx8mq-phanbell.dts
> > > index 4892ad5ee1..bc6b2638ee 100644
> > > --- a/arch/arm/dts/imx8mq-phanbell.dts
> > > +++ b/arch/arm/dts/imx8mq-phanbell.dts
> > > @@ -34,6 +34,7 @@
> > >                 regulator-max-microvolt = <3300000>;
> > >                 gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>;
> > >                 enable-active-high;
> > > +               u-boot,off-on-delay-us = <20000>;
> > >         };
> > >  };
> > >
> > > --
> > > 2.17.1
> > >
>
> Regards,
> Andrey