[PATCH 0/5] Fixes to Tegra USB role switching and Smaug USB role switching enablement

[PATCH 0/5] Fixes to Tegra USB role switching and Smaug USB role switching enablement

[Diogo Ivo]

Hello,

This patch series contains two fixes for USB role switching on the
Tegra210 SoC, as well as enabling this feature on the Pixel C.

The first patch addresses a wrong check on the logic that disables the
VBUS regulator.

The second patch guarantees proper ordering of events when switching PHY
roles.

The third and fourth patches then add the necessary nodes and properties
in the Smaug DT in order for role switching to work. Currently with this
patch series this feature can only be controlled from userspace by writing
the desired role to sysfs as

echo "role" > /sys/class/usb_role/usb2-0-role-switch/role

with role being one of {device, host, none}.

Further patches will enable automatic role switching via the 'cros_ec_typec'
driver which is currently broken on Smaug.

N.B: This series does not add a 'connector' node under the 'usb-role-switch'
property added on patch 04/04 because for Smaug the connector should instead
be under the node for 'cros_ec_typec' node and as stated above this
driver is currently broken for this device. If it is deemed better to
describe it but explicitly disable the node let me know and I will send
out a v2.

Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
---
Diogo Ivo (5):
      usb: host: tegra: Remove redundant pm_runtime_mark_last_busy() call
      phy: tegra: xusb: Fix USB2 port regulator disable logic
      phy: tegra: xusb: Fix ordering issue when switching roles on USB2 ports
      arm64: tegra: smaug: Complete and enable tegra-udc node
      arm64: tegra: smaug: Add usb-role-switch support

 arch/arm64/boot/dts/nvidia/tegra210-smaug.dts | 13 +++++++++++++
 drivers/phy/tegra/xusb-tegra210.c             |  5 +++--
 drivers/phy/tegra/xusb.c                      | 23 +++++++++++++++++++++++
 drivers/phy/tegra/xusb.h                      |  1 +
 drivers/usb/gadget/udc/tegra-xudc.c           |  4 ++++
 drivers/usb/host/xhci-tegra.c                 | 17 ++++++++++-------
 include/linux/phy/tegra/xusb.h                |  1 +
 7 files changed, 55 insertions(+), 9 deletions(-)
---
base-commit: a8817ff3b5cd99b0a5af57a92d1a3a7980612550
change-id: 20251201-diogo-tegra_phy-86c89cab7377

Best regards,
-- 
Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>

[PATCH 1/5] usb: host: tegra: Remove redundant pm_runtime_mark_last_busy() call

[Diogo Ivo]

As pm_runtime_put_autosuspend() called at the end of tegra_xhci_id_work()
already calls pm_runtime_mark_last_busy() remove the prior redundant call.

Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
---
 drivers/usb/host/xhci-tegra.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
index 5255b1002893..9c69fccdc6e8 100644
--- a/drivers/usb/host/xhci-tegra.c
+++ b/drivers/usb/host/xhci-tegra.c
@@ -1399,8 +1399,6 @@ static void tegra_xhci_id_work(struct work_struct *work)
 		}
 
 		tegra_xhci_set_port_power(tegra, true, true);
