Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1267082

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

Show all headers | View raw

On 10/11/15 18:07, Lars-Peter Clausen wrote:
> 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.

Yes, I agree that this does sound a bit odd, but ...

>> For example, during OF boot:
>>
>> [a]  irq_create_of_mapping()
>>    - irq_create_fwspec_mapping()
>>      - irq_set_irq_type()

The above means that if someone calls of_irq_get() (or
platform_get_irq()), before request_irq(), then this will call
irq_create_of_mapping() and hence, call irq_set_irq_type. So should
irq_create_fwspec_mapping() be setting the type in the first place? I
can see it is convenient to do it here.

>> 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().

Right. May be we can look at the various entry points to the chip
operators to get a feel for which public APIs need to be handled.

Cheers
Jon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[RFC PATCH 0/2] Add support for Tegra210 AGIC Jon Hunter <jonathanh@nvidia.com> - 2015-11-10 15:40 +0100
  [RFC PATCH 1/2] genirq: Add runtime resume/suspend support for IRQ chips Jon Hunter <jonathanh@nvidia.com> - 2015-11-10 15:50 +0100
    Re: [RFC PATCH 1/2] genirq: Add runtime resume/suspend support for  IRQ chips Thomas Gleixner <tglx@linutronix.de> - 2015-11-10 16:30 +0100
      Re: [RFC PATCH 1/2] genirq: Add runtime resume/suspend support for  IRQ chips Jon Hunter <jonathanh@nvidia.com> - 2015-11-10 17:00 +0100
        Re: [RFC PATCH 1/2] genirq: Add runtime resume/suspend support for  IRQ chips Grygorii Strashko <grygorii.strashko@ti.com> - 2015-11-10 17:50 +0100
          Re: [RFC PATCH 1/2] genirq: Add runtime resume/suspend support for  IRQ chips Lars-Peter Clausen <lars@metafoo.de> - 2015-11-10 19:10 +0100
            Re: [RFC PATCH 1/2] genirq: Add runtime resume/suspend support for  IRQ chips Jon Hunter <jonathanh@nvidia.com> - 2015-11-11 11:20 +0100
              Re: [RFC PATCH 1/2] genirq: Add runtime resume/suspend support for  IRQ chips Grygorii Strashko <grygorii.strashko@ti.com> - 2015-11-11 16:50 +0100
                Re: [RFC PATCH 1/2] genirq: Add runtime resume/suspend support for  IRQ chips Jon Hunter <jonathanh@nvidia.com> - 2015-11-12 12:00 +0100
                Re: [RFC PATCH 1/2] genirq: Add runtime resume/suspend support for  IRQ chips Lars-Peter Clausen <lars@metafoo.de> - 2015-11-12 14:30 +0100
                Re: [RFC PATCH 1/2] genirq: Add runtime resume/suspend support for  IRQ chips Jon Hunter <jonathanh@nvidia.com> - 2015-11-12 14:40 +0100
                Re: [RFC PATCH 1/2] genirq: Add runtime resume/suspend support for  IRQ chips Lars-Peter Clausen <lars@metafoo.de> - 2015-11-12 14:50 +0100
                Re: [RFC PATCH 1/2] genirq: Add runtime resume/suspend support for  IRQ chips Jon Hunter <jonathanh@nvidia.com> - 2015-11-12 15:10 +0100
                Re: [RFC PATCH 1/2] genirq: Add runtime resume/suspend support for  IRQ chips Lars-Peter Clausen <lars@metafoo.de> - 2015-11-12 15:40 +0100
                Re: [RFC PATCH 1/2] genirq: Add runtime resume/suspend support for  IRQ chips Jon Hunter <jonathanh@nvidia.com> - 2015-11-12 16:40 +0100
                Re: [RFC PATCH 1/2] genirq: Add runtime resume/suspend support for  IRQ chips Grygorii Strashko <grygorii.strashko@ti.com> - 2015-11-12 14:40 +0100
                Re: [RFC PATCH 1/2] genirq: Add runtime resume/suspend support for  IRQ chips Lars-Peter Clausen <lars@metafoo.de> - 2015-11-12 14:40 +0100
                Re: [RFC PATCH 1/2] genirq: Add runtime resume/suspend support for  IRQ chips Grygorii Strashko <grygorii.strashko@ti.com> - 2015-11-13 19:10 +0100
        Re: [RFC PATCH 1/2] genirq: Add runtime resume/suspend support for  IRQ chips Sören Brinkmann <soren.brinkmann@xilinx.com> - 2015-11-12 18:40 +0100
    Re: [RFC PATCH 1/2] genirq: Add runtime resume/suspend support for IRQ chips Kevin Hilman <khilman@kernel.org> - 2015-11-13 00:30 +0100
      Re: [RFC PATCH 1/2] genirq: Add runtime resume/suspend support for  IRQ chips Jon Hunter <jonathanh@nvidia.com> - 2015-11-13 10:10 +0100
        Re: [RFC PATCH 1/2] genirq: Add runtime resume/suspend support for  IRQ chips Thomas Gleixner <tglx@linutronix.de> - 2015-11-13 21:10 +0100
          Re: [RFC PATCH 1/2] genirq: Add runtime resume/suspend support for  IRQ chips Jon Hunter <jonathanh@nvidia.com> - 2015-11-16 10:50 +0100
            Re: [RFC PATCH 1/2] genirq: Add runtime resume/suspend support for  IRQ chips Geert Uytterhoeven <geert@linux-m68k.org> - 2015-11-16 10:50 +0100
              Re: [RFC PATCH 1/2] genirq: Add runtime resume/suspend support for  IRQ chips Jon Hunter <jonathanh@nvidia.com> - 2015-11-16 11:40 +0100
                Re: [RFC PATCH 1/2] genirq: Add runtime resume/suspend support for  IRQ chips Geert Uytterhoeven <geert@linux-m68k.org> - 2015-11-16 11:50 +0100

csiph-web