Message ID | 20231220103811.228515-4-andrejs.cainikovs@gmail.com |
---|---|
State | Accepted |
Commit | 8a72d193cf15cf0d6936886db4b2dd75956fb4d8 |
Delegated to: | Fabio Estevam |
Headers | show |
Series | colibri-imx8x: configure module usb hub to bypass mode | expand |
Hi Andrejs, On Wed, Dec 20, 2023 at 7:39 AM Andrejs Cainikovs <andrejs.cainikovs@gmail.com> wrote: > diff --git a/arch/arm/dts/fsl-imx8qxp-colibri-u-boot.dtsi b/arch/arm/dts/fsl-imx8qxp-colibri-u-boot.dtsi > index a6af4e5e2b7..6ab6b1f9ee6 100644 > --- a/arch/arm/dts/fsl-imx8qxp-colibri-u-boot.dtsi > +++ b/arch/arm/dts/fsl-imx8qxp-colibri-u-boot.dtsi > @@ -84,6 +84,21 @@ > bootph-some-ram; > }; > > +&gpio_expander_43 { > + usb-bypass-n-hog { > + gpio-hog; > + gpios = <5 GPIO_ACTIVE_LOW>; > + line-name = "usb-bypass-n"; > + output-high; > + }; > + usb-reset-n-hog { > + gpio-hog; > + gpios = <4 GPIO_ACTIVE_LOW>; > + line-name = "usb-reset-n"; > + output-low; > + }; > +}; Patch looks good from a U-Boot perspective, but just want to understand why the imx8qxp/imx8qm boards in U-Boot use devicetrees based on downstream NXP instead of the mainline kernel dts? Shouldn't this 'hub out of reset' be added in mainline Linux first and then just sync the dts in U-Boot?
Hello Fabio, On Wed, Dec 20, 2023 at 07:48:50AM -0300, Fabio Estevam wrote: > On Wed, Dec 20, 2023 at 7:39 AM Andrejs Cainikovs > <andrejs.cainikovs@gmail.com> wrote: > > > diff --git a/arch/arm/dts/fsl-imx8qxp-colibri-u-boot.dtsi b/arch/arm/dts/fsl-imx8qxp-colibri-u-boot.dtsi > > index a6af4e5e2b7..6ab6b1f9ee6 100644 > > --- a/arch/arm/dts/fsl-imx8qxp-colibri-u-boot.dtsi > > +++ b/arch/arm/dts/fsl-imx8qxp-colibri-u-boot.dtsi > > @@ -84,6 +84,21 @@ > > bootph-some-ram; > > }; > > > > +&gpio_expander_43 { > > + usb-bypass-n-hog { > > + gpio-hog; > > + gpios = <5 GPIO_ACTIVE_LOW>; > > + line-name = "usb-bypass-n"; > > + output-high; > > + }; > > + usb-reset-n-hog { > > + gpio-hog; > > + gpios = <4 GPIO_ACTIVE_LOW>; > > + line-name = "usb-reset-n"; > > + output-low; > > + }; > > +}; > > Patch looks good from a U-Boot perspective, but just want to > understand why the imx8qxp/imx8qm boards > in U-Boot use devicetrees based on downstream NXP instead of the > mainline kernel dts? > > Shouldn't this 'hub out of reset' be added in mainline Linux first and > then just sync the dts in U-Boot? Valid point. In general re-using the DTS for the kernel would not work. The bypass and reset signals are part of the USB HUB node [1], and we would need to add such a driver to U-Boot to be able to properly implement it (despite the recent plans to make SPL a full blown bootloader and U-Boot proper the operating system and kick out Linux I would not go into that direction ;-) Given that I believe that overriding the Linux DTS to use these 2 signals are simple GPIO HOG for U-Boot is the correct approach. Francesco [1] Linux: Documentation/devicetree/bindings/usb/smsc,usb3503.yaml
Hi Francesco, On Wed, Dec 20, 2023 at 8:30 AM Francesco Dolcini <francesco@dolcini.it> wrote: > Valid point. > > In general re-using the DTS for the kernel would not work. > > The bypass and reset signals are part of the USB HUB node [1], and we > would need to add such a driver to U-Boot to be able to properly > implement it (despite the recent plans to make SPL a full blown > bootloader and U-Boot proper the operating system and kick out Linux I > would not go into that direction ;-) > > Given that I believe that overriding the Linux DTS to use these 2 > signals are simple GPIO HOG for U-Boot is the correct approach. Yes, this series can go as-is. My main concern is why U-Boot still uses the NXP-based fsl-imx8qxp-mek.dts, fsl-imx8qxp-colibri.dts variant instead of the mainline versions. The other i.MX SoCs in U-Boot are better synced with the Linux kernel DTs. I don't have access to i.MX8QXP/i.MX8QM boards to help on this, but for better long-term support it would be nice if someone could sync the U-Boot i.MX8QXP/i.MX8QM DTs with Linux.
On Wed, Dec 20, 2023 at 08:39:38AM -0300, Fabio Estevam wrote: > Hi Francesco, > > On Wed, Dec 20, 2023 at 8:30 AM Francesco Dolcini <francesco@dolcini.it> wrote: > > > Valid point. > > > > In general re-using the DTS for the kernel would not work. > > > > The bypass and reset signals are part of the USB HUB node [1], and we > > would need to add such a driver to U-Boot to be able to properly > > implement it (despite the recent plans to make SPL a full blown > > bootloader and U-Boot proper the operating system and kick out Linux I > > would not go into that direction ;-) > > > > Given that I believe that overriding the Linux DTS to use these 2 > > signals are simple GPIO HOG for U-Boot is the correct approach. > > Yes, this series can go as-is. > > My main concern is why U-Boot still uses the NXP-based > fsl-imx8qxp-mek.dts, fsl-imx8qxp-colibri.dts > variant instead of the mainline versions. > > The other i.MX SoCs in U-Boot are better synced with the Linux kernel DTs. > > I don't have access to i.MX8QXP/i.MX8QM boards to help on this, but > for better long-term support it would be nice if someone could sync > the U-Boot i.MX8QXP/i.MX8QM DTs with Linux. Understood. Yes, this is some real work that someone should take up. Just doing the sync will likely break everything now. At the moment the situation with i.MX8QXP/i.MX8QM is not as good as i.MX8M* neither in U-Boot nor in Linux :-/ Francesco
On Wed, Dec 20, 2023 at 8:45 AM Francesco Dolcini <francesco@dolcini.it> wrote: > Understood. Yes, this is some real work that someone should take up. > Just doing the sync will likely break everything now. Exactly. > At the moment the situation with i.MX8QXP/i.MX8QM is not as good as > i.MX8M* neither in U-Boot nor in Linux :-/ The NXP folks are on Cc here, so hopefully they could help improving this. Thanks
diff --git a/arch/arm/dts/fsl-imx8qxp-colibri-u-boot.dtsi b/arch/arm/dts/fsl-imx8qxp-colibri-u-boot.dtsi index a6af4e5e2b7..6ab6b1f9ee6 100644 --- a/arch/arm/dts/fsl-imx8qxp-colibri-u-boot.dtsi +++ b/arch/arm/dts/fsl-imx8qxp-colibri-u-boot.dtsi @@ -84,6 +84,21 @@ bootph-some-ram; }; +&gpio_expander_43 { + usb-bypass-n-hog { + gpio-hog; + gpios = <5 GPIO_ACTIVE_LOW>; + line-name = "usb-bypass-n"; + output-high; + }; + usb-reset-n-hog { + gpio-hog; + gpios = <4 GPIO_ACTIVE_LOW>; + line-name = "usb-reset-n"; + output-low; + }; +}; + &gpio0 { bootph-some-ram; }; diff --git a/arch/arm/dts/fsl-imx8qxp-colibri.dts b/arch/arm/dts/fsl-imx8qxp-colibri.dts index 295090ad711..b479921aff9 100644 --- a/arch/arm/dts/fsl-imx8qxp-colibri.dts +++ b/arch/arm/dts/fsl-imx8qxp-colibri.dts @@ -319,8 +319,6 @@ gpio-controller; #gpio-cells = <2>; reg = <0x43>; - initial_io_dir = <0xff>; - initial_output = <0x05>; }; }; diff --git a/configs/colibri-imx8x_defconfig b/configs/colibri-imx8x_defconfig index 13c16bde3d8..ce60c509582 100644 --- a/configs/colibri-imx8x_defconfig +++ b/configs/colibri-imx8x_defconfig @@ -65,6 +65,7 @@ CONFIG_BOOTCOUNT_LIMIT=y CONFIG_BOOTCOUNT_ENV=y CONFIG_CLK_IMX8=y CONFIG_CPU=y +CONFIG_GPIO_HOG=y CONFIG_FXL6408_GPIO=y CONFIG_MXC_GPIO=y CONFIG_DM_I2C=y