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.
>> >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.)
>
>
>> 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.
>> For example the UHCI case. A GPE is fired, we need to clear/disable
>> the wakeup event. PCI bus can't handle it, as UHCI has special
>> registers for this, so we need call into device specific handling.
>
>So the bus-level PCI operation would end up with a device
>that's either (a) known to have fired PME#, or (b) known
>to be a device with legacy PM -- like UHCI -- which it's
>presuming has been wakeup-enabled.
>
>Then in the (a) case it can clear PME# the normal way, and
>would rarely need to do anything else.
>
>While in the (b) case the UHCI driver would need to have
>provided some device-specific -- not bus-generic -- op
>called like dev->driver->pm.is_wakeup(), to diddle those
>registers (only for Intel silicon, not UHCI from VIA or
>Genesys or anyone else).
>
>
>I think I'm getting a bit more clear on what you're trying
>to do with this. Restructure it a bit and I will be able
>to see that from reading the code (as will others). :)
So we are on the same page here both bus and device requires a op like .is_wakeup(). And what I need to do is to address the pci bridge case.
Thanks,
Shaohua
_______________________________________________
linux-pm mailing list
linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/linux-pm
