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

[Detlev Casanova <detlev.casanova@collabora.com>]

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.