lkml.org 
[lkml]   [2020]   [Dec]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3 1/2] media: dt-bindings: Convert video-interfaces.txt properties to schemas
On Wed, Dec 16, 2020 at 04:52:20PM +0200, Laurent Pinchart wrote:
> Hi Rob,
>
> Thank you for the patch.
>
> On Thu, Dec 10, 2020 at 03:16:24PM -0600, Rob Herring wrote:
> > Convert video-interfaces.txt to DT schema. As it contains a mixture of
> > device level and endpoint properties, split it up into 2 schemas.
> >
> > Binding schemas will need to reference both the graph.yaml and
> > video-interfaces.yaml schemas. The exact schema depends on how many
> > ports and endpoints for the binding. A single port with a single
> > endpoint looks similar to this:
> >
> > port:
> > $ref: /schemas/graph.yaml#/$defs/port-base
> >
> > properties:
> > endpoint:
> > $ref: video-interfaces.yaml#
> > unevaluatedProperties: false
> >
> > properties:
> > bus-width:
> > enum: [ 8, 10, 12, 16 ]
> >
> > pclk-sample: true
> > hsync-active: true
> > vsync-active: true
> >
> > required:
> > - bus-width
> >
> > additionalProperties: false
> >
> > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Acked-by: Jacopo Mondi <jacopo@jmondi.org>
> > Signed-off-by: Rob Herring <robh@kernel.org>
> > ---
> > I need acks for dual licensing from the listed maintainers.
> >
> > v3:
> > - Support up to 9 physical lanes
> > - Set lane-polarities array bounds
> > ---
> > .../media/video-interface-devices.yaml | 406 +++++++++++
> > .../bindings/media/video-interfaces.txt | 640 +-----------------
> > .../bindings/media/video-interfaces.yaml | 346 ++++++++++
> > 3 files changed, 753 insertions(+), 639 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/media/video-interface-devices.yaml
> > create mode 100644 Documentation/devicetree/bindings/media/video-interfaces.yaml


> > diff --git a/Documentation/devicetree/bindings/media/video-interfaces.yaml b/Documentation/devicetree/bindings/media/video-interfaces.yaml
> > new file mode 100644
> > index 000000000000..fefca7d98718
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/video-interfaces.yaml
> > @@ -0,0 +1,346 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/video-interfaces.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Common bindings for video receiver and transmitter interface endpoints
> > +
> > +maintainers:
> > + - Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > + - Sakari Ailus <sakari.ailus@linux.intel.com>
> > +
> > +description: |
> > + Video data pipelines usually consist of external devices, e.g. camera sensors,
> > + controlled over an I2C, SPI or UART bus, and SoC internal IP blocks, including
> > + video DMA engines and video data processors.
> > +
> > + SoC internal blocks are described by DT nodes, placed similarly to other SoC
> > + blocks. External devices are represented as child nodes of their respective
> > + bus controller nodes, e.g. I2C.
> > +
> > + Data interfaces on all video devices are described by their child 'port' nodes.
> > + Configuration of a port depends on other devices participating in the data
> > + transfer and is described by 'endpoint' subnodes.
> > +
> > + device {
> > + ...
> > + ports {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + port@0 {
> > + ...
> > + endpoint@0 { ... };
> > + endpoint@1 { ... };
> > + };
> > + port@1 { ... };
> > + };
> > + };
> > +
> > + If a port can be configured to work with more than one remote device on the same
> > + bus, an 'endpoint' child node must be provided for each of them. If more than
> > + one port is present in a device node or there is more than one endpoint at a
> > + port, or port node needs to be associated with a selected hardware interface,
> > + a common scheme using '#address-cells', '#size-cells' and 'reg' properties is
> > + used.
> > +
> > + All 'port' nodes can be grouped under optional 'ports' node, which allows to
> > + specify #address-cells, #size-cells properties independently for the 'port'
> > + and 'endpoint' nodes and any child device nodes a device might have.
> > +
> > + Two 'endpoint' nodes are linked with each other through their 'remote-endpoint'
> > + phandles. An endpoint subnode of a device contains all properties needed for
> > + configuration of this device for data exchange with other device. In most
> > + cases properties at the peer 'endpoint' nodes will be identical, however they
> > + might need to be different when there is any signal modifications on the bus
> > + between two devices, e.g. there are logic signal inverters on the lines.
> > +
> > + It is allowed for multiple endpoints at a port to be active simultaneously,
> > + where supported by a device. For example, in case where a data interface of
> > + a device is partitioned into multiple data busses, e.g. 16-bit input port
> > + divided into two separate ITU-R BT.656 8-bit busses. In such case bus-width
> > + and data-shift properties can be used to assign physical data lines to each
> > + endpoint node (logical bus).
> > +
> > + Documenting bindings for devices
> > + --------------------------------
> > +
> > + All required and optional bindings the device supports shall be explicitly
> > + documented in device DT binding documentation. This also includes port and
> > + endpoint nodes for the device, including unit-addresses and reg properties
> > + where relevant.
> > +
> > + Please also see Documentation/devicetree/bindings/graph.txt .
>
> Should this be dropped, or modified to reference the YAML schema for OF
> graph ?

Yeah. A lot of the above I feel is just duplicate of how graphs work,
but I left it as-is.


> > + clock-lanes:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + # Assume up to 9 physical lane indices
> > + maximum: 8
> > + description:
> > + Physical clock lane index. Position of an entry determines
>
> s/index/indexes/ (or indices) as there are potentially multiple entries
> (even if in practice, for all bus types we currently support, only one
> clock lane is supported) ?

The original text did say 'array', but there aren't any cases, I can't
really see why or how you'd have more than 1, and it seemed silly to
make it an array type with 'maxItems: 1'. Wouldn't a 2 clock case be
dual interface like we do for DSI?

>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks.

Rob

\
 
 \ /
  Last update: 2020-12-18 19:21    [W:0.376 / U:0.500 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site