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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1896150.tdWV9SEqCh@trenzalore>
Date: Wed, 04 Sep 2024 10:20:44 -0400
From: Detlev Casanova <detlev.casanova@...labora.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc: linux-kernel@...r.kernel.org, Ulf Hansson <ulf.hansson@...aro.org>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Jisheng Zhang <jszhang@...nel.org>,
 linux-mmc@...r.kernel.org, devicetree@...r.kernel.org, kernel@...labora.com
Subject: Re: [PATCH 1/1] dt-bindings: mmc: Add support for rk3576 eMMC

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@...labora.com>
> >>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.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@...west-1.amazonses.com/ [1]:
> > https://lore.kernel.org/all/m5ua5jnbv4u36glqt2qrps35asuqfycxedgjrfhodi5bv
> > s2r2h@...4qxt4gx74/
> 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.