linux-kernel - Re: [PATCH v9 06/15] dt-bindings: mfd: amd,pensando-elbasr: Add AMD Pensando System Resource chip

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: <a3c4feaf-c98d-5507-11f1-3dd1129f7360@linaro.org>
Date:   Thu, 19 Jan 2023 08:58:29 +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 06/15] dt-bindings: mfd: amd,pensando-elbasr: Add AMD
 Pensando System Resource chip

On 19/01/2023 04:51, Brad Larson wrote:
> Add support for the AMD Pensando SoC System Resource chip using the
> SPI interface.  The device functions are accessed using four
> chip-selects.  This device is present for all Pensando SoC designs.
> 
> Signed-off-by: Brad Larson <blarson@....com>
> ---
> 
> Changes since v6:
> - Instead of four nodes, one per chip-select, a single
>   node is used with reset-cells in the parent.
> - No MFD API is used anymore in the driver so it made
>   sense to move this to drivers/spi.
> - This driver is common for all Pensando SoC based designs
>   so changed the name to pensando-sr.c to not make it Elba
>   SoC specific.
> - Added property cs for the chip-select number which is used
>   by the driver to create /dev/pensr0.<cs> 
> 
> ---
>  .../bindings/spi/amd,pensando-sr.yaml         | 68 +++++++++++++++++++
>  1 file changed, 68 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/spi/amd,pensando-sr.yaml
> 
> diff --git a/Documentation/devicetree/bindings/spi/amd,pensando-sr.yaml b/Documentation/devicetree/bindings/spi/amd,pensando-sr.yaml
> new file mode 100644
> index 000000000000..8504652f6e19
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/amd,pensando-sr.yaml
> @@ -0,0 +1,68 @@
> +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/spi/amd,pensando-sr.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: AMD Pensando SoC Resource Controller
> +
> +description: |
> +  AMD Pensando SoC Resource Controller is a set of
> +  control/status registers accessed on four chip-selects.
> +  This device is present in all Pensando SoC based designs.
> +
> +maintainers:
> +  - Brad Larson <blarson@....com>
> +
> +properties:
> +  compatible:
> +    contains:

That's not correct syntax. Please start from existing schema or
example-schema. Drop contains.

> +      enum:
> +        - amd,pensando-sr
> +
> +  reg:
> +    minItems: 1

maxItems. Which example or existing schema pointed you to use minItems?

> +
> +  cs:
> +    minItems: 1
> +    maxItems: 4
> +    description:
> +      Device chip select

Drop entire property. Isn't reg for this on SPI bus?

> +
> +  '#reset-cells':
> +    const: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  spi-max-frequency: true

Drop. Missing reference to spi-peripheral-props.

> +
> +required:
> +  - compatible
> +  - cs
> +  - spi-max-frequency
> +  - '#reset-cells'
> +
> +unevaluatedProperties: false

This does not make sense on its own. It works with additional ref. When
you add ref to spi props, it will be fine. But without it you should use
additionalProperties: false.


Best regards,
Krzysztof