diff mbox series

[net-next,v7,13/16] ARM: dts: qcom: ipq8064-rb3011: Add Switch LED for each port

Message ID 20230417151738.19426-14-ansuelsmth@gmail.com (mailing list archive)
State Accepted
Commit 09930f1fb8750d5f4e668ab40733ab63cab58ed7
Headers show
Series net: Add basic LED support for switch/phy | expand

Commit Message

Christian Marangi April 17, 2023, 3:17 p.m. UTC
Add Switch LED for each port for MikroTik RB3011UiAS-RM.

MikroTik RB3011UiAS-RM is a 10 port device with 2 qca8337 switch chips
connected.

It was discovered that in the hardware design all 3 Switch LED trace of
the related port is connected to the same LED. This was discovered by
setting to 'always on' the related led in the switch regs and noticing
that all 3 LED for the specific port (for example for port 1) cause the
connected LED for port 1 to turn on. As an extra test we tried enabling
2 different LED for the port resulting in the LED turned off only if
every led in the reg was off.

Aside from this funny and strange hardware implementation, the device
itself have one green LED for each port, resulting in 10 green LED one
for each of the 10 supported port.

Cc: Jonathan McDowell <noodles@earth.li>
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 arch/arm/boot/dts/qcom-ipq8064-rb3011.dts | 120 ++++++++++++++++++++++
 1 file changed, 120 insertions(+)

Comments

Krzysztof Kozlowski April 19, 2023, 12:53 p.m. UTC | #1
On 17/04/2023 17:17, Christian Marangi wrote:
> Add Switch LED for each port for MikroTik RB3011UiAS-RM.
> 
> MikroTik RB3011UiAS-RM is a 10 port device with 2 qca8337 switch chips
> connected.
> 
> It was discovered that in the hardware design all 3 Switch LED trace of
> the related port is connected to the same LED. This was discovered by
> setting to 'always on' the related led in the switch regs and noticing
> that all 3 LED for the specific port (for example for port 1) cause the
> connected LED for port 1 to turn on. As an extra test we tried enabling
> 2 different LED for the port resulting in the LED turned off only if
> every led in the reg was off.
> 
> Aside from this funny and strange hardware implementation, the device
> itself have one green LED for each port, resulting in 10 green LED one
> for each of the 10 supported port.
> 
> Cc: Jonathan McDowell <noodles@earth.li>
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>  arch/arm/boot/dts/qcom-ipq8064-rb3011.dts | 120 ++++++++++++++++++++++

Please do not send the DTS patches to the net-next, but to the Qualcomm
SoC maintainers. The DTS must not be mixed with driver code.

Best regards,
Krzysztof
Christian Marangi April 19, 2023, 2:38 p.m. UTC | #2
On Wed, Apr 19, 2023 at 02:53:45PM +0200, Krzysztof Kozlowski wrote:
> On 17/04/2023 17:17, Christian Marangi wrote:
> > Add Switch LED for each port for MikroTik RB3011UiAS-RM.
> > 
> > MikroTik RB3011UiAS-RM is a 10 port device with 2 qca8337 switch chips
> > connected.
> > 
> > It was discovered that in the hardware design all 3 Switch LED trace of
> > the related port is connected to the same LED. This was discovered by
> > setting to 'always on' the related led in the switch regs and noticing
> > that all 3 LED for the specific port (for example for port 1) cause the
> > connected LED for port 1 to turn on. As an extra test we tried enabling
> > 2 different LED for the port resulting in the LED turned off only if
> > every led in the reg was off.
> > 
> > Aside from this funny and strange hardware implementation, the device
> > itself have one green LED for each port, resulting in 10 green LED one
> > for each of the 10 supported port.
> > 
> > Cc: Jonathan McDowell <noodles@earth.li>
> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > ---
> >  arch/arm/boot/dts/qcom-ipq8064-rb3011.dts | 120 ++++++++++++++++++++++
> 
> Please do not send the DTS patches to the net-next, but to the Qualcomm
> SoC maintainers. The DTS must not be mixed with driver code.
> 

Hi,

sorry for the mess, it was asked to give an user of the LED feature for
qca8k so I was a bit confused on where to include it and at the end I
decided to put it in this series.

What was the correct way? 2 different series and reference the DT one in
the net-next? (or not targetting net-next at all?)
Krzysztof Kozlowski April 19, 2023, 6:15 p.m. UTC | #3
On 19/04/2023 16:38, Christian Marangi wrote:
> On Wed, Apr 19, 2023 at 02:53:45PM +0200, Krzysztof Kozlowski wrote:
>> On 17/04/2023 17:17, Christian Marangi wrote:
>>> Add Switch LED for each port for MikroTik RB3011UiAS-RM.
>>>
>>> MikroTik RB3011UiAS-RM is a 10 port device with 2 qca8337 switch chips
>>> connected.
>>>
>>> It was discovered that in the hardware design all 3 Switch LED trace of
>>> the related port is connected to the same LED. This was discovered by
>>> setting to 'always on' the related led in the switch regs and noticing
>>> that all 3 LED for the specific port (for example for port 1) cause the
>>> connected LED for port 1 to turn on. As an extra test we tried enabling
>>> 2 different LED for the port resulting in the LED turned off only if
>>> every led in the reg was off.
>>>
>>> Aside from this funny and strange hardware implementation, the device
>>> itself have one green LED for each port, resulting in 10 green LED one
>>> for each of the 10 supported port.
>>>
>>> Cc: Jonathan McDowell <noodles@earth.li>
>>> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
>>> ---
>>>  arch/arm/boot/dts/qcom-ipq8064-rb3011.dts | 120 ++++++++++++++++++++++
>>
>> Please do not send the DTS patches to the net-next, but to the Qualcomm
>> SoC maintainers. The DTS must not be mixed with driver code.
>>
> 
> Hi,
> 
> sorry for the mess, it was asked to give an user of the LED feature for
> qca8k so I was a bit confused on where to include it and at the end I
> decided to put it in this series.
> 
> What was the correct way? 2 different series and reference the DT one in
> the net-next? (or not targetting net-next at all?)

