Re: [PATCH v2] usb: usb-acpi: Set port connect type of not connectable ports correctly

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 2024-02-23 15:03, Mathias Nyman wrote:
Ports with  _UPC (USB Port Capability) ACPI objects stating they are
"not connectable" are not wired to any connector or internal device.
They only exist inside the host controller.

These ports may not have an ACPI _PLD (Physical Location of Device)
object.

Rework the code so that _UPC is read even if _PLD does not exist, and
make sure the port->connect_type is set to "USB_PORT_NOT_USED" instead
of "USB_PORT_CONNECT_TYPE_UNKNOWN".

No bugs or known issues are reported due to possibly not parsing _UPC,
and thus leaving the port connect type as "unknown" instead of
"not used". Nice to have this fixed but no need to add it to stable
kernels, or urgency to get it upstream.

With this patch (f3ac348e6e04501479fecf55250b25ff2092540b in next-20240307), my machine fails to boot. I was able to get some kernel console output from the pstore when compiling USB support as a module instead of built in. Reverting it on top of next-20240307 resolves the issue for me.

Please tell me if there's anything else you need.

Kind regards,
Klara Modin


Signed-off-by: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx>
---
v1 -> v2
   - Commit message improvements
   - send patch separately for easier picking to usb-next

  drivers/usb/core/usb-acpi.c | 46 ++++++++++++++++++-------------------
  1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/drivers/usb/core/usb-acpi.c b/drivers/usb/core/usb-acpi.c
index a34b22537d7c..f250dfc3b14d 100644
--- a/drivers/usb/core/usb-acpi.c
+++ b/drivers/usb/core/usb-acpi.c
@@ -142,12 +142,19 @@ int usb_acpi_set_power_state(struct usb_device *hdev, int index, bool enable)
  }
  EXPORT_SYMBOL_GPL(usb_acpi_set_power_state);
