Thread (34 messages) 34 messages, 4 authors, 14d ago

Re: [PATCH v3 05/11] mfd: lm3533: Convert to use OF bindings

From: Andy Shevchenko <hidden>
Date: 2026-06-02 08:24:03
Also in: dri-devel, linux-devicetree, linux-iio, linux-leds, lkml

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.
...
quoted hunk ↗ jump to hunk
@@ -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

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help