[PATCH 0/5] gpio: update i.MX93/8ULP and support i.MX95

[PATCH 0/5] gpio: update i.MX93/8ULP and support i.MX95

[Peng Fan (OSS)]

From hardware perspective i.MX8ULP GPIO supports two interrupts,
1st for Trustzone non-secure irq, 2nd for Trustzone secure irq.

While i.MX7ULP GPIO only supports one interrupt, so i.MX8ULP is not
compatible with i.MX7ULP.

i.MX93 GPIO is directly derived from i.MX8ULP, so make i.MX93 compatible
with i.MX8ULP. i.MX95 GPIO is same as i.MX93, so also compatible with
i.MX8ULP

There maybe dtbs_check failure if only test the 1st patch. After
the patchset applied, no failure.

---
Peng Fan (5):
      dt-bindings: gpio: vf610: correct i.MX8ULP and i.MX93 interrupts
      dt-bindings: gpio: vf610: add i.MX95 compatible
      gpio: vf610: add i.MX8ULP of_device_id entry
      arm64: dts: imx8ulp: update gpio node
      arm64: dts: imx93: update gpio node

 .../devicetree/bindings/gpio/gpio-vf610.yaml       | 25 +++++++++++++++++++---
 arch/arm64/boot/dts/freescale/imx8ulp.dtsi         | 15 +++++++------
 arch/arm64/boot/dts/freescale/imx93.dtsi           | 20 ++++++++++-------
 drivers/gpio/gpio-vf610.c                          |  1 +
 4 files changed, 44 insertions(+), 17 deletions(-)
---
base-commit: e143016b56ecb0fcda5bb6026b0a25fe55274f56
change-id: 20230914-vf610-gpio-46edacd2b513

Best regards,
-- 
Peng Fan <peng.fan@nxp.com>

[PATCH 1/5] dt-bindings: gpio: vf610: correct i.MX8ULP and i.MX93 interrupts

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

i.MX8ULP and i.MX93 actually has two interrupts for each gpio
controller, one for Trustzone non-secure world, one for secure world.

And i.MX93 is directly derived from i.MX8ULP, so make i.MX93 GPIO
compatible with i.MX8ULP

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 .../devicetree/bindings/gpio/gpio-vf610.yaml       | 27 ++++++++++++++++++----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpio/gpio-vf610.yaml b/Documentation/devicetree/bindings/gpio/gpio-vf610.yaml
index 7c2d152e8617..2593cc96fcbb 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-vf610.yaml
+++ b/Documentation/devicetree/bindings/gpio/gpio-vf610.yaml
@@ -20,15 +20,14 @@ description: |
 properties:
   compatible:
     oneOf:
+      - const: fsl,imx8ulp-gpio
       - const: fsl,vf610-gpio
       - items:
           - const: fsl,imx7ulp-gpio
           - const: fsl,vf610-gpio
       - items:
-          - enum:
-              - fsl,imx93-gpio
-              - fsl,imx8ulp-gpio
-          - const: fsl,imx7ulp-gpio
+          - const: fsl,imx93-gpio
+          - const: fsl,imx8ulp-gpio
 
   reg:
     description: The first reg tuple represents the PORT module, the second tuple
@@ -36,7 +35,8 @@ properties:
     maxItems: 2
 
   interrupts:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
 
   interrupt-controller: true
 
@@ -77,6 +77,23 @@ required:
   - "#gpio-cells"
   - gpio-controller
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - fsl,vf610-gpio
+              - fsl,imx7ulp-gpio
+    then:
+      properties:
+        interrupts:
+          maxItems: 1
+    else:
+      properties:
+        interrupts:
+          maxItems: 2
+
 additionalProperties: false
 
 examples:

-- 
2.37.1

[PATCH 2/5] dt-bindings: gpio: vf610: add i.MX95 compatible

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

Add i.MX95 compatible string which is compatible with i.MX8ULP

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 Documentation/devicetree/bindings/gpio/gpio-vf610.yaml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/gpio/gpio-vf610.yaml b/Documentation/devicetree/bindings/gpio/gpio-vf610.yaml
index 2593cc96fcbb..d0db2089cfab 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-vf610.yaml
+++ b/Documentation/devicetree/bindings/gpio/gpio-vf610.yaml
@@ -26,7 +26,9 @@ properties:
           - const: fsl,imx7ulp-gpio
           - const: fsl,vf610-gpio
       - items:
