* Jean Pihet <[email protected]> wrote: > Ingo, > > On Tue, Oct 26, 2010 at 9:10 AM, Ingo Molnar <[email protected]> wrote: > > > > * Thomas Renninger <[email protected]> wrote: > > > >> Changes in V2: > >> - Introduce PWR_EVENT_EXIT instead of 0 to mark non-power state > >> - Use u32 instead of u64 for cpuid, state which is by far enough > > ... > > >> > >> +#define PWR_EVENT_EXIT 0xFFFFFFFF > > > > Shouldnt this be part of the POWER_ enum? (and you can write -1 there) > > > >> +#ifndef _TRACE_POWER_ENUM_ > >> +#define _TRACE_POWER_ENUM_ > >> +enum { > >> + POWER_NONE = 0, > >> + POWER_CSTATE = 1, > >> + POWER_PSTATE = 2, > >> +}; > >> +#endif > > > > Since we are cleaning up all these events, those enum definitions dont > > really look > > logical. For example, what is 'POWER_NONE'? Can a CPU have 'no power'? > > The enum belongs to the deprecated API so I would rather not touch it. > Keeping the deprecated code isolated will make it easier to remove > later.
So what will replace it? We still have a state field. Passing in platform specific codes is a step backwards. > >> +TRACE_EVENT(machine_suspend, > >> + > >> + TP_PROTO(unsigned int state), > >> + > >> + TP_ARGS(state), > >> + > >> + TP_STRUCT__entry( > >> + __field( u32, state ) > >> + ), > > > > Hm, this event is not used anywhere in the submitted patches. Where is the > > patch > > that adds usage, and what are the possible values for 'state'? > > This will come as a separate patch, which fits all platforms. Cf. > http://marc.info/?l=linux-omap&m=128620575300682&w=2. > The state field is of type suspend_state_t, cf. include/linux/suspend.h Ok, that's at least generic. Needs the review of Rafael, to determine whether this state value is all we want to know when we enter suspend. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
