On Fri, Oct 7, 2022 at 5:56 PM Jean-Jacques Hiblot
<jjhiblot@xxxxxxxxxxxxxxx> wrote:
>
> By allowing to group multiple monochrome LED into multicolor LEDs,
> all involved LEDs can be controlled in-sync. This enables using effects
> using triggers, etc.
...
> +config LEDS_GRP_MULTICOLOR
> + tristate "Multi-color LED grouping support"
> + depends on OF
But there is no compilation requirement for that.
If you want to tell that this is functional requirement, you may use
depends on COMPILE_TEST || OF
pattern
...
> + struct device *dev = &pdev->dev;
> + struct led_init_data init_data = {};
> + struct led_classdev *cdev;
> + struct mc_subled *subled;
> + struct led_mcg_priv *priv;
> + int i, count, ret;
> + unsigned int max_brightness;
Perhaps keep it before previous line?
> + count = 0;
> + max_brightness = 0;
> + for (;;) {
> + struct led_classdev *led_cdev;
> +
> + led_cdev = devm_of_led_get_optional(dev, count);
> + if (!led_cdev)
> + /* Reached the end of the list */
> + break;
This will require {} according to the style guide. Maybe move the
comment on top of the if (!led_cdev) check?
> + if (IS_ERR(led_cdev))
> + return dev_err_probe(dev, PTR_ERR(led_cdev),
> + "Unable to get led #%d", count);
I would check for an error first, this is a common pattern in the Linux kernel.
> + count++;
> +
> + priv->monochromatics = devm_krealloc_array(dev, priv->monochromatics,
> + count, sizeof(*priv->monochromatics),
> + GFP_KERNEL);
> + if (!priv->monochromatics)
> + return -ENOMEM;
> +
> + priv->monochromatics[count - 1] = led_cdev;
> +
> + max_brightness = max(max_brightness, led_cdev->max_brightness);
> + }
> +
> + subled = devm_kcalloc(dev, count, sizeof(*subled), GFP_KERNEL);
> + if (!subled)
> + return -ENOMEM;
> + priv->mc_cdev.subled_info = subled;
> +
> + for (i = 0; i < count; i++) {
> + struct led_classdev *led_cdev = priv->monochromatics[i];
> +
> + subled[i].color_index = led_cdev->color;
> + /* configure the LED intensity to its maximum */
> + subled[i].intensity = max_brightness;
> + }
> +
> + /* init the multicolor's LED class device */
> + cdev = &priv->mc_cdev.led_cdev;
> + cdev->flags = LED_CORE_SUSPENDRESUME;
> + cdev->brightness_set_blocking = led_mcg_set;
> + cdev->max_brightness = max_brightness;
> + cdev->color = LED_COLOR_ID_MULTI;
> + priv->mc_cdev.num_colors = count;
> +
> + init_data.fwnode = dev_fwnode(dev);
> + ret = devm_led_classdev_multicolor_register_ext(dev, &priv->mc_cdev,
> + &init_data);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "failed to register multicolor led for %s.\n",
> + cdev->name);
> +
> + ret = led_mcg_set(cdev, cdev->brightness);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "failed to set led value for %s.",
> + cdev->name);
> +
> + for (i = 0; i < count; i++) {
> + struct led_classdev *led_cdev = priv->monochromatics[i];
> +
> + /* Make the sysfs of the monochromatic LED read-only */
> + led_cdev->flags |= LED_SYSFS_DISABLE;
> + }
> +
> + return 0;
> +}
--
With Best Regards,
Andy Shevchenko
