gpio/ls1043a: Correct ls1043a gpio nodes compatible in dts file
diff mbox

Message ID 1457600240-47141-2-git-send-email-Gang.Liu@nxp.com
State New
Headers show

Commit Message

Liu Gang March 10, 2016, 8:57 a.m. UTC
The ls1043a belongs to the Freescale QorIQ platform, and QorIQ
platform's gpio nodes should use compatible "fsl,qoriq-gpio".

Signed-off-by: Liu Gang <Gang.Liu@nxp.com>
---
 arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Scott Wood March 16, 2016, 6:47 p.m. UTC | #1
On 03/10/2016 03:06 AM, Liu Gang wrote:
> The ls1043a belongs to the Freescale QorIQ platform, and QorIQ
> platform's gpio nodes should use compatible "fsl,qoriq-gpio".

Why?  I don't see any version register anything else in this block that
could be used to identify potential differences from chip to chip.  At
the least, both compatibles should be present.

Where is the binding for this?

-Scott

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Wood March 18, 2016, 6:49 p.m. UTC | #2
On 03/17/2016 01:57 AM, Gang Liu wrote:
> On 3/17/2016 2:47 AM, Scott Wood wrote:
>> On 03/10/2016 03:06 AM, Liu Gang wrote:
>>> The ls1043a belongs to the Freescale QorIQ platform, and QorIQ
>>> platform's gpio nodes should use compatible "fsl,qoriq-gpio".
>> Why?  I don't see any version register anything else in this block that
>> could be used to identify potential differences from chip to chip.  At
>> the least, both compatibles should be present.
>>
>> Where is the binding for this?
>>
>> -Scott
>>
>>
> Hi, Scott,
> Currently all QorIQ platform's GPIO IP blocks are same, they are all no
> version register,
> and have same registers, offset address and other features, so the code
> can cover all of
> them.

So?  We have no idea what the hardware people will do in the future.

> There is just one exceptional ls2080, it has the little-endian GPIO
> registers, and we added
> a "little-endian" property in the GPIO nodes.
> 
> Add in addition, we have deeply discussed about the GPIO compatible name
> before, you
> suggested using "fsl,qoriq-gpio" for all the QorIQ platforms. Please
> find some excerpt
> from former mail context.

It's in established use so I won't suggest that we get rid of it.  But
ideally the chip-specific compatible should be in the device tree (not
the driver until/unless necessary) as well, so it seemed odd to get rid
of it.

If we were starting from scratch, then instead of "fsl,qoriq-gpio" it
should have been a chip-based compatible for some arbitrary chip that
contains this exact logic (in addition to the chip-specific compatible
for the actual chip).  Neither case requires adding every chip name to
the driver.

-Scott

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Liu Gang March 22, 2016, 2:03 p.m. UTC | #3
Hi, Scott,

I think you mean that GPIO node needs two compatibles, one for general GPIO IP block ("fsl,qoriq-gpio")
and another for chip-specific ("fsl,ls1043a-gpio").
The driver can just include general GPIO IP block compatible, and if there is a errata for ls1043 GPIO, the
chip-specific compatible can be used to fix the errata, right?

If it's right, do you think the "fsl,ls1043a-gpio" and "fsl,qoriq-gpio" are ok for our Layerscape platforms?
So maybe I need to create more patches for ls1021, ls2085 platform supported in opensource.

Best Regards,
Liu Gang

-----Original Message-----
From: Scott Wood 
Sent: Saturday, March 19, 2016 2:49 AM
To: Gang Liu <gang.liu@nxp.com>; linus.walleij@linaro.org; linux-gpio@vger.kernel.org; linux-arm-kernel@lists.infradead.org; khilman@linaro.org
Cc: robh+dt@kernel.org; Leo Li <leo.li@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>
Subject: Re: [PATCH] gpio/ls1043a: Correct ls1043a gpio nodes compatible in dts file

> Hi, Scott,
> Currently all QorIQ platform's GPIO IP blocks are same, they are all 
> no version register, and have same registers, offset address and other 
> features, so the code can cover all of them.

So?  We have no idea what the hardware people will do in the future.

> There is just one exceptional ls2080, it has the little-endian GPIO 
> registers, and we added a "little-endian" property in the GPIO nodes.
> 
> Add in addition, we have deeply discussed about the GPIO compatible 
> name before, you suggested using "fsl,qoriq-gpio" for all the QorIQ 
> platforms. Please find some excerpt from former mail context.

It's in established use so I won't suggest that we get rid of it.  But ideally the chip-specific compatible should be in the device tree (not the driver until/unless necessary) as well, so it seemed odd to get rid of it.

If we were starting from scratch, then instead of "fsl,qoriq-gpio" it should have been a chip-based compatible for some arbitrary chip that contains this exact logic (in addition to the chip-specific compatible for the actual chip).  Neither case requires adding every chip name to the driver.

-Scott

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Wood March 22, 2016, 4 p.m. UTC | #4
On 03/22/2016 09:03 AM, Gang Liu wrote:
> Hi, Scott,
> 
> I think you mean that GPIO node needs two compatibles, one for general GPIO IP block ("fsl,qoriq-gpio")
> and another for chip-specific ("fsl,ls1043a-gpio").
> The driver can just include general GPIO IP block compatible, and if there is a errata for ls1043 GPIO, the
> chip-specific compatible can be used to fix the errata, right?

Yes.

> If it's right, do you think the "fsl,ls1043a-gpio" and "fsl,qoriq-gpio" are ok for our Layerscape platforms?

Yes.

-Scott

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
index 42a6154..e356362 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
@@ -284,7 +284,7 @@ 
 		};
 
 		gpio1: gpio@2300000 {
-			compatible = "fsl,ls1043a-gpio";
+			compatible = "fsl,qoriq-gpio";
 			reg = <0x0 0x2300000 0x0 0x10000>;
 			interrupts = <0 66 0x4>;
 			gpio-controller;
@@ -294,7 +294,7 @@ 
 		};
 
 		gpio2: gpio@2310000 {
-			compatible = "fsl,ls1043a-gpio";
+			compatible = "fsl,qoriq-gpio";
 			reg = <0x0 0x2310000 0x0 0x10000>;
 			interrupts = <0 67 0x4>;
 			gpio-controller;
@@ -304,7 +304,7 @@ 
 		};
 
 		gpio3: gpio@2320000 {
-			compatible = "fsl,ls1043a-gpio";
+			compatible = "fsl,qoriq-gpio";
 			reg = <0x0 0x2320000 0x0 0x10000>;
 			interrupts = <0 68 0x4>;
 			gpio-controller;
@@ -314,7 +314,7 @@ 
 		};
 
 		gpio4: gpio@2330000 {
-			compatible = "fsl,ls1043a-gpio";
+			compatible = "fsl,qoriq-gpio";
 			reg = <0x0 0x2330000 0x0 0x10000>;
 			interrupts = <0 134 0x4>;
 			gpio-controller;