[PATCH v2 1/1] leds: syscon: Add tristate option

[PATCH v2 1/1] leds: syscon: Add tristate option

[Bevan Weiss]

This reverts commit f7d98a65d031 ("leds: syscon: Make the driver
explicitly non-modular").

OpenWrt builds with a single .config for a given subtarget, whilst boards
below that subtarget may not want to carry the disk usage of all options.
So there is a preference to have all functionality not required on all
boards as modules that can simply not be included in a given board rootfs
to reduce disk usage.

Additional changes:
- (int)(struct platform_device *pdev) => void return
- Add MODULE_DESCRIPTION and MODULE_LICENSE macros
- Add tristate option to Kconfig

Signed-off-by: Bevan Weiss <bevan.weiss@gmail.com>
---
 drivers/leds/Kconfig       |  4 ++--
 drivers/leds/leds-syscon.c | 20 +++++++++++++++++---
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 597d7a79c988..ea3afc76a9c6 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -903,8 +903,8 @@ config LEDS_POWERNV
 	  will be called leds-powernv.
 
 config LEDS_SYSCON
-	bool "LED support for LEDs on system controllers"
-	depends on LEDS_CLASS=y
+	tristate "LED support for LEDs on system controllers"
+	depends on LEDS_CLASS
 	depends on MFD_SYSCON
 	depends on OF
 	help
diff --git a/drivers/leds/leds-syscon.c b/drivers/leds/leds-syscon.c
index d633ad519d0c..bc968e8a326d 100644
--- a/drivers/leds/leds-syscon.c
+++ b/drivers/leds/leds-syscon.c
@@ -6,7 +6,7 @@
  * Author: Linus Walleij <linus.walleij@linaro.org>
  */
 #include <linux/io.h>
-#include <linux/init.h>
+#include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/stat.h>
@@ -121,17 +121,31 @@ static int syscon_led_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static void syscon_led_remove(struct platform_device *pdev)
+{
+	struct syscon_led *sled = platform_get_drvdata(pdev);
+
+	led_classdev_unregister(&sled->cdev);
+	/* Turn it off */
+	regmap_update_bits(sled->map, sled->offset, sled->mask, 0);
+}
+
 static const struct of_device_id of_syscon_leds_match[] = {
 	{ .compatible = "register-bit-led", },
 	{},
 };
 
+MODULE_DEVICE_TABLE(of, of_syscon_leds_match);
+
 static struct platform_driver syscon_led_driver = {
 	.probe		= syscon_led_probe,
+	.remove		= syscon_led_remove,
 	.driver		= {
 		.name	= "leds-syscon",
 		.of_match_table = of_syscon_leds_match,
-		.suppress_bind_attrs = true,
 	},
 };
-builtin_platform_driver(syscon_led_driver);
+module_platform_driver(syscon_led_driver);
+
+MODULE_DESCRIPTION("SYSCON LED driver");
+MODULE_LICENSE("GPL");
-- 
2.43.0

[PATCH v2 0/1] leds: syscon: Add tristate option

[Bevan Weiss]

> My first submission to a linux-kernel list, so apologies for any errors.
Apologies, that didn't age well, I should have looked at the file history.
It looks like the module=>builtin was done before, so this change reverts
that.

It also adds MODULE_DESCRIPTION and MODULE_LICENSE along with the tristate
Kconfig.

In OpenWrt, this allows leds-syscon to be .config'd as a module in 
subtargets, and then included / excluded from rootfs filesystems as 
required for specific boards.  Without this, every board would need to have
leds-syscon compiled as builtin just to provide it for one specific board.
And OpenWrt devices are often space-constrained, where this is a
significant concern. 


Bevan Weiss (1):
  leds: syscon: Add tristate option

 drivers/leds/Kconfig       |  4 ++--
 drivers/leds/leds-syscon.c | 20 +++++++++++++++++---
 2 files changed, 19 insertions(+), 5 deletions(-)

-- 
2.43.0

[PATCH v2 1/1] leds: syscon: Add tristate option

[Bevan Weiss]

This reverts commit f7d98a65d031 ("leds: syscon: Make the driver
explicitly non-modular").

OpenWrt builds with a single .config for a given subtarget, whilst boards
below that subtarget may not want to carry the disk usage of all options.
So there is a preference to have all functionality not required on all
boards as modules that can simply not be included in a given board rootfs
to reduce disk usage.

Additional changes:
- (int)(struct platform_device *pdev) => void return
- Add MODULE_DESCRIPTION and MODULE_LICENSE macros
- Add tristate option to Kconfig

Signed-off-by: Bevan Weiss <bevan.weiss@gmail.com>
---
 drivers/leds/Kconfig       |  4 ++--
 drivers/leds/leds-syscon.c | 20 +++++++++++++++++---
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 597d7a79c988..ea3afc76a9c6 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -903,8 +903,8 @@ config LEDS_POWERNV
 	  will be called leds-powernv.
 
 config LEDS_SYSCON
-	bool "LED support for LEDs on system controllers"
-	depends on LEDS_CLASS=y
+	tristate "LED support for LEDs on system controllers"
+	depends on LEDS_CLASS
 	depends on MFD_SYSCON
 	depends on OF
 	help
diff --git a/drivers/leds/leds-syscon.c b/drivers/leds/leds-syscon.c
index d633ad519d0c..bc968e8a326d 100644
--- a/drivers/leds/leds-syscon.c
+++ b/drivers/leds/leds-syscon.c
@@ -6,7 +6,7 @@
  * Author: Linus Walleij <linus.walleij@linaro.org>
  */
 #include <linux/io.h>
-#include <linux/init.h>
+#include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/stat.h>
@@ -121,17 +121,31 @@ static int syscon_led_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static void syscon_led_remove(struct platform_device *pdev)
+{
+	struct syscon_led *sled = platform_get_drvdata(pdev);
+
+	led_classdev_unregister(&sled->cdev);
+	/* Turn it off */
+	regmap_update_bits(sled->map, sled->offset, sled->mask, 0);
+}
+
 static const struct of_device_id of_syscon_leds_match[] = {
 	{ .compatible = "register-bit-led", },
 	{},
 };
 
+MODULE_DEVICE_TABLE(of, of_syscon_leds_match);
+
 static struct platform_driver syscon_led_driver = {
 	.probe		= syscon_led_probe,
+	.remove		= syscon_led_remove,
 	.driver		= {
 		.name	= "leds-syscon",
 		.of_match_table = of_syscon_leds_match,
-		.suppress_bind_attrs = true,
 	},
 };
-builtin_platform_driver(syscon_led_driver);
+module_platform_driver(syscon_led_driver);
+
+MODULE_DESCRIPTION("SYSCON LED driver");
+MODULE_LICENSE("GPL");
-- 
2.43.0

Re: [PATCH v2 1/1] leds: syscon: Add tristate option

[Lee Jones]

On Sun, 08 Mar 2026, Bevan Weiss wrote:

> This reverts commit f7d98a65d031 ("leds: syscon: Make the driver
> explicitly non-modular").
> 
> OpenWrt builds with a single .config for a given subtarget, whilst boards
> below that subtarget may not want to carry the disk usage of all options.
> So there is a preference to have all functionality not required on all
> boards as modules that can simply not be included in a given board rootfs
> to reduce disk usage.
> 
> Additional changes:
> - (int)(struct platform_device *pdev) => void return

Please rephrase into a human parseable sentence.

> - Add MODULE_DESCRIPTION and MODULE_LICENSE macros
> - Add tristate option to Kconfig
> 
> Signed-off-by: Bevan Weiss <bevan.weiss@gmail.com>
> ---
>  drivers/leds/Kconfig       |  4 ++--
>  drivers/leds/leds-syscon.c | 20 +++++++++++++++++---
>  2 files changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 597d7a79c988..ea3afc76a9c6 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -903,8 +903,8 @@ config LEDS_POWERNV
>  	  will be called leds-powernv.
>  
>  config LEDS_SYSCON
> -	bool "LED support for LEDs on system controllers"
> -	depends on LEDS_CLASS=y
> +	tristate "LED support for LEDs on system controllers"
> +	depends on LEDS_CLASS
>  	depends on MFD_SYSCON
>  	depends on OF
>  	help
> diff --git a/drivers/leds/leds-syscon.c b/drivers/leds/leds-syscon.c
> index d633ad519d0c..bc968e8a326d 100644
> --- a/drivers/leds/leds-syscon.c
> +++ b/drivers/leds/leds-syscon.c
> @@ -6,7 +6,7 @@
>   * Author: Linus Walleij <linus.walleij@linaro.org>
>   */
>  #include <linux/io.h>
> -#include <linux/init.h>
> +#include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/platform_device.h>
>  #include <linux/stat.h>
> @@ -121,17 +121,31 @@ static int syscon_led_probe(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static void syscon_led_remove(struct platform_device *pdev)
> +{
> +	struct syscon_led *sled = platform_get_drvdata(pdev);
> +
> +	led_classdev_unregister(&sled->cdev);

The driver uses devm_led_classdev_register() in probe, which automatically
handles unregistering the LED class device when the driver is detached.
Calling it here manually will result in a double-unregister.

> +	/* Turn it off */

Turn what off?

> +	regmap_update_bits(sled->map, sled->offset, sled->mask, 0);

If 0 was defined, as it should be, then you can drop the comment.

> +}
> +
>  static const struct of_device_id of_syscon_leds_match[] = {
>  	{ .compatible = "register-bit-led", },
>  	{},
>  };
>  
> +MODULE_DEVICE_TABLE(of, of_syscon_leds_match);
> +
>  static struct platform_driver syscon_led_driver = {
>  	.probe		= syscon_led_probe,
> +	.remove		= syscon_led_remove,
>  	.driver		= {
>  		.name	= "leds-syscon",
>  		.of_match_table = of_syscon_leds_match,
> -		.suppress_bind_attrs = true,
>  	},
>  };
> -builtin_platform_driver(syscon_led_driver);
> +module_platform_driver(syscon_led_driver);
> +
> +MODULE_DESCRIPTION("SYSCON LED driver");
> +MODULE_LICENSE("GPL");

The MODULE_LICENSE() string should match the SPDX identifier at the top
of the file.

-- 
Lee Jones [李琼斯]

Re: [PATCH v2 1/1] leds: syscon: Add tristate option

[Bevan Weiss]

On Thu, 19 Mar 2026 10:13:27 +0000
Lee Jones <lee@kernel.org> wrote:

> > Additional changes:
> > - (int)(struct platform_device *pdev) => void return  
> 
> Please rephrase into a human parseable sentence.

Thanks, will do.

> > +	led_classdev_unregister(&sled->cdev);  
> 
> The driver uses devm_led_classdev_register() in probe, which
> automatically handles unregistering the LED class device when the
> driver is detached. Calling it here manually will result in a
> double-unregister.

My oversight, will fix.

> > +	/* Turn it off */  
> 
> Turn what off?
> 
> > +	regmap_update_bits(sled->map, sled->offset, sled->mask,
> > 0);  
> 
> If 0 was defined, as it should be, then you can drop the comment.

This is carry over from the original source, when it was originally
modularised.  I'll update the comment to explicitly mention that it's
the LED which is turned off on module remove.

> > +MODULE_LICENSE("GPL");  
> 
> The MODULE_LICENSE() string should match the SPDX identifier at the
> top of the file.
> 

Sorry, I'm not quite understanding this in the context of other files.
Most modules within the led subsystem seem to have 
'GPL-2.0-or-later' as the SPDX identifier, and MODULE_LICENSE("GPL"),
as I currently have here.
Is there a different subtlety I'm missing?
e.g. leds-adp5520.c, leds-as3668.c, leds-blinkm.c
Perhaps MODULE_LICENSE("GPL v2+")? although I don't see similar
anywhere in the kernel.
I'd expect MODULE_LICENSE("GPL v2") to be for 'GPL-2.0-only' licenses.


Thanks and regards,
Bevan Weiss