Messages in this thread | | | Date | Tue, 28 Jan 2003 02:21:59 +0100 (MET) | From | Mikael Pettersson <> | Subject | Re: Switch APIC to driver model (and make S3 sleep with APIC on) |
| |
On Mon, 20 Jan 2003 23:25:27 +0100, Pavel Machek wrote: >This switches apic code to driver model, cleans code up a lot, and >makes S3 while apic is used work. Please apply,
Please don't apply this. It breaks stuff:
1. apic_suspend() unconditionally calls disable_apic_nmi_watchdog() apic_resume() unconditionally calls setup_apic_nmi_watchdog() apic_pm_state.perfctr_pmdev removed
- You're calling local-APIC NMI watchdog procedures even if the local-APIC NMI watchdog isn't active. Bad. - You're hardcoding that the local-APIC NMI watchdog is the only possible sub-client of the local APIC. Not true. - perfctr_pmdev exists precisely to handle both these cases in a clean way.
2. You unconditionally register apic_driver with its suspend/resume methods through a device_initcall().
This breaks if a UP_APIC or SMP kernel runs on a CPU with no or an unusable local APIC. apic_pm_init2() does a runtime check for successful init before doing a pm_register().
3. You severed the link between the PM API and the local APIC.
This breaks APM suspend when the local APIC is enabled. The machine will hang (or immediately resume). I tested this, and the driver model "stuff" simply doesn't do the right thing yet.
I you just want SOFTWARE_SUSPEND to work, why not simply post the appropriate PM_SUSPEND and PM_RESUME events? That should work without any changes to apic.c or nmi.c.
/Mikael - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |