Message-ID: <CAD++jLkMxzbAVgDb86zWdfAuTNZ+e83t2RA--zpsGGzV6m7=WQ@mail.gmail.com>
Date: Sun, 7 Dec 2025 21:28:59 +0100
From: Linus Walleij <linusw@...nel.org>
To: Mintu Patel <mintupatel89@...il.com>
Cc: Saurabh Kumar <saurabhsingh14june@...il.com>, 
	Shahid Kagadgar <shahidkagadgar3821@...il.com>, Dilshad Alam <Dil.alam@...il.com>, 
	Linus Walleij <linus.walleij@...aro.org>, Bartosz Golaszewski <brgl@...ev.pl>, 
	"Rafael J. Wysocki" <rafael@...nel.org>, Len Brown <len.brown@...el.com>, Pavel Machek <pavel@....cz>, 
	linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-pm@...r.kernel.org
Subject: Re: [PATCH] Dumping GPIOs state during suspend state

Hi Mintu,

thanks for your patch!

Mainly I am awaiting the power state maintainers comment on this.

On Sat, Dec 6, 2025 at 5:30 PM Mintu Patel <mintupatel89@...il.com> wrote:

> It is difficult to find out which GPIOs are going high during suspend

Why is it not hard to find out which GPIOs are going low during suspend?
I don't quite get this initial sentence, why does polarity even matter
in this scenario?

> without manual probing with multimeters or oscilloscopes.

I understand this part, the system is suspended so it can't be inspected.

> In order to save the power especifically for battery operated devices,
> It becomes must to pull GPIOs low during suspend.

This assumes a certain type of silicon architecture does it not.

IIUC it is important to pull GPIOs low *or* high in some cases,
and what really needs to be avoided is pins being in tristate
(high impedance, high-Z) becaus this can really cause leak
current. Is this correct?

GPIO does have a notion about high
impedance state, because it has .set_config() that can actually
put a pin into this state, while it is mostly handled by a back-end
in pin control when available, so it is not something all GPIO
drivers are semantically aware of.

Consider if you might want hooks like this in pin control
rather than in GPIO, but maybe you want both.

> As of now there is no way to trace which GPIOs are high during suspend
> without manual probing with multimeters or oscilloscopes.

This is already mentioned above.

> This patch would help the developer to find the GPIOs are high during
> suspend state without struggling with hardware devices.

Fair enough, that is a reasonable usecase.

> The patch would print the below logs:
>
> [  244.029392] GPIO Chip : 3000000.pinctrl
> [  244.029394] GPIO   Suspend state
> [  244.029404] 3      1
> [  244.029418] 17      1
> [  244.029424] 21      1
> [  244.029431] 30      1
> [  244.029436] 32      1
> [  244.029440] 33      1
> [  244.029444] 34      1
> [  244.029447] 35      1
> [  244.029453] 41      1
> [  244.029462] 51      1
> [  244.029470] 57      1
> [  244.029492] 90      1
> [  244.029496] 91      1
> [  244.029515] 114      1
> [  244.029519] 115      1
> [  244.029523] 117      1
>
> Based on the above logs, GPIOs 3, 17, etc are high during suspend state.
> Thus particular driver/submodule can be modified for configuring
> the GPIOs accordingly for suspend and resume states.

I think I understand what you are trying to solve: you want to inspect
what state the pins are not "during" suspend, but *at the moment
suspend happens*, there is a difference there, and this is why
some of the text is confusing me.

I would do this differently.

What I would do it:

- Go over all pins both when going into suspend *and* when going
  out of suspend. This creates a snapshot of the GPIO lines
  before and after suspend.

- Store not just pins that are high, because that is a pecularity of
  the system you are trying to debug here, store the state of
  *all* pins, because other users of this facility may very well
  want to know which pins are *low* at suspend and resume.

> +#ifdef CONFIG_DEBUG_FS

Why debug_fs? All this does is pr_info(), it doesn't need
debugfs at all.

It could perhaps depend on CONFIG_DEBUG but I would
suggest that you create a new Kconfig especially for this,
like CONFIG_DEBUG_GPIO_STATES_AT_SUSPEND_RESUME
OK maybe a bit long but you get my idea.

> +static void fetching_gpios_state_suspend(struct gpio_chip *chip) {

Use "fetch" (passive form) not "fetching" (active form).

But no, rename the function:
print_gpio_chip_states() because that is what it does.

> +
> +       int gpio_num, value = 0;
> +
> +       pr_info("GPIO Chip : %s\n",chip->label);
> +       pr_info("GPIO   Suspend state\n");
> +       for (gpio_num = 0; gpio_num <chip->ngpio; gpio_num++ ) {
> +               value = chip->get(chip, gpio_num);
> +                       if(value != 0) {

As mentioned I would do this for all states not just high
pins.

> +                       pr_info("%d      %d\n",gpio_num, value);
> +                       }
> +       }
> +       }

This is just printing the state if rg

> +void gpio_state_fetch_at_suspend() {

I would call this
gpio_print_state_at_suspend()
because that is what it does.

> +#ifdef CONFIG_DEBUG_FS
> +extern void gpio_state_fetch_at_suspend(void);
> +#endif

What about putting it in a new include
include/linux/gpio/system.h and include that?

>  const char * const pm_labels[] = {
>         [PM_SUSPEND_TO_IDLE] = "freeze",
>         [PM_SUSPEND_STANDBY] = "standby",
> @@ -429,6 +433,10 @@ static int suspend_enter(suspend_state_t state, bool *wakeup)
>         if (suspend_test(TEST_PLATFORM))
>                 goto Platform_wake;
>
> +#ifdef CONFIG_DEBUG_FS

This Kconfig needs to be changed as mentioned.

> +       gpio_state_fetch_at_suspend();
> +#endif

As mentioned I would like this to be printed also on resume.

Yours,
Linus Walleij

Powered by blists - more mailing lists