* [PATCH] mfd: rohm: Factor out power button registration
@ 2026-06-18 18:58 Dmitry Torokhov
2026-06-19 9:02 ` Lee Jones
2026-06-25 8:00 ` Matti Vaittinen
0 siblings, 2 replies; 6+ messages in thread
From: Dmitry Torokhov @ 2026-06-18 18:58 UTC (permalink / raw)
To: Lee Jones; +Cc: Matti Vaittinen, linux-kernel
Factor out the power button registration logic using software nodes
from rohm-bd718x7 and rohm-bd71828 drivers into a shared module
rohm-pwrbutton.
This reduces duplication and makes it easier to support other ROHM
PMICs with similar power button configurations.
Suggested-by: Lee Jones <lee@kernel.org>
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
MAINTAINERS | 2 +
drivers/mfd/Kconfig | 6 ++
drivers/mfd/Makefile | 1 +
drivers/mfd/rohm-bd71828.c | 84 ++------------------------
drivers/mfd/rohm-bd718x7.c | 84 ++------------------------
drivers/mfd/rohm-pwrbutton.c | 112 +++++++++++++++++++++++++++++++++++
drivers/mfd/rohm-pwrbutton.h | 12 ++++
7 files changed, 141 insertions(+), 160 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index f1caa6e5198b..40c46a7363fb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -23524,6 +23524,8 @@ F: drivers/mfd/rohm-bd71828.c
F: drivers/mfd/rohm-bd718x7.c
F: drivers/mfd/rohm-bd9576.c
F: drivers/mfd/rohm-bd96801.c
+F: drivers/mfd/rohm-pwrbutton.c
+F: drivers/mfd/rohm-pwrbutton.h
F: drivers/regulator/bd71815-regulator.c
F: drivers/regulator/bd71828-regulator.c
F: drivers/regulator/bd718x7-regulator.c
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 763ce6a34782..8d04e1b1f8c8 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -2208,6 +2208,10 @@ config MFD_STW481X
in various ST Microelectronics and ST-Ericsson embedded
Nomadik series.
+config MFD_ROHM_PWRBUTTON
+ tristate
+ select MFD_CORE
+
config MFD_ROHM_BD718XX
tristate "ROHM BD71837 Power Management IC"
depends on I2C=y
@@ -2215,6 +2219,7 @@ config MFD_ROHM_BD718XX
select REGMAP_I2C
select REGMAP_IRQ
select MFD_CORE
+ select MFD_ROHM_PWRBUTTON
help
Select this option to get support for the ROHM BD71837
Power Management ICs. BD71837 is designed to power processors like
@@ -2228,6 +2233,7 @@ config MFD_ROHM_BD71828
select REGMAP_I2C
select REGMAP_IRQ
select MFD_CORE
+ select MFD_ROHM_PWRBUTTON
help
Select this option to get support for the ROHM BD71815, BD71828,
BD71879, BD72720 and BD73900 Power Management ICs (PMICs). These are
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index dd4bb7e77c33..72d3944b0ad8 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -273,6 +273,7 @@ obj-$(CONFIG_MFD_STM32_TIMERS) += stm32-timers.o
obj-$(CONFIG_MFD_MXS_LRADC) += mxs-lradc.o
obj-$(CONFIG_MFD_SC27XX_PMIC) += sprd-sc27xx-spi.o
obj-$(CONFIG_RAVE_SP_CORE) += rave-sp.o
+obj-$(CONFIG_MFD_ROHM_PWRBUTTON) += rohm-pwrbutton.o
obj-$(CONFIG_MFD_ROHM_BD71828) += rohm-bd71828.o
obj-$(CONFIG_MFD_ROHM_BD718XX) += rohm-bd718x7.o
obj-$(CONFIG_MFD_ROHM_BD957XMUF) += rohm-bd9576.o
diff --git a/drivers/mfd/rohm-bd71828.c b/drivers/mfd/rohm-bd71828.c
index 5fb6142cf087..dfc23cd13ef2 100644
--- a/drivers/mfd/rohm-bd71828.c
+++ b/drivers/mfd/rohm-bd71828.c
@@ -5,8 +5,6 @@
* ROHM BD718[15/28/79] and BD72720 PMIC driver
*/
-#include <linux/device/devres.h>
-#include <linux/gfp_types.h>
#include <linux/i2c.h>
#include <linux/input.h>
#include <linux/interrupt.h>
@@ -19,10 +17,11 @@
#include <linux/mfd/rohm-generic.h>
#include <linux/module.h>
#include <linux/of.h>
-#include <linux/property.h>
#include <linux/regmap.h>
#include <linux/types.h>
+#include "rohm-pwrbutton.h"
+
#define BD72720_TYPED_IRQ_REG(_irq, _stat_offset, _mask, _type_offset) \
[_irq] = { \
.reg_offset = (_stat_offset), \
@@ -859,83 +858,7 @@ static int set_clk_mode(struct device *dev, struct regmap *regmap,
OUT32K_MODE_CMOS);
}
-static const struct property_entry bd71828_powerkey_parent_props[] = {
- PROPERTY_ENTRY_STRING("label", "bd71828-pwrkey"),
- { }
-};
-
-static const struct property_entry bd71828_powerkey_props[] = {
- PROPERTY_ENTRY_U32("linux,code", KEY_POWER),
- PROPERTY_ENTRY_BOOL("wakeup-source"),
- { }
-};
-
-#define GPIO_KEYS 0 /* Node corresponding to gpio-keys device itself */
-#define PWRON_KEY 1 /* Node describing power button in gpio-keys */
-
-static int bd71828_i2c_register_swnodes(const struct software_node *nodes)
-{
- const struct software_node * const node_group[] = {
- &nodes[GPIO_KEYS], &nodes[PWRON_KEY], NULL
- };
-
- return software_node_register_node_group(node_group);
-}
-
-static void bd71828_i2c_unregister_swnodes(void *data)
-{
- const struct software_node *nodes = data;
- const struct software_node * const node_group[] = {
- &nodes[GPIO_KEYS], &nodes[PWRON_KEY], NULL
- };
-
- software_node_unregister_node_group(node_group);
-}
-
-static int bd71828_i2c_register_pwrbutton(struct device *dev, int button_irq,
- struct irq_domain *irq_domain)
-{
- const struct resource res[] = {
- DEFINE_RES_IRQ_NAMED(button_irq, "bd71828-pwrkey"),
- };
- struct mfd_cell gpio_keys_cell = {
- .name = "gpio-keys",
- .resources = res,
- .num_resources = ARRAY_SIZE(res),
- };
- struct software_node *nodes;
- int ret;
- nodes = devm_kcalloc(dev, 2, sizeof(*nodes), GFP_KERNEL);
- if (!nodes)
- return -ENOMEM;
-
- nodes[GPIO_KEYS].name = devm_kasprintf(dev, GFP_KERNEL, "%s-power-key", dev_name(dev));
- if (!nodes[GPIO_KEYS].name)
- return -ENOMEM;
-
- nodes[GPIO_KEYS].properties = bd71828_powerkey_parent_props;
-
- nodes[PWRON_KEY].parent = &nodes[GPIO_KEYS];
- nodes[PWRON_KEY].properties = bd71828_powerkey_props;
-
- ret = bd71828_i2c_register_swnodes(nodes);
- if (ret)
- return ret;
-
- ret = devm_add_action_or_reset(dev, bd71828_i2c_unregister_swnodes, nodes);
- if (ret)
- return ret;
-
- gpio_keys_cell.swnode = &nodes[GPIO_KEYS];
-
- ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, &gpio_keys_cell, 1,
- NULL, 0, irq_domain);
- if (ret)
- return dev_err_probe(dev, ret, "Failed to register power-button");
-
- return 0;
-}
static struct i2c_client *bd71828_dev;
static void bd71828_power_off(void)
@@ -1096,7 +1019,8 @@ static int bd71828_i2c_probe(struct i2c_client *i2c)
return dev_err_probe(&i2c->dev, ret, "Failed to create subdevices\n");
if (button_irq) {
- ret = bd71828_i2c_register_pwrbutton(&i2c->dev, button_irq, irq_domain);
+ ret = rohm_mfd_register_pwrbutton(&i2c->dev, button_irq,
+ "bd71828-pwrkey", true, irq_domain);
if (ret)
return ret;
}
diff --git a/drivers/mfd/rohm-bd718x7.c b/drivers/mfd/rohm-bd718x7.c
index be2acc429fe3..bab3b1c2445b 100644
--- a/drivers/mfd/rohm-bd718x7.c
+++ b/drivers/mfd/rohm-bd718x7.c
@@ -7,8 +7,6 @@
// Datasheet for BD71837MWV available from
// https://www.rohm.com/datasheet/BD71837MWV/bd71837mwv-e
-#include <linux/device/devres.h>
-#include <linux/gfp_types.h>
#include <linux/i2c.h>
#include <linux/input.h>
#include <linux/interrupt.h>
@@ -16,10 +14,11 @@
#include <linux/mfd/core.h>
#include <linux/module.h>
#include <linux/of.h>
-#include <linux/property.h>
#include <linux/regmap.h>
#include <linux/types.h>
+#include "rohm-pwrbutton.h"
+
static struct mfd_cell bd71837_mfd_cells[] = {
{ .name = "bd71837-clk", },
{ .name = "bd71837-pmic", },
@@ -105,83 +104,7 @@ static int bd718xx_init_press_duration(struct regmap *regmap,
return 0;
}
-static const struct property_entry bd718xx_powerkey_parent_props[] = {
- PROPERTY_ENTRY_STRING("label", "bd718xx-pwrkey"),
- { }
-};
-
-static const struct property_entry bd718xx_powerkey_props[] = {
- PROPERTY_ENTRY_U32("linux,code", KEY_POWER),
- { }
-};
-
-static const struct resource bd718xx_powerkey_resources[] = {
- DEFINE_RES_IRQ_NAMED(BD718XX_INT_PWRBTN_S, "bd718xx-pwrkey"),
-};
-
-#define GPIO_KEYS 0 /* Node corresponding to gpio-keys device itself */
-#define PWRON_KEY 1 /* Node describing power button in gpio-keys */
-
-static int bd718xx_i2c_register_swnodes(const struct software_node *nodes)
-{
- const struct software_node * const node_group[] = {
- &nodes[GPIO_KEYS], &nodes[PWRON_KEY], NULL
- };
-
- return software_node_register_node_group(node_group);
-}
-
-static void bd718xx_i2c_unregister_swnodes(void *data)
-{
- const struct software_node *nodes = data;
- const struct software_node * const node_group[] = {
- &nodes[GPIO_KEYS], &nodes[PWRON_KEY], NULL
- };
-
- software_node_unregister_node_group(node_group);
-}
-static int bd718xx_i2c_register_pwrbutton(struct device *dev,
- struct irq_domain *irq_domain)
-{
- struct mfd_cell gpio_keys_cell = {
- .name = "gpio-keys",
- .resources = bd718xx_powerkey_resources,
- .num_resources = ARRAY_SIZE(bd718xx_powerkey_resources),
- };
- struct software_node *nodes;
- int ret;
-
- nodes = devm_kcalloc(dev, 2, sizeof(*nodes), GFP_KERNEL);
- if (!nodes)
- return -ENOMEM;
-
- nodes[GPIO_KEYS].name = devm_kasprintf(dev, GFP_KERNEL, "%s-power-key", dev_name(dev));
- if (!nodes[GPIO_KEYS].name)
- return -ENOMEM;
-
- nodes[GPIO_KEYS].properties = bd718xx_powerkey_parent_props;
-
- nodes[PWRON_KEY].parent = &nodes[GPIO_KEYS];
- nodes[PWRON_KEY].properties = bd718xx_powerkey_props;
-
- ret = bd718xx_i2c_register_swnodes(nodes);
- if (ret)
- return ret;
-
- ret = devm_add_action_or_reset(dev, bd718xx_i2c_unregister_swnodes, nodes);
- if (ret)
- return ret;
-
- gpio_keys_cell.swnode = &nodes[GPIO_KEYS];
-
- ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, &gpio_keys_cell, 1,
- NULL, 0, irq_domain);
- if (ret)
- return dev_err_probe(dev, ret, "Failed to register power-button");
-
- return 0;
-}
static int bd718xx_i2c_probe(struct i2c_client *i2c)
{
@@ -235,7 +158,8 @@ static int bd718xx_i2c_probe(struct i2c_client *i2c)
if (ret)
return dev_err_probe(&i2c->dev, ret, "Failed to create subdevices\n");
- ret = bd718xx_i2c_register_pwrbutton(&i2c->dev, irq_domain);
+ ret = rohm_mfd_register_pwrbutton(&i2c->dev, BD718XX_INT_PWRBTN_S,
+ "bd718xx-pwrkey", false, irq_domain);
if (ret)
return ret;
diff --git a/drivers/mfd/rohm-pwrbutton.c b/drivers/mfd/rohm-pwrbutton.c
new file mode 100644
index 000000000000..e0cb74fdd73c
--- /dev/null
+++ b/drivers/mfd/rohm-pwrbutton.c
@@ -0,0 +1,112 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Shared helper for ROHM PMIC power button registration
+ *
+ * Copyright 2018, 2019 ROHM Semiconductors
+ * Copyright 2026 Google LLC
+ */
+
+#include <linux/device/devres.h>
+#include <linux/gfp_types.h>
+#include <linux/input.h>
+#include <linux/mfd/core.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/slab.h>
+
+#include "rohm-pwrbutton.h"
+
+#define GPIO_KEYS 0 /* Node corresponding to gpio-keys device itself */
+#define PWRON_KEY 1 /* Node describing power button in gpio-keys */
+
+static int rohm_mfd_pwrbutton_register_swnodes(const struct software_node *nodes)
+{
+ const struct software_node * const node_group[] = {
+ &nodes[GPIO_KEYS], &nodes[PWRON_KEY], NULL
+ };
+
+ return software_node_register_node_group(node_group);
+}
+
+static void rohm_mfd_pwrbutton_unregister_swnodes(void *data)
+{
+ const struct software_node *nodes = data;
+ const struct software_node * const node_group[] = {
+ &nodes[GPIO_KEYS], &nodes[PWRON_KEY], NULL
+ };
+
+ software_node_unregister_node_group(node_group);
+}
+
+int rohm_mfd_register_pwrbutton(struct device *dev, int irq, const char *name,
+ bool wakeup, struct irq_domain *irq_domain)
+{
+ const struct resource res[] = {
+ DEFINE_RES_IRQ_NAMED(irq, name),
+ };
+ struct mfd_cell gpio_keys_cell = {
+ .name = "gpio-keys",
+ .resources = res,
+ .num_resources = ARRAY_SIZE(res),
+ };
+ struct property_entry *parent_props;
+ struct property_entry *child_props;
+ struct software_node *nodes;
+ int n_props;
+ int ret;
+
+ if (irq <= 0)
+ return -EINVAL;
+
+ nodes = devm_kcalloc(dev, 2, sizeof(*nodes), GFP_KERNEL);
+ if (!nodes)
+ return -ENOMEM;
+
+ nodes[GPIO_KEYS].name = devm_kasprintf(dev, GFP_KERNEL, "%s-power-key", dev_name(dev));
+ if (!nodes[GPIO_KEYS].name)
+ return -ENOMEM;
+
+ parent_props = devm_kcalloc(dev, 2, sizeof(*parent_props), GFP_KERNEL);
+ if (!parent_props)
+ return -ENOMEM;
+
+ parent_props[0] = PROPERTY_ENTRY_STRING("label", name);
+ nodes[GPIO_KEYS].properties = parent_props;
+
+ n_props = 2; /* linux,code and terminator */
+ if (wakeup)
+ n_props++;
+
+ child_props = devm_kcalloc(dev, n_props, sizeof(*child_props), GFP_KERNEL);
+ if (!child_props)
+ return -ENOMEM;
+
+ child_props[0] = PROPERTY_ENTRY_U32("linux,code", KEY_POWER);
+ if (wakeup)
+ child_props[1] = PROPERTY_ENTRY_BOOL("wakeup-source");
+
+ nodes[PWRON_KEY].parent = &nodes[GPIO_KEYS];
+ nodes[PWRON_KEY].properties = child_props;
+
+ ret = rohm_mfd_pwrbutton_register_swnodes(nodes);
+ if (ret)
+ return ret;
+
+ ret = devm_add_action_or_reset(dev, rohm_mfd_pwrbutton_unregister_swnodes, nodes);
+ if (ret)
+ return ret;
+
+ gpio_keys_cell.swnode = &nodes[GPIO_KEYS];
+
+ ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, &gpio_keys_cell, 1,
+ NULL, 0, irq_domain);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to register power-button");
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(rohm_mfd_register_pwrbutton);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Dmitry Torokhov <dmitry.torokhov@gmail.com>");
+MODULE_DESCRIPTION("Shared helper for ROHM PMIC power button registration");
diff --git a/drivers/mfd/rohm-pwrbutton.h b/drivers/mfd/rohm-pwrbutton.h
new file mode 100644
index 000000000000..985bd37a4ad0
--- /dev/null
+++ b/drivers/mfd/rohm-pwrbutton.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#ifndef __LINUX_MFD_ROHM_PWRBUTTON_H__
+#define __LINUX_MFD_ROHM_PWRBUTTON_H__
+
+struct device;
+struct irq_domain;
+
+int rohm_mfd_register_pwrbutton(struct device *dev, int irq, const char *name,
+ bool wakeup, struct irq_domain *irq_domain);
+
+#endif /* __LINUX_MFD_ROHM_PWRBUTTON_H__ */
--
2.55.0.rc0.738.g0c8ab3ebcc-goog
--
Dmitry
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] mfd: rohm: Factor out power button registration
2026-06-18 18:58 [PATCH] mfd: rohm: Factor out power button registration Dmitry Torokhov
@ 2026-06-19 9:02 ` Lee Jones
2026-06-25 8:00 ` Matti Vaittinen
1 sibling, 0 replies; 6+ messages in thread
From: Lee Jones @ 2026-06-19 9:02 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Matti Vaittinen, linux-kernel
On Thu, 18 Jun 2026, Dmitry Torokhov wrote:
> Factor out the power button registration logic using software nodes
> from rohm-bd718x7 and rohm-bd71828 drivers into a shared module
> rohm-pwrbutton.
>
> This reduces duplication and makes it easier to support other ROHM
> PMICs with similar power button configurations.
>
> Suggested-by: Lee Jones <lee@kernel.org>
> Assisted-by: Antigravity:gemini-3.5-flash
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Quick drive-by nit: Drop the term 'mfd' from function names please.
> ---
> MAINTAINERS | 2 +
> drivers/mfd/Kconfig | 6 ++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/rohm-bd71828.c | 84 ++------------------------
> drivers/mfd/rohm-bd718x7.c | 84 ++------------------------
> drivers/mfd/rohm-pwrbutton.c | 112 +++++++++++++++++++++++++++++++++++
> drivers/mfd/rohm-pwrbutton.h | 12 ++++
> 7 files changed, 141 insertions(+), 160 deletions(-)
--
Lee Jones
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mfd: rohm: Factor out power button registration
2026-06-18 18:58 [PATCH] mfd: rohm: Factor out power button registration Dmitry Torokhov
2026-06-19 9:02 ` Lee Jones
@ 2026-06-25 8:00 ` Matti Vaittinen
2026-06-25 18:33 ` Dmitry Torokhov
1 sibling, 1 reply; 6+ messages in thread
From: Matti Vaittinen @ 2026-06-25 8:00 UTC (permalink / raw)
To: Dmitry Torokhov, Lee Jones; +Cc: linux-kernel
On 18/06/2026 21:58, Dmitry Torokhov wrote:
> Factor out the power button registration logic using software nodes
> from rohm-bd718x7 and rohm-bd71828 drivers into a shared module
> rohm-pwrbutton.
>
> This reduces duplication and makes it easier to support other ROHM
> PMICs with similar power button configurations.
>
> Suggested-by: Lee Jones <lee@kernel.org>
> Assisted-by: Antigravity:gemini-3.5-flash
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Thanks a lot Dmitry. I like the idea of having generic helpers for
crearting the swnodes for the power-button!
> ---
> MAINTAINERS | 2 +
> drivers/mfd/Kconfig | 6 ++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/rohm-bd71828.c | 84 ++------------------------
> drivers/mfd/rohm-bd718x7.c | 84 ++------------------------
> drivers/mfd/rohm-pwrbutton.c | 112 +++++++++++++++++++++++++++++++++++
> drivers/mfd/rohm-pwrbutton.h | 12 ++++
> 7 files changed, 141 insertions(+), 160 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f1caa6e5198b..40c46a7363fb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -23524,6 +23524,8 @@ F: drivers/mfd/rohm-bd71828.c
> F: drivers/mfd/rohm-bd718x7.c
> F: drivers/mfd/rohm-bd9576.c
> F: drivers/mfd/rohm-bd96801.c
> +F: drivers/mfd/rohm-pwrbutton.c
> +F: drivers/mfd/rohm-pwrbutton.h
> F: drivers/regulator/bd71815-regulator.c
> F: drivers/regulator/bd71828-regulator.c
> F: drivers/regulator/bd718x7-regulator.c
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 763ce6a34782..8d04e1b1f8c8 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -2208,6 +2208,10 @@ config MFD_STW481X
> in various ST Microelectronics and ST-Ericsson embedded
> Nomadik series.
>
> +config MFD_ROHM_PWRBUTTON
> + tristate
> + select MFD_CORE
I would drop the MFD dependency from the helper. It'd be nice to be able
to add the SW-nodes representing power-button(s) outside MFD as well.
Furthermore, I would drop the _ROHM_ from it as it should be useful for
other vendors as well.
I might then add another, "MFD-helper", which includes the MFD-specific
bits. If there are no other users but ROHM drivers in MFD, then that
could carry the ROHM in the name.
Finally, I am unsure if we should select the GPIO_KEYS? How do you see it?
> +
> config MFD_ROHM_BD718XX
> tristate "ROHM BD71837 Power Management IC"
> depends on I2C=y
> @@ -2215,6 +2219,7 @@ config MFD_ROHM_BD718XX
> select REGMAP_I2C
> select REGMAP_IRQ
> select MFD_CORE
> + select MFD_ROHM_PWRBUTTON
> help
> Select this option to get support for the ROHM BD71837
> Power Management ICs. BD71837 is designed to power processors like
> @@ -2228,6 +2233,7 @@ config MFD_ROHM_BD71828
> select REGMAP_I2C
> select REGMAP_IRQ
> select MFD_CORE
> + select MFD_ROHM_PWRBUTTON
> help
> Select this option to get support for the ROHM BD71815, BD71828,
> BD71879, BD72720 and BD73900 Power Management ICs (PMICs). These are
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index dd4bb7e77c33..72d3944b0ad8 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -273,6 +273,7 @@ obj-$(CONFIG_MFD_STM32_TIMERS) += stm32-timers.o
> obj-$(CONFIG_MFD_MXS_LRADC) += mxs-lradc.o
> obj-$(CONFIG_MFD_SC27XX_PMIC) += sprd-sc27xx-spi.o
> obj-$(CONFIG_RAVE_SP_CORE) += rave-sp.o
> +obj-$(CONFIG_MFD_ROHM_PWRBUTTON) += rohm-pwrbutton.o
> obj-$(CONFIG_MFD_ROHM_BD71828) += rohm-bd71828.o
> obj-$(CONFIG_MFD_ROHM_BD718XX) += rohm-bd718x7.o
> obj-$(CONFIG_MFD_ROHM_BD957XMUF) += rohm-bd9576.o
> diff --git a/drivers/mfd/rohm-bd71828.c b/drivers/mfd/rohm-bd71828.c
> index 5fb6142cf087..dfc23cd13ef2 100644
> --- a/drivers/mfd/rohm-bd71828.c
> +++ b/drivers/mfd/rohm-bd71828.c
> @@ -5,8 +5,6 @@
> * ROHM BD718[15/28/79] and BD72720 PMIC driver
> */
>
> -#include <linux/device/devres.h>
> -#include <linux/gfp_types.h>
> #include <linux/i2c.h>
> #include <linux/input.h>
> #include <linux/interrupt.h>
> @@ -19,10 +17,11 @@
> #include <linux/mfd/rohm-generic.h>
> #include <linux/module.h>
> #include <linux/of.h>
> -#include <linux/property.h>
> #include <linux/regmap.h>
> #include <linux/types.h>
>
> +#include "rohm-pwrbutton.h"
> +
> #define BD72720_TYPED_IRQ_REG(_irq, _stat_offset, _mask, _type_offset) \
> [_irq] = { \
> .reg_offset = (_stat_offset), \
> @@ -859,83 +858,7 @@ static int set_clk_mode(struct device *dev, struct regmap *regmap,
> OUT32K_MODE_CMOS);
> }
>
> -static const struct property_entry bd71828_powerkey_parent_props[] = {
> - PROPERTY_ENTRY_STRING("label", "bd71828-pwrkey"),
> - { }
> -};
> -
> -static const struct property_entry bd71828_powerkey_props[] = {
> - PROPERTY_ENTRY_U32("linux,code", KEY_POWER),
> - PROPERTY_ENTRY_BOOL("wakeup-source"),
> - { }
> -};
> -
> -#define GPIO_KEYS 0 /* Node corresponding to gpio-keys device itself */
> -#define PWRON_KEY 1 /* Node describing power button in gpio-keys */
> -
> -static int bd71828_i2c_register_swnodes(const struct software_node *nodes)
> -{
> - const struct software_node * const node_group[] = {
> - &nodes[GPIO_KEYS], &nodes[PWRON_KEY], NULL
> - };
> -
> - return software_node_register_node_group(node_group);
> -}
> -
> -static void bd71828_i2c_unregister_swnodes(void *data)
> -{
> - const struct software_node *nodes = data;
> - const struct software_node * const node_group[] = {
> - &nodes[GPIO_KEYS], &nodes[PWRON_KEY], NULL
> - };
> -
> - software_node_unregister_node_group(node_group);
> -}
> -
> -static int bd71828_i2c_register_pwrbutton(struct device *dev, int button_irq,
> - struct irq_domain *irq_domain)
> -{
> - const struct resource res[] = {
> - DEFINE_RES_IRQ_NAMED(button_irq, "bd71828-pwrkey"),
> - };
> - struct mfd_cell gpio_keys_cell = {
> - .name = "gpio-keys",
> - .resources = res,
> - .num_resources = ARRAY_SIZE(res),
> - };
> - struct software_node *nodes;
> - int ret;
>
> - nodes = devm_kcalloc(dev, 2, sizeof(*nodes), GFP_KERNEL);
> - if (!nodes)
> - return -ENOMEM;
> -
> - nodes[GPIO_KEYS].name = devm_kasprintf(dev, GFP_KERNEL, "%s-power-key", dev_name(dev));
> - if (!nodes[GPIO_KEYS].name)
> - return -ENOMEM;
> -
> - nodes[GPIO_KEYS].properties = bd71828_powerkey_parent_props;
> -
> - nodes[PWRON_KEY].parent = &nodes[GPIO_KEYS];
> - nodes[PWRON_KEY].properties = bd71828_powerkey_props;
> -
> - ret = bd71828_i2c_register_swnodes(nodes);
> - if (ret)
> - return ret;
> -
> - ret = devm_add_action_or_reset(dev, bd71828_i2c_unregister_swnodes, nodes);
> - if (ret)
> - return ret;
> -
> - gpio_keys_cell.swnode = &nodes[GPIO_KEYS];
> -
> - ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, &gpio_keys_cell, 1,
> - NULL, 0, irq_domain);
> - if (ret)
> - return dev_err_probe(dev, ret, "Failed to register power-button");
> -
> - return 0;
> -}
>
> static struct i2c_client *bd71828_dev;
> static void bd71828_power_off(void)
> @@ -1096,7 +1019,8 @@ static int bd71828_i2c_probe(struct i2c_client *i2c)
> return dev_err_probe(&i2c->dev, ret, "Failed to create subdevices\n");
>
> if (button_irq) {
> - ret = bd71828_i2c_register_pwrbutton(&i2c->dev, button_irq, irq_domain);
> + ret = rohm_mfd_register_pwrbutton(&i2c->dev, button_irq,
> + "bd71828-pwrkey", true, irq_domain);
> if (ret)
> return ret;
> }
> diff --git a/drivers/mfd/rohm-bd718x7.c b/drivers/mfd/rohm-bd718x7.c
> index be2acc429fe3..bab3b1c2445b 100644
> --- a/drivers/mfd/rohm-bd718x7.c
> +++ b/drivers/mfd/rohm-bd718x7.c
> @@ -7,8 +7,6 @@
> // Datasheet for BD71837MWV available from
> // https://www.rohm.com/datasheet/BD71837MWV/bd71837mwv-e
>
> -#include <linux/device/devres.h>
> -#include <linux/gfp_types.h>
> #include <linux/i2c.h>
> #include <linux/input.h>
> #include <linux/interrupt.h>
> @@ -16,10 +14,11 @@
> #include <linux/mfd/core.h>
> #include <linux/module.h>
> #include <linux/of.h>
> -#include <linux/property.h>
> #include <linux/regmap.h>
> #include <linux/types.h>
>
> +#include "rohm-pwrbutton.h"
> +
> static struct mfd_cell bd71837_mfd_cells[] = {
> { .name = "bd71837-clk", },
> { .name = "bd71837-pmic", },
> @@ -105,83 +104,7 @@ static int bd718xx_init_press_duration(struct regmap *regmap,
> return 0;
> }
>
> -static const struct property_entry bd718xx_powerkey_parent_props[] = {
> - PROPERTY_ENTRY_STRING("label", "bd718xx-pwrkey"),
> - { }
> -};
> -
> -static const struct property_entry bd718xx_powerkey_props[] = {
> - PROPERTY_ENTRY_U32("linux,code", KEY_POWER),
> - { }
> -};
> -
> -static const struct resource bd718xx_powerkey_resources[] = {
> - DEFINE_RES_IRQ_NAMED(BD718XX_INT_PWRBTN_S, "bd718xx-pwrkey"),
> -};
> -
> -#define GPIO_KEYS 0 /* Node corresponding to gpio-keys device itself */
> -#define PWRON_KEY 1 /* Node describing power button in gpio-keys */
> -
> -static int bd718xx_i2c_register_swnodes(const struct software_node *nodes)
> -{
> - const struct software_node * const node_group[] = {
> - &nodes[GPIO_KEYS], &nodes[PWRON_KEY], NULL
> - };
> -
> - return software_node_register_node_group(node_group);
> -}
> -
> -static void bd718xx_i2c_unregister_swnodes(void *data)
> -{
> - const struct software_node *nodes = data;
> - const struct software_node * const node_group[] = {
> - &nodes[GPIO_KEYS], &nodes[PWRON_KEY], NULL
> - };
> -
> - software_node_unregister_node_group(node_group);
> -}
>
> -static int bd718xx_i2c_register_pwrbutton(struct device *dev,
> - struct irq_domain *irq_domain)
> -{
> - struct mfd_cell gpio_keys_cell = {
> - .name = "gpio-keys",
> - .resources = bd718xx_powerkey_resources,
> - .num_resources = ARRAY_SIZE(bd718xx_powerkey_resources),
> - };
> - struct software_node *nodes;
> - int ret;
> -
> - nodes = devm_kcalloc(dev, 2, sizeof(*nodes), GFP_KERNEL);
> - if (!nodes)
> - return -ENOMEM;
> -
> - nodes[GPIO_KEYS].name = devm_kasprintf(dev, GFP_KERNEL, "%s-power-key", dev_name(dev));
> - if (!nodes[GPIO_KEYS].name)
> - return -ENOMEM;
> -
> - nodes[GPIO_KEYS].properties = bd718xx_powerkey_parent_props;
> -
> - nodes[PWRON_KEY].parent = &nodes[GPIO_KEYS];
> - nodes[PWRON_KEY].properties = bd718xx_powerkey_props;
> -
> - ret = bd718xx_i2c_register_swnodes(nodes);
> - if (ret)
> - return ret;
> -
> - ret = devm_add_action_or_reset(dev, bd718xx_i2c_unregister_swnodes, nodes);
> - if (ret)
> - return ret;
> -
> - gpio_keys_cell.swnode = &nodes[GPIO_KEYS];
> -
> - ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, &gpio_keys_cell, 1,
> - NULL, 0, irq_domain);
> - if (ret)
> - return dev_err_probe(dev, ret, "Failed to register power-button");
> -
> - return 0;
> -}
>
> static int bd718xx_i2c_probe(struct i2c_client *i2c)
> {
> @@ -235,7 +158,8 @@ static int bd718xx_i2c_probe(struct i2c_client *i2c)
> if (ret)
> return dev_err_probe(&i2c->dev, ret, "Failed to create subdevices\n");
>
> - ret = bd718xx_i2c_register_pwrbutton(&i2c->dev, irq_domain);
> + ret = rohm_mfd_register_pwrbutton(&i2c->dev, BD718XX_INT_PWRBTN_S,
> + "bd718xx-pwrkey", false, irq_domain);
> if (ret)
> return ret;
>
> diff --git a/drivers/mfd/rohm-pwrbutton.c b/drivers/mfd/rohm-pwrbutton.c
> new file mode 100644
> index 000000000000..e0cb74fdd73c
> --- /dev/null
> +++ b/drivers/mfd/rohm-pwrbutton.c
> @@ -0,0 +1,112 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Shared helper for ROHM PMIC power button registration
> + *
> + * Copyright 2018, 2019 ROHM Semiconductors
> + * Copyright 2026 Google LLC
> + */
> +
> +#include <linux/device/devres.h>
> +#include <linux/gfp_types.h>
> +#include <linux/input.h>
> +#include <linux/mfd/core.h>
> +#include <linux/module.h>
> +#include <linux/property.h>
> +#include <linux/slab.h>
> +
> +#include "rohm-pwrbutton.h"
> +
> +#define GPIO_KEYS 0 /* Node corresponding to gpio-keys device itself */
> +#define PWRON_KEY 1 /* Node describing power button in gpio-keys */
> +
> +static int rohm_mfd_pwrbutton_register_swnodes(const struct software_node *nodes)
> +{
> + const struct software_node * const node_group[] = {
> + &nodes[GPIO_KEYS], &nodes[PWRON_KEY], NULL
> + };
> +
> + return software_node_register_node_group(node_group);
> +}
> +
> +static void rohm_mfd_pwrbutton_unregister_swnodes(void *data)
> +{
> + const struct software_node *nodes = data;
> + const struct software_node * const node_group[] = {
> + &nodes[GPIO_KEYS], &nodes[PWRON_KEY], NULL
> + };
> +
> + software_node_unregister_node_group(node_group);
> +}
> +
> +int rohm_mfd_register_pwrbutton(struct device *dev, int irq, const char *name,
> + bool wakeup, struct irq_domain *irq_domain)
> +{
> + const struct resource res[] = {
> + DEFINE_RES_IRQ_NAMED(irq, name),
> + };
> + struct mfd_cell gpio_keys_cell = {
> + .name = "gpio-keys",
> + .resources = res,
> + .num_resources = ARRAY_SIZE(res),
> + };
> + struct property_entry *parent_props;
> + struct property_entry *child_props;
> + struct software_node *nodes;
> + int n_props;
> + int ret;
> +
> + if (irq <= 0)
> + return -EINVAL;
> +
> + nodes = devm_kcalloc(dev, 2, sizeof(*nodes), GFP_KERNEL);
> + if (!nodes)
> + return -ENOMEM;
> +
> + nodes[GPIO_KEYS].name = devm_kasprintf(dev, GFP_KERNEL, "%s-power-key", dev_name(dev));
> + if (!nodes[GPIO_KEYS].name)
> + return -ENOMEM;
> +
> + parent_props = devm_kcalloc(dev, 2, sizeof(*parent_props), GFP_KERNEL);
> + if (!parent_props)
> + return -ENOMEM;
> +
> + parent_props[0] = PROPERTY_ENTRY_STRING("label", name);
> + nodes[GPIO_KEYS].properties = parent_props;
> +
> + n_props = 2; /* linux,code and terminator */
> + if (wakeup)
> + n_props++;
> +
> + child_props = devm_kcalloc(dev, n_props, sizeof(*child_props), GFP_KERNEL);
> + if (!child_props)
> + return -ENOMEM;
> +
> + child_props[0] = PROPERTY_ENTRY_U32("linux,code", KEY_POWER);
> + if (wakeup)
> + child_props[1] = PROPERTY_ENTRY_BOOL("wakeup-source");
> +
> + nodes[PWRON_KEY].parent = &nodes[GPIO_KEYS];
> + nodes[PWRON_KEY].properties = child_props;
> +
> + ret = rohm_mfd_pwrbutton_register_swnodes(nodes);
> + if (ret)
> + return ret;
> +
> + ret = devm_add_action_or_reset(dev, rohm_mfd_pwrbutton_unregister_swnodes, nodes);
> + if (ret)
> + return ret;
> +
> + gpio_keys_cell.swnode = &nodes[GPIO_KEYS];
> +
> + ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, &gpio_keys_cell, 1,
> + NULL, 0, irq_domain);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to register power-button");
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(rohm_mfd_register_pwrbutton);
As explained, I would split this to something like:
pwrbutton_create_swnodes() // Shouldn't depend on MFD. Allocates and
registers the swnodes for power-button + registers the devm-release.
(rohm)_register_pwrbutton() // Should call pwrbutton_create_swnodes() to
allocate and populate the swnodes, and then fill-in the MFC-cell data +
call the devm_mfd_add_devices().
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Dmitry Torokhov <dmitry.torokhov@gmail.com>");
> +MODULE_DESCRIPTION("Shared helper for ROHM PMIC power button registration");
> diff --git a/drivers/mfd/rohm-pwrbutton.h b/drivers/mfd/rohm-pwrbutton.h
> new file mode 100644
> index 000000000000..985bd37a4ad0
> --- /dev/null
> +++ b/drivers/mfd/rohm-pwrbutton.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +
> +#ifndef __LINUX_MFD_ROHM_PWRBUTTON_H__
> +#define __LINUX_MFD_ROHM_PWRBUTTON_H__
> +
> +struct device;
> +struct irq_domain;
> +
> +int rohm_mfd_register_pwrbutton(struct device *dev, int irq, const char *name,
> + bool wakeup, struct irq_domain *irq_domain);
> +
> +#endif /* __LINUX_MFD_ROHM_PWRBUTTON_H__ */
Yours,
-- Matti
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mfd: rohm: Factor out power button registration
2026-06-25 8:00 ` Matti Vaittinen
@ 2026-06-25 18:33 ` Dmitry Torokhov
2026-06-26 7:35 ` Matti Vaittinen
0 siblings, 1 reply; 6+ messages in thread
From: Dmitry Torokhov @ 2026-06-25 18:33 UTC (permalink / raw)
To: Matti Vaittinen; +Cc: Lee Jones, linux-kernel
Hi Matti,
On Thu, Jun 25, 2026 at 11:00:08AM +0300, Matti Vaittinen wrote:
> On 18/06/2026 21:58, Dmitry Torokhov wrote:
> > Factor out the power button registration logic using software nodes
> > from rohm-bd718x7 and rohm-bd71828 drivers into a shared module
> > rohm-pwrbutton.
> >
> > This reduces duplication and makes it easier to support other ROHM
> > PMICs with similar power button configurations.
> >
> > Suggested-by: Lee Jones <lee@kernel.org>
> > Assisted-by: Antigravity:gemini-3.5-flash
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>
> Thanks a lot Dmitry. I like the idea of having generic helpers for crearting
> the swnodes for the power-button!
>
> > ---
> > MAINTAINERS | 2 +
> > drivers/mfd/Kconfig | 6 ++
> > drivers/mfd/Makefile | 1 +
> > drivers/mfd/rohm-bd71828.c | 84 ++------------------------
> > drivers/mfd/rohm-bd718x7.c | 84 ++------------------------
> > drivers/mfd/rohm-pwrbutton.c | 112 +++++++++++++++++++++++++++++++++++
> > drivers/mfd/rohm-pwrbutton.h | 12 ++++
> > 7 files changed, 141 insertions(+), 160 deletions(-)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index f1caa6e5198b..40c46a7363fb 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -23524,6 +23524,8 @@ F: drivers/mfd/rohm-bd71828.c
> > F: drivers/mfd/rohm-bd718x7.c
> > F: drivers/mfd/rohm-bd9576.c
> > F: drivers/mfd/rohm-bd96801.c
> > +F: drivers/mfd/rohm-pwrbutton.c
> > +F: drivers/mfd/rohm-pwrbutton.h
> > F: drivers/regulator/bd71815-regulator.c
> > F: drivers/regulator/bd71828-regulator.c
> > F: drivers/regulator/bd718x7-regulator.c
> > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > index 763ce6a34782..8d04e1b1f8c8 100644
> > --- a/drivers/mfd/Kconfig
> > +++ b/drivers/mfd/Kconfig
> > @@ -2208,6 +2208,10 @@ config MFD_STW481X
> > in various ST Microelectronics and ST-Ericsson embedded
> > Nomadik series.
> > +config MFD_ROHM_PWRBUTTON
> > + tristate
> > + select MFD_CORE
>
> I would drop the MFD dependency from the helper. It'd be nice to be able to
> add the SW-nodes representing power-button(s) outside MFD as well.
Do you have potential users in mind for this? I mean specifically
single IRQ-backed buttons besides these 2 ROHM controllers?
I would prefer no over-generalize if there are no users. Kernel modules
are not totally free.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mfd: rohm: Factor out power button registration
2026-06-25 18:33 ` Dmitry Torokhov
@ 2026-06-26 7:35 ` Matti Vaittinen
2026-07-06 22:27 ` Dmitry Torokhov
0 siblings, 1 reply; 6+ messages in thread
From: Matti Vaittinen @ 2026-06-26 7:35 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Lee Jones, linux-kernel
On 25/06/2026 21:33, Dmitry Torokhov wrote:
> Hi Matti,
>
> On Thu, Jun 25, 2026 at 11:00:08AM +0300, Matti Vaittinen wrote:
>> On 18/06/2026 21:58, Dmitry Torokhov wrote:
>>> Factor out the power button registration logic using software nodes
>>> from rohm-bd718x7 and rohm-bd71828 drivers into a shared module
>>> rohm-pwrbutton.
>>>
>>> This reduces duplication and makes it easier to support other ROHM
>>> PMICs with similar power button configurations.
>>>
>>> Suggested-by: Lee Jones <lee@kernel.org>
>>> Assisted-by: Antigravity:gemini-3.5-flash
>>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>>
>> Thanks a lot Dmitry. I like the idea of having generic helpers for crearting
>> the swnodes for the power-button!
>>
>>> ---
>>> MAINTAINERS | 2 +
>>> drivers/mfd/Kconfig | 6 ++
>>> drivers/mfd/Makefile | 1 +
>>> drivers/mfd/rohm-bd71828.c | 84 ++------------------------
>>> drivers/mfd/rohm-bd718x7.c | 84 ++------------------------
>>> drivers/mfd/rohm-pwrbutton.c | 112 +++++++++++++++++++++++++++++++++++
>>> drivers/mfd/rohm-pwrbutton.h | 12 ++++
>>> 7 files changed, 141 insertions(+), 160 deletions(-)
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index f1caa6e5198b..40c46a7363fb 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -23524,6 +23524,8 @@ F: drivers/mfd/rohm-bd71828.c
>>> F: drivers/mfd/rohm-bd718x7.c
>>> F: drivers/mfd/rohm-bd9576.c
>>> F: drivers/mfd/rohm-bd96801.c
>>> +F: drivers/mfd/rohm-pwrbutton.c
>>> +F: drivers/mfd/rohm-pwrbutton.h
>>> F: drivers/regulator/bd71815-regulator.c
>>> F: drivers/regulator/bd71828-regulator.c
>>> F: drivers/regulator/bd718x7-regulator.c
>>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>>> index 763ce6a34782..8d04e1b1f8c8 100644
>>> --- a/drivers/mfd/Kconfig
>>> +++ b/drivers/mfd/Kconfig
>>> @@ -2208,6 +2208,10 @@ config MFD_STW481X
>>> in various ST Microelectronics and ST-Ericsson embedded
>>> Nomadik series.
>>> +config MFD_ROHM_PWRBUTTON
>>> + tristate
>>> + select MFD_CORE
>>
>> I would drop the MFD dependency from the helper. It'd be nice to be able to
>> add the SW-nodes representing power-button(s) outside MFD as well.
>
> Do you have potential users in mind for this? I mean specifically
> single IRQ-backed buttons besides these 2 ROHM controllers?
I don't think it needs to be a single IRQ backed? There are quite a few
IRQs in these PMICs. But no, I don't have concrete users for you (as of
now), but a power-button which generates and IRQ does really sound like
a very generic concept to me.
> I would prefer no over-generalize if there are no users. Kernel modules
> are not totally free.
I suppose these IRQ backed power-buttons would in most cases be handled
by the GPIO_KEYS. Perhaps you could consider exporting the helper from
it instead of having it in MFD(?)
Well, I am not going to insist - I feel like you're already going an
extra mile with this - but I really believe more generic helper would be
useful :)
Yours,
-- Matti
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mfd: rohm: Factor out power button registration
2026-06-26 7:35 ` Matti Vaittinen
@ 2026-07-06 22:27 ` Dmitry Torokhov
0 siblings, 0 replies; 6+ messages in thread
From: Dmitry Torokhov @ 2026-07-06 22:27 UTC (permalink / raw)
To: Matti Vaittinen; +Cc: Lee Jones, linux-kernel
Hi Matti,
On Fri, Jun 26, 2026 at 10:35:19AM +0300, Matti Vaittinen wrote:
> On 25/06/2026 21:33, Dmitry Torokhov wrote:
> > Hi Matti,
> >
> > On Thu, Jun 25, 2026 at 11:00:08AM +0300, Matti Vaittinen wrote:
> > > On 18/06/2026 21:58, Dmitry Torokhov wrote:
> > > > Factor out the power button registration logic using software nodes
> > > > from rohm-bd718x7 and rohm-bd71828 drivers into a shared module
> > > > rohm-pwrbutton.
> > > >
> > > > This reduces duplication and makes it easier to support other ROHM
> > > > PMICs with similar power button configurations.
> > > >
> > > > Suggested-by: Lee Jones <lee@kernel.org>
> > > > Assisted-by: Antigravity:gemini-3.5-flash
> > > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > >
> > > Thanks a lot Dmitry. I like the idea of having generic helpers for crearting
> > > the swnodes for the power-button!
> > >
> > > > ---
> > > > MAINTAINERS | 2 +
> > > > drivers/mfd/Kconfig | 6 ++
> > > > drivers/mfd/Makefile | 1 +
> > > > drivers/mfd/rohm-bd71828.c | 84 ++------------------------
> > > > drivers/mfd/rohm-bd718x7.c | 84 ++------------------------
> > > > drivers/mfd/rohm-pwrbutton.c | 112 +++++++++++++++++++++++++++++++++++
> > > > drivers/mfd/rohm-pwrbutton.h | 12 ++++
> > > > 7 files changed, 141 insertions(+), 160 deletions(-)
> > > >
> > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > index f1caa6e5198b..40c46a7363fb 100644
> > > > --- a/MAINTAINERS
> > > > +++ b/MAINTAINERS
> > > > @@ -23524,6 +23524,8 @@ F: drivers/mfd/rohm-bd71828.c
> > > > F: drivers/mfd/rohm-bd718x7.c
> > > > F: drivers/mfd/rohm-bd9576.c
> > > > F: drivers/mfd/rohm-bd96801.c
> > > > +F: drivers/mfd/rohm-pwrbutton.c
> > > > +F: drivers/mfd/rohm-pwrbutton.h
> > > > F: drivers/regulator/bd71815-regulator.c
> > > > F: drivers/regulator/bd71828-regulator.c
> > > > F: drivers/regulator/bd718x7-regulator.c
> > > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > > > index 763ce6a34782..8d04e1b1f8c8 100644
> > > > --- a/drivers/mfd/Kconfig
> > > > +++ b/drivers/mfd/Kconfig
> > > > @@ -2208,6 +2208,10 @@ config MFD_STW481X
> > > > in various ST Microelectronics and ST-Ericsson embedded
> > > > Nomadik series.
> > > > +config MFD_ROHM_PWRBUTTON
> > > > + tristate
> > > > + select MFD_CORE
> > >
> > > I would drop the MFD dependency from the helper. It'd be nice to be able to
> > > add the SW-nodes representing power-button(s) outside MFD as well.
> >
> > Do you have potential users in mind for this? I mean specifically
> > single IRQ-backed buttons besides these 2 ROHM controllers?
>
> I don't think it needs to be a single IRQ backed? There are quite a few IRQs
> in these PMICs. But no, I don't have concrete users for you (as of now), but
> a power-button which generates and IRQ does really sound like a very generic
> concept to me.
>
> > I would prefer no over-generalize if there are no users. Kernel modules
> > are not totally free.
>
> I suppose these IRQ backed power-buttons would in most cases be handled by
> the GPIO_KEYS. Perhaps you could consider exporting the helper from it
> instead of having it in MFD(?)
I want through drivers for similar buttons and I currently do not see
any other candidates for the conversion besides the 2 ROHM drivers. This
is because the rest of the power button drivers implement true tracking
of the button state (instead of only detecting press via an IRQ and
simulating release).
The only potential users of the helper would be new drivers, but new
drivers outside of MFD realm will use ACPI/DT and not static device
properties as they will handle new devices on new boards. It is only
within the context of MFD that I can see slight possibility of more
drivers using this mechanism (and not regular DT configuration) and that
is why I do not think putting the helper in gpio-keys would make sense.
Additionally putting it in gpio-keys will add unwanted (IMO) dependency
(module-load-wise).
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-07-06 22:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-18 18:58 [PATCH] mfd: rohm: Factor out power button registration Dmitry Torokhov
2026-06-19 9:02 ` Lee Jones
2026-06-25 8:00 ` Matti Vaittinen
2026-06-25 18:33 ` Dmitry Torokhov
2026-06-26 7:35 ` Matti Vaittinen
2026-07-06 22:27 ` Dmitry Torokhov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox