diff mbox series

[v9,5/9] ARM: dts: sun8i: v3s: Add nodes for MIPI CSI-2 support

Message ID 20260518153339.619947-6-paulk@sys-base.io (mailing list archive)
State New
Headers
Series Allwinner A31/A83T MIPI CSI-2 and A31 ISP / Platform Support |

Commit Message

Paul Kocialkowski May 18, 2026, 3:33 p.m. UTC 
From: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

MIPI CSI-2 is supported on the V3s with an A31-based MIPI CSI-2 bridge
controller. The controller uses a separate D-PHY, which is the same
that is otherwise used for MIPI DSI, but used in Rx mode.

On the V3s, the CSI0 controller is dedicated to MIPI CSI-2 as it does
not have access to any parallel interface pins.

Add all the necessary nodes (CSI0, MIPI CSI-2 bridge and D-PHY) to
support the MIPI CSI-2 interface.

Note that a fwnode graph link is created between CSI0 and MIPI CSI-2
even when no sensor is connected. This will result in a probe failure
for the controller as long as no sensor is connected but this is fine
since no other interface is available.

The interconnects property is used to inherit the proper DMA offset.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 arch/arm/boot/dts/allwinner/sun8i-v3s.dtsi | 71 ++++++++++++++++++++++
 1 file changed, 71 insertions(+)

Comments

sashiko-bot@kernel.org May 18, 2026, 4:11 p.m. UTC | #1 
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `dphy` node is missing the required `interrupts` property and a SoC-specific compatible string.
--

commit ceae1b5da24380c4a30d9a8b480a23eaee3c0814
Author: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

ARM: dts: sun8i: v3s: Add nodes for MIPI CSI-2 support

This commit adds MIPI CSI-2 support to the Allwinner V3s SoC. It introduces
the device tree nodes for the CSI0 controller, the MIPI CSI-2 bridge, and
the D-PHY block.

> diff --git a/arch/arm/boot/dts/allwinner/sun8i-v3s.dtsi b/arch/arm/boot/dts/allwinner/sun8i-v3s.dtsi
> --- a/arch/arm/boot/dts/allwinner/sun8i-v3s.dtsi
> +++ b/arch/arm/boot/dts/allwinner/sun8i-v3s.dtsi
[ ... ]
> +		dphy: d-phy@1cb2000 {
> +			compatible = "allwinner,sun6i-a31-mipi-dphy";

Should a SoC-specific compatible string like "allwinner,sun8i-v3s-mipi-dphy"
be included here alongside the generic a31 fallback?

> +			reg = <0x01cb2000 0x1000>;
> +			clocks = <&ccu CLK_BUS_CSI>,
> +				 <&ccu CLK_MIPI_CSI>;
> +			clock-names = "bus", "mod";
> +			resets = <&ccu RST_BUS_CSI>;

Will this node fail dtbs_check schema validation? The existing device tree
binding schema for "allwinner,sun6i-a31-mipi-dphy" requires an interrupts
property, which appears to be missing here.

If the V3s hardware physically lacks a D-PHY interrupt, would it be better
to add the V3s-specific compatible string and update the binding schema
to make the interrupts property conditionally optional for the V3s?

> +			allwinner,direction = "rx";
> +			status = "disabled";
> +			#phy-cells = <0>;
> +		};
> +
Chen-Yu Tsai May 24, 2026, 5:49 p.m. UTC | #2 
On Mon, May 18, 2026 at 6:12 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The `dphy` node is missing the required `interrupts` property and a SoC-specific compatible string.
> --
>
> commit ceae1b5da24380c4a30d9a8b480a23eaee3c0814
> Author: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
>
> ARM: dts: sun8i: v3s: Add nodes for MIPI CSI-2 support
>
> This commit adds MIPI CSI-2 support to the Allwinner V3s SoC. It introduces
> the device tree nodes for the CSI0 controller, the MIPI CSI-2 bridge, and
> the D-PHY block.
>
> > diff --git a/arch/arm/boot/dts/allwinner/sun8i-v3s.dtsi b/arch/arm/boot/dts/allwinner/sun8i-v3s.dtsi
> > --- a/arch/arm/boot/dts/allwinner/sun8i-v3s.dtsi
> > +++ b/arch/arm/boot/dts/allwinner/sun8i-v3s.dtsi
> [ ... ]
> > +             dphy: d-phy@1cb2000 {
> > +                     compatible = "allwinner,sun6i-a31-mipi-dphy";
>
> Should a SoC-specific compatible string like "allwinner,sun8i-v3s-mipi-dphy"
> be included here alongside the generic a31 fallback?

Following our general guideline, I believe it should.

> > +                     reg = <0x01cb2000 0x1000>;
> > +                     clocks = <&ccu CLK_BUS_CSI>,
> > +                              <&ccu CLK_MIPI_CSI>;
> > +                     clock-names = "bus", "mod";
> > +                     resets = <&ccu RST_BUS_CSI>;
>
> Will this node fail dtbs_check schema validation? The existing device tree
> binding schema for "allwinner,sun6i-a31-mipi-dphy" requires an interrupts
> property, which appears to be missing here.

Yeah, this is definitely going to fail validation.

> If the V3s hardware physically lacks a D-PHY interrupt, would it be better
> to add the V3s-specific compatible string and update the binding schema
> to make the interrupts property conditionally optional for the V3s?

FWIW the vendor kernel has some code for the D-PHY's interrupt bits [1].
Could you add the interrupt (I'm guessing the MIPI-CSI interrupt?) and
give it a quick test?


Thanks
ChenYu

[1] https://github.com/gediz/v3s-linux-sdk/blob/master/linux-3.4/drivers/media/video/sunxi-vfe/mipi_csi/dphy/dphy_reg.c


> > +                     allwinner,direction = "rx";
> > +                     status = "disabled";
> > +                     #phy-cells = <0>;
> > +             };
> > +
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260518153339.619947-1-paulk@sys-base.io?part=5
>
Paul Kocialkowski May 26, 2026, 2:13 p.m. UTC | #3 
Hi Chen-Yu,

On Sun 24 May 26, 20:49, Chen-Yu Tsai wrote:
> On Mon, May 18, 2026 at 6:12 PM <sashiko-bot@kernel.org> wrote:
> >
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > - [Medium] The `dphy` node is missing the required `interrupts` property and a SoC-specific compatible string.
> > --
> >
> > commit ceae1b5da24380c4a30d9a8b480a23eaee3c0814
> > Author: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> >
> > ARM: dts: sun8i: v3s: Add nodes for MIPI CSI-2 support
> >
> > This commit adds MIPI CSI-2 support to the Allwinner V3s SoC. It introduces
> > the device tree nodes for the CSI0 controller, the MIPI CSI-2 bridge, and
> > the D-PHY block.
> >
> > > diff --git a/arch/arm/boot/dts/allwinner/sun8i-v3s.dtsi b/arch/arm/boot/dts/allwinner/sun8i-v3s.dtsi
> > > --- a/arch/arm/boot/dts/allwinner/sun8i-v3s.dtsi
> > > +++ b/arch/arm/boot/dts/allwinner/sun8i-v3s.dtsi
> > [ ... ]
> > > +             dphy: d-phy@1cb2000 {
> > > +                     compatible = "allwinner,sun6i-a31-mipi-dphy";
> >
> > Should a SoC-specific compatible string like "allwinner,sun8i-v3s-mipi-dphy"
> > be included here alongside the generic a31 fallback?
> 
> Following our general guideline, I believe it should.

Yes I guess it makes sense. I don't think we've been doing that in every case
but it doesn't hurt.

> > > +                     reg = <0x01cb2000 0x1000>;
> > > +                     clocks = <&ccu CLK_BUS_CSI>,
> > > +                              <&ccu CLK_MIPI_CSI>;
> > > +                     clock-names = "bus", "mod";
> > > +                     resets = <&ccu RST_BUS_CSI>;
> >
> > Will this node fail dtbs_check schema validation? The existing device tree
> > binding schema for "allwinner,sun6i-a31-mipi-dphy" requires an interrupts
> > property, which appears to be missing here.
> 
> Yeah, this is definitely going to fail validation.

To be very clear about this, the interrupt is absolutely not required for
proper operation of the unit and there's a chance we might eventually see
a SoC that doesn't have it wired.

I could just hook the interrupt for now and we could make it optional if ever
needed, but I could also mark is as optional now if you prefer.

> > If the V3s hardware physically lacks a D-PHY interrupt, would it be better
> > to add the V3s-specific compatible string and update the binding schema
> > to make the interrupts property conditionally optional for the V3s?
> 
> FWIW the vendor kernel has some code for the D-PHY's interrupt bits [1].
> Could you add the interrupt (I'm guessing the MIPI-CSI interrupt?) and
> give it a quick test?

Thanks for digging this up! It really looks like it's just used for debug
purposes.

I have previously used the mipi csi-2 interrupt for the mipi csi-2 controller
and never for the d-phy. There's a chance it's the same interrupt that is wired
to both units (like it is for isp/csi).

I'll give it a try when I get back home. If it doesn't trigger, it probbaly
means it's not wired to the d-phy and should really be made optional.

All the best,

Paul

> 
> 
> Thanks
> ChenYu
> 
> [1] https://github.com/gediz/v3s-linux-sdk/blob/master/linux-3.4/drivers/media/video/sunxi-vfe/mipi_csi/dphy/dphy_reg.c
> 
> 
> > > +                     allwinner,direction = "rx";
> > > +                     status = "disabled";
> > > +                     #phy-cells = <0>;
> > > +             };
> > > +
> >
> > --
> > Sashiko AI review · https://sashiko.dev/#/patchset/20260518153339.619947-1-paulk@sys-base.io?part=5
> >
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/allwinner/sun8i-v3s.dtsi b/arch/arm/boot/dts/allwinner/sun8i-v3s.dtsi
index 02d6c62b3874..bfe02295f45d 100644
--- a/arch/arm/boot/dts/allwinner/sun8i-v3s.dtsi
+++ b/arch/arm/boot/dts/allwinner/sun8i-v3s.dtsi
@@ -671,6 +671,77 @@  gic: interrupt-controller@1c81000 {
 			interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
 		};
 
+		csi0: camera@1cb0000 {
+			compatible = "allwinner,sun8i-v3s-csi";
+			reg = <0x01cb0000 0x1000>;
+			interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_CSI>,
+				 <&ccu CLK_CSI_SCLK>,
+				 <&ccu CLK_DRAM_CSI>;
+			clock-names = "bus", "mod", "ram";
+			resets = <&ccu RST_BUS_CSI>;
+			interconnects = <&mbus 5>;
+			interconnect-names = "dma-mem";
+			status = "disabled";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@1 {
+					reg = <1>;
+
+					csi0_in_mipi_csi2: endpoint {
+						remote-endpoint = <&mipi_csi2_out_csi0>;
+					};
+				};
+			};
+		};
+
+		mipi_csi2: csi@1cb1000 {
+			compatible = "allwinner,sun8i-v3s-mipi-csi2",
+				     "allwinner,sun6i-a31-mipi-csi2";
+			reg = <0x01cb1000 0x1000>;
+			interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_CSI>,
+				 <&ccu CLK_CSI_SCLK>;
+			clock-names = "bus", "mod";
+			resets = <&ccu RST_BUS_CSI>;
+			status = "disabled";
+
+			phys = <&dphy>;
+			phy-names = "dphy";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				mipi_csi2_in: port@0 {
+					reg = <0>;
+				};
+
+				mipi_csi2_out: port@1 {
+					reg = <1>;
+
+					mipi_csi2_out_csi0: endpoint {
+						remote-endpoint = <&csi0_in_mipi_csi2>;
+					};
+				};
+			};
+		};
+
+		dphy: d-phy@1cb2000 {
+			compatible = "allwinner,sun6i-a31-mipi-dphy";
+			reg = <0x01cb2000 0x1000>;
+			clocks = <&ccu CLK_BUS_CSI>,
+				 <&ccu CLK_MIPI_CSI>;
+			clock-names = "bus", "mod";
+			resets = <&ccu RST_BUS_CSI>;
+			allwinner,direction = "rx";
+			status = "disabled";
+			#phy-cells = <0>;
+		};
+
 		csi1: camera@1cb4000 {
 			compatible = "allwinner,sun8i-v3s-csi";
 			reg = <0x01cb4000 0x3000>;