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

[Maxime Ripard]

On Mon, Dec 13, 2021 at 09:25:16PM +0100, Uwe Kleine-König wrote:
> Hello Maxime,
> 
> On 12/13/21 17:49, Maxime Ripard wrote:
> > On Thu, Dec 09, 2021 at 10:21:39PM +0100, Uwe Kleine-König wrote:
> > > I used the wrong git wrapper so this patch appeared with the wrong sender
> > > address. If need be I can resend from the address used in the S-o-b.
> > 
> > Your SoB seems to match the From header, so unless you'd like to
> > contribute with your pengutronix it should be fine?
> 
> The problem is that the sender of the patch (i.e. my pengutronix self)
> didn't add a S-o-b.

I don't think that's a problem, but I might be wrong

> > > On 12/9/21 19:18, Uwe Kleine-König wrote:
> > > > From: Uwe Kleine-König <uwe at kleine-koenig.org>
> > > > 
> > > > The cm4-io board comes with an PCF85063. Add it to the device tree to make
> > > > it usable. The i2c0 bus can use two different pinmux settings to use
> > > > different pins. To keep the bus appearing on the usual pin pair (gpio0 +
> > > > gpio1) use a pinctrl-muxed setting as the upstream dts does.
> > > > 
> > > > Signed-off-by: Uwe Kleine-König <uwe at kleine-koenig.org>
> > > > ---
> > > >    arch/arm/boot/dts/bcm2711-rpi-cm4-io.dts | 35 ++++++++++++++++++++++++
> > > >    1 file changed, 35 insertions(+)
> > > > 
> > > > diff --git a/arch/arm/boot/dts/bcm2711-rpi-cm4-io.dts b/arch/arm/boot/dts/bcm2711-rpi-cm4-io.dts
> > > > index 19600b629be5..5ddad146b541 100644
> > > > --- a/arch/arm/boot/dts/bcm2711-rpi-cm4-io.dts
> > > > +++ b/arch/arm/boot/dts/bcm2711-rpi-cm4-io.dts
> > > > @@ -18,6 +18,41 @@ led-pwr {
> > > >    			linux,default-trigger = "default-on";
> > > >    		};
> > > >    	};
> > > > +
> > > > +	i2c0mux {
> > > > +		compatible = "i2c-mux-pinctrl";
> > > > +		#address-cells = <1>;
> > > > +		#size-cells = <0>;
> > > > +
> > > > +		i2c-parent = <&i2c0>;
> > > > +
> > > > +		pinctrl-names = "i2c0", "i2c0-vc";
> > > > +		pinctrl-0 = <&i2c0_gpio0>;
> > > > +		pinctrl-1 = <&i2c0_gpio44>;
> > > > +
> > > > +		i2c at 0 {
> > > > +			reg = <0>;
> > > > +			#address-cells = <1>;
> > > > +			#size-cells = <0>;
> > > > +		};
> > > > +
> > > > +		i2c at 1 {
> > > > +			reg = <1>;
> > > > +			#address-cells = <1>;
> > > > +			#size-cells = <0>;
> > > > +
> > > > +			rtc at 51 {
> > > > +				/* Attention: An alarm resets the machine */
> > > > +				compatible = "nxp,pcf85063";
> > > > +				reg = <0x51>;
> > > > +			};
> > > > +		};
> > > > +	};
> > > > +};
> > > > +
> > > > +&i2c0 {
> > > > +	/delete-property/ pinctrl-names;
> > > > +	/delete-property/ pinctrl-0;
> > > >    };
> > > >    &ddc0 {
> > > 
> > > Just some further information:
> > > 
> > > Cyril (who I talked to on irc) and I found that not only i2c0 can be used to
> > > access the gpio bus on pins 44/45 (as I did in v1) but that also &i2c1 can
> > > drive these pins. However similar to &i2c0, &i2c1 is used on pins 2/3 per
> > > default and so is also available on the default gpio header.
> > > 
> > > Also note that this is a breaking change, because overlays that added
> > > devices to &i2c0 before (expecting them to be accessed via gpio0/1) need to
> > > modify /i2c0mux/i2c at 0 now instead of /soc/i2c at 7e205000 (aka &i2c0).
> > 
> > We can mitigate that by setting the i2c0 label to the /i2c0mux/i2c at 0
> > node, since the overlay will not resolve what i2c0 is connected to until
> > it's applied.
> 
> It depends on the overlay if that works or not. If the node to patch is
> specified by label, this is fine. If however the node is referenced by path
> (which is the usual thing for mainline, as the dtbs are compiled without
> -@), this change is a problem.

I'm not sure that using path is as ubiquitous with mainline as you imply
(I've never used or saw that myself), but yeah, any overlay that would
use the path directly would be broken.

> I'm not aware how bcm2711-rpi-cm4-io.dts could make the i2c0 label point to
> the i2c at 0 node that is introduced here. After all the i2c label is defined
> in another file. Is there some syntax to do that?
> 
> > Also, I think it's worth noting that this change will mimic what the RPi
> > kernel is doing.
> 
> The commit log has "Use [...] as the upstream dts does.". Isn't that good
> enough?

Isn't upstream usually kernel.org kernels?

Maxime
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20211215/78f54cfb/attachment-0001.sig>