[PATCH v4 0/2] ARM: dts: bcm2711-rpi-cm4-io: Add rtc on a pinctrl-muxed i2c bus

[Dave Stevenson]

Hi Laurent

On Tue, 1 Aug 2023 at 21:33, Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> Hi Dave,
>
> On Tue, Aug 01, 2023 at 05:43:15PM +0300, Laurent Pinchart wrote:
> > On Tue, Aug 01, 2023 at 03:22:17PM +0100, Dave Stevenson wrote:
> > > On Mon, 31 Jul 2023 at 22:55, Laurent Pinchart wrote:
> > > >
> > > > Hello,
> > > >
> > > > This series is an attempt to revive support for pinmuxed I2C0 on the
> > > > Raspberry Pi BCM2711-based board.
> > > >
> > > > On BCM2711-based boards, the I2C0 controller can be muxed between pins
> > > > 0+1 or 44+45. The former is exposed through the 40-pins GPIO connector,
> > > > and the latter is used for the RTC on the CM4 I/O board, but also routed
> > > > to the display and camera connectors on the Raspberry Pi 4B board. The
> > > > other BCM2711-based board, the Raspberry Pi 400, does not expose or
> > > > connect anything to pins 44+45.
> > > >
> > > > A previous version was posted ([1]) a year and a half ago by Uwe. It
> > > > bundled the pinmuxing and RTC in a single patch, with the mux added to
> > > > the CM4 I/O board device tree. This version splits this in two, and
> > > > moves the pinumxing to the bcm2711-rpi.dtsi to also support the
> > > > Raspberry Pi 4B.
> > > >
> > > > The Raspberry Pi downstream kernel has a more complex DT architecture in
> > > > order to support different I2C0 pinmuxing for other boards. Two files,
> > > > bcm283x-rpi-i2c0mux_0_28.dtsi and bcm283x-rpi-i2c0mux_0_44.dtsi, define
> > > > the two I2C0 pinxmuxing options (pins 0+1 and 28+29, or pins 0+1 and
> > > > 44+45). Each board .dts then includes the appropriate file. I'm hoping
> > > > to avoid this additional complexity for now, by addressing the I2C0
> > > > pinmuxing for BCM2711-based boards only. If/when support for I2C0
> > > > pinmuxing on boards will be needed, we can revisit this topic.
> > > >
> > > > Compared to the Raspberry Pi downstream kernel, the two muxed I2C buses
> > > > are labelled i2c0_0 and i2c0_1 instead of i2c0 and i2c_csi_dsi. This
> > > > change was made to keep the naming of the I2C controller labels
> > > > consistent, avoiding renaming of the I2C0 controller's label from i2c0
> > > > to i2c0if.
> > > >
> > > > Dave, are you fine with the differences between this patch series and
> > > > the downstream kernel, or do you expect them to cause issues ?
> > >
> > > I've checked with Phil. There's nothing too untoward that will cause
> > > us any significant grief.
> >
> > Thanks for checking.
> >
> > In the meantime, I realized that the CM4S is 2711-based and, according
> > to the downstream DT, multiplexes I2C0 on pins 28+29, not 44+45 :-(
> > Umang and Kieran also told me that we want to test camera support on the
> > Pi 3B. It looks like the only viable approach to support all that will
> > be to include per-board I2C0 pinmux .dtsi as done in the downstream
> > kernel. I'll send a v5.
> >
> > > Phil has commented that the RTC is an PCF85063AT, so that compatible
> > > string should be "nxp,pcf85063a" if you actually want to make use of
> > > the alarm output.
> > > Then again the driver support for the alarm output appears to want it
> > > routed to an IRQ rather than as a system reset/wakeup, so it probably
> > > makes little difference. It llargely depends on how exact you want to
> > > be in your hardware description.
> >
> > I'll update the compatible string, it's an easy change and it's nice to
> > be accurate.
>
> The driver will print a warning if the quartz-load-femtofarads property
> isn't set in DT (it can be either 7pF or 12.5pF).

Really? The bindings [1] don't list it as required, and just trying it
hasn't logged an error.
The return value from of_property_read_u32 isn't checked at [2], so
load remains unchanged at 7000 if not present.

> If I'm not mistaken,
> the quartz oscillator on the CM4 I/O board is calibrated for a 6pF load
> capacitance, so 7pF is the closest and best value. Could you confirm
> that ?

The relevant hardware person isn't in at present, but schematics for
the RTC are on page 13 of the cm4io-datasheet [4].
Crystal is listed as a X32K768S300, and Farnell's copy of the
datasheet [4] say that has a 6pF load capacitance.

  Dave

[1] https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/rtc/nxp%2Cpcf85063.yaml
[2] https://github.com/torvalds/linux/blob/master/drivers/rtc/rtc-pcf85063.c#L344
[3] https://datasheets.raspberrypi.com/cm4io/cm4io-datasheet.pdf
[4] https://www.farnell.com/datasheets/1881558.pdf

> > > > [1] https://lore.kernel.org/linux-arm-kernel/20211231115109.94626-1-uwe@kleine-koenig.org/
> > > >
> > > > Uwe Kleine-König (2):
> > > >   ARM: dts: bcm2711-rpi: Add pinctrl-based multiplexing for I2C0
> > > >   ARM: dts: bcm2711-rpi-cm4-io: Add RTC on I2C0
> > > >
> > > >  arch/arm/boot/dts/bcm2711-rpi-cm4-io.dts | 16 ++++++++++++
> > > >  arch/arm/boot/dts/bcm2711-rpi.dtsi       | 31 ++++++++++++++++++++++++
> > > >  2 files changed, 47 insertions(+)
>
> --
> Regards,
>
> Laurent Pinchart