| Message ID | 20210126124004.52550-1-tony@atomide.com |
|---|---|
| Headers | show |
| Series | Update dra7 devicetree files to probe with genpd | expand |
Hi, * Tony Lindgren <tony@atomide.com> [210126 12:41]: > After updating pci-dra7xx driver to probe with ti-sysc and genpd, I > noticed that dra7xx_pcie_probe() would not run if a power-domains property > was configured for the interconnect target module. > > Turns out that module_platform_driver_probe uses platform_driver_probe(), > while module_platform_driver_probe uses platform_driver_register(). I just noticed that the above is confusing with module_platform_driver_probe mentioned twice. It should be: Turns out that module_platform_driver_probe uses platform_driver_probe(), while builtin_platform_driver uses platform_driver_register(). Regards, Tony
On 26/01/21 6:09 pm, Tony Lindgren wrote: > Hi all, > > Here's v2 series to update dra7 to probe with ti-sysc and genpd like we've > already done for am3 and 4. > > These patches are against v5.11-rc1, and depend on the following commits > in my fixes branch: > > 7078a5ba7a58 ("soc: ti: omap-prm: Fix boot time errors for rst_map_012 bits 0 and 1") > 2a39af3870e9 ("ARM: OMAP2+: Fix booting for am335x after moving to simple-pm-bus") > > These patches also depend on the series: > > [PATCH 0/3] Few ti-sysc changes for v5.12 merge window > > Please review and test, I've also pushed out a temporary testing branch to > make testing easier to [0][1]. Looks good to me and didn't observe any issues in my testing. Tested-by: Kishon Vijay Abraham I <kishon@ti.com> > > Regards, > > Tony > > Changes since v1: > > - Split the series into two parts, looks like most of the emails did not > make it to the lists > > - Dropped Balaji from Cc as the email address bounces > > [0] git://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git tmp-testing-genpd-dra7 > [1] https://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git/log/?h=omap-for-v5.12/tmp-testing-genpd-dra7 > > Tony Lindgren (15): > PCI: pci-dra7xx: Prepare for deferred probe with > module_platform_driver > ARM: dts: Update pcie ranges for dra7 > ARM: dts: Configure interconnect target module for dra7 pcie > ARM: dts: Properly configure dra7 edma sysconfig registers > ARM: dts: Move dra7 l3 noc to a separate node > ARM: dts: Configure interconnect target module for dra7 qspi > ARM: dts: Configure interconnect target module for dra7 sata > ARM: dts: Configure interconnect target module for dra7 mpu > ARM: dts: Configure interconnect target module for dra7 dmm > ARM: dts: Configure simple-pm-bus for dra7 l4_wkup > ARM: dts: Configure simple-pm-bus for dra7 l4_per1 > ARM: dts: Configure simple-pm-bus for dra7 l4_per2 > ARM: dts: Configure simple-pm-bus for dra7 l4_per3 > ARM: dts: Configure simple-pm-bus for dra7 l4_cfg > ARM: dts: Configure simple-pm-bus for dra7 l3 > > arch/arm/boot/dts/dra7-l4.dtsi | 76 ++++++--- > arch/arm/boot/dts/dra7.dtsi | 216 ++++++++++++++++-------- > drivers/pci/controller/dwc/pci-dra7xx.c | 13 +- > 3 files changed, 213 insertions(+), 92 deletions(-) >
Hello Tony, Roger, All, Le 26/01/2021 à 13:39, Tony Lindgren a écrit : > We can now probe devices with device tree only configuration using > ti-sysc interconnect target module driver. Let's configure the > module, but keep the legacy "ti,hwmods" peroperty to avoid new boot > time warnings. The legacy property will be removed in later patches > together with the legacy platform data. > > Note that the old sysc register offset is wrong, the real offset is at > 0x1100 as listed in TRM for SATA_SYSCONFIG register. Looks like we've been > happily using sata on the bootloader configured sysconfig register and > nobody noticed. Also the old register range for SATAMAC_wrapper registers > is wrong at 7 while it should be 8. But that too seems harmless. > > There is also an L3 parent interconnect range that we don't seem to be > using. That can be added as needed later on. Since the switch from a kernel 5.10 to 6.1, the dra7 (AM574x) sata interface doesn't work as expected. Using a kernel 6.1 with a preformated ext4 SATA disc, any copied file will be corrupted when the filesystem is umounted. mount /dev/sda1 /mnt cp /<test_file> /mnt/ sync sha256sum /mnt/<test_file> /<test_file> <same hash> umount /mnt mount /dev/sda1 /mnt sha256sum /mnt/<test_file> /<test_file> /mnt/<test_file> is corrupted. git bisect report 8af15365a368 ("ARM: dts: Configure interconnect target module for dra7 sata") as the first bad commit [1] (merged in 5.13). While looking for existing SATA issue on dra7 SoC, I found this old patch: "On TI Platforms using LPAE, SATA breaks with 64-bit DMA. Restrict it to 32-bit." [2]. Even if it's not the correct fix, disabling 64-bit DMA allows to use the sata disc correctly. The discussion about this issue seems to have stopped [3] and the suggested change was never merged. The SATA port is unlikely not available on TI AM57 EVM boards or the beaglebone-AI. Any suggestion? [1] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=8af15365a36845c4c15d4c8046ddccff331d5263 [2] https://lore.kernel.org/all/20200206111728.6703-1-rogerq@ti.com/T/ [3] https://lore.kernel.org/lkml/c753a232-403d-6ed2-89fd-09476c887391@ti.com/ Best regards, Romain > > Signed-off-by: Tony Lindgren <tony@atomide.com> > --- > arch/arm/boot/dts/dra7-l4.dtsi | 29 ++++++++++++++++++++++++++--- > arch/arm/boot/dts/dra7.dtsi | 12 ------------ > 2 files changed, 26 insertions(+), 15 deletions(-) > > diff --git a/arch/arm/boot/dts/dra7-l4.dtsi b/arch/arm/boot/dts/dra7-l4.dtsi > --- a/arch/arm/boot/dts/dra7-l4.dtsi > +++ b/arch/arm/boot/dts/dra7-l4.dtsi > @@ -572,11 +572,34 @@ target-module@8000 { /* 0x4a108000, ap 29 1e.0 */ > }; > > target-module@40000 { /* 0x4a140000, ap 31 06.0 */ > - compatible = "ti,sysc"; > - status = "disabled"; > - #address-cells = <1>; > + compatible = "ti,sysc-omap4", "ti,sysc"; > + ti,hwmods = "sata"; > + reg = <0x400fc 4>, > + <0x41100 4>; > + reg-names = "rev", "sysc"; > + ti,sysc-midle = <SYSC_IDLE_FORCE>, > + <SYSC_IDLE_NO>, > + <SYSC_IDLE_SMART>; > + ti,sysc-sidle = <SYSC_IDLE_FORCE>, > + <SYSC_IDLE_NO>, > + <SYSC_IDLE_SMART>, > + <SYSC_IDLE_SMART_WKUP>; > + power-domains = <&prm_l3init>; > + clocks = <&l3init_clkctrl DRA7_L3INIT_SATA_CLKCTRL 0>; > + clock-names = "fck"; > #size-cells = <1>; > + #address-cells = <1>; > ranges = <0x0 0x40000 0x10000>; > + > + sata: sata@0 { > + compatible = "snps,dwc-ahci"; > + reg = <0 0x1100>, <0x1100 0x8>; > + interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>; > + phys = <&sata_phy>; > + phy-names = "sata-phy"; > + clocks = <&l3init_clkctrl DRA7_L3INIT_SATA_CLKCTRL 8>; > + ports-implemented = <0x1>; > + }; > }; > > target-module@51000 { /* 0x4a151000, ap 33 50.0 */ > diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi > --- a/arch/arm/boot/dts/dra7.dtsi > +++ b/arch/arm/boot/dts/dra7.dtsi > @@ -785,18 +785,6 @@ qspi: spi@0 { > }; > }; > > - /* OCP2SCP3 */ > - sata: sata@4a141100 { > - compatible = "snps,dwc-ahci"; > - reg = <0x4a140000 0x1100>, <0x4a141100 0x7>; > - interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>; > - phys = <&sata_phy>; > - phy-names = "sata-phy"; > - clocks = <&l3init_clkctrl DRA7_L3INIT_SATA_CLKCTRL 8>; > - ti,hwmods = "sata"; > - ports-implemented = <0x1>; > - }; > - > /* OCP2SCP1 */ > /* IRQ for DWC3_3 and DWC3_4 need IRQ crossbar */ >
Hello, Le 12/11/2024 à 15:15, Romain Naour a écrit : > Hello Tony, Roger, All, > > Le 26/01/2021 à 13:39, Tony Lindgren a écrit : >> We can now probe devices with device tree only configuration using >> ti-sysc interconnect target module driver. Let's configure the >> module, but keep the legacy "ti,hwmods" peroperty to avoid new boot >> time warnings. The legacy property will be removed in later patches >> together with the legacy platform data. >> >> Note that the old sysc register offset is wrong, the real offset is at >> 0x1100 as listed in TRM for SATA_SYSCONFIG register. Looks like we've been >> happily using sata on the bootloader configured sysconfig register and >> nobody noticed. Also the old register range for SATAMAC_wrapper registers >> is wrong at 7 while it should be 8. But that too seems harmless. >> >> There is also an L3 parent interconnect range that we don't seem to be >> using. That can be added as needed later on. > > Since the switch from a kernel 5.10 to 6.1, the dra7 (AM574x) sata interface > doesn't work as expected. > > Using a kernel 6.1 with a preformated ext4 SATA disc, any copied file will be > corrupted when the filesystem is umounted. > > mount /dev/sda1 /mnt > cp /<test_file> /mnt/ > sync > sha256sum /mnt/<test_file> /<test_file> > <same hash> > umount /mnt > > mount /dev/sda1 /mnt > sha256sum /mnt/<test_file> /<test_file> > /mnt/<test_file> is corrupted. > > git bisect report 8af15365a368 ("ARM: dts: Configure interconnect target module > for dra7 sata") as the first bad commit [1] (merged in 5.13). > > While looking for existing SATA issue on dra7 SoC, I found this old patch: > > "On TI Platforms using LPAE, SATA breaks with 64-bit DMA. Restrict it to > 32-bit." [2]. > > Even if it's not the correct fix, disabling 64-bit DMA allows to use the sata > disc correctly. The discussion about this issue seems to have stopped [3] and > the suggested change was never merged. > > The SATA port is unlikely not available on TI AM57 EVM boards or the beaglebone-AI. > > Any suggestion? It seems we have to use a pseudo-bus to constrain sata dma size (see [1]) But the sata node is placed inside a "ti,sysc-omap4" node, it's not clear if we can do that. target-module@40000 { /* 0x4a140000, ap 31 06.0 */ compatible = "ti,sysc-omap4", "ti,sysc"; reg = <0x400fc 4>, <0x41100 4>; reg-names = "rev", "sysc"; ti,sysc-midle = <SYSC_IDLE_FORCE>, <SYSC_IDLE_NO>, <SYSC_IDLE_SMART>; ti,sysc-sidle = <SYSC_IDLE_FORCE>, <SYSC_IDLE_NO>, <SYSC_IDLE_SMART>, <SYSC_IDLE_SMART_WKUP>; power-domains = <&prm_l3init>; clocks = <&l3init_clkctrl DRA7_L3INIT_SATA_CLKCTRL 0>; clock-names = "fck"; #size-cells = <2>; #address-cells = <2>; // [2] parent-bus-address ranges = <0x0 0x0 0x40000 0x0 0x10000>; aux_bus: aux_bus { #address-cells = <2>; // [1] child-bus-address #size-cells = <2>; // [3] length compatible = "simple-bus"; ranges; dma-ranges = <0x0 0x0 0x0 0x0 0x1 0x00000000>; /* | [1] |[2] | [3] | */ /* dma-ranges = <child-bus-address, parent-bus-address, length> */ sata: sata@0 { compatible = "snps,dwc-ahci"; reg = <0x0 0x0 0x0 0x1100>, <0x0 0x0 0x0 0x8>; interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>; phys = <&sata_phy>; phy-names = "sata-phy"; clocks = <&l3init_clkctrl DRA7_L3INIT_SATA_CLKCTRL 8>; ports-implemented = <0x1>; }; }; }; Note: ti,sysc-omap4 doesn't allows anything than #address-cells = <1> and #size-cells = <1> before commit [2]. bus: ti-sysc: Remove open coded "ranges" parsing "ranges" is a standard property and we have common helper functions for parsing it, so let's use them. [1] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=2a2ab4d5206d25875e30a8a8153f0b4f3c951ee4 [2] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=41e4f807f85d02a44426b87e01ed98b191dbbf9d Best regards, Romain > > [1] > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=8af15365a36845c4c15d4c8046ddccff331d5263 > [2] https://lore.kernel.org/all/20200206111728.6703-1-rogerq@ti.com/T/ > [3] https://lore.kernel.org/lkml/c753a232-403d-6ed2-89fd-09476c887391@ti.com/ > > Best regards, > Romain > > >>
On 2024-11-14 10:22 am, Romain Naour wrote: > Hello, > > Le 12/11/2024 à 15:15, Romain Naour a écrit : >> Hello Tony, Roger, All, >> >> Le 26/01/2021 à 13:39, Tony Lindgren a écrit : >>> We can now probe devices with device tree only configuration using >>> ti-sysc interconnect target module driver. Let's configure the >>> module, but keep the legacy "ti,hwmods" peroperty to avoid new boot >>> time warnings. The legacy property will be removed in later patches >>> together with the legacy platform data. >>> >>> Note that the old sysc register offset is wrong, the real offset is at >>> 0x1100 as listed in TRM for SATA_SYSCONFIG register. Looks like we've been >>> happily using sata on the bootloader configured sysconfig register and >>> nobody noticed. Also the old register range for SATAMAC_wrapper registers >>> is wrong at 7 while it should be 8. But that too seems harmless. >>> >>> There is also an L3 parent interconnect range that we don't seem to be >>> using. That can be added as needed later on. >> >> Since the switch from a kernel 5.10 to 6.1, the dra7 (AM574x) sata interface >> doesn't work as expected. >> >> Using a kernel 6.1 with a preformated ext4 SATA disc, any copied file will be >> corrupted when the filesystem is umounted. >> >> mount /dev/sda1 /mnt >> cp /<test_file> /mnt/ >> sync >> sha256sum /mnt/<test_file> /<test_file> >> <same hash> >> umount /mnt >> >> mount /dev/sda1 /mnt >> sha256sum /mnt/<test_file> /<test_file> >> /mnt/<test_file> is corrupted. >> >> git bisect report 8af15365a368 ("ARM: dts: Configure interconnect target module >> for dra7 sata") as the first bad commit [1] (merged in 5.13). >> >> While looking for existing SATA issue on dra7 SoC, I found this old patch: >> >> "On TI Platforms using LPAE, SATA breaks with 64-bit DMA. Restrict it to >> 32-bit." [2]. >> >> Even if it's not the correct fix, disabling 64-bit DMA allows to use the sata >> disc correctly. The discussion about this issue seems to have stopped [3] and >> the suggested change was never merged. >> >> The SATA port is unlikely not available on TI AM57 EVM boards or the beaglebone-AI. >> >> Any suggestion? > > It seems we have to use a pseudo-bus to constrain sata dma size (see [1]) No, the target-module node already represents a parent "bus", at least as far as the DT abstraction cares - just add a dma-ranges property there. Anything which has a ranges property to represent passing MMIO traffic downstream can equally have dma-ranges to represent DMA traffic coming back upstream. Thanks, Robin. > But the sata node is placed inside a "ti,sysc-omap4" node, it's not clear if we > can do that. > > target-module@40000 { /* 0x4a140000, ap 31 06.0 */ > compatible = "ti,sysc-omap4", "ti,sysc"; > reg = <0x400fc 4>, > <0x41100 4>; > reg-names = "rev", "sysc"; > ti,sysc-midle = <SYSC_IDLE_FORCE>, > <SYSC_IDLE_NO>, > <SYSC_IDLE_SMART>; > ti,sysc-sidle = <SYSC_IDLE_FORCE>, > <SYSC_IDLE_NO>, > <SYSC_IDLE_SMART>, > <SYSC_IDLE_SMART_WKUP>; > power-domains = <&prm_l3init>; > clocks = <&l3init_clkctrl DRA7_L3INIT_SATA_CLKCTRL 0>; > clock-names = "fck"; > > #size-cells = <2>; > #address-cells = <2>; // [2] parent-bus-address > ranges = <0x0 0x0 0x40000 0x0 0x10000>; > > aux_bus: aux_bus { > #address-cells = <2>; // [1] child-bus-address > #size-cells = <2>; // [3] length > compatible = "simple-bus"; > ranges; > dma-ranges = <0x0 0x0 0x0 0x0 0x1 0x00000000>; > /* | [1] |[2] | [3] | */ > /* dma-ranges = <child-bus-address, parent-bus-address, length> */ > > sata: sata@0 { > compatible = "snps,dwc-ahci"; > reg = <0x0 0x0 0x0 0x1100>, <0x0 0x0 0x0 0x8>; > interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>; > phys = <&sata_phy>; > phy-names = "sata-phy"; > clocks = <&l3init_clkctrl DRA7_L3INIT_SATA_CLKCTRL 8>; > ports-implemented = <0x1>; > }; > }; > }; > > Note: ti,sysc-omap4 doesn't allows anything than #address-cells = <1> and > #size-cells = <1> before commit [2]. > > bus: ti-sysc: Remove open coded "ranges" parsing > > "ranges" is a standard property and we have common helper functions for > parsing it, so let's use them. > > > [1] > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=2a2ab4d5206d25875e30a8a8153f0b4f3c951ee4 > > [2] > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=41e4f807f85d02a44426b87e01ed98b191dbbf9d > > Best regards, > Romain > > >> >> [1] >> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=8af15365a36845c4c15d4c8046ddccff331d5263 >> [2] https://lore.kernel.org/all/20200206111728.6703-1-rogerq@ti.com/T/ >> [3] https://lore.kernel.org/lkml/c753a232-403d-6ed2-89fd-09476c887391@ti.com/ >> >> Best regards, >> Romain >> >> >>> >
Hi Romain, On 12/11/2024 16:15, Romain Naour wrote: > Hello Tony, Roger, All, > > Le 26/01/2021 à 13:39, Tony Lindgren a écrit : >> We can now probe devices with device tree only configuration using >> ti-sysc interconnect target module driver. Let's configure the >> module, but keep the legacy "ti,hwmods" peroperty to avoid new boot >> time warnings. The legacy property will be removed in later patches >> together with the legacy platform data. >> >> Note that the old sysc register offset is wrong, the real offset is at >> 0x1100 as listed in TRM for SATA_SYSCONFIG register. Looks like we've been >> happily using sata on the bootloader configured sysconfig register and >> nobody noticed. Also the old register range for SATAMAC_wrapper registers >> is wrong at 7 while it should be 8. But that too seems harmless. >> >> There is also an L3 parent interconnect range that we don't seem to be >> using. That can be added as needed later on. > > Since the switch from a kernel 5.10 to 6.1, the dra7 (AM574x) sata interface > doesn't work as expected. > > Using a kernel 6.1 with a preformated ext4 SATA disc, any copied file will be > corrupted when the filesystem is umounted. > > mount /dev/sda1 /mnt > cp /<test_file> /mnt/ > sync > sha256sum /mnt/<test_file> /<test_file> > <same hash> > umount /mnt > > mount /dev/sda1 /mnt > sha256sum /mnt/<test_file> /<test_file> > /mnt/<test_file> is corrupted. > > git bisect report 8af15365a368 ("ARM: dts: Configure interconnect target module > for dra7 sata") as the first bad commit [1] (merged in 5.13). > > While looking for existing SATA issue on dra7 SoC, I found this old patch: > > "On TI Platforms using LPAE, SATA breaks with 64-bit DMA. Restrict it to > 32-bit." [2]. > > Even if it's not the correct fix, disabling 64-bit DMA allows to use the sata > disc correctly. The discussion about this issue seems to have stopped [3] and > the suggested change was never merged. If I remember right the following commit fixed the issue back then. cfb5d65f2595 ARM: dts: dra7: Add bus_dma_limit for L3 bus But, when commit [1] moved the SATA node from L3 bus to L4_cfg it lost the bus_dma_limit that we added at the L3 bus and hence the regression. I think we should add the same 2GB dma ranges limit into l4_cfg bus so all modules can inherit it. > > The SATA port is unlikely not available on TI AM57 EVM boards or the beaglebone-AI. > > Any suggestion? > > [1] > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=8af15365a36845c4c15d4c8046ddccff331d5263 > [2] https://lore.kernel.org/all/20200206111728.6703-1-rogerq@ti.com/T/ > [3] https://lore.kernel.org/lkml/c753a232-403d-6ed2-89fd-09476c887391@ti.com/ > > Best regards, > Romain > > >> >> Signed-off-by: Tony Lindgren <tony@atomide.com> >> --- >> arch/arm/boot/dts/dra7-l4.dtsi | 29 ++++++++++++++++++++++++++--- >> arch/arm/boot/dts/dra7.dtsi | 12 ------------ >> 2 files changed, 26 insertions(+), 15 deletions(-) >> >> diff --git a/arch/arm/boot/dts/dra7-l4.dtsi b/arch/arm/boot/dts/dra7-l4.dtsi >> --- a/arch/arm/boot/dts/dra7-l4.dtsi >> +++ b/arch/arm/boot/dts/dra7-l4.dtsi >> @@ -572,11 +572,34 @@ target-module@8000 { /* 0x4a108000, ap 29 1e.0 */ >> }; >> >> target-module@40000 { /* 0x4a140000, ap 31 06.0 */ >> - compatible = "ti,sysc"; >> - status = "disabled"; >> - #address-cells = <1>; >> + compatible = "ti,sysc-omap4", "ti,sysc"; >> + ti,hwmods = "sata"; >> + reg = <0x400fc 4>, >> + <0x41100 4>; >> + reg-names = "rev", "sysc"; >> + ti,sysc-midle = <SYSC_IDLE_FORCE>, >> + <SYSC_IDLE_NO>, >> + <SYSC_IDLE_SMART>; >> + ti,sysc-sidle = <SYSC_IDLE_FORCE>, >> + <SYSC_IDLE_NO>, >> + <SYSC_IDLE_SMART>, >> + <SYSC_IDLE_SMART_WKUP>; >> + power-domains = <&prm_l3init>; >> + clocks = <&l3init_clkctrl DRA7_L3INIT_SATA_CLKCTRL 0>; >> + clock-names = "fck"; >> #size-cells = <1>; >> + #address-cells = <1>; >> ranges = <0x0 0x40000 0x10000>; >> + >> + sata: sata@0 { >> + compatible = "snps,dwc-ahci"; >> + reg = <0 0x1100>, <0x1100 0x8>; >> + interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>; >> + phys = <&sata_phy>; >> + phy-names = "sata-phy"; >> + clocks = <&l3init_clkctrl DRA7_L3INIT_SATA_CLKCTRL 8>; >> + ports-implemented = <0x1>; >> + }; >> }; >> >> target-module@51000 { /* 0x4a151000, ap 33 50.0 */ >> diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi >> --- a/arch/arm/boot/dts/dra7.dtsi >> +++ b/arch/arm/boot/dts/dra7.dtsi >> @@ -785,18 +785,6 @@ qspi: spi@0 { >> }; >> }; >> >> - /* OCP2SCP3 */ >> - sata: sata@4a141100 { >> - compatible = "snps,dwc-ahci"; >> - reg = <0x4a140000 0x1100>, <0x4a141100 0x7>; >> - interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>; >> - phys = <&sata_phy>; >> - phy-names = "sata-phy"; >> - clocks = <&l3init_clkctrl DRA7_L3INIT_SATA_CLKCTRL 8>; >> - ti,hwmods = "sata"; >> - ports-implemented = <0x1>; >> - }; >> - >> /* OCP2SCP1 */ >> /* IRQ for DWC3_3 and DWC3_4 need IRQ crossbar */ >>
Hi Roger, Robin, All, Le 14/11/2024 à 12:02, Roger Quadros a écrit : > Hi Romain, > > On 12/11/2024 16:15, Romain Naour wrote: >> Hello Tony, Roger, All, >> >> Le 26/01/2021 à 13:39, Tony Lindgren a écrit : >>> We can now probe devices with device tree only configuration using >>> ti-sysc interconnect target module driver. Let's configure the >>> module, but keep the legacy "ti,hwmods" peroperty to avoid new boot >>> time warnings. The legacy property will be removed in later patches >>> together with the legacy platform data. >>> >>> Note that the old sysc register offset is wrong, the real offset is at >>> 0x1100 as listed in TRM for SATA_SYSCONFIG register. Looks like we've been >>> happily using sata on the bootloader configured sysconfig register and >>> nobody noticed. Also the old register range for SATAMAC_wrapper registers >>> is wrong at 7 while it should be 8. But that too seems harmless. >>> >>> There is also an L3 parent interconnect range that we don't seem to be >>> using. That can be added as needed later on. >> >> Since the switch from a kernel 5.10 to 6.1, the dra7 (AM574x) sata interface >> doesn't work as expected. >> >> Using a kernel 6.1 with a preformated ext4 SATA disc, any copied file will be >> corrupted when the filesystem is umounted. >> >> mount /dev/sda1 /mnt >> cp /<test_file> /mnt/ >> sync >> sha256sum /mnt/<test_file> /<test_file> >> <same hash> >> umount /mnt >> >> mount /dev/sda1 /mnt >> sha256sum /mnt/<test_file> /<test_file> >> /mnt/<test_file> is corrupted. >> >> git bisect report 8af15365a368 ("ARM: dts: Configure interconnect target module >> for dra7 sata") as the first bad commit [1] (merged in 5.13). >> >> While looking for existing SATA issue on dra7 SoC, I found this old patch: >> >> "On TI Platforms using LPAE, SATA breaks with 64-bit DMA. Restrict it to >> 32-bit." [2]. >> >> Even if it's not the correct fix, disabling 64-bit DMA allows to use the sata >> disc correctly. The discussion about this issue seems to have stopped [3] and >> the suggested change was never merged. > > If I remember right the following commit fixed the issue back then. > > cfb5d65f2595 ARM: dts: dra7: Add bus_dma_limit for L3 bus > > But, when commit [1] moved the SATA node from L3 bus to L4_cfg it lost the bus_dma_limit > that we added at the L3 bus and hence the regression. > > I think we should add the same 2GB dma ranges limit into l4_cfg bus so all modules > can inherit it. Thanks for your reply! It seems l4_cfg can inherit dma-ranges property from ocp node using "dma-ranges;". But then segment@100000 node (0x4a100000) needs "dma-ranges;" too. With the following patch applied, the SATA drive works correctly. diff --git a/arch/arm/boot/dts/dra7-l4.dtsi b/arch/arm/boot/dts/dra7-l4.dtsi index 1aaffd034c39..3ac770298844 100644 --- a/arch/arm/boot/dts/dra7-l4.dtsi +++ b/arch/arm/boot/dts/dra7-l4.dtsi @@ -12,6 +12,7 @@ &l4_cfg { /* 0x4a000000 */ ranges = <0x00000000 0x4a000000 0x100000>, /* segment 0 */ <0x00100000 0x4a100000 0x100000>, /* segment 1 */ <0x00200000 0x4a200000 0x100000>; /* segment 2 */ + dma-ranges; segment@0 { /* 0x4a000000 */ compatible = "simple-pm-bus"; @@ -557,6 +558,7 @@ segment@100000 { /* 0x4a100000 */ <0x0007e000 0x0017e000 0x001000>, /* ap 124 */ <0x00059000 0x00159000 0x001000>, /* ap 125 */ <0x0005a000 0x0015a000 0x001000>; /* ap 126 */ + dma-ranges; target-module@2000 { /* 0x4a102000, ap 27 3c.0 */ compatible = "ti,sysc"; Sorry, I'm not familliar with property inheritance between devicetree nodes, especially with dma-ranges. Does this change seem correct to you? Best regards, Romain > >> >> The SATA port is unlikely not available on TI AM57 EVM boards or the beaglebone-AI. >> >> Any suggestion? >> >> [1] >> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=8af15365a36845c4c15d4c8046ddccff331d5263 >> [2] https://lore.kernel.org/all/20200206111728.6703-1-rogerq@ti.com/T/ >> [3] https://lore.kernel.org/lkml/c753a232-403d-6ed2-89fd-09476c887391@ti.com/ >> >> Best regards, >> Romain >> >> >>> >>> Signed-off-by: Tony Lindgren <tony@atomide.com> >>> --- >>> arch/arm/boot/dts/dra7-l4.dtsi | 29 ++++++++++++++++++++++++++--- >>> arch/arm/boot/dts/dra7.dtsi | 12 ------------ >>> 2 files changed, 26 insertions(+), 15 deletions(-) >>> >>> diff --git a/arch/arm/boot/dts/dra7-l4.dtsi b/arch/arm/boot/dts/dra7-l4.dtsi >>> --- a/arch/arm/boot/dts/dra7-l4.dtsi >>> +++ b/arch/arm/boot/dts/dra7-l4.dtsi >>> @@ -572,11 +572,34 @@ target-module@8000 { /* 0x4a108000, ap 29 1e.0 */ >>> }; >>> >>> target-module@40000 { /* 0x4a140000, ap 31 06.0 */ >>> - compatible = "ti,sysc"; >>> - status = "disabled"; >>> - #address-cells = <1>; >>> + compatible = "ti,sysc-omap4", "ti,sysc"; >>> + ti,hwmods = "sata"; >>> + reg = <0x400fc 4>, >>> + <0x41100 4>; >>> + reg-names = "rev", "sysc"; >>> + ti,sysc-midle = <SYSC_IDLE_FORCE>, >>> + <SYSC_IDLE_NO>, >>> + <SYSC_IDLE_SMART>; >>> + ti,sysc-sidle = <SYSC_IDLE_FORCE>, >>> + <SYSC_IDLE_NO>, >>> + <SYSC_IDLE_SMART>, >>> + <SYSC_IDLE_SMART_WKUP>; >>> + power-domains = <&prm_l3init>; >>> + clocks = <&l3init_clkctrl DRA7_L3INIT_SATA_CLKCTRL 0>; >>> + clock-names = "fck"; >>> #size-cells = <1>; >>> + #address-cells = <1>; >>> ranges = <0x0 0x40000 0x10000>; >>> + >>> + sata: sata@0 { >>> + compatible = "snps,dwc-ahci"; >>> + reg = <0 0x1100>, <0x1100 0x8>; >>> + interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>; >>> + phys = <&sata_phy>; >>> + phy-names = "sata-phy"; >>> + clocks = <&l3init_clkctrl DRA7_L3INIT_SATA_CLKCTRL 8>; >>> + ports-implemented = <0x1>; >>> + }; >>> }; >>> >>> target-module@51000 { /* 0x4a151000, ap 33 50.0 */ >>> diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi >>> --- a/arch/arm/boot/dts/dra7.dtsi >>> +++ b/arch/arm/boot/dts/dra7.dtsi >>> @@ -785,18 +785,6 @@ qspi: spi@0 { >>> }; >>> }; >>> >>> - /* OCP2SCP3 */ >>> - sata: sata@4a141100 { >>> - compatible = "snps,dwc-ahci"; >>> - reg = <0x4a140000 0x1100>, <0x4a141100 0x7>; >>> - interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>; >>> - phys = <&sata_phy>; >>> - phy-names = "sata-phy"; >>> - clocks = <&l3init_clkctrl DRA7_L3INIT_SATA_CLKCTRL 8>; >>> - ti,hwmods = "sata"; >>> - ports-implemented = <0x1>; >>> - }; >>> - >>> /* OCP2SCP1 */ >>> /* IRQ for DWC3_3 and DWC3_4 need IRQ crossbar */ >>> >
On 14/11/2024 15:50, Romain Naour wrote: > Hi Roger, Robin, All, > > Le 14/11/2024 à 12:02, Roger Quadros a écrit : >> Hi Romain, >> >> On 12/11/2024 16:15, Romain Naour wrote: >>> Hello Tony, Roger, All, >>> >>> Le 26/01/2021 à 13:39, Tony Lindgren a écrit : >>>> We can now probe devices with device tree only configuration using >>>> ti-sysc interconnect target module driver. Let's configure the >>>> module, but keep the legacy "ti,hwmods" peroperty to avoid new boot >>>> time warnings. The legacy property will be removed in later patches >>>> together with the legacy platform data. >>>> >>>> Note that the old sysc register offset is wrong, the real offset is at >>>> 0x1100 as listed in TRM for SATA_SYSCONFIG register. Looks like we've been >>>> happily using sata on the bootloader configured sysconfig register and >>>> nobody noticed. Also the old register range for SATAMAC_wrapper registers >>>> is wrong at 7 while it should be 8. But that too seems harmless. >>>> >>>> There is also an L3 parent interconnect range that we don't seem to be >>>> using. That can be added as needed later on. >>> >>> Since the switch from a kernel 5.10 to 6.1, the dra7 (AM574x) sata interface >>> doesn't work as expected. >>> >>> Using a kernel 6.1 with a preformated ext4 SATA disc, any copied file will be >>> corrupted when the filesystem is umounted. >>> >>> mount /dev/sda1 /mnt >>> cp /<test_file> /mnt/ >>> sync >>> sha256sum /mnt/<test_file> /<test_file> >>> <same hash> >>> umount /mnt >>> >>> mount /dev/sda1 /mnt >>> sha256sum /mnt/<test_file> /<test_file> >>> /mnt/<test_file> is corrupted. >>> >>> git bisect report 8af15365a368 ("ARM: dts: Configure interconnect target module >>> for dra7 sata") as the first bad commit [1] (merged in 5.13). >>> >>> While looking for existing SATA issue on dra7 SoC, I found this old patch: >>> >>> "On TI Platforms using LPAE, SATA breaks with 64-bit DMA. Restrict it to >>> 32-bit." [2]. >>> >>> Even if it's not the correct fix, disabling 64-bit DMA allows to use the sata >>> disc correctly. The discussion about this issue seems to have stopped [3] and >>> the suggested change was never merged. >> >> If I remember right the following commit fixed the issue back then. >> >> cfb5d65f2595 ARM: dts: dra7: Add bus_dma_limit for L3 bus >> >> But, when commit [1] moved the SATA node from L3 bus to L4_cfg it lost the bus_dma_limit >> that we added at the L3 bus and hence the regression. >> >> I think we should add the same 2GB dma ranges limit into l4_cfg bus so all modules >> can inherit it. > > Thanks for your reply! > > It seems l4_cfg can inherit dma-ranges property from ocp node using > "dma-ranges;". But then segment@100000 node (0x4a100000) needs "dma-ranges;" too. > > With the following patch applied, the SATA drive works correctly. > > diff --git a/arch/arm/boot/dts/dra7-l4.dtsi b/arch/arm/boot/dts/dra7-l4.dtsi > index 1aaffd034c39..3ac770298844 100644 > --- a/arch/arm/boot/dts/dra7-l4.dtsi > +++ b/arch/arm/boot/dts/dra7-l4.dtsi > @@ -12,6 +12,7 @@ &l4_cfg { /* > 0x4a000000 */ > ranges = <0x00000000 0x4a000000 0x100000>, /* segment 0 */ > <0x00100000 0x4a100000 0x100000>, /* segment 1 */ > <0x00200000 0x4a200000 0x100000>; /* segment 2 */ > + dma-ranges; > > segment@0 { /* 0x4a000000 */ > compatible = "simple-pm-bus"; > @@ -557,6 +558,7 @@ segment@100000 { /* > 0x4a100000 */ > <0x0007e000 0x0017e000 0x001000>, /* ap 124 */ > <0x00059000 0x00159000 0x001000>, /* ap 125 */ > <0x0005a000 0x0015a000 0x001000>; /* ap 126 */ > + dma-ranges; > > target-module@2000 { /* 0x4a102000, ap 27 3c.0 */ > compatible = "ti,sysc"; > > > Sorry, I'm not familliar with property inheritance between devicetree nodes, > especially with dma-ranges. Does this change seem correct to you? I think this is correct. A similar fix [4] was done for PCIe as well. [4] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=90d4d3f4ea45370d482fa609dbae4d2281b4074f > > Best regards, > Romain > > >> >>> >>> The SATA port is unlikely not available on TI AM57 EVM boards or the beaglebone-AI. >>> >>> Any suggestion? >>> >>> [1] >>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=8af15365a36845c4c15d4c8046ddccff331d5263 >>> [2] https://lore.kernel.org/all/20200206111728.6703-1-rogerq@ti.com/T/ >>> [3] https://lore.kernel.org/lkml/c753a232-403d-6ed2-89fd-09476c887391@ti.com/ >>> >>> Best regards, >>> Romain >>> >>> >>>> >>>> Signed-off-by: Tony Lindgren <tony@atomide.com> >>>> --- >>>> arch/arm/boot/dts/dra7-l4.dtsi | 29 ++++++++++++++++++++++++++--- >>>> arch/arm/boot/dts/dra7.dtsi | 12 ------------ >>>> 2 files changed, 26 insertions(+), 15 deletions(-) >>>> >>>> diff --git a/arch/arm/boot/dts/dra7-l4.dtsi b/arch/arm/boot/dts/dra7-l4.dtsi >>>> --- a/arch/arm/boot/dts/dra7-l4.dtsi >>>> +++ b/arch/arm/boot/dts/dra7-l4.dtsi >>>> @@ -572,11 +572,34 @@ target-module@8000 { /* 0x4a108000, ap 29 1e.0 */ >>>> }; >>>> >>>> target-module@40000 { /* 0x4a140000, ap 31 06.0 */ >>>> - compatible = "ti,sysc"; >>>> - status = "disabled"; >>>> - #address-cells = <1>; >>>> + compatible = "ti,sysc-omap4", "ti,sysc"; >>>> + ti,hwmods = "sata"; >>>> + reg = <0x400fc 4>, >>>> + <0x41100 4>; >>>> + reg-names = "rev", "sysc"; >>>> + ti,sysc-midle = <SYSC_IDLE_FORCE>, >>>> + <SYSC_IDLE_NO>, >>>> + <SYSC_IDLE_SMART>; >>>> + ti,sysc-sidle = <SYSC_IDLE_FORCE>, >>>> + <SYSC_IDLE_NO>, >>>> + <SYSC_IDLE_SMART>, >>>> + <SYSC_IDLE_SMART_WKUP>; >>>> + power-domains = <&prm_l3init>; >>>> + clocks = <&l3init_clkctrl DRA7_L3INIT_SATA_CLKCTRL 0>; >>>> + clock-names = "fck"; >>>> #size-cells = <1>; >>>> + #address-cells = <1>; >>>> ranges = <0x0 0x40000 0x10000>; >>>> + >>>> + sata: sata@0 { >>>> + compatible = "snps,dwc-ahci"; >>>> + reg = <0 0x1100>, <0x1100 0x8>; >>>> + interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>; >>>> + phys = <&sata_phy>; >>>> + phy-names = "sata-phy"; >>>> + clocks = <&l3init_clkctrl DRA7_L3INIT_SATA_CLKCTRL 8>; >>>> + ports-implemented = <0x1>; >>>> + }; >>>> }; >>>> >>>> target-module@51000 { /* 0x4a151000, ap 33 50.0 */ >>>> diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi >>>> --- a/arch/arm/boot/dts/dra7.dtsi >>>> +++ b/arch/arm/boot/dts/dra7.dtsi >>>> @@ -785,18 +785,6 @@ qspi: spi@0 { >>>> }; >>>> }; >>>> >>>> - /* OCP2SCP3 */ >>>> - sata: sata@4a141100 { >>>> - compatible = "snps,dwc-ahci"; >>>> - reg = <0x4a140000 0x1100>, <0x4a141100 0x7>; >>>> - interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>; >>>> - phys = <&sata_phy>; >>>> - phy-names = "sata-phy"; >>>> - clocks = <&l3init_clkctrl DRA7_L3INIT_SATA_CLKCTRL 8>; >>>> - ti,hwmods = "sata"; >>>> - ports-implemented = <0x1>; >>>> - }; >>>> - >>>> /* OCP2SCP1 */ >>>> /* IRQ for DWC3_3 and DWC3_4 need IRQ crossbar */ >>>> >> >
Le 14/11/2024 à 16:08, Roger Quadros a écrit : > > > On 14/11/2024 15:50, Romain Naour wrote: >> Hi Roger, Robin, All, >> >> Le 14/11/2024 à 12:02, Roger Quadros a écrit : [...] >> >> Thanks for your reply! >> >> It seems l4_cfg can inherit dma-ranges property from ocp node using >> "dma-ranges;". But then segment@100000 node (0x4a100000) needs "dma-ranges;" too. >> >> With the following patch applied, the SATA drive works correctly. >> >> diff --git a/arch/arm/boot/dts/dra7-l4.dtsi b/arch/arm/boot/dts/dra7-l4.dtsi >> index 1aaffd034c39..3ac770298844 100644 >> --- a/arch/arm/boot/dts/dra7-l4.dtsi >> +++ b/arch/arm/boot/dts/dra7-l4.dtsi >> @@ -12,6 +12,7 @@ &l4_cfg { /* >> 0x4a000000 */ >> ranges = <0x00000000 0x4a000000 0x100000>, /* segment 0 */ >> <0x00100000 0x4a100000 0x100000>, /* segment 1 */ >> <0x00200000 0x4a200000 0x100000>; /* segment 2 */ >> + dma-ranges; >> >> segment@0 { /* 0x4a000000 */ >> compatible = "simple-pm-bus"; >> @@ -557,6 +558,7 @@ segment@100000 { /* >> 0x4a100000 */ >> <0x0007e000 0x0017e000 0x001000>, /* ap 124 */ >> <0x00059000 0x00159000 0x001000>, /* ap 125 */ >> <0x0005a000 0x0015a000 0x001000>; /* ap 126 */ >> + dma-ranges; >> >> target-module@2000 { /* 0x4a102000, ap 27 3c.0 */ >> compatible = "ti,sysc"; >> >> >> Sorry, I'm not familliar with property inheritance between devicetree nodes, >> especially with dma-ranges. Does this change seem correct to you? > > I think this is correct. > A similar fix [4] was done for PCIe as well. > > [4] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=90d4d3f4ea45370d482fa609dbae4d2281b4074f Thank you for your help, I just sent the patch: https://lore.kernel.org/linux-omap/20241114155759.1155567-1-romain.naour@smile.fr/T/#u Best regards, Romain > >> >> Best regards, >> Romain
Hi all, Here's v2 series to update dra7 to probe with ti-sysc and genpd like we've already done for am3 and 4. These patches are against v5.11-rc1, and depend on the following commits in my fixes branch: 7078a5ba7a58 ("soc: ti: omap-prm: Fix boot time errors for rst_map_012 bits 0 and 1") 2a39af3870e9 ("ARM: OMAP2+: Fix booting for am335x after moving to simple-pm-bus") These patches also depend on the series: [PATCH 0/3] Few ti-sysc changes for v5.12 merge window Please review and test, I've also pushed out a temporary testing branch to make testing easier to [0][1]. Regards, Tony Changes since v1: - Split the series into two parts, looks like most of the emails did not make it to the lists - Dropped Balaji from Cc as the email address bounces [0] git://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git tmp-testing-genpd-dra7 [1] https://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git/log/?h=omap-for-v5.12/tmp-testing-genpd-dra7 Tony Lindgren (15): PCI: pci-dra7xx: Prepare for deferred probe with module_platform_driver ARM: dts: Update pcie ranges for dra7 ARM: dts: Configure interconnect target module for dra7 pcie ARM: dts: Properly configure dra7 edma sysconfig registers ARM: dts: Move dra7 l3 noc to a separate node ARM: dts: Configure interconnect target module for dra7 qspi ARM: dts: Configure interconnect target module for dra7 sata ARM: dts: Configure interconnect target module for dra7 mpu ARM: dts: Configure interconnect target module for dra7 dmm ARM: dts: Configure simple-pm-bus for dra7 l4_wkup ARM: dts: Configure simple-pm-bus for dra7 l4_per1 ARM: dts: Configure simple-pm-bus for dra7 l4_per2 ARM: dts: Configure simple-pm-bus for dra7 l4_per3 ARM: dts: Configure simple-pm-bus for dra7 l4_cfg ARM: dts: Configure simple-pm-bus for dra7 l3 arch/arm/boot/dts/dra7-l4.dtsi | 76 ++++++--- arch/arm/boot/dts/dra7.dtsi | 216 ++++++++++++++++-------- drivers/pci/controller/dwc/pci-dra7xx.c | 13 +- 3 files changed, 213 insertions(+), 92 deletions(-)