On Mon, Jun 01, 2026 at 06:18:25PM +0300, Svyatoslav Ryhel wrote:
> Since there are no users of this driver via platform data, remove the
> platform data support and switch to using Device Tree bindings.
...
> @@ -57,6 +60,9 @@ struct lm3533_als {
>
> atomic_t zone;
> struct mutex thresh_mutex;
> +
> + bool pwm_mode;
> + u32 r_select;
> };
Have you run `pahole`? Does it agree with the layout you made here?
...
> - als->irq = lm3533->irq;
> + als->irq = platform_get_irq_optional(pdev, 0);
> +
Redundant blank line.
> + if (als->irq == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
What about other error codes when IRQ is found by can't be retrieved for some
reasons? IIRC we check against ENXIO in similar cases
als->irq = platform_get_irq_optional(pdev, 0);
if (als->irq == -ENXIO)
als->irq = 0;
if (als->irq < 0)
return als->irq;
...
> + led->pwm = 0;
Isn't it 0 by zalloc ?
> + device_property_read_u32(&pdev->dev, "ti,pwm-config-mask", &led->pwm);
...
> #define LM3533_BOOST_FREQ_MASK 0x01
> #define LM3533_BOOST_FREQ_SHIFT 0
> +#define LM3533_BOOST_FREQ_MIN 500000
> +#define LM3533_BOOST_FREQ_MAX 1000000
HZ_PER_KHZ (since you included units.h)?
...
> + nchilds = device_get_child_node_count(dev);
> + if (!nchilds || nchilds > LM3533_CELLS_MAX) {
> + dev_err(dev, "num of child nodes is not supported\n");
> + return -ENODEV;
Why not dev_err_probe() here and elsewhere? It looks inconsistent with this
patch.
> }
...
> + device_for_each_child_node_scoped(lm3533->dev, child) {
> + if (!fwnode_device_is_available(child))
> + continue;
Do we need this check?
...
> + dev_err(dev, "invalid LED node %s\n",
> + fwnode_get_name(child));
%pfw
...
> + ret = sysfs_create_group(&dev->kobj, &lm3533_attribute_group);
No way. You should use .dev_groups.
> + if (ret) {
> + dev_err(dev, "failed to create sysfs attributes\n");
> goto err_unregister;
> }
...
Can you think on how to split this change to smaller steps? I believe it's
possible.
--
With Best Regards,
Andy Shevchenko