-static enum usb_port_connect_type usb_acpi_get_connect_type(acpi_handle handle,
-		struct acpi_pld_info *pld)
+/*
+ * Private to usb-acpi, all the core needs to know is that
+ * port_dev->location is non-zero when it has been set by the firmware.
+ */
+#define USB_ACPI_LOCATION_VALID (1 << 31)
+
+static void
+usb_acpi_get_connect_type(struct usb_port *port_dev, acpi_handle *handle)
  {
  	enum usb_port_connect_type connect_type = USB_PORT_CONNECT_TYPE_UNKNOWN;
  	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
  	union acpi_object *upc = NULL;
+	struct acpi_pld_info *pld;
  	acpi_status status;
/*
@@ -158,6 +165,12 @@ static enum usb_port_connect_type usb_acpi_get_connect_type(acpi_handle handle,
  	 * a usb device is directly hard-wired to the port. If no visible and
  	 * no connectable, the port would be not used.
  	 */
+
+	status = acpi_get_physical_device_location(handle, &pld);
+	if (ACPI_SUCCESS(status) && pld)
+		port_dev->location = USB_ACPI_LOCATION_VALID |
+			pld->group_token << 8 | pld->group_position;
+
  	status = acpi_evaluate_object(handle, "_UPC", NULL, &buffer);
  	if (ACPI_FAILURE(status))
  		goto out;
@@ -166,25 +179,22 @@ static enum usb_port_connect_type usb_acpi_get_connect_type(acpi_handle handle,
  	if (!upc || (upc->type != ACPI_TYPE_PACKAGE) || upc->package.count != 4)
  		goto out;
+ /* UPC states port is connectable */
  	if (upc->package.elements[0].integer.value)
-		if (pld->user_visible)
+		if (!pld)
+			; /* keep connect_type as unknown */
+		else if (pld->user_visible)
  			connect_type = USB_PORT_CONNECT_TYPE_HOT_PLUG;
  		else
  			connect_type = USB_PORT_CONNECT_TYPE_HARD_WIRED;
-	else if (!pld->user_visible)
+	else
  		connect_type = USB_PORT_NOT_USED;
  out:
+	port_dev->connect_type = connect_type;
  	kfree(upc);
-	return connect_type;
+	ACPI_FREE(pld);
  }
-
-/*
- * Private to usb-acpi, all the core needs to know is that
- * port_dev->location is non-zero when it has been set by the firmware.
- */
-#define USB_ACPI_LOCATION_VALID (1 << 31)
-
  static struct acpi_device *
  usb_acpi_get_companion_for_port(struct usb_port *port_dev)
  {
@@ -222,22 +232,12 @@ static struct acpi_device *
  usb_acpi_find_companion_for_port(struct usb_port *port_dev)
  {
  	struct acpi_device *adev;
-	struct acpi_pld_info *pld;
-	acpi_handle *handle;
-	acpi_status status;
adev = usb_acpi_get_companion_for_port(port_dev);
  	if (!adev)
  		return NULL;
- handle = adev->handle;
-	status = acpi_get_physical_device_location(handle, &pld);
-	if (ACPI_SUCCESS(status) && pld) {
-		port_dev->location = USB_ACPI_LOCATION_VALID
-			| pld->group_token << 8 | pld->group_position;
-		port_dev->connect_type = usb_acpi_get_connect_type(handle, pld);
-		ACPI_FREE(pld);
-	}
+	usb_acpi_get_connect_type(port_dev, adev->handle);
return adev;
  }
# bad: [1843e16d2df9d98427ef8045589571749d627cf7] Add linux-next specific files for 20240307
git bisect start 'next/master'
# status: waiting for good commit(s), bad commit known
# good: [67be068d31d423b857ffd8c34dbcc093f8dfff76] Merge tag 'vfs-6.8-release.fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs
git bisect good 67be068d31d423b857ffd8c34dbcc093f8dfff76
# good: [1a0a33e22e715a4cc7bb2709cfda8e83b8c756b2] Merge branch 'main' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git
git bisect good 1a0a33e22e715a4cc7bb2709cfda8e83b8c756b2
# good: [d96296508172e8d89203c45e2afbecfa6e6f5ab0] Merge branch 'master' of git://www.linux-watchdog.org/linux-watchdog-next.git
git bisect good d96296508172e8d89203c45e2afbecfa6e6f5ab0
# bad: [23d3ac3f173ed65eaa996b5f372329667fae3aba] Merge branch 'usb-next' of git://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git
git bisect bad 23d3ac3f173ed65eaa996b5f372329667fae3aba
# good: [621f909c060bc6fee2b86ed6716a1ebdfd1d5eff] Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git
git bisect good 621f909c060bc6fee2b86ed6716a1ebdfd1d5eff
# good: [210e8df477b2b834e0fa0a0d86559d6080d8ec42] Merge branch 'next' of https://github.com/kvm-x86/linux.git
git bisect good 210e8df477b2b834e0fa0a0d86559d6080d8ec42
# bad: [82e82130a78b75a9ce5225df24d5a0b1b3290eb0] usb: core: Set connect_type of ports based on DT node
git bisect bad 82e82130a78b75a9ce5225df24d5a0b1b3290eb0
# good: [5e7ea65daf13a95a6cc63d1377e4c500e4e1340f] usb: gadget: uvc: refactor the check for a valid buffer in the pump worker
git bisect good 5e7ea65daf13a95a6cc63d1377e4c500e4e1340f
# good: [73473b3033a633d640ef065aa98d60fbe2d40ddb] Merge tag 'thunderbolt-for-v6.9-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/westeri/thunderbolt into usb-next
git bisect good 73473b3033a633d640ef065aa98d60fbe2d40ddb
# good: [41717b88abf1cacd953e9ea2ace2f62eaf763c48] usb: dwc3: qcom: Remove ACPI support from glue driver
git bisect good 41717b88abf1cacd953e9ea2ace2f62eaf763c48
# bad: [8d36c0e433e0807700fe967953f8b0beb0dc5159] usb: dwc3: of-simple: Add compatible for hi3798mv200 DWC3 controller
git bisect bad 8d36c0e433e0807700fe967953f8b0beb0dc5159
# bad: [4d0a5a9915793377c0fe1a8d78de6bcd92cea963] usb: typec: ucsi: Clean up UCSI_CABLE_PROP macros
git bisect bad 4d0a5a9915793377c0fe1a8d78de6bcd92cea963
# bad: [0e28790195fa65fde41fa127a89e0903388f6285] usb: typec: tcpm: fix SOP' sequences in tcpm_pd_svdm
git bisect bad 0e28790195fa65fde41fa127a89e0903388f6285
# bad: [f3ac348e6e04501479fecf55250b25ff2092540b] usb: usb-acpi: Set port connect type of not connectable ports correctly
git bisect bad f3ac348e6e04501479fecf55250b25ff2092540b
# first bad commit: [f3ac348e6e04501479fecf55250b25ff2092540b] usb: usb-acpi: Set port connect type of not connectable ports correctly

Attachment: dmesg-1-decoded.txt.gz
Description: application/gzip

Attachment: dmesg-2-decoded.txt.gz
Description: application/gzip

Attachment: lshw.gz
Description: application/gzip

Attachment: lspci.gz
Description: application/gzip

Attachment: lsusb.gz
Description: application/gzip


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux