Messages in this thread | | | Date | Thu, 17 Aug 2017 22:42:13 +0800 | From | jeffy <> | Subject | Re: [V6,3/9] irqdomain: Don't set type when mapping an IRQ |
| |
Hi Marc,
Thanks for your reply.
On 08/17/2017 09:34 PM, Marc Zyngier wrote: > On 17/08/17 13:46, jeffy wrote: >> Hi guys, >> >> I hit an problem when testing a level triggered irq with: >> >> irq = platform_get_irq_byname(pdev, "wake"); >> ...<-- irq trigger type is correct >> irq_set_status_flags(irq, IRQ_NOAUTOEN); >> ...<-- irq trigger type become zero >> request_threaded_irq(irq, ...) >> >> the trigger type lost(irqd_get_trigger_type returns zero) after >> irq_set_status_flags. >> >> >> it looks like irq_set_status_flags would try to use >> irq_settings_is_level to get level trigger type, which would get zero >> since we removed set type here...could you help to check this, thanks :) > > Hmm. That's quite unexpected. Can you please try this? > > diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c > index a3cc37c0c85e..3675c6004f2a 100644 > --- a/kernel/irq/chip.c > +++ b/kernel/irq/chip.c > @@ -1000,7 +1000,7 @@ EXPORT_SYMBOL_GPL(irq_set_chip_and_handler_name); > > void irq_modify_status(unsigned int irq, unsigned long clr, unsigned long set) > { > - unsigned long flags; > + unsigned long flags, trigger, tmp; > struct irq_desc *desc = irq_get_desc_lock(irq, &flags, 0); > > if (!desc) > @@ -1014,6 +1014,8 @@ void irq_modify_status(unsigned int irq, unsigned long clr, unsigned long set) > > irq_settings_clr_and_set(desc, clr, set); > > + trigger = irqd_get_trigger_type(&desc->irq_data); > + > irqd_clear(&desc->irq_data, IRQD_NO_BALANCING | IRQD_PER_CPU | > IRQD_TRIGGER_MASK | IRQD_LEVEL | IRQD_MOVE_PCNTXT); > if (irq_settings_has_no_balance_set(desc)) > @@ -1025,7 +1027,11 @@ void irq_modify_status(unsigned int irq, unsigned long clr, unsigned long set) > if (irq_settings_is_level(desc)) > irqd_set(&desc->irq_data, IRQD_LEVEL); > > - irqd_set(&desc->irq_data, irq_settings_get_trigger_mask(desc)); > + tmp = irq_settings_get_trigger_mask(desc); > + if (tmp != IRQ_TYPE_NONE) > + trigger = tmp; > + > + irqd_set(&desc->irq_data, trigger); > > irq_put_desc_unlock(desc, flags); > } > > Basically, save the trigger info before clearing it. If the interrupt > has already been configured, use that. Otherwise restore the previous > trigger settings. But I'm slightly confused as to which field is used > for what...
Thanks:), this patch works well, so
Tested-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> > M. >
| |