[PATCH] arm: dts: bcm2711: Describe Ethernet LEDs

[PATCH] arm: dts: bcm2711: Describe Ethernet LEDs

[Florian Fainelli]

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

Describe the Ethernet LEDs for the Raspberry Pi 4 model B board as well
as the Raspberry Pi 4 CM board.

Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
---
 arch/arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts | 18 ++++++++++++++++++
 .../arm/boot/dts/broadcom/bcm2711-rpi-cm4.dtsi | 18 ++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/arch/arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts
index d5f8823230db..41db78cb0836 100644
--- a/arch/arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts
+++ b/arch/arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts
@@ -5,6 +5,7 @@
 #include "bcm283x-rpi-led-deprecated.dtsi"
 #include "bcm283x-rpi-usb-peripheral.dtsi"
 #include "bcm283x-rpi-wifi-bt.dtsi"
+#include <dt-bindings/leds/common.h>
 
 / {
 	compatible = "raspberrypi,4-model-b", "brcm,bcm2711";
@@ -197,6 +198,23 @@ &genet_mdio {
 	phy1: ethernet-phy@1 {
 		/* No PHY interrupt */
 		reg = <0x1>;
+
+		leds {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			leds@0 {
+				color = <LED_COLOR_ID_GREEN>;
+				default-state = "keep";
+				reg = <0>;
+			};
+
+			leds@1 {
+				color = <LED_COLOR_ID_AMBER>;
+				default-state = "keep";
+				reg = <1>;
+			};
+		};
 	};
 };
 
diff --git a/arch/arm/boot/dts/broadcom/bcm2711-rpi-cm4.dtsi b/arch/arm/boot/dts/broadcom/bcm2711-rpi-cm4.dtsi
index 48e63ab7848c..3860a134d31a 100644
--- a/arch/arm/boot/dts/broadcom/bcm2711-rpi-cm4.dtsi
+++ b/arch/arm/boot/dts/broadcom/bcm2711-rpi-cm4.dtsi
@@ -3,6 +3,7 @@
 #include "bcm2711.dtsi"
 #include "bcm2711-rpi.dtsi"
 #include "bcm283x-rpi-wifi-bt.dtsi"
+#include <dt-bindings/leds/common.h>
 
 / {
 	compatible = "raspberrypi,4-compute-module", "brcm,bcm2711";
@@ -91,6 +92,23 @@ &genet_mdio {
 	phy1: ethernet-phy@0 {
 		/* No PHY interrupt */
 		reg = <0x0>;
+
+		leds {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			leds@1 {
+				color = <LED_COLOR_ID_GREEN>;
+				default-state = "keep";
+				reg = <1>;
+			};
+
+			leds@2 {
+				color = <LED_COLOR_ID_AMBER>;
+				default-state = "keep";
+				reg = <2>;
+			};
+		};
 	};
 };
 
-- 
2.34.1


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]

Re: [PATCH] arm: dts: bcm2711: Describe Ethernet LEDs

[Andrew Lunn]

On Mon, Aug 21, 2023 at 12:27:11PM -0700, Florian Fainelli wrote:
> Describe the Ethernet LEDs for the Raspberry Pi 4 model B board as well
> as the Raspberry Pi 4 CM board.
> 
> Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
> ---
>  arch/arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts | 18 ++++++++++++++++++
>  .../arm/boot/dts/broadcom/bcm2711-rpi-cm4.dtsi | 18 ++++++++++++++++++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts
> index d5f8823230db..41db78cb0836 100644
> --- a/arch/arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts
> +++ b/arch/arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts
> @@ -5,6 +5,7 @@
>  #include "bcm283x-rpi-led-deprecated.dtsi"
>  #include "bcm283x-rpi-usb-peripheral.dtsi"
>  #include "bcm283x-rpi-wifi-bt.dtsi"
> +#include <dt-bindings/leds/common.h>
>  
>  / {
>  	compatible = "raspberrypi,4-model-b", "brcm,bcm2711";
> @@ -197,6 +198,23 @@ &genet_mdio {
>  	phy1: ethernet-phy@1 {
>  		/* No PHY interrupt */
>  		reg = <0x1>;
> +
> +		leds {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			leds@0 {
> +				color = <LED_COLOR_ID_GREEN>;
> +				default-state = "keep";
> +				reg = <0>;
> +			};
> +
> +			leds@1 {
> +				color = <LED_COLOR_ID_AMBER>;
> +				default-state = "keep";
> +				reg = <1>;

Hi Florian

When we submitted leds in DT nodes as part of the merge, Rob wanted
the function property set. armada-370-rd.dts has:

function = LED_FUNCTION_WAN

because that port would typically be connected to your cable router
etc. But LED_FUNCTION_LAN also exists. This property can influence
naming so adding it later could be an ABI violation.

     Andrew

Re: [PATCH] arm: dts: bcm2711: Describe Ethernet LEDs

[Stefan Wahren]

Hi Florian,

Am 21.08.23 um 21:27 schrieb Florian Fainelli:
> Describe the Ethernet LEDs for the Raspberry Pi 4 model B board as well
> as the Raspberry Pi 4 CM board.
>
> Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
> ---
>   arch/arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts | 18 ++++++++++++++++++
>   .../arm/boot/dts/broadcom/bcm2711-rpi-cm4.dtsi | 18 ++++++++++++++++++
>   2 files changed, 36 insertions(+)
>
> diff --git a/arch/arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts
> index d5f8823230db..41db78cb0836 100644
> --- a/arch/arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts
> +++ b/arch/arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts
> @@ -5,6 +5,7 @@
>   #include "bcm283x-rpi-led-deprecated.dtsi"
>   #include "bcm283x-rpi-usb-peripheral.dtsi"
>   #include "bcm283x-rpi-wifi-bt.dtsi"
> +#include <dt-bindings/leds/common.h>
>
>   / {
>   	compatible = "raspberrypi,4-model-b", "brcm,bcm2711";
> @@ -197,6 +198,23 @@ &genet_mdio {
>   	phy1: ethernet-phy@1 {
>   		/* No PHY interrupt */
>   		reg = <0x1>;
> +
> +		leds {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			leds@0 {
> +				color = <LED_COLOR_ID_GREEN>;
> +				default-state = "keep";
> +				reg = <0>;
> +			};
> +
> +			leds@1 {
> +				color = <LED_COLOR_ID_AMBER>;
> +				default-state = "keep";
> +				reg = <1>;
> +			};
> +		};
>   	};
>   };
>
> diff --git a/arch/arm/boot/dts/broadcom/bcm2711-rpi-cm4.dtsi b/arch/arm/boot/dts/broadcom/bcm2711-rpi-cm4.dtsi
> index 48e63ab7848c..3860a134d31a 100644
> --- a/arch/arm/boot/dts/broadcom/bcm2711-rpi-cm4.dtsi
> +++ b/arch/arm/boot/dts/broadcom/bcm2711-rpi-cm4.dtsi
> @@ -3,6 +3,7 @@
>   #include "bcm2711.dtsi"
>   #include "bcm2711-rpi.dtsi"
>   #include "bcm283x-rpi-wifi-bt.dtsi"
> +#include <dt-bindings/leds/common.h>
>
>   / {
>   	compatible = "raspberrypi,4-compute-module", "brcm,bcm2711";
> @@ -91,6 +92,23 @@ &genet_mdio {
>   	phy1: ethernet-phy@0 {
>   		/* No PHY interrupt */
>   		reg = <0x0>;
> +
> +		leds {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			leds@1 {
> +				color = <LED_COLOR_ID_GREEN>;
> +				default-state = "keep";
> +				reg = <1>;
> +			};
> +
> +			leds@2 {
> +				color = <LED_COLOR_ID_AMBER>;
> +				default-state = "keep";
> +				reg = <2>;
> +			};
> +		};

looking at my hardware setup, i see that the Ethernet PHY is located on
the compute module, but the Ethernet jack including the LEDs is located
on the IO board.

Maybe we should add the LEDs to the IO board dts?

Best regards

>   	};
>   };
>

Re: [PATCH] arm: dts: bcm2711: Describe Ethernet LEDs

[Stefan Wahren]

Hi Florian,

Am 21.08.23 um 21:27 schrieb Florian Fainelli:
> Describe the Ethernet LEDs for the Raspberry Pi 4 model B board as well
> as the Raspberry Pi 4 CM board.
>
> Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
> ---
>   arch/arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts | 18 ++++++++++++++++++
>   .../arm/boot/dts/broadcom/bcm2711-rpi-cm4.dtsi | 18 ++++++++++++++++++
>   2 files changed, 36 insertions(+)
>
> diff --git a/arch/arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts
> index d5f8823230db..41db78cb0836 100644
> --- a/arch/arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts
> +++ b/arch/arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts
> @@ -5,6 +5,7 @@
>   #include "bcm283x-rpi-led-deprecated.dtsi"
>   #include "bcm283x-rpi-usb-peripheral.dtsi"
>   #include "bcm283x-rpi-wifi-bt.dtsi"
> +#include <dt-bindings/leds/common.h>
>
>   / {
>   	compatible = "raspberrypi,4-model-b", "brcm,bcm2711";
> @@ -197,6 +198,23 @@ &genet_mdio {
>   	phy1: ethernet-phy@1 {
>   		/* No PHY interrupt */
>   		reg = <0x1>;
> +
> +		leds {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			leds@0 {
> +				color = <LED_COLOR_ID_GREEN>;
> +				default-state = "keep";
> +				reg = <0>;
> +			};
> +
> +			leds@1 {
> +				color = <LED_COLOR_ID_AMBER>;
> +				default-state = "keep";
> +				reg = <1>;
> +			};
> +		};

the Raspberry Pi 400 Ethernet Jack doesn't have LEDs, but the
bcm2711-rpi-400.dts does include bcm2711-rpi-4.dtb. So this complete new
node must be deleted there.

Best regards

>   	};
>   };

Re: [PATCH] arm: dts: bcm2711: Describe Ethernet LEDs

[Florian Fainelli]

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

On 8/22/23 06:18, Andrew Lunn wrote:
> On Mon, Aug 21, 2023 at 12:27:11PM -0700, Florian Fainelli wrote:
>> Describe the Ethernet LEDs for the Raspberry Pi 4 model B board as well
>> as the Raspberry Pi 4 CM board.
>>
>> Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
>> ---
>>   arch/arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts | 18 ++++++++++++++++++
>>   .../arm/boot/dts/broadcom/bcm2711-rpi-cm4.dtsi | 18 ++++++++++++++++++
>>   2 files changed, 36 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts b/arch/arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts
>> index d5f8823230db..41db78cb0836 100644
>> --- a/arch/arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts
>> +++ b/arch/arm/boot/dts/broadcom/bcm2711-rpi-4-b.dts
>> @@ -5,6 +5,7 @@
>>   #include "bcm283x-rpi-led-deprecated.dtsi"
>>   #include "bcm283x-rpi-usb-peripheral.dtsi"
>>   #include "bcm283x-rpi-wifi-bt.dtsi"
>> +#include <dt-bindings/leds/common.h>
>>   
>>   / {
>>   	compatible = "raspberrypi,4-model-b", "brcm,bcm2711";
>> @@ -197,6 +198,23 @@ &genet_mdio {
>>   	phy1: ethernet-phy@1 {
>>   		/* No PHY interrupt */
>>   		reg = <0x1>;
>> +
>> +		leds {
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +
>> +			leds@0 {
>> +				color = <LED_COLOR_ID_GREEN>;
>> +				default-state = "keep";
>> +				reg = <0>;
>> +			};
>> +
>> +			leds@1 {
>> +				color = <LED_COLOR_ID_AMBER>;
>> +				default-state = "keep";
>> +				reg = <1>;
> 
> Hi Florian
> 
> When we submitted leds in DT nodes as part of the merge, Rob wanted
> the function property set. armada-370-rd.dts has:
> 
> function = LED_FUNCTION_WAN
> 
> because that port would typically be connected to your cable router
> etc. But LED_FUNCTION_LAN also exists. This property can influence
> naming so adding it later could be an ABI violation.

So in this case, the amber LED indicates the link activity, and the 
green LED indicates the link status. AFAICT we still do not have 
function names defined for those, the closest I can think is to do:

function = LED_FUNCTION_ACTIVITY for the amber LED

and for the green LED:

function = (LED_FUNCTION_RX | LED_FUNCTION_TX)

is that acceptable?
-- 
Florian


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]

Re: [PATCH] arm: dts: bcm2711: Describe Ethernet LEDs

[Andrew Lunn]

> So in this case, the amber LED indicates the link activity, and the green
> LED indicates the link status. AFAICT we still do not have function names
> defined for those, the closest I can think is to do:
> 
> function = LED_FUNCTION_ACTIVITY for the amber LED
> 
> and for the green LED:
> 
> function = (LED_FUNCTION_RX | LED_FUNCTION_TX)
> 
> is that acceptable?

I have a WIP DT binding for exact meaning of the LED.

https://github.com/lunn/linux v6.5-rc4-net-next-led-bindings

It is too late for this merge window, so i will likely post it as an
RFC in a weeks time.

We probably need a discussion, LED_FUNCTION_LAN gives the high level
description, and contributes to the naming, and then this binding
gives the specific meaning of the LED? Or do we want to define
LED_FUNCTION_* for details?

	Andrew