Hi,
On Mon, Mar 27, 2023 at 07:29:39AM +0200, open...@aiyionpri.me wrote:
> From: Jan-Niklas Burfeind <g...@aiyionpri.me>
>
> this is similar to
> commit 583ac0e11df7 ("mpc85xx: update lp5521 led-controller node for
> 5.10")
>
> Google uses white for running and red for an issue
>
> Signed-off-by: Jan-Niklas Burfeind <g...@aiyionpri.me>
> ---
> Good morning.
>
> Currently the tristate LED does not have an assigned function,
> while it does have several in stock firmware.
>
> Once the multi-intensity section becomes active, the device should use
> white/blueish light as running indicator.
> For now blue is used for LED_RUNNING.
>
> If there's a way to use white instead of blue, just let me know;
> I'd gladly fix that at once.
I'll admit, I wasn't familiar with these led-* aliases (and the custom
package/base-files/files/lib/functions/leds.sh that parses them) until
now. But it looks like they only support a single LED for a given
function, so while we might like to enable the red, blue and green (to
get white), that doesn't seem possible without switching over the to
full multi-color LED device tree binding. But then, I don't think LUCI
knows how to configure multi-color LEDs yet.
I guess the choices you made here are better than the "nothing" that is
currently here, so:
Reviewed-by: Brian Norris <computersforpe...@gmail.com>
One more note below:
> --- a/target/linux/ipq40xx/files/arch/arm/boot/dts/qcom-ipq4019-wifi.dts
> +++ b/target/linux/ipq40xx/files/arch/arm/boot/dts/qcom-ipq4019-wifi.dts
> @@ -255,12 +259,13 @@
> clock-mode = /bits/ 8 <1>;
>
> #if 1
> - led@0 {
> + led0_red: led@0 {
> reg = <0>;
> chan-name = "LED0_Red";
> led-cur = /bits/ 8 <0x64>;
> max-cur = /bits/ 8 <0x78>;
> color = <LED_COLOR_ID_RED>;
> + function = LED_FUNCTION_FAULT;
Are you sure this 'function' property is actually doing anything? I know
the common DT bindings provide this, but I'm pretty sure the lp5523
driver doesn't actually use it for anything -- it derives its LED names
from either 'chan-name' or as a combination of a few other properties
('label', channel number, etc.).
I guess it's not wrong, per se, to include it, but I did purposely only
specify the properties that made sense for this driver.
Same comment applies to the other 'function' uses.
Brian
> };
>
> led@1 {
> @@ -271,12 +276,13 @@
> color = <LED_COLOR_ID_GREEN>;
> };
>
> - led@2 {
> + led0_blue: led@2 {
> reg = <2>;
> chan-name = "LED0_Blue";
> led-cur = /bits/ 8 <0x64>;
> max-cur = /bits/ 8 <0x78>;
> color = <LED_COLOR_ID_BLUE>;
> + function = LED_FUNCTION_POWER;
> };
> #else
> /*
> @@ -285,6 +291,7 @@
> * # echo 255 > /sys/class/leds/tricolor/brightness
> */
> multi-led@2 {
> + function = LED_FUNCTION_POWER;
> reg = <2>;
> color = <LED_COLOR_ID_RGB>;
> #address-cells = <1>;
> --
> 2.40.0
>
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
