[03/24] leds: dt-bindings: Add LED_FUNCTION definitions

Add common LED function definitions for use in Device Tree.
The function names were extracted from existing dts files
after eliminating oddities.

Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Cc: Baolin Wang <baolin.wang@linaro.org>
Cc: Daniel Mack <daniel@zonque.org>
Cc: Dan Murphy <dmurphy@ti.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Oleh Kravchenko <oleg@kaa.org.ua>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Simon Shields <simon@lineageos.org>
Cc: Xiaotong Lu <xiaotong.lu@spreadtrum.com>
---
 include/dt-bindings/leds/functions.h | 101 +++++++++++++++++++++++++++++++++++
 1 file changed, 101 insertions(+)
 create mode 100644 include/dt-bindings/leds/functions.h

On 6.11.2018 23:07, Jacek Anaszewski wrote:
> Add common LED function definitions for use in Device Tree.
> The function names were extracted from existing dts files
> after eliminating oddities.
> 
> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> Cc: Baolin Wang <baolin.wang@linaro.org>
> Cc: Daniel Mack <daniel@zonque.org>
> Cc: Dan Murphy <dmurphy@ti.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Oleh Kravchenko <oleg@kaa.org.ua>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: Simon Shields <simon@lineageos.org>
> Cc: Xiaotong Lu <xiaotong.lu@spreadtrum.com>
> ---
>   include/dt-bindings/leds/functions.h | 101 +++++++++++++++++++++++++++++++++++
>   1 file changed, 101 insertions(+)
>   create mode 100644 include/dt-bindings/leds/functions.h

Hi Jacek,
Just nit picking. I think the subject should be:

"dt-bindings: leds: ..." to be consistent.

Best regards,
Michal

Hi Michal

On 11/07/2018 09:36 AM, Vokáč Michal wrote:
> On 6.11.2018 23:07, Jacek Anaszewski wrote:
>> Add common LED function definitions for use in Device Tree.
>> The function names were extracted from existing dts files
>> after eliminating oddities.
>>
>> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
>> Cc: Baolin Wang <baolin.wang@linaro.org>
>> Cc: Daniel Mack <daniel@zonque.org>
>> Cc: Dan Murphy <dmurphy@ti.com>
>> Cc: Linus Walleij <linus.walleij@linaro.org>
>> Cc: Oleh Kravchenko <oleg@kaa.org.ua>
>> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
>> Cc: Simon Shields <simon@lineageos.org>
>> Cc: Xiaotong Lu <xiaotong.lu@spreadtrum.com>
>> ---
>>   include/dt-bindings/leds/functions.h | 101 +++++++++++++++++++++++++++++++++++
>>   1 file changed, 101 insertions(+)
>>   create mode 100644 include/dt-bindings/leds/functions.h
> 
> Hi Jacek,
> Just nit picking. I think the subject should be:
> 
> "dt-bindings: leds: ..." to be consistent.

Thanks for the heads-up. I'll change it in the v2.

On Tue, Nov 6, 2018 at 4:07 PM Jacek Anaszewski
<jacek.anaszewski@gmail.com> wrote:
>
> Add common LED function definitions for use in Device Tree.
> The function names were extracted from existing dts files
> after eliminating oddities.
>
> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> Cc: Baolin Wang <baolin.wang@linaro.org>
> Cc: Daniel Mack <daniel@zonque.org>
> Cc: Dan Murphy <dmurphy@ti.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Oleh Kravchenko <oleg@kaa.org.ua>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: Simon Shields <simon@lineageos.org>
> Cc: Xiaotong Lu <xiaotong.lu@spreadtrum.com>
> ---
>  include/dt-bindings/leds/functions.h | 101 +++++++++++++++++++++++++++++++++++
>  1 file changed, 101 insertions(+)
>  create mode 100644 include/dt-bindings/leds/functions.h
>
> diff --git a/include/dt-bindings/leds/functions.h b/include/dt-bindings/leds/functions.h
> new file mode 100644
> index 0000000..3f94e09
> --- /dev/null
> +++ b/include/dt-bindings/leds/functions.h
> @@ -0,0 +1,101 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#define LED_FUNCTION_2G "2g"
> +#define LED_FUNCTION_ACTIVITY "activity"
> +#define LED_FUNCTION_ADSL "adsl"
> +#define LED_FUNCTION_ALARM "alarm"
> +#define LED_FUNCTION_ALIVE "alive"
> +#define LED_FUNCTION_ALL "all"
> +#define LED_FUNCTION_APP "app"
> +#define LED_FUNCTION_AUX "aux"
> +#define LED_FUNCTION_BACKUP "backup"
> +#define LED_FUNCTION_BACKLIGHT "backlight"
> +#define LED_FUNCTION_BACKLIGHT_CLUSTER "backlight_cluster"

