Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH 0/6] platform/surface: Add Surface Aggregator device
 registry
To:     Maximilian Luz <luzmaximilian@gmail.com>
Cc:     Mark Gross <mgross@linux.intel.com>,
        platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20210208193508.3038055-1-luzmaximilian@gmail.com>
 <d11ec6b6-8fca-7174-3be8-22683798ffb6@redhat.com>
 <33e4cf40-4ed8-0b82-d4b2-ec483d8562e1@gmail.com>
From:   Hans de Goede <hdegoede@redhat.com>
Message-ID: <633dd8d1-1c84-25f0-73fb-ba667999242a@redhat.com>
Date:   Mon, 8 Feb 2021 21:07:34 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.7.0
MIME-Version: 1.0
In-Reply-To: <33e4cf40-4ed8-0b82-d4b2-ec483d8562e1@gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk

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
>>>
>>
>