Re: [asus-nb-wmi] i8042 optional dependecy?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 8/31/20 10:21 AM, Marius Iacob wrote:
On 20-08-31 09:36:24, Hans de Goede wrote:
Hi,

On 8/30/20 11:17 PM, Marius Iacob wrote:
On 20-08-24 22:23:41, Hans de Goede wrote:
Hi,

On 8/24/20 9:00 PM, Marius Iacob wrote:
On 20-08-24 12:02:12, Hans de Goede wrote:
Hi,

On 8/24/20 10:25 AM, Andy Shevchenko wrote:
On Sun, Aug 23, 2020 at 08:58:35PM +0300, Marius Iacob wrote:

I have an ASUS T103HAF and while trying to load asus-nb-wmi module it fails because it has i8042 as dependecy and that module does not load on my device.

Can you be more specific, why that module is not loaded?

Yes that would be my first question too, have you tried passing "i8042.reset=1" and/or "i8042.nomux=1" on the
kernel cmdline? Typically passing "i8042.nomux=1" fixes all kinda i8042 issues.


I'm sorry, forgot to mention, because my device is a 2-in-1 it uses a detachable keyboard/touchpad and is connected by USB interface. So when trying to load i8042 module (also tried reset/nomux) it always says in dmesg "i8042: PNP: No PS/2 controller found." I'm guessing there is no PS/2 controller on this device...

Ah I see, and I guess that after the "i8042: PNP: No PS/2 controller found." message (which is fine) the module fails to load, right ?

Yes, modprobe: ERROR: could not insert 'i8042': No such device, and there's no trace of i8042 in lsmod output.

That really is a bug in the i8042 code, if a module is providing symbols to another module it should never exit with an error from
its module_init function.

i8042 is doing a full init in its module_init. I've bypassed the PNP controller check and it fail while trying to talk to the controller:
	i8042: PNP detection disabled
	i8042: Can't read CTR while initializing i8042
	i8042: probe of i8042 failed with error -5
I'm guessing it's module_init should look like asus-wmi's: a message saying it's loaded and return 0; but I don't have enough kernel development experience to do this modification.

I haven't looked at the code (yet) but going by your description the trick would be to keep the PnP
check in the module_init function and instead of -ENODEV just return 0 when the check fails so that
the rest of the init function gets skipped.

You probably want to check module_exit in this case to see if that is safe to run with the
rest of module_init skipped.

Let me know if you need more help with this, I believe that fixing this should be pretty easy.

Unfortunately the PNP check is 2 layers deeper from module_init and it's expected to return 0 for success for the rest of the init procedure to continue (so that it's not that straightforward). The module seems to be built with a full init procedure on load in mind. I've looked at the code for quite a bit and it seems that it's a bit of patch to write, and most of the places the i8042 code is used (in other modules) expects the module to be not just loaded but fully initialized. So this should be a consideration also.

Please give the attached patch a try, I believe that this should fix the i8042 issue.

Once you have let me know that this works I'll replace the:

Reported-by: Marius Iacob <themariusus@xxxxxxxxx>

By:

Reported-and-tested-by: Marius Iacob <themariusus@xxxxxxxxx>

And submit the patch upstream. Note the input subsys
maintainers seems to be a bit slow to respond lately,
so I'm not sure how fast we can get this reviewed / merged.

Anyways first lets test it and see if it helps :)

Regards,

Hans
>From 863a3a4b719de5d8131d1636dfbb7cfcc55540e9 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@xxxxxxxxxx>
Date: Mon, 31 Aug 2020 12:02:46 +0200
Subject: [PATCH v8] Input: i8042 - Allow insmod to succeed on devices without
 an i8042 controller

The i8042 module exports several symbols which may be used by other
modules.

Before this commit it would refuse to load (when built as a module itself)
on systems without an i8042 controller.

This is a problem specifically for the asus-nb-wmi module. Many Asus
laptops support the Asus WMI interface. Some of them have an i8042
controller and need to use i8042_install_filter() to filter some kbd
events. Other models do not have an i8042 controller (e.g. they use an
USB attached kbd).

Before this commit the asus-nb-wmi driver could not be loaded on Asus
models without an i8042 controller, when the i8042 code was built as
a module (as Arch Linux does) because the module_init function of the
i8042 module would fail with -ENODEV and thus the i8042_install_filter
symbol could not be loaded.

This commit fixes this by exiting from module_init with a return code
of 0 if no controller is found.  It also adds a i8042_present bool to
make the module_exit function a no-op in this case and also adds a
check for i8042_present to the exported i8042_command function.

The latter i8042_present check should not really be necessary because
when builtin that function can already be used on systems without
an i8042 controller, but better safe then sorry.

Reported-by: Marius Iacob <themariusus@xxxxxxxxx>
Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
---
 drivers/input/serio/i8042.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
index d3eda48032e3..944cbb519c6d 100644
--- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -122,6 +122,7 @@ module_param_named(unmask_kbd_data, i8042_unmask_kbd_data, bool, 0600);
 MODULE_PARM_DESC(unmask_kbd_data, "Unconditional enable (may reveal sensitive data) of normally sanitize-filtered kbd data traffic debug log [pre-condition: i8042.debug=1 enabled]");
 #endif
 
+static bool i8042_present;
 static bool i8042_bypass_aux_irq_test;
 static char i8042_kbd_firmware_id[128];
 static char i8042_aux_firmware_id[128];
@@ -343,6 +344,9 @@ int i8042_command(unsigned char *param, int command)
 	unsigned long flags;
 	int retval;
 
+	if (!i8042_present)
+		return -1;
+
 	spin_lock_irqsave(&i8042_lock, flags);
 	retval = __i8042_command(param, command);
 	spin_unlock_irqrestore(&i8042_lock, flags);
@@ -1612,12 +1616,15 @@ static int __init i8042_init(void)
 
 	err = i8042_platform_init();
 	if (err)
-		return err;
+		return (err == -ENODEV) ? 0 : err;
 
 	err = i8042_controller_check();
 	if (err)
 		goto err_platform_exit;
 
+	/* Set this before creating the dev to allow i8042_command to work right away */
+	i8042_present = true;
+
 	pdev = platform_create_bundle(&i8042_driver, i8042_probe, NULL, 0, NULL, 0);
 	if (IS_ERR(pdev)) {
 		err = PTR_ERR(pdev);
@@ -1636,6 +1643,9 @@ static int __init i8042_init(void)
 
 static void __exit i8042_exit(void)
 {
+	if (!i8042_present)
+		return;
+
 	platform_device_unregister(i8042_platform_device);
 	platform_driver_unregister(&i8042_driver);
 	i8042_platform_exit();
-- 
2.28.0


[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux