[PATCH v3] leds: is31fl319x: modernize registration

[PATCH v3] leds: is31fl319x: modernize registration

[Andreas Kemnade]

Use _ext version to have properties parsed to avoid needing to parse
them in the driver itself. More modern properties are recognized and
the leds can be referenced via phandle.
Due to the maximum current mechanics, leds are not registered right
in the first iteration over the nodes.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
Changes in v3:
- fix typo (Sashiko)
- check availability of device match data (Sashiko)
- Link to v2: https://patch.msgid.link/20260703-led-modern-v2-1-d07dd0d6ec6f@kemnade.info

Changes in v2:
- put node free action after cdef is initialized (Sashiko)
- Link to v1: https://patch.msgid.link/20260702-led-modern-v1-1-a4af2e9aad60@kemnade.info

To: Lee Jones <lee@kernel.org>
To: Pavel Machek <pavel@kernel.org>
Cc: linux-leds@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/leds/leds-is31fl319x.c | 61 +++++++++++++++++++++---------------------
 1 file changed, 31 insertions(+), 30 deletions(-)

diff --git a/drivers/leds/leds-is31fl319x.c b/drivers/leds/leds-is31fl319x.c
index 80f38dba0fba..83cb769d8cb4 100644
--- a/drivers/leds/leds-is31fl319x.c
+++ b/drivers/leds/leds-is31fl319x.c
@@ -98,7 +98,7 @@ struct is31fl319x_chip {
 		struct is31fl319x_chip  *chip;
 		struct led_classdev     cdev;
 		u32                     max_microamp;
-		bool                    configured;
+		struct fwnode_handle *fwnode;
 	} leds[IS31FL319X_MAX_LEDS];
 };
 
@@ -363,31 +363,17 @@ static const struct of_device_id of_is31fl319x_match[] = {
 };
 MODULE_DEVICE_TABLE(of, of_is31fl319x_match);
 
-static int is31fl319x_parse_child_fw(const struct device *dev,
-				     const struct fwnode_handle *child,
-				     struct is31fl319x_led *led,
-				     struct is31fl319x_chip *is31)
+static void is31_free_fwnode(void *data)
 {
-	struct led_classdev *cdev = &led->cdev;
-	int ret;
-
-	if (fwnode_property_read_string(child, "label", &cdev->name))
-		cdev->name = fwnode_get_name(child);
+	struct is31fl319x_chip *is31 = data;
+	int i;
 
-	ret = fwnode_property_read_string(child, "linux,default-trigger", &cdev->default_trigger);
-	if (ret < 0 && ret != -EINVAL) /* is optional */
-		return ret;
+	for (i = 0; i < is31->cdef->num_leds; i++) {
+		if (is31->leds[i].fwnode)
+			fwnode_handle_put(is31->leds[i].fwnode);
 
-	led->max_microamp = is31->cdef->current_default;
-	ret = fwnode_property_read_u32(child, "led-max-microamp", &led->max_microamp);
-	if (!ret) {
-		if (led->max_microamp < is31->cdef->current_min)
-			return -EINVAL;	/* not supported */
-		led->max_microamp = min(led->max_microamp,
-					is31->cdef->current_max);
+		is31->leds[i].fwnode = NULL;
 	}
-
-	return 0;
 }
 
 static int is31fl319x_parse_fw(struct device *dev, struct is31fl319x_chip *is31)
@@ -402,6 +388,12 @@ static int is31fl319x_parse_fw(struct device *dev, struct is31fl319x_chip *is31)
 				     "Failed to get shutdown gpio\n");
 
 	is31->cdef = device_get_match_data(dev);
+	if (!is31->cdef)
+		return -ENODEV;
+
+	ret = devm_add_action_or_reset(dev, is31_free_fwnode, is31);
+	if (ret)
+		return ret;
 
 	count = 0;
 	device_for_each_child_node_scoped(dev, child)
@@ -427,14 +419,20 @@ static int is31fl319x_parse_fw(struct device *dev, struct is31fl319x_chip *is31)
 
 		led = &is31->leds[reg - 1];
 
