On 1/19/2024 04:22, Mika Westerberg wrote:
On Fri, Jan 19, 2024 at 09:48:29AM +0200, Mika Westerberg wrote:
On Fri, Jan 19, 2024 at 07:37:56AM +0200, Mika Westerberg wrote:
On Thu, Jan 18, 2024 at 08:12:56AM -0800, Dmitry Torokhov wrote:
On Thu, Jan 18, 2024 at 09:47:07AM -0600, Mario Limonciello wrote:
On 1/18/2024 00:00, Mika Westerberg wrote:
Before my patch, you see that the JHL6540 controller is inaccurately
labeled “removable”:
$ udevadm info -a -p /sys/bus/pci/devices/0000:05:00.0 | grep -e
{removable} -e {device} -e {vendor} -e looking
looking at device '/devices/pci0000:00/0000:00:1d.4/0000:05:00.0':
ATTR{device}=="0x15d3"
ATTR{removable}=="removable"
ATTR{vendor}=="0x8086"
This is actually accurate. The Thunderbolt controller is itself
hot-removable and that BTW happens to be hot-removed when fwupd applies
firmware upgrades to the device.
This is quite interesting take. Does fwupd rip the controller out of the
box to update it? By that account your touchpad is also removable as it
may stop functioning when its firmware gets updated.
The Thunderbolt controller is connected to a hotpluggable PCIe root port
so it will be dissappear from the userspace so that "removable" in that
sense is accurate.
There are systems as well where the Thunderbolt (and/or xHCI) controller
only appears if there is anything plugged to the physical Type-C ports
and it gets removed pretty soon after the physical device gets
unplugged. These are also the same Alpine Ridge and Titan Ridge
controllers that this patch is dealing with.
I tried to think about some sort of more generic heuristic how to figure
out that the controller is actually inside the physical system but there
is a problem that the same controller can appear on the bus as well, eg.
you plug in Thunderbolt dock and that one has xHCI controller too. That
device should definitely be "removable". With the "software CM" systems
we have a couple of additional hints in the ACPI tables that can be used
to identify the "tunneled" ports but this does not apply to the older
systems I'm afraid.
The below "might" work:
1. A device that is directly behind a PCIe root or downstream port that
has ->external_facing == 1.
2. It is a PCIe endpoint.
3. It is a sibling to or has any of the below PCI IDs (see
drivers/thunderbolt/nhi.h for the definitions):
PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_4C_NHI
PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_2C_NHI
PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_LP_USBONLY_NHI
PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_USBONLY_NHI
PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_USBONLY_NHI
PCI_DEVICE_ID_INTEL_TITAN_RIDGE_2C_NHI
PCI_DEVICE_ID_INTEL_TITAN_RIDGE_4C_NHI
And for all USB4 we can use the PCI class:
PCI_CLASS_SERIAL_USB_USB4
(4. With USB4 systems we could also add the check that the device is not
below the tunneled PCIe ports but that's kind of redundant).
I think this covers the existing devices as well as future discrete USB4
controllers and marks both the NHI and the xHCI as "fixed" which we
could also use to lift the bounce buffering restriction from them.
@Mario, did I miss anything?
The bounce buffering restriction is only for unaligned DMA isn't it?
Does that tend to happen a lot?
But otherwise I think this does a good job. It will cover external
enclosure cases too because of having to check it's directly behind a
root port.
But it should also include comments about why it's not needed on newer
systems (IE the ACPI hints for someone with no prior knowledge looking
at this to find).
