diff mbox series

[v4] usb: export firmware port location in sysfs

Message ID 20180928134031.2121-1-bjorn@mork.no (mailing list archive)
State New, archived
Headers show
Series [v4] usb: export firmware port location in sysfs | expand

Commit Message

Bjørn Mork Sept. 28, 2018, 1:40 p.m. UTC
The platform firmware "location" data is used to find port peer
relationships. But firmware is an unreliable source, and there are
real world examples of errors leading to missing or wrong peer
relationships.  Debugging this is currently hard.

Exporting the location attribute makes it easier to spot mismatches
between the firmware data and the real world.

Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
v4: remove unrelated ABI doc changes, thanks to Alan Stern for noticing
v3: resurrect missing hunk, as pointed out by the kbuild test robot
v2: Sorry, forgot to rebase the old branch before sending v1

This patch got stuck in one of my debugging branches.  It proved very
useful to me while trying to figure out why the "peer" link was useless
on a specific host. And if it was useful to me once, then maybe it will
be to someone else as well...

tl;dr; The full use case for anyone interested:

Some current LTE modems with USB3 SS support come with a bootloader
supporting USB2 only. The application and bootloader modes are provided
by different softwares running on the modem (which of course is just
another Linux system with a UDC).  None of the descriptors are therefore
guaranteed to be identical, or even similar.

Doing a firmware upgrade of such a device involves
 - some preparation in application mode (USB3), 
 - rebooting into bootloader mode (USB2), and 
 - finally booting into the upgraded application firmware (USB3) to verify
   the upgrade. 

The firmware upgrade tool should make sure it talks to the same physical
device in all three phases.  The only semi-reliable way to do that is to
look for "new" devices in the expected mode, connected to the same physical
USB port. But the locical port will change due to the USB2/3 switch, and
all we are left with is the "peer" link.  Which can, and do, fail due to
buggy ACPI tables.

This patch won't solve that problem, but it makes it a lot easier to
detect.

Lesson for the next time: Either submit rightaway or just delete it.
Three attempts to get this even building without a warning is more
than embarrassing.  Sorry about that.  It *is* build tested now. And
even run tested. Honestly ;-)


 Documentation/ABI/testing/sysfs-bus-usb | 10 ++++++++++
 drivers/usb/core/port.c                 | 10 ++++++++++
 2 files changed, 20 insertions(+)
diff mbox series

Patch

diff --git a/Documentation/ABI/testing/sysfs-bus-usb b/Documentation/ABI/testing/sysfs-bus-usb
index 08d456e07b53..7ada65345251 100644
--- a/Documentation/ABI/testing/sysfs-bus-usb
+++ b/Documentation/ABI/testing/sysfs-bus-usb
@@ -189,6 +189,16 @@  Description:
 		The file will read "hotplug", "wired" and "not used" if the
 		information is available, and "unknown" otherwise.
 
+What:		/sys/bus/usb/devices/.../(hub interface)/portX/location
+Date:		October 2018
+Contact:	Bjørn Mork <bjorn@mork.no>
+Description:
+		Some platforms provide usb port physical location through
+		firmware. This is used by the kernel to pair up logical ports
+		mapping to the same physical connector. The attribute exposes the
+		raw location value as a hex integer.
+
+
 What:		/sys/bus/usb/devices/.../(hub interface)/portX/quirks
 Date:		May 2018
 Contact:	Nicolas Boichat <drinkcat@chromium.org>
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index 4a2143195395..1a06a4b5fbb1 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -16,6 +16,15 @@  static int usb_port_block_power_off;
 
 static const struct attribute_group *port_dev_group[];
 
+static ssize_t location_show(struct device *dev,
+			     struct device_attribute *attr, char *buf)
+{
+	struct usb_port *port_dev = to_usb_port(dev);
+
+	return sprintf(buf, "0x%08x\n", port_dev->location);
+}
+static DEVICE_ATTR_RO(location);
+
 static ssize_t connect_type_show(struct device *dev,
 				 struct device_attribute *attr, char *buf)
 {
@@ -140,6 +149,7 @@  static DEVICE_ATTR_RW(usb3_lpm_permit);
 
 static struct attribute *port_dev_attrs[] = {
 	&dev_attr_connect_type.attr,
+	&dev_attr_location.attr,
 	&dev_attr_quirks.attr,
 	&dev_attr_over_current_count.attr,
 	NULL,