Re: [RFC PATCH 1/2] genirq: Add runtime resume/suspend support for IRQ chips

[Lars-Peter Clausen]

On 11/10/2015 05:47 PM, Grygorii Strashko wrote:
[...]
>> I was trying to simplify matters by placing the resume call in
>> __setup_irq() as opposed to requested_threaded_irq(). However, the would
>> mean the resume is inside the bus_lock and may be I should not assume
>> that I can sleep here.
>>
>>> Can you folks please agree on something which is correct and complete?
>>
>> Soren I am happy to defer to your patch and drop this. My only comment
>> would be what about the request_percpu_irq() path in your patch?
>>
> 
> I have the same comment here as I asked Soren:
> 1) There are no restrictions to call irq set_irq_type() whenever,
> as result HW can be accessed before request_x_irq()/__setup_irq().
> And this is used quite widely now :(
> 

Changing the configuration of a resource that is not owned seems to be
fairly broken. In the worst case this will overwrite the configuration that
was set by owner of the resource.

Especially those that call irq_set_irq_type() directly before request_irq(),
given that you supply the trigger type to request_irq() which will make sure
that there are no conflicts and the configure.

This is a bit like calling gpio_set_direction() before you call
gpio_request(), which will also have PM issues.

> For example, during OF boot:
> 
> [a]  irq_create_of_mapping()
>    - irq_create_fwspec_mapping()
>      - irq_set_irq_type()
> 
> or 
> 	irq_set_irq_type(irq, IRQ_TYPE_LEVEL_HIGH);
> 	irq_set_chained_handler(irq, mx31ads_expio_irq_handler);
> 
> or
> 	irq_set_irq_type(alarm_irq, IRQ_TYPE_EDGE_BOTH);
> 	err = devm_request_irq(&pdev->dev, alarm_irq, fan_alarm_irq_handler,
> (there are ~200 occurrences of irq set_irq_type in Kernel)
> 
> 2) if i'm not wrong, the same is valid for irq_set_irq_wake() and irq_set_affinity()
> 
> I'm not saying all these code is correct, but that what's now in kernel :(
> I've tried to test Soren's patch with omap-gpio and immediately hit case [a] :.( 

All functions for which are part of the public API and for which it is legal
to call them without calling request_irq() (or similar) first will need to
have pm_get()/pm_put().
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/