[RFC] generic GPIO parameter API

[RFC] generic GPIO parameter API

[Guennadi Liakhovetski]

Hi,

as far as I understand, the current GPIO API only presents very basic GPIO 
functionality: direction and level reading and writing. Whereas many GPIO 
controllers have many further configurable parameters: pull-ups and 
pull-downs, drive strength, slew rate, etc. And it is desirable to be able 
to access those features too. Of course, we cannot extent the API with all 
these possible functions. Would a generic GPIO parameter handling API be 
desirable? Like

struct gpio_parameter {
	char *name;
	void *arg;
	int (*get)(struct gpio_chip *chip, void *arg, unsigned offset, 
			int value);
	int (*set)(struct gpio_chip *chip, void *arg, unsigned offset, 
			int value);
};

int gpio_register_parameter(struct gpio_chip *chip, struct gpio_parameter 
				*param);
struct gpio_parameter *gpio_find_parameter(struct gpio_chip *chip, char 
						*name);

The parameters should be accessible from the kernel and over sysfs, based 
on the gpio-sysfs interface. Would this be useful?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

Re: [RFC] generic GPIO parameter API

[Guennadi Liakhovetski]

On Mon, 2 Jun 2008, Guennadi Liakhovetski wrote:

> struct gpio_parameter {
> 	char *name;
> 	void *arg;
> 	int (*get)(struct gpio_chip *chip, void *arg, unsigned offset, 
> 			int value);
> 	int (*set)(struct gpio_chip *chip, void *arg, unsigned offset, 
> 			int value);
> };

The (*get) should have "int *value" of course

> int gpio_register_parameter(struct gpio_chip *chip, struct gpio_parameter 
> 				*param);
> struct gpio_parameter *gpio_find_parameter(struct gpio_chip *chip, char 
> 						*name);

Actually, I think, it would be even better to just add two fields

	struct gpio_parameter	*param;
	int			param_n;

to struct gpio_chip.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

Re: [RFC] generic GPIO parameter API

[Ben Nizette]

On Mon, 2008-06-02 at 19:54 +0200, Guennadi Liakhovetski wrote:
> On Mon, 2 Jun 2008, Guennadi Liakhovetski wrote:
> > int gpio_register_parameter(struct gpio_chip *chip, struct gpio_parameter 
> > 				*param);
> > struct gpio_parameter *gpio_find_parameter(struct gpio_chip *chip, char 
> > 						*name);
> 
> Actually, I think, it would be even better to just add two fields
> 
> 	struct gpio_parameter	*param;
> 	int			param_n;
> 
> to struct gpio_chip.
> 

I like the idea in general.  The biggest worry I have is trying to find
the parameter for you to fiddle with.  The driver which is going to want
to set the parameters is going to have the gpio number, not the
gpio_chip.  Also, the fact that the parameters are uniquely identified
by strings is a bit awkward.  I can see people registering the same kind
of parameter for different chips like "pullup", "Pullup", "pu" etc
making the driver's task even harder.

So, I reckon if we're to do this we should stick with the current style
of gpio calls for the outside interface, maybe something more like

int gpio_set_param(int gpio, int param, int val);
int gpio_get_param(int gpio, int param);

