[PATCH] atkbd: restore autorepeat rate after resume

[PATCH] atkbd: restore autorepeat rate after resume

[Alan Stern]

From: Linus Torvalds <torvalds@osdl.org>

This patch (as728) makes the AT keyboard driver store the current
autorepeat rate so that it can be restored properly following a 
suspend/resume cycle.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>

---

Index: usb-2.6/drivers/input/keyboard/atkbd.c
===================================================================
--- usb-2.6.orig/drivers/input/keyboard/atkbd.c
+++ usb-2.6/drivers/input/keyboard/atkbd.c
@@ -39,6 +39,8 @@ static int atkbd_set = 2;
 module_param_named(set, atkbd_set, int, 0);
 MODULE_PARM_DESC(set, "Select keyboard code set (2 = default, 3 = PS/2 native)");
 
+static int atkbd_repeatrate;
+
 #if defined(__i386__) || defined(__x86_64__) || defined(__hppa__)
 static int atkbd_reset;
 #else
@@ -477,7 +479,7 @@ static void atkbd_event_work(void *data)
 			j++;
 		dev->rep[REP_PERIOD] = period[i];
 		dev->rep[REP_DELAY] = delay[j];
-		param[0] = i | (j << 5);
+		param[0] = atkbd_repeatrate = i | (j << 5);
 		ps2_command(&atkbd->ps2dev, param, ATKBD_CMD_SETREP);
 	}
 
