Re: [PATCH v2 1/2] dt-bindings: input: Add bindings for Azoteq IQS7210A/7211A/E

[Rob Herring <robh@kernel.org>]

On Sun, Jun 11, 2023 at 08:06:24PM -0500, Jeff LaBundy wrote:
> Hi Rob,
> 
> On Fri, Jun 09, 2023 at 08:25:38AM -0600, Rob Herring wrote:
> > On Mon, May 29, 2023 at 07:33:47PM -0500, Jeff LaBundy wrote:
> > > Add bindings for the Azoteq IQS7210A/7211A/E family of trackpad/
> > > touchscreen controllers.
> > > 
> > > Signed-off-by: Jeff LaBundy <jeff@labundy.com>
> > > ---
> > > Changes in v2:
> > >  - Renamed 'azoteq,default-comms' to 'azoteq,forced-comms-default' and redefined
> > >    0, 1 and 2 as unspecified, 0 and 1, respectively
> > >  - Defined ATI upon its first occurrence
> > >  - Redefined 'azoteq,gesture-angle' in units of degrees
> > >  - Declared 'azoteq,rx-enable' to depend upon 'azoteq,tx-enable' within the
> > >    'trackpad' node
> > > 
> > > Hi Rob,
> > > 
> > > I attempted to reference existing properties from a common binding [1] as per
> > > your feedback in [2], however 'make DT_CHECKER_FLAGS=-m dt_binding_check' fails
> > > with the message 'Vendor specific properties must have a type and description
> > > unless they have a defined, common suffix.'
> > 
> > Is that because you have differing constraints in each case?
> 
> In the failing example [2], I have started with a simple boolean that carries
> nothing but a type and description. From the new azoteq,common.yaml:
> 
> properties:
>   [...]
> 
>   azoteq,use-prox:
>     type: boolean
>     description: foo
> 
> And from the first consumer:
> 
> patternProperties:
>   "^hall-switch-(north|south)$":
>     type: object
>     allOf:
>       - $ref: input.yaml#
>       - $ref: azoteq,common.yaml#
>     description: bar
> 
>     properties:
>       linux,code: true
>       azoteq,use-prox: true
> 
> However, the tooling presents the following:
> 
>   CHKDT   Documentation/devicetree/bindings/processed-schema.json
> /home/jlabundy/work/linux/Documentation/devicetree/bindings/input/iqs62x-keys.yaml: patternProperties:^hall-switch-(north|south)$:properties:azoteq,use-prox: True is not of type 'object'
> 	hint: Vendor specific properties must have a type and description unless they have a defined, common suffix.
> 	from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
> 
> [...]
> 
> I am committed to addressing your feedback; to help me do so, can you help me
> identify what I've done wrong, and/or point me to an example that successfully
> passes dt_binding_check?

You're not doing anything wrong. There's 2 options here. The first is we 
could just relax the check to allow boolean schema for vendor 
properties. The main issue with that is we then have to look for that 
improperly used and it doesn't help if you do have additional 
constraints to add on top of the common definition. The former can 
mostly be addressed by checking there is a type associated with the 
property. I'm going to look into adding that.

Alternatively, you could drop listing the properties and 
use 'unevaluatedProperties'. That's not quite equal to what you have. 
Presumably, you have 'additionalProperties' in this case, so listing 
them serves the purpose of defining what subset of properties each node 
uses from the referenced schema. We frequently don't worry if there are 
common properties not used by a specific schema. This also wouldn't work 
if you have additional constraints to add.

Rob