[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20201216173841.GC651087@robh.at.kernel.org>
Date: Wed, 16 Dec 2020 11:38:41 -0600
From: Rob Herring <robh@...nel.org>
To: Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc: Guennadi Liakhovetski <g.liakhovetski@....de>,
Sakari Ailus <sakari.ailus@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Jacopo Mondi <jacopo@...ndi.org>,
Laurent Pinchart <laurent.pinchart+renesas@...asonboard.com>,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-media@...r.kernel.org
Subject: Re: [PATCH v3 2/2] dt-bindings: media: Use graph and
video-interfaces schemas
On Wed, Dec 16, 2020 at 05:19:55PM +0200, Laurent Pinchart wrote:
> Hi Rob,
>
> Thank you for the patch.
>
> On Thu, Dec 10, 2020 at 03:16:25PM -0600, Rob Herring wrote:
> > Now that we have graph and video-interfaces schemas, rework the media
> > related schemas to use them.
> >
> > Cc: Maxime Ripard <mripard@...nel.org>
> > Cc: Mauro Carvalho Chehab <mchehab@...nel.org>
> > Cc: Jacopo Mondi <jacopo@...ndi.org>
> > Cc: Laurent Pinchart <laurent.pinchart+renesas@...asonboard.com>
> > Cc: linux-media@...r.kernel.org
> > Signed-off-by: Rob Herring <robh@...nel.org>
> > ---
> > diff --git a/Documentation/devicetree/bindings/media/i2c/mipi-ccs.yaml b/Documentation/devicetree/bindings/media/i2c/mipi-ccs.yaml
> > index d94bd67ccea1..3657f2f41098 100644
> > --- a/Documentation/devicetree/bindings/media/i2c/mipi-ccs.yaml
> > +++ b/Documentation/devicetree/bindings/media/i2c/mipi-ccs.yaml
> > @@ -73,19 +73,16 @@ properties:
> > enum: [ 0, 180 ]
> >
> > port:
> > - type: object
> > + $ref: /schemas/graph.yaml#/$defs/port-base
> > + additionalProperties: false
> > +
> > properties:
> > endpoint:
> > - type: object
> > + $ref: /schemas/media/video-interfaces.yaml#
> > + unevaluatedProperties: false
> > +
> > properties:
> > - link-frequencies:
> > - $ref: /schemas/types.yaml#/definitions/uint64-array
> > - description: List of allowed data link frequencies.
> > - data-lanes:
> > - minItems: 1
> > - maxItems: 8
>
> Don't we need
>
> link-frequencies: true
> data-lanes: true
>
> to convey the fact that those properties are applicable for this device
> ? This applies to a few locations below too.
Adding them would convey that to the reader, but wouldn't change the
schema validation. We'd have to use 'additionalProperties' instead and
also add 'remote-endpoint' everywhere (and 'reg' sometimes). I prefer
not doing all that, but if we want it just for purposes of documenting
the usage, that's fine.
> > bus-type:
> > - description: The type of the data bus.
> > oneOf:
> > - const: 1 # CSI-2 C-PHY
> > - const: 3 # CCP2
> > diff --git a/Documentation/devicetree/bindings/media/i2c/ov5647.yaml b/Documentation/devicetree/bindings/media/i2c/ov5647.yaml
> > index 280c62afae13..3b1ea9da437a 100644
> > --- a/Documentation/devicetree/bindings/media/i2c/ov5647.yaml
> > +++ b/Documentation/devicetree/bindings/media/i2c/ov5647.yaml
> > @@ -31,27 +31,15 @@ properties:
> > maxItems: 1
> >
> > port:
> > - type: object
> > - description: |-
> > - Should contain one endpoint sub-node used to model connection to the
> > - video receiver according to the specification defined in
> > - Documentation/devicetree/bindings/media/video-interfaces.txt.
> > + $ref: /schemas/graph.yaml#/$defs/port-base
> >
> > properties:
> > endpoint:
> > - type: object
> > + $ref: /schemas/media/video-interfaces.yaml#
> > + unevaluatedProperties: false
> >
> > properties:
> > - remote-endpoint:
> > - description: |-
> > - phandle to the video receiver input port.
> > -
> > - clock-noncontinuous:
> > - type: boolean
> > - description: |-
> > - Set to true to allow MIPI CSI-2 non-continuous clock operations.
> > -
> > - additionalProperties: false
> > + clock-noncontinuous: true
> >
> > additionalProperties: false
> >
> > diff --git a/Documentation/devicetree/bindings/media/i2c/ov8856.yaml b/Documentation/devicetree/bindings/media/i2c/ov8856.yaml
> > index cde85553fd01..c29b057ae922 100644
> > --- a/Documentation/devicetree/bindings/media/i2c/ov8856.yaml
> > +++ b/Documentation/devicetree/bindings/media/i2c/ov8856.yaml
> > @@ -57,16 +57,13 @@ properties:
> > active low.
> >
> > port:
> > - type: object
> > + $ref: /schemas/graph.yaml#/$defs/port-base
> > additionalProperties: false
> > - description:
> > - A node containing an output port node with an endpoint definition
> > - as documented in
> > - Documentation/devicetree/bindings/media/video-interfaces.txt
> >
> > properties:
> > endpoint:
> > - type: object
> > + $ref: /schemas/media/video-interfaces.yaml#
> > + unevaluatedProperties: false
> >
> > properties:
> > data-lanes:
> > @@ -79,18 +76,13 @@ properties:
> > - const: 4
> >
> > link-frequencies:
> > - $ref: /schemas/types.yaml#/definitions/uint64-array
> > - description:
> > - Allowed data bus frequencies. 360000000, 180000000 Hz or both
> > - are supported by the driver.
> > -
> > + maxItems: 2
> > + items:
> > + enum: [ 360000000, 180000000 ]
>
> This is a limitation of the driver, not the device. Should we keep this
> information in a comment, to eventually get it fixed and drop the
> limitation from the bindings ?
If your dts has anything else, then it won't work. Warning on that seems
valuable to me, so I think we should keep it. If someone with a better
driver complains, we can drop it.
I can keep the description with 'Frequencies listed are driver, not h/w
limitations'.
Rob
Powered by blists - more mailing lists