On Monday, 25 September 2006 23:35, Pavel Machek wrote:
> Hi!
>
> > > Note that we could get rid of that platform_mode variable if we had the
> > > possibility to specify defaults in the struct config_par (without
> > > resorting
> > > to platform_mode i would have had multiple incarnations of
> > >
> > > if (strcmp(shutdown_method, "reboot") || strcmp(shutdown_method,
> > > "shutdown"))
> > > foo;
> > >
> > > which i wanted to avoid). Would a patch for this be something of interest?
> >
> > Forget about that, i should have inspected the code closer before writing
> > :-)
> > Updated patch follows.
> >
> > Implement the userspace part of "use platform mode to suspend machines".
> >
> > Index: suspend.c
> > ===================================================================
> > RCS file: /cvsroot/suspend/suspend/suspend.c,v
> > retrieving revision 1.57
> > diff -u -p -r1.57 suspend.c
> > --- suspend.c 24 Sep 2006 08:09:59 -0000 1.57
> > +++ suspend.c 25 Sep 2006 17:37:53 -0000
> > @@ -67,7 +67,7 @@ static char s2ram;
> > static char early_writeout;
> > static char splash_param;
> > #define SHUTDOWN_LEN 16
> > -static char shutdown_method[SHUTDOWN_LEN];
> > +static char shutdown_method[SHUTDOWN_LEN] = "platform";
> >
> > static int suspend_swappiness = SUSPEND_SWAPPINESS;
> > static struct splash splash;
> > @@ -659,13 +659,18 @@ static int reset_signature(int fd)
> > }
> > #endif
> >
> > -static void suspend_shutdown(void)
> > +static void suspend_shutdown(int dev)
>
> Can you perhaps call the parameter "snapshot_fd" to be consistent with
> rest of code?
>
> > @@ -99,6 +104,21 @@ static inline int atomic_restore(int dev
> > return ioctl(dev, SNAPSHOT_ATOMIC_RESTORE, 0);
> > }
> >
> > +static inline int platform_prepare(int dev)
> > +{
> > + return ioctl (dev, SNAPSHOT_PMOPS, PMOPS_FINISH);
> ~ please do not add space between function and
> its arguments
>
> > +}
> > +
> > +static inline int platform_enter(int dev)
> > +{
> > + return ioctl (dev, SNAPSHOT_PMOPS, PMOPS_FINISH);
> > +}
> > +
> > +static inline int platform_finish(int dev)
> > +{
> > + return ioctl (dev, SNAPSHOT_PMOPS, PMOPS_FINISH);
> > +}
> > +
> > static inline int free_snapshot(int dev)
> > {
> > return ioctl(dev, SNAPSHOT_FREE, 0);
>
> Otherwise looks okay to me.
I'd prefer to have a variable, say use_platform_suspend, that is set to one
in main() if shutdown_method contains the string "platform", so that we can
get rid of the repeated strcmp(shutdown_method, "platform") invocations.
Also, when platform_finish() is called, the variable ret is not necessary, and
we don't intend to implement S3 in suspend_shutdown() (s2both is for that).
Greetings,
Rafael
--
You never change things by fighting the existing reality.
R. Buckminster Fuller
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
Suspend-devel mailing list
Suspend-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/suspend-devel
