[PATCH 0/1] dt-bindings: mmc: Add support for rk3576 eMMC

[PATCH 0/1] dt-bindings: mmc: Add support for rk3576 eMMC

[Detlev Casanova]

This patch has been extracted from the [0] patch set to be rebased on
the next branch of the mmc tree as requested in [1].

[0]: https://lore.kernel.org/all/010201919989e3de-60b56341-85e0-4869-89d1-362407c4f2ec-000000@eu-west-1.amazonses.com/
[1]: https://lore.kernel.org/all/CAPDyKFoJoqwNTKvpK93QtK1wA9vzVUTzCrP32s_HEZcrujN2Mg@mail.gmail.com/

Detlev.

Detlev Casanova (1):
  dt-bindings: mmc: Add support for rk3576 eMMC

 .../bindings/mmc/snps,dwcmshc-sdhci.yaml      | 34 ++++++++++++++-----
 1 file changed, 26 insertions(+), 8 deletions(-)

-- 
2.46.0

[PATCH 1/1] dt-bindings: mmc: Add support for rk3576 eMMC

[Detlev Casanova]

The device is compatible with rk3588, so add an entry for the 2
compatibles together.

The rk3576 device has a power-domain that needs to be on for the eMMC to
be used. Add it as a requirement.

Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../bindings/mmc/snps,dwcmshc-sdhci.yaml      | 34 ++++++++++++++-----
 1 file changed, 26 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
index 80d50178d2e3..84a667f0c526 100644
--- a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
+++ b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
@@ -12,14 +12,18 @@ maintainers:
 
 properties:
   compatible:
-    enum:
-      - rockchip,rk3568-dwcmshc
-      - rockchip,rk3588-dwcmshc
-      - snps,dwcmshc-sdhci
-      - sophgo,cv1800b-dwcmshc
-      - sophgo,sg2002-dwcmshc
-      - sophgo,sg2042-dwcmshc
-      - thead,th1520-dwcmshc
+    oneOf:
+      - items:
+          - const: rockchip,rk3576-dwcmshc
+          - const: rockchip,rk3588-dwcmshc
+      - enum:
+          - rockchip,rk3568-dwcmshc
+          - rockchip,rk3588-dwcmshc
+          - snps,dwcmshc-sdhci
+          - sophgo,cv1800b-dwcmshc
+          - sophgo,sg2002-dwcmshc
+          - sophgo,sg2042-dwcmshc
+          - thead,th1520-dwcmshc
 
   reg:
     maxItems: 1
@@ -35,6 +39,9 @@ properties:
     minItems: 1
     maxItems: 5
 
+  power-domains:
+    maxItems: 1
+
   resets:
     maxItems: 5
 
@@ -97,6 +104,17 @@ allOf:
             - const: block
             - const: timer
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: rockchip,rk3576-dwcmshc
+
+    then:
+      properties:
+        power-domains:
+          minItems: 1
+
 unevaluatedProperties: false
 
 examples:
-- 
2.46.0

Re: [PATCH 1/1] dt-bindings: mmc: Add support for rk3576 eMMC

[Krzysztof Kozlowski]

On Tue, Sep 03, 2024 at 10:51:36AM -0400, Detlev Casanova wrote:
> The device is compatible with rk3588, so add an entry for the 2
> compatibles together.
> 
> The rk3576 device has a power-domain that needs to be on for the eMMC to
> be used. Add it as a requirement.
> 
> Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

NAK

Drop fake tag. It is impossible to receive a review-tag from me on THE
FIRST version.  I almost never provide reviews out of mailing lists.

And since there is no changelog here and no versioning, this obviously
is not v2 or v3.

> ---
>  .../bindings/mmc/snps,dwcmshc-sdhci.yaml      | 34 ++++++++++++++-----
>  1 file changed, 26 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
> index 80d50178d2e3..84a667f0c526 100644
> --- a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
> +++ b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
> @@ -12,14 +12,18 @@ maintainers:
>  
>  properties:
>    compatible:
> -    enum:
> -      - rockchip,rk3568-dwcmshc
> -      - rockchip,rk3588-dwcmshc
> -      - snps,dwcmshc-sdhci
> -      - sophgo,cv1800b-dwcmshc
> -      - sophgo,sg2002-dwcmshc
> -      - sophgo,sg2042-dwcmshc
> -      - thead,th1520-dwcmshc
> +    oneOf:
> +      - items:
> +          - const: rockchip,rk3576-dwcmshc
> +          - const: rockchip,rk3588-dwcmshc
> +      - enum:
> +          - rockchip,rk3568-dwcmshc
> +          - rockchip,rk3588-dwcmshc
> +          - snps,dwcmshc-sdhci
> +          - sophgo,cv1800b-dwcmshc
> +          - sophgo,sg2002-dwcmshc
> +          - sophgo,sg2042-dwcmshc
> +          - thead,th1520-dwcmshc
>  
>    reg:
>      maxItems: 1
> @@ -35,6 +39,9 @@ properties:
>      minItems: 1
>      maxItems: 5
>  
> +  power-domains:
> +    maxItems: 1
> +
>    resets:
>      maxItems: 5
>  
> @@ -97,6 +104,17 @@ allOf:
>              - const: block
>              - const: timer
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: rockchip,rk3576-dwcmshc
> +
> +    then:
> +      properties:
> +        power-domains:
> +          minItems: 1

