Re: [PATCH v2] dell-wmi: process Dell Instant Launch hotkey on Dell Vostro V131

From: MichaÅ KÄpieÅ
Date: Fri Dec 04 2015 - 07:36:59 EST


> > --- a/drivers/platform/x86/dell-wmi.c
> > +++ b/drivers/platform/x86/dell-wmi.c
> > @@ -47,6 +47,38 @@ static int acpi_video;
> >
> > MODULE_ALIAS("wmi:"DELL_EVENT_GUID);
> >
> > +struct quirk_entry {
> > + bool process_dell_instant_launch;
> > +};
> > +
> > +static struct quirk_entry *quirks;
> > +
> > +static struct quirk_entry quirk_unknown = {
> > +};
> > +
> > +static struct quirk_entry quirk_dell_vostro_v131 = {
> > + .process_dell_instant_launch = true,
> > +};
> > +
> > +static int __init dmi_matched(const struct dmi_system_id *dmi)
> > +{
> > + quirks = dmi->driver_data;
> > + return 1;
> > +}
> > +
> > +static const struct dmi_system_id dell_wmi_quirks[] __initconst = {
> > + {
> > + .callback = dmi_matched,
> > + .ident = "Dell Vostro V131",
> > + .matches = {
> > + DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> > + DMI_MATCH(DMI_PRODUCT_NAME, "Vostro V131"),
> > + },
> > + .driver_data = &quirk_dell_vostro_v131,
> > + },
> > + { }
> > +};
> > +
>
> It is not possible to simplify this part of code? Currently we only need
> boolean variable: ignore 0xe025 event or not. I think that whole quirk
> structure is not needed yet (and I would be happy if never in future).

Of course it is possible. I just got the feeling that using a quirk
structure is the way to go for this subsystem as it currently contains 7
drivers using something like this. Moreover, I saw that in some commits
initially adding a quirk structure to a driver (2d8b90be, a979e2e2) that
structure contained just one field.

So, between KISS and following the beaten path, I chose the latter. If
you still think this patch should be reworked to use a single global
boolean instead, please let me know and I'll prepare a v3.

> > @@ -432,6 +467,8 @@ static int __init dell_wmi_init(void)
> > return -ENODEV;
> > }
> >
> > + quirks = &quirk_unknown;
>
> Unknown sounds like something is not know or we do not know what it is.
> But here we know exactly what is needed (= ignore 0xe025 key).

Again, not my idea, I just thought it would be wise to follow what seems
to be an established pattern:

$ git grep 'quirk.*unknown' drivers/platform/x86/

--
Best regards,
MichaÅ KÄpieÅ
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/