Message ID | 20220527023447.2460025-1-luzmaximilian@gmail.com (mailing list archive) |
---|---|
Headers | show |
Series | platform/surface: aggregator: Add support for client hot-removal | expand |
Hi, On 5/27/22 04:34, Maximilian Luz wrote: > Summary: > > Add support for the HID type cover input devices on the Pro 8 and all > requirements for that. > > > Blurb from v1: > > This series adds support for the type cover of the Surface Pro 8. On > the Pro 8, the type cover is (unlike on previous generations) handled > via the Surface System Aggregator Module (SSAM). As the type cover is > detachable, care needs to be taken and the respective SSAM (HID) > client devices need to be properly removed when detached and > re-initialized when attached. > > Therefore, this series does three things: > > 1. Improve hot-removal support for SSAM client devices. When > hot-removing clients, subsequent communication may time out. > > In the worst case, this can lead to problems when devices are > detached and re-attached quickly, before we can remove their > respective kernel representations. This can then lead to devices > being in an uninitialized state, preventing, for example, touchpad > gestures from working properly as the required HID feature report > has not been sent. > > Therefore, handle hot-removal of devices more gracefully by > avoiding communication once it has been detected and ensure that > devices are actually removed. > > 2. Generify SSAM subsystem hubs and add a KIP hub. On the Surface Pro > 8, the KIP subsystem (only that abbreviation is known) is > responsible for managing type-cover devices. This hub acts as the > controller for device removal similar to the BAS (detachable base) > subsystem hub on the Surface Book 3 (therefore we can share most > of the code between them). > > 3. Add the (HID) type-cover clients of the Surface Pro 8 to the > aggregator registry. > > > Changes in v2: > > - Introduce "platform/surface: aggregator: Allow is_ssam_device() to be > used when CONFIG_SURFACE_AGGREGATOR_BUS is disabled" to fix an > undefined reference build issue when CONFIG_SURFACE_AGGREGATOR_BUS > is disabled. > > - Make SSAM hub device UIDs consistent. > - Introduce "platform/surface: aggregator_registry: Change device ID > for base hub" to make association between hub and subsystem target > category more obvious. > - Change hub device ID for KIP subsystem hub to be consistent with > the id of the already existing BAS hub. Thank you for your patch-series, I've applied the series to my review-hans branch: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans Once I've run some tests on this branch the patches there will be added to the platform-drivers-x86/for-next branch and eventually will be included in the pdx86 pull-request to Linus for the next merge-window. Jiri, Benjamin, note I've also taken the one small(ish) HID patch which is a part of this series, despite it lacking an Ack from either of you. I hope this is ok, if not let me know. Regards, Hans > Maximilian Luz (12): > platform/surface: aggregator: Allow is_ssam_device() to be used when > CONFIG_SURFACE_AGGREGATOR_BUS is disabled > platform/surface: aggregator: Allow devices to be marked as > hot-removed > platform/surface: aggregator: Allow notifiers to avoid communication > on unregistering > platform/surface: aggregator_registry: Use client device wrappers for > notifier registration > power/supply: surface_charger: Use client device wrappers for notifier > registration > power/supply: surface_battery: Use client device wrappers for notifier > registration > HID: surface-hid: Add support for hot-removal > platform/surface: aggregator: Add comment for KIP subsystem category > platform/surface: aggregator_registry: Generify subsystem hub > functionality > platform/surface: aggregator_registry: Change device ID for base hub > platform/surface: aggregator_registry: Add KIP device hub > platform/surface: aggregator_registry: Add support for keyboard cover > on Surface Pro 8 > > .../driver-api/surface_aggregator/client.rst | 6 +- > drivers/hid/surface-hid/surface_hid_core.c | 38 +- > .../platform/surface/aggregator/controller.c | 53 ++- > .../surface/surface_aggregator_registry.c | 403 +++++++++++++----- > drivers/power/supply/surface_battery.c | 4 +- > drivers/power/supply/surface_charger.c | 4 +- > include/linux/surface_aggregator/controller.h | 24 +- > include/linux/surface_aggregator/device.h | 125 +++++- > include/linux/surface_aggregator/serial_hub.h | 2 +- > 9 files changed, 513 insertions(+), 146 deletions(-) >
On 6/13/22 17:27, Hans de Goede wrote: > Hi, > > On 5/27/22 04:34, Maximilian Luz wrote: >> Summary: >> >> Add support for the HID type cover input devices on the Pro 8 and all >> requirements for that. >> >> >> Blurb from v1: >> >> This series adds support for the type cover of the Surface Pro 8. On >> the Pro 8, the type cover is (unlike on previous generations) handled >> via the Surface System Aggregator Module (SSAM). As the type cover is >> detachable, care needs to be taken and the respective SSAM (HID) >> client devices need to be properly removed when detached and >> re-initialized when attached. >> >> Therefore, this series does three things: >> >> 1. Improve hot-removal support for SSAM client devices. When >> hot-removing clients, subsequent communication may time out. >> >> In the worst case, this can lead to problems when devices are >> detached and re-attached quickly, before we can remove their >> respective kernel representations. This can then lead to devices >> being in an uninitialized state, preventing, for example, touchpad >> gestures from working properly as the required HID feature report >> has not been sent. >> >> Therefore, handle hot-removal of devices more gracefully by >> avoiding communication once it has been detected and ensure that >> devices are actually removed. >> >> 2. Generify SSAM subsystem hubs and add a KIP hub. On the Surface Pro >> 8, the KIP subsystem (only that abbreviation is known) is >> responsible for managing type-cover devices. This hub acts as the >> controller for device removal similar to the BAS (detachable base) >> subsystem hub on the Surface Book 3 (therefore we can share most >> of the code between them). >> >> 3. Add the (HID) type-cover clients of the Surface Pro 8 to the >> aggregator registry. >> >> >> Changes in v2: >> >> - Introduce "platform/surface: aggregator: Allow is_ssam_device() to be >> used when CONFIG_SURFACE_AGGREGATOR_BUS is disabled" to fix an >> undefined reference build issue when CONFIG_SURFACE_AGGREGATOR_BUS >> is disabled. >> >> - Make SSAM hub device UIDs consistent. >> - Introduce "platform/surface: aggregator_registry: Change device ID >> for base hub" to make association between hub and subsystem target >> category more obvious. >> - Change hub device ID for KIP subsystem hub to be consistent with >> the id of the already existing BAS hub. > > Thank you for your patch-series, I've applied the series to my > review-hans branch: > https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans > > Once I've run some tests on this branch the patches there will be > added to the platform-drivers-x86/for-next branch and eventually > will be included in the pdx86 pull-request to Linus for the next > merge-window. > > Jiri, Benjamin, note I've also taken the one small(ish) HID patch > which is a part of this series, despite it lacking an Ack from > either of you. I hope this is ok, if not let me know. Sorry I am well behind on my patch processing. The patch is simple enough and if you reviewed the rest, that is fine by me. Just for the archives: For the HID part Acked-by: Benjamin Tissoires <benjamin.tisssoires@redhat.com> (no need to force push your branch unless you think it's really important to have my ack). Cheers, Benjamin > > Regards, > > Hans > > > > >> Maximilian Luz (12): >> platform/surface: aggregator: Allow is_ssam_device() to be used when >> CONFIG_SURFACE_AGGREGATOR_BUS is disabled >> platform/surface: aggregator: Allow devices to be marked as >> hot-removed >> platform/surface: aggregator: Allow notifiers to avoid communication >> on unregistering >> platform/surface: aggregator_registry: Use client device wrappers for >> notifier registration >> power/supply: surface_charger: Use client device wrappers for notifier >> registration >> power/supply: surface_battery: Use client device wrappers for notifier >> registration >> HID: surface-hid: Add support for hot-removal >> platform/surface: aggregator: Add comment for KIP subsystem category >> platform/surface: aggregator_registry: Generify subsystem hub >> functionality >> platform/surface: aggregator_registry: Change device ID for base hub >> platform/surface: aggregator_registry: Add KIP device hub >> platform/surface: aggregator_registry: Add support for keyboard cover >> on Surface Pro 8 >> >> .../driver-api/surface_aggregator/client.rst | 6 +- >> drivers/hid/surface-hid/surface_hid_core.c | 38 +- >> .../platform/surface/aggregator/controller.c | 53 ++- >> .../surface/surface_aggregator_registry.c | 403 +++++++++++++----- >> drivers/power/supply/surface_battery.c | 4 +- >> drivers/power/supply/surface_charger.c | 4 +- >> include/linux/surface_aggregator/controller.h | 24 +- >> include/linux/surface_aggregator/device.h | 125 +++++- >> include/linux/surface_aggregator/serial_hub.h | 2 +- >> 9 files changed, 513 insertions(+), 146 deletions(-) >> >