Why minItems? This does not look right. I don't get what you are trying
to say here.

Best regards,
Krzysztof

Re: [PATCH 1/1] dt-bindings: mmc: Add support for rk3576 eMMC

[Detlev Casanova]

On Wednesday, 4 September 2024 02:00:27 EDT Krzysztof Kozlowski wrote:
> On Tue, Sep 03, 2024 at 10:51:36AM -0400, Detlev Casanova wrote:
> > The device is compatible with rk3588, so add an entry for the 2
> > compatibles together.
> > 
> > The rk3576 device has a power-domain that needs to be on for the eMMC to
> > be used. Add it as a requirement.
> > 
> > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> NAK
> 
> Drop fake tag. It is impossible to receive a review-tag from me on THE
> FIRST version.  I almost never provide reviews out of mailing lists.
> 
> And since there is no changelog here and no versioning, this obviously
> is not v2 or v3.

That's because the patch was from another patchset[0]. Only this patch needed
a rebase on the mmc tree, so I sent it separately. You reviewed it here [1].

[0]: https://lore.kernel.org/all/010201919989e3de-60b56341-85e0-4869-89d1-362407c4f2ec-000000@eu-west-1.amazonses.com/
[1]: https://lore.kernel.org/all/m5ua5jnbv4u36glqt2qrps35asuqfycxedgjrfhodi5bvs2r2h@xvy4qxt4gx74/

> > ---
> > 
> >  .../bindings/mmc/snps,dwcmshc-sdhci.yaml      | 34 ++++++++++++++-----
> >  1 file changed, 26 insertions(+), 8 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
> > b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml index
> > 80d50178d2e3..84a667f0c526 100644
> > --- a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
> > +++ b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
> > 
> > @@ -12,14 +12,18 @@ maintainers:
> >  properties:
> >    compatible:
> > -    enum:
> > -      - rockchip,rk3568-dwcmshc
> > -      - rockchip,rk3588-dwcmshc
> > -      - snps,dwcmshc-sdhci
> > -      - sophgo,cv1800b-dwcmshc
> > -      - sophgo,sg2002-dwcmshc
> > -      - sophgo,sg2042-dwcmshc
> > -      - thead,th1520-dwcmshc
> > +    oneOf:
> > +      - items:
> > +          - const: rockchip,rk3576-dwcmshc
> > +          - const: rockchip,rk3588-dwcmshc
> > +      - enum:
> > +          - rockchip,rk3568-dwcmshc
> > +          - rockchip,rk3588-dwcmshc
> > +          - snps,dwcmshc-sdhci
> > +          - sophgo,cv1800b-dwcmshc
> > +          - sophgo,sg2002-dwcmshc
> > +          - sophgo,sg2042-dwcmshc
> > +          - thead,th1520-dwcmshc
> > 
> >    reg:
> >      maxItems: 1
> > 
> > @@ -35,6 +39,9 @@ properties:
> >      minItems: 1
> >      maxItems: 5
> > 
> > +  power-domains:
> > +    maxItems: 1
> > +
> > 
> >    resets:
> >      maxItems: 5
> > 
> > @@ -97,6 +104,17 @@ allOf:
> >              - const: block
> >              - const: timer
> > 
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: rockchip,rk3576-dwcmshc
> > +
> > +    then:
> > +      properties:
> > +        power-domains:
> > +          minItems: 1
> 
> Why minItems? This does not look right. I don't get what you are trying
> to say here.

I'm saying that for the rockchip,rk3576-dwcmshc compatible, 1 power-domain
node has to be set.


Regards,
Detlev.

Re: [PATCH 1/1] dt-bindings: mmc: Add support for rk3576 eMMC

[Krzysztof Kozlowski]

