[PATCH] gpio: Add Altera GPIO driver

[Tobias Klauser]

On 2012-03-13 at 19:13:24 +0100, Grant Likely <grant.likely at secretlab.ca> wrote:
> On Tue, 13 Mar 2012 12:31:26 +0100, Tobias Klauser <tklauser at distanz.ch> wrote:
> > On 2012-03-12 at 17:04:18 +0100, Grant Likely <grant.likely at secretlab.ca> wrote:
> > > On Mon,  6 Feb 2012 18:59:02 +0100, Tobias Klauser <tklauser at distanz.ch> wrote:
> > > > This driver supports the Altera PIO core.
> > > > 
> > > > Signed-off-by: Thomas Chou <thomas at wytron.com.tw>
> > > > Signed-off-by: Tobias Klauser <tklauser at distanz.ch>
> > > > ---
> > > > This driver was submitted already about a year ago by Thomas, was
> > > > adjusted by him according to some remarks made by Grant and was then
> > > > included in the -mm tree [1] for some time but dropped again
> > > > (couldn't find out when and why exactly).
> > > > 
> > > > [0] http://article.gmane.org/gmane.linux.drivers.devicetree/4231
> > > > [1] http://article.gmane.org/gmane.linux.kernel.commits.mm/65119
> > > > 
> > > > So this is a new attempt towards getting this driver into mainline
> > > > from the nios2 tree. The following changes have been made atop the
> > > > version submitted by Thomas:
> > > > 
> > > > - Changed driver name to new naming convention
> > > > - Usage of of_property_read_u32 in the probe function
> > > 
> > > This looks like a pretty generic memory-mapped gpio chip.  It should use the
> > > gpio-generic infrastructure instead of open-coding all of the accessor hooks.
> > 
> > Thanks a lot for the hint.
> > 
> > I'm trying to figure out how this could work together with
> > of_mm_gpio_chip which we use in the driver. Both bgpio_chip and
> > of_mm_gpio_chip contain a struct gpio_chip, so I suppose we can only
> > "be" either one of them in our driver? What would be the prefered way to
> > do this?
> 
> Hmmm, you are correct.  However, of_mm_gpio_chip() is the only part of
> drivers/of/gpio.c that isn't usable by gpio-generic.c.  Focus on gpio-generic.

I'll do that, thanks.

> > Also as we need to register the driver from platform specific setup code
> > (in the out-of-mainline nios2 port only atm) very early on, we don't
> > have a struct platform_device or struct device available which we would
> > need to pass to bgpio_init. What's the best way to solve this?
> 
> There are 2 halves to gpio-generic.  The library of helper functions (get/set/etc.)
> and the basic-mmio-gpio device driver.  It /should/ be safe to call bgpio_init without
> a struct platform_device, but right now the code doesn't protect against it.  That is
> a bug that you can easily fix.
> 
> However, why are you initializing it very early in platform specific code?  That is
> very strongly discouraged.  GPIO controllers are just another device.

I couldn't really find out a reason why we do this, maybe it's just
because the driver was initially based on gpio-xilinx and it's what
they do there. I'll check with Thomas (the original author of
gpio-altera) about this.

> > And another issue I came across: gpio-generic only seems to support
> > 8/16/32/64 bit wide GPIO registers. The Altera GPIO controller can have
> > from 1 to 32 bits per register depending on the configuration in the
> > FPGA (though it will still always be a 32bit register). In bgpio_init
> > the size passed is checked for being a number of 2 and the number then
> > written to the bits member. What would be the best way to still
> > determine the "real" number of GPIOs available in the driver then?
> 
> The gpio-generic code is a work in progress; feel free to submit patches to get
> it to behave the way you need it to.  Ultimately, I want all simple mm gpio drivers
> to use gpio-generic.

Ok. I hope I can provide a good example with gpio-altera then :)

Thanks
Tobias