> - much more verbose suspend output in order to actually enable people
> to see what's going on inside the still highly problematic suspend handling
> (on my Dell Inspiron 8000, only ACPI S1 works at all, and only
> after a LOT of investigation and tweaking)
> And let's be extra aggressive about drivers that don't have suspend
> handlers yet but might need them and thus cause problems on resume
> due to missing hardware state saving...
> A lot of drivers don't handle that yet, so point that out properly for the
> time being.
> - fix invidual suspend handlers' output to be able to properly identify
> them
> - rename strangely named INTERESTING define to IGNORE_SPECIAL_THREADS which
> makes much more sense
> - some spelling stuff
> - update my email address ;)
>
> This patch doesn't include the pccardd suspend fix, since I noticed that
> it has been fixed by someone else already ;-\
> (which was pccardd-suspend-fix.patch)
>
> Patch against vanilla 2.5.70.
>
> I just hope that the right person knows that (s)he is supposed to commit it.
> (or not...)
Well, you are supposed to push it, yourself.
Separate spelling fixes, that goes to linus with "Spelling fixes" subject.
> @@ -35,19 +35,25 @@
> struct list_head * node;
> int error = 0;
>
> - printk(KERN_EMERG "Suspending devices\n");
> + printk(KERN_EMERG "Suspending devices (%d, %d)\n", state, level);
>
> down_write(&devices_subsys.rwsem);
> list_for_each(node,&devices_subsys.kset.list) {
> struct device * dev = to_dev(node);
> - if (dev->driver && dev->driver->suspend) {
> - pr_debug("suspending device %s\n",dev->name);
> - error = dev->driver->suspend(dev,state,level);
> - if (error)
> - printk(KERN_ERR "%s: suspend returned %d\n",dev->name,error);
> + if (dev->driver)
> + {
Formating!
> + if (dev->driver->suspend) {
> + printk("suspending device %s\n",dev->name);
> + error = dev->driver->suspend(dev,state,level);
> + if (error)
> + printk(KERN_ERR "%s: suspend failed, error %d\n",dev->name,error);
> + }
> + else
> + printk("no suspend function (yet?) in driver for %s\n",dev->name);
> }
> }
> up_write(&devices_subsys.rwsem);
> +
> return error;
> }
>
I like this patch, Patrick Mochel is probably right person to push it through...
> @@ -62,13 +68,16 @@
> void device_resume(u32 level)
> {
> struct list_head * node;
> + int error = 0;
>
> down_write(&devices_subsys.rwsem);
> list_for_each_prev(node,&devices_subsys.kset.list) {
> struct device * dev = to_dev(node);
> if (dev->driver && dev->driver->resume) {
> - pr_debug("resuming device %s\n",dev->name);
> - dev->driver->resume(dev,level);
> + printk("resuming device %s\n",dev->name);
> + error = dev->driver->resume(dev,level);
> + if (error)
> + printk(KERN_ERR "%s: resume failed, error %d\n",dev->name,error);
> }
> }
> up_write(&devices_subsys.rwsem);
Same here.
> diff -urN linux-2.5.70.orig/drivers/ide/ide-disk.c linux-2.5.70/drivers/ide/ide-disk.c
> --- linux-2.5.70.orig/drivers/ide/ide-disk.c 2003-05-28 17:58:47.000000000 +0200
> +++ linux-2.5.70/drivers/ide/ide-disk.c 2003-05-28 18:38:10.000000000 +0200
> @@ -1509,7 +1509,7 @@
> {
> ide_drive_t *drive = dev->driver_data;
>
> - printk("Suspending device %p\n", dev->driver_data);
> + printk("IDE device %s: suspend to state %d\n", drive->name, state);
>
> /* I hope that every freeze operation from the upper levels have
> * already been done...
> @@ -1519,7 +1519,6 @@
> return 0;
>
> /* set the drive to standby */
> - printk(KERN_INFO "suspending: %s ", drive->name);
> do_idedisk_standby(drive);
> drive->blocked = 1;
>
Kill these. Someone else is fixing IDE, you don't want to clash.
> diff -urN linux-2.5.70.orig/kernel/suspend.c linux-2.5.70/kernel/suspend.c
> --- linux-2.5.70.orig/kernel/suspend.c 2003-05-28 17:59:02.000000000 +0200
> +++ linux-2.5.70/kernel/suspend.c 2003-05-27 16:05:06.000000000 +0200
> @@ -24,7 +24,7 @@
> * Straightened the critical function in order to prevent compilers from
> * playing tricks with local variables.
> *
> - * Andreas Mohr <a.mohr@mailto.de>
> + * Andreas Mohr <andi@lisas.de>
> *
> * Alex Badea <vampire@go.ro>:
> * Fixed runaway init
> @@ -164,7 +164,7 @@
> * Refrigerator and related stuff
> */
>
> -#define INTERESTING(p) \
> +#define IGNORE_SPECIAL_THREADS(p) \
> /* We don't want to touch kernel_threads..*/ \
> if (p->flags & PF_IOTHREAD) \
> continue; \
> @@ -205,6 +205,9 @@
> unsigned long start_time;
> struct task_struct *g, *p;
>
> + /* keep iterating over all threads and signal to them that they
> + * are supposed to freeze (in signal handler), until timeout happens.
> + */
> printk( "Stopping tasks: " );
> start_time = jiffies;
> do {
> @@ -212,14 +215,16 @@
> read_lock(&tasklist_lock);
> do_each_thread(g, p) {
> unsigned long flags;
> - INTERESTING(p);
> + IGNORE_SPECIAL_THREADS(p);
> if (p->flags & PF_FROZEN)
> continue;
>
> + p->flags |= PF_FREEZE; /* this task should be frozen */
> /* FIXME: smp problem here: we may not access other process' flags
> without locking */
> - p->flags |= PF_FREEZE;
No its actually right the other way.
> spin_lock_irqsave(&p->sighand->siglock, flags);
> + /* send signal to thread in order to schedule to it
> + * and freeze it in its own context there */
> signal_wake_up(p, 0);
> spin_unlock_irqrestore(&p->sighand->siglock, flags);
> todo++;
> @@ -232,7 +237,7 @@
> return todo;
> }
> } while(todo);
> -
> +
> printk( "|\n" );
> BUG_ON(in_atomic());
> return 0;
You are adding space before tab. RIght solution is to kill both.
> @@ -245,7 +250,7 @@
> printk( "Restarting tasks..." );
> read_lock(&tasklist_lock);
> do_each_thread(g, p) {
> - INTERESTING(p);
> + IGNORE_SPECIAL_THREADS(p);
>
> if (p->flags & PF_FROZEN) p->flags &= ~PF_FROZEN;
> else
>
I have nothing against that rename, but you take care of pushing that
yourselves. Tell Linus I okayed it.
Pavel
-- When do you have a heart between your knees? [Johanka's followup: and *two* hearts?] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/