diff mbox series

[v7,8/9] ARM: dts: turris-omnia: Add MCU system-controller node

Message ID 20240424173809.7214-9-kabel@kernel.org (mailing list archive)
State Superseded
Headers show
Series Turris Omnia MCU driver | expand

Commit Message

Marek Behún April 24, 2024, 5:38 p.m. UTC
Turris Omnia's MCU provides various features that can be configured over
I2C at address 0x2a. Add device-tree node.

Fixes: 26ca8b52d6e1 ("ARM: dts: add support for Turris Omnia")
Signed-off-by: Marek Behún <kabel@kernel.org>
---
 .../dts/marvell/armada-385-turris-omnia.dts   | 22 ++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

Comments

Gregory CLEMENT April 26, 2024, 4:01 p.m. UTC | #1
Hello Marek,

> Turris Omnia's MCU provides various features that can be configured over
> I2C at address 0x2a. Add device-tree node.
>
> Fixes: 26ca8b52d6e1 ("ARM: dts: add support for Turris Omnia")

Is it really a fix ?

It seems that the node you add depends on the firmware support that was
not yet added.

Gregory

> Signed-off-by: Marek Behún <kabel@kernel.org>
> ---
>  .../dts/marvell/armada-385-turris-omnia.dts   | 22 ++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/marvell/armada-385-turris-omnia.dts b/arch/arm/boot/dts/marvell/armada-385-turris-omnia.dts
> index 7b755bb4e4e7..59079d63fe27 100644
> --- a/arch/arm/boot/dts/marvell/armada-385-turris-omnia.dts
> +++ b/arch/arm/boot/dts/marvell/armada-385-turris-omnia.dts
> @@ -218,7 +218,22 @@ i2c@0 {
>  			#size-cells = <0>;
>  			reg = <0>;
>  
> -			/* STM32F0 command interface at address 0x2a */
> +			mcu: system-controller@2a {
> +				compatible = "cznic,turris-omnia-mcu";
> +				reg = <0x2a>;
> +
> +				pinctrl-names = "default";
> +				pinctrl-0 = <&mcu_pins>;
> +
> +				interrupt-parent = <&gpio1>;
> +				interrupts = <11 IRQ_TYPE_NONE>;
> +
> +				gpio-controller;
> +				#gpio-cells = <3>;
> +
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +			};
>  
>  			led-controller@2b {
>  				compatible = "cznic,turris-omnia-leds";
> @@ -501,6 +516,11 @@ fixed-link {
>  };
>  
>  &pinctrl {
> +	mcu_pins: mcu-pins {
> +		marvell,pins = "mpp43";
> +		marvell,function = "gpio";
> +	};
> +
>  	pcawan_pins: pcawan-pins {
>  		marvell,pins = "mpp46";
>  		marvell,function = "gpio";
> -- 
> 2.43.2
Marek Behún April 26, 2024, 4:40 p.m. UTC | #2
On Fri, 26 Apr 2024 18:01:20 +0200
Gregory CLEMENT <gregory.clement@bootlin.com> wrote:

> Hello Marek,
> 
> > Turris Omnia's MCU provides various features that can be configured over
> > I2C at address 0x2a. Add device-tree node.
> >
> > Fixes: 26ca8b52d6e1 ("ARM: dts: add support for Turris Omnia")  
> 
> Is it really a fix ?
> 
> It seems that the node you add depends on the firmware support that was
> not yet added.
> 
> Gregory

It is a fix in the sense that Omnia always had the MCU, and so
device-tree without the MCU node was incorrect.

Since device-trees should be (if possible) backwards and forwards
compatible, this should not be a problem.

But I am guessing it might be because of what new U-Boot does: if it
finds the MCU node in the device-tree, it will add a reset gpio property
for ethernet PHY, linking it to the MCU node. So if this
patch got backported to older kernel where the MCU driver is not
present, and the board had this new U-Boot, the kernel would not create
the eth2 interface, because it would be waiting for a driver providing
the GPIO.

So hmm, maybe I should drop the Fixes tag.
Andrew Lunn April 26, 2024, 6:21 p.m. UTC | #3
> But I am guessing it might be because of what new U-Boot does: if it
> finds the MCU node in the device-tree, it will add a reset gpio property
> for ethernet PHY, linking it to the MCU node. So if this
> patch got backported to older kernel where the MCU driver is not
> present, and the board had this new U-Boot, the kernel would not create
> the eth2 interface, because it would be waiting for a driver providing
> the GPIO.

You should make this clear in the commit message. There is a danger
the ML algorithm which picks patches for back porting decides this is
a candidate. Hopefully a human will read the commit message and then
reject it.

       Andrew
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/marvell/armada-385-turris-omnia.dts b/arch/arm/boot/dts/marvell/armada-385-turris-omnia.dts
index 7b755bb4e4e7..59079d63fe27 100644
--- a/arch/arm/boot/dts/marvell/armada-385-turris-omnia.dts
+++ b/arch/arm/boot/dts/marvell/armada-385-turris-omnia.dts
@@ -218,7 +218,22 @@  i2c@0 {
 			#size-cells = <0>;
 			reg = <0>;
 
-			/* STM32F0 command interface at address 0x2a */
+			mcu: system-controller@2a {
+				compatible = "cznic,turris-omnia-mcu";
+				reg = <0x2a>;
+
+				pinctrl-names = "default";
+				pinctrl-0 = <&mcu_pins>;
+
+				interrupt-parent = <&gpio1>;
+				interrupts = <11 IRQ_TYPE_NONE>;
+
+				gpio-controller;
+				#gpio-cells = <3>;
+
+				interrupt-controller;
+				#interrupt-cells = <2>;
+			};
 
 			led-controller@2b {
 				compatible = "cznic,turris-omnia-leds";
@@ -501,6 +516,11 @@  fixed-link {
 };
 
 &pinctrl {
+	mcu_pins: mcu-pins {
+		marvell,pins = "mpp43";
+		marvell,function = "gpio";
+	};
+
 	pcawan_pins: pcawan-pins {
 		marvell,pins = "mpp46";
 		marvell,function = "gpio";