linux-kernel - Re: [PATCH v9 03/15] dt-bindings: spi: cdns: Add compatible for 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <42be3c33-1055-a63e-d0ad-673d85c27d3e@linaro.org>
Date:   Thu, 19 Jan 2023 08:53:20 +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 03/15] dt-bindings: spi: cdns: Add compatible for AMD
 Pensando Elba SoC

On 19/01/2023 04:51, Brad Larson wrote:
> Document the cadence qspi controller compatible for AMD Pensando
> Elba SoC boards.  The Elba qspi fifo size is 1024.
> 
> Signed-off-by: Brad Larson <blarson@....com>
> 
> ---
> 
> Changes since v6:
> - Add 1024 to cdns,fifo-depth property to resolve dtbs_check error
> 
> ---
>  .../devicetree/bindings/spi/cdns,qspi-nor.yaml     | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
> index 4707294d8f59..a6556854234f 100644
> --- a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
> +++ b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
> @@ -20,11 +20,23 @@ allOf:
>        required:
>          - power-domains
>  
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - amd,pensando-elba-qspi
> +    then:
> +      properties:
> +        cdns,fifo-depth:
> +          enum: [ 128, 256, 1024 ]
> +          default: 1024

This won't work either... did you test it? Is 1024 really allowed?

> +
>  properties:
>    compatible:
>      oneOf:
>        - items:
>            - enum:
> +              - amd,pensando-elba-qspi
>                - ti,k2g-qspi
>                - ti,am654-ospi
>                - intel,lgm-qspi
> @@ -48,7 +60,7 @@ properties:
>      description:
>        Size of the data FIFO in words.
>      $ref: "/schemas/types.yaml#/definitions/uint32"
> -    enum: [ 128, 256 ]
> +    enum: [ 128, 256, 1024 ]

The answer is here - your change is meaningless... I mean, really think
about it, why do you allow 1024 only for your variant and then
immediately allow for all variants? This does not make sense.

If you tested with proper patch you would see:

elba-asic.dtb: spi@...0: cdns,fifo-depth:0:0: 1024 is not one of [128, 256]


>      default: 128
>  
>    cdns,fifo-width:

Best regards,
Krzysztof