Message ID | 20190310182836.20841-4-jacek.anaszewski@gmail.com |
---|---|
State | Changes Requested, archived |
Headers | show |
Series | Add generic support for composing LED class device name | expand |
Context | Check | Description |
---|---|---|
robh/checkpatch | success |
Jacek On 3/10/19 1:28 PM, 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> > --- > include/dt-bindings/leds/common.h | 38 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 38 insertions(+) > > diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h > index e171d0a6beb2..ffcd46317307 100644 > --- a/include/dt-bindings/leds/common.h > +++ b/include/dt-bindings/leds/common.h > @@ -19,4 +19,42 @@ > #define LEDS_BOOST_ADAPTIVE 1 > #define LEDS_BOOST_FIXED 2 > > +/* Standard LED functions */ > +#define LED_FUNCTION_ACTIVITY "activity" > +#define LED_FUNCTION_ADSL "adsl" > +#define LED_FUNCTION_ALARM "alarm" > +#define LED_FUNCTION_BACKLIGHT "backlight" > +#define LED_FUNCTION_BLUETOOTH "bluetooth" > +#define LED_FUNCTION_BOOT "boot" > +#define LED_FUNCTION_CHRG "chrg" Why not spell out charge? > +#define LED_FUNCTION_DEBUG "debug" > +#define LED_FUNCTION_DISK "disk" > +#define LED_FUNCTION_DISK_READ "disk-read" > +#define LED_FUNCTION_DISK_WRITE "disk-write" > +#define LED_FUNCTION_FAULT "fault" > +#define LED_FUNCTION_FLASH "flash" > +#define LED_FUNCTION_HDDERR "hdderr" > +#define LED_FUNCTION_HEARTBEAT "heartbeat" > +#define LED_FUNCTION_INDICATOR "indicator" > +#define LED_FUNCTION_INFO "info" > +#define LED_FUNCTION_INTERNET "internet" LEDs function can be for keyboards and keypads. Keypad for hand held devices #define LED_FUNCTION_KEYBOARD "keyboard" #define LED_FUNCTION_KEYPAD "keypad" > +#define LED_FUNCTION_LAN "lan" > +#define LED_FUNCTION_MMC "mmc" > +#define LED_FUNCTION_NAND "nand" > +#define LED_FUNCTION_ON "on" > +#define LED_FUNCTION_PROGRAMMING "programming" > +#define LED_FUNCTION_PWR "pwr" Same comment as above on chrg > +#define LED_FUNCTION_RX "rx" > +#define LED_FUNCTION_SD "sd" > +#define LED_FUNCTION_SLEEP "sleep" > +#define LED_FUNCTION_STANDBY "standby" > +#define LED_FUNCTION_STATUS "status" > +#define LED_FUNCTION_TORCH "torch" > +#define LED_FUNCTION_TV "tv" > +#define LED_FUNCTION_TX "tx" > +#define LED_FUNCTION_USB "usb" > +#define LED_FUNCTION_WAN "wan" > +#define LED_FUNCTION_WLAN "wlan" > +#define LED_FUNCTION_WPS "wps" > + Dan > #endif /* __DT_BINDINGS_LEDS_H */ >
Hi Dan, Thanks for the review. On 3/11/19 1:22 PM, Dan Murphy wrote: > Jacek > > On 3/10/19 1:28 PM, 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> >> --- >> include/dt-bindings/leds/common.h | 38 ++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 38 insertions(+) >> >> diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h >> index e171d0a6beb2..ffcd46317307 100644 >> --- a/include/dt-bindings/leds/common.h >> +++ b/include/dt-bindings/leds/common.h >> @@ -19,4 +19,42 @@ >> #define LEDS_BOOST_ADAPTIVE 1 >> #define LEDS_BOOST_FIXED 2 >> >> +/* Standard LED functions */ >> +#define LED_FUNCTION_ACTIVITY "activity" >> +#define LED_FUNCTION_ADSL "adsl" >> +#define LED_FUNCTION_ALARM "alarm" >> +#define LED_FUNCTION_BACKLIGHT "backlight" >> +#define LED_FUNCTION_BLUETOOTH "bluetooth" >> +#define LED_FUNCTION_BOOT "boot" >> +#define LED_FUNCTION_CHRG "chrg" > > Why not spell out charge? It was in one of existing dts files. I have no issue with that, will change it to "charge". > >> +#define LED_FUNCTION_DEBUG "debug" >> +#define LED_FUNCTION_DISK "disk" >> +#define LED_FUNCTION_DISK_READ "disk-read" >> +#define LED_FUNCTION_DISK_WRITE "disk-write" >> +#define LED_FUNCTION_FAULT "fault" >> +#define LED_FUNCTION_FLASH "flash" >> +#define LED_FUNCTION_HDDERR "hdderr" >> +#define LED_FUNCTION_HEARTBEAT "heartbeat" >> +#define LED_FUNCTION_INDICATOR "indicator" >> +#define LED_FUNCTION_INFO "info" >> +#define LED_FUNCTION_INTERNET "internet" > > LEDs function can be for keyboards and keypads. > > Keypad for hand held devices > > #define LED_FUNCTION_KEYBOARD "keyboard" > #define LED_FUNCTION_KEYPAD "keypad" Ack. > >> +#define LED_FUNCTION_LAN "lan" >> +#define LED_FUNCTION_MMC "mmc" >> +#define LED_FUNCTION_NAND "nand" >> +#define LED_FUNCTION_ON "on" >> +#define LED_FUNCTION_PROGRAMMING "programming" >> +#define LED_FUNCTION_PWR "pwr" > > Same comment as above on chrg Ack. >> +#define LED_FUNCTION_RX "rx" >> +#define LED_FUNCTION_SD "sd" >> +#define LED_FUNCTION_SLEEP "sleep" >> +#define LED_FUNCTION_STANDBY "standby" >> +#define LED_FUNCTION_STATUS "status" >> +#define LED_FUNCTION_TORCH "torch" >> +#define LED_FUNCTION_TV "tv" >> +#define LED_FUNCTION_TX "tx" >> +#define LED_FUNCTION_USB "usb" >> +#define LED_FUNCTION_WAN "wan" >> +#define LED_FUNCTION_WLAN "wlan" >> +#define LED_FUNCTION_WPS "wps" >> + > > Dan > >> #endif /* __DT_BINDINGS_LEDS_H */ >> > >
On Sun, Mar 10, 2019 at 07:28:14PM +0100, 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. I'd like this to be suggestions of what to use rather than what's already out there. So my comments are in that context. > > 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> > --- > include/dt-bindings/leds/common.h | 38 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 38 insertions(+) > > diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h > index e171d0a6beb2..ffcd46317307 100644 > --- a/include/dt-bindings/leds/common.h > +++ b/include/dt-bindings/leds/common.h > @@ -19,4 +19,42 @@ > #define LEDS_BOOST_ADAPTIVE 1 > #define LEDS_BOOST_FIXED 2 > > +/* Standard LED functions */ > +#define LED_FUNCTION_ACTIVITY "activity" of what? > +#define LED_FUNCTION_ADSL "adsl" wan? > +#define LED_FUNCTION_ALARM "alarm" > +#define LED_FUNCTION_BACKLIGHT "backlight" > +#define LED_FUNCTION_BLUETOOTH "bluetooth" > +#define LED_FUNCTION_BOOT "boot" What about boot? > +#define LED_FUNCTION_CHRG "chrg" > +#define LED_FUNCTION_DEBUG "debug" > +#define LED_FUNCTION_DISK "disk" > +#define LED_FUNCTION_DISK_READ "disk-read" > +#define LED_FUNCTION_DISK_WRITE "disk-write" > +#define LED_FUNCTION_FAULT "fault" > +#define LED_FUNCTION_FLASH "flash" > +#define LED_FUNCTION_HDDERR "hdderr" disk-err for consistency? > +#define LED_FUNCTION_HEARTBEAT "heartbeat" > +#define LED_FUNCTION_INDICATOR "indicator" > +#define LED_FUNCTION_INFO "info" of what? > +#define LED_FUNCTION_INTERNET "internet" Same as wan? > +#define LED_FUNCTION_LAN "lan" > +#define LED_FUNCTION_MMC "mmc" Same as disk? > +#define LED_FUNCTION_NAND "nand" > +#define LED_FUNCTION_ON "on" > +#define LED_FUNCTION_PROGRAMMING "programming" > +#define LED_FUNCTION_PWR "pwr" > +#define LED_FUNCTION_RX "rx" > +#define LED_FUNCTION_SD "sd" > +#define LED_FUNCTION_SLEEP "sleep" > +#define LED_FUNCTION_STANDBY "standby" Same things? > +#define LED_FUNCTION_STATUS "status" > +#define LED_FUNCTION_TORCH "torch" > +#define LED_FUNCTION_TV "tv" > +#define LED_FUNCTION_TX "tx" > +#define LED_FUNCTION_USB "usb" > +#define LED_FUNCTION_WAN "wan" > +#define LED_FUNCTION_WLAN "wlan" > +#define LED_FUNCTION_WPS "wps" > + > #endif /* __DT_BINDINGS_LEDS_H */ > -- > 2.11.0 >
Hi Rob, Thanks for the review. On 3/28/19 1:03 AM, Rob Herring wrote: > On Sun, Mar 10, 2019 at 07:28:14PM +0100, 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. > > I'd like this to be suggestions of what to use rather than what's > already out there. So my comments are in that context. > >> >> 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> >> --- >> include/dt-bindings/leds/common.h | 38 ++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 38 insertions(+) >> >> diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h >> index e171d0a6beb2..ffcd46317307 100644 >> --- a/include/dt-bindings/leds/common.h >> +++ b/include/dt-bindings/leds/common.h >> @@ -19,4 +19,42 @@ >> #define LEDS_BOOST_ADAPTIVE 1 >> #define LEDS_BOOST_FIXED 2 >> >> +/* Standard LED functions */ >> +#define LED_FUNCTION_ACTIVITY "activity" > > of what? We have activity trigger, which provides instant feedback on the cpu activity. This would nicely map to that. >> +#define LED_FUNCTION_ADSL "adsl" > > wan? Good point. >> +#define LED_FUNCTION_ALARM "alarm" >> +#define LED_FUNCTION_BACKLIGHT "backlight" >> +#define LED_FUNCTION_BLUETOOTH "bluetooth" >> +#define LED_FUNCTION_BOOT "boot" > > What about boot? Is lit when bootloader code is executed? Turned off just before starting the kernel. >> +#define LED_FUNCTION_CHRG "chrg" >> +#define LED_FUNCTION_DEBUG "debug" >> +#define LED_FUNCTION_DISK "disk" >> +#define LED_FUNCTION_DISK_READ "disk-read" >> +#define LED_FUNCTION_DISK_WRITE "disk-write" >> +#define LED_FUNCTION_FAULT "fault" >> +#define LED_FUNCTION_FLASH "flash" >> +#define LED_FUNCTION_HDDERR "hdderr" > > disk-err for consistency? Ack. >> +#define LED_FUNCTION_HEARTBEAT "heartbeat" >> +#define LED_FUNCTION_INDICATOR "indicator" >> +#define LED_FUNCTION_INFO "info" > > of what? Right, doesn't make sense alone. >> +#define LED_FUNCTION_INTERNET "internet" > > Same as wan? OK, to be removed. >> +#define LED_FUNCTION_LAN "lan" >> +#define LED_FUNCTION_MMC "mmc" > > Same as disk? Not sure. The possibility to have separate LEDs for different types of mass storage devices could be useful. >> +#define LED_FUNCTION_NAND "nand" >> +#define LED_FUNCTION_ON "on" >> +#define LED_FUNCTION_PROGRAMMING "programming" >> +#define LED_FUNCTION_PWR "pwr" >> +#define LED_FUNCTION_RX "rx" >> +#define LED_FUNCTION_SD "sd" > >> +#define LED_FUNCTION_SLEEP "sleep" >> +#define LED_FUNCTION_STANDBY "standby" > > Same things? Ack. Standby feels more technical so I'd remove "sleep". >> +#define LED_FUNCTION_STATUS "status" >> +#define LED_FUNCTION_TORCH "torch" >> +#define LED_FUNCTION_TV "tv" >> +#define LED_FUNCTION_TX "tx" >> +#define LED_FUNCTION_USB "usb" >> +#define LED_FUNCTION_WAN "wan" >> +#define LED_FUNCTION_WLAN "wlan" >> +#define LED_FUNCTION_WPS "wps" >> + >> #endif /* __DT_BINDINGS_LEDS_H */ >> -- >> 2.11.0 >> >
diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h index e171d0a6beb2..ffcd46317307 100644 --- a/include/dt-bindings/leds/common.h +++ b/include/dt-bindings/leds/common.h @@ -19,4 +19,42 @@ #define LEDS_BOOST_ADAPTIVE 1 #define LEDS_BOOST_FIXED 2 +/* Standard LED functions */ +#define LED_FUNCTION_ACTIVITY "activity" +#define LED_FUNCTION_ADSL "adsl" +#define LED_FUNCTION_ALARM "alarm" +#define LED_FUNCTION_BACKLIGHT "backlight" +#define LED_FUNCTION_BLUETOOTH "bluetooth" +#define LED_FUNCTION_BOOT "boot" +#define LED_FUNCTION_CHRG "chrg" +#define LED_FUNCTION_DEBUG "debug" +#define LED_FUNCTION_DISK "disk" +#define LED_FUNCTION_DISK_READ "disk-read" +#define LED_FUNCTION_DISK_WRITE "disk-write" +#define LED_FUNCTION_FAULT "fault" +#define LED_FUNCTION_FLASH "flash" +#define LED_FUNCTION_HDDERR "hdderr" +#define LED_FUNCTION_HEARTBEAT "heartbeat" +#define LED_FUNCTION_INDICATOR "indicator" +#define LED_FUNCTION_INFO "info" +#define LED_FUNCTION_INTERNET "internet" +#define LED_FUNCTION_LAN "lan" +#define LED_FUNCTION_MMC "mmc" +#define LED_FUNCTION_NAND "nand" +#define LED_FUNCTION_ON "on" +#define LED_FUNCTION_PROGRAMMING "programming" +#define LED_FUNCTION_PWR "pwr" +#define LED_FUNCTION_RX "rx" +#define LED_FUNCTION_SD "sd" +#define LED_FUNCTION_SLEEP "sleep" +#define LED_FUNCTION_STANDBY "standby" +#define LED_FUNCTION_STATUS "status" +#define LED_FUNCTION_TORCH "torch" +#define LED_FUNCTION_TV "tv" +#define LED_FUNCTION_TX "tx" +#define LED_FUNCTION_USB "usb" +#define LED_FUNCTION_WAN "wan" +#define LED_FUNCTION_WLAN "wlan" +#define LED_FUNCTION_WPS "wps" + #endif /* __DT_BINDINGS_LEDS_H */
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> --- include/dt-bindings/leds/common.h | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)