-          - const: fsl,imx93-gpio
+          - enum:
+              - fsl,imx93-gpio
+              - fsl,imx95-gpio
           - const: fsl,imx8ulp-gpio
 
   reg:

-- 
2.37.1

[PATCH 3/5] gpio: vf610: add i.MX8ULP of_device_id entry

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

i.MX8ULP supports two interrupts, while i.MX7ULP supports one interrupt.
So from hardware perspective, they are not compatible.

So add entry for i.MX8ULP.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/gpio/gpio-vf610.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpio/gpio-vf610.c b/drivers/gpio/gpio-vf610.c
index dbc7ba0ee72c..88f7215cdf4b 100644
--- a/drivers/gpio/gpio-vf610.c
+++ b/drivers/gpio/gpio-vf610.c
@@ -67,6 +67,7 @@ static const struct fsl_gpio_soc_data imx_data = {
 static const struct of_device_id vf610_gpio_dt_ids[] = {
 	{ .compatible = "fsl,vf610-gpio",	.data = NULL, },
 	{ .compatible = "fsl,imx7ulp-gpio",	.data = &imx_data, },
+	{ .compatible = "fsl,imx8ulp-gpio",	.data = &imx_data, },
 	{ /* sentinel */ }
 };
 

-- 
2.37.1

[PATCH 4/5] arm64: dts: imx8ulp: update gpio node

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

The i.MX8ULP GPIO supports two interrupts, and actually not compatible
with i.MX7ULP. Update the node following dt-binding doc.

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

diff --git a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
index 8a6596d5a581..d80ba041ee0e 100644
--- a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
@@ -484,11 +484,12 @@ fec: ethernet@29950000 {
 		};
 
 		gpioe: gpio@2d000080 {
-				compatible = "fsl,imx8ulp-gpio", "fsl,imx7ulp-gpio";
+				compatible = "fsl,imx8ulp-gpio";
 				reg = <0x2d000080 0x1000>, <0x2d000040 0x40>;
 				gpio-controller;
 				#gpio-cells = <2>;
-				interrupts = <GIC_SPI 129 IRQ_TYPE_LEVEL_HIGH>;
+				interrupts = <GIC_SPI 129 IRQ_TYPE_LEVEL_HIGH>,
+					     <GIC_SPI 130 IRQ_TYPE_LEVEL_HIGH>;
 				interrupt-controller;
 				#interrupt-cells = <2>;
 				clocks = <&pcc4 IMX8ULP_CLK_RGPIOE>,
@@ -498,11 +499,12 @@ gpioe: gpio@2d000080 {
 		};
 
 		gpiof: gpio@2d010080 {
-				compatible = "fsl,imx8ulp-gpio", "fsl,imx7ulp-gpio";
+				compatible = "fsl,imx8ulp-gpio";
 				reg = <0x2d010080 0x1000>, <0x2d010040 0x40>;
 				gpio-controller;
 				#gpio-cells = <2>;
-				interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>;
+				interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>,
+					     <GIC_SPI 132 IRQ_TYPE_LEVEL_HIGH>;
 				interrupt-controller;
 				#interrupt-cells = <2>;
 				clocks = <&pcc4 IMX8ULP_CLK_RGPIOF>,
@@ -533,11 +535,12 @@ pcc5: clock-controller@2da70000 {
 		};
 
 		gpiod: gpio@2e200080 {
-			compatible = "fsl,imx8ulp-gpio", "fsl,imx7ulp-gpio";
+			compatible = "fsl,imx8ulp-gpio";
 			reg = <0x2e200080 0x1000>, <0x2e200040 0x40>;
 			gpio-controller;
 			#gpio-cells = <2>;
-			interrupts = <GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH>;
+			interrupts = <GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 128 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
 			clocks = <&pcc5 IMX8ULP_CLK_RGPIOD>,

-- 
2.37.1

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

[Peng Fan (OSS)]

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";
 			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>,

-- 
2.37.1

Re: [PATCH 1/5] dt-bindings: gpio: vf610: correct i.MX8ULP and i.MX93 interrupts

[Krzysztof Kozlowski]

On 14/09/2023 04:20, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> i.MX8ULP and i.MX93 actually has two interrupts for each gpio
> controller, one for

...

> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - fsl,vf610-gpio
> +              - fsl,imx7ulp-gpio
> +    then:
> +      properties:
> +        interrupts:
> +          maxItems: 1
> +    else:
> +      properties:
> +        interrupts:
> +          maxItems: 2

Instead describe the items with "items:" and descriptions.

Best regards,
Krzysztof

Re: [PATCH 2/5] dt-bindings: gpio: vf610: add i.MX95 compatible

[Krzysztof Kozlowski]

On 14/09/2023 04:20, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> Add i.MX95 compatible string which is compatible with i.MX8ULP
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  Documentation/devicetree/bindings/gpio/gpio-vf610.yaml | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-vf610.yaml b/Documentation/devicetree/bindings/gpio/gpio-vf610.yaml
> index 2593cc96fcbb..d0db2089cfab 100644
> --- a/Documentation/devicetree/bindings/gpio/gpio-vf610.yaml
> +++ b/Documentation/devicetree/bindings/gpio/gpio-vf610.yaml
> @@ -26,7 +26,9 @@ properties:
>            - const: fsl,imx7ulp-gpio
>            - const: fsl,vf610-gpio
>        - items:
> -          - const: fsl,imx93-gpio
> +          - enum:

Just make it an enum in your previous patch, so you won't change this
line twice.

> +              - fsl,imx93-gpio

Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

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 :(

Best regards,
Krzysztof

Re: [PATCH 3/5] gpio: vf610: add i.MX8ULP of_device_id entry

[Krzysztof Kozlowski]

On 14/09/2023 04:20, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> i.MX8ULP supports two interrupts, while i.MX7ULP supports one interrupt.
> So from hardware perspective, they are not compatible.
> 
> So add entry for i.MX8ULP.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/gpio/gpio-vf610.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpio/gpio-vf610.c b/drivers/gpio/gpio-vf610.c
> index dbc7ba0ee72c..88f7215cdf4b 100644
> --- a/drivers/gpio/gpio-vf610.c
> +++ b/drivers/gpio/gpio-vf610.c
> @@ -67,6 +67,7 @@ static const struct fsl_gpio_soc_data imx_data = {
>  static const struct of_device_id vf610_gpio_dt_ids[] = {
>  	{ .compatible = "fsl,vf610-gpio",	.data = NULL, },
>  	{ .compatible = "fsl,imx7ulp-gpio",	.data = &imx_data, },
> +	{ .compatible = "fsl,imx8ulp-gpio",	.data = &imx_data, },

Why? It is the same as imx7. No need.

Best regards,
Krzysztof

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

[Peng Fan]

> 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
doc keep as it is now?

Thanks,
Peng

> 
> Best regards,
> Krzysztof

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

[Krzysztof Kozlowski]

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?



Best regards,
Krzysztof

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

[Peng Fan]

> 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

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

[Krzysztof Kozlowski]

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

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

[Peng Fan]

> 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

Re: [PATCH 3/5] gpio: vf610: add i.MX8ULP of_device_id entry

[Linus Walleij]

On Thu, Sep 14, 2023 at 7:48 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
> On 14/09/2023 04:20, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > i.MX8ULP supports two interrupts, while i.MX7ULP supports one interrupt.
> > So from hardware perspective, they are not compatible.
> >
> > So add entry for i.MX8ULP.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  drivers/gpio/gpio-vf610.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/gpio/gpio-vf610.c b/drivers/gpio/gpio-vf610.c
> > index dbc7ba0ee72c..88f7215cdf4b 100644
> > --- a/drivers/gpio/gpio-vf610.c
> > +++ b/drivers/gpio/gpio-vf610.c
> > @@ -67,6 +67,7 @@ static const struct fsl_gpio_soc_data imx_data = {
> >  static const struct of_device_id vf610_gpio_dt_ids[] = {
> >       { .compatible = "fsl,vf610-gpio",       .data = NULL, },
> >       { .compatible = "fsl,imx7ulp-gpio",     .data = &imx_data, },
> > +     { .compatible = "fsl,imx8ulp-gpio",     .data = &imx_data, },
>
> Why? It is the same as imx7. No need.

Because compatible = "fsl,imx7ulp-gpio" is not what is going to be in the
device tree, but compatible = "fsl,imx8ulp-gpio"?

What am I missing here? Maybe the commit message is weird.

Yours,
Linus Walleij

Re: [PATCH 3/5] gpio: vf610: add i.MX8ULP of_device_id entry

[Krzysztof Kozlowski]

On 14/09/2023 10:48, Linus Walleij wrote:
> On Thu, Sep 14, 2023 at 7:48 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>> On 14/09/2023 04:20, Peng Fan (OSS) wrote:
>>> From: Peng Fan <peng.fan@nxp.com>
>>>
>>> i.MX8ULP supports two interrupts, while i.MX7ULP supports one interrupt.
>>> So from hardware perspective, they are not compatible.
>>>
>>> So add entry for i.MX8ULP.
>>>
>>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>>> ---
>>>  drivers/gpio/gpio-vf610.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/gpio/gpio-vf610.c b/drivers/gpio/gpio-vf610.c
>>> index dbc7ba0ee72c..88f7215cdf4b 100644
>>> --- a/drivers/gpio/gpio-vf610.c
>>> +++ b/drivers/gpio/gpio-vf610.c
>>> @@ -67,6 +67,7 @@ static const struct fsl_gpio_soc_data imx_data = {
>>>  static const struct of_device_id vf610_gpio_dt_ids[] = {
>>>       { .compatible = "fsl,vf610-gpio",       .data = NULL, },
>>>       { .compatible = "fsl,imx7ulp-gpio",     .data = &imx_data, },
>>> +     { .compatible = "fsl,imx8ulp-gpio",     .data = &imx_data, },
>>
>> Why? It is the same as imx7. No need.
> 
> Because compatible = "fsl,imx7ulp-gpio" is not what is going to be in the
> device tree, but compatible = "fsl,imx8ulp-gpio"?
> 
> What am I missing here? Maybe the commit message is weird.
> 

If the devices used before and are still going to use same driver data,
they look compatible from OS point of view. Therefore usually we express
such compatibility and do not add unneeded device_id entries.

Now whether the devices are truly compatible or not, I don't know and
with some recent emails I am bit confused.

Best regards,
Krzysztof

RE: [PATCH 3/5] gpio: vf610: add i.MX8ULP of_device_id entry

[Peng Fan]

> Subject: Re: [PATCH 3/5] gpio: vf610: add i.MX8ULP of_device_id entry
> 
> On 14/09/2023 10:48, Linus Walleij wrote:
> > On Thu, Sep 14, 2023 at 7:48 AM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >> On 14/09/2023 04:20, Peng Fan (OSS) wrote:
> >>> From: Peng Fan <peng.fan@nxp.com>
> >>>
> >>> i.MX8ULP supports two interrupts, while i.MX7ULP supports one
> interrupt.
> >>> So from hardware perspective, they are not compatible.
> >>>
> >>> So add entry for i.MX8ULP.
> >>>
> >>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> >>> ---
> >>>  drivers/gpio/gpio-vf610.c | 1 +
> >>>  1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/drivers/gpio/gpio-vf610.c b/drivers/gpio/gpio-vf610.c
> >>> index dbc7ba0ee72c..88f7215cdf4b 100644
> >>> --- a/drivers/gpio/gpio-vf610.c
> >>> +++ b/drivers/gpio/gpio-vf610.c
> >>> @@ -67,6 +67,7 @@ static const struct fsl_gpio_soc_data imx_data = {
> >>> static const struct of_device_id vf610_gpio_dt_ids[] = {
> >>>       { .compatible = "fsl,vf610-gpio",       .data = NULL, },
> >>>       { .compatible = "fsl,imx7ulp-gpio",     .data = &imx_data, },
> >>> +     { .compatible = "fsl,imx8ulp-gpio",     .data = &imx_data, },
> >>
> >> Why? It is the same as imx7. No need.
> >
> > Because compatible = "fsl,imx7ulp-gpio" is not what is going to be in
> > the device tree, but compatible = "fsl,imx8ulp-gpio"?
> >
> > What am I missing here? Maybe the commit message is weird.
> >
> 
> If the devices used before and are still going to use same driver data, they
> look compatible from OS point of view. Therefore usually we express such
> compatibility and do not add unneeded device_id entries.
> 
> Now whether the devices are truly compatible or not, I don't know and with
> some recent emails I am bit confused.

Some tricks in dtb are made to make the driver could work for both i.MX7ULP,
i.MX8ULP, i.MX93 with fsl,imx7ulp-gpio.

Such as i.MX8ULP:
reg = <0x2d000080 0x1000>, <0x2d000040 0x40>;
Actually the two regs are belong to one physical continuous space,
<0x2d000000 0x1000>
Just to i.MX8ULP could reuse the vf610 gpio driver, the regs are partitioned
into two with some offset added

So from hw, I think they are not compatible, just some sw tricks to
make the linux driver could work for both platform.

Thanks,
Peng.

> 
> Best regards,
> Krzysztof

Re: [PATCH 3/5] gpio: vf610: add i.MX8ULP of_device_id entry

[Krzysztof Kozlowski]

On 14/09/2023 11:08, Peng Fan wrote:
>> Subject: Re: [PATCH 3/5] gpio: vf610: add i.MX8ULP of_device_id entry
>>
>> On 14/09/2023 10:48, Linus Walleij wrote:
>>> On Thu, Sep 14, 2023 at 7:48 AM Krzysztof Kozlowski
>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>> On 14/09/2023 04:20, Peng Fan (OSS) wrote:
>>>>> From: Peng Fan <peng.fan@nxp.com>
>>>>>
>>>>> i.MX8ULP supports two interrupts, while i.MX7ULP supports one
>> interrupt.
>>>>> So from hardware perspective, they are not compatible.
>>>>>
>>>>> So add entry for i.MX8ULP.
>>>>>
>>>>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>>>>> ---
>>>>>  drivers/gpio/gpio-vf610.c | 1 +
>>>>>  1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/drivers/gpio/gpio-vf610.c b/drivers/gpio/gpio-vf610.c
>>>>> index dbc7ba0ee72c..88f7215cdf4b 100644
>>>>> --- a/drivers/gpio/gpio-vf610.c
>>>>> +++ b/drivers/gpio/gpio-vf610.c
>>>>> @@ -67,6 +67,7 @@ static const struct fsl_gpio_soc_data imx_data = {
>>>>> static const struct of_device_id vf610_gpio_dt_ids[] = {
>>>>>       { .compatible = "fsl,vf610-gpio",       .data = NULL, },
>>>>>       { .compatible = "fsl,imx7ulp-gpio",     .data = &imx_data, },
>>>>> +     { .compatible = "fsl,imx8ulp-gpio",     .data = &imx_data, },
>>>>
>>>> Why? It is the same as imx7. No need.
>>>
>>> Because compatible = "fsl,imx7ulp-gpio" is not what is going to be in
>>> the device tree, but compatible = "fsl,imx8ulp-gpio"?
>>>
>>> What am I missing here? Maybe the commit message is weird.
>>>
>>
>> If the devices used before and are still going to use same driver data, they
>> look compatible from OS point of view. Therefore usually we express such
>> compatibility and do not add unneeded device_id entries.
>>
>> Now whether the devices are truly compatible or not, I don't know and with
>> some recent emails I am bit confused.
> 
> Some tricks in dtb are made to make the driver could work for both i.MX7ULP,
> i.MX8ULP, i.MX93 with fsl,imx7ulp-gpio.
> 
> Such as i.MX8ULP:
> reg = <0x2d000080 0x1000>, <0x2d000040 0x40>;
> Actually the two regs are belong to one physical continuous space,
> <0x2d000000 0x1000>
> Just to i.MX8ULP could reuse the vf610 gpio driver, the regs are partitioned
> into two with some offset added
> 
> So from hw, I think they are not compatible, just some sw tricks to
> make the linux driver could work for both platform.

OK, that sounds like reason to clean this up - drivers, bindings and
finally DTS - all with proper explanation justifying affected DTS users.

Best regards,
Krzysztof

Re: [PATCH 1/5] dt-bindings: gpio: vf610: correct i.MX8ULP and i.MX93 interrupts

[Rob Herring]

On Thu, Sep 14, 2023 at 07:47:29AM +0200, Krzysztof Kozlowski wrote:
> On 14/09/2023 04:20, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> > 
> > i.MX8ULP and i.MX93 actually has two interrupts for each gpio
> > controller, one for
> 
> ...
> 
> > +allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - fsl,vf610-gpio
> > +              - fsl,imx7ulp-gpio
> > +    then:
> > +      properties:
> > +        interrupts:
> > +          maxItems: 1
> > +    else:
> > +      properties:
> > +        interrupts:
> > +          maxItems: 2
> 
> Instead describe the items with "items:" and descriptions.

Except not here, but in the top level 'interrupts' entry.

Rob