Heippa, On Wed, Aug 28, 2024 at 03:14:47PM +0300, Tomi Valkeinen wrote: > Hi, > > On 28/08/2024 14:12, Laurent Pinchart wrote: > > On Wed, Aug 28, 2024 at 11:06:23AM +0000, Sakari Ailus wrote: > > > On Thu, Jun 20, 2024 at 02:07:51PM +0300, Tomi Valkeinen wrote: > > > > Add DT bindings for raspberrypi,rp1-cfe. > > > > > > > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxxxxxxxxxxxx> > > > > --- > > > > .../bindings/media/raspberrypi,rp1-cfe.yaml | 98 ++++++++++++++++++++++ > > > > 1 file changed, 98 insertions(+) > > > > > > > > diff --git a/Documentation/devicetree/bindings/media/raspberrypi,rp1-cfe.yaml b/Documentation/devicetree/bindings/media/raspberrypi,rp1-cfe.yaml > > > > new file mode 100644 > > > > index 000000000000..851533de2305 > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/media/raspberrypi,rp1-cfe.yaml > > > > @@ -0,0 +1,98 @@ > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > > +%YAML 1.2 > > > > +--- > > > > +$id: http://devicetree.org/schemas/media/raspberrypi,rp1-cfe.yaml# > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > + > > > > +title: Raspberry Pi PiSP Camera Front End > > > > + > > > > +maintainers: > > > > + - Tomi Valkeinen <tomi.valkeinen@xxxxxxxxxxxxxxxx> > > > > + - Raspberry Pi Kernel Maintenance <kernel-list@xxxxxxxxxxxxxxx> > > > > + > > > > +description: | > > > > + The Raspberry Pi PiSP Camera Front End is a module in Raspberrypi 5's RP1 I/O > > > > + controller, that contains: > > > > + - MIPI D-PHY > > > > + - MIPI CSI-2 receiver > > > > + - Simple image processor (called PiSP Front End, or FE) > > > > + > > > > + The FE documentation is available at: > > > > + https://datasheets.raspberrypi.com/camera/raspberry-pi-image-signal-processor-specification.pdf > > > > + > > > > + The PHY and CSI-2 receiver part have no public documentation. > > > > + > > > > +properties: > > > > + compatible: > > > > + items: > > > > + - const: raspberrypi,rp1-cfe > > > > + > > > > + reg: > > > > + items: > > > > + - description: CSI-2 registers > > > > + - description: D-PHY registers > > > > + - description: MIPI CFG (a simple top-level mux) registers > > > > + - description: FE registers > > > > + > > > > + interrupts: > > > > + maxItems: 1 > > > > + > > > > + clocks: > > > > + maxItems: 1 > > > > + > > > > + port: > > > > + $ref: /schemas/graph.yaml#/$defs/port-base > > > > + additionalProperties: false > > > > + description: CSI-2 RX Port > > > > + > > > > + properties: > > > > + endpoint: > > > > + $ref: video-interfaces.yaml# > > > > + unevaluatedProperties: false > > > > + > > > > + properties: > > > > + data-lanes: > > > > + minItems: 1 > > > > + maxItems: 4 > > > > + > > > > + clock-lanes: > > > > + maxItems: 1 > > > > > > minItems needs to be 1 as well. > > Hmm, I see a lot of > > clock-lanes: > maxItems: 1 > > in the device tree bindings. And > https://docs.kernel.org/devicetree/bindings/writing-schema.html says "Cases > that have only a single entry just need to express that with maxItems". Fair enough. > > > > > > > Or... is this actually configurable in hardware? > > > > Looking at the driver, lane reordering is not supported, so we could > > drop this property. If the hardware is found to support it later, it can > > easily be added back without any backward compatibility issue. > > Re-ordering is not supported. I guess clock-lanes can be dropped, although I > feel that if we have the clock lane in the hardware, and the numbering of > data-lanes must take that into account, then: > > clock-lanes = <0>; > data-lanes = <1 2>; > > looks better than: > > data-lanes = <1 2>; /* and implicit clk lane 0 */ > > But I can't think of any practical benefit it brings... The clock-lanes property is optional, it only provides any information if the clock lane is configurable. Please drop it. -- Terveisin, Sakari Ailus