I'd prefer we go further and only define 1 endorsed function name.
Backlight, disk, network, fault/error all seem to have a few
duplicates. I'm also thinking the functions make linux,default-trigger
redundant in some cases.

I'm not a huge fan of making these defines, but it does make it
perfectly clear if folks use standard names or not.

> +#define LED_FUNCTION_BEEP "beep"
> +#define LED_FUNCTION_BLUETOOTH "bluetooth"
> +#define LED_FUNCTION_BOOT "boot"
> +#define LED_FUNCTION_BOTTOM "bottom"
> +#define LED_FUNCTION_BRICK_STATUS "brick-status"
> +#define LED_FUNCTION_CEL "cel"
> +#define LED_FUNCTION_CEL_PWR "cel-pwr"
> +#define LED_FUNCTION_CEL_RESET "cel-reset"
> +#define LED_FUNCTION_CHRG "chrg"
> +#define LED_FUNCTION_COM "com"
> +#define LED_FUNCTION_COPY "copy"
> +#define LED_FUNCTION_CPU "cpu"
> +#define LED_FUNCTION_DEBUG "debug"
> +#define LED_FUNCTION_DIA "dia"
> +#define LED_FUNCTION_DISK "disk"
> +#define LED_FUNCTION_DISPLAY_CLUSTER "display_cluster"
> +#define LED_FUNCTION_DOWN "down"
> +#define LED_FUNCTION_DSL "dsl"
> +#define LED_FUNCTION_ENOCEAN "enocean"
> +#define LED_FUNCTION_ENTER "enter"
> +#define LED_FUNCTION_ERROR "error"
> +#define LED_FUNCTION_ESATA "esata"
> +#define LED_FUNCTION_ETHERNET_STATUS "ethernet-status"
> +#define LED_FUNCTION_FAIL "fail"
> +#define LED_FUNCTION_FAULT "fault"
> +#define LED_FUNCTION_FLASH "flash"
> +#define LED_FUNCTION_FRONT "front"
> +#define LED_FUNCTION_FUNC "func"
> +#define LED_FUNCTION_GPIO "gpio"
> +#define LED_FUNCTION_GSM "gsm"
> +#define LED_FUNCTION_HD "hd"
> +#define LED_FUNCTION_HDD "hdd"
> +#define LED_FUNCTION_HDDERR "hdderr"
> +#define LED_FUNCTION_HEALTH "health"
> +#define LED_FUNCTION_HEARTBEAT "heartbeat"
> +#define LED_FUNCTION_HOME "home"
> +#define LED_FUNCTION_INDICATOR "indicator"
> +#define LED_FUNCTION_INET "inet"
> +#define LED_FUNCTION_INFO "info"
> +#define LED_FUNCTION_INTERNET "internet"
> +#define LED_FUNCTION_KEYPAD "keypad"
> +#define LED_FUNCTION_LAN "lan"
> +#define LED_FUNCTION_LEFT "left"
> +#define LED_FUNCTION_LIVE "live"
> +#define LED_FUNCTION_MICROSD "microsd"
> +#define LED_FUNCTION_MISC "misc"
> +#define LED_FUNCTION_MMC "mmc"
> +#define LED_FUNCTION_NAND "nand"
> +#define LED_FUNCTION_NETWORK "network"
> +#define LED_FUNCTION_ON "on"
> +#define LED_FUNCTION_OS "os"
> +#define LED_FUNCTION_PANEL "panel"
> +#define LED_FUNCTION_PMU_STAT "pmu_stat"
> +#define LED_FUNCTION_PROG "prog"
> +#define LED_FUNCTION_PROGRAMMING "programming"
> +#define LED_FUNCTION_PULSE "pulse"
> +#define LED_FUNCTION_PWR "pwr"
> +#define LED_FUNCTION_QSS "qss"
> +#define LED_FUNCTION_REAR "rear"
> +#define LED_FUNCTION_REBUILD "rebuild"
> +#define LED_FUNCTION_RIGHT "right"
> +#define LED_FUNCTION_ROUTER "router"
> +#define LED_FUNCTION_RS "rs"
> +#define LED_FUNCTION_RX "rx"
> +#define LED_FUNCTION_SATA "sata"
> +#define LED_FUNCTION_SD "sd"
> +#define LED_FUNCTION_SLEEP "sleep"
> +#define LED_FUNCTION_STANDBY "standby"
> +#define LED_FUNCTION_STATUS "status"
> +#define LED_FUNCTION_SW "sw"
> +#define LED_FUNCTION_SWRDY "swrdy"
> +#define LED_FUNCTION_SYSTEM "system"
> +#define LED_FUNCTION_TEL "tel"
> +#define LED_FUNCTION_TOP "top"
> +#define LED_FUNCTION_TORCH "torch"
> +#define LED_FUNCTION_TV "tv"
> +#define LED_FUNCTION_TX "tx"
> +#define LED_FUNCTION_UP "up"
> +#define LED_FUNCTION_USB "usb"
> +#define LED_FUNCTION_USB_COPY "usb_copy"
> +#define LED_FUNCTION_USER "user"
> +#define LED_FUNCTION_USR "usr"
> +#define LED_FUNCTION_WAN "wan"
> +#define LED_FUNCTION_WIFI "wifi"
> +#define LED_FUNCTION_WIRELESS "wireless"
> +#define LED_FUNCTION_WLAN "wlan"
> +#define LED_FUNCTION_WMODE "wmode"
> +#define LED_FUNCTION_WPS "wps"
> --
> 2.1.4
>

