[PATCH v3 3/4] dt-bindings: mmc: fsl-imx-esdhc: add NXP S32G3 support

Wadim Mueller posted 4 patches 1 year, 9 months ago
There is a newer version of this series
[PATCH v3 3/4] dt-bindings: mmc: fsl-imx-esdhc: add NXP S32G3 support
Posted by Wadim Mueller 1 year, 9 months ago
Add a compatible string for the SDHC binding of NXP S32G3 platforms. Here
we use "nxp,s32g2-usdhc" as fallback since the s32g2-usdhc
driver works also on S32G3 platforms.

Signed-off-by: Wadim Mueller <wafgo01@gmail.com>
---
 Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
index 82eb7a24c857..b42b4368fa4e 100644
--- a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
+++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
@@ -35,6 +35,7 @@ properties:
           - fsl,imx8mm-usdhc
           - fsl,imxrt1050-usdhc
           - nxp,s32g2-usdhc
+          - nxp,s32g3-usdhc
       - items:
           - const: fsl,imx50-esdhc
           - const: fsl,imx53-esdhc
@@ -90,6 +91,9 @@ properties:
           - enum:
               - fsl,imxrt1170-usdhc
           - const: fsl,imxrt1050-usdhc
+      - items:
+          - const: nxp,s32g3-usdhc
+          - const: nxp,s32g2-usdhc
 
   reg:
     maxItems: 1
-- 
2.25.1
Re: [PATCH v3 3/4] dt-bindings: mmc: fsl-imx-esdhc: add NXP S32G3 support
Posted by Krzysztof Kozlowski 1 year, 9 months ago
On 21/03/2024 16:41, Wadim Mueller wrote:
> Add a compatible string for the SDHC binding of NXP S32G3 platforms. Here
> we use "nxp,s32g2-usdhc" as fallback since the s32g2-usdhc
> driver works also on S32G3 platforms.
> 
> Signed-off-by: Wadim Mueller <wafgo01@gmail.com>
> ---
>  Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
> index 82eb7a24c857..b42b4368fa4e 100644
> --- a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
> +++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
> @@ -35,6 +35,7 @@ properties:
>            - fsl,imx8mm-usdhc
>            - fsl,imxrt1050-usdhc
>            - nxp,s32g2-usdhc
> +          - nxp,s32g3-usdhc
>        - items:
>            - const: fsl,imx50-esdhc
>            - const: fsl,imx53-esdhc
> @@ -90,6 +91,9 @@ properties:
>            - enum:
>                - fsl,imxrt1170-usdhc
>            - const: fsl,imxrt1050-usdhc
> +      - items:
> +          - const: nxp,s32g3-usdhc
> +          - const: nxp,s32g2-usdhc

No, that's just wrong. G3 is not and is compatible with G2? There is no
dualism here. Either it is or it is not. Not both.

Best regards,
Krzysztof
Re: [PATCH v3 3/4] dt-bindings: mmc: fsl-imx-esdhc: add NXP S32G3 support
Posted by Wadim Mueller 1 year, 9 months ago
On Thu, Mar 21, 2024 at 06:53:34PM +0100, Krzysztof Kozlowski wrote:
> On 21/03/2024 16:41, Wadim Mueller wrote:
> > Add a compatible string for the SDHC binding of NXP S32G3 platforms. Here
> > we use "nxp,s32g2-usdhc" as fallback since the s32g2-usdhc
> > driver works also on S32G3 platforms.
> > 
> > Signed-off-by: Wadim Mueller <wafgo01@gmail.com>
> > ---
> >  Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
> > index 82eb7a24c857..b42b4368fa4e 100644
> > --- a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
> > +++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
> > @@ -35,6 +35,7 @@ properties:
> >            - fsl,imx8mm-usdhc
> >            - fsl,imxrt1050-usdhc
> >            - nxp,s32g2-usdhc
> > +          - nxp,s32g3-usdhc
> >        - items:
> >            - const: fsl,imx50-esdhc
> >            - const: fsl,imx53-esdhc
> > @@ -90,6 +91,9 @@ properties:
> >            - enum:
> >                - fsl,imxrt1170-usdhc
> >            - const: fsl,imxrt1050-usdhc
> > +      - items:
> > +          - const: nxp,s32g3-usdhc
> > +          - const: nxp,s32g2-usdhc
> 
> No, that's just wrong. G3 is not and is compatible with G2? There is no
> dualism here. Either it is or it is not. Not both.
>

I am trying to understand your statement but I am not sure whether I get
it right.

Let me try to explain what I understand is wrong with this patch. 

Having nxp,s32g2-usdhc and nxp,s32g2-usdhc in one enum