@@ -676,10 +678,10 @@ static int atkbd_activate(struct atkbd *
 		return -1;
 
 /*
- * Set autorepeat to fastest possible.
+ * Set autorepeat to whatever it used to be.
  */
 
-	param[0] = 0;
+	param[0] = atkbd_repeatrate;
 	if (ps2_command(ps2dev, param, ATKBD_CMD_SETREP))
 		return -1;
 

Re: [PATCH] atkbd: restore autorepeat rate after resume

[Dmitry Torokhov]

On 6/26/06, Alan Stern <stern@rowland.harvard.edu> wrote:
> From: Linus Torvalds <torvalds@osdl.org>
>
> This patch (as728) makes the AT keyboard driver store the current
> autorepeat rate so that it can be restored properly following a
> suspend/resume cycle.
>

Alan,

I think it should be a per-device, not global parameter. Anyway, I'll
adjust adn apply, thank you.

-- 
Dmitry

Re: [PATCH] atkbd: restore autorepeat rate after resume

[Vojtech Pavlik]

On Mon, Jun 26, 2006 at 10:21:29AM -0400, Alan Stern wrote:
> From: Linus Torvalds <torvalds@osdl.org>
> 
> This patch (as728) makes the AT keyboard driver store the current
> autorepeat rate so that it can be restored properly following a 
> suspend/resume cycle.
> 
> Signed-off-by: Alan Stern <stern@rowland.harvard.edu>

IMO this should be done in keyboard.c, similar to the LED setting upon
keyboard plug-in.

> ---
> 
> Index: usb-2.6/drivers/input/keyboard/atkbd.c
> ===================================================================
> --- usb-2.6.orig/drivers/input/keyboard/atkbd.c
> +++ usb-2.6/drivers/input/keyboard/atkbd.c
> @@ -39,6 +39,8 @@ static int atkbd_set = 2;
>  module_param_named(set, atkbd_set, int, 0);
>  MODULE_PARM_DESC(set, "Select keyboard code set (2 = default, 3 = PS/2 native)");
>  
> +static int atkbd_repeatrate;
> +
>  #if defined(__i386__) || defined(__x86_64__) || defined(__hppa__)
>  static int atkbd_reset;
>  #else
> @@ -477,7 +479,7 @@ static void atkbd_event_work(void *data)
>  			j++;
>  		dev->rep[REP_PERIOD] = period[i];
>  		dev->rep[REP_DELAY] = delay[j];
> -		param[0] = i | (j << 5);
> +		param[0] = atkbd_repeatrate = i | (j << 5);
>  		ps2_command(&atkbd->ps2dev, param, ATKBD_CMD_SETREP);
>  	}
>  
> @@ -676,10 +678,10 @@ static int atkbd_activate(struct atkbd *
>  		return -1;
>  
>  /*
> - * Set autorepeat to fastest possible.
> + * Set autorepeat to whatever it used to be.
>   */
>  
> -	param[0] = 0;
> +	param[0] = atkbd_repeatrate;
>  	if (ps2_command(ps2dev, param, ATKBD_CMD_SETREP))
>  		return -1;
>  
> 
> 
> 

-- 
Vojtech Pavlik
Director SuSE Labs

Re: [PATCH] atkbd: restore autorepeat rate after resume

[Vojtech Pavlik]

On Mon, Jun 26, 2006 at 10:35:44AM -0400, Dmitry Torokhov wrote:
> On 6/26/06, Alan Stern <stern@rowland.harvard.edu> wrote:
> >From: Linus Torvalds <torvalds@osdl.org>
> >
> >This patch (as728) makes the AT keyboard driver store the current
> >autorepeat rate so that it can be restored properly following a
> >suspend/resume cycle.
> >
> 
> Alan,
> 
> I think it should be a per-device, not global parameter. Anyway, I'll
> adjust adn apply, thank you.
 
You can't make it per-device when there is no device when the keyboard
isn't plugged in. ;)

-- 
Vojtech Pavlik
Director SuSE Labs

Re: [PATCH] atkbd: restore autorepeat rate after resume

[Dmitry Torokhov]

On 6/26/06, Vojtech Pavlik <vojtech@suse.cz> wrote:
> On Mon, Jun 26, 2006 at 10:35:44AM -0400, Dmitry Torokhov wrote:
> > On 6/26/06, Alan Stern <stern@rowland.harvard.edu> wrote:
> > >From: Linus Torvalds <torvalds@osdl.org>
> > >
> > >This patch (as728) makes the AT keyboard driver store the current
> > >autorepeat rate so that it can be restored properly following a
> > >suspend/resume cycle.
> > >
> >
> > Alan,
> >
> > I think it should be a per-device, not global parameter. Anyway, I'll
> > adjust adn apply, thank you.
>
> You can't make it per-device when there is no device when the keyboard
> isn't plugged in. ;)
>

It there is no keyboard then you could not change repeat rate before
suspending and we don't have anyhting to restore ;)

-- 
Dmitry

Re: [PATCH] atkbd: restore autorepeat rate after resume

[Vojtech Pavlik]

On Mon, Jun 26, 2006 at 10:58:46AM -0400, Dmitry Torokhov wrote:
> On 6/26/06, Vojtech Pavlik <vojtech@suse.cz> wrote:
> >On Mon, Jun 26, 2006 at 10:35:44AM -0400, Dmitry Torokhov wrote:
> >> On 6/26/06, Alan Stern <stern@rowland.harvard.edu> wrote:
> >> >From: Linus Torvalds <torvalds@osdl.org>
> >> >
> >> >This patch (as728) makes the AT keyboard driver store the current
> >> >autorepeat rate so that it can be restored properly following a
> >> >suspend/resume cycle.
> >> >
> >>
> >> Alan,
> >>
> >> I think it should be a per-device, not global parameter. Anyway, I'll
> >> adjust adn apply, thank you.
> >
> >You can't make it per-device when there is no device when the keyboard
> >isn't plugged in. ;)
> 
> It there is no keyboard then you could not change repeat rate before
> suspending and we don't have anyhting to restore ;)
 
What the patch is trying to achieve is that you have the keyboard, set
the rate, unplug the keyboard, replug the keyboard, get the original
setting.

In the middle of the process, you have no device to attach the
information to. That's why the patch uses a global variable.

-- 
Vojtech Pavlik
Director SuSE Labs

Re: [PATCH] atkbd: restore autorepeat rate after resume

[Dmitry Torokhov]

On 6/26/06, Vojtech Pavlik <vojtech@suse.cz> wrote:
> On Mon, Jun 26, 2006 at 10:58:46AM -0400, Dmitry Torokhov wrote:
> > On 6/26/06, Vojtech Pavlik <vojtech@suse.cz> wrote:
> > >On Mon, Jun 26, 2006 at 10:35:44AM -0400, Dmitry Torokhov wrote:
> > >> On 6/26/06, Alan Stern <stern@rowland.harvard.edu> wrote:
> > >> >From: Linus Torvalds <torvalds@osdl.org>
> > >> >
> > >> >This patch (as728) makes the AT keyboard driver store the current
> > >> >autorepeat rate so that it can be restored properly following a
> > >> >suspend/resume cycle.
> > >> >
> > >>
> > >> Alan,
> > >>
> > >> I think it should be a per-device, not global parameter. Anyway, I'll
> > >> adjust adn apply, thank you.
> > >
> > >You can't make it per-device when there is no device when the keyboard
> > >isn't plugged in. ;)
> >
> > It there is no keyboard then you could not change repeat rate before
> > suspending and we don't have anyhting to restore ;)
>
> What the patch is trying to achieve is that you have the keyboard, set
> the rate, unplug the keyboard, replug the keyboard, get the original
> setting.
>
> In the middle of the process, you have no device to attach the
> information to. That's why the patch uses a global variable.
>

The original complaint was that we don't keep repeat rate after
suspend/resume cycle. I think pulling the cord and then plugging it
back in is completely different scenario, but even then it will also
work because we do not destroy keyboard device when cord is pulled
(there is no notification that device is gone). So input_dev is still
there and we can use dev->rep[] to restore the former settings.

-- 
Dmitry

Re: [PATCH] atkbd: restore autorepeat rate after resume

[Alan Stern]

On Mon, 26 Jun 2006, Vojtech Pavlik wrote:

> > It there is no keyboard then you could not change repeat rate before
> > suspending and we don't have anyhting to restore ;)
>  
> What the patch is trying to achieve is that you have the keyboard, set
> the rate, unplug the keyboard, replug the keyboard, get the original
> setting.

Maybe that's what Linus (who originally wrote the patch) was trying to 
achieve.  But my intention was that you could have the keyboard, set the 
rate, then do suspend-to-disk, and afterwards still have the rate that you 
set.

Alan Stern