gpiolib: add names file in gpio chip sysfs.

gpiolib: add names file in gpio chip sysfs.

[ben]

[-- Attachment #1: gpio-export-names.patch --]
[-- Type: text/plain, Size: 1566 bytes --]

Add a 'names' file to the sysfs entries for each chip to show which
have names.

Signed-off-by: Ben Dooks <ben@simtec.co.uk>
CC: David Brownell <dbrownell@users.sourceforge.net>

---
 drivers/gpio/gpiolib.c |   21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

Index: b/drivers/gpio/gpiolib.c
===================================================================
--- a/drivers/gpio/gpiolib.c	2009-08-03 17:51:40.000000000 +0100
+++ b/drivers/gpio/gpiolib.c	2009-08-03 17:53:04.000000000 +0100
@@ -303,6 +303,7 @@ static const struct attribute_group gpio
  *   /base ... matching gpio_chip.base (N)
  *   /label ... matching gpio_chip.label
  *   /ngpio ... matching gpio_chip.ngpio
+ *   /names ... matching gpio_chip.names
  */
 
 static ssize_t chip_base_show(struct device *dev,
@@ -332,10 +333,30 @@ static ssize_t chip_ngpio_show(struct de
 }
 static DEVICE_ATTR(ngpio, 0444, chip_ngpio_show, NULL);
 
+static ssize_t chip_names_show(struct device *dev,
+			       struct device_attribute *attr, char *buf)
+{
+	const struct gpio_chip*chip = dev_get_drvdata(dev);
+	char **names = chip->names;
+	int ptr = 0;
+	int name;
+
+	if (!names)
+		return -EINVAL;
+
+	for (name = 0; name < chip->ngpio && ptr < PAGE_SIZE; name++)
+		ptr += snprintf(buf + ptr, PAGE_SIZE - ptr,
+				"%s\n", names[name] ? names[name] : "");
+
+	return ptr;
+}
+DEVICE_ATTR(names, 0444, chip_names_show, NULL);
+
 static const struct attribute *gpiochip_attrs[] = {
 	&dev_attr_base.attr,
 	&dev_attr_label.attr,
 	&dev_attr_ngpio.attr,
+	&dev_attr_names.attr,
 	NULL,
 };
 

-- 

Re: gpiolib: add names file in gpio chip sysfs.

[David Brownell]

On Monday 03 August 2009, ben@fluff.org wrote:
> +static ssize_t chip_names_show(struct device *dev,
> +                              struct device_attribute *attr, char *buf)
> +{
> +       const struct gpio_chip*chip = dev_get_drvdata(dev);
> +       char **names = chip->names;
> +       int ptr = 0;
> +       int name;
> +
> +       if (!names)
> +               return -EINVAL;
> +
> +       for (name = 0; name < chip->ngpio && ptr < PAGE_SIZE; name++)
> +               ptr += snprintf(buf + ptr, PAGE_SIZE - ptr,
> +                               "%s\n", names[name] ? names[name] : "");

Surely the actual GPIO number should be there too?

This whole chip->names thing still seems odd to me,
since it's creating a redundant namespace.


> +
> +       return ptr;
> +}
> +DEVICE_ATTR(names, 0444, chip_names_show, NULL);
> +

Re: gpiolib: add names file in gpio chip sysfs.

[Andrew Morton]

On Mon, 03 Aug 2009 17:55:36 +0100
ben@fluff.org wrote:
>

Did you mean the commit to have an author of ben@fluff.org?  I assumed
not and rewrote it to

	Ben Dooks <ben@simtec.co.uk>

If you indeed want a different Author: and Signed-off-by: line then
please indicate that explicitly by putting a From: line at the top of
the changelog.

As your MUA didn't fill in the real-name part of the From: address,
it's nice to provide a From: line in the changelog so the patch
receiver doesn't have to type it in.

> Add a 'names' file to the sysfs entries for each chip to show which
> have names.

Why?

> Signed-off-by: Ben Dooks <ben@simtec.co.uk>
> CC: David Brownell <dbrownell@users.sourceforge.net>
> 
> ---
>  drivers/gpio/gpiolib.c |   21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> Index: b/drivers/gpio/gpiolib.c
> ===================================================================
> --- a/drivers/gpio/gpiolib.c	2009-08-03 17:51:40.000000000 +0100
> +++ b/drivers/gpio/gpiolib.c	2009-08-03 17:53:04.000000000 +0100
> @@ -303,6 +303,7 @@ static const struct attribute_group gpio
>   *   /base ... matching gpio_chip.base (N)
>   *   /label ... matching gpio_chip.label
>   *   /ngpio ... matching gpio_chip.ngpio
> + *   /names ... matching gpio_chip.names
>   */
>  
>  static ssize_t chip_base_show(struct device *dev,
> @@ -332,10 +333,30 @@ static ssize_t chip_ngpio_show(struct de
>  }
>  static DEVICE_ATTR(ngpio, 0444, chip_ngpio_show, NULL);
>  
> +static ssize_t chip_names_show(struct device *dev,
> +			       struct device_attribute *attr, char *buf)
> +{
> +	const struct gpio_chip*chip = dev_get_drvdata(dev);

Please use checkpatch.

> +	char **names = chip->names;
> +	int ptr = 0;
> +	int name;
> +
> +	if (!names)
> +		return -EINVAL;

Should this return -EINVAL?  Or should we simply return an empty read()?

> +	for (name = 0; name < chip->ngpio && ptr < PAGE_SIZE; name++)
> +		ptr += snprintf(buf + ptr, PAGE_SIZE - ptr,
> +				"%s\n", names[name] ? names[name] : "");
> +
> +	return ptr;
> +}
> +DEVICE_ATTR(names, 0444, chip_names_show, NULL);

I shall make this static.

>  static const struct attribute *gpiochip_attrs[] = {
>  	&dev_attr_base.attr,
>  	&dev_attr_label.attr,
>  	&dev_attr_ngpio.attr,
> +	&dev_attr_names.attr,
>  	NULL,
>  };
>  
> 
> -- 

Re: gpiolib: add names file in gpio chip sysfs.

[Ben Dooks]

On Tue, Aug 04, 2009 at 04:43:39PM -0700, Andrew Morton wrote:
> On Mon, 03 Aug 2009 17:55:36 +0100
> ben@fluff.org wrote:
> >
> 
> Did you mean the commit to have an author of ben@fluff.org?  I assumed
> not and rewrote it to
> 
> 	Ben Dooks <ben@simtec.co.uk>
> 
> If you indeed want a different Author: and Signed-off-by: line then
> please indicate that explicitly by putting a From: line at the top of
> the changelog.

sorry, occasionally whilst using quilt mail I get this wrong.
 
> As your MUA didn't fill in the real-name part of the From: address,
> it's nice to provide a From: line in the changelog so the patch
> receiver doesn't have to type it in.
> 
> > Add a 'names' file to the sysfs entries for each chip to show which
> > have names.
> 
> Why?

To export this information to the user. That was the only reason.

-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.

Re: gpiolib: add names file in gpio chip sysfs.

[Andrew Morton]

On Tue, 29 Sep 2009 11:47:35 +0100
Ben Dooks <ben@simtec.co.uk> wrote:

> On Tue, Aug 04, 2009 at 04:43:39PM -0700, Andrew Morton wrote:
> > On Mon, 03 Aug 2009 17:55:36 +0100
> > ben@fluff.org wrote:
> > >
> > 
> > Did you mean the commit to have an author of ben@fluff.org?  I assumed
> > not and rewrote it to
> > 
> > 	Ben Dooks <ben@simtec.co.uk>
> > 
> > If you indeed want a different Author: and Signed-off-by: line then
> > please indicate that explicitly by putting a From: line at the top of
> > the changelog.
> 
> sorry, occasionally whilst using quilt mail I get this wrong.
>  
> > As your MUA didn't fill in the real-name part of the From: address,
> > it's nice to provide a From: line in the changelog so the patch
> > receiver doesn't have to type it in.
> > 
> > > Add a 'names' file to the sysfs entries for each chip to show which
> > > have names.
> > 
> > Why?
> 
> To export this information to the user. That was the only reason.
> 

hm, OK.  I guess I just won't be understanding what value this patch
has.  Hopefully more gpio-inclined people do.

What's the status of this patch now?  I have a note that David had
issues with it - did they get resolved somehow?