linux-kernel - Re: [PATCH v6 1/2] dt-bindings: cros-ec: Reorganize property availability

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]

Date:   Tue, 14 Jun 2022 15:41:38 -0700
From:   Doug Anderson <dianders@...omium.org>
To:     Stephen Boyd <swboyd@...omium.org>
Cc:     Benson Leung <bleung@...omium.org>,
        LKML <linux-kernel@...r.kernel.org>, patches@...ts.linux.dev,
        Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>, chrome-platform@...ts.linux.dev,
        Guenter Roeck <groeck@...omium.org>,
        Craig Hesling <hesling@...omium.org>,
        Tom Hughes <tomhughes@...omium.org>,
        Alexandru M Stan <amstan@...omium.org>,
        Tzung-Bi Shih <tzungbi@...nel.org>,
        Matthias Kaehlcke <mka@...omium.org>,
        Lee Jones <lee.jones@...aro.org>
Subject: Re: [PATCH v6 1/2] dt-bindings: cros-ec: Reorganize property availability

Hi,

On Tue, Jun 14, 2022 at 12:51 PM Stephen Boyd <swboyd@...omium.org> wrote:
>
> Various properties in the cros-ec binding only apply to different
> compatible strings. For example, the interrupts and reg property are
> required for all cros-ec devices except for the rpmsg version. Add some
> conditions to update the availability of properties so that they can't
> be used with compatibles that don't support them.
>
> Cc: Rob Herring <robh@...nel.org>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>
> Cc: <devicetree@...r.kernel.org>
> Cc: <chrome-platform@...ts.linux.dev>
> Cc: Guenter Roeck <groeck@...omium.org>
> Cc: Douglas Anderson <dianders@...omium.org>
> Cc: Craig Hesling <hesling@...omium.org>
> Cc: Tom Hughes <tomhughes@...omium.org>
> Cc: Alexandru M Stan <amstan@...omium.org>
> Cc: Tzung-Bi Shih <tzungbi@...nel.org>
> Cc: Matthias Kaehlcke <mka@...omium.org>
> Cc: Benson Leung <bleung@...omium.org>
> Cc: Lee Jones <lee.jones@...aro.org>
> Signed-off-by: Stephen Boyd <swboyd@...omium.org>
> ---
>  .../bindings/chrome/google,cros-ec-typec.yaml |  1 +
>  .../bindings/extcon/extcon-usbc-cros-ec.yaml  |  1 +
>  .../i2c/google,cros-ec-i2c-tunnel.yaml        |  1 +
>  .../bindings/mfd/google,cros-ec.yaml          | 29 +++++++++++++------
>  .../bindings/pwm/google,cros-ec-pwm.yaml      |  1 +
>  .../regulator/google,cros-ec-regulator.yaml   |  1 +
>  .../bindings/sound/google,cros-ec-codec.yaml  |  1 +
>  7 files changed, 26 insertions(+), 9 deletions(-)

slight nit that from reading the subject of this patch I'd expect that
it was a no-op. Just a reorganization / cleanup. In fact, it actually
is more than a no-op. It enforces restrictions that should probably
have always been enforced. I think it'd be better if the subject was
something like "tighten property requirements" or something like that.


> @@ -158,12 +154,27 @@ allOf:
>                - google,cros-ec-rpmsg
>      then:
>        properties:
> +        controller-data: false
>          google,cros-ec-spi-pre-delay: false
>          google,cros-ec-spi-msg-delay: false
>          spi-max-frequency: false
>      else:
>        $ref: /schemas/spi/spi-peripheral-props.yaml
>
> +  - if:
> +      properties:
> +        compatible:
> +          not:
> +            contains:
> +              const: google,cros-ec-rpmsg
> +    then:
> +      properties:
> +        mediatek,rpmsg-name: false
> +
> +      required:
> +        - reg
> +        - interrupts

slight nit that think it would be easier to understand this bottom
section if you made the "SPI" and "RPMSG" sections more symmetric to
each other. I think it would be easy to just change the SPI one to say
"not SPI" instead of explicitly listing "i2c" and "rpmsg".

In any case, this overall looks pretty nice to me. My two requests are
both pretty small nits, so either with or without fixing them:

Reviewed-by: Douglas Anderson <dianders@...omium.org>