[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Zs8YXVQ4armJJN4X@kekkonen.localdomain>
Date: Wed, 28 Aug 2024 12:30:21 +0000
From: Sakari Ailus <sakari.ailus@...ux.intel.com>
To: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
Cc: Laurent Pinchart <laurent.pinchart@...asonboard.com>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Raspberry Pi Kernel Maintenance <kernel-list@...pberrypi.com>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>,
Florian Fainelli <florian.fainelli@...adcom.com>,
Broadcom internal kernel review list <bcm-kernel-feedback-list@...adcom.com>,
Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org, linux-rpi-kernel@...ts.infradead.org,
linux-arm-kernel@...ts.infradead.org,
Naushir Patuck <naush@...pberrypi.com>,
Jacopo Mondi <jacopo.mondi@...asonboard.com>,
Kieran Bingham <kieran.bingham@...asonboard.com>
Subject: Re: [PATCH v2 2/4] dt-bindings: media: Add bindings for
raspberrypi,rp1-cfe
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@...asonboard.com>
> > > > ---
> > > > .../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@...asonboard.com>
> > > > + - Raspberry Pi Kernel Maintenance <kernel-list@...pberrypi.com>
> > > > +
> > > > +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
Powered by blists - more mailing lists