From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jacek Anaszewski Subject: Re: [RFC PATCH] leds: multicolor: Add sysfs interface definition Date: Wed, 30 Jan 2019 21:20:09 +0100 Message-ID: References: <20190130183005.833-1-dmurphy@ti.com> <333a146a-a469-5b72-5e81-ff7f522dc598@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Return-path: In-Reply-To: <333a146a-a469-5b72-5e81-ff7f522dc598@ti.com> Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org To: Dan Murphy , robh+dt@kernel.org, pavel@ucw.cz Cc: linux-kernel@vger.kernel.org, linux-leds@vger.kernel.org List-Id: linux-leds@vger.kernel.org Dan, On 1/30/19 8:59 PM, Dan Murphy wrote: > Jacek > > On 1/30/19 1:37 PM, Jacek Anaszewski wrote: >> Hi Dan, >> >> Thank you for the RFC. >> >> One vital thing is missing - documentation of brightness file must >> be updated to define its semantics for LED multi color class. >> >> Either we need brightness-model file returning only "onoff" option >> available, or, for time being, fix the max_brightness for LED multi >> color class to 1 (which will map to max intensity level for each color). >> > > I can make max_brightness default to 1 if not set by the LED driver. > > But the LP50xx has brightness controls so setting max_brightness from the driver should over ride > the max of 1 in the upper level. Yes, so the max_brightness should be updated basing on current brightness-model. For LEDn_BRIGHTNESS of LP50xx we could have "hw" or maybe even better just "lp50xx-linear" and "lp50xx-logarithmic" - I just forgot about that capability of the device. > For devices that do not support brightness as a separate control we can create a file called > max_brightness_ that defines the max that a specific color can be set to. If max_brightness > is set to 1 then create max_brightness_. If max_brightness > 1 then do not create the files. Right. We will need dedicated max_brightness for each color. They should be placed also in the colors directory, next to the color files. > I don't think we have fully vetted the brightness-model yet so I prefer to omit > it and possibly introduce that later. We need to start from something. It will give better overview of the whole idea. Best regards, Jacek Anaszewski > Dan > >> Best regards, >> Jacek Anaszewski >> >> On 1/30/19 7:30 PM, Dan Murphy wrote: >>> Add a documentation of LED Multicolor LED class specific >>> sysfs attributes. >>> >>> Signed-off-by: Dan Murphy >>> --- >>>   .../ABI/testing/sysfs-class-led-multicolor    | 38 +++++++++++++++++++ >>>   1 file changed, 38 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..19f8da9b150e >>> --- /dev/null >>> +++ b/Documentation/ABI/testing/sysfs-class-led-multicolor >>> @@ -0,0 +1,38 @@ >>> +What:        /sys/class/leds//color/sync_enable >>> +Date:        January 2019 >>> +KernelVersion:    5.0 >>> +Contact:    Dan Murphy >>> +Description:    read/write >>> +        Writing a 1 to this file will enable the sychronization of all >>> +        the defined color LEDs within the LED node.  Writing a 0 to >>> +        this file will disable syncing. >>> + >>> +What:        /sys/class/leds//color/sync >>> +Date:        January 2019 >>> +KernelVersion:    5.0 >>> +Contact:    Dan Murphy >>> +Description:    write only >>> +        Writing a 1 to this file while sync_enable is set to 1 will >>> +        synchronize all defined LEDs within the LED node.  All LEDs >>> +        defined will be configured based on the brightness that has >>> +        been requested. >>> + >>> +        If sync_enable is set to 0 then writing a 1 to sync has no >>> +        affect on the LEDs. >>> + >>> +What:        /sys/class/leds//color/ >>> +Date:        January 2019 >>> +KernelVersion:    5.0 >>> +Contact:    Dan Murphy >>> +Description:    read/write >>> +        These files are dynamically created based on the colors defined >>> +        by the registrar of the class. >>> +        The led color(s) can be but not limited to red, green, blue, >>> +        white, amber and violet.  If sync is enabled then writing the >>> +        brightness value of the LED is deferred until a 1 is >>> +        written to /sys/class/leds//color/sync.  If syncing is >>> +        disabled then the LED brightness value will be written >>> +        immediately to the LED driver. >>> + >>> +        The value of the color is from 0 to >>> +        /sys/class/leds//max_brightness. >>> >> > >