[PATCH] pinctrl: core: print gpio in pins debugfs file

[PATCH] pinctrl: core: print gpio in pins debugfs file

[Drew Fustini]

If there is a gpio range mapping for the pin, then print out the gpio
number for the pin in the debugfs 'pins' file.

Here is an example output on the BeagleBone Black from:
/sys/kernel/debug/pinctrl/44e10800.pinmux-pinctrl-single/pins

pin 103 (PIN103) GPIO-113 44e1099c 00000027 pinctrl-single
pin 104 (PIN104) GPIO-114 44e109a0 0000002c pinctrl-single
pin 105 (PIN105) GPIO-115 44e109a4 00000027 pinctrl-single
pin 106 (PIN106) GPIO-116 44e109a8 00000027 pinctrl-single
pin 107 (PIN107) GPIO-117 44e109ac 00000027 pinctrl-single
pin 108 (PIN108) GPIO-19 44e109b0 00000027 pinctrl-single
pin 109 (PIN109) GPIO-20 44e109b4 00000027 pinctrl-single
pin 110 (PIN110) 44e109b8 00000030 pinctrl-single
pin 111 (PIN111) 44e109bc 00000028 pinctrl-single
pin 112 (PIN112) 44e109c0 00000030 pinctrl-single
pin 113 (PIN113) 44e109c4 00000028 pinctrl-single
pin 114 (PIN114) 44e109c8 00000028 pinctrl-single

Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Suggested-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Drew Fustini <drew@beagleboard.org>
---
Note:
- I am hoping to get feedback if another format is better.  Currently
  the column 'GPIO-xxx' will only be printed when there is a GPIO num

 drivers/pinctrl/core.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index 70e13fcd94c4..e3eee950a553 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -1633,6 +1633,8 @@ static int pinctrl_pins_show(struct seq_file *s, void *what)
 	struct pinctrl_dev *pctldev = s->private;
 	const struct pinctrl_ops *ops = pctldev->desc->pctlops;
 	unsigned i, pin;
+	struct pinctrl_gpio_range *range;
+	unsigned gpio_num;
 
 	seq_printf(s, "registered pins: %d\n", pctldev->desc->npins);
 
@@ -1650,6 +1652,16 @@ static int pinctrl_pins_show(struct seq_file *s, void *what)
 
 		seq_printf(s, "pin %d (%s) ", pin, desc->name);
 
+		gpio_num = 0;
+		list_for_each_entry(range, &pctldev->gpio_ranges, node) {
+			if ((pin >= range->pin_base) &&
+			    (pin < (range->pin_base + range->npins)))
+				gpio_num = range->base + (pin - range->pin_base);
+		}
+
+		if (gpio_num > 0)
+			seq_printf(s, "GPIO-%u ", gpio_num);
+
 		/* Driver-specific info per pin */
 		if (ops->pin_dbg_show)
 			ops->pin_dbg_show(pctldev, s, pin);
-- 
2.25.1

Re: [PATCH] pinctrl: core: print gpio in pins debugfs file

[Tony Lindgren]

* Drew Fustini <drew@beagleboard.org> [200718 15:53]:
> If there is a gpio range mapping for the pin, then print out the gpio
> number for the pin in the debugfs 'pins' file.
> 
> Here is an example output on the BeagleBone Black from:
> /sys/kernel/debug/pinctrl/44e10800.pinmux-pinctrl-single/pins
> 
> pin 103 (PIN103) GPIO-113 44e1099c 00000027 pinctrl-single
> pin 104 (PIN104) GPIO-114 44e109a0 0000002c pinctrl-single
> pin 105 (PIN105) GPIO-115 44e109a4 00000027 pinctrl-single
> pin 106 (PIN106) GPIO-116 44e109a8 00000027 pinctrl-single
> pin 107 (PIN107) GPIO-117 44e109ac 00000027 pinctrl-single
> pin 108 (PIN108) GPIO-19 44e109b0 00000027 pinctrl-single
> pin 109 (PIN109) GPIO-20 44e109b4 00000027 pinctrl-single
> pin 110 (PIN110) 44e109b8 00000030 pinctrl-single
> pin 111 (PIN111) 44e109bc 00000028 pinctrl-single
> pin 112 (PIN112) 44e109c0 00000030 pinctrl-single
> pin 113 (PIN113) 44e109c4 00000028 pinctrl-single
> pin 114 (PIN114) 44e109c8 00000028 pinctrl-single

