All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] ARM: dts: bcm2711-rpi-cm4-io: Add rtc on a pinctrl-muxed i2c bus
@ 2021-12-31 11:51 ` Uwe Kleine-König
  0 siblings, 0 replies; 22+ messages in thread
From: Uwe Kleine-König @ 2021-12-31 11:51 UTC (permalink / raw)
  To: Nicolas Saenz Julienne
  Cc: Rob Herring, devicetree, bcm-kernel-feedback-list,
	linux-rpi-kernel, linux-arm-kernel, Stefan Wahren,
	Cyril Brulebois, Dave Stevenson, Maxime Ripard

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>;
+			};
+		};
+	};
+};
+
+&i2c0 {
+	/delete-property/ pinctrl-names;
+	/delete-property/ pinctrl-0;
 };
 
 &ddc0 {

base-commit: fc74e0a40e4f9fd0468e34045b0c45bba11dcbb2
-- 
2.34.1


_______________________________________________
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] 22+ messages in thread

* [PATCH v3] ARM: dts: bcm2711-rpi-cm4-io: Add rtc on a pinctrl-muxed i2c bus
@ 2021-12-31 11:51 ` Uwe Kleine-König
  0 siblings, 0 replies; 22+ messages in thread
From: Uwe Kleine-König @ 2021-12-31 11:51 UTC (permalink / raw)
  To: Nicolas Saenz Julienne
  Cc: Rob Herring, devicetree, bcm-kernel-feedback-list,
	linux-rpi-kernel, linux-arm-kernel, Stefan Wahren,
	Cyril Brulebois, Dave Stevenson, Maxime Ripard

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>;
+			};
+		};
+	};
+};
+
+&i2c0 {
+	/delete-property/ pinctrl-names;
+	/delete-property/ pinctrl-0;
 };
 
 &ddc0 {

base-commit: fc74e0a40e4f9fd0468e34045b0c45bba11dcbb2
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v3] ARM: dts: bcm2711-rpi-cm4-io: Add rtc on a pinctrl-muxed i2c bus
  2021-12-31 11:51 ` Uwe Kleine-König
@ 2022-01-02  4:33   ` Cyril Brulebois
  -1 siblings, 0 replies; 22+ messages in thread
From: Cyril Brulebois @ 2022-01-02  4:33 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Nicolas Saenz Julienne, Rob Herring, devicetree,
	bcm-kernel-feedback-list, linux-rpi-kernel, linux-arm-kernel,
	Stefan Wahren, Dave Stevenson, Maxime Ripard

