lkml.org 
[lkml]   [2018]   [Feb]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH 6/7] platform/x86: fujitsu-laptop: More accurately represent the hotkey ring buffer managed by firmware
On Tue, Feb 27, 2018 at 11:15 PM, Michał Kępień <kernel@kempniu.pl> wrote:
> The MAX_HOTKEY_RINGBUFFER_SIZE constant is set to 100, which allows up
> to 100 hotkey events to be drained from the firmware ring buffer upon
> module load. However, no known firmware is capable of holding more than
> 16 hotkey events in its internal ring buffer:

> The RINGBUFFERSIZE constant is set to 40, which allows the module to
> queue up to 40 hotkey events for delivery to user space. As this value
> seems arbitrarily chosen and 16 should be more than enough for any
> practical use case, merge the two aforementioned constants into one
> (HOTKEY_RINGBUFFER_SIZE) in order to simplify code and more accurately
> represent the underlying data structure.

> +#define HOTKEY_RINGBUFFER_SIZE 16

This need the comment or a

BUILD_BUG_ON(!is_power_of_2(...));


> - ret = kfifo_alloc(&priv->fifo, RINGBUFFERSIZE * sizeof(int),
> + ret = kfifo_alloc(&priv->fifo, HOTKEY_RINGBUFFER_SIZE * sizeof(int),
> GFP_KERNEL);

> while (call_fext_func(device, FUNC_BUTTONS, OP_GET_EVENTS,
> 0x0, 0x0) != 0 &&
> - i++ < MAX_HOTKEY_RINGBUFFER_SIZE)
> + i++ < HOTKEY_RINGBUFFER_SIZE)
> ; /* No action, result is discarded */

This looks horrible.
Can it be redone

do {
if (call...() == 0)
break;
} while (i++ < ...);

?

> while ((irb = call_fext_func(device, FUNC_BUTTONS, OP_GET_EVENTS,
> 0x0, 0x0)) != 0 &&
> - i++ < MAX_HOTKEY_RINGBUFFER_SIZE) {
> + i++ < HOTKEY_RINGBUFFER_SIZE) {

Ditto.

> scancode = irb & 0x4ff;
> if (sparse_keymap_entry_from_scancode(priv->input, scancode))
> acpi_fujitsu_laptop_press(device, scancode);


--
With Best Regards,
Andy Shevchenko

\
 
 \ /
  Last update: 2018-02-28 17:35    [W:0.215 / U:0.912 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site