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

ZHIZHIKIN Andrey andrey.zhizhikin at leica-geosystems.com
Sat Dec 5 17:55:00 CET 2020 

Hello Adam,

> -----Original Message-----
> From: U-Boot <u-boot-bounces at lists.denx.de> On Behalf Of ZHIZHIKIN Andrey
> Sent: Saturday, December 5, 2020 4:09 PM
> To: Adam Ford <aford173 at gmail.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
> 
> 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:
> > > 
> > >
> >
> > 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.

This unfortunately would not work, since build system does not automatically
pick up '-uboot.dtsi' if it does not match to the entry in Makefile. Only those
DTS file names, which are explicitly present in the Makefile are searched for
corresponding '-u-boot.dtsi' to be auto-included.

I would have to abandon this idea and go ahead with enabling those bindings
only in '-u-boot.dtsi' files which have corresponding .dts files in the tree.

That would mean for all boards requiring to have high speed modes enabled -
corresponding bindings should be introduced separately in their respective device trees.

> 
> >
> > 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

-- andrey

More information about the U-Boot mailing list