linux-kernel - Re: [PATCH v9 02/15] dt-bindings: mmc: cdns: Add AMD Pensando Elba SoC

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 for Android: free password hash cracker in your pocket

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

Message-ID: <f85cdf27-7ea0-14a8-10b0-7a9ac137a040@linaro.org>
Date:   Thu, 19 Jan 2023 08:47:02 +0100
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Brad Larson <blarson@....com>, linux-arm-kernel@...ts.infradead.org
Cc:     linux-kernel@...r.kernel.org, linux-mmc@...r.kernel.org,
        linux-spi@...r.kernel.org, adrian.hunter@...el.com,
        alcooperx@...il.com, andy.shevchenko@...il.com, arnd@...db.de,
        brad@...sando.io, brendan.higgins@...ux.dev,
        briannorris@...omium.org, brijeshkumar.singh@....com,
        catalin.marinas@....com, davidgow@...gle.com, gsomlo@...il.com,
        gerg@...ux-m68k.org, krzk@...nel.org,
        krzysztof.kozlowski+dt@...aro.org, lee@...nel.org,
        lee.jones@...aro.org, broonie@...nel.org,
        yamada.masahiro@...ionext.com, p.zabel@...gutronix.de,
        piotrs@...ence.com, p.yadav@...com, rdunlap@...radead.org,
        robh+dt@...nel.org, samuel@...lland.org, fancer.lancer@...il.com,
        skhan@...uxfoundation.org, suravee.suthikulpanit@....com,
        thomas.lendacky@....com, tonyhuang.sunplus@...il.com,
        ulf.hansson@...aro.org, vaishnav.a@...com, will@...nel.org,
        devicetree@...r.kernel.org
Subject: Re: [PATCH v9 02/15] dt-bindings: mmc: cdns: Add AMD Pensando Elba
 SoC

On 19/01/2023 04:51, Brad Larson wrote:
> AMD Pensando Elba ARM 64-bit SoC is integrated with this IP and
> explicitly controls byte-lane enables.
> 
> Signed-off-by: Brad Larson <blarson@....com>
> 
> ---
> 
> Changes since v6:
> - Add reset-names and resets properties
> - Add if/then on property amd,pensando-elba-sd4hc to set reg property
>   values for minItems and maxItems
> 
> ---
>  .../devicetree/bindings/mmc/cdns,sdhci.yaml   | 28 ++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> index 8b1a0fdcb5e3..f7dd6f990f96 100644
> --- a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> +++ b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> @@ -16,12 +16,14 @@ properties:
>    compatible:
>      items:
>        - enum:
> +          - amd,pensando-elba-sd4hc
>            - microchip,mpfs-sd4hc
>            - socionext,uniphier-sd4hc
>        - const: cdns,sd4hc
>  
>    reg:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 2
>  
>    interrupts:
>      maxItems: 1
> @@ -111,12 +113,36 @@ properties:
>      minimum: 0
>      maximum: 0x7f
>  
> +  reset-names:
> +    items:
> +      - const: hw
> +
> +  resets:
> +    description:
> +      optional. phandle to the system reset controller with line index

Drop "optional"
Drop "phandle to the" and rephrase it to describe physical reset line.
Don't describe here DT syntax (phandle) but the hardware. What is
expected to be here?

> +      for mmc hw reset line if exists.
> +    maxItems: 1
> +
>  required:
>    - compatible
>    - reg
>    - interrupts
>    - clocks
>  
> +if:

Move the allO from the top here and put it under it. Saves indentation soon.

> +  properties:
> +    compatible:
> +      const: amd,pensando-elba-sd4hc
> +then:
> +  properties:
> +    reg:
> +      minItems: 2
> +else:
> +  properties:
> +    reg:
> +      minItems: 1
> +      maxItems: 2

No, why do you suddenly allow two items on all variants? This was not
described in your commit msg at all, so I expect here maxItems: 1.

Also, unless your reset is applicable to all variants, resets: false and
reset-names: false.

> +
>  unevaluatedProperties: false
>  
>  examples:

Best regards,
Krzysztof