arm64: dts: ls1088a: update sata node
Subsystems (2)
PATCH 1/1 arm64: dts: ls1088a: update sata node
1. Remove ls1043a compatible string from node
2. Fix the sata ecc register address error
--- arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi +2 -2
@@ -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>;
Signature · 1 line
2.1.0.27.g96db324
PATCH 1/1 arm64: dts: ls1088a: update sata node
Quoted 1 line · Yuantian Tang
1. Remove ls1043a compatible string from node
Can you explain a bit why the compatible string needs to be dropped?
Shawn
Quoted 31 lines
2. Fix the sata ecc register address error
Signed-off-by Tang Yuantian <[email protected]>--- arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi +2
-2@@ -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>;Signature · 7 lines
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 1/1 arm64: dts: ls1088a: update sata node
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 <[email protected]>
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
Quoted 1 line · Yuantian Tang
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
Quoted 31 lines
2. Fix the sata ecc register address error
Signed-off-by Tang Yuantian <[email protected]>--- arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi +2
-2@@ -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>;Signature · 7 lines
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 1/1 arm64: dts: ls1088a: update sata node
Quoted 11 lines · 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 <[email protected]>
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
Quoted 7 lines
Quoted 1 line · Yuantian Tang
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 1/1 arm64: dts: ls1088a: update sata node
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 <[email protected]>
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
Quoted 13 lines · 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 <[email protected]>
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
Quoted 7 lines
Quoted 1 line · Yuantian Tang
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 1/1 arm64: dts: ls1088a: update sata node
Quoted 1 line · Andy Tang
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?
Quoted 4 lines
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 1/1 arm64: dts: ls1088a: update sata node
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 <[email protected]>
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
Quoted 1 line · Andy Tang
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?
Quoted 4 lines
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 1/1 arm64: dts: ls1088a: update sata node
In reply to Shawn GuoOops, 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 <[email protected]>
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
Quoted 1 line · Andy Tang
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?
Quoted 4 lines
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