On Fri, 28 Jul 2023 12:10:44 +0200 Pavel Machek <pavel@xxxxxx> 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