[PATCH] arm64: dts: ls1088a: update sata node

[PATCH] arm64: dts: ls1088a: update sata node

[Yuantian Tang]

1. Remove ls1043a compatible string from node
2. Fix the sata ecc register address error

Signed-off-by: Tang Yuantian <andy.tang@nxp.com>
---
 arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
index df16284..c144d06 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
@@ -360,9 +360,9 @@
 		};
 
 		sata: sata at 3200000 {
-			compatible = "fsl,ls1088a-ahci", "fsl,ls1043a-ahci";
+			compatible = "fsl,ls1088a-ahci";
 			reg = <0x0 0x3200000 0x0 0x10000>,
-				<0x0 0x20140520 0x0 0x4>;
+				<0x7 0x100520 0x0 0x4>;
 			reg-names = "ahci", "sata-ecc";
 			interrupts = <0 133 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&clockgen 4 3>;
-- 
2.1.0.27.g96db324

[PATCH] arm64: dts: ls1088a: update sata node

[Shawn Guo]

On Mon, Jun 05, 2017 at 03:45:52PM +0800, Yuantian Tang wrote:
> 1. Remove ls1043a compatible string from node

Can you explain a bit why the compatible string needs to be dropped?

Shawn

> 2. Fix the sata ecc register address error
> 
> Signed-off-by: Tang Yuantian <andy.tang@nxp.com>
> ---
>  arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> index df16284..c144d06 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> @@ -360,9 +360,9 @@
>  		};
>  
>  		sata: sata at 3200000 {
> -			compatible = "fsl,ls1088a-ahci", "fsl,ls1043a-ahci";
> +			compatible = "fsl,ls1088a-ahci";
>  			reg = <0x0 0x3200000 0x0 0x10000>,
> -				<0x0 0x20140520 0x0 0x4>;
> +				<0x7 0x100520 0x0 0x4>;
>  			reg-names = "ahci", "sata-ecc";
>  			interrupts = <0 133 IRQ_TYPE_LEVEL_HIGH>;
>  			clocks = <&clockgen 4 3>;
> -- 
> 2.1.0.27.g96db324
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH] arm64: dts: ls1088a: update sata node

[Andy Tang]

Hi Shawn,

Please see the explanation below.

-----Original Message-----
From: Shawn Guo [mailto:shawnguo at kernel.org] 
Sent: Monday, June 05, 2017 11:15 PM
To: Andy Tang <andy.tang@nxp.com>
Cc: mark.rutland at arm.com; catalin.marinas at arm.com; will.deacon at arm.com; robh+dt at kernel.org; linux-arm-kernel at lists.infradead.org
Subject: Re: [PATCH] arm64: dts: ls1088a: update sata node

On Mon, Jun 05, 2017 at 03:45:52PM +0800, Yuantian Tang wrote:
> 1. Remove ls1043a compatible string from node

Can you explain a bit why the compatible string needs to be dropped?

A:  We thought ls1088 and ls1043 are 100% compatible in regard to sata.
But actually, they are not because their ecc register address is different.

Please let me know if I need to add those to commit message.

Thanks,
Andy

> 2. Fix the sata ecc register address error
> 
> Signed-off-by: Tang Yuantian <andy.tang@nxp.com>
> ---
>  arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> index df16284..c144d06 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi
> @@ -360,9 +360,9 @@
>  		};
>  
>  		sata: sata at 3200000 {
> -			compatible = "fsl,ls1088a-ahci", "fsl,ls1043a-ahci";
> +			compatible = "fsl,ls1088a-ahci";
>  			reg = <0x0 0x3200000 0x0 0x10000>,
> -				<0x0 0x20140520 0x0 0x4>;
> +				<0x7 0x100520 0x0 0x4>;
>  			reg-names = "ahci", "sata-ecc";
>  			interrupts = <0 133 IRQ_TYPE_LEVEL_HIGH>;
>  			clocks = <&clockgen 4 3>;
> -- 
> 2.1.0.27.g96db324
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH] arm64: dts: ls1088a: update sata node

