[PATCH v3] ARM: dts: bcm2711-rpi-cm4-io: Add rtc on a pinctrl-muxed i2c bus

[Laurent Pinchart]

Hi Uwe,

On Tue, Jan 18, 2022 at 11:41:19PM +0100, Uwe Kleine-König wrote:
> On 1/18/22 21:47, Laurent Pinchart wrote:
> > On Tue, Jan 18, 2022 at 12:00:50PM -0800, Florian Fainelli wrote:
> >> On 1/18/22 11:45 AM, Jean-Michel Hautbois wrote:
> >>> This is also needed for camera and display support.
> >>> I tested it successfully with imx219 + unicam on mainline.
> >>
> >> Thanks for testing, can you reply with a Tested-by tag so it could be
> >> applied to the commit message when this gets picked up?
> > 
> > Well, this also points out that there's an issue: if the mux is needed
> > for other devices, it shouldn't be in bcm2711-rpi-cm4-io.dts :-) We
> > could move it to bcm2711-rpi.dtsi (so far all bcm2711-based boards use
> > either I/O pins 0+1 or 44+45)
> 
> If I understand correctly it's not used on rpi-4-b, so bcm2711-rpi.dtsi 
> would be wrong.

rpi-4-b muxes I2C0 on pins 0+1 and 44+45. The latter is wired to the
camera connector, and used for the camera sensor. Same thing on rpi-cm4.
rpi-400 has no camera connector, but I believe the display I2C bus is
also on pins 44+45 (at least according to the downstream DT sources,
rpi-400 muxes I2C0 on 0+1 and 44+45 too).

> > , or move it to per-board files.
> 
> It is in an board file now?! So I don't understand your suggestion here.

Sorry, I meant have it in per-board files, not more it there.

> > In the
> > latter case, instead of duplicating the same block everywhere, it could
> > be moved to a .dtsi included in those board files. This is what the
> > downstream kernel does.
> 
> How does it call the dtsi file? I wonder if that is sensible expecting 
> that the devices on the bus are different for different boards?!

Downstream has a bcm283x-rpi-i2c0mux_0_44.dtsi that just contains

&i2c0mux {
	pinctrl-0 = <&i2c0_gpio0>;
	pinctrl-1 = <&i2c0_gpio44>;
};

with i2c0mux defined in bcm283x.dtsi as

	i2c0mux: i2c0mux {
		compatible = "i2c-mux-pinctrl";
		#address-cells = <1>;
		#size-cells = <0>;

		i2c-parent = <&i2c0if>;

		pinctrl-names = "i2c0", "i2c_csi_dsi";

		status = "disabled";

		i2c0: i2c at 0 {
			reg = <0>;
			#address-cells = <1>;
			#size-cells = <0>;
		};

		i2c_csi_dsi: i2c at 1 {
			reg = <1>;
			#address-cells = <1>;
			#size-cells = <0>;
		};
	};

The following board files #include "bcm283x-rpi-i2c0mux_0_44.dtsi":

- bcm2710-rpi-3-b.dts
- bcm2710-rpi-3-b-plus.dts
- bcm2710-rpi-zero-2-w.dts
- bcm2711-rpi-400.dts
- bcm2711-rpi-4-b.dts
- bcm2711-rpi-4-b.dts.orig
- bcm2711-rpi-cm4.dts

We don't have to replicate the exact same mechanism and use the same
names, but for rpi-4-b and rpi-cm4, to enable camera support (which
we're working on, Jean-Michel has posted a driver for the Unicam CSI-2
receiver to the linux-media mailing list, the ISP will follow), we need
the mux. Given that those two boards have a camera connector, I think it
makes sense to define the mux in a different file than
bcm2711-rpi-cm4-io.dts. The RTC node can stay in bcm2711-rpi-cm4-io.dts.

-- 
Regards,

Laurent Pinchart