Hi Rob,

Thanks for the review.

On 11/08/2018 04:13 PM, Rob Herring wrote:
> On Tue, Nov 6, 2018 at 4:07 PM Jacek Anaszewski
> <jacek.anaszewski@gmail.com> wrote:
>>
>> Add common LED function definitions for use in Device Tree.
>> The function names were extracted from existing dts files
>> after eliminating oddities.
>>
>> Signed-off-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
>> Cc: Baolin Wang <baolin.wang@linaro.org>
>> Cc: Daniel Mack <daniel@zonque.org>
>> Cc: Dan Murphy <dmurphy@ti.com>
>> Cc: Linus Walleij <linus.walleij@linaro.org>
>> Cc: Oleh Kravchenko <oleg@kaa.org.ua>
>> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
>> Cc: Simon Shields <simon@lineageos.org>
>> Cc: Xiaotong Lu <xiaotong.lu@spreadtrum.com>
>> ---
>>  include/dt-bindings/leds/functions.h | 101 +++++++++++++++++++++++++++++++++++
>>  1 file changed, 101 insertions(+)
>>  create mode 100644 include/dt-bindings/leds/functions.h
>>
>> diff --git a/include/dt-bindings/leds/functions.h b/include/dt-bindings/leds/functions.h
>> new file mode 100644
>> index 0000000..3f94e09
>> --- /dev/null
>> +++ b/include/dt-bindings/leds/functions.h
>> @@ -0,0 +1,101 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +#define LED_FUNCTION_2G "2g"
>> +#define LED_FUNCTION_ACTIVITY "activity"
>> +#define LED_FUNCTION_ADSL "adsl"
>> +#define LED_FUNCTION_ALARM "alarm"
>> +#define LED_FUNCTION_ALIVE "alive"
>> +#define LED_FUNCTION_ALL "all"
>> +#define LED_FUNCTION_APP "app"
>> +#define LED_FUNCTION_AUX "aux"
>> +#define LED_FUNCTION_BACKUP "backup"
>> +#define LED_FUNCTION_BACKLIGHT "backlight"
>> +#define LED_FUNCTION_BACKLIGHT_CLUSTER "backlight_cluster"
> 
> I'd prefer we go further and only define 1 endorsed function name.
> Backlight, disk, network, fault/error all seem to have a few

OK, I'll try to remove semantically akin duplicates.

> duplicates. I'm also thinking the functions make linux,default-trigger
> redundant in some cases.

Yes, this is tempting. Needs more thorough analysis, though.

