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

[Roger Quadros <rogerq@kernel.org>]

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 */
>>  

-- 
cheers,
-roger