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

[Bin Meng]

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.

> Signed-off-by: Philipp Tomsich <philipp.tomsich at theobroma-systems.com>
> ---
>
>  drivers/usb/host/xhci.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
>
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 4673738..dabba18 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -30,6 +30,7 @@
>  #include <asm/unaligned.h>
>  #include <linux/errno.h>
>  #include "xhci.h"
> +#include <power/regulator.h>
>
>  #ifndef CONFIG_USB_MAX_CONTROLLER_COUNT
>  #define CONFIG_USB_MAX_CONTROLLER_COUNT 1
> @@ -872,6 +873,58 @@ static u32 xhci_port_state_to_neutral(u32 state)
>  }
>
>  /**
> + * Switch power at an external regulator each port at the root hub, when
> + * the FEAT_POWER feature is set/cleared.
> + *
> + * @param ctrl pointer to the xHCI controller
> + * @param req_index port number as in the control message (one-based)
> + * @param enable boolean indicating whether to enable or disable power
> + * @return returns 0 on success, an error-code on failure
> + */
> +static int xhci_board_port_powerset(struct xhci_ctrl *ctrl, int req_index,
> +                                   bool enable)
> +{
> +#if CONFIG_IS_ENABLED(OF_CONTROL) && CONFIG_IS_ENABLED(DM_REGULATOR)
> +       /* We start counting ports at 0, while the request counts from 1. */
> +       int index = req_index - 1;
> +       struct udevice *dev = ctrl->dev;
> +       const char *regname = NULL;
> +       struct udevice *regulator;
> +       int ret;
> +
> +       debug("%s: ctrl '%s' port %d enable %s\n", __func__,
> +             dev_read_name(dev), req_index, enable ? "true" : "false");
> +
> +       ret = dev_read_string_index(dev, "xhci,port-power", index, &regname);
> +       if (ret < 0) {
> +               debug("%s: ctrl '%s' port %d: no entry in 'xhci,port-power'\n",
> +                     __func__, dev_read_name(dev), req_index);
> +               return ret;
> +       }
> +
> +       ret = regulator_get_by_platname(regname, &regulator);
> +       if (ret) {
> +               debug("%s: ctrl '%s' port %d: could not get regulator '%s'\n",
> +                     __func__, dev_read_name(dev), req_index, regname);
> +               return ret;
> +       }
> +
> +       regulator_set_enable(regulator, enable);
> +#endif
> +       return 0;
> +}
> +
> +static int xhci_board_port_poweron(struct xhci_ctrl *ctrl, int req_index)
> +{
> +       return xhci_board_port_powerset(ctrl, req_index, true);
> +}
> +
> +static int xhci_board_port_poweroff(struct xhci_ctrl *ctrl, int req_index)
> +{
> +       return xhci_board_port_powerset(ctrl, req_index, false);
> +}
> +
> +/**
>   * Submits the Requests to the XHCI Host Controller
>   *
>   * @param udev pointer to the USB device structure
> @@ -1036,6 +1089,7 @@ static int xhci_submit_root(struct usb_device *udev, unsigned long pipe,
>                         xhci_writel(status_reg, reg);
>                         break;
>                 case USB_PORT_FEAT_POWER:
> +                       xhci_board_port_poweron(ctrl, le16_to_cpu(req->index));
>                         reg |= PORT_POWER;
>                         xhci_writel(status_reg, reg);
>                         break;
> @@ -1056,6 +1110,7 @@ static int xhci_submit_root(struct usb_device *udev, unsigned long pipe,
>                         reg &= ~PORT_PE;
>                         break;
>                 case USB_PORT_FEAT_POWER:
> +                       xhci_board_port_poweroff(ctrl, le16_to_cpu(req->index));
>                         reg &= ~PORT_POWER;
>                         break;
>                 case USB_PORT_FEAT_C_RESET:
> --

Regards,
Bin