Re: [PATCH 09/21] input: gpio-keys: make legacy gpiolib optional

[Matti Vaittinen <mazziesaccount@gmail.com>]

On 11/08/2025 23:09, Andy Shevchenko wrote:
> On Mon, Aug 11, 2025 at 12:21:51PM -0700, Dmitry Torokhov wrote:
>> On Mon, Aug 11, 2025 at 03:52:19PM +0300, Andy Shevchenko wrote:
>>> On Mon, Aug 11, 2025 at 01:34:43PM +0300, Matti Vaittinen wrote:
>>>> On 08/08/2025 18:17, Arnd Bergmann wrote:
> 
> ...
> 
>>>> As such, this patch seems Ok to me, you can treat this as an ack :) This,
>>>> however made me ponder following - is this the tight way to handle the
>>>> power-button IRQ? I don't see any other MFD devices doing this in same way,
>>>> although I am pretty sure there are other PMICs with similar power-button
>>>> IRQ...
>>>>
>>>> I see for example the "drivers/mfd/rt5120.c" to invoke
>>>> "drivers/input/misc/rt5120-pwrkey.c" instead of using the gpio-keys. This,
>>>> however, feels like code duplication to me. I'd rather kept using the
>>>> gpio-keys, but seeing:
>>>>
>>>> git grep KEY_POWER drivers/mfd/
>>>> drivers/mfd/rohm-bd71828.c:     .code = KEY_POWER,
>>>> drivers/mfd/rohm-bd718x7.c:     .code = KEY_POWER,
>>>>
>>>> makes me wonder if there is more widely used (better) way?
>>>
>>> FWIW, on Intel platforms that use power button by PMIC we add a special driver
>>> for each of such cases.
>>
>> If we can make gpio-keys work for various power buttons that would be
>> great IMO. The MFD drivers in question already are using device tree,
>> but they do not define/expect nodes for the power buttons. If the nodes
>> were there then I think gpio-keys would work out of the box?
> 
> Looking at the, e.g., https://elixir.bootlin.com/linux/v6.16/source/drivers/platform/x86/intel/mrfld_pwrbtn.c,
> I am not sure it's as simply as it sounds. Basically it's an IRQ, which
> requires IRQ handling and proper acking/masking/etc.

In some (many?) cases the interrupts (acking/masking) are handled by an 
irqchip code. When this is the case, the gpio-keys (or any other 
power-button code) does not need to care about IRQ-specifics. (I don't 
know about the Intel driver though.)

Problem with many of the bd718* (and probably some other MFD drivers) 
is, that the interrupts are really relevant only for the drivers 
specific to this one device (like PMIC in ROHM case). When this is the 
case, the device is not really (from the HW perspective) an 
interrupt-controller, which means it shouldn't probably be marked as one 
in the device-tree either. It will then also mean that there can't be 
meaningful interrupt specification for the button IRQ in the 
device-tree, right?

Additionally, we have devices where most of the interrupts are internal 
to the PMIC, but then the PMIC also has some pins usable as GPIO, which 
can be used as interrupt sources. Eg, someone can connect another device 
to these pins - which makes the PMIC an interrupt-controller. For these 
PMICs the power-button IRQ can be provided via device-tree node (but the 
IRQ spec may become a bit hairy, since most of the IRQs are meant to be 
internal).

Hence, for me, providing the IRQ number in platform data seems still to 
be the right thing to do :)

TLDR; I agree with Dmitry. It's nice to have an easily re-usable 
power-button handler, which requires no IC-specific code. Gpio-keys 
works for simple IRQ based power-buttons where IRQ controller takes care 
of the acks/masks. I just wanted to know if it is for some reason 
discouraged, or if I've used it in a wrong way (because the grep 
resulted so few results).

Yours,
	-- Matti