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:

quoted

quoted

quoted

+
+	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?

quoted

quoted

quoted

+	if (irq == -EPROBE_DEFER)

Braces here, please.

ok, will fix in the next version.

quoted

quoted

quoted

+		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