On Mon, 20 Nov 2006 13:47:08 -0800
David Brownell <[email protected]> wrote:
> On Thursday 16 November 2006 6:54 am, Haavard Skinnemoen wrote:
> > I'm still not sure about how to implement the
> > gpio_request()/gpio_free() calls, though.
>
> Simplest would be to manipulate a bitmask:
>
> gpio_request() --> return test_and_set_bit(...) ? -EBUSY : 0;
> gpio_free() --> clear_bit(...)
Yeah, I'm fine with this part.
> Or if you want to track the identifiers and provide a debugfs dump
> of the active GPIOs and their status, use an array of strings (and
> spinlock its updates) or nuls as "fat bits", instead of a bit array.
Might be useful. Maybe we should add a "struct device" parameter to
gpio_request() as well, to allow platforms to associate pins with the
devices using them through sysfs? Or perhaps just to generate an
appropriate name (otherwise, each driver would have to generate a
unique string by themselves, for each device it controls.)
> > Since they're not supposed to
> > do any port configuration, I think I might convert them to no-ops
> > and do the allocation (and warn about conflicts) when configuring
> > the port multiplexer.
>
> What they're supposed to do is report conflicts between two
> drivers which both think they own the GPIO ... as errors.
>
> That's different from pin mux setup, which for most embedded
> systems is just making sure the SOC is configured to match
> what Linux expects. The boot loader usualy does some of that,
> but it's probably not validated to do more than reliably boot
> an operating system ... so pin muxing can't realistically
> report anything as an error. At best, it's a suggestion to
> update the bootloader someday.
I think I understand now. I have to use separate bitmasks for GPIO and
port mux setup.
If gpio_request() could do port mux configuration, one bitmask would be
enough to trap all errors. But after reading the rest of this thread,
I think separating the gpio and portmux APIs is a good idea, so I'm not
going to try to do this. Although I might set a bit in both masks when
configuring a pin for peripheral I/O, just to indicate that it isn't
usable for gpio at all.
> > Since the pin will be configured for one particular usage,
> > having multiple drivers try to use it will cause problems and
> > handing it out to the first driver that comes along will not really
> > solve anything...
>
> No, but letting the second one report the fatal error is a big help.
> And heck, you've got reasonable chance the first driver will work,
> if the second doesn't interfere with it! (Or maybe it's the other
> way around. At least you'd have logged a fatal error message ...)
You're right.
> > Of course, if other arches want gpio_request()/gpio_free(), I'm all
> > for keeping them.
>
> I thought you were someone who _wanted_ this mechanism?
> Or were you instead thinking of a pin mux mechanism?
Yes and yes ;)
Which is of course the source of all this confusion. I just had to
realize that the final definition of gpio_request was I little bit
different than I originally expected. This means that gpio_request() is
no longer _essential_ on avr32 (which corresponds nicely with its
classification as "optional" in your api proposal) but very nice to
have.
> > +int gpio_request(unsigned int gpio, const char *label)
> > +{
> > + struct pio_device *pio;
> > + unsigned int pin;
> > +
> > + pio = gpio_to_pio(gpio);
> > + if (!pio)
> > + return -ENODEV;
> > +
> > + pin = gpio & 0x1f;
> > + if (test_and_set_bit(pin, &pio->alloc_mask))
> > + return -EBUSY;
> > +
> > + pio_writel(pio, PER, 1 << pin);
>
> Enabling the pin as a GPIO is a pin mux responsibility, not
> a "keep drivers from stepping all over each other" thing.
Agreed. I'll change this.
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(gpio_request);
> > +
> > +void gpio_free(unsigned int gpio)
> > +{
> > + struct pio_device *pio;
> > + unsigned int pin;
> > +
> > + pio = gpio_to_pio(gpio);
> > + BUG_ON(!pio);
>
> Don't use BUG_ON in new code ... in this case it'd be best to just
> warn and return. Notice by the way that this isn't an inverse of
> the gpio_request(), since you aren't taking the pin back from the
> GPIO controller.
Ok, I'll drop the BUG_ON(). There's really no way to take the pin back
from the GPIO controller, as "gpio" is just one of three states where
the other two assigns the pin to a specific peripheral. Leaving the pin
configured for gpio seemed the best solution.
But I'm going to remove the port configuration stuff anyway, so it
doesn't matter.
> > +#define GPIO_PIOA_BASE (0)
> > +#define GPIO_PIN_PA0 (GPIO_PIOA_BASE + 0)
> > +#define GPIO_PIN_PA1 (GPIO_PIOA_BASE + 1)
> > +#define GPIO_PIN_PA2 (GPIO_PIOA_BASE + 2)
> > ...
>
> Someone had the suggestion to reduce the number of #defines
> by doing someting like
>
> #define GPIO_PIN_PA(N) (GPIO_PIOA_BASE + (N))
> #define GPIO_PIN_PB(N) (GPIO_PIOB_BASE + (N))
> #define GPIO_PIN_PC(N) (GPIO_PIOC_BASE + (N))
> #define GPIO_PIN_PD(N) (GPIO_PIOD_BASE + (N))
>
> I kind of like that, even if it's not such a direct match to
> Atmel's documentation.
I kind of like it too. It's a direct enough match for me.
> Aren't you going to want to add AVR-specific extensions so that
> drivers (or more typically, board init code) can manage pullups
> and input de-glitching?
Yeah, I will definitely do that, but I thought that was supposed to be
part of the portmux api?
Haavard
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
