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

[Paul Kocialkowski <paulk@sys-base.io>]

[-- Attachment #1: Type: text/plain, Size: 3856 bytes --]

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
> >

-- 
Paul Kocialkowski,

Independent contractor - sys-base - https://www.sys-base.io/
Free software developer - https://www.paulk.fr/

Expert in multimedia, graphics and embedded hardware support with Linux.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]