Re: [PATCH v2] dt-bindings: display: mcde: Convert to YAML schema

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Nov 11, 2020 at 9:59 PM Sam Ravnborg <sam@xxxxxxxxxxxx> wrote:
> On Wed, Nov 11, 2020 at 02:07:54PM +0100, Linus Walleij wrote:

> > -- clocks: an array of the MCDE clocks in this strict order:
> > -  MCDECLK (main MCDE clock), LCDCLK (LCD clock), PLLDSI
> > -  (HDMI clock), DSI0ESCLK (DSI0 energy save clock),
>
> > -  DSI1ESCLK (DSI1 energy save clock), DSI2ESCLK (DSI2 energy
> > -  save clock)
>
> I did not find these two clocks in the binding below.

The old bindings are wrong. These clocks belong on the DSI
host adapters, so they are in this part of the binding:

+      clocks:
+        description: phandles to the high speed and low power (energy
save) clocks
+          the high speed clock is not present on the third (dsi2) block, so it
+          should only have the "lp" clock
+        minItems: 1
+        maxItems: 2
+
+      clock-names:
+        oneOf:
+          - items:
+              - const: hs
+              - const: lp
+          - items:
+              - const: lp

All device trees have these in the right place, we just didn't notice that
the bindings were wrong exactly because we weren't using
formal YAML syntax. Now the strictness of this parser makes me
fix my bugs...

> > +  port:
> > +    type: object
> > +    description:
> > +      A DPI port node with endpoint definitions as defined in
> > +      Documentation/devicetree/bindings/media/video-interfaces.txt
> > +
> > +  "#address-cells":
> > +    const: 1
> > +
> > +  "#size-cells":
> > +    const: 1
> > +
> > +  ranges: true
>
> This is a transition from .txt to DT Schema so OK with this sub-node.
> But otherwise the dsi node should have been linked using graph nodes.
> So OK - just thinking out loud.

Actually when I introduced the MCDE DSI last year at first I used port
and graphs:
https://lore.kernel.org/dri-devel/20190207083647.20615-3-linus.walleij@xxxxxxxxxx/
Then Rob asked "why?":
https://lore.kernel.org/dri-devel/20190225223124.GA29057@bogus/
And then I removed it, as having a panel directly under a
DSI host is fine.

> > +patternProperties:
> > +  "^dsi@[0-9a-f]+$":
> > +    description: subnodes for the three DSI host adapters
> > +    type: object
> > +    allOf:
> > +      - $ref: dsi-controller.yaml#
(...)
> The dsi nodes needs the #address-cells and #size-cells - at least if a
> panel node is specified.

This is specified in the referenced schema dsi-controller.yaml.

> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - clocks
> > +  - clock-names
> > +  - epod-supply
> > +  - vana-supply
> > +
> > +additionalProperties: true
>
> Why are additional properties allowed here?

It's because the SoC peripherals have things like pin control
(currently handled by a quirk in the YAML validator I think) and
resets is something else I will likely add at some point, and then
this would result in warnings unless I lock-step changes in the
schema and DTS files.

I *can* disallow this if you insist.

Yours.
Linus Walleij
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux