diff mbox series

[v3,10/13] arm64: dts: mediatek: mt7988a-bpi-r4: add gpio leds

Message ID 20250608211452.72920-11-linux@fw-web.de
State New
Headers show
Series further mt7988 devicetree work | expand

Commit Message

Frank Wunderlich June 8, 2025, 9:14 p.m. UTC
From: Frank Wunderlich <frank-w@public-files.de>

Bananapi R4 has a green and a blue led which can be switched by gpio.
Green led is for running state so default on.

Green led also shares pin with eeprom writeprotect where led off allows
writing to eeprom.

Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
---
 .../dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Comments

AngeloGioacchino Del Regno June 11, 2025, 9:33 a.m. UTC | #1
Il 08/06/25 23:14, Frank Wunderlich ha scritto:
> From: Frank Wunderlich <frank-w@public-files.de>
> 
> Bananapi R4 has a green and a blue led which can be switched by gpio.
> Green led is for running state so default on.
> 
> Green led also shares pin with eeprom writeprotect where led off allows
> writing to eeprom.

This is extremely ugly from a hardware perspective and I'm sad to see such
a thing here.

Though, I don't see any other solution, so I can only give you an advice:
don't ever write to the eeprom from the kernel. Things will eventually go
south otherwise.

I would also advise to add a read-only property to the eeprom node.

Regardless (and sadly),

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Frank Wunderlich June 11, 2025, 10:33 a.m. UTC | #2
Hi
> Gesendet: Mittwoch, 11. Juni 2025 um 11:33
> Von: "AngeloGioacchino Del Regno" <angelogioacchino.delregno@collabora.com>
> Betreff: Re: [PATCH v3 10/13] arm64: dts: mediatek: mt7988a-bpi-r4: add gpio leds
>
> Il 08/06/25 23:14, Frank Wunderlich ha scritto:
> > From: Frank Wunderlich <frank-w@public-files.de>
> > 
> > Bananapi R4 has a green and a blue led which can be switched by gpio.
> > Green led is for running state so default on.
> > 
> > Green led also shares pin with eeprom writeprotect where led off allows
> > writing to eeprom.
> 
> This is extremely ugly from a hardware perspective and I'm sad to see such
> a thing here.

i know, but cannot change this...found this out while testing the eeprom...

where uboot allows the writing of eeprom currently without having this pin configured, linux does not and require green-led switch off.

> Though, I don't see any other solution, so I can only give you an advice:
> don't ever write to the eeprom from the kernel. Things will eventually go
> south otherwise.
> 
> I would also advise to add a read-only property to the eeprom node.

the eeprom is empty from factory, but can be used to store e.g. mac-adress which are random by default.

i tested a bit with tlv_eeprom command in uboot and some additional patches to archive this.

imho green-led as running indicator on (=write-protect of eeprom) is a good solution
=> readonly by default, but allowing to write it if user is really sure :)

> Regardless (and sadly),
> 
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Thanks for your opinion and RB

regards Frank
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi b/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi
index 30affedf84d4..21eb91c8609f 100644
--- a/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt7988a-bananapi-bpi-r4.dtsi
@@ -21,6 +21,25 @@  fan: pwm-fan {
 		status = "okay";
 	};
 
+	gpio-leds {
+		compatible = "gpio-leds";
+
+		led_green: led-green {
+			function = LED_FUNCTION_STATUS;
+			color = <LED_COLOR_ID_GREEN>;
+			gpios = <&pio 79 GPIO_ACTIVE_HIGH>;
+			default-state = "on";
+		};
+
+		led_blue: led-blue {
+			function = LED_FUNCTION_WPS;
+			color = <LED_COLOR_ID_BLUE>;
+			gpios = <&pio 63 GPIO_ACTIVE_HIGH>;
+			default-state = "off";
+		};
+	};
+
+
 	reg_1p8v: regulator-1p8v {
 		compatible = "regulator-fixed";
 		regulator-name = "fixed-1.8V";