* [PATCH v4] leds: Add driver for MAX8907 LED cell
@ 2026-10-27 11:50 Svyatoslav Ryhel
2026-11-04 15:16 ` Lee Jones
0 siblings, 1 reply; 5+ messages in thread
From: Svyatoslav Ryhel @ 2026-10-27 11:50 UTC (permalink / raw)
To: Lee Jones, Pavel Machek, Svyatoslav Ryhel; +Cc: linux-kernel, linux-leds, mfd
Add a small driver to control the LED cell provided by the MAX8907 PMIC.
The LED does not require any specific configuration and is folded into the
PMIC's node.
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
Changes in v2:
- rebased so patch is now applicable to linux/next
Changes in v3:
- added check if led is off for get callback
- added dedicated MAX8907_REG_WLED_MODE_ON mask
- switched to devm_led_classdev_register_ext
Changes in v4:
- platform_set_drvdata dropped
- init_data.devicename dropped
- added MODULE_ALIAS
---
drivers/leds/Kconfig | 7 +++
drivers/leds/Makefile | 1 +
drivers/leds/leds-max8907.c | 90 +++++++++++++++++++++++++++++++++++++
drivers/mfd/max8907.c | 1 +
include/linux/mfd/max8907.h | 2 +-
5 files changed, 100 insertions(+), 1 deletion(-)
create mode 100644 drivers/leds/leds-max8907.c
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 788e689fe875c..3796c562919fe 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -813,6 +813,13 @@ config LEDS_MAX77705
help
LED driver for MAX77705 PMIC from Maxim Integrated.
+config LEDS_MAX8907
+ tristate "LED support for MAX8907 PMIC"
+ depends on LEDS_CLASS && MFD_MAX8907
+ help
+ This option enables support for on-chip LED drivers on
+ MAXIM MAX8907 PMIC.
+
config LEDS_MAX8997
tristate "LED support for MAX8997 PMIC"
depends on LEDS_CLASS && MFD_MAX8997
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 3da746279bb81..24efda3fbaa44 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -67,6 +67,7 @@ obj-$(CONFIG_LEDS_LTC3220) += leds-ltc3220.o
obj-$(CONFIG_LEDS_MAX5970) += leds-max5970.o
obj-$(CONFIG_LEDS_MAX77650) += leds-max77650.o
obj-$(CONFIG_LEDS_MAX77705) += leds-max77705.o
+obj-$(CONFIG_LEDS_MAX8907) += leds-max8907.o
obj-$(CONFIG_LEDS_MAX8997) += leds-max8997.o
obj-$(CONFIG_LEDS_MC13783) += leds-mc13783.o
obj-$(CONFIG_LEDS_MENF21BMC) += leds-menf21bmc.o
diff --git a/drivers/leds/leds-max8907.c b/drivers/leds/leds-max8907.c
new file mode 100644
index 0000000000000..a5c445f75d1b1
--- /dev/null
+++ b/drivers/leds/leds-max8907.c
@@ -0,0 +1,90 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/err.h>
+#include <linux/leds.h>
+#include <linux/mfd/max8907.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+
+#define MAX8907_REG_WLED_MODE_ON BIT(0)
+
+struct max8907_led_data {
+ struct max8907 *max8907;
+ struct regmap *regmap;
+ struct led_classdev cdev;
+};
+
+static enum led_brightness max8907_led_get_brightness(struct led_classdev *cdev)
+{
+ struct max8907_led_data *ddata = container_of(cdev, struct max8907_led_data, cdev);
+ unsigned int brightness_lvl;
+ int ret;
+
+ ret = regmap_read(ddata->regmap, MAX8907_REG_WLED_MODE_CNTL, &brightness_lvl);
+ if (ret || !(brightness_lvl & MAX8907_REG_WLED_MODE_ON))
+ return LED_OFF;
+
+ ret = regmap_read(ddata->regmap, MAX8907_REG_WLED_CNTL, &brightness_lvl);
+ if (ret)
+ return LED_OFF;
+
+ return brightness_lvl;
+}
+
+static int max8907_led_set_brightness(struct led_classdev *cdev,
+ enum led_brightness brightness)
+{
+ struct max8907_led_data *ddata = container_of(cdev, struct max8907_led_data, cdev);
+ int brightness_lvl, ret;
+
+ brightness_lvl = clamp(brightness, LED_OFF, LED_FULL);
+
+ if (brightness_lvl) {
+ ret = regmap_write(ddata->regmap, MAX8907_REG_WLED_CNTL, brightness_lvl);
+ if (ret)
+ return ret;
+ }
+
+ return regmap_update_bits(ddata->regmap, MAX8907_REG_WLED_MODE_CNTL,
+ MAX8907_REG_WLED_MODE_ON,
+ brightness_lvl ? MAX8907_REG_WLED_MODE_ON : 0);
+}
+
+static int max8907_led_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct max8907 *max8907 = dev_get_drvdata(dev->parent);
+ struct led_init_data init_data = { };
+ struct max8907_led_data *ddata;
+
+ ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
+ if (!ddata)
+ return -ENOMEM;
+
+ ddata->max8907 = max8907;
+ ddata->regmap = max8907->regmap_gen;
+
+ init_data.fwnode = dev_fwnode(dev->parent);
+
+ ddata->cdev.brightness = LED_OFF;
+ ddata->cdev.flags = LED_CORE_SUSPENDRESUME;
+ ddata->cdev.max_brightness = LED_FULL;
+
+ ddata->cdev.brightness_get = max8907_led_get_brightness;
+ ddata->cdev.brightness_set_blocking = max8907_led_set_brightness;
+
+ return devm_led_classdev_register_ext(dev, &ddata->cdev, &init_data);
+}
+
+static struct platform_driver max8907_led_driver = {
+ .driver.name = "max8907-leds",
+ .probe = max8907_led_probe,
+};
+module_platform_driver(max8907_led_driver);
+
+MODULE_ALIAS("platform:max8907-leds");
+MODULE_AUTHOR("Svyatolsav Ryhel <clamor95@gmail.com>");
+MODULE_DESCRIPTION("MAX8907 LED driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/mfd/max8907.c b/drivers/mfd/max8907.c
index e282dd82ece69..826d7768fd8d0 100644
--- a/drivers/mfd/max8907.c
+++ b/drivers/mfd/max8907.c
@@ -20,6 +20,7 @@
#include <linux/slab.h>
static const struct mfd_cell max8907_cells[] = {
+ { .name = "max8907-leds", },
{ .name = "max8907-regulator", },
{ .name = "max8907-rtc", },
};
diff --git a/include/linux/mfd/max8907.h b/include/linux/mfd/max8907.h
index e222250f0cc1e..35b2ede54f856 100644
--- a/include/linux/mfd/max8907.h
+++ b/include/linux/mfd/max8907.h
@@ -112,7 +112,7 @@
#define MAX8907_REG_CHG_IRQ2_MASK 0x81
#define MAX8907_REG_CHG_STAT 0x82
#define MAX8907_REG_WLED_MODE_CNTL 0x84
-#define MAX8907_REG_ILED_CNTL 0x84
+#define MAX8907_REG_WLED_CNTL 0x85
#define MAX8907_REG_II1RR 0x8E
#define MAX8907_REG_II2RR 0x8F
#define MAX8907_REG_LDOCTL20 0x9C
--
2.53.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v4] leds: Add driver for MAX8907 LED cell
2026-10-27 11:50 [PATCH v4] leds: Add driver for MAX8907 LED cell Svyatoslav Ryhel
@ 2026-11-04 15:16 ` Lee Jones
2026-11-04 15:38 ` Svyatoslav Ryhel
2026-11-04 17:11 ` Svyatoslav Ryhel
0 siblings, 2 replies; 5+ messages in thread
From: Lee Jones @ 2026-11-04 15:16 UTC (permalink / raw)
To: Svyatoslav Ryhel; +Cc: Pavel Machek, linux-kernel, linux-leds, mfd
--- checkpatch.pl (1 issue) ---
Line 38: WARNING: please write a help paragraph that fully describes the config symbol with at least 4 lines
----------------------------------
On Tue, 27 Oct 2026, Svyatoslav Ryhel wrote:
> Add a small driver to control the LED cell provided by the MAX8907 PMIC.
> The LED does not require any specific configuration and is folded into the
> PMIC's node.
>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
> Changes in v2:
> - rebased so patch is now applicable to linux/next
>
> Changes in v3:
> - added check if led is off for get callback
> - added dedicated MAX8907_REG_WLED_MODE_ON mask
> - switched to devm_led_classdev_register_ext
>
> Changes in v4:
> - platform_set_drvdata dropped
> - init_data.devicename dropped
> - added MODULE_ALIAS
> ---
> drivers/leds/Kconfig | 7 +++
> drivers/leds/Makefile | 1 +
> drivers/leds/leds-max8907.c | 90 +++++++++++++++++++++++++++++++++++++
> drivers/mfd/max8907.c | 1 +
> include/linux/mfd/max8907.h | 2 +-
> 5 files changed, 100 insertions(+), 1 deletion(-)
> create mode 100644 drivers/leds/leds-max8907.c
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 788e689fe875c..3796c562919fe 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -813,6 +813,13 @@ config LEDS_MAX77705
> help
> LED driver for MAX77705 PMIC from Maxim Integrated.
>
> +config LEDS_MAX8907
> + tristate "LED support for MAX8907 PMIC"
> + depends on LEDS_CLASS && MFD_MAX8907
> + help
> + This option enables support for on-chip LED drivers on
> + MAXIM MAX8907 PMIC.
As requested on v3, please expand this help text to at least 4 lines to satisfy
'checkpatch.pl'.
> +
> config LEDS_MAX8997
> tristate "LED support for MAX8997 PMIC"
> depends on LEDS_CLASS && MFD_MAX8997
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index 3da746279bb81..24efda3fbaa44 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -67,6 +67,7 @@ obj-$(CONFIG_LEDS_LTC3220) += leds-ltc3220.o
> obj-$(CONFIG_LEDS_MAX5970) += leds-max5970.o
> obj-$(CONFIG_LEDS_MAX77650) += leds-max77650.o
> obj-$(CONFIG_LEDS_MAX77705) += leds-max77705.o
> +obj-$(CONFIG_LEDS_MAX8907) += leds-max8907.o
> obj-$(CONFIG_LEDS_MAX8997) += leds-max8997.o
> obj-$(CONFIG_LEDS_MC13783) += leds-mc13783.o
> obj-$(CONFIG_LEDS_MENF21BMC) += leds-menf21bmc.o
> diff --git a/drivers/leds/leds-max8907.c b/drivers/leds/leds-max8907.c
> new file mode 100644
> index 0000000000000..a5c445f75d1b1
> --- /dev/null
> +++ b/drivers/leds/leds-max8907.c
> @@ -0,0 +1,90 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/err.h>
> +#include <linux/leds.h>
> +#include <linux/mfd/max8907.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +
> +#define MAX8907_REG_WLED_MODE_ON BIT(0)
> +
> +struct max8907_led_data {
> + struct max8907 *max8907;
As asked on v3, what is 'max8907' used for?
> + struct regmap *regmap;
> + struct led_classdev cdev;
> +};
> +
> +static enum led_brightness max8907_led_get_brightness(struct led_classdev *cdev)
> +{
> + struct max8907_led_data *ddata = container_of(cdev, struct max8907_led_data, cdev);
> + unsigned int brightness_lvl;
> + int ret;
> +
> + ret = regmap_read(ddata->regmap, MAX8907_REG_WLED_MODE_CNTL, &brightness_lvl);
> + if (ret || !(brightness_lvl & MAX8907_REG_WLED_MODE_ON))
> + return LED_OFF;
> +
> + ret = regmap_read(ddata->regmap, MAX8907_REG_WLED_CNTL, &brightness_lvl);
> + if (ret)
> + return LED_OFF;
> +
> + return brightness_lvl;
> +}
> +
> +static int max8907_led_set_brightness(struct led_classdev *cdev,
> + enum led_brightness brightness)
> +{
> + struct max8907_led_data *ddata = container_of(cdev, struct max8907_led_data, cdev);
> + int brightness_lvl, ret;
> +
> + brightness_lvl = clamp(brightness, LED_OFF, LED_FULL);
> +
> + if (brightness_lvl) {
> + ret = regmap_write(ddata->regmap, MAX8907_REG_WLED_CNTL, brightness_lvl);
> + if (ret)
> + return ret;
> + }
> +
> + return regmap_update_bits(ddata->regmap, MAX8907_REG_WLED_MODE_CNTL,
> + MAX8907_REG_WLED_MODE_ON,
> + brightness_lvl ? MAX8907_REG_WLED_MODE_ON : 0);
> +}
> +
> +static int max8907_led_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct max8907 *max8907 = dev_get_drvdata(dev->parent);
> + struct led_init_data init_data = { };
> + struct max8907_led_data *ddata;
> +
> + ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
> + if (!ddata)
> + return -ENOMEM;
> +
> + ddata->max8907 = max8907;
> + ddata->regmap = max8907->regmap_gen;
> +
> + init_data.fwnode = dev_fwnode(dev->parent);
This will evaluate to 'pmic'. The sysfs attribute will be incorrect.
Since there is no child node, why not set 'ddata->cdev.name' directly
and use 'devm_led_classdev_register()'?
> +
> + ddata->cdev.brightness = LED_OFF;
> + ddata->cdev.flags = LED_CORE_SUSPENDRESUME;
> + ddata->cdev.max_brightness = LED_FULL;
> +
> + ddata->cdev.brightness_get = max8907_led_get_brightness;
> + ddata->cdev.brightness_set_blocking = max8907_led_set_brightness;
> +
> + return devm_led_classdev_register_ext(dev, &ddata->cdev, &init_data);
> +}
> +
> +static struct platform_driver max8907_led_driver = {
> + .driver.name = "max8907-leds",
Cute. But please expand.
> + .probe = max8907_led_probe,
> +};
> +module_platform_driver(max8907_led_driver);
> +
> +MODULE_ALIAS("platform:max8907-leds");
> +MODULE_AUTHOR("Svyatolsav Ryhel <clamor95@gmail.com>");
Your name is misspelt here.
> +MODULE_DESCRIPTION("MAX8907 LED driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/mfd/max8907.c b/drivers/mfd/max8907.c
> index e282dd82ece69..826d7768fd8d0 100644
> --- a/drivers/mfd/max8907.c
> +++ b/drivers/mfd/max8907.c
> @@ -20,6 +20,7 @@
> #include <linux/slab.h>
>
> static const struct mfd_cell max8907_cells[] = {
> + { .name = "max8907-leds", },
Separate patch.
> { .name = "max8907-regulator", },
> { .name = "max8907-rtc", },
> };
> diff --git a/include/linux/mfd/max8907.h b/include/linux/mfd/max8907.h
> index e222250f0cc1e..35b2ede54f856 100644
> --- a/include/linux/mfd/max8907.h
> +++ b/include/linux/mfd/max8907.h
> @@ -112,7 +112,7 @@
> #define MAX8907_REG_CHG_IRQ2_MASK 0x81
> #define MAX8907_REG_CHG_STAT 0x82
> #define MAX8907_REG_WLED_MODE_CNTL 0x84
> -#define MAX8907_REG_ILED_CNTL 0x84
> +#define MAX8907_REG_WLED_CNTL 0x85
> #define MAX8907_REG_II1RR 0x8E
> #define MAX8907_REG_II2RR 0x8F
> #define MAX8907_REG_LDOCTL20 0x9C
> --
> 2.53.0
>
>
--
Lee Jones
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v4] leds: Add driver for MAX8907 LED cell
2026-11-04 15:16 ` Lee Jones
@ 2026-11-04 15:38 ` Svyatoslav Ryhel
2026-11-04 16:02 ` Lee Jones
2026-11-04 17:11 ` Svyatoslav Ryhel
1 sibling, 1 reply; 5+ messages in thread
From: Svyatoslav Ryhel @ 2026-11-04 15:38 UTC (permalink / raw)
To: Lee Jones; +Cc: Pavel Machek, linux-kernel, linux-leds, mfd
ср, 4 лист. 2026 р. о 17:16 Lee Jones <lee@kernel.org> пише:
>
> --- checkpatch.pl (1 issue) ---
> Line 38: WARNING: please write a help paragraph that fully describes the config symbol with at least 4 lines
> ----------------------------------
>
> On Tue, 27 Oct 2026, Svyatoslav Ryhel wrote:
>
> > Add a small driver to control the LED cell provided by the MAX8907 PMIC.
> > The LED does not require any specific configuration and is folded into the
> > PMIC's node.
> >
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > ---
> > Changes in v2:
> > - rebased so patch is now applicable to linux/next
> >
> > Changes in v3:
> > - added check if led is off for get callback
> > - added dedicated MAX8907_REG_WLED_MODE_ON mask
> > - switched to devm_led_classdev_register_ext
> >
> > Changes in v4:
> > - platform_set_drvdata dropped
> > - init_data.devicename dropped
> > - added MODULE_ALIAS
> > ---
> > drivers/leds/Kconfig | 7 +++
> > drivers/leds/Makefile | 1 +
> > drivers/leds/leds-max8907.c | 90 +++++++++++++++++++++++++++++++++++++
> > drivers/mfd/max8907.c | 1 +
> > include/linux/mfd/max8907.h | 2 +-
> > 5 files changed, 100 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/leds/leds-max8907.c
> >
> > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> > index 788e689fe875c..3796c562919fe 100644
> > --- a/drivers/leds/Kconfig
> > +++ b/drivers/leds/Kconfig
> > @@ -813,6 +813,13 @@ config LEDS_MAX77705
> > help
> > LED driver for MAX77705 PMIC from Maxim Integrated.
> >
> > +config LEDS_MAX8907
> > + tristate "LED support for MAX8907 PMIC"
> > + depends on LEDS_CLASS && MFD_MAX8907
> > + help
> > + This option enables support for on-chip LED drivers on
> > + MAXIM MAX8907 PMIC.
>
> As requested on v3, please expand this help text to at least 4 lines to satisfy
> 'checkpatch.pl'.
>
> > +
> > config LEDS_MAX8997
> > tristate "LED support for MAX8997 PMIC"
> > depends on LEDS_CLASS && MFD_MAX8997
> > diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> > index 3da746279bb81..24efda3fbaa44 100644
> > --- a/drivers/leds/Makefile
> > +++ b/drivers/leds/Makefile
> > @@ -67,6 +67,7 @@ obj-$(CONFIG_LEDS_LTC3220) += leds-ltc3220.o
> > obj-$(CONFIG_LEDS_MAX5970) += leds-max5970.o
> > obj-$(CONFIG_LEDS_MAX77650) += leds-max77650.o
> > obj-$(CONFIG_LEDS_MAX77705) += leds-max77705.o
> > +obj-$(CONFIG_LEDS_MAX8907) += leds-max8907.o
> > obj-$(CONFIG_LEDS_MAX8997) += leds-max8997.o
> > obj-$(CONFIG_LEDS_MC13783) += leds-mc13783.o
> > obj-$(CONFIG_LEDS_MENF21BMC) += leds-menf21bmc.o
> > diff --git a/drivers/leds/leds-max8907.c b/drivers/leds/leds-max8907.c
> > new file mode 100644
> > index 0000000000000..a5c445f75d1b1
> > --- /dev/null
> > +++ b/drivers/leds/leds-max8907.c
> > @@ -0,0 +1,90 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +
> > +#include <linux/err.h>
> > +#include <linux/leds.h>
> > +#include <linux/mfd/max8907.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/property.h>
> > +#include <linux/regmap.h>
> > +
> > +#define MAX8907_REG_WLED_MODE_ON BIT(0)
> > +
> > +struct max8907_led_data {
> > + struct max8907 *max8907;
>
> As asked on v3, what is 'max8907' used for?
>
> > + struct regmap *regmap;
> > + struct led_classdev cdev;
> > +};
> > +
> > +static enum led_brightness max8907_led_get_brightness(struct led_classdev *cdev)
> > +{
> > + struct max8907_led_data *ddata = container_of(cdev, struct max8907_led_data, cdev);
> > + unsigned int brightness_lvl;
> > + int ret;
> > +
> > + ret = regmap_read(ddata->regmap, MAX8907_REG_WLED_MODE_CNTL, &brightness_lvl);
> > + if (ret || !(brightness_lvl & MAX8907_REG_WLED_MODE_ON))
> > + return LED_OFF;
> > +
> > + ret = regmap_read(ddata->regmap, MAX8907_REG_WLED_CNTL, &brightness_lvl);
> > + if (ret)
> > + return LED_OFF;
> > +
> > + return brightness_lvl;
> > +}
> > +
> > +static int max8907_led_set_brightness(struct led_classdev *cdev,
> > + enum led_brightness brightness)
> > +{
> > + struct max8907_led_data *ddata = container_of(cdev, struct max8907_led_data, cdev);
> > + int brightness_lvl, ret;
> > +
> > + brightness_lvl = clamp(brightness, LED_OFF, LED_FULL);
> > +
> > + if (brightness_lvl) {
> > + ret = regmap_write(ddata->regmap, MAX8907_REG_WLED_CNTL, brightness_lvl);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + return regmap_update_bits(ddata->regmap, MAX8907_REG_WLED_MODE_CNTL,
> > + MAX8907_REG_WLED_MODE_ON,
> > + brightness_lvl ? MAX8907_REG_WLED_MODE_ON : 0);
> > +}
> > +
> > +static int max8907_led_probe(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct max8907 *max8907 = dev_get_drvdata(dev->parent);
> > + struct led_init_data init_data = { };
> > + struct max8907_led_data *ddata;
> > +
> > + ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
> > + if (!ddata)
> > + return -ENOMEM;
> > +
> > + ddata->max8907 = max8907;
> > + ddata->regmap = max8907->regmap_gen;
> > +
> > + init_data.fwnode = dev_fwnode(dev->parent);
>
> This will evaluate to 'pmic'. The sysfs attribute will be incorrect.
>
> Since there is no child node, why not set 'ddata->cdev.name' directly
> and use 'devm_led_classdev_register()'?
>
> > +
> > + ddata->cdev.brightness = LED_OFF;
> > + ddata->cdev.flags = LED_CORE_SUSPENDRESUME;
> > + ddata->cdev.max_brightness = LED_FULL;
> > +
> > + ddata->cdev.brightness_get = max8907_led_get_brightness;
> > + ddata->cdev.brightness_set_blocking = max8907_led_set_brightness;
> > +
> > + return devm_led_classdev_register_ext(dev, &ddata->cdev, &init_data);
> > +}
> > +
> > +static struct platform_driver max8907_led_driver = {
> > + .driver.name = "max8907-leds",
>
> Cute. But please expand.
>
> > + .probe = max8907_led_probe,
> > +};
> > +module_platform_driver(max8907_led_driver);
> > +
> > +MODULE_ALIAS("platform:max8907-leds");
> > +MODULE_AUTHOR("Svyatolsav Ryhel <clamor95@gmail.com>");
>
> Your name is misspelt here.
>
> > +MODULE_DESCRIPTION("MAX8907 LED driver");
> > +MODULE_LICENSE("GPL");
> > diff --git a/drivers/mfd/max8907.c b/drivers/mfd/max8907.c
> > index e282dd82ece69..826d7768fd8d0 100644
> > --- a/drivers/mfd/max8907.c
> > +++ b/drivers/mfd/max8907.c
> > @@ -20,6 +20,7 @@
> > #include <linux/slab.h>
> >
> > static const struct mfd_cell max8907_cells[] = {
> > + { .name = "max8907-leds", },
>
> Separate patch.
>
All valid, but way adding led requires a separate patch? Just to know.
The max8907.h change can be left with leds change? It changes reg name
used by led cell.
> > { .name = "max8907-regulator", },
> > { .name = "max8907-rtc", },
> > };
> > diff --git a/include/linux/mfd/max8907.h b/include/linux/mfd/max8907.h
> > index e222250f0cc1e..35b2ede54f856 100644
> > --- a/include/linux/mfd/max8907.h
> > +++ b/include/linux/mfd/max8907.h
> > @@ -112,7 +112,7 @@
> > #define MAX8907_REG_CHG_IRQ2_MASK 0x81
> > #define MAX8907_REG_CHG_STAT 0x82
> > #define MAX8907_REG_WLED_MODE_CNTL 0x84
> > -#define MAX8907_REG_ILED_CNTL 0x84
> > +#define MAX8907_REG_WLED_CNTL 0x85
> > #define MAX8907_REG_II1RR 0x8E
> > #define MAX8907_REG_II2RR 0x8F
> > #define MAX8907_REG_LDOCTL20 0x9C
> > --
> > 2.53.0
> >
> >
>
> --
> Lee Jones
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v4] leds: Add driver for MAX8907 LED cell
2026-11-04 15:38 ` Svyatoslav Ryhel
@ 2026-11-04 16:02 ` Lee Jones
0 siblings, 0 replies; 5+ messages in thread
From: Lee Jones @ 2026-11-04 16:02 UTC (permalink / raw)
To: Svyatoslav Ryhel; +Cc: Pavel Machek, linux-kernel, linux-leds, mfd
On Wed, 04 Nov 2026, Svyatoslav Ryhel wrote:
> ср, 4 лист. 2026 р. о 17:16 Lee Jones <lee@kernel.org> пише:
> >
> > --- checkpatch.pl (1 issue) ---
> > Line 38: WARNING: please write a help paragraph that fully describes the config symbol with at least 4 lines
> > ----------------------------------
> >
> > On Tue, 27 Oct 2026, Svyatoslav Ryhel wrote:
> >
> > > Add a small driver to control the LED cell provided by the MAX8907 PMIC.
> > > The LED does not require any specific configuration and is folded into the
> > > PMIC's node.
> > >
> > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > ---
> > > Changes in v2:
> > > - rebased so patch is now applicable to linux/next
> > >
> > > Changes in v3:
> > > - added check if led is off for get callback
> > > - added dedicated MAX8907_REG_WLED_MODE_ON mask
> > > - switched to devm_led_classdev_register_ext
> > >
> > > Changes in v4:
> > > - platform_set_drvdata dropped
> > > - init_data.devicename dropped
> > > - added MODULE_ALIAS
> > > ---
> > > drivers/leds/Kconfig | 7 +++
> > > drivers/leds/Makefile | 1 +
> > > drivers/leds/leds-max8907.c | 90 +++++++++++++++++++++++++++++++++++++
> > > drivers/mfd/max8907.c | 1 +
> > > include/linux/mfd/max8907.h | 2 +-
> > > 5 files changed, 100 insertions(+), 1 deletion(-)
> > > create mode 100644 drivers/leds/leds-max8907.c
> > >
> > > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> > > index 788e689fe875c..3796c562919fe 100644
> > > --- a/drivers/leds/Kconfig
> > > +++ b/drivers/leds/Kconfig
> > > @@ -813,6 +813,13 @@ config LEDS_MAX77705
> > > help
> > > LED driver for MAX77705 PMIC from Maxim Integrated.
> > >
> > > +config LEDS_MAX8907
> > > + tristate "LED support for MAX8907 PMIC"
> > > + depends on LEDS_CLASS && MFD_MAX8907
> > > + help
> > > + This option enables support for on-chip LED drivers on
> > > + MAXIM MAX8907 PMIC.
> >
> > As requested on v3, please expand this help text to at least 4 lines to satisfy
> > 'checkpatch.pl'.
> >
> > > +
> > > config LEDS_MAX8997
> > > tristate "LED support for MAX8997 PMIC"
> > > depends on LEDS_CLASS && MFD_MAX8997
> > > diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> > > index 3da746279bb81..24efda3fbaa44 100644
> > > --- a/drivers/leds/Makefile
> > > +++ b/drivers/leds/Makefile
> > > @@ -67,6 +67,7 @@ obj-$(CONFIG_LEDS_LTC3220) += leds-ltc3220.o
> > > obj-$(CONFIG_LEDS_MAX5970) += leds-max5970.o
> > > obj-$(CONFIG_LEDS_MAX77650) += leds-max77650.o
> > > obj-$(CONFIG_LEDS_MAX77705) += leds-max77705.o
> > > +obj-$(CONFIG_LEDS_MAX8907) += leds-max8907.o
> > > obj-$(CONFIG_LEDS_MAX8997) += leds-max8997.o
> > > obj-$(CONFIG_LEDS_MC13783) += leds-mc13783.o
> > > obj-$(CONFIG_LEDS_MENF21BMC) += leds-menf21bmc.o
> > > diff --git a/drivers/leds/leds-max8907.c b/drivers/leds/leds-max8907.c
> > > new file mode 100644
> > > index 0000000000000..a5c445f75d1b1
> > > --- /dev/null
> > > +++ b/drivers/leds/leds-max8907.c
> > > @@ -0,0 +1,90 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +
> > > +#include <linux/err.h>
> > > +#include <linux/leds.h>
> > > +#include <linux/mfd/max8907.h>
> > > +#include <linux/module.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/property.h>
> > > +#include <linux/regmap.h>
> > > +
> > > +#define MAX8907_REG_WLED_MODE_ON BIT(0)
> > > +
> > > +struct max8907_led_data {
> > > + struct max8907 *max8907;
> >
> > As asked on v3, what is 'max8907' used for?
> >
> > > + struct regmap *regmap;
> > > + struct led_classdev cdev;
> > > +};
> > > +
> > > +static enum led_brightness max8907_led_get_brightness(struct led_classdev *cdev)
> > > +{
> > > + struct max8907_led_data *ddata = container_of(cdev, struct max8907_led_data, cdev);
> > > + unsigned int brightness_lvl;
> > > + int ret;
> > > +
> > > + ret = regmap_read(ddata->regmap, MAX8907_REG_WLED_MODE_CNTL, &brightness_lvl);
> > > + if (ret || !(brightness_lvl & MAX8907_REG_WLED_MODE_ON))
> > > + return LED_OFF;
> > > +
> > > + ret = regmap_read(ddata->regmap, MAX8907_REG_WLED_CNTL, &brightness_lvl);
> > > + if (ret)
> > > + return LED_OFF;
> > > +
> > > + return brightness_lvl;
> > > +}
> > > +
> > > +static int max8907_led_set_brightness(struct led_classdev *cdev,
> > > + enum led_brightness brightness)
> > > +{
> > > + struct max8907_led_data *ddata = container_of(cdev, struct max8907_led_data, cdev);
> > > + int brightness_lvl, ret;
> > > +
> > > + brightness_lvl = clamp(brightness, LED_OFF, LED_FULL);
> > > +
> > > + if (brightness_lvl) {
> > > + ret = regmap_write(ddata->regmap, MAX8907_REG_WLED_CNTL, brightness_lvl);
> > > + if (ret)
> > > + return ret;
> > > + }
> > > +
> > > + return regmap_update_bits(ddata->regmap, MAX8907_REG_WLED_MODE_CNTL,
> > > + MAX8907_REG_WLED_MODE_ON,
> > > + brightness_lvl ? MAX8907_REG_WLED_MODE_ON : 0);
> > > +}
> > > +
> > > +static int max8907_led_probe(struct platform_device *pdev)
> > > +{
> > > + struct device *dev = &pdev->dev;
> > > + struct max8907 *max8907 = dev_get_drvdata(dev->parent);
> > > + struct led_init_data init_data = { };
> > > + struct max8907_led_data *ddata;
> > > +
> > > + ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
> > > + if (!ddata)
> > > + return -ENOMEM;
> > > +
> > > + ddata->max8907 = max8907;
> > > + ddata->regmap = max8907->regmap_gen;
> > > +
> > > + init_data.fwnode = dev_fwnode(dev->parent);
> >
> > This will evaluate to 'pmic'. The sysfs attribute will be incorrect.
> >
> > Since there is no child node, why not set 'ddata->cdev.name' directly
> > and use 'devm_led_classdev_register()'?
> >
> > > +
> > > + ddata->cdev.brightness = LED_OFF;
> > > + ddata->cdev.flags = LED_CORE_SUSPENDRESUME;
> > > + ddata->cdev.max_brightness = LED_FULL;
> > > +
> > > + ddata->cdev.brightness_get = max8907_led_get_brightness;
> > > + ddata->cdev.brightness_set_blocking = max8907_led_set_brightness;
> > > +
> > > + return devm_led_classdev_register_ext(dev, &ddata->cdev, &init_data);
> > > +}
> > > +
> > > +static struct platform_driver max8907_led_driver = {
> > > + .driver.name = "max8907-leds",
> >
> > Cute. But please expand.
> >
> > > + .probe = max8907_led_probe,
> > > +};
> > > +module_platform_driver(max8907_led_driver);
> > > +
> > > +MODULE_ALIAS("platform:max8907-leds");
> > > +MODULE_AUTHOR("Svyatolsav Ryhel <clamor95@gmail.com>");
> >
> > Your name is misspelt here.
> >
> > > +MODULE_DESCRIPTION("MAX8907 LED driver");
> > > +MODULE_LICENSE("GPL");
> > > diff --git a/drivers/mfd/max8907.c b/drivers/mfd/max8907.c
> > > index e282dd82ece69..826d7768fd8d0 100644
> > > --- a/drivers/mfd/max8907.c
> > > +++ b/drivers/mfd/max8907.c
> > > @@ -20,6 +20,7 @@
> > > #include <linux/slab.h>
> > >
> > > static const struct mfd_cell max8907_cells[] = {
> > > + { .name = "max8907-leds", },
> >
> > Separate patch.
> >
>
> All valid, but way adding led requires a separate patch? Just to know.
Build-deps only.
> The max8907.h change can be left with leds change? It changes reg name
> used by led cell.
Yes. That's a build-dep.
> > > { .name = "max8907-regulator", },
> > > { .name = "max8907-rtc", },
> > > };
> > > diff --git a/include/linux/mfd/max8907.h b/include/linux/mfd/max8907.h
> > > index e222250f0cc1e..35b2ede54f856 100644
> > > --- a/include/linux/mfd/max8907.h
> > > +++ b/include/linux/mfd/max8907.h
> > > @@ -112,7 +112,7 @@
> > > #define MAX8907_REG_CHG_IRQ2_MASK 0x81
> > > #define MAX8907_REG_CHG_STAT 0x82
> > > #define MAX8907_REG_WLED_MODE_CNTL 0x84
> > > -#define MAX8907_REG_ILED_CNTL 0x84
> > > +#define MAX8907_REG_WLED_CNTL 0x85
> > > #define MAX8907_REG_II1RR 0x8E
> > > #define MAX8907_REG_II2RR 0x8F
> > > #define MAX8907_REG_LDOCTL20 0x9C
> > > --
> > > 2.53.0
> > >
> > >
> >
> > --
> > Lee Jones
--
Lee Jones
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v4] leds: Add driver for MAX8907 LED cell
2026-11-04 15:16 ` Lee Jones
2026-11-04 15:38 ` Svyatoslav Ryhel
@ 2026-11-04 17:11 ` Svyatoslav Ryhel
1 sibling, 0 replies; 5+ messages in thread
From: Svyatoslav Ryhel @ 2026-11-04 17:11 UTC (permalink / raw)
To: Lee Jones; +Cc: Pavel Machek, linux-kernel, linux-leds, mfd
ср, 4 лист. 2026 р. о 17:16 Lee Jones <lee@kernel.org> пише:
>
> --- checkpatch.pl (1 issue) ---
> Line 38: WARNING: please write a help paragraph that fully describes the config symbol with at least 4 lines
> ----------------------------------
>
> On Tue, 27 Oct 2026, Svyatoslav Ryhel wrote:
>
> > Add a small driver to control the LED cell provided by the MAX8907 PMIC.
> > The LED does not require any specific configuration and is folded into the
> > PMIC's node.
> >
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > ---
> > Changes in v2:
> > - rebased so patch is now applicable to linux/next
> >
> > Changes in v3:
> > - added check if led is off for get callback
> > - added dedicated MAX8907_REG_WLED_MODE_ON mask
> > - switched to devm_led_classdev_register_ext
> >
> > Changes in v4:
> > - platform_set_drvdata dropped
> > - init_data.devicename dropped
> > - added MODULE_ALIAS
> > ---
> > drivers/leds/Kconfig | 7 +++
> > drivers/leds/Makefile | 1 +
> > drivers/leds/leds-max8907.c | 90 +++++++++++++++++++++++++++++++++++++
> > drivers/mfd/max8907.c | 1 +
> > include/linux/mfd/max8907.h | 2 +-
> > 5 files changed, 100 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/leds/leds-max8907.c
> >
> > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> > index 788e689fe875c..3796c562919fe 100644
> > --- a/drivers/leds/Kconfig
> > +++ b/drivers/leds/Kconfig
> > @@ -813,6 +813,13 @@ config LEDS_MAX77705
> > help
> > LED driver for MAX77705 PMIC from Maxim Integrated.
> >
> > +config LEDS_MAX8907
> > + tristate "LED support for MAX8907 PMIC"
> > + depends on LEDS_CLASS && MFD_MAX8907
> > + help
> > + This option enables support for on-chip LED drivers on
> > + MAXIM MAX8907 PMIC.
>
> As requested on v3, please expand this help text to at least 4 lines to satisfy
> 'checkpatch.pl'.
>
> > +
> > config LEDS_MAX8997
> > tristate "LED support for MAX8997 PMIC"
> > depends on LEDS_CLASS && MFD_MAX8997
> > diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> > index 3da746279bb81..24efda3fbaa44 100644
> > --- a/drivers/leds/Makefile
> > +++ b/drivers/leds/Makefile
> > @@ -67,6 +67,7 @@ obj-$(CONFIG_LEDS_LTC3220) += leds-ltc3220.o
> > obj-$(CONFIG_LEDS_MAX5970) += leds-max5970.o
> > obj-$(CONFIG_LEDS_MAX77650) += leds-max77650.o
> > obj-$(CONFIG_LEDS_MAX77705) += leds-max77705.o
> > +obj-$(CONFIG_LEDS_MAX8907) += leds-max8907.o
> > obj-$(CONFIG_LEDS_MAX8997) += leds-max8997.o
> > obj-$(CONFIG_LEDS_MC13783) += leds-mc13783.o
> > obj-$(CONFIG_LEDS_MENF21BMC) += leds-menf21bmc.o
> > diff --git a/drivers/leds/leds-max8907.c b/drivers/leds/leds-max8907.c
> > new file mode 100644
> > index 0000000000000..a5c445f75d1b1
> > --- /dev/null
> > +++ b/drivers/leds/leds-max8907.c
> > @@ -0,0 +1,90 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +
> > +#include <linux/err.h>
> > +#include <linux/leds.h>
> > +#include <linux/mfd/max8907.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/property.h>
> > +#include <linux/regmap.h>
> > +
> > +#define MAX8907_REG_WLED_MODE_ON BIT(0)
> > +
> > +struct max8907_led_data {
> > + struct max8907 *max8907;
>
> As asked on v3, what is 'max8907' used for?
>
> > + struct regmap *regmap;
> > + struct led_classdev cdev;
> > +};
> > +
> > +static enum led_brightness max8907_led_get_brightness(struct led_classdev *cdev)
> > +{
> > + struct max8907_led_data *ddata = container_of(cdev, struct max8907_led_data, cdev);
> > + unsigned int brightness_lvl;
> > + int ret;
> > +
> > + ret = regmap_read(ddata->regmap, MAX8907_REG_WLED_MODE_CNTL, &brightness_lvl);
> > + if (ret || !(brightness_lvl & MAX8907_REG_WLED_MODE_ON))
> > + return LED_OFF;
> > +
> > + ret = regmap_read(ddata->regmap, MAX8907_REG_WLED_CNTL, &brightness_lvl);
> > + if (ret)
> > + return LED_OFF;
> > +
> > + return brightness_lvl;
> > +}
> > +
> > +static int max8907_led_set_brightness(struct led_classdev *cdev,
> > + enum led_brightness brightness)
> > +{
> > + struct max8907_led_data *ddata = container_of(cdev, struct max8907_led_data, cdev);
> > + int brightness_lvl, ret;
> > +
> > + brightness_lvl = clamp(brightness, LED_OFF, LED_FULL);
> > +
> > + if (brightness_lvl) {
> > + ret = regmap_write(ddata->regmap, MAX8907_REG_WLED_CNTL, brightness_lvl);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + return regmap_update_bits(ddata->regmap, MAX8907_REG_WLED_MODE_CNTL,
> > + MAX8907_REG_WLED_MODE_ON,
> > + brightness_lvl ? MAX8907_REG_WLED_MODE_ON : 0);
> > +}
> > +
> > +static int max8907_led_probe(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct max8907 *max8907 = dev_get_drvdata(dev->parent);
> > + struct led_init_data init_data = { };
> > + struct max8907_led_data *ddata;
> > +
> > + ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
> > + if (!ddata)
> > + return -ENOMEM;
> > +
> > + ddata->max8907 = max8907;
> > + ddata->regmap = max8907->regmap_gen;
> > +
> > + init_data.fwnode = dev_fwnode(dev->parent);
>
> This will evaluate to 'pmic'. The sysfs attribute will be incorrect.
>
> Since there is no child node, why not set 'ddata->cdev.name' directly
> and use 'devm_led_classdev_register()'?
>
This is required to be able to reference led in the device tree. Since
led node is folded into pmic's node, pmic's node is used to bind led.
> > +
> > + ddata->cdev.brightness = LED_OFF;
> > + ddata->cdev.flags = LED_CORE_SUSPENDRESUME;
> > + ddata->cdev.max_brightness = LED_FULL;
> > +
> > + ddata->cdev.brightness_get = max8907_led_get_brightness;
> > + ddata->cdev.brightness_set_blocking = max8907_led_set_brightness;
> > +
> > + return devm_led_classdev_register_ext(dev, &ddata->cdev, &init_data);
> > +}
> > +
> > +static struct platform_driver max8907_led_driver = {
> > + .driver.name = "max8907-leds",
>
> Cute. But please expand.
>
> > + .probe = max8907_led_probe,
> > +};
> > +module_platform_driver(max8907_led_driver);
> > +
> > +MODULE_ALIAS("platform:max8907-leds");
> > +MODULE_AUTHOR("Svyatolsav Ryhel <clamor95@gmail.com>");
>
> Your name is misspelt here.
>
> > +MODULE_DESCRIPTION("MAX8907 LED driver");
> > +MODULE_LICENSE("GPL");
> > diff --git a/drivers/mfd/max8907.c b/drivers/mfd/max8907.c
> > index e282dd82ece69..826d7768fd8d0 100644
> > --- a/drivers/mfd/max8907.c
> > +++ b/drivers/mfd/max8907.c
> > @@ -20,6 +20,7 @@
> > #include <linux/slab.h>
> >
> > static const struct mfd_cell max8907_cells[] = {
> > + { .name = "max8907-leds", },
>
> Separate patch.
>
> > { .name = "max8907-regulator", },
> > { .name = "max8907-rtc", },
> > };
> > diff --git a/include/linux/mfd/max8907.h b/include/linux/mfd/max8907.h
> > index e222250f0cc1e..35b2ede54f856 100644
> > --- a/include/linux/mfd/max8907.h
> > +++ b/include/linux/mfd/max8907.h
> > @@ -112,7 +112,7 @@
> > #define MAX8907_REG_CHG_IRQ2_MASK 0x81
> > #define MAX8907_REG_CHG_STAT 0x82
> > #define MAX8907_REG_WLED_MODE_CNTL 0x84
> > -#define MAX8907_REG_ILED_CNTL 0x84
> > +#define MAX8907_REG_WLED_CNTL 0x85
> > #define MAX8907_REG_II1RR 0x8E
> > #define MAX8907_REG_II2RR 0x8F
> > #define MAX8907_REG_LDOCTL20 0x9C
> > --
> > 2.53.0
> >
> >
>
> --
> Lee Jones
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-11-04 17:11 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-27 11:50 [PATCH v4] leds: Add driver for MAX8907 LED cell Svyatoslav Ryhel
2026-11-04 15:16 ` Lee Jones
2026-11-04 15:38 ` Svyatoslav Ryhel
2026-11-04 16:02 ` Lee Jones
2026-11-04 17:11 ` Svyatoslav Ryhel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®