| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <YgqDQs/GJVxZMB+C@aptenodytes>
Date: Mon, 14 Feb 2022 17:28:50 +0100
From: Paul Kocialkowski <paul.kocialkowski@...tlin.com>
To: Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc: Rob Herring <robh@...nel.org>, linux-media@...r.kernel.org,
devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-sunxi@...ts.linux.dev, linux-kernel@...r.kernel.org,
linux-phy@...ts.infradead.org, linux-clk@...r.kernel.org,
linux-staging@...ts.linux.dev, Yong Deng <yong.deng@...ewell.com>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Maxime Ripard <mripard@...nel.org>,
Sakari Ailus <sakari.ailus@...ux.intel.com>,
Hans Verkuil <hans.verkuil@...co.com>,
Chen-Yu Tsai <wens@...e.org>,
Jernej Skrabec <jernej.skrabec@...il.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Helen Koike <helen.koike@...labora.com>,
Thomas Petazzoni <thomas.petazzoni@...tlin.com>
Subject: Re: [PATCH v2 61/66] dt-bindings: media: Add Allwinner A31 ISP
bindings documentation
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@...tlin.com>
> > > > ---
> > > > .../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@...tlin.com>
> > > > +
> > > > +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
Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)
Powered by blists - more mailing lists