Re: [PATCH v3 1/3] Input/i8042: Merge quirk tables

From: Hans de Goede
Date: Wed Mar 09 2022 - 12:18:15 EST

Next message: nicolas saenz julienne: "Re: [PATCH 16/19] context_tracking: Convert state to atomic_t"
Previous message: David Rientjes: "Re: [PATCH 2/2] mm: slub: Delete useless parameter of alloc_slab_page()"
In reply to: Werner Sembach: "[PATCH v3 1/3] Input/i8042: Merge quirk tables"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi,

On 3/8/22 18:05, Werner Sembach wrote:
> Merge i8042 quirk tables to reduce code duplication for devices that need
> more than one quirk.
>
> Also align quirkable options with command line parameters and make vendor
> wide quirks per device overwriteable on a per device basis.
>
> Some duplication on the ASUS devices is required to mirror the exact
> behaviour of the previous code.

Can you explain this a bit more ?

>
> Signed-off-by: Werner Sembach <wse@xxxxxxxxxxxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> ---
> drivers/input/serio/i8042-x86ia64io.h | 1125 ++++++++++++++-----------
> 1 file changed, 640 insertions(+), 485 deletions(-)
>
> diff --git a/drivers/input/serio/i8042-x86ia64io.h b/drivers/input/serio/i8042-x86ia64io.h
> index 148a7c5fd0e2..689b9ee3e742 100644
> --- a/drivers/input/serio/i8042-x86ia64io.h
> +++ b/drivers/input/serio/i8042-x86ia64io.h
> @@ -67,675 +67,821 @@ static inline void i8042_write_command(int val)
>
> #include <linux/dmi.h>
>
> -static const struct dmi_system_id __initconst i8042_dmi_noloop_table[] = {
> +#define SERIO_QUIRK_NOKBD BIT(0)
> +#define SERIO_QUIRK_NOAUX BIT(1)
> +#define SERIO_QUIRK_NOMUX BIT(2)
> +#define SERIO_QUIRK_FORCEMUX BIT(3)
> +#define SERIO_QUIRK_UNLOCK BIT(4)
> +#define SERIO_QUIRK_PROBE_DEFER BIT(5)
> +#define SERIO_QUIRK_RESET_ALWAYS BIT(6)
> +#define SERIO_QUIRK_RESET_NEVER BIT(7)
> +#define SERIO_QUIRK_DIECT BIT(8)
> +#define SERIO_QUIRK_DUMBKBD BIT(9)
> +#define SERIO_QUIRK_NOLOOP BIT(10)
> +#define SERIO_QUIRK_NOTIMEOUT BIT(11)
> +#define SERIO_QUIRK_KBDRESET BIT(12)
> +#define SERIO_QUIRK_DRITEK BIT(13)
> +#define SERIO_QUIRK_NOPNP BIT(14)
> +
> +/* Quirk table for different mainboards. Options similar or identical to i8042
> + * module parameters.
> + * ORDERING IS IMPORTANT! The first match will be apllied and the rest ignored.
> + * This allows entries to overwrite vendor wide quirks on a per device basis.
> + * Where this is irrelevant, entries are sorted case sensitive by DMI_SYS_VENDOR
> + * and/or DMI_BOARD_VENDOR to make it easier to avoid dublicate entries.
> + */
> +static const struct dmi_system_id i8042_dmi_quirk_table[] __initconst = {

<snip>

> @@ -1167,11 +1307,6 @@ static int __init i8042_pnp_init(void)
> bool pnp_data_busted = false;
> int err;
>
> -#ifdef CONFIG_X86
> - if (dmi_check_system(i8042_dmi_nopnp_table))
> - i8042_nopnp = true;
> -#endif
> -
> if (i8042_nopnp) {
> pr_info("PNP detection disabled\n");
> return 0;

have you checked that i8042_platform_init() *always*
gets called before i8042_pnp_init()?

Maybe just add something like this:

#ifdef CONFIG_X86
static void __init i8042_check_quirks(void)
{
const struct dmi_system_id *device_quirk_info;
uintptr_t quirks;

device_quirk_info = dmi_first_match(i8042_dmi_quirk_table);
if (!device_quirk_info)
return;

quirks = (uintptr_t)device_quirk_info->driver_data;

if (i8042_reset == I8042_RESET_DEFAULT) {
if (quirks & SERIO_QUIRK_RESET)
i8042_reset = I8042_RESET_ALWAYS;
if (quirks & SERIO_QUIRK_NOSELFTEST)
i8042_reset = I8042_RESET_NEVER;
}

/* Do more quirk checks */
}
#else
static inline void i8042_check_quirks(void) {}
#endif

(above the declaration of i8042_pnp_init())

And call i8042_check_quirks() in both
i8042_platform_init() and i8042_platform_init() ?

This also abstracts some of the CONFIG_X86
ifdef-ery out of the other functions.

> @@ -1277,6 +1412,9 @@ static inline void i8042_pnp_exit(void) { }
>
> static int __init i8042_platform_init(void)
> {
> + bool i8042_reset_always_quirk = false;
> + bool i8042_reset_never_quirk = false;

I'm not a fan of these 2 helper variables, you can do this directly,
see above.

> + const struct dmi_system_id *device_quirk_info;

All 3 these variables will trigger unused variable
settings when compiling without CONFIG_X86 set. Note
introducing the i8042_check_quirks() helper as I suggest
above avoids this without needing more #ifdef-ery.

> int retval;
>
> #ifdef CONFIG_X86
> @@ -1297,6 +1435,44 @@ static int __init i8042_platform_init(void)
> i8042_kbd_irq = I8042_MAP_IRQ(1);
> i8042_aux_irq = I8042_MAP_IRQ(12);
>
> +#ifdef CONFIG_X86
> + device_quirk_info = dmi_first_match(i8042_dmi_quirk_table);
> + if (device_quirk_info) {
> + if ((uintptr_t)device_quirk_info->driver_data & SERIO_QUIRK_NOKBD)
> + i8042_nokbd = true;
> + if ((uintptr_t)device_quirk_info->driver_data & SERIO_QUIRK_NOAUX)
> + i8042_noaux = true;
> + if ((uintptr_t)device_quirk_info->driver_data & SERIO_QUIRK_NOMUX)
> + i8042_nomux = true;
> + if ((uintptr_t)device_quirk_info->driver_data & SERIO_QUIRK_FORCEMUX)
> + i8042_nomux = false;
> + if ((uintptr_t)device_quirk_info->driver_data & SERIO_QUIRK_UNLOCK)
> + i8042_unlock = true;
> + if ((uintptr_t)device_quirk_info->driver_data & SERIO_QUIRK_PROBE_DEFER)
> + i8042_probe_defer = true;
> + if ((uintptr_t)device_quirk_info->driver_data & SERIO_QUIRK_RESET_ALWAYS)
> + i8042_reset_always_quirk = true;
> + if ((uintptr_t)device_quirk_info->driver_data & SERIO_QUIRK_RESET_NEVER)
> + i8042_reset_never_quirk = true;
> + if ((uintptr_t)device_quirk_info->driver_data & SERIO_QUIRK_DIECT)
> + i8042_direct = true;
> + if ((uintptr_t)device_quirk_info->driver_data & SERIO_QUIRK_DUMBKBD)
> + i8042_dumbkbd = true;
> + if ((uintptr_t)device_quirk_info->driver_data & SERIO_QUIRK_NOLOOP)
> + i8042_noloop = true;
> + if ((uintptr_t)device_quirk_info->driver_data & SERIO_QUIRK_NOTIMEOUT)
> + i8042_notimeout = true;
> + if ((uintptr_t)device_quirk_info->driver_data & SERIO_QUIRK_KBDRESET)
> + i8042_kbdreset = true;
> + if ((uintptr_t)device_quirk_info->driver_data & SERIO_QUIRK_DRITEK)
> + i8042_dritek = true;
> +#ifdef CONFIG_PNP
> + if ((uintptr_t)device_quirk_info->driver_data & SERIO_QUIRK_NOPNP)
> + i8042_nopnp = true;
> +#endif
> + }
> +#endif
> +
> retval = i8042_pnp_init();
> if (retval)
> return retval;
> @@ -1308,34 +1484,13 @@ static int __init i8042_platform_init(void)
> #ifdef CONFIG_X86
> /* Honor module parameter when value is not default */
> if (i8042_reset == I8042_RESET_DEFAULT) {
> - if (dmi_check_system(i8042_dmi_reset_table))
> + if (i8042_reset_always_quirk)
> i8042_reset = I8042_RESET_ALWAYS;
>
> - if (dmi_check_system(i8042_dmi_noselftest_table))
> + if (i8042_reset_never_quirk)
> i8042_reset = I8042_RESET_NEVER;
> }
>
> - if (dmi_check_system(i8042_dmi_noloop_table))
> - i8042_noloop = true;
> -
> - if (dmi_check_system(i8042_dmi_nomux_table))
> - i8042_nomux = true;
> -
> - if (dmi_check_system(i8042_dmi_forcemux_table))
> - i8042_nomux = false;
> -
> - if (dmi_check_system(i8042_dmi_notimeout_table))
> - i8042_notimeout = true;
> -
> - if (dmi_check_system(i8042_dmi_dritek_table))
> - i8042_dritek = true;
> -
> - if (dmi_check_system(i8042_dmi_kbdreset_table))
> - i8042_kbdreset = true;
> -
> - if (dmi_check_system(i8042_dmi_probe_defer_table))
> - i8042_probe_defer = true;
> -
> /*
> * A20 was already enabled during early kernel init. But some buggy
> * BIOSes (in MSI Laptops) require A20 to be enabled using 8042 to

Regards,

Hans

Next message: nicolas saenz julienne: "Re: [PATCH 16/19] context_tracking: Convert state to atomic_t"
Previous message: David Rientjes: "Re: [PATCH 2/2] mm: slub: Delete useless parameter of alloc_slab_page()"
In reply to: Werner Sembach: "[PATCH v3 1/3] Input/i8042: Merge quirk tables"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]