[patch 2.6.28-rc4] gpiolib: extend gpio label column width in debugfs file

[patch 2.6.28-rc4] gpiolib: extend gpio label column width in debugfs file

[Jarkko Nikula]

There are already various drivers having bigger label than 12 bytes. Most
of them fit well under 20 bytes but make column width exact so that
oversized labels don't mess up output alignment.

Signed-off-by: Jarkko Nikula <jarkko.nikula@nokia.com>
---
 drivers/gpio/gpiolib.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index faa1cc6..82020ab 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1134,7 +1134,7 @@ static void gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip)
 			continue;
 
 		is_out = test_bit(FLAG_IS_OUT, &gdesc->flags);
-		seq_printf(s, " gpio-%-3d (%-12s) %s %s",
+		seq_printf(s, " gpio-%-3d (%-20.20s) %s %s",
 			gpio, gdesc->label,
 			is_out ? "out" : "in ",
 			chip->get
-- 
1.5.6.5

Re: [patch 2.6.28-rc4] gpiolib: extend gpio label column width in debugfs file

[Andrew Morton]

On Wed, 12 Nov 2008 08:56:13 +0200
Jarkko Nikula <jarkko.nikula@nokia.com> wrote:

> There are already various drivers having bigger label than 12 bytes. Most
> of them fit well under 20 bytes but make column width exact so that
> oversized labels don't mess up output alignment.

Please provide before-and-after example output for this sort of change
so that we can better understand its effect.

> Signed-off-by: Jarkko Nikula <jarkko.nikula@nokia.com>
> ---
>  drivers/gpio/gpiolib.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index faa1cc6..82020ab 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -1134,7 +1134,7 @@ static void gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip)
>  			continue;
>  
>  		is_out = test_bit(FLAG_IS_OUT, &gdesc->flags);
> -		seq_printf(s, " gpio-%-3d (%-12s) %s %s",
> +		seq_printf(s, " gpio-%-3d (%-20.20s) %s %s",
>  			gpio, gdesc->label,
>  			is_out ? "out" : "in ",
>  			chip->get

This is a non-backward-compatible change to a userspace interface.  It
looks like a pretty safe one, but this is always a fairly big deal.

If we're going to make this change then we should backport it to
2.6.27.x, 2.6.26.x and to 2.6.25.x to minimise the chance that someone
will write a parser which works on one kernel version and fails on
another.

David?  Thoughts?

Re: [patch 2.6.28-rc4] gpiolib: extend gpio label column width in debugfs file

[David Brownell]

On Thursday 13 November 2008, Andrew Morton wrote:
> On Wed, 12 Nov 2008 08:56:13 +0200
> Jarkko Nikula <jarkko.nikula@nokia.com> wrote:
> 
> > There are already various drivers having bigger label than 12 bytes. Most
> > of them fit well under 20 bytes but make column width exact so that
> > oversized labels don't mess up output alignment.
> 
> Please provide before-and-after example output for this sort of change
> so that we can better understand its effect.
> 
> > Signed-off-by: Jarkko Nikula <jarkko.nikula@nokia.com>

I meant to send:

Acked-by: David Brownell <dbrownell@users.sourceforge.net>

... thanks Jarkko.


> > ---
> >  drivers/gpio/gpiolib.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> > index faa1cc6..82020ab 100644
> > --- a/drivers/gpio/gpiolib.c
> > +++ b/drivers/gpio/gpiolib.c
> > @@ -1134,7 +1134,7 @@ static void gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip)
> >  			continue;
> >  
> >  		is_out = test_bit(FLAG_IS_OUT, &gdesc->flags);
> > -		seq_printf(s, " gpio-%-3d (%-12s) %s %s",
> > +		seq_printf(s, " gpio-%-3d (%-20.20s) %s %s",
> >  			gpio, gdesc->label,
> >  			is_out ? "out" : "in ",
> >  			chip->get
> 
> This is a non-backward-compatible change to a userspace interface.  It
> looks like a pretty safe one, but this is always a fairly big deal.

Not really; this is for debugfs, and formats for debug data
are not ABI-suitable.  Developers need the flexibility to
change that stuff ... for example, they can *already* override
the default format to show pullups/pulldowns, drive strength,
polarity inversion, debouncing, and other hardware-specific goodies.
 

> If we're going to make this change then we should backport it to
> 2.6.27.x, 2.6.26.x and to 2.6.25.x to minimise the chance that someone
> will write a parser which works on one kernel version and fails on
> another.
> 
> David?  Thoughts?

This is explicitly not supported for ABI-ish purposes.

If they aren't even using those parenthesis to delimit things, I'd
really need to question what happened to their grey cells!

- Dave

Re: [patch 2.6.28-rc4] gpiolib: extend gpio label column width in debugfs file

[Andrew Morton]

On Thu, 13 Nov 2008 17:13:38 -0800
David Brownell <david-b@pacbell.net> wrote:

> On Thursday 13 November 2008, Andrew Morton wrote:
> > On Wed, 12 Nov 2008 08:56:13 +0200
> > Jarkko Nikula <jarkko.nikula@nokia.com> wrote:
> > 
> > > There are already various drivers having bigger label than 12 bytes. Most
> > > of them fit well under 20 bytes but make column width exact so that
> > > oversized labels don't mess up output alignment.
> > 
> > Please provide before-and-after example output for this sort of change
> > so that we can better understand its effect.
> > 
> > > Signed-off-by: Jarkko Nikula <jarkko.nikula@nokia.com>
> 
> I meant to send:
> 
> Acked-by: David Brownell <dbrownell@users.sourceforge.net>
> 
> ... thanks Jarkko.
> 

OK.

> > > ---
> > >  drivers/gpio/gpiolib.c |    2 +-
> > >  1 files changed, 1 insertions(+), 1 deletions(-)
> > > 
> > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> > > index faa1cc6..82020ab 100644
> > > --- a/drivers/gpio/gpiolib.c
> > > +++ b/drivers/gpio/gpiolib.c
> > > @@ -1134,7 +1134,7 @@ static void gpiolib_dbg_show(struct seq_file *s, struct gpio_chip *chip)
> > >  			continue;
> > >  
> > >  		is_out = test_bit(FLAG_IS_OUT, &gdesc->flags);
> > > -		seq_printf(s, " gpio-%-3d (%-12s) %s %s",
> > > +		seq_printf(s, " gpio-%-3d (%-20.20s) %s %s",
> > >  			gpio, gdesc->label,
> > >  			is_out ? "out" : "in ",
> > >  			chip->get
> > 
> > This is a non-backward-compatible change to a userspace interface.  It
> > looks like a pretty safe one, but this is always a fairly big deal.
> 
> Not really; this is for debugfs, and formats for debug data
> are not ABI-suitable.  Developers need the flexibility to
> change that stuff ...

Well that's what we like to say, but it's delusional.  If we add stuff
then later change it, other people's stuff will break.  Just as with sysfs,
procfs, syscalls, etc.

The only difference is that with debugfs we have this weaselly excuse to
get people off our backs.

> for example, they can *already* override
> the default format to show pullups/pulldowns, drive strength,
> polarity inversion, debouncing, and other hardware-specific goodies.
>  
> 
> > If we're going to make this change then we should backport it to
> > 2.6.27.x, 2.6.26.x and to 2.6.25.x to minimise the chance that someone
> > will write a parser which works on one kernel version and fails on
> > another.
> > 
> > David?  Thoughts?
> 
> This is explicitly not supported for ABI-ish purposes.
> 
> If they aren't even using those parenthesis to delimit things, I'd
> really need to question what happened to their grey cells!

True.