Re: [PATCH] mfd: rohm: Factor out power button registration

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

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