| Message ID | 20230427180845.127439-1-festevam@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Stefano Babic |
| Headers | show |
| Series | [1/3] arm: dts: imx8mm: Sync with Linux 6.3 | expand |
On Thu, Apr 27, 2023 at 11:08 AM Fabio Estevam <festevam@gmail.com> wrote: > > From: Fabio Estevam <festevam@denx.de> > > Sync imx8mm.dtsi with Linux 6.3. > > The motivation for doing this sync was a bug when doing "ums 0 mmc 1" > on imx8mm-evk. It worked well for the first time, but after doing > a CTRL+C and launching the ums again, the command did not work. > > Adam Ford suggested to sync imx8mm.dtsi with the Linux dts, as there was > a recent USB power domain reorganization there. > > After syncing the imx8mm.dtsi with Linux, the ums command works without > problem after a CTRL+C. > > Suggested-by: Adam Ford <aford173@gmail.com> > Signed-off-by: Fabio Estevam <festevam@denx.de> > --- > arch/arm/dts/imx8mm.dtsi | 52 +++++++++++++++++++++++++++++----------- > 1 file changed, 38 insertions(+), 14 deletions(-) > > diff --git a/arch/arm/dts/imx8mm.dtsi b/arch/arm/dts/imx8mm.dtsi > index afb90f59c83c..31f4548f85cf 100644 > --- a/arch/arm/dts/imx8mm.dtsi > +++ b/arch/arm/dts/imx8mm.dtsi > @@ -139,6 +139,7 @@ > A53_L2: l2-cache0 { > compatible = "cache"; > cache-level = <2>; > + cache-unified; > cache-size = <0x80000>; > cache-line-size = <64>; > cache-sets = <512>; > @@ -276,6 +277,7 @@ > assigned-clocks = <&clk IMX8MM_CLK_USB_PHY_REF>; > assigned-clock-parents = <&clk IMX8MM_SYS_PLL1_100M>; > clock-names = "main_clk"; > + power-domains = <&pgc_otg1>; > }; > > usbphynop2: usbphynop2 { > @@ -285,6 +287,7 @@ > assigned-clocks = <&clk IMX8MM_CLK_USB_PHY_REF>; > assigned-clock-parents = <&clk IMX8MM_SYS_PLL1_100M>; > clock-names = "main_clk"; > + power-domains = <&pgc_otg2>; > }; > > soc: soc@0 { > @@ -493,6 +496,8 @@ > compatible = "fsl,imx8mm-tmu"; > reg = <0x30260000 0x10000>; > clocks = <&clk IMX8MM_CLK_TMU_ROOT>; > + nvmem-cells = <&tmu_calib>; > + nvmem-cell-names = "calib"; > #thermal-sensor-cells = <0>; > }; > > @@ -547,8 +552,8 @@ > reg = <0x30330000 0x10000>; > }; > > - gpr: iomuxc-gpr@30340000 { > - compatible = "fsl,imx8mm-iomuxc-gpr", "fsl,imx6q-iomuxc-gpr", "syscon"; > + gpr: syscon@30340000 { > + compatible = "fsl,imx8mm-iomuxc-gpr", "syscon"; > reg = <0x30340000 0x10000>; > }; > > @@ -560,22 +565,40 @@ > #address-cells = <1>; > #size-cells = <1>; > > - imx8mm_uid: unique-id@410 { > + /* > + * The register address below maps to the MX8M > + * Fusemap Description Table entries this way. > + * Assuming > + * reg = <ADDR SIZE>; > + * then > + * Fuse Address = (ADDR * 4) + 0x400 > + * Note that if SIZE is greater than 4, then > + * each subsequent fuse is located at offset > + * +0x10 in Fusemap Description Table (e.g. > + * reg = <0x4 0x8> describes fuses 0x410 and > + * 0x420). > + */ > + imx8mm_uid: unique-id@4 { /* 0x410-0x420 */ > reg = <0x4 0x8>; > }; > > - cpu_speed_grade: speed-grade@10 { > + cpu_speed_grade: speed-grade@10 { /* 0x440 */ > reg = <0x10 4>; > }; > > - fec_mac_address: mac-address@90 { > + tmu_calib: calib@3c { /* 0x4f0 */ > + reg = <0x3c 4>; > + }; > + > + fec_mac_address: mac-address@90 { /* 0x640 */ > reg = <0x90 6>; > }; > }; > > - anatop: anatop@30360000 { > - compatible = "fsl,imx8mm-anatop", "syscon"; > + anatop: clock-controller@30360000 { > + compatible = "fsl,imx8mm-anatop"; > reg = <0x30360000 0x10000>; > + #clock-cells = <1>; > }; > > snvs: snvs@30370000 { > @@ -674,13 +697,11 @@ > pgc_otg1: power-domain@2 { > #power-domain-cells = <0>; > reg = <IMX8MM_POWER_DOMAIN_OTG1>; > - power-domains = <&pgc_hsiomix>; > }; > > pgc_otg2: power-domain@3 { > #power-domain-cells = <0>; > reg = <IMX8MM_POWER_DOMAIN_OTG2>; > - power-domains = <&pgc_hsiomix>; > }; > > pgc_gpumix: power-domain@4 { > @@ -1186,7 +1207,7 @@ > assigned-clock-parents = <&clk IMX8MM_SYS_PLL2_500M>; > phys = <&usbphynop1>; > fsl,usbmisc = <&usbmisc1 0>; > - power-domains = <&pgc_otg1>; > + power-domains = <&pgc_hsiomix>; > status = "disabled"; > }; > > @@ -1206,7 +1227,7 @@ > assigned-clock-parents = <&clk IMX8MM_SYS_PLL2_500M>; > phys = <&usbphynop2>; > fsl,usbmisc = <&usbmisc2 0>; > - power-domains = <&pgc_otg2>; > + power-domains = <&pgc_hsiomix>; > status = "disabled"; > }; > > @@ -1238,16 +1259,15 @@ > <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>; > - interrupt-names = "gpmi0", "gpmi1", "gpmi2", "gpmi3"; > #dma-cells = <1>; > dma-channels = <4>; > clocks = <&clk IMX8MM_CLK_NAND_USDHC_BUS_RAWNAND_CLK>; > }; > > - gpmi: nand-controller@33002000{ > + gpmi: nand-controller@33002000 { > compatible = "fsl,imx8mm-gpmi-nand", "fsl,imx7d-gpmi-nand"; > #address-cells = <1>; > - #size-cells = <1>; > + #size-cells = <0>; > reg = <0x33002000 0x2000>, <0x33004000 0x4000>; > reg-names = "gpmi-nand", "bch"; > interrupts = <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>; > @@ -1282,6 +1302,10 @@ > <0 0 0 4 &gic GIC_SPI 122 IRQ_TYPE_LEVEL_HIGH>; > fsl,max-link-speed = <2>; > linux,pci-domain = <0>; > + clocks = <&clk IMX8MM_CLK_PCIE1_ROOT>, > + <&clk IMX8MM_CLK_PCIE1_PHY>, > + <&clk IMX8MM_CLK_PCIE1_AUX>; > + clock-names = "pcie", "pcie_bus", "pcie_aux"; > power-domains = <&pgc_pcie>; > resets = <&src IMX8MQ_RESET_PCIE_CTRL_APPS_EN>, > <&src IMX8MQ_RESET_PCIE_CTRL_APPS_TURNOFF>; > -- > 2.34.1 > Fabio, This causes a hang on imx8mm boards when usbotg2 (usb@32e50000) is enabled. You can re-create this on the imx8mm-evk with: diff --git a/arch/arm/dts/imx8mm-evk.dtsi b/arch/arm/dts/imx8mm-evk.dtsi index 7d6317d95b13..898639e33d5e 100644 --- a/arch/arm/dts/imx8mm-evk.dtsi +++ b/arch/arm/dts/imx8mm-evk.dtsi @@ -417,6 +417,10 @@ }; }; +&usbotg2 { + status = "okay"; +}; + &usdhc2 { assigned-clocks = <&clk IMX8MM_CLK_USDHC2>; assigned-clock-rates = <200000000>; Note the imx8mm-evk does have the 2nd host controller but its currently not enabled due to missing bits to deal with the USB 3.0 GPIO controlled mux. Is there perhaps a corresponding change necessary in the imx8m-power-domain driver? Best Regards, Tim
On Thu, Apr 27, 2023 at 3:56 PM Tim Harvey <tharvey@gateworks.com> wrote: > Fabio, > > This causes a hang on imx8mm boards when usbotg2 (usb@32e50000) is > enabled. You can re-create this on the imx8mm-evk with: I am able to reproduce the hang after enabling usbotg2, but this hang is not caused by the imx8mm.dtsi sync. The hang also happens if I revert the imx8mm.dtsi sync. The usbotg2 is a different issue. > Note the imx8mm-evk does have the 2nd host controller but its > currently not enabled due to missing bits to deal with the USB 3.0 > GPIO controlled mux. > > Is there perhaps a corresponding change necessary in the > imx8m-power-domain driver? I haven't checked, but yes, it is very likely some imx8m-power-domain changes are needed.
On Thu, Apr 27, 2023 at 12:11 PM Fabio Estevam <festevam@gmail.com> wrote: > > On Thu, Apr 27, 2023 at 3:56 PM Tim Harvey <tharvey@gateworks.com> wrote: > > > Fabio, > > > > This causes a hang on imx8mm boards when usbotg2 (usb@32e50000) is > > enabled. You can re-create this on the imx8mm-evk with: > > I am able to reproduce the hang after enabling usbotg2, but this hang > is not caused > by the imx8mm.dtsi sync. The hang also happens if I revert the imx8mm.dtsi sync. > > The usbotg2 is a different issue. > > > Note the imx8mm-evk does have the 2nd host controller but its > > currently not enabled due to missing bits to deal with the USB 3.0 > > GPIO controlled mux. > > > > Is there perhaps a corresponding change necessary in the > > imx8m-power-domain driver? > > I haven't checked, but yes, it is very likely some imx8m-power-domain > changes are needed. Fabio, The patch series from Eugen Hristev which implements reference counting for regulators [1] resolves my issue here so I consider this thread closed. Let's move the discussion regarding your dt sync to those threads. Best Regards, Tim [1] https://patchwork.ozlabs.org/project/uboot/list/?series=351536
On Thu, Apr 27, 2023 at 12:14 PM Tim Harvey <tharvey@gateworks.com> wrote: > > On Thu, Apr 27, 2023 at 12:11 PM Fabio Estevam <festevam@gmail.com> wrote: > > > > On Thu, Apr 27, 2023 at 3:56 PM Tim Harvey <tharvey@gateworks.com> wrote: > > > > > Fabio, > > > > > > This causes a hang on imx8mm boards when usbotg2 (usb@32e50000) is > > > enabled. You can re-create this on the imx8mm-evk with: > > > > I am able to reproduce the hang after enabling usbotg2, but this hang > > is not caused > > by the imx8mm.dtsi sync. The hang also happens if I revert the imx8mm.dtsi sync. > > > > The usbotg2 is a different issue. > > > > > Note the imx8mm-evk does have the 2nd host controller but its > > > currently not enabled due to missing bits to deal with the USB 3.0 > > > GPIO controlled mux. > > > > > > Is there perhaps a corresponding change necessary in the > > > imx8m-power-domain driver? > > > > I haven't checked, but yes, it is very likely some imx8m-power-domain > > changes are needed. > > Fabio, > > The patch series from Eugen Hristev which implements reference > counting for regulators [1] resolves my issue here so I consider this > thread closed. > > Let's move the discussion regarding your dt sync to those threads. > > Best Regards, > > Tim > [1] https://patchwork.ozlabs.org/project/uboot/list/?series=351536 Fabio, Sorry, responded to the wrong thread here. The thread regarding imx8mm hang on usb_stop is resolved with the regulator reference counting support. The dt sync here still needs some work apparently. Best Regards, Tim
On Thu, Apr 27, 2023 at 1:08 PM Fabio Estevam <festevam@gmail.com> wrote: > > From: Fabio Estevam <festevam@denx.de> > > Sync imx8mm.dtsi with Linux 6.3. > > The motivation for doing this sync was a bug when doing "ums 0 mmc 1" > on imx8mm-evk. It worked well for the first time, but after doing > a CTRL+C and launching the ums again, the command did not work. > > Adam Ford suggested to sync imx8mm.dtsi with the Linux dts, as there was > a recent USB power domain reorganization there. > > After syncing the imx8mm.dtsi with Linux, the ums command works without > problem after a CTRL+C. > > Suggested-by: Adam Ford <aford173@gmail.com> > Signed-off-by: Fabio Estevam <festevam@denx.de> Reviewed-by: Adam Ford <aford173@gmail.com> > --- > arch/arm/dts/imx8mm.dtsi | 52 +++++++++++++++++++++++++++++----------- > 1 file changed, 38 insertions(+), 14 deletions(-) > > diff --git a/arch/arm/dts/imx8mm.dtsi b/arch/arm/dts/imx8mm.dtsi > index afb90f59c83c..31f4548f85cf 100644 > --- a/arch/arm/dts/imx8mm.dtsi > +++ b/arch/arm/dts/imx8mm.dtsi > @@ -139,6 +139,7 @@ > A53_L2: l2-cache0 { > compatible = "cache"; > cache-level = <2>; > + cache-unified; > cache-size = <0x80000>; > cache-line-size = <64>; > cache-sets = <512>; > @@ -276,6 +277,7 @@ > assigned-clocks = <&clk IMX8MM_CLK_USB_PHY_REF>; > assigned-clock-parents = <&clk IMX8MM_SYS_PLL1_100M>; > clock-names = "main_clk"; > + power-domains = <&pgc_otg1>; > }; > > usbphynop2: usbphynop2 { > @@ -285,6 +287,7 @@ > assigned-clocks = <&clk IMX8MM_CLK_USB_PHY_REF>; > assigned-clock-parents = <&clk IMX8MM_SYS_PLL1_100M>; > clock-names = "main_clk"; > + power-domains = <&pgc_otg2>; > }; > > soc: soc@0 { > @@ -493,6 +496,8 @@ > compatible = "fsl,imx8mm-tmu"; > reg = <0x30260000 0x10000>; > clocks = <&clk IMX8MM_CLK_TMU_ROOT>; > + nvmem-cells = <&tmu_calib>; > + nvmem-cell-names = "calib"; > #thermal-sensor-cells = <0>; > }; > > @@ -547,8 +552,8 @@ > reg = <0x30330000 0x10000>; > }; > > - gpr: iomuxc-gpr@30340000 { > - compatible = "fsl,imx8mm-iomuxc-gpr", "fsl,imx6q-iomuxc-gpr", "syscon"; > + gpr: syscon@30340000 { > + compatible = "fsl,imx8mm-iomuxc-gpr", "syscon"; > reg = <0x30340000 0x10000>; > }; > > @@ -560,22 +565,40 @@ > #address-cells = <1>; > #size-cells = <1>; > > - imx8mm_uid: unique-id@410 { > + /* > + * The register address below maps to the MX8M > + * Fusemap Description Table entries this way. > + * Assuming > + * reg = <ADDR SIZE>; > + * then > + * Fuse Address = (ADDR * 4) + 0x400 > + * Note that if SIZE is greater than 4, then > + * each subsequent fuse is located at offset > + * +0x10 in Fusemap Description Table (e.g. > + * reg = <0x4 0x8> describes fuses 0x410 and > + * 0x420). > + */ > + imx8mm_uid: unique-id@4 { /* 0x410-0x420 */ > reg = <0x4 0x8>; > }; > > - cpu_speed_grade: speed-grade@10 { > + cpu_speed_grade: speed-grade@10 { /* 0x440 */ > reg = <0x10 4>; > }; > > - fec_mac_address: mac-address@90 { > + tmu_calib: calib@3c { /* 0x4f0 */ > + reg = <0x3c 4>; > + }; > + > + fec_mac_address: mac-address@90 { /* 0x640 */ > reg = <0x90 6>; > }; > }; > > - anatop: anatop@30360000 { > - compatible = "fsl,imx8mm-anatop", "syscon"; > + anatop: clock-controller@30360000 { > + compatible = "fsl,imx8mm-anatop"; > reg = <0x30360000 0x10000>; > + #clock-cells = <1>; > }; > > snvs: snvs@30370000 { > @@ -674,13 +697,11 @@ > pgc_otg1: power-domain@2 { > #power-domain-cells = <0>; > reg = <IMX8MM_POWER_DOMAIN_OTG1>; > - power-domains = <&pgc_hsiomix>; > }; > > pgc_otg2: power-domain@3 { > #power-domain-cells = <0>; > reg = <IMX8MM_POWER_DOMAIN_OTG2>; > - power-domains = <&pgc_hsiomix>; > }; > > pgc_gpumix: power-domain@4 { > @@ -1186,7 +1207,7 @@ > assigned-clock-parents = <&clk IMX8MM_SYS_PLL2_500M>; > phys = <&usbphynop1>; > fsl,usbmisc = <&usbmisc1 0>; > - power-domains = <&pgc_otg1>; > + power-domains = <&pgc_hsiomix>; > status = "disabled"; > }; > > @@ -1206,7 +1227,7 @@ > assigned-clock-parents = <&clk IMX8MM_SYS_PLL2_500M>; > phys = <&usbphynop2>; > fsl,usbmisc = <&usbmisc2 0>; > - power-domains = <&pgc_otg2>; > + power-domains = <&pgc_hsiomix>; > status = "disabled"; > }; > > @@ -1238,16 +1259,15 @@ > <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>; > - interrupt-names = "gpmi0", "gpmi1", "gpmi2", "gpmi3"; > #dma-cells = <1>; > dma-channels = <4>; > clocks = <&clk IMX8MM_CLK_NAND_USDHC_BUS_RAWNAND_CLK>; > }; > > - gpmi: nand-controller@33002000{ > + gpmi: nand-controller@33002000 { > compatible = "fsl,imx8mm-gpmi-nand", "fsl,imx7d-gpmi-nand"; > #address-cells = <1>; > - #size-cells = <1>; > + #size-cells = <0>; > reg = <0x33002000 0x2000>, <0x33004000 0x4000>; > reg-names = "gpmi-nand", "bch"; > interrupts = <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>; > @@ -1282,6 +1302,10 @@ > <0 0 0 4 &gic GIC_SPI 122 IRQ_TYPE_LEVEL_HIGH>; > fsl,max-link-speed = <2>; > linux,pci-domain = <0>; > + clocks = <&clk IMX8MM_CLK_PCIE1_ROOT>, > + <&clk IMX8MM_CLK_PCIE1_PHY>, > + <&clk IMX8MM_CLK_PCIE1_AUX>; > + clock-names = "pcie", "pcie_bus", "pcie_aux"; > power-domains = <&pgc_pcie>; > resets = <&src IMX8MQ_RESET_PCIE_CTRL_APPS_EN>, > <&src IMX8MQ_RESET_PCIE_CTRL_APPS_TURNOFF>; > -- > 2.34.1 >
On Thu, Apr 27, 2023 at 4:19 PM Tim Harvey <tharvey@gateworks.com> wrote: > Fabio, > > Sorry, responded to the wrong thread here. The thread regarding imx8mm > hang on usb_stop is resolved with the regulator reference counting > support. > > The dt sync here still needs some work apparently. I am confused now. Which issue exactly does this patch cause you?
On Thu, Apr 27, 2023 at 12:26 PM Fabio Estevam <festevam@gmail.com> wrote: > > On Thu, Apr 27, 2023 at 4:19 PM Tim Harvey <tharvey@gateworks.com> wrote: > > > Fabio, > > > > Sorry, responded to the wrong thread here. The thread regarding imx8mm > > hang on usb_stop is resolved with the regulator reference counting > > support. > > > > The dt sync here still needs some work apparently. > > I am confused now. > > Which issue exactly does this patch cause you? Fabio, Sorry for the confusion. This imx8mm dt sync patch will hang on imx8mm boards that use 'both' usbotg1 and usbotg2. You can reproduce this hang on your imx8mm-evk by enabling usbotg2 in the dt (the board has it but it is not enabled due to the gpio based usb 3.0 mux not being sorted out yet): +&usbotg2 { + dr_mode = "otg"; + status = "okay"; +}; + u-boot=> usb start && usb tree starting USB... Bus usb@32e40000: Bus usb@32e50000: ^^^ imx8mm-evk hangs This hang issue is not resolved by the regulator reference counter support and I am assuming that the power domain changes in this patch need to go along with a change in the power domain driver as well. I don't know yet if the imx8mn/imx8mp dt sync's in your series show issues during testing. Best Regards, Tim
On Thu, Apr 27, 2023 at 4:44 PM Tim Harvey <tharvey@gateworks.com> wrote: > Fabio, > > Sorry for the confusion. > > This imx8mm dt sync patch will hang on imx8mm boards that use 'both' > usbotg1 and usbotg2. You can reproduce this hang on your imx8mm-evk by > enabling usbotg2 in the dt (the board has it but it is not enabled due > to the gpio based usb 3.0 mux not being sorted out yet): > +&usbotg2 { > + dr_mode = "otg"; > + status = "okay"; > +}; > + > > u-boot=> usb start && usb tree > starting USB... > Bus usb@32e40000: Bus usb@32e50000: > ^^^ imx8mm-evk hangs Yes, I can reproduce the hang, but it happens with or without the imx8mm dt sync. This hang is a separate issue, not dt related, as far as I understand. The imx8mm dts sync does solve the issue of running 'ums' after CTRL+C.
On Thu, Apr 27, 2023 at 12:49 PM Fabio Estevam <festevam@gmail.com> wrote: > > On Thu, Apr 27, 2023 at 4:44 PM Tim Harvey <tharvey@gateworks.com> wrote: > > > Fabio, > > > > Sorry for the confusion. > > > > This imx8mm dt sync patch will hang on imx8mm boards that use 'both' > > usbotg1 and usbotg2. You can reproduce this hang on your imx8mm-evk by > > enabling usbotg2 in the dt (the board has it but it is not enabled due > > to the gpio based usb 3.0 mux not being sorted out yet): > > +&usbotg2 { > > + dr_mode = "otg"; > > + status = "okay"; > > +}; > > + > > > > u-boot=> usb start && usb tree > > starting USB... > > Bus usb@32e40000: Bus usb@32e50000: > > ^^^ imx8mm-evk hangs > > Yes, I can reproduce the hang, but it happens with or without the > imx8mm dt sync. > Fabio, I do 'not' see a hang on imx8mm-evk on 'usb start && usb tree' on master (my other issue was on a 'usb stop' but only with usb controllers in host mode). > This hang is a separate issue, not dt related, as far as I understand. > > The imx8mm dts sync does solve the issue of running 'ums' after CTRL+C. I don't agree. The hang 'is' related because all my imx8mm-venice-* boards which use 'both' USB controllers hang with this patch on a 'usb start' and don't hang without it. While a basic 'review' of the patch looks good but actual product testing shows issues. As a maintainer for ARM FREESCALE IMX you must have another imx8mm board which uses both usbotg devices to test against and verify you see what I see? Until we know what other fix is needed to go along with this: Nacked-by: Tim Harvey <tharvey@gateworks.com> I've verified that it's the changes from Linux commit 4585c79ff477f ("arm64: dts: imx8mm: correct usb power domains") that causes the hang, but I don't know why yet. Why are we seeing different behavior on the imx8mm-evk? Are we on different branches? My testing today is on caf0a88d9f31 Best Regards, Tim
On Thu, Apr 27, 2023 at 5:25 PM Tim Harvey <tharvey@gateworks.com> wrote: > > On Thu, Apr 27, 2023 at 12:49 PM Fabio Estevam <festevam@gmail.com> wrote: > > > > On Thu, Apr 27, 2023 at 4:44 PM Tim Harvey <tharvey@gateworks.com> wrote: > > > > > Fabio, > > > > > > Sorry for the confusion. > > > > > > This imx8mm dt sync patch will hang on imx8mm boards that use 'both' > > > usbotg1 and usbotg2. You can reproduce this hang on your imx8mm-evk by > > > enabling usbotg2 in the dt (the board has it but it is not enabled due > > > to the gpio based usb 3.0 mux not being sorted out yet): > > > +&usbotg2 { > > > + dr_mode = "otg"; > > > + status = "okay"; > > > +}; > > > + > > > > > > u-boot=> usb start && usb tree > > > starting USB... > > > Bus usb@32e40000: Bus usb@32e50000: > > > ^^^ imx8mm-evk hangs > > > > Yes, I can reproduce the hang, but it happens with or without the > > imx8mm dt sync. > > > > Fabio, > > I do 'not' see a hang on imx8mm-evk on 'usb start && usb tree' on > master (my other issue was on a 'usb stop' but only with usb > controllers in host mode). > > > This hang is a separate issue, not dt related, as far as I understand. > > > > The imx8mm dts sync does solve the issue of running 'ums' after CTRL+C. > > I don't agree. The hang 'is' related because all my imx8mm-venice-* > boards which use 'both' USB controllers hang with this patch on a 'usb > start' and don't hang without it. While a basic 'review' of the patch > looks good but actual product testing shows issues. As a maintainer > for ARM FREESCALE IMX you must have another imx8mm board which uses > both usbotg devices to test against and verify you see what I see? > > Until we know what other fix is needed to go along with this: > Nacked-by: Tim Harvey <tharvey@gateworks.com> What is the harm is sync'ing the device tree with the kernel? I seemed like you found a solution with the regulator patch. Did I misunderstand that? adam > > I've verified that it's the changes from Linux commit 4585c79ff477f > ("arm64: dts: imx8mm: correct usb power domains") that causes the > hang, but I don't know why yet. > > Why are we seeing different behavior on the imx8mm-evk? Are we on > different branches? My testing today is on caf0a88d9f31 > > Best Regards, > > Tim
On Fri, Apr 28, 2023 at 4:57 AM Adam Ford <aford173@gmail.com> wrote: > > On Thu, Apr 27, 2023 at 5:25 PM Tim Harvey <tharvey@gateworks.com> wrote: > > > > On Thu, Apr 27, 2023 at 12:49 PM Fabio Estevam <festevam@gmail.com> wrote: > > > > > > On Thu, Apr 27, 2023 at 4:44 PM Tim Harvey <tharvey@gateworks.com> wrote: > > > > > > > Fabio, > > > > > > > > Sorry for the confusion. > > > > > > > > This imx8mm dt sync patch will hang on imx8mm boards that use 'both' > > > > usbotg1 and usbotg2. You can reproduce this hang on your imx8mm-evk by > > > > enabling usbotg2 in the dt (the board has it but it is not enabled due > > > > to the gpio based usb 3.0 mux not being sorted out yet): > > > > +&usbotg2 { > > > > + dr_mode = "otg"; > > > > + status = "okay"; > > > > +}; > > > > + > > > > > > > > u-boot=> usb start && usb tree > > > > starting USB... > > > > Bus usb@32e40000: Bus usb@32e50000: > > > > ^^^ imx8mm-evk hangs > > > > > > Yes, I can reproduce the hang, but it happens with or without the > > > imx8mm dt sync. > > > > > > > Fabio, > > > > I do 'not' see a hang on imx8mm-evk on 'usb start && usb tree' on > > master (my other issue was on a 'usb stop' but only with usb > > controllers in host mode). > > > > > This hang is a separate issue, not dt related, as far as I understand. > > > > > > The imx8mm dts sync does solve the issue of running 'ums' after CTRL+C. > > > > I don't agree. The hang 'is' related because all my imx8mm-venice-* > > boards which use 'both' USB controllers hang with this patch on a 'usb > > start' and don't hang without it. While a basic 'review' of the patch > > looks good but actual product testing shows issues. As a maintainer > > for ARM FREESCALE IMX you must have another imx8mm board which uses > > both usbotg devices to test against and verify you see what I see? > > > > Until we know what other fix is needed to go along with this: > > Nacked-by: Tim Harvey <tharvey@gateworks.com> > > What is the harm is sync'ing the device tree with the kernel? I seemed > like you found a solution with the regulator patch. Did I > misunderstand that? > > adam Adam, No, the regulator patch did 'not' resolve the issue created by syncing the imx8mm dt (I caused confusion by responding to the wrong thread - the regulator patch resolved a different issue). Could you please verify my results on a board that uses both usbotg1 and usbotg2? What I see is on master + this imx8mm dt sync (specifically the changes from Linux commit 4585c79ff477f ("arm64: dts: imx8mm: correct usb power domains")) the board hangs on usb start when bringing up usbotg2. Best Regards, Tim
On Fri, Apr 28, 2023 at 10:27 AM Tim Harvey <tharvey@gateworks.com> wrote: > > On Fri, Apr 28, 2023 at 4:57 AM Adam Ford <aford173@gmail.com> wrote: > > > > On Thu, Apr 27, 2023 at 5:25 PM Tim Harvey <tharvey@gateworks.com> wrote: > > > > > > On Thu, Apr 27, 2023 at 12:49 PM Fabio Estevam <festevam@gmail.com> wrote: > > > > > > > > On Thu, Apr 27, 2023 at 4:44 PM Tim Harvey <tharvey@gateworks.com> wrote: > > > > > > > > > Fabio, > > > > > > > > > > Sorry for the confusion. > > > > > > > > > > This imx8mm dt sync patch will hang on imx8mm boards that use 'both' > > > > > usbotg1 and usbotg2. You can reproduce this hang on your imx8mm-evk by > > > > > enabling usbotg2 in the dt (the board has it but it is not enabled due > > > > > to the gpio based usb 3.0 mux not being sorted out yet): > > > > > +&usbotg2 { > > > > > + dr_mode = "otg"; > > > > > + status = "okay"; > > > > > +}; > > > > > + > > > > > > > > > > u-boot=> usb start && usb tree > > > > > starting USB... > > > > > Bus usb@32e40000: Bus usb@32e50000: > > > > > ^^^ imx8mm-evk hangs > > > > > > > > Yes, I can reproduce the hang, but it happens with or without the > > > > imx8mm dt sync. > > > > > > > > > > Fabio, > > > > > > I do 'not' see a hang on imx8mm-evk on 'usb start && usb tree' on > > > master (my other issue was on a 'usb stop' but only with usb > > > controllers in host mode). > > > > > > > This hang is a separate issue, not dt related, as far as I understand. > > > > > > > > The imx8mm dts sync does solve the issue of running 'ums' after CTRL+C. > > > > > > I don't agree. The hang 'is' related because all my imx8mm-venice-* > > > boards which use 'both' USB controllers hang with this patch on a 'usb > > > start' and don't hang without it. While a basic 'review' of the patch > > > looks good but actual product testing shows issues. As a maintainer > > > for ARM FREESCALE IMX you must have another imx8mm board which uses > > > both usbotg devices to test against and verify you see what I see? > > > > > > Until we know what other fix is needed to go along with this: > > > Nacked-by: Tim Harvey <tharvey@gateworks.com> > > > > What is the harm is sync'ing the device tree with the kernel? I seemed > > like you found a solution with the regulator patch. Did I > > misunderstand that? > > > > adam > > Adam, > > No, the regulator patch did 'not' resolve the issue created by syncing > the imx8mm dt (I caused confusion by responding to the wrong thread - > the regulator patch resolved a different issue). Ok. > > Could you please verify my results on a board that uses both usbotg1 > and usbotg2? What I see is on master + this imx8mm dt sync > (specifically the changes from Linux commit 4585c79ff477f ("arm64: > dts: imx8mm: correct usb power domains")) the board hangs on usb start > when bringing up usbotg2. I can, but I am about to board a plane to go visit some sick family, but I'll try to do it early next week. I have a board with both USB controllers enabled. My OTG2 is host-only, so I think it's similar to your setup. Should I apply the regulator patch when I test? adam > > Best Regards, > > Tim
On Fri, Apr 28, 2023 at 8:32 AM Adam Ford <aford173@gmail.com> wrote: > > On Fri, Apr 28, 2023 at 10:27 AM Tim Harvey <tharvey@gateworks.com> wrote: > > > > On Fri, Apr 28, 2023 at 4:57 AM Adam Ford <aford173@gmail.com> wrote: > > > > > > On Thu, Apr 27, 2023 at 5:25 PM Tim Harvey <tharvey@gateworks.com> wrote: > > > > > > > > On Thu, Apr 27, 2023 at 12:49 PM Fabio Estevam <festevam@gmail.com> wrote: > > > > > > > > > > On Thu, Apr 27, 2023 at 4:44 PM Tim Harvey <tharvey@gateworks.com> wrote: > > > > > > > > > > > Fabio, > > > > > > > > > > > > Sorry for the confusion. > > > > > > > > > > > > This imx8mm dt sync patch will hang on imx8mm boards that use 'both' > > > > > > usbotg1 and usbotg2. You can reproduce this hang on your imx8mm-evk by > > > > > > enabling usbotg2 in the dt (the board has it but it is not enabled due > > > > > > to the gpio based usb 3.0 mux not being sorted out yet): > > > > > > +&usbotg2 { > > > > > > + dr_mode = "otg"; > > > > > > + status = "okay"; > > > > > > +}; > > > > > > + > > > > > > > > > > > > u-boot=> usb start && usb tree > > > > > > starting USB... > > > > > > Bus usb@32e40000: Bus usb@32e50000: > > > > > > ^^^ imx8mm-evk hangs > > > > > > > > > > Yes, I can reproduce the hang, but it happens with or without the > > > > > imx8mm dt sync. > > > > > > > > > > > > > Fabio, > > > > > > > > I do 'not' see a hang on imx8mm-evk on 'usb start && usb tree' on > > > > master (my other issue was on a 'usb stop' but only with usb > > > > controllers in host mode). > > > > > > > > > This hang is a separate issue, not dt related, as far as I understand. > > > > > > > > > > The imx8mm dts sync does solve the issue of running 'ums' after CTRL+C. > > > > > > > > I don't agree. The hang 'is' related because all my imx8mm-venice-* > > > > boards which use 'both' USB controllers hang with this patch on a 'usb > > > > start' and don't hang without it. While a basic 'review' of the patch > > > > looks good but actual product testing shows issues. As a maintainer > > > > for ARM FREESCALE IMX you must have another imx8mm board which uses > > > > both usbotg devices to test against and verify you see what I see? > > > > > > > > Until we know what other fix is needed to go along with this: > > > > Nacked-by: Tim Harvey <tharvey@gateworks.com> > > > > > > What is the harm is sync'ing the device tree with the kernel? I seemed > > > like you found a solution with the regulator patch. Did I > > > misunderstand that? > > > > > > adam > > > > Adam, > > > > No, the regulator patch did 'not' resolve the issue created by syncing > > the imx8mm dt (I caused confusion by responding to the wrong thread - > > the regulator patch resolved a different issue). > > Ok. > > > > Could you please verify my results on a board that uses both usbotg1 > > and usbotg2? What I see is on master + this imx8mm dt sync > > (specifically the changes from Linux commit 4585c79ff477f ("arm64: > > dts: imx8mm: correct usb power domains")) the board hangs on usb start > > when bringing up usbotg2. Adam, Sorry to hear that :( > > I can, but I am about to board a plane to go visit some sick family, > but I'll try to do it early next week. > I have a board with both USB controllers enabled. My OTG2 is > host-only, so I think it's similar to your setup. Yes I think that is similar enough to test. In my experience simply enabling otg2 via dt on imx8mm-evk shows the issue I see here but Fabio says he sees a hang on 'usb start' even before this dt sync and I don't know why my results on an imx8mm-evk differ. > > Should I apply the regulator patch when I test? No, don't apply that as this exposes another issue: Error enabling VBUS supply (ret=-114) I'm still looking into that. I'm assuming when the regulaor refcnt support gets merged it may expose a lot of issues from unbalanced regulator enable/disable calls. The regulator refcnt series resolved the hang I see on 'usb stop' for boards where otg2 is in host mode (internal usb_hub device powers down the power domain before ehci_shutdown tries to access the registers to disable the ports). Tim
Hi Tim, On Fri, Apr 28, 2023 at 12:48 PM Tim Harvey <tharvey@gateworks.com> wrote: > Yes I think that is similar enough to test. In my experience simply > enabling otg2 via dt on imx8mm-evk shows the issue I see here but > Fabio says he sees a hang on 'usb start' even before this dt sync and > I don't know why my results on an imx8mm-evk differ. I started from scratch today and now our results match. Applied the following change against U-Boot master: diff --git a/arch/arm/dts/imx8mm-evk.dtsi b/arch/arm/dts/imx8mm-evk.dtsi index 7d6317d95b13..898639e33d5e 100644 --- a/arch/arm/dts/imx8mm-evk.dtsi +++ b/arch/arm/dts/imx8mm-evk.dtsi @@ -417,6 +417,10 @@ }; }; +&usbotg2 { + status = "okay"; +}; + &usdhc2 { assigned-clocks = <&clk IMX8MM_CLK_USDHC2>; assigned-clock-rates = <200000000>; diff --git a/configs/imx8mm_evk_defconfig b/configs/imx8mm_evk_defconfig index ab9ad41b4548..70c7a21f2d9f 100644 --- a/configs/imx8mm_evk_defconfig +++ b/configs/imx8mm_evk_defconfig @@ -119,3 +119,4 @@ CONFIG_CI_UDC=y CONFIG_SDP_LOADADDR=0x40400000 CONFIG_USB_GADGET_DOWNLOAD=y CONFIG_IMX_WATCHDOG=y +CONFIG_CMD_USB=y
On Fri, Apr 28, 2023 at 11:26 AM Fabio Estevam <festevam@gmail.com> wrote: > > Hi Tim, > > On Fri, Apr 28, 2023 at 12:48 PM Tim Harvey <tharvey@gateworks.com> wrote: > > > Yes I think that is similar enough to test. In my experience simply > > enabling otg2 via dt on imx8mm-evk shows the issue I see here but > > Fabio says he sees a hang on 'usb start' even before this dt sync and > > I don't know why my results on an imx8mm-evk differ. > > I started from scratch today and now our results match. > > Applied the following change against U-Boot master: > > diff --git a/arch/arm/dts/imx8mm-evk.dtsi b/arch/arm/dts/imx8mm-evk.dtsi > index 7d6317d95b13..898639e33d5e 100644 > --- a/arch/arm/dts/imx8mm-evk.dtsi > +++ b/arch/arm/dts/imx8mm-evk.dtsi > @@ -417,6 +417,10 @@ > }; > }; > > +&usbotg2 { > + status = "okay"; > +}; > + > &usdhc2 { > assigned-clocks = <&clk IMX8MM_CLK_USDHC2>; > assigned-clock-rates = <200000000>; > diff --git a/configs/imx8mm_evk_defconfig b/configs/imx8mm_evk_defconfig > index ab9ad41b4548..70c7a21f2d9f 100644 > --- a/configs/imx8mm_evk_defconfig > +++ b/configs/imx8mm_evk_defconfig > @@ -119,3 +119,4 @@ CONFIG_CI_UDC=y > CONFIG_SDP_LOADADDR=0x40400000 > CONFIG_USB_GADGET_DOWNLOAD=y > CONFIG_IMX_WATCHDOG=y > +CONFIG_CMD_USB=y > -- > 2.34.1 > > Running "usb start" does not hang. > > Running "ums 0 mmc 1", CTRL+C and then "ums 0 mmc 1" does not work (SD > card is not mounted on PC on the second time). > > After applying the imx8mm.dtsi sync with kernel 6.3: > > Running "ums 0 mmc 1", CTRL+C and then "ums 0 mmc 1" works fine. > > "usb start" hangs. > > So, yes, I agree we cannot do the imx8mm.dtsi sync with 6.3 right now > as we need to fix the USB hang first. > > If anyone has any ideas as to why syncing the commit below: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/arm64/boot/dts/freescale/imx8mm.dtsi?h=v6.3&id=4585c79ff477f9517b7f384a4fce351417e8fa36 > > causes issues in U-Boot, please let us know. I am not in a place to test this as I am traveling, but I thought I'd throw out an idea. The power-domain looks like it moved to the usbphynop2 driver which has the compatible name of "usb-nop-xceiv" Is there a a driver for this? Does it get enabled? If not, maybe we could update the imx8mm-u-u-boot.dtsi to restore the power-domains to a driver that is present. adam > > Thanks
On Fri, Apr 28, 2023 at 9:44 AM Adam Ford <aford173@gmail.com> wrote: > > On Fri, Apr 28, 2023 at 11:26 AM Fabio Estevam <festevam@gmail.com> wrote: > > > > Hi Tim, > > > > On Fri, Apr 28, 2023 at 12:48 PM Tim Harvey <tharvey@gateworks.com> wrote: > > > > > Yes I think that is similar enough to test. In my experience simply > > > enabling otg2 via dt on imx8mm-evk shows the issue I see here but > > > Fabio says he sees a hang on 'usb start' even before this dt sync and > > > I don't know why my results on an imx8mm-evk differ. > > > > I started from scratch today and now our results match. > > > > Applied the following change against U-Boot master: > > > > diff --git a/arch/arm/dts/imx8mm-evk.dtsi b/arch/arm/dts/imx8mm-evk.dtsi > > index 7d6317d95b13..898639e33d5e 100644 > > --- a/arch/arm/dts/imx8mm-evk.dtsi > > +++ b/arch/arm/dts/imx8mm-evk.dtsi > > @@ -417,6 +417,10 @@ > > }; > > }; > > > > +&usbotg2 { > > + status = "okay"; > > +}; > > + > > &usdhc2 { > > assigned-clocks = <&clk IMX8MM_CLK_USDHC2>; > > assigned-clock-rates = <200000000>; > > diff --git a/configs/imx8mm_evk_defconfig b/configs/imx8mm_evk_defconfig > > index ab9ad41b4548..70c7a21f2d9f 100644 > > --- a/configs/imx8mm_evk_defconfig > > +++ b/configs/imx8mm_evk_defconfig > > @@ -119,3 +119,4 @@ CONFIG_CI_UDC=y > > CONFIG_SDP_LOADADDR=0x40400000 > > CONFIG_USB_GADGET_DOWNLOAD=y > > CONFIG_IMX_WATCHDOG=y > > +CONFIG_CMD_USB=y > > -- > > 2.34.1 > > > > Running "usb start" does not hang. > > > > Running "ums 0 mmc 1", CTRL+C and then "ums 0 mmc 1" does not work (SD > > card is not mounted on PC on the second time). > > > > After applying the imx8mm.dtsi sync with kernel 6.3: > > > > Running "ums 0 mmc 1", CTRL+C and then "ums 0 mmc 1" works fine. > > > > "usb start" hangs. > > > > So, yes, I agree we cannot do the imx8mm.dtsi sync with 6.3 right now > > as we need to fix the USB hang first. > > > > If anyone has any ideas as to why syncing the commit below: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/arm64/boot/dts/freescale/imx8mm.dtsi?h=v6.3&id=4585c79ff477f9517b7f384a4fce351417e8fa36 > > > > causes issues in U-Boot, please let us know. > > I am not in a place to test this as I am traveling, but I thought I'd > throw out an idea. The power-domain looks like it moved to the > usbphynop2 driver which has the compatible name of "usb-nop-xceiv" > Is there a a driver for this? Does it get enabled? > If not, maybe we could update the imx8mm-u-u-boot.dtsi to restore the > power-domains to a driver that is present. > Adam, Ya, I think you were on the right track here. There is a driver (driver/phy/nop-phy.c) which does get enabled but with the dt sync the phy's power domain gets enabled after EHCI registers are accessed. I believe the fix we need is the following which moves phy setup before the register access (where it should have been along with the case for !defined(CONFIG_PHY): diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c index 91633f013a55..fae20838c60a 100644 --- a/drivers/usb/host/ehci-mx6.c +++ b/drivers/usb/host/ehci-mx6.c @@ -703,6 +703,10 @@ static int ehci_usb_probe(struct udevice *dev) usb_internal_phy_clock_gate(priv->phy_addr, 1); usb_phy_enable(ehci, priv->phy_addr); #endif +#else + ret = generic_setup_phy(dev, &priv->phy, 0); + if (ret) + goto err_regulator; #endif #if CONFIG_IS_ENABLED(DM_REGULATOR) @@ -725,12 +729,6 @@ static int ehci_usb_probe(struct udevice *dev) mdelay(10); -#if defined(CONFIG_PHY) - ret = generic_setup_phy(dev, &priv->phy, 0); - if (ret) - goto err_regulator; -#endif - hccr = (struct ehci_hccr *)((uintptr_t)&ehci->caplength); hcor = (struct ehci_hcor *)((uintptr_t)hccr + HC_LENGTH(ehci_readl(&(hccr)->cr_capbase))); If everyone agrees here I'll submit a formal patch which should get applied through Marek via the usb tree before the dt sync. Best Regards, Tim
Hi Tim, On Fri, Apr 28, 2023 at 2:59 PM Tim Harvey <tharvey@gateworks.com> wrote: > I believe the fix we need is the following which moves phy setup > before the register access (where it should have been along with the > case for !defined(CONFIG_PHY): ... > If everyone agrees here I'll submit a formal patch which should get > applied through Marek via the usb tree before the dt sync. This works for me, thanks. When you submit it, feel free to add: Tested-by: Fabio Estevam <festevam@denx.de>
On Fri, Apr 28, 2023 at 11:19 AM Fabio Estevam <festevam@gmail.com> wrote: > > Hi Tim, > > On Fri, Apr 28, 2023 at 2:59 PM Tim Harvey <tharvey@gateworks.com> wrote: > > > I believe the fix we need is the following which moves phy setup > > before the register access (where it should have been along with the > > case for !defined(CONFIG_PHY): > ... > > If everyone agrees here I'll submit a formal patch which should get > > applied through Marek via the usb tree before the dt sync. > > This works for me, thanks. > > When you submit it, feel free to add: > > Tested-by: Fabio Estevam <festevam@denx.de> Fabio, with commit bb6ea0fe9290 ("usb: ehci-mx6: move phy setup before register access") now in imx/master: Tested-by: Tim Harvey <tharvey@gateworks.com> #imx8mm-venice-gw73xx-0x Best Regards, Tim
On Wed, May 3, 2023 at 9:11 AM Tim Harvey <tharvey@gateworks.com> wrote: > > On Fri, Apr 28, 2023 at 11:19 AM Fabio Estevam <festevam@gmail.com> wrote: > > > > Hi Tim, > > > > On Fri, Apr 28, 2023 at 2:59 PM Tim Harvey <tharvey@gateworks.com> wrote: > > > > > I believe the fix we need is the following which moves phy setup > > > before the register access (where it should have been along with the > > > case for !defined(CONFIG_PHY): > > ... > > > If everyone agrees here I'll submit a formal patch which should get > > > applied through Marek via the usb tree before the dt sync. > > > > This works for me, thanks. > > > > When you submit it, feel free to add: > > > > Tested-by: Fabio Estevam <festevam@denx.de> > > Fabio, > > with commit bb6ea0fe9290 ("usb: ehci-mx6: move phy setup before > register access") now in imx/master: > Tested-by: Tim Harvey <tharvey@gateworks.com> #imx8mm-venice-gw73xx-0x > Stefano, It doesn't look like this got picked up in your latest tree for some reason. Best regards, Tim
Hi Tim, Fabio, On 14.07.23 02:42, Tim Harvey wrote: > On Wed, May 3, 2023 at 9:11 AM Tim Harvey <tharvey@gateworks.com> wrote: >> >> On Fri, Apr 28, 2023 at 11:19 AM Fabio Estevam <festevam@gmail.com> wrote: >>> >>> Hi Tim, >>> >>> On Fri, Apr 28, 2023 at 2:59 PM Tim Harvey <tharvey@gateworks.com> wrote: >>> >>>> I believe the fix we need is the following which moves phy setup >>>> before the register access (where it should have been along with the >>>> case for !defined(CONFIG_PHY): >>> ... >>>> If everyone agrees here I'll submit a formal patch which should get >>>> applied through Marek via the usb tree before the dt sync. >>> >>> This works for me, thanks. >>> >>> When you submit it, feel free to add: >>> >>> Tested-by: Fabio Estevam <festevam@denx.de> >> >> Fabio, >> >> with commit bb6ea0fe9290 ("usb: ehci-mx6: move phy setup before >> register access") now in imx/master: >> Tested-by: Tim Harvey <tharvey@gateworks.com> #imx8mm-venice-gw73xx-0x >> > > Stefano, > > It doesn't look like this got picked up in your latest tree for some reason. > Series disappeared from my list in patchworks, maybe because I erroneously thought that a V2 will be sent. I will pick up the series, thanks for advising. Best regards, Stefano > Best regards, > > Tim
On Thu, Jul 13, 2023 at 10:17 PM Stefano Babic <sbabic@denx.de> wrote: > > Hi Tim, Fabio, > > On 14.07.23 02:42, Tim Harvey wrote: > > On Wed, May 3, 2023 at 9:11 AM Tim Harvey <tharvey@gateworks.com> wrote: > >> > >> On Fri, Apr 28, 2023 at 11:19 AM Fabio Estevam <festevam@gmail.com> wrote: > >>> > >>> Hi Tim, > >>> > >>> On Fri, Apr 28, 2023 at 2:59 PM Tim Harvey <tharvey@gateworks.com> wrote: > >>> > >>>> I believe the fix we need is the following which moves phy setup > >>>> before the register access (where it should have been along with the > >>>> case for !defined(CONFIG_PHY): > >>> ... > >>>> If everyone agrees here I'll submit a formal patch which should get > >>>> applied through Marek via the usb tree before the dt sync. > >>> > >>> This works for me, thanks. > >>> > >>> When you submit it, feel free to add: > >>> > >>> Tested-by: Fabio Estevam <festevam@denx.de> > >> > >> Fabio, > >> > >> with commit bb6ea0fe9290 ("usb: ehci-mx6: move phy setup before > >> register access") now in imx/master: > >> Tested-by: Tim Harvey <tharvey@gateworks.com> #imx8mm-venice-gw73xx-0x > >> > > > > Stefano, > > > > It doesn't look like this got picked up in your latest tree for some reason. > > > > Series disappeared from my list in patchworks, maybe because I > erroneously thought that a V2 will be sent. I will pick up the series, > thanks for advising. > > Best regards, > Stefano > Hi Stefano, This series [1] is still missing - can you pick it up? [PATCH 1/3] arm: dts: imx8mm: Sync with Linux 6.3 [PATCH 2/3] arm: dts: imx8mn: Sync with Linux 6.3 [PATCH 3/3] arm: dts: imx8mp: Sync with Linux 6.3 Best Regards, Tim [1] https://patchwork.ozlabs.org/project/uboot/list/?series=352685&state=*
On 17.10.23 21:21, Tim Harvey wrote: > On Thu, Jul 13, 2023 at 10:17 PM Stefano Babic <sbabic@denx.de> wrote: >> >> Hi Tim, Fabio, >> >> On 14.07.23 02:42, Tim Harvey wrote: >>> On Wed, May 3, 2023 at 9:11 AM Tim Harvey <tharvey@gateworks.com> wrote: >>>> >>>> On Fri, Apr 28, 2023 at 11:19 AM Fabio Estevam <festevam@gmail.com> wrote: >>>>> >>>>> Hi Tim, >>>>> >>>>> On Fri, Apr 28, 2023 at 2:59 PM Tim Harvey <tharvey@gateworks.com> wrote: >>>>> >>>>>> I believe the fix we need is the following which moves phy setup >>>>>> before the register access (where it should have been along with the >>>>>> case for !defined(CONFIG_PHY): >>>>> ... >>>>>> If everyone agrees here I'll submit a formal patch which should get >>>>>> applied through Marek via the usb tree before the dt sync. >>>>> >>>>> This works for me, thanks. >>>>> >>>>> When you submit it, feel free to add: >>>>> >>>>> Tested-by: Fabio Estevam <festevam@denx.de> >>>> >>>> Fabio, >>>> >>>> with commit bb6ea0fe9290 ("usb: ehci-mx6: move phy setup before >>>> register access") now in imx/master: >>>> Tested-by: Tim Harvey <tharvey@gateworks.com> #imx8mm-venice-gw73xx-0x >>>> >>> >>> Stefano, >>> >>> It doesn't look like this got picked up in your latest tree for some reason. >>> >> >> Series disappeared from my list in patchworks, maybe because I >> erroneously thought that a V2 will be sent. I will pick up the series, >> thanks for advising. >> >> Best regards, >> Stefano >> > > Hi Stefano, > > This series [1] is still missing - can you pick it up? > > [PATCH 1/3] arm: dts: imx8mm: Sync with Linux 6.3 > [PATCH 2/3] arm: dts: imx8mn: Sync with Linux 6.3 > [PATCH 3/3] arm: dts: imx8mp: Sync with Linux 6.3 > > Best Regards, > > Tim > [1] https://patchwork.ozlabs.org/project/uboot/list/?series=352685&state=* It was set as "Superseeded", that is the reason why it disappeared by my list. Thanks for pointing out, I have added again to be merged. Regards, Stefano
Hi Fabio, Tim, On 17.10.23 22:28, Stefano Babic wrote: > > > On 17.10.23 21:21, Tim Harvey wrote: >> On Thu, Jul 13, 2023 at 10:17 PM Stefano Babic <sbabic@denx.de> wrote: >>> >>> Hi Tim, Fabio, >>> >>> On 14.07.23 02:42, Tim Harvey wrote: >>>> On Wed, May 3, 2023 at 9:11 AM Tim Harvey <tharvey@gateworks.com> >>>> wrote: >>>>> >>>>> On Fri, Apr 28, 2023 at 11:19 AM Fabio Estevam <festevam@gmail.com> >>>>> wrote: >>>>>> >>>>>> Hi Tim, >>>>>> >>>>>> On Fri, Apr 28, 2023 at 2:59 PM Tim Harvey <tharvey@gateworks.com> >>>>>> wrote: >>>>>> >>>>>>> I believe the fix we need is the following which moves phy setup >>>>>>> before the register access (where it should have been along with the >>>>>>> case for !defined(CONFIG_PHY): >>>>>> ... >>>>>>> If everyone agrees here I'll submit a formal patch which should get >>>>>>> applied through Marek via the usb tree before the dt sync. >>>>>> >>>>>> This works for me, thanks. >>>>>> >>>>>> When you submit it, feel free to add: >>>>>> >>>>>> Tested-by: Fabio Estevam <festevam@denx.de> >>>>> >>>>> Fabio, >>>>> >>>>> with commit bb6ea0fe9290 ("usb: ehci-mx6: move phy setup before >>>>> register access") now in imx/master: >>>>> Tested-by: Tim Harvey <tharvey@gateworks.com> #imx8mm-venice-gw73xx-0x >>>>> >>>> >>>> Stefano, >>>> >>>> It doesn't look like this got picked up in your latest tree for some >>>> reason. >>>> >>> >>> Series disappeared from my list in patchworks, maybe because I >>> erroneously thought that a V2 will be sent. I will pick up the series, >>> thanks for advising. >>> >>> Best regards, >>> Stefano >>> >> >> Hi Stefano, >> >> This series [1] is still missing - can you pick it up? >> >> [PATCH 1/3] arm: dts: imx8mm: Sync with Linux 6.3 >> [PATCH 2/3] arm: dts: imx8mn: Sync with Linux 6.3 >> [PATCH 3/3] arm: dts: imx8mp: Sync with Linux 6.3 >> >> Best Regards, >> >> Tim >> [1] >> https://patchwork.ozlabs.org/project/uboot/list/?series=352685&state=* > > > It was set as "Superseeded", that is the reason why it disappeared by my > list. Thanks for pointing out, I have added again to be merged. > Rather they were forgotten, and they need a rebase. Can you check ? Best regards, Stefano
diff --git a/arch/arm/dts/imx8mm.dtsi b/arch/arm/dts/imx8mm.dtsi index afb90f59c83c..31f4548f85cf 100644 --- a/arch/arm/dts/imx8mm.dtsi +++ b/arch/arm/dts/imx8mm.dtsi @@ -139,6 +139,7 @@ A53_L2: l2-cache0 { compatible = "cache"; cache-level = <2>; + cache-unified; cache-size = <0x80000>; cache-line-size = <64>; cache-sets = <512>; @@ -276,6 +277,7 @@ assigned-clocks = <&clk IMX8MM_CLK_USB_PHY_REF>; assigned-clock-parents = <&clk IMX8MM_SYS_PLL1_100M>; clock-names = "main_clk"; + power-domains = <&pgc_otg1>; }; usbphynop2: usbphynop2 { @@ -285,6 +287,7 @@ assigned-clocks = <&clk IMX8MM_CLK_USB_PHY_REF>; assigned-clock-parents = <&clk IMX8MM_SYS_PLL1_100M>; clock-names = "main_clk"; + power-domains = <&pgc_otg2>; }; soc: soc@0 { @@ -493,6 +496,8 @@ compatible = "fsl,imx8mm-tmu"; reg = <0x30260000 0x10000>; clocks = <&clk IMX8MM_CLK_TMU_ROOT>; + nvmem-cells = <&tmu_calib>; + nvmem-cell-names = "calib"; #thermal-sensor-cells = <0>; }; @@ -547,8 +552,8 @@ reg = <0x30330000 0x10000>; }; - gpr: iomuxc-gpr@30340000 { - compatible = "fsl,imx8mm-iomuxc-gpr", "fsl,imx6q-iomuxc-gpr", "syscon"; + gpr: syscon@30340000 { + compatible = "fsl,imx8mm-iomuxc-gpr", "syscon"; reg = <0x30340000 0x10000>; }; @@ -560,22 +565,40 @@ #address-cells = <1>; #size-cells = <1>; - imx8mm_uid: unique-id@410 { + /* + * The register address below maps to the MX8M + * Fusemap Description Table entries this way. + * Assuming + * reg = <ADDR SIZE>; + * then + * Fuse Address = (ADDR * 4) + 0x400 + * Note that if SIZE is greater than 4, then + * each subsequent fuse is located at offset + * +0x10 in Fusemap Description Table (e.g. + * reg = <0x4 0x8> describes fuses 0x410 and + * 0x420). + */ + imx8mm_uid: unique-id@4 { /* 0x410-0x420 */ reg = <0x4 0x8>; }; - cpu_speed_grade: speed-grade@10 { + cpu_speed_grade: speed-grade@10 { /* 0x440 */ reg = <0x10 4>; }; - fec_mac_address: mac-address@90 { + tmu_calib: calib@3c { /* 0x4f0 */ + reg = <0x3c 4>; + }; + + fec_mac_address: mac-address@90 { /* 0x640 */ reg = <0x90 6>; }; }; - anatop: anatop@30360000 { - compatible = "fsl,imx8mm-anatop", "syscon"; + anatop: clock-controller@30360000 { + compatible = "fsl,imx8mm-anatop"; reg = <0x30360000 0x10000>; + #clock-cells = <1>; }; snvs: snvs@30370000 { @@ -674,13 +697,11 @@ pgc_otg1: power-domain@2 { #power-domain-cells = <0>; reg = <IMX8MM_POWER_DOMAIN_OTG1>; - power-domains = <&pgc_hsiomix>; }; pgc_otg2: power-domain@3 { #power-domain-cells = <0>; reg = <IMX8MM_POWER_DOMAIN_OTG2>; - power-domains = <&pgc_hsiomix>; }; pgc_gpumix: power-domain@4 { @@ -1186,7 +1207,7 @@ assigned-clock-parents = <&clk IMX8MM_SYS_PLL2_500M>; phys = <&usbphynop1>; fsl,usbmisc = <&usbmisc1 0>; - power-domains = <&pgc_otg1>; + power-domains = <&pgc_hsiomix>; status = "disabled"; }; @@ -1206,7 +1227,7 @@ assigned-clock-parents = <&clk IMX8MM_SYS_PLL2_500M>; phys = <&usbphynop2>; fsl,usbmisc = <&usbmisc2 0>; - power-domains = <&pgc_otg2>; + power-domains = <&pgc_hsiomix>; status = "disabled"; }; @@ -1238,16 +1259,15 @@ <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>; - interrupt-names = "gpmi0", "gpmi1", "gpmi2", "gpmi3"; #dma-cells = <1>; dma-channels = <4>; clocks = <&clk IMX8MM_CLK_NAND_USDHC_BUS_RAWNAND_CLK>; }; - gpmi: nand-controller@33002000{ + gpmi: nand-controller@33002000 { compatible = "fsl,imx8mm-gpmi-nand", "fsl,imx7d-gpmi-nand"; #address-cells = <1>; - #size-cells = <1>; + #size-cells = <0>; reg = <0x33002000 0x2000>, <0x33004000 0x4000>; reg-names = "gpmi-nand", "bch"; interrupts = <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>; @@ -1282,6 +1302,10 @@ <0 0 0 4 &gic GIC_SPI 122 IRQ_TYPE_LEVEL_HIGH>; fsl,max-link-speed = <2>; linux,pci-domain = <0>; + clocks = <&clk IMX8MM_CLK_PCIE1_ROOT>, + <&clk IMX8MM_CLK_PCIE1_PHY>, + <&clk IMX8MM_CLK_PCIE1_AUX>; + clock-names = "pcie", "pcie_bus", "pcie_aux"; power-domains = <&pgc_pcie>; resets = <&src IMX8MQ_RESET_PCIE_CTRL_APPS_EN>, <&src IMX8MQ_RESET_PCIE_CTRL_APPS_TURNOFF>;