On Thu, Feb 27, 2020 at 10:22:43PM +0100, Jacek Anaszewski wrote: > On 2/27/20 2:07 PM, Dan Murphy wrote: > > Pavel > > > > On 2/27/20 6:43 AM, Greg KH wrote: > >> On Thu, Feb 27, 2020 at 11:58:08AM +0100, Pavel Machek wrote: > >>> Hi, Jacek! > >>> > >>> (and thanks for doing this). > >>> > >>>> We have here long lasting discussion related to LED multicolor class > >>>> sysfs interface design. We went through several iterations and worked > >>>> out the solution with individual file per each color sub-LED in the > >>>> color directory as shown below: > >>>> > >>>> /sys/class/leds/<led>/colors/<color>_intensity > >>>> > >>>> This is in line with one-value-per-file sysfs rule, that is being > >>>> frequently highlighted, and we even had not so long ago a patch > >>>> for led cpu trigger solving the problem caused by this rule not > >>>> being adhered to. > >>> Yep. One of the problems is that it is nice to change all the hardware > >>> channels at once to produce color (it is often on i2c -- and slow), so > >>> current proposals use "interesting" kind of latching. > >>> > >>>> Now we have the voice below bringing to attention another caveat > >>>> from sysfs documentation: > >>>> > >>>> "it is socially acceptable to express an array of values of the same > >>>> type" > >>>> > >>>> and proposing the interface in the form of two files: > >>>> > >>>> channel_intensity (file containing array of u32's) > >>>> channel_names (usually containing "red green blue") > >>> And thus I want to have it in one file, so it is naturaly atomic. RGB > >>> leds with 3 channels are common; I have not user yet, but there are > >>> RGBW with 4 channels (and some more exotic stuff). I don't expect to > >>> have more than 5 channels. > > > > This is not an accurate statement. Right now a user can have up to 8 > > channels to cover all the LEDs defined in the LED core > > > > And if the led_colors array expands then this array can expand. > > > > We have no control on how many entries the user will put in their DT so > > again this number is completely arbitrary. > > I believe that some of mechanisms that were devised for the most > recent implementation proposal of LED mc class will need > to be reused for the array approach. E.g. available_colors bitmask > will make the parsing resistant to duplicates. > > Of course LED multicolor DT node design should be applicable as well > to the array approach. > > >> Writing 3 or 4 or 5 numbers all at once in a single sysfs file to > >> represent a single output should be fine. > >> thanks, > > Thank you for making this clear. > > Effectively, the way to go as I see it now is just moving from > colors directory to channel_intensity and channel_names files. Wait, we already have an interface for this and you want to create a competing one? Why? What's wrong with what you have now? Do you have a pointer to the Documentation/ABI/ entries that you have now that you feel do not work well? thanks, greg k-h
