On Mon, Feb 7, 2022 at 2:44 PM Sven Schwermer <sven@xxxxxxxxxxxxxxxx> wrote:
>
> Hi Rob,
>
> Thanks for your comments.
>
> On 2/7/22 21:21, Rob Herring wrote:
> >> +properties:
> >> + compatible:
> >> + const: pwm-leds-multicolor
> >> +
> >> + multi-led:
> >> + type: object
> >> + allOf:
> >> + - $ref: leds-class-multicolor.yaml#
> >
> > This schema says 'multi-led' here should have a child called
> > "^multi-led@([0-9a-f])$". You are off a level.
>
> So it should have been?
>
> properties:
> compatible:
> const: pwm-leds-multicolor
> allOf:
> - $ref: leds-class-multicolor.yaml#
Not quite. DT property names and json-schema vocab names should never
be at the same level. So allOf should be at the root level.
> This would imply that the multi-led node requires a unit address (reg
> property). That doesn't make sense in this case. How should we resolve this?
I meant to mention that. Update the regex pattern to allow just
'multi-led': "^multi-led(@[0-9a-f])?$"
> >> + patternProperties:
> >> + "^led-[0-9a-z]+$":
> >> + type: object
> >
> > $ref: common.yaml#
> > additionalProperties: false
>
> Sounds good.
>
> >> + properties:
> >> + pwms:
> >> + maxItems: 1
> >> +
> >> + pwm-names: true
> >> +
> >> + color:
> >> + $ref: common.yaml#/properties/color
> >
> > And then drop this ref.
>
> Curiosity question: why? Should I refer to an unsigned integer type instead?
Generally we want schemas to apply to nodes rather than individual
properties. Think of each node as a class and nodes can be 1 or more
subclasses. It's more important when using 'unevaluatedProperties' in
combination with refs.
'color: true' is all you need here. So it's less duplication. Not so
much here since it is just 1 property, but in general.
>
> >> + rgb-led {
> >> + compatible = "pwm-leds-multicolor";
> >> +
> >> + multi-led {
> >
> > Can't this be collapsed into 1 level? I don't see "pwm-leds-multicolor"
> > having other child nodes.
>
> It could. The reason I added the multi-led level is because the
> leds-class-multicolor.yaml schema calls for it. Perhaps I missed the
> intention of that schema but isn't it there to create a uniform binding
> schema structure across drivers?
Yeah, I guess that's a good enough reason.
Rob
