On Mon, 2008-06-02 at 19:54 +0200, Guennadi Liakhovetski wrote:
quoted text
> 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.


--
unsubscribe notice
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/