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

[Dave Stevenson]

Hi Laurent and Uwe.

On Tue, 18 Jan 2022 at 22:59, Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> 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).

Minor correction - Pi 400 has no camera or display connector.
I'm double checking with the hardware folk, but AFAIK 44&45 aren't
used for any other purpose, so leaving the i2c0mux defined to them
doesn't cause any issues.

Camera and DSI display connectors on all Raspberry Pi regular boards
share the same I2C GPIOs muxing. (The original Pi model A & B didn't
route I2C to the display).
On a CMIO board, CAM0 & DISP0 connectors share an I2C muxing (normally
0&1), and CAM1 & DISP1 connectors share a muxing (either 28&29, or
44&45).

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

For completeness, the earlier boards use a
bcm283x-rpi-i2c0mux_0_28.dtsi file as they use GPIOs 28&29 for the
camera & display I2C. If my memory serves correctly, it had to move to
allow connecting the SDIO interface to the Wifi chip on the boards you
list above.
The one awkward one is the very original rev1 256MB Model B (and 128MB
model A?) which routed GPIOs 2&3 and BSC1 to the camera connector.
Life's never easy!

  Dave

> 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