Send a separate patchset and provide a lore URL to it in the bindings
(or vice versa).

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/qcom-ipq8064-rb3011.dts b/arch/arm/boot/dts/qcom-ipq8064-rb3011.dts
index 47a5d1849c72..4d509876294b 100644
--- a/arch/arm/boot/dts/qcom-ipq8064-rb3011.dts
+++ b/arch/arm/boot/dts/qcom-ipq8064-rb3011.dts
@@ -65,26 +65,86 @@  fixed-link {
 				port@1 {
 					reg = <1>;
 					label = "sw1";
+
+					leds {
+						#address-cells = <1>;
+						#size-cells = <0>;
+
+						led@0 {
+							reg = <0>;
+							color = <LED_COLOR_ID_GREEN>;
+							function = LED_FUNCTION_LAN;
+							default-state = "keep";
+						};
+					};
 				};
 
 				port@2 {
 					reg = <2>;
 					label = "sw2";
+
+					leds {
+						#address-cells = <1>;
+						#size-cells = <0>;
+
+						led@0 {
+							reg = <0>;
+							color = <LED_COLOR_ID_GREEN>;
+							function = LED_FUNCTION_LAN;
+							default-state = "keep";
+						};
+					};
 				};
 
 				port@3 {
 					reg = <3>;
 					label = "sw3";
+
+					leds {
+						#address-cells = <1>;
+						#size-cells = <0>;
+
+						led@0 {
+							reg = <0>;
+							color = <LED_COLOR_ID_GREEN>;
+							function = LED_FUNCTION_LAN;
+							default-state = "keep";
+						};
+					};
 				};
 
 				port@4 {
 					reg = <4>;
 					label = "sw4";
+
+					leds {
+						#address-cells = <1>;
+						#size-cells = <0>;
+
+						led@0 {
+							reg = <0>;
+							color = <LED_COLOR_ID_GREEN>;
+							function = LED_FUNCTION_LAN;
+							default-state = "keep";
+						};
+					};
 				};
 
 				port@5 {
 					reg = <5>;
 					label = "sw5";
+
+					leds {
+						#address-cells = <1>;
+						#size-cells = <0>;
+
+						led@0 {
+							reg = <0>;
+							color = <LED_COLOR_ID_GREEN>;
+							function = LED_FUNCTION_LAN;
+							default-state = "keep";
+						};
+					};
 				};
 			};
 		};
@@ -130,26 +190,86 @@  fixed-link {
 				port@1 {
 					reg = <1>;
 					label = "sw6";
+
+					leds {
+						#address-cells = <1>;
+						#size-cells = <0>;
+
+						led@0 {
+							reg = <0>;
+							color = <LED_COLOR_ID_GREEN>;
+							function = LED_FUNCTION_LAN;
+							default-state = "keep";
+						};
+					};
 				};
 
 				port@2 {
 					reg = <2>;
 					label = "sw7";
+
+					leds {
+						#address-cells = <1>;
+						#size-cells = <0>;
+
+						led@0 {
+							reg = <0>;
+							color = <LED_COLOR_ID_GREEN>;
+							function = LED_FUNCTION_LAN;
+							default-state = "keep";
+						};
+					};
 				};
 
 				port@3 {
 					reg = <3>;
 					label = "sw8";
+
+					leds {
+						#address-cells = <1>;
+						#size-cells = <0>;
+
+						led@0 {
+							reg = <0>;
+							color = <LED_COLOR_ID_GREEN>;
+							function = LED_FUNCTION_LAN;
+							default-state = "keep";
+						};
+					};
 				};
 
 				port@4 {
 					reg = <4>;
 					label = "sw9";
+
+					leds {
+						#address-cells = <1>;
+						#size-cells = <0>;
+
+						led@0 {
+							reg = <0>;
+							color = <LED_COLOR_ID_GREEN>;
+							function = LED_FUNCTION_LAN;
+							default-state = "keep";
+						};
+					};
 				};
 
 				port@5 {
 					reg = <5>;
 					label = "sw10";
+
+					leds {
+						#address-cells = <1>;
+						#size-cells = <0>;
+
+						led@0 {
+							reg = <0>;
+							color = <LED_COLOR_ID_GREEN>;
+							function = LED_FUNCTION_LAN;
+							default-state = "keep";
+						};
+					};
 				};
 			};
 		};