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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 1/19/2022 1:44 AM, Dave Stevenson wrote:
Hi Laurent and Uwe.

On Tue, 18 Jan 2022 at 22:59, Laurent Pinchart
<laurent.pinchart@xxxxxxxxxxxxxxxx> wrote:

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).

Minor correction - Pi 400 has no camera or display connector.
I'm double checking with the hardware folk, but AFAIK 44&45 aren't
used for any other purpose, so leaving the i2c0mux defined to them
doesn't cause any issues.

Camera and DSI display connectors on all Raspberry Pi regular boards
share the same I2C GPIOs muxing. (The original Pi model A & B didn't
route I2C to the display).
On a CMIO board, CAM0 & DISP0 connectors share an I2C muxing (normally
0&1), and CAM1 & DISP1 connectors share a muxing (either 28&29, or
44&45).

, 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

For completeness, the earlier boards use a
bcm283x-rpi-i2c0mux_0_28.dtsi file as they use GPIOs 28&29 for the
camera & display I2C. If my memory serves correctly, it had to move to
allow connecting the SDIO interface to the Wifi chip on the boards you
list above.
The one awkward one is the very original rev1 256MB Model B (and 128MB
model A?) which routed GPIOs 2&3 and BSC1 to the camera connector.
Life's never easy!

   Dave

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.

Uwe, were you going to follow Laurent's suggestion and create a specific file that other boards can include? You have a few more days before I sent the pull request for 5.18. Thanks!
--
Florian



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux