Re: [PATCH v5] PCI: Xilinx NWL: Modifying irq chip for legacy interrupts

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 09/02/17 15:16, Bharat Kumar Gogada wrote:
>>
>> On 09/02/17 12:01, Bharat Kumar Gogada wrote:
>>>> On 06/02/17 07:03, Bharat Kumar Gogada wrote:
>>>>> +static struct irq_chip nwl_leg_irq_chip = {
>>>>> +	.name = "nwl_pcie:legacy",
>>>>> +	.irq_enable = nwl_unmask_leg_irq,
>>>>> +	.irq_disable = nwl_mask_leg_irq,
>>>>
>>>> You don't need these two if they are implemented in terms of mask/unmask.
>>>
>>> These are being invoked by some drivers other than interrupt flow.
>>> Ex: drivers/net/wireless/ath/ath9k/main.c
>>> static int ath_reset_internal(struct ath_softc *sc, struct
>>> ath9k_channel *hchan) {
>>>          ....
>>>          disable_irq(sc->irq);
>>>          tasklet_disable(&sc->intr_tq);
>>>         ...
>>>         ...
>>>         enable_irq(sc->irq);
>>>         spin_unlock_bh(&sc->sc_pcu_lock); } For us masking/unmasking
>>> is the way to enable/disable interrupts.
>>
>> And if you looked at the way disable_irq is implemented, you would have found
>> out that it falls back to masking if there is no disable method, preserving the
>> semantic you expect.
>>
> Yes I did see, but this fall back requires extra "IRQ_DISABLE_UNLAZY" flag to be set to each virq. 

No it doesn't. If you do a disable_irq(), the interrupt is flagged as
disabled, but nothing gets done. If an interrupt actually fires, then
the interrupts gets masked, and the handler is not called.

So just drop these two methods, because if this doesn't work, you have
bigger issues.

	M.
-- 
Jazz is not dead. It just smells funny...



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux