[PATCH v2 1/2] dt-bindings: leds: add LED_FUNCTION for wlan2g/wlan5g

[PATCH v2 1/2] dt-bindings: leds: add LED_FUNCTION for wlan2g/wlan5g

[Adrian Schmutzler]

Many consumer "routers" have dedicated LEDs for specific WiFi bands,
e.g. one for 2.4 GHz and one for 5 GHz. These LEDs specifically
indicate the state of the relevant band, so the latter should be
included in the function name. LED_FUNCTION_WLAN will remain for
general cases or when the LED is used for more than one band.

This essentially is equivalent to how we use LED_FUNCTION_LAN and
LED_FUNCTION_WAN instead of just having LED_FUNCTION_ETHERNET.

Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>

---

Changes in v2:
- Without typo this time. Sorry.
---
 include/dt-bindings/leds/common.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h
index 52b619d44ba2..debbd406ff17 100644
--- a/include/dt-bindings/leds/common.h
+++ b/include/dt-bindings/leds/common.h
@@ -89,6 +89,8 @@
 #define LED_FUNCTION_USB "usb"
 #define LED_FUNCTION_WAN "wan"
 #define LED_FUNCTION_WLAN "wlan"
+#define LED_FUNCTION_WLAN2G "wlan2g"
+#define LED_FUNCTION_WLAN5G "wlan5g"
 #define LED_FUNCTION_WPS "wps"
 
 #endif /* __DT_BINDINGS_LEDS_H */
-- 
2.20.1

[PATCH v2 2/2] dt-bindings: leds: add LED_FUNCTION_RSSI

[Adrian Schmutzler]

Several consumer "routers" and CPE devices have dedicated LEDs to
show the received signal strength indicator (RSSI). This is
different from the "WLAN" LEDs that just show enabled/disabled
state and sometimes rx/tx activity.

Add a LED function for these LEDs.

Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>

---

Changes in v2:
none
---
 include/dt-bindings/leds/common.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h
index debbd406ff17..c4821a44e422 100644
--- a/include/dt-bindings/leds/common.h
+++ b/include/dt-bindings/leds/common.h
@@ -81,6 +81,7 @@
 #define LED_FUNCTION_MTD "mtd"
 #define LED_FUNCTION_PANIC "panic"
 #define LED_FUNCTION_PROGRAMMING "programming"
+#define LED_FUNCTION_RSSI "rssi"
 #define LED_FUNCTION_RX "rx"
 #define LED_FUNCTION_SD "sd"
 #define LED_FUNCTION_STANDBY "standby"
-- 
2.20.1

RE: [PATCH v2 1/2] dt-bindings: leds: add LED_FUNCTION for wlan2g/wlan5g

[Adrian Schmutzler]

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

> -----Original Message-----
> From: Adrian Schmutzler [mailto:freifunk@adrianschmutzler.de]
> Sent: Samstag, 19. September 2020 19:28
> To: Rob Herring <robh+dt@kernel.org>; Pavel Machek <pavel@ucw.cz>; Dan
> Murphy <dmurphy@ti.com>; Linus Walleij <linus.walleij@linaro.org>;
> devicetree@vger.kernel.org; Adrian Schmutzler
> <freifunk@adrianschmutzler.de>
> Subject: [PATCH v2 1/2] dt-bindings: leds: add LED_FUNCTION for
> wlan2g/wlan5g
> 
> Many consumer "routers" have dedicated LEDs for specific WiFi bands, e.g.
> one for 2.4 GHz and one for 5 GHz. These LEDs specifically indicate the state
> of the relevant band, so the latter should be included in the function name.
> LED_FUNCTION_WLAN will remain for general cases or when the LED is used
> for more than one band.
> 
> This essentially is equivalent to how we use LED_FUNCTION_LAN and
> LED_FUNCTION_WAN instead of just having LED_FUNCTION_ETHERNET.

I only just became aware of the linux-leds@vger.kernel.org mailing list, and resubmitted there.

Sorry for the noise. Maybe get_maintainers.sh should be updated for this file.

Best

Adrian

> 
> Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
> 
> ---
> 
> Changes in v2:
> - Without typo this time. Sorry.
> ---
>  include/dt-bindings/leds/common.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/dt-bindings/leds/common.h b/include/dt-
> bindings/leds/common.h
> index 52b619d44ba2..debbd406ff17 100644
> --- a/include/dt-bindings/leds/common.h
> +++ b/include/dt-bindings/leds/common.h
> @@ -89,6 +89,8 @@
>  #define LED_FUNCTION_USB "usb"
>  #define LED_FUNCTION_WAN "wan"
>  #define LED_FUNCTION_WLAN "wlan"
> +#define LED_FUNCTION_WLAN2G "wlan2g"
> +#define LED_FUNCTION_WLAN5G "wlan5g"
>  #define LED_FUNCTION_WPS "wps"
> 
>  #endif /* __DT_BINDINGS_LEDS_H */
> --
> 2.20.1