[-- Attachment #1: Type: text/plain, Size: 2936 bytes --]

Uwe Kleine-König <uwe@kleine-koenig.org> (2021-12-31):
> 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>

Tested-by: Cyril Brulebois <cyril@debamax.com>


Test hardware:
 - 1 Compute Module 4 IO Board;
 - 1 GPIO Extension Board, plugged onto the 40-pin header (mainly used
   for its labels, to ensure I'm using the correct pins);
 - 1 Pimoroni 5x5 RGB LED matrix, plugged onto SDA0/SCL0 as exposed by
   the GPIO Extension Board (plus 3.3V/GND of course);
 - Debian 11 userspace.

With the updated DTB, and provided the right kernel modules have been
enabled (CONFIG_RTC_DRV_PCF85063, CONFIG_I2C_MUX_PINCTRL), the RTC comes
up automatically; I'm also able to use the relevant library to display a
rotating rainbow on the LED matrix while being able to read from / write
to the RTC:
  https://github.com/pimoroni/rgbmatrix5x5-python

In passing, I'm seeing the RTC exclusively on (userspace) i2c-2 while
I'm seeing the LED matrix on both (userspace) i2c-2 and i2c-3:

    # i2cdetect -y 2
         0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
    00:                         -- -- -- -- 0c -- -- -- 
    10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
    20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 2f 
    30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
    40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
    50: -- UU -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
    60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
    70: -- -- -- -- -- -- -- --                         
    
    # i2cdetect -y 3
         0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
    00:                         -- -- -- -- -- -- -- -- 
    10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
    20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
    30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
    40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
    50: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
    60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
    70: -- -- -- -- 74 -- -- --                         

Tweaking the library to use either of those makes the rotation rainbow
display successfully (via python3-smbus):

    self.i2c = smbus.SMBus(2)
    self.i2c = smbus.SMBus(3)


Cheers,
-- 
Cyril Brulebois -- Debian Consultant @ DEBAMAX -- https://debamax.com/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3] ARM: dts: bcm2711-rpi-cm4-io: Add rtc on a pinctrl-muxed i2c bus
@ 2022-01-02  4:33   ` Cyril Brulebois
  0 siblings, 0 replies; 22+ messages in thread
From: Cyril Brulebois @ 2022-01-02  4:33 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Nicolas Saenz Julienne, Rob Herring, devicetree,
	bcm-kernel-feedback-list, linux-rpi-kernel, linux-arm-kernel,
	Stefan Wahren, Dave Stevenson, Maxime Ripard


[-- Attachment #1.1: Type: text/plain, Size: 2936 bytes --]

Uwe Kleine-König <uwe@kleine-koenig.org> (2021-12-31):
> 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>

Tested-by: Cyril Brulebois <cyril@debamax.com>


Test hardware:
 - 1 Compute Module 4 IO Board;
 - 1 GPIO Extension Board, plugged onto the 40-pin header (mainly used
   for its labels, to ensure I'm using the correct pins);
 - 1 Pimoroni 5x5 RGB LED matrix, plugged onto SDA0/SCL0 as exposed by
   the GPIO Extension Board (plus 3.3V/GND of course);
 - Debian 11 userspace.

With the updated DTB, and provided the right kernel modules have been
enabled (CONFIG_RTC_DRV_PCF85063, CONFIG_I2C_MUX_PINCTRL), the RTC comes
up automatically; I'm also able to use the relevant library to display a
rotating rainbow on the LED matrix while being able to read from / write
to the RTC:
  https://github.com/pimoroni/rgbmatrix5x5-python

In passing, I'm seeing the RTC exclusively on (userspace) i2c-2 while
I'm seeing the LED matrix on both (userspace) i2c-2 and i2c-3:

    # i2cdetect -y 2
         0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
    00:                         -- -- -- -- 0c -- -- -- 
    10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
    20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 2f 
    30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
    40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
    50: -- UU -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
    60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
    70: -- -- -- -- -- -- -- --                         
    
    # i2cdetect -y 3
         0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
    00:                         -- -- -- -- -- -- -- -- 
    10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
    20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
    30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
    40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
    50: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
    60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 
    70: -- -- -- -- 74 -- -- --                         

Tweaking the library to use either of those makes the rotation rainbow
display successfully (via python3-smbus):

    self.i2c = smbus.SMBus(2)
    self.i2c = smbus.SMBus(3)


Cheers,
-- 
Cyril Brulebois -- Debian Consultant @ DEBAMAX -- https://debamax.com/

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 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] 22+ messages in thread

* Re: [PATCH v3] ARM: dts: bcm2711-rpi-cm4-io: Add rtc on a pinctrl-muxed i2c bus
  2022-01-02  4:33   ` Cyril Brulebois
@ 2022-01-02  6:34     ` Cyril Brulebois
  -1 siblings, 0 replies; 22+ messages in thread
From: Cyril Brulebois @ 2022-01-02  6:34 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Nicolas Saenz Julienne, Rob Herring, devicetree,
	bcm-kernel-feedback-list, linux-rpi-kernel, linux-arm-kernel,
	Stefan Wahren, Dave Stevenson, Maxime Ripard

[-- Attachment #1: Type: text/plain, Size: 1066 bytes --]

Cyril Brulebois <cyril@debamax.com> (2022-01-02):
> In passing, I'm seeing the RTC exclusively on (userspace) i2c-2 while
> I'm seeing the LED matrix on both (userspace) i2c-2 and i2c-3: […]

After a few more tests/reboots, checking the output of `i2cdetect -l`
gives the appropriate information, so that one can pick the right i2c-N
device, by looking at the *-mux entries and their channel IDs:

    …
    i2c-2	i2c       	bcm2835 (i2c@7e205000)          	I2C adapter
    …
    i2c-4	i2c       	i2c-2-mux (chan_id 0)           	I2C adapter
    i2c-5	i2c       	i2c-2-mux (chan_id 1)           	I2C adapter

And indeed, those channel IDs match what's been defined in the DTB, with
#0 set to i2c0_gpio0 (e.g. LED matrix on 40-pin header) and #1 set to
i2c0_gpio44 (e.g. RTC, fan control, etc.), which seem quite consistent
this time:

 - `i2cdetect -y 4` reports only 74 (the LED matrix);
 - `i2cdetect -y 5` reports 0c, 2f, and 51 as UU (the RTC).


Cheers,
-- 
Cyril Brulebois -- Debian Consultant @ DEBAMAX -- https://debamax.com/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3] ARM: dts: bcm2711-rpi-cm4-io: Add rtc on a pinctrl-muxed i2c bus
@ 2022-01-02  6:34     ` Cyril Brulebois
  0 siblings, 0 replies; 22+ messages in thread
From: Cyril Brulebois @ 2022-01-02  6:34 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Nicolas Saenz Julienne, Rob Herring, devicetree,
	bcm-kernel-feedback-list, linux-rpi-kernel, linux-arm-kernel,
	Stefan Wahren, Dave Stevenson, Maxime Ripard


[-- Attachment #1.1: Type: text/plain, Size: 1066 bytes --]

Cyril Brulebois <cyril@debamax.com> (2022-01-02):
> In passing, I'm seeing the RTC exclusively on (userspace) i2c-2 while
> I'm seeing the LED matrix on both (userspace) i2c-2 and i2c-3: […]

After a few more tests/reboots, checking the output of `i2cdetect -l`
gives the appropriate information, so that one can pick the right i2c-N
device, by looking at the *-mux entries and their channel IDs:

    …
    i2c-2	i2c       	bcm2835 (i2c@7e205000)          	I2C adapter
    …
    i2c-4	i2c       	i2c-2-mux (chan_id 0)           	I2C adapter
    i2c-5	i2c       	i2c-2-mux (chan_id 1)           	I2C adapter

And indeed, those channel IDs match what's been defined in the DTB, with
#0 set to i2c0_gpio0 (e.g. LED matrix on 40-pin header) and #1 set to
i2c0_gpio44 (e.g. RTC, fan control, etc.), which seem quite consistent
this time:

 - `i2cdetect -y 4` reports only 74 (the LED matrix);
 - `i2cdetect -y 5` reports 0c, 2f, and 51 as UU (the RTC).


Cheers,
-- 
Cyril Brulebois -- Debian Consultant @ DEBAMAX -- https://debamax.com/

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 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] 22+ messages in thread

* Re: [PATCH v3] ARM: dts: bcm2711-rpi-cm4-io: Add rtc on a pinctrl-muxed i2c bus
  2021-12-31 11:51 ` Uwe Kleine-König
@ 2022-01-18 19:45   ` Jean-Michel Hautbois
  -1 siblings, 0 replies; 22+ messages in thread
From: Jean-Michel Hautbois @ 2022-01-18 19:45 UTC (permalink / raw)
  To: Uwe Kleine-König, Nicolas Saenz Julienne
  Cc: Rob Herring, devicetree, bcm-kernel-feedback-list,
	linux-rpi-kernel, linux-arm-kernel, Stefan Wahren,
	Cyril Brulebois, Dave Stevenson, Maxime Ripard, Laurent Pinchart

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
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3] ARM: dts: bcm2711-rpi-cm4-io: Add rtc on a pinctrl-muxed i2c bus
@ 2022-01-18 19:45   ` Jean-Michel Hautbois
  0 siblings, 0 replies; 22+ messages in thread
From: Jean-Michel Hautbois @ 2022-01-18 19:45 UTC (permalink / raw)
  To: Uwe Kleine-König, Nicolas Saenz Julienne
  Cc: Rob Herring, devicetree, bcm-kernel-feedback-list,
	linux-rpi-kernel, linux-arm-kernel, Stefan Wahren,
	Cyril Brulebois, Dave Stevenson, Maxime Ripard, Laurent Pinchart

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
> 

_______________________________________________
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] 22+ messages in thread

* Re: [PATCH v3] ARM: dts: bcm2711-rpi-cm4-io: Add rtc on a pinctrl-muxed i2c bus
  2022-01-18 19:45   ` Jean-Michel Hautbois
@ 2022-01-18 20:00     ` Florian Fainelli
  -1 siblings, 0 replies; 22+ messages in thread
From: Florian Fainelli @ 2022-01-18 20:00 UTC (permalink / raw)
  To: Jean-Michel Hautbois, Uwe Kleine-König, Nicolas Saenz Julienne
  Cc: Rob Herring, devicetree, bcm-kernel-feedback-list,
	linux-rpi-kernel, linux-arm-kernel, Stefan Wahren,
	Cyril Brulebois, Dave Stevenson, Maxime Ripard, Laurent Pinchart

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?
-- 
Florian

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3] ARM: dts: bcm2711-rpi-cm4-io: Add rtc on a pinctrl-muxed i2c bus
@ 2022-01-18 20:00     ` Florian Fainelli
  0 siblings, 0 replies; 22+ messages in thread
From: Florian Fainelli @ 2022-01-18 20:00 UTC (permalink / raw)
  To: Jean-Michel Hautbois, Uwe Kleine-König, Nicolas Saenz Julienne
  Cc: Rob Herring, devicetree, bcm-kernel-feedback-list,
	linux-rpi-kernel, linux-arm-kernel, Stefan Wahren,
	Cyril Brulebois, Dave Stevenson, Maxime Ripard, Laurent Pinchart

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?
-- 
Florian

_______________________________________________
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] 22+ messages in thread

