On 25-03-21, 17:40, Thierry Reding wrote:
> +static int tegra210_usb3_enable_phy_sleepwalk(struct tegra_xusb_lane *lane,
> + enum usb_device_speed speed)
> +{
> + struct tegra_xusb_padctl *padctl = lane->pad->padctl;
> + int port = tegra210_usb3_lane_map(lane);
> + struct device *dev = padctl->dev;
> + u32 value;
> +
> + if (port < 0) {
> + dev_err(dev, "invalid usb3 port number\n");
> + return -EINVAL;
> + }
> +
> + dev_dbg(dev, "phy enable sleepwalk usb3 %d\n", port);
Too much noise for my taste :)
(here and other places)
> +static int tegra210_pmc_utmi_enable_phy_sleepwalk(struct tegra_xusb_lane *lane,
> + enum usb_device_speed speed)
> +{
> + struct tegra_xusb_padctl *padctl = lane->pad->padctl;
> + struct tegra210_xusb_padctl *priv = to_tegra210_xusb_padctl(padctl);
> + struct device *dev = padctl->dev;
> + unsigned int port = lane->index;
> + u32 value, tctrl, pctrl, rpd_ctrl;
> +
> + if (!priv->regmap)
> + return -EOPNOTSUPP;
> +
> + if (speed > USB_SPEED_HIGH)
> + return -EINVAL;
> +
> + dev_dbg(dev, "phy enable sleepwalk usb2 %d speed %d\n", port, speed);
> +
> + value = padctl_readl(padctl, XUSB_PADCTL_USB2_BIAS_PAD_CTL1);
> + tctrl = TCTRL_VALUE(value);
> + pctrl = PCTRL_VALUE(value);
> +
> + value = padctl_readl(padctl, XUSB_PADCTL_USB2_OTG_PADX_CTL1(port));
> + rpd_ctrl = RPD_CTRL_VALUE(value);
> +
> + /* ensure sleepwalk logic is disabled */
> + value = padctl_pmc_readl(priv, PMC_UTMIP_UHSIC_SLEEP_CFG(port));
> + value &= ~UTMIP_MASTER_ENABLE(port);
> + padctl_pmc_writel(priv, value, PMC_UTMIP_UHSIC_SLEEP_CFG(port));
> +
> + /* ensure sleepwalk logics are in low power mode */
> + value = padctl_pmc_readl(priv, PMC_UTMIP_MASTER_CONFIG);
> + value |= UTMIP_PWR(port);
> + padctl_pmc_writel(priv, value, PMC_UTMIP_MASTER_CONFIG);
We really should have a read_modify_write() helper.. quite repeat of
this here
> +
> + /* set debounce time */
> + value = padctl_pmc_readl(priv, PMC_USB_DEBOUNCE_DEL);
> + value &= ~UTMIP_LINE_DEB_CNT(~0);
> + value |= UTMIP_LINE_DEB_CNT(0x1);
> + padctl_pmc_writel(priv, value, PMC_USB_DEBOUNCE_DEL);
> +
> + /* ensure fake events of sleepwalk logic are desiabled */
> + value = padctl_pmc_readl(priv, PMC_UTMIP_UHSIC_FAKE(port));
> + value &= ~(UTMIP_FAKE_USBOP_VAL(port) | UTMIP_FAKE_USBON_VAL(port) |
> + UTMIP_FAKE_USBOP_EN(port) | UTMIP_FAKE_USBON_EN(port));
> + padctl_pmc_writel(priv, value, PMC_UTMIP_UHSIC_FAKE(port));
> +
> + /* ensure wake events of sleepwalk logic are not latched */
> + value = padctl_pmc_readl(priv, PMC_UTMIP_UHSIC_LINE_WAKEUP);
> + value &= ~UTMIP_LINE_WAKEUP_EN(port);
> + padctl_pmc_writel(priv, value, PMC_UTMIP_UHSIC_LINE_WAKEUP);
> +
> + /* disable wake event triggers of sleepwalk logic */
> + value = padctl_pmc_readl(priv, PMC_UTMIP_UHSIC_SLEEP_CFG(port));
> + value &= ~UTMIP_WAKE_VAL(port, ~0);
> + value |= UTMIP_WAKE_VAL_NONE(port);
> + padctl_pmc_writel(priv, value, PMC_UTMIP_UHSIC_SLEEP_CFG(port));
> +
> + /* power down the line state detectors of the pad */
> + value = padctl_pmc_readl(priv, PMC_USB_AO);
> + value |= (USBOP_VAL_PD(port) | USBON_VAL_PD(port));
> + padctl_pmc_writel(priv, value, PMC_USB_AO);
> +
> + /* save state per speed */
> + value = padctl_pmc_readl(priv, PMC_UTMIP_UHSIC_SAVED_STATE(port));
> + value &= ~SPEED(port, ~0);
> + if (speed == USB_SPEED_HIGH)
> + value |= UTMI_HS(port);
> + else if (speed == USB_SPEED_FULL)
> + value |= UTMI_FS(port);
> + else if (speed == USB_SPEED_LOW)
> + value |= UTMI_LS(port);
> + else
> + value |= UTMI_RST(port);
This could look better with a switch statement
> + padctl_pmc_writel(priv, value, PMC_UTMIP_UHSIC_SAVED_STATE(port));
> +
> + /* enable the trigger of the sleepwalk logic */
> + value = padctl_pmc_readl(priv, PMC_UTMIP_UHSIC_SLEEPWALK_CFG(port));
> + value |= UTMIP_LINEVAL_WALK_EN(port);
> + padctl_pmc_writel(priv, value, PMC_UTMIP_UHSIC_SLEEPWALK_CFG(port));
> +
> + /* reset the walk pointer and clear the alarm of the sleepwalk logic,
> + * as well as capture the configuration of the USB2.0 pad
> + */
/*
* multi
* line style please
*/
> + value = padctl_pmc_readl(priv, PMC_UTMIP_UHSIC_TRIGGERS);
> + value |= (UTMIP_CLR_WALK_PTR(port) | UTMIP_CLR_WAKE_ALARM(port) |
> + UTMIP_CAP_CFG(port));
> + padctl_pmc_writel(priv, value, PMC_UTMIP_UHSIC_TRIGGERS);
> +
> + /* program electrical parameters read from XUSB PADCTL */
> + value = padctl_pmc_readl(priv, PMC_UTMIP_TERM_PAD_CFG);
> + value &= ~(TCTRL_VAL(~0) | PCTRL_VAL(~0));
> + value |= (TCTRL_VAL(tctrl) | PCTRL_VAL(pctrl));
> + padctl_pmc_writel(priv, value, PMC_UTMIP_TERM_PAD_CFG);
> +
> + value = padctl_pmc_readl(priv, PMC_UTMIP_PAD_CFGX(port));
> + value &= ~RPD_CTRL_PX(~0);
> + value |= RPD_CTRL_PX(rpd_ctrl);
> + padctl_pmc_writel(priv, value, PMC_UTMIP_PAD_CFGX(port));
> +
> + /* setup the pull-ups and pull-downs of the signals during the four
> + * stages of sleepwalk.
> + * if device is connected, program sleepwalk logic to maintain a J and
> + * keep driving K upon seeing remote wake.
> + */
> + value = padctl_pmc_readl(priv, PMC_UTMIP_SLEEPWALK_PX(port));
> + value = (UTMIP_USBOP_RPD_A | UTMIP_USBOP_RPD_B | UTMIP_USBOP_RPD_C |
> + UTMIP_USBOP_RPD_D);
> + value |= (UTMIP_USBON_RPD_A | UTMIP_USBON_RPD_B | UTMIP_USBON_RPD_C |
> + UTMIP_USBON_RPD_D);
> + if (speed == USB_SPEED_UNKNOWN) {
> + value |= (UTMIP_HIGHZ_A | UTMIP_HIGHZ_B | UTMIP_HIGHZ_C |
> + UTMIP_HIGHZ_D);
> + } else if ((speed == USB_SPEED_HIGH) || (speed == USB_SPEED_FULL)) {
> + /* J state: D+/D- = high/low, K state: D+/D- = low/high */
> + value |= UTMIP_HIGHZ_A;
> + value |= UTMIP_AP_A;
> + value |= (UTMIP_AN_B | UTMIP_AN_C | UTMIP_AN_D);
> + } else if (speed == USB_SPEED_LOW) {
> + /* J state: D+/D- = low/high, K state: D+/D- = high/low */
> + value |= UTMIP_HIGHZ_A;
> + value |= UTMIP_AN_A;
> + value |= (UTMIP_AP_B | UTMIP_AP_C | UTMIP_AP_D);
> + }
no else? err case? Also this could use a switch too
> +static int tegra210_pmc_utmi_disable_phy_sleepwalk(struct tegra_xusb_lane *lane)
> +{
> + struct tegra_xusb_padctl *padctl = lane->pad->padctl;
> + struct tegra210_xusb_padctl *priv = to_tegra210_xusb_padctl(padctl);
> + struct device *dev = padctl->dev;
> + unsigned int port = lane->index;
> + u32 value;
> +
> + if (!priv->regmap)
> + return -EOPNOTSUPP;
That should be an error like EIO as we always expect regmap to be set,
no an unsupported error right?
--
~Vinod