with the different parameters defined as an enum in some gpio.h
somewhere.  Where to keep the gpio_parameters and how to search/find
them should be up to the implementation (though the gpiolib
implementation would probably look quite like what you've got above).

Note you'll probably want a char *name in there somewhere for the sysfs
interface, but I don't think it should be the primary mechanism for
identification.

Anyway, that's my $0.02  :-)

	--Ben.

Re: [RFC] generic GPIO parameter API

[Guennadi Liakhovetski]

On Tue, 3 Jun 2008, Ben Nizette wrote:

> On Mon, 2008-06-02 at 19:54 +0200, Guennadi Liakhovetski wrote:
> > On Mon, 2 Jun 2008, Guennadi Liakhovetski wrote:
> > > int gpio_register_parameter(struct gpio_chip *chip, struct gpio_parameter 
> > > 				*param);
> > > struct gpio_parameter *gpio_find_parameter(struct gpio_chip *chip, char 
> > > 						*name);
> > 
> > Actually, I think, it would be even better to just add two fields
> > 
> > 	struct gpio_parameter	*param;
> > 	int			param_n;
> > 
> > to struct gpio_chip.
> 
> I like the idea in general.  The biggest worry I have is trying to find
> the parameter for you to fiddle with.

Oh, this doesn't worry me - I have a driver here for a controller with 
switchable pullups.

> The driver which is going to want
> to set the parameters is going to have the gpio number, not the
> gpio_chip.

Sure, right.

> Also, the fact that the parameters are uniquely identified
> by strings is a bit awkward.  I can see people registering the same kind
> of parameter for different chips like "pullup", "Pullup", "pu" etc
> making the driver's task even harder.

Well, I thought about that too, but then I decided there would have to be 
too many of those macros. But we can try it that way too.

> So, I reckon if we're to do this we should stick with the current style
> of gpio calls for the outside interface, maybe something more like
> 
> int gpio_set_param(int gpio, int param, int val);
> int gpio_get_param(int gpio, int param);

For the get I would rather pass it "int *val" because we don't know which 
values are valid and which are an error code for this specific parameter.

> with the different parameters defined as an enum in some gpio.h
> somewhere.  Where to keep the gpio_parameters and how to search/find
> them should be up to the implementation (though the gpiolib
> implementation would probably look quite like what you've got above).
> 
> Note you'll probably want a char *name in there somewhere for the sysfs
> interface, but I don't think it should be the primary mechanism for
> identification.
> 
> Anyway, that's my $0.02  :-)

Thanks for the comments
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

Re: [RFC] generic GPIO parameter API

[Ben Nizette]

On Tue, 2008-06-03 at 08:42 +0200, Guennadi Liakhovetski wrote:
> On Tue, 3 Jun 2008, Ben Nizette wrote:
> 
> > On Mon, 2008-06-02 at 19:54 +0200, Guennadi Liakhovetski wrote:
> > > On Mon, 2 Jun 2008, Guennadi Liakhovetski wrote:
> > > > int gpio_register_parameter(struct gpio_chip *chip, struct gpio_parameter 
> > > > 				*param);
> > > > struct gpio_parameter *gpio_find_parameter(struct gpio_chip *chip, char 
> > > > 						*name);
> > > 
> > > Actually, I think, it would be even better to just add two fields
> > > 
> > > 	struct gpio_parameter	*param;
> > > 	int			param_n;
> > > 
> > > to struct gpio_chip.
> > 
> > I like the idea in general.  The biggest worry I have is trying to find
> > the parameter for you to fiddle with.
> 
> Oh, this doesn't worry me - I have a driver here for a controller with 
> switchable pullups.

You're talking about a gpio chip driver?  How does the end user go about
turning the pullups on and off?  How does the end user know that that's
what they want to do?

> 
> > The driver which is going to want
> > to set the parameters is going to have the gpio number, not the
> > gpio_chip.
> 
> Sure, right.
> 
> > Also, the fact that the parameters are uniquely identified
> > by strings is a bit awkward.  I can see people registering the same kind
> > of parameter for different chips like "pullup", "Pullup", "pu" etc
> > making the driver's task even harder.
> 
> Well, I thought about that too, but then I decided there would have to be 
> too many of those macros. But we can try it that way too.
> 
> > So, I reckon if we're to do this we should stick with the current style
> > of gpio calls for the outside interface, maybe something more like
> > 
> > int gpio_set_param(int gpio, int param, int val);
> > int gpio_get_param(int gpio, int param);
> 
> For the get I would rather pass it "int *val" because we don't know which 
> values are valid and which are an error code for this specific parameter.

Well everything else in the world just uses negative returns for errors,
I'm sure that any parameter get/set routines can conform with that, no?
This way is more consistent with, gpio_{get,set}_val etc not to mention
the rest of the kernel.

	--Ben.

Re: [RFC] generic GPIO parameter API

[Guennadi Liakhovetski]

On Tue, 3 Jun 2008, Ben Nizette wrote:

> 
> On Tue, 2008-06-03 at 08:42 +0200, Guennadi Liakhovetski wrote:
> > On Tue, 3 Jun 2008, Ben Nizette wrote:
> > 
> > > On Mon, 2008-06-02 at 19:54 +0200, Guennadi Liakhovetski wrote:
> > > > On Mon, 2 Jun 2008, Guennadi Liakhovetski wrote:
> > > > > int gpio_register_parameter(struct gpio_chip *chip, struct gpio_parameter 
> > > > > 				*param);
> > > > > struct gpio_parameter *gpio_find_parameter(struct gpio_chip *chip, char 
> > > > > 						*name);
> > > > 
> > > > Actually, I think, it would be even better to just add two fields
> > > > 
> > > > 	struct gpio_parameter	*param;
> > > > 	int			param_n;
> > > > 
> > > > to struct gpio_chip.
> > > 
> > > I like the idea in general.  The biggest worry I have is trying to find
> > > the parameter for you to fiddle with.
> > 
> > Oh, this doesn't worry me - I have a driver here for a controller with 
> > switchable pullups.
> 
> You're talking about a gpio chip driver?

Yes.

> How does the end user go about turning the pullups on and off?

Either using the in-kernel API or over sysfs, if it's a user-space app.

> How does the end user know that that's what they want to do?

That's their problem, isn't it? We are talking about an embedded system, 
where applications are written with datasheets and schematics in hand. So, 
you will know whether or not you want to switch pullups.

> > > So, I reckon if we're to do this we should stick with the current style
> > > of gpio calls for the outside interface, maybe something more like
> > > 
> > > int gpio_set_param(int gpio, int param, int val);
> > > int gpio_get_param(int gpio, int param);
> > 
> > For the get I would rather pass it "int *val" because we don't know which 
> > values are valid and which are an error code for this specific parameter.
> 
> Well everything else in the world just uses negative returns for errors,
> I'm sure that any parameter get/set routines can conform with that, no?
> This way is more consistent with, gpio_{get,set}_val etc not to mention
> the rest of the kernel.

gpio_get_val() is easy - you can only get a 0 or a 1 in success case 
there. Whereas with an arbitrary gpio parameter you don't know what valid 
values it can return. Ok, practically, I can hardly imagine a GPIO 
parameter with 2^32 valid values, but who knows...

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

Re: [RFC] generic GPIO parameter API

[Ben Nizette]

On Tue, 2008-06-03 at 10:29 +0200, Guennadi Liakhovetski wrote:
> On Tue, 3 Jun 2008, Ben Nizette wrote:
> > On Tue, 2008-06-03 at 08:42 +0200, Guennadi Liakhovetski wrote:
> > > On Tue, 3 Jun 2008, Ben Nizette wrote:
> > > > 
> > > > I like the idea in general.  The biggest worry I have is trying to find
> > > > the parameter for you to fiddle with.
> > > 
> > > Oh, this doesn't worry me - I have a driver here for a controller with 
> > > switchable pullups.
> > 
> > You're talking about a gpio chip driver?
> 
> Yes.
> 
> > How does the end user go about turning the pullups on and off?
> 
> Either using the in-kernel API 

I guess it's this bit I was wondering about more precisely.  _Which_ in
kernel API?  The gpio_find_parameter thing you had above?  If so, how
are you discovering the gpio_chip?  (Note that if you are indeed
discovering the gpio_chip, this isn't portable. gpio chips shouldn't be
known outside of gpiolib, gpiolib's optional and separate from the gpio
framework).

> or over sysfs, if it's a user-space app.
> 
> > How does the end user know that that's what they want to do?
> 
> That's their problem, isn't it? We are talking about an embedded system, 
> where applications are written with datasheets and schematics in hand. So, 
> you will know whether or not you want to switch pullups.
> 
> > > > So, I reckon if we're to do this we should stick with the current style
> > > > of gpio calls for the outside interface, maybe something more like
> > > > 
> > > > int gpio_set_param(int gpio, int param, int val);
> > > > int gpio_get_param(int gpio, int param);
> > > 
> > > For the get I would rather pass it "int *val" because we don't know which 
> > > values are valid and which are an error code for this specific parameter.
> > 
> > Well everything else in the world just uses negative returns for errors,
> > I'm sure that any parameter get/set routines can conform with that, no?
> > This way is more consistent with, gpio_{get,set}_val etc not to mention
> > the rest of the kernel.
> 
> gpio_get_val() is easy - you can only get a 0 or a 1 in success case 
> there. Whereas with an arbitrary gpio parameter you don't know what valid 
> values it can return. Ok, practically, I can hardly imagine a GPIO 
> parameter with 2^32 valid values, but who knows...
> 

Hmm, in the absence of a solid use case I'm a fan of sticking with
tradition.  I can just see people forgetting to put an &foo in get but
just foo in set (I know I would).  But so long as it's solidly
documented then I guess I wouldn't be able to complain :-)

Just so long as we agree that there should be this kind of interface in
the gpio framework, quite apart from how it's implemented inside
gpiolib.

Cheers,
	--Ben.

> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer

Re: [RFC] generic GPIO parameter API

[David Brownell]

On Monday 02 June 2008, Guennadi Liakhovetski wrote:
> Hi,
> 
> as far as I understand, the current GPIO API only presents very basic GPIO 
> functionality: direction and level reading and writing. Whereas many GPIO 
> controllers have many further configurable parameters: pull-ups and 
> pull-downs, drive strength, slew rate, etc.

Not at all how I'd describe it.  Those omitted mechanisms are part
of pin configuration, in the same way as function multiplexing is.
(That is, assigning a given pin for use as a GPIO, vs hooking it up
to an I2C, MMC, SPI, LCD, I2S, or memory controller.)

Those mechanisms are highly chip-specific.  Far more so than simple
boolean GPIO mechanisms.  With rare exceptions, they're needed only
in platform-specific board setup code, which is already accessing
lots of platform-specific programming interfaces.  I'm really not
keen on having them become part of the GPIO framework.  The thought
makes me think of trying to swallow a kitchen sink; sorry ...



> And it is desirable to be able  
> to access those features too. Of course, we cannot extent the API with all 
> these possible functions. Would a generic GPIO parameter handling API be 
> desirable?

Were a general interface for this needed, I'd call it a "pin config"
interface (or something similar).

Some platforms would have simple mappings of GPIOs to pins; others
don't, with a GPIO appearing on multiple pins.  (Potentially with
different drive options, etc.)  Such a mapping would make this API
seem quite simple, where it's practical.

A reverse mapping would (pin to GPIO) would likewise not always
be simple, when multiple GPIOs could be routed to a given pin.
It'd be easy to handle the case of a pin that has no GPIO function;
just return an errno instead of a GPIO number.

Of course such a programming interface wouldn't be usable on all
systems ... ones where the configurations are ganged don't have
per-pin parameters like this.  (Examples include cases where one
bit reconfigures several pins, or several parameters of one pin.)


> Like 
> 
> struct gpio_parameter {
> 	char *name;
> 	void *arg;
> 	int (*get)(struct gpio_chip *chip, void *arg, unsigned offset, 
> 			int value);
> 	int (*set)(struct gpio_chip *chip, void *arg, unsigned offset, 
> 			int value);
> };
> 
> int gpio_register_parameter(struct gpio_chip *chip, struct gpio_parameter 
> 				*param);
> struct gpio_parameter *gpio_find_parameter(struct gpio_chip *chip, char 
> 						*name);
> 
> The parameters should be accessible from the kernel and over sysfs, based 
> on the gpio-sysfs interface. Would this be useful?

I don't have any use cases for such an interface myself.

That's the sort of thing I'd expect to do in configfs, if I
needed a userspace solution.

Have you looked at the existing solutions for such stuff?  IMO
the platform code solves this well enough.  Of interest, the
model it uses isn't "pin and attributes", in most cases.  (Or
at least, not directly.)  Examples that come quickly to mind:

  arch/arm/plat-omap/mux.c
  arch/arm/mach-omap1/mux.c
  arch/arm/mach-omap2/mux.c
  include/asm-arm/arch-omap/mux.h
	OMAP2/OMAP3 has a much cleaner hardware design than OMAP1.
	In both cases there's a plethora of pin/ball config options at
	several levels ... family (omap15xx, omap16xx, omap7xx, etc)
	chip (1611, 2430, etc), revision (es2.0, es2.1, etc) etc.

	A conclusion here is that such configuration is rarely used
	outside board setup code.  (And such usage is being removed.)

  arch/arm/mach-at91/gpio.c
  include/asm-arm/arch-at91/gpio.h ... the at91_set_*() calls
  arch/avr32/mach-at32ap/pio.c
  include/asm-avr32/arch-at32ap/portmux.h ... at32_select_*()
	Essentially the same very simple hardware design, on two
	different SOC frameworks.  Again, such configuration is
	rarely used outside board setup (maybe never).

	These don't use symbolic pin config identifiers, though
	some similar code (in u-Boot or some other pre-Linux code)
	uses them to specify pin configurations that affect several
	characteristics (somewhat like the new MFP stuff on PXA).

  arch/arm/mach-pxa/mfp*c
  include/asm-arm/arch-pxa/mfp*h
	Confusing terminology (never talks "pin", always "GPIO"
	even for non-GPIO functions) and what I think of as a
	kind of quirky hardware model ... and this MFP stuff is
	pretty new, I've not seen pxa3xx docs to explain it.
	But again, this is mostly for board setup.

  arch/arm/mach-davinci/mux.c
  include/asm-arm/arch-davinici/mux.h
	Not clear from the source code, but these are all
	ganged config options.  See the chip docs.

