Re: [PATCH v5 3/3] dt-bindings: cros-ec: Add ChromeOS fingerprint binding

Rob Herring
2022-05-16T15:20:11Z

On Wed, May 11, 2022 at 06:39:21PM -0700, Stephen Boyd wrote:
> Add a binding to describe the fingerprint processor found on Chromebooks
> with a fingerprint sensor. Previously we've been describing this with
> the google,cros-ec-spi binding but it lacks gpio and regulator control
> used during firmware flashing.
>
> 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 | 80 ++++++++++++++++---
> 1 file changed, 71 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> index 409ecef967ce..e5fe60beb9fe 100644
> --- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> +++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> @@ -19,15 +19,19 @@ description:
> properties:
> compatible:
> oneOf:
> - - description:
> - For implementations of the EC is connected through I2C.
> - const: google,cros-ec-i2c
> - - description:
> - For implementations of the EC is connected through SPI.
> - const: google,cros-ec-spi
> - - description:
> - For implementations of the EC is connected through RPMSG.
> - const: google,cros-ec-rpmsg
> + # For implementations of the EC is connected through I2C.

Using 'description' was preferred.

> + - items:
> + - const: google,cros-ec-i2c
> + # For implementations of the FPMCU connected through SPI.
> + - items:
> + - const: google,cros-ec-fp
> + - const: google,cros-ec-spi
> + # For implementations of the EC is connected through SPI.
> + - items:
> + - const: google,cros-ec-spi
> + # For implementations of the EC is connected through RPMSG.
> + - items:
> + - const: google,cros-ec-rpmsg
>
> controller-data:
> description:
> @@ -132,6 +136,15 @@ properties:
>
> additionalProperties: false
>
> + reset-gpios:
> + maxItems: 1
> +
> + boot0-gpios:
> + maxItems: 1
> + description: Assert for bootloader mode.
> +
> + vdd-supply: true
> +
> patternProperties:
> "^i2c-tunnel[0-9]*$":
> type: object
> @@ -176,6 +189,37 @@ allOf:
> - reg
> - interrupts
>
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: google,cros-ec-fp
> + then:
> + properties:
> + '#address-cells': false
> + '#size-cells': false
> + typec: false
> + ec-pwm: false
> + keyboard-controller: false
> + proximity: false
> + codecs: false
> + cbas: false
> +
> + patternProperties:
> + "^i2c-tunnel[0-9]*$": false
> + "^regulator@[0-9]+$": false
> + "^extcon[0-9]*$": false

Is the list of what's allowed shorter? If so, you could list those
properties and use 'additionalProperties: false'.

Or maybe this is a sign that this should be a separate schema document.

> +
> + required:
> + - reset-gpios
> + - boot0-gpios
> + - vdd-supply
> + else:
> + properties:
> + reset-gpios: false
> + boot0-gpios: false
> + vdd-supply: false