Re: [bug report] Input: atkbd - restore repeat rate when resuming

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

Hi Dan,

On Mon, Mar 03, 2025 at 11:11:35AM +0300, Dan Carpenter wrote:
> [ This bug seems really ancient.  But so far as I can see it really is
>   super old.  I'm not sure why it's only showing up now.  -dan ]
> 
> Hello Dmitry Torokhov,
> 
> Commit 3d0f0fa0cb55 ("Input: atkbd - restore repeat rate when
> resuming") from Aug 4, 2006 (linux-next), leads to the following
> Smatch static checker warning:
> 
> 	drivers/input/keyboard/atkbd.c:604 atkbd_set_repeat_rate()
> 	warn: sleeping in atomic context
> 
> drivers/input/keyboard/atkbd.c
>     583 static int atkbd_set_repeat_rate(struct atkbd *atkbd)
>     584 {
>     585         const short period[32] =
>     586                 { 33,  37,  42,  46,  50,  54,  58,  63,  67,  75,  83,  92, 100, 109, 116, 125,
>     587                  133, 149, 167, 182, 200, 217, 232, 250, 270, 303, 333, 370, 400, 435, 470, 500 };
>     588         const short delay[4] =
>     589                 { 250, 500, 750, 1000 };
>     590 
>     591         struct input_dev *dev = atkbd->dev;
>     592         unsigned char param;
>     593         int i = 0, j = 0;
>     594 
>     595         while (i < ARRAY_SIZE(period) - 1 && period[i] < dev->rep[REP_PERIOD])
>     596                 i++;
>     597         dev->rep[REP_PERIOD] = period[i];
>     598 
>     599         while (j < ARRAY_SIZE(delay) - 1 && delay[j] < dev->rep[REP_DELAY])
>     600                 j++;
>     601         dev->rep[REP_DELAY] = delay[j];
>     602 
>     603         param = i | (j << 5);
> --> 604         return ps2_command(&atkbd->ps2dev, &param, ATKBD_CMD_SETREP);
> 
> This fucntion call takes a mutex.
> 
>     605 }
> 
> The call tree is:
> 
> atkbd_reconnect() <- disables preempt
> -> atkbd_set_repeat_rate()
> 
> In atkbd_reconnect() it's the atkbd_disable(atkbd) which takes a
> spinlock.

I think your tools are confused by the guard notation. atkbd_disable()
not only takes but also releases the spinlock.

Thanks.

-- 
Dmitry