On 04/09/2024 14:56, Detlev Casanova wrote:
> On Wednesday, 4 September 2024 02:00:27 EDT Krzysztof Kozlowski wrote:
>> On Tue, Sep 03, 2024 at 10:51:36AM -0400, Detlev Casanova wrote:
>>> The device is compatible with rk3588, so add an entry for the 2
>>> compatibles together.
>>>
>>> The rk3576 device has a power-domain that needs to be on for the eMMC to
>>> be used. Add it as a requirement.
>>>
>>> Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>
>> NAK
>>
>> Drop fake tag. It is impossible to receive a review-tag from me on THE
>> FIRST version.  I almost never provide reviews out of mailing lists.
>>
>> And since there is no changelog here and no versioning, this obviously
>> is not v2 or v3.
> 
> That's because the patch was from another patchset[0]. Only this patch needed
> a rebase on the mmc tree, so I sent it separately. You reviewed it here [1].
> 
> [0]: https://lore.kernel.org/all/010201919989e3de-60b56341-85e0-4869-89d1-362407c4f2ec-000000@eu-west-1.amazonses.com/
> [1]: https://lore.kernel.org/all/m5ua5jnbv4u36glqt2qrps35asuqfycxedgjrfhodi5bvs2r2h@xvy4qxt4gx74/

Hm, ok, changelog should explained this and the versioning should
continue, including history of this patch.

> 
>>> ---
>>>
>>>  .../bindings/mmc/snps,dwcmshc-sdhci.yaml      | 34 ++++++++++++++-----
>>>  1 file changed, 26 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
>>> b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml index
>>> 80d50178d2e3..84a667f0c526 100644
>>> --- a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
>>> +++ b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
>>>
>>> @@ -12,14 +12,18 @@ maintainers:
>>>  properties:
>>>    compatible:
>>> -    enum:
>>> -      - rockchip,rk3568-dwcmshc
>>> -      - rockchip,rk3588-dwcmshc
>>> -      - snps,dwcmshc-sdhci
>>> -      - sophgo,cv1800b-dwcmshc
>>> -      - sophgo,sg2002-dwcmshc
>>> -      - sophgo,sg2042-dwcmshc
>>> -      - thead,th1520-dwcmshc
>>> +    oneOf:
>>> +      - items:
>>> +          - const: rockchip,rk3576-dwcmshc
>>> +          - const: rockchip,rk3588-dwcmshc
>>> +      - enum:
>>> +          - rockchip,rk3568-dwcmshc
>>> +          - rockchip,rk3588-dwcmshc
>>> +          - snps,dwcmshc-sdhci
>>> +          - sophgo,cv1800b-dwcmshc
>>> +          - sophgo,sg2002-dwcmshc
>>> +          - sophgo,sg2042-dwcmshc
>>> +          - thead,th1520-dwcmshc
>>>
>>>    reg:
>>>      maxItems: 1
>>>
>>> @@ -35,6 +39,9 @@ properties:
>>>      minItems: 1
>>>      maxItems: 5
>>>
>>> +  power-domains:
>>> +    maxItems: 1
>>> +
>>>
>>>    resets:
>>>      maxItems: 5
>>>
>>> @@ -97,6 +104,17 @@ allOf:
>>>              - const: block
>>>              - const: timer
>>>
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            const: rockchip,rk3576-dwcmshc
>>> +
>>> +    then:
>>> +      properties:
>>> +        power-domains:
>>> +          minItems: 1
>>
>> Why minItems? This does not look right. I don't get what you are trying
>> to say here.
> 
> I'm saying that for the rockchip,rk3576-dwcmshc compatible, 1 power-domain
> node has to be set.

The top-level property already says this. You need to disallow it for
other variants (:false).



Best regards,
Krzysztof

Re: [PATCH 1/1] dt-bindings: mmc: Add support for rk3576 eMMC

[Detlev Casanova]

On Wednesday, 4 September 2024 09:02:40 EDT Krzysztof Kozlowski wrote:
> On 04/09/2024 14:56, Detlev Casanova wrote:
> > On Wednesday, 4 September 2024 02:00:27 EDT Krzysztof Kozlowski wrote:
> >> On Tue, Sep 03, 2024 at 10:51:36AM -0400, Detlev Casanova wrote:
> >>> The device is compatible with rk3588, so add an entry for the 2
> >>> compatibles together.
> >>> 
> >>> The rk3576 device has a power-domain that needs to be on for the eMMC to
> >>> be used. Add it as a requirement.
> >>> 
> >>> Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> >>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >> 
> >> NAK
> >> 
> >> Drop fake tag. It is impossible to receive a review-tag from me on THE
> >> FIRST version.  I almost never provide reviews out of mailing lists.
> >> 
> >> And since there is no changelog here and no versioning, this obviously
> >> is not v2 or v3.
> > 
> > That's because the patch was from another patchset[0]. Only this patch
> > needed a rebase on the mmc tree, so I sent it separately. You reviewed it
> > here [1].
> > 
> > [0]:
> > https://lore.kernel.org/all/010201919989e3de-60b56341-85e0-4869-89d1-3624
> > 07c4f2ec-000000@eu-west-1.amazonses.com/ [1]:
> > https://lore.kernel.org/all/m5ua5jnbv4u36glqt2qrps35asuqfycxedgjrfhodi5bv
> > s2r2h@xvy4qxt4gx74/
> Hm, ok, changelog should explained this and the versioning should
> continue, including history of this patch.

Sorry about that, I'll send a v4 after this one with the corresponding 
changelog then.

> >>> ---
> >>> 
> >>>  .../bindings/mmc/snps,dwcmshc-sdhci.yaml      | 34 ++++++++++++++-----
> >>>  1 file changed, 26 insertions(+), 8 deletions(-)
> >>> 
> >>> diff --git
> >>> a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
> >>> b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml index
> >>> 80d50178d2e3..84a667f0c526 100644
> >>> --- a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
> >>> +++ b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
> >>> 
> >>> @@ -12,14 +12,18 @@ maintainers:
> >>>  properties:
> >>>    compatible:
> >>> -    enum:
> >>> -      - rockchip,rk3568-dwcmshc
> >>> -      - rockchip,rk3588-dwcmshc
> >>> -      - snps,dwcmshc-sdhci
> >>> -      - sophgo,cv1800b-dwcmshc
> >>> -      - sophgo,sg2002-dwcmshc
> >>> -      - sophgo,sg2042-dwcmshc
> >>> -      - thead,th1520-dwcmshc
> >>> +    oneOf:
> >>> +      - items:
> >>> +          - const: rockchip,rk3576-dwcmshc
> >>> +          - const: rockchip,rk3588-dwcmshc
> >>> +      - enum:
> >>> +          - rockchip,rk3568-dwcmshc
> >>> +          - rockchip,rk3588-dwcmshc
> >>> +          - snps,dwcmshc-sdhci
> >>> +          - sophgo,cv1800b-dwcmshc
> >>> +          - sophgo,sg2002-dwcmshc
> >>> +          - sophgo,sg2042-dwcmshc
> >>> +          - thead,th1520-dwcmshc
> >>> 
> >>>    reg:
> >>>      maxItems: 1
> >>> 
> >>> @@ -35,6 +39,9 @@ properties:
> >>>      minItems: 1
> >>>      maxItems: 5
> >>> 
> >>> +  power-domains:
> >>> +    maxItems: 1
> >>> +
> >>> 
> >>>    resets:
> >>>      maxItems: 5
> >>> 
> >>> @@ -97,6 +104,17 @@ allOf:
> >>>              - const: block
> >>>              - const: timer
> >>> 
> >>> +  - if:
> >>> +      properties:
> >>> +        compatible:
> >>> +          contains:
> >>> +            const: rockchip,rk3576-dwcmshc
> >>> +
> >>> +    then:
> >>> +      properties:
> >>> +        power-domains:
> >>> +          minItems: 1
> >> 
> >> Why minItems? This does not look right. I don't get what you are trying
> >> to say here.
> > 
> > I'm saying that for the rockchip,rk3576-dwcmshc compatible, 1 power-domain
> > node has to be set.
> 
> The top-level property already says this. You need to disallow it for
> other variants (:false).

Ok, something like this:

allOf:
  - if:
      properties:
        compatible:
          contains:
            const: rockchip,rk3576-dwcmshc

    then:
      required:
        - power-domains

    else:
      properties:
        power-domains: false

Detlev.

Re: [PATCH 1/1] dt-bindings: mmc: Add support for rk3576 eMMC

[Krzysztof Kozlowski]

On 04/09/2024 16:20, Detlev Casanova wrote:
>>>>> +      properties:
>>>>> +        power-domains:
>>>>> +          minItems: 1
>>>>
>>>> Why minItems? This does not look right. I don't get what you are trying
>>>> to say here.
>>>
>>> I'm saying that for the rockchip,rk3576-dwcmshc compatible, 1 power-domain
>>> node has to be set.
>>
>> The top-level property already says this. You need to disallow it for
>> other variants (:false).
> 
> Ok, something like this:
> 
> allOf:
>   - if:
>       properties:
>         compatible:
>           contains:
>             const: rockchip,rk3576-dwcmshc
> 
>     then:
>       required:
>         - power-domains
> 
>     else:
>       properties:
>         power-domains: false

Yes, if they are required. Otherwise use "if: not:" and just disallow them.

Best regards,
Krzysztof