-		if (led->configured)
+		if (led->fwnode)
 			return dev_err_probe(dev, -EINVAL, "led %u is already configured\n", reg);
 
-		ret = is31fl319x_parse_child_fw(dev, child, led, is31);
-		if (ret)
-			return dev_err_probe(dev, ret, "led %u DT parsing failed\n", reg);
+		led->max_microamp = is31->cdef->current_default;
+		ret = fwnode_property_read_u32(child, "led-max-microamp", &led->max_microamp);
+		if (!ret) {
+			if (led->max_microamp < is31->cdef->current_min)
+				return dev_err_probe(dev, -EINVAL, "invalid maximum current\n");
+
+			led->max_microamp = min(led->max_microamp,
+						is31->cdef->current_max);
+		}
 
-		led->configured = true;
+		led->fwnode = fwnode_handle_get(child);
 	}
 
 	is31->audio_gain_db = 0;
@@ -531,7 +529,7 @@ static int is31fl319x_probe(struct i2c_client *client)
 	 */
 	aggregated_led_microamp = is31->cdef->current_max;
 	for (i = 0; i < is31->cdef->num_leds; i++)
-		if (is31->leds[i].configured &&
+		if (is31->leds[i].fwnode &&
 		    is31->leds[i].max_microamp < aggregated_led_microamp)
 			aggregated_led_microamp = is31->leds[i].max_microamp;
 
@@ -545,14 +543,17 @@ static int is31fl319x_probe(struct i2c_client *client)
 
 	for (i = 0; i < is31->cdef->num_leds; i++) {
 		struct is31fl319x_led *led = &is31->leds[i];
+		struct led_init_data init_data = {};
 
-		if (!led->configured)
+		if (!led->fwnode)
 			continue;
 
+		init_data.fwnode = led->fwnode;
+
 		led->chip = is31;
 		led->cdev.brightness_set_blocking = is31->cdef->brightness_set;
 
-		err = devm_led_classdev_register(&client->dev, &led->cdev);
+		err = devm_led_classdev_register_ext(&client->dev, &led->cdev, &init_data);
 		if (err < 0)
 			return err;
 	}

---
base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
change-id: 20260702-led-modern-d427ed011f83

Best regards,
--  
Andreas Kemnade <andreas@kemnade.info>

Re: [PATCH v3] leds: is31fl319x: modernize registration

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The commit message description starts with a lowercase letter, which violates the LED subsystem guidelines.
--

--- Patch [1]: [PATCH v3] leds: is31fl319x: modernize registration ---
commit 8ca9b33466bf8fe661e87d3758c14a3394d87a31
Author: Andreas Kemnade <andreas@kemnade.info>

leds: is31fl319x: modernize registration

This commit updates the IS31FL319x LED driver to use the extended managed
registration API for parsing properties automatically. It also adds proper
fwnode lifecycle management and defers LED registration until the maximum
aggregated current is calculated.

> leds: is31fl319x: modernize registration

[Severity: Low]
This isn't a bug, but should the description after the subsystem prefix be
capitalized? The LED subsystem guidelines recommend formatting commit messages
as "leds: <Driver>: <Capitalized description>".

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804-led-modern-v3-1-297720eec19f@kemnade.info?part=1

Re: [PATCH v3] leds: is31fl319x: modernize registration

[Andreas Kemnade]

On Tue, 04 Aug 2026 13:55:14 +0000
sashiko-bot@kernel.org wrote:

> > leds: is31fl319x: modernize registration  
> 
> [Severity: Low]
> This isn't a bug, but should the description after the subsystem prefix be
> capitalized? The LED subsystem guidelines recommend formatting commit messages
> as "leds: <Driver>: <Capitalized description>".


~/linux$ grep -R 'leds: <Driver>: <Capitalized description>"' Documentation/
~/linux$ ls Documentation/leds/
index.rst                  leds-lp5521.rst      leds-sc27xx.rst
leds-blinkm.rst            leds-lp5523.rst      leds-st1202.rst
leds-cht-wcove.rst         leds-lp5562.rst      ledtrig-oneshot.rst
leds-class-flash.rst       leds-lp55xx.rst      ledtrig-transient.rst
leds-class-multicolor.rst  leds-lp5812.rst      ledtrig-usbport.rst
leds-class.rst             leds-mlxcpld.rst     uleds.rst
leds-el15203000.rst        leds-mt6370-rgb.rst  well-known-leds.txt
leds-lm3556.rst            leds-qcom-lpg.rst
leds-lp3944.rst            leds-s2m-rgb.rst

nothing look like special guidelines for the system. I do not want to
see such comments without a reference.
Also MAINTAINERS does not show any additional policy for the LED subsystem.

Regards,
Andreas

Re: [PATCH v3] leds: is31fl319x: modernize registration

[Lee Jones]

On Tue, 04 Aug 2026, Andreas Kemnade wrote:

> On Tue, 04 Aug 2026 13:55:14 +0000
> sashiko-bot@kernel.org wrote:
> 
> > > leds: is31fl319x: modernize registration  
> > 
> > [Severity: Low]
> > This isn't a bug, but should the description after the subsystem prefix be
> > capitalized? The LED subsystem guidelines recommend formatting commit messages
> > as "leds: <Driver>: <Capitalized description>".
> 
> 
> ~/linux$ grep -R 'leds: <Driver>: <Capitalized description>"' Documentation/
> ~/linux$ ls Documentation/leds/
> index.rst                  leds-lp5521.rst      leds-sc27xx.rst
> leds-blinkm.rst            leds-lp5523.rst      leds-st1202.rst
> leds-cht-wcove.rst         leds-lp5562.rst      ledtrig-oneshot.rst
> leds-class-flash.rst       leds-lp55xx.rst      ledtrig-transient.rst
> leds-class-multicolor.rst  leds-lp5812.rst      ledtrig-usbport.rst
> leds-class.rst             leds-mlxcpld.rst     uleds.rst
> leds-el15203000.rst        leds-mt6370-rgb.rst  well-known-leds.txt
> leds-lm3556.rst            leds-qcom-lpg.rst
> leds-lp3944.rst            leds-s2m-rgb.rst
> 
> nothing look like special guidelines for the system. I do not want to
> see such comments without a reference.
> Also MAINTAINERS does not show any additional policy for the LED subsystem.

`git log --oneline -- <subsystem>` is your friend.

 % git log --oneline -n10 -- drivers/leds
8d6b6c05b8e3 leds: pca9532:  Fix phantom device registration on missing hardware
98c5c7b0d426 leds: gpio:     Make legacy gpiolib interface optional
b6e08e0ad4cf leds: bcm63138: Use %pe to print pinctrl error instead of %ld
07e028d93a57 leds: ltc3220:  Add Support for LTC3220 18 channel LED Driver
cf197514bdfd leds: st1202:   Validate LED reg property against channel count
0767335233a8 leds: st1202:   Disable channel when brightness is set to zero
7cbe470366bd leds: st1202:   Fix brightness having no effect while pattern mode is active
dcc31246aaf0 leds: st1202:   Fix spurious pattern sequence start in setup
d2ca0e2b6d64 leds: st1202:   Set all pattern PWM slots to full after clearing pattern
d32f8bdc2b41 leds: st1202:   Fix pattern duration prescaler and pattern_clear skip marker

-- 
Lee Jones

Re: [PATCH v3] leds: is31fl319x: modernize registration

[Andreas Kemnade]

On Thu, 6 Aug 2026 11:45:25 +0100
Lee Jones <lee@kernel.org> wrote:

> On Tue, 04 Aug 2026, Andreas Kemnade wrote:
> 
> > On Tue, 04 Aug 2026 13:55:14 +0000
> > sashiko-bot@kernel.org wrote:
> >   
> > > > leds: is31fl319x: modernize registration    
> > > 
> > > [Severity: Low]
> > > This isn't a bug, but should the description after the subsystem prefix be
> > > capitalized? The LED subsystem guidelines recommend formatting commit messages
> > > as "leds: <Driver>: <Capitalized description>".  
> > 
> > 
> > ~/linux$ grep -R 'leds: <Driver>: <Capitalized description>"' Documentation/
> > ~/linux$ ls Documentation/leds/
> > index.rst                  leds-lp5521.rst      leds-sc27xx.rst
> > leds-blinkm.rst            leds-lp5523.rst      leds-st1202.rst
> > leds-cht-wcove.rst         leds-lp5562.rst      ledtrig-oneshot.rst
> > leds-class-flash.rst       leds-lp55xx.rst      ledtrig-transient.rst
> > leds-class-multicolor.rst  leds-lp5812.rst      ledtrig-usbport.rst
> > leds-class.rst             leds-mlxcpld.rst     uleds.rst
> > leds-el15203000.rst        leds-mt6370-rgb.rst  well-known-leds.txt
> > leds-lm3556.rst            leds-qcom-lpg.rst
> > leds-lp3944.rst            leds-s2m-rgb.rst
> > 
> > nothing look like special guidelines for the system. I do not want to
> > see such comments without a reference.
> > Also MAINTAINERS does not show any additional policy for the LED subsystem.  
> 
> `git log --oneline -- <subsystem>` is your friend.
> 
That is looking at common practice, not guidelines, which is of course
often a good idea.

So Sahiko is hallucinating about non-existant guidelines which
cannot be referenced of course. Well, at least it gives quite an amount of
useful hintse.

Regards,
Andreas

Re: [PATCH v3] leds: is31fl319x: modernize registration

[Lee Jones]

On Thu, 06 Aug 2026, Andreas Kemnade wrote:

> On Thu, 6 Aug 2026 11:45:25 +0100
> Lee Jones <lee@kernel.org> wrote:
> 
> > On Tue, 04 Aug 2026, Andreas Kemnade wrote:
> > 
> > > On Tue, 04 Aug 2026 13:55:14 +0000
> > > sashiko-bot@kernel.org wrote:
> > >   
> > > > > leds: is31fl319x: modernize registration    
> > > > 
> > > > [Severity: Low]
> > > > This isn't a bug, but should the description after the subsystem prefix be
> > > > capitalized? The LED subsystem guidelines recommend formatting commit messages
> > > > as "leds: <Driver>: <Capitalized description>".  
> > > 
> > > 
> > > ~/linux$ grep -R 'leds: <Driver>: <Capitalized description>"' Documentation/
> > > ~/linux$ ls Documentation/leds/
> > > index.rst                  leds-lp5521.rst      leds-sc27xx.rst
> > > leds-blinkm.rst            leds-lp5523.rst      leds-st1202.rst
> > > leds-cht-wcove.rst         leds-lp5562.rst      ledtrig-oneshot.rst
> > > leds-class-flash.rst       leds-lp55xx.rst      ledtrig-transient.rst
> > > leds-class-multicolor.rst  leds-lp5812.rst      ledtrig-usbport.rst
> > > leds-class.rst             leds-mlxcpld.rst     uleds.rst
> > > leds-el15203000.rst        leds-mt6370-rgb.rst  well-known-leds.txt
> > > leds-lm3556.rst            leds-qcom-lpg.rst
> > > leds-lp3944.rst            leds-s2m-rgb.rst
> > > 
> > > nothing look like special guidelines for the system. I do not want to
> > > see such comments without a reference.
> > > Also MAINTAINERS does not show any additional policy for the LED subsystem.  
> > 
> > `git log --oneline -- <subsystem>` is your friend.
> > 
> That is looking at common practice, not guidelines, which is of course
> often a good idea.
> 
> So Sahiko is hallucinating about non-existant guidelines which
> cannot be referenced of course. Well, at least it gives quite an amount of
> useful hintse.

It's not hallucinating, I put it there. =:-)

https://github.com/sashiko-dev/sashiko/blob/main/third_party/prompts/kernel/subsystem/leds.md

Specifically:

https://github.com/sashiko-dev/sashiko/blob/main/third_party/prompts/kernel/subsystem/leds.md?plain=1#L8

-- 
Lee Jones