Re: [PATCH v5 2/3] dt-bindings: cros-ec: Reorganize property availability

Doug Anderson
2022-05-12T23:45:05Z

Hi,

On Wed, May 11, 2022 at 6:39 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+dt@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/mfd/google,cros-ec.yaml | 39 +++++++++++++------
> 1 file changed, 27 insertions(+), 12 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> index 99eda9ab0651..409ecef967ce 100644
> --- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> +++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> @@ -148,18 +148,33 @@ patternProperties:
> required:
> - compatible
>
> -if:
> - properties:
> - compatible:
> - contains:
> - enum:
> - - google,cros-ec-i2c
> - - google,cros-ec-rpmsg
> -then:
> - properties:
> - google,cros-ec-spi-pre-delay: false
> - google,cros-ec-spi-msg-delay: false
> - spi-max-frequency: false
> +allOf:
> + - if:
> + properties:
> + compatible:
> + not:
> + contains:
> + const: google,cros-ec-spi
> + then:
> + properties:
> + controller-data: false
> + google,cros-ec-spi-pre-delay: false
> + google,cros-ec-spi-msg-delay: false
> + spi-max-frequency: false
> +
> + - if:
> + properties:
> + compatible:
> + not:
> + contains:
> + const: google,cros-ec-rpmsg
> + then:
> + properties:
> + mediatek,rpmsg-name: false
> +
> + required:
> + - reg
> + - interrupts

Is it worth having an "else" clause here? to explicitly make
"mediatek,rpmsg-name" required for mediatek?

In any case, this seems right from my (albeit) limited yaml knowledge
and seems like an improvement.

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