| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <b7e97aa3-8f2d-4a59-8a38-577717404865@gmail.com>
Date: Mon, 11 Aug 2025 13:34:43 +0300
From: Matti Vaittinen <mazziesaccount@...il.com>
To: Arnd Bergmann <arnd@...nel.org>, Bartosz Golaszewski <brgl@...ev.pl>,
Linus Walleij <linus.walleij@...aro.org>, linux-gpio@...r.kernel.org,
Dmitry Torokhov <dmitry.torokhov@...il.com>, Lee Jones <lee@...nel.org>
Cc: Arnd Bergmann <arnd@...db.de>,
Gatien Chevallier <gatien.chevallier@...s.st.com>,
Fabrice Gasnier <fabrice.gasnier@...s.st.com>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Bartosz Golaszewski <bartosz.golaszewski@...aro.org>,
Thomas Gleixner <tglx@...utronix.de>,
Charles Keepax <ckeepax@...nsource.cirrus.com>,
Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
Christophe JAILLET <christophe.jaillet@...adoo.fr>,
linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 09/21] input: gpio-keys: make legacy gpiolib optional
Hi dee Ho peeps,
On 08/08/2025 18:17, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@...db.de>
>
> Most users of gpio-keys and gpio-keys-polled use modern gpiolib
> interfaces, but there are still number of ancient sh, arm32 and x86
> machines that have never been converted.
>
> Add an #ifdef block for the parts of the driver that are only
> used on those legacy machines.
>
> Signed-off-by: Arnd Bergmann <arnd@...db.de>
> ---
> drivers/input/keyboard/gpio_keys.c | 5 +++--
> drivers/input/keyboard/gpio_keys_polled.c | 2 ++
> drivers/mfd/rohm-bd71828.c | 2 ++
> drivers/mfd/rohm-bd718x7.c | 2 ++
> include/linux/gpio_keys.h | 2 ++
> 5 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
> index f9db86da0818..984b20f773ed 100644
> --- a/drivers/input/keyboard/gpio_keys.c
> +++ b/drivers/input/keyboard/gpio_keys.c
> @@ -528,6 +528,7 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
> */
> bdata->gpiod = NULL;
> }
> +#ifdef CONFIG_GPIOLIB_LEGACY
> } else if (gpio_is_valid(button->gpio)) {
> /*
> * Legacy GPIO number, so request the GPIO here and
> @@ -546,6 +547,7 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
>
> if (button->active_low ^ gpiod_is_active_low(bdata->gpiod))
> gpiod_toggle_active_low(bdata->gpiod);
> +#endif
> }
>
> if (bdata->gpiod) {
> @@ -583,8 +585,7 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
> if (irq < 0) {
> error = irq;
> dev_err_probe(dev, error,
> - "Unable to get irq number for GPIO %d\n",
> - button->gpio);
> + "Unable to get irq number for GPIO\n");
> return error;
> }
> bdata->irq = irq;
> diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
> index e6707d72210e..0ae0e53910ea 100644
> --- a/drivers/input/keyboard/gpio_keys_polled.c
> +++ b/drivers/input/keyboard/gpio_keys_polled.c
> @@ -301,6 +301,7 @@ static int gpio_keys_polled_probe(struct platform_device *pdev)
> return dev_err_probe(dev, PTR_ERR(bdata->gpiod),
> "failed to get gpio\n");
> }
> +#ifdef CONFIG_GPIOLIB_LEGACY
> } else if (gpio_is_valid(button->gpio)) {
> /*
> * Legacy GPIO number so request the GPIO here and
> @@ -323,6 +324,7 @@ static int gpio_keys_polled_probe(struct platform_device *pdev)
>
> if (button->active_low ^ gpiod_is_active_low(bdata->gpiod))
> gpiod_toggle_active_low(bdata->gpiod);
> +#endif
> }
>
> bdata->last_state = -1;
> diff --git a/drivers/mfd/rohm-bd71828.c b/drivers/mfd/rohm-bd71828.c
> index a14b7aa69c3c..fb68694fadca 100644
> --- a/drivers/mfd/rohm-bd71828.c
> +++ b/drivers/mfd/rohm-bd71828.c
> @@ -21,7 +21,9 @@
>
> static struct gpio_keys_button button = {
> .code = KEY_POWER,
> +#ifdef CONFIG_GPIOLIB_LEGACY
> .gpio = -1,
> +#endif
> .type = EV_KEY,
> };
>
> diff --git a/drivers/mfd/rohm-bd718x7.c b/drivers/mfd/rohm-bd718x7.c
> index 25e494a93d48..6c99ab62e31b 100644
> --- a/drivers/mfd/rohm-bd718x7.c
> +++ b/drivers/mfd/rohm-bd718x7.c
> @@ -20,7 +20,9 @@
>
> static struct gpio_keys_button button = {
> .code = KEY_POWER,
> +#ifdef CONFIG_GPIOLIB_LEGACY
> .gpio = -1,
> +#endif
> .type = EV_KEY,
> };
>
> diff --git a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h
> index 80fa930b04c6..e8d6dc290efb 100644
> --- a/include/linux/gpio_keys.h
> +++ b/include/linux/gpio_keys.h
> @@ -25,7 +25,9 @@ struct device;
> */
> struct gpio_keys_button {
> unsigned int code;
> +#ifdef CONFIG_GPIOLIB_LEGACY
> int gpio;
> +#endif
> int active_low;
> const char *desc;
> unsigned int type;
AFAIR, these ROHM PMICs (bd718[15, 27, 28, 37, 47, 50, 78, 85]) all
provide a 'button IRQ', from a power button. (Or, couple of IRQs but
let's skip the details) The gpio-keys is used to send the KEY_POWER
event when IRQ is detected.
The IRQ comes from the PMIC, and the regmap_irq chip provided by the MFD
provides it. This IRQ information is delivered to the gpio-keys from the
MFD driver via platform data. That's basically what these "button"
structs are here for. No GPIO line information (only the IRQ number) is
needed to be delivered to the gpio-keys. This problematic assignment:
> +#ifdef CONFIG_GPIOLIB_LEGACY
> .gpio = -1,
> +#endif
is only needed to invalidate the gpio information so that the gpio-keys
wont use it, only the IRQ.
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?
Yours,
-- Matti
Powered by blists - more mailing lists