It's worth noting that FPGA pin configurations are fancier than
any of these, in all cases I've had occasion to look at, but
such configuration is done when the configuration bitstream is
generated (from Verilog, VHDL, or whatever) not at runtime.

- Dave

Re: [RFC] generic GPIO parameter API

[Guennadi Liakhovetski]

Hi David,

On Thu, 5 Jun 2008, David Brownell wrote:

> On Monday 02 June 2008, Guennadi Liakhovetski wrote:
> > Hi,
> > 
> > as far as I understand, the current GPIO API only presents very basic GPIO 
> > functionality: direction and level reading and writing. Whereas many GPIO 
> > controllers have many further configurable parameters: pull-ups and 
> > pull-downs, drive strength, slew rate, etc.
> 
> Not at all how I'd describe it.  Those omitted mechanisms are part
> of pin configuration, in the same way as function multiplexing is.
> (That is, assigning a given pin for use as a GPIO, vs hooking it up
> to an I2C, MMC, SPI, LCD, I2S, or memory controller.)
> 
> Those mechanisms are highly chip-specific.  Far more so than simple
> boolean GPIO mechanisms.  With rare exceptions, they're needed only
> in platform-specific board setup code, which is already accessing
> lots of platform-specific programming interfaces.  I'm really not
> keen on having them become part of the GPIO framework.  The thought
> makes me think of trying to swallow a kitchen sink; sorry ...

Thanks for the lengthy explanation with numerous examples - this is 
appreciated. But I'll omit that for this reply. Just a short question so 
far - how would you handle an external GPIO expander with an additional 
functionality, naimly, a possibility to switch pull-ups to GPIO pins, 
specifically, it defines 4 modes for a pin: out high, out low, in without 
a pullup and in with a pullup. And if the user application wants to decide 
at run-time which pull-ups to switch and when? It's a MAX7301 from Maxim 
btw.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

Re: [RFC] generic GPIO parameter API

[Guennadi Liakhovetski]

On Thu, 5 Jun 2008, David Brownell wrote:

> On Monday 02 June 2008, Guennadi Liakhovetski wrote:
> > Hi,
> > 
> > as far as I understand, the current GPIO API only presents very basic GPIO 
> > functionality: direction and level reading and writing. Whereas many GPIO 
> > controllers have many further configurable parameters: pull-ups and 
> > pull-downs, drive strength, slew rate, etc.
> 
> Not at all how I'd describe it.  Those omitted mechanisms are part
> of pin configuration, in the same way as function multiplexing is.
> (That is, assigning a given pin for use as a GPIO, vs hooking it up
> to an I2C, MMC, SPI, LCD, I2S, or memory controller.)

Yes, on the one hand you're right, this belongs to pin-configuration. But, 
otoh, will anyone ever want to change these parameters on non-generic 
pins? And should this be allowed? Whereas for GPIOs it clearly makes a 
sense - on a test-board you have a header with free-hanging GPIOs. How do 
you want to configure them? This depends on what the user connects to them 
and you don't necessarily want to modify your kernel to reconfigure them.

