From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.6 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E4879C433DF for ; Tue, 9 Jun 2020 08:35:54 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id B9DC9207ED for ; Tue, 9 Jun 2020 08:35:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1591691754; bh=JAspZs5Ru9i8zsF1Luz8OtOT1JFIDt+vTMcdcHBI0I4=; h=Date:From:To:Cc:Subject:References:In-Reply-To:List-ID:From; b=YN8gsTtGp9BalV/864v/Qp+U8SCewqPUWgD/qoRkHvvou0CZgsKwc3JCjq+x5Qfos G0AmE9YCIS8QVpmhMpXPp7rJsYpCx7iGnBlnbW5uC04aK7FQ6hR/kVxJCbU5TkrkFd E2IZ9r00MFaiN3LnKf5iCbvL3CUfO1IBKbzpvIgQ= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728098AbgFIIfk (ORCPT ); Tue, 9 Jun 2020 04:35:40 -0400 Received: from mail.kernel.org ([198.145.29.99]:33632 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726463AbgFIIfk (ORCPT ); Tue, 9 Jun 2020 04:35:40 -0400 Received: from pali.im (pali.im [31.31.79.79]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 9B59B20737; Tue, 9 Jun 2020 08:35:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1591691738; bh=JAspZs5Ru9i8zsF1Luz8OtOT1JFIDt+vTMcdcHBI0I4=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=KqvCpCz/b13WSCGzKw9vbrGW5Yu6oykakpg1pqzo5OcgLxrhKUsukm5ua19vlzmgF feRVXjGmkU6DjTz/68hoDRau2WHgCTFkPblLzAzk6//ZR5E9strqoMF3vipHVQssaV W+9uaDsvHPFhOfR5c5C4sJqT9KTenPJOC/cAGNNs= Received: by pali.im (Postfix) id D1EA8884; Tue, 9 Jun 2020 10:35:36 +0200 (CEST) Date: Tue, 9 Jun 2020 10:35:36 +0200 From: Pali =?utf-8?B?Um9ow6Fy?= To: Y Paritcher Cc: Randy Dunlap , linux-kernel@vger.kernel.org, platform-driver-x86@vger.kernel.org, Matthew Garrett , Mario.Limonciello@dell.com Subject: Re: [PATCH v2 3/3] platform/x86: dell-wmi: add new dmi keys to bios_to_linux_keycode Message-ID: <20200609083536.y4xhdrkydfwam4mk@pali> References: <8053252a-83ad-bcaa-2830-ccfbca1b4152@infradead.org> <20200608235508.wthtgilgmifwfgz2@pali> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: NeoMutt/20180716 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Monday 08 June 2020 20:43:45 Y Paritcher wrote: > On 6/8/20 7:55 PM, Pali Rohár wrote: > > Hello! > > > > On Monday 08 June 2020 16:27:10 Randy Dunlap wrote: > >> Hi-- > >> > >> On 6/8/20 4:05 PM, Y Paritcher wrote: > >>> Increase length of bios_to_linux_keycode to 2 bytes (the true size of a > >>> keycode) to allow for a new keycode 0xffff, this silences the following > >>> messages being logged at startup on a Dell Inspiron 5593: > >>> > >>> dell_wmi: firmware scancode 0x48 maps to unrecognized keycode 0xffff > >>> dell_wmi: firmware scancode 0x50 maps to unrecognized keycode 0xffff > > > > Which keys generate these two scancodes? Or how have you been able to > > trigger these scancodes (in case they are not generated by key press)? > > > > It is important to know for which key or event or feature we need to > > include this patch and therefore what feature is currently > > non-functional on that laptop. > > > > As I said before: > The DMI contains a table of firmware scancode to linux keycode mappings. > this is parsed at boot and used together with the bios_to_linux_keycode > entries & dell_wmi_keymap_type_ tables to create a keymap. > > If a DMI entry does not have a corresponding entry in bios_to_linux_keycode > we log a message to allow adding the correct linux keycode if known. > This is regardless of if the key actually exists on the device. > > To date, I have not been able to generate this keycode on my computer. Ok, so you have just these bios scan codes in your DMI table, but you do not know what they means nor if you can trigger them somehow. You should include this information into commit message. It is possible that your particular laptop model does not have physically keys which can trigger these codes... (Just guessing) > >>> as per this code comment: > >>> > >>> Log if we find an entry in the DMI table that we don't > >>> understand. If this happens, we should figure out what > >>> the entry means and add it to bios_to_linux_keycode. > >>> > >>> These are keycodes included in the 0xB2 DMI table, for which the > >>> corosponding keys are not known. > >> > >> corresponding > >> > >>> > >>> Now when a user will encounter this key, a proper message wil be printed: > >>> > >>> dell_wmi: Unknown key with type 0xXXXX and code 0xXXXX pressed > >>> > >>> This will then allow the key to be identified properly. > >>> > >>> Signed-off-by: Y Paritcher > >>> --- > >>> drivers/platform/x86/dell-wmi.c | 8 +++----- > >>> 1 file changed, 3 insertions(+), 5 deletions(-) > >>> > >>> diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c > >>> index 6b510f8431a3..dae1db96b5a0 100644 > >>> --- a/drivers/platform/x86/dell-wmi.c > >>> +++ b/drivers/platform/x86/dell-wmi.c > >>> @@ -196,7 +196,7 @@ struct dell_dmi_results { > >>> }; > >>> > >>> /* Uninitialized entries here are KEY_RESERVED == 0. */ > >>> -static const u16 bios_to_linux_keycode[256] = { > >>> +static const u16 bios_to_linux_keycode[65536] = { > >> > >> It surely seems odd to me to expand an array from 512 bytes to 128 Kbytes > >> just to handle one special case. Can't it be handled in code as a > >> special case? > > > > I already wrote that more developers would not be happy about this > > change. I would rather to see e.g. that Randy's suggestion with 0xffff > > check as increasing memory usage. > > > > Will change > > >>> [0] = KEY_MEDIA, > >>> [1] = KEY_NEXTSONG, > >>> [2] = KEY_PLAYPAUSE, > >>> @@ -237,6 +237,7 @@ static const u16 bios_to_linux_keycode[256] = { > >>> [37] = KEY_UNKNOWN, > >>> [38] = KEY_MICMUTE, > >>> [255] = KEY_PROG3, > >>> + [65535] = KEY_UNKNOWN, > > > > Looking at the last two lines... and for me it looks like that 0x00FF > > and 0xFFFF are just "placeholders" or special values for unknown / > > custom / unsupported / reserved / special / ... codes. > > > > Probably so, but i have no way of knowing. This was question for Mario as he is probably the only person who can bring some light to this area. > I just don't think there is a point spamming a users log with info that > they can't do anything with. If this is turned into a debug print then > i don't care to leave this as is, i had thought this might be helpful > just to know that this keycode mapping appears in the wild. > > > It is really suspicious why first 38 values are defined, then there is > > gap, then one value 255 and then huge gap to 65535. > > > > Mario, this looks like some mapping table between internal Dell BIOS key > > code and standard Linux key code. Are you able to get access to some > > documentation which contains explanation of those Dell key numbers? > > It could really help us to understand these gaps and what is correct > > interpretation of these numbers. > > > > E.g. I remember that pressing Fn+Q or Fn+W on some Dell Latitude > > generates code 255, which could prove my thesis about "special codes" > > (which are probably not found in e.g. Windows or Linux mapping tables). > > > >>> }; > >>> > >>> /* > >>> @@ -503,10 +504,7 @@ static void handle_dmi_entry(const struct dmi_header *dm, void *opaque) > >>> &table->keymap[i]; > >>> > >>> /* Uninitialized entries are 0 aka KEY_RESERVED. */ > >>> - u16 keycode = (bios_entry->keycode < > >>> - ARRAY_SIZE(bios_to_linux_keycode)) ? > >>> - bios_to_linux_keycode[bios_entry->keycode] : > >>> - KEY_RESERVED; > >>> + u16 keycode = bios_to_linux_keycode[bios_entry->keycode]; > >>> > >>> /* > >>> * Log if we find an entry in the DMI table that we don't > >>> > >> > >> Something like: > >> > >> u16 keycode; > >> > >> keycode = bios_entry->keycode == 0xffff ? KEY_UNKNOWN : > >> (bios_entry->keycode < > >> ARRAY_SIZE(bios_to_linux_keycode)) ? > >> bios_to_linux_keycode[bios_entry->keycode] : > >> KEY_RESERVED; > >> > >> > >> > >> Also please fix this: > >> (no To-header on input) <> > > > > Hint: specifying git send-email with '--to' argument instead of '--cc' > > should help. > > > > Sorry about that. > >> > >> -- > >> ~Randy > >>