* Re: [PATCH v3] ARM: dts: bcm2711-rpi-cm4-io: Add rtc on a pinctrl-muxed i2c bus
  2022-01-18 20:00     ` Florian Fainelli
@ 2022-01-18 20:02       ` Jean-Michel Hautbois
  -1 siblings, 0 replies; 22+ messages in thread
From: Jean-Michel Hautbois @ 2022-01-18 20:02 UTC (permalink / raw)
  To: Florian Fainelli, Uwe Kleine-König, Nicolas Saenz Julienne
  Cc: Rob Herring, devicetree, bcm-kernel-feedback-list,
	linux-rpi-kernel, linux-arm-kernel, Stefan Wahren,
	Cyril Brulebois, Dave Stevenson, Maxime Ripard, Laurent Pinchart

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>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3] ARM: dts: bcm2711-rpi-cm4-io: Add rtc on a pinctrl-muxed i2c bus
@ 2022-01-18 20:02       ` Jean-Michel Hautbois
  0 siblings, 0 replies; 22+ messages in thread
From: Jean-Michel Hautbois @ 2022-01-18 20:02 UTC (permalink / raw)
  To: Florian Fainelli, Uwe Kleine-König, Nicolas Saenz Julienne
  Cc: Rob Herring, devicetree, bcm-kernel-feedback-list,
	linux-rpi-kernel, linux-arm-kernel, Stefan Wahren,
	Cyril Brulebois, Dave Stevenson, Maxime Ripard, Laurent Pinchart

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>

_______________________________________________
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] 22+ messages in thread

* Re: [PATCH v3] ARM: dts: bcm2711-rpi-cm4-io: Add rtc on a pinctrl-muxed i2c bus
  2022-01-18 20:00     ` Florian Fainelli
@ 2022-01-18 20:47       ` Laurent Pinchart
  -1 siblings, 0 replies; 22+ messages in thread
From: Laurent Pinchart @ 2022-01-18 20:47 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Jean-Michel Hautbois, Uwe Kleine-König,
	Nicolas Saenz Julienne, Rob Herring, devicetree,
	bcm-kernel-feedback-list, linux-rpi-kernel, linux-arm-kernel,
	Stefan Wahren, Cyril Brulebois, Dave Stevenson, Maxime Ripard

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.

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3] ARM: dts: bcm2711-rpi-cm4-io: Add rtc on a pinctrl-muxed i2c bus
@ 2022-01-18 20:47       ` Laurent Pinchart
  0 siblings, 0 replies; 22+ messages in thread
From: Laurent Pinchart @ 2022-01-18 20:47 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Jean-Michel Hautbois, Uwe Kleine-König,
	Nicolas Saenz Julienne, Rob Herring, devicetree,
	bcm-kernel-feedback-list, linux-rpi-kernel, linux-arm-kernel,
	Stefan Wahren, Cyril Brulebois, Dave Stevenson, Maxime Ripard

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.

-- 
Regards,

Laurent Pinchart

_______________________________________________
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] 22+ messages in thread

* Re: [PATCH v3] ARM: dts: bcm2711-rpi-cm4-io: Add rtc on a pinctrl-muxed i2c bus
  2022-01-18 20:47       ` Laurent Pinchart
@ 2022-01-18 22:41         ` Uwe Kleine-König
  -1 siblings, 0 replies; 22+ messages in thread