As for various difficulties - they should be solvable, I think. For 
example, for GPIO pins, that can only be configured in groups - you just 
first verify that the other GPIOs in this group are either free or 
configured compatibly, or you fail. Of course these functions are 
hardware-specific, so we let every specific driver handle them.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

Re: [RFC] generic GPIO parameter API

[Mark Brown]

On Mon, Jun 09, 2008 at 06:23:50PM +0200, Guennadi Liakhovetski wrote:
> On Thu, 5 Jun 2008, David Brownell wrote:
> > On Monday 02 June 2008, Guennadi Liakhovetski wrote:

> > > as far as I understand, the current GPIO API only presents very basic GPIO 
> > > functionality: direction and level reading and writing. Whereas many GPIO 
> > > controllers have many further configurable parameters: pull-ups and 
> > > pull-downs, drive strength, slew rate, etc.

> > Not at all how I'd describe it.  Those omitted mechanisms are part
> > of pin configuration, in the same way as function multiplexing is.
> > (That is, assigning a given pin for use as a GPIO, vs hooking it up
> > to an I2C, MMC, SPI, LCD, I2S, or memory controller.)

> Yes, on the one hand you're right, this belongs to pin-configuration. But, 
> otoh, will anyone ever want to change these parameters on non-generic 
> pins? And should this be allowed? Whereas for GPIOs it clearly makes a 

They do get configured like that - for example, most I2S devices are
able to operate as both clock masters and clock slaves.  Often this
accomplished (at least in part) by configuring the relevant pins as
inputs or outputs.

Re: [RFC] generic GPIO parameter API

[Guennadi Liakhovetski]

On Mon, 9 Jun 2008, Mark Brown wrote:

> On Mon, Jun 09, 2008 at 06:23:50PM +0200, Guennadi Liakhovetski wrote:
> > On Thu, 5 Jun 2008, David Brownell wrote:
> > > On Monday 02 June 2008, Guennadi Liakhovetski wrote:
> 
> > > > as far as I understand, the current GPIO API only presents very basic GPIO 
> > > > functionality: direction and level reading and writing. Whereas many GPIO 
> > > > controllers have many further configurable parameters: pull-ups and 
> > > > pull-downs, drive strength, slew rate, etc.
> 
> > > Not at all how I'd describe it.  Those omitted mechanisms are part
> > > of pin configuration, in the same way as function multiplexing is.
> > > (That is, assigning a given pin for use as a GPIO, vs hooking it up
> > > to an I2C, MMC, SPI, LCD, I2S, or memory controller.)
> 
> > Yes, on the one hand you're right, this belongs to pin-configuration. But, 
> > otoh, will anyone ever want to change these parameters on non-generic 
> > pins? And should this be allowed? Whereas for GPIOs it clearly makes a 
> 
> They do get configured like that - for example, most I2S devices are
> able to operate as both clock masters and clock slaves.  Often this
> accomplished (at least in part) by configuring the relevant pins as
> inputs or outputs.

The primary purpose of the GPIO-parameter proposal was to allow to 
configure these parameters from the user-space over sysfs by extending the 
gpio-sysfs patch. And I2S master / slave operation should not be 
switchable over sysfs, should it?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer

Re: [RFC] generic GPIO parameter API

[Mark Brown]

On Mon, Jun 09, 2008 at 07:09:00PM +0200, Guennadi Liakhovetski wrote:

> The primary purpose of the GPIO-parameter proposal was to allow to 
> configure these parameters from the user-space over sysfs by extending the 
> gpio-sysfs patch. And I2S master / slave operation should not be 
> switchable over sysfs, should it?

Not via that API, anyway (apart from anything else both ends of the link
need to agree) - I was more mentioning it as an example of a non-GPIO
function that could use the GPIO configuration.