Some timer source requires re-initialization after S3 resume, due to reset when backing to S0. Some cleanup to allow such change generic. Signed-off-by Guanqun Lu <guanqun.lu@intel.com> Signed-off-by Kevin Tian <kevin.tian@intel.com> -- Guanqun _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 18/11/08 09:21, "Lu, Guanqun" <guanqun.lu@intel.com> wrote:> Some timer source requires re-initialization after S3 resume, > due to reset when backing to S0. Some cleanup to allow such > change generic. > > Signed-off-by Guanqun Lu <guanqun.lu@intel.com> > Signed-off-by Kevin Tian <kevin.tian@intel.com>The hpet.c changes are ugly. What''s so special about hpet_broadcast_init() that it needs to distinguish between boot and S3 resume? In the latter case it executes with APs still offline and IRQs disabled, so what''s wrong with rewriting the fixmap entry, re-initialising the spinlock and hpet_event info? Rather than passing a param to plt_src.init() instead define a plt_src.resume() for HPET only, doing only hpet_setup(). Again, I thinkw e ought to be able to avoid a resume/not-resume boolean pretty easily. Please split yor cleanup of time.c (the listing of platform timer structures and iterating over them into a separate patch [so send two patches, 1/2 and 2/2]). -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
>From: Keir Fraser >Sent: Tuesday, November 18, 2008 5:57 PM > >On 18/11/08 09:21, "Lu, Guanqun" <guanqun.lu@intel.com> wrote: > >> Some timer source requires re-initialization after S3 resume, >> due to reset when backing to S0. Some cleanup to allow such >> change generic. >> >> Signed-off-by Guanqun Lu <guanqun.lu@intel.com> >> Signed-off-by Kevin Tian <kevin.tian@intel.com> > >The hpet.c changes are ugly. What's so special about >hpet_broadcast_init() >that it needs to distinguish between boot and S3 resume? In >the latter case >it executes with APs still offline and IRQs disabled, so >what's wrong with >rewriting the fixmap entry, re-initialising the spinlock and hpet_event >info?Is it safe to set fixmap entry twice without unmap? If yes, then agree same logic applies to both boot and S3 resume.> >Rather than passing a param to plt_src.init() instead define a >plt_src.resume() for HPET only, doing only hpet_setup(). >Again, I thinkw e >ought to be able to avoid a resume/not-resume boolean pretty easily. > >Please split yor cleanup of time.c (the listing of platform >timer structures >and iterating over them into a separate patch [so send two >patches, 1/2 and >2/2]).OK, we'll change that. Thanks Kevin _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 18/11/08 10:04, "Tian, Kevin" <kevin.tian@intel.com> wrote:>> The hpet.c changes are ugly. What''s so special about >> hpet_broadcast_init() >> that it needs to distinguish between boot and S3 resume? In >> the latter case >> it executes with APs still offline and IRQs disabled, so >> what''s wrong with >> rewriting the fixmap entry, re-initialising the spinlock and hpet_event >> info? > > Is it safe to set fixmap entry twice without unmap? If yes, then > agree same logic applies to both boot and S3 resume.Yes, all it does is write a pte. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Tue, Nov 18, 2008 at 6:15 PM, Keir Fraser <keir.fraser@eu.citrix.com> wrote:> On 18/11/08 10:04, "Tian, Kevin" <kevin.tian@intel.com> wrote: > >>> The hpet.c changes are ugly. What''s so special about >>> hpet_broadcast_init() >>> that it needs to distinguish between boot and S3 resume? In >>> the latter case >>> it executes with APs still offline and IRQs disabled, so >>> what''s wrong with >>> rewriting the fixmap entry, re-initialising the spinlock and hpet_event >>> info? >> >> Is it safe to set fixmap entry twice without unmap? If yes, then >> agree same logic applies to both boot and S3 resume. > > Yes, all it does is write a pte.Thanks for you info, and we will rewrite the patch accordingly.> > -- Keir > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel >-- Guanqun _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel