[U-Boot] [PATCH] gpio: add driver for PCA9539 16-bit I2C gpio expander

[Chris Packham]

On Tue, Dec 7, 2010 at 1:13 PM, Peter Tyser <ptyser at xes-inc.com> wrote:
> Hi Chris,
>
> On Tue, 2010-12-07 at 10:48 +1300, Chris Packham wrote:
>> This adds support for the PCA9539 family of gpio devices which have 16
>> output pins. The devices are similar to chips that use the pca953x driver
>> except the register map is expanded (in a non-backwards compatible manner)
>> to allow for the extra 8 pins.
>>
>> This driver has one gotcha that you can only set the top or bottom 8 bits
>> at a time. For example to set the 16 pins to be outputs you need to make
>> the function calls
>>
>>   pca9539_set_dir(PCA9539_ADDR, 0xff00, 0x0000);
>>   pca9539_set_dir(PCA9539_ADDR, 0x00ff, 0x0000);
>>
>> Signed-off-by: Chris Packham <chris.packham at alliedtelesis.co.nz>
>> ---
>> Hi,
>>
>> I have a couple of questions related to my patch. Firstly is anyone
>> interested in
>> support for these gpio devices? Secondly is my gotcha acceptable or should I
>> put some effort into making it handle a 16-bit argument in 1 hit?
>>
>> I toyed with the idea of making this part of the pca953x driver. I
>> didn't because
>> the register layout is different so I'd still need to define PCA9539 specific
>> functions and macros (although pca953x_reg_write could be re-used). I also
>> figured that some people may want to define only CONFIG_PCA9539 (that is
>> the case for me at the moment but the overhead of CONFIG_PCA953X wouldn't
>> push the size of my image over any boundary).
>
> I think it'd be nice to add support for 16-bit pca953x devices.  I don't
> personally use them, but others might in the future, or they may choose
> to use them since they are well supported in U-Boot and Linux.  If you
> already use the pca9539, getting it in mainline U-Boot will be nice for
> you in the long run either way.
>
> I think it'd be worth attempting to combine your changes in the existing
> pca953x driver though.  They are similar enough that it should be doable
> without too much headache, and it'll be more maintainable to have a
> unified driver.

I'll give it a try and see what the code looks like (see below).

> Linux combines the 2 for what its worth (see drivers/gpio/pca953x.c).
> It uses the number of GPIO pins to conditionally support 8 or 16-bit
> expanders.

Thanks I knew Linux supported it I just hadn't checked how. I hadn't
considered using word read/writes in my naive poking around with i2c
md I managed to convince myself that it wouldn't work, I'll check
again.

> You could do the same thing to the U-Boot pca953x driver.
> Eg at the top you could add:
> #ifdef CONFIG_PCA953X_16BIT
> #define NGPIO = 16
> #else
> #define NGPIO = 8
> #endif

I have a small problem with this due to the fact that we have some
designs here that use a pca9534 and a pca9539. Having a compile-time
conditional like this means you can only support one or the other.
Linux can handle this because it's actually got a structure that
contains the 8 vs 16 whereas u-boot only has the address an no other
information. Any suggestions on handling this would be welcome.

>
> You could also change the read/write parameters to be uint16's instead
> of the current uint8's to support 16-bits of GPIO.  I think with those 2
> changes, plus some additional conditionals you could combine the 2
> drivers without too much effort.
>
> Best,
> Peter
>
>