Re: [PATCH] swsusp: Do not use pm_ops

["Rafael J. Wysocki" <rjw@sisk.pl>]

On Thursday, 3 May 2007 10:41, Johannes Berg wrote:
> On Wed, 2007-05-02 at 22:13 +0200, Rafael J. Wysocki wrote:
> 
> > +void hibernation_set_ops(struct hibernation_ops *ops)
> > +{
> > +	if (ops && !(ops->prepare && ops->enter && ops->finish)) {
> > +		printk(KERN_ERR "Wrong definition of hibernation operations! "
> > +			"Using defaults\n");
> > +		return;
> > +	}
> 
> Why not BUG_ON here as I had before? I don't see much point in giving a
> runtime warning, and the docs clearly state that you must assign all
> three items. Oh, I see I had a bug before when ops was NULL, but you can
> still do
>   BUG_ON(ops && !(ops->prepare && ops->enter && ops->finish));

Well, BUG_ON() is extremely user-unfriendly, and it'd trigger even if the user
actually didn't intend to suspend at all.

IMO this is not a "we can't continue if that condition is not satisfied"
situation.
 
> > -	pr_debug("PM: suspend-to-disk mode set to '%s'\n",
> > -		 pm_disk_modes[mode]);
> > +	if (!error)
> > +		pr_debug("PM: suspend-to-disk mode set to '%s'\n",
> > +			 hibernation_modes[mode]);
> 
> Isn't that an unrelated bugfix ;) just kidding

You mean the 'if (!error)'?  Well ... ;-)

> Looks good to me but I haven't checked the acpi in detail. If I
> remember, I'll try to give it all a go on my G5 later today.

OK

Greetings,
Rafael