[-- Attachment #2: openpgp-digital-signature.asc --]
[-- Type: application/pgp-signature, Size: 834 bytes --]

Re: [PATCH v2 1/2] dt-bindings: leds: add LED_FUNCTION for wlan2g/wlan5g

[Rob Herring]

On Sat, Sep 19, 2020 at 11:45:18PM +0200, Adrian Schmutzler wrote:
> > -----Original Message-----
> > From: Adrian Schmutzler [mailto:freifunk@adrianschmutzler.de]
> > Sent: Samstag, 19. September 2020 19:28
> > To: Rob Herring <robh+dt@kernel.org>; Pavel Machek <pavel@ucw.cz>; Dan
> > Murphy <dmurphy@ti.com>; Linus Walleij <linus.walleij@linaro.org>;
> > devicetree@vger.kernel.org; Adrian Schmutzler
> > <freifunk@adrianschmutzler.de>
> > Subject: [PATCH v2 1/2] dt-bindings: leds: add LED_FUNCTION for
> > wlan2g/wlan5g
> > 
> > Many consumer "routers" have dedicated LEDs for specific WiFi bands, e.g.
> > one for 2.4 GHz and one for 5 GHz. These LEDs specifically indicate the state
> > of the relevant band, so the latter should be included in the function name.
> > LED_FUNCTION_WLAN will remain for general cases or when the LED is used
> > for more than one band.
> > 
> > This essentially is equivalent to how we use LED_FUNCTION_LAN and
> > LED_FUNCTION_WAN instead of just having LED_FUNCTION_ETHERNET.
> 
> I only just became aware of the linux-leds@vger.kernel.org mailing list, and resubmitted there.

It should both there and the DT list.

> Sorry for the noise. Maybe get_maintainers.sh should be updated for this file.

Patches welcome.

Rob

Re: [PATCH v2 1/2] dt-bindings: leds: add LED_FUNCTION for wlan2g/wlan5g

[Pavel Machek]

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

Hi!

> > >>> In fact the function should not be "wlan" (nor "wlan2g" or "wlan5g", but
> > >>> "activity".  
> > 
> > I disagree. Activity should be reserved for the activity trigger.
> > I've had a patch [0] documenting standard LED functions, but it
> > eventually didn't make to the mainline, I'll try to send an update.
> 
> Hmm. The thing is that activity is sometimes interpreted as the union
> of rx and tx, or read and write. I think the pair (device,function)
> could be used better to infer the actual trigger and its settings, than
> just (function,). For example:
> 	device	function		trigger
> 	system	activity		cpu activity
> 	(empty)	activity		cpu activity
> 	eth0	activity		netdev rx/tx
> 	sda	activity		disk read/write on sda
> 	wlan0	activity		phy rx/tx

I believe that makes sense.

> (This is another thing that is wrong: there should be only phy, or
> wireless-phy trigger, and the mode (rx/tx/assoc/radio) and device
> (phy0, phy1, ...) should be set via device_name file, as in netdev
> trigger. Can we reimplement it and leave this ABI under configuration
> option _LEAGACY?).
> 
> > IMO if LED is not physically integrated with any device, then it should
> > not be named after the device that is to be initially associated with
> > via trigger. This association can be later changed in userspace, which
> > will render the name invalid. And current associated device can be read
> > by reading triggers sysfs file, provided that the trigger conveys
> > that information like in case of presented above phy* triggers.
> 
> There are devices which have LEDs connected via a LED controller for
> example via I2C bus, but the individual LEDs are dedicated (in the way
> that there is an icon or text written on the device's case next to each
> LED). In this case the trigger-source should be defined in device tree
> in such a way that it aligns with the manufacturer's intended function
> of the LED. And in this case I think the devicename part of the LED
> should be derived from this trigger source.

I agree here.

In ideal world, we would have same interface for

1) capslock LED is integrated on USB keyboard

2) casplock LED on i2c and keyboard on GPIO lines

We are not there, yet, but I believe it makes sense as a goal.

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH v2 1/2] dt-bindings: leds: add LED_FUNCTION for wlan2g/wlan5g

[Jacek Anaszewski]

On 9/20/20 5:28 PM, Marek Behun wrote:
> On Sun, 20 Sep 2020 16:59:01 +0200
> Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote:
[...]

>>
>> In case of my mt7601u dongle it looks like below:
>>
>> /sys/class/leds/mt7601u-phy2$ cat trigger
>> none kbd-scrolllock kbd-numlock kbd-capslock kbd-kanalock kbd-shiftlock
>> kbd-altgrlock kbd-ctrllock kbd-altlock kbd-shiftllock kbd-shiftrlock
>> kbd-ctrlllock kbd-ctrlrlock usb-gadget usb-host timer disk-activity
>> disk-read disk-write ide-disk mtd nand-disk heartbeat cpu cpu0 cpu1 cpu2
>> cpu3 cpu4 cpu5 cpu6 cpu7 panic pattern rfkill-any rfkill-none rfkill2
>> phy2rx phy2tx phy2assoc phy2radio [phy2tpt]
>>
> 
> (This is another thing that is wrong: there should be only phy, or
> wireless-phy trigger, and the mode (rx/tx/assoc/radio) and device
> (phy0, phy1, ...) should be set via device_name file, as in netdev
> trigger. Can we reimplement it and leave this ABI under configuration
> option _LEAGACY?).

I agree.

>> IMO if LED is not physically integrated with any device, then it should
>> not be named after the device that is to be initially associated with
>> via trigger. This association can be later changed in userspace, which
>> will render the name invalid. And current associated device can be read
>> by reading triggers sysfs file, provided that the trigger conveys
>> that information like in case of presented above phy* triggers.
> 
> There are devices which have LEDs connected via a LED controller for
> example via I2C bus, but the individual LEDs are dedicated (in the way
> that there is an icon or text written on the device's case next to each
> LED). In this case the trigger-source should be defined in device tree
> in such a way that it aligns with the manufacturer's intended function
> of the LED. And in this case I think the devicename part of the LED
> should be derived from this trigger source.

Agreed about trigger source, but I'd rather not go for consulting LED
name with trigger source. Actually I was considering that back then,
but it turned out to be troublesome as if would have required
implementing that mechanism for associations with all subsystems.

And also you would need an intermediary layer to allow asynchronous
matching of LEDs with their trigger sources (something like
drivers/media/v4l2-core/v4l2-async.c). This would be an overkill.

> 
> Sure, if for example an ethernet PHY registers its LEDs, it can
> hardcode init_data.devicename to "ethernet-phyN" or something like
> that. But for LEDs on a generic LED controller...
> 
> I think we should get opinions from other people in this.
> 
>> OTOH, a LED with devicename describing its physical location will
>> not change this location, even after changing the trigger
>> (or trigger source), thus it proves correct to have fixed devicename
>> section for the LED, but only if it is a part of some other pluggable
>> device.
>>
>> [0]
>> https://lore.kernel.org/linux-leds/20190609190803.14815-27-jacek.anaszewski@gmail.com/
>>
> 
> Marek
> 

-- 
Best regards,
Jacek Anaszewski

Re: [PATCH v2 1/2] dt-bindings: leds: add LED_FUNCTION for wlan2g/wlan5g

[Marek Behun]

On Sun, 20 Sep 2020 16:59:01 +0200
Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote:

