Re: [PATCH] media: dt-bindings: sony,imx290: Update usage example

[Dave Stevenson <dave.stevenson@raspberrypi.com>]

Hi Laurent & Niklas

On Wed, 30 Apr 2025 at 14:19, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> On Wed, Apr 30, 2025 at 03:03:10PM +0200, Geert Uytterhoeven wrote:
> > On Wed, 30 Apr 2025 at 14:58, Niklas Söderlund wrote:
> > > Since commit 98e0500eadb7 ("media: i2c: imx290: Add configurable link
> > > frequency and pixel rate") the driver expects two specific
> > > link-frequency settings 2-lane (445500000, 297000000) and 4-lane
> > > (222750000, 148500000) operation. The driver fails to probe without
> > > these exact settings.
> > >
> > > Update the example in the bindings to match this to make it easier for
> > > users to incorporate this sensor in their device tree descriptions
> > > without having to read the driver sources when the driver fails to
> > > probe.
> > >
> > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> >
> > Thanks for your patch!
> >
> > > --- a/Documentation/devicetree/bindings/media/i2c/sony,imx290.yaml
> > > +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx290.yaml
> > > @@ -136,7 +136,7 @@ examples:
> > >              port {
> > >                  imx290_ep: endpoint {
> > >                      data-lanes = <1 2 3 4>;
> > > -                    link-frequencies = /bits/ 64 <445500000>;
> > > +                    link-frequencies = /bits/ 64 <222750000 148500000>;
> > >                      remote-endpoint = <&csiphy0_ep>;
> > >                  };
> > >              };
> >
> > I guess the link-frequencies property should gain a rule that it
> > needs two values, too?
>
> The driver doesn't require two frequencies (unless I'm mistaken), it
> could operate with a single one (albeit not in all resolutions), so I
> don't think we should require two frequencies in the bindings.

The driver does require both due to 98e0500eadb7 ("media: i2c: imx290:
Add configurable link frequency and pixel rate") and
imx290_check_link_freqs()

However I'd agree that it'd be better to make the driver accept just
the one and make any compensations, rather than amend the binding. I'm
happy to try and find a few minutes to make a patch for that.

My experience of this family of sensors says that we should be able to
run any resolution at any link frequency, but it needs changes to
HBLANK to ensure there is sufficient time per line.
Dropping to the lower link freq for the 720p mode is only because that
is what the datasheet describes for the precanned HD720p. The window
cropping mode lists no such requirement, and yet could produce exactly
that same 720p output.

  Dave

> --
> Regards,
>
> Laurent Pinchart