From: Uwe Kleine-König @ 2022-01-18 22:41 UTC (permalink / raw)
  To: Laurent Pinchart, Florian Fainelli
  Cc: Jean-Michel Hautbois, Nicolas Saenz Julienne, Rob Herring,
	devicetree, bcm-kernel-feedback-list, linux-rpi-kernel,
	linux-arm-kernel, Stefan Wahren, Cyril Brulebois, Dave Stevenson,
	Maxime Ripard


[-- Attachment #1.1: Type: text/plain, Size: 1246 bytes --]

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3] ARM: dts: bcm2711-rpi-cm4-io: Add rtc on a pinctrl-muxed i2c bus
@ 2022-01-18 22:41         ` Uwe Kleine-König
  0 siblings, 0 replies; 22+ messages in thread
From: Uwe Kleine-König @ 2022-01-18 22:41 UTC (permalink / raw)
  To: Laurent Pinchart, Florian Fainelli
  Cc: Jean-Michel Hautbois, Nicolas Saenz Julienne, Rob Herring,
	devicetree, 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: 1246 bytes --]

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


[-- 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] 22+ messages in thread

* Re: [PATCH v3] ARM: dts: bcm2711-rpi-cm4-io: Add rtc on a pinctrl-muxed i2c bus
  2022-01-18 22:41         ` Uwe Kleine-König
@ 2022-01-18 22:59           ` Laurent Pinchart
  -1 siblings, 0 replies; 22+ messages in thread
From: Laurent Pinchart @ 2022-01-18 22:59 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Florian Fainelli, Jean-Michel Hautbois, Nicolas Saenz Julienne,
	Rob Herring, devicetree, bcm-kernel-feedback-list,
	linux-rpi-kernel, linux-arm-kernel, Stefan Wahren,
	Cyril Brulebois, Dave Stevenson, Maxime Ripard

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.

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3] ARM: dts: bcm2711-rpi-cm4-io: Add rtc on a pinctrl-muxed i2c bus
@ 2022-01-18 22:59           ` Laurent Pinchart
  0 siblings, 0 replies; 22+ messages in thread
From: Laurent Pinchart @ 2022-01-18 22:59 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Florian Fainelli, Jean-Michel Hautbois, Nicolas Saenz Julienne,
	Rob Herring, devicetree, bcm-kernel-feedback-list,
	linux-rpi-kernel, linux-arm-kernel, Stefan Wahren,
	Cyril Brulebois, Dave Stevenson, Maxime Ripard

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.

-- 
Regards,

Laurent Pinchart

_______________________________________________
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] 22+ messages in thread

* Re: [PATCH v3] ARM: dts: bcm2711-rpi-cm4-io: Add rtc on a pinctrl-muxed i2c bus
  2022-01-18 22:59           ` Laurent Pinchart
@ 2022-01-19  9:44             ` Dave Stevenson
  -1 siblings, 0 replies; 22+ messages in thread
From: Dave Stevenson @ 2022-01-19  9:44 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Uwe Kleine-König, Florian Fainelli, Jean-Michel Hautbois,
	Nicolas Saenz Julienne, Rob Herring, devicetree,
	bcm-kernel-feedback-list, linux-rpi-kernel, linux-arm-kernel,
	Stefan Wahren, Cyril Brulebois, Maxime Ripard

