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
