AMD Geode CS553X GPIO driver

[David Brownell]

On Monday 29 December 2008, Deepak Saxena wrote:
> On Dec 27 2008, at 09:42, David Brownell was caught saying:
> > On Saturday 27 December 2008, jordan at cosmicpenguin.net wrote:
> > > > On Saturday 27 December 2008, jordan at cosmicpenguin.net wrote:
> > > >> The
> > > >> generic GPIO infrastructure is missing a few of the more obscure GPIO
> > > >> features on the Geode that are used in the DCON code, but I know Deepak
> > > >> was planning to write code for those; until then we can use the "legacy'
> > > >> functions.
> > > >
> > > > Could you elaborate please?  If the issue is pin configuration,
> > > > then that *should* be a platform-specific function.
> > > 
> > > It is pin configuration - auxiliary functions, edge detection, event
> > > configuration, etc.  If they work better as platform-specific functions,
> > > then that is fine with me.  Deepak was looking into the whole mess - he
> > > probably has more insight into the matter then I do.
> > 
> > Auxiliary functions, a.k.a. "pin muxing", is highly platform-specific.
> > 
> > Likewise pullups/pulldowns, drive strength, output voltage, and drive
> > type (push/pull vs open-drain).  As a rule, that all gets done once as
> > part of board setup (because we can't trust boot loaders to be reliable
> > for much more than starting a kernel image) from __init code, and is
> > never touched again.
> 
> Ack. What I had been thinking about and start poking at a little was
> to add a generic API that would just directly call a platform level
> code:
> 
> gpio_set_alternate_function(int function_type);

That has *nothing* to do with GPIOs though; not even a GPIO in
the parameter list!  And all valid function_type parameters
are SOC-specific.  So IMO much better to have

	cs553x_mux(...)

Although I've seen recent patches

  http://linux.omap.com/pipermail/davinci-linux-open-source/2008-December/010003.html
  http://linux.omap.com/pipermail/davinci-linux-open-source/2008-December/010033.html

which almost suggest some reusable infrastructure.  With
both patches applied, the code in the first patch handles
pin mux (several modes, mostly not per-pin), irq mux, and
DMA event mux ... by just patching bitfields in registers.

Thing is, I know platforms where that won't work since
establishing pin functionality requires updating more than
one register.  So the most I can do is suggest that code
might be worth using for a clone-and-modify for cs553x.


> Where function_type would be a chipset/platform specific #define.  I 
> started writing some code for this but realized that it would just
> get overly complicated as an alternate function might touch GPIO
> lines that are handled by different gpio_chips (for example, on
> Geode we have a high and low bank and my original thought had been
> to have one gpio_chip for each one).

I skimmed the '5535 docs and suggest one gpio_chip for all
28 GPIOs is the way to go.


> One thing that might be useful to add is a gpio_claim() API of
> some sort so that low-level platform code or a driver can mark 
> a GPIO as being in use. Might help with early board enablement
> debug where someone accidently miss-configures something.

There's already gpiochip_reserve().  Although, I'd suggest
just always registering the '553x gpios starting at zero,
early enough that arch initcalls can use GPIOs, so that
issue can never arise.


> > Stuff like IRQ edge configuration is irq_chip stuff; it's common to
> > need a specialized irq_chip to work in conjunction with a gpio_chip.
> > Board setup can establish initial values, and most drivers will never
> > change them, but irq_chip.set_type() can still be provided.
> 
> Yep. The hard part AFAIK is that the minute we do that, we change the 
> way IRQ handling works for x86 as we don't currently use the normal 
> embedded of mapping irq_chips to different IRQ controllers on an SOC 
> or platform...  we're just assuming it all looks like a PC. Adding 
> support for this this is on my TODO  list...

I don't follow that.  I know there *are* irq_chip structs on x86.
So I can't quite see why it'd need to work differently ... just
init the irq_desc structs to point to the right irq_chip, chain
as needed.  Maybe I'd see if I needed to implement it.  :)

- Dave