> I'm not a huge fan of making these defines, but it does make it
> perfectly clear if folks use standard names or not.
> 
>> +#define LED_FUNCTION_BEEP "beep"
>> +#define LED_FUNCTION_BLUETOOTH "bluetooth"
>> +#define LED_FUNCTION_BOOT "boot"
>> +#define LED_FUNCTION_BOTTOM "bottom"
>> +#define LED_FUNCTION_BRICK_STATUS "brick-status"
>> +#define LED_FUNCTION_CEL "cel"
>> +#define LED_FUNCTION_CEL_PWR "cel-pwr"
>> +#define LED_FUNCTION_CEL_RESET "cel-reset"
>> +#define LED_FUNCTION_CHRG "chrg"
>> +#define LED_FUNCTION_COM "com"
>> +#define LED_FUNCTION_COPY "copy"
>> +#define LED_FUNCTION_CPU "cpu"
>> +#define LED_FUNCTION_DEBUG "debug"
>> +#define LED_FUNCTION_DIA "dia"
>> +#define LED_FUNCTION_DISK "disk"
>> +#define LED_FUNCTION_DISPLAY_CLUSTER "display_cluster"
>> +#define LED_FUNCTION_DOWN "down"
>> +#define LED_FUNCTION_DSL "dsl"
>> +#define LED_FUNCTION_ENOCEAN "enocean"
>> +#define LED_FUNCTION_ENTER "enter"
>> +#define LED_FUNCTION_ERROR "error"
>> +#define LED_FUNCTION_ESATA "esata"
>> +#define LED_FUNCTION_ETHERNET_STATUS "ethernet-status"
>> +#define LED_FUNCTION_FAIL "fail"
>> +#define LED_FUNCTION_FAULT "fault"
>> +#define LED_FUNCTION_FLASH "flash"
>> +#define LED_FUNCTION_FRONT "front"
>> +#define LED_FUNCTION_FUNC "func"
>> +#define LED_FUNCTION_GPIO "gpio"
>> +#define LED_FUNCTION_GSM "gsm"
>> +#define LED_FUNCTION_HD "hd"
>> +#define LED_FUNCTION_HDD "hdd"
>> +#define LED_FUNCTION_HDDERR "hdderr"
>> +#define LED_FUNCTION_HEALTH "health"
>> +#define LED_FUNCTION_HEARTBEAT "heartbeat"
>> +#define LED_FUNCTION_HOME "home"
>> +#define LED_FUNCTION_INDICATOR "indicator"
>> +#define LED_FUNCTION_INET "inet"
>> +#define LED_FUNCTION_INFO "info"
>> +#define LED_FUNCTION_INTERNET "internet"
>> +#define LED_FUNCTION_KEYPAD "keypad"
>> +#define LED_FUNCTION_LAN "lan"
>> +#define LED_FUNCTION_LEFT "left"
>> +#define LED_FUNCTION_LIVE "live"
>> +#define LED_FUNCTION_MICROSD "microsd"
>> +#define LED_FUNCTION_MISC "misc"
>> +#define LED_FUNCTION_MMC "mmc"
>> +#define LED_FUNCTION_NAND "nand"
>> +#define LED_FUNCTION_NETWORK "network"
>> +#define LED_FUNCTION_ON "on"
>> +#define LED_FUNCTION_OS "os"
>> +#define LED_FUNCTION_PANEL "panel"
>> +#define LED_FUNCTION_PMU_STAT "pmu_stat"
>> +#define LED_FUNCTION_PROG "prog"
>> +#define LED_FUNCTION_PROGRAMMING "programming"
>> +#define LED_FUNCTION_PULSE "pulse"
>> +#define LED_FUNCTION_PWR "pwr"
>> +#define LED_FUNCTION_QSS "qss"
>> +#define LED_FUNCTION_REAR "rear"
>> +#define LED_FUNCTION_REBUILD "rebuild"
>> +#define LED_FUNCTION_RIGHT "right"
>> +#define LED_FUNCTION_ROUTER "router"
>> +#define LED_FUNCTION_RS "rs"
>> +#define LED_FUNCTION_RX "rx"
>> +#define LED_FUNCTION_SATA "sata"
>> +#define LED_FUNCTION_SD "sd"
>> +#define LED_FUNCTION_SLEEP "sleep"
>> +#define LED_FUNCTION_STANDBY "standby"
>> +#define LED_FUNCTION_STATUS "status"
>> +#define LED_FUNCTION_SW "sw"
>> +#define LED_FUNCTION_SWRDY "swrdy"
>> +#define LED_FUNCTION_SYSTEM "system"
>> +#define LED_FUNCTION_TEL "tel"
>> +#define LED_FUNCTION_TOP "top"
>> +#define LED_FUNCTION_TORCH "torch"
>> +#define LED_FUNCTION_TV "tv"
>> +#define LED_FUNCTION_TX "tx"
>> +#define LED_FUNCTION_UP "up"
>> +#define LED_FUNCTION_USB "usb"
>> +#define LED_FUNCTION_USB_COPY "usb_copy"
>> +#define LED_FUNCTION_USER "user"
>> +#define LED_FUNCTION_USR "usr"
>> +#define LED_FUNCTION_WAN "wan"
>> +#define LED_FUNCTION_WIFI "wifi"
>> +#define LED_FUNCTION_WIRELESS "wireless"
>> +#define LED_FUNCTION_WLAN "wlan"
>> +#define LED_FUNCTION_WMODE "wmode"
>> +#define LED_FUNCTION_WPS "wps"
>> --
>> 2.1.4
>>
>