> >>> In fact the function should not be "wlan" (nor "wlan2g" or "wlan5g", but
> >>> "activity".  
> 
> I disagree. Activity should be reserved for the activity trigger.
> I've had a patch [0] documenting standard LED functions, but it
> eventually didn't make to the mainline, I'll try to send an update.

Hmm. The thing is that activity is sometimes interpreted as the union
of rx and tx, or read and write. I think the pair (device,function)
could be used better to infer the actual trigger and its settings, than
just (function,). For example:
	device	function		trigger
	system	activity		cpu activity
	(empty)	activity		cpu activity
	eth0	activity		netdev rx/tx
	sda	activity		disk read/write on sda
	wlan0	activity		phy rx/tx

Are you open for discussion on this? Or do you consider this to be
already decided and closed? I would like to hear other opinions here,
but if this was discussed than I guess I shall just stick to the
decisions already made for this...

> > 
> > Thank you Jacek, I will look into this.
> > 
> > Currently my ideas are as follows:
> > - each LED trigger that has settable trigger source (currently only
> >    netdev, gpio, phy (in wireless subsystem) and maybe disk in the
> >    future) shall implement a method for translating device/node pointer
> >    to LED devicename
> > - if trigger-sources is given, the LED registration function shall to
> >    call this new trigger method for all triggers giving the trigger
> >    source as parameter
> > - the first of the triggers that returns successfully will decide the
> >    devicename part of the LED
> > - if none of the triggers return successfully, 2 things can happen, and
> >    I am not yet sure which one should:
> >      1. the registration fails with -EPROBE_DEFER, because LED name
> >         cannot be composed, either trigger module is missing or driver
> >         for the trigger source is missing
> >      2. the registration succeeds but devicename part of LED will be
> >         missing. Since Pavel does not want LED renaming implemented,
> >         this can be only solved by forcing LED driver unbind and rebind
> > 
> > What do you think?  
> 
> I don't think that initially set trigger source should have any impact
> on the LED device name. It is rather the other way round - if the LED
> is physically integrated with the device (e.g. wlan dongle case), then
> it is justified to add a devicename section to it. This is what current
> wlan drivers do, and additionally they register trigger(s) with the same
> devicename prefix, and register the LED with one of them.
> 
> In case of my mt7601u dongle it looks like below:
> 
> /sys/class/leds/mt7601u-phy2$ cat trigger
> none kbd-scrolllock kbd-numlock kbd-capslock kbd-kanalock kbd-shiftlock 
> kbd-altgrlock kbd-ctrllock kbd-altlock kbd-shiftllock kbd-shiftrlock 
> kbd-ctrlllock kbd-ctrlrlock usb-gadget usb-host timer disk-activity 
> disk-read disk-write ide-disk mtd nand-disk heartbeat cpu cpu0 cpu1 cpu2 
> cpu3 cpu4 cpu5 cpu6 cpu7 panic pattern rfkill-any rfkill-none rfkill2 
> phy2rx phy2tx phy2assoc phy2radio [phy2tpt]
> 

(This is another thing that is wrong: there should be only phy, or
wireless-phy trigger, and the mode (rx/tx/assoc/radio) and device
(phy0, phy1, ...) should be set via device_name file, as in netdev
trigger. Can we reimplement it and leave this ABI under configuration
option _LEAGACY?).

> IMO if LED is not physically integrated with any device, then it should
> not be named after the device that is to be initially associated with
> via trigger. This association can be later changed in userspace, which
> will render the name invalid. And current associated device can be read
> by reading triggers sysfs file, provided that the trigger conveys
> that information like in case of presented above phy* triggers.

There are devices which have LEDs connected via a LED controller for
example via I2C bus, but the individual LEDs are dedicated (in the way
that there is an icon or text written on the device's case next to each
LED). In this case the trigger-source should be defined in device tree
in such a way that it aligns with the manufacturer's intended function
of the LED. And in this case I think the devicename part of the LED
should be derived from this trigger source.

Sure, if for example an ethernet PHY registers its LEDs, it can
hardcode init_data.devicename to "ethernet-phyN" or something like
that. But for LEDs on a generic LED controller...

I think we should get opinions from other people in this.

> OTOH, a LED with devicename describing its physical location will
> not change this location, even after changing the trigger
> (or trigger source), thus it proves correct to have fixed devicename
> section for the LED, but only if it is a part of some other pluggable
> device.
> 
> [0] 
> https://lore.kernel.org/linux-leds/20190609190803.14815-27-jacek.anaszewski@gmail.com/
> 

Marek

Re: [PATCH v2 1/2] dt-bindings: leds: add LED_FUNCTION for wlan2g/wlan5g

[Jacek Anaszewski]

On 9/20/20 3:37 PM, Marek Behun wrote:
> On Sun, 20 Sep 2020 15:16:11 +0200
> Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote:
> 
>> Hi Marek,
>>
>> On 9/19/20 10:31 PM, Marek Behun wrote:
>>> On Sat, 19 Sep 2020 21:24:26 +0200
>>> Adrian Schmutzler <freifunk@adrianschmutzler.de> wrote:
>>>    
>>>> Many consumer "routers" have dedicated LEDs for specific WiFi bands,
>>>> e.g. one for 2.4 GHz and one for 5 GHz. These LEDs specifically
>>>> indicate the state of the relevant band, so the latter should be
>>>> included in the function name. LED_FUNCTION_WLAN will remain for
>>>> general cases or when the LED is used for more than one band.
>>>>
>>>> This essentially is equivalent to how we use LED_FUNCTION_LAN and
>>>> LED_FUNCTION_WAN instead of just having LED_FUNCTION_ETHERNET.
>>>
>>> Dont. If you want the LED name to inform the user about the WiFi
>>> device it is being triggered on, it instead should come from the
>>> devicename part:
>>>     "wlan0:blue:activity"
>>>
>>> In fact the function should not be "wlan" (nor "wlan2g" or "wlan5g", but
>>> "activity".

I disagree. Activity should be reserved for the activity trigger.
I've had a patch [0] documenting standard LED functions, but it
eventually didn't make to the mainline, I'll try to send an update.

>>> I am going to try to work on this subsystem so that if trigger source
>>> of the LED is set to a WiFi device and function is set to activity, the
>>> LED shall blink on wifi activity.
>>>
>>> This way we can also avoid using the `linux,default-trigger` property
>>> in favour of `function`, i.e. if I have:
>>>
>>>      wlan0: wifi@12300 {
>>>        compatible = "some-wifi";
>>>        #trigger-source-cells = <0>;
>>>      }
>>>
>>>      led {
>>>        color = <LED_COLOR_ID_BLUE>;
>>>        function = LED_FUNCTION_ACTIVITY;
>>>        trigger-sources = <&wlan0>;
>>>      };
>>>
>>> Than this will automatically name the LED as
>>>     wlan0:blue:activity
>>> and if the corresponding trigger is available, it should set the
>>> trigger even if no `linux,default-trigger` property is present.
>>
>> Since wlan0 is DT label, then you will probably not be able to retrieve
>> its name in the driver but only a pointer to the struct device_node
>> associated with the label. And actually from the userspace user POV
>> this name is not too informative. What is informative is
>> unique identifier of the wlan device present in the system, associated
>> with the LED.
>>
>> And wlan drivers broadly use wiphy_name() to get phy identifier and
>> use it for composing associated LED device name.
>>
>> This way we get LED name in the form (here for Mediatek wlan dongle):
>>
>> mt7601u-phy0
>>
>> This is not in alignment with LED naming pattern and there also are
>> other variations for different drivers in
>> drivers/net/wireless, so it would be good to standardize that.
>>
>> While implementing LED core support for LED name composition I created
>> also a script for validating LED name and printing LED hardware
>> related information so that could be a good starting point.
>>
>> The script is in the tree:
>>
>> tools/leds/get_led_device_info.sh
>>
>> and for said Mediatek dongle it gives following output:
>>
>> <quote>
>>
>> $./tools/leds/get_led_device_info.sh /sys/class/leds/mt7601u-phy0
>>
>> #####################################
>> # LED class device hardware details #
>> #####################################
>>
>> bus:			usb
>> idVendor:		148f
>> idProduct:		7601
>> manufacturer:		MediaTek
>> product:		802.11 n WLAN
>> driver:			mt7601u
>>
>> ####################################
>> # LED class device name validation #
>> ####################################
>>
>> ":" delimiter not detected.	[ FAILED ]
>>
>> </quote>
>>
>> And for the LEDs exposed by USB keyboard it prints below stuff:
>>
>> <quote>
>>
>> $./tools/leds/get_led_device_info.sh /sys/class/leds/input1\:\:capslock
>>
>> #####################################
>> # LED class device hardware details #
>> #####################################
>>
>> bus:			usb
>> idVendor:		046d
>> idProduct:		c31c
>> manufacturer:		Logitech
>> product:		USB Keyboard
>> driver:			usbhid
>> associated input node:	input1
>>
>> ####################################
>> # LED class device name validation #
>> ####################################
>>
>> devicename :	input1               [ OK ]
>> function   :	capslock             [ OK ]     Matching definition:
>> LED_FUNCTION_CAPSLOCK
>>
>> </quoute>
>>
>> The script currently detects LEDs associations only with wireless and
>> input devices.
>>
> 
> Thank you Jacek, I will look into this.
> 
> Currently my ideas are as follows:
> - each LED trigger that has settable trigger source (currently only
>    netdev, gpio, phy (in wireless subsystem) and maybe disk in the
>    future) shall implement a method for translating device/node pointer
>    to LED devicename
> - if trigger-sources is given, the LED registration function shall to
>    call this new trigger method for all triggers giving the trigger
>    source as parameter
> - the first of the triggers that returns successfully will decide the
>    devicename part of the LED
> - if none of the triggers return successfully, 2 things can happen, and
>    I am not yet sure which one should:
>      1. the registration fails with -EPROBE_DEFER, because LED name
>         cannot be composed, either trigger module is missing or driver
>         for the trigger source is missing
>      2. the registration succeeds but devicename part of LED will be
>         missing. Since Pavel does not want LED renaming implemented,
>         this can be only solved by forcing LED driver unbind and rebind
> 
> What do you think?

I don't think that initially set trigger source should have any impact
on the LED device name. It is rather the other way round - if the LED
is physically integrated with the device (e.g. wlan dongle case), then
it is justified to add a devicename section to it. This is what current
wlan drivers do, and additionally they register trigger(s) with the same
devicename prefix, and register the LED with one of them.

In case of my mt7601u dongle it looks like below:

/sys/class/leds/mt7601u-phy2$ cat trigger
none kbd-scrolllock kbd-numlock kbd-capslock kbd-kanalock kbd-shiftlock 
kbd-altgrlock kbd-ctrllock kbd-altlock kbd-shiftllock kbd-shiftrlock 
kbd-ctrlllock kbd-ctrlrlock usb-gadget usb-host timer disk-activity 
disk-read disk-write ide-disk mtd nand-disk heartbeat cpu cpu0 cpu1 cpu2 
cpu3 cpu4 cpu5 cpu6 cpu7 panic pattern rfkill-any rfkill-none rfkill2 
phy2rx phy2tx phy2assoc phy2radio [phy2tpt]

IMO if LED is not physically integrated with any device, then it should
not be named after the device that is to be initially associated with
via trigger. This association can be later changed in userspace, which
will render the name invalid. And current associated device can be read
by reading triggers sysfs file, provided that the trigger conveys
that information like in case of presented above phy* triggers.

OTOH, a LED with devicename describing its physical location will
not change this location, even after changing the trigger
(or trigger source), thus it proves correct to have fixed devicename
section for the LED, but only if it is a part of some other pluggable
device.

[0] 
https://lore.kernel.org/linux-leds/20190609190803.14815-27-jacek.anaszewski@gmail.com/

-- 
Best regards,
Jacek Anaszewski

Re: [PATCH v2 1/2] dt-bindings: leds: add LED_FUNCTION for wlan2g/wlan5g

[Marek Behun]

On Sun, 20 Sep 2020 15:16:11 +0200
Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote:

> Hi Marek,
> 
> On 9/19/20 10:31 PM, Marek Behun wrote:
> > On Sat, 19 Sep 2020 21:24:26 +0200
> > Adrian Schmutzler <freifunk@adrianschmutzler.de> wrote:
> >   
> >> Many consumer "routers" have dedicated LEDs for specific WiFi bands,
> >> e.g. one for 2.4 GHz and one for 5 GHz. These LEDs specifically
> >> indicate the state of the relevant band, so the latter should be
> >> included in the function name. LED_FUNCTION_WLAN will remain for
> >> general cases or when the LED is used for more than one band.
> >>
> >> This essentially is equivalent to how we use LED_FUNCTION_LAN and
> >> LED_FUNCTION_WAN instead of just having LED_FUNCTION_ETHERNET.  
> > 
> > Dont. If you want the LED name to inform the user about the WiFi
> > device it is being triggered on, it instead should come from the
> > devicename part:
> >    "wlan0:blue:activity"
> > 
> > In fact the function should not be "wlan" (nor "wlan2g" or "wlan5g", but
> > "activity".
> > 
> > I am going to try to work on this subsystem so that if trigger source
> > of the LED is set to a WiFi device and function is set to activity, the
> > LED shall blink on wifi activity.
> > 
> > This way we can also avoid using the `linux,default-trigger` property
> > in favour of `function`, i.e. if I have:
> > 
> >     wlan0: wifi@12300 {
> >       compatible = "some-wifi";
> >       #trigger-source-cells = <0>;
> >     }
> > 
> >     led {
> >       color = <LED_COLOR_ID_BLUE>;
> >       function = LED_FUNCTION_ACTIVITY;
> >       trigger-sources = <&wlan0>;
> >     };
> > 
> > Than this will automatically name the LED as
> >    wlan0:blue:activity
> > and if the corresponding trigger is available, it should set the
> > trigger even if no `linux,default-trigger` property is present.  
> 
> Since wlan0 is DT label, then you will probably not be able to retrieve
> its name in the driver but only a pointer to the struct device_node
> associated with the label. And actually from the userspace user POV
> this name is not too informative. What is informative is
> unique identifier of the wlan device present in the system, associated
> with the LED.
> 
> And wlan drivers broadly use wiphy_name() to get phy identifier and
> use it for composing associated LED device name.
> 
> This way we get LED name in the form (here for Mediatek wlan dongle):
> 
> mt7601u-phy0
> 
> This is not in alignment with LED naming pattern and there also are
> other variations for different drivers in
> drivers/net/wireless, so it would be good to standardize that.
> 
> While implementing LED core support for LED name composition I created
> also a script for validating LED name and printing LED hardware
> related information so that could be a good starting point.
> 
> The script is in the tree:
> 
> tools/leds/get_led_device_info.sh
> 
> and for said Mediatek dongle it gives following output:
> 
> <quote>
> 
> $./tools/leds/get_led_device_info.sh /sys/class/leds/mt7601u-phy0
> 
> #####################################
> # LED class device hardware details #
> #####################################
> 
> bus:			usb
> idVendor:		148f
> idProduct:		7601
> manufacturer:		MediaTek
> product:		802.11 n WLAN
> driver:			mt7601u
> 
> ####################################
> # LED class device name validation #
> ####################################
> 
> ":" delimiter not detected.	[ FAILED ]
> 
> </quote>
> 
> And for the LEDs exposed by USB keyboard it prints below stuff:
> 
> <quote>
> 
> $./tools/leds/get_led_device_info.sh /sys/class/leds/input1\:\:capslock
> 
> #####################################
> # LED class device hardware details #
> #####################################
> 
> bus:			usb
> idVendor:		046d
> idProduct:		c31c
> manufacturer:		Logitech
> product:		USB Keyboard
> driver:			usbhid
> associated input node:	input1
> 
> ####################################
> # LED class device name validation #
> ####################################
> 
> devicename :	input1               [ OK ]
> function   :	capslock             [ OK ]     Matching definition: 
> LED_FUNCTION_CAPSLOCK
> 
> </quoute>
> 
> The script currently detects LEDs associations only with wireless and
> input devices.
> 

Thank you Jacek, I will look into this.

Currently my ideas are as follows:
- each LED trigger that has settable trigger source (currently only
  netdev, gpio, phy (in wireless subsystem) and maybe disk in the
  future) shall implement a method for translating device/node pointer
  to LED devicename
- if trigger-sources is given, the LED registration function shall to
  call this new trigger method for all triggers giving the trigger
  source as parameter
- the first of the triggers that returns successfully will decide the
  devicename part of the LED
- if none of the triggers return successfully, 2 things can happen, and
  I am not yet sure which one should:
    1. the registration fails with -EPROBE_DEFER, because LED name
       cannot be composed, either trigger module is missing or driver
       for the trigger source is missing
    2. the registration succeeds but devicename part of LED will be
       missing. Since Pavel does not want LED renaming implemented,
       this can be only solved by forcing LED driver unbind and rebind

What do you think?

Marek

Re: [PATCH v2 1/2] dt-bindings: leds: add LED_FUNCTION for wlan2g/wlan5g

[Jacek Anaszewski]

Hi Marek,

On 9/19/20 10:31 PM, Marek Behun wrote:
> On Sat, 19 Sep 2020 21:24:26 +0200
> Adrian Schmutzler <freifunk@adrianschmutzler.de> wrote:
> 
>> Many consumer "routers" have dedicated LEDs for specific WiFi bands,
>> e.g. one for 2.4 GHz and one for 5 GHz. These LEDs specifically
>> indicate the state of the relevant band, so the latter should be
>> included in the function name. LED_FUNCTION_WLAN will remain for
>> general cases or when the LED is used for more than one band.
>>
>> This essentially is equivalent to how we use LED_FUNCTION_LAN and
>> LED_FUNCTION_WAN instead of just having LED_FUNCTION_ETHERNET.
> 
> Dont. If you want the LED name to inform the user about the WiFi
> device it is being triggered on, it instead should come from the
> devicename part:
>    "wlan0:blue:activity"
> 
> In fact the function should not be "wlan" (nor "wlan2g" or "wlan5g", but
> "activity".
> 
> I am going to try to work on this subsystem so that if trigger source
> of the LED is set to a WiFi device and function is set to activity, the
> LED shall blink on wifi activity.
> 
> This way we can also avoid using the `linux,default-trigger` property
> in favour of `function`, i.e. if I have:
> 
>     wlan0: wifi@12300 {
>       compatible = "some-wifi";
>       #trigger-source-cells = <0>;
>     }
> 
>     led {
>       color = <LED_COLOR_ID_BLUE>;
>       function = LED_FUNCTION_ACTIVITY;
>       trigger-sources = <&wlan0>;
>     };
> 
> Than this will automatically name the LED as
>    wlan0:blue:activity
> and if the corresponding trigger is available, it should set the
> trigger even if no `linux,default-trigger` property is present.

Since wlan0 is DT label, then you will probably not be able to retrieve
its name in the driver but only a pointer to the struct device_node
associated with the label. And actually from the userspace user POV
this name is not too informative. What is informative is
unique identifier of the wlan device present in the system, associated
with the LED.

And wlan drivers broadly use wiphy_name() to get phy identifier and
use it for composing associated LED device name.

This way we get LED name in the form (here for Mediatek wlan dongle):

mt7601u-phy0

This is not in alignment with LED naming pattern and there also are
other variations for different drivers in
drivers/net/wireless, so it would be good to standardize that.

While implementing LED core support for LED name composition I created
also a script for validating LED name and printing LED hardware
related information so that could be a good starting point.

The script is in the tree:

tools/leds/get_led_device_info.sh

and for said Mediatek dongle it gives following output:

<quote>

$./tools/leds/get_led_device_info.sh /sys/class/leds/mt7601u-phy0

#####################################
# LED class device hardware details #
#####################################

bus:			usb
idVendor:		148f
idProduct:		7601
manufacturer:		MediaTek
product:		802.11 n WLAN
driver:			mt7601u

####################################
# LED class device name validation #
####################################

":" delimiter not detected.	[ FAILED ]

</quote>

And for the LEDs exposed by USB keyboard it prints below stuff:

<quote>

$./tools/leds/get_led_device_info.sh /sys/class/leds/input1\:\:capslock

#####################################
# LED class device hardware details #
#####################################

bus:			usb
idVendor:		046d
idProduct:		c31c
manufacturer:		Logitech
product:		USB Keyboard
driver:			usbhid
associated input node:	input1

####################################
# LED class device name validation #
####################################

devicename :	input1               [ OK ]
function   :	capslock             [ OK ]     Matching definition: 
LED_FUNCTION_CAPSLOCK

</quoute>

The script currently detects LEDs associations only with wireless and
input devices.

-- 
Best regards,
Jacek Anaszewski

RE: [PATCH v2 1/2] dt-bindings: leds: add LED_FUNCTION for wlan2g/wlan5g

[Adrian Schmutzler]

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

> -----Original Message-----
> From: Marek Behun [mailto:marek.behun@nic.cz]
> Sent: Sonntag, 20. September 2020 02:07
> To: Adrian Schmutzler <mail@adrianschmutzler.de>
> Cc: linux-leds@vger.kernel.org
> Subject: Re: [PATCH v2 1/2] dt-bindings: leds: add LED_FUNCTION for
> wlan2g/wlan5g
> 
> On Sun, 20 Sep 2020 01:09:49 +0200
> "Adrian Schmutzler" <mail@adrianschmutzler.de> wrote:
> 
> > > > As far as I understand it, the color/function system does not
> > > > provide a
> > > comparable lever, as the final name is only constructed in led-core.c?
> > > >
> > >
> > > The files in /sys/class/leds/ are symlinks. The actual files are in
> > > /sys/devices/ somewhere. If you know the path of your LED in the
> > > device hierarchy, you can find it that way. If your script can
> > > access the LED by reading device-tree, you can implement your script
> > > so that you can find the LED in the hiearchy in /sys/devices/ (or
> > > simply byt reading where do the symlinks in /sys/class/leds/ point to).
> >
> > Thanks for that pointer; unfortunately though, I was only able to retrieve
> lists of leds in [device:]color:function syntax and lists of the DT nodes, but
> nothing where a single node from DT is linked or can be related to just one of
> the [device:]color:function identifiers.
> >
> > Best
> >
> > Adrian
> >
> 
> Which driver is this? Normally there is of_node symlink in sysfs device
> directory...

This is ath79 target on OpenWrt master. I have an of_node symlink for the parent "leds" node, but not for the individual leds:

root@OpenWrt:~# ls /sys/devices/
pci0000:00/  platform/    system/      virtual/
root@OpenWrt:~# ls /sys/devices/platform/
Fixed MDIO bus.0/  gpio-export/       reg-dummy/         uevent
ahb/               keys/              regulatory.0/      usb-phy/
ath9k-leds/        leds/              serial8250/
root@OpenWrt:~# ls /sys/devices/platform/leds/
driver/          leds/            of_node/         uevent
driver_override  modalias         subsystem/
root@OpenWrt:~# ls /sys/devices/platform/leds/leds/
green:status  green:usb     green:usb_1   green:wlan-1  green:wps
root@OpenWrt:~# ls /sys/devices/platform/leds/leds/green\:status/
brightness      device          max_brightness  subsystem       trigger         uevent
root@OpenWrt:~# cat /sys/devices/platform/leds/leds/green\:status/uevent
OF_NAME=system
OF_FULLNAME=/leds/system
OF_COMPATIBLE_N=0
root@OpenWrt:~# ls /sys/devices/platform/leds/of_node/
compatible  name        qss         system      usb1        usb2        wlan2g
root@OpenWrt:~# ls /sys/devices/platform/leds/of_node/system/
color          default-state  function       gpios          name

In the meantime I found the reference in uevent above, but that's not really convenient.
I don't think we do anything special about leds here. The target has CONFIG_LEDS_GPIO=y set in config. Kernel 5.4.66.

The configuration looks like this (enumerator on wlan2g was for testing only; there is a separate ath9k-leds node which I ignored for this discussion):

/ {
	aliases {
		led-boot = &led_system;
		led-failsafe = &led_system;
		led-running = &led_system;
		led-upgrade = &led_system;
	};

	leds {
		compatible = "gpio-leds";

		wlan2g {
			function = LED_FUNCTION_WLAN;
			function-enumerator = <1>;
			color = <LED_COLOR_ID_GREEN>;
			gpios = <&gpio 13 GPIO_ACTIVE_LOW>;
			linux,default-trigger = "phy0tpt";
		};

		led_system: system {
			function = LED_FUNCTION_STATUS;
			color = <LED_COLOR_ID_GREEN>;
			gpios = <&gpio 14 GPIO_ACTIVE_LOW>;
			default-state = "on";
		};

		qss {
			label = "tp-link:green:qss";
			function = LED_FUNCTION_WPS;
			color = <LED_COLOR_ID_GREEN>;
			gpios = <&gpio 15 GPIO_ACTIVE_LOW>;
		};

		usb1 {
			function = LED_FUNCTION_USB;
			color = <LED_COLOR_ID_GREEN>;
			gpios = <&gpio 11 GPIO_ACTIVE_LOW>;
			trigger-sources = <&hub_port1>;
			linux,default-trigger = "usbport";
		};

		usb2 {
			function = LED_FUNCTION_USB;
			color = <LED_COLOR_ID_GREEN>;
			gpios = <&gpio 12 GPIO_ACTIVE_LOW>;
			trigger-sources = <&hub_port2>;
			linux,default-trigger = "usbport";
		};
	};
};

While I hacked together an ugly solution to exploit uevent (with recursive grep ...), I'd really prefer a more convenient option so the "new" concept doesn't turn into a drawback from the start.

Thanks for your help so far!

Best

Adrian

[-- Attachment #2: openpgp-digital-signature.asc --]
[-- Type: application/pgp-signature, Size: 834 bytes --]

Re: [PATCH v2 1/2] dt-bindings: leds: add LED_FUNCTION for wlan2g/wlan5g

[Marek Behun]

On Sun, 20 Sep 2020 01:09:49 +0200
"Adrian Schmutzler" <mail@adrianschmutzler.de> wrote:

> > > As far as I understand it, the color/function system does not provide a  
> > comparable lever, as the final name is only constructed in led-core.c?  
> > >  
> > 
> > The files in /sys/class/leds/ are symlinks. The actual files are in /sys/devices/
> > somewhere. If you know the path of your LED in the device hierarchy, you
> > can find it that way. If your script can access the LED by reading device-tree,
> > you can implement your script so that you can find the LED in the hiearchy in
> > /sys/devices/ (or simply byt reading where do the symlinks in
> > /sys/class/leds/ point to).  
> 
> Thanks for that pointer; unfortunately though, I was only able to retrieve lists of leds in [device:]color:function syntax and lists of the DT nodes, but nothing where a single node from DT is linked or can be related to just one of the [device:]color:function identifiers.
> 
> Best
> 
> Adrian
>

Which driver is this? Normally there is of_node symlink in sysfs device
directory...

RE: [PATCH v2 1/2] dt-bindings: leds: add LED_FUNCTION for wlan2g/wlan5g

[Adrian Schmutzler]

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

> > As far as I understand it, the color/function system does not provide a
> comparable lever, as the final name is only constructed in led-core.c?
> >
> 
> The files in /sys/class/leds/ are symlinks. The actual files are in /sys/devices/
> somewhere. If you know the path of your LED in the device hierarchy, you
> can find it that way. If your script can access the LED by reading device-tree,
> you can implement your script so that you can find the LED in the hiearchy in
> /sys/devices/ (or simply byt reading where do the symlinks in
> /sys/class/leds/ point to).

Thanks for that pointer; unfortunately though, I was only able to retrieve lists of leds in [device:]color:function syntax and lists of the DT nodes, but nothing where a single node from DT is linked or can be related to just one of the [device:]color:function identifiers.

Best

Adrian

> 
> Marek

[-- Attachment #2: openpgp-digital-signature.asc --]
[-- Type: application/pgp-signature, Size: 834 bytes --]

Re: [PATCH v2 1/2] dt-bindings: leds: add LED_FUNCTION for wlan2g/wlan5g

[Marek Behun]

On Sat, 19 Sep 2020 23:40:50 +0200
"Adrian Schmutzler" <mail@adrianschmutzler.de> wrote:

> > -----Original Message-----
> > From: Marek Behun [mailto:marek.behun@nic.cz]
> > Sent: Samstag, 19. September 2020 22:32
> > To: Adrian Schmutzler <freifunk@adrianschmutzler.de>
> > Cc: linux-leds@vger.kernel.org
> > Subject: Re: [PATCH v2 1/2] dt-bindings: leds: add LED_FUNCTION for
> > wlan2g/wlan5g
> > 
> > On Sat, 19 Sep 2020 21:24:26 +0200
> > Adrian Schmutzler <freifunk@adrianschmutzler.de> wrote:
> >   
> > > Many consumer "routers" have dedicated LEDs for specific WiFi bands,
> > > e.g. one for 2.4 GHz and one for 5 GHz. These LEDs specifically
> > > indicate the state of the relevant band, so the latter should be
> > > included in the function name. LED_FUNCTION_WLAN will remain for
> > > general cases or when the LED is used for more than one band.
> > >
> > > This essentially is equivalent to how we use LED_FUNCTION_LAN and
> > > LED_FUNCTION_WAN instead of just having LED_FUNCTION_ETHERNET.  
> > 
> > Dont. If you want the LED name to inform the user about the WiFi device it is
> > being triggered on, it instead should come from the devicename part:
> >   "wlan0:blue:activity"
> > 
> > In fact the function should not be "wlan" (nor "wlan2g" or "wlan5g", but
> > "activity".
> > 
> > I am going to try to work on this subsystem so that if trigger source of the LED
> > is set to a WiFi device and function is set to activity, the LED shall blink on wifi
> > activity.
> > 
> > This way we can also avoid using the `linux,default-trigger` property in favour
> > of `function`, i.e. if I have:
> > 
> >    wlan0: wifi@12300 {
> >      compatible = "some-wifi";
> >      #trigger-source-cells = <0>;
> >    }
> > 
> >    led {
> >      color = <LED_COLOR_ID_BLUE>;
> >      function = LED_FUNCTION_ACTIVITY;
> >      trigger-sources = <&wlan0>;
> >    };
> > 
> > Than this will automatically name the LED as
> >   wlan0:blue:activity
> > and if the corresponding trigger is available, it should set the trigger even if
> > no `linux,default-trigger` property is present.  
> 
> Thanks for the explanation, I understand why this makes sense conceptually.
> 
> However, I assume your use of the word "will" indicates that I won't be able to build a solution like this with kernel 5.4?
> So, easiest surrogate there would be to just stick to label property for the moment ...?

Yes.

> How is the "device" part determined? Can I just change the DT label from wlan0 to wlan2g to get a more descriptive name, which will be translated into the led name like wlan2g:blue:activity?

The label "wlan0" in the example above (wlan0: wifi@12300) is not stored
is DT binary. It is just a name for phandle reference. The actual name
of the DT node is "wifi@12300".

Currently the devicename part is given by the LED driver. Some drivers
do this, some do not. Those who do set it differently, for example
they give name of the parent device of the LED (which is often name of
the LED controller).

This should be changed by the core so that if trigger-source is given,
then the devicename part is taken from the trigger-source. But
currently this is not implemented.

> And finally, though partly off-topic:
> Is there any way to determine the full "led name" from device-tree? We currently have aliases for certain leds, like
> 	led-upgrade = &led_somename;
> 
> The allowed us to construct the /sys/class/leds/<label> path directly from /proc/device-tree (in a user-space script), as we could access the label property of the LED.
> As far as I understand it, the color/function system does not provide a comparable lever, as the final name is only constructed in led-core.c?
> 

The files in /sys/class/leds/ are symlinks. The actual files are in
/sys/devices/ somewhere. If you know the path of your LED in the
device hierarchy, you can find it that way. If your script can access
the LED by reading device-tree, you can implement your script so that
you can find the LED in the hiearchy in /sys/devices/ (or simply byt
reading where do the symlinks in /sys/class/leds/ point to).

Marek

RE: [PATCH v2 1/2] dt-bindings: leds: add LED_FUNCTION for wlan2g/wlan5g

[Adrian Schmutzler]

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

> -----Original Message-----
> From: Marek Behun [mailto:marek.behun@nic.cz]
> Sent: Samstag, 19. September 2020 22:32
> To: Adrian Schmutzler <freifunk@adrianschmutzler.de>
> Cc: linux-leds@vger.kernel.org
> Subject: Re: [PATCH v2 1/2] dt-bindings: leds: add LED_FUNCTION for
> wlan2g/wlan5g
> 
> On Sat, 19 Sep 2020 21:24:26 +0200
> Adrian Schmutzler <freifunk@adrianschmutzler.de> wrote:
> 
> > Many consumer "routers" have dedicated LEDs for specific WiFi bands,
> > e.g. one for 2.4 GHz and one for 5 GHz. These LEDs specifically
> > indicate the state of the relevant band, so the latter should be
> > included in the function name. LED_FUNCTION_WLAN will remain for
> > general cases or when the LED is used for more than one band.
> >
> > This essentially is equivalent to how we use LED_FUNCTION_LAN and
> > LED_FUNCTION_WAN instead of just having LED_FUNCTION_ETHERNET.
> 
> Dont. If you want the LED name to inform the user about the WiFi device it is
> being triggered on, it instead should come from the devicename part:
>   "wlan0:blue:activity"
> 
> In fact the function should not be "wlan" (nor "wlan2g" or "wlan5g", but
> "activity".
> 
> I am going to try to work on this subsystem so that if trigger source of the LED
> is set to a WiFi device and function is set to activity, the LED shall blink on wifi
> activity.
> 
> This way we can also avoid using the `linux,default-trigger` property in favour
> of `function`, i.e. if I have:
> 
>    wlan0: wifi@12300 {
>      compatible = "some-wifi";
>      #trigger-source-cells = <0>;
>    }
> 
>    led {
>      color = <LED_COLOR_ID_BLUE>;
>      function = LED_FUNCTION_ACTIVITY;
>      trigger-sources = <&wlan0>;
>    };
> 
> Than this will automatically name the LED as
>   wlan0:blue:activity
> and if the corresponding trigger is available, it should set the trigger even if
> no `linux,default-trigger` property is present.

Thanks for the explanation, I understand why this makes sense conceptually.

However, I assume your use of the word "will" indicates that I won't be able to build a solution like this with kernel 5.4?
So, easiest surrogate there would be to just stick to label property for the moment ...?

How is the "device" part determined? Can I just change the DT label from wlan0 to wlan2g to get a more descriptive name, which will be translated into the led name like wlan2g:blue:activity?

And finally, though partly off-topic:
Is there any way to determine the full "led name" from device-tree? We currently have aliases for certain leds, like
	led-upgrade = &led_somename;

The allowed us to construct the /sys/class/leds/<label> path directly from /proc/device-tree (in a user-space script), as we could access the label property of the LED.
As far as I understand it, the color/function system does not provide a comparable lever, as the final name is only constructed in led-core.c?

Best

Adrian

[-- Attachment #2: openpgp-digital-signature.asc --]
[-- Type: application/pgp-signature, Size: 834 bytes --]

Re: [PATCH v2 1/2] dt-bindings: leds: add LED_FUNCTION for wlan2g/wlan5g

[Marek Behun]

On Sat, 19 Sep 2020 21:24:26 +0200
Adrian Schmutzler <freifunk@adrianschmutzler.de> wrote:

> Many consumer "routers" have dedicated LEDs for specific WiFi bands,
> e.g. one for 2.4 GHz and one for 5 GHz. These LEDs specifically
> indicate the state of the relevant band, so the latter should be
> included in the function name. LED_FUNCTION_WLAN will remain for
> general cases or when the LED is used for more than one band.
> 
> This essentially is equivalent to how we use LED_FUNCTION_LAN and
> LED_FUNCTION_WAN instead of just having LED_FUNCTION_ETHERNET.

Dont. If you want the LED name to inform the user about the WiFi
device it is being triggered on, it instead should come from the
devicename part:
  "wlan0:blue:activity"

In fact the function should not be "wlan" (nor "wlan2g" or "wlan5g", but
"activity".

I am going to try to work on this subsystem so that if trigger source
of the LED is set to a WiFi device and function is set to activity, the
LED shall blink on wifi activity.

This way we can also avoid using the `linux,default-trigger` property
in favour of `function`, i.e. if I have:

   wlan0: wifi@12300 {
     compatible = "some-wifi";
     #trigger-source-cells = <0>;
   }

   led {
     color = <LED_COLOR_ID_BLUE>;
     function = LED_FUNCTION_ACTIVITY;
     trigger-sources = <&wlan0>;
   };

Than this will automatically name the LED as
  wlan0:blue:activity
and if the corresponding trigger is available, it should set the
trigger even if no `linux,default-trigger` property is present.

Marek

[PATCH v2 1/2] dt-bindings: leds: add LED_FUNCTION for wlan2g/wlan5g

[Adrian Schmutzler]

Many consumer "routers" have dedicated LEDs for specific WiFi bands,
e.g. one for 2.4 GHz and one for 5 GHz. These LEDs specifically
indicate the state of the relevant band, so the latter should be
included in the function name. LED_FUNCTION_WLAN will remain for
general cases or when the LED is used for more than one band.

This essentially is equivalent to how we use LED_FUNCTION_LAN and
LED_FUNCTION_WAN instead of just having LED_FUNCTION_ETHERNET.

Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>

---

Changes in v2:
- Without typo this time. Sorry.
---
 include/dt-bindings/leds/common.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h
index 52b619d44ba2..debbd406ff17 100644
--- a/include/dt-bindings/leds/common.h
+++ b/include/dt-bindings/leds/common.h
@@ -89,6 +89,8 @@
 #define LED_FUNCTION_USB "usb"
 #define LED_FUNCTION_WAN "wan"
 #define LED_FUNCTION_WLAN "wlan"
+#define LED_FUNCTION_WLAN2G "wlan2g"
+#define LED_FUNCTION_WLAN5G "wlan5g"
 #define LED_FUNCTION_WPS "wps"
 
 #endif /* __DT_BINDINGS_LEDS_H */
-- 
2.20.1