2009-04-15 13:19:51 Missing hardware change on GPIO pin
Jean-Francois Argentino (FRANCE)
Message: 72705
Hello,
I've made a driver based on the "simple-gpio" one, but which add the interrupt fonctionality, you'll find it attached. I found strange that, when I configure a GPIO to generate an interrupt on a falling edge, it sometime don't see it, but if I read the level on the GPIO, it has changed (what I confirm by spying the GPIO pin with a scope). If somebody could point me where I make a mistake in the driver. If not, maybe I'm wrong when I think that the interrupt layer can't miss any hardware event?
More over, I can't use the "set_irq_type" function to change the trigger type for the GPIO since the GPIO layer says that the interrupt is already requested by the driver...
bfin_gpio.h
bfin_gpio_rev6243.c
TranslateQuoteReplyEditDelete
2009-04-16 03:08:07 Missing hardware change on GPIO pin
Michael Hennerich (GERMANY)
Message: 72724 What Linux kernel version or Blackfin release version are you using?
On Blackfin once a GPIO is configured for edge triggered interrupts,
reading the pins state doesn't return the current level.
We implemented a workaround for this in our SVN trunk.
Can you try to test your driver on trunk to see if it fixes your
problem?
-Michael
int bfin_gpio_get_value(unsigned gpio)
{
#ifdef CONFIG_BF54x
return (1 & (gpio_array[gpio_bank(gpio)]->data >
gpio_sub_n(gpio)));
#else
unsigned long flags;
if (unlikely(get_gpio_edge(gpio))) {
int ret;
local_irq_save_hw(flags);
set_gpio_edge(gpio, 0);
ret = get_gpio_data(gpio);
set_gpio_edge(gpio, 1);
local_irq_restore_hw(flags);
return ret;
} else
return get_gpio_data(gpio);
#endif
}
EXPORT_SYMBOL(bfin_gpio_get_value);
QuoteReplyEditDelete
2009-04-16 06:19:09 Re: Missing hardware change on GPIO pin
Jean-Francois Argentino (FRANCE)
Message: 72742
Sorry, it looks like all the problems I'm facing make me forgot how a good post must look. I'm using kernel and toolchain from the SVN trunk on a Bluetechnix CM-BF537E rev0.3.
I'm aware of the table 14-2 from the HRM, but as far as I understand it, only the POLAR is important since if a rising edge occurs, the level is high... And it looks like it works that way since in the driver I'm using (attached in my first post), I read the pin level with the function:
static inline void gpio_update_value (int minor) {
gpio_info[minor].event.value = (get_gpio_polar (minor) == 0U) ? gpio_get_value (minor) : ! gpio_get_value (minor);
}
And it looks like it works because, since if I set a timeout (I'm lucky the edges I'm waiting for are more or less periodic), when it expires, if I read the pin level this way I'm reading the true pin level.
TranslateQuoteReplyEditDelete
2009-04-16 08:11:24 Re: Missing hardware change on GPIO pin
Michael Hennerich (GERMANY)
Message: 72751
I took a short look at your code.
This looks strange - have a look at irqflags it is used with
spin_lock_irqsave and given as argument with request_irq!!!
if (irqflags) {
spin_lock_irqsave (&gpio->lock, irqflags);
ret = request_irq (gpio_to_irq (minor), gpio_irq_handler,
IRQF_DISABLED | irqflags, GPIO_NAME,
&gpio_info[minor]);
if (ret < 0) {
spin_unlock_irqrestore (&gpio->lock, irqflags);
You are right the interrupt subsystem shouldn't miss an interrupt.
You should use set_irq_type or give the IRQF_TRIGGER_xxx flag with
request_irq.
Otherwise the kernel doesn't know that it is going to be an edge
triggered interrupt.
There might be a bug if you try request_irq with the IRQF_TRIGGER Flag
and then in addition use set_irq_type.
-Michael
QuoteReplyEditDelete
2009-04-16 08:41:45 Re: Missing hardware change on GPIO pin
Michael Hennerich (GERMANY)
Message: 72752
>There might be a bug if you try request_irq with the IRQF_TRIGGER Flag
>and then in addition use set_irq_type.
I removed the check that might have bugged you.
Index: bfin_gpio.c
===================================================================
--- bfin_gpio.c (revision 6266)
+++ bfin_gpio.c (working copy)
@@ -1021,15 +1021,6 @@
local_irq_save_hw(flags);
- if (unlikely(reserved_gpio_irq_map[gpio_bank(gpio)] &
gpio_bit(gpio))) {
- if (system_state == SYSTEM_BOOTING)
- dump_stack();
- printk(KERN_ERR
- "bfin-gpio: GPIO %d is already reserved as
gpio-irq !\n",
- gpio);
- local_irq_restore_hw(flags);
- return -EBUSY;
- }
if (unlikely(reserved_peri_map[gpio_bank(gpio)] &
gpio_bit(gpio))) {
if (system_state == SYSTEM_BOOTING)
dump_stack();
QuoteReplyEditDelete
2009-04-16 09:14:25 Re: Missing hardware change on GPIO pin
Jean-Francois Argentino (FRANCE)
Message: 72755
> This looks strange - have a look at irqflags it is used with
> spin_lock_irqsave and given as argument with request_irq!!!
Oh my god!!! I really need some holidays, thank you very much to pointing me this one...
I'll test asap the correction you've commit to be able to use "set_irq_type" instead of freeing then requesting IRQ.
TranslateQuoteReplyEditDelete
2009-04-16 12:40:16 Re: Missing hardware change on GPIO pin
Jean-Francois Argentino (FRANCE)
Message: 72763
I've just run a test with the bug corrected and I still missing some edges...
TranslateQuoteReplyEditDelete
2009-04-24 13:17:21 Re: Missing hardware change on GPIO pin
Mike Frysinger (UNITED STATES)
Message: 73176
well, that isnt entirely true ... there is undocumented behavior on the BF537 where you can read the data value even while in interrupt mode. but the design guys say this happens by accident, so you cant really rely on it ;).
QuoteReplyEditDelete
2009-04-24 13:19:09 Re: Missing hardware change on GPIO pin
Mike Frysinger (UNITED STATES)
Message: 73177
ive seen reports that set_irq_type() is deprecated and we shouldnt be encouraging its usage -- use IRQF_XXX defines when requesting the irq instead
QuoteReplyEditDelete
2009-04-27 08:41:08 Re: Missing hardware change on GPIO pin
Jean-Francois Argentino (FRANCE)
Message: 73283
>>ive seen reports that set_irq_type() is deprecated and we shouldnt be encouraging its usage -- use IRQF_XXX defines when requesting the irq instead
Does it mean that if I want to change the trigger for an already registered IRQ, I have to release it first? Or is there any other way to make this 'on the fly'?
>>well, that isnt entirely true ... there is undocumented behavior on the BF537 where you can read the data value even while in interrupt mode. but the design guys say this happens by accident, so you cant really rely on it ;).
OK, that could be an explaination since I think that I always read the pin when the IRQ wake the driver, so if I take the IRQ in account only when the level I read is the good one...
TranslateQuoteReplyEditDelete
2009-04-27 09:01:24 Re: Missing hardware change on GPIO pin
Michael Hennerich (GERMANY)
Message: 73285 >>>ive seen reports that set_irq_type() is deprecated and we shouldnt be encouraging its usage -- use
>IRQF_XXX defines when requesting the irq instead
>Does it mean that if I want to change the trigger for an already registered IRQ, I have to release it
set_irq_type() is not depreciated. There was a bug (svn trunk) in the Blackfin arch portion of the kernel that prevented set_irq_type() to work properly once the irq was requested.
This bug was fixed.
>first? Or is there any other way to make this 'on the fly'?
You should be able to do it on the fly using set_irq_type()
>>>well, that isnt entirely true ... there is undocumented behavior on the BF537 where you can read
>the data value even while in interrupt mode. but the design guys say this happens by accident, so
>you cant really rely on it ;).
I guess this comment applies to read GPIO states when in peripheral mode.
GPIOs in interrupt mode can natively been read when the GPIO interrupt is setup for Level Sensitive Interrupts.
On trunk we have a software workaround that also allows you to read the current pinstate also when configured for Edge Sensitive Interrupts.
-Michael
>OK, that could be an explaination since I think that I always read the pin when the IRQ wake the
>driver, so if I take the IRQ in account only when the level I read is the good one...
QuoteReplyEditDelete
2009-04-27 12:52:48 Re: Missing hardware change on GPIO pin
Mike Frysinger (UNITED STATES)
Message: 73297
i mean doing things like request_irq(); set_irq_type(); is wrong. using set_irq_type() at some point in the future is a different story.
QuoteReplyEditDelete
2009-04-27 12:54:05 Re: Missing hardware change on GPIO pin
Mike Frysinger (UNITED STATES)
Message: 73298
yeah, i guess that doesnt apply here. i'm thinking of the case where a pin is used with some peripheral and you want to read the raw value of the pin without going back to gpio mode. has nothing to do with gpio irqs.