Hi!

> Add common LED function definitions for use in Device Tree.
> The function names were extracted from existing dts files
> after eliminating oddities.

Thanks for doing this.

> diff --git a/include/dt-bindings/leds/functions.h b/include/dt-bindings/leds/functions.h
> new file mode 100644
> index 0000000..3f94e09
> --- /dev/null
> +++ b/include/dt-bindings/leds/functions.h
> @@ -0,0 +1,101 @@
> +// SPDX-License-Identifier: GPL-2.0
> +

> +#define LED_FUNCTION_2G "2g"
> +#define LED_FUNCTION_ACTIVITY "activity"
> +#define LED_FUNCTION_ADSL "adsl"
> +#define LED_FUNCTION_ALARM "alarm"
> +#define LED_FUNCTION_ALIVE "alive"
> +#define LED_FUNCTION_ALL "all"
> +#define LED_FUNCTION_APP "app"
> +#define LED_FUNCTION_AUX "aux"
> +#define LED_FUNCTION_BACKUP "backup"
> +#define LED_FUNCTION_BACKLIGHT "backlight"
> +#define LED_FUNCTION_BACKLIGHT_CLUSTER "backlight_cluster"

Sounds like one of backlight and backlight_cluster should be deprecated?

> +#define LED_FUNCTION_BEEP "beep"
> +#define LED_FUNCTION_BLUETOOTH "bluetooth"
> +#define LED_FUNCTION_BOOT "boot"
> +#define LED_FUNCTION_BOTTOM "bottom"

I believe some of these should get comments. For example what does
"all" and "bottom" LEDs do?

> +#define LED_FUNCTION_BRICK_STATUS "brick-status"
> +#define LED_FUNCTION_CEL "cel"
> +#define LED_FUNCTION_CEL_PWR "cel-pwr"
> +#define LED_FUNCTION_CEL_RESET "cel-reset"
> +#define LED_FUNCTION_CHRG "chrg"
> +#define LED_FUNCTION_COM "com"
> +#define LED_FUNCTION_COPY "copy"
> +#define LED_FUNCTION_CPU "cpu"
> +#define LED_FUNCTION_DEBUG "debug"
> +#define LED_FUNCTION_DIA "dia"

What does this one do?

> +#define LED_FUNCTION_DISK "disk"

We have disk, hd, hdd and sata. Deprecate some?

> +#define LED_FUNCTION_DISPLAY_CLUSTER "display_cluster"
> +#define LED_FUNCTION_DOWN "down"
> +#define LED_FUNCTION_DSL "dsl"
> +#define LED_FUNCTION_ENOCEAN "enocean"
> +#define LED_FUNCTION_ENTER "enter"
> +#define LED_FUNCTION_ERROR "error"
> +#define LED_FUNCTION_ESATA "esata"
> +#define LED_FUNCTION_ETHERNET_STATUS "ethernet-status"
> +#define LED_FUNCTION_FAIL "fail"
> +#define LED_FUNCTION_FAULT "fault"
> +#define LED_FUNCTION_FLASH "flash"
> +#define LED_FUNCTION_FRONT "front"
> +#define LED_FUNCTION_FUNC "func"
> +#define LED_FUNCTION_GPIO "gpio"
> +#define LED_FUNCTION_GSM "gsm"

> +#define LED_FUNCTION_HD "hd"
> +#define LED_FUNCTION_HDD "hdd"
> +#define LED_FUNCTION_HDDERR "hdderr"

Ok, I'll 

> +#define LED_FUNCTION_HEALTH "health"
> +#define LED_FUNCTION_HEARTBEAT "heartbeat"

Sounds same as alive, deprecate alive?

> +#define LED_FUNCTION_WIFI "wifi"
> +#define LED_FUNCTION_WIRELESS "wireless"
> +#define LED_FUNCTION_WLAN "wlan"

Same as wifi and wireless, I guess, deprecate some?

I'll touch same subject in another email.
									Pavel

Hi Pavel,

Thanks for the review.

