Re: [PATCH v4 2/2] leds: Add PWM multicolor driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Feb 6, 2022 at 9:36 PM <sven@xxxxxxxxxxxxxxxx> wrote:
>
> From: Sven Schwermer <sven.schwermer@xxxxxxxxxxxxxxxxxxxxxxxxxxx>
>
> By allowing to group multiple monochrome PWM LEDs into multicolor LEDs,
> all involved LEDs can be controlled in-sync. This enables using effects
> using triggers, etc.

Thanks for the update!

...

> +       mcnode = device_get_named_child_node(&pdev->dev, "multi-led");
> +       if (!mcnode)
> +               return dev_err_probe(&pdev->dev, -ENODEV,
> +                                    "expected multi-led node\n");

> +       fwnode_for_each_child_node(mcnode, fwnode) {
> +               pwmled = &priv->leds[priv->mc_cdev.num_colors];
> +               pwmled->pwm = devm_fwnode_pwm_get(&pdev->dev, fwnode, NULL);
> +               if (IS_ERR(pwmled->pwm)) {
> +                       ret = PTR_ERR(pwmled->pwm);
> +                       dev_err(&pdev->dev, "unable to request PWM: %d\n", ret);

> +                       fwnode_handle_put(fwnode);
> +                       goto release_mcnode;

I would make a single return point for this...

> +               }
> +               pwm_init_state(pwmled->pwm, &pwmled->state);
> +
> +               ret = fwnode_property_read_u32(fwnode, "color", &color);
> +               if (ret) {
> +                       dev_err(&pdev->dev, "cannot read color: %d\n", ret);

> +                       fwnode_handle_put(fwnode);
> +                       goto release_mcnode;

...and this.

> +               }
> +
> +               subled[priv->mc_cdev.num_colors].color_index = color;
> +               priv->mc_cdev.num_colors++;
> +       }

...

Something like

  err_release_fwnode:
      fwnode_handle_put(fwnode);

here

> +release_mcnode:
> +       fwnode_handle_put(mcnode);
> +       return ret;
> +}

If you consider that this makes ->probe() error path a bit confusing,
you can split out the loop to a helper where that single error path
will be engaged.

-- 
With Best Regards,
Andy Shevchenko



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux