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. > > > I guess most people do not have this problem because in the typical distro config the i8042 driver is built into the kernel. > > > > I guess I could ask the Arch Linux devs to build the kernel with i8042 in it. But that wouldn't be my first choice. > > > > > > > > > I see that i8042 is used in asus-nb-wmi for a quirk, so it's not necessary all the time. How can I make it an optional dependecy? > > > > > > > > > > include/linux/i8042.h > > > > > > > > > > Contains a stub for i8042_install_filter() for when CONFIG_SERIO_I8042 is not enabled, so you can build > > > > > your own kernel with that option unset. But we really ought to come up with a better fix which will also > > > > > work for standard distro kernels, see above. > > > > > > > > I've built a asus-nb-wmi module without i8042 references and it works. I had to add "BATC" for battery RSOC (my battery is named BATC). And yes, I also have tried to make the module work for my device around the i8042 dependecy, tried to add IS_REACHABLE(CONFIG_SERIO_I8042) in .c and imply SERIO_I8042 in Kconfig, but it didn't work, so I don't think I was on the right track. I have little experience with kernel development and I figured asking some more experienced people was a better idea. > > > > Thanks for your reply. If you have any ideas I'm more than happy to try/code them out and submit a patch if/when it works. > > > > > > If I understand things correctly (see above) then the right fix for the I8042 driver issue is to patch > > > its module_init function to return with 0 instead of -ENODEV (I guess) when it hits the code path with the > > > "i8042: PNP: No PS/2 controller found." message. > > > > > > Let me know if you need any help with that. > > > > As I explained above, I think it's more complicated than that. If you have any suggestions of what would be a good course of action here I'd be happy to do the grunt work. > > See my answer above, again let me know if you need more help (I will > likely just write a patch for this myself then). > > > > As for the "I had to add "BATC" for battery RSOC (my battery is named BATC)" bit I'm missing some > > > context there / not entirely following you, but lets first tackle the i8042 thing and then look > > > at that other issue later. > > > > The short story for this is that the battery on my device is named BATC and I've had to add a check for BATC (there were alredy checks for BAT0,BAT1,BATT) in asus_wmi_battery_add in asus-wmi.c that is used when asus-nb-wmi is loaded. And this was the main reason I wanted to make this module work, for the battery charge limit. > > Ah I see, I was not aware of the existing checks and yes being able to set > battery charge limits is quite useful. So this needs to go into a separate > patch (obviously, since the other patch will be for the i8042 code), but > otherwise this sounds fine for upstream. I will definitely write a patch for this when the i8042 issue is resolved. Regards, Marius