This looks nice to me, maybe just show NA if no GPIO name is
available?

This is debugfs so the output format can change at any point
AFAIK.

Regards,

Tony

Re: [PATCH] pinctrl: core: print gpio in pins debugfs file

[Linus Walleij]

Hi Drew,

thanks for this patch, we're going the right direction here
and creating things that are generically useful.

On Sat, Jul 18, 2020 at 5:53 PM Drew Fustini <drew@beagleboard.org> wrote:

> pin 103 (PIN103) GPIO-113 44e1099c 00000027 pinctrl-single
> pin 104 (PIN104) GPIO-114 44e109a0 0000002c pinctrl-single
(...)

Uh oh, that is the global GPIO number that we want to get
rid of.

> +               gpio_num = 0;
> +               list_for_each_entry(range, &pctldev->gpio_ranges, node) {
> +                       if ((pin >= range->pin_base) &&
> +                           (pin < (range->pin_base + range->npins)))
> +                               gpio_num = range->base + (pin - range->pin_base);

There should be a break; here should it not?

> +               }
> +
> +               if (gpio_num > 0)
> +                       seq_printf(s, "GPIO-%u ", gpio_num);

Can we print the gpio_chip name and offset instead?
I want to discourage the world from thinking about these
global GPIO numbers.

You can fetch the gpio_chip for the range pretty easily
with

struct gpio_chip *chip = gpio_to_chip(gpio_num);

Also notice that this code needs to be
#ifdef CONFIG_GPIOLIB somehow
(maybe IS_ENABLED() works) because there
are pin controllers in use without gpiolib believe it
or not.

Yours,
Linus Walleij

Re: [PATCH] pinctrl: core: print gpio in pins debugfs file

[Drew Fustini]

On Mon, Jul 20, 2020 at 04:26:21PM +0200, Linus Walleij wrote:
> Hi Drew,
> 
> thanks for this patch, we're going the right direction here
> and creating things that are generically useful.
> 
> On Sat, Jul 18, 2020 at 5:53 PM Drew Fustini <drew@beagleboard.org> wrote:
> 
> > pin 103 (PIN103) GPIO-113 44e1099c 00000027 pinctrl-single
> > pin 104 (PIN104) GPIO-114 44e109a0 0000002c pinctrl-single
> (...)
> 
> Uh oh, that is the global GPIO number that we want to get
> rid of.
> 
> > +               gpio_num = 0;
> > +               list_for_each_entry(range, &pctldev->gpio_ranges, node) {
> > +                       if ((pin >= range->pin_base) &&
> > +                           (pin < (range->pin_base + range->npins)))
> > +                               gpio_num = range->base + (pin - range->pin_base);
> 
> There should be a break; here should it not?

Yes, good point. 

> > +               }
> > +
> > +               if (gpio_num > 0)
> > +                       seq_printf(s, "GPIO-%u ", gpio_num);
> 
> Can we print the gpio_chip name and offset instead?
> I want to discourage the world from thinking about these
> global GPIO numbers.
> 
> You can fetch the gpio_chip for the range pretty easily
> with
> 
> struct gpio_chip *chip = gpio_to_chip(gpio_num);
> 
> Also notice that this code needs to be
> #ifdef CONFIG_GPIOLIB somehow
> (maybe IS_ENABLED() works) because there
> are pin controllers in use without gpiolib believe it
> or not.
> 
> Yours,
> Linus Walleij

I've just posted a v2 [0] which makes sure the gpio information is only
if CONFIG_GPIOLIB.

I had a hard time finding line offset so I printed the line name
instead.  I would appreciate it if you could point me in the right place
to get the offset as that is cleaner in the 'pins' output.

thanks,
drew

[0] https://lore.kernel.org/linux-omap/20200720191740.1974132-1-drew@beagleboard.org/