[PATCH 0/4] correct the lpuart compatible for imx8dxl/imx8ulp/imx93

[PATCH 0/4] correct the lpuart compatible for imx8dxl/imx8ulp/imx93

[Sherry Sun]

This patch set corrects the lpuart compatible for imx8dxl/imx8ulp/imx93
platforms, also add the imx8ulp compatible string in fsl-lpuart dt-binding doc. 

Sherry Sun (4):
  arm64: dts: imx8dxl: remove "fsl,imx7ulp-lpuart" compatible for
    imx8dxl
  arm64: dts: imx8ulp: remove "fsl,imx7ulp-lpuart" compatible for
    i.MX8ULP
  arm64: dts: imx93: use "fsl,imx8ulp-lpuart" compatible for imx93
  dt-bindings: serial: fsl-lpuart: add imx8ulp compatible string

 .../devicetree/bindings/serial/fsl-lpuart.yaml   |  7 +++----
 .../boot/dts/freescale/imx8dxl-ss-adma.dtsi      |  8 ++++----
 arch/arm64/boot/dts/freescale/imx8ulp.dtsi       |  8 ++++----
 arch/arm64/boot/dts/freescale/imx93.dtsi         | 16 ++++++++--------
 4 files changed, 19 insertions(+), 20 deletions(-)

-- 
2.17.1

[PATCH 1/4] arm64: dts: imx8dxl: remove "fsl,imx7ulp-lpuart" compatible for imx8dxl

[Sherry Sun]

Drop the i.MX8DXL backwards compatibility with i.MX7ULP since they
differ from clocks handling point of view.

dtbs_check has the following warnings:
linux-next/arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb: serial@5a060000: compatible: 'oneOf' conditional failed, one must be fixed:
        ['fsl,imx8qxp-lpuart', 'fsl,imx7ulp-lpuart'] is too long
        'fsl,imx93-lpuart' was expected
        'fsl,imx8qxp-lpuart' is not one of ['fsl,imx8qm-lpuart', 'fsl,imx8dxl-lpuart']
        'fsl,imxrt1050-lpuart' was expected
        'fsl,imx8ulp-lpuart' was expected
        'fsl,imx8qxp-lpuart' was expected
        'fsl,imxrt1170-lpuart' was expected
        From schema: linux-next/Documentation/devicetree/bindings/serial/fsl-lpuart.yaml

Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8dxl-ss-adma.dtsi | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8dxl-ss-adma.dtsi b/arch/arm64/boot/dts/freescale/imx8dxl-ss-adma.dtsi
index 6881330ab4c6..e2eeddf38aa3 100644
--- a/arch/arm64/boot/dts/freescale/imx8dxl-ss-adma.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8dxl-ss-adma.dtsi
@@ -36,22 +36,22 @@
 };
 
 &lpuart0 {
-	compatible = "fsl,imx8qxp-lpuart", "fsl,imx7ulp-lpuart";
+	compatible = "fsl,imx8dxl-lpuart", "fsl,imx8qxp-lpuart";
 	interrupts = <GIC_SPI 228 IRQ_TYPE_LEVEL_HIGH>;
 };
 
 &lpuart1 {
-	compatible = "fsl,imx8qxp-lpuart", "fsl,imx7ulp-lpuart";
+	compatible = "fsl,imx8dxl-lpuart", "fsl,imx8qxp-lpuart";
 	interrupts = <GIC_SPI 229 IRQ_TYPE_LEVEL_HIGH>;
 };
 
 &lpuart2 {
-	compatible = "fsl,imx8qxp-lpuart", "fsl,imx7ulp-lpuart";
+	compatible = "fsl,imx8dxl-lpuart", "fsl,imx8qxp-lpuart";
 	interrupts = <GIC_SPI 230 IRQ_TYPE_LEVEL_HIGH>;
 };
 
 &lpuart3 {
-	compatible = "fsl,imx8qxp-lpuart", "fsl,imx7ulp-lpuart";
+	compatible = "fsl,imx8dxl-lpuart", "fsl,imx8qxp-lpuart";
 	interrupts = <GIC_SPI 231 IRQ_TYPE_LEVEL_HIGH>;
 };
 
