Input: adp5588-keys - cache GPIO state before registering the gpiochip

[PATCH] Input: adp5588-keys - cache GPIO state before registering the gpiochip

Posted by Alvin Šipraga 1 month, 1 week ago

So as not to clobber any pre-programmed GPIO state in the execution
of its gpiochip ops, the driver caches things during probe time.
However, since those ops can be called both during and immediately after
the call to devm_gpiochip_add_data(), it is imperative that things are
cached before that. That's not the case right now, so reorder the two
steps to prevent any clobbering.

In a conrete example which motivated this change, a bootloader was
preconfiguring an important GPIO output to HIGH before booting the
kernel. Linux would then inadvertently set that output to LOW while
configuring a GPIO hog on a discrete GPIO line within the same 8-bit
bank (because the cached value was 0=LOW).

Fixes: ba9f507a1bea ("Input: adp5588-keys - export unused GPIO pins")
Signed-off-by: Alvin Šipraga <alvin.sipraga@analog.com>
---
 drivers/input/keyboard/adp5588-keys.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/input/keyboard/adp5588-keys.c b/drivers/input/keyboard/adp5588-keys.c
index 40371f5bd9ba..4f0ddff5baba 100644
--- a/drivers/input/keyboard/adp5588-keys.c
+++ b/drivers/input/keyboard/adp5588-keys.c
@@ -446,12 +446,6 @@ static int adp5588_gpio_add(struct adp5588_kpad *kpad)
 
 	mutex_init(&kpad->gpio_lock);
 
-	error = devm_gpiochip_add_data(dev, &kpad->gc, kpad);
-	if (error) {
-		dev_err(dev, "gpiochip_add failed: %d\n", error);
-		return error;
-	}
-
 	for (i = 0; i <= ADP5588_BANK(ADP5588_MAXGPIO); i++) {
 		kpad->dat_out[i] = adp5588_read(kpad->client,
 						GPIO_DAT_OUT1 + i);
@@ -459,6 +453,12 @@ static int adp5588_gpio_add(struct adp5588_kpad *kpad)
 		kpad->pull_dis[i] = adp5588_read(kpad->client, GPIO_PULL1 + i);
 	}
 
+	error = devm_gpiochip_add_data(dev, &kpad->gc, kpad);
+	if (error) {
+		dev_err(dev, "gpiochip_add failed: %d\n", error);
+		return error;
+	}
+
 	return 0;
 }
 

---
base-commit: 44f3468a0aef1aabdad551898ab7cfa2a9d20e99
change-id: 20260818-adp5588-gpio-cache-23fb8d0a3190

Best regards,
--  
Alvin Šipraga <alvin.sipraga@analog.com>

Re: [PATCH] Input: adp5588-keys - cache GPIO state before registering the gpiochip

Posted by Dmitry Torokhov 1 month ago

On Tue, Aug 18, 2026 at 06:00:02PM +0200, Alvin Šipraga wrote:
> So as not to clobber any pre-programmed GPIO state in the execution
> of its gpiochip ops, the driver caches things during probe time.
> However, since those ops can be called both during and immediately after
> the call to devm_gpiochip_add_data(), it is imperative that things are
> cached before that. That's not the case right now, so reorder the two
> steps to prevent any clobbering.
> 
> In a conrete example which motivated this change, a bootloader was
> preconfiguring an important GPIO output to HIGH before booting the
> kernel. Linux would then inadvertently set that output to LOW while
> configuring a GPIO hog on a discrete GPIO line within the same 8-bit
> bank (because the cached value was 0=LOW).
> 
> Fixes: ba9f507a1bea ("Input: adp5588-keys - export unused GPIO pins")
> Signed-off-by: Alvin Šipraga <alvin.sipraga@analog.com>

Applied, thank you.

-- 
Dmitry

Re: [PATCH] Input: adp5588-keys - cache GPIO state before registering the gpiochip

Posted by Nuno Sá 1 month, 1 week ago

On Tue, Aug 18, 2026 at 06:00:02PM +0200, Alvin Šipraga wrote:
> So as not to clobber any pre-programmed GPIO state in the execution
> of its gpiochip ops, the driver caches things during probe time.
> However, since those ops can be called both during and immediately after
> the call to devm_gpiochip_add_data(), it is imperative that things are
> cached before that. That's not the case right now, so reorder the two
> steps to prevent any clobbering.
> 
> In a conrete example which motivated this change, a bootloader was
> preconfiguring an important GPIO output to HIGH before booting the
> kernel. Linux would then inadvertently set that output to LOW while
> configuring a GPIO hog on a discrete GPIO line within the same 8-bit
> bank (because the cached value was 0=LOW).
> 
> Fixes: ba9f507a1bea ("Input: adp5588-keys - export unused GPIO pins")
> Signed-off-by: Alvin Šipraga <alvin.sipraga@analog.com>
> ---

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

>  drivers/input/keyboard/adp5588-keys.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/input/keyboard/adp5588-keys.c b/drivers/input/keyboard/adp5588-keys.c
> index 40371f5bd9ba..4f0ddff5baba 100644
> --- a/drivers/input/keyboard/adp5588-keys.c
> +++ b/drivers/input/keyboard/adp5588-keys.c
> @@ -446,12 +446,6 @@ static int adp5588_gpio_add(struct adp5588_kpad *kpad)
>  
>  	mutex_init(&kpad->gpio_lock);
>  
> -	error = devm_gpiochip_add_data(dev, &kpad->gc, kpad);
> -	if (error) {
> -		dev_err(dev, "gpiochip_add failed: %d\n", error);
> -		return error;
> -	}
> -
>  	for (i = 0; i <= ADP5588_BANK(ADP5588_MAXGPIO); i++) {
>  		kpad->dat_out[i] = adp5588_read(kpad->client,
>  						GPIO_DAT_OUT1 + i);
> @@ -459,6 +453,12 @@ static int adp5588_gpio_add(struct adp5588_kpad *kpad)
>  		kpad->pull_dis[i] = adp5588_read(kpad->client, GPIO_PULL1 + i);
>  	}
>  
> +	error = devm_gpiochip_add_data(dev, &kpad->gc, kpad);
> +	if (error) {
> +		dev_err(dev, "gpiochip_add failed: %d\n", error);
> +		return error;
> +	}
> +
>  	return 0;
>  }
>  
> 
> ---
> base-commit: 44f3468a0aef1aabdad551898ab7cfa2a9d20e99
> change-id: 20260818-adp5588-gpio-cache-23fb8d0a3190
> 
> Best regards,
> --  
> Alvin Šipraga <alvin.sipraga@analog.com>
>