[PATCH] qmi-firmware-update: support USB3->USB2 mode changes during upgrade
Bjørn Mork
bjorn at mork.no
Wed Jan 24 12:48:04 UTC 2018
I guess we need USB/xhci experts if we want an explaination. It all
looks completely wrong to me.
Made another attempt on understanding drivers/usb/core/port.c, and I
now have a working theory. The real magic happens here, and the comment
hints where this cold go wrong - The "platform firmware" is trusted oever
anything else:
/*
* Find the peer port either via explicit platform firmware "location"
* data, the peer hcd for root hubs, or the upstream peer relationship
* for all other hubs.
*/
static void find_and_link_peer(struct usb_hub *hub, int port1)
{
struct usb_port *port_dev = hub->ports[port1 - 1], *peer;
struct usb_device *hdev = hub->hdev;
struct usb_device *peer_hdev;
struct usb_hub *peer_hub;
/*
* If location data is available then we can only peer this port
* by a location match, not the default peer (lest we create a
* situation where we need to go back and undo a default peering
* when the port is later peered by location data)
*/
if (port_dev->location) {
/* we link the peer in match_location() if found */
usb_for_each_dev(port_dev, match_location);
return;
} else if (!hdev->parent) {
struct usb_hcd *hcd = bus_to_hcd(hdev->bus);
struct usb_hcd *peer_hcd = hcd->shared_hcd;
if (!peer_hcd)
return;
peer_hdev = peer_hcd->self.root_hub;
} else {
..
The enabling magic is in drivers/usb/core/usb-acpi.c:
status = acpi_get_physical_device_location(handle, &pld);
if (ACPI_FAILURE(status) || !pld)
return adev;
port_dev->location = USB_ACPI_LOCATION_VALID
| pld->group_token << 8 | pld->group_position;
So this depens on the BIOS vendor managing to get the
"physical_device_location" correct (or at least equal) for both ports.
If ACPI tells us that the port locations are different, or that only one
of the ports have a location, then there will be no peer relationship.
I tried to figure out if there exists a simple way to make the kernel
tell us what happens when it calls acpi_get_physical_device_location(),
but I did not find much. This call will most likely return something
into &pld. The real question is what that is. And I see no way to
figure it out without patching the kernel.
We should fix that. A simple straight forward fix would be something
like the attached patch. Are you in a position to test that? It would
be good if we could confirm the theory that your BIOS gets this wrong.
How you do that depend a bit on your system. With a default Debian
stable kernel, this is how I tested it:
bjorn at miraculix:/usr/local/src/git/linux$ uname -a
Linux miraculix 4.9.0-5-amd64 #1 SMP Debian 4.9.65-3+deb9u2 (2018-01-04) x86_64 GNU/Linux
bjorn at miraculix:/usr/local/src/git/linux$ git checkout -b test-v4.9.65 v4.9.65
Switched to a new branch 'test-v4.9.65'
bjorn at miraculix:/usr/local/src/git/linux$ git am /tmp/0001-usb-export-firmware-port-location-in-sysfs.patch
Applying: usb: export firmware port location in sysfs
bjorn at miraculix:/usr/local/src/git/linux$ make -C /lib/modules/`uname -r`/build SUBDIRS=`pwd`/drivers/usb/core usbcore.ko
make: Entering directory '/usr/src/linux-headers-4.9.0-5-amd64'
CC [M] /usr/local/src/git/linux/drivers/usb/core/port.o
LD [M] /usr/local/src/git/linux/drivers/usb/core/usbcore.o
MODPOST 1 modules
LD [M] /usr/local/src/git/linux/drivers/usb/core/usbcore.ko
make: Leaving directory '/usr/src/linux-headers-4.9.0-5-amd64'
It's fully possible to test without rebooting. The hard part is to
unload all USB modules. Use lsmod to figure out what you need to
unload. I did (yes, I have tested many different USB devices since my
last reboot, so I had lots of USB drivers to unload):
# modprobe -r usbmon usbhid uas usb_storage btusb ax88179_178a cdc_mbim qmi_wwan cdc_ncm
# modprobe -r qcserial option cp210x pl2303
modprobe: FATAL: Module qcserial is in use.
# systemctl stop ModemManager
# modprobe -r qcserial option cp210x pl2303
# modprobe -r uvcvideo pegasus xpad xhci_pci xhci_hcd
Then loading a minimum of the drivers, only this time with the newly
build usbcore.ko:
# modprobe usb_common
# insmod /usr/local/src/git/linux/drivers/usb/core/usbcore.ko
# modprobe xhci_pci
The rest will autoload at this point. The patch will simply show
whatever 'location' we got from ACPI in sysfs:
bjorn at miraculix:/usr/local/src/git/linux$ grep . /sys/bus/usb/devices/usb*/*-0\:1.0/usb*-port*/location
/sys/bus/usb/devices/usb1/1-0:1.0/usb1-port10/location:0x80000000
/sys/bus/usb/devices/usb1/1-0:1.0/usb1-port11/location:0x80000000
/sys/bus/usb/devices/usb1/1-0:1.0/usb1-port12/location:0x80000000
/sys/bus/usb/devices/usb1/1-0:1.0/usb1-port1/location:0x80000001
/sys/bus/usb/devices/usb1/1-0:1.0/usb1-port2/location:0x80000000
/sys/bus/usb/devices/usb1/1-0:1.0/usb1-port3/location:0x80000002
/sys/bus/usb/devices/usb1/1-0:1.0/usb1-port4/location:0x80000004
/sys/bus/usb/devices/usb1/1-0:1.0/usb1-port5/location:0x80000000
/sys/bus/usb/devices/usb1/1-0:1.0/usb1-port6/location:0x80000003
/sys/bus/usb/devices/usb1/1-0:1.0/usb1-port7/location:0x80000000
/sys/bus/usb/devices/usb1/1-0:1.0/usb1-port8/location:0x80000000
/sys/bus/usb/devices/usb1/1-0:1.0/usb1-port9/location:0x80000000
/sys/bus/usb/devices/usb2/2-0:1.0/usb2-port1/location:0x80000001
/sys/bus/usb/devices/usb2/2-0:1.0/usb2-port2/location:0x80000000
/sys/bus/usb/devices/usb2/2-0:1.0/usb2-port3/location:0x80000002
/sys/bus/usb/devices/usb2/2-0:1.0/usb2-port4/location:0x80000000
/sys/bus/usb/devices/usb2/2-0:1.0/usb2-port5/location:0x80000000
/sys/bus/usb/devices/usb2/2-0:1.0/usb2-port6/location:0x80000003
The high bit is the USB_ACPI_LOCATION_VALID quoted above, telling us
that my firmware did return a valid acpi_get_physical_device_location()
result for every port, as expected. And you can easily see how the peer
ports align up. But you can also easily see the kernel bug here... The
resulting peer relationships are:
bjorn at miraculix:/usr/local/src/git/linux$ ls -ld /sys/bus/usb/devices/usb*/*-0:1.0/usb*-port*/peer
lrwxrwxrwx 1 root root 0 Jan 24 13:36 /sys/bus/usb/devices/usb1/1-0:1.0/usb1-port1/peer -> ../../../usb2/2-0:1.0/usb2-port1
lrwxrwxrwx 1 root root 0 Jan 24 13:36 /sys/bus/usb/devices/usb1/1-0:1.0/usb1-port2/peer -> ../../../usb2/2-0:1.0/usb2-port2
lrwxrwxrwx 1 root root 0 Jan 24 13:36 /sys/bus/usb/devices/usb1/1-0:1.0/usb1-port3/peer -> ../../../usb2/2-0:1.0/usb2-port3
lrwxrwxrwx 1 root root 0 Jan 24 13:36 /sys/bus/usb/devices/usb1/1-0:1.0/usb1-port6/peer -> ../../../usb2/2-0:1.0/usb2-port6
lrwxrwxrwx 1 root root 0 Jan 24 13:36 /sys/bus/usb/devices/usb2/2-0:1.0/usb2-port1/peer -> ../../../usb1/1-0:1.0/usb1-port1
lrwxrwxrwx 1 root root 0 Jan 24 13:36 /sys/bus/usb/devices/usb2/2-0:1.0/usb2-port2/peer -> ../../../usb1/1-0:1.0/usb1-port2
lrwxrwxrwx 1 root root 0 Jan 24 13:36 /sys/bus/usb/devices/usb2/2-0:1.0/usb2-port3/peer -> ../../../usb1/1-0:1.0/usb1-port3
lrwxrwxrwx 1 root root 0 Jan 24 13:36 /sys/bus/usb/devices/usb2/2-0:1.0/usb2-port6/peer -> ../../../usb1/1-0:1.0/usb1-port6
But the 'usb1-port2 <-> usb2-port2' connection is in fact bogus, and
completely arbitrary. There are a number of matching locations. These
two were only chosen because they happened to be first on the list.
Now, that does not matter much for me, because these ports are not in
use:
bjorn at miraculix:/usr/local/src/git/linux$ grep . /sys/bus/usb/devices/usb*/*-0\:1.0/usb*-port2/connect_type
/sys/bus/usb/devices/usb1/1-0:1.0/usb1-port2/connect_type:not used
/sys/bus/usb/devices/usb2/2-0:1.0/usb2-port2/connect_type:not used
But I believe it should be fixed. I guess I'll end up sending more than
one patch here. But let's first figure out if my suspicion is
justified, or if the BIOS vendor is innocent after all.
Bjørn
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-usb-export-firmware-port-location-in-sysfs.patch
Type: text/x-diff
Size: 1426 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/libqmi-devel/attachments/20180124/bd7588cb/attachment.patch>
-------------- next part --------------
Paul Gildea <gildeap at tcd.ie> writes:
> dmesg indicated SuperSpeed for 2-2. Here is the output of that command:
>
> ls -ld /sys/bus/usb/devices/usb*/*-0:1.0/usb*-port*/peer
> lrwxrwxrwx 1 root root 0 Jan 24 11:47
> /sys/bus/usb/devices/usb1/1-0:1.0/usb1-port11/peer ->
> ../../../usb2/2-0:1.0/usb2-port5/
> lrwxrwxrwx 1 root root 0 Jan 24 11:47
> /sys/bus/usb/devices/usb1/1-0:1.0/usb1-port6/peer ->
> ../../../usb2/2-0:1.0/usb2-port6/
> lrwxrwxrwx 1 root root 0 Jan 24 11:47
> /sys/bus/usb/devices/usb1/1-0:1.0/usb1-port7/peer ->
> ../../../usb2/2-0:1.0/usb2-port7/
> lrwxrwxrwx 1 root root 0 Jan 24 11:47
> /sys/bus/usb/devices/usb1/1-0:1.0/usb1-port8/peer ->
> ../../../usb2/2-0:1.0/usb2-port8/
> lrwxrwxrwx 1 root root 0 Jan 24 11:47
> /sys/bus/usb/devices/usb2/2-0:1.0/usb2-port5/peer ->
> ../../../usb1/1-0:1.0/usb1-port11/
> lrwxrwxrwx 1 root root 0 Jan 24 11:47
> /sys/bus/usb/devices/usb2/2-0:1.0/usb2-port6/peer ->
> ../../../usb1/1-0:1.0/usb1-port6/
> lrwxrwxrwx 1 root root 0 Jan 24 11:47
> /sys/bus/usb/devices/usb2/2-0:1.0/usb2-port7/peer ->
> ../../../usb1/1-0:1.0/usb1-port7/
> lrwxrwxrwx 1 root root 0 Jan 24 11:47
> /sys/bus/usb/devices/usb2/2-0:1.0/usb2-port8/peer ->
> ../../../usb1/1-0:1.0/usb1-port8/
> lrwxrwxrwx 1 root root 0 Jan 24 11:47
> /sys/bus/usb/devices/usb3/3-0:1.0/usb3-port1/peer ->
> ../../../usb4/4-0:1.0/usb4-port1/
> lrwxrwxrwx 1 root root 0 Jan 24 11:47
> /sys/bus/usb/devices/usb3/3-0:1.0/usb3-port2/peer ->
> ../../../usb4/4-0:1.0/usb4-port2/
> lrwxrwxrwx 1 root root 0 Jan 24 11:45
> /sys/bus/usb/devices/usb3/3-0:1.0/usb3-port3/peer ->
> ../../../usb4/4-0:1.0/usb4-port3/
> lrwxrwxrwx 1 root root 0 Jan 24 11:46
> /sys/bus/usb/devices/usb3/3-0:1.0/usb3-port4/peer ->
> ../../../usb4/4-0:1.0/usb4-port4/
> lrwxrwxrwx 1 root root 0 Jan 24 11:47
> /sys/bus/usb/devices/usb4/4-0:1.0/usb4-port1/peer ->
> ../../../usb3/3-0:1.0/usb3-port1/
> lrwxrwxrwx 1 root root 0 Jan 24 11:47
> /sys/bus/usb/devices/usb4/4-0:1.0/usb4-port2/peer ->
> ../../../usb3/3-0:1.0/usb3-port2/
> lrwxrwxrwx 1 root root 0 Jan 24 11:47
> /sys/bus/usb/devices/usb4/4-0:1.0/usb4-port3/peer ->
> ../../../usb3/3-0:1.0/usb3-port3/
> lrwxrwxrwx 1 root root 0 Jan 24 11:46
> /sys/bus/usb/devices/usb4/4-0:1.0/usb4-port4/peer ->
> ../../../usb3/3-0:1.0/usb3-port4/
>
>
> and this is the mapping for my modems, USB3 -> USB2:
>
>
> /sys/devices/pci0000:00/0000:00:14.0/usb2/2-1 ->
> /sys/devices/pci0000:00/0000:00:14.0/usb1/1-4
> /sys/devices/pci0000:00/0000:00:14.0/usb2/2-2 ->
> /sys/devices/pci0000:00/0000:00:14.0/usb1/1-1
> /sys/devices/pci0000:00/0000:00:14.0/usb2/2-3 ->
> /sys/devices/pci0000:00/0000:00:14.0/usb1/1-6
> /sys/devices/pci0000:00/0000:00:14.0/usb2/2-4 ->
> /sys/devices/pci0000:00/0000:00:14.0/usb1/1-8
>
>
> My S/N is the same in all modes for MC7455's, useful at least as a backup
> method if it is unreliable.
>
> --
> Paul
>
>
> On 23 Jan 2018 18:06, "Bjørn Mork" <bjorn at mork.no> wrote:
>
>> Paul Gildea <gildeap at tcd.ie> writes:
>>
>> > Checking 2-2 there is no such peer file inside port for some reason.
>>
>> OK, at least that explains the failure. And the 2-2 device is of course
>> running at SuperSpeed?
>>
>> Tried to read drivers/usb/core/port.c , but not sure I understand if/how
>> this is supposed to work. Looks like there are plenty of possible
>> failure modes. And most will be silently ignored unless you enable
>> kernel debug messages. Yuck.
>>
>> Could you do a 'ls -ld /sys/bus/usb/devices/usb*/*-0:1.0/usb*-port*/peer'?
>>
>> I tried finding examples of missing peers, but all my Linux systems with
>> USB3 look fine. My laptop:
>>
>>
>> bjorn at miraculix:~$ ls -ld /sys/bus/usb/devices/usb*/*-0:
>> 1.0/usb*-port*/peer
>> lrwxrwxrwx 1 root root 0 Jan 12 09:59 /sys/bus/usb/devices/usb1/1-0:1.0/usb1-port1/peer
>> -> ../../../usb2/2-0:1.0/usb2-port1
>> lrwxrwxrwx 1 root root 0 Jan 11 13:53 /sys/bus/usb/devices/usb1/1-0:1.0/usb1-port2/peer
>> -> ../../../usb2/2-0:1.0/usb2-port2
>> lrwxrwxrwx 1 root root 0 Jan 12 09:58 /sys/bus/usb/devices/usb1/1-0:1.0/usb1-port3/peer
>> -> ../../../usb2/2-0:1.0/usb2-port3
>> lrwxrwxrwx 1 root root 0 Jan 12 09:12 /sys/bus/usb/devices/usb1/1-0:1.0/usb1-port6/peer
>> -> ../../../usb2/2-0:1.0/usb2-port6
>> lrwxrwxrwx 1 root root 0 Jan 11 13:52 /sys/bus/usb/devices/usb2/2-0:1.0/usb2-port1/peer
>> -> ../../../usb1/1-0:1.0/usb1-port1
>> lrwxrwxrwx 1 root root 0 Jan 12 09:55 /sys/bus/usb/devices/usb2/2-0:1.0/usb2-port2/peer
>> -> ../../../usb1/1-0:1.0/usb1-port2
>> lrwxrwxrwx 1 root root 0 Jan 12 09:58 /sys/bus/usb/devices/usb2/2-0:1.0/usb2-port3/peer
>> -> ../../../usb1/1-0:1.0/usb1-port3
>> lrwxrwxrwx 1 root root 0 Jan 12 09:12 /sys/bus/usb/devices/usb2/2-0:1.0/usb2-port6/peer
>> -> ../../../usb1/1-0:1.0/usb1-port6
>>
>>
>> A desktop:
>>
>> bjorn at canardo:~$ ls -ld /sys/bus/usb/devices/usb*/*-0:1.0/usb*-port*/peer
>> lrwxrwxrwx 1 root root 0 Jan 23 18:44 /sys/bus/usb/devices/usb1/1-0:1.0/usb1-port10/peer
>> -> ../../../usb2/2-0:1.0/usb2-port10
>> lrwxrwxrwx 1 root root 0 Jan 23 18:44 /sys/bus/usb/devices/usb1/1-0:1.0/usb1-port1/peer
>> -> ../../../usb2/2-0:1.0/usb2-port1
>> lrwxrwxrwx 1 root root 0 Jan 23 18:44 /sys/bus/usb/devices/usb1/1-0:1.0/usb1-port2/peer
>> -> ../../../usb2/2-0:1.0/usb2-port2
>> lrwxrwxrwx 1 root root 0 Jan 23 18:44 /sys/bus/usb/devices/usb1/1-0:1.0/usb1-port3/peer
>> -> ../../../usb2/2-0:1.0/usb2-port3
>> lrwxrwxrwx 1 root root 0 Jan 23 18:44 /sys/bus/usb/devices/usb1/1-0:1.0/usb1-port4/peer
>> -> ../../../usb2/2-0:1.0/usb2-port4
>> lrwxrwxrwx 1 root root 0 Jan 23 18:44 /sys/bus/usb/devices/usb1/1-0:1.0/usb1-port5/peer
>> -> ../../../usb2/2-0:1.0/usb2-port5
>> lrwxrwxrwx 1 root root 0 Jan 23 18:44 /sys/bus/usb/devices/usb1/1-0:1.0/usb1-port6/peer
>> -> ../../../usb2/2-0:1.0/usb2-port6
>> lrwxrwxrwx 1 root root 0 Jan 23 18:44 /sys/bus/usb/devices/usb1/1-0:1.0/usb1-port7/peer
>> -> ../../../usb2/2-0:1.0/usb2-port7
>> lrwxrwxrwx 1 root root 0 Jan 23 18:44 /sys/bus/usb/devices/usb1/1-0:1.0/usb1-port8/peer
>> -> ../../../usb2/2-0:1.0/usb2-port8
>> lrwxrwxrwx 1 root root 0 Jan 23 18:44 /sys/bus/usb/devices/usb1/1-0:1.0/usb1-port9/peer
>> -> ../../../usb2/2-0:1.0/usb2-port9
>> lrwxrwxrwx 1 root root 0 Jan 23 18:44 /sys/bus/usb/devices/usb2/2-0:1.0/usb2-port10/peer
>> -> ../../../usb1/1-0:1.0/usb1-port10
>> lrwxrwxrwx 1 root root 0 Jan 23 18:44 /sys/bus/usb/devices/usb2/2-0:1.0/usb2-port1/peer
>> -> ../../../usb1/1-0:1.0/usb1-port1
>> lrwxrwxrwx 1 root root 0 Jan 23 18:44 /sys/bus/usb/devices/usb2/2-0:1.0/usb2-port2/peer
>> -> ../../../usb1/1-0:1.0/usb1-port2
>> lrwxrwxrwx 1 root root 0 Jan 23 18:44 /sys/bus/usb/devices/usb2/2-0:1.0/usb2-port3/peer
>> -> ../../../usb1/1-0:1.0/usb1-port3
>> lrwxrwxrwx 1 root root 0 Jan 23 18:44 /sys/bus/usb/devices/usb2/2-0:1.0/usb2-port4/peer
>> -> ../../../usb1/1-0:1.0/usb1-port4
>> lrwxrwxrwx 1 root root 0 Jan 23 18:44 /sys/bus/usb/devices/usb2/2-0:1.0/usb2-port5/peer
>> -> ../../../usb1/1-0:1.0/usb1-port5
>> lrwxrwxrwx 1 root root 0 Jan 23 18:44 /sys/bus/usb/devices/usb2/2-0:1.0/usb2-port6/peer
>> -> ../../../usb1/1-0:1.0/usb1-port6
>> lrwxrwxrwx 1 root root 0 Jan 23 18:44 /sys/bus/usb/devices/usb2/2-0:1.0/usb2-port7/peer
>> -> ../../../usb1/1-0:1.0/usb1-port7
>> lrwxrwxrwx 1 root root 0 Jan 23 18:44 /sys/bus/usb/devices/usb2/2-0:1.0/usb2-port8/peer
>> -> ../../../usb1/1-0:1.0/usb1-port8
>> lrwxrwxrwx 1 root root 0 Jan 23 18:44 /sys/bus/usb/devices/usb2/2-0:1.0/usb2-port9/peer
>> -> ../../../usb1/1-0:1.0/usb1-port9
>>
>> A Linksys router:
>>
>> root at wrt1900ac-1:~# ls -ld /sys/bus/usb/devices/usb*/*-0:
>> 1.0/usb*-port*/peer
>> lrwxrwxrwx 1 root root 0 Jan 12 09:50
>> /sys/bus/usb/devices/usb2/2-0:1.0/usb2-port1/peer ->
>> ../../../usb3/3-0:1.0/usb3-port1
>> lrwxrwxrwx 1 root root 0 Jan 12 09:51
>> /sys/bus/usb/devices/usb2/2-0:1.0/usb2-port2/peer ->
>> ../../../usb3/3-0:1.0/usb3-port2
>> lrwxrwxrwx 1 root root 0 Jan 12 09:50
>> /sys/bus/usb/devices/usb3/3-0:1.0/usb3-port1/peer ->
>> ../../../usb2/2-0:1.0/usb2-port1
>> lrwxrwxrwx 1 root root 0 Jan 12 09:51
>> /sys/bus/usb/devices/usb3/3-0:1.0/usb3-port2/peer ->
>> ../../../usb2/2-0:1.0/usb2-port2
>>
>>
>> I was really hoping we could use these peer links, because I cannot see
>> how else we're supposed to make the EM7455 upgrade work on SuperSpeed
>> capable ports.
>>
>> OK, I see one alternative: The MC7455/EM7455 serial number is the same
>> in bootloader and application mode (at least on my modems...). We could
>> save it and accept the bootloader QDL device on any USB port if the
>> serial number matches.
>>
>> Note that this particular serial number behaviour is in no way
>> guaranteed anywhere. So it does feel somewhat less safe than matching
>> USB ports. My EM7565 will change the serial number, for example. But its
>> bootloader use SuperSpeed so it doesn't have the same upgrade issue.
>>
>>
>> Bjørn
>>
More information about the libqmi-devel
mailing list