| Message ID | 20190725182818.29556-2-dmurphy@ti.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v4,1/9] leds: multicolor: Add sysfs interface definition | expand |
Hi Dan, Thank you for the v4. I have a bunch of comments below. Please take a look. On 7/25/19 8:28 PM, Dan Murphy wrote: > Add a documentation of LED Multicolor LED class specific > sysfs attributes. > > Signed-off-by: Dan Murphy <dmurphy@ti.com> > --- > .../ABI/testing/sysfs-class-led-multicolor | 67 +++++++++++++++++++ > 1 file changed, 67 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-class-led-multicolor > > diff --git a/Documentation/ABI/testing/sysfs-class-led-multicolor b/Documentation/ABI/testing/sysfs-class-led-multicolor > new file mode 100644 > index 000000000000..59839f0eae76 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-class-led-multicolor > @@ -0,0 +1,67 @@ > +What: /sys/class/leds/<led>/brightness > +Date: Sept 2019 > +KernelVersion: TBD > +Contact: Dan Murphy <dmurphy@ti.com> > +Description: read/write > + The multicolor class will redirect the device drivers call back > + function for brightness control to the multicolor class > + brightness control function. > + > + Writing to this file will update all LEDs within the group to a > + calculated percentage of what each color LED in the group is set > + to. Please refer to the leds-class-multicolor.txt in the > + Documentation directory for a complete description. Instead of redirecting the reader to led-class-multicolor.txt I'd prefer to have at least the formula to calculate the colors laid out here. Aside of that - it is more helpful to have a full path to the referenced file. > + > + The value of the color is from 0 to > + /sys/class/leds/<led>/max_brightness. > + > +What: /sys/class/leds/<led>/colors/color_mix > +Date: Sept 2019 > +KernelVersion: TBD > +Contact: Dan Murphy <dmurphy@ti.com> > +Description: read/write > + The color_mix file allows writing all registered multicolor LEDs > + virtually at the same time. The value(s) written to this file I'd drop parentheses form "value(s)". Multi color LED class device is supposed to always have more then one LED. And if I understand it correctly we have to pass intensities of all colors supported by LED multicolor class device here, even we're changing single one. > + contain the intensity values for each multicolor LED within > + the colors directory. The color indexes are reported in the > + color_id file as defined in this document. This is a bit misleading. It sounds as if single color_id file would be reporting more than one index. > + Please refer to the leds-class-multicolor.txt in the > + Documentation directory for a complete description. Here, similarly as for brightness, I would prefer to have complete documentation of this file. How about: The values written to this file should contain the intensity values of each multicolor LED within the colors directory. The index of given color is reported by the color_id file present in colors/<color> directory. The index determines the position in the sequence of intensities on which the related intensity should be passed to this file. And here we could have the examples from leds-class-multicolor.txt. > + > +What: /sys/class/leds/<led>/colors/<led_color>/color_id > +Date: Sept 2019 > +KernelVersion: TBD > +Contact: Dan Murphy <dmurphy@ti.com> > +Description: read only > + This file when read will return the index of the color in the > + color_mix. > + Please refer to the leds-class-multicolor.txt in the > + Documentation directory for a complete description. > + > +What: /sys/class/leds/<led>/colors/<led_color>/intensity > +Date: Sept 2019 > +KernelVersion: TBD > +Contact: Dan Murphy <dmurphy@ti.com> > +Description: read/write > + The led_color directory is dynamically created based on the > + colors defined by the registrar of the class. > + The led_color can be but not limited to red, green, blue, > + white, amber, yellow and violet. Drivers can also declare a Instead of this vague sentence about the available colors I propose to maintain the list of supported colors in leds-class.rst or in a separate file and keep it in sync with the led_colors array. Then we could refer to that file here. > + LED color for presentation. There is one directory per color I'd not let drivers define their custom colors. It would entail issues related to lack of generic LED_COLOR_ID and DT parsing failure. > + presented. The brightness file is created under each > + led_color directory and controls the individual LED color > + setting. > + > + The value of the color is from 0 to > + /sys/class/leds/<led>/colors/<led_color>/max_intensity. > + > +What: /sys/class/leds/<led>/colors/<led_color>/max_intensity > +Date: Sept 2019 > +KernelVersion: TBD > +Contact: Dan Murphy <dmurphy@ti.com> > +Description: read only > + Maximum intensity level for the LED color, default is > + 255 (LED_FULL). > + > + If the LED does not support different intensity levels, this > + should be 1. > -- Best regards, Jacek Anaszewski
diff --git a/Documentation/ABI/testing/sysfs-class-led-multicolor b/Documentation/ABI/testing/sysfs-class-led-multicolor new file mode 100644 index 000000000000..59839f0eae76 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-class-led-multicolor @@ -0,0 +1,67 @@ +What: /sys/class/leds/<led>/brightness +Date: Sept 2019 +KernelVersion: TBD +Contact: Dan Murphy <dmurphy@ti.com> +Description: read/write + The multicolor class will redirect the device drivers call back + function for brightness control to the multicolor class + brightness control function. + + Writing to this file will update all LEDs within the group to a + calculated percentage of what each color LED in the group is set + to. Please refer to the leds-class-multicolor.txt in the + Documentation directory for a complete description. + + The value of the color is from 0 to + /sys/class/leds/<led>/max_brightness. + +What: /sys/class/leds/<led>/colors/color_mix +Date: Sept 2019 +KernelVersion: TBD +Contact: Dan Murphy <dmurphy@ti.com> +Description: read/write + The color_mix file allows writing all registered multicolor LEDs + virtually at the same time. The value(s) written to this file + contain the intensity values for each multicolor LED within + the colors directory. The color indexes are reported in the + color_id file as defined in this document. + Please refer to the leds-class-multicolor.txt in the + Documentation directory for a complete description. + +What: /sys/class/leds/<led>/colors/<led_color>/color_id +Date: Sept 2019 +KernelVersion: TBD +Contact: Dan Murphy <dmurphy@ti.com> +Description: read only + This file when read will return the index of the color in the + color_mix. + Please refer to the leds-class-multicolor.txt in the + Documentation directory for a complete description. + +What: /sys/class/leds/<led>/colors/<led_color>/intensity +Date: Sept 2019 +KernelVersion: TBD +Contact: Dan Murphy <dmurphy@ti.com> +Description: read/write + The led_color directory is dynamically created based on the + colors defined by the registrar of the class. + The led_color can be but not limited to red, green, blue, + white, amber, yellow and violet. Drivers can also declare a + LED color for presentation. There is one directory per color + presented. The brightness file is created under each + led_color directory and controls the individual LED color + setting. + + The value of the color is from 0 to + /sys/class/leds/<led>/colors/<led_color>/max_intensity. + +What: /sys/class/leds/<led>/colors/<led_color>/max_intensity +Date: Sept 2019 +KernelVersion: TBD +Contact: Dan Murphy <dmurphy@ti.com> +Description: read only + Maximum intensity level for the LED color, default is + 255 (LED_FULL). + + If the LED does not support different intensity levels, this + should be 1.
Add a documentation of LED Multicolor LED class specific sysfs attributes. Signed-off-by: Dan Murphy <dmurphy@ti.com> --- .../ABI/testing/sysfs-class-led-multicolor | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-class-led-multicolor -- 2.22.0.214.g8dca754b1e