Hi, On Fri 11 Feb 22, 22:52, Laurent Pinchart wrote: > Hi Rob, > > On Fri, Feb 11, 2022 at 09:13:30AM -0600, Rob Herring wrote: > > On Mon, Feb 07, 2022 at 05:51:21PM +0200, Laurent Pinchart wrote: > > > On Sat, Feb 05, 2022 at 07:54:24PM +0100, Paul Kocialkowski wrote: > > > > This introduces YAML bindings documentation for the Allwinner A31 Image > > > > Signal Processor (ISP). > > > > > > > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@xxxxxxxxxxx> > > > > --- > > > > .../media/allwinner,sun6i-a31-isp.yaml | 117 ++++++++++++++++++ > > > > 1 file changed, 117 insertions(+) > > > > create mode 100644 Documentation/devicetree/bindings/media/allwinner,sun6i-a31-isp.yaml > > > > > > > > diff --git a/Documentation/devicetree/bindings/media/allwinner,sun6i-a31-isp.yaml b/Documentation/devicetree/bindings/media/allwinner,sun6i-a31-isp.yaml > > > > new file mode 100644 > > > > index 000000000000..2d87022c43ce > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/media/allwinner,sun6i-a31-isp.yaml > > > > @@ -0,0 +1,117 @@ > > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > > > +%YAML 1.2 > > > > +--- > > > > +$id: http://devicetree.org/schemas/media/allwinner,sun6i-a31-isp.yaml# > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > + > > > > +title: Allwinner A31 Image Signal Processor Driver (ISP) Device Tree Bindings > > > > + > > > > +maintainers: > > > > + - Paul Kocialkowski <paul.kocialkowski@xxxxxxxxxxx> > > > > + > > > > +properties: > > > > + compatible: > > > > + enum: > > > > + - allwinner,sun6i-a31-isp > > > > + - allwinner,sun8i-v3s-isp > > > > + > > > > + reg: > > > > + maxItems: 1 > > > > + > > > > + interrupts: > > > > + maxItems: 1 > > > > + > > > > + clocks: > > > > + items: > > > > + - description: Bus Clock > > > > + - description: Module Clock > > > > + - description: DRAM Clock > > > > > > That's interesting, does the ISP have a dedicated DRAM ? > > > > > > > + > > > > + clock-names: > > > > + items: > > > > + - const: bus > > > > + - const: mod > > > > + - const: ram > > > > + > > > > + resets: > > > > + maxItems: 1 > > > > + > > > > + ports: > > > > + $ref: /schemas/graph.yaml#/properties/ports > > > > + > > > > + properties: > > > > + port@0: > > > > + $ref: /schemas/graph.yaml#/$defs/port-base > > > > + description: CSI0 input port > > > > + > > > > + properties: > > > > + reg: > > > > + const: 0 > > > > + > > > > + endpoint: > > > > + $ref: video-interfaces.yaml# > > > > + unevaluatedProperties: false > > > > > > If no other property than remote-endpoint are allowed, I'd write > > > > > > endpoint: > > > $ref: video-interfaces.yaml# > > > remote-endpoint: true > > > > You just mixed a node and a property... > > Yes, I meant > > endpoint: > $ref: video-interfaces.yaml# > properties: > remote-endpoint: true > > and actually add > > additionalProperties: false > > > 'remote-endpoint' is always allowed, so need to put it here and every > > other user. So 'unevaluatedProperties' is correct. But it would be good > > to define what properties from video-interfaces.yaml are used here. > > I've been looking at this recently. The usual pattern is to write > > endpoint: > $ref: video-interfaces.yaml# > unevaluatedProperties: false > properties: > hsync-polarity: true > vsync-polarity: true > > to express that the hsync-polarity and vsync-polarity properties are > used. However, this will still validate fine if, for instance, > data-lanes was specified in the device tree. Shouldn't we use > additionalProperties instead of unevaluatedProperties here ? If so, > specifying remote-endpoint: true seems needed. My understanding is that unevaluatedProperties well allow all properties defined in the included video-interfaces.yaml ref but reject others while additionalProperties will reject any unspecified local property, even if it is declared in the ref. In any case with the ISP maybe we don't even want to take the ref from video-interfaces.yaml since we are dealing with an internal fifo between two devices. Maybe it would be more appropriate to ref /schemas/graph.yaml#/$defs/endpoint-base, which already defines remote-endpoint too. What do you think? Paul -- Paul Kocialkowski, Bootlin Embedded Linux and kernel engineering https://bootlin.com
Attachment:
signature.asc
Description: PGP signature