On 11/11/2018 12:31 PM, Pavel Machek wrote:
> Hi!
> 
>> Add common LED function definitions for use in Device Tree.
>> The function names were extracted from existing dts files
>> after eliminating oddities.
> 
> Thanks for doing this.
> 
>> diff --git a/include/dt-bindings/leds/functions.h b/include/dt-bindings/leds/functions.h
>> new file mode 100644
>> index 0000000..3f94e09
>> --- /dev/null
>> +++ b/include/dt-bindings/leds/functions.h
>> @@ -0,0 +1,101 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
> 
>> +#define LED_FUNCTION_2G "2g"
>> +#define LED_FUNCTION_ACTIVITY "activity"
>> +#define LED_FUNCTION_ADSL "adsl"
>> +#define LED_FUNCTION_ALARM "alarm"
>> +#define LED_FUNCTION_ALIVE "alive"
>> +#define LED_FUNCTION_ALL "all"
>> +#define LED_FUNCTION_APP "app"
>> +#define LED_FUNCTION_AUX "aux"
>> +#define LED_FUNCTION_BACKUP "backup"
>> +#define LED_FUNCTION_BACKLIGHT "backlight"
>> +#define LED_FUNCTION_BACKLIGHT_CLUSTER "backlight_cluster"
> 
> Sounds like one of backlight and backlight_cluster should be deprecated?

I think so.

>> +#define LED_FUNCTION_BEEP "beep"
>> +#define LED_FUNCTION_BLUETOOTH "bluetooth"
>> +#define LED_FUNCTION_BOOT "boot"
>> +#define LED_FUNCTION_BOTTOM "bottom"
> 
> I believe some of these should get comments. For example what does
> "all" and "bottom" LEDs do?

It would be best to ask the author, but for now,
I'd just remove all of "bottom", "up", "left", "right"
etc.

>> +#define LED_FUNCTION_BRICK_STATUS "brick-status"
>> +#define LED_FUNCTION_CEL "cel"
>> +#define LED_FUNCTION_CEL_PWR "cel-pwr"
>> +#define LED_FUNCTION_CEL_RESET "cel-reset"
>> +#define LED_FUNCTION_CHRG "chrg"
>> +#define LED_FUNCTION_COM "com"
>> +#define LED_FUNCTION_COPY "copy"
>> +#define LED_FUNCTION_CPU "cpu"
>> +#define LED_FUNCTION_DEBUG "debug"
>> +#define LED_FUNCTION_DIA "dia"
> 
> What does this one do?

I'd opt for something like "diagnostics", but this is a blind shot.
Let's remove it then and, and let people add more meaningful
definition in case it is needed.

>> +#define LED_FUNCTION_DISK "disk"
> 
> We have disk, hd, hdd and sata. Deprecate some?

Disk should be the best choice, especially given that we have
identically named trigger.

>> +#define LED_FUNCTION_DISPLAY_CLUSTER "display_cluster"
>> +#define LED_FUNCTION_DOWN "down"
>> +#define LED_FUNCTION_DSL "dsl"
>> +#define LED_FUNCTION_ENOCEAN "enocean"
>> +#define LED_FUNCTION_ENTER "enter"
>> +#define LED_FUNCTION_ERROR "error"
>> +#define LED_FUNCTION_ESATA "esata"
>> +#define LED_FUNCTION_ETHERNET_STATUS "ethernet-status"
>> +#define LED_FUNCTION_FAIL "fail"
>> +#define LED_FUNCTION_FAULT "fault"
>> +#define LED_FUNCTION_FLASH "flash"
>> +#define LED_FUNCTION_FRONT "front"
>> +#define LED_FUNCTION_FUNC "func"
>> +#define LED_FUNCTION_GPIO "gpio"
>> +#define LED_FUNCTION_GSM "gsm"
> 
>> +#define LED_FUNCTION_HD "hd"
>> +#define LED_FUNCTION_HDD "hdd"
>> +#define LED_FUNCTION_HDDERR "hdderr"
> 
> Ok, I'll 

Hmm?

> 
>> +#define LED_FUNCTION_HEALTH "health"
>> +#define LED_FUNCTION_HEARTBEAT "heartbeat"
> 
> Sounds same as alive, deprecate alive?

Heartbeat may be designated specifically for registration
for events from the trigger with the same name.

>> +#define LED_FUNCTION_WIFI "wifi"
>> +#define LED_FUNCTION_WIRELESS "wireless"
>> +#define LED_FUNCTION_WLAN "wlan"
> 
> Same as wifi and wireless, I guess, deprecate some?

I'd remove "wireless" and "wlan".

> I'll touch same subject in another email.
> 									Pavel
>

Hi!

> >> +#define LED_FUNCTION_BACKLIGHT "backlight"
> >> +#define LED_FUNCTION_BACKLIGHT_CLUSTER "backlight_cluster"
> > 
> > Sounds like one of backlight and backlight_cluster should be deprecated?
> 
> I think so.

