Remove devices on detaching the Book clipboard

[qzed]

Detaching the clipboard currently causes some issues regarding proper de-initialization/re-initialization of devices attached to the base on the Surface Book 3. When a base detachment has been detected, these devices should be removed via the drivers and re-added on attachment.

Affected devices are

• secondary/base battery
• HID devices, specifically keyboard and touchpad

[qzed]

@steveniemitz, @xanf, @NickyTope I've now implemented basic support for this on the cleanup branch. Can you check that it works properly? Detaching the base should remove the devices, re-attaching it should re-add them, which in turn should properly set them up, so the touchpad should now work as intended. You can also try to suspend, detach while suspended, then wake and re-attach, which should (hopefully) also work.

Since some things have changed (including directory layout for the modules), the "easy" way for loading/unloading the modules has also changed: https://github.com/linux-surface/surface-aggregator-module/wiki/Testing-and-Installing#buildtest-the-modules. Basically, run make modprobe-unload to unload in-kernel modules and then make insmod to load the ones built (you need to run make before to build the module, that's not done automatically with those commands).

Installing the module via DKMS should also work, but at the moment some modules won't be auto-loaded yet (specifically keyboard, battery, and performance mode) due to me implementing a new device type. This requires some in-kernel support that I plan to add shortly.

[NickyTope]

Hi @qzed apologies for going missing on these issues/features for the past month or so, have been working in an environment that has a windows only client and so not been keeping up to date here...

I've tried to test the changes above just now but perhaps I am missing something?

I have built and used the modules in the feature/sb3-v3 branch and they work great but do the keyboard / trackpad do not come back after a detach / reattach.

after doing sudo make sudo make rmmod and sudo make insmod in the cleanup branch I do not have any keyboard / trackpad events at all..

I get the below error when doing the insmod
insmod: ERROR: could not insert module clients/surface_sam_device_hub.ko: Unknown symbol in module

[qzed]

Which kernel version are you on? There are some incompatibilities with v4.19, so you'll have to go with non-lts.

[NickyTope]

just upgrading from 5.7 to 5.8 and will retry there

[qzed]

Is there any indication which symbol couldn't be found? Also have the previous modules been fully unloaded (check lsmod | grep surface_sam)?

[NickyTope]

Ok, after upgrade to 5.8 and reboot, I no longer get the Unknown symbol error, the cleanup branch does not work for me though.. does a clean insmod with no output but still no keyb/trackpad

lsmod shows 2 modules still loaded as below

Switched to branch 'cleanup'
Your branch is up to date with 'origin/cleanup'.
  ~/apps/surface-aggregator-module   cleanup  cd module                        
  ~/apps/surface-aggregator-module/module   cleanup  ls
Makefile  dkms.conf  include  src
  ~/apps/surface-aggregator-module/module   cleanup  sudo make             
make -C /lib/modules/"5.8.1-arch1-1-surface"/build M=/home/nt/apps/surface-aggregator-module/module/src modules
make[1]: Entering directory '/usr/lib/modules/5.8.1-arch1-1-surface/build'
  CC [M]  /home/nt/apps/surface-aggregator-module/module/src/core.o
  CC [M]  /home/nt/apps/surface-aggregator-module/module/src/ssh_parser.o
  CC [M]  /home/nt/apps/surface-aggregator-module/module/src/ssh_packet_layer.o
  CC [M]  /home/nt/apps/surface-aggregator-module/module/src/ssh_request_layer.o
  CC [M]  /home/nt/apps/surface-aggregator-module/module/src/controller.o
  CC [M]  /home/nt/apps/surface-aggregator-module/module/src/bus.o
  LD [M]  /home/nt/apps/surface-aggregator-module/module/src/surface_sam_ssh.o
  CC [M]  /home/nt/apps/surface-aggregator-module/module/src/clients/surface_sam_san.o
  CC [M]  /home/nt/apps/surface-aggregator-module/module/src/clients/surface_sam_dtx.o
  CC [M]  /home/nt/apps/surface-aggregator-module/module/src/clients/surface_sam_hps.o
  CC [M]  /home/nt/apps/surface-aggregator-module/module/src/clients/surface_sam_vhf.o
  CC [M]  /home/nt/apps/surface-aggregator-module/module/src/clients/surface_sam_device_hub.o
  CC [M]  /home/nt/apps/surface-aggregator-module/module/src/clients/surface_sam_sid_perfmode.o
  CC [M]  /home/nt/apps/surface-aggregator-module/module/src/clients/surface_sam_sid_power.o
  CC [M]  /home/nt/apps/surface-aggregator-module/module/src/clients/surface_sam_sid_vhf.o
  CC [M]  /home/nt/apps/surface-aggregator-module/module/src/clients/surface_sam_debugfs.o
  MODPOST /home/nt/apps/surface-aggregator-module/module/src/Module.symvers
  CC [M]  /home/nt/apps/surface-aggregator-module/module/src/clients/surface_sam_debugfs.mod.o
  LD [M]  /home/nt/apps/surface-aggregator-module/module/src/clients/surface_sam_debugfs.ko
  CC [M]  /home/nt/apps/surface-aggregator-module/module/src/clients/surface_sam_device_hub.mod.o
  LD [M]  /home/nt/apps/surface-aggregator-module/module/src/clients/surface_sam_device_hub.ko
  CC [M]  /home/nt/apps/surface-aggregator-module/module/src/clients/surface_sam_dtx.mod.o
  LD [M]  /home/nt/apps/surface-aggregator-module/module/src/clients/surface_sam_dtx.ko
  CC [M]  /home/nt/apps/surface-aggregator-module/module/src/clients/surface_sam_hps.mod.o
  LD [M]  /home/nt/apps/surface-aggregator-module/module/src/clients/surface_sam_hps.ko
  CC [M]  /home/nt/apps/surface-aggregator-module/module/src/clients/surface_sam_san.mod.o
  LD [M]  /home/nt/apps/surface-aggregator-module/module/src/clients/surface_sam_san.ko
  CC [M]  /home/nt/apps/surface-aggregator-module/module/src/clients/surface_sam_sid_perfmode.mod.o
  LD [M]  /home/nt/apps/surface-aggregator-module/module/src/clients/surface_sam_sid_perfmode.ko
  CC [M]  /home/nt/apps/surface-aggregator-module/module/src/clients/surface_sam_sid_power.mod.o
  LD [M]  /home/nt/apps/surface-aggregator-module/module/src/clients/surface_sam_sid_power.ko
  CC [M]  /home/nt/apps/surface-aggregator-module/module/src/clients/surface_sam_sid_vhf.mod.o
  LD [M]  /home/nt/apps/surface-aggregator-module/module/src/clients/surface_sam_sid_vhf.ko
  CC [M]  /home/nt/apps/surface-aggregator-module/module/src/clients/surface_sam_vhf.mod.o
  LD [M]  /home/nt/apps/surface-aggregator-module/module/src/clients/surface_sam_vhf.ko
  CC [M]  /home/nt/apps/surface-aggregator-module/module/src/surface_sam_ssh.mod.o
  LD [M]  /home/nt/apps/surface-aggregator-module/module/src/surface_sam_ssh.ko
