On Tue, Nov 26, 2013 at 12:49 PM, Mika Westerberg
<[email protected]> wrote:
> On Tue, Nov 26, 2013 at 11:23:39AM +0100, Linus Walleij wrote:
>> +static unsigned int lp_irq_startup(struct irq_data *d)
>> +{
>> + struct lp_gpio *lg = irq_data_get_irq_chip_data(d);
>> +
>> + if (gpio_lock_as_irq(&lg->chip, irqd_to_hwirq(d)))
>> + dev_err(lg->chip.dev,
>> + "unable to lock HW IRQ %lu for IRQ\n",
>> + irqd_to_hwirq(d));
>> + lp_irq_enable(d);
>
> I may be missing something but doesn't this now end up calling
> lp_irq_enable() twice? First in ->irq_startup() and then later on in
> ->irq_enable().
kernel/irq/chip.c:
int irq_startup(struct irq_desc *desc, bool resend)
{
int ret = 0;
irq_state_clr_disabled(desc);
desc->depth = 0;
if (desc->irq_data.chip->irq_startup) {
ret = desc->irq_data.chip->irq_startup(&desc->irq_data);
irq_state_clr_masked(desc);
} else {
irq_enable(desc);
}
if (resend)
check_irq_resend(desc, desc->irq_data.irq);
return ret;
}
If this hook exists, calls irq_startup() on the chip.
Else, calls irq_enable() which looks like this:
void irq_enable(struct irq_desc *desc)
{
irq_state_clr_disabled(desc);
if (desc->irq_data.chip->irq_enable)
desc->irq_data.chip->irq_enable(&desc->irq_data);
else
desc->irq_data.chip->irq_unmask(&desc->irq_data);
irq_state_clr_masked(desc);
}
I.e. calls .enable() or .unmask().
So there is a strict semantic requirement that if you implement
startup() it should perform the same as .enable(), or .unmask()
depending on whether the former is implemented.
I think this patch is OK ... can you test it on the Lynxpoint?
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
