Input: atkbd: Fix release quirk for Dell models
diff mbox

Message ID 1427639779-27377-1-git-send-email-pali.rohar@gmail.com
State New, archived
Headers show

Commit Message

Pali Rohár March 29, 2015, 2:36 p.m. UTC
This patch fixes commit 61579ba83934 ("Input: atkbd - expand Latitude's force
release quirk to other Dells"). Before that commit release quirks were called
for all Dell Latitude models. After that commit only for Portable Dell devices.
But lot of Latitude models are Laptop or Notebook DMI devices so quirks are not
called.

Release quirks are still needed also for new Dell Latitude models, so this patch
enables quirks for all Portable, Laptop, Notebook and Sub-Notebook Dell devices.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/input/keyboard/atkbd.c |   48 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

Comments

Pali Rohár April 4, 2015, 10:48 p.m. UTC | #1
On Sunday 29 March 2015 16:36:19 Pali Rohár wrote:
> This patch fixes commit 61579ba83934 ("Input: atkbd - expand
> Latitude's force release quirk to other Dells"). Before that
> commit release quirks were called for all Dell Latitude
> models. After that commit only for Portable Dell devices. But
> lot of Latitude models are Laptop or Notebook DMI devices so
> quirks are not called.
> 
> Release quirks are still needed also for new Dell Latitude
> models, so this patch enables quirks for all Portable,
> Laptop, Notebook and Sub-Notebook Dell devices.
> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>

Dmitry, Matthew: Can be this patch pushed to 4.0 and backported 
to stable releases? It fix regression caused by specified commit. 
I tested this patch on Latitude E6440 and now all keys are 
correctly released.
Dmitry Torokhov April 5, 2015, 9:48 p.m. UTC | #2
Hi Pali,

On Sun, Mar 29, 2015 at 04:36:19PM +0200, Pali Rohár wrote:
> This patch fixes commit 61579ba83934 ("Input: atkbd - expand Latitude's force
> release quirk to other Dells"). Before that commit release quirks were called
> for all Dell Latitude models. After that commit only for Portable Dell devices.
> But lot of Latitude models are Laptop or Notebook DMI devices so quirks are not
> called.
> 
> Release quirks are still needed also for new Dell Latitude models, so this patch
> enables quirks for all Portable, Laptop, Notebook and Sub-Notebook Dell devices.

Does Dell use all these types for their laptops? What models do you know
that need this quirk?

Thanks.

> 
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> ---
>  drivers/input/keyboard/atkbd.c |   48 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
> 
> diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
> index 387c51f..3188493 100644
> --- a/drivers/input/keyboard/atkbd.c
> +++ b/drivers/input/keyboard/atkbd.c
> @@ -1664,6 +1664,30 @@ static const struct dmi_system_id atkbd_dmi_quirk_table[] __initconst = {
>  	},
>  	{
>  		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> +			DMI_MATCH(DMI_CHASSIS_TYPE, "9"), /* Laptop */
> +		},
> +		.callback = atkbd_setup_forced_release,
> +		.driver_data = atkbd_dell_laptop_forced_release_keys,
> +	},
> +	{
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> +			DMI_MATCH(DMI_CHASSIS_TYPE, "10"), /* Notebook */
> +		},
> +		.callback = atkbd_setup_forced_release,
> +		.driver_data = atkbd_dell_laptop_forced_release_keys,
> +	},
> +	{
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> +			DMI_MATCH(DMI_CHASSIS_TYPE, "14"), /* Sub-Notebook */
> +		},
> +		.callback = atkbd_setup_forced_release,
> +		.driver_data = atkbd_dell_laptop_forced_release_keys,
> +	},
> +	{
> +		.matches = {
>  			DMI_MATCH(DMI_SYS_VENDOR, "Dell Computer Corporation"),
>  			DMI_MATCH(DMI_CHASSIS_TYPE, "8"), /* Portable */
>  		},
> @@ -1672,6 +1696,30 @@ static const struct dmi_system_id atkbd_dmi_quirk_table[] __initconst = {
>  	},
>  	{
>  		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Dell Computer Corporation"),
> +			DMI_MATCH(DMI_CHASSIS_TYPE, "9"), /* Laptop */
> +		},
> +		.callback = atkbd_setup_forced_release,
> +		.driver_data = atkbd_dell_laptop_forced_release_keys,
> +	},
> +	{
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Dell Computer Corporation"),
> +			DMI_MATCH(DMI_CHASSIS_TYPE, "10"), /* Notebook */
> +		},
> +		.callback = atkbd_setup_forced_release,
> +		.driver_data = atkbd_dell_laptop_forced_release_keys,
> +	},
> +	{
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Dell Computer Corporation"),
> +			DMI_MATCH(DMI_CHASSIS_TYPE, "14"), /* Sub-Notebook */
> +		},
> +		.callback = atkbd_setup_forced_release,
> +		.driver_data = atkbd_dell_laptop_forced_release_keys,
> +	},
> +	{
> +		.matches = {
>  			DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
>  			DMI_MATCH(DMI_PRODUCT_NAME, "HP 2133"),
>  		},
> -- 
> 1.7.9.5
>
Pali Rohár April 5, 2015, 10 p.m. UTC | #3
On Sunday 05 April 2015 23:48:33 Dmitry Torokhov wrote:
> Hi Pali,
> 
> On Sun, Mar 29, 2015 at 04:36:19PM +0200, Pali Rohár wrote:
> > This patch fixes commit 61579ba83934 ("Input: atkbd - expand
> > Latitude's force release quirk to other Dells"). Before
> > that commit release quirks were called for all Dell
> > Latitude models. After that commit only for Portable Dell
> > devices. But lot of Latitude models are Laptop or Notebook
> > DMI devices so quirks are not called.
> > 
> > Release quirks are still needed also for new Dell Latitude
> > models, so this patch enables quirks for all Portable,
> > Laptop, Notebook and Sub-Notebook Dell devices.
> 
> Does Dell use all these types for their laptops? What models
> do you know that need this quirk?
> 
> Thanks.
> 

I do not if Dell use all types, but months ago Matthew wrote to 
include also other numbers not only 9 (Laptop) and you agreed.

I do not know exact list of models which needs these quirks, but 
before that commit (61579ba83934) it was used for all Latitude 
models. At least I see that switches do not generate release 
events and on older Latitude machines some Fn keys do not 
generate them too.
Dmitry Torokhov April 5, 2015, 11:53 p.m. UTC | #4
On Mon, Apr 06, 2015 at 12:00:32AM +0200, Pali Rohár wrote:
> On Sunday 05 April 2015 23:48:33 Dmitry Torokhov wrote:
> > Hi Pali,
> > 
> > On Sun, Mar 29, 2015 at 04:36:19PM +0200, Pali Rohár wrote:
> > > This patch fixes commit 61579ba83934 ("Input: atkbd - expand
> > > Latitude's force release quirk to other Dells"). Before
> > > that commit release quirks were called for all Dell
> > > Latitude models. After that commit only for Portable Dell
> > > devices. But lot of Latitude models are Laptop or Notebook
> > > DMI devices so quirks are not called.
> > > 
> > > Release quirks are still needed also for new Dell Latitude
> > > models, so this patch enables quirks for all Portable,
> > > Laptop, Notebook and Sub-Notebook Dell devices.
> > 
> > Does Dell use all these types for their laptops? What models
> > do you know that need this quirk?
> > 
> > Thanks.
> > 
> 
> I do not if Dell use all types, but months ago Matthew wrote to 
> include also other numbers not only 9 (Laptop) and you agreed.

Hmm, I tried looking back but I could not quite find the discussion.

> 
> I do not know exact list of models which needs these quirks, but 
> before that commit (61579ba83934) it was used for all Latitude 
> models. At least I see that switches do not generate release 
> events and on older Latitude machines some Fn keys do not 
> generate them too.

So the question is still: what models do need this quirk and what
chassis type they are used.

Regardless, I have not accepted quirks for force_release and atkbd
keymaps for many years now as the task to adjust both keymap and
force_release list has been offloaded to udev.

Thanks.
Pali Rohár April 6, 2015, 8:06 a.m. UTC | #5
On Monday 06 April 2015 01:53:43 Dmitry Torokhov wrote:
> On Mon, Apr 06, 2015 at 12:00:32AM +0200, Pali Rohár wrote:
> > On Sunday 05 April 2015 23:48:33 Dmitry Torokhov wrote:
> > > Hi Pali,
> > > 
> > > On Sun, Mar 29, 2015 at 04:36:19PM +0200, Pali Rohár wrote:
> > > > This patch fixes commit 61579ba83934 ("Input: atkbd -
> > > > expand Latitude's force release quirk to other Dells").
> > > > Before that commit release quirks were called for all
> > > > Dell Latitude models. After that commit only for
> > > > Portable Dell devices. But lot of Latitude models are
> > > > Laptop or Notebook DMI devices so quirks are not
> > > > called.
> > > > 
> > > > Release quirks are still needed also for new Dell
> > > > Latitude models, so this patch enables quirks for all
> > > > Portable, Laptop, Notebook and Sub-Notebook Dell
> > > > devices.
> > > 
> > > Does Dell use all these types for their laptops? What
> > > models do you know that need this quirk?
> > > 
> > > Thanks.
> > 
> > I do not if Dell use all types, but months ago Matthew wrote
> > to include also other numbers not only 9 (Laptop) and you
> > agreed.
> 
> Hmm, I tried looking back but I could not quite find the
> discussion.
> 

Search for Message-ID: <20141222175632.GB18556@dtor-ws>
(this should be unique identifier for emails)

> > I do not know exact list of models which needs these quirks,
> > but before that commit (61579ba83934) it was used for all
> > Latitude models. At least I see that switches do not
> > generate release events and on older Latitude machines some
> > Fn keys do not generate them too.
> 
> So the question is still: what models do need this quirk and
> what chassis type they are used.
> 

My model:

$ cat /sys/class/dmi/id/chassis_type 
9
$ cat /sys/class/dmi/id/sys_vendor
Dell Inc.
$ cat /sys/class/dmi/id/product_name
Latitude E6440

> Regardless, I have not accepted quirks for force_release and
> atkbd keymaps for many years now as the task to adjust both
> keymap and force_release list has been offloaded to udev.
> 
> Thanks.

Ok, and what to do with kernel regressions? As before commit 
61579ba83934 it worked, because quirk was called for all Latitude 
(independently on chassis_type).
Dmitry Torokhov April 6, 2015, 4:20 p.m. UTC | #6
On Mon, Apr 06, 2015 at 10:06:10AM +0200, Pali Rohár wrote:
> On Monday 06 April 2015 01:53:43 Dmitry Torokhov wrote:
> > On Mon, Apr 06, 2015 at 12:00:32AM +0200, Pali Rohár wrote:
> > > On Sunday 05 April 2015 23:48:33 Dmitry Torokhov wrote:
> > > > Hi Pali,
> > > > 
> > > > On Sun, Mar 29, 2015 at 04:36:19PM +0200, Pali Rohár wrote:
> > > > > This patch fixes commit 61579ba83934 ("Input: atkbd -
> > > > > expand Latitude's force release quirk to other Dells").
> > > > > Before that commit release quirks were called for all
> > > > > Dell Latitude models. After that commit only for
> > > > > Portable Dell devices. But lot of Latitude models are
> > > > > Laptop or Notebook DMI devices so quirks are not
> > > > > called.
> > > > > 
> > > > > Release quirks are still needed also for new Dell
> > > > > Latitude models, so this patch enables quirks for all
> > > > > Portable, Laptop, Notebook and Sub-Notebook Dell
> > > > > devices.
> > > > 
> > > > Does Dell use all these types for their laptops? What
> > > > models do you know that need this quirk?
> > > > 
> > > > Thanks.
> > > 
> > > I do not if Dell use all types, but months ago Matthew wrote
> > > to include also other numbers not only 9 (Laptop) and you
> > > agreed.
> > 
> > Hmm, I tried looking back but I could not quite find the
> > discussion.
> > 
> 
> Search for Message-ID: <20141222175632.GB18556@dtor-ws>
> (this should be unique identifier for emails)

Ah, indeed. Sorry, at this time I completely forgot that we are
controlling this from userspace nowadays.

> 
> > > I do not know exact list of models which needs these quirks,
> > > but before that commit (61579ba83934) it was used for all
> > > Latitude models. At least I see that switches do not
> > > generate release events and on older Latitude machines some
> > > Fn keys do not generate them too.
> > 
> > So the question is still: what models do need this quirk and
> > what chassis type they are used.
> > 
> 
> My model:
> 
> $ cat /sys/class/dmi/id/chassis_type 
> 9
> $ cat /sys/class/dmi/id/sys_vendor
> Dell Inc.
> $ cat /sys/class/dmi/id/product_name
> Latitude E6440
> 
> > Regardless, I have not accepted quirks for force_release and
> > atkbd keymaps for many years now as the task to adjust both
> > keymap and force_release list has been offloaded to udev.
> > 
> > Thanks.
> 
> Ok, and what to do with kernel regressions? As before commit 
> 61579ba83934 it worked, because quirk was called for all Latitude 
> (independently on chassis_type).

No, this is not a kernel regression because it would mean that your
laptop worked properly with then current kernel and then stopped
working. Given that the change went in in 2008 and your laptop is a bit
newer than that you can't claim kernel regression here; as far as I can
see the change was reasonable for the lineage we had back then, or at
least noone notified us for about 6 years.

So please do adjust udev rules, there are quite a bit for tweaks for
Dell and other manufacturer's laptop keyboards there).

Thanks.
Pali Rohár April 8, 2015, 8:54 p.m. UTC | #7
On Monday 06 April 2015 18:20:56 Dmitry Torokhov wrote:
> On Mon, Apr 06, 2015 at 10:06:10AM +0200, Pali Rohár wrote:
> > On Monday 06 April 2015 01:53:43 Dmitry Torokhov wrote:
> > > On Mon, Apr 06, 2015 at 12:00:32AM +0200, Pali Rohár wrote:
> > > > On Sunday 05 April 2015 23:48:33 Dmitry Torokhov wrote:
> > > > > Hi Pali,
> > > > > 
> > > > > On Sun, Mar 29, 2015 at 04:36:19PM +0200, Pali Rohár
> > > > > wrote:
> > > > > > This patch fixes commit 61579ba83934 ("Input: atkbd
> > > > > > - expand Latitude's force release quirk to other
> > > > > > Dells"). Before that commit release quirks were
> > > > > > called for all Dell Latitude models. After that
> > > > > > commit only for Portable Dell devices. But lot of
> > > > > > Latitude models are Laptop or Notebook DMI devices
> > > > > > so quirks are not called.
> > > > > > 
> > > > > > Release quirks are still needed also for new Dell
> > > > > > Latitude models, so this patch enables quirks for
> > > > > > all Portable, Laptop, Notebook and Sub-Notebook
> > > > > > Dell devices.
> > > > > 
> > > > > Does Dell use all these types for their laptops? What
> > > > > models do you know that need this quirk?
> > > > > 
> > > > > Thanks.
> > > > 
> > > > I do not if Dell use all types, but months ago Matthew
> > > > wrote to include also other numbers not only 9 (Laptop)
> > > > and you agreed.
> > > 
> > > Hmm, I tried looking back but I could not quite find the
> > > discussion.
> > 
> > Search for Message-ID: <20141222175632.GB18556@dtor-ws>
> > (this should be unique identifier for emails)
> 
> Ah, indeed. Sorry, at this time I completely forgot that we
> are controlling this from userspace nowadays.
> 
> > > > I do not know exact list of models which needs these
> > > > quirks, but before that commit (61579ba83934) it was
> > > > used for all Latitude models. At least I see that
> > > > switches do not generate release events and on older
> > > > Latitude machines some Fn keys do not generate them
> > > > too.
> > > 
> > > So the question is still: what models do need this quirk
> > > and what chassis type they are used.
> > 
> > My model:
> > 
> > $ cat /sys/class/dmi/id/chassis_type
> > 9
> > $ cat /sys/class/dmi/id/sys_vendor
> > Dell Inc.
> > $ cat /sys/class/dmi/id/product_name
> > Latitude E6440
> > 
> > > Regardless, I have not accepted quirks for force_release
> > > and atkbd keymaps for many years now as the task to
> > > adjust both keymap and force_release list has been
> > > offloaded to udev.
> > > 
> > > Thanks.
> > 
> > Ok, and what to do with kernel regressions? As before commit
> > 61579ba83934 it worked, because quirk was called for all
> > Latitude (independently on chassis_type).
> 
> No, this is not a kernel regression because it would mean that
> your laptop worked properly with then current kernel and then
> stopped working. Given that the change went in in 2008 and
> your laptop is a bit newer than that you can't claim kernel
> regression here; as far as I can see the change was
> reasonable for the lineage we had back then, or at least
> noone notified us for about 6 years.
> 
> So please do adjust udev rules, there are quite a bit for
> tweaks for Dell and other manufacturer's laptop keyboards
> there).
> 
> Thanks.

Ok, thanks for information. Maybe you can add some comment to 
source code or documentation, that this part of code is now 
handled by userspace. So other people send patches to correct 
place/projects in future.

Patch
diff mbox

diff --git a/drivers/input/keyboard/atkbd.c b/drivers/input/keyboard/atkbd.c
index 387c51f..3188493 100644
--- a/drivers/input/keyboard/atkbd.c
+++ b/drivers/input/keyboard/atkbd.c
@@ -1664,6 +1664,30 @@  static const struct dmi_system_id atkbd_dmi_quirk_table[] __initconst = {
 	},
 	{
 		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+			DMI_MATCH(DMI_CHASSIS_TYPE, "9"), /* Laptop */
+		},
+		.callback = atkbd_setup_forced_release,
+		.driver_data = atkbd_dell_laptop_forced_release_keys,
+	},
+	{
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+			DMI_MATCH(DMI_CHASSIS_TYPE, "10"), /* Notebook */
+		},
+		.callback = atkbd_setup_forced_release,
+		.driver_data = atkbd_dell_laptop_forced_release_keys,
+	},
+	{
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+			DMI_MATCH(DMI_CHASSIS_TYPE, "14"), /* Sub-Notebook */
+		},
+		.callback = atkbd_setup_forced_release,
+		.driver_data = atkbd_dell_laptop_forced_release_keys,
+	},
+	{
+		.matches = {
 			DMI_MATCH(DMI_SYS_VENDOR, "Dell Computer Corporation"),
 			DMI_MATCH(DMI_CHASSIS_TYPE, "8"), /* Portable */
 		},
@@ -1672,6 +1696,30 @@  static const struct dmi_system_id atkbd_dmi_quirk_table[] __initconst = {
 	},
 	{
 		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Dell Computer Corporation"),
+			DMI_MATCH(DMI_CHASSIS_TYPE, "9"), /* Laptop */
+		},
+		.callback = atkbd_setup_forced_release,
+		.driver_data = atkbd_dell_laptop_forced_release_keys,
+	},
+	{
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Dell Computer Corporation"),
+			DMI_MATCH(DMI_CHASSIS_TYPE, "10"), /* Notebook */
+		},
+		.callback = atkbd_setup_forced_release,
+		.driver_data = atkbd_dell_laptop_forced_release_keys,
+	},
+	{
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Dell Computer Corporation"),
+			DMI_MATCH(DMI_CHASSIS_TYPE, "14"), /* Sub-Notebook */
+		},
+		.callback = atkbd_setup_forced_release,
+		.driver_data = atkbd_dell_laptop_forced_release_keys,
+	},
+	{
+		.matches = {
 			DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
 			DMI_MATCH(DMI_PRODUCT_NAME, "HP 2133"),
 		},