| Message ID | 20220527023447.2460025-1-luzmaximilian@gmail.com |
|---|---|
| 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(-) >> >
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. 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(-)