| Message ID | 20210126124004.52550-8-tony@atomide.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Update dra7 devicetree files to probe with genpd | expand |
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
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 */
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. 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(-)