| Message ID | 20241121134108.2029925-4-niklas.soderlund+renesas@ragnatech.se |
|---|---|
| State | New |
| Headers | show |
| Series | media: v4l: fwnode: Add support for CSI-2 C-PHY line orders | expand |
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! Gr{oetje,eeting}s, Geert
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
Hi Niklas, On Sat, Jan 4, 2025 at 1:17 PM Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> wrote: > On 2024-12-27 14:22:31 +0100, Geert Uytterhoeven wrote: > > 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. Note that it's getting a bit late for the alternatives, as I plan to send my PRs for soc today, or tomorrow the latest. Thanks! Gr{oetje,eeting}s, Geert
Hi Geert, On 2025-01-06 10:45:51 +0100, Geert Uytterhoeven wrote: > Hi Niklas, > > On Sat, Jan 4, 2025 at 1:17 PM Niklas Söderlund > <niklas.soderlund+renesas@ragnatech.se> wrote: > > On 2024-12-27 14:22:31 +0100, Geert Uytterhoeven wrote: > > > 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. > > Note that it's getting a bit late for the alternatives, as I plan to send > my PRs for soc today, or tomorrow the latest. Thanks for letting us know. As we all are slowly wakening from the holiday season maybe the best alternative is to go with option 2, numerical values to avoid the issue? Then in next cycle follow up with using the defines?
On 2025-01-06 11:02:36 +0100, Niklas Söderlund wrote: > > Note that it's getting a bit late for the alternatives, as I plan to > > send > > my PRs for soc today, or tomorrow the latest. > > Thanks for letting us know. As we all are slowly wakening from the > holiday season maybe the best alternative is to go with option 2, > numerical values to avoid the issue? Then in next cycle follow up with > using the defines? I have now sent a patch for option 2 and I will send a follow up patch once -rc1 is out to use the defines instead of numericals. Thanks for your help!
diff --git a/arch/arm64/boot/dts/renesas/white-hawk-csi-dsi.dtsi b/arch/arm64/boot/dts/renesas/white-hawk-csi-dsi.dtsi index 3006b0a64f41..a5d1c1008e7e 100644 --- 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>; }; };
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 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> --- arch/arm64/boot/dts/renesas/white-hawk-csi-dsi.dtsi | 6 ++++++ 1 file changed, 6 insertions(+)