[PATCH 0/3] gpiolib: fix debugfs newline separators

[PATCH 0/3] gpiolib: fix debugfs newline separators

[Johan Hovold]

I've noticed this before on some systems but never got around to looking
into why the gpio debugfs newline separators are sometimes missing.

On recent Qualcomm machines with 10+ gpio chips this can get really
annoying when a third of the separators are missing (e.g. when verifying
pin settings). Hence the CC stable tag.

Johan


Johan Hovold (3):
  gpiolib: fix debugfs newline separators
  gpiolib: fix debugfs dangling chip separator
  gpiolib: clean up debugfs separator handling

 drivers/gpio/gpiolib.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

-- 
2.45.2

[PATCH 1/3] gpiolib: fix debugfs newline separators

[Johan Hovold]

The gpiolib debugfs interface exports a list of all gpio chips in a
system and the state of their pins.

The gpio chip sections are supposed to be separated by a newline
character, but a long-standing bug prevents the separator from
being included when output is generated in multiple sessions, making the
output inconsistent and hard to read.

Make sure to only suppress the newline separator at the beginning of the
file as intended.

Fixes: f9c4a31f6150 ("gpiolib: Use seq_file's iterator interface")
Cc: stable@vger.kernel.org	# 3.7
Cc: Thierry Reding <treding@nvidia.com>
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/gpio/gpiolib.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index d5952ab7752c..e27488a90bc9 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4926,6 +4926,8 @@ static void *gpiolib_seq_start(struct seq_file *s, loff_t *pos)
 		return NULL;
 
 	s->private = priv;
