Re: [PATCH v2 3/4] arm64: dts: renesas: white-hawk-csi-dsi: Define CSI-2 data line orders

["Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>]

Hi Geert,

Thanks for your feedback.

On 2024-12-27 14:22:31 +0100, Geert Uytterhoeven wrote:
> Hi Niklas, Sakari, Mauro,
> 
> On Thu, Nov 21, 2024 at 2:41 PM Niklas Söderlund
> <niklas.soderlund+renesas@ragnatech.se> wrote:
> > The second CSI-2 C-PHY data-lane have a different line order (BCA) then
> > the two other data-lanes (ABC) for both connected CSI-2 receivers,
> > describe this in the device tree.
> >
> > This have worked in the past as the R-Car CSI-2 driver did not have
> 
> has
> 
> > documentation for the line order configuration and a magic value was
> > written to the register for this specific setup. Now the registers
> > involved are documented and the hardware description as well as the
> > driver needs to be corrected.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> 
> Thanks for your patch!
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> > --- a/arch/arm64/boot/dts/renesas/white-hawk-csi-dsi.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/white-hawk-csi-dsi.dtsi
> > @@ -21,6 +21,9 @@ csi40_in: endpoint {
> >                                 bus-type = <MEDIA_BUS_TYPE_CSI2_CPHY>;
> >                                 clock-lanes = <0>;
> >                                 data-lanes = <1 2 3>;
> > +                               line-orders = <MEDIA_BUS_CSI2_CPHY_LINE_ORDER_ABC
> > +                                              MEDIA_BUS_CSI2_CPHY_LINE_ORDER_BCA
> > +                                              MEDIA_BUS_CSI2_CPHY_LINE_ORDER_ABC>;
> >                                 remote-endpoint = <&max96712_out0>;
> >                         };
> >                 };
> > @@ -41,6 +44,9 @@ csi41_in: endpoint {
> >                                 bus-type = <MEDIA_BUS_TYPE_CSI2_CPHY>;
> >                                 clock-lanes = <0>;
> >                                 data-lanes = <1 2 3>;
> > +                               line-orders = <MEDIA_BUS_CSI2_CPHY_LINE_ORDER_ABC
> > +                                              MEDIA_BUS_CSI2_CPHY_LINE_ORDER_BCA
> > +                                              MEDIA_BUS_CSI2_CPHY_LINE_ORDER_ABC>;
> >                                 remote-endpoint = <&max96712_out1>;
> >                         };
> >                 };
> 
> Using the MEDIA_BUS_CSI2_CPHY_LINE_ORDER_* definitions has a hard
> dependency on commit 91a7088096a49eb4 ("media: dt-bindings: Add property
> to describe CSI-2 C-PHY line orders") in media/master, hence I cannot
> take this patch in renesas-devel until that dependency is resolved.
> 
> However, according to the cover letter, commit 573b4adddbd22baf ("media:
> v4l: fwnode: Parse MiPI DisCo for C-PHY line-orders") in media/master
> causes a regression in the absence of the line-orders properties
> (which I had missed before, unfortunately).
> So I think it is best if this patch goes in through the media tree,
> which already has the prerequisites and the regression:
> Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> Alternatively, I can:
>   1. Cherry-pick commit 91a7088096a49eb4 first,
>   2. Replace the MEDIA_BUS_CSI2_CPHY_LINE_ORDER_* definitions by
>      their numerical values.
> 
> Please let me know if you prefer option 1 or 2.
> Thanks!

My preference would be for this patch to go thru the media tree with 
your tags to create the least churn, if Sakari is OK with that ofc.

If not I leave it up to Sakari which option is most preferable to him, 
I'm OK with both alternatives.

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

-- 
Kind Regards,
Niklas Söderlund