Hi Laurent On Fri, 1 Mar 2024 at 21:32, Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> wrote: > > From: Uwe Kleine-König <uwe@xxxxxxxxxxxxxxxxx> > > BCM2711-based Raspberry Pi boards (4B, CM4 and 400) multiplex the I2C0 > controller over two sets of pins, GPIO0+1 and GPIO44+45. The former is > exposed on the 40-pin header, while the latter is used for the CSI and > DSI connectors. It's true for all Pis that I2C0 is exposed over 2 sets of gpios. Seeing as we want to support cameras on Pi0-3, is there a reason not to include the mux on those? Looking back I had started this way back in [1] with all the variants. I thought I'd posted the v2 follow up, but can't find it. The original Pi 1 models A & B were the annoyances. The rev1 put the camera on i2c1 GPIOs 2&3, with the rev2 on i2c0 with GPIOs 0&1. Whilst it would be nice to have support for all platforms, this doesn't stop us moving the mux into bcm283x-rpi.dtsi at a later date to add support for the other devices. I'm happy enough having the first step of getting Pi4 working, with others being done later. [1] https://linux-rpi-kernel.infradead.narkive.com/lmzYlT3c/rfc-arm-dts-add-i2cmux-pinctrl-config-to-raspberry-pi-i2c-0 > Add a pinctrl-based I2C bus multiplexer to bcm2711-rpi.dtsi to model > this multiplexing. The two child buses are named i2c0_0 and i2c0_1. > > 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 i2c0_0 node introduced here > now. > > Signed-off-by: Uwe Kleine-König <uwe@xxxxxxxxxxxxxxxxx> > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > --- > Changes since v3: > > - Split addition of the RTC to a separate patch > - Move the mux to bcm2711-rpi.dtsi > --- > arch/arm/boot/dts/broadcom/bcm2711-rpi.dtsi | 31 +++++++++++++++++++++ > 1 file changed, 31 insertions(+) > > diff --git a/arch/arm/boot/dts/broadcom/bcm2711-rpi.dtsi b/arch/arm/boot/dts/broadcom/bcm2711-rpi.dtsi > index 86188eabeb24..826ed6efa9ff 100644 > --- a/arch/arm/boot/dts/broadcom/bcm2711-rpi.dtsi > +++ b/arch/arm/boot/dts/broadcom/bcm2711-rpi.dtsi > @@ -17,6 +17,32 @@ aliases { > pcie0 = &pcie0; > blconfig = &blconfig; > }; > + > + i2c0mux: 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>; > + > + status = "disabled"; Why defaulting to disabled? The current mainline DT defaults to i2c0 being enabled on GPIOs 0&1 (done via bcm2835-rpi.dtsi). If the mux is disabled, then this change has left i2c0 being enabled but with no pinctrl property, so it's not connected to the outside world. GPIOs 44&45 have never had any other user, therefore claiming them for the mux isn't a regression in my view. As long as we can enable the other platforms later, and with the minor caveat over being enabled or not: Acked-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx> Minor point that CONFIG_I2C_MUX_PINCTRL appears not to be in the arm64 defconfig. I don't know what the policy is there, but there seem to be many other SoCs throwing modules in there for their configurations. It is in arm/multi_v7_defconfig. Dave > + > + i2c0_0: i2c@0 { > + reg = <0>; > + #address-cells = <1>; > + #size-cells = <0>; > + }; > + > + i2c0_1: i2c@1 { > + reg = <1>; > + #address-cells = <1>; > + #size-cells = <0>; > + }; > + }; > }; > > &firmware { > @@ -49,6 +75,11 @@ &hvs { > clocks = <&firmware_clocks 4>; > }; > > +&i2c0 { > + /delete-property/ pinctrl-names; > + /delete-property/ pinctrl-0; > +}; > + > &rmem { > /* > * RPi4's co-processor will copy the board's bootloader configuration > -- > Regards, > > Laurent Pinchart >
