* [PATCH v2] ARM: dts: bcm2711-rpi-cm4-io: Add rtc on a pinctrl-muxed i2c bus
@ 2021-12-09 18:18 Uwe Kleine-König
2021-12-09 21:21 ` Uwe Kleine-König
2021-12-13 16:46 ` Maxime Ripard
0 siblings, 2 replies; 7+ messages in thread
From: Uwe Kleine-König @ 2021-12-09 18:18 UTC (permalink / raw)
To: Nicolas Saenz Julienne
Cc: bcm-kernel-feedback-list, linux-rpi-kernel, linux-arm-kernel,
Stefan Wahren, Cyril Brulebois, Dave Stevenson, Maxime Ripard,
Uwe Kleine-König
From: Uwe Kleine-König <uwe@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@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@0 {
+ reg = <0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
+ i2c@1 {
+ reg = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ rtc@51 {
+ /* Attention: An alarm resets the machine */
+ compatible = "nxp,pcf85063";
+ reg = <0x51>;
+ };
+ };
+ };
+};
+
+&i2c0 {
+ /delete-property/ pinctrl-names;
+ /delete-property/ pinctrl-0;
};
&ddc0 {
base-commit: 0fcfb00b28c0b7884635dacf38e46d60bf3d4eb1
--
2.30.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] ARM: dts: bcm2711-rpi-cm4-io: Add rtc on a pinctrl-muxed i2c bus
2021-12-09 18:18 [PATCH v2] ARM: dts: bcm2711-rpi-cm4-io: Add rtc on a pinctrl-muxed i2c bus Uwe Kleine-König
@ 2021-12-09 21:21 ` Uwe Kleine-König
2021-12-13 16:49 ` Maxime Ripard
2021-12-13 16:46 ` Maxime Ripard
1 sibling, 1 reply; 7+ messages in thread
From: Uwe Kleine-König @ 2021-12-09 21:21 UTC (permalink / raw)
To: Uwe Kleine-König, Nicolas Saenz Julienne
Cc: bcm-kernel-feedback-list, linux-rpi-kernel, linux-arm-kernel,
Stefan Wahren, Cyril Brulebois, Dave Stevenson, Maxime Ripard
[-- Attachment #1.1.1: Type: text/plain, Size: 2381 bytes --]
Hello,
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.
On 12/9/21 19:18, Uwe Kleine-König wrote:
> From: Uwe Kleine-König <uwe@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@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@0 {
> + reg = <0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> +
> + i2c@1 {
> + reg = <1>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + rtc@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@0 now instead of /soc/i2c@7e205000 (aka &i2c0).
Best regards
Uwe
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] ARM: dts: bcm2711-rpi-cm4-io: Add rtc on a pinctrl-muxed i2c bus
2021-12-09 18:18 [PATCH v2] ARM: dts: bcm2711-rpi-cm4-io: Add rtc on a pinctrl-muxed i2c bus Uwe Kleine-König
2021-12-09 21:21 ` Uwe Kleine-König
@ 2021-12-13 16:46 ` Maxime Ripard
1 sibling, 0 replies; 7+ messages in thread
From: Maxime Ripard @ 2021-12-13 16:46 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Nicolas Saenz Julienne, bcm-kernel-feedback-list,
linux-rpi-kernel, linux-arm-kernel, Stefan Wahren,
Cyril Brulebois, Dave Stevenson, Uwe Kleine-König
[-- Attachment #1.1: Type: text/plain, Size: 541 bytes --]
Hi,
On Thu, Dec 09, 2021 at 07:18:13PM +0100, Uwe Kleine-König wrote:
> From: Uwe Kleine-König <uwe@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@kleine-koenig.org>
Reviewed-by: Maxime Ripard <maxime@cerno.tech>
Maxime
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] ARM: dts: bcm2711-rpi-cm4-io: Add rtc on a pinctrl-muxed i2c bus
2021-12-09 21:21 ` Uwe Kleine-König
@ 2021-12-13 16:49 ` Maxime Ripard
2021-12-13 20:25 ` Uwe Kleine-König
0 siblings, 1 reply; 7+ messages in thread
From: Maxime Ripard @ 2021-12-13 16:49 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Uwe Kleine-König, Nicolas Saenz Julienne,
bcm-kernel-feedback-list, linux-rpi-kernel, linux-arm-kernel,
Stefan Wahren, Cyril Brulebois, Dave Stevenson
[-- Attachment #1.1: Type: text/plain, Size: 2944 bytes --]
On Thu, Dec 09, 2021 at 10:21:39PM +0100, Uwe Kleine-König wrote:
> Hello,
>
> 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?
> On 12/9/21 19:18, Uwe Kleine-König wrote:
> > From: Uwe Kleine-König <uwe@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@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@0 {
> > + reg = <0>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + };
> > +
> > + i2c@1 {
> > + reg = <1>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + rtc@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@0 now instead of /soc/i2c@7e205000 (aka &i2c0).
We can mitigate that by setting the i2c0 label to the /i2c0mux/i2c@0
node, since the overlay will not resolve what i2c0 is connected to until
it's applied.
Also, I think it's worth noting that this change will mimic what the RPi
kernel is doing.
Maxime
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] ARM: dts: bcm2711-rpi-cm4-io: Add rtc on a pinctrl-muxed i2c bus
2021-12-13 16:49 ` Maxime Ripard
@ 2021-12-13 20:25 ` Uwe Kleine-König
2021-12-15 9:56 ` Maxime Ripard
0 siblings, 1 reply; 7+ messages in thread
From: Uwe Kleine-König @ 2021-12-13 20:25 UTC (permalink / raw)
To: Maxime Ripard
Cc: Uwe Kleine-König, Nicolas Saenz Julienne,
bcm-kernel-feedback-list, linux-rpi-kernel, linux-arm-kernel,
Stefan Wahren, Cyril Brulebois, Dave Stevenson
[-- Attachment #1.1.1: Type: text/plain, Size: 3690 bytes --]
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.
>> On 12/9/21 19:18, Uwe Kleine-König wrote:
>>> From: Uwe Kleine-König <uwe@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@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@0 {
>>> + reg = <0>;
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> + };
>>> +
>>> + i2c@1 {
>>> + reg = <1>;
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> +
>>> + rtc@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@0 now instead of /soc/i2c@7e205000 (aka &i2c0).
>
> We can mitigate that by setting the i2c0 label to the /i2c0mux/i2c@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 aware how bcm2711-rpi-cm4-io.dts could make the i2c0 label point
to the i2c@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?
Best regards
Uwe
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] ARM: dts: bcm2711-rpi-cm4-io: Add rtc on a pinctrl-muxed i2c bus
2021-12-13 20:25 ` Uwe Kleine-König
@ 2021-12-15 9:56 ` Maxime Ripard
2021-12-16 21:29 ` Uwe Kleine-König
0 siblings, 1 reply; 7+ messages in thread
From: Maxime Ripard @ 2021-12-15 9:56 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Uwe Kleine-König, Nicolas Saenz Julienne,
bcm-kernel-feedback-list, linux-rpi-kernel, linux-arm-kernel,
Stefan Wahren, Cyril Brulebois, Dave Stevenson
[-- Attachment #1.1: Type: text/plain, Size: 4357 bytes --]
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@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@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@0 {
> > > > + reg = <0>;
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > + };
> > > > +
> > > > + i2c@1 {
> > > > + reg = <1>;
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > +
> > > > + rtc@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@0 now instead of /soc/i2c@7e205000 (aka &i2c0).
> >
> > We can mitigate that by setting the i2c0 label to the /i2c0mux/i2c@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@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
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] ARM: dts: bcm2711-rpi-cm4-io: Add rtc on a pinctrl-muxed i2c bus
2021-12-15 9:56 ` Maxime Ripard
@ 2021-12-16 21:29 ` Uwe Kleine-König
0 siblings, 0 replies; 7+ messages in thread
From: Uwe Kleine-König @ 2021-12-16 21:29 UTC (permalink / raw)
To: Maxime Ripard
Cc: Uwe Kleine-König, Nicolas Saenz Julienne,
bcm-kernel-feedback-list, linux-rpi-kernel, linux-arm-kernel,
Stefan Wahren, Cyril Brulebois, Dave Stevenson, Rob Herring
[-- Attachment #1.1: Type: text/plain, Size: 5538 bytes --]
Hello Maxime,
(added Rob to Cc for the dt question)
On Wed, Dec 15, 2021 at 10:56:37AM +0100, Maxime Ripard wrote:
> On Mon, Dec 13, 2021 at 09:25:16PM +0100, Uwe Kleine-König wrote:
> > 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@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@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@0 {
> > > > > + reg = <0>;
> > > > > + #address-cells = <1>;
> > > > > + #size-cells = <0>;
> > > > > + };
> > > > > +
> > > > > + i2c@1 {
> > > > > + reg = <1>;
> > > > > + #address-cells = <1>;
> > > > > + #size-cells = <0>;
> > > > > +
> > > > > + rtc@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@0 now instead of /soc/i2c@7e205000 (aka &i2c0).
> > >
> > > We can mitigate that by setting the i2c0 label to the /i2c0mux/i2c@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@0 node that is introduced here. After all the i2c label is defined
> > in another file. Is there some syntax to do that?
I still have no idea how to move the i2c0 label.
@Rob: An idea was to declare the i2c@0 node that the patch above
introduces should become the new &i2c0 as e.g. an overlay that adds an
i2c device should add it there now instead of the original bus. Is this
possible? And a good idea?
Apart from not knowing if this is possible at all, in the meantime I
doubt this to be a good idea. I think this adds to much confusion because
it complicates (human and machine) parsing of the device tree. If i2c0
is referenced before the move it should certainly continue to point to
/soc/i2c@7e205000, so usage of i2c0 depends on ordering.
> > > 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?
Ah, indeed. Will change to "as the vendor dts does".
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-12-16 21:31 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-12-09 18:18 [PATCH v2] ARM: dts: bcm2711-rpi-cm4-io: Add rtc on a pinctrl-muxed i2c bus Uwe Kleine-König
2021-12-09 21:21 ` Uwe Kleine-König
2021-12-13 16:49 ` Maxime Ripard
2021-12-13 20:25 ` Uwe Kleine-König
2021-12-15 9:56 ` Maxime Ripard
2021-12-16 21:29 ` Uwe Kleine-König
2021-12-13 16:46 ` Maxime Ripard
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox