On Mon, 12 Aug 2024, Hans de Goede wrote:
> Hi,
>
> Thank you for the new version, much better, almost there I would say.
>
> On 7/19/24 11:59 AM, Carlos Ferreira wrote:
> > This driver adds supports for 4 zone keyboard rgb on omen laptops
> > using the multicolor led api.
> >
> > Tested on the HP Omen 15-en1001np.
> >
> > Signed-off-by: Carlos Ferreira <carlosmiguelferreira.2003@xxxxxxxxx>
> > +static int __init fourzone_leds_init(struct platform_device *device)
> > +{
> > + enum led_brightness hw_brightness;
> > + u32 colors[KBD_ZONE_COUNT * 3];
> > + int ret, i, j;
> > +
> > + ret = fourzone_get_hw_colors(colors);
> > + if (ret < 0)
> > + return ret;
> > +
> > + hw_brightness = fourzone_get_hw_brightness();
> > +
> > + for (i = 0; i < KBD_ZONE_COUNT; i++) {
> > + for (j = 0; j < 3; j++)
> > + fourzone_leds[i].subleds[j] = (struct mc_subled) {
> > + .color_index = j + 1,
> > + .brightness = hw_brightness ? colors[i * 3 + j] : 0,
>
> I think it would be cleaner to drop setting subled brightness here and instead
> call led_mc_calc_color_components() below ... :
>
> > + .intensity = colors[i * 3 + j],
> > + };
> > +
> > + fourzone_leds[i].mc_led = (struct led_classdev_mc) {
> > + .led_cdev = {
> > + .name = fourzone_zone_names[i],
> > + .brightness = hw_brightness ? 255 : 0,
> > + .max_brightness = 255,
> > + .brightness_set = fourzone_set_brightness,
> > + .color = LED_COLOR_ID_RGB,
> > + .flags = LED_BRIGHT_HW_CHANGED | LED_RETAIN_AT_SHUTDOWN,
> > + },
> > + .num_colors = 3,
> > + .subled_info = fourzone_leds[i].subleds;
> > + };
> > +
>
> With this all setup, you can now call:
>
> led_mc_calc_color_components(&fourzone_leds[i].mc_led, fourzone_leds[i].mc_led.led_cdev.brightness);
One additional thing, having a temporary variable for fourzone_leds[i]
would be advicable to reduce the line lengths / complexity of the
expressions.
> here, this makes how the subled brightness is set here (on init) identical with
> how it is done on set_brightness calls which is more consistent.
>
> > + fourzone_leds[i].brightness = 255;
> > +
> > + ret = devm_led_classdev_multicolor_register(&device->dev, &fourzone_leds[i].mc_led);
> > + if (ret)
> > + return -ENODEV;
> > + }
> > +
> > + return 0;
> > +}
--
i.
