lkml.org 
[lkml]   [2022]   [Jun]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v6 1/2] dt-bindings: cros-ec: Reorganize property availability
Hi,

On Tue, Jun 14, 2022 at 12:51 PM Stephen Boyd <swboyd@chromium.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@kernel.org>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: <devicetree@vger.kernel.org>
> Cc: <chrome-platform@lists.linux.dev>
> Cc: Guenter Roeck <groeck@chromium.org>
> Cc: Douglas Anderson <dianders@chromium.org>
> Cc: Craig Hesling <hesling@chromium.org>
> Cc: Tom Hughes <tomhughes@chromium.org>
> Cc: Alexandru M Stan <amstan@chromium.org>
> Cc: Tzung-Bi Shih <tzungbi@kernel.org>
> Cc: Matthias Kaehlcke <mka@chromium.org>
> Cc: Benson Leung <bleung@chromium.org>
> Cc: Lee Jones <lee.jones@linaro.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.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@chromium.org>

\
 
 \ /
  Last update: 2022-06-15 00:42    [W:0.102 / U:0.260 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site