| Message ID | 1496648752-27627-1-git-send-email-andy.tang@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@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@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Shawn, Please see the explanation below. -----Original Message----- From: Shawn Guo [mailto:shawnguo@kernel.org] Sent: Monday, June 05, 2017 11:15 PM To: Andy Tang <andy.tang@nxp.com> Cc: mark.rutland@arm.com; catalin.marinas@arm.com; will.deacon@arm.com; robh+dt@kernel.org; linux-arm-kernel@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@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@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
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@kernel.org] > Sent: Monday, June 05, 2017 11:15 PM > To: Andy Tang <andy.tang@nxp.com> > Cc: mark.rutland@arm.com; catalin.marinas@arm.com; will.deacon@arm.com; robh+dt@kernel.org; linux-arm-kernel@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
Hi Shawn, Please see my explanation below. -----Original Message----- From: Shawn Guo [mailto:shawnguo@kernel.org] Sent: Wednesday, June 07, 2017 11:26 AM To: Andy Tang <andy.tang@nxp.com> Cc: mark.rutland@arm.com; catalin.marinas@arm.com; robh+dt@kernel.org; will.deacon@arm.com; linux-arm-kernel@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@kernel.org] > Sent: Monday, June 05, 2017 11:15 PM > To: Andy Tang <andy.tang@nxp.com> > Cc: mark.rutland@arm.com; catalin.marinas@arm.com; > will.deacon@arm.com; robh+dt@kernel.org; > linux-arm-kernel@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
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
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@kernel.org] Sent: Wednesday, June 07, 2017 2:02 PM To: Andy Tang <andy.tang@nxp.com> Cc: mark.rutland@arm.com; catalin.marinas@arm.com; robh+dt@kernel.org; will.deacon@arm.com; linux-arm-kernel@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
Oops, bindings has not been updated yet. Will update this patch. Thanks for your reminder. Regards, Andy -----Original Message----- From: Shawn Guo [mailto:shawnguo@kernel.org] Sent: Wednesday, June 07, 2017 2:02 PM To: Andy Tang <andy.tang@nxp.com> Cc: mark.rutland@arm.com; catalin.marinas@arm.com; robh+dt@kernel.org; will.deacon@arm.com; linux-arm-kernel@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
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@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>;
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(-)