On Thu, 21 Jan 2016, Ulf Hansson wrote: > >> > I don't think using __free_irq() is the correct place to decrease the > >> > runtime PM usage count. It will keep the irqchip runtime resumed even > >> > if there are no irqs enabled for it. > >> > > >> > Instead I would rather allow the irqchip to be runtime suspended, when > >> > there are no irqs enabled on it. > > > > Which is a no no, as you might lose interrupts that way. We disable interrupts > > lazy, i.e. we do not mask them. So no, you cannot do that from > > enable/disable_irq(). > > Thanks for the input! > > My main point around the approach suggested in $subject patch, is that > I don't think it's *enough* fine grained. > > From a runtime PM perspective, we should allow the irqchip to enter a > low power state when there are no IRQs enabled for it. > > As enable|disable_irq() isn't the place to do runtime PM reference > counting from, perhaps there is another option? Maybe the irqchip > driver itself is better suited to take these decisions? The irq chip driver does not have more information than the core code, actually it has less. Let's look at disable_irq(). It's normaly used for short periods of time where a driver needs to make sure that the interrupt is not handled. That's hardly a point to do power management, because its going to be reenabled right away. That's one of the reasons for doing the lazy masking, though the main reason is to prevent losing interrupt on edge driven irq lines. So as long as an interrupt handler is installed, there is no sane way that we can decide to power down the irq chip, unless that chip has the magic ability to relay incoming interrupts while powered down :) There is also the issue with shared interrupts.... So the only sane way to power down the irq chip is to remove the handlers when the device is not in use. Actually some of the subsystems/drivers do that already. Though there might be a case where the device driver is still in use, but it has brought down the device into a quiescent state, which guarantees that no interrupts happen unless its powered up again. That would be an opportunity to shut down the interrupt and do power management as well. So currently we only have the option of freeing the interrupt and requesting it again. It might be conveniant to avoid that by having extra functions which are less heavyweight. Something like irq_[de]activate(irq, void *dev_id), which would mask/unmask the interrupt and do power management on the irq chip. If you can come up with a simple comparision of such an approach with the free/request_irq() method which shows that it is superior, I'm surely not in the way. No need to code that core part, just show how a few drivers would use those functions and why this is less intrusive and simpler to use for the developer that going for the free/request() solution. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
