Re: [PATCH v8 1/9] leds: multicolor: Add sysfs interface definition

[Dan Murphy]

Jacek

Thanks for the review

On 9/21/19 5:55 AM, Jacek Anaszewski wrote:
> Dan,
>
> On 9/20/19 7:41 PM, Dan Murphy wrote:
> > Add a documentation of LED Multicolor LED class specific
> > sysfs attributes.
> >
> > Signed-off-by: Dan Murphy <dmurphy@xxxxxx>
> > ---
> >   .../ABI/testing/sysfs-class-led-multicolor    | 43 +++++++++++++++++++
> >   1 file changed, 43 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..39fc73becfec
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/sysfs-class-led-multicolor
> > @@ -0,0 +1,43 @@
> > +What:		/sys/class/leds/<led>/brightness
> > +Date:		Sept 2019
> > +KernelVersion:	5.5
> > +Contact:	Dan Murphy <dmurphy@xxxxxx>
> > +Description:	read/write
> > +		Writing to this file will update all LEDs within the group to a
> > +		calculated percentage of what each color LED intensity is set
> > +		to. The percentage is calculated via the equation below:
> > +
> > +		led_brightness = requested_value * led_color_intensity/led_color_max_intensity
> How about:
>
> led_brightness = brightness * <color>_intensity/<color>_max_intensity

Ack


> > +
> > +		For additional details please refer to
> > +		Documentation/leds/leds-class-multicolor.rst.
> > +
> > +		The value of the color is from 0 to
> > +		/sys/class/leds/<led>/max_brightness.
> > +
> > +What:		/sys/class/leds/<led>/colors/<led_color>_intensity
> s/led_color/color/

Ack


> > +Date:		Sept 2019
> > +KernelVersion:	5.5
> > +Contact:	Dan Murphy <dmurphy@xxxxxx>
> > +Description:	read/write
> > +		The led_color directory is dynamically created based on the
> > +		colors defined by the registrar of the class.
> > +		The value for the led_color is defined in the
> > +		include/dt-bindings/leds/common.h. There is one directory per
> > +		color presented.  The intensity file is created under each
> > +		led_color directory and controls the individual LED color
> > +		setting.
> We no longer have led_color directories so this description needs
> to be adjusted.
>
> And I'd not mention where the colors are defined. This is documentation
> for the user, who does not need to know about implementation details.

I thought I updated this but I will re-write this

Ack


> > +
> > +		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:	5.5
> > +Contact:	Dan Murphy <dmurphy@xxxxxx>
> > +Description:	read only
> > +		Maximum intensity level for the LED color, default is
> > +		255 (LED_FULL).
> Mentioning the default value here is pointless IMO. Userspace cannot
> change it anyway.
Ack


> > +
> > +		If the LED does not support different intensity levels, this
> > +		should be 1.
> This is less relevant for MC class, and also it is rather obvious.
> I'd skip this sentence.
Ack


Dan