On Thu, Nov 12, 2020 at 01:19:34PM +0100, Linus Walleij wrote: > 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: Please include this info in the changelog to avoid confusing others. > > + 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. OK, thanks for the explanation. > > > > +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. Add reset now as an optional property and you are covered. The HW does not change, and this describes the HW and not the drivers. Sam
