On Mon, 2016-11-07 at 19:35 -0700, Tony Lindgren wrote:
> * Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx> [161107 07:21]:
> > On 2016-11-07 15:48:50 [+0200], Andy Shevchenko wrote:
> > > On Thu, 2016-11-03 at 17:29 +0100, Sebastian Andrzej Siewior
> > > wrote:
> > > > On 2016-11-03 18:14:23 [+0200], Andy Shevchenko wrote:
> > > > > We can't and basically don't need to call runtime PM in IRQ
> > > > > handler.
> > > > > If IRQ is
> > > > > ours, device must be powered on. Otherwise check if the device
> > > > > is
> > > > > powered off
> > > > > and return immediately.
> > > >
> > > > The old beagle board does PM put once it does not use the UART
> > > > anymore
> > > > /
> > > > the UART is idle.
> > >
> > > And how can it generate interrupts?
> >
> > It has this
> > priv->wakeirq = irq_of_parse_and_map(pdev->dev.of_node, 1);
> > line in the driver. I *think* it is a different interrupt (same
> > source
> > but different PM setup and thus different number) and the driver
> > needs
> > to be invoked in order to ACK it.
>
> Yes it's a separate always-on interrupt controller for pin wake-up
> events. It uses the Linux generic wakeirq support, see commit
> 4990d4fe327b ("PM / Wakeirq: Add automated device wake IRQ handling")
> for some more info. Basically all the driver has to do in this case:
>
> device_init_wakeup(dev, true);
> dev_pm_set_dedicated_wake_irq(dev, wakeirq);
This is basically what is done in my patches for our UART. We switch RxD
or CTS to GPIO and make it wake source.
>
> Note that handling of the wakeirq can also happen at the bus level,
> see what i2c bus is doing for example with commit 3fffd1283927 ("i2c:
> allow specifying separate wakeup interrupt in device tree").
>
> > > > You need to PM get the device even if the device is not yet
> > > > suspended
> > > > because it might get suspended in the middle of your operation
> > > > (maybe
> > > > not while you serve the IRQ but we have threaded interrupts).
> > >
> > > I grepped code and found no driver under 8250 which is using
> > > threaded
> > > IRQ handler. What did I miss?
> >
> > Documentation/kernel-parameters.txt: threadirqs [KNL]
Okay, so, the scenario you perhaps concerned is:
1. HW generates interrupt.
2. It's queued to be handled by threads.
3. Meanwhile the HW might be powered off (automatic suspend assumed). Tx
can't be for sure, Rx side then. But I barely imagine how it is
possible. Even if we get it powered off it should wake the system again
(via wake source, which is RxD/CTS or out-of-bound one).
4. In the handler we check if the device suspended. So, return to the
question below.
Where is the issue in my logic?
> For dedicated wakeirqs we want to use threaded IRQ as things can take
> tens of milliseconds to power up devices from deeper idle states.
>
> For the regular device interrupt it's a different story :)
>
> > > > If the HW is off then it will be woken by an RX interrupt and I
> > > > think
> > > > it
> > > > is a different one (I remember Tony wanted to rework that part
> > > > so that
> > > > the device/UART does not need to care about that).
> > >
> > > Rx interrupt from other device?
> > >
> > > Or OMAP UART has inbound wake source?
> >
> > See above. It is the "priv->wakeirq" thing.
Yeah, which is out-of-bound one and there is no difference to how we are
using it in 8250_dw.
> > It is a wake source which is the RX pin setup differently (my memory
> > here). Tony wanted to change PM code in regard to this as he wasn't
> > too
> > happy about it, too.
> >
> >
> > > > This should work in general but I think that is racy with
> > > > threaded
> > > > interrupts: What stops the PM core to shutdown the HW before
> > > > serial_port_in() ?
> > >
> > > Basically what I understand from this the question is: "Is
> > > pm_runtime_suspended() synchronous with regard to
> > > pm_runtime_get()/pm_runtime_suspend()?".
> >
> > I guess so.
>
> Yeah pm_runtime will take care of things when implemented properly.
It's not serialized, so, you might get it called in between of
suspending-suspended states. Which means in case of enforced threaded
IRQs we need to be sure that access is happened on powered on device.
I might miss something, but I guess the best people to ask are Thomas
and Rafael (Cc'ed).
--
Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
