[PATCH 2/2] ARM: dts: imx8m: add UHS or HS400/HS400ES properties

ZHIZHIKIN Andrey andrey.zhizhikin at leica-geosystems.com
Sat Dec 5 16:08:35 CET 2020 

Hello Adam,

> -----Original Message-----
> From: Adam Ford <aford173 at gmail.com>
> Sent: Saturday, December 5, 2020 3:31 PM
> To: ZHIZHIKIN Andrey <andrey.zhizhikin at leica-geosystems.com>
> Cc: U-Boot Mailing List <u-boot at lists.denx.de>; Stefano Babic
> <sbabic at denx.de>; Ye Li <ye.li at nxp.com>
> Subject: Re: [PATCH 2/2] ARM: dts: imx8m: add UHS or HS400/HS400ES properties
> 
> 
> On Sat, Dec 5, 2020 at 8:21 AM ZHIZHIKIN Andrey <andrey.zhizhikin at leica-
> geosystems.com> wrote:
> >
> > Hello Adam,
> >
> > > -----Original Message-----
> > > From: Adam Ford <aford173 at gmail.com>
> > > Sent: Saturday, December 5, 2020 1:43 AM
> > > To: ZHIZHIKIN Andrey <andrey.zhizhikin at leica-geosystems.com>
> > > Cc: U-Boot Mailing List <u-boot at lists.denx.de>; Stefano Babic
> > > <sbabic at denx.de>; Ye Li <ye.li at nxp.com>
> > > Subject: Re: [PATCH 2/2] ARM: dts: imx8m: add UHS or HS400/HS400ES
> > > properties
> > >
> > > On Tue, Dec 1, 2020 at 7:32 AM Andrey Zhizhikin
> > > <andrey.zhizhikin at leica- geosystems.com> wrote:
> > > >
> > > > i.MX8M series provide support for high speed grades in their usdhc
> > > > controllers, which has eMMC and SDHC connected to them.
> > > >
> > > > Enable this support across the entire i.MX8M family by providing
> > > > quirks to usdhc controllers designated by storage media connected to them.
> > > >
> > > > Signed-off-by: Andrey Zhizhikin
> > > > <andrey.zhizhikin at leica-geosystems.com>
> > > > Cc: Stefano Babic <sbabic at denx.de>
> > > > Cc: Ye Li <ye.li at nxp.com>
> > > > ---
> > > >  arch/arm/dts/fsl-imx8qm-mek-u-boot.dtsi  | 3 +++
> > > > arch/arm/dts/fsl-imx8qxp-mek-u-boot.dtsi | 3 +++
> > > >  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              | 3 +++
> > > >  6 files changed, 21 insertions(+)
> > > >
> > > > diff --git a/arch/arm/dts/fsl-imx8qm-mek-u-boot.dtsi
> > > > b/arch/arm/dts/fsl-imx8qm-mek-u-boot.dtsi
> > > > index 80d6475b7c..2f86fcce3e 100644
> > > > --- a/arch/arm/dts/fsl-imx8qm-mek-u-boot.dtsi
> > > > +++ b/arch/arm/dts/fsl-imx8qm-mek-u-boot.dtsi
> > > > @@ -118,8 +118,11 @@
> > > >
> > > >  &usdhc1 {
> > > >         u-boot,dm-spl;
> > > > +       u-boot,mmc-hs400-1_8v;
> > > >  };
> > > >
> > >
> > > I don't think the "u-boot," prefix is needed.  Looking at other
> > > boards' device trees, they don't seem to have this.
> >
> > Correct, I've already realized that and currently working on V3 of the series:
> > https://lists.denx.de/pipermail/u-boot/2020-December/434159.html
> >
> 
> Awesome!
> 
> > Thanks a lot for testing it though!
> >
> > > I tried it on the beacon imx8mm-beacon-kit, and it didn't work until
> > > I removed the "u-boot,"
> >
> > I believe you had it tested with your patches on top, since it would
> > also require additional config options to be set, which you introduced for
> imx8mm-beacon-kit.
> >
> I wouldn't have even thought about it until I saw your patch, so thanks for
> bringing it to my attention.  :-)
> 
> > As I see it now - those changes might be useful and applicable to all
> > imx8m* derivatives and I'm thinking to move those binding to base dtsi files
> then.
> >
> > Would there be any objections here?
> 
> None from me.