[Shawn Guo]

On Wed, Jun 07, 2017 at 02:38:36AM +0000, Andy Tang wrote:
> Hi Shawn,
> 
> Please see the explanation below.
> 
> -----Original Message-----
> From: Shawn Guo [mailto:shawnguo at kernel.org] 
> Sent: Monday, June 05, 2017 11:15 PM
> To: Andy Tang <andy.tang@nxp.com>
> Cc: mark.rutland at arm.com; catalin.marinas at arm.com; will.deacon at arm.com; robh+dt at kernel.org; linux-arm-kernel at lists.infradead.org
> Subject: Re: [PATCH] arm64: dts: ls1088a: update sata node
> 
> On Mon, Jun 05, 2017 at 03:45:52PM +0800, Yuantian Tang wrote:
> > 1. Remove ls1043a compatible string from node
> 
> Can you explain a bit why the compatible string needs to be dropped?
> 
> A:  We thought ls1088 and ls1043 are 100% compatible in regard to sata.
> But actually, they are not because their ecc register address is different.

It's quite common that the same IP block is integrated into different
SoCs with different register address.  That shouldn't be the reason for
a new compatible.

Shawn

[PATCH] arm64: dts: ls1088a: update sata node

[Andy Tang]

Hi Shawn,

Please see my explanation below.

-----Original Message-----
From: Shawn Guo [mailto:shawnguo at kernel.org] 
Sent: Wednesday, June 07, 2017 11:26 AM
To: Andy Tang <andy.tang@nxp.com>
Cc: mark.rutland at arm.com; catalin.marinas at arm.com; robh+dt at kernel.org; will.deacon at arm.com; linux-arm-kernel at lists.infradead.org
Subject: Re: [PATCH] arm64: dts: ls1088a: update sata node

On Wed, Jun 07, 2017 at 02:38:36AM +0000, Andy Tang wrote:
> Hi Shawn,
> 
> Please see the explanation below.
> 
> -----Original Message-----
> From: Shawn Guo [mailto:shawnguo at kernel.org]
> Sent: Monday, June 05, 2017 11:15 PM
> To: Andy Tang <andy.tang@nxp.com>
> Cc: mark.rutland at arm.com; catalin.marinas at arm.com; 
> will.deacon at arm.com; robh+dt at kernel.org; 
> linux-arm-kernel at lists.infradead.org
> Subject: Re: [PATCH] arm64: dts: ls1088a: update sata node
> 
> On Mon, Jun 05, 2017 at 03:45:52PM +0800, Yuantian Tang wrote:
> > 1. Remove ls1043a compatible string from node
> 
> Can you explain a bit why the compatible string needs to be dropped?
> 
> A:  We thought ls1088 and ls1043 are 100% compatible in regard to sata.
> But actually, they are not because their ecc register address is different.

It's quite common that the same IP block is integrated into different SoCs with different register address.  That shouldn't be the reason for a new compatible.

A: ls1088 is not a new compatible. For ls1088 soc, it should be "fsl,ls1088a-ahci".
Previously, we thought ls1088 is compatible with ls1043, so that we defined compatible string as:  "fsl,ls1088a-ahci", "fsl,ls1043a-ahci"; By doing so, we can reuse code.

Now we found they are not 100% compatible because ECC register address is different. ECC register doesn't belong to SATA IP. It is a global register which controls many IPs. SATA ecc is controlled by only one bit in this register.

So we have to remove ls1043 compatible string from ls1088 sata node and make ls1088 sata node unique.

Thanks,
Andy

Shawn

[PATCH] arm64: dts: ls1088a: update sata node

[Shawn Guo]

On Wed, Jun 07, 2017 at 03:41:03AM +0000, Andy Tang wrote:
> A: ls1088 is not a new compatible. For ls1088 soc, it should be "fsl,ls1088a-ahci".

