Re: [PATCH v14 13/19] leds: lp55xx: Add multicolor framework support to lp55xx

[Jacek Anaszewski <jacek.anaszewski@gmail.com>]

Dan,

I forgot to mention one thing below.

On 10/18/19 2:25 PM, Dan Murphy wrote:
> Add multicolor framework support for the lp55xx family.
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
[...]

> -	led->cdev.default_trigger = pdata->led_config[chan].default_trigger;
>  
>  	if (led->chan_nr >= max_channel) {
>  		dev_err(dev, "Use channel numbers between 0 and %d\n",
> @@ -170,18 +242,13 @@ static int lp55xx_init_led(struct lp55xx_led *led,
>  		return -EINVAL;
>  	}
>  
> -	led->cdev.brightness_set_blocking = lp55xx_set_brightness;
> -	led->cdev.groups = lp55xx_led_groups;
> -
> -	if (pdata->led_config[chan].name) {
> -		led->cdev.name = pdata->led_config[chan].name;
> -	} else {
> -		snprintf(name, sizeof(name), "%s:channel%d",
> -			pdata->label ? : chip->cl->name, chan);
> -		led->cdev.name = name;
> -	}
> +#if IS_ENABLED(CONFIG_LEDS_CLASS_MULTI_COLOR)
> +	if (pdata->led_config[chan].num_colors > 1)
> +		ret = devm_led_classdev_multicolor_register(dev, &led->mc_cdev);
> +	else
> +#endif

This looks odd. I think that no-ops from previous version were OK.

> +		ret = devm_led_classdev_register(dev, &led->cdev);
>  
> -	ret = devm_led_classdev_register(dev, &led->cdev);
>  	if (ret) {
>  		dev_err(dev, "led register err: %d\n", ret);
>  		return ret;
> @@ -525,6 +592,82 @@ void lp55xx_unregister_sysfs(struct lp55xx_chip *chip)
>  }
>  EXPORT_SYMBOL_GPL(lp55xx_unregister_sysfs);
>  


-- 
Best regards,
Jacek Anaszewski