Re: [PATCH v5] dt-bindings: mfd: Convert ChromeOS EC bindings to json-schema

[Rob Herring <robh@kernel.org>]

On Wed, Feb 26, 2020 at 11:10:09AM +0800, Ikjoon Jang wrote:
> Convert the ChromeOS EC bindings to json-schema.
> 
> Signed-off-by: Ikjoon Jang <ikjn@chromium.org>
> ---
> v5: unset additionalProperties
> v4: text reflows, add type references, and fix examples
> v3: node name changed in rpmsg example
> v2: cleanup description, fix typos, remove LPC, and add RPMSG example
> 
> some properties defined under allOf/if/compatible/google,cros-ec-spi
> are recongnized as additional properties by dt-validate.
> 
> Is there more cleaner ways to handle this? (property depending on
> another one)

See below.

> ---
>  .../devicetree/bindings/mfd/cros-ec.txt       |  76 ----------
>  .../devicetree/bindings/mfd/cros-ec.yaml      | 140 ++++++++++++++++++
>  2 files changed, 140 insertions(+), 76 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/mfd/cros-ec.txt
>  create mode 100644 Documentation/devicetree/bindings/mfd/cros-ec.yaml


> diff --git a/Documentation/devicetree/bindings/mfd/cros-ec.yaml b/Documentation/devicetree/bindings/mfd/cros-ec.yaml
> new file mode 100644
> index 000000000000..8416a1299321
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/cros-ec.yaml
> @@ -0,0 +1,140 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/cros-ec.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ChromeOS Embedded Controller
> +
> +maintainers:
> +  - Benson Leung <bleung@chromium.org>
> +  - Enric Balletbo i Serra <enric.balletbo@collabora.com>
> +  - Guenter Roeck <groeck@chromium.org>
> +
> +description:
> +  Google's ChromeOS EC is a microcontroller which talks to the AP and
> +  implements various functions such as keyboard and battery charging.
> +  The EC can be connected through various interfaces (I2C, SPI, and others)
> +  and the compatible string specifies which interface is being used.
> +
> +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
> +
> +  google,has-vbc-nvram:
> +    description:
> +      Some implementations of the EC include a small nvram space used to
> +      store verified boot context data. This boolean flag is used to specify
> +      whether this nvram is present or not.
> +    type: boolean
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          const: google,cros-ec-i2c
> +    then:
> +      properties:
> +        reg:
> +          description: I2C slave address
> +          maxItems: 1

No need for defining 'reg' here.

> +
> +      required:
> +        - reg

The SPI and I2C bus schemas make 'reg' required, so you don't really 
need this part either.

> +  - if:
> +      properties:
> +        compatible:
> +          const: google,cros-ec-spi
> +    then:
> +      properties:
> +        reg:
> +          description: SPI chip select
> +          maxItems: 1
> +
> +        google,cros-ec-spi-pre-delay:
> +          description:
> +            This property specifies the delay in usecs between the
> +            assertion of the CS and the first clock pulse.
> +          allOf:
> +            - $ref: /schemas/types.yaml#/definitions/uint32
> +            - default: 0
> +            - minimum: 0
> +
> +        google,cros-ec-spi-msg-delay:
> +          description:
> +            This property specifies the delay in usecs between messages.
> +          allOf:
> +            - $ref: /schemas/types.yaml#/definitions/uint32
> +            - default: 0
> +            - minimum: 0

I'd just move these 2 to the main section and be done with it. It 
doesn't really hurt to have these in the I2C or RPMSG case as they are 
just ignored. The way you would express it is:

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

Rob