Re: [PATCH leds v1 04/10] leds: max77650: use struct led_init_data when registering

[Bartosz Golaszewski]

On Thu, Sep 17, 2020 at 1:16 AM Marek Behún <marek.behun@xxxxxx> wrote:
>
> By using struct led_init_data when registering we do not need to parse
> `label` DT property nor `linux,default-trigger` property.
>
> Previously if the `label` DT property was not present, the code composed
> name for the LED in the form
>   "max77650::"
> For backwards compatibility we therefore set
>   init_data->default_label = ":";
> so that the LED will not get a different name if `label` property is not
> present.
>
> Signed-off-by: Marek Behún <marek.behun@xxxxxx>
> Cc: Bartosz Golaszewski <bgolaszewski@xxxxxxxxxxxx>
> ---
>  drivers/leds/leds-max77650.c | 24 ++++++++----------------
>  1 file changed, 8 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/leds/leds-max77650.c b/drivers/leds/leds-max77650.c
> index a0d4b725c9178..1eeac56b00146 100644
> --- a/drivers/leds/leds-max77650.c
> +++ b/drivers/leds/leds-max77650.c
> @@ -66,7 +66,6 @@ static int max77650_led_probe(struct platform_device *pdev)
>         struct max77650_led *leds, *led;
>         struct device *dev;
>         struct regmap *map;
> -       const char *label;
>         int rv, num_leds;
>         u32 reg;
>
> @@ -86,6 +85,8 @@ static int max77650_led_probe(struct platform_device *pdev)
>                 return -ENODEV;
>
>         device_for_each_child_node(dev, child) {
> +               struct led_init_data init_data = {};
> +
>                 rv = fwnode_property_read_u32(child, "reg", &reg);
>                 if (rv || reg >= MAX77650_LED_NUM_LEDS) {
>                         rv = -EINVAL;
> @@ -99,22 +100,13 @@ static int max77650_led_probe(struct platform_device *pdev)
>                 led->cdev.brightness_set_blocking = max77650_led_brightness_set;
>                 led->cdev.max_brightness = MAX77650_LED_MAX_BRIGHTNESS;
>
> -               rv = fwnode_property_read_string(child, "label", &label);
> -               if (rv) {
> -                       led->cdev.name = "max77650::";
> -               } else {
> -                       led->cdev.name = devm_kasprintf(dev, GFP_KERNEL,
> -                                                       "max77650:%s", label);
> -                       if (!led->cdev.name) {
> -                               rv = -ENOMEM;
> -                               goto err_node_put;
> -                       }
> -               }
> -
> -               fwnode_property_read_string(child, "linux,default-trigger",
> -                                           &led->cdev.default_trigger);
> +               init_data.fwnode = child;
> +               init_data.devicename = "max77650";
> +               /* for backwards compatibility if `label` is not present */
> +               init_data.default_label = ":";
>
> -               rv = devm_led_classdev_register(dev, &led->cdev);
> +               rv = devm_led_classdev_register_ext(dev, &led->cdev,
> +                                                   &init_data);
>                 if (rv)
>                         goto err_node_put;
>
> --
> 2.26.2
>

I don't know this new API very well but looks good to me.

Acked-by: Bartosz Golaszewski <bgolaszewski@xxxxxxxxxxxx>