Message-ID: <20080528112109.47747cb5@hskinnemo-gx745.norway.atmel.com>
Date:	Wed, 28 May 2008 11:21:09 +0200
From:	Haavard Skinnemoen <haavard.skinnemoen@...el.com>
To:	David Brownell <david-b@...bell.net>
Cc:	Ben Nizette <bn@...sdigital.com>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	kernel@...32linux.org
Subject: Re: Latest gpio gumph

David Brownell <david-b@...bell.net> wrote:
> On Wednesday 28 May 2008, Haavard Skinnemoen wrote:
> > > 
> > >  * gpio_direction_output() should disable the pullups just like
> > >    at32_select_gpio(... AT32_GPIOF_OUTPUT) does, for consistency
> > >    between those alternative initialization paths.
> > 
> > But then we need to keep track of whether pullups used to be enabled so
> > that we can re-enable it in gpio_direction_input(), don't we?
> 
> "Need"?  I'd figure that changing direction like that would be
> uncommon without something determining signal level (like an
> external driver or pullup) ... and if nothing did so, then it'd
> be important to use the AVR32-private API with pullup control.

If you enable the internal pullup during port configuration, it should
stay that way, I think. But I think at32_select_gpio() should be fixed
so that when the user specifies AT32_GPIOF_OUTPUT | AT32_GPIOF_PULLUP,
the pullup will be turned on.

> > I can't see the harm of keeping the pullup enabled while the port is
> > configured as output. For consistency, I'd rather honor the pullup flag
> > in at32_select_gpio() regardless of AT32_GPIOF_OUTPUT.
> 
> I guess I don't like the idea of facilitating the constant current
> waste that implies if output is being driven low.  Even if it's not
> a huge current waste!  (These pullups being a lot weaker than I'd
> have expected, at typically 190 kOhm.)  No big deal here I guess.

I don't think we're talking about a lot of pins that need to switch
direction on the fly, and the pullup is very weak as you say. And a
floating input might waste a lot more power than the pullup ever will.

> For an open drain output it's probably less of an issue, unless
> there are too many pullups.

The board designer should know this and set the AT32_GPIOF_PULLUP flag
as appropriate.

> > >  * On the odd chance some code uses a pin as a GPIO IRQ without
> > >    calling gpio_request() or gpio_direction_input(), the debug
> > >    dump should still show its pin status.
> > 
> > Hmm. I guess that makes sense, though I would be lying if I said I care
> > all that much. I think gpiolib is going pretty far to accommodate buggy
> > drivers that don't call gpio_request() as it is.
> 
> For diagnostic/debug code, I'd say it's reasonably useful to cope
> with buglets like that. 
> 
> I actually observed that happening.  Setup code was passing the irq
> to the driver, and everything worked fine because the reset default
> was fine.  I happened to notice that /sys/kernel/debug/gpio output
> didn't match up to /proc/interrupts (bug) ... but it would have been
> much faster to see the bug if the listing for that pin had a "?" label
> showing that it hadn't been requested.

Yes, but it will only catch that particular case, not missing
gpio_request() calls in general.

I'm not really opposed to the second change; I would have applied it if
it came separately. But I think the first change is wrong.

Haavard
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists