Re: [RFC PATCH v11 4/5] PCI / PM: Add support for the PCIe WAKE# signal for OF

Rafael J. Wysocki
2017-12-27T00:58:05Z

On Tuesday, December 26, 2017 2:06:47 AM CET JeffyChen wrote:
> Hi Rafael,
>
> Thanks for your reply :)
>
> On 12/26/2017 08:11 AM, Rafael J. Wysocki wrote:
> >> >+
> >> >+ dn = pci_device_to_OF_node(ppdev);
> >> >+ if (!dn)
> >> >+ return 0;
> >> >+
> >> >+ irq = of_irq_get_byname(dn, "wakeup");
> > Why is this a property of the bridge and not of the device itself?
>
> That is suggested by Brian, because in that way, the wakeup pin would
> not "tied to what exact device is installed (or no device, if it's a slot)."

But I don't think it works when there are two devices using different WAKE#
interrupt lines under the same bridge. Or how does it work then?

> >> >+ if (irq == -EPROBE_DEFER)
> > Braces here, please.
> ok, will fix in the next version.
>
> >
> >> >+ return irq;
> >> >+ /* Ignore other errors, since a missing wakeup is non-fatal. */
> >> >+ else if (irq < 0) {
> >> >+ dev_info(&pdev->dev, "cannot get wakeup interrupt: %d\n", irq);
> >> >+ return 0;
> >> >+ }
> >> >+
> >> >+ device_init_wakeup(&pdev->dev, true);
> > Why do you call this before dev_pm_set_dedicated_wake_irq()?
>
> hmmm, i thought so too, but it turns out the dedicated wake irq
> framework requires device_init_wakeup(dev, true) before attach the wake irq:
>
> int device_wakeup_attach_irq(struct device *dev,
> struct wake_irq *wakeirq)
> {
> struct wakeup_source *ws;
>
> ws = dev->power.wakeup;
> if (!ws) {
> dev_err(dev, "forgot to call device_init_wakeup?\n");
> return -EINVAL;
>

Well, that's a framework issue, fair enough.

That said, what if user space removes the wakeup source from under you
concurrently via sysfs? Tony?

Thanks,
Rafael