> >            - nxp,s32g2-usdhc
> > +          - nxp,s32g3-usdhc

would mean that those are 
__not__ compatible with each other, whereas the anouther item

> > +      - items:
> > +          - const: nxp,s32g3-usdhc
> > +          - const: nxp,s32g2-usdhc
> 

where both const entries are side by side means that those are compatible. Which is
paradox. Is this undersanding correct?

So if I want to have the follwing im my DTS for the mmc node

usdhc0: mmc@402f0000 {
			compatible = "nxp,s32g3-usdhc",
				     "nxp,s32g2-usdhc";
				     ...
}

The schema update should contain just this part?

i@@ -90,6 +90,9 @@ properties:
           - enum:
               - fsl,imxrt1170-usdhc
           - const: fsl,imxrt1050-usdhc
+      - items:
+          - const: nxp,s32g3-usdhc
+          - const: nxp,s32g2-usdhc
 
   reg:
     maxItems: 1


Is this correct?

With this patch in place I dont see any issues with an 

"make CHECK_DTBS=y freescale/s32g399a-rdb3.dtb"

as well as "make dt_binding_check dtbs_check" seems to be OK with this.


Thanks for your guidence so far, much appreciated!

Best Regard
Wadim



> Best regards,
> Krzysztof
>
Re: [PATCH v3 3/4] dt-bindings: mmc: fsl-imx-esdhc: add NXP S32G3 support
Posted by Krzysztof Kozlowski 1 year, 9 months ago
On 22/03/2024 10:45, Wadim Mueller wrote:
> On Thu, Mar 21, 2024 at 06:53:34PM +0100, Krzysztof Kozlowski wrote:
>> On 21/03/2024 16:41, Wadim Mueller wrote:
>>> Add a compatible string for the SDHC binding of NXP S32G3 platforms. Here
>>> we use "nxp,s32g2-usdhc" as fallback since the s32g2-usdhc
>>> driver works also on S32G3 platforms.
>>>
>>> Signed-off-by: Wadim Mueller <wafgo01@gmail.com>
>>> ---
>>>  Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
>>> index 82eb7a24c857..b42b4368fa4e 100644
>>> --- a/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
>>> +++ b/Documentation/devicetree/bindings/mmc/fsl-imx-esdhc.yaml
>>> @@ -35,6 +35,7 @@ properties:
>>>            - fsl,imx8mm-usdhc
>>>            - fsl,imxrt1050-usdhc
>>>            - nxp,s32g2-usdhc
>>> +          - nxp,s32g3-usdhc
>>>        - items:
>>>            - const: fsl,imx50-esdhc
>>>            - const: fsl,imx53-esdhc
>>> @@ -90,6 +91,9 @@ properties:
>>>            - enum:
>>>                - fsl,imxrt1170-usdhc
>>>            - const: fsl,imxrt1050-usdhc
>>> +      - items:
>>> +          - const: nxp,s32g3-usdhc
>>> +          - const: nxp,s32g2-usdhc
>>
>> No, that's just wrong. G3 is not and is compatible with G2? There is no
>> dualism here. Either it is or it is not. Not both.
>>
> 
> I am trying to understand your statement but I am not sure whether I get
> it right.
> 
> Let me try to explain what I understand is wrong with this patch. 
> 
> Having nxp,s32g2-usdhc and nxp,s32g2-usdhc in one enum
> 
>>>            - nxp,s32g2-usdhc
>>> +          - nxp,s32g3-usdhc
> 
> would mean that those are 
> __not__ compatible with each other, whereas the anouther item
> 
>>> +      - items:
>>> +          - const: nxp,s32g3-usdhc
>>> +          - const: nxp,s32g2-usdhc
>>
> 
> where both const entries are side by side means that those are compatible. Which is
> paradox. Is this undersanding correct?

Yes, you placed the same compatible in two separate places. It has two
separate meanings.

> 
> So if I want to have the follwing im my DTS for the mmc node
> 
> usdhc0: mmc@402f0000 {
> 			compatible = "nxp,s32g3-usdhc",
> 				     "nxp,s32g2-usdhc";
> 				     ...
> }
> 
> The schema update should contain just this part?
> 
> i@@ -90,6 +90,9 @@ properties:
>            - enum:
>                - fsl,imxrt1170-usdhc
>            - const: fsl,imxrt1050-usdhc
> +      - items:
> +          - const: nxp,s32g3-usdhc
> +          - const: nxp,s32g2-usdhc
>  
>    reg:
>      maxItems: 1
> 
> 
> Is this correct?

Yes.



Best regards,
Krzysztof