| Message ID | 20201201085821.23050-3-andrey.zhizhikin@leica-geosystems.com |
|---|---|
| State | Superseded |
| Delegated to: | Stefano Babic |
| Headers | show |
| Series | imx8m: introduce high speed mode support in usdhc | expand |
On Tue, Dec 1, 2020 at 7:32 AM Andrey Zhizhikin <andrey.zhizhikin@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@leica-geosystems.com> > Cc: Stefano Babic <sbabic@denx.de> > Cc: Ye Li <ye.li@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. I tried it on the beacon imx8mm-beacon-kit, and it didn't work until I removed the "u-boot," 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 = <®_usdhc2_vmmc>; > + u-boot,sd-uhs-sdr104; > + u-boot,sd-uhs-ddr50; > status = "okay"; > }; > > -- > 2.17.1 >
Hello Adam, > -----Original Message----- > From: Adam Ford <aford173@gmail.com> > Sent: Saturday, December 5, 2020 1:43 AM > To: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com> > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Stefano Babic > <sbabic@denx.de>; Ye Li <ye.li@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@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@leica-geosystems.com> > > Cc: Stefano Babic <sbabic@denx.de> > > Cc: Ye Li <ye.li@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 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. 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? > > 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 = <®_usdhc2_vmmc>; > > + u-boot,sd-uhs-sdr104; > > + u-boot,sd-uhs-ddr50; > > status = "okay"; > > }; > > > > -- > > 2.17.1 > > -- andrey
On Sat, Dec 5, 2020 at 8:21 AM ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com> wrote: > > Hello Adam, > > > -----Original Message----- > > From: Adam Ford <aford173@gmail.com> > > Sent: Saturday, December 5, 2020 1:43 AM > > To: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com> > > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Stefano Babic > > <sbabic@denx.de>; Ye Li <ye.li@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@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@leica-geosystems.com> > > > Cc: Stefano Babic <sbabic@denx.de> > > > Cc: Ye Li <ye.li@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. 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. 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. 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. 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 = <®_usdhc2_vmmc>; > > > + u-boot,sd-uhs-sdr104; > > > + u-boot,sd-uhs-ddr50; > > > status = "okay"; > > > }; > > > > > > -- > > > 2.17.1 > > > > > -- andrey
Hello Adam, > -----Original Message----- > From: Adam Ford <aford173@gmail.com> > Sent: Saturday, December 5, 2020 3:31 PM > To: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com> > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Stefano Babic > <sbabic@denx.de>; Ye Li <ye.li@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@leica- > geosystems.com> wrote: > > > > Hello Adam, > > > > > -----Original Message----- > > > From: Adam Ford <aford173@gmail.com> > > > Sent: Saturday, December 5, 2020 1:43 AM > > > To: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com> > > > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Stefano Babic > > > <sbabic@denx.de>; Ye Li <ye.li@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@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@leica-geosystems.com> > > > > Cc: Stefano Babic <sbabic@denx.de> > > > > Cc: Ye Li <ye.li@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 = <®_usdhc2_vmmc>; > > > > + u-boot,sd-uhs-sdr104; > > > > + u-boot,sd-uhs-ddr50; > > > > status = "okay"; > > > > }; > > > > > > > > -- > > > > 2.17.1 > > > > > > > > -- Andrey -- andrey
Hello Adam, > -----Original Message----- > From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of ZHIZHIKIN Andrey > Sent: Saturday, December 5, 2020 4:09 PM > To: Adam Ford <aford173@gmail.com> > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Stefano Babic > <sbabic@denx.de>; Ye Li <ye.li@nxp.com> > Subject: RE: [PATCH 2/2] ARM: dts: imx8m: add UHS or HS400/HS400ES properties > > Hello Adam, > > > -----Original Message----- > > From: Adam Ford <aford173@gmail.com> > > Sent: Saturday, December 5, 2020 3:31 PM > > To: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com> > > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Stefano Babic > > <sbabic@denx.de>; Ye Li <ye.li@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@leica- geosystems.com> wrote: > > > > > > Hello Adam, > > > > > > > -----Original Message----- > > > > From: Adam Ford <aford173@gmail.com> > > > > Sent: Saturday, December 5, 2020 1:43 AM > > > > To: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com> > > > > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Stefano Babic > > > > <sbabic@denx.de>; Ye Li <ye.li@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@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@leica-geosystems.com> > > > > > Cc: Stefano Babic <sbabic@denx.de> > > > > > Cc: Ye Li <ye.li@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 = <®_usdhc2_vmmc>; > > > > > + u-boot,sd-uhs-sdr104; > > > > > + u-boot,sd-uhs-ddr50; > > > > > status = "okay"; > > > > > }; > > > > > > > > > > -- > > > > > 2.17.1 > > > > > > > > > > > -- Andrey > > -- andrey -- andrey
On Sat, Dec 5, 2020 at 10:55 AM ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com> wrote: > > Hello Adam, > > > -----Original Message----- > > From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of ZHIZHIKIN Andrey > > Sent: Saturday, December 5, 2020 4:09 PM > > To: Adam Ford <aford173@gmail.com> > > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Stefano Babic > > <sbabic@denx.de>; Ye Li <ye.li@nxp.com> > > Subject: RE: [PATCH 2/2] ARM: dts: imx8m: add UHS or HS400/HS400ES properties > > > > Hello Adam, > > > > > -----Original Message----- > > > From: Adam Ford <aford173@gmail.com> > > > Sent: Saturday, December 5, 2020 3:31 PM > > > To: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com> > > > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Stefano Babic > > > <sbabic@denx.de>; Ye Li <ye.li@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@leica- geosystems.com> wrote: > > > > > > > > Hello Adam, > > > > > > > > > -----Original Message----- > > > > > From: Adam Ford <aford173@gmail.com> > > > > > Sent: Saturday, December 5, 2020 1:43 AM > > > > > To: ZHIZHIKIN Andrey <andrey.zhizhikin@leica-geosystems.com> > > > > > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Stefano Babic > > > > > <sbabic@denx.de>; Ye Li <ye.li@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@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@leica-geosystems.com> > > > > > > Cc: Stefano Babic <sbabic@denx.de> > > > > > > Cc: Ye Li <ye.li@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. That's too bad. I wonder if at some point we as 8mm users can create a imx8mm-u-boot.dtsi file and include it in each of the board -u-boot.dtsi files to reduce the redundant copies of all the same stuff. This way if a u-boot specific feature gets modified, added, or removed, we can fix the dt node in one place instead of multiple. I'm not proposing that we do iit now, but just brainstorming ideas. adam > > > > > > > > > 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 = <®_usdhc2_vmmc>; > > > > > > + u-boot,sd-uhs-sdr104; > > > > > > + u-boot,sd-uhs-ddr50; > > > > > > status = "okay"; > > > > > > }; > > > > > > > > > > > > -- > > > > > > 2.17.1 > > > > > > > > > > > > > > -- Andrey > > > > -- andrey > > -- andrey
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; }; &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 = <®_usdhc2_vmmc>; + u-boot,sd-uhs-sdr104; + u-boot,sd-uhs-ddr50; status = "okay"; };
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@leica-geosystems.com> Cc: Stefano Babic <sbabic@denx.de> Cc: Ye Li <ye.li@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(+)