linux-kernel - Re: [PATCH v1 09/30] reset: starfive: jh7100: Move necessary properties to device tree

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: <20220930204905.GA903203-robh@kernel.org>
Date:   Fri, 30 Sep 2022 15:49:05 -0500
From:   Rob Herring <robh@...nel.org>
To:     Hal Feng <hal.feng@...ux.starfivetech.com>
Cc:     linux-riscv@...ts.infradead.org, devicetree@...r.kernel.org,
        linux-clk@...r.kernel.org, linux-gpio@...r.kernel.org,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Palmer Dabbelt <palmer@...belt.com>,
        Albert Ou <aou@...s.berkeley.edu>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Marc Zyngier <maz@...nel.org>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Stephen Boyd <sboyd@...nel.org>,
        Michael Turquette <mturquette@...libre.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        Emil Renner Berthing <kernel@...il.dk>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 09/30] reset: starfive: jh7100: Move necessary
 properties to device tree

On Thu, Sep 29, 2022 at 10:32:04PM +0800, Hal Feng wrote:
> Store the necessary properties in device tree instead of .c file,
> in order to apply this reset driver to other StarFive SoCs.
> 
> Signed-off-by: Hal Feng <hal.feng@...ux.starfivetech.com>
> ---
>  .../bindings/reset/starfive,jh7100-reset.yaml | 20 ++++++++
>  arch/riscv/boot/dts/starfive/jh7100.dtsi      |  3 ++
>  drivers/reset/reset-starfive-jh7100.c         | 50 +++++++++++++------
>  3 files changed, 57 insertions(+), 16 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/reset/starfive,jh7100-reset.yaml b/Documentation/devicetree/bindings/reset/starfive,jh7100-reset.yaml
> index 300359a5e14b..3eff3f72a1ed 100644
> --- a/Documentation/devicetree/bindings/reset/starfive,jh7100-reset.yaml
> +++ b/Documentation/devicetree/bindings/reset/starfive,jh7100-reset.yaml
> @@ -20,19 +20,39 @@ properties:
>    "#reset-cells":
>      const: 1
>  
> +  starfive,assert-offset:
> +    description: Offset of the first ASSERT register
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +  starfive,status-offset:
> +    description: Offset of the first STATUS register
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +  starfive,nr-resets:
> +    description: Number of reset signals
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
>  required:
>    - compatible
>    - reg
>    - "#reset-cells"
> +  - starfive,assert-offset
> +  - starfive,status-offset
> +  - starfive,nr-resets

Adding required properties is a red flag. You can't add required 
properties to an existing binding. That breaks the ABI unless the OS 
deals with the properties being absent. If the OS has to do that, then 
why add them in the first place? All this should be implied by the 
compatible string.

Rob