+	if (*pos > 0)
+		priv->newline = true;
 	priv->idx = srcu_read_lock(&gpio_devices_srcu);
 
 	list_for_each_entry_srcu(gdev, &gpio_devices, list,
-- 
2.45.2

[PATCH 2/3] gpiolib: fix debugfs dangling chip separator

[Johan Hovold]

Add the missing newline after entries for recently removed gpio chips
so that the chip sections are separated by a newline as intended.

Fixes: e348544f7994 ("gpio: protect the list of GPIO devices with SRCU")
Cc: stable@vger.kernel.org	# 6.9
Cc: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/gpio/gpiolib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index e27488a90bc9..2b02655abb56 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4971,7 +4971,7 @@ static int gpiolib_seq_show(struct seq_file *s, void *v)
 
 	gc = srcu_dereference(gdev->chip, &gdev->srcu);
 	if (!gc) {
-		seq_printf(s, "%s%s: (dangling chip)",
+		seq_printf(s, "%s%s: (dangling chip)\n",
 			   priv->newline ? "\n" : "",
 			   dev_name(&gdev->dev));
 		return 0;
-- 
2.45.2

[PATCH 3/3] gpiolib: clean up debugfs separator handling

[Johan Hovold]

Add the newline separator before generating the gpio chip entry to make
the code easier to read.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/gpio/gpiolib.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 2b02655abb56..6da73a373d9b 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4967,19 +4967,19 @@ static int gpiolib_seq_show(struct seq_file *s, void *v)
 	struct gpio_chip *gc;
 	struct device *parent;
 
+	if (priv->newline)
+		seq_putc(s, '\n');
+
 	guard(srcu)(&gdev->srcu);
 
 	gc = srcu_dereference(gdev->chip, &gdev->srcu);
 	if (!gc) {
-		seq_printf(s, "%s%s: (dangling chip)\n",
-			   priv->newline ? "\n" : "",
-			   dev_name(&gdev->dev));
+		seq_printf(s, "%s: (dangling chip)\n", dev_name(&gdev->dev));
 		return 0;
 	}
 
-	seq_printf(s, "%s%s: GPIOs %u-%u", priv->newline ? "\n" : "",
-		   dev_name(&gdev->dev),
-		   gdev->base, gdev->base + gdev->ngpio - 1);
+	seq_printf(s, "%s: GPIOs %u-%u", dev_name(&gdev->dev), gdev->base,
+		   gdev->base + gdev->ngpio - 1);
 	parent = gc->parent;
 	if (parent)
 		seq_printf(s, ", parent: %s/%s",
-- 
2.45.2

Re: [PATCH 2/3] gpiolib: fix debugfs dangling chip separator

[Bartosz Golaszewski]

On Mon, Oct 28, 2024 at 1:50 PM Johan Hovold <johan+linaro@kernel.org> wrote:
>
> Add the missing newline after entries for recently removed gpio chips
> so that the chip sections are separated by a newline as intended.
>
> Fixes: e348544f7994 ("gpio: protect the list of GPIO devices with SRCU")
> Cc: stable@vger.kernel.org      # 6.9
> Cc: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  drivers/gpio/gpiolib.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index e27488a90bc9..2b02655abb56 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -4971,7 +4971,7 @@ static int gpiolib_seq_show(struct seq_file *s, void *v)
>
>         gc = srcu_dereference(gdev->chip, &gdev->srcu);
>         if (!gc) {
> -               seq_printf(s, "%s%s: (dangling chip)",
> +               seq_printf(s, "%s%s: (dangling chip)\n",
>                            priv->newline ? "\n" : "",
>                            dev_name(&gdev->dev));
>                 return 0;
> --
> 2.45.2
>

But with this change we go from an incorrect:

# cat /sys/kernel/debug/gpio
gpiochip0: (dangling chip)
gpiochip1: (dangling chip)
gpiochip2: (dangling chip)root@qemux86-64:~#

to still incorrect:

# cat /sys/kernel/debug/gpio
gpiochip0: (dangling chip)

gpiochip1: (dangling chip)

gpiochip2: (dangling chip)

Bart

Re: [PATCH 2/3] gpiolib: fix debugfs dangling chip separator

[Johan Hovold]

On Thu, Oct 31, 2024 at 06:02:43PM +0100, Bartosz Golaszewski wrote:

> But with this change we go from an incorrect:
> 
> # cat /sys/kernel/debug/gpio
> gpiochip0: (dangling chip)
> gpiochip1: (dangling chip)
> gpiochip2: (dangling chip)root@qemux86-64:~#
> 
> to still incorrect:
> 
> # cat /sys/kernel/debug/gpio
> gpiochip0: (dangling chip)
> 
> gpiochip1: (dangling chip)
> 
> gpiochip2: (dangling chip)

Why do you think this is incorrect? Every chip section is separated by
an empty line, just as it should be:

gpiochip0: GPIOs 512-517, parent: platform/c42d000.spmi:pmic@0:gpio@8800, c42d000.spmi:pmic@0:gpio@8800:
 gpio1 : in   low  normal  vin-0 no pull                     push-pull  low     atest-1 dtest-0
 gpio2 : in   low  normal  vin-0 no pull                     push-pull  low     atest-1 dtest-0
 gpio3 : out  low  func1   vin-0 pull-down 10uA              push-pull  low     atest-1 dtest-0
 gpio4 : in   low  normal  vin-0 pull-down 10uA              push-pull  low     atest-1 dtest-0
 gpio5 : ---
 gpio6 : in   high normal  vin-0 pull-up 30uA                push-pull  low     atest-1 dtest-0

gpiochip1: GPIOs 518-529, parent: platform/c42d000.spmi:pmic@1:gpio@8800, c42d000.spmi:pmic@1:gpio@8800:
 gpio1 : in   low  normal  vin-0 pull-down 10uA              push-pull  low     atest-1 dtest-0
 gpio2 : in   low  normal  vin-0 pull-down 10uA              push-pull  low     atest-1 dtest-0
 gpio3 : ---
 gpio4 : ---
 gpio5 : in   high normal  vin-0 pull-up 30uA                push-pull  low     atest-1 dtest-0
 gpio6 : in   high normal  vin-1 pull-up 30uA                push-pull  low     atest-1 dtest-0
 gpio7 : out  high func1   vin-1 no pull                     push-pull  low     atest-1 dtest-0
 gpio8 : in   low  normal  vin-0 pull-down 10uA              push-pull  low     atest-1 dtest-0
 gpio9 : in   low  normal  vin-0 pull-down 10uA              push-pull  low     atest-1 dtest-0
 gpio10: out  high normal  vin-1 no pull                     push-pull  low     atest-1 dtest-0
 gpio11: out  high normal  vin-1 no pull                     push-pull  low     atest-1 dtest-0
 gpio12: in   low  normal  vin-1 pull-down 10uA              push-pull  low     atest-1 dtest-0

gpiochip2: GPIOs 530-537, parent: platform/c42d000.spmi:pmic@2:gpio@8800, c42d000.spmi:pmic@2:gpio@8800:
 gpio1 : in   low  normal  vin-0 pull-down 10uA              push-pull  low     atest-1 dtest-0
 gpio2 : in   low  normal  vin-0 pull-down 10uA              push-pull  low     atest-1 dtest-0
 gpio3 : in   low  normal  vin-0 pull-down 10uA              push-pull  low     atest-1 dtest-0
 gpio4 : out  high normal  vin-1 pull-down 10uA              push-pull  medium  atest-1 dtest-0
 gpio5 : in   low  normal  vin-1 pull-down 10uA              push-pull  low     atest-1 dtest-0
 gpio6 : out  high normal  vin-1 pull-down 10uA              push-pull  low     atest-1 dtest-0
 gpio7 : in   low  normal  vin-0 pull-down 10uA              push-pull  low     atest-1 dtest-0
 gpio8 : out  low  normal  vin-1 pull-down 10uA              push-pull  low     atest-1 dtest-0

Johan

Re: [PATCH 2/3] gpiolib: fix debugfs dangling chip separator

[Bartosz Golaszewski]

On Thu, Oct 31, 2024 at 6:07 PM Johan Hovold <johan@kernel.org> wrote:
>
> On Thu, Oct 31, 2024 at 06:02:43PM +0100, Bartosz Golaszewski wrote:
>
> > But with this change we go from an incorrect:
> >
> > # cat /sys/kernel/debug/gpio
> > gpiochip0: (dangling chip)
> > gpiochip1: (dangling chip)
> > gpiochip2: (dangling chip)root@qemux86-64:~#
> >
> > to still incorrect:
> >
> > # cat /sys/kernel/debug/gpio
> > gpiochip0: (dangling chip)
> >
> > gpiochip1: (dangling chip)
> >
> > gpiochip2: (dangling chip)
>
> Why do you think this is incorrect? Every chip section is separated by
> an empty line, just as it should be:
>
> gpiochip0: GPIOs 512-517, parent: platform/c42d000.spmi:pmic@0:gpio@8800, c42d000.spmi:pmic@0:gpio@8800:
>  gpio1 : in   low  normal  vin-0 no pull                     push-pull  low     atest-1 dtest-0
>  gpio2 : in   low  normal  vin-0 no pull                     push-pull  low     atest-1 dtest-0
>  gpio3 : out  low  func1   vin-0 pull-down 10uA              push-pull  low     atest-1 dtest-0
>  gpio4 : in   low  normal  vin-0 pull-down 10uA              push-pull  low     atest-1 dtest-0
>  gpio5 : ---
>  gpio6 : in   high normal  vin-0 pull-up 30uA                push-pull  low     atest-1 dtest-0
>
> gpiochip1: GPIOs 518-529, parent: platform/c42d000.spmi:pmic@1:gpio@8800, c42d000.spmi:pmic@1:gpio@8800:
>  gpio1 : in   low  normal  vin-0 pull-down 10uA              push-pull  low     atest-1 dtest-0
>  gpio2 : in   low  normal  vin-0 pull-down 10uA              push-pull  low     atest-1 dtest-0
>  gpio3 : ---
>  gpio4 : ---
>  gpio5 : in   high normal  vin-0 pull-up 30uA                push-pull  low     atest-1 dtest-0
>  gpio6 : in   high normal  vin-1 pull-up 30uA                push-pull  low     atest-1 dtest-0
>  gpio7 : out  high func1   vin-1 no pull                     push-pull  low     atest-1 dtest-0
>  gpio8 : in   low  normal  vin-0 pull-down 10uA              push-pull  low     atest-1 dtest-0
>  gpio9 : in   low  normal  vin-0 pull-down 10uA              push-pull  low     atest-1 dtest-0
>  gpio10: out  high normal  vin-1 no pull                     push-pull  low     atest-1 dtest-0
>  gpio11: out  high normal  vin-1 no pull                     push-pull  low     atest-1 dtest-0
>  gpio12: in   low  normal  vin-1 pull-down 10uA              push-pull  low     atest-1 dtest-0
>
> gpiochip2: GPIOs 530-537, parent: platform/c42d000.spmi:pmic@2:gpio@8800, c42d000.spmi:pmic@2:gpio@8800:
>  gpio1 : in   low  normal  vin-0 pull-down 10uA              push-pull  low     atest-1 dtest-0
>  gpio2 : in   low  normal  vin-0 pull-down 10uA              push-pull  low     atest-1 dtest-0
>  gpio3 : in   low  normal  vin-0 pull-down 10uA              push-pull  low     atest-1 dtest-0
>  gpio4 : out  high normal  vin-1 pull-down 10uA              push-pull  medium  atest-1 dtest-0
>  gpio5 : in   low  normal  vin-1 pull-down 10uA              push-pull  low     atest-1 dtest-0
>  gpio6 : out  high normal  vin-1 pull-down 10uA              push-pull  low     atest-1 dtest-0
>  gpio7 : in   low  normal  vin-0 pull-down 10uA              push-pull  low     atest-1 dtest-0
>  gpio8 : out  low  normal  vin-1 pull-down 10uA              push-pull  low     atest-1 dtest-0
>
> Johan

Ah, makes more sense in the context of mixed good and dangling output.

Nevermind my comment.

Bart

Re: (subset) [PATCH 0/3] gpiolib: fix debugfs newline separators

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>


On Mon, 28 Oct 2024 13:49:57 +0100, Johan Hovold wrote:
> I've noticed this before on some systems but never got around to looking
> into why the gpio debugfs newline separators are sometimes missing.
> 
> On recent Qualcomm machines with 10+ gpio chips this can get really
> annoying when a third of the separators are missing (e.g. when verifying
> pin settings). Hence the CC stable tag.
> 
> [...]

Applied, thanks!

[1/3] gpiolib: fix debugfs newline separators
      commit: 3e8b7238b427e05498034c240451af5f5495afda
[2/3] gpiolib: fix debugfs dangling chip separator
      commit: 604888f8c3d01fddd9366161efc65cb3182831f1

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Re: [PATCH 3/3] gpiolib: clean up debugfs separator handling

[Bartosz Golaszewski]

On Mon, Oct 28, 2024 at 1:50 PM Johan Hovold <johan+linaro@kernel.org> wrote:
>
> Add the newline separator before generating the gpio chip entry to make
> the code easier to read.
>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>

This isn't a fix so I'll queue it once the first two patches are in mainline.

Bart

Re: (subset) [PATCH 0/3] gpiolib: fix debugfs newline separators

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>


On Mon, 28 Oct 2024 13:49:57 +0100, Johan Hovold wrote:
> I've noticed this before on some systems but never got around to looking
> into why the gpio debugfs newline separators are sometimes missing.
> 
> On recent Qualcomm machines with 10+ gpio chips this can get really
> annoying when a third of the separators are missing (e.g. when verifying
> pin settings). Hence the CC stable tag.
> 
> [...]

Applied, thanks!

[3/3] gpiolib: clean up debugfs separator handling
      commit: b9b7e4a67de1c9d40138f89e86c92ee72e631f47

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@linaro.org>