On Tuesday, 9 of September 2008, Li, Shaohua wrote:
> Ok, you comments on pci bridge makes sense. I missed that case, will change the code to support it.
>
> >On Monday 08 September 2008, Li, Shaohua wrote:
> >>
> >> >> + * @wakeup_event: Checks if a wakeup event occurs.
> >> >> + * If yes, wakeup event should be disabled.
> >> >
> >> >And ... what else?? What does the return value indicate?
> >> >Should anything be done with it other than printing it out
> >> >if it's nonzero and we're debugging?
> >>
> >> Return 0 if the device invokes a wakeup event. In this case, driver
> >> should clear/disable wakeup event.
> >> Return < 0 if device didn't invoke wakeup.
> >
> >So it's effectively "bool"? I'd declare it as "bool" then...
> >and rename it to sound like a predicate; "is_wakedev()" maybe.
> Right.
>
> >> >> @@ -151,6 +153,7 @@ struct pm_ops {
> >> >> int (*thaw)(struct device *dev);
> >> >> int (*poweroff)(struct device *dev);
> >> >> int (*restore)(struct device *dev);
> >> >> + int (*wakeup_event)(struct device *dev);
> >> >
> >> >My reaction to adding this method is: why do it here rather
> >> >than at the bus level?
> >
> >Your answer probably should have been: "The recent method
> >reshuffling makes that method work both at the level of the
> >'struct bus_type' and at the level of 'struct device'. So
> >this _does_ add it at the bus level."
> Right.
Still, I'm not sure if we need the method at the device core level at all.
The suspend/resume callbacks are needed at the device core level, because the
_core_ initiates the operation (suspend, resume etc.), but in this case the
operation will be initiated in a core-independent way (for example, by an
interrupt handler in case of the PCI Express native PME).
Surely, we'll need such a callback on the PCI core level that may have to
invoke the device driver to handle wake-up after identifying the device that
caused the event to happen.
> >> >In my particular experience there are two basic types of wakeup
> >> >event:
> >> >
> >> > ...
> >> >
> >> > * ACPI GPEs. Bus-specific ... and similar to GPIO IRQs.
> >> > Also sharable; bytecode is used to map the GPE and
> >> > some register state to the ACPI device(s) which
> >> > issued that GPE.
> >>
> >> This isn't bus specific.
> >
> >It's bus-specific in the sense of "ACPI bus" (/sys/bus/acpi).
> >If the DSDT defines a GPE and maps it to an ACPI device, it will
> >follow those rules.
> >
> >Devices not on the "ACPI bus" -- like add-on PCI cards, USB
> >peripherals, and other devices which have some such ACPI device
> >higher in the driver model tre -- don't use GPEs to wake up.
> >(An ancestral device may well do so: a PCI bridge, a USB
> >host controller, etc.)
Well, I'm not exactly sure how PME# is supposed to be handled for add-on cards.
My understanding so far has been that all of the PME# signals are supposed to
be line ORed and routed (around all of the bridges) to the host bridge that
should generate an interrupt of some sort (eg. ACPI SCI). Isn't that correct?
> >> ACPI devices can map to any physical devices, like PCI, IDE
> >> drive, PNP device. In this case, a bus specific mechanism can't
> >> handle all.
> >
> >Sure it can. And it'd be more natural to do it that way too.
> >I'd be tempted to call them from ACPI like
> >
> > struct device *dev;
> > acpi_handle adev;
> >
> > /* adev received a wake event notification */
> >
> > ...
> > dev = acpi_get_physical_device(adev);
> > if (dev) {
> > /* this chunk of code should probably live in
> > * drivers/base/power/... somewhere and be
> > * part of this patch 1/5 so it's more clear
> > * what the infrastructure is doing.
> > */
> > if (... && dev->bus->pm.base->is_wakedev(dev)) {
> > ... call dev->driver->pm.resume(dev)
> > ... else if resume is null, issue diagnostic
> > } else {
> > ... report suitable generic confusion ...
> > }
> > } else {
> > ... report ACPI-specific confusion ...
> > }
> > ...
> >
> >When that's a PCI device -- bridge or otherwise -- that
> >would call something resembling your patch #3 (but fixed
> >to handle the case of PCI bridges, so add-in cards will
> >work properly).
> >
> >And for a non-ACPI system, whatever IRQ handler receives the
> >PME# signal would just call that chunk of code for the PCI
> >root bridge(s) it handles. All done. :)
> I can move my ' device_receive_wakeup_event()' into ' pci_pm_wakeup_event()',
> and export the routine. Then IRQ handler can call it. Assume this is what you
> want.
Would pci_pm_wakeup_event() be responsible for invoking a device driver's
callback in that case?
Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
