diff mbox series

[5/5] arm64: dts: imx93: update gpio node

Message ID 20230914-vf610-gpio-v1-5-3ed418182a6a@nxp.com
State Superseded
Headers show
Series [1/5] dt-bindings: gpio: vf610: correct i.MX8ULP and i.MX93 interrupts | expand

Commit Message

Peng Fan (OSS) Sept. 14, 2023, 2:21 a.m. UTC 
From: Peng Fan <peng.fan@nxp.com>

Per binding doc, i.MX93 GPIO supports two interrupts, and not
compatible with i.MX7ULP. So update the node

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx93.dtsi | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

Comments

Peng Fan Sept. 14, 2023, 6:53 a.m. UTC | #1 
> Subject: Re: [PATCH 5/5] arm64: dts: imx93: update gpio node
> 
> On 14/09/2023 08:04, Peng Fan wrote:
> >> Subject: Re: [PATCH 5/5] arm64: dts: imx93: update gpio node
> >>
> >> On 14/09/2023 04:21, Peng Fan (OSS) wrote:
> >>> From: Peng Fan <peng.fan@nxp.com>
> >>>
> >>> Per binding doc, i.MX93 GPIO supports two interrupts, and not
> >>> compatible with i.MX7ULP. So update the node
> >>>
> >>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> >>> ---
> >>>  arch/arm64/boot/dts/freescale/imx93.dtsi | 20 ++++++++++++--------
> >>>  1 file changed, 12 insertions(+), 8 deletions(-)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/freescale/imx93.dtsi
> >>> b/arch/arm64/boot/dts/freescale/imx93.dtsi
> >>> index 6f85a05ee7e1..011c34a57c53 100644
> >>> --- a/arch/arm64/boot/dts/freescale/imx93.dtsi
> >>> +++ b/arch/arm64/boot/dts/freescale/imx93.dtsi
> >>> @@ -825,11 +825,12 @@ usdhc3: mmc@428b0000 {
> >>>  		};
> >>>
> >>>  		gpio2: gpio@43810080 {
> >>> -			compatible = "fsl,imx93-gpio", "fsl,imx7ulp-gpio";
> >>> +			compatible = "fsl,imx93-gpio", "fsl,imx8ulp-gpio";
> >>
> >> As your driver change points, it is breaking users, so no :(
> >
> > ok. Although i.MX93 GPIO is not compatible with i.MX7ULP from HW
> > perspective, the compatible string should keep as it is now and
> > binding
> 
> If it is not compatible, then how could it work before?

i.MX7ULP reg:
0h Port Data Output Register (PDOR) 
4h Port Set Output Register (PSOR) 
8h Port Clear Output Register (PCOR) 
Ch Port Toggle Output Register (PTOR) 
10h Port Data Input Register (PDIR) 
14h Port Data Direction Register (PDDR)

i.MX8ULP/93 has different registers address, but 
i.MX93 registers has 0x40 off as below:
40h Port Data Output (PDOR)

Even linux i.MX7ULP gpio driver could work with i.MX8ULP/93
GPIO HW with dts node using an 0x40 offset + base addr
for i.MX93 gpio. I think from hw design, they are
not compatible. Besides the upper differences,
there are other differences.

So I think better move to new compatible string.

Thanks,
Peng.

> 
> 
> 
> Best regards,
> Krzysztof
Krzysztof Kozlowski Sept. 14, 2023, 7:11 a.m. UTC | #2 
On 14/09/2023 08:53, Peng Fan wrote:
>> Subject: Re: [PATCH 5/5] arm64: dts: imx93: update gpio node
>>
>> On 14/09/2023 08:04, Peng Fan wrote:
>>>> Subject: Re: [PATCH 5/5] arm64: dts: imx93: update gpio node
>>>>
>>>> On 14/09/2023 04:21, Peng Fan (OSS) wrote:
>>>>> From: Peng Fan <peng.fan@nxp.com>
>>>>>
>>>>> Per binding doc, i.MX93 GPIO supports two interrupts, and not
>>>>> compatible with i.MX7ULP. So update the node
>>>>>
>>>>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>>>>> ---
>>>>>  arch/arm64/boot/dts/freescale/imx93.dtsi | 20 ++++++++++++--------
>>>>>  1 file changed, 12 insertions(+), 8 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/freescale/imx93.dtsi
>>>>> b/arch/arm64/boot/dts/freescale/imx93.dtsi
>>>>> index 6f85a05ee7e1..011c34a57c53 100644
>>>>> --- a/arch/arm64/boot/dts/freescale/imx93.dtsi
>>>>> +++ b/arch/arm64/boot/dts/freescale/imx93.dtsi
>>>>> @@ -825,11 +825,12 @@ usdhc3: mmc@428b0000 {
>>>>>  		};
>>>>>
>>>>>  		gpio2: gpio@43810080 {
>>>>> -			compatible = "fsl,imx93-gpio", "fsl,imx7ulp-gpio";
>>>>> +			compatible = "fsl,imx93-gpio", "fsl,imx8ulp-gpio";
>>>>
>>>> As your driver change points, it is breaking users, so no :(
>>>
>>> ok. Although i.MX93 GPIO is not compatible with i.MX7ULP from HW
>>> perspective, the compatible string should keep as it is now and
>>> binding
>>
>> If it is not compatible, then how could it work before?
> 
> i.MX7ULP reg:
> 0h Port Data Output Register (PDOR) 
> 4h Port Set Output Register (PSOR) 
> 8h Port Clear Output Register (PCOR) 
> Ch Port Toggle Output Register (PTOR) 
> 10h Port Data Input Register (PDIR) 
> 14h Port Data Direction Register (PDDR)
> 
> i.MX8ULP/93 has different registers address, but 
> i.MX93 registers has 0x40 off as below:
> 40h Port Data Output (PDOR)
> 
> Even linux i.MX7ULP gpio driver could work with i.MX8ULP/93
> GPIO HW with dts node using an 0x40 offset + base addr
> for i.MX93 gpio. I think from hw design, they are
> not compatible. Besides the upper differences,
> there are other differences.

Sorry, I don't understand it. I asked how could they work before in
Linux, if they are not compatible, and you pasted regs.

So again - if they are not compatible, how could it work? Or maybe it
never worked? But then commit msg would say it.

Best regards,
Krzysztof
Peng Fan Sept. 14, 2023, 7:26 a.m. UTC | #3 
> Subject: Re: [PATCH 5/5] arm64: dts: imx93: update gpio node
> 
> On 14/09/2023 08:53, Peng Fan wrote:
> >> Subject: Re: [PATCH 5/5] arm64: dts: imx93: update gpio node
> >>
> >> On 14/09/2023 08:04, Peng Fan wrote:
> >>>> Subject: Re: [PATCH 5/5] arm64: dts: imx93: update gpio node
> >>>>
> >>>> On 14/09/2023 04:21, Peng Fan (OSS) wrote:
> >>>>> From: Peng Fan <peng.fan@nxp.com>
> >>>>>
> >>>>> Per binding doc, i.MX93 GPIO supports two interrupts, and not
> >>>>> compatible with i.MX7ULP. So update the node
> >>>>>
> >>>>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> >>>>> ---
> >>>>>  arch/arm64/boot/dts/freescale/imx93.dtsi | 20
> >>>>> ++++++++++++--------
> >>>>>  1 file changed, 12 insertions(+), 8 deletions(-)
> >>>>>
> >>>>> diff --git a/arch/arm64/boot/dts/freescale/imx93.dtsi
> >>>>> b/arch/arm64/boot/dts/freescale/imx93.dtsi
> >>>>> index 6f85a05ee7e1..011c34a57c53 100644
> >>>>> --- a/arch/arm64/boot/dts/freescale/imx93.dtsi
> >>>>> +++ b/arch/arm64/boot/dts/freescale/imx93.dtsi
> >>>>> @@ -825,11 +825,12 @@ usdhc3: mmc@428b0000 {
> >>>>>  		};
> >>>>>
> >>>>>  		gpio2: gpio@43810080 {
> >>>>> -			compatible = "fsl,imx93-gpio", "fsl,imx7ulp-gpio";
> >>>>> +			compatible = "fsl,imx93-gpio", "fsl,imx8ulp-gpio";
> >>>>
> >>>> As your driver change points, it is breaking users, so no :(
> >>>
> >>> ok. Although i.MX93 GPIO is not compatible with i.MX7ULP from HW
> >>> perspective, the compatible string should keep as it is now and
> >>> binding
> >>
> >> If it is not compatible, then how could it work before?
> >
> > i.MX7ULP reg:
> > 0h Port Data Output Register (PDOR)
> > 4h Port Set Output Register (PSOR)
> > 8h Port Clear Output Register (PCOR)
> > Ch Port Toggle Output Register (PTOR) 10h Port Data Input Register
> > (PDIR) 14h Port Data Direction Register (PDDR)
> >
> > i.MX8ULP/93 has different registers address, but
> > i.MX93 registers has 0x40 off as below:
> > 40h Port Data Output (PDOR)
> >
> > Even linux i.MX7ULP gpio driver could work with i.MX8ULP/93 GPIO HW
> > with dts node using an 0x40 offset + base addr for i.MX93 gpio. I
> > think from hw design, they are not compatible. Besides the upper
> > differences, there are other differences.
> 
> Sorry, I don't understand it. I asked how could they work before in Linux, if
> they are not compatible, and you pasted regs.
> 
> So again - if they are not compatible, how could it work? Or maybe it never
> worked? But then commit msg would say it.

Hm. From hw design perspective they are not compatible, I think.
But from programming model perspective, partial of i.MX93/8ULP registers
are same as i.MX7ULP, with offset not start from 0.

From programming model, we could say they are compatible.

Then for i.MX95, I could still use "fsl,imx95-gpio", "fsl,imx7ulp-gpio".

Thanks,
Peng.

> 
> Best regards,
> Krzysztof
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/freescale/imx93.dtsi b/arch/arm64/boot/dts/freescale/imx93.dtsi
index 6f85a05ee7e1..011c34a57c53 100644
--- a/arch/arm64/boot/dts/freescale/imx93.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx93.dtsi
@@ -825,11 +825,12 @@  usdhc3: mmc@428b0000 {
 		};
 
 		gpio2: gpio@43810080 {
-			compatible = "fsl,imx93-gpio", "fsl,imx7ulp-gpio";
+			compatible = "fsl,imx93-gpio", "fsl,imx8ulp-gpio";
 			reg = <0x43810080 0x1000>, <0x43810040 0x40>;
 			gpio-controller;
 			#gpio-cells = <2>;
-			interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>;
+			interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 58 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
 			clocks = <&clk IMX93_CLK_GPIO2_GATE>,
@@ -839,11 +840,12 @@  gpio2: gpio@43810080 {
 		};
 
 		gpio3: gpio@43820080 {
-			compatible = "fsl,imx93-gpio", "fsl,imx7ulp-gpio";
+			compatible = "fsl,imx93-gpio", "fsl,imx8ulp-gpio";
 			reg = <0x43820080 0x1000>, <0x43820040 0x40>;
 			gpio-controller;
 			#gpio-cells = <2>;
-			interrupts = <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>;
+			interrupts = <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 60 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
 			clocks = <&clk IMX93_CLK_GPIO3_GATE>,
@@ -854,11 +856,12 @@  gpio3: gpio@43820080 {
 		};
 
 		gpio4: gpio@43830080 {
-			compatible = "fsl,imx93-gpio", "fsl,imx7ulp-gpio";
+			compatible = "fsl,imx93-gpio", "fsl,imx8ulp-gpio";
 			reg = <0x43830080 0x1000>, <0x43830040 0x40>;
 			gpio-controller;
 			#gpio-cells = <2>;
-			interrupts = <GIC_SPI 189 IRQ_TYPE_LEVEL_HIGH>;
+			interrupts = <GIC_SPI 189 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 190 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
 			clocks = <&clk IMX93_CLK_GPIO4_GATE>,
@@ -868,11 +871,12 @@  gpio4: gpio@43830080 {
 		};
 
 		gpio1: gpio@47400080 {
-			compatible = "fsl,imx93-gpio", "fsl,imx7ulp-gpio";
+			compatible = "fsl,imx93-gpio", "fsl,imx8ulp-gpio";
 			reg = <0x47400080 0x1000>, <0x47400040 0x40>;
 			gpio-controller;
 			#gpio-cells = <2>;
-			interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>;
+			interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
 			clocks = <&clk IMX93_CLK_GPIO1_GATE>,