вт, 2 черв. 2026 р. о 11:24 Andy Shevchenko [off-list ref] пише:

On Mon, Jun 01, 2026 at 06:18:25PM +0300, Svyatoslav Ryhel wrote:

quoted

Since there are no users of this driver via platform data, remove the
platform data support and switch to using Device Tree bindings.

...

quoted

@@ -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?

Noted.

...

quoted

-     als->irq = lm3533->irq;
+     als->irq = platform_get_irq_optional(pdev, 0);

quoted

+

Redundant blank line.

Simplifies code perception, whatever.

quoted

+     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

Then we treat it as no IRQ. Original implementation cares only if IRQ
is present or no.

        als->irq = platform_get_irq_optional(pdev, 0);
        if (als->irq == -ENXIO)
                als->irq = 0;
        if (als->irq < 0)
                return als->irq;

...

quoted

+     led->pwm = 0;

Isn't it 0 by zalloc ?

It is, thanks.

quoted

+     device_property_read_u32(&pdev->dev, "ti,pwm-config-mask", &led->pwm);

...

quoted

 #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)?

500 * HZ_PER_KHZ
1000 * HZ_PER_KHZ

You meant this? Sure.

...

quoted

+     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.

I must have overlooked it, thanks. WDYM elsewhere, this is the only occurance.

quoted

      }

...

quoted

+     device_for_each_child_node_scoped(lm3533->dev, child) {

quoted

+             if (!fwnode_device_is_available(child))
+                     continue;

Do we need this check?

This is nice to have if the node is disabled. If we assume that there
are no disabled nodes, I can remove it.

...

quoted

+                             dev_err(dev, "invalid LED node %s\n",
+                                     fwnode_get_name(child));

%pfw

Noted.

...

quoted

+     ret = sysfs_create_group(&dev->kobj, &lm3533_attribute_group);

No way. You should use .dev_groups.

I did not change how driver does this, just swapped lm3533->dev to
dev. I will set is back as it was.

quoted

+     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.

No, I am done with tinkering with this patchset. It is broken enough
and it has inflated enough.

--
With Best Regards,
Andy Shevchenko