Subject: Re: [PATCH] dt-bindings: Another round of adding missing 'additionalProperties'

From: Jonathan Cameron <[email-masked]>
Date: Mon, 5 Oct 2020 10:34:36 +0100

On Fri, 2 Oct 2020 18:41:43 -0500
Rob Herring <[email-masked]> wrote:

> Another round of wack-a-mole. The json-schema default is additional
> unknown properties are allowed, but for DT all properties should be
> defined.
> 
> Cc: Thierry Reding <[email-masked]>
> Cc: Linus Walleij <[email-masked]>
> Cc: Stephen Boyd <[email-masked]>
> Cc: Shawn Guo <[email-masked]>
> Cc: Bjorn Andersson <[email-masked]>
> Cc: Baolin Wang <[email-masked]>
> Cc: Guenter Roeck <[email-masked]>
> Cc: Jonathan Cameron <[email-masked]>
> Cc: Mauro Carvalho Chehab <[email-masked]>
> Cc: Laurent Pinchart <[email-masked]>
> Cc: Lee Jones <[email-masked]>
> Cc: Ulf Hansson <[email-masked]>
> Cc: "David S. Miller" <[email-masked]>
> Cc: Bjorn Helgaas <[email-masked]>
> Cc: Vinod Koul <[email-masked]>
> Cc: Liam Girdwood <[email-masked]>
> Cc: Mark Brown <[email-masked]>
> Cc: Greg Kroah-Hartman <[email-masked]>
> Cc: Daniel Lezcano <[email-masked]>
> Cc: [email-masked]
> Cc: [email-masked]
> Cc: [email-masked]
> Cc: [email-masked]
> Cc: [email-masked]
> Cc: [email-masked]
> Cc: [email-masked]
> Cc: [email-masked]
> Cc: [email-masked]
> Cc: [email-masked]
> Cc: [email-masked]
> Cc: [email-masked]
> Cc: [email-masked]
> Cc: [email-masked]
> Cc: [email-masked]
> Cc: [email-masked]
> Cc: [email-masked]
> Cc: [email-masked]
> Cc: [email-masked]
> Cc: [email-masked]
> Signed-off-by: Rob Herring <[email-masked]>
> ---

Hi Rob,

Acked-by: Jonathan Cameron <[email-masked]> # for iio

However, one of these made me wonder if the binding was simply wrong...
(definitely highlights why we should have additionalProperties: false
where ever possible).

...

> diff --git a/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> index abd8d25e1136..4c1c083d0e92 100644
> --- a/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> +++ b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> @@ -47,11 +47,17 @@ properties:
>    vddio-supply:
>      description: Regulator that provides power to the bus
>  
> +  spi-max-frequency: true
> +  spi-cpha: true
> +  spi-cpol: true

It isn't completely unheard of for a device to operate in multiple SPI modes, but
it does seem to be fairly unusual.  I took a look at the datasheet and at least
from the provided timing diagrams, these are both required in SPI mode.

http://invensense.tdk.com/wp-content/uploads/2020/09/DS-000292-ICM-42605-v1.5.pdf

That doesn't make the binding wrong as such, but we could be tighter in checking this!

I'll add this to my list to take a closer look at sometime soonish.

Thanks.

Jonathan

> +
>  required:
>    - compatible
>    - reg
>    - interrupts
>  
> +additionalProperties: false
> +
>  examples:
>    - |