Re: [PATCH 0/4] Add device links between tunneled USB3 devices and USB4 Host

[Mathias Nyman <mathias.nyman@linux.intel.com>]

On 24.6.2024 7.59, Mika Westerberg wrote:
> On Fri, Jun 21, 2024 at 11:30:05AM -0500, Mario Limonciello wrote:
>> On 6/21/2024 01:19, Mika Westerberg wrote:
>>> Hi Mario,
>>>
>>> On Thu, Jun 20, 2024 at 01:36:56PM -0500, Mario Limonciello wrote:
>>>> On 6/20/2024 01:41, Mika Westerberg wrote:
>>>>> +CC Mario from AMD side to check that we are good and don't break
>>>>> anything accidentally.
>>>>>
>>>>> On Wed, Jun 19, 2024 at 04:03:01PM +0300, Mathias Nyman wrote:
>>>>>> The relationship between a USB4 Host Interface providing USB3 tunnels,
>>>>>> and tunneled USB3 devices is not represented in device hierarchy.
>>>>>>
>>>>>> This caused issues with power managment as devices may suspend and
>>>>>> resume in incorrect order.
>>>>>> A device link between the USB4 Host Interface and the USB3 xHCI was
>>>>>> originally added to solve this, preventing the USB4 Host Interface from
>>>>>> suspending if the USB3 xHCI Host was still active.
>>>>>> This unfortunately also prevents USB4 Host Interface from suspending even
>>>>>> if the USB3 xHCI Host is only serving native non-tunneled USB devices.
>>>>>>
>>>>>> Improve the current powermanagement situation by creating device links
>>>>>> directly from tunneled USB3 devices to the USB4 Host Interface they depend
>>>>>> on instead of a device link between the hosts.
>>>>>> This way USB4 host may suspend when the last tunneled device is
>>>>>> disconnected.
>>>>>>
>>>>>> Intel xHCI hosts are capable of reporting if connected USB3 devices are
>>>>>> tunneled via vendor specific capabilities.
>>>>>> Use this until a standard way is available.
>>>>>>
>>>>>> Mathias Nyman (4):
>>>>>>      xhci: Add USB4 tunnel detection for USB3 devices on Intel hosts
>>>>>>      usb: Add tunneled parameter to usb device structure
>>>>>>      usb: acpi: add device link between tunneled USB3 device and USB4 Host
>>>>>>        Interface
>>>>>>      thunderbolt: Don't create device link from USB4 Host Interface to USB3
>>>>>>        xHC host
>>>>>>
>>>>>>     drivers/thunderbolt/acpi.c       | 40 ++++++------------------
>>>>>>     drivers/usb/core/usb-acpi.c      | 52 ++++++++++++++++++++++++++++++++
>>>>>>     drivers/usb/host/xhci-ext-caps.h |  5 +++
>>>>>>     drivers/usb/host/xhci-hub.c      | 29 ++++++++++++++++++
>>>>>>     drivers/usb/host/xhci.c          | 12 ++++++++
>>>>>>     drivers/usb/host/xhci.h          |  1 +
>>>>>>     include/linux/usb.h              |  2 ++
>>>>>>     7 files changed, 111 insertions(+), 30 deletions(-)
>>>>>>
>>>>>> -- 
>>>>>> 2.25.1
>>>>
>>>> Hi Mika,
>>>>
>>>> Thanks for looping me in.  Unfortunately with this is appears the XHCI
>>>> controller link never gets created.  I've not checked functional impact from
>>>> this, but I'd guess there "should" be some functional problems too.
>>>
>>> Thanks for checking!
>>>
>>> I think the code that sets up the device link based on ->tunneled should
>>> do that always if the hardware cannot tell if this is native or tunneled
>>> link to keep the existing functionality and only do the "optimization"
>>> if the hardware is capable of identifying that.
>>>
>>> Perhaps it can be a callback provided by the xHCI controller driver
>>> (->is_tunneled()) that then defaults to true if the
>>> "usb4-host-interface" property is there and in case of Intel hardware
>>> also checks the proprietary register?

How about changing the boolean udev->tunneled into a 3 value enum with
"link_unknown", "link_native", and "link_tunneled" options.

"link_unknown" would be default, xhci driver only changes this to "link_tunneled" or
"link_native" if the host can detect tunnels.

device link to USB4 host would be created for USB3 devices that have
"usb4-host-interface" property and udev->tunneled != native.

Thanks
Mathias