make[1]: Leaving directory '/usr/lib/modules/5.8.1-arch1-1-surface/build'
  ~/apps/surface-aggregator-module/module   cleanup  sudo make modprobe-unload
modprobe: FATAL: Module surface_sam_device_hub not found.
  ~/apps/surface-aggregator-module/module   cleanup  sudo lsmod | grep surface_sam
surface_sam_sid_gpelid    20480  0
surface_sam_sid        20480  0
  ~/apps/surface-aggregator-module/module   cleanup  sudo make rmmod              
rmmod: ERROR: Module surface_sam_debugfs is not currently loaded
rmmod: ERROR: Module surface_sam_sid_vhf is not currently loaded
rmmod: ERROR: Module surface_sam_sid_power is not currently loaded
rmmod: ERROR: Module surface_sam_sid_perfmode is not currently loaded
rmmod: ERROR: Module surface_sam_device_hub is not currently loaded
rmmod: ERROR: Module surface_sam_hps is not currently loaded
rmmod: ERROR: Module surface_sam_dtx is not currently loaded
rmmod: ERROR: Module surface_sam_vhf is not currently loaded
rmmod: ERROR: Module surface_sam_san is not currently loaded
rmmod: ERROR: Module surface_sam_ssh is not currently loaded
  ~/apps/surface-aggregator-module/module   cleanup  sudo lsmod | grep surface_sam
surface_sam_sid_gpelid    20480  0
surface_sam_sid        20480  0
  ~/apps/surface-aggregator-module/module   cleanup  sudo make insmod 

[qzed]

Ah yeah, so it seems that the SID and GPELID modules don't get unloaded because they have been removed in the cleanup branch. You'll manually need to modprobe -r <name> them.

With the old SID module still loaded, it's pretty much guaranteed that the keyboard isn't working because the new device hub registers against the same platform device (so the SID module blocks that), and the device hub basically provides the device the keyboard driver loads against.

[NickyTope]

Spot on !!!

Can confirm this works with events coming back after reattach, will try suspend now also

[NickyTope]

ok, after doing a sudo systemctl suspend then detaching, I can press the power button and the system looks like it resumes (the screen comes back on) however after reattaching even my external keyboard does nothing.. This is not a real use case for me anyway, I'm just glad that I can reattach the screen in reverse because that's how I use it on my desk at work..

[qzed]

So something on resume is hanging up the system? Can you check if normal suspend/resume works?

[qzed]

Can confirm this works with events coming back after reattach, will try suspend now also

Oh and did you test the touchpad, specifically gestures and multi-touch?

[NickyTope]

Yes can confirm trackpad works with gestures after reattach, can scroll this window with two finger gesture perfectly!!

[qzed]

Neat! Now only the suspend/resume thing remains. That's kind of a niche issue (and I think in general you should only detach the device if it's on), but I think we should still try to figure out what's going wrong there.