On Thu, Dec 17, 2020 at 12:40:01PM +0300, Dmitry Osipenko wrote: > Support programming of waking up from a low power mode by implementing the > generic set_wakeup() callback of the USB PHY API. > > Tested-by: Matt Merhar <[email protected]> > Tested-by: Nicolas Chauvet <[email protected]> > Tested-by: Peter Geis <[email protected]> > Tested-by: Ion Agorria <[email protected]> > Signed-off-by: Dmitry Osipenko <[email protected]> > --- > drivers/usb/phy/phy-tegra-usb.c | 94 +++++++++++++++++++++++++++++-- > include/linux/usb/tegra_usb_phy.h | 2 + > 2 files changed, 90 insertions(+), 6 deletions(-) > > diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c > index 1296524e1bee..46a1f61877ad 100644 > --- a/drivers/usb/phy/phy-tegra-usb.c > +++ b/drivers/usb/phy/phy-tegra-usb.c > @@ -45,6 +45,7 @@ > #define TEGRA_PORTSC1_RWC_BITS (PORT_CSC | PORT_PEC | PORT_OCC) > > #define USB_SUSP_CTRL 0x400 > +#define USB_WAKE_ON_RESUME_EN BIT(2) > #define USB_WAKE_ON_CNNT_EN_DEV BIT(3) > #define USB_WAKE_ON_DISCON_EN_DEV BIT(4) > #define USB_SUSP_CLR BIT(5) > @@ -56,6 +57,15 @@ > #define USB_SUSP_SET BIT(14) > #define USB_WAKEUP_DEBOUNCE_COUNT(x) (((x) & 0x7) << 16) > > +#define USB_PHY_VBUS_SENSORS 0x404 > +#define B_SESS_VLD_WAKEUP_EN BIT(6) > +#define B_VBUS_VLD_WAKEUP_EN BIT(14) > +#define A_SESS_VLD_WAKEUP_EN BIT(22) > +#define A_VBUS_VLD_WAKEUP_EN BIT(30) > + > +#define USB_PHY_VBUS_WAKEUP_ID 0x408 > +#define VBUS_WAKEUP_WAKEUP_EN BIT(30) > + > #define USB1_LEGACY_CTRL 0x410 > #define USB1_NO_LEGACY_MODE BIT(0) > #define USB1_VBUS_SENSE_CTL_MASK (3 << 1) > @@ -334,6 +344,11 @@ static int utmip_pad_power_on(struct tegra_usb_phy *phy) > writel_relaxed(val, base + UTMIP_BIAS_CFG0); > } > > + if (phy->pad_wakeup) { > + phy->pad_wakeup = false; > + utmip_pad_count--; > + } > + > spin_unlock(&utmip_pad_lock); > > clk_disable_unprepare(phy->pad_clk); > @@ -359,6 +374,17 @@ static int utmip_pad_power_off(struct tegra_usb_phy *phy) > goto ulock; > } > > + /* > + * In accordance to TRM, OTG and Bias pad circuits could be turned off > + * to save power if wake is enabled, but the VBUS-change detection > + * method is board-specific and these circuits may need to be enabled > + * to generate wakeup event, hence we will just keep them both enabled. > + */ > + if (phy->wakeup_enabled) { > + phy->pad_wakeup = true; > + utmip_pad_count++; > + } > + > if (--utmip_pad_count == 0) { > val = readl_relaxed(base + UTMIP_BIAS_CFG0); > val |= UTMIP_OTGPD | UTMIP_BIASPD; > @@ -503,11 +529,24 @@ static int utmi_phy_power_on(struct tegra_usb_phy *phy) > writel_relaxed(val, base + UTMIP_PLL_CFG1); > } > > + val = readl_relaxed(base + USB_SUSP_CTRL); > + val &= ~USB_WAKE_ON_RESUME_EN; > + writel_relaxed(val, base + USB_SUSP_CTRL); > + > if (phy->mode == USB_DR_MODE_PERIPHERAL) { > val = readl_relaxed(base + USB_SUSP_CTRL); > val &= ~(USB_WAKE_ON_CNNT_EN_DEV | USB_WAKE_ON_DISCON_EN_DEV); > writel_relaxed(val, base + USB_SUSP_CTRL); > > + val = readl_relaxed(base + USB_PHY_VBUS_WAKEUP_ID); > + val &= ~VBUS_WAKEUP_WAKEUP_EN; > + writel_relaxed(val, base + USB_PHY_VBUS_WAKEUP_ID); > + > + val = readl_relaxed(base + USB_PHY_VBUS_SENSORS); > + val &= ~(A_VBUS_VLD_WAKEUP_EN | A_SESS_VLD_WAKEUP_EN); > + val &= ~(B_VBUS_VLD_WAKEUP_EN | B_SESS_VLD_WAKEUP_EN); > + writel_relaxed(val, base + USB_PHY_VBUS_SENSORS); > + > val = readl_relaxed(base + UTMIP_BAT_CHRG_CFG0); > val &= ~UTMIP_PD_CHRG; > writel_relaxed(val, base + UTMIP_BAT_CHRG_CFG0); > @@ -605,31 +644,55 @@ static int utmi_phy_power_off(struct tegra_usb_phy *phy) > > utmi_phy_clk_disable(phy); > > - if (phy->mode == USB_DR_MODE_PERIPHERAL) { > - val = readl_relaxed(base + USB_SUSP_CTRL); > - val &= ~USB_WAKEUP_DEBOUNCE_COUNT(~0); > - val |= USB_WAKE_ON_CNNT_EN_DEV | USB_WAKEUP_DEBOUNCE_COUNT(5); > - writel_relaxed(val, base + USB_SUSP_CTRL); > - } > + /* PHY won't resume if reset is asserted */ > + if (phy->wakeup_enabled) > + goto chrg_cfg0; > > val = readl_relaxed(base + USB_SUSP_CTRL); > val |= UTMIP_RESET; > writel_relaxed(val, base + USB_SUSP_CTRL); > > +chrg_cfg0:
I found this diffcult to read until I realized that it was basically
just the equivalent of this:
if (!phy->wakeup_enabled) {
val = readl_relaxed(base + USB_SUSP_CTRL);
val |= UTMIP_RESET;
writel_relaxed(val, base + USB_SUSP_CTRL);
}
> val = readl_relaxed(base + UTMIP_BAT_CHRG_CFG0);
> val |= UTMIP_PD_CHRG;
> writel_relaxed(val, base + UTMIP_BAT_CHRG_CFG0);
>
> + if (phy->wakeup_enabled)
> + goto xcvr_cfg1;
> +
> val = readl_relaxed(base + UTMIP_XCVR_CFG0);
> val |= UTMIP_FORCE_PD_POWERDOWN | UTMIP_FORCE_PD2_POWERDOWN |
> UTMIP_FORCE_PDZI_POWERDOWN;
> writel_relaxed(val, base + UTMIP_XCVR_CFG0);
>
> +xcvr_cfg1:
Similarly, I think this is more readable as:
if (!phy->wakeup_enabled) {
val = readl_relaxed(base + UTMIP_XCVR_CFG0);
val |= UTMIP_FORCE_PD_POWERDOWN | UTMIP_FORCE_PD2_POWERDOWN |
UTMIP_FORCE_PDZI_POWERDOWN;
writel_relaxed(val, base + UTMIP_XCVR_CFG0);
}
> val = readl_relaxed(base + UTMIP_XCVR_CFG1);
> val |= UTMIP_FORCE_PDDISC_POWERDOWN | UTMIP_FORCE_PDCHRP_POWERDOWN |
> UTMIP_FORCE_PDDR_POWERDOWN;
> writel_relaxed(val, base + UTMIP_XCVR_CFG1);
>
> + if (phy->wakeup_enabled) {
Which then also matches the style of this conditional here.
> + val = readl_relaxed(base + USB_SUSP_CTRL);
> + val &= ~USB_WAKEUP_DEBOUNCE_COUNT(~0);
> + val |= USB_WAKEUP_DEBOUNCE_COUNT(5);
> + val |= USB_WAKE_ON_RESUME_EN;
> + writel_relaxed(val, base + USB_SUSP_CTRL);
> +
> + /*
> + * Ask VBUS sensor to generate wake event once cable is
> + * connected.
> + */
> + if (phy->mode == USB_DR_MODE_PERIPHERAL) {
> + val = readl_relaxed(base + USB_PHY_VBUS_WAKEUP_ID);
> + val |= VBUS_WAKEUP_WAKEUP_EN;
> + writel_relaxed(val, base + USB_PHY_VBUS_WAKEUP_ID);
> +
> + val = readl_relaxed(base + USB_PHY_VBUS_SENSORS);
> + val |= A_VBUS_VLD_WAKEUP_EN;
> + writel_relaxed(val, base + USB_PHY_VBUS_SENSORS);
> + }
> + }
> +
> return utmip_pad_power_off(phy);
> }
>
> @@ -765,6 +828,15 @@ static int ulpi_phy_power_off(struct tegra_usb_phy *phy)
> usleep_range(5000, 6000);
> clk_disable_unprepare(phy->clk);
>
> + /*
> + * Wakeup currently unimplemented for ULPI, thus PHY needs to be
> + * force-resumed.
> + */
> + if (WARN_ON_ONCE(phy->wakeup_enabled)) {
> + ulpi_phy_power_on(phy);
> + return -EOPNOTSUPP;
> + }
How do we control phy->wakeup_enabled? Is this something that we can set
in device tree, for example? I hope so, because otherwise this would
cause a nasty splat every time we try to power-off a ULPI PHY.
Thierry
signature.asc
Description: PGP signature
