Re: [PATCH 2/3] dt-bindings: input: convert cros-ec-keyb to json-schema

["Ricardo Cañuelo" <ricardo.canuelo@collabora.com>]

Hi Enric,

Thanks for reviewing the patch and for your suggestions. I'll prepare a
new series with the fixes.

On lun 05-10-2020 11:03:54, Enric Balletbo i Serra wrote:
> Note that there was already an attempt for this here [1]. So I think you should
> address those comments too.
> 
> cc'ing Dmitry as he is the input maintainer.
> 
> [1] https://patchwork.kernel.org/patch/11350059/

Thanks, I wasn't aware of that.

> > +unevaluatedProperties: false
> > +
> 
> Not sure about unevaluatedProperties does here, I might miss something. But,
> shouldn't you add `additionalProperties: false` instead?

The idea of using this came from
Documentation/devicetree/bindings/example-schema.yaml, when it explains
the "additionalProperties: false" line:

    This can't be used in cases where another schema is referenced
    (i.e. allOf: [{$ref: ...}]).  If and only if another schema is
    referenced and arbitrary children nodes can appear,
    "unevaluatedProperties: false" could be used.  Typical example is
    I2C controller where no name pattern matching for children can be
    added.

This binding references matrix-keymap.yaml and it may use some
properties defined in it (although they're not
subnodes). bindings/input/imx-keypad.yaml does the same.

The alternative would be to define "additionalProperties: false" and
redefine the matrix-keymap properties used in this binding too
(keypad,num-rows keypad,num-columns and linux,keymap). Or to ditch
"additionalProperties: false" altogether, but I don't think that's a
proper solution.

> > +examples:
> > +  - |
> > +    cros-ec-keyb {
> 
> The keyboard controller is always a subnode inside the cros_ec, please use a
> complete example.

Ok, I'll do that.

Cheers,
Ricardo