Great!

> 
> If I understand correctly, we could add a imx8mm-u-boot-dtsi with these U-Boot
> additions, and all imx8mm boards would include them instead of modifying each
> board's u-boot-dtsi.

I would make this change in the V3 then.

> 
> My only concern there is whether or not enabling hs400, hs200 or
> sdh104 on boards that don't have devices compatible with such flags.
> I would expect them to autonegotiate, but I have no way to test it.

According to the specification, the speed mode should be auto-negotiated
so it should be safe to include those binding across.

> 
> In theory, we could also make those Kconfig options 'imply' HS400, HS200, etc.
> automatically in the Kconfig which would eliminate the need to modify each
> defconfig file.  Implying that option would allow users to remove them in their
> respective defconfig files if they don't want the feature.

Looking at the way how high speed modes are enabled in U-Boot, it does require
that both config options and DT bindings are to be turned on, with bindings
providing a fine-grained control over which modes are supported.

I might look into this part further on, but would rather leave it outside of this series.

> 
> adam
> 
> >
> > >
> > > With that removed, I can get the following:
> > >
> > > u-boot=> mmc info
> > > Device: FSL_SDHC
> > > Manufacturer ID: 27
> > > OEM: 5048
> > > Name: SD32G
> > > Bus Speed: 200000000
> > > Mode: UHS SDR104 (208MHz)
> > > Rd Block Len: 512
> > > SD version 3.0
> > > High Capacity: Yes
> > > Capacity: 29 GiB
> > > Bus Width: 4-bit
> > > Erase Group Size: 512 Bytes
> > > u-boot=>
> > >
> > > and
> > >
> > > u-boot=> mmc info
> > > Device: FSL_SDHC
> > > Manufacturer ID: 45
> > > OEM: 100
> > > Name: DG403
> > > Bus Speed: 200000000
> > > Mode: HS400 (200MHz)
> > > Rd Block Len: 512
> > > MMC version 5.1
> > > High Capacity: Yes
> > > Capacity: 29.1 GiB
> > > Bus Width: 8-bit DDR
> > > Erase Group Size: 512 KiB
> > > HC WP Group Size: 8 MiB
> > > User Capacity: 29.1 GiB WRREL
> > > Boot Capacity: 4 MiB ENH
> > > RPMB Capacity: 4 MiB ENH
> > > Boot area 0 is not write protected
> > > Boot area 1 is not write protected
> > > u-boot=>
> > >
> > > adam
> > >
> > > >  &usdhc2 {
> > > >         u-boot,dm-spl;
> > > > +       u-boot,sd-uhs-sdr104;
> > > > +       u-boot,sd-uhs-ddr50;
> > > >  };
> > > > diff --git a/arch/arm/dts/fsl-imx8qxp-mek-u-boot.dtsi
> > > > b/arch/arm/dts/fsl-imx8qxp-mek-u-boot.dtsi
> > > > index 771ab635f1..f4332edac5 100644
> > > > --- a/arch/arm/dts/fsl-imx8qxp-mek-u-boot.dtsi
> > > > +++ b/arch/arm/dts/fsl-imx8qxp-mek-u-boot.dtsi
> > > > @@ -118,8 +118,11 @@
> > > >
> > > >  &usdhc1 {
> > > >         u-boot,dm-spl;
> > > > +       u-boot,mmc-hs400-1_8v;
> > > >  };
> > > >
> > > >  &usdhc2 {
> > > >         u-boot,dm-spl;
> > > > +       u-boot,sd-uhs-sdr104;
> > > > +       u-boot,sd-uhs-ddr50;
> > > >  };
> > > > diff --git a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> > > > b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> > > > index 9f77d3c6ff..67666a08ec 100644
> > > > --- a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> > > > +++ b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
> > > > @@ -100,10 +100,14 @@
> > > >
> > > >  &usdhc2 {
> > > >         u-boot,dm-spl;
> > > > +       u-boot,sd-uhs-sdr104;
> > > > +       u-boot,sd-uhs-ddr50;
> > > >  };
> > > >
> > > >  &usdhc3 {
> > > >         u-boot,dm-spl;
> > > > +       u-boot,mmc-hs400-1_8v;
> > > > +       u-boot,mmc-hs400-enhanced-strobe;
> > > >  };
> > > >
> > > >  &i2c1 {
> > > > diff --git a/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi
> > > > b/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi
> > > > index 98b0b9891b..e03e635213 100644
> > > > --- a/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi
> > > > +++ b/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi
> > > > @@ -97,10 +97,14 @@
> > > >
> > > >  &usdhc2 {
> > > >         u-boot,dm-spl;
> > > > +       u-boot,sd-uhs-sdr104;
> > > > +       u-boot,sd-uhs-ddr50;
> > > >  };
> > > >
> > > >  &usdhc3 {
> > > >         u-boot,dm-spl;
> > > > +       u-boot,mmc-hs400-1_8v;
> > > > +       u-boot,mmc-hs400-enhanced-strobe;
> > > >  };
> > > >
> > > >  &wdog1 {
> > > > diff --git a/arch/arm/dts/imx8mp-evk-u-boot.dtsi
> > > > b/arch/arm/dts/imx8mp-evk-u-boot.dtsi
> > > > index 2452e9175c..0776b24a6e 100644
> > > > --- a/arch/arm/dts/imx8mp-evk-u-boot.dtsi
> > > > +++ b/arch/arm/dts/imx8mp-evk-u-boot.dtsi
> > > > @@ -126,10 +126,14 @@
> > > >
> > > >  &usdhc2 {
> > > >         u-boot,dm-spl;
> > > > +       u-boot,sd-uhs-sdr104;
> > > > +       u-boot,sd-uhs-ddr50;
> > > >  };
> > > >
> > > >  &usdhc3 {
> > > >         u-boot,dm-spl;
> > > > +       u-boot,mmc-hs400-1_8v;
> > > > +       u-boot,mmc-hs400-enhanced-strobe;
> > > >  };
> > > >
> > > >  &wdog1 {
> > > > diff --git a/arch/arm/dts/imx8mq-evk.dts
> > > > b/arch/arm/dts/imx8mq-evk.dts index 9663683f69..985e7e7f8b 100644
> > > > --- a/arch/arm/dts/imx8mq-evk.dts
> > > > +++ b/arch/arm/dts/imx8mq-evk.dts
> > > > @@ -291,6 +291,7 @@
> > > >         non-removable;
> > > >         no-sd;
> > > >         no-sdio;
> > > > +       u-boot,mmc-hs400-1_8v;
> > > >         status = "okay";
> > > >  };
> > > >
> > > > @@ -301,6 +302,8 @@
> > > >         pinctrl-2 = <&pinctrl_usdhc2_200mhz>;
> > > >         cd-gpios = <&gpio2 12 GPIO_ACTIVE_LOW>;
> > > >         vmmc-supply = <&reg_usdhc2_vmmc>;
> > > > +       u-boot,sd-uhs-sdr104;
> > > > +       u-boot,sd-uhs-ddr50;
> > > >         status = "okay";
> > > >  };
> > > >
> > > > --
> > > > 2.17.1
> > > >
> >
> > -- Andrey

-- andrey

More information about the U-Boot mailing list