[3/8] phy: tegra: xusb: t210: rearrange UPHY init
diff mbox series

Message ID 20190614074652.21960-4-jckuo@nvidia.com
State New
Headers show
Series
  • Tegra XHCI controller ELPG support
Related show

Commit Message

JC Kuo June 14, 2019, 7:46 a.m. UTC
This commit is a preparation for enabling XUSB LP0 support.
It rearranges T210 XUSB PADCTL UPHY initialization sequence,
for the following reasons:

1. PLLE hardware power sequencer has to be enabled only after
   both PEX UPHY PLL and SATA UPHY PLL are initialized.

2. Once UPHY PLL hardware power sequncer is enabled, do not
   assert reset to PEX/SATA PLLs.

3. At LP0 exit, XUSB_PADCTL_ELPG_PROGRAM1_AUX_MUX_LP0_CLAMP_EN,
   XUSB_PADCTL_ELPG_PROGRAM1_AUX_MUX_LP0_CLAMP_EN_EARLY, and
   XUSB_PADCTL_ELPG_PROGRAM1_AUX_MUX_LP0_VCORE_DOWN bits have
   to be cleared after XUSB_PADCTL_USB3_PAD_MUX_SATA_IDDQ_DISABLE
   and XUSB_PADCTL_USB3_PAD_MUX_PCIE_IDDQ_DISABLE bits get set.

4. Move XUSB_PADCTL_SS_PORT_MAP and XUSB_PADCTL_UPHY_USB3_PADX_ECTL*
   registers programming from tegra210_usb3_port_enable() to
   tegra210_pcie_phy_power_on()/tegra210_sata_phy_power_on() so that
   XUSB USB3 ports will be programmed at LP0 exit.

Signed-off-by: JC Kuo <jckuo@nvidia.com>
---
 drivers/phy/tegra/xusb-tegra210.c | 443 ++++++++++++++++++------------
 drivers/phy/tegra/xusb.c          |   2 +-
 drivers/phy/tegra/xusb.h          |   2 +
 3 files changed, 264 insertions(+), 183 deletions(-)

Comments

Jon Hunter July 4, 2019, 1:32 p.m. UTC | #1
On 14/06/2019 08:46, JC Kuo wrote:
> This commit is a preparation for enabling XUSB LP0 support.

By LP0 do you mean ELPG? If so please stick to using one name for
referring to the power-state in question.

> It rearranges T210 XUSB PADCTL UPHY initialization sequence,

Please use Tegra210 and not T210.

> for the following reasons:
> 
> 1. PLLE hardware power sequencer has to be enabled only after
>    both PEX UPHY PLL and SATA UPHY PLL are initialized.
> 
> 2. Once UPHY PLL hardware power sequncer is enabled, do not

s/sequncer/sequencer

>    assert reset to PEX/SATA PLLs.

Maybe worth clarifying why here.
	
> 
> 3. At LP0 exit, XUSB_PADCTL_ELPG_PROGRAM1_AUX_MUX_LP0_CLAMP_EN,
>    XUSB_PADCTL_ELPG_PROGRAM1_AUX_MUX_LP0_CLAMP_EN_EARLY, and
>    XUSB_PADCTL_ELPG_PROGRAM1_AUX_MUX_LP0_VCORE_DOWN bits have
>    to be cleared after XUSB_PADCTL_USB3_PAD_MUX_SATA_IDDQ_DISABLE
>    and XUSB_PADCTL_USB3_PAD_MUX_PCIE_IDDQ_DISABLE bits get set.
> 
> 4. Move XUSB_PADCTL_SS_PORT_MAP and XUSB_PADCTL_UPHY_USB3_PADX_ECTL*
>    registers programming from tegra210_usb3_port_enable() to
>    tegra210_pcie_phy_power_on()/tegra210_sata_phy_power_on() so that
>    XUSB USB3 ports will be programmed at LP0 exit.

Looks like you are moving all the code from the port enable to the phy
enable and after this change the port enable does nothing. Do we not
differentiate between phy and port? I think a bit more description is
necessary here to describe the impact of this change.

> 
> Signed-off-by: JC Kuo <jckuo@nvidia.com>
> ---
>  drivers/phy/tegra/xusb-tegra210.c | 443 ++++++++++++++++++------------
>  drivers/phy/tegra/xusb.c          |   2 +-
>  drivers/phy/tegra/xusb.h          |   2 +
>  3 files changed, 264 insertions(+), 183 deletions(-)
> 
> diff --git a/drivers/phy/tegra/xusb-tegra210.c b/drivers/phy/tegra/xusb-tegra210.c
> index 18cea8311d22..007bf352b45e 100644
> --- a/drivers/phy/tegra/xusb-tegra210.c
> +++ b/drivers/phy/tegra/xusb-tegra210.c
> @@ -240,6 +240,8 @@ to_tegra210_xusb_padctl(struct tegra_xusb_padctl *padctl)
>  	return container_of(padctl, struct tegra210_xusb_padctl, base);
>  }
>  
> +static int tegra210_usb3_lane_map(struct tegra_xusb_lane *lane);
> +

Can we avoid adding this prototype?

>  /* must be called under padctl->lock */
>  static int tegra210_pex_uphy_enable(struct tegra_xusb_padctl *padctl)
>  {
> @@ -453,35 +455,44 @@ static int tegra210_pex_uphy_enable(struct tegra_xusb_padctl *padctl)
>  static void tegra210_pex_uphy_disable(struct tegra_xusb_padctl *padctl)
>  {
>  	struct tegra_xusb_pcie_pad *pcie = to_pcie_pad(padctl->pcie);
> -
> -	mutex_lock(&padctl->lock);
> +	u32 value;
> +	int i;
>  
>  	if (WARN_ON(pcie->enable == 0))
> -		goto unlock;
> +		return;
>  
>  	if (--pcie->enable > 0)
> -		goto unlock;
> +		return;
>  
> -	reset_control_assert(pcie->rst);
> +	for (i = 0; i < padctl->pcie->soc->num_lanes; i++) {
> +		value = padctl_readl(padctl, XUSB_PADCTL_USB3_PAD_MUX);
> +		value &= ~XUSB_PADCTL_USB3_PAD_MUX_PCIE_IDDQ_DISABLE(i);
> +		padctl_writel(padctl, value, XUSB_PADCTL_USB3_PAD_MUX);
> +	}
>  	clk_disable_unprepare(pcie->pll);
> -
> -unlock:
> -	mutex_unlock(&padctl->lock);
>  }
>  
>  /* must be called under padctl->lock */
> -static int tegra210_sata_uphy_enable(struct tegra_xusb_padctl *padctl, bool usb)
> +static int tegra210_sata_uphy_enable(struct tegra_xusb_padctl *padctl)
>  {
>  	struct tegra_xusb_sata_pad *sata = to_sata_pad(padctl->sata);
> +	struct tegra_xusb_lane *lane = tegra_xusb_find_lane(padctl, "sata", 0);
>  	unsigned long timeout;
>  	u32 value;
>  	int err;
> +	bool usb = false;
>  
>  	if (sata->enable > 0) {
>  		sata->enable++;
>  		return 0;
>  	}
>  
> +	if (!lane)
> +		return 0;
> +
> +	if (tegra_xusb_lane_check(lane, "usb3-ss"))
> +		usb = true;

This return a boolean type so you can just ...

	usb = tegra_xusb_lane_check(lane, "usb3-ss");

> +
>  	err = clk_prepare_enable(sata->pll);
>  	if (err < 0)
>  		return err;
> @@ -695,30 +706,36 @@ static int tegra210_sata_uphy_enable(struct tegra_xusb_padctl *padctl, bool usb)
>  static void tegra210_sata_uphy_disable(struct tegra_xusb_padctl *padctl)
>  {
>  	struct tegra_xusb_sata_pad *sata = to_sata_pad(padctl->sata);
> -
> -	mutex_lock(&padctl->lock);
> +	u32 value;
> +	int i;
>  
>  	if (WARN_ON(sata->enable == 0))
> -		goto unlock;
> +		return;
>  
>  	if (--sata->enable > 0)
> -		goto unlock;
> +		return;
>  
> -	reset_control_assert(sata->rst);
> +	for (i = 0; i < padctl->sata->soc->num_lanes; i++) {
> +		value = padctl_readl(padctl, XUSB_PADCTL_USB3_PAD_MUX);
> +		value &= ~XUSB_PADCTL_USB3_PAD_MUX_SATA_IDDQ_DISABLE(i);
> +		padctl_writel(padctl, value, XUSB_PADCTL_USB3_PAD_MUX);
> +	}
>  	clk_disable_unprepare(sata->pll);
> -
> -unlock:
> -	mutex_unlock(&padctl->lock);
>  }
>  
>  static int tegra210_xusb_padctl_enable(struct tegra_xusb_padctl *padctl)
>  {
> -	u32 value;
> +	return 0;
> +}
>  
> -	mutex_lock(&padctl->lock);
> +static int tegra210_xusb_padctl_disable(struct tegra_xusb_padctl *padctl)
> +{
> +	return 0;
> +}

Why bother keeping these functions at all if they now do nothing?

>  
> -	if (padctl->enable++ > 0)
> -		goto out;
> +static void tegra210_aux_mux_lp0_clamp_disable(struct tegra_xusb_padctl *padctl)

Any reason for renaming these? These appear to deal with the XUSB_PADCTL
and so the previous names seem fine.

> +{
> +	u32 value;
>  
>  	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
>  	value &= ~XUSB_PADCTL_ELPG_PROGRAM1_AUX_MUX_LP0_CLAMP_EN;
> @@ -735,24 +752,12 @@ static int tegra210_xusb_padctl_enable(struct tegra_xusb_padctl *padctl)
>  	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
>  	value &= ~XUSB_PADCTL_ELPG_PROGRAM1_AUX_MUX_LP0_VCORE_DOWN;
>  	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
> -
> -out:
> -	mutex_unlock(&padctl->lock);
> -	return 0;
>  }
>  
> -static int tegra210_xusb_padctl_disable(struct tegra_xusb_padctl *padctl)
> +static void tegra210_aux_mux_lp0_clamp_enable(struct tegra_xusb_padctl *padctl)
>  {
>  	u32 value;
>  
> -	mutex_lock(&padctl->lock);
> -
> -	if (WARN_ON(padctl->enable == 0))
> -		goto out;
> -
> -	if (--padctl->enable > 0)
> -		goto out;
> -
>  	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
>  	value |= XUSB_PADCTL_ELPG_PROGRAM1_AUX_MUX_LP0_VCORE_DOWN;
>  	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
> @@ -768,12 +773,76 @@ static int tegra210_xusb_padctl_disable(struct tegra_xusb_padctl *padctl)
>  	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
>  	value |= XUSB_PADCTL_ELPG_PROGRAM1_AUX_MUX_LP0_CLAMP_EN;
>  	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
> +}
> +
> +static int tegra210_uphy_init(struct tegra_xusb_padctl *padctl)
> +{
> +	struct tegra_xusb_pcie_pad *pcie;
> +	struct tegra_xusb_sata_pad *sata;
> +	u32 value;
> +	int err;
> +	int i;
> +
> +	if (tegra210_plle_hw_sequence_is_enabled()) {
> +		dev_dbg(padctl->dev, "PLLE is already in HW control\n");
> +		/* skip pll initialization, update plle refcount only */
> +		if (padctl->pcie) {
> +			pcie = to_pcie_pad(padctl->pcie);
> +			if (pcie->enable == 0) {
> +				err = clk_prepare_enable(pcie->pll);
> +				if (err < 0)
> +					return err;
> +				pcie->enable++;

Do we need all this additional ref counting around clk_prepare_enable?

> +			}
> +		}
> +		if (padctl->sata) {
> +			sata = to_sata_pad(padctl->sata);
> +			if (sata->enable == 0) {
> +				err = clk_prepare_enable(sata->pll);
> +				if (err < 0)
> +					return err;
> +				sata->enable++;

Same here.

> +			}
> +		}
> +		goto skip_pll_init;
> +	}
> +
> +	if (padctl->pcie)
> +		tegra210_pex_uphy_enable(padctl);
> +	if (padctl->sata)
> +		tegra210_sata_uphy_enable(padctl);
> +
> +	tegra210_plle_hw_sequence_start();
> +
> +skip_pll_init:
> +	for (i = 0; i < padctl->pcie->soc->num_lanes; i++) {
> +		value = padctl_readl(padctl, XUSB_PADCTL_USB3_PAD_MUX);
> +		value |= XUSB_PADCTL_USB3_PAD_MUX_PCIE_IDDQ_DISABLE(i);
> +		padctl_writel(padctl, value, XUSB_PADCTL_USB3_PAD_MUX);
> +	}
> +
> +	for (i = 0; i < padctl->sata->soc->num_lanes; i++) {
> +		value = padctl_readl(padctl, XUSB_PADCTL_USB3_PAD_MUX);
> +		value |= XUSB_PADCTL_USB3_PAD_MUX_SATA_IDDQ_DISABLE(i);
> +		padctl_writel(padctl, value, XUSB_PADCTL_USB3_PAD_MUX);
> +	}
> +
> +	tegra210_aux_mux_lp0_clamp_disable(padctl);
>  
> -out:
> -	mutex_unlock(&padctl->lock);
>  	return 0;
>  }
>  
> +static void __maybe_unused
> +tegra210_uphy_deinit(struct tegra_xusb_padctl *padctl)
> +{
> +	tegra210_aux_mux_lp0_clamp_enable(padctl);
> +
> +	if (padctl->pcie)
> +		tegra210_pex_uphy_disable(padctl);
> +	if (padctl->sata)
> +		tegra210_sata_uphy_disable(padctl);

What about the clocks that were enabled?

> +}
> +
>  static int tegra210_hsic_set_idle(struct tegra_xusb_padctl *padctl,
>  				  unsigned int index, bool idle)
>  {
> @@ -1420,6 +1489,113 @@ static const struct tegra_xusb_lane_soc tegra210_pcie_lanes[] = {
>  	TEGRA210_LANE("pcie-6", 0x028, 24, 0x3, pcie),
>  };
>  
> +static int tegra210_usb3_phy_power_on(struct phy *phy)
> +{
> +	struct device *dev = &phy->dev;
> +	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
> +	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
> +	struct tegra_xusb_usb3_port *usb3 = tegra_xusb_find_usb3_port(padctl,
> +					    tegra210_usb3_lane_map(lane));

I think that this should be placed on separate lines. Other places you
check the return value of tegra210_usb3_lane_map() but here you don't.
We should be consistent.

> +	int index;
> +	u32 value;
> +
> +	if (!usb3) {
> +		dev_err(dev, "no USB3 port found for lane %u\n", lane->index);
> +		return -ENODEV;
> +	}
> +	index = usb3->base.index;
> +
> +	value = padctl_readl(padctl, XUSB_PADCTL_SS_PORT_MAP);
> +
> +	if (!usb3->internal)
> +		value &= ~XUSB_PADCTL_SS_PORT_MAP_PORTX_INTERNAL(index);
> +	else
> +		value |= XUSB_PADCTL_SS_PORT_MAP_PORTX_INTERNAL(index);
> +
> +	value &= ~XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP_MASK(index);
> +	value |= XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP(index, usb3->port);
> +	padctl_writel(padctl, value, XUSB_PADCTL_SS_PORT_MAP);
> +
> +	value = padctl_readl(padctl, XUSB_PADCTL_UPHY_USB3_PADX_ECTL1(index));
> +	value &= ~(XUSB_PADCTL_UPHY_USB3_PAD_ECTL1_TX_TERM_CTRL_MASK <<
> +		   XUSB_PADCTL_UPHY_USB3_PAD_ECTL1_TX_TERM_CTRL_SHIFT);
> +	value |= XUSB_PADCTL_UPHY_USB3_PAD_ECTL1_TX_TERM_CTRL_VAL <<
> +		 XUSB_PADCTL_UPHY_USB3_PAD_ECTL1_TX_TERM_CTRL_SHIFT;
> +	padctl_writel(padctl, value, XUSB_PADCTL_UPHY_USB3_PADX_ECTL1(index));
> +
> +	value = padctl_readl(padctl, XUSB_PADCTL_UPHY_USB3_PADX_ECTL2(index));
> +	value &= ~(XUSB_PADCTL_UPHY_USB3_PAD_ECTL2_RX_CTLE_MASK <<
> +		   XUSB_PADCTL_UPHY_USB3_PAD_ECTL2_RX_CTLE_SHIFT);
> +	value |= XUSB_PADCTL_UPHY_USB3_PAD_ECTL2_RX_CTLE_VAL <<
> +		 XUSB_PADCTL_UPHY_USB3_PAD_ECTL2_RX_CTLE_SHIFT;
> +	padctl_writel(padctl, value, XUSB_PADCTL_UPHY_USB3_PADX_ECTL2(index));
> +
> +	padctl_writel(padctl, XUSB_PADCTL_UPHY_USB3_PAD_ECTL3_RX_DFE_VAL,
> +		      XUSB_PADCTL_UPHY_USB3_PADX_ECTL3(index));
> +
> +	value = padctl_readl(padctl, XUSB_PADCTL_UPHY_USB3_PADX_ECTL4(index));
> +	value &= ~(XUSB_PADCTL_UPHY_USB3_PAD_ECTL4_RX_CDR_CTRL_MASK <<
> +		   XUSB_PADCTL_UPHY_USB3_PAD_ECTL4_RX_CDR_CTRL_SHIFT);
> +	value |= XUSB_PADCTL_UPHY_USB3_PAD_ECTL4_RX_CDR_CTRL_VAL <<
> +		 XUSB_PADCTL_UPHY_USB3_PAD_ECTL4_RX_CDR_CTRL_SHIFT;
> +	padctl_writel(padctl, value, XUSB_PADCTL_UPHY_USB3_PADX_ECTL4(index));
> +
> +	padctl_writel(padctl, XUSB_PADCTL_UPHY_USB3_PAD_ECTL6_RX_EQ_CTRL_H_VAL,
> +		      XUSB_PADCTL_UPHY_USB3_PADX_ECTL6(index));
> +
> +	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
> +	value &= ~XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_VCORE_DOWN(index);
> +	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
> +
> +	usleep_range(100, 200);
> +
> +	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
> +	value &= ~XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN_EARLY(index);
> +	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
> +
> +	usleep_range(100, 200);
> +
> +	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
> +	value &= ~XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN(index);
> +	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
> +
> +	return 0;
> +}
> +
> +static int tegra210_usb3_phy_power_off(struct phy *phy)
> +{
> +	struct device *dev = &phy->dev;
> +	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
> +	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
> +	struct tegra_xusb_usb3_port *usb3 = tegra_xusb_find_usb3_port(padctl,
> +					    tegra210_usb3_lane_map(lane));
> +	int index;
> +	u32 value;
> +
> +	if (!usb3) {
> +		dev_err(dev, "no USB3 port found for lane %u\n", lane->index);
> +		return -ENODEV;
> +	}
> +	index = usb3->base.index;
> +
> +	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
> +	value |= XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN_EARLY(index);
> +	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
> +
> +	usleep_range(100, 200);
> +
> +	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
> +	value |= XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN(index);
> +	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
> +
> +	usleep_range(250, 350);
> +
> +	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
> +	value |= XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_VCORE_DOWN(index);
> +	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
> +
> +	return 0;
> +}
>  static struct tegra_xusb_lane *
>  tegra210_pcie_lane_probe(struct tegra_xusb_pad *pad, struct device_node *np,
>  			 unsigned int index)
> @@ -1461,6 +1637,13 @@ static const struct tegra_xusb_lane_ops tegra210_pcie_lane_ops = {
>  static int tegra210_pcie_phy_init(struct phy *phy)
>  {
>  	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
> +	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
> +
> +	mutex_lock(&padctl->lock);
> +
> +	tegra210_uphy_init(padctl);
> +
> +	mutex_unlock(&padctl->lock);
>  
>  	return tegra210_xusb_padctl_enable(lane->pad->padctl);
>  }
> @@ -1476,20 +1659,13 @@ static int tegra210_pcie_phy_power_on(struct phy *phy)
>  {
>  	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
>  	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
> -	u32 value;
> -	int err;
> +	int err = 0;
>  
>  	mutex_lock(&padctl->lock);
>  
> -	err = tegra210_pex_uphy_enable(padctl);
> -	if (err < 0)
> -		goto unlock;
> +	if (tegra_xusb_lane_check(lane, "usb3-ss"))
> +		err = tegra210_usb3_phy_power_on(phy);
>  
> -	value = padctl_readl(padctl, XUSB_PADCTL_USB3_PAD_MUX);
> -	value |= XUSB_PADCTL_USB3_PAD_MUX_PCIE_IDDQ_DISABLE(lane->index);
> -	padctl_writel(padctl, value, XUSB_PADCTL_USB3_PAD_MUX);
> -
> -unlock:
>  	mutex_unlock(&padctl->lock);
>  	return err;
>  }
> @@ -1498,15 +1674,15 @@ static int tegra210_pcie_phy_power_off(struct phy *phy)
>  {
>  	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
>  	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
> -	u32 value;
> +	int err = 0;
>  
> -	value = padctl_readl(padctl, XUSB_PADCTL_USB3_PAD_MUX);
> -	value &= ~XUSB_PADCTL_USB3_PAD_MUX_PCIE_IDDQ_DISABLE(lane->index);
> -	padctl_writel(padctl, value, XUSB_PADCTL_USB3_PAD_MUX);
> +	mutex_lock(&padctl->lock);
>  
> -	tegra210_pex_uphy_disable(padctl);
> +	if (tegra_xusb_lane_check(lane, "usb3-ss"))
> +		err = tegra210_usb3_phy_power_off(phy);
>  
> -	return 0;
> +	mutex_unlock(&padctl->lock);
> +	return err;
>  }
>  
>  static const struct phy_ops tegra210_pcie_phy_ops = {
> @@ -1632,7 +1808,13 @@ static const struct tegra_xusb_lane_ops tegra210_sata_lane_ops = {
>  static int tegra210_sata_phy_init(struct phy *phy)
>  {
>  	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
> +	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
> +
> +	mutex_lock(&padctl->lock);
> +
> +	tegra210_uphy_init(padctl);
>  
> +	mutex_unlock(&padctl->lock);
>  	return tegra210_xusb_padctl_enable(lane->pad->padctl);
>  }
>  
> @@ -1647,20 +1829,13 @@ static int tegra210_sata_phy_power_on(struct phy *phy)
>  {
>  	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
>  	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
> -	u32 value;
> -	int err;
> +	int err = 0;
>  
>  	mutex_lock(&padctl->lock);
>  
> -	err = tegra210_sata_uphy_enable(padctl, false);
> -	if (err < 0)
> -		goto unlock;
> -
> -	value = padctl_readl(padctl, XUSB_PADCTL_USB3_PAD_MUX);
> -	value |= XUSB_PADCTL_USB3_PAD_MUX_SATA_IDDQ_DISABLE(lane->index);
> -	padctl_writel(padctl, value, XUSB_PADCTL_USB3_PAD_MUX);
> +	if (tegra_xusb_lane_check(lane, "usb3-ss"))
> +		err = tegra210_usb3_phy_power_on(phy);
>  
> -unlock:
>  	mutex_unlock(&padctl->lock);
>  	return err;
>  }
> @@ -1669,15 +1844,15 @@ static int tegra210_sata_phy_power_off(struct phy *phy)
>  {
>  	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
>  	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
> -	u32 value;
> +	int err = 0;
>  
> -	value = padctl_readl(padctl, XUSB_PADCTL_USB3_PAD_MUX);
> -	value &= ~XUSB_PADCTL_USB3_PAD_MUX_SATA_IDDQ_DISABLE(lane->index);
> -	padctl_writel(padctl, value, XUSB_PADCTL_USB3_PAD_MUX);
> +	mutex_lock(&padctl->lock);
>  
> -	tegra210_sata_uphy_disable(lane->pad->padctl);
> +	if (tegra_xusb_lane_check(lane, "usb3-ss"))
> +		err = tegra210_usb3_phy_power_off(phy);
>  
> -	return 0;
> +	mutex_unlock(&padctl->lock);
> +	return err;
>  }
>  
>  static const struct phy_ops tegra210_sata_phy_ops = {
> @@ -1802,125 +1977,11 @@ static const struct tegra_xusb_port_ops tegra210_hsic_port_ops = {
>  
>  static int tegra210_usb3_port_enable(struct tegra_xusb_port *port)
>  {
> -	struct tegra_xusb_usb3_port *usb3 = to_usb3_port(port);
> -	struct tegra_xusb_padctl *padctl = port->padctl;
> -	struct tegra_xusb_lane *lane = usb3->base.lane;
> -	unsigned int index = port->index;
> -	u32 value;
> -	int err;
> -
> -	value = padctl_readl(padctl, XUSB_PADCTL_SS_PORT_MAP);
> -
> -	if (!usb3->internal)
> -		value &= ~XUSB_PADCTL_SS_PORT_MAP_PORTX_INTERNAL(index);
> -	else
> -		value |= XUSB_PADCTL_SS_PORT_MAP_PORTX_INTERNAL(index);
> -
> -	value &= ~XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP_MASK(index);
> -	value |= XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP(index, usb3->port);
> -	padctl_writel(padctl, value, XUSB_PADCTL_SS_PORT_MAP);
> -
> -	/*
> -	 * TODO: move this code into the PCIe/SATA PHY ->power_on() callbacks
> -	 * and conditionalize based on mux function? This seems to work, but
> -	 * might not be the exact proper sequence.
> -	 */
> -	err = regulator_enable(usb3->supply);
> -	if (err < 0)
> -		return err;
> -
> -	value = padctl_readl(padctl, XUSB_PADCTL_UPHY_USB3_PADX_ECTL1(index));
> -	value &= ~(XUSB_PADCTL_UPHY_USB3_PAD_ECTL1_TX_TERM_CTRL_MASK <<
> -		   XUSB_PADCTL_UPHY_USB3_PAD_ECTL1_TX_TERM_CTRL_SHIFT);
> -	value |= XUSB_PADCTL_UPHY_USB3_PAD_ECTL1_TX_TERM_CTRL_VAL <<
> -		 XUSB_PADCTL_UPHY_USB3_PAD_ECTL1_TX_TERM_CTRL_SHIFT;
> -	padctl_writel(padctl, value, XUSB_PADCTL_UPHY_USB3_PADX_ECTL1(index));
> -
> -	value = padctl_readl(padctl, XUSB_PADCTL_UPHY_USB3_PADX_ECTL2(index));
> -	value &= ~(XUSB_PADCTL_UPHY_USB3_PAD_ECTL2_RX_CTLE_MASK <<
> -		   XUSB_PADCTL_UPHY_USB3_PAD_ECTL2_RX_CTLE_SHIFT);
> -	value |= XUSB_PADCTL_UPHY_USB3_PAD_ECTL2_RX_CTLE_VAL <<
> -		 XUSB_PADCTL_UPHY_USB3_PAD_ECTL2_RX_CTLE_SHIFT;
> -	padctl_writel(padctl, value, XUSB_PADCTL_UPHY_USB3_PADX_ECTL2(index));
> -
> -	padctl_writel(padctl, XUSB_PADCTL_UPHY_USB3_PAD_ECTL3_RX_DFE_VAL,
> -		      XUSB_PADCTL_UPHY_USB3_PADX_ECTL3(index));
> -
> -	value = padctl_readl(padctl, XUSB_PADCTL_UPHY_USB3_PADX_ECTL4(index));
> -	value &= ~(XUSB_PADCTL_UPHY_USB3_PAD_ECTL4_RX_CDR_CTRL_MASK <<
> -		   XUSB_PADCTL_UPHY_USB3_PAD_ECTL4_RX_CDR_CTRL_SHIFT);
> -	value |= XUSB_PADCTL_UPHY_USB3_PAD_ECTL4_RX_CDR_CTRL_VAL <<
> -		 XUSB_PADCTL_UPHY_USB3_PAD_ECTL4_RX_CDR_CTRL_SHIFT;
> -	padctl_writel(padctl, value, XUSB_PADCTL_UPHY_USB3_PADX_ECTL4(index));
> -
> -	padctl_writel(padctl, XUSB_PADCTL_UPHY_USB3_PAD_ECTL6_RX_EQ_CTRL_H_VAL,
> -		      XUSB_PADCTL_UPHY_USB3_PADX_ECTL6(index));
> -
> -	if (lane->pad == padctl->sata)
> -		err = tegra210_sata_uphy_enable(padctl, true);
> -	else
> -		err = tegra210_pex_uphy_enable(padctl);
> -
> -	if (err) {
> -		dev_err(&port->dev, "%s: failed to enable UPHY: %d\n",
> -			__func__, err);
> -		return err;
> -	}
> -
> -	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
> -	value &= ~XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_VCORE_DOWN(index);
> -	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
> -
> -	usleep_range(100, 200);
> -
> -	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
> -	value &= ~XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN_EARLY(index);
> -	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
> -
> -	usleep_range(100, 200);
> -
> -	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
> -	value &= ~XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN(index);
> -	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
> -
>  	return 0;
>  }
>  
>  static void tegra210_usb3_port_disable(struct tegra_xusb_port *port)
>  {
> -	struct tegra_xusb_usb3_port *usb3 = to_usb3_port(port);
> -	struct tegra_xusb_padctl *padctl = port->padctl;
> -	struct tegra_xusb_lane *lane = port->lane;
> -	unsigned int index = port->index;
> -	u32 value;
> -
> -	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
> -	value |= XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN_EARLY(index);
> -	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
> -
> -	usleep_range(100, 200);
> -
> -	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
> -	value |= XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN(index);
> -	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
> -
> -	usleep_range(250, 350);
> -
> -	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
> -	value |= XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_VCORE_DOWN(index);
> -	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
> -
> -	if (lane->pad == padctl->sata)
> -		tegra210_sata_uphy_disable(padctl);
> -	else
> -		tegra210_pex_uphy_disable(padctl);
> -
> -	regulator_disable(usb3->supply);
> -
> -	value = padctl_readl(padctl, XUSB_PADCTL_SS_PORT_MAP);
> -	value &= ~XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP_MASK(index);
> -	value |= XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP(index, 0x7);
> -	padctl_writel(padctl, value, XUSB_PADCTL_SS_PORT_MAP);
>  }
>  
>  static const struct tegra_xusb_lane_map tegra210_usb3_map[] = {
> @@ -1933,6 +1994,24 @@ static const struct tegra_xusb_lane_map tegra210_usb3_map[] = {
>  	{ 0, NULL,   0 }
>  };
>  
> +static int tegra210_usb3_lane_map(struct tegra_xusb_lane *lane)
> +{
> +	const struct tegra_xusb_lane_map *map;
> +
> +	for (map = tegra210_usb3_map; map->type; map++) {
> +		if (map->index == lane->index &&
> +		    strcmp(map->type, lane->pad->soc->name) == 0) {
> +			dev_dbg(lane->pad->padctl->dev,
> +				"lane = %s map to port = usb3-%d\n",
> +				lane->pad->soc->lanes[lane->index].name,
> +				map->port);
> +			return map->port;
> +		}
> +	}
> +
> +	return -1;

Return a proper errno please.

> +}
> +
>  static struct tegra_xusb_lane *
>  tegra210_usb3_port_map(struct tegra_xusb_port *port)
>  {
> diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
> index 2ea8497af82a..7fbba53f6097 100644
> --- a/drivers/phy/tegra/xusb.c
> +++ b/drivers/phy/tegra/xusb.c
> @@ -370,7 +370,7 @@ static int tegra_xusb_setup_pads(struct tegra_xusb_padctl *padctl)
>  	return 0;
>  }
>  
> -static bool tegra_xusb_lane_check(struct tegra_xusb_lane *lane,
> +bool tegra_xusb_lane_check(struct tegra_xusb_lane *lane,
>  				  const char *function)
>  {
>  	const char *func = lane->soc->funcs[lane->function];
> diff --git a/drivers/phy/tegra/xusb.h b/drivers/phy/tegra/xusb.h
> index 093076ca27fd..1bfe14b2a274 100644
> --- a/drivers/phy/tegra/xusb.h
> +++ b/drivers/phy/tegra/xusb.h
> @@ -127,6 +127,8 @@ struct tegra_xusb_lane_ops {
>  	void (*remove)(struct tegra_xusb_lane *lane);
>  };
>  
> +bool tegra_xusb_lane_check(struct tegra_xusb_lane *lane, const char *function);
> +
>  /*
>   * pads
>   */
>
JC Kuo July 5, 2019, 6:48 a.m. UTC | #2
On 7/4/19 9:32 PM, Jon Hunter wrote:
> 
> On 14/06/2019 08:46, JC Kuo wrote:
>> This commit is a preparation for enabling XUSB LP0 support.
> 
> By LP0 do you mean ELPG? If so please stick to using one name for
> referring to the power-state in question.
> 
>> It rearranges T210 XUSB PADCTL UPHY initialization sequence,
> 
> Please use Tegra210 and not T210.
Thanks, I will correct this.
> 
>> for the following reasons:
>>
>> 1. PLLE hardware power sequencer has to be enabled only after
>>    both PEX UPHY PLL and SATA UPHY PLL are initialized.
>>
>> 2. Once UPHY PLL hardware power sequncer is enabled, do not
> 
> s/sequncer/sequencer
Thanks, I will correct this.
> 
>>    assert reset to PEX/SATA PLLs.
> 
> Maybe worth clarifying why here.
When UPHY PLLs are managed by hardware power sequencers, asserting reset to the
PLL will break PLL and sequencer functionality.
> 	
>>
>> 3. At LP0 exit, XUSB_PADCTL_ELPG_PROGRAM1_AUX_MUX_LP0_CLAMP_EN,
>>    XUSB_PADCTL_ELPG_PROGRAM1_AUX_MUX_LP0_CLAMP_EN_EARLY, and
>>    XUSB_PADCTL_ELPG_PROGRAM1_AUX_MUX_LP0_VCORE_DOWN bits have
>>    to be cleared after XUSB_PADCTL_USB3_PAD_MUX_SATA_IDDQ_DISABLE
>>    and XUSB_PADCTL_USB3_PAD_MUX_PCIE_IDDQ_DISABLE bits get set.
>>
>> 4. Move XUSB_PADCTL_SS_PORT_MAP and XUSB_PADCTL_UPHY_USB3_PADX_ECTL*
>>    registers programming from tegra210_usb3_port_enable() to
>>    tegra210_pcie_phy_power_on()/tegra210_sata_phy_power_on() so that
>>    XUSB USB3 ports will be programmed at LP0 exit.
> 
> Looks like you are moving all the code from the port enable to the phy
> enable and after this change the port enable does nothing. Do we not
> differentiate between phy and port? I think a bit more description is
> necessary here to describe the impact of this change.
Sorry that I am not sure whether I should use "LP0" or "SC7" for Linux system
suspend (either to ram or disk). Should I use SC7 instead of LP0?
*_port_enable() APIs will only get invoked once in driver .probe(). At system
resume, hardware (XUSB PADCTL block) is in power-on-reset state. We need to
program hardware once again, so I moved the programming sequence to
*_phy_power_on() that will be invoked at system resume.
> 
>>
>> Signed-off-by: JC Kuo <jckuo@nvidia.com>
>> ---
>>  drivers/phy/tegra/xusb-tegra210.c | 443 ++++++++++++++++++------------
>>  drivers/phy/tegra/xusb.c          |   2 +-
>>  drivers/phy/tegra/xusb.h          |   2 +
>>  3 files changed, 264 insertions(+), 183 deletions(-)
>>
>> diff --git a/drivers/phy/tegra/xusb-tegra210.c b/drivers/phy/tegra/xusb-tegra210.c
>> index 18cea8311d22..007bf352b45e 100644
>> --- a/drivers/phy/tegra/xusb-tegra210.c
>> +++ b/drivers/phy/tegra/xusb-tegra210.c
>> @@ -240,6 +240,8 @@ to_tegra210_xusb_padctl(struct tegra_xusb_padctl *padctl)
>>  	return container_of(padctl, struct tegra210_xusb_padctl, base);
>>  }
>>  
>> +static int tegra210_usb3_lane_map(struct tegra_xusb_lane *lane);
>> +
> 
> Can we avoid adding this prototype?
Thanks, I will move tegra210_usb3_lane_map() function here.
> 
>>  /* must be called under padctl->lock */
>>  static int tegra210_pex_uphy_enable(struct tegra_xusb_padctl *padctl)
>>  {
>> @@ -453,35 +455,44 @@ static int tegra210_pex_uphy_enable(struct tegra_xusb_padctl *padctl)
>>  static void tegra210_pex_uphy_disable(struct tegra_xusb_padctl *padctl)
>>  {
>>  	struct tegra_xusb_pcie_pad *pcie = to_pcie_pad(padctl->pcie);
>> -
>> -	mutex_lock(&padctl->lock);
>> +	u32 value;
>> +	int i;
>>  
>>  	if (WARN_ON(pcie->enable == 0))
>> -		goto unlock;
>> +		return;
>>  
>>  	if (--pcie->enable > 0)
>> -		goto unlock;
>> +		return;
>>  
>> -	reset_control_assert(pcie->rst);
>> +	for (i = 0; i < padctl->pcie->soc->num_lanes; i++) {
>> +		value = padctl_readl(padctl, XUSB_PADCTL_USB3_PAD_MUX);
>> +		value &= ~XUSB_PADCTL_USB3_PAD_MUX_PCIE_IDDQ_DISABLE(i);
>> +		padctl_writel(padctl, value, XUSB_PADCTL_USB3_PAD_MUX);
>> +	}
>>  	clk_disable_unprepare(pcie->pll);
>> -
>> -unlock:
>> -	mutex_unlock(&padctl->lock);
>>  }
>>  
>>  /* must be called under padctl->lock */
>> -static int tegra210_sata_uphy_enable(struct tegra_xusb_padctl *padctl, bool usb)
>> +static int tegra210_sata_uphy_enable(struct tegra_xusb_padctl *padctl)
>>  {
>>  	struct tegra_xusb_sata_pad *sata = to_sata_pad(padctl->sata);
>> +	struct tegra_xusb_lane *lane = tegra_xusb_find_lane(padctl, "sata", 0);
>>  	unsigned long timeout;
>>  	u32 value;
>>  	int err;
>> +	bool usb = false;
>>  
>>  	if (sata->enable > 0) {
>>  		sata->enable++;
>>  		return 0;
>>  	}
>>  
>> +	if (!lane)
>> +		return 0;
>> +
>> +	if (tegra_xusb_lane_check(lane, "usb3-ss"))
>> +		usb = true;
> 
> This return a boolean type so you can just ...
> 
> 	usb = tegra_xusb_lane_check(lane, "usb3-ss");
Thanks. I will modify this.
> 
>> +
>>  	err = clk_prepare_enable(sata->pll);
>>  	if (err < 0)
>>  		return err;
>> @@ -695,30 +706,36 @@ static int tegra210_sata_uphy_enable(struct tegra_xusb_padctl *padctl, bool usb)
>>  static void tegra210_sata_uphy_disable(struct tegra_xusb_padctl *padctl)
>>  {
>>  	struct tegra_xusb_sata_pad *sata = to_sata_pad(padctl->sata);
>> -
>> -	mutex_lock(&padctl->lock);
>> +	u32 value;
>> +	int i;
>>  
>>  	if (WARN_ON(sata->enable == 0))
>> -		goto unlock;
>> +		return;
>>  
>>  	if (--sata->enable > 0)
>> -		goto unlock;
>> +		return;
>>  
>> -	reset_control_assert(sata->rst);
>> +	for (i = 0; i < padctl->sata->soc->num_lanes; i++) {
>> +		value = padctl_readl(padctl, XUSB_PADCTL_USB3_PAD_MUX);
>> +		value &= ~XUSB_PADCTL_USB3_PAD_MUX_SATA_IDDQ_DISABLE(i);
>> +		padctl_writel(padctl, value, XUSB_PADCTL_USB3_PAD_MUX);
>> +	}
>>  	clk_disable_unprepare(sata->pll);
>> -
>> -unlock:
>> -	mutex_unlock(&padctl->lock);
>>  }
>>  
>>  static int tegra210_xusb_padctl_enable(struct tegra_xusb_padctl *padctl)
>>  {
>> -	u32 value;
>> +	return 0;
>> +}
>>  
>> -	mutex_lock(&padctl->lock);
>> +static int tegra210_xusb_padctl_disable(struct tegra_xusb_padctl *padctl)
>> +{
>> +	return 0;
>> +}
> 
> Why bother keeping these functions at all if they now do nothing?
Thanks. I will remove the functions.
> 
>>  
>> -	if (padctl->enable++ > 0)
>> -		goto out;
>> +static void tegra210_aux_mux_lp0_clamp_disable(struct tegra_xusb_padctl *padctl)
> 
> Any reason for renaming these? These appear to deal with the XUSB_PADCTL
> and so the previous names seem fine.
> 
To me, enabling XUSB_PADCTL means the sequence of 1) enabling clock to XUSB
PADCTL and 2) de-asserting reset to XUSB PADCTL; disabling XUSB_PADCTL means the
reverse operation. These two functions are for toggling clamping and vcore_down
signals which can be done only after XUSB_PADCTL is enabled.

>> +{
>> +	u32 value;
>>  
>>  	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
>>  	value &= ~XUSB_PADCTL_ELPG_PROGRAM1_AUX_MUX_LP0_CLAMP_EN;
>> @@ -735,24 +752,12 @@ static int tegra210_xusb_padctl_enable(struct tegra_xusb_padctl *padctl)
>>  	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
>>  	value &= ~XUSB_PADCTL_ELPG_PROGRAM1_AUX_MUX_LP0_VCORE_DOWN;
>>  	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
>> -
>> -out:
>> -	mutex_unlock(&padctl->lock);
>> -	return 0;
>>  }
>>  
>> -static int tegra210_xusb_padctl_disable(struct tegra_xusb_padctl *padctl)
>> +static void tegra210_aux_mux_lp0_clamp_enable(struct tegra_xusb_padctl *padctl)
>>  {
>>  	u32 value;
>>  
>> -	mutex_lock(&padctl->lock);
>> -
>> -	if (WARN_ON(padctl->enable == 0))
>> -		goto out;
>> -
>> -	if (--padctl->enable > 0)
>> -		goto out;
>> -
>>  	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
>>  	value |= XUSB_PADCTL_ELPG_PROGRAM1_AUX_MUX_LP0_VCORE_DOWN;
>>  	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
>> @@ -768,12 +773,76 @@ static int tegra210_xusb_padctl_disable(struct tegra_xusb_padctl *padctl)
>>  	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
>>  	value |= XUSB_PADCTL_ELPG_PROGRAM1_AUX_MUX_LP0_CLAMP_EN;
>>  	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
>> +}
>> +
>> +static int tegra210_uphy_init(struct tegra_xusb_padctl *padctl)
>> +{
>> +	struct tegra_xusb_pcie_pad *pcie;
>> +	struct tegra_xusb_sata_pad *sata;
>> +	u32 value;
>> +	int err;
>> +	int i;
>> +
>> +	if (tegra210_plle_hw_sequence_is_enabled()) {
>> +		dev_dbg(padctl->dev, "PLLE is already in HW control\n");
>> +		/* skip pll initialization, update plle refcount only */
>> +		if (padctl->pcie) {
>> +			pcie = to_pcie_pad(padctl->pcie);
>> +			if (pcie->enable == 0) {
>> +				err = clk_prepare_enable(pcie->pll);
>> +				if (err < 0)
>> +					return err;
>> +				pcie->enable++;
> 
> Do we need all this additional ref counting around clk_prepare_enable?
I will change pcie->enable/sata->enable to be boolean type variable. Thanks.
> 
>> +			}
>> +		}
>> +		if (padctl->sata) {
>> +			sata = to_sata_pad(padctl->sata);
>> +			if (sata->enable == 0) {
>> +				err = clk_prepare_enable(sata->pll);
>> +				if (err < 0)
>> +					return err;
>> +				sata->enable++;
> 
> Same here.
> 
>> +			}
>> +		}
>> +		goto skip_pll_init;
>> +	}
>> +
>> +	if (padctl->pcie)
>> +		tegra210_pex_uphy_enable(padctl);
>> +	if (padctl->sata)
>> +		tegra210_sata_uphy_enable(padctl);
>> +
>> +	tegra210_plle_hw_sequence_start();
>> +
>> +skip_pll_init:
>> +	for (i = 0; i < padctl->pcie->soc->num_lanes; i++) {
>> +		value = padctl_readl(padctl, XUSB_PADCTL_USB3_PAD_MUX);
>> +		value |= XUSB_PADCTL_USB3_PAD_MUX_PCIE_IDDQ_DISABLE(i);
>> +		padctl_writel(padctl, value, XUSB_PADCTL_USB3_PAD_MUX);
>> +	}
>> +
>> +	for (i = 0; i < padctl->sata->soc->num_lanes; i++) {
>> +		value = padctl_readl(padctl, XUSB_PADCTL_USB3_PAD_MUX);
>> +		value |= XUSB_PADCTL_USB3_PAD_MUX_SATA_IDDQ_DISABLE(i);
>> +		padctl_writel(padctl, value, XUSB_PADCTL_USB3_PAD_MUX);
>> +	}
>> +
>> +	tegra210_aux_mux_lp0_clamp_disable(padctl);
>>  
>> -out:
>> -	mutex_unlock(&padctl->lock);
>>  	return 0;
>>  }
>>  
>> +static void __maybe_unused
>> +tegra210_uphy_deinit(struct tegra_xusb_padctl *padctl)
>> +{
>> +	tegra210_aux_mux_lp0_clamp_enable(padctl);
>> +
>> +	if (padctl->pcie)
>> +		tegra210_pex_uphy_disable(padctl);
>> +	if (padctl->sata)
>> +		tegra210_sata_uphy_disable(padctl);
> 
> What about the clocks that were enabled?
pcie->pll will be disabled in tegra210_pex_uphy_disable().
sata->pll will be disabled in tegra210_sata_uphy_disable().

> 
>> +}
>> +
>>  static int tegra210_hsic_set_idle(struct tegra_xusb_padctl *padctl,
>>  				  unsigned int index, bool idle)
>>  {
>> @@ -1420,6 +1489,113 @@ static const struct tegra_xusb_lane_soc tegra210_pcie_lanes[] = {
>>  	TEGRA210_LANE("pcie-6", 0x028, 24, 0x3, pcie),
>>  };
>>  
>> +static int tegra210_usb3_phy_power_on(struct phy *phy)
>> +{
>> +	struct device *dev = &phy->dev;
>> +	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
>> +	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
>> +	struct tegra_xusb_usb3_port *usb3 = tegra_xusb_find_usb3_port(padctl,
>> +					    tegra210_usb3_lane_map(lane));
> 
> I think that this should be placed on separate lines. Other places you
> check the return value of tegra210_usb3_lane_map() but here you don't.
> We should be consistent.
Thanks. I will modify this.
> 
>> +	int index;
>> +	u32 value;
>> +
>> +	if (!usb3) {
>> +		dev_err(dev, "no USB3 port found for lane %u\n", lane->index);
>> +		return -ENODEV;
>> +	}
>> +	index = usb3->base.index;
>> +
>> +	value = padctl_readl(padctl, XUSB_PADCTL_SS_PORT_MAP);
>> +
>> +	if (!usb3->internal)
>> +		value &= ~XUSB_PADCTL_SS_PORT_MAP_PORTX_INTERNAL(index);
>> +	else
>> +		value |= XUSB_PADCTL_SS_PORT_MAP_PORTX_INTERNAL(index);
>> +
>> +	value &= ~XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP_MASK(index);
>> +	value |= XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP(index, usb3->port);
>> +	padctl_writel(padctl, value, XUSB_PADCTL_SS_PORT_MAP);
>> +
>> +	value = padctl_readl(padctl, XUSB_PADCTL_UPHY_USB3_PADX_ECTL1(index));
>> +	value &= ~(XUSB_PADCTL_UPHY_USB3_PAD_ECTL1_TX_TERM_CTRL_MASK <<
>> +		   XUSB_PADCTL_UPHY_USB3_PAD_ECTL1_TX_TERM_CTRL_SHIFT);
>> +	value |= XUSB_PADCTL_UPHY_USB3_PAD_ECTL1_TX_TERM_CTRL_VAL <<
>> +		 XUSB_PADCTL_UPHY_USB3_PAD_ECTL1_TX_TERM_CTRL_SHIFT;
>> +	padctl_writel(padctl, value, XUSB_PADCTL_UPHY_USB3_PADX_ECTL1(index));
>> +
>> +	value = padctl_readl(padctl, XUSB_PADCTL_UPHY_USB3_PADX_ECTL2(index));
>> +	value &= ~(XUSB_PADCTL_UPHY_USB3_PAD_ECTL2_RX_CTLE_MASK <<
>> +		   XUSB_PADCTL_UPHY_USB3_PAD_ECTL2_RX_CTLE_SHIFT);
>> +	value |= XUSB_PADCTL_UPHY_USB3_PAD_ECTL2_RX_CTLE_VAL <<
>> +		 XUSB_PADCTL_UPHY_USB3_PAD_ECTL2_RX_CTLE_SHIFT;
>> +	padctl_writel(padctl, value, XUSB_PADCTL_UPHY_USB3_PADX_ECTL2(index));
>> +
>> +	padctl_writel(padctl, XUSB_PADCTL_UPHY_USB3_PAD_ECTL3_RX_DFE_VAL,
>> +		      XUSB_PADCTL_UPHY_USB3_PADX_ECTL3(index));
>> +
>> +	value = padctl_readl(padctl, XUSB_PADCTL_UPHY_USB3_PADX_ECTL4(index));
>> +	value &= ~(XUSB_PADCTL_UPHY_USB3_PAD_ECTL4_RX_CDR_CTRL_MASK <<
>> +		   XUSB_PADCTL_UPHY_USB3_PAD_ECTL4_RX_CDR_CTRL_SHIFT);
>> +	value |= XUSB_PADCTL_UPHY_USB3_PAD_ECTL4_RX_CDR_CTRL_VAL <<
>> +		 XUSB_PADCTL_UPHY_USB3_PAD_ECTL4_RX_CDR_CTRL_SHIFT;
>> +	padctl_writel(padctl, value, XUSB_PADCTL_UPHY_USB3_PADX_ECTL4(index));
>> +
>> +	padctl_writel(padctl, XUSB_PADCTL_UPHY_USB3_PAD_ECTL6_RX_EQ_CTRL_H_VAL,
>> +		      XUSB_PADCTL_UPHY_USB3_PADX_ECTL6(index));
>> +
>> +	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
>> +	value &= ~XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_VCORE_DOWN(index);
>> +	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
>> +
>> +	usleep_range(100, 200);
>> +
>> +	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
>> +	value &= ~XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN_EARLY(index);
>> +	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
>> +
>> +	usleep_range(100, 200);
>> +
>> +	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
>> +	value &= ~XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN(index);
>> +	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
>> +
>> +	return 0;
>> +}
>> +
>> +static int tegra210_usb3_phy_power_off(struct phy *phy)
>> +{
>> +	struct device *dev = &phy->dev;
>> +	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
>> +	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
>> +	struct tegra_xusb_usb3_port *usb3 = tegra_xusb_find_usb3_port(padctl,
>> +					    tegra210_usb3_lane_map(lane));
>> +	int index;
>> +	u32 value;
>> +
>> +	if (!usb3) {
>> +		dev_err(dev, "no USB3 port found for lane %u\n", lane->index);
>> +		return -ENODEV;
>> +	}
>> +	index = usb3->base.index;
>> +
>> +	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
>> +	value |= XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN_EARLY(index);
>> +	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
>> +
>> +	usleep_range(100, 200);
>> +
>> +	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
>> +	value |= XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN(index);
>> +	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
>> +
>> +	usleep_range(250, 350);
>> +
>> +	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
>> +	value |= XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_VCORE_DOWN(index);
>> +	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
>> +
>> +	return 0;
>> +}
>>  static struct tegra_xusb_lane *
>>  tegra210_pcie_lane_probe(struct tegra_xusb_pad *pad, struct device_node *np,
>>  			 unsigned int index)
>> @@ -1461,6 +1637,13 @@ static const struct tegra_xusb_lane_ops tegra210_pcie_lane_ops = {
>>  static int tegra210_pcie_phy_init(struct phy *phy)
>>  {
>>  	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
>> +	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
>> +
>> +	mutex_lock(&padctl->lock);
>> +
>> +	tegra210_uphy_init(padctl);
>> +
>> +	mutex_unlock(&padctl->lock);
>>  
>>  	return tegra210_xusb_padctl_enable(lane->pad->padctl);
>>  }
>> @@ -1476,20 +1659,13 @@ static int tegra210_pcie_phy_power_on(struct phy *phy)
>>  {
>>  	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
>>  	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
>> -	u32 value;
>> -	int err;
>> +	int err = 0;
>>  
>>  	mutex_lock(&padctl->lock);
>>  
>> -	err = tegra210_pex_uphy_enable(padctl);
>> -	if (err < 0)
>> -		goto unlock;
>> +	if (tegra_xusb_lane_check(lane, "usb3-ss"))
>> +		err = tegra210_usb3_phy_power_on(phy);
>>  
>> -	value = padctl_readl(padctl, XUSB_PADCTL_USB3_PAD_MUX);
>> -	value |= XUSB_PADCTL_USB3_PAD_MUX_PCIE_IDDQ_DISABLE(lane->index);
>> -	padctl_writel(padctl, value, XUSB_PADCTL_USB3_PAD_MUX);
>> -
>> -unlock:
>>  	mutex_unlock(&padctl->lock);
>>  	return err;
>>  }
>> @@ -1498,15 +1674,15 @@ static int tegra210_pcie_phy_power_off(struct phy *phy)
>>  {
>>  	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
>>  	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
>> -	u32 value;
>> +	int err = 0;
>>  
>> -	value = padctl_readl(padctl, XUSB_PADCTL_USB3_PAD_MUX);
>> -	value &= ~XUSB_PADCTL_USB3_PAD_MUX_PCIE_IDDQ_DISABLE(lane->index);
>> -	padctl_writel(padctl, value, XUSB_PADCTL_USB3_PAD_MUX);
>> +	mutex_lock(&padctl->lock);
>>  
>> -	tegra210_pex_uphy_disable(padctl);
>> +	if (tegra_xusb_lane_check(lane, "usb3-ss"))
>> +		err = tegra210_usb3_phy_power_off(phy);
>>  
>> -	return 0;
>> +	mutex_unlock(&padctl->lock);
>> +	return err;
>>  }
>>  
>>  static const struct phy_ops tegra210_pcie_phy_ops = {
>> @@ -1632,7 +1808,13 @@ static const struct tegra_xusb_lane_ops tegra210_sata_lane_ops = {
>>  static int tegra210_sata_phy_init(struct phy *phy)
>>  {
>>  	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
>> +	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
>> +
>> +	mutex_lock(&padctl->lock);
>> +
>> +	tegra210_uphy_init(padctl);
>>  
>> +	mutex_unlock(&padctl->lock);
>>  	return tegra210_xusb_padctl_enable(lane->pad->padctl);
>>  }
>>  
>> @@ -1647,20 +1829,13 @@ static int tegra210_sata_phy_power_on(struct phy *phy)
>>  {
>>  	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
>>  	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
>> -	u32 value;
>> -	int err;
>> +	int err = 0;
>>  
>>  	mutex_lock(&padctl->lock);
>>  
>> -	err = tegra210_sata_uphy_enable(padctl, false);
>> -	if (err < 0)
>> -		goto unlock;
>> -
>> -	value = padctl_readl(padctl, XUSB_PADCTL_USB3_PAD_MUX);
>> -	value |= XUSB_PADCTL_USB3_PAD_MUX_SATA_IDDQ_DISABLE(lane->index);
>> -	padctl_writel(padctl, value, XUSB_PADCTL_USB3_PAD_MUX);
>> +	if (tegra_xusb_lane_check(lane, "usb3-ss"))
>> +		err = tegra210_usb3_phy_power_on(phy);
>>  
>> -unlock:
>>  	mutex_unlock(&padctl->lock);
>>  	return err;
>>  }
>> @@ -1669,15 +1844,15 @@ static int tegra210_sata_phy_power_off(struct phy *phy)
>>  {
>>  	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
>>  	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
>> -	u32 value;
>> +	int err = 0;
>>  
>> -	value = padctl_readl(padctl, XUSB_PADCTL_USB3_PAD_MUX);
>> -	value &= ~XUSB_PADCTL_USB3_PAD_MUX_SATA_IDDQ_DISABLE(lane->index);
>> -	padctl_writel(padctl, value, XUSB_PADCTL_USB3_PAD_MUX);
>> +	mutex_lock(&padctl->lock);
>>  
>> -	tegra210_sata_uphy_disable(lane->pad->padctl);
>> +	if (tegra_xusb_lane_check(lane, "usb3-ss"))
>> +		err = tegra210_usb3_phy_power_off(phy);
>>  
>> -	return 0;
>> +	mutex_unlock(&padctl->lock);
>> +	return err;
>>  }
>>  
>>  static const struct phy_ops tegra210_sata_phy_ops = {
>> @@ -1802,125 +1977,11 @@ static const struct tegra_xusb_port_ops tegra210_hsic_port_ops = {
>>  
>>  static int tegra210_usb3_port_enable(struct tegra_xusb_port *port)
>>  {
>> -	struct tegra_xusb_usb3_port *usb3 = to_usb3_port(port);
>> -	struct tegra_xusb_padctl *padctl = port->padctl;
>> -	struct tegra_xusb_lane *lane = usb3->base.lane;
>> -	unsigned int index = port->index;
>> -	u32 value;
>> -	int err;
>> -
>> -	value = padctl_readl(padctl, XUSB_PADCTL_SS_PORT_MAP);
>> -
>> -	if (!usb3->internal)
>> -		value &= ~XUSB_PADCTL_SS_PORT_MAP_PORTX_INTERNAL(index);
>> -	else
>> -		value |= XUSB_PADCTL_SS_PORT_MAP_PORTX_INTERNAL(index);
>> -
>> -	value &= ~XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP_MASK(index);
>> -	value |= XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP(index, usb3->port);
>> -	padctl_writel(padctl, value, XUSB_PADCTL_SS_PORT_MAP);
>> -
>> -	/*
>> -	 * TODO: move this code into the PCIe/SATA PHY ->power_on() callbacks
>> -	 * and conditionalize based on mux function? This seems to work, but
>> -	 * might not be the exact proper sequence.
>> -	 */
>> -	err = regulator_enable(usb3->supply);
>> -	if (err < 0)
>> -		return err;
>> -
>> -	value = padctl_readl(padctl, XUSB_PADCTL_UPHY_USB3_PADX_ECTL1(index));
>> -	value &= ~(XUSB_PADCTL_UPHY_USB3_PAD_ECTL1_TX_TERM_CTRL_MASK <<
>> -		   XUSB_PADCTL_UPHY_USB3_PAD_ECTL1_TX_TERM_CTRL_SHIFT);
>> -	value |= XUSB_PADCTL_UPHY_USB3_PAD_ECTL1_TX_TERM_CTRL_VAL <<
>> -		 XUSB_PADCTL_UPHY_USB3_PAD_ECTL1_TX_TERM_CTRL_SHIFT;
>> -	padctl_writel(padctl, value, XUSB_PADCTL_UPHY_USB3_PADX_ECTL1(index));
>> -
>> -	value = padctl_readl(padctl, XUSB_PADCTL_UPHY_USB3_PADX_ECTL2(index));
>> -	value &= ~(XUSB_PADCTL_UPHY_USB3_PAD_ECTL2_RX_CTLE_MASK <<
>> -		   XUSB_PADCTL_UPHY_USB3_PAD_ECTL2_RX_CTLE_SHIFT);
>> -	value |= XUSB_PADCTL_UPHY_USB3_PAD_ECTL2_RX_CTLE_VAL <<
>> -		 XUSB_PADCTL_UPHY_USB3_PAD_ECTL2_RX_CTLE_SHIFT;
>> -	padctl_writel(padctl, value, XUSB_PADCTL_UPHY_USB3_PADX_ECTL2(index));
>> -
>> -	padctl_writel(padctl, XUSB_PADCTL_UPHY_USB3_PAD_ECTL3_RX_DFE_VAL,
>> -		      XUSB_PADCTL_UPHY_USB3_PADX_ECTL3(index));
>> -
>> -	value = padctl_readl(padctl, XUSB_PADCTL_UPHY_USB3_PADX_ECTL4(index));
>> -	value &= ~(XUSB_PADCTL_UPHY_USB3_PAD_ECTL4_RX_CDR_CTRL_MASK <<
>> -		   XUSB_PADCTL_UPHY_USB3_PAD_ECTL4_RX_CDR_CTRL_SHIFT);
>> -	value |= XUSB_PADCTL_UPHY_USB3_PAD_ECTL4_RX_CDR_CTRL_VAL <<
>> -		 XUSB_PADCTL_UPHY_USB3_PAD_ECTL4_RX_CDR_CTRL_SHIFT;
>> -	padctl_writel(padctl, value, XUSB_PADCTL_UPHY_USB3_PADX_ECTL4(index));
>> -
>> -	padctl_writel(padctl, XUSB_PADCTL_UPHY_USB3_PAD_ECTL6_RX_EQ_CTRL_H_VAL,
>> -		      XUSB_PADCTL_UPHY_USB3_PADX_ECTL6(index));
>> -
>> -	if (lane->pad == padctl->sata)
>> -		err = tegra210_sata_uphy_enable(padctl, true);
>> -	else
>> -		err = tegra210_pex_uphy_enable(padctl);
>> -
>> -	if (err) {
>> -		dev_err(&port->dev, "%s: failed to enable UPHY: %d\n",
>> -			__func__, err);
>> -		return err;
>> -	}
>> -
>> -	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
>> -	value &= ~XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_VCORE_DOWN(index);
>> -	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
>> -
>> -	usleep_range(100, 200);
>> -
>> -	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
>> -	value &= ~XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN_EARLY(index);
>> -	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
>> -
>> -	usleep_range(100, 200);
>> -
>> -	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
>> -	value &= ~XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN(index);
>> -	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
>> -
>>  	return 0;
>>  }
>>  
>>  static void tegra210_usb3_port_disable(struct tegra_xusb_port *port)
>>  {
>> -	struct tegra_xusb_usb3_port *usb3 = to_usb3_port(port);
>> -	struct tegra_xusb_padctl *padctl = port->padctl;
>> -	struct tegra_xusb_lane *lane = port->lane;
>> -	unsigned int index = port->index;
>> -	u32 value;
>> -
>> -	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
>> -	value |= XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN_EARLY(index);
>> -	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
>> -
>> -	usleep_range(100, 200);
>> -
>> -	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
>> -	value |= XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN(index);
>> -	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
>> -
>> -	usleep_range(250, 350);
>> -
>> -	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
>> -	value |= XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_VCORE_DOWN(index);
>> -	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
>> -
>> -	if (lane->pad == padctl->sata)
>> -		tegra210_sata_uphy_disable(padctl);
>> -	else
>> -		tegra210_pex_uphy_disable(padctl);
>> -
>> -	regulator_disable(usb3->supply);
>> -
>> -	value = padctl_readl(padctl, XUSB_PADCTL_SS_PORT_MAP);
>> -	value &= ~XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP_MASK(index);
>> -	value |= XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP(index, 0x7);
>> -	padctl_writel(padctl, value, XUSB_PADCTL_SS_PORT_MAP);
>>  }
>>  
>>  static const struct tegra_xusb_lane_map tegra210_usb3_map[] = {
>> @@ -1933,6 +1994,24 @@ static const struct tegra_xusb_lane_map tegra210_usb3_map[] = {
>>  	{ 0, NULL,   0 }
>>  };
>>  
>> +static int tegra210_usb3_lane_map(struct tegra_xusb_lane *lane)
>> +{
>> +	const struct tegra_xusb_lane_map *map;
>> +
>> +	for (map = tegra210_usb3_map; map->type; map++) {
>> +		if (map->index == lane->index &&
>> +		    strcmp(map->type, lane->pad->soc->name) == 0) {
>> +			dev_dbg(lane->pad->padctl->dev,
>> +				"lane = %s map to port = usb3-%d\n",
>> +				lane->pad->soc->lanes[lane->index].name,
>> +				map->port);
>> +			return map->port;
>> +		}
>> +	}
>> +
>> +	return -1;
> 
> Return a proper errno please.
I will change the errno to be -EINVAL. Thanks.
> 
>> +}
>> +
>>  static struct tegra_xusb_lane *
>>  tegra210_usb3_port_map(struct tegra_xusb_port *port)
>>  {
>> diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
>> index 2ea8497af82a..7fbba53f6097 100644
>> --- a/drivers/phy/tegra/xusb.c
>> +++ b/drivers/phy/tegra/xusb.c
>> @@ -370,7 +370,7 @@ static int tegra_xusb_setup_pads(struct tegra_xusb_padctl *padctl)
>>  	return 0;
>>  }
>>  
>> -static bool tegra_xusb_lane_check(struct tegra_xusb_lane *lane,
>> +bool tegra_xusb_lane_check(struct tegra_xusb_lane *lane,
>>  				  const char *function)
>>  {
>>  	const char *func = lane->soc->funcs[lane->function];
>> diff --git a/drivers/phy/tegra/xusb.h b/drivers/phy/tegra/xusb.h
>> index 093076ca27fd..1bfe14b2a274 100644
>> --- a/drivers/phy/tegra/xusb.h
>> +++ b/drivers/phy/tegra/xusb.h
>> @@ -127,6 +127,8 @@ struct tegra_xusb_lane_ops {
>>  	void (*remove)(struct tegra_xusb_lane *lane);
>>  };
>>  
>> +bool tegra_xusb_lane_check(struct tegra_xusb_lane *lane, const char *function);
>> +
>>  /*
>>   * pads
>>   */
>>
>
Peter De Schrijver July 8, 2019, 7:55 a.m. UTC | #3
On Fri, Jul 05, 2019 at 02:48:49PM +0800, JC Kuo wrote:
> > Looks like you are moving all the code from the port enable to the phy
> > enable and after this change the port enable does nothing. Do we not
> > differentiate between phy and port? I think a bit more description is
> > necessary here to describe the impact of this change.
> Sorry that I am not sure whether I should use "LP0" or "SC7" for Linux system
> suspend (either to ram or disk). Should I use SC7 instead of LP0?

Please use SC7 rather than LP0.

Peter.

Patch
diff mbox series

diff --git a/drivers/phy/tegra/xusb-tegra210.c b/drivers/phy/tegra/xusb-tegra210.c
index 18cea8311d22..007bf352b45e 100644
--- a/drivers/phy/tegra/xusb-tegra210.c
+++ b/drivers/phy/tegra/xusb-tegra210.c
@@ -240,6 +240,8 @@  to_tegra210_xusb_padctl(struct tegra_xusb_padctl *padctl)
 	return container_of(padctl, struct tegra210_xusb_padctl, base);
 }
 
+static int tegra210_usb3_lane_map(struct tegra_xusb_lane *lane);
+
 /* must be called under padctl->lock */
 static int tegra210_pex_uphy_enable(struct tegra_xusb_padctl *padctl)
 {
@@ -453,35 +455,44 @@  static int tegra210_pex_uphy_enable(struct tegra_xusb_padctl *padctl)
 static void tegra210_pex_uphy_disable(struct tegra_xusb_padctl *padctl)
 {
 	struct tegra_xusb_pcie_pad *pcie = to_pcie_pad(padctl->pcie);
-
-	mutex_lock(&padctl->lock);
+	u32 value;
+	int i;
 
 	if (WARN_ON(pcie->enable == 0))
-		goto unlock;
+		return;
 
 	if (--pcie->enable > 0)
-		goto unlock;
+		return;
 
-	reset_control_assert(pcie->rst);
+	for (i = 0; i < padctl->pcie->soc->num_lanes; i++) {
+		value = padctl_readl(padctl, XUSB_PADCTL_USB3_PAD_MUX);
+		value &= ~XUSB_PADCTL_USB3_PAD_MUX_PCIE_IDDQ_DISABLE(i);
+		padctl_writel(padctl, value, XUSB_PADCTL_USB3_PAD_MUX);
+	}
 	clk_disable_unprepare(pcie->pll);
-
-unlock:
-	mutex_unlock(&padctl->lock);
 }
 
 /* must be called under padctl->lock */
-static int tegra210_sata_uphy_enable(struct tegra_xusb_padctl *padctl, bool usb)
+static int tegra210_sata_uphy_enable(struct tegra_xusb_padctl *padctl)
 {
 	struct tegra_xusb_sata_pad *sata = to_sata_pad(padctl->sata);
+	struct tegra_xusb_lane *lane = tegra_xusb_find_lane(padctl, "sata", 0);
 	unsigned long timeout;
 	u32 value;
 	int err;
+	bool usb = false;
 
 	if (sata->enable > 0) {
 		sata->enable++;
 		return 0;
 	}
 
+	if (!lane)
+		return 0;
+
+	if (tegra_xusb_lane_check(lane, "usb3-ss"))
+		usb = true;
+
 	err = clk_prepare_enable(sata->pll);
 	if (err < 0)
 		return err;
@@ -695,30 +706,36 @@  static int tegra210_sata_uphy_enable(struct tegra_xusb_padctl *padctl, bool usb)
 static void tegra210_sata_uphy_disable(struct tegra_xusb_padctl *padctl)
 {
 	struct tegra_xusb_sata_pad *sata = to_sata_pad(padctl->sata);
-
-	mutex_lock(&padctl->lock);
+	u32 value;
+	int i;
 
 	if (WARN_ON(sata->enable == 0))
-		goto unlock;
+		return;
 
 	if (--sata->enable > 0)
-		goto unlock;
+		return;
 
-	reset_control_assert(sata->rst);
+	for (i = 0; i < padctl->sata->soc->num_lanes; i++) {
+		value = padctl_readl(padctl, XUSB_PADCTL_USB3_PAD_MUX);
+		value &= ~XUSB_PADCTL_USB3_PAD_MUX_SATA_IDDQ_DISABLE(i);
+		padctl_writel(padctl, value, XUSB_PADCTL_USB3_PAD_MUX);
+	}
 	clk_disable_unprepare(sata->pll);
-
-unlock:
-	mutex_unlock(&padctl->lock);
 }
 
 static int tegra210_xusb_padctl_enable(struct tegra_xusb_padctl *padctl)
 {
-	u32 value;
+	return 0;
+}
 
-	mutex_lock(&padctl->lock);
+static int tegra210_xusb_padctl_disable(struct tegra_xusb_padctl *padctl)
+{
+	return 0;
+}
 
-	if (padctl->enable++ > 0)
-		goto out;
+static void tegra210_aux_mux_lp0_clamp_disable(struct tegra_xusb_padctl *padctl)
+{
+	u32 value;
 
 	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
 	value &= ~XUSB_PADCTL_ELPG_PROGRAM1_AUX_MUX_LP0_CLAMP_EN;
@@ -735,24 +752,12 @@  static int tegra210_xusb_padctl_enable(struct tegra_xusb_padctl *padctl)
 	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
 	value &= ~XUSB_PADCTL_ELPG_PROGRAM1_AUX_MUX_LP0_VCORE_DOWN;
 	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
-
-out:
-	mutex_unlock(&padctl->lock);
-	return 0;
 }
 
-static int tegra210_xusb_padctl_disable(struct tegra_xusb_padctl *padctl)
+static void tegra210_aux_mux_lp0_clamp_enable(struct tegra_xusb_padctl *padctl)
 {
 	u32 value;
 
-	mutex_lock(&padctl->lock);
-
-	if (WARN_ON(padctl->enable == 0))
-		goto out;
-
-	if (--padctl->enable > 0)
-		goto out;
-
 	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
 	value |= XUSB_PADCTL_ELPG_PROGRAM1_AUX_MUX_LP0_VCORE_DOWN;
 	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
@@ -768,12 +773,76 @@  static int tegra210_xusb_padctl_disable(struct tegra_xusb_padctl *padctl)
 	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
 	value |= XUSB_PADCTL_ELPG_PROGRAM1_AUX_MUX_LP0_CLAMP_EN;
 	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
+}
+
+static int tegra210_uphy_init(struct tegra_xusb_padctl *padctl)
+{
+	struct tegra_xusb_pcie_pad *pcie;
+	struct tegra_xusb_sata_pad *sata;
+	u32 value;
+	int err;
+	int i;
+
+	if (tegra210_plle_hw_sequence_is_enabled()) {
+		dev_dbg(padctl->dev, "PLLE is already in HW control\n");
+		/* skip pll initialization, update plle refcount only */
+		if (padctl->pcie) {
+			pcie = to_pcie_pad(padctl->pcie);
+			if (pcie->enable == 0) {
+				err = clk_prepare_enable(pcie->pll);
+				if (err < 0)
+					return err;
+				pcie->enable++;
+			}
+		}
+		if (padctl->sata) {
+			sata = to_sata_pad(padctl->sata);
+			if (sata->enable == 0) {
+				err = clk_prepare_enable(sata->pll);
+				if (err < 0)
+					return err;
+				sata->enable++;
+			}
+		}
+		goto skip_pll_init;
+	}
+
+	if (padctl->pcie)
+		tegra210_pex_uphy_enable(padctl);
+	if (padctl->sata)
+		tegra210_sata_uphy_enable(padctl);
+
+	tegra210_plle_hw_sequence_start();
+
+skip_pll_init:
+	for (i = 0; i < padctl->pcie->soc->num_lanes; i++) {
+		value = padctl_readl(padctl, XUSB_PADCTL_USB3_PAD_MUX);
+		value |= XUSB_PADCTL_USB3_PAD_MUX_PCIE_IDDQ_DISABLE(i);
+		padctl_writel(padctl, value, XUSB_PADCTL_USB3_PAD_MUX);
+	}
+
+	for (i = 0; i < padctl->sata->soc->num_lanes; i++) {
+		value = padctl_readl(padctl, XUSB_PADCTL_USB3_PAD_MUX);
+		value |= XUSB_PADCTL_USB3_PAD_MUX_SATA_IDDQ_DISABLE(i);
+		padctl_writel(padctl, value, XUSB_PADCTL_USB3_PAD_MUX);
+	}
+
+	tegra210_aux_mux_lp0_clamp_disable(padctl);
 
-out:
-	mutex_unlock(&padctl->lock);
 	return 0;
 }
 
+static void __maybe_unused
+tegra210_uphy_deinit(struct tegra_xusb_padctl *padctl)
+{
+	tegra210_aux_mux_lp0_clamp_enable(padctl);
+
+	if (padctl->pcie)
+		tegra210_pex_uphy_disable(padctl);
+	if (padctl->sata)
+		tegra210_sata_uphy_disable(padctl);
+}
+
 static int tegra210_hsic_set_idle(struct tegra_xusb_padctl *padctl,
 				  unsigned int index, bool idle)
 {
@@ -1420,6 +1489,113 @@  static const struct tegra_xusb_lane_soc tegra210_pcie_lanes[] = {
 	TEGRA210_LANE("pcie-6", 0x028, 24, 0x3, pcie),
 };
 
+static int tegra210_usb3_phy_power_on(struct phy *phy)
+{
+	struct device *dev = &phy->dev;
+	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
+	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
+	struct tegra_xusb_usb3_port *usb3 = tegra_xusb_find_usb3_port(padctl,
+					    tegra210_usb3_lane_map(lane));
+	int index;
+	u32 value;
+
+	if (!usb3) {
+		dev_err(dev, "no USB3 port found for lane %u\n", lane->index);
+		return -ENODEV;
+	}
+	index = usb3->base.index;
+
+	value = padctl_readl(padctl, XUSB_PADCTL_SS_PORT_MAP);
+
+	if (!usb3->internal)
+		value &= ~XUSB_PADCTL_SS_PORT_MAP_PORTX_INTERNAL(index);
+	else
+		value |= XUSB_PADCTL_SS_PORT_MAP_PORTX_INTERNAL(index);
+
+	value &= ~XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP_MASK(index);
+	value |= XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP(index, usb3->port);
+	padctl_writel(padctl, value, XUSB_PADCTL_SS_PORT_MAP);
+
+	value = padctl_readl(padctl, XUSB_PADCTL_UPHY_USB3_PADX_ECTL1(index));
+	value &= ~(XUSB_PADCTL_UPHY_USB3_PAD_ECTL1_TX_TERM_CTRL_MASK <<
+		   XUSB_PADCTL_UPHY_USB3_PAD_ECTL1_TX_TERM_CTRL_SHIFT);
+	value |= XUSB_PADCTL_UPHY_USB3_PAD_ECTL1_TX_TERM_CTRL_VAL <<
+		 XUSB_PADCTL_UPHY_USB3_PAD_ECTL1_TX_TERM_CTRL_SHIFT;
+	padctl_writel(padctl, value, XUSB_PADCTL_UPHY_USB3_PADX_ECTL1(index));
+
+	value = padctl_readl(padctl, XUSB_PADCTL_UPHY_USB3_PADX_ECTL2(index));
+	value &= ~(XUSB_PADCTL_UPHY_USB3_PAD_ECTL2_RX_CTLE_MASK <<
+		   XUSB_PADCTL_UPHY_USB3_PAD_ECTL2_RX_CTLE_SHIFT);
+	value |= XUSB_PADCTL_UPHY_USB3_PAD_ECTL2_RX_CTLE_VAL <<
+		 XUSB_PADCTL_UPHY_USB3_PAD_ECTL2_RX_CTLE_SHIFT;
+	padctl_writel(padctl, value, XUSB_PADCTL_UPHY_USB3_PADX_ECTL2(index));
+
+	padctl_writel(padctl, XUSB_PADCTL_UPHY_USB3_PAD_ECTL3_RX_DFE_VAL,
+		      XUSB_PADCTL_UPHY_USB3_PADX_ECTL3(index));
+
+	value = padctl_readl(padctl, XUSB_PADCTL_UPHY_USB3_PADX_ECTL4(index));
+	value &= ~(XUSB_PADCTL_UPHY_USB3_PAD_ECTL4_RX_CDR_CTRL_MASK <<
+		   XUSB_PADCTL_UPHY_USB3_PAD_ECTL4_RX_CDR_CTRL_SHIFT);
+	value |= XUSB_PADCTL_UPHY_USB3_PAD_ECTL4_RX_CDR_CTRL_VAL <<
+		 XUSB_PADCTL_UPHY_USB3_PAD_ECTL4_RX_CDR_CTRL_SHIFT;
+	padctl_writel(padctl, value, XUSB_PADCTL_UPHY_USB3_PADX_ECTL4(index));
+
+	padctl_writel(padctl, XUSB_PADCTL_UPHY_USB3_PAD_ECTL6_RX_EQ_CTRL_H_VAL,
+		      XUSB_PADCTL_UPHY_USB3_PADX_ECTL6(index));
+
+	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
+	value &= ~XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_VCORE_DOWN(index);
+	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
+
+	usleep_range(100, 200);
+
+	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
+	value &= ~XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN_EARLY(index);
+	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
+
+	usleep_range(100, 200);
+
+	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
+	value &= ~XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN(index);
+	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
+
+	return 0;
+}
+
+static int tegra210_usb3_phy_power_off(struct phy *phy)
+{
+	struct device *dev = &phy->dev;
+	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
+	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
+	struct tegra_xusb_usb3_port *usb3 = tegra_xusb_find_usb3_port(padctl,
+					    tegra210_usb3_lane_map(lane));
+	int index;
+	u32 value;
+
+	if (!usb3) {
+		dev_err(dev, "no USB3 port found for lane %u\n", lane->index);
+		return -ENODEV;
+	}
+	index = usb3->base.index;
+
+	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
+	value |= XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN_EARLY(index);
+	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
+
+	usleep_range(100, 200);
+
+	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
+	value |= XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN(index);
+	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
+
+	usleep_range(250, 350);
+
+	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
+	value |= XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_VCORE_DOWN(index);
+	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
+
+	return 0;
+}
 static struct tegra_xusb_lane *
 tegra210_pcie_lane_probe(struct tegra_xusb_pad *pad, struct device_node *np,
 			 unsigned int index)
@@ -1461,6 +1637,13 @@  static const struct tegra_xusb_lane_ops tegra210_pcie_lane_ops = {
 static int tegra210_pcie_phy_init(struct phy *phy)
 {
 	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
+	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
+
+	mutex_lock(&padctl->lock);
+
+	tegra210_uphy_init(padctl);
+
+	mutex_unlock(&padctl->lock);
 
 	return tegra210_xusb_padctl_enable(lane->pad->padctl);
 }
@@ -1476,20 +1659,13 @@  static int tegra210_pcie_phy_power_on(struct phy *phy)
 {
 	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
 	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
-	u32 value;
-	int err;
+	int err = 0;
 
 	mutex_lock(&padctl->lock);
 
-	err = tegra210_pex_uphy_enable(padctl);
-	if (err < 0)
-		goto unlock;
+	if (tegra_xusb_lane_check(lane, "usb3-ss"))
+		err = tegra210_usb3_phy_power_on(phy);
 
-	value = padctl_readl(padctl, XUSB_PADCTL_USB3_PAD_MUX);
-	value |= XUSB_PADCTL_USB3_PAD_MUX_PCIE_IDDQ_DISABLE(lane->index);
-	padctl_writel(padctl, value, XUSB_PADCTL_USB3_PAD_MUX);
-
-unlock:
 	mutex_unlock(&padctl->lock);
 	return err;
 }
@@ -1498,15 +1674,15 @@  static int tegra210_pcie_phy_power_off(struct phy *phy)
 {
 	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
 	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
-	u32 value;
+	int err = 0;
 
-	value = padctl_readl(padctl, XUSB_PADCTL_USB3_PAD_MUX);
-	value &= ~XUSB_PADCTL_USB3_PAD_MUX_PCIE_IDDQ_DISABLE(lane->index);
-	padctl_writel(padctl, value, XUSB_PADCTL_USB3_PAD_MUX);
+	mutex_lock(&padctl->lock);
 
-	tegra210_pex_uphy_disable(padctl);
+	if (tegra_xusb_lane_check(lane, "usb3-ss"))
+		err = tegra210_usb3_phy_power_off(phy);
 
-	return 0;
+	mutex_unlock(&padctl->lock);
+	return err;
 }
 
 static const struct phy_ops tegra210_pcie_phy_ops = {
@@ -1632,7 +1808,13 @@  static const struct tegra_xusb_lane_ops tegra210_sata_lane_ops = {
 static int tegra210_sata_phy_init(struct phy *phy)
 {
 	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
+	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
+
+	mutex_lock(&padctl->lock);
+
+	tegra210_uphy_init(padctl);
 
+	mutex_unlock(&padctl->lock);
 	return tegra210_xusb_padctl_enable(lane->pad->padctl);
 }
 
@@ -1647,20 +1829,13 @@  static int tegra210_sata_phy_power_on(struct phy *phy)
 {
 	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
 	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
-	u32 value;
-	int err;
+	int err = 0;
 
 	mutex_lock(&padctl->lock);
 
-	err = tegra210_sata_uphy_enable(padctl, false);
-	if (err < 0)
-		goto unlock;
-
-	value = padctl_readl(padctl, XUSB_PADCTL_USB3_PAD_MUX);
-	value |= XUSB_PADCTL_USB3_PAD_MUX_SATA_IDDQ_DISABLE(lane->index);
-	padctl_writel(padctl, value, XUSB_PADCTL_USB3_PAD_MUX);
+	if (tegra_xusb_lane_check(lane, "usb3-ss"))
+		err = tegra210_usb3_phy_power_on(phy);
 
-unlock:
 	mutex_unlock(&padctl->lock);
 	return err;
 }
@@ -1669,15 +1844,15 @@  static int tegra210_sata_phy_power_off(struct phy *phy)
 {
 	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
 	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
-	u32 value;
+	int err = 0;
 
-	value = padctl_readl(padctl, XUSB_PADCTL_USB3_PAD_MUX);
-	value &= ~XUSB_PADCTL_USB3_PAD_MUX_SATA_IDDQ_DISABLE(lane->index);
-	padctl_writel(padctl, value, XUSB_PADCTL_USB3_PAD_MUX);
+	mutex_lock(&padctl->lock);
 
-	tegra210_sata_uphy_disable(lane->pad->padctl);
+	if (tegra_xusb_lane_check(lane, "usb3-ss"))
+		err = tegra210_usb3_phy_power_off(phy);
 
-	return 0;
+	mutex_unlock(&padctl->lock);
+	return err;
 }
 
 static const struct phy_ops tegra210_sata_phy_ops = {
@@ -1802,125 +1977,11 @@  static const struct tegra_xusb_port_ops tegra210_hsic_port_ops = {
 
 static int tegra210_usb3_port_enable(struct tegra_xusb_port *port)
 {
-	struct tegra_xusb_usb3_port *usb3 = to_usb3_port(port);
-	struct tegra_xusb_padctl *padctl = port->padctl;
-	struct tegra_xusb_lane *lane = usb3->base.lane;
-	unsigned int index = port->index;
-	u32 value;
-	int err;
-
-	value = padctl_readl(padctl, XUSB_PADCTL_SS_PORT_MAP);
-
-	if (!usb3->internal)
-		value &= ~XUSB_PADCTL_SS_PORT_MAP_PORTX_INTERNAL(index);
-	else
-		value |= XUSB_PADCTL_SS_PORT_MAP_PORTX_INTERNAL(index);
-
-	value &= ~XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP_MASK(index);
-	value |= XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP(index, usb3->port);
-	padctl_writel(padctl, value, XUSB_PADCTL_SS_PORT_MAP);
-
-	/*
-	 * TODO: move this code into the PCIe/SATA PHY ->power_on() callbacks
-	 * and conditionalize based on mux function? This seems to work, but
-	 * might not be the exact proper sequence.
-	 */
-	err = regulator_enable(usb3->supply);
-	if (err < 0)
-		return err;
-
-	value = padctl_readl(padctl, XUSB_PADCTL_UPHY_USB3_PADX_ECTL1(index));
-	value &= ~(XUSB_PADCTL_UPHY_USB3_PAD_ECTL1_TX_TERM_CTRL_MASK <<
-		   XUSB_PADCTL_UPHY_USB3_PAD_ECTL1_TX_TERM_CTRL_SHIFT);
-	value |= XUSB_PADCTL_UPHY_USB3_PAD_ECTL1_TX_TERM_CTRL_VAL <<
-		 XUSB_PADCTL_UPHY_USB3_PAD_ECTL1_TX_TERM_CTRL_SHIFT;
-	padctl_writel(padctl, value, XUSB_PADCTL_UPHY_USB3_PADX_ECTL1(index));
-
-	value = padctl_readl(padctl, XUSB_PADCTL_UPHY_USB3_PADX_ECTL2(index));
-	value &= ~(XUSB_PADCTL_UPHY_USB3_PAD_ECTL2_RX_CTLE_MASK <<
-		   XUSB_PADCTL_UPHY_USB3_PAD_ECTL2_RX_CTLE_SHIFT);
-	value |= XUSB_PADCTL_UPHY_USB3_PAD_ECTL2_RX_CTLE_VAL <<
-		 XUSB_PADCTL_UPHY_USB3_PAD_ECTL2_RX_CTLE_SHIFT;
-	padctl_writel(padctl, value, XUSB_PADCTL_UPHY_USB3_PADX_ECTL2(index));
-
-	padctl_writel(padctl, XUSB_PADCTL_UPHY_USB3_PAD_ECTL3_RX_DFE_VAL,
-		      XUSB_PADCTL_UPHY_USB3_PADX_ECTL3(index));
-
-	value = padctl_readl(padctl, XUSB_PADCTL_UPHY_USB3_PADX_ECTL4(index));
-	value &= ~(XUSB_PADCTL_UPHY_USB3_PAD_ECTL4_RX_CDR_CTRL_MASK <<
-		   XUSB_PADCTL_UPHY_USB3_PAD_ECTL4_RX_CDR_CTRL_SHIFT);
-	value |= XUSB_PADCTL_UPHY_USB3_PAD_ECTL4_RX_CDR_CTRL_VAL <<
-		 XUSB_PADCTL_UPHY_USB3_PAD_ECTL4_RX_CDR_CTRL_SHIFT;
-	padctl_writel(padctl, value, XUSB_PADCTL_UPHY_USB3_PADX_ECTL4(index));
-
-	padctl_writel(padctl, XUSB_PADCTL_UPHY_USB3_PAD_ECTL6_RX_EQ_CTRL_H_VAL,
-		      XUSB_PADCTL_UPHY_USB3_PADX_ECTL6(index));
-
-	if (lane->pad == padctl->sata)
-		err = tegra210_sata_uphy_enable(padctl, true);
-	else
-		err = tegra210_pex_uphy_enable(padctl);
-
-	if (err) {
-		dev_err(&port->dev, "%s: failed to enable UPHY: %d\n",
-			__func__, err);
-		return err;
-	}
-
-	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
-	value &= ~XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_VCORE_DOWN(index);
-	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
-
-	usleep_range(100, 200);
-
-	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
-	value &= ~XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN_EARLY(index);
-	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
-
-	usleep_range(100, 200);
-
-	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
-	value &= ~XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN(index);
-	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
-
 	return 0;
 }
 
 static void tegra210_usb3_port_disable(struct tegra_xusb_port *port)
 {
-	struct tegra_xusb_usb3_port *usb3 = to_usb3_port(port);
-	struct tegra_xusb_padctl *padctl = port->padctl;
-	struct tegra_xusb_lane *lane = port->lane;
-	unsigned int index = port->index;
-	u32 value;
-
-	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
-	value |= XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN_EARLY(index);
-	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
-
-	usleep_range(100, 200);
-
-	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
-	value |= XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_CLAMP_EN(index);
-	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
-
-	usleep_range(250, 350);
-
-	value = padctl_readl(padctl, XUSB_PADCTL_ELPG_PROGRAM1);
-	value |= XUSB_PADCTL_ELPG_PROGRAM1_SSPX_ELPG_VCORE_DOWN(index);
-	padctl_writel(padctl, value, XUSB_PADCTL_ELPG_PROGRAM1);
-
-	if (lane->pad == padctl->sata)
-		tegra210_sata_uphy_disable(padctl);
-	else
-		tegra210_pex_uphy_disable(padctl);
-
-	regulator_disable(usb3->supply);
-
-	value = padctl_readl(padctl, XUSB_PADCTL_SS_PORT_MAP);
-	value &= ~XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP_MASK(index);
-	value |= XUSB_PADCTL_SS_PORT_MAP_PORTX_MAP(index, 0x7);
-	padctl_writel(padctl, value, XUSB_PADCTL_SS_PORT_MAP);
 }
 
 static const struct tegra_xusb_lane_map tegra210_usb3_map[] = {
@@ -1933,6 +1994,24 @@  static const struct tegra_xusb_lane_map tegra210_usb3_map[] = {
 	{ 0, NULL,   0 }
 };
 
+static int tegra210_usb3_lane_map(struct tegra_xusb_lane *lane)
+{
+	const struct tegra_xusb_lane_map *map;
+
+	for (map = tegra210_usb3_map; map->type; map++) {
+		if (map->index == lane->index &&
+		    strcmp(map->type, lane->pad->soc->name) == 0) {
+			dev_dbg(lane->pad->padctl->dev,
+				"lane = %s map to port = usb3-%d\n",
+				lane->pad->soc->lanes[lane->index].name,
+				map->port);
+			return map->port;
+		}
+	}
+
+	return -1;
+}
+
 static struct tegra_xusb_lane *
 tegra210_usb3_port_map(struct tegra_xusb_port *port)
 {
diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
index 2ea8497af82a..7fbba53f6097 100644
--- a/drivers/phy/tegra/xusb.c
+++ b/drivers/phy/tegra/xusb.c
@@ -370,7 +370,7 @@  static int tegra_xusb_setup_pads(struct tegra_xusb_padctl *padctl)
 	return 0;
 }
 
-static bool tegra_xusb_lane_check(struct tegra_xusb_lane *lane,
+bool tegra_xusb_lane_check(struct tegra_xusb_lane *lane,
 				  const char *function)
 {
 	const char *func = lane->soc->funcs[lane->function];
diff --git a/drivers/phy/tegra/xusb.h b/drivers/phy/tegra/xusb.h
index 093076ca27fd..1bfe14b2a274 100644
--- a/drivers/phy/tegra/xusb.h
+++ b/drivers/phy/tegra/xusb.h
@@ -127,6 +127,8 @@  struct tegra_xusb_lane_ops {
 	void (*remove)(struct tegra_xusb_lane *lane);
 };
 
+bool tegra_xusb_lane_check(struct tegra_xusb_lane *lane, const char *function);
+
 /*
  * pads
  */