* Rafael J. Wysocki <rjw@xxxxxxx> wrote: > > > Index: linux-2.6/arch/x86/kernel/apm_32.c > > > =================================================================== > > > --- linux-2.6.orig/arch/x86/kernel/apm_32.c > > > +++ linux-2.6/arch/x86/kernel/apm_32.c > > > > > + > > > + suspend_device_irqs(); > > > device_power_down(PMSG_SUSPEND); > > > + > > > + local_irq_disable(); > > > > hm, this is a very repetitive pattern, all around the various > > suspend/resume variants. Might make sense to make: > > > > device_power_down(PMSG_SUSPEND); > > > > do the irq line disabling plus the local irq disabling > > automatically. That also means it cannot be forgotten. The > > symmetric action should happen for PMSG_RESUME. > > > > Is there ever a case where we want a different pattern? > > Even if there's no such case, I prefer to call > local_irq_disable() explicitly in here, so that it's clearly > known where it happens to anyone reading this code. That property can be implied in the function name: device_power_down_irq_disable(PMSG_SUSPEND); Open-coding it, if it looks the same in all the cases just increases the chances that someone somewhere copies them incorrectly. Ingo _______________________________________________ linux-pm mailing list linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/linux-pm