Agreed.

> >> +#define LED_FUNCTION_DEBUG "debug"
> >> +#define LED_FUNCTION_DIA "dia"
> > 
> > What does this one do?
> 
> I'd opt for something like "diagnostics", but this is a blind shot.
> Let's remove it then and, and let people add more meaningful
> definition in case it is needed.
> 
> >> +#define LED_FUNCTION_DISK "disk"
> > 
> > We have disk, hd, hdd and sata. Deprecate some?
> 
> Disk should be the best choice, especially given that we have
> identically named trigger.

Ok.

> >> +#define LED_FUNCTION_HD "hd"
> >> +#define LED_FUNCTION_HDD "hdd"
> >> +#define LED_FUNCTION_HDDERR "hdderr"
> > 
> > Ok, I'll 
> 
> Hmm?

I was going to say that I'll bring it up in different email.

I believe we should have disk:green:activity and disk:red:error, not
"green:hdd" and "red:hdderr".

> >> +#define LED_FUNCTION_HEALTH "health"
> >> +#define LED_FUNCTION_HEARTBEAT "heartbeat"
> > 
> > Sounds same as alive, deprecate alive?
> 
> Heartbeat may be designated specifically for registration
> for events from the trigger with the same name.

Ok. What is "alive" then?

> >> +#define LED_FUNCTION_WIFI "wifi"
> >> +#define LED_FUNCTION_WIRELESS "wireless"
> >> +#define LED_FUNCTION_WLAN "wlan"
> > 
> > Same as wifi and wireless, I guess, deprecate some?
> 
> I'd remove "wireless" and "wlan".

Actually I'd keep wlan, but... :-).

We may want to do add some comments there, and sort it "most
common/recommended first" or something.

Best regards (and thanks for doing the work),
									Pavel

On 11/11/2018 09:20 PM, Pavel Machek wrote:
> Hi!
> 
>>>> +#define LED_FUNCTION_BACKLIGHT "backlight"
>>>> +#define LED_FUNCTION_BACKLIGHT_CLUSTER "backlight_cluster"
>>>
>>> Sounds like one of backlight and backlight_cluster should be deprecated?
>>
>> I think so.
> 
> Agreed.
> 
>>>> +#define LED_FUNCTION_DEBUG "debug"
>>>> +#define LED_FUNCTION_DIA "dia"
>>>
>>> What does this one do?
>>
>> I'd opt for something like "diagnostics", but this is a blind shot.
>> Let's remove it then and, and let people add more meaningful
>> definition in case it is needed.
>>
>>>> +#define LED_FUNCTION_DISK "disk"
>>>
>>> We have disk, hd, hdd and sata. Deprecate some?
>>
>> Disk should be the best choice, especially given that we have
>> identically named trigger.
> 
> Ok.
> 
>>>> +#define LED_FUNCTION_HD "hd"
>>>> +#define LED_FUNCTION_HDD "hdd"
>>>> +#define LED_FUNCTION_HDDERR "hdderr"
>>>
>>> Ok, I'll 
>>
>> Hmm?
> 
> I was going to say that I'll bring it up in different email.
> 
> I believe we should have disk:green:activity and disk:red:error, not
> "green:hdd" and "red:hdderr".

How would you propose to name the section corresponding to "disk",
if "function" is occupied by "activity"? Should it deserve a separate
DT property?

>>>> +#define LED_FUNCTION_HEALTH "health"
>>>> +#define LED_FUNCTION_HEARTBEAT "heartbeat"
>>>
>>> Sounds same as alive, deprecate alive?
>>
>> Heartbeat may be designated specifically for registration
>> for events from the trigger with the same name.
> 
> Ok. What is "alive" then?

Tells whether something is active or not?
In this case it seems to overlap with "pwr" probably.

>>>> +#define LED_FUNCTION_WIFI "wifi"
>>>> +#define LED_FUNCTION_WIRELESS "wireless"
>>>> +#define LED_FUNCTION_WLAN "wlan"
>>>
>>> Same as wifi and wireless, I guess, deprecate some?
>>
>> I'd remove "wireless" and "wlan".
> 
> Actually I'd keep wlan, but... :-).

It may depend of the regional preferences.

> We may want to do add some comments there, and sort it "most
> common/recommended first" or something.
> 
> Best regards (and thanks for doing the work),

I'm doing it also for myself to avoid extra lines for
explaining the LED naming quirks every time a new driver
is submitted :-)

Hi Jacek,

On 07/11/2018 0.07, Jacek Anaszewski wrote:
> Add common LED function definitions for use in Device Tree.
> The function names were extracted from existing dts files
> after eliminating oddities.

