Re: [PATCH v2 1/3] leds: turris-omnia: change max brightness from 255 to 1

["Marek Behún" <kabel@kernel.org>]

On Fri, 28 Jul 2023 12:10:44 +0200
Pavel Machek <pavel@ucw.cz> wrote:

> Hi!
> 
> > The HW exposes color control for three channels (RGB), each channel with
> > range 0-255 (so 16M colors). The driver exposes this via the
> > multi_intensity sysfs file. This is communicated to the HW via
> > LED_SET_COLOR I2C command.
> > 
> > HW also exposes setting the LED ON and OFF, via the LED_SET_STATE
> > I2C command.
> > 
> > We currently have the following sysfs files via which we set LED state
> > and color:
> >   brightness
> >   multi_intensity
> > 
> > Because currently the driver sets max_brightness to 255, the actual
> > color that is sent to HW is recalculated via
> > led_mc_calc_color_components(). For example with
> > 
> >   $ echo 255 255 100 >multi_intensity
> >   $ echo 150 >brightness
> > 
> > the led_mc_calc_color_components() function recalculates the channel
> > intensities with formula
> >   brightness * channel_intensity / max_brightness
> > and so the (255, 255, 100) tuple is converted to (150, 150, 58) before
> > sending to HW.  
> 
> And this seems ok.
> 
> > What I think would make more sense is to make the two sysfs files
> >   brightness
> >   multi_intensity
> > correspond 1-to-1 with I2C commands LED_SET_STATE and LED_SET_COLOR.  
> 
> We want your driver to be have same API as other drivers, 1-to-1
> correspondence with I2c commands is not important.
> 
> NAK-ed-by: Pavel
> 
> Best regards,
> 								Pavel

Hmm, thinking more about it I guess you are right. And I thought of
what I think is better change anyway. I shall send patch for review,
let's see what you think about that one :)

Marek