-		pm_runtime_mark_last_busy(tegra->dev);
-
 	} else {
 		if (tegra->otg_usb3_port >= 0)
 			tegra_xhci_set_port_power(tegra, false, false);

-- 
2.52.0

Re: [PATCH 1/5] usb: host: tegra: Remove redundant pm_runtime_mark_last_busy() call

[Diogo Ivo]

Please ignore this patch as this has already been addressed in a patch
in this merge window; sorry for the extra noise.

On 12/4/25 21:27, Diogo Ivo wrote:
> As pm_runtime_put_autosuspend() called at the end of tegra_xhci_id_work()
> already calls pm_runtime_mark_last_busy() remove the prior redundant call.
> 
> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
> ---
>   drivers/usb/host/xhci-tegra.c | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
> index 5255b1002893..9c69fccdc6e8 100644
> --- a/drivers/usb/host/xhci-tegra.c
> +++ b/drivers/usb/host/xhci-tegra.c
> @@ -1399,8 +1399,6 @@ static void tegra_xhci_id_work(struct work_struct *work)
>   		}
>   
>   		tegra_xhci_set_port_power(tegra, true, true);
> -		pm_runtime_mark_last_busy(tegra->dev);
> -
>   	} else {
>   		if (tegra->otg_usb3_port >= 0)
>   			tegra_xhci_set_port_power(tegra, false, false);
> 

[PATCH 2/5] phy: tegra: xusb: Fix USB2 port regulator disable logic

[Diogo Ivo]

The USB2 PHY mode handling on Tegra210 incorrectly relied on
regulator_is_enabled() when determining whether the VBUS supply should
be disabled during role changes. This is because regulator_is_enabled()
reports exactly what is states and not if there is an unbalanced number
of calls between regulator_enable() and regulator_disable(). For
example, regulator_is_enabled() always reports true on a fixed-regulator
with no enable gpio, which is the case on the Pixel C.

This then leads to the PHY driver wrongfully calling regulator_disable()
when transitioning from USB_ROLE_DEVICE to USB_ROLE_NONE since the driver
did not previously call the corresponding regulator_enable().

Fix this by keeping track of the current role and updating the logic to
disable the regulator only when the previous role was USB_ROLE_HOST.

While at it fix a small typo in a comment.

Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
---
 drivers/phy/tegra/xusb-tegra210.c | 5 +++--
 drivers/phy/tegra/xusb.h          | 1 +
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/tegra/xusb-tegra210.c b/drivers/phy/tegra/xusb-tegra210.c
index 3409924498e9..63ad57d95514 100644
--- a/drivers/phy/tegra/xusb-tegra210.c
+++ b/drivers/phy/tegra/xusb-tegra210.c
@@ -1934,9 +1934,9 @@ static int tegra210_usb2_phy_set_mode(struct phy *phy, enum phy_mode mode,
 			/*
 			 * When port is peripheral only or role transitions to
 			 * USB_ROLE_NONE from USB_ROLE_DEVICE, regulator is not
-			 * be enabled.
+			 * enabled.
 			 */
-			if (regulator_is_enabled(port->supply))
+			if (port->role == USB_ROLE_HOST)
 				regulator_disable(port->supply);
 
 			tegra210_xusb_padctl_id_override(padctl, false);
@@ -1944,6 +1944,7 @@ static int tegra210_usb2_phy_set_mode(struct phy *phy, enum phy_mode mode,
 		}
 	}
 
+	port->role = submode;
 	mutex_unlock(&padctl->lock);
 
 	return err;
diff --git a/drivers/phy/tegra/xusb.h b/drivers/phy/tegra/xusb.h
index d2b5f9565132..273af147dfd3 100644
--- a/drivers/phy/tegra/xusb.h
+++ b/drivers/phy/tegra/xusb.h
@@ -317,6 +317,7 @@ struct tegra_xusb_usb2_port {
 	enum usb_dr_mode mode;
 	bool internal;
 	int usb3_port_fake;
+	enum usb_role role;
 };
 
 static inline struct tegra_xusb_usb2_port *

-- 
2.52.0

Re: [PATCH 2/5] phy: tegra: xusb: Fix USB2 port regulator disable logic

[Vinod Koul]

On 04-12-25, 21:27, Diogo Ivo wrote:
> The USB2 PHY mode handling on Tegra210 incorrectly relied on
> regulator_is_enabled() when determining whether the VBUS supply should
> be disabled during role changes. This is because regulator_is_enabled()
> reports exactly what is states and not if there is an unbalanced number
> of calls between regulator_enable() and regulator_disable(). For
> example, regulator_is_enabled() always reports true on a fixed-regulator
> with no enable gpio, which is the case on the Pixel C.
> 
> This then leads to the PHY driver wrongfully calling regulator_disable()
> when transitioning from USB_ROLE_DEVICE to USB_ROLE_NONE since the driver
> did not previously call the corresponding regulator_enable().
> 
> Fix this by keeping track of the current role and updating the logic to
> disable the regulator only when the previous role was USB_ROLE_HOST.
> 
> While at it fix a small typo in a comment.

Never mix a patch with something else please. More imp if it is fix
which will go to rcX. Please send a different patch for typo

> 
> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
> ---
>  drivers/phy/tegra/xusb-tegra210.c | 5 +++--
>  drivers/phy/tegra/xusb.h          | 1 +
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/phy/tegra/xusb-tegra210.c b/drivers/phy/tegra/xusb-tegra210.c
> index 3409924498e9..63ad57d95514 100644
> --- a/drivers/phy/tegra/xusb-tegra210.c
> +++ b/drivers/phy/tegra/xusb-tegra210.c
> @@ -1934,9 +1934,9 @@ static int tegra210_usb2_phy_set_mode(struct phy *phy, enum phy_mode mode,
>  			/*
>  			 * When port is peripheral only or role transitions to
>  			 * USB_ROLE_NONE from USB_ROLE_DEVICE, regulator is not
> -			 * be enabled.
> +			 * enabled.
>  			 */
> -			if (regulator_is_enabled(port->supply))
> +			if (port->role == USB_ROLE_HOST)
>  				regulator_disable(port->supply);
>  
>  			tegra210_xusb_padctl_id_override(padctl, false);
> @@ -1944,6 +1944,7 @@ static int tegra210_usb2_phy_set_mode(struct phy *phy, enum phy_mode mode,
>  		}
>  	}
>  
> +	port->role = submode;
>  	mutex_unlock(&padctl->lock);
>  
>  	return err;
> diff --git a/drivers/phy/tegra/xusb.h b/drivers/phy/tegra/xusb.h
> index d2b5f9565132..273af147dfd3 100644
> --- a/drivers/phy/tegra/xusb.h
> +++ b/drivers/phy/tegra/xusb.h
> @@ -317,6 +317,7 @@ struct tegra_xusb_usb2_port {
>  	enum usb_dr_mode mode;
>  	bool internal;
>  	int usb3_port_fake;
> +	enum usb_role role;
>  };

Jonathan can we get some t-b for these two patches

-- 
~Vinod

Re: [PATCH 2/5] phy: tegra: xusb: Fix USB2 port regulator disable logic

[Diogo Ivo]

On 12/24/25 07:27, Vinod Koul wrote:
> On 04-12-25, 21:27, Diogo Ivo wrote:
>> The USB2 PHY mode handling on Tegra210 incorrectly relied on
>> regulator_is_enabled() when determining whether the VBUS supply should
>> be disabled during role changes. This is because regulator_is_enabled()
>> reports exactly what is states and not if there is an unbalanced number
>> of calls between regulator_enable() and regulator_disable(). For
>> example, regulator_is_enabled() always reports true on a fixed-regulator
>> with no enable gpio, which is the case on the Pixel C.
>>
>> This then leads to the PHY driver wrongfully calling regulator_disable()
>> when transitioning from USB_ROLE_DEVICE to USB_ROLE_NONE since the driver
>> did not previously call the corresponding regulator_enable().
>>
>> Fix this by keeping track of the current role and updating the logic to
>> disable the regulator only when the previous role was USB_ROLE_HOST.
>>
>> While at it fix a small typo in a comment.
> 
> Never mix a patch with something else please. More imp if it is fix
> which will go to rcX. Please send a different patch for typo

Ok, will split for v2.

>> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
>> ---
>>   drivers/phy/tegra/xusb-tegra210.c | 5 +++--
>>   drivers/phy/tegra/xusb.h          | 1 +
>>   2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/phy/tegra/xusb-tegra210.c b/drivers/phy/tegra/xusb-tegra210.c
>> index 3409924498e9..63ad57d95514 100644
>> --- a/drivers/phy/tegra/xusb-tegra210.c
>> +++ b/drivers/phy/tegra/xusb-tegra210.c
>> @@ -1934,9 +1934,9 @@ static int tegra210_usb2_phy_set_mode(struct phy *phy, enum phy_mode mode,
>>   			/*
>>   			 * When port is peripheral only or role transitions to
>>   			 * USB_ROLE_NONE from USB_ROLE_DEVICE, regulator is not
>> -			 * be enabled.
>> +			 * enabled.
>>   			 */
>> -			if (regulator_is_enabled(port->supply))
>> +			if (port->role == USB_ROLE_HOST)
>>   				regulator_disable(port->supply);
>>   
>>   			tegra210_xusb_padctl_id_override(padctl, false);
>> @@ -1944,6 +1944,7 @@ static int tegra210_usb2_phy_set_mode(struct phy *phy, enum phy_mode mode,
>>   		}
>>   	}
>>   
>> +	port->role = submode;
>>   	mutex_unlock(&padctl->lock);
>>   
>>   	return err;
>> diff --git a/drivers/phy/tegra/xusb.h b/drivers/phy/tegra/xusb.h
>> index d2b5f9565132..273af147dfd3 100644
>> --- a/drivers/phy/tegra/xusb.h
>> +++ b/drivers/phy/tegra/xusb.h
>> @@ -317,6 +317,7 @@ struct tegra_xusb_usb2_port {
>>   	enum usb_dr_mode mode;
>>   	bool internal;
>>   	int usb3_port_fake;
>> +	enum usb_role role;
>>   };
> 
> Jonathan can we get some t-b for these two patches
> 

Re: [PATCH 2/5] phy: tegra: xusb: Fix USB2 port regulator disable logic

[Jon Hunter]

On 04/12/2025 21:27, Diogo Ivo wrote:
> The USB2 PHY mode handling on Tegra210 incorrectly relied on
> regulator_is_enabled() when determining whether the VBUS supply should
> be disabled during role changes. This is because regulator_is_enabled()
> reports exactly what is states and not if there is an unbalanced number
> of calls between regulator_enable() and regulator_disable(). For
> example, regulator_is_enabled() always reports true on a fixed-regulator
> with no enable gpio, which is the case on the Pixel C.
> 
> This then leads to the PHY driver wrongfully calling regulator_disable()
> when transitioning from USB_ROLE_DEVICE to USB_ROLE_NONE since the driver
> did not previously call the corresponding regulator_enable().
> 
> Fix this by keeping track of the current role and updating the logic to
> disable the regulator only when the previous role was USB_ROLE_HOST.
> 
> While at it fix a small typo in a comment.
> 
> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
> ---
>   drivers/phy/tegra/xusb-tegra210.c | 5 +++--
>   drivers/phy/tegra/xusb.h          | 1 +
>   2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/phy/tegra/xusb-tegra210.c b/drivers/phy/tegra/xusb-tegra210.c
> index 3409924498e9..63ad57d95514 100644
> --- a/drivers/phy/tegra/xusb-tegra210.c
> +++ b/drivers/phy/tegra/xusb-tegra210.c
> @@ -1934,9 +1934,9 @@ static int tegra210_usb2_phy_set_mode(struct phy *phy, enum phy_mode mode,
>   			/*
>   			 * When port is peripheral only or role transitions to
>   			 * USB_ROLE_NONE from USB_ROLE_DEVICE, regulator is not
> -			 * be enabled.
> +			 * enabled.
>   			 */
> -			if (regulator_is_enabled(port->supply))
> +			if (port->role == USB_ROLE_HOST)
>   				regulator_disable(port->supply);
>   
>   			tegra210_xusb_padctl_id_override(padctl, false);
> @@ -1944,6 +1944,7 @@ static int tegra210_usb2_phy_set_mode(struct phy *phy, enum phy_mode mode,
>   		}
>   	}
>   
> +	port->role = submode;
>   	mutex_unlock(&padctl->lock);
>   
>   	return err;
> diff --git a/drivers/phy/tegra/xusb.h b/drivers/phy/tegra/xusb.h
> index d2b5f9565132..273af147dfd3 100644
> --- a/drivers/phy/tegra/xusb.h
> +++ b/drivers/phy/tegra/xusb.h
> @@ -317,6 +317,7 @@ struct tegra_xusb_usb2_port {
>   	enum usb_dr_mode mode;
>   	bool internal;
>   	int usb3_port_fake;
> +	enum usb_role role;
>   };


A similar fix was made to the Tegra186 code by commit cefc1caee9dd 
("phy: tegra: xusb: Fix unbalanced regulator disable in UTMI PHY mode"). 
Although the above looks simpler, I am wondering if we should make a 
similar change to the Tegra210 code so that they both are implemented in 
the same way?

Jon
-- 
nvpublic

Re: [PATCH 2/5] phy: tegra: xusb: Fix USB2 port regulator disable logic

[Diogo Ivo]

On 1/13/26 12:01, Jon Hunter wrote:
> 
> On 04/12/2025 21:27, Diogo Ivo wrote:
>> The USB2 PHY mode handling on Tegra210 incorrectly relied on
>> regulator_is_enabled() when determining whether the VBUS supply should
>> be disabled during role changes. This is because regulator_is_enabled()
>> reports exactly what is states and not if there is an unbalanced number
>> of calls between regulator_enable() and regulator_disable(). For
>> example, regulator_is_enabled() always reports true on a fixed-regulator
>> with no enable gpio, which is the case on the Pixel C.
>>
>> This then leads to the PHY driver wrongfully calling regulator_disable()
>> when transitioning from USB_ROLE_DEVICE to USB_ROLE_NONE since the driver
>> did not previously call the corresponding regulator_enable().
>>
>> Fix this by keeping track of the current role and updating the logic to
>> disable the regulator only when the previous role was USB_ROLE_HOST.
>>
>> While at it fix a small typo in a comment.
>>
>> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
>> ---
>>   drivers/phy/tegra/xusb-tegra210.c | 5 +++--
>>   drivers/phy/tegra/xusb.h          | 1 +
>>   2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/phy/tegra/xusb-tegra210.c b/drivers/phy/tegra/ 
>> xusb-tegra210.c
>> index 3409924498e9..63ad57d95514 100644
>> --- a/drivers/phy/tegra/xusb-tegra210.c
>> +++ b/drivers/phy/tegra/xusb-tegra210.c
>> @@ -1934,9 +1934,9 @@ static int tegra210_usb2_phy_set_mode(struct phy 
>> *phy, enum phy_mode mode,
>>               /*
>>                * When port is peripheral only or role transitions to
>>                * USB_ROLE_NONE from USB_ROLE_DEVICE, regulator is not
>> -             * be enabled.
>> +             * enabled.
>>                */
>> -            if (regulator_is_enabled(port->supply))
>> +            if (port->role == USB_ROLE_HOST)
>>                   regulator_disable(port->supply);
>>               tegra210_xusb_padctl_id_override(padctl, false);
>> @@ -1944,6 +1944,7 @@ static int tegra210_usb2_phy_set_mode(struct phy 
>> *phy, enum phy_mode mode,
>>           }
>>       }
>> +    port->role = submode;
>>       mutex_unlock(&padctl->lock);
>>       return err;
>> diff --git a/drivers/phy/tegra/xusb.h b/drivers/phy/tegra/xusb.h
>> index d2b5f9565132..273af147dfd3 100644
>> --- a/drivers/phy/tegra/xusb.h
>> +++ b/drivers/phy/tegra/xusb.h
>> @@ -317,6 +317,7 @@ struct tegra_xusb_usb2_port {
>>       enum usb_dr_mode mode;
>>       bool internal;
>>       int usb3_port_fake;
>> +    enum usb_role role;
>>   };
> 
> 
> A similar fix was made to the Tegra186 code by commit cefc1caee9dd 
> ("phy: tegra: xusb: Fix unbalanced regulator disable in UTMI PHY mode"). 
> Although the above looks simpler, I am wondering if we should make a 
> similar change to the Tegra210 code so that they both are implemented in 
> the same way?

Looking at cefc1caee9dd my approach leads to less changes but I do agree
that standardization benefits us here. However in that case I think we
can take it a step further and actually just have a single function
tegra_xusb_padctl_id_override() (and likewise for vbus_override() and
set_mode()) since they all seem to do the same thing in both platforms.

If not I can simply make this patch look like cefc1caee9dd. Let me know
what you think!

Diogo

> Jon

Re: [PATCH 2/5] phy: tegra: xusb: Fix USB2 port regulator disable logic

[Jon Hunter]

On 13/01/2026 13:59, Diogo Ivo wrote:
> 
> 
> On 1/13/26 12:01, Jon Hunter wrote:
>>
>> On 04/12/2025 21:27, Diogo Ivo wrote:
>>> The USB2 PHY mode handling on Tegra210 incorrectly relied on
>>> regulator_is_enabled() when determining whether the VBUS supply should
>>> be disabled during role changes. This is because regulator_is_enabled()
>>> reports exactly what is states and not if there is an unbalanced number
>>> of calls between regulator_enable() and regulator_disable(). For
>>> example, regulator_is_enabled() always reports true on a fixed-regulator
>>> with no enable gpio, which is the case on the Pixel C.
>>>
>>> This then leads to the PHY driver wrongfully calling regulator_disable()
>>> when transitioning from USB_ROLE_DEVICE to USB_ROLE_NONE since the 
>>> driver
>>> did not previously call the corresponding regulator_enable().
>>>
>>> Fix this by keeping track of the current role and updating the logic to
>>> disable the regulator only when the previous role was USB_ROLE_HOST.
>>>
>>> While at it fix a small typo in a comment.
>>>
>>> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
>>> ---
>>>   drivers/phy/tegra/xusb-tegra210.c | 5 +++--
>>>   drivers/phy/tegra/xusb.h          | 1 +
>>>   2 files changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/phy/tegra/xusb-tegra210.c b/drivers/phy/tegra/ 
>>> xusb-tegra210.c
>>> index 3409924498e9..63ad57d95514 100644
>>> --- a/drivers/phy/tegra/xusb-tegra210.c
>>> +++ b/drivers/phy/tegra/xusb-tegra210.c
>>> @@ -1934,9 +1934,9 @@ static int tegra210_usb2_phy_set_mode(struct 
>>> phy *phy, enum phy_mode mode,
>>>               /*
>>>                * When port is peripheral only or role transitions to
>>>                * USB_ROLE_NONE from USB_ROLE_DEVICE, regulator is not
>>> -             * be enabled.
>>> +             * enabled.
>>>                */
>>> -            if (regulator_is_enabled(port->supply))
>>> +            if (port->role == USB_ROLE_HOST)
>>>                   regulator_disable(port->supply);
>>>               tegra210_xusb_padctl_id_override(padctl, false);
>>> @@ -1944,6 +1944,7 @@ static int tegra210_usb2_phy_set_mode(struct 
>>> phy *phy, enum phy_mode mode,
>>>           }
>>>       }
>>> +    port->role = submode;
>>>       mutex_unlock(&padctl->lock);
>>>       return err;
>>> diff --git a/drivers/phy/tegra/xusb.h b/drivers/phy/tegra/xusb.h
>>> index d2b5f9565132..273af147dfd3 100644
>>> --- a/drivers/phy/tegra/xusb.h
>>> +++ b/drivers/phy/tegra/xusb.h
>>> @@ -317,6 +317,7 @@ struct tegra_xusb_usb2_port {
>>>       enum usb_dr_mode mode;
>>>       bool internal;
>>>       int usb3_port_fake;
>>> +    enum usb_role role;
>>>   };
>>
>>
>> A similar fix was made to the Tegra186 code by commit cefc1caee9dd 
>> ("phy: tegra: xusb: Fix unbalanced regulator disable in UTMI PHY 
>> mode"). Although the above looks simpler, I am wondering if we should 
>> make a similar change to the Tegra210 code so that they both are 
>> implemented in the same way?
> 
> Looking at cefc1caee9dd my approach leads to less changes but I do agree
> that standardization benefits us here. However in that case I think we
> can take it a step further and actually just have a single function
> tegra_xusb_padctl_id_override() (and likewise for vbus_override() and
> set_mode()) since they all seem to do the same thing in both platforms.

Yes I think that would be fine. I can't say I have looked at that in 
detail but that would seem like the logical way to go.

Jon

-- 
nvpublic

Re: [PATCH 2/5] phy: tegra: xusb: Fix USB2 port regulator disable logic

[Diogo Ivo]

On 1/13/26 14:42, Jon Hunter wrote:
> 
> On 13/01/2026 13:59, Diogo Ivo wrote:
>>
>>
>> On 1/13/26 12:01, Jon Hunter wrote:
>>>
>>> On 04/12/2025 21:27, Diogo Ivo wrote:
>>>> The USB2 PHY mode handling on Tegra210 incorrectly relied on
>>>> regulator_is_enabled() when determining whether the VBUS supply should
>>>> be disabled during role changes. This is because regulator_is_enabled()
>>>> reports exactly what is states and not if there is an unbalanced number
>>>> of calls between regulator_enable() and regulator_disable(). For
>>>> example, regulator_is_enabled() always reports true on a fixed- 
>>>> regulator
>>>> with no enable gpio, which is the case on the Pixel C.
>>>>
>>>> This then leads to the PHY driver wrongfully calling 
>>>> regulator_disable()
>>>> when transitioning from USB_ROLE_DEVICE to USB_ROLE_NONE since the 
>>>> driver
>>>> did not previously call the corresponding regulator_enable().
>>>>
>>>> Fix this by keeping track of the current role and updating the logic to
>>>> disable the regulator only when the previous role was USB_ROLE_HOST.
>>>>
>>>> While at it fix a small typo in a comment.
>>>>
>>>> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
>>>> ---
>>>>   drivers/phy/tegra/xusb-tegra210.c | 5 +++--
>>>>   drivers/phy/tegra/xusb.h          | 1 +
>>>>   2 files changed, 4 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/phy/tegra/xusb-tegra210.c b/drivers/phy/tegra/ 
>>>> xusb-tegra210.c
>>>> index 3409924498e9..63ad57d95514 100644
>>>> --- a/drivers/phy/tegra/xusb-tegra210.c
>>>> +++ b/drivers/phy/tegra/xusb-tegra210.c
>>>> @@ -1934,9 +1934,9 @@ static int tegra210_usb2_phy_set_mode(struct 
>>>> phy *phy, enum phy_mode mode,
>>>>               /*
>>>>                * When port is peripheral only or role transitions to
>>>>                * USB_ROLE_NONE from USB_ROLE_DEVICE, regulator is not
>>>> -             * be enabled.
>>>> +             * enabled.
>>>>                */
>>>> -            if (regulator_is_enabled(port->supply))
>>>> +            if (port->role == USB_ROLE_HOST)
>>>>                   regulator_disable(port->supply);
>>>>               tegra210_xusb_padctl_id_override(padctl, false);
>>>> @@ -1944,6 +1944,7 @@ static int tegra210_usb2_phy_set_mode(struct 
>>>> phy *phy, enum phy_mode mode,
>>>>           }
>>>>       }
>>>> +    port->role = submode;
>>>>       mutex_unlock(&padctl->lock);
>>>>       return err;
>>>> diff --git a/drivers/phy/tegra/xusb.h b/drivers/phy/tegra/xusb.h
>>>> index d2b5f9565132..273af147dfd3 100644
>>>> --- a/drivers/phy/tegra/xusb.h
>>>> +++ b/drivers/phy/tegra/xusb.h
>>>> @@ -317,6 +317,7 @@ struct tegra_xusb_usb2_port {
>>>>       enum usb_dr_mode mode;
>>>>       bool internal;
>>>>       int usb3_port_fake;
>>>> +    enum usb_role role;
>>>>   };
>>>
>>>
>>> A similar fix was made to the Tegra186 code by commit cefc1caee9dd 
>>> ("phy: tegra: xusb: Fix unbalanced regulator disable in UTMI PHY 
>>> mode"). Although the above looks simpler, I am wondering if we should 
>>> make a similar change to the Tegra210 code so that they both are 
>>> implemented in the same way?
>>
>> Looking at cefc1caee9dd my approach leads to less changes but I do agree
>> that standardization benefits us here. However in that case I think we
>> can take it a step further and actually just have a single function
>> tegra_xusb_padctl_id_override() (and likewise for vbus_override() and
>> set_mode()) since they all seem to do the same thing in both platforms.
> 
> Yes I think that would be fine. I can't say I have looked at that in 
> detail but that would seem like the logical way to go.

Ok, then I'll do just that.

> Jon

[PATCH 3/5] phy: tegra: xusb: Fix ordering issue when switching roles on USB2 ports

[Diogo Ivo]

The current implementation of USB2 role switching on Tegra relies on
whichever the previous USB controller driver was using the PHY to first
"yield" it back to USB_ROLE_NONE before the next controller configures
it for the new role. However, no mechanism to guarantee this ordering
was implemented, and currently, in the general case, the configuration
functions tegra_xhci_id_work() and tegra_xudc_usb_role_sw_work() end up
running in the same order regardless of the transition being HOST->DEVICE
or DEVICE->HOST, leading to one of these transitions ending up in a
non-working state due to the new configuration being clobbered by the
previous controller driver setting USB_ROLE_NONE after the fact.

Fix this by introducing a helper that waits for the USB2 port’s current
role to become USB_ROLE_NONE and add it in the configuration functions
above before setting the role to either USB_ROLE_HOST or
USB_ROLE_DEVICE. The specific parameters of the helper function are
choices that seem reasonable in my testing and have no other basis.

This was tested on a Tegra210 platform (Smaug). However, due to the similar
approach in Tegra186 it is likely that not only this problem exists there
but that this patch also fixes it.

Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
---
 drivers/phy/tegra/xusb.c            | 23 +++++++++++++++++++++++
 drivers/usb/gadget/udc/tegra-xudc.c |  4 ++++
 drivers/usb/host/xhci-tegra.c       | 15 ++++++++++-----
 include/linux/phy/tegra/xusb.h      |  1 +
 4 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
index c89df95aa6ca..e05c3f2d1421 100644
--- a/drivers/phy/tegra/xusb.c
+++ b/drivers/phy/tegra/xusb.c
@@ -740,6 +740,29 @@ static void tegra_xusb_parse_usb_role_default_mode(struct tegra_xusb_port *port)
 	}
 }
 
+bool tegra_xusb_usb2_port_wait_role_none(struct tegra_xusb_padctl *padctl, int index)
+{
+	struct tegra_xusb_usb2_port *usb2 = tegra_xusb_find_usb2_port(padctl,
+								      index);
+	int retries = 5;
+
+	if (!usb2) {
+		dev_err(&usb2->base.dev, "no port found for USB2 lane %u\n", index);
+		return false;
+	}
+
+	do {
+		if (usb2->role == USB_ROLE_NONE)
+			return true;
+
+		usleep_range(50, 60);
+	} while (retries--);
+
+	dev_err(&usb2->base.dev, "timed out waiting for USB_ROLE_NONE");
+
+	return false;
+}
+
 static int tegra_xusb_usb2_port_parse_dt(struct tegra_xusb_usb2_port *usb2)
 {
 	struct tegra_xusb_port *port = &usb2->base;
diff --git a/drivers/usb/gadget/udc/tegra-xudc.c b/drivers/usb/gadget/udc/tegra-xudc.c
index 0c38fc37b6e6..72d725659e5f 100644
--- a/drivers/usb/gadget/udc/tegra-xudc.c
+++ b/drivers/usb/gadget/udc/tegra-xudc.c
@@ -698,8 +698,12 @@ static void tegra_xudc_restore_port_speed(struct tegra_xudc *xudc)
 
 static void tegra_xudc_device_mode_on(struct tegra_xudc *xudc)
 {
+	int port = tegra_xusb_padctl_get_port_number(xudc->curr_utmi_phy);
 	int err;
 
+	if (!tegra_xusb_usb2_port_wait_role_none(xudc->padctl, port))
+		return;
+
 	pm_runtime_get_sync(xudc->dev);
 
 	tegra_phy_xusb_utmi_pad_power_on(xudc->curr_utmi_phy);
diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
index 9c69fccdc6e8..9944593166a3 100644
--- a/drivers/usb/host/xhci-tegra.c
+++ b/drivers/usb/host/xhci-tegra.c
@@ -1352,18 +1352,23 @@ static void tegra_xhci_id_work(struct work_struct *work)
 	struct tegra_xusb_mbox_msg msg;
 	struct phy *phy = tegra_xusb_get_phy(tegra, "usb2",
 						    tegra->otg_usb2_port);
+	enum usb_role role = USB_ROLE_NONE;
 	u32 status;
 	int ret;
 
 	dev_dbg(tegra->dev, "host mode %s\n", str_on_off(tegra->host_mode));
 
-	mutex_lock(&tegra->lock);
 
-	if (tegra->host_mode)
-		phy_set_mode_ext(phy, PHY_MODE_USB_OTG, USB_ROLE_HOST);
-	else
-		phy_set_mode_ext(phy, PHY_MODE_USB_OTG, USB_ROLE_NONE);
+	if (tegra->host_mode) {
+		if (!tegra_xusb_usb2_port_wait_role_none(tegra->padctl,
+							 tegra->otg_usb2_port))
+			return;
 
+		role = USB_ROLE_HOST;
+	}
+
+	mutex_lock(&tegra->lock);
+	phy_set_mode_ext(phy, PHY_MODE_USB_OTG, role);
 	mutex_unlock(&tegra->lock);
 
 	tegra->otg_usb3_port = tegra_xusb_padctl_get_usb3_companion(tegra->padctl,
diff --git a/include/linux/phy/tegra/xusb.h b/include/linux/phy/tegra/xusb.h
index 6ca51e0080ec..a0d3d5b7cf33 100644
--- a/include/linux/phy/tegra/xusb.h
+++ b/include/linux/phy/tegra/xusb.h
@@ -33,5 +33,6 @@ int tegra_xusb_padctl_disable_phy_sleepwalk(struct tegra_xusb_padctl *padctl, st
 int tegra_xusb_padctl_enable_phy_wake(struct tegra_xusb_padctl *padctl, struct phy *phy);
 int tegra_xusb_padctl_disable_phy_wake(struct tegra_xusb_padctl *padctl, struct phy *phy);
 bool tegra_xusb_padctl_remote_wake_detected(struct tegra_xusb_padctl *padctl, struct phy *phy);
+bool tegra_xusb_usb2_port_wait_role_none(struct tegra_xusb_padctl *padctl, int index);
 
 #endif /* PHY_TEGRA_XUSB_H */

-- 
2.52.0

Re: [PATCH 3/5] phy: tegra: xusb: Fix ordering issue when switching roles on USB2 ports

[Jon Hunter]

On 04/12/2025 21:27, Diogo Ivo wrote:
> The current implementation of USB2 role switching on Tegra relies on
> whichever the previous USB controller driver was using the PHY to first
> "yield" it back to USB_ROLE_NONE before the next controller configures
> it for the new role. However, no mechanism to guarantee this ordering
> was implemented, and currently, in the general case, the configuration
> functions tegra_xhci_id_work() and tegra_xudc_usb_role_sw_work() end up
> running in the same order regardless of the transition being HOST->DEVICE
> or DEVICE->HOST, leading to one of these transitions ending up in a
> non-working state due to the new configuration being clobbered by the
> previous controller driver setting USB_ROLE_NONE after the fact.
> 
> Fix this by introducing a helper that waits for the USB2 port’s current
> role to become USB_ROLE_NONE and add it in the configuration functions
> above before setting the role to either USB_ROLE_HOST or
> USB_ROLE_DEVICE. The specific parameters of the helper function are
> choices that seem reasonable in my testing and have no other basis.
> 
> This was tested on a Tegra210 platform (Smaug). However, due to the similar
> approach in Tegra186 it is likely that not only this problem exists there
> but that this patch also fixes it.
> 
> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
> ---
>   drivers/phy/tegra/xusb.c            | 23 +++++++++++++++++++++++
>   drivers/usb/gadget/udc/tegra-xudc.c |  4 ++++
>   drivers/usb/host/xhci-tegra.c       | 15 ++++++++++-----
>   include/linux/phy/tegra/xusb.h      |  1 +
>   4 files changed, 38 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
> index c89df95aa6ca..e05c3f2d1421 100644
> --- a/drivers/phy/tegra/xusb.c
> +++ b/drivers/phy/tegra/xusb.c
> @@ -740,6 +740,29 @@ static void tegra_xusb_parse_usb_role_default_mode(struct tegra_xusb_port *port)
>   	}
>   }
>   
> +bool tegra_xusb_usb2_port_wait_role_none(struct tegra_xusb_padctl *padctl, int index)
> +{
> +	struct tegra_xusb_usb2_port *usb2 = tegra_xusb_find_usb2_port(padctl,
> +								      index);
> +	int retries = 5;
> +
> +	if (!usb2) {
> +		dev_err(&usb2->base.dev, "no port found for USB2 lane %u\n", index);
> +		return false;
> +	}
> +
> +	do {
> +		if (usb2->role == USB_ROLE_NONE)
> +			return true;
> +
> +		usleep_range(50, 60);
> +	} while (retries--);
> +
> +	dev_err(&usb2->base.dev, "timed out waiting for USB_ROLE_NONE");
> +
> +	return false;
> +}


This patch is causing the following build error today with -next ...

   MODPOST Module.symvers
ERROR: modpost: "tegra_xusb_usb2_port_wait_role_none" 
[drivers/usb/host/xhci-tegra.ko] undefined!

The above function symbol needs to be exported.

Jon

-- 
nvpublic

Re: [PATCH 3/5] phy: tegra: xusb: Fix ordering issue when switching roles on USB2 ports

[Jon Hunter]

On 13/01/2026 11:35, Jon Hunter wrote:
> 
> On 04/12/2025 21:27, Diogo Ivo wrote:
>> The current implementation of USB2 role switching on Tegra relies on
>> whichever the previous USB controller driver was using the PHY to first
>> "yield" it back to USB_ROLE_NONE before the next controller configures
>> it for the new role. However, no mechanism to guarantee this ordering
>> was implemented, and currently, in the general case, the configuration
>> functions tegra_xhci_id_work() and tegra_xudc_usb_role_sw_work() end up
>> running in the same order regardless of the transition being HOST->DEVICE
>> or DEVICE->HOST, leading to one of these transitions ending up in a
>> non-working state due to the new configuration being clobbered by the
>> previous controller driver setting USB_ROLE_NONE after the fact.
>>
>> Fix this by introducing a helper that waits for the USB2 port’s current
>> role to become USB_ROLE_NONE and add it in the configuration functions
>> above before setting the role to either USB_ROLE_HOST or
>> USB_ROLE_DEVICE. The specific parameters of the helper function are
>> choices that seem reasonable in my testing and have no other basis.
>>
>> This was tested on a Tegra210 platform (Smaug). However, due to the 
>> similar
>> approach in Tegra186 it is likely that not only this problem exists there
>> but that this patch also fixes it.
>>
>> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
>> ---
>>   drivers/phy/tegra/xusb.c            | 23 +++++++++++++++++++++++
>>   drivers/usb/gadget/udc/tegra-xudc.c |  4 ++++
>>   drivers/usb/host/xhci-tegra.c       | 15 ++++++++++-----
>>   include/linux/phy/tegra/xusb.h      |  1 +
>>   4 files changed, 38 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
>> index c89df95aa6ca..e05c3f2d1421 100644
>> --- a/drivers/phy/tegra/xusb.c
>> +++ b/drivers/phy/tegra/xusb.c
>> @@ -740,6 +740,29 @@ static void 
>> tegra_xusb_parse_usb_role_default_mode(struct tegra_xusb_port *port)
>>       }
>>   }
>> +bool tegra_xusb_usb2_port_wait_role_none(struct tegra_xusb_padctl 
>> *padctl, int index)
>> +{
>> +    struct tegra_xusb_usb2_port *usb2 = 
>> tegra_xusb_find_usb2_port(padctl,
>> +                                      index);
>> +    int retries = 5;
>> +
>> +    if (!usb2) {
>> +        dev_err(&usb2->base.dev, "no port found for USB2 lane %u\n", 
>> index);
>> +        return false;
>> +    }
>> +
>> +    do {
>> +        if (usb2->role == USB_ROLE_NONE)
>> +            return true;
>> +
>> +        usleep_range(50, 60);
>> +    } while (retries--);
>> +
>> +    dev_err(&usb2->base.dev, "timed out waiting for USB_ROLE_NONE");
>> +
>> +    return false;
>> +}
> 
> 
> This patch is causing the following build error today with -next ...

Sorry this is not in -next, I had just applied locally on top!

>    MODPOST Module.symvers
> ERROR: modpost: "tegra_xusb_usb2_port_wait_role_none" [drivers/usb/host/ 
> xhci-tegra.ko] undefined!
> 
> The above function symbol needs to be exported.

Nonetheless this needs to be fixed.

Jon

-- 
nvpublic

Re: [PATCH 3/5] phy: tegra: xusb: Fix ordering issue when switching roles on USB2 ports

[Diogo Ivo]

On 1/13/26 11:44, Jon Hunter wrote:
> 
> On 13/01/2026 11:35, Jon Hunter wrote:
>>
>> On 04/12/2025 21:27, Diogo Ivo wrote:
>>> The current implementation of USB2 role switching on Tegra relies on
>>> whichever the previous USB controller driver was using the PHY to first
>>> "yield" it back to USB_ROLE_NONE before the next controller configures
>>> it for the new role. However, no mechanism to guarantee this ordering
>>> was implemented, and currently, in the general case, the configuration
>>> functions tegra_xhci_id_work() and tegra_xudc_usb_role_sw_work() end up
>>> running in the same order regardless of the transition being HOST- 
>>> >DEVICE
>>> or DEVICE->HOST, leading to one of these transitions ending up in a
>>> non-working state due to the new configuration being clobbered by the
>>> previous controller driver setting USB_ROLE_NONE after the fact.
>>>
>>> Fix this by introducing a helper that waits for the USB2 port’s current
>>> role to become USB_ROLE_NONE and add it in the configuration functions
>>> above before setting the role to either USB_ROLE_HOST or
>>> USB_ROLE_DEVICE. The specific parameters of the helper function are
>>> choices that seem reasonable in my testing and have no other basis.
>>>
>>> This was tested on a Tegra210 platform (Smaug). However, due to the 
>>> similar
>>> approach in Tegra186 it is likely that not only this problem exists 
>>> there
>>> but that this patch also fixes it.
>>>
>>> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
>>> ---
>>>   drivers/phy/tegra/xusb.c            | 23 +++++++++++++++++++++++
>>>   drivers/usb/gadget/udc/tegra-xudc.c |  4 ++++
>>>   drivers/usb/host/xhci-tegra.c       | 15 ++++++++++-----
>>>   include/linux/phy/tegra/xusb.h      |  1 +
>>>   4 files changed, 38 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
>>> index c89df95aa6ca..e05c3f2d1421 100644
>>> --- a/drivers/phy/tegra/xusb.c
>>> +++ b/drivers/phy/tegra/xusb.c
>>> @@ -740,6 +740,29 @@ static void 
>>> tegra_xusb_parse_usb_role_default_mode(struct tegra_xusb_port *port)
>>>       }
>>>   }
>>> +bool tegra_xusb_usb2_port_wait_role_none(struct tegra_xusb_padctl 
>>> *padctl, int index)
>>> +{
>>> +    struct tegra_xusb_usb2_port *usb2 = 
>>> tegra_xusb_find_usb2_port(padctl,
>>> +                                      index);
>>> +    int retries = 5;
>>> +
>>> +    if (!usb2) {
>>> +        dev_err(&usb2->base.dev, "no port found for USB2 lane %u\n", 
>>> index);
>>> +        return false;
>>> +    }
>>> +
>>> +    do {
>>> +        if (usb2->role == USB_ROLE_NONE)
>>> +            return true;
>>> +
>>> +        usleep_range(50, 60);
>>> +    } while (retries--);
>>> +
>>> +    dev_err(&usb2->base.dev, "timed out waiting for USB_ROLE_NONE");
>>> +
>>> +    return false;
>>> +}
>>
>>
>> This patch is causing the following build error today with -next ...
> 
> Sorry this is not in -next, I had just applied locally on top!
> 
>>    MODPOST Module.symvers
>> ERROR: modpost: "tegra_xusb_usb2_port_wait_role_none" [drivers/usb/ 
>> host/ xhci-tegra.ko] undefined!
>>
>> The above function symbol needs to be exported.

You're right, thanks for catching this. I'll fix it in v2.

> 
> Nonetheless this needs to be fixed.
> 
> Jon
> 

Re: [PATCH 3/5] phy: tegra: xusb: Fix ordering issue when switching roles on USB2 ports

[Jon Hunter]

On 04/12/2025 21:27, Diogo Ivo wrote:
> The current implementation of USB2 role switching on Tegra relies on
> whichever the previous USB controller driver was using the PHY to first
> "yield" it back to USB_ROLE_NONE before the next controller configures
> it for the new role. However, no mechanism to guarantee this ordering
> was implemented, and currently, in the general case, the configuration
> functions tegra_xhci_id_work() and tegra_xudc_usb_role_sw_work() end up
> running in the same order regardless of the transition being HOST->DEVICE
> or DEVICE->HOST, leading to one of these transitions ending up in a
> non-working state due to the new configuration being clobbered by the
> previous controller driver setting USB_ROLE_NONE after the fact.
> 
> Fix this by introducing a helper that waits for the USB2 port’s current
> role to become USB_ROLE_NONE and add it in the configuration functions
> above before setting the role to either USB_ROLE_HOST or
> USB_ROLE_DEVICE. The specific parameters of the helper function are
> choices that seem reasonable in my testing and have no other basis.

This is no information here about why 6 * 50/60us is deemed to be 
sufficient? May be it is, but a comment would be nice.

> This was tested on a Tegra210 platform (Smaug). However, due to the similar
> approach in Tegra186 it is likely that not only this problem exists there
> but that this patch also fixes it.
> 
> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
> ---
>   drivers/phy/tegra/xusb.c            | 23 +++++++++++++++++++++++
>   drivers/usb/gadget/udc/tegra-xudc.c |  4 ++++
>   drivers/usb/host/xhci-tegra.c       | 15 ++++++++++-----
>   include/linux/phy/tegra/xusb.h      |  1 +
>   4 files changed, 38 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
> index c89df95aa6ca..e05c3f2d1421 100644
> --- a/drivers/phy/tegra/xusb.c
> +++ b/drivers/phy/tegra/xusb.c
> @@ -740,6 +740,29 @@ static void tegra_xusb_parse_usb_role_default_mode(struct tegra_xusb_port *port)
>   	}
>   }
>   
> +bool tegra_xusb_usb2_port_wait_role_none(struct tegra_xusb_padctl *padctl, int index)
> +{
> +	struct tegra_xusb_usb2_port *usb2 = tegra_xusb_find_usb2_port(padctl,
> +								      index);
> +	int retries = 5;
> +
> +	if (!usb2) {
> +		dev_err(&usb2->base.dev, "no port found for USB2 lane %u\n", index);

This appears to be a bug. If !usb2 then dereference usb2->base anyway.


> +		return false;
> +	}
> +
> +	do {
> +		if (usb2->role == USB_ROLE_NONE)
> +			return true;
> +
> +		usleep_range(50, 60);
> +	} while (retries--);
> +
> +	dev_err(&usb2->base.dev, "timed out waiting for USB_ROLE_NONE");
> +
> +	return false;
> +}
> +
>   static int tegra_xusb_usb2_port_parse_dt(struct tegra_xusb_usb2_port *usb2)
>   {
>   	struct tegra_xusb_port *port = &usb2->base;
> diff --git a/drivers/usb/gadget/udc/tegra-xudc.c b/drivers/usb/gadget/udc/tegra-xudc.c
> index 0c38fc37b6e6..72d725659e5f 100644
> --- a/drivers/usb/gadget/udc/tegra-xudc.c
> +++ b/drivers/usb/gadget/udc/tegra-xudc.c
> @@ -698,8 +698,12 @@ static void tegra_xudc_restore_port_speed(struct tegra_xudc *xudc)
>   
>   static void tegra_xudc_device_mode_on(struct tegra_xudc *xudc)
>   {
> +	int port = tegra_xusb_padctl_get_port_number(xudc->curr_utmi_phy);
>   	int err;
>   
> +	if (!tegra_xusb_usb2_port_wait_role_none(xudc->padctl, port))
> +		return;
> +
>   	pm_runtime_get_sync(xudc->dev);
>   
>   	tegra_phy_xusb_utmi_pad_power_on(xudc->curr_utmi_phy);
> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
> index 9c69fccdc6e8..9944593166a3 100644
> --- a/drivers/usb/host/xhci-tegra.c
> +++ b/drivers/usb/host/xhci-tegra.c
> @@ -1352,18 +1352,23 @@ static void tegra_xhci_id_work(struct work_struct *work)
>   	struct tegra_xusb_mbox_msg msg;
>   	struct phy *phy = tegra_xusb_get_phy(tegra, "usb2",
>   						    tegra->otg_usb2_port);
> +	enum usb_role role = USB_ROLE_NONE;
>   	u32 status;
>   	int ret;
>   
>   	dev_dbg(tegra->dev, "host mode %s\n", str_on_off(tegra->host_mode));
>   
> -	mutex_lock(&tegra->lock);
>   

Extra blank line here.

> -	if (tegra->host_mode)
> -		phy_set_mode_ext(phy, PHY_MODE_USB_OTG, USB_ROLE_HOST);
> -	else
> -		phy_set_mode_ext(phy, PHY_MODE_USB_OTG, USB_ROLE_NONE);
> +	if (tegra->host_mode) {
> +		if (!tegra_xusb_usb2_port_wait_role_none(tegra->padctl,
> +							 tegra->otg_usb2_port))
> +			return;
>   
> +		role = USB_ROLE_HOST;
> +	}
> +
> +	mutex_lock(&tegra->lock);
> +	phy_set_mode_ext(phy, PHY_MODE_USB_OTG, role);
>   	mutex_unlock(&tegra->lock);

I am trying to understand why you opted to implement it this way around 
and not add the wait loop after setting to the mode to USB_ROLE_NONE in 
the original code all within the context of the mutex?

Thanks
Jon

-- 
nvpublic

Re: [PATCH 3/5] phy: tegra: xusb: Fix ordering issue when switching roles on USB2 ports

[Diogo Ivo]

On 1/13/26 11:56, Jon Hunter wrote:
> 
> On 04/12/2025 21:27, Diogo Ivo wrote:
>> The current implementation of USB2 role switching on Tegra relies on
>> whichever the previous USB controller driver was using the PHY to first
>> "yield" it back to USB_ROLE_NONE before the next controller configures
>> it for the new role. However, no mechanism to guarantee this ordering
>> was implemented, and currently, in the general case, the configuration
>> functions tegra_xhci_id_work() and tegra_xudc_usb_role_sw_work() end up
>> running in the same order regardless of the transition being HOST->DEVICE
>> or DEVICE->HOST, leading to one of these transitions ending up in a
>> non-working state due to the new configuration being clobbered by the
>> previous controller driver setting USB_ROLE_NONE after the fact.
>>
>> Fix this by introducing a helper that waits for the USB2 port’s current
>> role to become USB_ROLE_NONE and add it in the configuration functions
>> above before setting the role to either USB_ROLE_HOST or
>> USB_ROLE_DEVICE. The specific parameters of the helper function are
>> choices that seem reasonable in my testing and have no other basis.
> 
> This is no information here about why 6 * 50/60us is deemed to be 
> sufficient? May be it is, but a comment would be nice.
> 
>> This was tested on a Tegra210 platform (Smaug). However, due to the 
>> similar
>> approach in Tegra186 it is likely that not only this problem exists there
>> but that this patch also fixes it.
>>
>> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
>> ---
>>   drivers/phy/tegra/xusb.c            | 23 +++++++++++++++++++++++
>>   drivers/usb/gadget/udc/tegra-xudc.c |  4 ++++
>>   drivers/usb/host/xhci-tegra.c       | 15 ++++++++++-----
>>   include/linux/phy/tegra/xusb.h      |  1 +
>>   4 files changed, 38 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
>> index c89df95aa6ca..e05c3f2d1421 100644
>> --- a/drivers/phy/tegra/xusb.c
>> +++ b/drivers/phy/tegra/xusb.c
>> @@ -740,6 +740,29 @@ static void 
>> tegra_xusb_parse_usb_role_default_mode(struct tegra_xusb_port *port)
>>       }
>>   }
>> +bool tegra_xusb_usb2_port_wait_role_none(struct tegra_xusb_padctl 
>> *padctl, int index)
>> +{
>> +    struct tegra_xusb_usb2_port *usb2 = 
>> tegra_xusb_find_usb2_port(padctl,
>> +                                      index);
>> +    int retries = 5;
>> +
>> +    if (!usb2) {
>> +        dev_err(&usb2->base.dev, "no port found for USB2 lane %u\n", 
>> index);
> 
> This appears to be a bug. If !usb2 then dereference usb2->base anyway.

It is a bug, will fix in v2.

>> +        return false;
>> +    }
>> +
>> +    do {
>> +        if (usb2->role == USB_ROLE_NONE)
>> +            return true;
>> +
>> +        usleep_range(50, 60);
>> +    } while (retries--);
>> +
>> +    dev_err(&usb2->base.dev, "timed out waiting for USB_ROLE_NONE");
>> +
>> +    return false;
>> +}
>> +
>>   static int tegra_xusb_usb2_port_parse_dt(struct tegra_xusb_usb2_port 
>> *usb2)
>>   {
>>       struct tegra_xusb_port *port = &usb2->base;
>> diff --git a/drivers/usb/gadget/udc/tegra-xudc.c b/drivers/usb/gadget/ 
>> udc/tegra-xudc.c
>> index 0c38fc37b6e6..72d725659e5f 100644
>> --- a/drivers/usb/gadget/udc/tegra-xudc.c
>> +++ b/drivers/usb/gadget/udc/tegra-xudc.c
>> @@ -698,8 +698,12 @@ static void tegra_xudc_restore_port_speed(struct 
>> tegra_xudc *xudc)
>>   static void tegra_xudc_device_mode_on(struct tegra_xudc *xudc)
>>   {
>> +    int port = tegra_xusb_padctl_get_port_number(xudc->curr_utmi_phy);
>>       int err;
>> +    if (!tegra_xusb_usb2_port_wait_role_none(xudc->padctl, port))
>> +        return;
>> +
>>       pm_runtime_get_sync(xudc->dev);
>>       tegra_phy_xusb_utmi_pad_power_on(xudc->curr_utmi_phy);
>> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci- 
>> tegra.c
>> index 9c69fccdc6e8..9944593166a3 100644
>> --- a/drivers/usb/host/xhci-tegra.c
>> +++ b/drivers/usb/host/xhci-tegra.c
>> @@ -1352,18 +1352,23 @@ static void tegra_xhci_id_work(struct 
>> work_struct *work)
>>       struct tegra_xusb_mbox_msg msg;
>>       struct phy *phy = tegra_xusb_get_phy(tegra, "usb2",
>>                               tegra->otg_usb2_port);
>> +    enum usb_role role = USB_ROLE_NONE;
>>       u32 status;
>>       int ret;
>>       dev_dbg(tegra->dev, "host mode %s\n", str_on_off(tegra- 
>> >host_mode));
>> -    mutex_lock(&tegra->lock);
> 
> Extra blank line here.

Will fix in v2.

>> -    if (tegra->host_mode)
>> -        phy_set_mode_ext(phy, PHY_MODE_USB_OTG, USB_ROLE_HOST);
>> -    else
>> -        phy_set_mode_ext(phy, PHY_MODE_USB_OTG, USB_ROLE_NONE);
>> +    if (tegra->host_mode) {
>> +        if (!tegra_xusb_usb2_port_wait_role_none(tegra->padctl,
>> +                             tegra->otg_usb2_port))
>> +            return;
>> +        role = USB_ROLE_HOST;
>> +    }
>> +
>> +    mutex_lock(&tegra->lock);
>> +    phy_set_mode_ext(phy, PHY_MODE_USB_OTG, role);
>>       mutex_unlock(&tegra->lock);
> 
> I am trying to understand why you opted to implement it this way around 
> and not add the wait loop after setting to the mode to USB_ROLE_NONE in 
> the original code all within the context of the mutex?

I did that to minimize the amount of time we wait while holding the
mutex, as we can now possibly wait a significant amount of time for the
role switch. Is this an unneccessary optimization?

Thanks,
Diogo

> Thanks
> Jon

Re: [PATCH 3/5] phy: tegra: xusb: Fix ordering issue when switching roles on USB2 ports

[Jon Hunter]

On 13/01/2026 14:05, Diogo Ivo wrote:
> 
> 
> On 1/13/26 11:56, Jon Hunter wrote:
>>
>> On 04/12/2025 21:27, Diogo Ivo wrote:
>>> The current implementation of USB2 role switching on Tegra relies on
>>> whichever the previous USB controller driver was using the PHY to first
>>> "yield" it back to USB_ROLE_NONE before the next controller configures
>>> it for the new role. However, no mechanism to guarantee this ordering
>>> was implemented, and currently, in the general case, the configuration
>>> functions tegra_xhci_id_work() and tegra_xudc_usb_role_sw_work() end up
>>> running in the same order regardless of the transition being HOST- 
>>> >DEVICE
>>> or DEVICE->HOST, leading to one of these transitions ending up in a
>>> non-working state due to the new configuration being clobbered by the
>>> previous controller driver setting USB_ROLE_NONE after the fact.
>>>
>>> Fix this by introducing a helper that waits for the USB2 port’s current
>>> role to become USB_ROLE_NONE and add it in the configuration functions
>>> above before setting the role to either USB_ROLE_HOST or
>>> USB_ROLE_DEVICE. The specific parameters of the helper function are
>>> choices that seem reasonable in my testing and have no other basis.
>>
>> This is no information here about why 6 * 50/60us is deemed to be 
>> sufficient? May be it is, but a comment would be nice.
>>
>>> This was tested on a Tegra210 platform (Smaug). However, due to the 
>>> similar
>>> approach in Tegra186 it is likely that not only this problem exists 
>>> there
>>> but that this patch also fixes it.
>>>
>>> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
>>> ---
>>>   drivers/phy/tegra/xusb.c            | 23 +++++++++++++++++++++++
>>>   drivers/usb/gadget/udc/tegra-xudc.c |  4 ++++
>>>   drivers/usb/host/xhci-tegra.c       | 15 ++++++++++-----
>>>   include/linux/phy/tegra/xusb.h      |  1 +
>>>   4 files changed, 38 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
>>> index c89df95aa6ca..e05c3f2d1421 100644
>>> --- a/drivers/phy/tegra/xusb.c
>>> +++ b/drivers/phy/tegra/xusb.c
>>> @@ -740,6 +740,29 @@ static void 
>>> tegra_xusb_parse_usb_role_default_mode(struct tegra_xusb_port *port)
>>>       }
>>>   }
>>> +bool tegra_xusb_usb2_port_wait_role_none(struct tegra_xusb_padctl 
>>> *padctl, int index)
>>> +{
>>> +    struct tegra_xusb_usb2_port *usb2 = 
>>> tegra_xusb_find_usb2_port(padctl,
>>> +                                      index);
>>> +    int retries = 5;
>>> +
>>> +    if (!usb2) {
>>> +        dev_err(&usb2->base.dev, "no port found for USB2 lane %u\n", 
>>> index);
>>
>> This appears to be a bug. If !usb2 then dereference usb2->base anyway.
> 
> It is a bug, will fix in v2.
> 
>>> +        return false;
>>> +    }
>>> +
>>> +    do {
>>> +        if (usb2->role == USB_ROLE_NONE)
>>> +            return true;
>>> +
>>> +        usleep_range(50, 60);
>>> +    } while (retries--);
>>> +
>>> +    dev_err(&usb2->base.dev, "timed out waiting for USB_ROLE_NONE");
>>> +
>>> +    return false;
>>> +}
>>> +
>>>   static int tegra_xusb_usb2_port_parse_dt(struct 
>>> tegra_xusb_usb2_port *usb2)
>>>   {
>>>       struct tegra_xusb_port *port = &usb2->base;
>>> diff --git a/drivers/usb/gadget/udc/tegra-xudc.c b/drivers/usb/ 
>>> gadget/ udc/tegra-xudc.c
>>> index 0c38fc37b6e6..72d725659e5f 100644
>>> --- a/drivers/usb/gadget/udc/tegra-xudc.c
>>> +++ b/drivers/usb/gadget/udc/tegra-xudc.c
>>> @@ -698,8 +698,12 @@ static void tegra_xudc_restore_port_speed(struct 
>>> tegra_xudc *xudc)
>>>   static void tegra_xudc_device_mode_on(struct tegra_xudc *xudc)
>>>   {
>>> +    int port = tegra_xusb_padctl_get_port_number(xudc->curr_utmi_phy);
>>>       int err;
>>> +    if (!tegra_xusb_usb2_port_wait_role_none(xudc->padctl, port))
>>> +        return;
>>> +
>>>       pm_runtime_get_sync(xudc->dev);
>>>       tegra_phy_xusb_utmi_pad_power_on(xudc->curr_utmi_phy);
>>> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci- 
>>> tegra.c
>>> index 9c69fccdc6e8..9944593166a3 100644
>>> --- a/drivers/usb/host/xhci-tegra.c
>>> +++ b/drivers/usb/host/xhci-tegra.c
>>> @@ -1352,18 +1352,23 @@ static void tegra_xhci_id_work(struct 
>>> work_struct *work)
>>>       struct tegra_xusb_mbox_msg msg;
>>>       struct phy *phy = tegra_xusb_get_phy(tegra, "usb2",
>>>                               tegra->otg_usb2_port);
>>> +    enum usb_role role = USB_ROLE_NONE;
>>>       u32 status;
>>>       int ret;
>>>       dev_dbg(tegra->dev, "host mode %s\n", str_on_off(tegra- 
>>> >host_mode));
>>> -    mutex_lock(&tegra->lock);
>>
>> Extra blank line here.
> 
> Will fix in v2.
> 
>>> -    if (tegra->host_mode)
>>> -        phy_set_mode_ext(phy, PHY_MODE_USB_OTG, USB_ROLE_HOST);
>>> -    else
>>> -        phy_set_mode_ext(phy, PHY_MODE_USB_OTG, USB_ROLE_NONE);
>>> +    if (tegra->host_mode) {
>>> +        if (!tegra_xusb_usb2_port_wait_role_none(tegra->padctl,
>>> +                             tegra->otg_usb2_port))
>>> +            return;
>>> +        role = USB_ROLE_HOST;
>>> +    }
>>> +
>>> +    mutex_lock(&tegra->lock);
>>> +    phy_set_mode_ext(phy, PHY_MODE_USB_OTG, role);
>>>       mutex_unlock(&tegra->lock);
>>
>> I am trying to understand why you opted to implement it this way 
>> around and not add the wait loop after setting to the mode to 
>> USB_ROLE_NONE in the original code all within the context of the mutex?
> 
> I did that to minimize the amount of time we wait while holding the
> mutex, as we can now possibly wait a significant amount of time for the
> role switch. Is this an unneccessary optimization?

Do you mean it will be longer than a few 100us?

Jon

-- 
nvpublic

Re: [PATCH 3/5] phy: tegra: xusb: Fix ordering issue when switching roles on USB2 ports

[Diogo Ivo]

On 1/13/26 14:48, Jon Hunter wrote:
> 
> On 13/01/2026 14:05, Diogo Ivo wrote:
>>
>>
>> On 1/13/26 11:56, Jon Hunter wrote:
>>>
>>> On 04/12/2025 21:27, Diogo Ivo wrote:
>>>> The current implementation of USB2 role switching on Tegra relies on
>>>> whichever the previous USB controller driver was using the PHY to first
>>>> "yield" it back to USB_ROLE_NONE before the next controller configures
>>>> it for the new role. However, no mechanism to guarantee this ordering
>>>> was implemented, and currently, in the general case, the configuration
>>>> functions tegra_xhci_id_work() and tegra_xudc_usb_role_sw_work() end up
>>>> running in the same order regardless of the transition being HOST- 
>>>> >DEVICE
>>>> or DEVICE->HOST, leading to one of these transitions ending up in a
>>>> non-working state due to the new configuration being clobbered by the
>>>> previous controller driver setting USB_ROLE_NONE after the fact.
>>>>
>>>> Fix this by introducing a helper that waits for the USB2 port’s current
>>>> role to become USB_ROLE_NONE and add it in the configuration functions
>>>> above before setting the role to either USB_ROLE_HOST or
>>>> USB_ROLE_DEVICE. The specific parameters of the helper function are
>>>> choices that seem reasonable in my testing and have no other basis.
>>>
>>> This is no information here about why 6 * 50/60us is deemed to be 
>>> sufficient? May be it is, but a comment would be nice.
>>>
>>>> This was tested on a Tegra210 platform (Smaug). However, due to the 
>>>> similar
>>>> approach in Tegra186 it is likely that not only this problem exists 
>>>> there
>>>> but that this patch also fixes it.
>>>>
>>>> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
>>>> ---
>>>>   drivers/phy/tegra/xusb.c            | 23 +++++++++++++++++++++++
>>>>   drivers/usb/gadget/udc/tegra-xudc.c |  4 ++++
>>>>   drivers/usb/host/xhci-tegra.c       | 15 ++++++++++-----
>>>>   include/linux/phy/tegra/xusb.h      |  1 +
>>>>   4 files changed, 38 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
>>>> index c89df95aa6ca..e05c3f2d1421 100644
>>>> --- a/drivers/phy/tegra/xusb.c
>>>> +++ b/drivers/phy/tegra/xusb.c
>>>> @@ -740,6 +740,29 @@ static void 
>>>> tegra_xusb_parse_usb_role_default_mode(struct tegra_xusb_port *port)
>>>>       }
>>>>   }
>>>> +bool tegra_xusb_usb2_port_wait_role_none(struct tegra_xusb_padctl 
>>>> *padctl, int index)
>>>> +{
>>>> +    struct tegra_xusb_usb2_port *usb2 = 
>>>> tegra_xusb_find_usb2_port(padctl,
>>>> +                                      index);
>>>> +    int retries = 5;
>>>> +
>>>> +    if (!usb2) {
>>>> +        dev_err(&usb2->base.dev, "no port found for USB2 lane 
>>>> %u\n", index);
>>>
>>> This appears to be a bug. If !usb2 then dereference usb2->base anyway.
>>
>> It is a bug, will fix in v2.
>>
>>>> +        return false;
>>>> +    }
>>>> +
>>>> +    do {
>>>> +        if (usb2->role == USB_ROLE_NONE)
>>>> +            return true;
>>>> +
>>>> +        usleep_range(50, 60);
>>>> +    } while (retries--);
>>>> +
>>>> +    dev_err(&usb2->base.dev, "timed out waiting for USB_ROLE_NONE");
>>>> +
>>>> +    return false;
>>>> +}
>>>> +
>>>>   static int tegra_xusb_usb2_port_parse_dt(struct 
>>>> tegra_xusb_usb2_port *usb2)
>>>>   {
>>>>       struct tegra_xusb_port *port = &usb2->base;
>>>> diff --git a/drivers/usb/gadget/udc/tegra-xudc.c b/drivers/usb/ 
>>>> gadget/ udc/tegra-xudc.c
>>>> index 0c38fc37b6e6..72d725659e5f 100644
>>>> --- a/drivers/usb/gadget/udc/tegra-xudc.c
>>>> +++ b/drivers/usb/gadget/udc/tegra-xudc.c
>>>> @@ -698,8 +698,12 @@ static void 
>>>> tegra_xudc_restore_port_speed(struct tegra_xudc *xudc)
>>>>   static void tegra_xudc_device_mode_on(struct tegra_xudc *xudc)
>>>>   {
>>>> +    int port = tegra_xusb_padctl_get_port_number(xudc->curr_utmi_phy);
>>>>       int err;
>>>> +    if (!tegra_xusb_usb2_port_wait_role_none(xudc->padctl, port))
>>>> +        return;
>>>> +
>>>>       pm_runtime_get_sync(xudc->dev);
>>>>       tegra_phy_xusb_utmi_pad_power_on(xudc->curr_utmi_phy);
>>>> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci- 
>>>> tegra.c
>>>> index 9c69fccdc6e8..9944593166a3 100644
>>>> --- a/drivers/usb/host/xhci-tegra.c
>>>> +++ b/drivers/usb/host/xhci-tegra.c
>>>> @@ -1352,18 +1352,23 @@ static void tegra_xhci_id_work(struct 
>>>> work_struct *work)
>>>>       struct tegra_xusb_mbox_msg msg;
>>>>       struct phy *phy = tegra_xusb_get_phy(tegra, "usb2",
>>>>                               tegra->otg_usb2_port);
>>>> +    enum usb_role role = USB_ROLE_NONE;
>>>>       u32 status;
>>>>       int ret;
>>>>       dev_dbg(tegra->dev, "host mode %s\n", str_on_off(tegra- 
>>>> >host_mode));
>>>> -    mutex_lock(&tegra->lock);
>>>
>>> Extra blank line here.
>>
>> Will fix in v2.
>>
>>>> -    if (tegra->host_mode)
>>>> -        phy_set_mode_ext(phy, PHY_MODE_USB_OTG, USB_ROLE_HOST);
>>>> -    else
>>>> -        phy_set_mode_ext(phy, PHY_MODE_USB_OTG, USB_ROLE_NONE);
>>>> +    if (tegra->host_mode) {
>>>> +        if (!tegra_xusb_usb2_port_wait_role_none(tegra->padctl,
>>>> +                             tegra->otg_usb2_port))
>>>> +            return;
>>>> +        role = USB_ROLE_HOST;
>>>> +    }
>>>> +
>>>> +    mutex_lock(&tegra->lock);
>>>> +    phy_set_mode_ext(phy, PHY_MODE_USB_OTG, role);
>>>>       mutex_unlock(&tegra->lock);
>>>
>>> I am trying to understand why you opted to implement it this way 
>>> around and not add the wait loop after setting to the mode to 
>>> USB_ROLE_NONE in the original code all within the context of the mutex?
>>
>> I did that to minimize the amount of time we wait while holding the
>> mutex, as we can now possibly wait a significant amount of time for the
>> role switch. Is this an unneccessary optimization?
> 
> Do you mean it will be longer than a few 100us?

Currently the worst case in wait_role_none() is around 300us but again
this is simply because I chose the values with no criteria except that
in my testing they have worked thus far. Do you have access to any
internal documentation where the transition length is documented?

In any case I think that the underlying principle of minimizing the time
we hold the mutex is solid, no?

Diogo

> Jon
> 

Re: [PATCH 3/5] phy: tegra: xusb: Fix ordering issue when switching roles on USB2 ports

[Jon Hunter]

On 13/01/2026 15:10, Diogo Ivo wrote:

...

>>>>> -    if (tegra->host_mode)
>>>>> -        phy_set_mode_ext(phy, PHY_MODE_USB_OTG, USB_ROLE_HOST);
>>>>> -    else
>>>>> -        phy_set_mode_ext(phy, PHY_MODE_USB_OTG, USB_ROLE_NONE);
>>>>> +    if (tegra->host_mode) {
>>>>> +        if (!tegra_xusb_usb2_port_wait_role_none(tegra->padctl,
>>>>> +                             tegra->otg_usb2_port))
>>>>> +            return;
>>>>> +        role = USB_ROLE_HOST;
>>>>> +    }
>>>>> +
>>>>> +    mutex_lock(&tegra->lock);
>>>>> +    phy_set_mode_ext(phy, PHY_MODE_USB_OTG, role);
>>>>>       mutex_unlock(&tegra->lock);
>>>>
>>>> I am trying to understand why you opted to implement it this way 
>>>> around and not add the wait loop after setting to the mode to 
>>>> USB_ROLE_NONE in the original code all within the context of the mutex?
>>>
>>> I did that to minimize the amount of time we wait while holding the
>>> mutex, as we can now possibly wait a significant amount of time for the
>>> role switch. Is this an unneccessary optimization?
>>
>> Do you mean it will be longer than a few 100us?
> 
> Currently the worst case in wait_role_none() is around 300us but again
> this is simply because I chose the values with no criteria except that
> in my testing they have worked thus far. Do you have access to any
> internal documentation where the transition length is documented?
> 
> In any case I think that the underlying principle of minimizing the time
> we hold the mutex is solid, no?

Yes, but it was unclear to me if there could be a case where we are 
waiting for USB_ROLE_NONE but we have already transitioned to host. May 
be that will never happen, but it was not clear.

Jon

-- 
nvpublic

Re: [PATCH 3/5] phy: tegra: xusb: Fix ordering issue when switching roles on USB2 ports

[Diogo Ivo]

Hi Jonathan,

On 1/13/26 11:56, Jon Hunter wrote:
> 
> On 04/12/2025 21:27, Diogo Ivo wrote:
>> The current implementation of USB2 role switching on Tegra relies on
>> whichever the previous USB controller driver was using the PHY to first
>> "yield" it back to USB_ROLE_NONE before the next controller configures
>> it for the new role. However, no mechanism to guarantee this ordering
>> was implemented, and currently, in the general case, the configuration
>> functions tegra_xhci_id_work() and tegra_xudc_usb_role_sw_work() end up
>> running in the same order regardless of the transition being HOST->DEVICE
>> or DEVICE->HOST, leading to one of these transitions ending up in a
>> non-working state due to the new configuration being clobbered by the
>> previous controller driver setting USB_ROLE_NONE after the fact.
>>
>> Fix this by introducing a helper that waits for the USB2 port’s current
>> role to become USB_ROLE_NONE and add it in the configuration functions
>> above before setting the role to either USB_ROLE_HOST or
>> USB_ROLE_DEVICE. The specific parameters of the helper function are
>> choices that seem reasonable in my testing and have no other basis.
> 
> This is no information here about why 6 * 50/60us is deemed to be 
> sufficient? May be it is, but a comment would be nice.

I missed this review comment and I'm not sure what you mean here. Do you
want me to comment on the commit message on how I chose these
parameters? If so it's as stated in the current message, I simply tested
with these parameters and it worked and I really have no better basis
for choosing them. If you mean adding a comment in the code I can do
that for v2.

Thanks,
Diogo

>> This was tested on a Tegra210 platform (Smaug). However, due to the 
>> similar
>> approach in Tegra186 it is likely that not only this problem exists there
>> but that this patch also fixes it.
>>
>> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
>> ---
>>   drivers/phy/tegra/xusb.c            | 23 +++++++++++++++++++++++
>>   drivers/usb/gadget/udc/tegra-xudc.c |  4 ++++
>>   drivers/usb/host/xhci-tegra.c       | 15 ++++++++++-----
>>   include/linux/phy/tegra/xusb.h      |  1 +
>>   4 files changed, 38 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
>> index c89df95aa6ca..e05c3f2d1421 100644
>> --- a/drivers/phy/tegra/xusb.c
>> +++ b/drivers/phy/tegra/xusb.c
>> @@ -740,6 +740,29 @@ static void 
>> tegra_xusb_parse_usb_role_default_mode(struct tegra_xusb_port *port)
>>       }
>>   }
>> +bool tegra_xusb_usb2_port_wait_role_none(struct tegra_xusb_padctl 
>> *padctl, int index)
>> +{
>> +    struct tegra_xusb_usb2_port *usb2 = 
>> tegra_xusb_find_usb2_port(padctl,
>> +                                      index);
>> +    int retries = 5;
>> +
>> +    if (!usb2) {
>> +        dev_err(&usb2->base.dev, "no port found for USB2 lane %u\n", 
>> index);
> 
> This appears to be a bug. If !usb2 then dereference usb2->base anyway.
> 
> 
>> +        return false;
>> +    }
>> +
>> +    do {
>> +        if (usb2->role == USB_ROLE_NONE)
>> +            return true;
>> +
>> +        usleep_range(50, 60);
>> +    } while (retries--);
>> +
>> +    dev_err(&usb2->base.dev, "timed out waiting for USB_ROLE_NONE");
>> +
>> +    return false;
>> +}
>> +
>>   static int tegra_xusb_usb2_port_parse_dt(struct tegra_xusb_usb2_port 
>> *usb2)
>>   {
>>       struct tegra_xusb_port *port = &usb2->base;
>> diff --git a/drivers/usb/gadget/udc/tegra-xudc.c b/drivers/usb/gadget/ 
>> udc/tegra-xudc.c
>> index 0c38fc37b6e6..72d725659e5f 100644
>> --- a/drivers/usb/gadget/udc/tegra-xudc.c
>> +++ b/drivers/usb/gadget/udc/tegra-xudc.c
>> @@ -698,8 +698,12 @@ static void tegra_xudc_restore_port_speed(struct 
>> tegra_xudc *xudc)
>>   static void tegra_xudc_device_mode_on(struct tegra_xudc *xudc)
>>   {
>> +    int port = tegra_xusb_padctl_get_port_number(xudc->curr_utmi_phy);
>>       int err;
>> +    if (!tegra_xusb_usb2_port_wait_role_none(xudc->padctl, port))
>> +        return;
>> +
>>       pm_runtime_get_sync(xudc->dev);
>>       tegra_phy_xusb_utmi_pad_power_on(xudc->curr_utmi_phy);
>> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci- 
>> tegra.c
>> index 9c69fccdc6e8..9944593166a3 100644
>> --- a/drivers/usb/host/xhci-tegra.c
>> +++ b/drivers/usb/host/xhci-tegra.c
>> @@ -1352,18 +1352,23 @@ static void tegra_xhci_id_work(struct 
>> work_struct *work)
>>       struct tegra_xusb_mbox_msg msg;
>>       struct phy *phy = tegra_xusb_get_phy(tegra, "usb2",
>>                               tegra->otg_usb2_port);
>> +    enum usb_role role = USB_ROLE_NONE;
>>       u32 status;
>>       int ret;
>>       dev_dbg(tegra->dev, "host mode %s\n", str_on_off(tegra- 
>> >host_mode));
>> -    mutex_lock(&tegra->lock);
> 
> Extra blank line here.
> 
>> -    if (tegra->host_mode)
>> -        phy_set_mode_ext(phy, PHY_MODE_USB_OTG, USB_ROLE_HOST);
>> -    else
>> -        phy_set_mode_ext(phy, PHY_MODE_USB_OTG, USB_ROLE_NONE);
>> +    if (tegra->host_mode) {
>> +        if (!tegra_xusb_usb2_port_wait_role_none(tegra->padctl,
>> +                             tegra->otg_usb2_port))
>> +            return;
>> +        role = USB_ROLE_HOST;
>> +    }
>> +
>> +    mutex_lock(&tegra->lock);
>> +    phy_set_mode_ext(phy, PHY_MODE_USB_OTG, role);
>>       mutex_unlock(&tegra->lock);
> 
> I am trying to understand why you opted to implement it this way around 
> and not add the wait loop after setting to the mode to USB_ROLE_NONE in 
> the original code all within the context of the mutex?
> 
> Thanks
> Jon
> 

Re: [PATCH 3/5] phy: tegra: xusb: Fix ordering issue when switching roles on USB2 ports

[Jon Hunter]

On 15/01/2026 11:06, Diogo Ivo wrote:
> Hi Jonathan,
> 
> On 1/13/26 11:56, Jon Hunter wrote:
>>
>> On 04/12/2025 21:27, Diogo Ivo wrote:
>>> The current implementation of USB2 role switching on Tegra relies on
>>> whichever the previous USB controller driver was using the PHY to first
>>> "yield" it back to USB_ROLE_NONE before the next controller configures
>>> it for the new role. However, no mechanism to guarantee this ordering
>>> was implemented, and currently, in the general case, the configuration
>>> functions tegra_xhci_id_work() and tegra_xudc_usb_role_sw_work() end up
>>> running in the same order regardless of the transition being HOST- 
>>> >DEVICE
>>> or DEVICE->HOST, leading to one of these transitions ending up in a
>>> non-working state due to the new configuration being clobbered by the
>>> previous controller driver setting USB_ROLE_NONE after the fact.
>>>
>>> Fix this by introducing a helper that waits for the USB2 port’s current
>>> role to become USB_ROLE_NONE and add it in the configuration functions
>>> above before setting the role to either USB_ROLE_HOST or
>>> USB_ROLE_DEVICE. The specific parameters of the helper function are
>>> choices that seem reasonable in my testing and have no other basis.
>>
>> This is no information here about why 6 * 50/60us is deemed to be 
>> sufficient? May be it is, but a comment would be nice.
> 
> I missed this review comment and I'm not sure what you mean here. Do you
> want me to comment on the commit message on how I chose these
> parameters? If so it's as stated in the current message, I simply tested
> with these parameters and it worked and I really have no better basis
> for choosing them. If you mean adding a comment in the code I can do
> that for v2.

Yes please be explicit about how you arrived at these numbers. Ie. based 
upon your testing on what platform, etc.

Thanks
Jon

-- 
nvpublic

[PATCH 4/5] arm64: tegra: smaug: Complete and enable tegra-udc node

[Diogo Ivo]

Complete the missing properties in the tegra-udc node and enable it for
Smaug.

Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
---
 arch/arm64/boot/dts/nvidia/tegra210-smaug.dts | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts b/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts
index 5aa6afd56cbc..b8d854f90be7 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts
+++ b/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts
@@ -1843,6 +1843,17 @@ mmc@700b0600 {
 		status = "okay";
 	};
 
+	usb@700d0000 {
+		phys = <&{/padctl@7009f000/pads/usb2/lanes/usb2-0}>,
+		       <&{/padctl@7009f000/pads/pcie/lanes/pcie-6}>;
+		phy-names = "usb2-0", "usb3-0";
+
+		avddio-usb-supply = <&avddio_1v05>;
+		hvdd-usb-supply = <&pp3300>;
+
+		status = "okay";
+	};
+
 	clock@70110000 {
 		status = "okay";
 		nvidia,cf = <6>;

-- 
2.52.0

[PATCH 5/5] arm64: tegra: smaug: Add usb-role-switch support

[Diogo Ivo]

The USB2 port on Smaug is configured for OTG operation but lacked the
required 'usb-role-switch' property, leading to a failed probe and a
non-functioning USB port. Add the property along with setting the default
role to host.

Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
---
 arch/arm64/boot/dts/nvidia/tegra210-smaug.dts | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts b/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts
index b8d854f90be7..49bf23d6f593 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts
+++ b/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts
@@ -1809,6 +1809,8 @@ usb2-0 {
 				status = "okay";
 				vbus-supply = <&usbc_vbus>;
 				mode = "otg";
+				usb-role-switch;
+				role-switch-default-mode = "host";
 			};
 
 			usb3-0 {

-- 
2.52.0

Re: [PATCH 5/5] arm64: tegra: smaug: Add usb-role-switch support

[Jon Hunter]

On 04/12/2025 21:27, Diogo Ivo wrote:
> The USB2 port on Smaug is configured for OTG operation but lacked the
> required 'usb-role-switch' property, leading to a failed probe and a
> non-functioning USB port. Add the property along with setting the default
> role to host.
> 
> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
> ---
>   arch/arm64/boot/dts/nvidia/tegra210-smaug.dts | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts b/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts
> index b8d854f90be7..49bf23d6f593 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts
> +++ b/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts
> @@ -1809,6 +1809,8 @@ usb2-0 {
>   				status = "okay";
>   				vbus-supply = <&usbc_vbus>;
>   				mode = "otg";
> +				usb-role-switch;
> +				role-switch-default-mode = "host";
>   			};


This change does add the following warning when building with CHECK_DTBS
...

arch/arm64/boot/dts/nvidia/tegra210-smaug.dtb: padctl@7009f000 (nvidia,tegra210-xusb-padctl): ports:usb2-0: 'role-switch-default-mode' does not match any of the regexes: '^pinctrl-[0-9]+$'
	from schema $id: http://devicetree.org/schemas/phy/nvidia,tegra210-xusb-padctl.yaml

I know that there are many warnings seen for the smaug DTB, but it would
be good to ensure we don't add more.

Cheers
Jon

-- 
nvpublic

Re: [PATCH 5/5] arm64: tegra: smaug: Add usb-role-switch support

[Diogo Ivo]

On 1/12/26 22:03, Jon Hunter wrote:
> 
> On 04/12/2025 21:27, Diogo Ivo wrote:
>> The USB2 port on Smaug is configured for OTG operation but lacked the
>> required 'usb-role-switch' property, leading to a failed probe and a
>> non-functioning USB port. Add the property along with setting the default
>> role to host.
>>
>> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
>> ---
>>   arch/arm64/boot/dts/nvidia/tegra210-smaug.dts | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts b/arch/ 
>> arm64/boot/dts/nvidia/tegra210-smaug.dts
>> index b8d854f90be7..49bf23d6f593 100644
>> --- a/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts
>> +++ b/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts
>> @@ -1809,6 +1809,8 @@ usb2-0 {
>>                   status = "okay";
>>                   vbus-supply = <&usbc_vbus>;
>>                   mode = "otg";
>> +                usb-role-switch;
>> +                role-switch-default-mode = "host";
>>               };
> 
> 
> This change does add the following warning when building with CHECK_DTBS
> ...
> 
> arch/arm64/boot/dts/nvidia/tegra210-smaug.dtb: padctl@7009f000 
> (nvidia,tegra210-xusb-padctl): ports:usb2-0: 'role-switch-default-mode' 
> does not match any of the regexes: '^pinctrl-[0-9]+$'
>      from schema $id: http://devicetree.org/schemas/phy/nvidia,tegra210- 
> xusb-padctl.yaml
> 
> I know that there are many warnings seen for the smaug DTB, but it would
> be good to ensure we don't add more.

The 'role-switch-default-mode' property is read by the driver to set the
initial role for the port [0] and is needed in order for the port to work
when booting so in order to fix the warning this property needs to be added
to the binding.

As for the other warning ('connector' is a dependency of 'usb-role-switch')
again I think the binding needs to be adjusted since in the Pixel C the
connector node should be under the (as for now not present)
cros-ec-typec node and the usb2-0 is then modeled as a remote-endpoint
for the full connector. I am currently working on fixing the cros-ec-typec
driver and already have a working fix for automatic role switching but
in any case I think the binding is what needs changing. If you agree
with this then I will add the necessary changes to the DT binding in v2.

Thanks,
Diogo

[0]: 
https://elixir.bootlin.com/linux/v6.18.4/source/drivers/phy/tegra/xusb.c#L730

> Cheers
> Jon

Re: [PATCH 5/5] arm64: tegra: smaug: Add usb-role-switch support

[Jon Hunter]

On 13/01/2026 14:20, Diogo Ivo wrote:
> 
> 
> On 1/12/26 22:03, Jon Hunter wrote:
>>
>> On 04/12/2025 21:27, Diogo Ivo wrote:
>>> The USB2 port on Smaug is configured for OTG operation but lacked the
>>> required 'usb-role-switch' property, leading to a failed probe and a
>>> non-functioning USB port. Add the property along with setting the 
>>> default
>>> role to host.
>>>
>>> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
>>> ---
>>>   arch/arm64/boot/dts/nvidia/tegra210-smaug.dts | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts b/arch/ 
>>> arm64/boot/dts/nvidia/tegra210-smaug.dts
>>> index b8d854f90be7..49bf23d6f593 100644
>>> --- a/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts
>>> +++ b/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts
>>> @@ -1809,6 +1809,8 @@ usb2-0 {
>>>                   status = "okay";
>>>                   vbus-supply = <&usbc_vbus>;
>>>                   mode = "otg";
>>> +                usb-role-switch;
>>> +                role-switch-default-mode = "host";
>>>               };
>>
>>
>> This change does add the following warning when building with CHECK_DTBS
>> ...
>>
>> arch/arm64/boot/dts/nvidia/tegra210-smaug.dtb: padctl@7009f000 
>> (nvidia,tegra210-xusb-padctl): ports:usb2-0: 'role-switch-default- 
>> mode' does not match any of the regexes: '^pinctrl-[0-9]+$'
>>      from schema $id: http://devicetree.org/schemas/phy/ 
>> nvidia,tegra210- xusb-padctl.yaml
>>
>> I know that there are many warnings seen for the smaug DTB, but it would
>> be good to ensure we don't add more.
> 
> The 'role-switch-default-mode' property is read by the driver to set the
> initial role for the port [0] and is needed in order for the port to work
> when booting so in order to fix the warning this property needs to be added
> to the binding.

Correct.

> As for the other warning ('connector' is a dependency of 'usb-role-switch')
> again I think the binding needs to be adjusted since in the Pixel C the
> connector node should be under the (as for now not present)
> cros-ec-typec node and the usb2-0 is then modeled as a remote-endpoint
> for the full connector. I am currently working on fixing the cros-ec-typec
> driver and already have a working fix for automatic role switching but
> in any case I think the binding is what needs changing. If you agree
> with this then I will add the necessary changes to the DT binding in v2.

Yes in both cases we need fixes to the DT binding that's all.

Jon

-- 
nvpublic

Re: [PATCH 5/5] arm64: tegra: smaug: Add usb-role-switch support

[Diogo Ivo]

On 1/13/26 14:49, Jon Hunter wrote:
> 
> On 13/01/2026 14:20, Diogo Ivo wrote:
>>
>>
>> On 1/12/26 22:03, Jon Hunter wrote:
>>>
>>> On 04/12/2025 21:27, Diogo Ivo wrote:
>>>> The USB2 port on Smaug is configured for OTG operation but lacked the
>>>> required 'usb-role-switch' property, leading to a failed probe and a
>>>> non-functioning USB port. Add the property along with setting the 
>>>> default
>>>> role to host.
>>>>
>>>> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
>>>> ---
>>>>   arch/arm64/boot/dts/nvidia/tegra210-smaug.dts | 2 ++
>>>>   1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts b/arch/ 
>>>> arm64/boot/dts/nvidia/tegra210-smaug.dts
>>>> index b8d854f90be7..49bf23d6f593 100644
>>>> --- a/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts
>>>> +++ b/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts
>>>> @@ -1809,6 +1809,8 @@ usb2-0 {
>>>>                   status = "okay";
>>>>                   vbus-supply = <&usbc_vbus>;
>>>>                   mode = "otg";
>>>> +                usb-role-switch;
>>>> +                role-switch-default-mode = "host";
>>>>               };
>>>
>>>
>>> This change does add the following warning when building with CHECK_DTBS
>>> ...
>>>
>>> arch/arm64/boot/dts/nvidia/tegra210-smaug.dtb: padctl@7009f000 
>>> (nvidia,tegra210-xusb-padctl): ports:usb2-0: 'role-switch-default- 
>>> mode' does not match any of the regexes: '^pinctrl-[0-9]+$'
>>>      from schema $id: http://devicetree.org/schemas/phy/ 
>>> nvidia,tegra210- xusb-padctl.yaml
>>>
>>> I know that there are many warnings seen for the smaug DTB, but it would
>>> be good to ensure we don't add more.
>>
>> The 'role-switch-default-mode' property is read by the driver to set the
>> initial role for the port [0] and is needed in order for the port to work
>> when booting so in order to fix the warning this property needs to be 
>> added
>> to the binding.
> 
> Correct.
> 
>> As for the other warning ('connector' is a dependency of 'usb-role- 
>> switch')
>> again I think the binding needs to be adjusted since in the Pixel C the
>> connector node should be under the (as for now not present)
>> cros-ec-typec node and the usb2-0 is then modeled as a remote-endpoint
>> for the full connector. I am currently working on fixing the cros-ec- 
>> typec
>> driver and already have a working fix for automatic role switching but
>> in any case I think the binding is what needs changing. If you agree
>> with this then I will add the necessary changes to the DT binding in v2.
> 
> Yes in both cases we need fixes to the DT binding that's all.

Perfect, will do for v2.

Diogo

> Jon
> 

Re: [PATCH 0/5] Fixes to Tegra USB role switching and Smaug USB role switching enablement

[Rob Herring]

On Thu, 04 Dec 2025 21:27:16 +0000, Diogo Ivo wrote:
> Hello,
> 
> This patch series contains two fixes for USB role switching on the
> Tegra210 SoC, as well as enabling this feature on the Pixel C.
> 
> The first patch addresses a wrong check on the logic that disables the
> VBUS regulator.
> 
> The second patch guarantees proper ordering of events when switching PHY
> roles.
> 
> The third and fourth patches then add the necessary nodes and properties
> in the Smaug DT in order for role switching to work. Currently with this
> patch series this feature can only be controlled from userspace by writing
> the desired role to sysfs as
> 
> echo "role" > /sys/class/usb_role/usb2-0-role-switch/role
> 
> with role being one of {device, host, none}.
> 
> Further patches will enable automatic role switching via the 'cros_ec_typec'
> driver which is currently broken on Smaug.
> 
> N.B: This series does not add a 'connector' node under the 'usb-role-switch'
> property added on patch 04/04 because for Smaug the connector should instead
> be under the node for 'cros_ec_typec' node and as stated above this
> driver is currently broken for this device. If it is deemed better to
> describe it but explicitly disable the node let me know and I will send
> out a v2.
> 
> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
> ---
> Diogo Ivo (5):
>       usb: host: tegra: Remove redundant pm_runtime_mark_last_busy() call
>       phy: tegra: xusb: Fix USB2 port regulator disable logic
>       phy: tegra: xusb: Fix ordering issue when switching roles on USB2 ports
>       arm64: tegra: smaug: Complete and enable tegra-udc node
>       arm64: tegra: smaug: Add usb-role-switch support
> 
>  arch/arm64/boot/dts/nvidia/tegra210-smaug.dts | 13 +++++++++++++
>  drivers/phy/tegra/xusb-tegra210.c             |  5 +++--
>  drivers/phy/tegra/xusb.c                      | 23 +++++++++++++++++++++++
>  drivers/phy/tegra/xusb.h                      |  1 +
>  drivers/usb/gadget/udc/tegra-xudc.c           |  4 ++++
>  drivers/usb/host/xhci-tegra.c                 | 17 ++++++++++-------
>  include/linux/phy/tegra/xusb.h                |  1 +
>  7 files changed, 55 insertions(+), 9 deletions(-)
> ---
> base-commit: a8817ff3b5cd99b0a5af57a92d1a3a7980612550
> change-id: 20251201-diogo-tegra_phy-86c89cab7377
> 
> Best regards,
> --
> Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
> 
> 
> 


My bot found new DTB warnings on the .dts files added or changed in this
series.

Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.

If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:

  pip3 install dtschema --upgrade


This patch series was applied (using b4) to base:
 Base: base-commit a8817ff3b5cd99b0a5af57a92d1a3a7980612550 not known, ignoring
 Base: attempting to guess base-commit...
 Base: tags/v6.18-rc7-8-gf402ecd7a8b6 (exact match)
 Base: tags/v6.18-rc7-8-gf402ecd7a8b6 (use --merge-base to override)

If this is not the correct base, please add 'base-commit' tag
(or use b4 which does this automatically)

New warnings running 'make CHECK_DTBS=y for arch/arm64/boot/dts/nvidia/' for 20251204-diogo-tegra_phy-v1-0-51a2016d0be8@tecnico.ulisboa.pt:

arch/arm64/boot/dts/nvidia/tegra210-smaug.dtb: padctl@7009f000 (nvidia,tegra210-xusb-padctl): ports:usb2-0: 'role-switch-default-mode' does not match any of the regexes: '^pinctrl-[0-9]+$'
	from schema $id: http://devicetree.org/schemas/phy/nvidia,tegra210-xusb-padctl.yaml
arch/arm64/boot/dts/nvidia/tegra210-smaug.dtb: padctl@7009f000 (nvidia,tegra210-xusb-padctl): ports:usb2-0: 'connector' is a dependency of 'usb-role-switch'
	from schema $id: http://devicetree.org/schemas/phy/nvidia,tegra210-xusb-padctl.yaml

Re: [PATCH 0/5] Fixes to Tegra USB role switching and Smaug USB role switching enablement

[Jon Hunter]

Hi Diogo,

On 05/12/2025 22:36, Rob Herring wrote:
> 
> On Thu, 04 Dec 2025 21:27:16 +0000, Diogo Ivo wrote:
>> Hello,
>>
>> This patch series contains two fixes for USB role switching on the
>> Tegra210 SoC, as well as enabling this feature on the Pixel C.
>>
>> The first patch addresses a wrong check on the logic that disables the
>> VBUS regulator.
>>
>> The second patch guarantees proper ordering of events when switching PHY
>> roles.
>>
>> The third and fourth patches then add the necessary nodes and properties
>> in the Smaug DT in order for role switching to work. Currently with this
>> patch series this feature can only be controlled from userspace by writing
>> the desired role to sysfs as
>>
>> echo "role" > /sys/class/usb_role/usb2-0-role-switch/role
>>
>> with role being one of {device, host, none}.
>>
>> Further patches will enable automatic role switching via the 'cros_ec_typec'
>> driver which is currently broken on Smaug.
>>
>> N.B: This series does not add a 'connector' node under the 'usb-role-switch'
>> property added on patch 04/04 because for Smaug the connector should instead
>> be under the node for 'cros_ec_typec' node and as stated above this
>> driver is currently broken for this device. If it is deemed better to
>> describe it but explicitly disable the node let me know and I will send
>> out a v2.
>>
>> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
>> ---
>> Diogo Ivo (5):
>>        usb: host: tegra: Remove redundant pm_runtime_mark_last_busy() call
>>        phy: tegra: xusb: Fix USB2 port regulator disable logic
>>        phy: tegra: xusb: Fix ordering issue when switching roles on USB2 ports
>>        arm64: tegra: smaug: Complete and enable tegra-udc node
>>        arm64: tegra: smaug: Add usb-role-switch support
>>
>>   arch/arm64/boot/dts/nvidia/tegra210-smaug.dts | 13 +++++++++++++
>>   drivers/phy/tegra/xusb-tegra210.c             |  5 +++--
>>   drivers/phy/tegra/xusb.c                      | 23 +++++++++++++++++++++++
>>   drivers/phy/tegra/xusb.h                      |  1 +
>>   drivers/usb/gadget/udc/tegra-xudc.c           |  4 ++++
>>   drivers/usb/host/xhci-tegra.c                 | 17 ++++++++++-------
>>   include/linux/phy/tegra/xusb.h                |  1 +
>>   7 files changed, 55 insertions(+), 9 deletions(-)
>> ---
>> base-commit: a8817ff3b5cd99b0a5af57a92d1a3a7980612550
>> change-id: 20251201-diogo-tegra_phy-86c89cab7377
>>
>> Best regards,
>> --
>> Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
>>
>>
>>
> 
> 
> My bot found new DTB warnings on the .dts files added or changed in this
> series.
> 
> Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
> are fixed by another series. Ultimately, it is up to the platform
> maintainer whether these warnings are acceptable or not. No need to reply
> unless the platform maintainer has comments.
> 
> If you already ran DT checks and didn't see these error(s), then
> make sure dt-schema is up to date:
> 
>    pip3 install dtschema --upgrade
> 
> 
> This patch series was applied (using b4) to base:
>   Base: base-commit a8817ff3b5cd99b0a5af57a92d1a3a7980612550 not known, ignoring
>   Base: attempting to guess base-commit...
>   Base: tags/v6.18-rc7-8-gf402ecd7a8b6 (exact match)
>   Base: tags/v6.18-rc7-8-gf402ecd7a8b6 (use --merge-base to override)
> 
> If this is not the correct base, please add 'base-commit' tag
> (or use b4 which does this automatically)
> 
> New warnings running 'make CHECK_DTBS=y for arch/arm64/boot/dts/nvidia/' for 20251204-diogo-tegra_phy-v1-0-51a2016d0be8@tecnico.ulisboa.pt:
> 
> arch/arm64/boot/dts/nvidia/tegra210-smaug.dtb: padctl@7009f000 (nvidia,tegra210-xusb-padctl): ports:usb2-0: 'role-switch-default-mode' does not match any of the regexes: '^pinctrl-[0-9]+$'
> 	from schema $id: http://devicetree.org/schemas/phy/nvidia,tegra210-xusb-padctl.yaml
> arch/arm64/boot/dts/nvidia/tegra210-smaug.dtb: padctl@7009f000 (nvidia,tegra210-xusb-padctl): ports:usb2-0: 'connector' is a dependency of 'usb-role-switch'
> 	from schema $id: http://devicetree.org/schemas/phy/nvidia,tegra210-xusb-padctl.yaml

Per the report above and my other email, this series adds more warnings 
and we are trying to avoid that, even if such warnings are seen on other 
Tegra platforms.

Jon

-- 
nvpublic

Re: [PATCH 0/5] Fixes to Tegra USB role switching and Smaug USB role switching enablement

[Diogo Ivo]

Hello,

On 12/4/25 21:27, Diogo Ivo wrote:
> Hello,
> 
> This patch series contains two fixes for USB role switching on the
> Tegra210 SoC, as well as enabling this feature on the Pixel C.
> 
> The first patch addresses a wrong check on the logic that disables the
> VBUS regulator.
> 
> The second patch guarantees proper ordering of events when switching PHY
> roles.
> 
> The third and fourth patches then add the necessary nodes and properties
> in the Smaug DT in order for role switching to work. Currently with this
> patch series this feature can only be controlled from userspace by writing
> the desired role to sysfs as
> 
> echo "role" > /sys/class/usb_role/usb2-0-role-switch/role
> 
> with role being one of {device, host, none}.
> 
> Further patches will enable automatic role switching via the 'cros_ec_typec'
> driver which is currently broken on Smaug.
> 
> N.B: This series does not add a 'connector' node under the 'usb-role-switch'
> property added on patch 04/04 because for Smaug the connector should instead
> be under the node for 'cros_ec_typec' node and as stated above this
> driver is currently broken for this device. If it is deemed better to
> describe it but explicitly disable the node let me know and I will send
> out a v2.
> 
> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
> ---
> Diogo Ivo (5):
>        usb: host: tegra: Remove redundant pm_runtime_mark_last_busy() call
>        phy: tegra: xusb: Fix USB2 port regulator disable logic
>        phy: tegra: xusb: Fix ordering issue when switching roles on USB2 ports
>        arm64: tegra: smaug: Complete and enable tegra-udc node
>        arm64: tegra: smaug: Add usb-role-switch support
> 
>   arch/arm64/boot/dts/nvidia/tegra210-smaug.dts | 13 +++++++++++++
>   drivers/phy/tegra/xusb-tegra210.c             |  5 +++--
>   drivers/phy/tegra/xusb.c                      | 23 +++++++++++++++++++++++
>   drivers/phy/tegra/xusb.h                      |  1 +
>   drivers/usb/gadget/udc/tegra-xudc.c           |  4 ++++
>   drivers/usb/host/xhci-tegra.c                 | 17 ++++++++++-------
>   include/linux/phy/tegra/xusb.h                |  1 +
>   7 files changed, 55 insertions(+), 9 deletions(-)
> ---
> base-commit: a8817ff3b5cd99b0a5af57a92d1a3a7980612550
> change-id: 20251201-diogo-tegra_phy-86c89cab7377

Gentle ping on this series.

Thank you,
Diogo

Re: (subset) [PATCH 0/5] Fixes to Tegra USB role switching and Smaug USB role switching enablement

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>


On Thu, 04 Dec 2025 21:27:16 +0000, Diogo Ivo wrote:
> This patch series contains two fixes for USB role switching on the
> Tegra210 SoC, as well as enabling this feature on the Pixel C.
> 
> The first patch addresses a wrong check on the logic that disables the
> VBUS regulator.
> 
> The second patch guarantees proper ordering of events when switching PHY
> roles.
> 
> [...]

Applied, thanks!

[4/5] arm64: tegra: smaug: Complete and enable tegra-udc node
      (no commit info)
[5/5] arm64: tegra: smaug: Add usb-role-switch support
      (no commit info)

Best regards,
-- 
Thierry Reding <treding@nvidia.com>