Is your intent here is to standardize the function definitions and to 
aid in that is to specify list of string defines?

Without a meaning what all of those mean it does complete the original goal.

In your list there are many things that could easily have multiple 
meanings for different audiences.

Some examples:

#define LED_FUNCTION_2G "2g"
- Does this mean that 2 metric grams has been detected in scale or 
cellular 2G connectivity?

#define LED_FUNCTION_ALL "all"
- This doesn't ring a bell to me what it could be in reality. All leds 
on doesn't sound right.

#define LED_FUNCTION_AUX "aux"
- There can be many things aux and multiple aux things in one device.

#define LED_FUNCTION_HD "hd"
- Is there a high definition video playing? Or audio? Or harddisk 
failure led?

You have already come up with long list of items. I am just wondering 
what is the logic in order to get to "common" list?

Can you just add custom items in device tree without being in the list?

Would it be better to start with a short simple list with meanings 
defined properly?

When do you then remove entries from the list? Let's say 3G networks are 
currently getting turned off world wide which kinda deprecates the term 
from definitions and probably should be then removed from the list (if 
it would be there).

Is there planned to be some auto connection from function to some other 
automated functionality? Or why wouldn't the label keyword be enough as 
it seems to be exactly the same thing? (without the common list -- which 
could be implemented for label too if seen as a good thing)

Thanks,
Vesa Jääskeläinen

Hi Vesa,

On 11/12/2018 01:25 AM, Vesa Jääskeläinen wrote:
> Hi Jacek,
> 
> On 07/11/2018 0.07, Jacek Anaszewski wrote:
>> Add common LED function definitions for use in Device Tree.
>> The function names were extracted from existing dts files
>> after eliminating oddities.
> 
> Is your intent here is to standardize the function definitions and to
> aid in that is to specify list of string defines?

The calls for LED names standardization emerged some time go, and this
is just a submission of the subject for a discussion.

> Without a meaning what all of those mean it does complete the original
> goal.
> 
> In your list there are many things that could easily have multiple
> meanings for different audiences.
> 
> Some examples:
> 
> #define LED_FUNCTION_2G "2g"
> - Does this mean that 2 metric grams has been detected in scale or
> cellular 2G connectivity?

Most probably the latter. Should we have the generic macro for it
is another question. Actually I found it in the leds-tlc591xx.txt
bindings. There are also "wlan_2g" occurrences in the armada385-linksys*
dts files.

> #define LED_FUNCTION_ALL "all"
> - This doesn't ring a bell to me what it could be in reality. All leds
> on doesn't sound right.

Yeah, I must admit I didn't spend too much time assessing how much
all of them make sense. I just wanted we had more complete picture
of what functions are pre-existing in the bindings and dts files.

> #define LED_FUNCTION_AUX "aux"
> - There can be many things aux and multiple aux things in one device.
> 
> #define LED_FUNCTION_HD "hd"
> - Is there a high definition video playing? Or audio? Or harddisk
> failure led?
> 
> You have already come up with long list of items. I am just wondering
> what is the logic in order to get to "common" list?
> 
> Can you just add custom items in device tree without being in the list?
> 
> Would it be better to start with a short simple list with meanings
> defined properly?

This seems to be the most reasonable approach. My first thought was
to provide function definitions for as many as possible from the
pre-existing ones, but it is not making too much sense as it tunes out.

> When do you then remove entries from the list? Let's say 3G networks are
> currently getting turned off world wide which kinda deprecates the term
> from definitions and probably should be then removed from the list (if
> it would be there).
> 
> Is there planned to be some auto connection from function to some other
> automated functionality? 

One proposition was to register the LED for a trigger events basing on
the function name, like e.g "heartbeat".

> Or why wouldn't the label keyword be enough as
> it seems to be exactly the same thing? (without the common list -- which
> could be implemented for label too if seen as a good thing)

LED_FUNCTION definitions are not meant to be limited for use only
with color:name scheme, if that's what you had on mind.

On Tue, Nov 6, 2018 at 11:07 PM Jacek Anaszewski
<jacek.anaszewski@gmail.com> wrote:

> +#define LED_FUNCTION_DISK "disk"

I recently added triggers for "disk-read" and "disk-write".
This was because the D-Link DNS-313 does have dedicated
LEDs for each usecase, and so does DIR-685.

So I think you may need
LED_FUNCTION_DISK_READ "disk-read"
and
LED_FUNCTION_DISK_WRITE "disk-write"
to cover these cases.

Yours,
Linus Walleij