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]

 



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@xxxxxx/T/
>> [3] https://lore.kernel.org/lkml/c753a232-403d-6ed2-89fd-09476c887391@xxxxxx/
>>
>> Best regards,
>> Romain
>>
>>
>>>
>>> Signed-off-by: Tony Lindgren <tony@xxxxxxxxxxx>
>>> ---
>>>  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 */
>>>  
> 





[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