Re: 2.6.0-test6: APM unable to suspend (the 2.6.0-test2 saga continues)

[Vojtech Pavlik]

On Tue, Dec 30, 2003 at 08:45:35PM -0500, Dmitry Torokhov wrote:
> On Tuesday 30 December 2003 06:00 pm, Vojtech Pavlik wrote:
> > On Tue, Dec 30, 2003 at 07:53:03PM +0000, Russell King wrote:
> > > So it looks like i8042 could do with hooking some power management
> > > to disable this timer before suspend and resume it afterwards.
> > >
> > > Vojtech?
> >
> > Agreed. There should already be some in -mm kernels, and I'll make sure
> > the timer is deleted before suspend. Thanks for finding this.
> 
> What about something like the patch below? (Now, I don't suspend my notebook
> so it has not been tested, just compiled.)
> 
> Dmitry
> 

I like it, see comments.

> +/*
> + * Do not add extra 'i8042.' prefix to all parameters if compiled into the kernel
> + */
> +#undef MODULE_PARAM_PREFIX
> +#define MODULE_PARAM_PREFIX /* empty */
> +
>  static unsigned int i8042_noaux;
>  module_param(i8042_noaux, bool, 0);

Well, I think it might be cleaner to just drop the i8042_ prefix and go
with the "i8042." prefix if that's the 2.6 way. It'll annoy a couple
people, but's it's the way to go in the future.

> -	i8042_ctr = i8042_initial_ctr;
> -
> -	if (i8042_command(&i8042_ctr, I8042_CMD_CTL_WCTR))
> -		printk(KERN_WARNING "i8042.c: Can't restore CTR.\n");
> +static int i8042_controller_suspend(void)
> +{
> +	del_timer_sync(&i8042_timer);
> +	i8042_controller_reset();
>  
> +	return 0;
>  }

Ok here we remove the timer.

> +/*
> + * Restart timer (for autorepeats)
> + */ 
> +	mod_timer(&i8042_timer, jiffies + I8042_POLL_PERIOD);
>  

Note it's not for autorepeats. It's for problems with characters stuck
in the controller (like if the mouse sends a byte and the interface is
not enabled, blocking the controller indefinitely), we poll the
controller now and then making sure no characters are forgotten there.

> - * Resume handler for the new PM scheme (driver model)
> + * Suspend/resume handlers for the new PM scheme (driver model)
>   */
> +static int i8042_suspend(struct sys_device *dev, u32 state)
> +{
> +	return i8042_controller_suspend();
> +}
> +
>  static int i8042_resume(struct sys_device *dev)
>  {
>  	return i8042_controller_resume();
>  }
>  
>  static struct sysdev_class kbc_sysclass = {
> -       set_kset_name("i8042"),
> -       .resume = i8042_resume,
> +	set_kset_name("i8042"),
> +	.suspend = i8042_suspend,
> +	.resume = i8042_resume,
>  };

Good.

-- 
Vojtech Pavlik
SuSE Labs, SuSE CR
-
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/