[PATCH v2 0/2] usb: tegra: power down UTMI pad

[PATCH v2 0/2] usb: tegra: power down UTMI pad

[Jim Lin]

1. Make functions to be generic and public for tegra host/gadget driver 
 to power on/down UTMI pad
2. For tegra gadget driver to power down pad after disconnected
--
v2: update copyright year
    drivers/usb/gadget/udc/tegra-xudc.c
    include/linux/phy/tegra/xusb.h

Jim Lin (2):
  phy: tegra: xusb: add utmi pad power on/down ops
  usb: gadget: tegra: Reduce pad power

 drivers/phy/tegra/xusb-tegra186.c   | 19 ++++++++++++-------
 drivers/phy/tegra/xusb.c            | 22 +++++++++++++++++++++-
 drivers/phy/tegra/xusb.h            |  4 +++-
 drivers/usb/gadget/udc/tegra-xudc.c |  6 +++++-
 include/linux/phy/tegra/xusb.h      |  4 +++-
 5 files changed, 44 insertions(+), 11 deletions(-)

-- 
2.17.1

[PATCH v2 1/2] phy: tegra: xusb: add utmi pad power on/down ops

[Jim Lin]

Add utmi_pad_power_on/down ops for each SOC instead of exporting
tegra_phy_xusb_utmi_pad_power_on/down directly for Tegra186 chip.

Signed-off-by: BH Hsieh <bhsieh@nvidia.com>
Signed-off-by: Jim Lin <jilin@nvidia.com>
---
v2: update copyright year 

 drivers/phy/tegra/xusb-tegra186.c | 19 ++++++++++++-------
 drivers/phy/tegra/xusb.c          | 22 +++++++++++++++++++++-
 drivers/phy/tegra/xusb.h          |  4 +++-
 include/linux/phy/tegra/xusb.h    |  4 +++-
 4 files changed, 39 insertions(+), 10 deletions(-)

diff --git a/drivers/phy/tegra/xusb-tegra186.c b/drivers/phy/tegra/xusb-tegra186.c
index ae3915ed9fef..5abdf81aa143 100644
--- a/drivers/phy/tegra/xusb-tegra186.c
+++ b/drivers/phy/tegra/xusb-tegra186.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
- * Copyright (c) 2016-2020, NVIDIA CORPORATION.  All rights reserved.
+ * Copyright (c) 2016-2022, NVIDIA CORPORATION.  All rights reserved.
  */
 
 #include <linux/delay.h>
@@ -638,7 +638,7 @@ static void tegra186_utmi_bias_pad_power_off(struct tegra_xusb_padctl *padctl)
 	mutex_unlock(&padctl->lock);
 }
 
-static void tegra_phy_xusb_utmi_pad_power_on(struct phy *phy)
+static void tegra186_utmi_pad_power_on(struct phy *phy)
 {
 	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
 	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
@@ -656,6 +656,8 @@ static void tegra_phy_xusb_utmi_pad_power_on(struct phy *phy)
 		return;
 	}
 
+	dev_dbg(dev, "power on UTMI pad %u\n", index);
+
 	tegra186_utmi_bias_pad_power_on(padctl);
 
 	udelay(2);
@@ -669,7 +671,7 @@ static void tegra_phy_xusb_utmi_pad_power_on(struct phy *phy)
 	padctl_writel(padctl, value, XUSB_PADCTL_USB2_OTG_PADX_CTL1(index));
 }
 
