2024-03-21 15:43:41

by Wadim Mueller

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

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 <[email protected]>
---
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



2024-03-21 17:53:51

by Krzysztof Kozlowski

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

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 <[email protected]>
> ---
> 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


2024-03-22 09:45:58

by Wadim Mueller

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

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 <[email protected]>
> > ---
> > 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
>

2024-03-22 15:07:37

by Krzysztof Kozlowski

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

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 <[email protected]>
>>> ---
>>> 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