[U-Boot] [PATCH 1/2] usb: xhci: implement FEAT_POWER hook for switching regulators for ports

[Bin Meng]

Hi Philipp,

On Fri, Nov 17, 2017 at 9:50 PM, Dr. Philipp Tomsich
<philipp.tomsich at theobroma-systems.com> wrote:
>
>> On 17 Nov 2017, at 14:19, Bin Meng <bmeng.cn at gmail.com> wrote:
>>
>> Hi Philipp,
>>
>> On Fri, Nov 17, 2017 at 5:43 PM, Dr. Philipp Tomsich
>> <philipp.tomsich at theobroma-systems.com> wrote:
>>> Bin,
>>>
>>> On 17 Nov 2017, at 10:27, Bin Meng <bmeng.cn at gmail.com> wrote:
>>>
>>> Hi Philipp,
>>>
>>> On Tue, Nov 7, 2017 at 6:04 AM, Philipp Tomsich
>>> <philipp.tomsich at theobroma-systems.com> wrote:
>>>
>>> When the FEAT_POWER flag is set/cleared for a port, power to this port
>>> should be enabled/disabled.  As many embedded xHCI controllers do not
>>> expose a signal to control this, extra effort may be required.
>>>
>>> In order to link up setting/clearing FEAT_POWER with the regulator
>>> framework (so either a regulator or a GPIO modelled as a fixed
>>> regulator) can be switched, two callbacks are implemented in this
>>> change: if regulators are available an optional property
>>> 'xhci,port-power' can contain a stringlist with names of regulators
>>> that should be switched to control the power of ports (each entry in
>>> the stringlist corresponds to its respective regulator).
>>>
>>> For some versions of the RK3399-Q7 (at least revisions v1.1 and v1.2
>>> are affected), we need to turn on the power for the port connected to
>>> the on-module USB hub only when FEAT_POWER is set to ensure that the
>>> hub does not enter a low-power mode that U-Boot's USB stack can't deal
>>> with.  Note that Linux eventually manages to attach the hub even when
>>> it's in its low-power state after a few seconds.
>>>
>>>
>>> Please help me understand the problem. At first glance, I don't think
>>> such change should be put in the xHCI core codes.
>>> To me, this patch sounds like we use the regulator to enable/disable
>>> the power to an external on-board USB hub. For the xHC root port, this
>>> port power on/off is already configured by the xHC registers.
>>>
>>>
>>> The problem is that we have only a limited window of time between enabling
>>> the power to the USB hub and the USB controller starting its link-up
>>> procedure
>>> (i.e. reset of the port and waiting for a port-change), as the hub will
>>> otherwise
>>> go into a low-power mode and only come up for a short period of time every
>>> [I don’t remember the amount of time] milliseconds.
>>>
>>
>> Does the USB hub keep online all the time when the power is supplied
>> by the regulator? Is this entering low-power mode automatic? Can it be
>> turned off?
>
> Once power starts probing for a link and (if it misses the link establishment)
> then goes into this low power mode where it wakes up every [x] milliseconds.
> So if we miss the initial active period for the attachment, then we’re pretty
> much lost within the constraints (i.e. w/o asynchronous port-change notifications)
> of the U-Boot USB stack.
>

Yes, I understand the lower-power mode.

> Turning this behaviour off requires an additional SPI flash for the USB hub,
> which may (as in “no new version has been scheduled yet”) be added in a future
> revision of the board.  However, there are affected revisions of this product in
> the field that would benefit from fixing this (w/o this fix, only the dual-role port
> can be used from within U-Boot).
>

So from your description, this automatic low-power mode entering is
designed to be the default behavior. Turning it off requires an SPI
flash which I believe is for the hub to load the configuration data to
turn it off. OK, we have to endure this.

>>
>>> While this is no problem on Linux (the USB hub eventually gets detected
>>> after
>>> a delay of 2-3 seconds), the U-Boot USB stack (as it’s not interrupt driven)
>>> usually misses the port-change event (i.e. the probing has already moved on
>>> by the time the USB hub tries again).
>>>
>>> By having a callback directly from within the xHCI root-hub code, we can
>>> ensure that we always catch the USB hub.  We can’t fix this from the board
>>> code, as the time between us enabling the regulator and the probing from
>>> within ‘usb start’ will be unpredictable.  My first instinct was to have a
>>> call
>>> to a board-specific weak-function from xhci, but I finally settled on using
>>> the regulator-framework to have some that can be reused in the general
>>> case and also allows for on-demand power-on/off of on-board USB devices
>>> (if anybody has the need for it).
>>>
>>
>> If turning off the low-power mode is not possible, there is already a
>> weak function usb_hub_reset_devices() in current U-Boot USB stack. I
>> think you can use that existing one?
>
> I don’t think so, as the on-module hub won’t even show up on the bus if
> it’s in its low-power mode and usb_hub_configure (which in turn calls the
> usb_hub_reset_devices-hook) wouldn’t even be invoked for this case.
>

usb_hub_configure() calls usb_hub_power_on() which in turn set the
port feature (USB_PORT_FEAT_POWER). And usb_hub_reset_devices() is
called right after usb_hub_power_on().

> Furthermore, usb_hub_reset_devices(int) only passes a port-number and
> we need to be selective on a specific root-port of a specific controller (we
> have 2 xHCIs active in our default DTS).

Then this usb_hub_reset_devices() needs to be enhanced to provide an
additional argument for the xHC controller.

It's quite odd for the xHCI codes to handle any external power
regulator requirements. The names
xhci_board_port_poweron()/xhci_board_port_poweroff() mislead people to
believe they are something related to the root port power, but in fact
they are not. It's just for such low-power mode USB hub devices.

Regards,
Bin