-static void tegra_phy_xusb_utmi_pad_power_down(struct phy *phy)
+static void tegra186_utmi_pad_power_down(struct phy *phy)
 {
 	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
 	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
@@ -679,6 +681,8 @@ static void tegra_phy_xusb_utmi_pad_power_down(struct phy *phy)
 	if (!phy)
 		return;
 
+	dev_dbg(padctl->dev, "power down UTMI pad %u\n", index);
+
 	value = padctl_readl(padctl, XUSB_PADCTL_USB2_OTG_PADX_CTL0(index));
 	value |= USB2_OTG_PD;
 	padctl_writel(padctl, value, XUSB_PADCTL_USB2_OTG_PADX_CTL0(index));
@@ -849,15 +853,14 @@ static int tegra186_utmi_phy_power_on(struct phy *phy)
 	value |= RPD_CTRL(priv->calib.rpd_ctrl);
 	padctl_writel(padctl, value, XUSB_PADCTL_USB2_OTG_PADX_CTL1(index));
 
-	/* TODO: pad power saving */
-	tegra_phy_xusb_utmi_pad_power_on(phy);
+	tegra186_utmi_pad_power_on(phy);
+
 	return 0;
 }
 
 static int tegra186_utmi_phy_power_off(struct phy *phy)
 {
-	/* TODO: pad power saving */
-	tegra_phy_xusb_utmi_pad_power_down(phy);
+	tegra186_utmi_pad_power_down(phy);
 
 	return 0;
 }
@@ -1486,6 +1489,8 @@ static const struct tegra_xusb_padctl_ops tegra186_xusb_padctl_ops = {
 	.suspend_noirq = tegra186_xusb_padctl_suspend_noirq,
 	.resume_noirq = tegra186_xusb_padctl_resume_noirq,
 	.vbus_override = tegra186_xusb_padctl_vbus_override,
+	.utmi_pad_power_on = tegra186_utmi_pad_power_on,
+	.utmi_pad_power_down = tegra186_utmi_pad_power_down,
 };
 
 #if IS_ENABLED(CONFIG_ARCH_TEGRA_186_SOC)
diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
index 963de5913e50..49873718d54a 100644
--- a/drivers/phy/tegra/xusb.c
+++ b/drivers/phy/tegra/xusb.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * Copyright (c) 2014-2020, NVIDIA CORPORATION.  All rights reserved.
+ * Copyright (c) 2014-2022, NVIDIA CORPORATION.  All rights reserved.
  */
 
 #include <linux/delay.h>
@@ -1458,6 +1458,26 @@ int tegra_phy_xusb_utmi_port_reset(struct phy *phy)
 }
 EXPORT_SYMBOL_GPL(tegra_phy_xusb_utmi_port_reset);
 
