rpi5: USB over-current condition triggered via gpio

nbuchwitz commented 9 months ago

Describe the bug

I've been debugging usb port power switching using libusb and rpi5 (another topic/issue). While researching how and if the vbus may be switched with rp1, I came upon the usb_vbus_pins definition in bcm2712-rpi-5-b.dts.

Gpio 42 appears to switch the vbus for all USB ports at once, whereas GPIO 43 appears to detect overcurrent. If I query gpio 43, the Raspberry Pi 5 enters overcurrent protection and all USB ports are disabled. Even though this appears to be correct, it could be a problem because the typical Pi user has access to all gpio pins, and a rogue application can block all USB ports.

I don't know where the usb_vbus_pins are used, but maybe a gpio hog for at least the over current pin can prevent further damage?

Steps to reproduce the behaviour

Straight to over current: gpioget 4 43

or just some switching: gpioset 4 42=0

Device (s)

Raspberry Pi 5

System

Raspberry Pi reference 2023-10-10
Generated using pi-gen, https://github.com/RPi-Distro/pi-gen, 962bf483c8f326405794827cce8c0313fd5880a8, stage4

2024/01/05 15:57:40 
Copyright (c) 2012 Broadcom
version 30cc5f37 (release) (embedded)

Linux pi5 6.1.0-rpi7-rpi-2712 #1 SMP PREEMPT Debian 1:6.1.63-1+rpt1 (2023-11-24) aarch64 GNU/Linux

Logs

No response

Additional context

No response

pelwell commented 9 months ago

It's interesting that gpioget 4 43 behaves differently to gpioget 0 24. I expected them both to change the fsel/alt function to make them a GPIO, but gpioget 0 24 is happy to return the pin level while keeping it as a UART RTS pin.

nbuchwitz commented 9 months ago

Right. A pinctrl set 43 a2 pu resets the over current pin as expected and usb is unblocked. Tested some other gpios (also on different chips) and most of them persists the fsel. FAN_PWM is an exception and does also change it's fsel:

nbw@pi5:~/uhubctl $ pinctrl | grep FAN_PWM
45: a0    pd | hi // FAN_PWM/GPIO45 = PWM1_CHAN3
nbw@pi5:~/uhubctl $ gpioget 4 45
0
nbw@pi5:~/uhubctl $ pinctrl | grep FAN_PWM
45: ip    pd | lo // FAN_PWM/GPIO45 = input

Could it be, that the other pins (eg. UART RTS) are held by some drivers? Gpiod lists them as unused, but at least the BT pins should be used by hci_uart.

pelwell commented 9 months ago

I don't think I can put gpio-hogs on these pins without also making them GPIOs, which I don't want to do - they need to have the vbus1 alt function so the RP1 USB hardware can handle them appropriately.

nbuchwitz commented 9 months ago

Right, a hog can only be output or input ... So there isn't really a solution to prevent this besides a dummy driver / user space application which binds to the gpio label I guess.

Related question: What's the deal with vbus0 (28/29), vbus2 (50/51) and vbus3 (52/53)? Are these meant for individual port vbus control? Thus with access to these gpios one could control each port individually? 50-53 seem unused, while 35/36 unfortunately are used .

pelwell commented 9 months ago

I think RP1 has a flexible mapping from ports to vbus<n> functions for use in other applications, and similarly for vbus_oc<n>, but as Pi 5 has a single VBUS power switch and overcurrent detector it's all academic.

P33M commented 9 months ago

Yes, the supportable configurations are

Ganged and grouped Vbus/OC
Grouped Vbus/OC
Individual Vbus/OC Individual breaks out each port's overcurrent and vbus_en signal, derived from the respective xhci PORTSC register. Grouped will multiplex both ports on one controller together, for a single vbus_en/overcurrent pair per controller. Ganged and grouped multiplexes both controllers together for a single vbus_en/overcurrent pair.

nbuchwitz commented 9 months ago

Thanks for the clarification, @P33M.

So the hardware design of pi 5 is ganged and thus should set HUB_CHAR_COMMON_LPSM (0x0) in wHubCharacteristics. Unfortunately this is not the case and the RP1 reports HUB_CHAR_INDV_PORT_LPSM (0x1). Is there a way RP1 needs to be strapped? How does the configuration work (esp. which vbus is taken for ganged / grouped vbus)?

_EDIT: PortPwrCtrlMask has all bits set, which indicates multiple gangs (11.11.1 usb 2.0 specs). So if the rp1 is in "Ganged and grouped Vbus/OC" mode, why not HUB_CHAR_COMMON_LPSM?_

nbw@pi5:~/dev/rpi/linux $ sudo lsusb -vv -s 1:1

Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
[...]
Hub Descriptor:
  bLength               9
  bDescriptorType      41
  nNbrPorts             2
  wHubCharacteristic 0x0009
    Per-port power switching
    Per-port overcurrent protection
    TT think time 8 FS bits
  bPwrOn2PwrGood       10 * 2 milli seconds
  bHubContrCurrent      0 milli Ampere
  DeviceRemovable    0x00
  PortPwrCtrlMask    0xff
 Hub Port Status:
   Port 1: 0000.0100 power
   Port 2: 0000.0100 power
can't get device qualifier: Resource temporarily unavailable
can't get debug descriptor: Resource temporarily unavailable
Device Status:     0x0001
  Self Powered

EDIT 2: I guess because it's kinda deprecated:

Note: To ensure compatibility with previous versions of USB Software, hubs must implement the Logical Power Switching Mode field in wHubCharacteristics. This is because some versions of SW will not use the SetPortFeature() request if the hub indicates in wHubCharacteristics that the port does not support port power switching. Otherwise, the Logical Power Switching Mode field in wHubCharacteristics would have become redundant as of this version of the specification.

lategoodbye commented 9 months ago

I don't think I can put gpio-hogs on these pins without also making them GPIOs, which I don't want to do - they need to have the vbus1 alt function so the RP1 USB hardware can handle them appropriately.

Yes, gpio-hogs is not the proper solution here. But how about moving the affected pins under the pinctrl of the USB driver like most of the other boards do?

pelwell commented 9 months ago

What, like this? https://github.com/raspberrypi/linux/blob/rpi-6.1.y/arch/arm/boot/dts/bcm2712-rpi-5-b.dts#L241

lategoodbye commented 9 months ago

Yes, this looks like as a good starting point.

Maybe the USB driver needs to claim these pins? Or there is an issue with the rp1 pinctrl driver. I vaguely remember a strict flag for pinmux.

pelwell commented 9 months ago

Claim them as what? They're already claimed for pinmux:

Format: pin (name): mux_owner gpio_owner hog?
...
pin 42 (gpio42): 1f00200000.usb (GPIO UNCLAIMED) function vbus1 group gpio42

Claiming them for GPIO would change the FSEL, which is what we're trying to avoid.

Getting RP1 pinctrl driver to enable strict pinmux mode does make a visible difference:

pi@raspberrypi:~$ cat /sys/kernel/debug/pinctrl/*rp1/pinmux-pins
Pinmux settings per pin
Format: pin (name): mux_owner|gpio_owner (strict) hog?
...
pin 42 (gpio42): device 1f00200000.usb function vbus1 group gpio42

However, it doesn't fix the problem:

pi@raspberrypi:~$ pinctrl get 42; gpioget USB_VBUS_EN; pinctrl get 42
42: a2    pd | hi // USB_VBUS_EN/GPIO42 = VBUS_EN1
"USB_VBUS_EN"=inactive
42: ip    pd | lo // USB_VBUS_EN/GPIO42 = input

raspberrypi / linux