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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20151204123650.GA12988@eudyptula.hq.kempniu.pl>
Date:	Fri, 4 Dec 2015 13:36:50 +0100
From:	Michał Kępień <kernel@...pniu.pl>
To:	Pali Rohár <pali.rohar@...il.com>
Cc:	Matthew Garrett <mjg59@...f.ucam.org>,
	Darren Hart <dvhart@...radead.org>,
	platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] dell-wmi: process Dell Instant Launch hotkey on Dell
 Vostro V131

> > --- 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/