On 09/19/2016 02:30 PM, Arnd Bergmann wrote:
> On Monday, September 19, 2016 11:54:29 AM CEST Mark Brown wrote:
>> On Mon, Sep 19, 2016 at 12:31:04PM +0200, Arnd Bergmann wrote:
>>
>>> My guess is that this is indeed the correct fix, but I don't understand
>>> at all what the function does, and Marcin Niestroj said the same thing,
>>> he just copied the code from a driver that is always built-in.
>>
>> Yes, most of the code is copied from regmap-irq. Setting the parent is
>> needed for edge triggered interrupts that don't latch state when
>> disabled but isn't needed in the general case.
>
> I found this in the tps65217 data sheet:
>
> 9.3.7 Interrupt Pin (nINT)
> The interrupt pin is used to signal any event or fault
> condition to the host processor. Whenever a fault or event
> occurs in the IC the corresponding interrupt bit is set in
> the INT register, and the open-drain output is pulled low.
> The nINT pin is released (returns to Hi-Z state) and fault
> bits are cleared when the INT register is read by the
> host. However, if a failure persists, the corresponding
> INT bit remains set and the nINT pin is pulled low again
> after a maximum of 32 µs.
>
> Interrupt events include pushbutton pressed/released, USB
> and AC voltage status change.
> The MASK bits in the INT register are used to mask events
> from generating interrupts. The MASK settings affect the
> nINT pin only and have no impact on protection and
> monitor circuits themselves. Note that persisting event
> conditions such as ISINK enabled shutdown can cause the
> nINT pin to be pulled low for an extended period of
> time which can keep the host in a loop trying to resolve
> the interrupt. If this behavior is not desired, set the
> corresponding mask bit after receiving the interrupt and
> keep polling the INT register to see when the event
> condition has disappeared. Then unmask the interrupt bit
> again.
>
> Does this tell us if it's needed or not?
>
The irq_set_parent() is required for proper supporting of lazy irq disabling
for threaded nested irqs and allow them to be re-triggered in the following case:
driverX:
disable(nested_irq);
...
parent irq triggered
|- handle_nested_irq()
|- mark nested_irq with IRQS_PENDING
...
driverX:
enable(nested_irq);
|- if IRQS_PENDING -> resend parent_irq [*], because nested threaded irq can't
be resend by itself
[*] if parent_irq is not set the IRQS_PENDING status will be ignored and irq might be lost.
Also, it was "mandatory" to be done before commit 75a06189fc5
"genirq: Prevent resend to interrupts marked IRQ_NESTED_THREAD"
which made setting of parent_irq optional.
So, if is it required or not depends not only on HW, but, rather, on MFD drivers implementation
(Interrupt controller and MFD cell drivers).
--
regards,
-grygorii
--
To unsubscribe from this list: send the line "unsubscribe linux-next" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
