Re: [PATCH 07/15] ARM: dts: Configure interconnect target module for dra7 sata

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxx/T/
[3] https://lore.kernel.org/lkml/c753a232-403d-6ed2-89fd-09476c887391@xxxxxx/

Best regards,
Romain









[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux