diff mbox series

[v2,03/12] phy: tegra: xusb: t210: rearrange UPHY init

Message ID 20200831044043.1561074-4-jckuo@nvidia.com (mailing list archive)
State Superseded
Headers show
Series Tegra XHCI controller ELPG support | expand

Commit Message

JC Kuo Aug. 31, 2020, 4:40 a.m. UTC
This commit is a preparation for enabling XUSB SC7 support.
It rearranges Tegra210 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.
   tegra210_uphy_init() -> tegra210_pex_uphy_enable()
                        -> tegra210_sata_uphy_enable()
                        -> tegra210_plle_hw_sequence_start()
                        -> tegra210_aux_mux_lp0_clamp_disable()

2. Once UPHY PLL hardware power sequencer is enabled, do not assert
   reset to PEX/SATA PLLs, otherwise UPHY PLL operation will be
   broken.
   reset_control_assert(pcie->rst) and reset_control_assert(sata->rst)
   are removed from PEX/SATA UPHY disable procedure.

3. At cold boot and SC7 exit, the following bits must be cleared after
   PEX/SATA lanes are out of IDDQ (IDDQ_DISABLE=1).
   a. XUSB_PADCTL_ELPG_PROGRAM1_AUX_MUX_LP0_CLAMP_EN,
   b. XUSB_PADCTL_ELPG_PROGRAM1_AUX_MUX_LP0_CLAMP_EN_EARLY
   c. XUSB_PADCTL_ELPG_PROGRAM1_AUX_MUX_LP0_VCORE_DOWN

   tegra210_pex_uphy_enable() and tegra210_sata_uphy_enable() are in
   charge of bringing lanes out of IDDQ, and then AUX_MUX_LP0_* bits
   will be cleared by tegra210_aux_mux_lp0_clamp_disable().

4. The programming sequence in tegra210_usb3_port_enable() is required
   for both cold boot and SC7 exit, and must be performed only after
   PEX/SATA UPHY is initialized. Therefore, this commit moves the
   programming sequence to .power_on() stub which is invoked after
   .init(). PEX/SATA UPHY is initialzied in .init().

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

Comments

Thierry Reding Aug. 31, 2020, 11:42 a.m. UTC | #1
Please start commit subjects with a capital letter after the prefix.
Also, please avoid t210 as abbreviation and use tegra210 instead.

The above should be something like:

    phy: tegra: xusb: tegra210: Rearrange UPHY init

Or perhaps:

    phy: tegra: xusb: Rearrange UPHY init on Tegra210

On Mon, Aug 31, 2020 at 12:40:34PM +0800, JC Kuo wrote:
> This commit is a preparation for enabling XUSB SC7 support.
> It rearranges Tegra210 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.
>    tegra210_uphy_init() -> tegra210_pex_uphy_enable()
>                         -> tegra210_sata_uphy_enable()
>                         -> tegra210_plle_hw_sequence_start()
>                         -> tegra210_aux_mux_lp0_clamp_disable()
> 
> 2. Once UPHY PLL hardware power sequencer is enabled, do not assert
>    reset to PEX/SATA PLLs, otherwise UPHY PLL operation will be
>    broken.
>    reset_control_assert(pcie->rst) and reset_control_assert(sata->rst)
>    are removed from PEX/SATA UPHY disable procedure.
> 
> 3. At cold boot and SC7 exit, the following bits must be cleared after
>    PEX/SATA lanes are out of IDDQ (IDDQ_DISABLE=1).
>    a. XUSB_PADCTL_ELPG_PROGRAM1_AUX_MUX_LP0_CLAMP_EN,
>    b. XUSB_PADCTL_ELPG_PROGRAM1_AUX_MUX_LP0_CLAMP_EN_EARLY
>    c. XUSB_PADCTL_ELPG_PROGRAM1_AUX_MUX_LP0_VCORE_DOWN
> 
>    tegra210_pex_uphy_enable() and tegra210_sata_uphy_enable() are in
>    charge of bringing lanes out of IDDQ, and then AUX_MUX_LP0_* bits
>    will be cleared by tegra210_aux_mux_lp0_clamp_disable().
> 
> 4. The programming sequence in tegra210_usb3_port_enable() is required
>    for both cold boot and SC7 exit, and must be performed only after
>    PEX/SATA UPHY is initialized. Therefore, this commit moves the
>    programming sequence to .power_on() stub which is invoked after
>    .init(). PEX/SATA UPHY is initialzied in .init().
> 
> Signed-off-by: JC Kuo <jckuo@nvidia.com>
> ---
>  drivers/phy/tegra/xusb-tegra210.c | 495 ++++++++++++++++--------------
>  drivers/phy/tegra/xusb.c          |   2 +-
>  drivers/phy/tegra/xusb.h          |   6 +-
>  3 files changed, 270 insertions(+), 233 deletions(-)

You've listed 4 logically separate changes in the commit message, so I'm
wondering if it's possible to split this patch into 4 different ones. It
might not be worth doing that if they all basically fix the sequence in
one go, but it's pretty difficult to review this as-is.

> 
> diff --git a/drivers/phy/tegra/xusb-tegra210.c b/drivers/phy/tegra/xusb-tegra210.c
> index 66bd4613835b..3a2d9797fb9f 100644
> --- a/drivers/phy/tegra/xusb-tegra210.c
> +++ b/drivers/phy/tegra/xusb-tegra210.c
> @@ -256,23 +256,52 @@ to_tegra210_xusb_padctl(struct tegra_xusb_padctl *padctl)
>  	return container_of(padctl, struct tegra210_xusb_padctl, base);
>  }
>  
> +static const struct tegra_xusb_lane_map tegra210_usb3_map[] = {
> +	{ 0, "pcie", 6 },
> +	{ 1, "pcie", 5 },
> +	{ 2, "pcie", 0 },
> +	{ 2, "pcie", 3 },
> +	{ 3, "pcie", 4 },
> +	{ 3, "pcie", 4 },
> +	{ 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",

"mapped to port"?

> +				lane->pad->soc->lanes[lane->index].name,
> +				map->port);
> +			return map->port;
> +		}
> +	}
> +
> +	return -EINVAL;
> +}
> +
>  /* must be called under padctl->lock */
>  static int tegra210_pex_uphy_enable(struct tegra_xusb_padctl *padctl)
>  {
>  	struct tegra_xusb_pcie_pad *pcie = to_pcie_pad(padctl->pcie);
>  	unsigned long timeout;
>  	u32 value;
> -	int err;
> +	int err, i;

i should be unsigned to match the type of padctl->pcie->soc->num_lanes.

>  
> -	if (pcie->enable > 0) {
> -		pcie->enable++;
> +	if (pcie->enable)
>  		return 0;
> -	}
>  
>  	err = clk_prepare_enable(pcie->pll);
>  	if (err < 0)
>  		return err;
>  
> +	if (tegra210_plle_hw_sequence_is_enabled())
> +		goto skip_pll_init;
> +
>  	err = reset_control_deassert(pcie->rst);

Is it guaranteed that the reset is asserted if the PLLE HW sequencer is
enabled? I suppose with the change to not enable the sequencer by
default in one of the earlier patches this may indeed be a valid
assumption.

>  	if (err < 0)
>  		goto disable;
> @@ -455,7 +484,14 @@ static int tegra210_pex_uphy_enable(struct tegra_xusb_padctl *padctl)
>  
>  	tegra210_xusb_pll_hw_sequence_start();
>  
> -	pcie->enable++;
> +skip_pll_init:
> +	pcie->enable = true;
> +
> +	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);
> +	}
>  
>  	return 0;
>  
> @@ -469,34 +505,42 @@ 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);
> +	u32 value;
> +	int i;

Same as above.

>  
> -	mutex_lock(&padctl->lock);
> -
> -	if (WARN_ON(pcie->enable == 0))
> -		goto unlock;
> +	if (WARN_ON(!pcie->enable))
> +		return;
>  
> -	if (--pcie->enable > 0)
> -		goto unlock;
> +	pcie->enable = false;
>  
> -	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);

Please leave a blank line after a block for better readability.

> -
> -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;
> +	int err, i;

Same comment as above for "i".

> +	bool usb;
>  
> -	if (sata->enable > 0) {
> -		sata->enable++;
> +	if (sata->enable)

Do we want a WARN_ON() here for symmetry with the implementation of
tegra210_sata_uphy_disable() below?

>  		return 0;
> -	}
> +
> +	if (!lane)
> +		return 0;
> +
> +	if (tegra210_plle_hw_sequence_is_enabled())
> +		goto skip_pll_init;
> +
> +	usb = tegra_xusb_lane_check(lane, "usb3-ss");
>  
>  	err = clk_prepare_enable(sata->pll);
>  	if (err < 0)
> @@ -697,7 +741,14 @@ static int tegra210_sata_uphy_enable(struct tegra_xusb_padctl *padctl, bool usb)
>  
>  	tegra210_sata_pll_hw_sequence_start();
>  
> -	sata->enable++;
> +skip_pll_init:
> +	sata->enable = true;
> +
> +	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);
> +	}
>  
>  	return 0;
>  
> @@ -711,31 +762,26 @@ 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);
> +	u32 value;
> +	int i;

unsigned int, please.

>  
> -	mutex_lock(&padctl->lock);
> -
> -	if (WARN_ON(sata->enable == 0))
> -		goto unlock;
> +	if (WARN_ON(!sata->enable))
> +		return;
>  
> -	if (--sata->enable > 0)
> -		goto unlock;
> +	sata->enable = false;
>  
> -	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)
> +static void tegra210_aux_mux_lp0_clamp_disable(struct tegra_xusb_padctl *padctl)
>  {
>  	u32 value;
>  
> -	mutex_lock(&padctl->lock);
> -
> -	if (padctl->enable++ > 0)
> -		goto out;
> -
>  	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);
> @@ -751,24 +797,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);
> @@ -784,12 +818,36 @@ 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)
> +{
> +	if (padctl->pcie)
> +		tegra210_pex_uphy_enable(padctl);
> +	if (padctl->sata)
> +		tegra210_sata_uphy_enable(padctl);
> +
> +	if (!tegra210_plle_hw_sequence_is_enabled())
> +		tegra210_plle_hw_sequence_start();
> +	else
> +		dev_dbg(padctl->dev, "PLLE is already in HW control\n");
> +
> +	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);

Do we need tegra210_plle_hw_sequence_stop() here?

> +
> +	if (padctl->pcie)
> +		tegra210_pex_uphy_disable(padctl);
> +	if (padctl->sata)
> +		tegra210_sata_uphy_disable(padctl);

Maybe reverse the order of these two so that they are symmetrical with
tegra210_uphy_init()? Also, single blank lines between the two blocks
make this easier to read, in my opinion.

Thierry
JC Kuo Sept. 4, 2020, 9:10 a.m. UTC | #2
Hi Thierry,
Thanks for review.

On 8/31/20 7:42 PM, Thierry Reding wrote:
> Please start commit subjects with a capital letter after the prefix.
> Also, please avoid t210 as abbreviation and use tegra210 instead.
> 
> The above should be something like:
> 
>     phy: tegra: xusb: tegra210: Rearrange UPHY init
> 
> Or perhaps:
> 
>     phy: tegra: xusb: Rearrange UPHY init on Tegra210
I will take this one. Thanks.
> 
> On Mon, Aug 31, 2020 at 12:40:34PM +0800, JC Kuo wrote:
>> This commit is a preparation for enabling XUSB SC7 support.
>> It rearranges Tegra210 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.
>>    tegra210_uphy_init() -> tegra210_pex_uphy_enable()
>>                         -> tegra210_sata_uphy_enable()
>>                         -> tegra210_plle_hw_sequence_start()
>>                         -> tegra210_aux_mux_lp0_clamp_disable()
>>
>> 2. Once UPHY PLL hardware power sequencer is enabled, do not assert
>>    reset to PEX/SATA PLLs, otherwise UPHY PLL operation will be
>>    broken.
>>    reset_control_assert(pcie->rst) and reset_control_assert(sata->rst)
>>    are removed from PEX/SATA UPHY disable procedure.
>>
>> 3. At cold boot and SC7 exit, the following bits must be cleared after
>>    PEX/SATA lanes are out of IDDQ (IDDQ_DISABLE=1).
>>    a. XUSB_PADCTL_ELPG_PROGRAM1_AUX_MUX_LP0_CLAMP_EN,
>>    b. XUSB_PADCTL_ELPG_PROGRAM1_AUX_MUX_LP0_CLAMP_EN_EARLY
>>    c. XUSB_PADCTL_ELPG_PROGRAM1_AUX_MUX_LP0_VCORE_DOWN
>>
>>    tegra210_pex_uphy_enable() and tegra210_sata_uphy_enable() are in
>>    charge of bringing lanes out of IDDQ, and then AUX_MUX_LP0_* bits
>>    will be cleared by tegra210_aux_mux_lp0_clamp_disable().
>>
>> 4. The programming sequence in tegra210_usb3_port_enable() is required
>>    for both cold boot and SC7 exit, and must be performed only after
>>    PEX/SATA UPHY is initialized. Therefore, this commit moves the
>>    programming sequence to .power_on() stub which is invoked after
>>    .init(). PEX/SATA UPHY is initialzied in .init().
>>
>> Signed-off-by: JC Kuo <jckuo@nvidia.com>
>> ---
>>  drivers/phy/tegra/xusb-tegra210.c | 495 ++++++++++++++++--------------
>>  drivers/phy/tegra/xusb.c          |   2 +-
>>  drivers/phy/tegra/xusb.h          |   6 +-
>>  3 files changed, 270 insertions(+), 233 deletions(-)
> 
> You've listed 4 logically separate changes in the commit message, so I'm
> wondering if it's possible to split this patch into 4 different ones. It
> might not be worth doing that if they all basically fix the sequence in
> one go, but it's pretty difficult to review this as-is.
I found #1 and #3 are not possible to be split. I will submit #2 and #4 as
separate changes.

> 
>>
>> diff --git a/drivers/phy/tegra/xusb-tegra210.c b/drivers/phy/tegra/xusb-tegra210.c
>> index 66bd4613835b..3a2d9797fb9f 100644
>> --- a/drivers/phy/tegra/xusb-tegra210.c
>> +++ b/drivers/phy/tegra/xusb-tegra210.c
>> @@ -256,23 +256,52 @@ to_tegra210_xusb_padctl(struct tegra_xusb_padctl *padctl)
>>  	return container_of(padctl, struct tegra210_xusb_padctl, base);
>>  }
>>  
>> +static const struct tegra_xusb_lane_map tegra210_usb3_map[] = {
>> +	{ 0, "pcie", 6 },
>> +	{ 1, "pcie", 5 },
>> +	{ 2, "pcie", 0 },
>> +	{ 2, "pcie", 3 },
>> +	{ 3, "pcie", 4 },
>> +	{ 3, "pcie", 4 },
>> +	{ 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",
> 
> "mapped to port"?
Yes, each PEX/SATA lane maps to an USB3 (super-speed) port.
> 
>> +				lane->pad->soc->lanes[lane->index].name,
>> +				map->port);
>> +			return map->port;
>> +		}
>> +	}
>> +
>> +	return -EINVAL;
>> +}
>> +
>>  /* must be called under padctl->lock */
>>  static int tegra210_pex_uphy_enable(struct tegra_xusb_padctl *padctl)
>>  {
>>  	struct tegra_xusb_pcie_pad *pcie = to_pcie_pad(padctl->pcie);
>>  	unsigned long timeout;
>>  	u32 value;
>> -	int err;
>> +	int err, i;
> 
> i should be unsigned to match the type of padctl->pcie->soc->num_lanes.
I will fix this. Thanks.
> 
>>  
>> -	if (pcie->enable > 0) {
>> -		pcie->enable++;
>> +	if (pcie->enable)
>>  		return 0;
>> -	}
>>  
>>  	err = clk_prepare_enable(pcie->pll);
>>  	if (err < 0)
>>  		return err;
>>  
>> +	if (tegra210_plle_hw_sequence_is_enabled())
>> +		goto skip_pll_init;
>> +
>>  	err = reset_control_deassert(pcie->rst);
> 
> Is it guaranteed that the reset is asserted if the PLLE HW sequencer is
> enabled? I suppose with the change to not enable the sequencer by
> default in one of the earlier patches this may indeed be a valid
> assumption.
Yes, reset is de-asserted before PLLE initialization happens.
> 
>>  	if (err < 0)
>>  		goto disable;
>> @@ -455,7 +484,14 @@ static int tegra210_pex_uphy_enable(struct tegra_xusb_padctl *padctl)
>>  
>>  	tegra210_xusb_pll_hw_sequence_start();
>>  
>> -	pcie->enable++;
>> +skip_pll_init:
>> +	pcie->enable = true;
>> +
>> +	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);
>> +	}
>>  
>>  	return 0;
>>  
>> @@ -469,34 +505,42 @@ 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);
>> +	u32 value;
>> +	int i;
> 
> Same as above.
I will fix this. Thanks.
> 
>>  
>> -	mutex_lock(&padctl->lock);
>> -
>> -	if (WARN_ON(pcie->enable == 0))
>> -		goto unlock;
>> +	if (WARN_ON(!pcie->enable))
>> +		return;
>>  
>> -	if (--pcie->enable > 0)
>> -		goto unlock;
>> +	pcie->enable = false;
>>  
>> -	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);
> 
> Please leave a blank line after a block for better readability.
I will fix this. Thanks.
> 
>> -
>> -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;
>> +	int err, i;
> 
> Same comment as above for "i".
I will fix this. Thanks.
> 
>> +	bool usb;
>>  
>> -	if (sata->enable > 0) {
>> -		sata->enable++;
>> +	if (sata->enable)
> 
> Do we want a WARN_ON() here for symmetry with the implementation of
> tegra210_sata_uphy_disable() below?
Yes, I can add this.
> 
>>  		return 0;
>> -	}
>> +
>> +	if (!lane)
>> +		return 0;
>> +
>> +	if (tegra210_plle_hw_sequence_is_enabled())
>> +		goto skip_pll_init;
>> +
>> +	usb = tegra_xusb_lane_check(lane, "usb3-ss");
>>  
>>  	err = clk_prepare_enable(sata->pll);
>>  	if (err < 0)
>> @@ -697,7 +741,14 @@ static int tegra210_sata_uphy_enable(struct tegra_xusb_padctl *padctl, bool usb)
>>  
>>  	tegra210_sata_pll_hw_sequence_start();
>>  
>> -	sata->enable++;
>> +skip_pll_init:
>> +	sata->enable = true;
>> +
>> +	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);
>> +	}
>>  
>>  	return 0;
>>  
>> @@ -711,31 +762,26 @@ 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);
>> +	u32 value;
>> +	int i;
> 
> unsigned int, please.
I will fix this. Thanks.
> 
>>  
>> -	mutex_lock(&padctl->lock);
>> -
>> -	if (WARN_ON(sata->enable == 0))
>> -		goto unlock;
>> +	if (WARN_ON(!sata->enable))
>> +		return;
>>  
>> -	if (--sata->enable > 0)
>> -		goto unlock;
>> +	sata->enable = false;
>>  
>> -	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)
>> +static void tegra210_aux_mux_lp0_clamp_disable(struct tegra_xusb_padctl *padctl)
>>  {
>>  	u32 value;
>>  
>> -	mutex_lock(&padctl->lock);
>> -
>> -	if (padctl->enable++ > 0)
>> -		goto out;
>> -
>>  	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);
>> @@ -751,24 +797,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);
>> @@ -784,12 +818,36 @@ 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)
>> +{
>> +	if (padctl->pcie)
>> +		tegra210_pex_uphy_enable(padctl);
>> +	if (padctl->sata)
>> +		tegra210_sata_uphy_enable(padctl);
>> +
>> +	if (!tegra210_plle_hw_sequence_is_enabled())
>> +		tegra210_plle_hw_sequence_start();
>> +	else
>> +		dev_dbg(padctl->dev, "PLLE is already in HW control\n");
>> +
>> +	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);
> 
> Do we need tegra210_plle_hw_sequence_stop() here?
> 
PLLE hardware power sequencer must remain enabled at SC7 entry.
>> +
>> +	if (padctl->pcie)
>> +		tegra210_pex_uphy_disable(padctl);
>> +	if (padctl->sata)
>> +		tegra210_sata_uphy_disable(padctl);
> 
> Maybe reverse the order of these two so that they are symmetrical with
> tegra210_uphy_init()? Also, single blank lines between the two blocks
> make this easier to read, in my opinion.
I can do this. Thanks.
> 
> Thierry
>
diff mbox series

Patch

diff --git a/drivers/phy/tegra/xusb-tegra210.c b/drivers/phy/tegra/xusb-tegra210.c
index 66bd4613835b..3a2d9797fb9f 100644
--- a/drivers/phy/tegra/xusb-tegra210.c
+++ b/drivers/phy/tegra/xusb-tegra210.c
@@ -256,23 +256,52 @@  to_tegra210_xusb_padctl(struct tegra_xusb_padctl *padctl)
 	return container_of(padctl, struct tegra210_xusb_padctl, base);
 }
 
+static const struct tegra_xusb_lane_map tegra210_usb3_map[] = {
+	{ 0, "pcie", 6 },
+	{ 1, "pcie", 5 },
+	{ 2, "pcie", 0 },
+	{ 2, "pcie", 3 },
+	{ 3, "pcie", 4 },
+	{ 3, "pcie", 4 },
+	{ 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 -EINVAL;
+}
+
 /* must be called under padctl->lock */
 static int tegra210_pex_uphy_enable(struct tegra_xusb_padctl *padctl)
 {
 	struct tegra_xusb_pcie_pad *pcie = to_pcie_pad(padctl->pcie);
 	unsigned long timeout;
 	u32 value;
-	int err;
+	int err, i;
 
-	if (pcie->enable > 0) {
-		pcie->enable++;
+	if (pcie->enable)
 		return 0;
-	}
 
 	err = clk_prepare_enable(pcie->pll);
 	if (err < 0)
 		return err;
 
+	if (tegra210_plle_hw_sequence_is_enabled())
+		goto skip_pll_init;
+
 	err = reset_control_deassert(pcie->rst);
 	if (err < 0)
 		goto disable;
@@ -455,7 +484,14 @@  static int tegra210_pex_uphy_enable(struct tegra_xusb_padctl *padctl)
 
 	tegra210_xusb_pll_hw_sequence_start();
 
-	pcie->enable++;
+skip_pll_init:
+	pcie->enable = true;
+
+	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);
+	}
 
 	return 0;
 
@@ -469,34 +505,42 @@  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);
+	u32 value;
+	int i;
 
-	mutex_lock(&padctl->lock);
-
-	if (WARN_ON(pcie->enable == 0))
-		goto unlock;
+	if (WARN_ON(!pcie->enable))
+		return;
 
-	if (--pcie->enable > 0)
-		goto unlock;
+	pcie->enable = false;
 
-	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;
+	int err, i;
+	bool usb;
 
-	if (sata->enable > 0) {
-		sata->enable++;
+	if (sata->enable)
 		return 0;
-	}
+
+	if (!lane)
+		return 0;
+
+	if (tegra210_plle_hw_sequence_is_enabled())
+		goto skip_pll_init;
+
+	usb = tegra_xusb_lane_check(lane, "usb3-ss");
 
 	err = clk_prepare_enable(sata->pll);
 	if (err < 0)
@@ -697,7 +741,14 @@  static int tegra210_sata_uphy_enable(struct tegra_xusb_padctl *padctl, bool usb)
 
 	tegra210_sata_pll_hw_sequence_start();
 
-	sata->enable++;
+skip_pll_init:
+	sata->enable = true;
+
+	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);
+	}
 
 	return 0;
 
@@ -711,31 +762,26 @@  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);
+	u32 value;
+	int i;
 
-	mutex_lock(&padctl->lock);
-
-	if (WARN_ON(sata->enable == 0))
-		goto unlock;
+	if (WARN_ON(!sata->enable))
+		return;
 
-	if (--sata->enable > 0)
-		goto unlock;
+	sata->enable = false;
 
-	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)
+static void tegra210_aux_mux_lp0_clamp_disable(struct tegra_xusb_padctl *padctl)
 {
 	u32 value;
 
-	mutex_lock(&padctl->lock);
-
-	if (padctl->enable++ > 0)
-		goto out;
-
 	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);
@@ -751,24 +797,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);
@@ -784,12 +818,36 @@  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)
+{
+	if (padctl->pcie)
+		tegra210_pex_uphy_enable(padctl);
+	if (padctl->sata)
+		tegra210_sata_uphy_enable(padctl);
+
+	if (!tegra210_plle_hw_sequence_is_enabled())
+		tegra210_plle_hw_sequence_start();
+	else
+		dev_dbg(padctl->dev, "PLLE is already in HW control\n");
+
+	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)
 {
@@ -926,14 +984,12 @@  static int tegra210_usb2_phy_init(struct phy *phy)
 		 XUSB_PADCTL_USB2_PAD_MUX_USB2_BIAS_PAD_SHIFT;
 	padctl_writel(padctl, value, XUSB_PADCTL_USB2_PAD_MUX);
 
-	return tegra210_xusb_padctl_enable(padctl);
+	return 0;
 }
 
 static int tegra210_usb2_phy_exit(struct phy *phy)
 {
-	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
-
-	return tegra210_xusb_padctl_disable(lane->pad->padctl);
+	return 0;
 }
 
 static int tegra210_xusb_padctl_vbus_override(struct tegra_xusb_padctl *padctl,
@@ -1391,14 +1447,12 @@  static int tegra210_hsic_phy_init(struct phy *phy)
 		 XUSB_PADCTL_USB2_PAD_MUX_HSIC_PAD_TRK_SHIFT;
 	padctl_writel(padctl, value, XUSB_PADCTL_USB2_PAD_MUX);
 
-	return tegra210_xusb_padctl_enable(padctl);
+	return 0;
 }
 
 static int tegra210_hsic_phy_exit(struct phy *phy)
 {
-	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
-
-	return tegra210_xusb_padctl_disable(lane->pad->padctl);
+	return 0;
 }
 
 static int tegra210_hsic_phy_power_on(struct phy *phy)
@@ -1599,6 +1653,128 @@  static const struct tegra_xusb_lane_soc tegra210_pcie_lanes[] = {
 	TEGRA210_LANE("pcie-6", 0x028, 24, 0x3, pcie),
 };
 
+static struct tegra_xusb_usb3_port *
+tegra210_lane_to_usb3_port(struct tegra_xusb_lane *lane)
+{
+	int port;
+
+	if (!lane || !lane->pad || !lane->pad->padctl)
+		return NULL;
+
+	port = tegra210_usb3_lane_map(lane);
+	if (port < 0)
+		return NULL;
+
+	return tegra_xusb_find_usb3_port(lane->pad->padctl, port);
+}
+
+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 = tegra210_lane_to_usb3_port(lane);
+	unsigned 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 = tegra210_lane_to_usb3_port(lane);
+	unsigned 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)
@@ -1640,35 +1816,28 @@  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;
 
-	return tegra210_xusb_padctl_enable(lane->pad->padctl);
-}
+	mutex_lock(&padctl->lock);
 
-static int tegra210_pcie_phy_exit(struct phy *phy)
-{
-	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
+	tegra210_uphy_init(padctl);
 
-	return tegra210_xusb_padctl_disable(lane->pad->padctl);
+	mutex_unlock(&padctl->lock);
+
+	return 0;
 }
 
 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;
-
-	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);
+	if (tegra_xusb_lane_check(lane, "usb3-ss"))
+		err = tegra210_usb3_phy_power_on(phy);
 
-unlock:
 	mutex_unlock(&padctl->lock);
 	return err;
 }
@@ -1677,20 +1846,19 @@  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 = {
 	.init = tegra210_pcie_phy_init,
-	.exit = tegra210_pcie_phy_exit,
 	.power_on = tegra210_pcie_phy_power_on,
 	.power_off = tegra210_pcie_phy_power_off,
 	.owner = THIS_MODULE,
@@ -1811,35 +1979,27 @@  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;
 
-	return tegra210_xusb_padctl_enable(lane->pad->padctl);
-}
+	mutex_lock(&padctl->lock);
 
-static int tegra210_sata_phy_exit(struct phy *phy)
-{
-	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
+	tegra210_uphy_init(padctl);
 
-	return tegra210_xusb_padctl_disable(lane->pad->padctl);
+	mutex_unlock(&padctl->lock);
+	return 0;
 }
 
 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;
 }
@@ -1848,20 +2008,19 @@  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 = {
 	.init = tegra210_sata_phy_init,
-	.exit = tegra210_sata_phy_exit,
 	.power_on = tegra210_sata_phy_power_on,
 	.power_off = tegra210_sata_phy_power_off,
 	.owner = THIS_MODULE,
@@ -1984,137 +2143,13 @@  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[] = {
-	{ 0, "pcie", 6 },
-	{ 1, "pcie", 5 },
-	{ 2, "pcie", 0 },
-	{ 2, "pcie", 3 },
-	{ 3, "pcie", 4 },
-	{ 3, "pcie", 4 },
-	{ 0, NULL,   0 }
-};
-
 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 de4a46fe1763..b48b590aa0c1 100644
--- a/drivers/phy/tegra/xusb.c
+++ b/drivers/phy/tegra/xusb.c
@@ -376,7 +376,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 ea35af747066..0c828694cf2d 100644
--- a/drivers/phy/tegra/xusb.h
+++ b/drivers/phy/tegra/xusb.h
@@ -128,6 +128,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
  */
@@ -230,7 +232,7 @@  struct tegra_xusb_pcie_pad {
 	struct reset_control *rst;
 	struct clk *pll;
 
-	unsigned int enable;
+	bool enable;
 };
 
 static inline struct tegra_xusb_pcie_pad *
@@ -245,7 +247,7 @@  struct tegra_xusb_sata_pad {
 	struct reset_control *rst;
 	struct clk *pll;
 
-	unsigned int enable;
+	bool enable;
 };
 
 static inline struct tegra_xusb_sata_pad *