+void tegra_phy_xusb_utmi_pad_power_on(struct phy *phy)
+{
+	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
+	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
+
+	if (padctl->soc->ops->utmi_pad_power_on)
+		padctl->soc->ops->utmi_pad_power_on(phy);
+}
+EXPORT_SYMBOL_GPL(tegra_phy_xusb_utmi_pad_power_on);
+
+void tegra_phy_xusb_utmi_pad_power_down(struct phy *phy)
+{
+	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
+	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
+
+	if (padctl->soc->ops->utmi_pad_power_down)
+		padctl->soc->ops->utmi_pad_power_down(phy);
+}
+EXPORT_SYMBOL_GPL(tegra_phy_xusb_utmi_pad_power_down);
+
 int tegra_xusb_padctl_get_usb3_companion(struct tegra_xusb_padctl *padctl,
 				    unsigned int port)
 {
diff --git a/drivers/phy/tegra/xusb.h b/drivers/phy/tegra/xusb.h
index 034f7a2c28d6..8cfbbdbd6e0c 100644
--- a/drivers/phy/tegra/xusb.h
+++ b/drivers/phy/tegra/xusb.h
@@ -1,6 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
 /*
- * Copyright (c) 2014-2020, NVIDIA CORPORATION.  All rights reserved.
+ * Copyright (c) 2014-2022, NVIDIA CORPORATION.  All rights reserved.
  * Copyright (c) 2015, Google Inc.
  */
 
@@ -412,6 +412,8 @@ struct tegra_xusb_padctl_ops {
 				    unsigned int index, bool enable);
 	int (*vbus_override)(struct tegra_xusb_padctl *padctl, bool set);
 	int (*utmi_port_reset)(struct phy *phy);
+	void (*utmi_pad_power_on)(struct phy *phy);
+	void (*utmi_pad_power_down)(struct phy *phy);
 };
 
 struct tegra_xusb_padctl_soc {
diff --git a/include/linux/phy/tegra/xusb.h b/include/linux/phy/tegra/xusb.h
index 3a35e74cdc61..70998e6dd6fd 100644
--- a/include/linux/phy/tegra/xusb.h
+++ b/include/linux/phy/tegra/xusb.h
@@ -1,6 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
 /*
- * Copyright (c) 2016-2020, NVIDIA CORPORATION.  All rights reserved.
+ * Copyright (c) 2016-2022, NVIDIA CORPORATION.  All rights reserved.
  */
 
 #ifndef PHY_TEGRA_XUSB_H
@@ -21,6 +21,8 @@ int tegra_xusb_padctl_usb3_set_lfps_detect(struct tegra_xusb_padctl *padctl,
 					   unsigned int port, bool enable);
 int tegra_xusb_padctl_set_vbus_override(struct tegra_xusb_padctl *padctl,
 					bool val);
+void tegra_phy_xusb_utmi_pad_power_on(struct phy *phy);
+void tegra_phy_xusb_utmi_pad_power_down(struct phy *phy);
 int tegra_phy_xusb_utmi_port_reset(struct phy *phy);
 int tegra_xusb_padctl_get_usb3_companion(struct tegra_xusb_padctl *padctl,
 					 unsigned int port);
-- 
2.17.1

[PATCH v2 2/2] usb: gadget: tegra: Reduce pad power

[Jim Lin]

Program USB2 UTMI pad PD controls during port connect/disconnect.
Power down pad after disconnected to save power.

Signed-off-by: Jim Lin <jilin@nvidia.com>
---
v2: update copyright year

 drivers/usb/gadget/udc/tegra-xudc.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/tegra-xudc.c b/drivers/usb/gadget/udc/tegra-xudc.c
index 43f1b0d461c1..e59f19be6327 100644
--- a/drivers/usb/gadget/udc/tegra-xudc.c
+++ b/drivers/usb/gadget/udc/tegra-xudc.c
@@ -2,7 +2,7 @@
 /*
  * NVIDIA Tegra XUSB device mode controller
  *
- * Copyright (c) 2013-2019, NVIDIA CORPORATION.  All rights reserved.
+ * Copyright (c) 2013-2022, NVIDIA CORPORATION.  All rights reserved.
  * Copyright (c) 2015, Google Inc.
  */
 
@@ -703,6 +703,8 @@ static void tegra_xudc_device_mode_on(struct tegra_xudc *xudc)
 
 	pm_runtime_get_sync(xudc->dev);
 
+	tegra_phy_xusb_utmi_pad_power_on(xudc->curr_utmi_phy);
+
 	err = phy_power_on(xudc->curr_utmi_phy);
 	if (err < 0)
 		dev_err(xudc->dev, "UTMI power on failed: %d\n", err);
@@ -757,6 +759,8 @@ static void tegra_xudc_device_mode_off(struct tegra_xudc *xudc)
 	/* Make sure interrupt handler has completed before powergating. */
 	synchronize_irq(xudc->irq);
 
+	tegra_phy_xusb_utmi_pad_power_down(xudc->curr_utmi_phy);
+
 	err = phy_power_off(xudc->curr_utmi_phy);
 	if (err < 0)
 		dev_err(xudc->dev, "UTMI PHY power off failed: %d\n", err);
-- 
2.17.1

Re: [PATCH v2 1/2] phy: tegra: xusb: add utmi pad power on/down ops

[Vinod Koul]

On 16-08-22, 16:23, Jim Lin wrote:
> Add utmi_pad_power_on/down ops for each SOC instead of exporting
> tegra_phy_xusb_utmi_pad_power_on/down directly for Tegra186 chip.

Can you please help me understand why do we need to utmi power_on/down
exported and cant be handled thry phy-ops..

> 
> Signed-off-by: BH Hsieh <bhsieh@nvidia.com>
> Signed-off-by: Jim Lin <jilin@nvidia.com>
> ---
> v2: update copyright year 
> 
>  drivers/phy/tegra/xusb-tegra186.c | 19 ++++++++++++-------
>  drivers/phy/tegra/xusb.c          | 22 +++++++++++++++++++++-
>  drivers/phy/tegra/xusb.h          |  4 +++-
>  include/linux/phy/tegra/xusb.h    |  4 +++-
>  4 files changed, 39 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/phy/tegra/xusb-tegra186.c b/drivers/phy/tegra/xusb-tegra186.c
> index ae3915ed9fef..5abdf81aa143 100644
> --- a/drivers/phy/tegra/xusb-tegra186.c
> +++ b/drivers/phy/tegra/xusb-tegra186.c
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0
>  /*
> - * Copyright (c) 2016-2020, NVIDIA CORPORATION.  All rights reserved.
> + * Copyright (c) 2016-2022, NVIDIA CORPORATION.  All rights reserved.
>   */
>  
>  #include <linux/delay.h>
> @@ -638,7 +638,7 @@ static void tegra186_utmi_bias_pad_power_off(struct tegra_xusb_padctl *padctl)
>  	mutex_unlock(&padctl->lock);
>  }
>  
> -static void tegra_phy_xusb_utmi_pad_power_on(struct phy *phy)
> +static void tegra186_utmi_pad_power_on(struct phy *phy)
>  {
>  	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
>  	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
> @@ -656,6 +656,8 @@ static void tegra_phy_xusb_utmi_pad_power_on(struct phy *phy)
>  		return;
>  	}
>  
> +	dev_dbg(dev, "power on UTMI pad %u\n", index);
> +
>  	tegra186_utmi_bias_pad_power_on(padctl);
>  
>  	udelay(2);
> @@ -669,7 +671,7 @@ static void tegra_phy_xusb_utmi_pad_power_on(struct phy *phy)
>  	padctl_writel(padctl, value, XUSB_PADCTL_USB2_OTG_PADX_CTL1(index));
>  }
>  
> -static void tegra_phy_xusb_utmi_pad_power_down(struct phy *phy)
> +static void tegra186_utmi_pad_power_down(struct phy *phy)
>  {
>  	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
>  	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
> @@ -679,6 +681,8 @@ static void tegra_phy_xusb_utmi_pad_power_down(struct phy *phy)
>  	if (!phy)
>  		return;
>  
> +	dev_dbg(padctl->dev, "power down UTMI pad %u\n", index);
> +
>  	value = padctl_readl(padctl, XUSB_PADCTL_USB2_OTG_PADX_CTL0(index));
>  	value |= USB2_OTG_PD;
>  	padctl_writel(padctl, value, XUSB_PADCTL_USB2_OTG_PADX_CTL0(index));
> @@ -849,15 +853,14 @@ static int tegra186_utmi_phy_power_on(struct phy *phy)
>  	value |= RPD_CTRL(priv->calib.rpd_ctrl);
>  	padctl_writel(padctl, value, XUSB_PADCTL_USB2_OTG_PADX_CTL1(index));
>  
> -	/* TODO: pad power saving */
> -	tegra_phy_xusb_utmi_pad_power_on(phy);
> +	tegra186_utmi_pad_power_on(phy);
> +
>  	return 0;
>  }
>  
>  static int tegra186_utmi_phy_power_off(struct phy *phy)
>  {
> -	/* TODO: pad power saving */
> -	tegra_phy_xusb_utmi_pad_power_down(phy);
> +	tegra186_utmi_pad_power_down(phy);
>  
>  	return 0;
>  }
> @@ -1486,6 +1489,8 @@ static const struct tegra_xusb_padctl_ops tegra186_xusb_padctl_ops = {
>  	.suspend_noirq = tegra186_xusb_padctl_suspend_noirq,
>  	.resume_noirq = tegra186_xusb_padctl_resume_noirq,
>  	.vbus_override = tegra186_xusb_padctl_vbus_override,
> +	.utmi_pad_power_on = tegra186_utmi_pad_power_on,
> +	.utmi_pad_power_down = tegra186_utmi_pad_power_down,
>  };
>  
>  #if IS_ENABLED(CONFIG_ARCH_TEGRA_186_SOC)
> diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
> index 963de5913e50..49873718d54a 100644
> --- a/drivers/phy/tegra/xusb.c
> +++ b/drivers/phy/tegra/xusb.c
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /*
> - * Copyright (c) 2014-2020, NVIDIA CORPORATION.  All rights reserved.
> + * Copyright (c) 2014-2022, NVIDIA CORPORATION.  All rights reserved.
>   */
>  
>  #include <linux/delay.h>
> @@ -1458,6 +1458,26 @@ int tegra_phy_xusb_utmi_port_reset(struct phy *phy)
>  }
>  EXPORT_SYMBOL_GPL(tegra_phy_xusb_utmi_port_reset);
>  
> +void tegra_phy_xusb_utmi_pad_power_on(struct phy *phy)
> +{
> +	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
> +	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
> +
> +	if (padctl->soc->ops->utmi_pad_power_on)
> +		padctl->soc->ops->utmi_pad_power_on(phy);
> +}
> +EXPORT_SYMBOL_GPL(tegra_phy_xusb_utmi_pad_power_on);
> +
> +void tegra_phy_xusb_utmi_pad_power_down(struct phy *phy)
> +{
> +	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
> +	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
> +
> +	if (padctl->soc->ops->utmi_pad_power_down)
> +		padctl->soc->ops->utmi_pad_power_down(phy);
> +}
> +EXPORT_SYMBOL_GPL(tegra_phy_xusb_utmi_pad_power_down);
> +
>  int tegra_xusb_padctl_get_usb3_companion(struct tegra_xusb_padctl *padctl,
>  				    unsigned int port)
>  {
> diff --git a/drivers/phy/tegra/xusb.h b/drivers/phy/tegra/xusb.h
> index 034f7a2c28d6..8cfbbdbd6e0c 100644
> --- a/drivers/phy/tegra/xusb.h
> +++ b/drivers/phy/tegra/xusb.h
> @@ -1,6 +1,6 @@
>  /* SPDX-License-Identifier: GPL-2.0-only */
>  /*
> - * Copyright (c) 2014-2020, NVIDIA CORPORATION.  All rights reserved.
> + * Copyright (c) 2014-2022, NVIDIA CORPORATION.  All rights reserved.
>   * Copyright (c) 2015, Google Inc.
>   */
>  
> @@ -412,6 +412,8 @@ struct tegra_xusb_padctl_ops {
>  				    unsigned int index, bool enable);
>  	int (*vbus_override)(struct tegra_xusb_padctl *padctl, bool set);
>  	int (*utmi_port_reset)(struct phy *phy);
> +	void (*utmi_pad_power_on)(struct phy *phy);
> +	void (*utmi_pad_power_down)(struct phy *phy);
>  };
>  
>  struct tegra_xusb_padctl_soc {
> diff --git a/include/linux/phy/tegra/xusb.h b/include/linux/phy/tegra/xusb.h
> index 3a35e74cdc61..70998e6dd6fd 100644
> --- a/include/linux/phy/tegra/xusb.h
> +++ b/include/linux/phy/tegra/xusb.h
> @@ -1,6 +1,6 @@
>  /* SPDX-License-Identifier: GPL-2.0-only */
>  /*
> - * Copyright (c) 2016-2020, NVIDIA CORPORATION.  All rights reserved.
> + * Copyright (c) 2016-2022, NVIDIA CORPORATION.  All rights reserved.
>   */
>  
>  #ifndef PHY_TEGRA_XUSB_H
> @@ -21,6 +21,8 @@ int tegra_xusb_padctl_usb3_set_lfps_detect(struct tegra_xusb_padctl *padctl,
>  					   unsigned int port, bool enable);
>  int tegra_xusb_padctl_set_vbus_override(struct tegra_xusb_padctl *padctl,
>  					bool val);
> +void tegra_phy_xusb_utmi_pad_power_on(struct phy *phy);
> +void tegra_phy_xusb_utmi_pad_power_down(struct phy *phy);
>  int tegra_phy_xusb_utmi_port_reset(struct phy *phy);
>  int tegra_xusb_padctl_get_usb3_companion(struct tegra_xusb_padctl *padctl,
>  					 unsigned int port);
> -- 
> 2.17.1

-- 
~Vinod

Re: [PATCH v2 1/2] phy: tegra: xusb: add utmi pad power on/down ops

[JC Kuo]

Hi Vinod,
Before the device or host is being attached, we can keep most of the
transceivers powered down (PD=1/PD_DR=1) to minimize power consumption. At this
stage, in .phy_power_on(), we enable only the single-ended receiver (PD_ZI=0)
for detecting connection. Upon detecting device's or host's connection, host or
controller driver will invoke tegra_phy_xusb_utmi_pad_power_on() to power on all
of the transceivers (PD=0/PD_DR=0) to equip full link functionality.

Thanks,
JC


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

Re: [PATCH v2 1/2] phy: tegra: xusb: add utmi pad power on/down ops

[Vinod Koul]

On 06-09-22, 10:43, JC Kuo wrote:
> Hi Vinod,

Please _do_ _not_ _top_ _post_

> Before the device or host is being attached, we can keep most of the
> transceivers powered down (PD=1/PD_DR=1) to minimize power consumption. At this
> stage, in .phy_power_on(), we enable only the single-ended receiver (PD_ZI=0)
> for detecting connection. Upon detecting device's or host's connection, host or
> controller driver will invoke tegra_phy_xusb_utmi_pad_power_on() to power on all
> of the transceivers (PD=0/PD_DR=0) to equip full link functionality.

Thanks for this explanation... It helps!

Just a suggestion, can this be moved into phy_init() you have detected
connection in phy_power_on(), the transceiver can be enabled in
phy_int... Would that work?

-- 
~Vinod

Re: [PATCH v2 1/2] phy: tegra: xusb: add utmi pad power on/down ops

[JC Kuo]

On 9/13/22 22:34, Vinod Koul wrote:
> On 06-09-22, 10:43, JC Kuo wrote:
>> Hi Vinod,
> 
> Please _do_ _not_ _top_ _post_
> 
>> Before the device or host is being attached, we can keep most of the
>> transceivers powered down (PD=1/PD_DR=1) to minimize power consumption. At this
>> stage, in .phy_power_on(), we enable only the single-ended receiver (PD_ZI=0)
>> for detecting connection. Upon detecting device's or host's connection, host or
>> controller driver will invoke tegra_phy_xusb_utmi_pad_power_on() to power on all
>> of the transceivers (PD=0/PD_DR=0) to equip full link functionality.
> 
> Thanks for this explanation... It helps!
> 
> Just a suggestion, can this be moved into phy_init() you have detected
> connection in phy_power_on(), the transceiver can be enabled in
> phy_int... Would that work?
> That would work, too. However, because Tegra USB has separate phys for USB3 SS
and USB2, I'd like to keep the USB2 phy operations as they are now, so that USB
host and device controller drivers do not have to distinguish the phy type and
invoke different phy stubs. Furthermore, PD_ZI=0 does really power on the USB2
phy, partially.

For example:
1. in .probe(),
    for_each_usb_phy {
         phy_init(phy);
    }

    for_each_usb3_phy {
         phy_power_on(phy);
    };

2. upon detecting connection,

    phy_power_on(the_target_usb2_phy);

Thanks,
JC

Re: [PATCH v2 1/2] phy: tegra: xusb: add utmi pad power on/down ops

[Vinod Koul]

On 14-09-22, 10:59, JC Kuo wrote:
> On 9/13/22 22:34, Vinod Koul wrote:
> > On 06-09-22, 10:43, JC Kuo wrote:

> > Thanks for this explanation... It helps!
> > 
> > Just a suggestion, can this be moved into phy_init() you have detected
> > connection in phy_power_on(), the transceiver can be enabled in
> > phy_int... Would that work?
> > That would work, too. However, because Tegra USB has separate phys for USB3 SS
> and USB2, I'd like to keep the USB2 phy operations as they are now, so that USB
> host and device controller drivers do not have to distinguish the phy type and
> invoke different phy stubs. Furthermore, PD_ZI=0 does really power on the USB2
> phy, partially.
> 
> For example:
> 1. in .probe(),
>     for_each_usb_phy {
>          phy_init(phy);
>     }
> 
>     for_each_usb3_phy {
>          phy_power_on(phy);
>     };
> 
> 2. upon detecting connection,
> 
>     phy_power_on(the_target_usb2_phy);

It should be always phy_init() in probe and once detection
phy_power_on()

that should be generic flow for all...

-- 
~Vinod