Hi Vinod, Before the device or host is being attached, we can keep most of the transceivers powered down (PD=1/PD_DR=1) to minimize power consumption. At this stage, in .phy_power_on(), we enable only the single-ended receiver (PD_ZI=0) for detecting connection. Upon detecting device's or host's connection, host or controller driver will invoke tegra_phy_xusb_utmi_pad_power_on() to power on all of the transceivers (PD=0/PD_DR=0) to equip full link functionality. Thanks, JC On 9/4/22 22:45, Vinod Koul wrote: > On 16-08-22, 16:23, Jim Lin wrote: >> Add utmi_pad_power_on/down ops for each SOC instead of exporting >> tegra_phy_xusb_utmi_pad_power_on/down directly for Tegra186 chip. > > Can you please help me understand why do we need to utmi power_on/down > exported and cant be handled thry phy-ops.. > >> >> Signed-off-by: BH Hsieh <bhsieh@xxxxxxxxxx> >> Signed-off-by: Jim Lin <jilin@xxxxxxxxxx> >> --- >> v2: update copyright year >> >> drivers/phy/tegra/xusb-tegra186.c | 19 ++++++++++++------- >> drivers/phy/tegra/xusb.c | 22 +++++++++++++++++++++- >> drivers/phy/tegra/xusb.h | 4 +++- >> include/linux/phy/tegra/xusb.h | 4 +++- >> 4 files changed, 39 insertions(+), 10 deletions(-) >> >> diff --git a/drivers/phy/tegra/xusb-tegra186.c b/drivers/phy/tegra/xusb-tegra186.c >> index ae3915ed9fef..5abdf81aa143 100644 >> --- a/drivers/phy/tegra/xusb-tegra186.c >> +++ b/drivers/phy/tegra/xusb-tegra186.c >> @@ -1,6 +1,6 @@ >> // SPDX-License-Identifier: GPL-2.0 >> /* >> - * Copyright (c) 2016-2020, NVIDIA CORPORATION. All rights reserved. >> + * Copyright (c) 2016-2022, NVIDIA CORPORATION. All rights reserved. >> */ >> >> #include <linux/delay.h> >> @@ -638,7 +638,7 @@ static void tegra186_utmi_bias_pad_power_off(struct tegra_xusb_padctl *padctl) >> mutex_unlock(&padctl->lock); >> } >> >> -static void tegra_phy_xusb_utmi_pad_power_on(struct phy *phy) >> +static void tegra186_utmi_pad_power_on(struct phy *phy) >> { >> struct tegra_xusb_lane *lane = phy_get_drvdata(phy); >> struct tegra_xusb_padctl *padctl = lane->pad->padctl; >> @@ -656,6 +656,8 @@ static void tegra_phy_xusb_utmi_pad_power_on(struct phy *phy) >> return; >> } >> >> + dev_dbg(dev, "power on UTMI pad %u\n", index); >> + >> tegra186_utmi_bias_pad_power_on(padctl); >> >> udelay(2); >> @@ -669,7 +671,7 @@ static void tegra_phy_xusb_utmi_pad_power_on(struct phy *phy) >> padctl_writel(padctl, value, XUSB_PADCTL_USB2_OTG_PADX_CTL1(index)); >> } >> >> -static void tegra_phy_xusb_utmi_pad_power_down(struct phy *phy) >> +static void tegra186_utmi_pad_power_down(struct phy *phy) >> { >> struct tegra_xusb_lane *lane = phy_get_drvdata(phy); >> struct tegra_xusb_padctl *padctl = lane->pad->padctl; >> @@ -679,6 +681,8 @@ static void tegra_phy_xusb_utmi_pad_power_down(struct phy *phy) >> if (!phy) >> return; >> >> + dev_dbg(padctl->dev, "power down UTMI pad %u\n", index); >> + >> value = padctl_readl(padctl, XUSB_PADCTL_USB2_OTG_PADX_CTL0(index)); >> value |= USB2_OTG_PD; >> padctl_writel(padctl, value, XUSB_PADCTL_USB2_OTG_PADX_CTL0(index)); >> @@ -849,15 +853,14 @@ static int tegra186_utmi_phy_power_on(struct phy *phy) >> value |= RPD_CTRL(priv->calib.rpd_ctrl); >> padctl_writel(padctl, value, XUSB_PADCTL_USB2_OTG_PADX_CTL1(index)); >> >> - /* TODO: pad power saving */ >> - tegra_phy_xusb_utmi_pad_power_on(phy); >> + tegra186_utmi_pad_power_on(phy); >> + >> return 0; >> } >> >> static int tegra186_utmi_phy_power_off(struct phy *phy) >> { >> - /* TODO: pad power saving */ >> - tegra_phy_xusb_utmi_pad_power_down(phy); >> + tegra186_utmi_pad_power_down(phy); >> >> return 0; >> } >> @@ -1486,6 +1489,8 @@ static const struct tegra_xusb_padctl_ops tegra186_xusb_padctl_ops = { >> .suspend_noirq = tegra186_xusb_padctl_suspend_noirq, >> .resume_noirq = tegra186_xusb_padctl_resume_noirq, >> .vbus_override = tegra186_xusb_padctl_vbus_override, >> + .utmi_pad_power_on = tegra186_utmi_pad_power_on, >> + .utmi_pad_power_down = tegra186_utmi_pad_power_down, >> }; >> >> #if IS_ENABLED(CONFIG_ARCH_TEGRA_186_SOC) >> diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c >> index 963de5913e50..49873718d54a 100644 >> --- a/drivers/phy/tegra/xusb.c >> +++ b/drivers/phy/tegra/xusb.c >> @@ -1,6 +1,6 @@ >> // SPDX-License-Identifier: GPL-2.0-only >> /* >> - * Copyright (c) 2014-2020, NVIDIA CORPORATION. All rights reserved. >> + * Copyright (c) 2014-2022, NVIDIA CORPORATION. All rights reserved. >> */ >> >> #include <linux/delay.h> >> @@ -1458,6 +1458,26 @@ int tegra_phy_xusb_utmi_port_reset(struct phy *phy) >> } >> EXPORT_SYMBOL_GPL(tegra_phy_xusb_utmi_port_reset); >> >> +void tegra_phy_xusb_utmi_pad_power_on(struct phy *phy) >> +{ >> + struct tegra_xusb_lane *lane = phy_get_drvdata(phy); >> + struct tegra_xusb_padctl *padctl = lane->pad->padctl; >> + >> + if (padctl->soc->ops->utmi_pad_power_on) >> + padctl->soc->ops->utmi_pad_power_on(phy); >> +} >> +EXPORT_SYMBOL_GPL(tegra_phy_xusb_utmi_pad_power_on); >> + >> +void tegra_phy_xusb_utmi_pad_power_down(struct phy *phy) >> +{ >> + struct tegra_xusb_lane *lane = phy_get_drvdata(phy); >> + struct tegra_xusb_padctl *padctl = lane->pad->padctl; >> + >> + if (padctl->soc->ops->utmi_pad_power_down) >> + padctl->soc->ops->utmi_pad_power_down(phy); >> +} >> +EXPORT_SYMBOL_GPL(tegra_phy_xusb_utmi_pad_power_down); >> + >> int tegra_xusb_padctl_get_usb3_companion(struct tegra_xusb_padctl *padctl, >> unsigned int port) >> { >> diff --git a/drivers/phy/tegra/xusb.h b/drivers/phy/tegra/xusb.h >> index 034f7a2c28d6..8cfbbdbd6e0c 100644 >> --- a/drivers/phy/tegra/xusb.h >> +++ b/drivers/phy/tegra/xusb.h >> @@ -1,6 +1,6 @@ >> /* SPDX-License-Identifier: GPL-2.0-only */ >> /* >> - * Copyright (c) 2014-2020, NVIDIA CORPORATION. All rights reserved. >> + * Copyright (c) 2014-2022, NVIDIA CORPORATION. All rights reserved. >> * Copyright (c) 2015, Google Inc. >> */ >> >> @@ -412,6 +412,8 @@ struct tegra_xusb_padctl_ops { >> unsigned int index, bool enable); >> int (*vbus_override)(struct tegra_xusb_padctl *padctl, bool set); >> int (*utmi_port_reset)(struct phy *phy); >> + void (*utmi_pad_power_on)(struct phy *phy); >> + void (*utmi_pad_power_down)(struct phy *phy); >> }; >> >> struct tegra_xusb_padctl_soc { >> diff --git a/include/linux/phy/tegra/xusb.h b/include/linux/phy/tegra/xusb.h >> index 3a35e74cdc61..70998e6dd6fd 100644 >> --- a/include/linux/phy/tegra/xusb.h >> +++ b/include/linux/phy/tegra/xusb.h >> @@ -1,6 +1,6 @@ >> /* SPDX-License-Identifier: GPL-2.0-only */ >> /* >> - * Copyright (c) 2016-2020, NVIDIA CORPORATION. All rights reserved. >> + * Copyright (c) 2016-2022, NVIDIA CORPORATION. All rights reserved. >> */ >> >> #ifndef PHY_TEGRA_XUSB_H >> @@ -21,6 +21,8 @@ int tegra_xusb_padctl_usb3_set_lfps_detect(struct tegra_xusb_padctl *padctl, >> unsigned int port, bool enable); >> int tegra_xusb_padctl_set_vbus_override(struct tegra_xusb_padctl *padctl, >> bool val); >> +void tegra_phy_xusb_utmi_pad_power_on(struct phy *phy); >> +void tegra_phy_xusb_utmi_pad_power_down(struct phy *phy); >> int tegra_phy_xusb_utmi_port_reset(struct phy *phy); >> int tegra_xusb_padctl_get_usb3_companion(struct tegra_xusb_padctl *padctl, >> unsigned int port); >> -- >> 2.17.1 >