-- 
2.17.1

[PATCH 2/4] arm64: dts: imx8ulp: remove "fsl,imx7ulp-lpuart" compatible for i.MX8ULP

[Sherry Sun]

i.MX8ULP has specific "fsl,imx8ulp-lpuart" compatible in lpuart driver,
"fsl,imx7ulp-lpuart" compatible is no longer needed, and i.MX8ULP is not
fully compatible with i.MX7ULP, for example, i.MX8ulp can support
EOP(end-of-packet) function while i.MX7ulp doesn't, so remove
"fsl,imx7ulp-lpuart" compatible.

Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8ulp.dtsi | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
index 57627bdaa851..e900f5b25694 100644
--- a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
@@ -245,7 +245,7 @@
 			};
 
 			lpuart4: serial@29390000 {
-				compatible = "fsl,imx8ulp-lpuart", "fsl,imx7ulp-lpuart";
+				compatible = "fsl,imx8ulp-lpuart";
 				reg = <0x29390000 0x1000>;
 				interrupts = <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&pcc3 IMX8ULP_CLK_LPUART4>;
@@ -254,7 +254,7 @@
 			};
 
 			lpuart5: serial@293a0000 {
-				compatible = "fsl,imx8ulp-lpuart", "fsl,imx7ulp-lpuart";
+				compatible = "fsl,imx8ulp-lpuart";
 				reg = <0x293a0000 0x1000>;
 				interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&pcc3 IMX8ULP_CLK_LPUART5>;
@@ -334,7 +334,7 @@
 			};
 
 			lpuart6: serial@29860000 {
-				compatible = "fsl,imx8ulp-lpuart", "fsl,imx7ulp-lpuart";
+				compatible = "fsl,imx8ulp-lpuart";
 				reg = <0x29860000 0x1000>;
 				interrupts = <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&pcc4 IMX8ULP_CLK_LPUART6>;
@@ -343,7 +343,7 @@
 			};
 
 			lpuart7: serial@29870000 {
-				compatible = "fsl,imx8ulp-lpuart", "fsl,imx7ulp-lpuart";
+				compatible = "fsl,imx8ulp-lpuart";
 				reg = <0x29870000 0x1000>;
 				interrupts = <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&pcc4 IMX8ULP_CLK_LPUART7>;
-- 
2.17.1

Re: [PATCH 2/4] arm64: dts: imx8ulp: remove "fsl,imx7ulp-lpuart" compatible for i.MX8ULP

[Krzysztof Kozlowski]

On 25/06/2023 11:48, Sherry Sun wrote:
> i.MX8ULP has specific "fsl,imx8ulp-lpuart" compatible in lpuart driver,
> "fsl,imx7ulp-lpuart" compatible is no longer needed, and i.MX8ULP is not
> fully compatible with i.MX7ULP, for example, i.MX8ulp can support
> EOP(end-of-packet) function while i.MX7ulp doesn't, so remove
> "fsl,imx7ulp-lpuart" compatible.

That's not how compatibility works. Your commit msg says that new device
comes with new features, but your code says they are incompatible. These
are two different things.

Either drop the patch or provide correct rationale.

Best regards,
Krzysztof

[PATCH 3/4] arm64: dts: imx93: use "fsl,imx8ulp-lpuart" compatible for imx93

[Sherry Sun]

i.MX93 is much more compatible with imx8ulp than imx7ulp, for example,
i.MX93 and i.MX8ulp can support EOP(end-of-packet) function while
i.MX7ulp doesn't support.

Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx93.dtsi | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx93.dtsi b/arch/arm64/boot/dts/freescale/imx93.dtsi
index 8643612ace8c..46fda327ae5e 100644
--- a/arch/arm64/boot/dts/freescale/imx93.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx93.dtsi
@@ -252,7 +252,7 @@
 			};
 
 			lpuart1: serial@44380000 {
-				compatible = "fsl,imx93-lpuart", "fsl,imx7ulp-lpuart";
+				compatible = "fsl,imx93-lpuart", "fsl,imx8ulp-lpuart";
 				reg = <0x44380000 0x1000>;
 				interrupts = <GIC_SPI 19 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clk IMX93_CLK_LPUART1_GATE>;
@@ -261,7 +261,7 @@
 			};
 
 			lpuart2: serial@44390000 {
-				compatible = "fsl,imx93-lpuart", "fsl,imx7ulp-lpuart";
+				compatible = "fsl,imx93-lpuart", "fsl,imx8ulp-lpuart";
 				reg = <0x44390000 0x1000>;
 				interrupts = <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clk IMX93_CLK_LPUART2_GATE>;
@@ -486,7 +486,7 @@
 			};
 
 			lpuart3: serial@42570000 {
-				compatible = "fsl,imx93-lpuart", "fsl,imx7ulp-lpuart";
+				compatible = "fsl,imx93-lpuart", "fsl,imx8ulp-lpuart";
 				reg = <0x42570000 0x1000>;
 				interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clk IMX93_CLK_LPUART3_GATE>;
@@ -495,7 +495,7 @@
 			};
 
 			lpuart4: serial@42580000 {
-				compatible = "fsl,imx93-lpuart", "fsl,imx7ulp-lpuart";
+				compatible = "fsl,imx93-lpuart", "fsl,imx8ulp-lpuart";
 				reg = <0x42580000 0x1000>;
 				interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clk IMX93_CLK_LPUART4_GATE>;
@@ -504,7 +504,7 @@
 			};
 
 			lpuart5: serial@42590000 {
-				compatible = "fsl,imx93-lpuart", "fsl,imx7ulp-lpuart";
+				compatible = "fsl,imx93-lpuart", "fsl,imx8ulp-lpuart";
 				reg = <0x42590000 0x1000>;
 				interrupts = <GIC_SPI 70 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clk IMX93_CLK_LPUART5_GATE>;
@@ -513,7 +513,7 @@
 			};
 
 			lpuart6: serial@425a0000 {
-				compatible = "fsl,imx93-lpuart", "fsl,imx7ulp-lpuart";
+				compatible = "fsl,imx93-lpuart", "fsl,imx8ulp-lpuart";
 				reg = <0x425a0000 0x1000>;
 				interrupts = <GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clk IMX93_CLK_LPUART6_GATE>;
@@ -551,7 +551,7 @@
 			};
 
 			lpuart7: serial@42690000 {
-				compatible = "fsl,imx93-lpuart", "fsl,imx7ulp-lpuart";
+				compatible = "fsl,imx93-lpuart", "fsl,imx8ulp-lpuart";
 				reg = <0x42690000 0x1000>;
 				interrupts = <GIC_SPI 210 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clk IMX93_CLK_LPUART7_GATE>;
@@ -560,7 +560,7 @@
 			};
 
 			lpuart8: serial@426a0000 {
-				compatible = "fsl,imx93-lpuart", "fsl,imx7ulp-lpuart";
+				compatible = "fsl,imx93-lpuart", "fsl,imx8ulp-lpuart";
 				reg = <0x426a0000 0x1000>;
 				interrupts = <GIC_SPI 211 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clk IMX93_CLK_LPUART8_GATE>;
-- 
2.17.1

[PATCH 4/4] dt-bindings: serial: fsl-lpuart: add imx8ulp compatible string

[Sherry Sun]

LPUART driver adds a new compatible string for imx8ulp, and imx93 is
much more compatible with imx8ulp than imx7ulp, so correct the
dt-binding here.

Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
---
 Documentation/devicetree/bindings/serial/fsl-lpuart.yaml | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/serial/fsl-lpuart.yaml b/Documentation/devicetree/bindings/serial/fsl-lpuart.yaml
index 93062403276b..5599be95400b 100644
--- a/Documentation/devicetree/bindings/serial/fsl-lpuart.yaml
+++ b/Documentation/devicetree/bindings/serial/fsl-lpuart.yaml
@@ -21,13 +21,12 @@ properties:
           - fsl,ls1021a-lpuart
           - fsl,ls1028a-lpuart
           - fsl,imx7ulp-lpuart
+          - fsl,imx8ulp-lpuart
           - fsl,imx8qxp-lpuart
           - fsl,imxrt1050-lpuart
       - items:
-          - enum:
-              - fsl,imx93-lpuart
-              - fsl,imx8ulp-lpuart
-          - const: fsl,imx7ulp-lpuart
+          - const: fsl,imx93-lpuart
+          - const: fsl,imx8ulp-lpuart
       - items:
           - enum:
               - fsl,imx8qm-lpuart
-- 
2.17.1

Re: [PATCH 4/4] dt-bindings: serial: fsl-lpuart: add imx8ulp compatible string

[Krzysztof Kozlowski]

On 25/06/2023 11:48, Sherry Sun wrote:
> LPUART driver adds a new compatible string for imx8ulp, and imx93 is

What driver adds or not, is not really correct argument for
compatibility. There are plenty of compatible devices which have both
device ID entries in the driver. Why would you drop their compatibility
between each other? It does not work like that... Provide clear
rationale for this.

> much more compatible with imx8ulp than imx7ulp, so correct the
> dt-binding here.
> 

"Much more compatible" is too vague.


Best regards,
Krzysztof

RE: [PATCH 4/4] dt-bindings: serial: fsl-lpuart: add imx8ulp compatible string

[Sherry Sun]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: 2023年6月25日 18:18
> To: Sherry Sun <sherry.sun@nxp.com>; robh+dt@kernel.org;
> krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org;
> shawnguo@kernel.org; s.hauer@pengutronix.de; festevam@gmail.com;
> Shenwei Wang <shenwei.wang@nxp.com>; gregkh@linuxfoundation.org
> Cc: devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> kernel@pengutronix.de; linux-serial@vger.kernel.org; linux-
> kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
> Subject: Re: [PATCH 4/4] dt-bindings: serial: fsl-lpuart: add imx8ulp
> compatible string
> 
> On 25/06/2023 11:48, Sherry Sun wrote:
> > LPUART driver adds a new compatible string for imx8ulp, and imx93 is
> 
> What driver adds or not, is not really correct argument for compatibility.
> There are plenty of compatible devices which have both device ID entries in
> the driver. Why would you drop their compatibility between each other? It
> does not work like that... Provide clear rationale for this.

Hi Krzysztof, 
We take imx7ulp lpuart as the basic version IP, then imx8ulp adds some improved features based on the imx7ulp lpuart IP.
Actually imx8ulp can also work with the imx7ulp version, we can say it is compatible with imx7ulp, but we need to enable those new features by default on imx8ulp, so we added the imx8ulp-lpuart compatible in lpuart driver. So for this case, maybe compatible = "fsl,imx8ulp-lpuart", "fsl,imx7ulp-lpuart" is correct for imx8ulp platform, right?

Then we have the imx93 family with the same lpuart version as imx8ulp, so how should we handle the compatible string for imx93 platform? Maybe compatible = "fsl,imx93-lpuart", "fsl,imx8ulp-lpuart", "fsl,imx7ulp-lpuart"? But three compatible strings looks too long for me, so I want to drop imx7ulp-lpuart and take imx8ulp-lpuart as the basic compatible string for imx93 and later others imx9 series, I am not sure if it is suitable, some suggestions here will be appreciate, thanks!

Best Regards
Sherry

> 
> > much more compatible with imx8ulp than imx7ulp, so correct the
> > dt-binding here.
> >
> 
> "Much more compatible" is too vague.
> 
> 
> Best regards,
> Krzysztof

Re: [PATCH 4/4] dt-bindings: serial: fsl-lpuart: add imx8ulp compatible string

[Krzysztof Kozlowski]

On 25/06/2023 15:12, Sherry Sun wrote:
> 
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Sent: 2023年6月25日 18:18
>> To: Sherry Sun <sherry.sun@nxp.com>; robh+dt@kernel.org;
>> krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org;
>> shawnguo@kernel.org; s.hauer@pengutronix.de; festevam@gmail.com;
>> Shenwei Wang <shenwei.wang@nxp.com>; gregkh@linuxfoundation.org
>> Cc: devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
>> kernel@pengutronix.de; linux-serial@vger.kernel.org; linux-
>> kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
>> Subject: Re: [PATCH 4/4] dt-bindings: serial: fsl-lpuart: add imx8ulp
>> compatible string
>>
>> On 25/06/2023 11:48, Sherry Sun wrote:
>>> LPUART driver adds a new compatible string for imx8ulp, and imx93 is
>>
>> What driver adds or not, is not really correct argument for compatibility.
>> There are plenty of compatible devices which have both device ID entries in
>> the driver. Why would you drop their compatibility between each other? It
>> does not work like that... Provide clear rationale for this.
> 
> Hi Krzysztof, 
> We take imx7ulp lpuart as the basic version IP, then imx8ulp adds some improved features based on the imx7ulp lpuart IP.
> Actually imx8ulp can also work with the imx7ulp version, we can say it is compatible with imx7ulp, but we need to enable those new features by default on imx8ulp, so we added the imx8ulp-lpuart compatible in lpuart driver. So for this case, maybe compatible = "fsl,imx8ulp-lpuart", "fsl,imx7ulp-lpuart" is correct for imx8ulp platform, right?
> 
> Then we have the imx93 family with the same lpuart version as imx8ulp, so how should we handle the compatible string for imx93 platform? Maybe compatible = "fsl,imx93-lpuart", "fsl,imx8ulp-lpuart", "fsl,imx7ulp-lpuart"? But three compatible strings looks too long for me, so I want to drop imx7ulp-lpuart and take imx8ulp-lpuart as the basic compatible string for imx93 and later others imx9 series, I am not sure if it is suitable, some suggestions here will be appreciate, thanks!
> 

Wrap your responses.

So write that the reason of this change is because looks too long for
you and we will judge that rationale. Three compatibles is still fine.

Best regards,
Krzysztof

RE: [PATCH 4/4] dt-bindings: serial: fsl-lpuart: add imx8ulp compatible string

[Sherry Sun]

> >>
> >> On 25/06/2023 11:48, Sherry Sun wrote:
> >>> LPUART driver adds a new compatible string for imx8ulp, and imx93 is
> >>
> >> What driver adds or not, is not really correct argument for compatibility.
> >> There are plenty of compatible devices which have both device ID
> >> entries in the driver. Why would you drop their compatibility between
> >> each other? It does not work like that... Provide clear rationale for this.
> >
> > Hi Krzysztof,
> > We take imx7ulp lpuart as the basic version IP, then imx8ulp adds some
> improved features based on the imx7ulp lpuart IP.
> > Actually imx8ulp can also work with the imx7ulp version, we can say it is
> compatible with imx7ulp, but we need to enable those new features by
> default on imx8ulp, so we added the imx8ulp-lpuart compatible in lpuart
> driver. So for this case, maybe compatible = "fsl,imx8ulp-lpuart",
> "fsl,imx7ulp-lpuart" is correct for imx8ulp platform, right?
> >
> > Then we have the imx93 family with the same lpuart version as imx8ulp, so
> how should we handle the compatible string for imx93 platform? Maybe
> compatible = "fsl,imx93-lpuart", "fsl,imx8ulp-lpuart", "fsl,imx7ulp-lpuart"?
> But three compatible strings looks too long for me, so I want to drop
> imx7ulp-lpuart and take imx8ulp-lpuart as the basic compatible string for
> imx93 and later others imx9 series, I am not sure if it is suitable, some
> suggestions here will be appreciate, thanks!
> >
> 
> Wrap your responses.
> 
> So write that the reason of this change is because looks too long for you and
> we will judge that rationale. Three compatibles is still fine.
> 
Hi Krzysztof,
Got it, thanks for your suggestion, I will use three compatibles for imx93.

Best Regards
Sherry