On Tue, Feb 27, 2018 at 11:15 PM, Michał Kępień <kernel@xxxxxxxxxx> 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