Re: swsusp: revert to 2.6.0-test3 state

[Patrick Mochel]

> This patch reverts swsusp to known good state (before Patrick made his
> untested changes to it). I had to do some changes to both swsusp.c and
> power.c to keep it compilable. Please apply,

Pavel, why do you have to be so difficult? I realize you're sore that I 
modified the code you maintain and unintentionally broke. However, it does 
not benefit either of us for you to intentionally break my code in return, 
especially considering I've since fixed the outstanding problems in my 
changes. 

> --- clean/kernel/power/main.c	2003-08-27 12:00:53.000000000 +0200
> +++ linux/kernel/power/main.c	2003-09-03 20:57:00.000000000 +0200
> @@ -178,25 +178,7 @@
>  	if (pm_disk_mode == PM_DISK_FIRMWARE)
>  		return pm_ops->enter(PM_SUSPEND_DISK);
>  
> -	if (!have_swsusp)
> -		return -EPERM;
> -
> -	pr_debug("PM: snapshotting memory.\n");
> -	in_suspend = 1;
> -	if ((error = swsusp_save()))
> -		goto Done;
> -
> -	if (in_suspend) {
> -		pr_debug("PM: writing image.\n");
> -		error = swsusp_write();
> -		if (!error)
> -			error = power_down(pm_disk_mode);
> -		pr_debug("PM: Power down failed.\n");
> -	} else
> -		pr_debug("PM: Image restored successfully.\n");
> -	swsusp_free();
> - Done:
> -	return error;
> +	BUG();
>  }

This is bullshit. It will not only introduce a compile warning, but it's
not user-friendly (I can forward flames from Linus about adding gratuitous
BUG()s to the kernel if you like). You also intentionally break my code,
instead of doing something reasonable like


+	return 0; 


> @@ -228,6 +210,11 @@
>  {
>  	int error = 0;
>  
> +	if ((state == PM_SUSPEND_DISK) && (pm_disk_mode != PM_DISK_FIRMWARE)) {
> +		software_suspend();
> +		return -EAGAIN;
> +	}

Why return -EAGAIN? 

Why even call software_suspend() at all. That's not the right thing to do, 
nor is it what I want to do (which I implied in saying that I would not 
use it). And, you've broken the possiblity of using the actualy ACPI S4 
low-power state. 

...

>  static int pm_resume(void)

> +	software_resume();
>  	return 0;
>  }

This is just silly, from a design point of view. You now have two 
functions that do the same thing, one just calls the other. Why? 

Please resubmit the patch without this crap, and I will not argue. 



	Pat

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/