Re: [PATCHv3 0/7] gpio: extend basic_mmio_gpio for different controllers

[Jamie Iles <jamie@jamieiles.com>]

On Wed, May 04, 2011 at 01:52:11AM +0400, Anton Vorontsov wrote:
> On Tue, May 03, 2011 at 03:13:20PM -0600, Grant Likely wrote:
> > > struct gpio_mmio_generic {
> > >        spinlock_t      lock;
> > >
> > >        /* initialized by user */
> > >        unsigned long   reg_data;
> > >        unsigned long   reg_set;
> > >        unsigned long   reg_clr;
> > >        unsigned long   reg_dir;
> > >
> > >        void __iomem    *reg_base;
> 
> This assumes that all reg_* will be mapped with a single ioremap().
> My solution (see below) doesn't have this issue.
> 
> > > void gpio_mmio_generic_setup(struct gpio_mmio_generic *gmg, int register_width);
> > > int gpio_mmio_generic_add(struct gpio_mmio_generic *gmg);
> > > void gpio_mmio_generic_remove(struct gpio_mmio_generic *gmg);
> 
> I'm not sure you need that complex API.
> 
> > > gpio_mmio_generic_setup() sets up the common callback ops in the
> > > embedded gpio_chip, but the decisions it makes could be overridden by
> > > the user before calling gpio_mmio_generic_add().
> > >
> > > I've not had time to prototype this yet, but I wanted to get it
> > > written down and out onto the list for feedback since I probably won't
> > > have any chance to get to it until after UDS.  Bonus points to anyone
> > > who wants to take the initiative to hack it together before I get to
> > > it.  Extra bonus points to anyone who also converts some of the other
> > > gpio controllers to use it.  :-D
> > 
> > And triple bonus points to anyone who thinks this is stupid and comes
> > up with a better approach.  :-)
> 
> This isn't stupid. And I started working on this, so what about
> http://lkml.org/lkml/2011/4/19/401 ? This is pretty much the same
> that you propose.

The advantage that Grant's proposal has though is that the user can 
override the gpio_chip callbacks.  When I tried porting over some 
existing ARM platforms, one of the blocking issues was that lots of 
platforms had some annoying small detail that was slightly different 
(such as doing muxing in the _get() callback or needing a to_irq 
callback).

If we make bgpio_chip public and return that from bgpio_probe 
unregistered then the calling code can override some of the methods then 
register the gpio_chip.

As a slight aside, if we don't want a platform_device per bank for 
devices with multiple banks then I don't think the named resource 
approach will work (at least I can't see a particularly nice mechanism).  
Any ideas?

Jamie