[PATCH 1/1] phy: tegra: xusb: Use a bitmask for UTMI pad power state tracking

Wayne Chang waynec at nvidia.com
Tue Apr 1 00:30:43 PDT 2025


Hi Thierry,

Thanks for your review! I'll update in the next patchset.

thanks,
Wayne.

On 3/21/25 20:22, Thierry Reding wrote:
> On Fri, Mar 14, 2025 at 03:33:48PM +0800, Wayne Chang wrote:
>> The current implementation uses bias_pad_enable as a reference count to
>> manage the shared bias pad for all UTMI PHYs. However, during system
>> suspension with connected USB devices, multiple power-down requests for
>> the UTMI pad result in a mismatch in the reference count, which in turn
>> produces warnings such as:
>>
>> [  237.762967] WARNING: CPU: 10 PID: 1618 at tegra186_utmi_pad_power_down+0x160/0x170
>> [  237.763103] Call trace:
>> [  237.763104]  tegra186_utmi_pad_power_down+0x160/0x170
>> [  237.763107]  tegra186_utmi_phy_power_off+0x10/0x30
>> [  237.763110]  phy_power_off+0x48/0x100
>> [  237.763113]  tegra_xusb_enter_elpg+0x204/0x500
>> [  237.763119]  tegra_xusb_suspend+0x48/0x140
>> [  237.763122]  platform_pm_suspend+0x2c/0xb0
>> [  237.763125]  dpm_run_callback.isra.0+0x20/0xa0
>> [  237.763127]  __device_suspend+0x118/0x330
>> [  237.763129]  dpm_suspend+0x10c/0x1f0
>> [  237.763130]  dpm_suspend_start+0x88/0xb0
>> [  237.763132]  suspend_devices_and_enter+0x120/0x500
>> [  237.763135]  pm_suspend+0x1ec/0x270
>>
>> The root cause was traced back to the dynamic power-down changes
>> introduced in commit a30951d31b25 ("xhci: tegra: USB2 pad power controls"),
>> where the UTMI pad was being powered down without verifying its current
>> state. This unbalanced behavior led to discrepancies in the reference
>> count.
>>
>> To rectify this issue, this patch replaces the single reference counter
>> with a bitmask, renamed to utmi_pad_enabled. Each bit in the mask
>> corresponds to one of the four USB2 PHYs, allowing us to track each pad's
>> enablement status individually.
>>
>> With this change:
>>    - The bias pad is powered on only when the mask is clear.
>>    - Each UTMI pad is powered on or down based on its corresponding bit
>>      in the mask, preventing redundant operations.
>>    - The overall power state of the shared bias pad is maintained
>>      correctly during suspend/resume cycles.
>>
>> Cc: stable at vger.kernel.org
>> Fixes: a30951d31b25 ("xhci: tegra: USB2 pad power controls")
>> Signed-off-by: Wayne Chang <waynec at nvidia.com>
>> ---
>>   drivers/phy/tegra/xusb-tegra186.c | 25 +++++++++++++++++--------
>>   1 file changed, 17 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/phy/tegra/xusb-tegra186.c b/drivers/phy/tegra/xusb-tegra186.c
>> index fae6242aa730..77bb27a34738 100644
>> --- a/drivers/phy/tegra/xusb-tegra186.c
>> +++ b/drivers/phy/tegra/xusb-tegra186.c
>> @@ -237,6 +237,8 @@
>>   #define   DATA0_VAL_PD				BIT(1)
>>   #define   USE_XUSB_AO				BIT(4)
>>   
>> +#define TEGRA_UTMI_PAD_MAX 4
>> +
>>   #define TEGRA186_LANE(_name, _offset, _shift, _mask, _type)		\
>>   	{								\
>>   		.name = _name,						\
>> @@ -269,7 +271,7 @@ struct tegra186_xusb_padctl {
>>   
>>   	/* UTMI bias and tracking */
>>   	struct clk *usb2_trk_clk;
>> -	unsigned int bias_pad_enable;
>> +	DECLARE_BITMAP(utmi_pad_enabled, TEGRA_UTMI_PAD_MAX);
>>   
>>   	/* padctl context */
>>   	struct tegra186_xusb_padctl_context context;
>> @@ -605,7 +607,7 @@ static void tegra186_utmi_bias_pad_power_on(struct tegra_xusb_padctl *padctl)
>>   
>>   	mutex_lock(&padctl->lock);
>>   
>> -	if (priv->bias_pad_enable++ > 0) {
>> +	if (!bitmap_empty(priv->utmi_pad_enabled, TEGRA_UTMI_PAD_MAX)) {
>>   		mutex_unlock(&padctl->lock);
>>   		return;
>>   	}
>> @@ -669,12 +671,7 @@ static void tegra186_utmi_bias_pad_power_off(struct tegra_xusb_padctl *padctl)
>>   
>>   	mutex_lock(&padctl->lock);
>>   
>> -	if (WARN_ON(priv->bias_pad_enable == 0)) {
>> -		mutex_unlock(&padctl->lock);
>> -		return;
>> -	}
>> -
>> -	if (--priv->bias_pad_enable > 0) {
>> +	if (!bitmap_empty(priv->utmi_pad_enabled, TEGRA_UTMI_PAD_MAX)) {
>>   		mutex_unlock(&padctl->lock);
>>   		return;
>>   	}
>> @@ -697,6 +694,7 @@ 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;
>> +	struct tegra186_xusb_padctl *priv = to_tegra186_xusb_padctl(padctl);
>>   	struct tegra_xusb_usb2_port *port;
>>   	struct device *dev = padctl->dev;
>>   	unsigned int index = lane->index;
>> @@ -705,6 +703,9 @@ static void tegra186_utmi_pad_power_on(struct phy *phy)
>>   	if (!phy)
>>   		return;
>>   
>> +	if (test_bit(index, priv->utmi_pad_enabled))
>> +		return;
> Don't we need to take the padctl->lock mutex before this...
>
>> +
>>   	port = tegra_xusb_find_usb2_port(padctl, index);
>>   	if (!port) {
>>   		dev_err(dev, "no port found for USB2 lane %u\n", index);
>> @@ -724,18 +725,24 @@ static void tegra186_utmi_pad_power_on(struct phy *phy)
>>   	value = padctl_readl(padctl, XUSB_PADCTL_USB2_OTG_PADX_CTL1(index));
>>   	value &= ~USB2_OTG_PD_DR;
>>   	padctl_writel(padctl, value, XUSB_PADCTL_USB2_OTG_PADX_CTL1(index));
>> +
>> +	set_bit(index, priv->utmi_pad_enabled);
> ... and release it here? Otherwise we might end up testing, setting and/
> or clearing from two pads concurrently and loose consistency.
>
>>   }
>>   
>>   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;
>> +	struct tegra186_xusb_padctl *priv = to_tegra186_xusb_padctl(padctl);
>>   	unsigned int index = lane->index;
>>   	u32 value;
>>   
>>   	if (!phy)
>>   		return;
>>   
>> +	if (!test_bit(index, priv->utmi_pad_enabled))
>> +		return;
>> +
> Same here...
>
>>   	dev_dbg(padctl->dev, "power down UTMI pad %u\n", index);
>>   
>>   	value = padctl_readl(padctl, XUSB_PADCTL_USB2_OTG_PADX_CTL0(index));
>> @@ -748,6 +755,8 @@ static void tegra186_utmi_pad_power_down(struct phy *phy)
>>   
>>   	udelay(2);
>>   
>> +	clear_bit(index, priv->utmi_pad_enabled);
> and here.
>
> Thierry




More information about the linux-phy mailing list