Hi, On 2/8/21 9:04 PM, Maximilian Luz wrote: > On 2/8/21 8:55 PM, Hans de Goede wrote: >> Hi, >> >> On 2/8/21 8:35 PM, Maximilian Luz wrote: >>> The Surface System Aggregator Module (SSAM) subsystem provides various >>> functionalities, which are separated by spreading them across multiple >>> devices and corresponding drivers. Parts of that functionality / some of >>> those devices, however, can (as far as we currently know) not be >>> auto-detected by conventional means. While older (specifically 5th- and >>> 6th-)generation models do advertise most of their functionality via >>> standard platform devices in ACPI, newer generations do not. >>> >>> As we are currently also not aware of any feasible way to query said >>> functionalities dynamically, this poses a problem. There is, however, a >>> device in ACPI that seems to be used by Windows for identifying >>> different Surface models: The Windows Surface Integration Device (WSID). >>> This device seems to have a HID corresponding to the overall set of >>> functionalities SSAM provides for the associated model. >>> >>> This series introduces a device registry based on software nodes and >>> device hubs to solve this problem. The registry is intended to contain >>> all required non-detectable information. >>> >>> The platform hub driver is loaded against the WSID device and >>> instantiates and manages SSAM devices based on the information provided >>> by the registry for the given WSID HID of the Surface model. All new >>> devices created by this hub added as child devices to this hub. >>> >>> In addition, a base hub is introduced to manage devices associated with >>> the detachable base part of the Surface Book 3, as this requires special >>> handling (i.e. devices need to be removed when the base is removed). >>> Again, all devices created by the base hub (i.e. associated with the >>> base) are added as child devices to this hub. >>> >>> In total, this will yield the following device structure >>> >>> WSID >>> |- SSAM device 1 (physical device) >>> |- SSAM device 2 (physical device) >>> |- SSAM device 3 (physical device) >>> |- ... >>> \- SSAM base hub (virtual device) >>> |- SSAM base device 1 (physical device) >>> |- SSAM base device 2 (physical device) >>> |- ... >>> >>> While software nodes seem to be well suited for this approach due to >>> extensibility, they still need to be hard-coded, so I'm open for ideas >>> on how this could be improved. >> >> This series looks good to me. >> >> One question is this 5.12 material, or is this intended for 5.13 >> (together with drivers attaching to the newly instantiated devices) ? >> >> I can drop this into for-next tomorrow, that gives it just about >> enough "baking" time in -next to still make it into 5.12 >> (this should be pretty safe to push to for-next despite it being >> somewhat close to the cutoff point as it is a new driver). >> >> Maximilian, do you want me to add this series to for-next tomorrow, >> or would you prefer for it to go to -next after 5.12-rc1 >> (and thus end up in 5.13) ? > > I wouldn't mind giving this a bit more time to maybe collect some more > reviews. > > The only real functionality that depends on this and would be ready for > 5.12 right now is the performance profile. Everything else will have to > go into 5.13 anyway (still need to cleanup and prepare patches for > that), so 5.13 seems to be the better target for me. Ok, that is perfectly fine with me. Regards, Hans >>> Maximilian Luz (6): >>> platform/surface: Set up Surface Aggregator device registry >>> platform/surface: aggregator_registry: Add base device hub >>> platform/surface: aggregator_registry: Add battery subsystem devices >>> platform/surface: aggregator_registry: Add platform profile device >>> platform/surface: aggregator_registry: Add DTX device >>> platform/surface: aggregator_registry: Add HID subsystem devices >>> >>> MAINTAINERS | 1 + >>> drivers/platform/surface/Kconfig | 26 + >>> drivers/platform/surface/Makefile | 1 + >>> .../surface/surface_aggregator_registry.c | 641 ++++++++++++++++++ >>> 4 files changed, 669 insertions(+) >>> create mode 100644 drivers/platform/surface/surface_aggregator_registry.c >>> >> >