Hi Laurent and Uwe.

On Tue, 18 Jan 2022 at 22:59, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> 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.
>
> --
> Regards,
>
> Laurent Pinchart

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3] ARM: dts: bcm2711-rpi-cm4-io: Add rtc on a pinctrl-muxed i2c bus
@ 2022-01-19  9:44             ` Dave Stevenson
  0 siblings, 0 replies; 22+ messages in thread
From: Dave Stevenson @ 2022-01-19  9:44 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Uwe Kleine-König, Florian Fainelli, Jean-Michel Hautbois,
	Nicolas Saenz Julienne, Rob Herring, devicetree,
	bcm-kernel-feedback-list, linux-rpi-kernel, linux-arm-kernel,
	Stefan Wahren, Cyril Brulebois, Maxime Ripard

Hi Laurent and Uwe.

On Tue, 18 Jan 2022 at 22:59, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> 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.
>
> --
> Regards,
>
> Laurent Pinchart

_______________________________________________
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] 22+ messages in thread

* Re: [PATCH v3] ARM: dts: bcm2711-rpi-cm4-io: Add rtc on a pinctrl-muxed i2c bus
  2022-01-19  9:44             ` Dave Stevenson
@ 2022-02-25  0:55               ` Florian Fainelli
  -1 siblings, 0 replies; 22+ messages in thread
From: Florian Fainelli @ 2022-02-25  0:55 UTC (permalink / raw)
  To: Dave Stevenson, Laurent Pinchart, Uwe Kleine-König
  Cc: Florian Fainelli, Jean-Michel Hautbois, Nicolas Saenz Julienne,
	Rob Herring, devicetree, bcm-kernel-feedback-list,
	linux-rpi-kernel, linux-arm-kernel, Stefan Wahren,
	Cyril Brulebois, Maxime Ripard



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@ideasonboard.com> 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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3] ARM: dts: bcm2711-rpi-cm4-io: Add rtc on a pinctrl-muxed i2c bus
@ 2022-02-25  0:55               ` Florian Fainelli
  0 siblings, 0 replies; 22+ messages in thread
From: Florian Fainelli @ 2022-02-25  0:55 UTC (permalink / raw)
  To: Dave Stevenson, Laurent Pinchart, Uwe Kleine-König
  Cc: Florian Fainelli, Jean-Michel Hautbois, Nicolas Saenz Julienne,
	Rob Herring, devicetree, bcm-kernel-feedback-list,
	linux-rpi-kernel, linux-arm-kernel, Stefan Wahren,
	Cyril Brulebois, Maxime Ripard



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@ideasonboard.com> 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

_______________________________________________
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] 22+ messages in thread

end of thread, other threads:[~2022-02-25  0:57 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-31 11:51 [PATCH v3] ARM: dts: bcm2711-rpi-cm4-io: Add rtc on a pinctrl-muxed i2c bus Uwe Kleine-König
2021-12-31 11:51 ` Uwe Kleine-König
2022-01-02  4:33 ` Cyril Brulebois
2022-01-02  4:33   ` Cyril Brulebois
2022-01-02  6:34   ` Cyril Brulebois
2022-01-02  6:34     ` Cyril Brulebois
2022-01-18 19:45 ` Jean-Michel Hautbois
2022-01-18 19:45   ` Jean-Michel Hautbois
2022-01-18 20:00   ` Florian Fainelli
2022-01-18 20:00     ` Florian Fainelli
2022-01-18 20:02     ` Jean-Michel Hautbois
2022-01-18 20:02       ` Jean-Michel Hautbois
2022-01-18 20:47     ` Laurent Pinchart
2022-01-18 20:47       ` Laurent Pinchart
2022-01-18 22:41       ` Uwe Kleine-König
2022-01-18 22:41         ` Uwe Kleine-König
2022-01-18 22:59         ` Laurent Pinchart
2022-01-18 22:59           ` Laurent Pinchart
2022-01-19  9:44           ` Dave Stevenson
2022-01-19  9:44             ` Dave Stevenson
2022-02-25  0:55             ` Florian Fainelli
2022-02-25  0:55               ` Florian Fainelli

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.