| Message ID | 20211231115109.94626-1-uwe@kleine-koenig.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] ARM: dts: bcm2711-rpi-cm4-io: Add rtc on a pinctrl-muxed i2c bus | expand |
Hi Uwe, Thanks for the patch ! On 31/12/2021 12:51, Uwe Kleine-König wrote: > 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 vendor dts does. > > Note that if you modified the dts before to add devices to the i2c bus > appearing on pins gpio0 + gpio1 (either directly in the dts or using an > overlay), you have to put these into the i2c@0 node introduced here now. > > Reviewed-by: Maxime Ripard <maxime@cerno.tech> > Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org> > --- > Hello, > > changes since v2 (20211216212948.nrfmm4jpbhoknfr5@pengutronix.de): > > - add Maxime's R-b tag > - change the commit log wording to say vendor dts instead of upstream > dts > - Add a paragraph to the commit log about breakage this commits > introduces. > > Best regards > Uwe > > 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>; > + }; > + }; > + }; > +}; This is also needed for camera and display support. I tested it successfully with imx219 + unicam on mainline. > + > +&i2c0 { > + /delete-property/ pinctrl-names; > + /delete-property/ pinctrl-0; > }; > > &ddc0 { > > base-commit: fc74e0a40e4f9fd0468e34045b0c45bba11dcbb2 >
On 1/18/22 11:45 AM, Jean-Michel Hautbois wrote: > Hi Uwe, > > Thanks for the patch ! > > On 31/12/2021 12:51, Uwe Kleine-König wrote: >> 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 vendor dts does. >> >> Note that if you modified the dts before to add devices to the i2c bus >> appearing on pins gpio0 + gpio1 (either directly in the dts or using an >> overlay), you have to put these into the i2c@0 node introduced here now. >> >> Reviewed-by: Maxime Ripard <maxime@cerno.tech> >> Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org> >> --- >> Hello, >> >> changes since v2 (20211216212948.nrfmm4jpbhoknfr5@pengutronix.de): >> >> - add Maxime's R-b tag >> - change the commit log wording to say vendor dts instead of upstream >> dts >> - Add a paragraph to the commit log about breakage this commits >> introduces. >> >> Best regards >> Uwe >> >> 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>; >> + }; >> + }; >> + }; >> +}; > > 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?
On 18/01/2022 21:00, Florian Fainelli wrote: > On 1/18/22 11:45 AM, Jean-Michel Hautbois wrote: >> Hi Uwe, >> >> Thanks for the patch ! >> >> On 31/12/2021 12:51, Uwe Kleine-König wrote: >>> 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 vendor dts does. >>> >>> Note that if you modified the dts before to add devices to the i2c bus >>> appearing on pins gpio0 + gpio1 (either directly in the dts or using an >>> overlay), you have to put these into the i2c@0 node introduced here now. >>> >>> Reviewed-by: Maxime Ripard <maxime@cerno.tech> >>> Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org> >>> --- >>> Hello, >>> >>> changes since v2 (20211216212948.nrfmm4jpbhoknfr5@pengutronix.de): >>> >>> - add Maxime's R-b tag >>> - change the commit log wording to say vendor dts instead of upstream >>> dts >>> - Add a paragraph to the commit log about breakage this commits >>> introduces. >>> >>> Best regards >>> Uwe >>> >>> 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>; >>> + }; >>> + }; >>> + }; >>> +}; >> >> 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? > Oh, missed this, sorry: Tested-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Hi Florian, On Tue, Jan 18, 2022 at 12:00:50PM -0800, Florian Fainelli wrote: > On 1/18/22 11:45 AM, Jean-Michel Hautbois wrote: > > On 31/12/2021 12:51, Uwe Kleine-König wrote: > >> 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 vendor dts does. > >> > >> Note that if you modified the dts before to add devices to the i2c bus > >> appearing on pins gpio0 + gpio1 (either directly in the dts or using an > >> overlay), you have to put these into the i2c@0 node introduced here now. > >> > >> Reviewed-by: Maxime Ripard <maxime@cerno.tech> > >> Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org> > >> --- > >> Hello, > >> > >> changes since v2 (20211216212948.nrfmm4jpbhoknfr5@pengutronix.de): > >> > >> - add Maxime's R-b tag > >> - change the commit log wording to say vendor dts instead of upstream > >> dts > >> - Add a paragraph to the commit log about breakage this commits > >> introduces. > >> > >> Best regards > >> Uwe > >> > >> 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>; > >> + }; > >> + }; > >> + }; > >> +}; > > > > 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), or move it to per-board files. 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.
Hello, 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. > , or move it to per-board files. It is in an board file now?! So I don't understand your suggestion here. > 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?! Best regards Uwe
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@0 { reg = <0>; #address-cells = <1>; #size-cells = <0>; }; i2c_csi_dsi: i2c@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.
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 {