lkml.org 
[lkml]   [2009]   [Sep]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRE: [PATCH] gpiolib: introduce for_each_gpio_in_chip macro
Date
From
On Friday, September 18, 2009 5:03 PM, Jaya Kumar wrote:
> On Thu, Aug 6, 2009 at 11:31 AM, Ben Nizette <bn@niasdigital.com> wrote:
>> On Tue, 2009-08-04 at 20:48 -0400, H Hartley Sweeten wrote:
>>
>>> For the record. The reason I sent this is I'm trying to work out an
>>> extension to gpiolib that adds gpio_port_* access to the API.  Most
>>> of the gpiolib drivers already the necessary logic since the raw I/O
>>> is performed on the entire 'chip'.  The API just needs the extensions
>>> added to request/free the port, set the direction and get/set the value.
>>>
>>> Is this a worthwhile addition?
>>
>> Plenty of people seem to think so.  Personally I haven't seen a great
>> use case except "'coz I can", but if you've got one I'd love to hear.
>
> Yes, you're right that there has been no major demand for it. There
> are (luckily?) only a moderate number of devices that are using gpio
> as their parallel bus interface. I've been supporting the batch-gpio
> patchset below out-of-the-tree because it has come in handy with a few
> e-paper display controllers and LCD 8080-IO that I've been developing
> with.

With the abundant number of GPIO's available on many of the ARM chips it
seemed to me a "port" extension would be worthwhile. It would allow a
side-band bus from the chip to access low-speed devices like a character
LCD as an example.

>>
>> Have you seen http://lkml.org/lkml/2009/1/25/10 ?  Donno what ended up
>> happening to that patchset..
>>
>
> I didn't pursue it further and have maintained it out-of-tree. I felt
> that David had concerns about the API I implemented so it was unlikely
> to get merged and I didn't have the motivation to implement another.
> :-)

Hmm.. That patchset is a lot different than what I was thinking of. Your
patch allows a variable width to the number of gpio's in the "port". But
it also still gets/sets the "port" by individual bit accesses to the
gpio_chip. By doing this I don't see how you could get a performance
increase.

The extension I was working on just allowed accessing the native width
of the gpio chip. Most of the gpiolib drivers read/write the bits in a
native width, the individual gpio pin is masked in/out. My patch just
allows access to the raw data without masking anything.

Take the .get method in pca953x.c driver as an example.

static int pca953x_gpio_get_value(struct gpio_chip *gc, unsigned off)
{
struct pca953x_chip *chip;
uint16_t reg_val;
int ret;
chip = container_of(gc, struct pca953x_chip, gpio_chip);

ret = pca953x_read_reg(chip, PCA953X_INPUT, &reg_val);
if (ret < 0) {
/* NOTE: diagnostic already emitted; that's all we should
* do unless gpio_*_value_cansleep() calls become different
* from their nonsleeping siblings (and report faults).
*/
return 0;
}
return (reg_val & (1u << off)) ? 1 : 0;
}
The native width of the device is either 8 or 16 bits. To get a gpio value
all of the bits are read then the desired gpio is masked out.

My thought was to just add the following methods to struct gpio_chip:

int (*port_direction_input)(struct gpio_chip *chip);
unsigned int (*port_get)(struct gpio_chip *chip);
int (*port_direction_output)(struct gpio_chip *chip, unsigned int value);
void (*port_set)(struct gpio_chip *chip, unsigned int value);

I basically stopped working on this after Ben's comment and getting
no other feedback. If this appears useful I can look at it again.

> Thanks,
> jaya
>
> ps: I'm in Portland for the festival of linux conferences this week
> and would be happy to work on this/discuss alternate APIs if it is of
> interest.

Regards,
Hartley
--
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/

\
 
 \ /
  Last update: 2009-09-19 02:47    [from the cache]
©2003-2011 Jasper Spaans