Have you added the compatible support into kernel driver and bindings
doc?

> Previously, we thought ls1088 is compatible with ls1043, so that we defined compatible string as:  "fsl,ls1088a-ahci", "fsl,ls1043a-ahci"; By doing so, we can reuse code.
> 
> Now we found they are not 100% compatible because ECC register address is different. ECC register doesn't belong to SATA IP. It is a global register which controls many IPs. SATA ecc is controlled by only one bit in this register.
> 

Okay, understood.  It sounds like the register should belong to a global
system controller.  If the SATA driver was designed to parse the ECC
register and bit from device tree with a phandle pointing to that
system controller, we do not need to have so many compatibles for the
same SATA IP.

Shawn

[PATCH] arm64: dts: ls1088a: update sata node

[Andy Tang]

Hi Shawn,

Yes, if there is a node in dts for ecc register/global register, we only need a phandler. Unfortunately,  ecc register actually is a hidden register.
If it were not for a sata workaround, we don't need to touch this register.

Thanks for your understanding.

Regards
Andy
 

-----Original Message-----
From: Shawn Guo [mailto:shawnguo at kernel.org] 
Sent: Wednesday, June 07, 2017 2:02 PM
To: Andy Tang <andy.tang@nxp.com>
Cc: mark.rutland at arm.com; catalin.marinas at arm.com; robh+dt at kernel.org; will.deacon at arm.com; linux-arm-kernel at lists.infradead.org
Subject: Re: [PATCH] arm64: dts: ls1088a: update sata node

On Wed, Jun 07, 2017 at 03:41:03AM +0000, Andy Tang wrote:
> A: ls1088 is not a new compatible. For ls1088 soc, it should be "fsl,ls1088a-ahci".

Have you added the compatible support into kernel driver and bindings doc?

> Previously, we thought ls1088 is compatible with ls1043, so that we defined compatible string as:  "fsl,ls1088a-ahci", "fsl,ls1043a-ahci"; By doing so, we can reuse code.
> 
> Now we found they are not 100% compatible because ECC register address is different. ECC register doesn't belong to SATA IP. It is a global register which controls many IPs. SATA ecc is controlled by only one bit in this register.
> 

Okay, understood.  It sounds like the register should belong to a global system controller.  If the SATA driver was designed to parse the ECC register and bit from device tree with a phandle pointing to that system controller, we do not need to have so many compatibles for the same SATA IP.

Shawn

[PATCH] arm64: dts: ls1088a: update sata node

[Andy Tang]

Oops, bindings has not been updated yet. Will update this patch.

Thanks for your reminder.

Regards,
Andy

-----Original Message-----
From: Shawn Guo [mailto:shawnguo at kernel.org] 
Sent: Wednesday, June 07, 2017 2:02 PM
To: Andy Tang <andy.tang@nxp.com>
Cc: mark.rutland at arm.com; catalin.marinas at arm.com; robh+dt at kernel.org; will.deacon at arm.com; linux-arm-kernel at lists.infradead.org
Subject: Re: [PATCH] arm64: dts: ls1088a: update sata node

On Wed, Jun 07, 2017 at 03:41:03AM +0000, Andy Tang wrote:
> A: ls1088 is not a new compatible. For ls1088 soc, it should be "fsl,ls1088a-ahci".

Have you added the compatible support into kernel driver and bindings doc?

> Previously, we thought ls1088 is compatible with ls1043, so that we defined compatible string as:  "fsl,ls1088a-ahci", "fsl,ls1043a-ahci"; By doing so, we can reuse code.
> 
> Now we found they are not 100% compatible because ECC register address is different. ECC register doesn't belong to SATA IP. It is a global register which controls many IPs. SATA ecc is controlled by only one bit in this register.
> 

Okay, understood.  It sounds like the register should belong to a global system controller.  If the SATA driver was designed to parse the ECC register and bit from device tree with a phandle pointing to that system controller, we do not need to have so many compatibles for the same SATA IP.

Shawn