Date: Fri, 4 Dec 2015 09:48:06 +0100
From: Pali =?utf-8?B?Um9ow6Fy?= <pali.rohar@gmail.com>
To: =?utf-8?B?TWljaGHFgiBLxJlwaWXFhA==?= <kernel@kempniu.pl>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>, Darren Hart <dvhart@infradead.org>,
        platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] dell-wmi: process Dell Instant Launch hotkey on Dell
 Vostro V131
Message-ID: <20151204084806.GM10982@pali>
References: <20151130211542.GE30553@malice.jf.intel.com>
 <1654e7bcde98f1cf89f698a1e359110c06c6fcd4.1448999372.git.kernel@kempniu.pl>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <1654e7bcde98f1cf89f698a1e359110c06c6fcd4.1448999372.git.kernel@kempniu.pl>
User-Agent: Mutt/1.5.23.1 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3761
Lines: 111

On Tuesday 01 December 2015 20:51:34 Michał Kępień wrote:
> On some laptop models (e.g. Dell Vostro V131), pressing the Dell Instant
> Launch hotkey does not raise an i8042 interrupt - only WMI event 0xe025
> is generated.  As there seems to be no ACPI method or SMI call to
> determine without possible side effects whether a given machine is
> capable of simulating a keypress when this hotkey is pressed, DMI
> matching is used to whitelist the models for which an input event should
> be generated when WMI event 0xe025 is received.
> 
> Signed-off-by: Michał Kępień <kernel@kempniu.pl>
> ---
> Changes from v1:
> 
>   - Use DMI matching instead of a module parameter
>   - Change flag name to improve readability
> 
> Darren, I am aware this patch conflicts with Andy's WMI rework series
> posted yesterday, so please just let me know if you want me to rebase.
> 
>  drivers/platform/x86/dell-wmi.c |   39 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 38 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
> index 8cb0f57..27c8f49 100644
> --- 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).

>  /*
>   * Certain keys are flagged as KE_IGNORE. All of these are either
>   * notifications (rather than requests for change) or are also sent
> @@ -87,7 +119,7 @@ static const struct key_entry dell_wmi_legacy_keymap[] __initconst = {
>  	{ KE_IGNORE, 0xe020, { KEY_MUTE } },
>  
>  	/* Shortcut and audio panel keys */
> -	{ KE_IGNORE, 0xe025, { KEY_RESERVED } },
> +	{ KE_KEY, 0xe025, { KEY_PROG4 } },
>  	{ KE_IGNORE, 0xe026, { KEY_RESERVED } },
>  
>  	{ KE_IGNORE, 0xe02e, { KEY_VOLUMEDOWN } },
> @@ -183,6 +215,9 @@ static void dell_wmi_process_key(int reported_key)
>  	     key->keycode == KEY_BRIGHTNESSDOWN) && acpi_video)
>  		return;
>  
> +	if (key->keycode == KEY_PROG4 && !quirks->process_dell_instant_launch)
> +		return;
> +
>  	sparse_keymap_report_entry(dell_wmi_input_dev, key, 1, true);
>  }
>  
> @@ -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).

> +	dmi_check_system(dell_wmi_quirks);
>  	dmi_walk(find_hk_type, NULL);
>  	acpi_video = acpi_video_get_backlight_type() != acpi_backlight_vendor;
>  

-- 
Pali Rohár
pali.rohar@gmail.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/