[PATCH v3 0/2] usb: phy: tegra: add support for HSIC mode

[PATCH v3 0/2] usb: phy: tegra: add support for HSIC mode

[Svyatoslav Ryhel]

Add support for HSIC mode into phy driver. HSIC mode is supported by
second USB controller in Tegra SoC and is used for connecting modem.
Linux kernel has support for HSIC mode in the controller driver itself
(Chipidea driver) but USB PHY part was missing until now.

Tested on LG Optimus Vu smartphone with Infineon/Comneon modem on usb2
line.

---
Changes in v2:
- initialized err in tegra_usb_phy_power functions
- rebased on top of 6.18

Changes in v3:
- initialized err in tegra_usb_phy_power_off function
---

Svyatoslav Ryhel (2):
  usb: phy: tegra: use phy type directly
  usb: phy: tegra: add HSIC support

 drivers/usb/phy/phy-tegra-usb.c   | 300 +++++++++++++++++++++++++++---
 include/linux/usb/tegra_usb_phy.h |   7 +-
 2 files changed, 276 insertions(+), 31 deletions(-)

-- 
2.51.0

[PATCH v3 1/2] usb: phy: tegra: use phy type directly

[Svyatoslav Ryhel]

Refactor to directly use enum usb_phy_interface to determine the PHY mode.
This change is in preparation for adding support for HSIC mode.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/usb/phy/phy-tegra-usb.c   | 51 +++++++++++++++++++------------
 include/linux/usb/tegra_usb_phy.h |  2 +-
 2 files changed, 33 insertions(+), 20 deletions(-)

diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c
index 1f527fcb42f6..afa5b5535f92 100644
--- a/drivers/usb/phy/phy-tegra-usb.c
+++ b/drivers/usb/phy/phy-tegra-usb.c
@@ -814,15 +814,24 @@ static int ulpi_phy_power_off(struct tegra_usb_phy *phy)
 
 static int tegra_usb_phy_power_on(struct tegra_usb_phy *phy)
 {
-	int err;
+	int err = 0;
 
 	if (phy->powered_on)
 		return 0;
 
-	if (phy->is_ulpi_phy)
-		err = ulpi_phy_power_on(phy);
-	else
+	switch (phy->phy_type) {
+	case USBPHY_INTERFACE_MODE_UTMI:
 		err = utmi_phy_power_on(phy);
+		break;
+
+	case USBPHY_INTERFACE_MODE_ULPI:
+		err = ulpi_phy_power_on(phy);
+		break;
+
+	default:
+		break;
+	}
+
 	if (err)
 		return err;
 
@@ -836,15 +845,24 @@ static int tegra_usb_phy_power_on(struct tegra_usb_phy *phy)
 
 static int tegra_usb_phy_power_off(struct tegra_usb_phy *phy)
 {
-	int err;
+	int err = 0;
 
 	if (!phy->powered_on)
 		return 0;
 
-	if (phy->is_ulpi_phy)
-		err = ulpi_phy_power_off(phy);
-	else
+	switch (phy->phy_type) {
+	case USBPHY_INTERFACE_MODE_UTMI:
 		err = utmi_phy_power_off(phy);
+		break;
+
+	case USBPHY_INTERFACE_MODE_ULPI:
+		err = ulpi_phy_power_off(phy);
+		break;
+
+	default:
+		break;
+	}
+
 	if (err)
 		return err;
 
@@ -863,7 +881,7 @@ static void tegra_usb_phy_shutdown(struct usb_phy *u_phy)
 	usb_phy_set_wakeup(u_phy, false);
 	tegra_usb_phy_power_off(phy);
 
-	if (!phy->is_ulpi_phy)
+	if (phy->phy_type == USBPHY_INTERFACE_MODE_UTMI)
 		utmip_pad_close(phy);
 
 	regulator_disable(phy->vbus);
@@ -1049,7 +1067,7 @@ static int tegra_usb_phy_init(struct usb_phy *u_phy)
 		goto disable_clk;
 	}
 
-	if (!phy->is_ulpi_phy) {
+	if (phy->phy_type == USBPHY_INTERFACE_MODE_UTMI) {
 		err = utmip_pad_open(phy);
 		if (err)
 			goto disable_vbus;
@@ -1066,7 +1084,7 @@ static int tegra_usb_phy_init(struct usb_phy *u_phy)
 	return 0;
 
 close_phy:
-	if (!phy->is_ulpi_phy)
+	if (phy->phy_type == USBPHY_INTERFACE_MODE_UTMI)
 		utmip_pad_close(phy);
 
 disable_vbus:
@@ -1104,8 +1122,6 @@ static int utmi_phy_probe(struct tegra_usb_phy *tegra_phy,
 	struct resource *res;
 	int err;
 
-	tegra_phy->is_ulpi_phy = false;
-
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
 	if (!res) {
 		dev_err(&pdev->dev, "Failed to get UTMI pad regs\n");
@@ -1280,7 +1296,6 @@ static int tegra_usb_phy_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
 	struct tegra_usb_phy *tegra_phy;
-	enum usb_phy_interface phy_type;
 	struct reset_control *reset;
 	struct gpio_desc *gpiod;
 	struct resource *res;
@@ -1342,8 +1357,8 @@ static int tegra_usb_phy_probe(struct platform_device *pdev)
 		return err;
 	}
 
-	phy_type = of_usb_get_phy_mode(np);
-	switch (phy_type) {
+	tegra_phy->phy_type = of_usb_get_phy_mode(np);
+	switch (tegra_phy->phy_type) {
 	case USBPHY_INTERFACE_MODE_UTMI:
 		err = utmi_phy_probe(tegra_phy, pdev);
 		if (err)
@@ -1369,8 +1384,6 @@ static int tegra_usb_phy_probe(struct platform_device *pdev)
 		break;
 
 	case USBPHY_INTERFACE_MODE_ULPI:
-		tegra_phy->is_ulpi_phy = true;
-
 		tegra_phy->clk = devm_clk_get(&pdev->dev, "ulpi-link");
 		err = PTR_ERR_OR_ZERO(tegra_phy->clk);
 		if (err) {
@@ -1410,7 +1423,7 @@ static int tegra_usb_phy_probe(struct platform_device *pdev)
 
 	default:
 		dev_err(&pdev->dev, "phy_type %u is invalid or unsupported\n",
-			phy_type);
+			tegra_phy->phy_type);
 		return -EINVAL;
 	}
 
diff --git a/include/linux/usb/tegra_usb_phy.h b/include/linux/usb/tegra_usb_phy.h
index c314fad5e375..e394f4880b7e 100644
--- a/include/linux/usb/tegra_usb_phy.h
+++ b/include/linux/usb/tegra_usb_phy.h
@@ -73,7 +73,7 @@ struct tegra_usb_phy {
 	struct usb_phy *ulpi;
 	struct usb_phy u_phy;
 	bool is_legacy_phy;
-	bool is_ulpi_phy;
+	enum usb_phy_interface phy_type;
 	struct gpio_desc *reset_gpio;
 	struct reset_control *pad_rst;
 	bool wakeup_enabled;
-- 
2.51.0

Re: [PATCH v3 1/2] usb: phy: tegra: use phy type directly

[Mikko Perttunen]

On Friday, January 23, 2026 12:11 AM Svyatoslav Ryhel wrote:
> Refactor to directly use enum usb_phy_interface to determine the PHY mode.
> This change is in preparation for adding support for HSIC mode.
> 
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
>  drivers/usb/phy/phy-tegra-usb.c   | 51 +++++++++++++++++++------------
>  include/linux/usb/tegra_usb_phy.h |  2 +-
>  2 files changed, 33 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c
> index 1f527fcb42f6..afa5b5535f92 100644
> --- a/drivers/usb/phy/phy-tegra-usb.c
> +++ b/drivers/usb/phy/phy-tegra-usb.c
> @@ -814,15 +814,24 @@ static int ulpi_phy_power_off(struct tegra_usb_phy *phy)
>  
>  static int tegra_usb_phy_power_on(struct tegra_usb_phy *phy)
>  {
> -	int err;
> +	int err = 0;
>  
>  	if (phy->powered_on)
>  		return 0;
>  
> -	if (phy->is_ulpi_phy)
> -		err = ulpi_phy_power_on(phy);
> -	else
> +	switch (phy->phy_type) {
> +	case USBPHY_INTERFACE_MODE_UTMI:
>  		err = utmi_phy_power_on(phy);
> +		break;
> +
> +	case USBPHY_INTERFACE_MODE_ULPI:
> +		err = ulpi_phy_power_on(phy);
> +		break;
> +
> +	default:
> +		break;
> +	}
> +
>  	if (err)
>  		return err;
>  
> @@ -836,15 +845,24 @@ static int tegra_usb_phy_power_on(struct tegra_usb_phy *phy)
>  
>  static int tegra_usb_phy_power_off(struct tegra_usb_phy *phy)
>  {
> -	int err;
> +	int err = 0;
>  
>  	if (!phy->powered_on)
>  		return 0;
>  
> -	if (phy->is_ulpi_phy)
> -		err = ulpi_phy_power_off(phy);
> -	else
> +	switch (phy->phy_type) {
> +	case USBPHY_INTERFACE_MODE_UTMI:
>  		err = utmi_phy_power_off(phy);
> +		break;
> +
> +	case USBPHY_INTERFACE_MODE_ULPI:
> +		err = ulpi_phy_power_off(phy);
> +		break;
> +
> +	default:
> +		break;
> +	}
> +
>  	if (err)
>  		return err;
>  
> @@ -863,7 +881,7 @@ static void tegra_usb_phy_shutdown(struct usb_phy *u_phy)
>  	usb_phy_set_wakeup(u_phy, false);
>  	tegra_usb_phy_power_off(phy);
>  
> -	if (!phy->is_ulpi_phy)
> +	if (phy->phy_type == USBPHY_INTERFACE_MODE_UTMI)
>  		utmip_pad_close(phy);
>  
>  	regulator_disable(phy->vbus);
> @@ -1049,7 +1067,7 @@ static int tegra_usb_phy_init(struct usb_phy *u_phy)
>  		goto disable_clk;
>  	}
>  
> -	if (!phy->is_ulpi_phy) {
> +	if (phy->phy_type == USBPHY_INTERFACE_MODE_UTMI) {
>  		err = utmip_pad_open(phy);
>  		if (err)
>  			goto disable_vbus;
> @@ -1066,7 +1084,7 @@ static int tegra_usb_phy_init(struct usb_phy *u_phy)
>  	return 0;
>  
>  close_phy:
> -	if (!phy->is_ulpi_phy)
> +	if (phy->phy_type == USBPHY_INTERFACE_MODE_UTMI)
>  		utmip_pad_close(phy);
>  
>  disable_vbus:
> @@ -1104,8 +1122,6 @@ static int utmi_phy_probe(struct tegra_usb_phy *tegra_phy,
>  	struct resource *res;
>  	int err;
>  
> -	tegra_phy->is_ulpi_phy = false;
> -
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>  	if (!res) {
>  		dev_err(&pdev->dev, "Failed to get UTMI pad regs\n");
> @@ -1280,7 +1296,6 @@ static int tegra_usb_phy_probe(struct platform_device *pdev)
>  {
>  	struct device_node *np = pdev->dev.of_node;
>  	struct tegra_usb_phy *tegra_phy;
> -	enum usb_phy_interface phy_type;
>  	struct reset_control *reset;
>  	struct gpio_desc *gpiod;
>  	struct resource *res;
> @@ -1342,8 +1357,8 @@ static int tegra_usb_phy_probe(struct platform_device *pdev)
>  		return err;
>  	}
>  
> -	phy_type = of_usb_get_phy_mode(np);
> -	switch (phy_type) {
> +	tegra_phy->phy_type = of_usb_get_phy_mode(np);
> +	switch (tegra_phy->phy_type) {
>  	case USBPHY_INTERFACE_MODE_UTMI:
>  		err = utmi_phy_probe(tegra_phy, pdev);
>  		if (err)
> @@ -1369,8 +1384,6 @@ static int tegra_usb_phy_probe(struct platform_device *pdev)
>  		break;
>  
>  	case USBPHY_INTERFACE_MODE_ULPI:
> -		tegra_phy->is_ulpi_phy = true;
> -
>  		tegra_phy->clk = devm_clk_get(&pdev->dev, "ulpi-link");
>  		err = PTR_ERR_OR_ZERO(tegra_phy->clk);
>  		if (err) {
> @@ -1410,7 +1423,7 @@ static int tegra_usb_phy_probe(struct platform_device *pdev)
>  
>  	default:
>  		dev_err(&pdev->dev, "phy_type %u is invalid or unsupported\n",
> -			phy_type);
> +			tegra_phy->phy_type);
>  		return -EINVAL;
>  	}
>  
> diff --git a/include/linux/usb/tegra_usb_phy.h b/include/linux/usb/tegra_usb_phy.h
> index c314fad5e375..e394f4880b7e 100644
> --- a/include/linux/usb/tegra_usb_phy.h
> +++ b/include/linux/usb/tegra_usb_phy.h
> @@ -73,7 +73,7 @@ struct tegra_usb_phy {
>  	struct usb_phy *ulpi;
>  	struct usb_phy u_phy;
>  	bool is_legacy_phy;
> -	bool is_ulpi_phy;
> +	enum usb_phy_interface phy_type;
>  	struct gpio_desc *reset_gpio;
>  	struct reset_control *pad_rst;
>  	bool wakeup_enabled;
> 

I would consider putting a WARN() inside the default: cases which should never run -- primarily just as an indicator to the reader that those code paths should never be taken. In any case,

Reviewed-by: Mikko Perttunen <mperttunen@nvidia.com>

[PATCH v3 2/2] usb: phy: tegra: add HSIC support

[Svyatoslav Ryhel]

Add support for HSIC USB mode, which can be set for second USB controller
and PHY on Tegra SoC along with already supported UTMI or ULPI.

Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
 drivers/usb/phy/phy-tegra-usb.c   | 249 ++++++++++++++++++++++++++++--
 include/linux/usb/tegra_usb_phy.h |   5 +
 2 files changed, 243 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c
index afa5b5535f92..4f0fde33647e 100644
--- a/drivers/usb/phy/phy-tegra-usb.c
+++ b/drivers/usb/phy/phy-tegra-usb.c
@@ -29,17 +29,26 @@
 #include <linux/usb/tegra_usb_phy.h>
 #include <linux/usb/ulpi.h>
 
+#define USB_TXFILLTUNING			0x154
+#define USB_FIFO_TXFILL_THRES(x)		(((x) & 0x1f) << 16)
+#define USB_FIFO_TXFILL_MASK			0x1f0000
+
 #define ULPI_VIEWPORT				0x170
 
 /* PORTSC PTS/PHCD bits, Tegra20 only */
 #define TEGRA_USB_PORTSC1			0x184
-#define TEGRA_USB_PORTSC1_PTS(x)		(((x) & 0x3) << 30)
-#define TEGRA_USB_PORTSC1_PHCD			BIT(23)
+#define   TEGRA_USB_PORTSC1_PTS(x)		(((x) & 0x3) << 30)
+#define   TEGRA_USB_PORTSC1_PHCD		BIT(23)
+#define   TEGRA_USB_PORTSC1_WKOC		BIT(22)
+#define   TEGRA_USB_PORTSC1_WKDS		BIT(21)
+#define   TEGRA_USB_PORTSC1_WKCN		BIT(20)
 
 /* HOSTPC1 PTS/PHCD bits, Tegra30 and above */
+#define TEGRA30_USB_PORTSC1			0x174
 #define TEGRA_USB_HOSTPC1_DEVLC			0x1b4
-#define TEGRA_USB_HOSTPC1_DEVLC_PTS(x)		(((x) & 0x7) << 29)
-#define TEGRA_USB_HOSTPC1_DEVLC_PHCD		BIT(22)
+#define   TEGRA_USB_HOSTPC1_DEVLC_PTS(x)	(((x) & 0x7) << 29)
+#define   TEGRA_USB_HOSTPC1_DEVLC_PHCD		BIT(22)
+#define   TEGRA_USB_HOSTPC1_DEVLC_PTS_HSIC	BIT(2)
 
 /* Bits of PORTSC1, which will get cleared by writing 1 into them */
 #define TEGRA_PORTSC1_RWC_BITS	(PORT_CSC | PORT_PEC | PORT_OCC)
@@ -51,11 +60,12 @@
 #define   USB_SUSP_CLR				BIT(5)
 #define   USB_PHY_CLK_VALID			BIT(7)
 #define   UTMIP_RESET				BIT(11)
-#define   UHSIC_RESET				BIT(11)
 #define   UTMIP_PHY_ENABLE			BIT(12)
 #define   ULPI_PHY_ENABLE			BIT(13)
 #define   USB_SUSP_SET				BIT(14)
+#define   UHSIC_RESET				BIT(14)
 #define   USB_WAKEUP_DEBOUNCE_COUNT(x)		(((x) & 0x7) << 16)
+#define   UHSIC_PHY_ENABLE			BIT(19)
 
 #define USB_PHY_VBUS_SENSORS			0x404
 #define   B_SESS_VLD_WAKEUP_EN			BIT(14)
@@ -156,6 +166,58 @@
 #define UTMIP_BIAS_CFG1				0x83c
 #define   UTMIP_BIAS_PDTRK_COUNT(x)		(((x) & 0x1f) << 3)
 
+/*
+ * Tegra20 has no UTMIP registers on PHY2 and UHSIC registers start from 0x800
+ * just where UTMIP registers should have been. This is the case only with Tegra20
+ * Tegra30+ have UTMIP registers at 0x800 and UHSIC registers shifter by 0x400
+ * to 0xc00, but register layout is preserved.
+ */
+#define UHSIC_PLL_CFG1				0x804
+#define   UHSIC_XTAL_FREQ_COUNT(x)		(((x) & 0xfff) << 0)
+#define   UHSIC_PLLU_ENABLE_DLY_COUNT(x)	(((x) & 0x1f) << 14)
+
+#define UHSIC_HSRX_CFG0				0x808
+#define   UHSIC_ELASTIC_UNDERRUN_LIMIT(x)	(((x) & 0x1f) << 2)
+#define   UHSIC_ELASTIC_OVERRUN_LIMIT(x)	(((x) & 0x1f) << 8)
+#define   UHSIC_IDLE_WAIT(x)			(((x) & 0x1f) << 13)
+
+#define UHSIC_HSRX_CFG1				0x80c
+#define   UHSIC_HS_SYNC_START_DLY(x)		(((x) & 0x1f) << 1)
+
+#define UHSIC_TX_CFG0				0x810
+#define   UHSIC_HS_READY_WAIT_FOR_VALID		BIT(9)
+
+#define UHSIC_MISC_CFG0				0x814
+#define   UHSIC_SUSPEND_EXIT_ON_EDGE		BIT(7)
+#define   UHSIC_DETECT_SHORT_CONNECT		BIT(8)
+#define   UHSIC_FORCE_XCVR_MODE			BIT(15)
+#define   UHSIC_DISABLE_BUSRESET		BIT(20)
+
+#define UHSIC_MISC_CFG1				0x818
+#define   UHSIC_PLLU_STABLE_COUNT(x)		(((x) & 0xfff) << 2)
+
+#define UHSIC_PADS_CFG0				0x81c
+#define   UHSIC_TX_RTUNEN			0xf000
+#define   UHSIC_TX_RTUNE(x)			(((x) & 0xf) << 12)
+
+#define UHSIC_PADS_CFG1				0x820
+#define   UHSIC_PD_BG				BIT(2)
+#define   UHSIC_PD_TX				BIT(3)
+#define   UHSIC_PD_TRK				BIT(4)
+#define   UHSIC_PD_RX				BIT(5)
+#define   UHSIC_PD_ZI				BIT(6)
+#define   UHSIC_RX_SEL				BIT(7)
+#define   UHSIC_RPD_DATA			BIT(9)
+#define   UHSIC_RPD_STROBE			BIT(10)
+#define   UHSIC_RPU_DATA			BIT(11)
+#define   UHSIC_RPU_STROBE			BIT(12)
+
+#define UHSIC_CMD_CFG0				0x824
+#define   UHSIC_PRETEND_CONNECT_DETECT		BIT(5)
+
+#define UHSIC_STAT_CFG0				0x828
+#define   UHSIC_CONNECT_DETECT			BIT(0)
+
 /* For Tegra30 and above only, the address is different in Tegra20 */
 #define USB_USBMODE				0x1f8
 #define   USB_USBMODE_MASK			(3 << 0)
@@ -174,7 +236,8 @@ struct tegra_xtal_freq {
 	u8 enable_delay;
 	u8 stable_count;
 	u8 active_delay;
-	u8 xtal_freq_count;
+	u8 utmi_xtal_freq_count;
+	u16 hsic_xtal_freq_count;
 	u16 debounce;
 };
 
@@ -184,7 +247,8 @@ static const struct tegra_xtal_freq tegra_freq_table[] = {
 		.enable_delay = 0x02,
 		.stable_count = 0x2F,
 		.active_delay = 0x04,
-		.xtal_freq_count = 0x76,
+		.utmi_xtal_freq_count = 0x76,
+		.hsic_xtal_freq_count = 0x1CA,
 		.debounce = 0x7530,
 	},
 	{
@@ -192,7 +256,8 @@ static const struct tegra_xtal_freq tegra_freq_table[] = {
 		.enable_delay = 0x02,
 		.stable_count = 0x33,
 		.active_delay = 0x05,
-		.xtal_freq_count = 0x7F,
+		.utmi_xtal_freq_count = 0x7F,
+		.hsic_xtal_freq_count = 0x1F0,
 		.debounce = 0x7EF4,
 	},
 	{
@@ -200,7 +265,8 @@ static const struct tegra_xtal_freq tegra_freq_table[] = {
 		.enable_delay = 0x03,
 		.stable_count = 0x4B,
 		.active_delay = 0x06,
-		.xtal_freq_count = 0xBB,
+		.utmi_xtal_freq_count = 0xBB,
+		.hsic_xtal_freq_count = 0x2DD,
 		.debounce = 0xBB80,
 	},
 	{
@@ -208,7 +274,8 @@ static const struct tegra_xtal_freq tegra_freq_table[] = {
 		.enable_delay = 0x04,
 		.stable_count = 0x66,
 		.active_delay = 0x09,
-		.xtal_freq_count = 0xFE,
+		.utmi_xtal_freq_count = 0xFE,
+		.hsic_xtal_freq_count = 0x3E0,
 		.debounce = 0xFDE8,
 	},
 };
@@ -541,7 +608,7 @@ static int utmi_phy_power_on(struct tegra_usb_phy *phy)
 		val = readl_relaxed(base + UTMIP_PLL_CFG1);
 		val &= ~(UTMIP_XTAL_FREQ_COUNT(~0) |
 			UTMIP_PLLU_ENABLE_DLY_COUNT(~0));
-		val |= UTMIP_XTAL_FREQ_COUNT(phy->freq->xtal_freq_count) |
+		val |= UTMIP_XTAL_FREQ_COUNT(phy->freq->utmi_xtal_freq_count) |
 			UTMIP_PLLU_ENABLE_DLY_COUNT(phy->freq->enable_delay);
 		writel_relaxed(val, base + UTMIP_PLL_CFG1);
 	}
@@ -812,6 +879,153 @@ static int ulpi_phy_power_off(struct tegra_usb_phy *phy)
 	return 0;
 }
 
+static u32 tegra_hsic_readl(struct tegra_usb_phy *phy, u32 reg)
+{
+	void __iomem *base = phy->regs;
+	u32 shift = phy->soc_config->uhsic_registers_shift;
+
+	return readl_relaxed(base + shift + reg);
+}
+
+static void tegra_hsic_writel(struct tegra_usb_phy *phy, u32 reg, u32 value)
+{
+	void __iomem *base = phy->regs;
+	u32 shift = phy->soc_config->uhsic_registers_shift;
+
+	writel_relaxed(value, base + shift + reg);
+}
+
+static int uhsic_phy_power_on(struct tegra_usb_phy *phy)
+{
+	struct tegra_utmip_config *config = phy->config;
+	void __iomem *base = phy->regs;
+	u32 val;
+
+	val = tegra_hsic_readl(phy, UHSIC_PADS_CFG1);
+	val &= ~(UHSIC_PD_BG | UHSIC_PD_TX | UHSIC_PD_TRK | UHSIC_PD_RX |
+		 UHSIC_PD_ZI | UHSIC_RPD_DATA | UHSIC_RPD_STROBE);
+	val |= UHSIC_RX_SEL;
+	tegra_hsic_writel(phy, UHSIC_PADS_CFG1, val);
+
+	udelay(2);
+
+	val = readl_relaxed(base + USB_SUSP_CTRL);
+	val |= UHSIC_RESET;
+	writel_relaxed(val, base + USB_SUSP_CTRL);
+
+	udelay(30);
+
+	val = readl_relaxed(base + USB_SUSP_CTRL);
+	val |= UHSIC_PHY_ENABLE;
+	writel_relaxed(val, base + USB_SUSP_CTRL);
+
+	val = tegra_hsic_readl(phy, UHSIC_HSRX_CFG0);
+	val &= ~(UHSIC_IDLE_WAIT(~0) |
+		 UHSIC_ELASTIC_UNDERRUN_LIMIT(~0) |
+		 UHSIC_ELASTIC_OVERRUN_LIMIT(~0));
+	val |= UHSIC_IDLE_WAIT(config->idle_wait_delay) |
+		UHSIC_ELASTIC_UNDERRUN_LIMIT(config->elastic_limit) |
+		UHSIC_ELASTIC_OVERRUN_LIMIT(config->elastic_limit);
+	tegra_hsic_writel(phy, UHSIC_HSRX_CFG0, val);
+
+	val = tegra_hsic_readl(phy, UHSIC_HSRX_CFG1);
+	val &= ~UHSIC_HS_SYNC_START_DLY(~0);
+	val |= UHSIC_HS_SYNC_START_DLY(config->hssync_start_delay);
+	tegra_hsic_writel(phy, UHSIC_HSRX_CFG1, val);
+
+	val = tegra_hsic_readl(phy, UHSIC_MISC_CFG0);
+	val |= UHSIC_SUSPEND_EXIT_ON_EDGE;
+	tegra_hsic_writel(phy, UHSIC_MISC_CFG0, val);
+
+	val = tegra_hsic_readl(phy, UHSIC_MISC_CFG1);
+	val &= ~UHSIC_PLLU_STABLE_COUNT(~0);
+	val |= UHSIC_PLLU_STABLE_COUNT(phy->freq->stable_count);
+	tegra_hsic_writel(phy, UHSIC_MISC_CFG1, val);
+
+	val = tegra_hsic_readl(phy, UHSIC_PLL_CFG1);
+	val &= ~(UHSIC_XTAL_FREQ_COUNT(~0) |
+		UHSIC_PLLU_ENABLE_DLY_COUNT(~0));
+	val |= UHSIC_XTAL_FREQ_COUNT(phy->freq->hsic_xtal_freq_count) |
+		UHSIC_PLLU_ENABLE_DLY_COUNT(phy->freq->enable_delay);
+	tegra_hsic_writel(phy, UHSIC_PLL_CFG1, val);
+
+	val = readl_relaxed(base + USB_SUSP_CTRL);
+	val &= ~UHSIC_RESET;
+	writel_relaxed(val, base + USB_SUSP_CTRL);
+
+	udelay(2);
+
+	if (phy->soc_config->requires_usbmode_setup) {
+		val = readl_relaxed(base + USB_USBMODE);
+		val &= ~USB_USBMODE_MASK;
+		if (phy->mode == USB_DR_MODE_HOST)
+			val |= USB_USBMODE_HOST;
+		else
+			val |= USB_USBMODE_DEVICE;
+		writel_relaxed(val, base + USB_USBMODE);
+	}
+
+	if (phy->soc_config->has_hostpc)
+		set_pts(phy, TEGRA_USB_HOSTPC1_DEVLC_PTS_HSIC);
+	else
+		set_pts(phy, 0);
+
+	val = readl_relaxed(base + USB_TXFILLTUNING);
+	if ((val & USB_FIFO_TXFILL_MASK) != USB_FIFO_TXFILL_THRES(0x10)) {
+		val = USB_FIFO_TXFILL_THRES(0x10);
+		writel_relaxed(val, base + USB_TXFILLTUNING);
+	}
+
+	if (phy->soc_config->has_hostpc) {
+		val = readl_relaxed(base + TEGRA30_USB_PORTSC1);
+		val &= ~(TEGRA_USB_PORTSC1_WKOC | TEGRA_USB_PORTSC1_WKDS |
+			 TEGRA_USB_PORTSC1_WKCN);
+		writel_relaxed(val, base + TEGRA30_USB_PORTSC1);
+	} else {
+		val = readl_relaxed(base + TEGRA_USB_PORTSC1);
+		val &= ~(TEGRA_USB_PORTSC1_WKOC | TEGRA_USB_PORTSC1_WKDS |
+			 TEGRA_USB_PORTSC1_WKCN);
+		writel_relaxed(val, base + TEGRA_USB_PORTSC1);
+	}
+
+	val = tegra_hsic_readl(phy, UHSIC_PADS_CFG0);
+	val &= ~UHSIC_TX_RTUNEN;
+	val |= UHSIC_TX_RTUNE(phy->soc_config->uhsic_tx_rtune);
+	tegra_hsic_writel(phy, UHSIC_PADS_CFG0, val);
+
+	if (utmi_wait_register(base + USB_SUSP_CTRL, USB_PHY_CLK_VALID,
+			       USB_PHY_CLK_VALID))
+		dev_err(phy->u_phy.dev,
+			"Timeout waiting for PHY to stabilize on enable (HSIC)\n");
+
+	return 0;
+}
+
+static int uhsic_phy_power_off(struct tegra_usb_phy *phy)
+{
+	void __iomem *base = phy->regs;
+	u32 val;
+
+	set_phcd(phy, true);
+
+	val = tegra_hsic_readl(phy, UHSIC_PADS_CFG1);
+	val |= (UHSIC_PD_BG | UHSIC_PD_TX | UHSIC_PD_TRK | UHSIC_PD_RX |
+		UHSIC_PD_ZI | UHSIC_RPD_DATA | UHSIC_RPD_STROBE);
+	tegra_hsic_writel(phy, UHSIC_PADS_CFG1, val);
+
+	val = readl_relaxed(base + USB_SUSP_CTRL);
+	val |= UHSIC_RESET;
+	writel_relaxed(val, base + USB_SUSP_CTRL);
+
+	udelay(30);
+
+	val = readl_relaxed(base + USB_SUSP_CTRL);
+	val &= ~UHSIC_PHY_ENABLE;
+	writel_relaxed(val, base + USB_SUSP_CTRL);
+
+	return 0;
+}
+
 static int tegra_usb_phy_power_on(struct tegra_usb_phy *phy)
 {
 	int err = 0;
@@ -828,6 +1042,10 @@ static int tegra_usb_phy_power_on(struct tegra_usb_phy *phy)
 		err = ulpi_phy_power_on(phy);
 		break;
 
+	case USBPHY_INTERFACE_MODE_HSIC:
+		err = uhsic_phy_power_on(phy);
+		break;
+
 	default:
 		break;
 	}
@@ -859,6 +1077,10 @@ static int tegra_usb_phy_power_off(struct tegra_usb_phy *phy)
 		err = ulpi_phy_power_off(phy);
 		break;
 
+	case USBPHY_INTERFACE_MODE_HSIC:
+		err = uhsic_phy_power_off(phy);
+		break;
+
 	default:
 		break;
 	}
@@ -1256,6 +1478,8 @@ static const struct tegra_phy_soc_config tegra20_soc_config = {
 	.requires_usbmode_setup = false,
 	.requires_extra_tuning_parameters = false,
 	.requires_pmc_ao_power_up = false,
+	.uhsic_registers_shift = 0,
+	.uhsic_tx_rtune = 0, /* 40 ohm */
 };
 
 static const struct tegra_phy_soc_config tegra30_soc_config = {
@@ -1264,6 +1488,8 @@ static const struct tegra_phy_soc_config tegra30_soc_config = {
 	.requires_usbmode_setup = true,
 	.requires_extra_tuning_parameters = true,
 	.requires_pmc_ao_power_up = true,
+	.uhsic_registers_shift = 0x400,
+	.uhsic_tx_rtune = 8,  /* 50 ohm */
 };
 
 static const struct of_device_id tegra_usb_phy_id_table[] = {
@@ -1360,6 +1586,7 @@ static int tegra_usb_phy_probe(struct platform_device *pdev)
 	tegra_phy->phy_type = of_usb_get_phy_mode(np);
 	switch (tegra_phy->phy_type) {
 	case USBPHY_INTERFACE_MODE_UTMI:
+	case USBPHY_INTERFACE_MODE_HSIC:
 		err = utmi_phy_probe(tegra_phy, pdev);
 		if (err)
 			return err;
diff --git a/include/linux/usb/tegra_usb_phy.h b/include/linux/usb/tegra_usb_phy.h
index e394f4880b7e..4e79f1c2173a 100644
--- a/include/linux/usb/tegra_usb_phy.h
+++ b/include/linux/usb/tegra_usb_phy.h
@@ -24,6 +24,9 @@ struct gpio_desc;
  * requires_extra_tuning_parameters: true if xcvr_hsslew, hssquelch_level
  *      and hsdiscon_level should be set for adequate signal quality
  * requires_pmc_ao_power_up: true if USB AO is powered down by default
+ * uhsic_registers_shift: for Tegra30+ where HSIC registers were shifted
+ *      comparing to Tegra20 by 0x400, since Tegra20 has no UTMIP on PHY2
+ * uhsic_tx_rtune: fine tuned 50 Ohm termination resistor for NMOS/PMOS driver
  */
 
 struct tegra_phy_soc_config {
@@ -32,6 +35,8 @@ struct tegra_phy_soc_config {
 	bool requires_usbmode_setup;
 	bool requires_extra_tuning_parameters;
 	bool requires_pmc_ao_power_up;
+	u32 uhsic_registers_shift;
+	u32 uhsic_tx_rtune;
 };
 
 struct tegra_utmip_config {
-- 
2.51.0

Re: [PATCH v3 2/2] usb: phy: tegra: add HSIC support

[Mikko Perttunen]

On Friday, January 23, 2026 12:11 AM Svyatoslav Ryhel wrote:
> Add support for HSIC USB mode, which can be set for second USB controller
> and PHY on Tegra SoC along with already supported UTMI or ULPI.
> 
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
>  drivers/usb/phy/phy-tegra-usb.c   | 249 ++++++++++++++++++++++++++++--
>  include/linux/usb/tegra_usb_phy.h |   5 +
>  2 files changed, 243 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c
> index afa5b5535f92..4f0fde33647e 100644
> --- a/drivers/usb/phy/phy-tegra-usb.c
> +++ b/drivers/usb/phy/phy-tegra-usb.c
> @@ -29,17 +29,26 @@
>  #include <linux/usb/tegra_usb_phy.h>
>  #include <linux/usb/ulpi.h>
>  
> +#define USB_TXFILLTUNING			0x154
> +#define USB_FIFO_TXFILL_THRES(x)		(((x) & 0x1f) << 16)
> +#define USB_FIFO_TXFILL_MASK			0x1f0000
> +
>  #define ULPI_VIEWPORT				0x170
>  
>  /* PORTSC PTS/PHCD bits, Tegra20 only */
>  #define TEGRA_USB_PORTSC1			0x184
> -#define TEGRA_USB_PORTSC1_PTS(x)		(((x) & 0x3) << 30)
> -#define TEGRA_USB_PORTSC1_PHCD			BIT(23)
> +#define   TEGRA_USB_PORTSC1_PTS(x)		(((x) & 0x3) << 30)
> +#define   TEGRA_USB_PORTSC1_PHCD		BIT(23)
> +#define   TEGRA_USB_PORTSC1_WKOC		BIT(22)
> +#define   TEGRA_USB_PORTSC1_WKDS		BIT(21)
> +#define   TEGRA_USB_PORTSC1_WKCN		BIT(20)
>  
>  /* HOSTPC1 PTS/PHCD bits, Tegra30 and above */
> +#define TEGRA30_USB_PORTSC1			0x174
>  #define TEGRA_USB_HOSTPC1_DEVLC			0x1b4
> -#define TEGRA_USB_HOSTPC1_DEVLC_PTS(x)		(((x) & 0x7) << 29)
> -#define TEGRA_USB_HOSTPC1_DEVLC_PHCD		BIT(22)
> +#define   TEGRA_USB_HOSTPC1_DEVLC_PTS(x)	(((x) & 0x7) << 29)
> +#define   TEGRA_USB_HOSTPC1_DEVLC_PHCD		BIT(22)
> +#define   TEGRA_USB_HOSTPC1_DEVLC_PTS_HSIC	BIT(2)

PTS is an enumeration, not a bitfield, so I would say '4' instead of 'BIT(2)'

>  
>  /* Bits of PORTSC1, which will get cleared by writing 1 into them */
>  #define TEGRA_PORTSC1_RWC_BITS	(PORT_CSC | PORT_PEC | PORT_OCC)
> @@ -51,11 +60,12 @@
>  #define   USB_SUSP_CLR				BIT(5)
>  #define   USB_PHY_CLK_VALID			BIT(7)
>  #define   UTMIP_RESET				BIT(11)
> -#define   UHSIC_RESET				BIT(11)
>  #define   UTMIP_PHY_ENABLE			BIT(12)
>  #define   ULPI_PHY_ENABLE			BIT(13)
>  #define   USB_SUSP_SET				BIT(14)
> +#define   UHSIC_RESET				BIT(14)
>  #define   USB_WAKEUP_DEBOUNCE_COUNT(x)		(((x) & 0x7) << 16)
> +#define   UHSIC_PHY_ENABLE			BIT(19)
>  
>  #define USB_PHY_VBUS_SENSORS			0x404
>  #define   B_SESS_VLD_WAKEUP_EN			BIT(14)
> @@ -156,6 +166,58 @@
>  #define UTMIP_BIAS_CFG1				0x83c
>  #define   UTMIP_BIAS_PDTRK_COUNT(x)		(((x) & 0x1f) << 3)
>  
> +/*
> + * Tegra20 has no UTMIP registers on PHY2 and UHSIC registers start from 0x800
> + * just where UTMIP registers should have been. This is the case only with Tegra20
> + * Tegra30+ have UTMIP registers at 0x800 and UHSIC registers shifter by 0x400

'shift', or 'are shifted'

> + * to 0xc00, but register layout is preserved.
> + */
> +#define UHSIC_PLL_CFG1				0x804
> +#define   UHSIC_XTAL_FREQ_COUNT(x)		(((x) & 0xfff) << 0)
> +#define   UHSIC_PLLU_ENABLE_DLY_COUNT(x)	(((x) & 0x1f) << 14)
> +
> +#define UHSIC_HSRX_CFG0				0x808
> +#define   UHSIC_ELASTIC_UNDERRUN_LIMIT(x)	(((x) & 0x1f) << 2)
> +#define   UHSIC_ELASTIC_OVERRUN_LIMIT(x)	(((x) & 0x1f) << 8)
> +#define   UHSIC_IDLE_WAIT(x)			(((x) & 0x1f) << 13)
> +
> +#define UHSIC_HSRX_CFG1				0x80c
> +#define   UHSIC_HS_SYNC_START_DLY(x)		(((x) & 0x1f) << 1)
> +
> +#define UHSIC_TX_CFG0				0x810
> +#define   UHSIC_HS_READY_WAIT_FOR_VALID		BIT(9)
> +
> +#define UHSIC_MISC_CFG0				0x814
> +#define   UHSIC_SUSPEND_EXIT_ON_EDGE		BIT(7)
> +#define   UHSIC_DETECT_SHORT_CONNECT		BIT(8)
> +#define   UHSIC_FORCE_XCVR_MODE			BIT(15)
> +#define   UHSIC_DISABLE_BUSRESET		BIT(20)
> +
> +#define UHSIC_MISC_CFG1				0x818
> +#define   UHSIC_PLLU_STABLE_COUNT(x)		(((x) & 0xfff) << 2)
> +
> +#define UHSIC_PADS_CFG0				0x81c
> +#define   UHSIC_TX_RTUNEN			0xf000
> +#define   UHSIC_TX_RTUNE(x)			(((x) & 0xf) << 12)
> +
> +#define UHSIC_PADS_CFG1				0x820
> +#define   UHSIC_PD_BG				BIT(2)
> +#define   UHSIC_PD_TX				BIT(3)
> +#define   UHSIC_PD_TRK				BIT(4)
> +#define   UHSIC_PD_RX				BIT(5)
> +#define   UHSIC_PD_ZI				BIT(6)
> +#define   UHSIC_RX_SEL				BIT(7)
> +#define   UHSIC_RPD_DATA			BIT(9)
> +#define   UHSIC_RPD_STROBE			BIT(10)
> +#define   UHSIC_RPU_DATA			BIT(11)
> +#define   UHSIC_RPU_STROBE			BIT(12)
> +
> +#define UHSIC_CMD_CFG0				0x824
> +#define   UHSIC_PRETEND_CONNECT_DETECT		BIT(5)
> +
> +#define UHSIC_STAT_CFG0				0x828
> +#define   UHSIC_CONNECT_DETECT			BIT(0)
> +
>  /* For Tegra30 and above only, the address is different in Tegra20 */
>  #define USB_USBMODE				0x1f8
>  #define   USB_USBMODE_MASK			(3 << 0)
> @@ -174,7 +236,8 @@ struct tegra_xtal_freq {
>  	u8 enable_delay;
>  	u8 stable_count;
>  	u8 active_delay;
> -	u8 xtal_freq_count;
> +	u8 utmi_xtal_freq_count;
> +	u16 hsic_xtal_freq_count;
>  	u16 debounce;
>  };
>  
> @@ -184,7 +247,8 @@ static const struct tegra_xtal_freq tegra_freq_table[] = {
>  		.enable_delay = 0x02,
>  		.stable_count = 0x2F,
>  		.active_delay = 0x04,
> -		.xtal_freq_count = 0x76,
> +		.utmi_xtal_freq_count = 0x76,
> +		.hsic_xtal_freq_count = 0x1CA,
>  		.debounce = 0x7530,
>  	},
>  	{
> @@ -192,7 +256,8 @@ static const struct tegra_xtal_freq tegra_freq_table[] = {
>  		.enable_delay = 0x02,
>  		.stable_count = 0x33,
>  		.active_delay = 0x05,
> -		.xtal_freq_count = 0x7F,
> +		.utmi_xtal_freq_count = 0x7F,
> +		.hsic_xtal_freq_count = 0x1F0,
>  		.debounce = 0x7EF4,
>  	},
>  	{
> @@ -200,7 +265,8 @@ static const struct tegra_xtal_freq tegra_freq_table[] = {
>  		.enable_delay = 0x03,
>  		.stable_count = 0x4B,
>  		.active_delay = 0x06,
> -		.xtal_freq_count = 0xBB,
> +		.utmi_xtal_freq_count = 0xBB,
> +		.hsic_xtal_freq_count = 0x2DD,
>  		.debounce = 0xBB80,
>  	},
>  	{
> @@ -208,7 +274,8 @@ static const struct tegra_xtal_freq tegra_freq_table[] = {
>  		.enable_delay = 0x04,
>  		.stable_count = 0x66,
>  		.active_delay = 0x09,
> -		.xtal_freq_count = 0xFE,
> +		.utmi_xtal_freq_count = 0xFE,
> +		.hsic_xtal_freq_count = 0x3E0,
>  		.debounce = 0xFDE8,
>  	},
>  };
> @@ -541,7 +608,7 @@ static int utmi_phy_power_on(struct tegra_usb_phy *phy)
>  		val = readl_relaxed(base + UTMIP_PLL_CFG1);
>  		val &= ~(UTMIP_XTAL_FREQ_COUNT(~0) |
>  			UTMIP_PLLU_ENABLE_DLY_COUNT(~0));
> -		val |= UTMIP_XTAL_FREQ_COUNT(phy->freq->xtal_freq_count) |
> +		val |= UTMIP_XTAL_FREQ_COUNT(phy->freq->utmi_xtal_freq_count) |
>  			UTMIP_PLLU_ENABLE_DLY_COUNT(phy->freq->enable_delay);
>  		writel_relaxed(val, base + UTMIP_PLL_CFG1);
>  	}
> @@ -812,6 +879,153 @@ static int ulpi_phy_power_off(struct tegra_usb_phy *phy)
>  	return 0;
>  }
>  
> +static u32 tegra_hsic_readl(struct tegra_usb_phy *phy, u32 reg)
> +{
> +	void __iomem *base = phy->regs;
> +	u32 shift = phy->soc_config->uhsic_registers_shift;
> +
> +	return readl_relaxed(base + shift + reg);
> +}
> +
> +static void tegra_hsic_writel(struct tegra_usb_phy *phy, u32 reg, u32 value)
> +{
> +	void __iomem *base = phy->regs;
> +	u32 shift = phy->soc_config->uhsic_registers_shift;
> +
> +	writel_relaxed(value, base + shift + reg);
> +}
> +
> +static int uhsic_phy_power_on(struct tegra_usb_phy *phy)
> +{
> +	struct tegra_utmip_config *config = phy->config;
> +	void __iomem *base = phy->regs;
> +	u32 val;
> +
> +	val = tegra_hsic_readl(phy, UHSIC_PADS_CFG1);
> +	val &= ~(UHSIC_PD_BG | UHSIC_PD_TX | UHSIC_PD_TRK | UHSIC_PD_RX |
> +		 UHSIC_PD_ZI | UHSIC_RPD_DATA | UHSIC_RPD_STROBE);
> +	val |= UHSIC_RX_SEL;

L4T r16 (Tegra30) keeps UHSIC_PD_TX set until after UHSIC_RESET has been cleared. Commit message says this avoids a signal glitch.

> +	tegra_hsic_writel(phy, UHSIC_PADS_CFG1, val);
> +
> +	udelay(2);
> +
> +	val = readl_relaxed(base + USB_SUSP_CTRL);
> +	val |= UHSIC_RESET;
> +	writel_relaxed(val, base + USB_SUSP_CTRL);
> +
> +	udelay(30);
> +
> +	val = readl_relaxed(base + USB_SUSP_CTRL);
> +	val |= UHSIC_PHY_ENABLE;
> +	writel_relaxed(val, base + USB_SUSP_CTRL);
> +
> +	val = tegra_hsic_readl(phy, UHSIC_HSRX_CFG0);
> +	val &= ~(UHSIC_IDLE_WAIT(~0) |
> +		 UHSIC_ELASTIC_UNDERRUN_LIMIT(~0) |
> +		 UHSIC_ELASTIC_OVERRUN_LIMIT(~0));
> +	val |= UHSIC_IDLE_WAIT(config->idle_wait_delay) |
> +		UHSIC_ELASTIC_UNDERRUN_LIMIT(config->elastic_limit) |
> +		UHSIC_ELASTIC_OVERRUN_LIMIT(config->elastic_limit);
> +	tegra_hsic_writel(phy, UHSIC_HSRX_CFG0, val);
> +
> +	val = tegra_hsic_readl(phy, UHSIC_HSRX_CFG1);
> +	val &= ~UHSIC_HS_SYNC_START_DLY(~0);
> +	val |= UHSIC_HS_SYNC_START_DLY(config->hssync_start_delay);
> +	tegra_hsic_writel(phy, UHSIC_HSRX_CFG1, val);
> +

L4T r16 (Tegra30) clears UHSIC_TX_CFG0.UHSIC_HS_READY_WAIT_FOR_VALID here. According to commit message this fixes an intermittent failure to connect to HSIC modem. It also sets UHSIC_MISC_CFG0.UHSIC_DISABLE_BUSRESET  below due to same issue.

> +	val = tegra_hsic_readl(phy, UHSIC_MISC_CFG0);
> +	val |= UHSIC_SUSPEND_EXIT_ON_EDGE;
> +	tegra_hsic_writel(phy, UHSIC_MISC_CFG0, val);
> +
> +	val = tegra_hsic_readl(phy, UHSIC_MISC_CFG1);
> +	val &= ~UHSIC_PLLU_STABLE_COUNT(~0);
> +	val |= UHSIC_PLLU_STABLE_COUNT(phy->freq->stable_count);
> +	tegra_hsic_writel(phy, UHSIC_MISC_CFG1, val);
> +
> +	val = tegra_hsic_readl(phy, UHSIC_PLL_CFG1);
> +	val &= ~(UHSIC_XTAL_FREQ_COUNT(~0) |
> +		UHSIC_PLLU_ENABLE_DLY_COUNT(~0));
> +	val |= UHSIC_XTAL_FREQ_COUNT(phy->freq->hsic_xtal_freq_count) |
> +		UHSIC_PLLU_ENABLE_DLY_COUNT(phy->freq->enable_delay);
> +	tegra_hsic_writel(phy, UHSIC_PLL_CFG1, val);
> +
> +	val = readl_relaxed(base + USB_SUSP_CTRL);
> +	val &= ~UHSIC_RESET;
> +	writel_relaxed(val, base + USB_SUSP_CTRL);
> +
> +	udelay(2);
> +
> +	if (phy->soc_config->requires_usbmode_setup) {
> +		val = readl_relaxed(base + USB_USBMODE);
> +		val &= ~USB_USBMODE_MASK;
> +		if (phy->mode == USB_DR_MODE_HOST)
> +			val |= USB_USBMODE_HOST;
> +		else
> +			val |= USB_USBMODE_DEVICE;
> +		writel_relaxed(val, base + USB_USBMODE);
> +	}
> +
> +	if (phy->soc_config->has_hostpc)
> +		set_pts(phy, TEGRA_USB_HOSTPC1_DEVLC_PTS_HSIC);
> +	else
> +		set_pts(phy, 0);

This (and below) are abusing has_hostpc to detect Tegra30 vs Tegra20. We should instead add soc_config fields hsic_pts_value and portsc1_offset.

> +
> +	val = readl_relaxed(base + USB_TXFILLTUNING);
> +	if ((val & USB_FIFO_TXFILL_MASK) != USB_FIFO_TXFILL_THRES(0x10)) {
> +		val = USB_FIFO_TXFILL_THRES(0x10);
> +		writel_relaxed(val, base + USB_TXFILLTUNING);
> +	}
> +
> +	if (phy->soc_config->has_hostpc) {
> +		val = readl_relaxed(base + TEGRA30_USB_PORTSC1);
> +		val &= ~(TEGRA_USB_PORTSC1_WKOC | TEGRA_USB_PORTSC1_WKDS |
> +			 TEGRA_USB_PORTSC1_WKCN);
> +		writel_relaxed(val, base + TEGRA30_USB_PORTSC1);
> +	} else {
> +		val = readl_relaxed(base + TEGRA_USB_PORTSC1);
> +		val &= ~(TEGRA_USB_PORTSC1_WKOC | TEGRA_USB_PORTSC1_WKDS |
> +			 TEGRA_USB_PORTSC1_WKCN);
> +		writel_relaxed(val, base + TEGRA_USB_PORTSC1);
> +	}
> +
> +	val = tegra_hsic_readl(phy, UHSIC_PADS_CFG0);
> +	val &= ~UHSIC_TX_RTUNEN;
> +	val |= UHSIC_TX_RTUNE(phy->soc_config->uhsic_tx_rtune);
> +	tegra_hsic_writel(phy, UHSIC_PADS_CFG0, val);
> +
> +	if (utmi_wait_register(base + USB_SUSP_CTRL, USB_PHY_CLK_VALID,
> +			       USB_PHY_CLK_VALID))
> +		dev_err(phy->u_phy.dev,
> +			"Timeout waiting for PHY to stabilize on enable (HSIC)\n");

Should return error (return value of utmi_wait_register) here?

> +
> +	return 0;
> +}
> +
> +static int uhsic_phy_power_off(struct tegra_usb_phy *phy)
> +{
> +	void __iomem *base = phy->regs;
> +	u32 val;
> +
> +	set_phcd(phy, true);
> +
> +	val = tegra_hsic_readl(phy, UHSIC_PADS_CFG1);
> +	val |= (UHSIC_PD_BG | UHSIC_PD_TX | UHSIC_PD_TRK | UHSIC_PD_RX |
> +		UHSIC_PD_ZI | UHSIC_RPD_DATA | UHSIC_RPD_STROBE);
> +	tegra_hsic_writel(phy, UHSIC_PADS_CFG1, val);
> +
> +	val = readl_relaxed(base + USB_SUSP_CTRL);
> +	val |= UHSIC_RESET;
> +	writel_relaxed(val, base + USB_SUSP_CTRL);
> +
> +	udelay(30);
> +
> +	val = readl_relaxed(base + USB_SUSP_CTRL);
> +	val &= ~UHSIC_PHY_ENABLE;
> +	writel_relaxed(val, base + USB_SUSP_CTRL);
> +
> +	return 0;
> +}
> +
>  static int tegra_usb_phy_power_on(struct tegra_usb_phy *phy)
>  {
>  	int err = 0;
> @@ -828,6 +1042,10 @@ static int tegra_usb_phy_power_on(struct tegra_usb_phy *phy)
>  		err = ulpi_phy_power_on(phy);
>  		break;
>  
> +	case USBPHY_INTERFACE_MODE_HSIC:
> +		err = uhsic_phy_power_on(phy);
> +		break;
> +
>  	default:
>  		break;
>  	}
> @@ -859,6 +1077,10 @@ static int tegra_usb_phy_power_off(struct tegra_usb_phy *phy)
>  		err = ulpi_phy_power_off(phy);
>  		break;
>  
> +	case USBPHY_INTERFACE_MODE_HSIC:
> +		err = uhsic_phy_power_off(phy);
> +		break;
> +
>  	default:
>  		break;
>  	}
> @@ -1256,6 +1478,8 @@ static const struct tegra_phy_soc_config tegra20_soc_config = {
>  	.requires_usbmode_setup = false,
>  	.requires_extra_tuning_parameters = false,
>  	.requires_pmc_ao_power_up = false,
> +	.uhsic_registers_shift = 0,
> +	.uhsic_tx_rtune = 0, /* 40 ohm */
>  };
>  
>  static const struct tegra_phy_soc_config tegra30_soc_config = {
> @@ -1264,6 +1488,8 @@ static const struct tegra_phy_soc_config tegra30_soc_config = {
>  	.requires_usbmode_setup = true,
>  	.requires_extra_tuning_parameters = true,
>  	.requires_pmc_ao_power_up = true,
> +	.uhsic_registers_shift = 0x400,
> +	.uhsic_tx_rtune = 8,  /* 50 ohm */
>  };
>  
>  static const struct of_device_id tegra_usb_phy_id_table[] = {
> @@ -1360,6 +1586,7 @@ static int tegra_usb_phy_probe(struct platform_device *pdev)
>  	tegra_phy->phy_type = of_usb_get_phy_mode(np);
>  	switch (tegra_phy->phy_type) {
>  	case USBPHY_INTERFACE_MODE_UTMI:
> +	case USBPHY_INTERFACE_MODE_HSIC:
>  		err = utmi_phy_probe(tegra_phy, pdev);
>  		if (err)
>  			return err;
> diff --git a/include/linux/usb/tegra_usb_phy.h b/include/linux/usb/tegra_usb_phy.h
> index e394f4880b7e..4e79f1c2173a 100644
> --- a/include/linux/usb/tegra_usb_phy.h
> +++ b/include/linux/usb/tegra_usb_phy.h
> @@ -24,6 +24,9 @@ struct gpio_desc;
>   * requires_extra_tuning_parameters: true if xcvr_hsslew, hssquelch_level
>   *      and hsdiscon_level should be set for adequate signal quality
>   * requires_pmc_ao_power_up: true if USB AO is powered down by default
> + * uhsic_registers_shift: for Tegra30+ where HSIC registers were shifted
> + *      comparing to Tegra20 by 0x400, since Tegra20 has no UTMIP on PHY2
> + * uhsic_tx_rtune: fine tuned 50 Ohm termination resistor for NMOS/PMOS driver
>   */
>  
>  struct tegra_phy_soc_config {
> @@ -32,6 +35,8 @@ struct tegra_phy_soc_config {
>  	bool requires_usbmode_setup;
>  	bool requires_extra_tuning_parameters;
>  	bool requires_pmc_ao_power_up;
> +	u32 uhsic_registers_shift;

I would rather call this 'uhsic_registers_offset'. Shift first brings to mind bitshifts, and we often have fields in these config structs for bit shifts called '*_shift'.

> +	u32 uhsic_tx_rtune;
>  };
>  
>  struct tegra_utmip_config {
> 

Re: [PATCH v3 2/2] usb: phy: tegra: add HSIC support

[Svyatoslav Ryhel]

пн, 2 лют. 2026 р. о 06:07 Mikko Perttunen <mperttunen@nvidia.com> пише:
>
> On Friday, January 23, 2026 12:11 AM Svyatoslav Ryhel wrote:
> > Add support for HSIC USB mode, which can be set for second USB controller
> > and PHY on Tegra SoC along with already supported UTMI or ULPI.
> >
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > ---
> >  drivers/usb/phy/phy-tegra-usb.c   | 249 ++++++++++++++++++++++++++++--
> >  include/linux/usb/tegra_usb_phy.h |   5 +
> >  2 files changed, 243 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c
> > index afa5b5535f92..4f0fde33647e 100644
> > --- a/drivers/usb/phy/phy-tegra-usb.c
> > +++ b/drivers/usb/phy/phy-tegra-usb.c
> > @@ -29,17 +29,26 @@
> >  #include <linux/usb/tegra_usb_phy.h>
> >  #include <linux/usb/ulpi.h>
> >
> > +#define USB_TXFILLTUNING                     0x154
> > +#define USB_FIFO_TXFILL_THRES(x)             (((x) & 0x1f) << 16)
> > +#define USB_FIFO_TXFILL_MASK                 0x1f0000
> > +
> >  #define ULPI_VIEWPORT                                0x170
> >
> >  /* PORTSC PTS/PHCD bits, Tegra20 only */
> >  #define TEGRA_USB_PORTSC1                    0x184
> > -#define TEGRA_USB_PORTSC1_PTS(x)             (((x) & 0x3) << 30)
> > -#define TEGRA_USB_PORTSC1_PHCD                       BIT(23)
> > +#define   TEGRA_USB_PORTSC1_PTS(x)           (((x) & 0x3) << 30)
> > +#define   TEGRA_USB_PORTSC1_PHCD             BIT(23)
> > +#define   TEGRA_USB_PORTSC1_WKOC             BIT(22)
> > +#define   TEGRA_USB_PORTSC1_WKDS             BIT(21)
> > +#define   TEGRA_USB_PORTSC1_WKCN             BIT(20)
> >
> >  /* HOSTPC1 PTS/PHCD bits, Tegra30 and above */
> > +#define TEGRA30_USB_PORTSC1                  0x174
> >  #define TEGRA_USB_HOSTPC1_DEVLC                      0x1b4
> > -#define TEGRA_USB_HOSTPC1_DEVLC_PTS(x)               (((x) & 0x7) << 29)
> > -#define TEGRA_USB_HOSTPC1_DEVLC_PHCD         BIT(22)
> > +#define   TEGRA_USB_HOSTPC1_DEVLC_PTS(x)     (((x) & 0x7) << 29)
> > +#define   TEGRA_USB_HOSTPC1_DEVLC_PHCD               BIT(22)
> > +#define   TEGRA_USB_HOSTPC1_DEVLC_PTS_HSIC   BIT(2)
>
> PTS is an enumeration, not a bitfield, so I would say '4' instead of 'BIT(2)'
>

Noted

> >
> >  /* Bits of PORTSC1, which will get cleared by writing 1 into them */
> >  #define TEGRA_PORTSC1_RWC_BITS       (PORT_CSC | PORT_PEC | PORT_OCC)
> > @@ -51,11 +60,12 @@
> >  #define   USB_SUSP_CLR                               BIT(5)
> >  #define   USB_PHY_CLK_VALID                  BIT(7)
> >  #define   UTMIP_RESET                                BIT(11)
> > -#define   UHSIC_RESET                                BIT(11)
> >  #define   UTMIP_PHY_ENABLE                   BIT(12)
> >  #define   ULPI_PHY_ENABLE                    BIT(13)
> >  #define   USB_SUSP_SET                               BIT(14)
> > +#define   UHSIC_RESET                                BIT(14)
> >  #define   USB_WAKEUP_DEBOUNCE_COUNT(x)               (((x) & 0x7) << 16)
> > +#define   UHSIC_PHY_ENABLE                   BIT(19)
> >
> >  #define USB_PHY_VBUS_SENSORS                 0x404
> >  #define   B_SESS_VLD_WAKEUP_EN                       BIT(14)
> > @@ -156,6 +166,58 @@
> >  #define UTMIP_BIAS_CFG1                              0x83c
> >  #define   UTMIP_BIAS_PDTRK_COUNT(x)          (((x) & 0x1f) << 3)
> >
> > +/*
> > + * Tegra20 has no UTMIP registers on PHY2 and UHSIC registers start from 0x800
> > + * just where UTMIP registers should have been. This is the case only with Tegra20
> > + * Tegra30+ have UTMIP registers at 0x800 and UHSIC registers shifter by 0x400
>
> 'shift', or 'are shifted'
>

Noted

> > + * to 0xc00, but register layout is preserved.
> > + */
> > +#define UHSIC_PLL_CFG1                               0x804
> > +#define   UHSIC_XTAL_FREQ_COUNT(x)           (((x) & 0xfff) << 0)
> > +#define   UHSIC_PLLU_ENABLE_DLY_COUNT(x)     (((x) & 0x1f) << 14)
> > +
> > +#define UHSIC_HSRX_CFG0                              0x808
> > +#define   UHSIC_ELASTIC_UNDERRUN_LIMIT(x)    (((x) & 0x1f) << 2)
> > +#define   UHSIC_ELASTIC_OVERRUN_LIMIT(x)     (((x) & 0x1f) << 8)
> > +#define   UHSIC_IDLE_WAIT(x)                 (((x) & 0x1f) << 13)
> > +
> > +#define UHSIC_HSRX_CFG1                              0x80c
> > +#define   UHSIC_HS_SYNC_START_DLY(x)         (((x) & 0x1f) << 1)
> > +
> > +#define UHSIC_TX_CFG0                                0x810
> > +#define   UHSIC_HS_READY_WAIT_FOR_VALID              BIT(9)
> > +
> > +#define UHSIC_MISC_CFG0                              0x814
> > +#define   UHSIC_SUSPEND_EXIT_ON_EDGE         BIT(7)
> > +#define   UHSIC_DETECT_SHORT_CONNECT         BIT(8)
> > +#define   UHSIC_FORCE_XCVR_MODE                      BIT(15)
> > +#define   UHSIC_DISABLE_BUSRESET             BIT(20)
> > +
> > +#define UHSIC_MISC_CFG1                              0x818
> > +#define   UHSIC_PLLU_STABLE_COUNT(x)         (((x) & 0xfff) << 2)
> > +
> > +#define UHSIC_PADS_CFG0                              0x81c
> > +#define   UHSIC_TX_RTUNEN                    0xf000
> > +#define   UHSIC_TX_RTUNE(x)                  (((x) & 0xf) << 12)
> > +
> > +#define UHSIC_PADS_CFG1                              0x820
> > +#define   UHSIC_PD_BG                                BIT(2)
> > +#define   UHSIC_PD_TX                                BIT(3)
> > +#define   UHSIC_PD_TRK                               BIT(4)
> > +#define   UHSIC_PD_RX                                BIT(5)
> > +#define   UHSIC_PD_ZI                                BIT(6)
> > +#define   UHSIC_RX_SEL                               BIT(7)
> > +#define   UHSIC_RPD_DATA                     BIT(9)
> > +#define   UHSIC_RPD_STROBE                   BIT(10)
> > +#define   UHSIC_RPU_DATA                     BIT(11)
> > +#define   UHSIC_RPU_STROBE                   BIT(12)
> > +
> > +#define UHSIC_CMD_CFG0                               0x824
> > +#define   UHSIC_PRETEND_CONNECT_DETECT               BIT(5)
> > +
> > +#define UHSIC_STAT_CFG0                              0x828
> > +#define   UHSIC_CONNECT_DETECT                       BIT(0)
> > +
> >  /* For Tegra30 and above only, the address is different in Tegra20 */
> >  #define USB_USBMODE                          0x1f8
> >  #define   USB_USBMODE_MASK                   (3 << 0)
> > @@ -174,7 +236,8 @@ struct tegra_xtal_freq {
> >       u8 enable_delay;
> >       u8 stable_count;
> >       u8 active_delay;
> > -     u8 xtal_freq_count;
> > +     u8 utmi_xtal_freq_count;
> > +     u16 hsic_xtal_freq_count;
> >       u16 debounce;
> >  };
> >
> > @@ -184,7 +247,8 @@ static const struct tegra_xtal_freq tegra_freq_table[] = {
> >               .enable_delay = 0x02,
> >               .stable_count = 0x2F,
> >               .active_delay = 0x04,
> > -             .xtal_freq_count = 0x76,
> > +             .utmi_xtal_freq_count = 0x76,
> > +             .hsic_xtal_freq_count = 0x1CA,
> >               .debounce = 0x7530,
> >       },
> >       {
> > @@ -192,7 +256,8 @@ static const struct tegra_xtal_freq tegra_freq_table[] = {
> >               .enable_delay = 0x02,
> >               .stable_count = 0x33,
> >               .active_delay = 0x05,
> > -             .xtal_freq_count = 0x7F,
> > +             .utmi_xtal_freq_count = 0x7F,
> > +             .hsic_xtal_freq_count = 0x1F0,
> >               .debounce = 0x7EF4,
> >       },
> >       {
> > @@ -200,7 +265,8 @@ static const struct tegra_xtal_freq tegra_freq_table[] = {
> >               .enable_delay = 0x03,
> >               .stable_count = 0x4B,
> >               .active_delay = 0x06,
> > -             .xtal_freq_count = 0xBB,
> > +             .utmi_xtal_freq_count = 0xBB,
> > +             .hsic_xtal_freq_count = 0x2DD,
> >               .debounce = 0xBB80,
> >       },
> >       {
> > @@ -208,7 +274,8 @@ static const struct tegra_xtal_freq tegra_freq_table[] = {
> >               .enable_delay = 0x04,
> >               .stable_count = 0x66,
> >               .active_delay = 0x09,
> > -             .xtal_freq_count = 0xFE,
> > +             .utmi_xtal_freq_count = 0xFE,
> > +             .hsic_xtal_freq_count = 0x3E0,
> >               .debounce = 0xFDE8,
> >       },
> >  };
> > @@ -541,7 +608,7 @@ static int utmi_phy_power_on(struct tegra_usb_phy *phy)
> >               val = readl_relaxed(base + UTMIP_PLL_CFG1);
> >               val &= ~(UTMIP_XTAL_FREQ_COUNT(~0) |
> >                       UTMIP_PLLU_ENABLE_DLY_COUNT(~0));
> > -             val |= UTMIP_XTAL_FREQ_COUNT(phy->freq->xtal_freq_count) |
> > +             val |= UTMIP_XTAL_FREQ_COUNT(phy->freq->utmi_xtal_freq_count) |
> >                       UTMIP_PLLU_ENABLE_DLY_COUNT(phy->freq->enable_delay);
> >               writel_relaxed(val, base + UTMIP_PLL_CFG1);
> >       }
> > @@ -812,6 +879,153 @@ static int ulpi_phy_power_off(struct tegra_usb_phy *phy)
> >       return 0;
> >  }
> >
> > +static u32 tegra_hsic_readl(struct tegra_usb_phy *phy, u32 reg)
> > +{
> > +     void __iomem *base = phy->regs;
> > +     u32 shift = phy->soc_config->uhsic_registers_shift;
> > +
> > +     return readl_relaxed(base + shift + reg);
> > +}
> > +
> > +static void tegra_hsic_writel(struct tegra_usb_phy *phy, u32 reg, u32 value)
> > +{
> > +     void __iomem *base = phy->regs;
> > +     u32 shift = phy->soc_config->uhsic_registers_shift;
> > +
> > +     writel_relaxed(value, base + shift + reg);
> > +}
> > +
> > +static int uhsic_phy_power_on(struct tegra_usb_phy *phy)
> > +{
> > +     struct tegra_utmip_config *config = phy->config;
> > +     void __iomem *base = phy->regs;
> > +     u32 val;
> > +
> > +     val = tegra_hsic_readl(phy, UHSIC_PADS_CFG1);
> > +     val &= ~(UHSIC_PD_BG | UHSIC_PD_TX | UHSIC_PD_TRK | UHSIC_PD_RX |
> > +              UHSIC_PD_ZI | UHSIC_RPD_DATA | UHSIC_RPD_STROBE);
> > +     val |= UHSIC_RX_SEL;
>
> L4T r16 (Tegra30) keeps UHSIC_PD_TX set until after UHSIC_RESET has been cleared. Commit message says this avoids a signal glitch.
>

I did not notice any difference with this change and without. Since it
does not affect detection of modem by my device I can apply it (is it
worth applying at all?). Should it be applied globally or for Tegra30+
only?

> > +     tegra_hsic_writel(phy, UHSIC_PADS_CFG1, val);
> > +
> > +     udelay(2);
> > +
> > +     val = readl_relaxed(base + USB_SUSP_CTRL);
> > +     val |= UHSIC_RESET;
> > +     writel_relaxed(val, base + USB_SUSP_CTRL);
> > +
> > +     udelay(30);
> > +
> > +     val = readl_relaxed(base + USB_SUSP_CTRL);
> > +     val |= UHSIC_PHY_ENABLE;
> > +     writel_relaxed(val, base + USB_SUSP_CTRL);
> > +
> > +     val = tegra_hsic_readl(phy, UHSIC_HSRX_CFG0);
> > +     val &= ~(UHSIC_IDLE_WAIT(~0) |
> > +              UHSIC_ELASTIC_UNDERRUN_LIMIT(~0) |
> > +              UHSIC_ELASTIC_OVERRUN_LIMIT(~0));
> > +     val |= UHSIC_IDLE_WAIT(config->idle_wait_delay) |
> > +             UHSIC_ELASTIC_UNDERRUN_LIMIT(config->elastic_limit) |
> > +             UHSIC_ELASTIC_OVERRUN_LIMIT(config->elastic_limit);
> > +     tegra_hsic_writel(phy, UHSIC_HSRX_CFG0, val);
> > +
> > +     val = tegra_hsic_readl(phy, UHSIC_HSRX_CFG1);
> > +     val &= ~UHSIC_HS_SYNC_START_DLY(~0);
> > +     val |= UHSIC_HS_SYNC_START_DLY(config->hssync_start_delay);
> > +     tegra_hsic_writel(phy, UHSIC_HSRX_CFG1, val);
> > +
>
> L4T r16 (Tegra30) clears UHSIC_TX_CFG0.UHSIC_HS_READY_WAIT_FOR_VALID here. According to commit message this fixes an intermittent failure to connect to HSIC modem. It also sets UHSIC_MISC_CFG0.UHSIC_DISABLE_BUSRESET  below due to same issue.
>

And this change causes my modem to fail entirely.

[   10.145976] usb 1-1: new high-speed USB device number 2 using ci_hdrc
[   10.295970] usb 1-1: device descriptor read/64, error -71
[   10.545975] usb 1-1: device descriptor read/64, error -71
[   10.795990] usb 1-1: new high-speed USB device number 3 using ci_hdrc
[   10.935970] usb 1-1: device descriptor read/64, error -71
[   11.185977] usb 1-1: device descriptor read/64, error -71

I assume UHSIC_DISABLE_BUSRESET, UHSIC_HS_READY_WAIT_FOR_VALID and
UHSIC_PD_TX workarounds are all consequences of how modem in
downstream kernel is called. Instead of relaying on modem to lead the
controller probe, downstream just brutally removes and reinits
controller until modem probes. I have never observed modem not
appearing without any of discussed patches.


> > +     val = tegra_hsic_readl(phy, UHSIC_MISC_CFG0);
> > +     val |= UHSIC_SUSPEND_EXIT_ON_EDGE;
> > +     tegra_hsic_writel(phy, UHSIC_MISC_CFG0, val);
> > +
> > +     val = tegra_hsic_readl(phy, UHSIC_MISC_CFG1);
> > +     val &= ~UHSIC_PLLU_STABLE_COUNT(~0);
> > +     val |= UHSIC_PLLU_STABLE_COUNT(phy->freq->stable_count);
> > +     tegra_hsic_writel(phy, UHSIC_MISC_CFG1, val);
> > +
> > +     val = tegra_hsic_readl(phy, UHSIC_PLL_CFG1);
> > +     val &= ~(UHSIC_XTAL_FREQ_COUNT(~0) |
> > +             UHSIC_PLLU_ENABLE_DLY_COUNT(~0));
> > +     val |= UHSIC_XTAL_FREQ_COUNT(phy->freq->hsic_xtal_freq_count) |
> > +             UHSIC_PLLU_ENABLE_DLY_COUNT(phy->freq->enable_delay);
> > +     tegra_hsic_writel(phy, UHSIC_PLL_CFG1, val);
> > +
> > +     val = readl_relaxed(base + USB_SUSP_CTRL);
> > +     val &= ~UHSIC_RESET;
> > +     writel_relaxed(val, base + USB_SUSP_CTRL);
> > +
> > +     udelay(2);
> > +
> > +     if (phy->soc_config->requires_usbmode_setup) {
> > +             val = readl_relaxed(base + USB_USBMODE);
> > +             val &= ~USB_USBMODE_MASK;
> > +             if (phy->mode == USB_DR_MODE_HOST)
> > +                     val |= USB_USBMODE_HOST;
> > +             else
> > +                     val |= USB_USBMODE_DEVICE;
> > +             writel_relaxed(val, base + USB_USBMODE);
> > +     }
> > +
> > +     if (phy->soc_config->has_hostpc)
> > +             set_pts(phy, TEGRA_USB_HOSTPC1_DEVLC_PTS_HSIC);
> > +     else
> > +             set_pts(phy, 0);
>
> This (and below) are abusing has_hostpc to detect Tegra30 vs Tegra20. We should instead add soc_config fields hsic_pts_value and portsc1_offset.
>
> > +
> > +     val = readl_relaxed(base + USB_TXFILLTUNING);
> > +     if ((val & USB_FIFO_TXFILL_MASK) != USB_FIFO_TXFILL_THRES(0x10)) {
> > +             val = USB_FIFO_TXFILL_THRES(0x10);
> > +             writel_relaxed(val, base + USB_TXFILLTUNING);
> > +     }
> > +
> > +     if (phy->soc_config->has_hostpc) {
> > +             val = readl_relaxed(base + TEGRA30_USB_PORTSC1);
> > +             val &= ~(TEGRA_USB_PORTSC1_WKOC | TEGRA_USB_PORTSC1_WKDS |
> > +                      TEGRA_USB_PORTSC1_WKCN);
> > +             writel_relaxed(val, base + TEGRA30_USB_PORTSC1);
> > +     } else {
> > +             val = readl_relaxed(base + TEGRA_USB_PORTSC1);
> > +             val &= ~(TEGRA_USB_PORTSC1_WKOC | TEGRA_USB_PORTSC1_WKDS |
> > +                      TEGRA_USB_PORTSC1_WKCN);
> > +             writel_relaxed(val, base + TEGRA_USB_PORTSC1);
> > +     }
> > +
> > +     val = tegra_hsic_readl(phy, UHSIC_PADS_CFG0);
> > +     val &= ~UHSIC_TX_RTUNEN;
> > +     val |= UHSIC_TX_RTUNE(phy->soc_config->uhsic_tx_rtune);
> > +     tegra_hsic_writel(phy, UHSIC_PADS_CFG0, val);
> > +
> > +     if (utmi_wait_register(base + USB_SUSP_CTRL, USB_PHY_CLK_VALID,
> > +                            USB_PHY_CLK_VALID))
> > +             dev_err(phy->u_phy.dev,
> > +                     "Timeout waiting for PHY to stabilize on enable (HSIC)\n");
>
> Should return error (return value of utmi_wait_register) here?
>

Noted.

> > +
> > +     return 0;
> > +}
> > +
> > +static int uhsic_phy_power_off(struct tegra_usb_phy *phy)
> > +{
> > +     void __iomem *base = phy->regs;
> > +     u32 val;
> > +
> > +     set_phcd(phy, true);
> > +
> > +     val = tegra_hsic_readl(phy, UHSIC_PADS_CFG1);
> > +     val |= (UHSIC_PD_BG | UHSIC_PD_TX | UHSIC_PD_TRK | UHSIC_PD_RX |
> > +             UHSIC_PD_ZI | UHSIC_RPD_DATA | UHSIC_RPD_STROBE);
> > +     tegra_hsic_writel(phy, UHSIC_PADS_CFG1, val);
> > +
> > +     val = readl_relaxed(base + USB_SUSP_CTRL);
> > +     val |= UHSIC_RESET;
> > +     writel_relaxed(val, base + USB_SUSP_CTRL);
> > +
> > +     udelay(30);
> > +
> > +     val = readl_relaxed(base + USB_SUSP_CTRL);
> > +     val &= ~UHSIC_PHY_ENABLE;
> > +     writel_relaxed(val, base + USB_SUSP_CTRL);
> > +
> > +     return 0;
> > +}
> > +
> >  static int tegra_usb_phy_power_on(struct tegra_usb_phy *phy)
> >  {
> >       int err = 0;
> > @@ -828,6 +1042,10 @@ static int tegra_usb_phy_power_on(struct tegra_usb_phy *phy)
> >               err = ulpi_phy_power_on(phy);
> >               break;
> >
> > +     case USBPHY_INTERFACE_MODE_HSIC:
> > +             err = uhsic_phy_power_on(phy);
> > +             break;
> > +
> >       default:
> >               break;
> >       }
> > @@ -859,6 +1077,10 @@ static int tegra_usb_phy_power_off(struct tegra_usb_phy *phy)
> >               err = ulpi_phy_power_off(phy);
> >               break;
> >
> > +     case USBPHY_INTERFACE_MODE_HSIC:
> > +             err = uhsic_phy_power_off(phy);
> > +             break;
> > +
> >       default:
> >               break;
> >       }
> > @@ -1256,6 +1478,8 @@ static const struct tegra_phy_soc_config tegra20_soc_config = {
> >       .requires_usbmode_setup = false,
> >       .requires_extra_tuning_parameters = false,
> >       .requires_pmc_ao_power_up = false,
> > +     .uhsic_registers_shift = 0,
> > +     .uhsic_tx_rtune = 0, /* 40 ohm */
> >  };
> >
> >  static const struct tegra_phy_soc_config tegra30_soc_config = {
> > @@ -1264,6 +1488,8 @@ static const struct tegra_phy_soc_config tegra30_soc_config = {
> >       .requires_usbmode_setup = true,
> >       .requires_extra_tuning_parameters = true,
> >       .requires_pmc_ao_power_up = true,
> > +     .uhsic_registers_shift = 0x400,
> > +     .uhsic_tx_rtune = 8,  /* 50 ohm */
> >  };
> >
> >  static const struct of_device_id tegra_usb_phy_id_table[] = {
> > @@ -1360,6 +1586,7 @@ static int tegra_usb_phy_probe(struct platform_device *pdev)
> >       tegra_phy->phy_type = of_usb_get_phy_mode(np);
> >       switch (tegra_phy->phy_type) {
> >       case USBPHY_INTERFACE_MODE_UTMI:
> > +     case USBPHY_INTERFACE_MODE_HSIC:
> >               err = utmi_phy_probe(tegra_phy, pdev);
> >               if (err)
> >                       return err;
> > diff --git a/include/linux/usb/tegra_usb_phy.h b/include/linux/usb/tegra_usb_phy.h
> > index e394f4880b7e..4e79f1c2173a 100644
> > --- a/include/linux/usb/tegra_usb_phy.h
> > +++ b/include/linux/usb/tegra_usb_phy.h
> > @@ -24,6 +24,9 @@ struct gpio_desc;
> >   * requires_extra_tuning_parameters: true if xcvr_hsslew, hssquelch_level
> >   *      and hsdiscon_level should be set for adequate signal quality
> >   * requires_pmc_ao_power_up: true if USB AO is powered down by default
> > + * uhsic_registers_shift: for Tegra30+ where HSIC registers were shifted
> > + *      comparing to Tegra20 by 0x400, since Tegra20 has no UTMIP on PHY2
> > + * uhsic_tx_rtune: fine tuned 50 Ohm termination resistor for NMOS/PMOS driver
> >   */
> >
> >  struct tegra_phy_soc_config {
> > @@ -32,6 +35,8 @@ struct tegra_phy_soc_config {
> >       bool requires_usbmode_setup;
> >       bool requires_extra_tuning_parameters;
> >       bool requires_pmc_ao_power_up;
> > +     u32 uhsic_registers_shift;
>
> I would rather call this 'uhsic_registers_offset'. Shift first brings to mind bitshifts, and we often have fields in these config structs for bit shifts called '*_shift'.
>

Yes, offset seems more appropriate.

> > +     u32 uhsic_tx_rtune;
> >  };
> >
> >  struct tegra_utmip_config {
> >

Mikko, thank you for your review, since HSIC patches already were
picked into linux/next I will prepare a few followup patches to
address some of uncertainties.

Re: [PATCH v3 2/2] usb: phy: tegra: add HSIC support

[Mikko Perttunen]

On Monday, February 2, 2026 3:37 PM Svyatoslav Ryhel wrote:
> пн, 2 лют. 2026 р. о 06:07 Mikko Perttunen <mperttunen@nvidia.com> пише:
> >
> > On Friday, January 23, 2026 12:11 AM Svyatoslav Ryhel wrote:
> > > Add support for HSIC USB mode, which can be set for second USB controller
> > > and PHY on Tegra SoC along with already supported UTMI or ULPI.
> > >
> > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > ---
> > >  drivers/usb/phy/phy-tegra-usb.c   | 249 ++++++++++++++++++++++++++++--
> > >  include/linux/usb/tegra_usb_phy.h |   5 +
> > >  2 files changed, 243 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c
> > > index afa5b5535f92..4f0fde33647e 100644
> > > --- a/drivers/usb/phy/phy-tegra-usb.c
> > > +++ b/drivers/usb/phy/phy-tegra-usb.c
> > > @@ -29,17 +29,26 @@
> > >  #include <linux/usb/tegra_usb_phy.h>
> > >  #include <linux/usb/ulpi.h>
> > >
> > > +#define USB_TXFILLTUNING                     0x154
> > > +#define USB_FIFO_TXFILL_THRES(x)             (((x) & 0x1f) << 16)
> > > +#define USB_FIFO_TXFILL_MASK                 0x1f0000
> > > +
> > >  #define ULPI_VIEWPORT                                0x170
> > >
> > >  /* PORTSC PTS/PHCD bits, Tegra20 only */
> > >  #define TEGRA_USB_PORTSC1                    0x184
> > > -#define TEGRA_USB_PORTSC1_PTS(x)             (((x) & 0x3) << 30)
> > > -#define TEGRA_USB_PORTSC1_PHCD                       BIT(23)
> > > +#define   TEGRA_USB_PORTSC1_PTS(x)           (((x) & 0x3) << 30)
> > > +#define   TEGRA_USB_PORTSC1_PHCD             BIT(23)
> > > +#define   TEGRA_USB_PORTSC1_WKOC             BIT(22)
> > > +#define   TEGRA_USB_PORTSC1_WKDS             BIT(21)
> > > +#define   TEGRA_USB_PORTSC1_WKCN             BIT(20)
> > >
> > >  /* HOSTPC1 PTS/PHCD bits, Tegra30 and above */
> > > +#define TEGRA30_USB_PORTSC1                  0x174
> > >  #define TEGRA_USB_HOSTPC1_DEVLC                      0x1b4
> > > -#define TEGRA_USB_HOSTPC1_DEVLC_PTS(x)               (((x) & 0x7) << 29)
> > > -#define TEGRA_USB_HOSTPC1_DEVLC_PHCD         BIT(22)
> > > +#define   TEGRA_USB_HOSTPC1_DEVLC_PTS(x)     (((x) & 0x7) << 29)
> > > +#define   TEGRA_USB_HOSTPC1_DEVLC_PHCD               BIT(22)
> > > +#define   TEGRA_USB_HOSTPC1_DEVLC_PTS_HSIC   BIT(2)
> >
> > PTS is an enumeration, not a bitfield, so I would say '4' instead of 'BIT(2)'
> >
> 
> Noted
> 
> > >
> > >  /* Bits of PORTSC1, which will get cleared by writing 1 into them */
> > >  #define TEGRA_PORTSC1_RWC_BITS       (PORT_CSC | PORT_PEC | PORT_OCC)
> > > @@ -51,11 +60,12 @@
> > >  #define   USB_SUSP_CLR                               BIT(5)
> > >  #define   USB_PHY_CLK_VALID                  BIT(7)
> > >  #define   UTMIP_RESET                                BIT(11)
> > > -#define   UHSIC_RESET                                BIT(11)
> > >  #define   UTMIP_PHY_ENABLE                   BIT(12)
> > >  #define   ULPI_PHY_ENABLE                    BIT(13)
> > >  #define   USB_SUSP_SET                               BIT(14)
> > > +#define   UHSIC_RESET                                BIT(14)
> > >  #define   USB_WAKEUP_DEBOUNCE_COUNT(x)               (((x) & 0x7) << 16)
> > > +#define   UHSIC_PHY_ENABLE                   BIT(19)
> > >
> > >  #define USB_PHY_VBUS_SENSORS                 0x404
> > >  #define   B_SESS_VLD_WAKEUP_EN                       BIT(14)
> > > @@ -156,6 +166,58 @@
> > >  #define UTMIP_BIAS_CFG1                              0x83c
> > >  #define   UTMIP_BIAS_PDTRK_COUNT(x)          (((x) & 0x1f) << 3)
> > >
> > > +/*
> > > + * Tegra20 has no UTMIP registers on PHY2 and UHSIC registers start from 0x800
> > > + * just where UTMIP registers should have been. This is the case only with Tegra20
> > > + * Tegra30+ have UTMIP registers at 0x800 and UHSIC registers shifter by 0x400
> >
> > 'shift', or 'are shifted'
> >
> 
> Noted
> 
> > > + * to 0xc00, but register layout is preserved.
> > > + */
> > > +#define UHSIC_PLL_CFG1                               0x804
> > > +#define   UHSIC_XTAL_FREQ_COUNT(x)           (((x) & 0xfff) << 0)
> > > +#define   UHSIC_PLLU_ENABLE_DLY_COUNT(x)     (((x) & 0x1f) << 14)
> > > +
> > > +#define UHSIC_HSRX_CFG0                              0x808
> > > +#define   UHSIC_ELASTIC_UNDERRUN_LIMIT(x)    (((x) & 0x1f) << 2)
> > > +#define   UHSIC_ELASTIC_OVERRUN_LIMIT(x)     (((x) & 0x1f) << 8)
> > > +#define   UHSIC_IDLE_WAIT(x)                 (((x) & 0x1f) << 13)
> > > +
> > > +#define UHSIC_HSRX_CFG1                              0x80c
> > > +#define   UHSIC_HS_SYNC_START_DLY(x)         (((x) & 0x1f) << 1)
> > > +
> > > +#define UHSIC_TX_CFG0                                0x810
> > > +#define   UHSIC_HS_READY_WAIT_FOR_VALID              BIT(9)
> > > +
> > > +#define UHSIC_MISC_CFG0                              0x814
> > > +#define   UHSIC_SUSPEND_EXIT_ON_EDGE         BIT(7)
> > > +#define   UHSIC_DETECT_SHORT_CONNECT         BIT(8)
> > > +#define   UHSIC_FORCE_XCVR_MODE                      BIT(15)
> > > +#define   UHSIC_DISABLE_BUSRESET             BIT(20)
> > > +
> > > +#define UHSIC_MISC_CFG1                              0x818
> > > +#define   UHSIC_PLLU_STABLE_COUNT(x)         (((x) & 0xfff) << 2)
> > > +
> > > +#define UHSIC_PADS_CFG0                              0x81c
> > > +#define   UHSIC_TX_RTUNEN                    0xf000
> > > +#define   UHSIC_TX_RTUNE(x)                  (((x) & 0xf) << 12)
> > > +
> > > +#define UHSIC_PADS_CFG1                              0x820
> > > +#define   UHSIC_PD_BG                                BIT(2)
> > > +#define   UHSIC_PD_TX                                BIT(3)
> > > +#define   UHSIC_PD_TRK                               BIT(4)
> > > +#define   UHSIC_PD_RX                                BIT(5)
> > > +#define   UHSIC_PD_ZI                                BIT(6)
> > > +#define   UHSIC_RX_SEL                               BIT(7)
> > > +#define   UHSIC_RPD_DATA                     BIT(9)
> > > +#define   UHSIC_RPD_STROBE                   BIT(10)
> > > +#define   UHSIC_RPU_DATA                     BIT(11)
> > > +#define   UHSIC_RPU_STROBE                   BIT(12)
> > > +
> > > +#define UHSIC_CMD_CFG0                               0x824
> > > +#define   UHSIC_PRETEND_CONNECT_DETECT               BIT(5)
> > > +
> > > +#define UHSIC_STAT_CFG0                              0x828
> > > +#define   UHSIC_CONNECT_DETECT                       BIT(0)
> > > +
> > >  /* For Tegra30 and above only, the address is different in Tegra20 */
> > >  #define USB_USBMODE                          0x1f8
> > >  #define   USB_USBMODE_MASK                   (3 << 0)
> > > @@ -174,7 +236,8 @@ struct tegra_xtal_freq {
> > >       u8 enable_delay;
> > >       u8 stable_count;
> > >       u8 active_delay;
> > > -     u8 xtal_freq_count;
> > > +     u8 utmi_xtal_freq_count;
> > > +     u16 hsic_xtal_freq_count;
> > >       u16 debounce;
> > >  };
> > >
> > > @@ -184,7 +247,8 @@ static const struct tegra_xtal_freq tegra_freq_table[] = {
> > >               .enable_delay = 0x02,
> > >               .stable_count = 0x2F,
> > >               .active_delay = 0x04,
> > > -             .xtal_freq_count = 0x76,
> > > +             .utmi_xtal_freq_count = 0x76,
> > > +             .hsic_xtal_freq_count = 0x1CA,
> > >               .debounce = 0x7530,
> > >       },
> > >       {
> > > @@ -192,7 +256,8 @@ static const struct tegra_xtal_freq tegra_freq_table[] = {
> > >               .enable_delay = 0x02,
> > >               .stable_count = 0x33,
> > >               .active_delay = 0x05,
> > > -             .xtal_freq_count = 0x7F,
> > > +             .utmi_xtal_freq_count = 0x7F,
> > > +             .hsic_xtal_freq_count = 0x1F0,
> > >               .debounce = 0x7EF4,
> > >       },
> > >       {
> > > @@ -200,7 +265,8 @@ static const struct tegra_xtal_freq tegra_freq_table[] = {
> > >               .enable_delay = 0x03,
> > >               .stable_count = 0x4B,
> > >               .active_delay = 0x06,
> > > -             .xtal_freq_count = 0xBB,
> > > +             .utmi_xtal_freq_count = 0xBB,
> > > +             .hsic_xtal_freq_count = 0x2DD,
> > >               .debounce = 0xBB80,
> > >       },
> > >       {
> > > @@ -208,7 +274,8 @@ static const struct tegra_xtal_freq tegra_freq_table[] = {
> > >               .enable_delay = 0x04,
> > >               .stable_count = 0x66,
> > >               .active_delay = 0x09,
> > > -             .xtal_freq_count = 0xFE,
> > > +             .utmi_xtal_freq_count = 0xFE,
> > > +             .hsic_xtal_freq_count = 0x3E0,
> > >               .debounce = 0xFDE8,
> > >       },
> > >  };
> > > @@ -541,7 +608,7 @@ static int utmi_phy_power_on(struct tegra_usb_phy *phy)
> > >               val = readl_relaxed(base + UTMIP_PLL_CFG1);
> > >               val &= ~(UTMIP_XTAL_FREQ_COUNT(~0) |
> > >                       UTMIP_PLLU_ENABLE_DLY_COUNT(~0));
> > > -             val |= UTMIP_XTAL_FREQ_COUNT(phy->freq->xtal_freq_count) |
> > > +             val |= UTMIP_XTAL_FREQ_COUNT(phy->freq->utmi_xtal_freq_count) |
> > >                       UTMIP_PLLU_ENABLE_DLY_COUNT(phy->freq->enable_delay);
> > >               writel_relaxed(val, base + UTMIP_PLL_CFG1);
> > >       }
> > > @@ -812,6 +879,153 @@ static int ulpi_phy_power_off(struct tegra_usb_phy *phy)
> > >       return 0;
> > >  }
> > >
> > > +static u32 tegra_hsic_readl(struct tegra_usb_phy *phy, u32 reg)
> > > +{
> > > +     void __iomem *base = phy->regs;
> > > +     u32 shift = phy->soc_config->uhsic_registers_shift;
> > > +
> > > +     return readl_relaxed(base + shift + reg);
> > > +}
> > > +
> > > +static void tegra_hsic_writel(struct tegra_usb_phy *phy, u32 reg, u32 value)
> > > +{
> > > +     void __iomem *base = phy->regs;
> > > +     u32 shift = phy->soc_config->uhsic_registers_shift;
> > > +
> > > +     writel_relaxed(value, base + shift + reg);
> > > +}
> > > +
> > > +static int uhsic_phy_power_on(struct tegra_usb_phy *phy)
> > > +{
> > > +     struct tegra_utmip_config *config = phy->config;
> > > +     void __iomem *base = phy->regs;
> > > +     u32 val;
> > > +
> > > +     val = tegra_hsic_readl(phy, UHSIC_PADS_CFG1);
> > > +     val &= ~(UHSIC_PD_BG | UHSIC_PD_TX | UHSIC_PD_TRK | UHSIC_PD_RX |
> > > +              UHSIC_PD_ZI | UHSIC_RPD_DATA | UHSIC_RPD_STROBE);
> > > +     val |= UHSIC_RX_SEL;
> >
> > L4T r16 (Tegra30) keeps UHSIC_PD_TX set until after UHSIC_RESET has been cleared. Commit message says this avoids a signal glitch.
> >
> 
> I did not notice any difference with this change and without. Since it
> does not affect detection of modem by my device I can apply it (is it
> worth applying at all?). Should it be applied globally or for Tegra30+
> only?

Downstream only has it for Tegra30, but that's probably just because it was tested in the Tegra30 timeframe. If it's not causing any issue on Tegra20 I would apply it globally. Considering it's a signal glitch it might only have an effect in marginal signal situations.

> 
> > > +     tegra_hsic_writel(phy, UHSIC_PADS_CFG1, val);
> > > +
> > > +     udelay(2);
> > > +
> > > +     val = readl_relaxed(base + USB_SUSP_CTRL);
> > > +     val |= UHSIC_RESET;
> > > +     writel_relaxed(val, base + USB_SUSP_CTRL);
> > > +
> > > +     udelay(30);
> > > +
> > > +     val = readl_relaxed(base + USB_SUSP_CTRL);
> > > +     val |= UHSIC_PHY_ENABLE;
> > > +     writel_relaxed(val, base + USB_SUSP_CTRL);
> > > +
> > > +     val = tegra_hsic_readl(phy, UHSIC_HSRX_CFG0);
> > > +     val &= ~(UHSIC_IDLE_WAIT(~0) |
> > > +              UHSIC_ELASTIC_UNDERRUN_LIMIT(~0) |
> > > +              UHSIC_ELASTIC_OVERRUN_LIMIT(~0));
> > > +     val |= UHSIC_IDLE_WAIT(config->idle_wait_delay) |
> > > +             UHSIC_ELASTIC_UNDERRUN_LIMIT(config->elastic_limit) |
> > > +             UHSIC_ELASTIC_OVERRUN_LIMIT(config->elastic_limit);
> > > +     tegra_hsic_writel(phy, UHSIC_HSRX_CFG0, val);
> > > +
> > > +     val = tegra_hsic_readl(phy, UHSIC_HSRX_CFG1);
> > > +     val &= ~UHSIC_HS_SYNC_START_DLY(~0);
> > > +     val |= UHSIC_HS_SYNC_START_DLY(config->hssync_start_delay);
> > > +     tegra_hsic_writel(phy, UHSIC_HSRX_CFG1, val);
> > > +
> >
> > L4T r16 (Tegra30) clears UHSIC_TX_CFG0.UHSIC_HS_READY_WAIT_FOR_VALID here. According to commit message this fixes an intermittent failure to connect to HSIC modem. It also sets UHSIC_MISC_CFG0.UHSIC_DISABLE_BUSRESET  below due to same issue.
> >
> 
> And this change causes my modem to fail entirely.
> 
> [   10.145976] usb 1-1: new high-speed USB device number 2 using ci_hdrc
> [   10.295970] usb 1-1: device descriptor read/64, error -71
> [   10.545975] usb 1-1: device descriptor read/64, error -71
> [   10.795990] usb 1-1: new high-speed USB device number 3 using ci_hdrc
> [   10.935970] usb 1-1: device descriptor read/64, error -71
> [   11.185977] usb 1-1: device descriptor read/64, error -71
> 
> I assume UHSIC_DISABLE_BUSRESET, UHSIC_HS_READY_WAIT_FOR_VALID and
> UHSIC_PD_TX workarounds are all consequences of how modem in
> downstream kernel is called. Instead of relaying on modem to lead the
> controller probe, downstream just brutally removes and reinits
> controller until modem probes. I have never observed modem not
> appearing without any of discussed patches.

The downstream commit says that without this workaround, the modem would sometimes not come up in stress tests. So I think it's possible there is still a rare hardware bug that this is working around, but perhaps we're missing some other part and that's why it's not working. I think it's fine to drop these changes.

> 
> 
> > > +     val = tegra_hsic_readl(phy, UHSIC_MISC_CFG0);
> > > +     val |= UHSIC_SUSPEND_EXIT_ON_EDGE;
> > > +     tegra_hsic_writel(phy, UHSIC_MISC_CFG0, val);
> > > +
> > > +     val = tegra_hsic_readl(phy, UHSIC_MISC_CFG1);
> > > +     val &= ~UHSIC_PLLU_STABLE_COUNT(~0);
> > > +     val |= UHSIC_PLLU_STABLE_COUNT(phy->freq->stable_count);
> > > +     tegra_hsic_writel(phy, UHSIC_MISC_CFG1, val);
> > > +
> > > +     val = tegra_hsic_readl(phy, UHSIC_PLL_CFG1);
> > > +     val &= ~(UHSIC_XTAL_FREQ_COUNT(~0) |
> > > +             UHSIC_PLLU_ENABLE_DLY_COUNT(~0));
> > > +     val |= UHSIC_XTAL_FREQ_COUNT(phy->freq->hsic_xtal_freq_count) |
> > > +             UHSIC_PLLU_ENABLE_DLY_COUNT(phy->freq->enable_delay);
> > > +     tegra_hsic_writel(phy, UHSIC_PLL_CFG1, val);
> > > +
> > > +     val = readl_relaxed(base + USB_SUSP_CTRL);
> > > +     val &= ~UHSIC_RESET;
> > > +     writel_relaxed(val, base + USB_SUSP_CTRL);
> > > +
> > > +     udelay(2);
> > > +
> > > +     if (phy->soc_config->requires_usbmode_setup) {
> > > +             val = readl_relaxed(base + USB_USBMODE);
> > > +             val &= ~USB_USBMODE_MASK;
> > > +             if (phy->mode == USB_DR_MODE_HOST)
> > > +                     val |= USB_USBMODE_HOST;
> > > +             else
> > > +                     val |= USB_USBMODE_DEVICE;
> > > +             writel_relaxed(val, base + USB_USBMODE);
> > > +     }
> > > +
> > > +     if (phy->soc_config->has_hostpc)
> > > +             set_pts(phy, TEGRA_USB_HOSTPC1_DEVLC_PTS_HSIC);
> > > +     else
> > > +             set_pts(phy, 0);
> >
> > This (and below) are abusing has_hostpc to detect Tegra30 vs Tegra20. We should instead add soc_config fields hsic_pts_value and portsc1_offset.
> >
> > > +
> > > +     val = readl_relaxed(base + USB_TXFILLTUNING);
> > > +     if ((val & USB_FIFO_TXFILL_MASK) != USB_FIFO_TXFILL_THRES(0x10)) {
> > > +             val = USB_FIFO_TXFILL_THRES(0x10);
> > > +             writel_relaxed(val, base + USB_TXFILLTUNING);
> > > +     }
> > > +
> > > +     if (phy->soc_config->has_hostpc) {
> > > +             val = readl_relaxed(base + TEGRA30_USB_PORTSC1);
> > > +             val &= ~(TEGRA_USB_PORTSC1_WKOC | TEGRA_USB_PORTSC1_WKDS |
> > > +                      TEGRA_USB_PORTSC1_WKCN);
> > > +             writel_relaxed(val, base + TEGRA30_USB_PORTSC1);
> > > +     } else {
> > > +             val = readl_relaxed(base + TEGRA_USB_PORTSC1);
> > > +             val &= ~(TEGRA_USB_PORTSC1_WKOC | TEGRA_USB_PORTSC1_WKDS |
> > > +                      TEGRA_USB_PORTSC1_WKCN);
> > > +             writel_relaxed(val, base + TEGRA_USB_PORTSC1);
> > > +     }
> > > +
> > > +     val = tegra_hsic_readl(phy, UHSIC_PADS_CFG0);
> > > +     val &= ~UHSIC_TX_RTUNEN;
> > > +     val |= UHSIC_TX_RTUNE(phy->soc_config->uhsic_tx_rtune);
> > > +     tegra_hsic_writel(phy, UHSIC_PADS_CFG0, val);
> > > +
> > > +     if (utmi_wait_register(base + USB_SUSP_CTRL, USB_PHY_CLK_VALID,
> > > +                            USB_PHY_CLK_VALID))
> > > +             dev_err(phy->u_phy.dev,
> > > +                     "Timeout waiting for PHY to stabilize on enable (HSIC)\n");
> >
> > Should return error (return value of utmi_wait_register) here?
> >
> 
> Noted.
> 
> > > +
> > > +     return 0;
> > > +}
> > > +
> > > +static int uhsic_phy_power_off(struct tegra_usb_phy *phy)
> > > +{
> > > +     void __iomem *base = phy->regs;
> > > +     u32 val;
> > > +
> > > +     set_phcd(phy, true);
> > > +
> > > +     val = tegra_hsic_readl(phy, UHSIC_PADS_CFG1);
> > > +     val |= (UHSIC_PD_BG | UHSIC_PD_TX | UHSIC_PD_TRK | UHSIC_PD_RX |
> > > +             UHSIC_PD_ZI | UHSIC_RPD_DATA | UHSIC_RPD_STROBE);
> > > +     tegra_hsic_writel(phy, UHSIC_PADS_CFG1, val);
> > > +
> > > +     val = readl_relaxed(base + USB_SUSP_CTRL);
> > > +     val |= UHSIC_RESET;
> > > +     writel_relaxed(val, base + USB_SUSP_CTRL);
> > > +
> > > +     udelay(30);
> > > +
> > > +     val = readl_relaxed(base + USB_SUSP_CTRL);
> > > +     val &= ~UHSIC_PHY_ENABLE;
> > > +     writel_relaxed(val, base + USB_SUSP_CTRL);
> > > +
> > > +     return 0;
> > > +}
> > > +
> > >  static int tegra_usb_phy_power_on(struct tegra_usb_phy *phy)
> > >  {
> > >       int err = 0;
> > > @@ -828,6 +1042,10 @@ static int tegra_usb_phy_power_on(struct tegra_usb_phy *phy)
> > >               err = ulpi_phy_power_on(phy);
> > >               break;
> > >
> > > +     case USBPHY_INTERFACE_MODE_HSIC:
> > > +             err = uhsic_phy_power_on(phy);
> > > +             break;
> > > +
> > >       default:
> > >               break;
> > >       }
> > > @@ -859,6 +1077,10 @@ static int tegra_usb_phy_power_off(struct tegra_usb_phy *phy)
> > >               err = ulpi_phy_power_off(phy);
> > >               break;
> > >
> > > +     case USBPHY_INTERFACE_MODE_HSIC:
> > > +             err = uhsic_phy_power_off(phy);
> > > +             break;
> > > +
> > >       default:
> > >               break;
> > >       }
> > > @@ -1256,6 +1478,8 @@ static const struct tegra_phy_soc_config tegra20_soc_config = {
> > >       .requires_usbmode_setup = false,
> > >       .requires_extra_tuning_parameters = false,
> > >       .requires_pmc_ao_power_up = false,
> > > +     .uhsic_registers_shift = 0,
> > > +     .uhsic_tx_rtune = 0, /* 40 ohm */
> > >  };
> > >
> > >  static const struct tegra_phy_soc_config tegra30_soc_config = {
> > > @@ -1264,6 +1488,8 @@ static const struct tegra_phy_soc_config tegra30_soc_config = {
> > >       .requires_usbmode_setup = true,
> > >       .requires_extra_tuning_parameters = true,
> > >       .requires_pmc_ao_power_up = true,
> > > +     .uhsic_registers_shift = 0x400,
> > > +     .uhsic_tx_rtune = 8,  /* 50 ohm */
> > >  };
> > >
> > >  static const struct of_device_id tegra_usb_phy_id_table[] = {
> > > @@ -1360,6 +1586,7 @@ static int tegra_usb_phy_probe(struct platform_device *pdev)
> > >       tegra_phy->phy_type = of_usb_get_phy_mode(np);
> > >       switch (tegra_phy->phy_type) {
> > >       case USBPHY_INTERFACE_MODE_UTMI:
> > > +     case USBPHY_INTERFACE_MODE_HSIC:
> > >               err = utmi_phy_probe(tegra_phy, pdev);
> > >               if (err)
> > >                       return err;
> > > diff --git a/include/linux/usb/tegra_usb_phy.h b/include/linux/usb/tegra_usb_phy.h
> > > index e394f4880b7e..4e79f1c2173a 100644
> > > --- a/include/linux/usb/tegra_usb_phy.h
> > > +++ b/include/linux/usb/tegra_usb_phy.h
> > > @@ -24,6 +24,9 @@ struct gpio_desc;
> > >   * requires_extra_tuning_parameters: true if xcvr_hsslew, hssquelch_level
> > >   *      and hsdiscon_level should be set for adequate signal quality
> > >   * requires_pmc_ao_power_up: true if USB AO is powered down by default
> > > + * uhsic_registers_shift: for Tegra30+ where HSIC registers were shifted
> > > + *      comparing to Tegra20 by 0x400, since Tegra20 has no UTMIP on PHY2
> > > + * uhsic_tx_rtune: fine tuned 50 Ohm termination resistor for NMOS/PMOS driver
> > >   */
> > >
> > >  struct tegra_phy_soc_config {
> > > @@ -32,6 +35,8 @@ struct tegra_phy_soc_config {
> > >       bool requires_usbmode_setup;
> > >       bool requires_extra_tuning_parameters;
> > >       bool requires_pmc_ao_power_up;
> > > +     u32 uhsic_registers_shift;
> >
> > I would rather call this 'uhsic_registers_offset'. Shift first brings to mind bitshifts, and we often have fields in these config structs for bit shifts called '*_shift'.
> >
> 
> Yes, offset seems more appropriate.
> 
> > > +     u32 uhsic_tx_rtune;
> > >  };
> > >
> > >  struct tegra_utmip_config {
> > >
> 
> Mikko, thank you for your review, since HSIC patches already were
> picked into linux/next I will prepare a few followup patches to
> address some of uncertainties.

Thank you!

Re: [PATCH v3 2/2] usb: phy: tegra: add HSIC support

[Svyatoslav Ryhel]

пн, 2 лют. 2026 р. о 10:06 Mikko Perttunen <mperttunen@nvidia.com> пише:
>
> On Monday, February 2, 2026 3:37 PM Svyatoslav Ryhel wrote:
> > пн, 2 лют. 2026 р. о 06:07 Mikko Perttunen <mperttunen@nvidia.com> пише:
> > >
> > > On Friday, January 23, 2026 12:11 AM Svyatoslav Ryhel wrote:
> > > > Add support for HSIC USB mode, which can be set for second USB controller
> > > > and PHY on Tegra SoC along with already supported UTMI or ULPI.
> > > >
> > > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > > ---
> > > >  drivers/usb/phy/phy-tegra-usb.c   | 249 ++++++++++++++++++++++++++++--
> > > >  include/linux/usb/tegra_usb_phy.h |   5 +
> > > >  2 files changed, 243 insertions(+), 11 deletions(-)
> > > >
> > > > diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c
> > > > index afa5b5535f92..4f0fde33647e 100644
> > > > --- a/drivers/usb/phy/phy-tegra-usb.c
> > > > +++ b/drivers/usb/phy/phy-tegra-usb.c
> > > > @@ -29,17 +29,26 @@
> > > >  #include <linux/usb/tegra_usb_phy.h>
> > > >  #include <linux/usb/ulpi.h>
> > > >
> > > > +#define USB_TXFILLTUNING                     0x154
> > > > +#define USB_FIFO_TXFILL_THRES(x)             (((x) & 0x1f) << 16)
> > > > +#define USB_FIFO_TXFILL_MASK                 0x1f0000
> > > > +
> > > >  #define ULPI_VIEWPORT                                0x170
> > > >
> > > >  /* PORTSC PTS/PHCD bits, Tegra20 only */
> > > >  #define TEGRA_USB_PORTSC1                    0x184
> > > > -#define TEGRA_USB_PORTSC1_PTS(x)             (((x) & 0x3) << 30)
> > > > -#define TEGRA_USB_PORTSC1_PHCD                       BIT(23)
> > > > +#define   TEGRA_USB_PORTSC1_PTS(x)           (((x) & 0x3) << 30)
> > > > +#define   TEGRA_USB_PORTSC1_PHCD             BIT(23)
> > > > +#define   TEGRA_USB_PORTSC1_WKOC             BIT(22)
> > > > +#define   TEGRA_USB_PORTSC1_WKDS             BIT(21)
> > > > +#define   TEGRA_USB_PORTSC1_WKCN             BIT(20)
> > > >
> > > >  /* HOSTPC1 PTS/PHCD bits, Tegra30 and above */
> > > > +#define TEGRA30_USB_PORTSC1                  0x174
> > > >  #define TEGRA_USB_HOSTPC1_DEVLC                      0x1b4
> > > > -#define TEGRA_USB_HOSTPC1_DEVLC_PTS(x)               (((x) & 0x7) << 29)
> > > > -#define TEGRA_USB_HOSTPC1_DEVLC_PHCD         BIT(22)
> > > > +#define   TEGRA_USB_HOSTPC1_DEVLC_PTS(x)     (((x) & 0x7) << 29)
> > > > +#define   TEGRA_USB_HOSTPC1_DEVLC_PHCD               BIT(22)
> > > > +#define   TEGRA_USB_HOSTPC1_DEVLC_PTS_HSIC   BIT(2)
> > >
> > > PTS is an enumeration, not a bitfield, so I would say '4' instead of 'BIT(2)'
> > >
> >
> > Noted
> >
> > > >
> > > >  /* Bits of PORTSC1, which will get cleared by writing 1 into them */
> > > >  #define TEGRA_PORTSC1_RWC_BITS       (PORT_CSC | PORT_PEC | PORT_OCC)
> > > > @@ -51,11 +60,12 @@
> > > >  #define   USB_SUSP_CLR                               BIT(5)
> > > >  #define   USB_PHY_CLK_VALID                  BIT(7)
> > > >  #define   UTMIP_RESET                                BIT(11)
> > > > -#define   UHSIC_RESET                                BIT(11)
> > > >  #define   UTMIP_PHY_ENABLE                   BIT(12)
> > > >  #define   ULPI_PHY_ENABLE                    BIT(13)
> > > >  #define   USB_SUSP_SET                               BIT(14)
> > > > +#define   UHSIC_RESET                                BIT(14)
> > > >  #define   USB_WAKEUP_DEBOUNCE_COUNT(x)               (((x) & 0x7) << 16)
> > > > +#define   UHSIC_PHY_ENABLE                   BIT(19)
> > > >
> > > >  #define USB_PHY_VBUS_SENSORS                 0x404
> > > >  #define   B_SESS_VLD_WAKEUP_EN                       BIT(14)
> > > > @@ -156,6 +166,58 @@
> > > >  #define UTMIP_BIAS_CFG1                              0x83c
> > > >  #define   UTMIP_BIAS_PDTRK_COUNT(x)          (((x) & 0x1f) << 3)
> > > >
> > > > +/*
> > > > + * Tegra20 has no UTMIP registers on PHY2 and UHSIC registers start from 0x800
> > > > + * just where UTMIP registers should have been. This is the case only with Tegra20
> > > > + * Tegra30+ have UTMIP registers at 0x800 and UHSIC registers shifter by 0x400
> > >
> > > 'shift', or 'are shifted'
> > >
> >
> > Noted
> >
> > > > + * to 0xc00, but register layout is preserved.
> > > > + */
> > > > +#define UHSIC_PLL_CFG1                               0x804
> > > > +#define   UHSIC_XTAL_FREQ_COUNT(x)           (((x) & 0xfff) << 0)
> > > > +#define   UHSIC_PLLU_ENABLE_DLY_COUNT(x)     (((x) & 0x1f) << 14)
> > > > +
> > > > +#define UHSIC_HSRX_CFG0                              0x808
> > > > +#define   UHSIC_ELASTIC_UNDERRUN_LIMIT(x)    (((x) & 0x1f) << 2)
> > > > +#define   UHSIC_ELASTIC_OVERRUN_LIMIT(x)     (((x) & 0x1f) << 8)
> > > > +#define   UHSIC_IDLE_WAIT(x)                 (((x) & 0x1f) << 13)
> > > > +
> > > > +#define UHSIC_HSRX_CFG1                              0x80c
> > > > +#define   UHSIC_HS_SYNC_START_DLY(x)         (((x) & 0x1f) << 1)
> > > > +
> > > > +#define UHSIC_TX_CFG0                                0x810
> > > > +#define   UHSIC_HS_READY_WAIT_FOR_VALID              BIT(9)
> > > > +
> > > > +#define UHSIC_MISC_CFG0                              0x814
> > > > +#define   UHSIC_SUSPEND_EXIT_ON_EDGE         BIT(7)
> > > > +#define   UHSIC_DETECT_SHORT_CONNECT         BIT(8)
> > > > +#define   UHSIC_FORCE_XCVR_MODE                      BIT(15)
> > > > +#define   UHSIC_DISABLE_BUSRESET             BIT(20)
> > > > +
> > > > +#define UHSIC_MISC_CFG1                              0x818
> > > > +#define   UHSIC_PLLU_STABLE_COUNT(x)         (((x) & 0xfff) << 2)
> > > > +
> > > > +#define UHSIC_PADS_CFG0                              0x81c
> > > > +#define   UHSIC_TX_RTUNEN                    0xf000
> > > > +#define   UHSIC_TX_RTUNE(x)                  (((x) & 0xf) << 12)
> > > > +
> > > > +#define UHSIC_PADS_CFG1                              0x820
> > > > +#define   UHSIC_PD_BG                                BIT(2)
> > > > +#define   UHSIC_PD_TX                                BIT(3)
> > > > +#define   UHSIC_PD_TRK                               BIT(4)
> > > > +#define   UHSIC_PD_RX                                BIT(5)
> > > > +#define   UHSIC_PD_ZI                                BIT(6)
> > > > +#define   UHSIC_RX_SEL                               BIT(7)
> > > > +#define   UHSIC_RPD_DATA                     BIT(9)
> > > > +#define   UHSIC_RPD_STROBE                   BIT(10)
> > > > +#define   UHSIC_RPU_DATA                     BIT(11)
> > > > +#define   UHSIC_RPU_STROBE                   BIT(12)
> > > > +
> > > > +#define UHSIC_CMD_CFG0                               0x824
> > > > +#define   UHSIC_PRETEND_CONNECT_DETECT               BIT(5)
> > > > +
> > > > +#define UHSIC_STAT_CFG0                              0x828
> > > > +#define   UHSIC_CONNECT_DETECT                       BIT(0)
> > > > +
> > > >  /* For Tegra30 and above only, the address is different in Tegra20 */
> > > >  #define USB_USBMODE                          0x1f8
> > > >  #define   USB_USBMODE_MASK                   (3 << 0)
> > > > @@ -174,7 +236,8 @@ struct tegra_xtal_freq {
> > > >       u8 enable_delay;
> > > >       u8 stable_count;
> > > >       u8 active_delay;
> > > > -     u8 xtal_freq_count;
> > > > +     u8 utmi_xtal_freq_count;
> > > > +     u16 hsic_xtal_freq_count;
> > > >       u16 debounce;
> > > >  };
> > > >
> > > > @@ -184,7 +247,8 @@ static const struct tegra_xtal_freq tegra_freq_table[] = {
> > > >               .enable_delay = 0x02,
> > > >               .stable_count = 0x2F,
> > > >               .active_delay = 0x04,
> > > > -             .xtal_freq_count = 0x76,
> > > > +             .utmi_xtal_freq_count = 0x76,
> > > > +             .hsic_xtal_freq_count = 0x1CA,
> > > >               .debounce = 0x7530,
> > > >       },
> > > >       {
> > > > @@ -192,7 +256,8 @@ static const struct tegra_xtal_freq tegra_freq_table[] = {
> > > >               .enable_delay = 0x02,
> > > >               .stable_count = 0x33,
> > > >               .active_delay = 0x05,
> > > > -             .xtal_freq_count = 0x7F,
> > > > +             .utmi_xtal_freq_count = 0x7F,
> > > > +             .hsic_xtal_freq_count = 0x1F0,
> > > >               .debounce = 0x7EF4,
> > > >       },
> > > >       {
> > > > @@ -200,7 +265,8 @@ static const struct tegra_xtal_freq tegra_freq_table[] = {
> > > >               .enable_delay = 0x03,
> > > >               .stable_count = 0x4B,
> > > >               .active_delay = 0x06,
> > > > -             .xtal_freq_count = 0xBB,
> > > > +             .utmi_xtal_freq_count = 0xBB,
> > > > +             .hsic_xtal_freq_count = 0x2DD,
> > > >               .debounce = 0xBB80,
> > > >       },
> > > >       {
> > > > @@ -208,7 +274,8 @@ static const struct tegra_xtal_freq tegra_freq_table[] = {
> > > >               .enable_delay = 0x04,
> > > >               .stable_count = 0x66,
> > > >               .active_delay = 0x09,
> > > > -             .xtal_freq_count = 0xFE,
> > > > +             .utmi_xtal_freq_count = 0xFE,
> > > > +             .hsic_xtal_freq_count = 0x3E0,
> > > >               .debounce = 0xFDE8,
> > > >       },
> > > >  };
> > > > @@ -541,7 +608,7 @@ static int utmi_phy_power_on(struct tegra_usb_phy *phy)
> > > >               val = readl_relaxed(base + UTMIP_PLL_CFG1);
> > > >               val &= ~(UTMIP_XTAL_FREQ_COUNT(~0) |
> > > >                       UTMIP_PLLU_ENABLE_DLY_COUNT(~0));
> > > > -             val |= UTMIP_XTAL_FREQ_COUNT(phy->freq->xtal_freq_count) |
> > > > +             val |= UTMIP_XTAL_FREQ_COUNT(phy->freq->utmi_xtal_freq_count) |
> > > >                       UTMIP_PLLU_ENABLE_DLY_COUNT(phy->freq->enable_delay);
> > > >               writel_relaxed(val, base + UTMIP_PLL_CFG1);
> > > >       }
> > > > @@ -812,6 +879,153 @@ static int ulpi_phy_power_off(struct tegra_usb_phy *phy)
> > > >       return 0;
> > > >  }
> > > >
> > > > +static u32 tegra_hsic_readl(struct tegra_usb_phy *phy, u32 reg)
> > > > +{
> > > > +     void __iomem *base = phy->regs;
> > > > +     u32 shift = phy->soc_config->uhsic_registers_shift;
> > > > +
> > > > +     return readl_relaxed(base + shift + reg);
> > > > +}
> > > > +
> > > > +static void tegra_hsic_writel(struct tegra_usb_phy *phy, u32 reg, u32 value)
> > > > +{
> > > > +     void __iomem *base = phy->regs;
> > > > +     u32 shift = phy->soc_config->uhsic_registers_shift;
> > > > +
> > > > +     writel_relaxed(value, base + shift + reg);
> > > > +}
> > > > +
> > > > +static int uhsic_phy_power_on(struct tegra_usb_phy *phy)
> > > > +{
> > > > +     struct tegra_utmip_config *config = phy->config;
> > > > +     void __iomem *base = phy->regs;
> > > > +     u32 val;
> > > > +
> > > > +     val = tegra_hsic_readl(phy, UHSIC_PADS_CFG1);
> > > > +     val &= ~(UHSIC_PD_BG | UHSIC_PD_TX | UHSIC_PD_TRK | UHSIC_PD_RX |
> > > > +              UHSIC_PD_ZI | UHSIC_RPD_DATA | UHSIC_RPD_STROBE);
> > > > +     val |= UHSIC_RX_SEL;
> > >
> > > L4T r16 (Tegra30) keeps UHSIC_PD_TX set until after UHSIC_RESET has been cleared. Commit message says this avoids a signal glitch.
> > >
> >
> > I did not notice any difference with this change and without. Since it
> > does not affect detection of modem by my device I can apply it (is it
> > worth applying at all?). Should it be applied globally or for Tegra30+
> > only?
>
> Downstream only has it for Tegra30, but that's probably just because it was tested in the Tegra30 timeframe. If it's not causing any issue on Tegra20 I would apply it globally. Considering it's a signal glitch it might only have an effect in marginal signal situations.
>
> >
> > > > +     tegra_hsic_writel(phy, UHSIC_PADS_CFG1, val);
> > > > +
> > > > +     udelay(2);
> > > > +
> > > > +     val = readl_relaxed(base + USB_SUSP_CTRL);
> > > > +     val |= UHSIC_RESET;
> > > > +     writel_relaxed(val, base + USB_SUSP_CTRL);
> > > > +
> > > > +     udelay(30);
> > > > +
> > > > +     val = readl_relaxed(base + USB_SUSP_CTRL);
> > > > +     val |= UHSIC_PHY_ENABLE;
> > > > +     writel_relaxed(val, base + USB_SUSP_CTRL);
> > > > +
> > > > +     val = tegra_hsic_readl(phy, UHSIC_HSRX_CFG0);
> > > > +     val &= ~(UHSIC_IDLE_WAIT(~0) |
> > > > +              UHSIC_ELASTIC_UNDERRUN_LIMIT(~0) |
> > > > +              UHSIC_ELASTIC_OVERRUN_LIMIT(~0));
> > > > +     val |= UHSIC_IDLE_WAIT(config->idle_wait_delay) |
> > > > +             UHSIC_ELASTIC_UNDERRUN_LIMIT(config->elastic_limit) |
> > > > +             UHSIC_ELASTIC_OVERRUN_LIMIT(config->elastic_limit);
> > > > +     tegra_hsic_writel(phy, UHSIC_HSRX_CFG0, val);
> > > > +
> > > > +     val = tegra_hsic_readl(phy, UHSIC_HSRX_CFG1);
> > > > +     val &= ~UHSIC_HS_SYNC_START_DLY(~0);
> > > > +     val |= UHSIC_HS_SYNC_START_DLY(config->hssync_start_delay);
> > > > +     tegra_hsic_writel(phy, UHSIC_HSRX_CFG1, val);
> > > > +
> > >
> > > L4T r16 (Tegra30) clears UHSIC_TX_CFG0.UHSIC_HS_READY_WAIT_FOR_VALID here. According to commit message this fixes an intermittent failure to connect to HSIC modem. It also sets UHSIC_MISC_CFG0.UHSIC_DISABLE_BUSRESET  below due to same issue.
> > >
> >
> > And this change causes my modem to fail entirely.
> >
> > [   10.145976] usb 1-1: new high-speed USB device number 2 using ci_hdrc
> > [   10.295970] usb 1-1: device descriptor read/64, error -71
> > [   10.545975] usb 1-1: device descriptor read/64, error -71
> > [   10.795990] usb 1-1: new high-speed USB device number 3 using ci_hdrc
> > [   10.935970] usb 1-1: device descriptor read/64, error -71
> > [   11.185977] usb 1-1: device descriptor read/64, error -71
> >
> > I assume UHSIC_DISABLE_BUSRESET, UHSIC_HS_READY_WAIT_FOR_VALID and
> > UHSIC_PD_TX workarounds are all consequences of how modem in
> > downstream kernel is called. Instead of relaying on modem to lead the
> > controller probe, downstream just brutally removes and reinits
> > controller until modem probes. I have never observed modem not
> > appearing without any of discussed patches.
>
> The downstream commit says that without this workaround, the modem would sometimes not come up in stress tests. So I think it's possible there is still a rare hardware bug that this is working around, but perhaps we're missing some other part and that's why it's not working. I think it's fine to drop these changes.
>

I propose not adding any of the three workarounds for now. I don't
have a Tegra20 with an HSIC modem, as it seems that most Tegra20
devices use ULPI mode. Among my personal devices, only the LG Optimus
Vu (P895) is a GSM device with an XMM HSIC modem, and it fails if I
add UHSIC_DISABLE_BUSRESET and UHSIC_HS_READY_WAIT_FOR_VALID. Since I
may be the only one at the moment actively working in this area of
legacy Tegra SoCs — and this discussion is public — anyone who has
issues can contact me to resolve them. Additionally, I will keep in
mind that there may be a need for these workarounds; if I find any
need for them during modem bring-ups in the legacy Tegra community, I
will submit dedicated patches with explanations.

> >
> >
> > > > +     val = tegra_hsic_readl(phy, UHSIC_MISC_CFG0);
> > > > +     val |= UHSIC_SUSPEND_EXIT_ON_EDGE;
> > > > +     tegra_hsic_writel(phy, UHSIC_MISC_CFG0, val);
> > > > +
> > > > +     val = tegra_hsic_readl(phy, UHSIC_MISC_CFG1);
> > > > +     val &= ~UHSIC_PLLU_STABLE_COUNT(~0);
> > > > +     val |= UHSIC_PLLU_STABLE_COUNT(phy->freq->stable_count);
> > > > +     tegra_hsic_writel(phy, UHSIC_MISC_CFG1, val);
> > > > +
> > > > +     val = tegra_hsic_readl(phy, UHSIC_PLL_CFG1);
> > > > +     val &= ~(UHSIC_XTAL_FREQ_COUNT(~0) |
> > > > +             UHSIC_PLLU_ENABLE_DLY_COUNT(~0));
> > > > +     val |= UHSIC_XTAL_FREQ_COUNT(phy->freq->hsic_xtal_freq_count) |
> > > > +             UHSIC_PLLU_ENABLE_DLY_COUNT(phy->freq->enable_delay);
> > > > +     tegra_hsic_writel(phy, UHSIC_PLL_CFG1, val);
> > > > +
> > > > +     val = readl_relaxed(base + USB_SUSP_CTRL);
> > > > +     val &= ~UHSIC_RESET;
> > > > +     writel_relaxed(val, base + USB_SUSP_CTRL);
> > > > +
> > > > +     udelay(2);
> > > > +
> > > > +     if (phy->soc_config->requires_usbmode_setup) {
> > > > +             val = readl_relaxed(base + USB_USBMODE);
> > > > +             val &= ~USB_USBMODE_MASK;
> > > > +             if (phy->mode == USB_DR_MODE_HOST)
> > > > +                     val |= USB_USBMODE_HOST;
> > > > +             else
> > > > +                     val |= USB_USBMODE_DEVICE;
> > > > +             writel_relaxed(val, base + USB_USBMODE);
> > > > +     }
> > > > +
> > > > +     if (phy->soc_config->has_hostpc)
> > > > +             set_pts(phy, TEGRA_USB_HOSTPC1_DEVLC_PTS_HSIC);
> > > > +     else
> > > > +             set_pts(phy, 0);
> > >
> > > This (and below) are abusing has_hostpc to detect Tegra30 vs Tegra20. We should instead add soc_config fields hsic_pts_value and portsc1_offset.
> > >
> > > > +
> > > > +     val = readl_relaxed(base + USB_TXFILLTUNING);
> > > > +     if ((val & USB_FIFO_TXFILL_MASK) != USB_FIFO_TXFILL_THRES(0x10)) {
> > > > +             val = USB_FIFO_TXFILL_THRES(0x10);
> > > > +             writel_relaxed(val, base + USB_TXFILLTUNING);
> > > > +     }
> > > > +
> > > > +     if (phy->soc_config->has_hostpc) {
> > > > +             val = readl_relaxed(base + TEGRA30_USB_PORTSC1);
> > > > +             val &= ~(TEGRA_USB_PORTSC1_WKOC | TEGRA_USB_PORTSC1_WKDS |
> > > > +                      TEGRA_USB_PORTSC1_WKCN);
> > > > +             writel_relaxed(val, base + TEGRA30_USB_PORTSC1);
> > > > +     } else {
> > > > +             val = readl_relaxed(base + TEGRA_USB_PORTSC1);
> > > > +             val &= ~(TEGRA_USB_PORTSC1_WKOC | TEGRA_USB_PORTSC1_WKDS |
> > > > +                      TEGRA_USB_PORTSC1_WKCN);
> > > > +             writel_relaxed(val, base + TEGRA_USB_PORTSC1);
> > > > +     }
> > > > +
> > > > +     val = tegra_hsic_readl(phy, UHSIC_PADS_CFG0);
> > > > +     val &= ~UHSIC_TX_RTUNEN;
> > > > +     val |= UHSIC_TX_RTUNE(phy->soc_config->uhsic_tx_rtune);
> > > > +     tegra_hsic_writel(phy, UHSIC_PADS_CFG0, val);
> > > > +
> > > > +     if (utmi_wait_register(base + USB_SUSP_CTRL, USB_PHY_CLK_VALID,
> > > > +                            USB_PHY_CLK_VALID))
> > > > +             dev_err(phy->u_phy.dev,
> > > > +                     "Timeout waiting for PHY to stabilize on enable (HSIC)\n");
> > >
> > > Should return error (return value of utmi_wait_register) here?
> > >
> >
> > Noted.
> >
> > > > +
> > > > +     return 0;
> > > > +}
> > > > +
> > > > +static int uhsic_phy_power_off(struct tegra_usb_phy *phy)
> > > > +{
> > > > +     void __iomem *base = phy->regs;
> > > > +     u32 val;
> > > > +
> > > > +     set_phcd(phy, true);
> > > > +
> > > > +     val = tegra_hsic_readl(phy, UHSIC_PADS_CFG1);
> > > > +     val |= (UHSIC_PD_BG | UHSIC_PD_TX | UHSIC_PD_TRK | UHSIC_PD_RX |
> > > > +             UHSIC_PD_ZI | UHSIC_RPD_DATA | UHSIC_RPD_STROBE);
> > > > +     tegra_hsic_writel(phy, UHSIC_PADS_CFG1, val);
> > > > +
> > > > +     val = readl_relaxed(base + USB_SUSP_CTRL);
> > > > +     val |= UHSIC_RESET;
> > > > +     writel_relaxed(val, base + USB_SUSP_CTRL);
> > > > +
> > > > +     udelay(30);
> > > > +
> > > > +     val = readl_relaxed(base + USB_SUSP_CTRL);
> > > > +     val &= ~UHSIC_PHY_ENABLE;
> > > > +     writel_relaxed(val, base + USB_SUSP_CTRL);
> > > > +
> > > > +     return 0;
> > > > +}
> > > > +
> > > >  static int tegra_usb_phy_power_on(struct tegra_usb_phy *phy)
> > > >  {
> > > >       int err = 0;
> > > > @@ -828,6 +1042,10 @@ static int tegra_usb_phy_power_on(struct tegra_usb_phy *phy)
> > > >               err = ulpi_phy_power_on(phy);
> > > >               break;
> > > >
> > > > +     case USBPHY_INTERFACE_MODE_HSIC:
> > > > +             err = uhsic_phy_power_on(phy);
> > > > +             break;
> > > > +
> > > >       default:
> > > >               break;
> > > >       }
> > > > @@ -859,6 +1077,10 @@ static int tegra_usb_phy_power_off(struct tegra_usb_phy *phy)
> > > >               err = ulpi_phy_power_off(phy);
> > > >               break;
> > > >
> > > > +     case USBPHY_INTERFACE_MODE_HSIC:
> > > > +             err = uhsic_phy_power_off(phy);
> > > > +             break;
> > > > +
> > > >       default:
> > > >               break;
> > > >       }
> > > > @@ -1256,6 +1478,8 @@ static const struct tegra_phy_soc_config tegra20_soc_config = {
> > > >       .requires_usbmode_setup = false,
> > > >       .requires_extra_tuning_parameters = false,
> > > >       .requires_pmc_ao_power_up = false,
> > > > +     .uhsic_registers_shift = 0,
> > > > +     .uhsic_tx_rtune = 0, /* 40 ohm */
> > > >  };
> > > >
> > > >  static const struct tegra_phy_soc_config tegra30_soc_config = {
> > > > @@ -1264,6 +1488,8 @@ static const struct tegra_phy_soc_config tegra30_soc_config = {
> > > >       .requires_usbmode_setup = true,
> > > >       .requires_extra_tuning_parameters = true,
> > > >       .requires_pmc_ao_power_up = true,
> > > > +     .uhsic_registers_shift = 0x400,
> > > > +     .uhsic_tx_rtune = 8,  /* 50 ohm */
> > > >  };
> > > >
> > > >  static const struct of_device_id tegra_usb_phy_id_table[] = {
> > > > @@ -1360,6 +1586,7 @@ static int tegra_usb_phy_probe(struct platform_device *pdev)
> > > >       tegra_phy->phy_type = of_usb_get_phy_mode(np);
> > > >       switch (tegra_phy->phy_type) {
> > > >       case USBPHY_INTERFACE_MODE_UTMI:
> > > > +     case USBPHY_INTERFACE_MODE_HSIC:
> > > >               err = utmi_phy_probe(tegra_phy, pdev);
> > > >               if (err)
> > > >                       return err;
> > > > diff --git a/include/linux/usb/tegra_usb_phy.h b/include/linux/usb/tegra_usb_phy.h
> > > > index e394f4880b7e..4e79f1c2173a 100644
> > > > --- a/include/linux/usb/tegra_usb_phy.h
> > > > +++ b/include/linux/usb/tegra_usb_phy.h
> > > > @@ -24,6 +24,9 @@ struct gpio_desc;
> > > >   * requires_extra_tuning_parameters: true if xcvr_hsslew, hssquelch_level
> > > >   *      and hsdiscon_level should be set for adequate signal quality
> > > >   * requires_pmc_ao_power_up: true if USB AO is powered down by default
> > > > + * uhsic_registers_shift: for Tegra30+ where HSIC registers were shifted
> > > > + *      comparing to Tegra20 by 0x400, since Tegra20 has no UTMIP on PHY2
> > > > + * uhsic_tx_rtune: fine tuned 50 Ohm termination resistor for NMOS/PMOS driver
> > > >   */
> > > >
> > > >  struct tegra_phy_soc_config {
> > > > @@ -32,6 +35,8 @@ struct tegra_phy_soc_config {
> > > >       bool requires_usbmode_setup;
> > > >       bool requires_extra_tuning_parameters;
> > > >       bool requires_pmc_ao_power_up;
> > > > +     u32 uhsic_registers_shift;
> > >
> > > I would rather call this 'uhsic_registers_offset'. Shift first brings to mind bitshifts, and we often have fields in these config structs for bit shifts called '*_shift'.
> > >
> >
> > Yes, offset seems more appropriate.
> >
> > > > +     u32 uhsic_tx_rtune;
> > > >  };
> > > >
> > > >  struct tegra_utmip_config {
> > > >
> >
> > Mikko, thank you for your review, since HSIC patches already were
> > picked into linux/next I will prepare a few followup patches to
> > address some of uncertainties.
>
> Thank you!
>
>
>

Re: [PATCH v3 2/2] usb: phy: tegra: add HSIC support

[Mikko Perttunen]

On Monday, February 2, 2026 5:24 PM Svyatoslav Ryhel wrote:
> пн, 2 лют. 2026 р. о 10:06 Mikko Perttunen <mperttunen@nvidia.com> пише:
> >
> > On Monday, February 2, 2026 3:37 PM Svyatoslav Ryhel wrote:
> > > пн, 2 лют. 2026 р. о 06:07 Mikko Perttunen <mperttunen@nvidia.com> пише:
> > > >
> > > > On Friday, January 23, 2026 12:11 AM Svyatoslav Ryhel wrote:
> > > > > Add support for HSIC USB mode, which can be set for second USB controller
> > > > > and PHY on Tegra SoC along with already supported UTMI or ULPI.
> > > > >
> > > > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > > > ---
> > > > >  drivers/usb/phy/phy-tegra-usb.c   | 249 ++++++++++++++++++++++++++++--
> > > > >  include/linux/usb/tegra_usb_phy.h |   5 +
> > > > >  2 files changed, 243 insertions(+), 11 deletions(-)
> > > > >
> > > > > diff --git a/drivers/usb/phy/phy-tegra-usb.c b/drivers/usb/phy/phy-tegra-usb.c
> > > > > index afa5b5535f92..4f0fde33647e 100644
> > > > > --- a/drivers/usb/phy/phy-tegra-usb.c
> > > > > +++ b/drivers/usb/phy/phy-tegra-usb.c
> > > > > @@ -29,17 +29,26 @@
> > > > >  #include <linux/usb/tegra_usb_phy.h>
> > > > >  #include <linux/usb/ulpi.h>
> > > > >
> > > > > +#define USB_TXFILLTUNING                     0x154
> > > > > +#define USB_FIFO_TXFILL_THRES(x)             (((x) & 0x1f) << 16)
> > > > > +#define USB_FIFO_TXFILL_MASK                 0x1f0000
> > > > > +
> > > > >  #define ULPI_VIEWPORT                                0x170
> > > > >
> > > > >  /* PORTSC PTS/PHCD bits, Tegra20 only */
> > > > >  #define TEGRA_USB_PORTSC1                    0x184
> > > > > -#define TEGRA_USB_PORTSC1_PTS(x)             (((x) & 0x3) << 30)
> > > > > -#define TEGRA_USB_PORTSC1_PHCD                       BIT(23)
> > > > > +#define   TEGRA_USB_PORTSC1_PTS(x)           (((x) & 0x3) << 30)
> > > > > +#define   TEGRA_USB_PORTSC1_PHCD             BIT(23)
> > > > > +#define   TEGRA_USB_PORTSC1_WKOC             BIT(22)
> > > > > +#define   TEGRA_USB_PORTSC1_WKDS             BIT(21)
> > > > > +#define   TEGRA_USB_PORTSC1_WKCN             BIT(20)
> > > > >
> > > > >  /* HOSTPC1 PTS/PHCD bits, Tegra30 and above */
> > > > > +#define TEGRA30_USB_PORTSC1                  0x174
> > > > >  #define TEGRA_USB_HOSTPC1_DEVLC                      0x1b4
> > > > > -#define TEGRA_USB_HOSTPC1_DEVLC_PTS(x)               (((x) & 0x7) << 29)
> > > > > -#define TEGRA_USB_HOSTPC1_DEVLC_PHCD         BIT(22)
> > > > > +#define   TEGRA_USB_HOSTPC1_DEVLC_PTS(x)     (((x) & 0x7) << 29)
> > > > > +#define   TEGRA_USB_HOSTPC1_DEVLC_PHCD               BIT(22)
> > > > > +#define   TEGRA_USB_HOSTPC1_DEVLC_PTS_HSIC   BIT(2)
> > > >
> > > > PTS is an enumeration, not a bitfield, so I would say '4' instead of 'BIT(2)'
> > > >
> > >
> > > Noted
> > >
> > > > >
> > > > >  /* Bits of PORTSC1, which will get cleared by writing 1 into them */
> > > > >  #define TEGRA_PORTSC1_RWC_BITS       (PORT_CSC | PORT_PEC | PORT_OCC)
> > > > > @@ -51,11 +60,12 @@
> > > > >  #define   USB_SUSP_CLR                               BIT(5)
> > > > >  #define   USB_PHY_CLK_VALID                  BIT(7)
> > > > >  #define   UTMIP_RESET                                BIT(11)
> > > > > -#define   UHSIC_RESET                                BIT(11)
> > > > >  #define   UTMIP_PHY_ENABLE                   BIT(12)
> > > > >  #define   ULPI_PHY_ENABLE                    BIT(13)
> > > > >  #define   USB_SUSP_SET                               BIT(14)
> > > > > +#define   UHSIC_RESET                                BIT(14)
> > > > >  #define   USB_WAKEUP_DEBOUNCE_COUNT(x)               (((x) & 0x7) << 16)
> > > > > +#define   UHSIC_PHY_ENABLE                   BIT(19)
> > > > >
> > > > >  #define USB_PHY_VBUS_SENSORS                 0x404
> > > > >  #define   B_SESS_VLD_WAKEUP_EN                       BIT(14)
> > > > > @@ -156,6 +166,58 @@
> > > > >  #define UTMIP_BIAS_CFG1                              0x83c
> > > > >  #define   UTMIP_BIAS_PDTRK_COUNT(x)          (((x) & 0x1f) << 3)
> > > > >
> > > > > +/*
> > > > > + * Tegra20 has no UTMIP registers on PHY2 and UHSIC registers start from 0x800
> > > > > + * just where UTMIP registers should have been. This is the case only with Tegra20
> > > > > + * Tegra30+ have UTMIP registers at 0x800 and UHSIC registers shifter by 0x400
> > > >
> > > > 'shift', or 'are shifted'
> > > >
> > >
> > > Noted
> > >
> > > > > + * to 0xc00, but register layout is preserved.
> > > > > + */
> > > > > +#define UHSIC_PLL_CFG1                               0x804
> > > > > +#define   UHSIC_XTAL_FREQ_COUNT(x)           (((x) & 0xfff) << 0)
> > > > > +#define   UHSIC_PLLU_ENABLE_DLY_COUNT(x)     (((x) & 0x1f) << 14)
> > > > > +
> > > > > +#define UHSIC_HSRX_CFG0                              0x808
> > > > > +#define   UHSIC_ELASTIC_UNDERRUN_LIMIT(x)    (((x) & 0x1f) << 2)
> > > > > +#define   UHSIC_ELASTIC_OVERRUN_LIMIT(x)     (((x) & 0x1f) << 8)
> > > > > +#define   UHSIC_IDLE_WAIT(x)                 (((x) & 0x1f) << 13)
> > > > > +
> > > > > +#define UHSIC_HSRX_CFG1                              0x80c
> > > > > +#define   UHSIC_HS_SYNC_START_DLY(x)         (((x) & 0x1f) << 1)
> > > > > +
> > > > > +#define UHSIC_TX_CFG0                                0x810
> > > > > +#define   UHSIC_HS_READY_WAIT_FOR_VALID              BIT(9)
> > > > > +
> > > > > +#define UHSIC_MISC_CFG0                              0x814
> > > > > +#define   UHSIC_SUSPEND_EXIT_ON_EDGE         BIT(7)
> > > > > +#define   UHSIC_DETECT_SHORT_CONNECT         BIT(8)
> > > > > +#define   UHSIC_FORCE_XCVR_MODE                      BIT(15)
> > > > > +#define   UHSIC_DISABLE_BUSRESET             BIT(20)
> > > > > +
> > > > > +#define UHSIC_MISC_CFG1                              0x818
> > > > > +#define   UHSIC_PLLU_STABLE_COUNT(x)         (((x) & 0xfff) << 2)
> > > > > +
> > > > > +#define UHSIC_PADS_CFG0                              0x81c
> > > > > +#define   UHSIC_TX_RTUNEN                    0xf000
> > > > > +#define   UHSIC_TX_RTUNE(x)                  (((x) & 0xf) << 12)
> > > > > +
> > > > > +#define UHSIC_PADS_CFG1                              0x820
> > > > > +#define   UHSIC_PD_BG                                BIT(2)
> > > > > +#define   UHSIC_PD_TX                                BIT(3)
> > > > > +#define   UHSIC_PD_TRK                               BIT(4)
> > > > > +#define   UHSIC_PD_RX                                BIT(5)
> > > > > +#define   UHSIC_PD_ZI                                BIT(6)
> > > > > +#define   UHSIC_RX_SEL                               BIT(7)
> > > > > +#define   UHSIC_RPD_DATA                     BIT(9)
> > > > > +#define   UHSIC_RPD_STROBE                   BIT(10)
> > > > > +#define   UHSIC_RPU_DATA                     BIT(11)
> > > > > +#define   UHSIC_RPU_STROBE                   BIT(12)
> > > > > +
> > > > > +#define UHSIC_CMD_CFG0                               0x824
> > > > > +#define   UHSIC_PRETEND_CONNECT_DETECT               BIT(5)
> > > > > +
> > > > > +#define UHSIC_STAT_CFG0                              0x828
> > > > > +#define   UHSIC_CONNECT_DETECT                       BIT(0)
> > > > > +
> > > > >  /* For Tegra30 and above only, the address is different in Tegra20 */
> > > > >  #define USB_USBMODE                          0x1f8
> > > > >  #define   USB_USBMODE_MASK                   (3 << 0)
> > > > > @@ -174,7 +236,8 @@ struct tegra_xtal_freq {
> > > > >       u8 enable_delay;
> > > > >       u8 stable_count;
> > > > >       u8 active_delay;
> > > > > -     u8 xtal_freq_count;
> > > > > +     u8 utmi_xtal_freq_count;
> > > > > +     u16 hsic_xtal_freq_count;
> > > > >       u16 debounce;
> > > > >  };
> > > > >
> > > > > @@ -184,7 +247,8 @@ static const struct tegra_xtal_freq tegra_freq_table[] = {
> > > > >               .enable_delay = 0x02,
> > > > >               .stable_count = 0x2F,
> > > > >               .active_delay = 0x04,
> > > > > -             .xtal_freq_count = 0x76,
> > > > > +             .utmi_xtal_freq_count = 0x76,
> > > > > +             .hsic_xtal_freq_count = 0x1CA,
> > > > >               .debounce = 0x7530,
> > > > >       },
> > > > >       {
> > > > > @@ -192,7 +256,8 @@ static const struct tegra_xtal_freq tegra_freq_table[] = {
> > > > >               .enable_delay = 0x02,
> > > > >               .stable_count = 0x33,
> > > > >               .active_delay = 0x05,
> > > > > -             .xtal_freq_count = 0x7F,
> > > > > +             .utmi_xtal_freq_count = 0x7F,
> > > > > +             .hsic_xtal_freq_count = 0x1F0,
> > > > >               .debounce = 0x7EF4,
> > > > >       },
> > > > >       {
> > > > > @@ -200,7 +265,8 @@ static const struct tegra_xtal_freq tegra_freq_table[] = {
> > > > >               .enable_delay = 0x03,
> > > > >               .stable_count = 0x4B,
> > > > >               .active_delay = 0x06,
> > > > > -             .xtal_freq_count = 0xBB,
> > > > > +             .utmi_xtal_freq_count = 0xBB,
> > > > > +             .hsic_xtal_freq_count = 0x2DD,
> > > > >               .debounce = 0xBB80,
> > > > >       },
> > > > >       {
> > > > > @@ -208,7 +274,8 @@ static const struct tegra_xtal_freq tegra_freq_table[] = {
> > > > >               .enable_delay = 0x04,
> > > > >               .stable_count = 0x66,
> > > > >               .active_delay = 0x09,
> > > > > -             .xtal_freq_count = 0xFE,
> > > > > +             .utmi_xtal_freq_count = 0xFE,
> > > > > +             .hsic_xtal_freq_count = 0x3E0,
> > > > >               .debounce = 0xFDE8,
> > > > >       },
> > > > >  };
> > > > > @@ -541,7 +608,7 @@ static int utmi_phy_power_on(struct tegra_usb_phy *phy)
> > > > >               val = readl_relaxed(base + UTMIP_PLL_CFG1);
> > > > >               val &= ~(UTMIP_XTAL_FREQ_COUNT(~0) |
> > > > >                       UTMIP_PLLU_ENABLE_DLY_COUNT(~0));
> > > > > -             val |= UTMIP_XTAL_FREQ_COUNT(phy->freq->xtal_freq_count) |
> > > > > +             val |= UTMIP_XTAL_FREQ_COUNT(phy->freq->utmi_xtal_freq_count) |
> > > > >                       UTMIP_PLLU_ENABLE_DLY_COUNT(phy->freq->enable_delay);
> > > > >               writel_relaxed(val, base + UTMIP_PLL_CFG1);
> > > > >       }
> > > > > @@ -812,6 +879,153 @@ static int ulpi_phy_power_off(struct tegra_usb_phy *phy)
> > > > >       return 0;
> > > > >  }
> > > > >
> > > > > +static u32 tegra_hsic_readl(struct tegra_usb_phy *phy, u32 reg)
> > > > > +{
> > > > > +     void __iomem *base = phy->regs;
> > > > > +     u32 shift = phy->soc_config->uhsic_registers_shift;
> > > > > +
> > > > > +     return readl_relaxed(base + shift + reg);
> > > > > +}
> > > > > +
> > > > > +static void tegra_hsic_writel(struct tegra_usb_phy *phy, u32 reg, u32 value)
> > > > > +{
> > > > > +     void __iomem *base = phy->regs;
> > > > > +     u32 shift = phy->soc_config->uhsic_registers_shift;
> > > > > +
> > > > > +     writel_relaxed(value, base + shift + reg);
> > > > > +}
> > > > > +
> > > > > +static int uhsic_phy_power_on(struct tegra_usb_phy *phy)
> > > > > +{
> > > > > +     struct tegra_utmip_config *config = phy->config;
> > > > > +     void __iomem *base = phy->regs;
> > > > > +     u32 val;
> > > > > +
> > > > > +     val = tegra_hsic_readl(phy, UHSIC_PADS_CFG1);
> > > > > +     val &= ~(UHSIC_PD_BG | UHSIC_PD_TX | UHSIC_PD_TRK | UHSIC_PD_RX |
> > > > > +              UHSIC_PD_ZI | UHSIC_RPD_DATA | UHSIC_RPD_STROBE);
> > > > > +     val |= UHSIC_RX_SEL;
> > > >
> > > > L4T r16 (Tegra30) keeps UHSIC_PD_TX set until after UHSIC_RESET has been cleared. Commit message says this avoids a signal glitch.
> > > >
> > >
> > > I did not notice any difference with this change and without. Since it
> > > does not affect detection of modem by my device I can apply it (is it
> > > worth applying at all?). Should it be applied globally or for Tegra30+
> > > only?
> >
> > Downstream only has it for Tegra30, but that's probably just because it was tested in the Tegra30 timeframe. If it's not causing any issue on Tegra20 I would apply it globally. Considering it's a signal glitch it might only have an effect in marginal signal situations.
> >
> > >
> > > > > +     tegra_hsic_writel(phy, UHSIC_PADS_CFG1, val);
> > > > > +
> > > > > +     udelay(2);
> > > > > +
> > > > > +     val = readl_relaxed(base + USB_SUSP_CTRL);
> > > > > +     val |= UHSIC_RESET;
> > > > > +     writel_relaxed(val, base + USB_SUSP_CTRL);
> > > > > +
> > > > > +     udelay(30);
> > > > > +
> > > > > +     val = readl_relaxed(base + USB_SUSP_CTRL);
> > > > > +     val |= UHSIC_PHY_ENABLE;
> > > > > +     writel_relaxed(val, base + USB_SUSP_CTRL);
> > > > > +
> > > > > +     val = tegra_hsic_readl(phy, UHSIC_HSRX_CFG0);
> > > > > +     val &= ~(UHSIC_IDLE_WAIT(~0) |
> > > > > +              UHSIC_ELASTIC_UNDERRUN_LIMIT(~0) |
> > > > > +              UHSIC_ELASTIC_OVERRUN_LIMIT(~0));
> > > > > +     val |= UHSIC_IDLE_WAIT(config->idle_wait_delay) |
> > > > > +             UHSIC_ELASTIC_UNDERRUN_LIMIT(config->elastic_limit) |
> > > > > +             UHSIC_ELASTIC_OVERRUN_LIMIT(config->elastic_limit);
> > > > > +     tegra_hsic_writel(phy, UHSIC_HSRX_CFG0, val);
> > > > > +
> > > > > +     val = tegra_hsic_readl(phy, UHSIC_HSRX_CFG1);
> > > > > +     val &= ~UHSIC_HS_SYNC_START_DLY(~0);
> > > > > +     val |= UHSIC_HS_SYNC_START_DLY(config->hssync_start_delay);
> > > > > +     tegra_hsic_writel(phy, UHSIC_HSRX_CFG1, val);
> > > > > +
> > > >
> > > > L4T r16 (Tegra30) clears UHSIC_TX_CFG0.UHSIC_HS_READY_WAIT_FOR_VALID here. According to commit message this fixes an intermittent failure to connect to HSIC modem. It also sets UHSIC_MISC_CFG0.UHSIC_DISABLE_BUSRESET  below due to same issue.
> > > >
> > >
> > > And this change causes my modem to fail entirely.
> > >
> > > [   10.145976] usb 1-1: new high-speed USB device number 2 using ci_hdrc
> > > [   10.295970] usb 1-1: device descriptor read/64, error -71
> > > [   10.545975] usb 1-1: device descriptor read/64, error -71
> > > [   10.795990] usb 1-1: new high-speed USB device number 3 using ci_hdrc
> > > [   10.935970] usb 1-1: device descriptor read/64, error -71
> > > [   11.185977] usb 1-1: device descriptor read/64, error -71
> > >
> > > I assume UHSIC_DISABLE_BUSRESET, UHSIC_HS_READY_WAIT_FOR_VALID and
> > > UHSIC_PD_TX workarounds are all consequences of how modem in
> > > downstream kernel is called. Instead of relaying on modem to lead the
> > > controller probe, downstream just brutally removes and reinits
> > > controller until modem probes. I have never observed modem not
> > > appearing without any of discussed patches.
> >
> > The downstream commit says that without this workaround, the modem would sometimes not come up in stress tests. So I think it's possible there is still a rare hardware bug that this is working around, but perhaps we're missing some other part and that's why it's not working. I think it's fine to drop these changes.
> >
> 
> I propose not adding any of the three workarounds for now. I don't
> have a Tegra20 with an HSIC modem, as it seems that most Tegra20
> devices use ULPI mode. Among my personal devices, only the LG Optimus
> Vu (P895) is a GSM device with an XMM HSIC modem, and it fails if I
> add UHSIC_DISABLE_BUSRESET and UHSIC_HS_READY_WAIT_FOR_VALID. Since I
> may be the only one at the moment actively working in this area of
> legacy Tegra SoCs — and this discussion is public — anyone who has
> issues can contact me to resolve them. Additionally, I will keep in
> mind that there may be a need for these workarounds; if I find any
> need for them during modem bring-ups in the legacy Tegra community, I
> will submit dedicated patches with explanations.

Sure, that sounds fine.

Mikko

> 
> > >
> > >
> > > > > +     val = tegra_hsic_readl(phy, UHSIC_MISC_CFG0);
> > > > > +     val |= UHSIC_SUSPEND_EXIT_ON_EDGE;
> > > > > +     tegra_hsic_writel(phy, UHSIC_MISC_CFG0, val);
> > > > > +
> > > > > +     val = tegra_hsic_readl(phy, UHSIC_MISC_CFG1);
> > > > > +     val &= ~UHSIC_PLLU_STABLE_COUNT(~0);
> > > > > +     val |= UHSIC_PLLU_STABLE_COUNT(phy->freq->stable_count);
> > > > > +     tegra_hsic_writel(phy, UHSIC_MISC_CFG1, val);
> > > > > +
> > > > > +     val = tegra_hsic_readl(phy, UHSIC_PLL_CFG1);
> > > > > +     val &= ~(UHSIC_XTAL_FREQ_COUNT(~0) |
> > > > > +             UHSIC_PLLU_ENABLE_DLY_COUNT(~0));
> > > > > +     val |= UHSIC_XTAL_FREQ_COUNT(phy->freq->hsic_xtal_freq_count) |
> > > > > +             UHSIC_PLLU_ENABLE_DLY_COUNT(phy->freq->enable_delay);
> > > > > +     tegra_hsic_writel(phy, UHSIC_PLL_CFG1, val);
> > > > > +
> > > > > +     val = readl_relaxed(base + USB_SUSP_CTRL);
> > > > > +     val &= ~UHSIC_RESET;
> > > > > +     writel_relaxed(val, base + USB_SUSP_CTRL);
> > > > > +
> > > > > +     udelay(2);
> > > > > +
> > > > > +     if (phy->soc_config->requires_usbmode_setup) {
> > > > > +             val = readl_relaxed(base + USB_USBMODE);
> > > > > +             val &= ~USB_USBMODE_MASK;
> > > > > +             if (phy->mode == USB_DR_MODE_HOST)
> > > > > +                     val |= USB_USBMODE_HOST;
> > > > > +             else
> > > > > +                     val |= USB_USBMODE_DEVICE;
> > > > > +             writel_relaxed(val, base + USB_USBMODE);
> > > > > +     }
> > > > > +
> > > > > +     if (phy->soc_config->has_hostpc)
> > > > > +             set_pts(phy, TEGRA_USB_HOSTPC1_DEVLC_PTS_HSIC);
> > > > > +     else
> > > > > +             set_pts(phy, 0);
> > > >
> > > > This (and below) are abusing has_hostpc to detect Tegra30 vs Tegra20. We should instead add soc_config fields hsic_pts_value and portsc1_offset.
> > > >
> > > > > +
> > > > > +     val = readl_relaxed(base + USB_TXFILLTUNING);
> > > > > +     if ((val & USB_FIFO_TXFILL_MASK) != USB_FIFO_TXFILL_THRES(0x10)) {
> > > > > +             val = USB_FIFO_TXFILL_THRES(0x10);
> > > > > +             writel_relaxed(val, base + USB_TXFILLTUNING);
> > > > > +     }
> > > > > +
> > > > > +     if (phy->soc_config->has_hostpc) {
> > > > > +             val = readl_relaxed(base + TEGRA30_USB_PORTSC1);
> > > > > +             val &= ~(TEGRA_USB_PORTSC1_WKOC | TEGRA_USB_PORTSC1_WKDS |
> > > > > +                      TEGRA_USB_PORTSC1_WKCN);
> > > > > +             writel_relaxed(val, base + TEGRA30_USB_PORTSC1);
> > > > > +     } else {
> > > > > +             val = readl_relaxed(base + TEGRA_USB_PORTSC1);
> > > > > +             val &= ~(TEGRA_USB_PORTSC1_WKOC | TEGRA_USB_PORTSC1_WKDS |
> > > > > +                      TEGRA_USB_PORTSC1_WKCN);
> > > > > +             writel_relaxed(val, base + TEGRA_USB_PORTSC1);
> > > > > +     }
> > > > > +
> > > > > +     val = tegra_hsic_readl(phy, UHSIC_PADS_CFG0);
> > > > > +     val &= ~UHSIC_TX_RTUNEN;
> > > > > +     val |= UHSIC_TX_RTUNE(phy->soc_config->uhsic_tx_rtune);
> > > > > +     tegra_hsic_writel(phy, UHSIC_PADS_CFG0, val);
> > > > > +
> > > > > +     if (utmi_wait_register(base + USB_SUSP_CTRL, USB_PHY_CLK_VALID,
> > > > > +                            USB_PHY_CLK_VALID))
> > > > > +             dev_err(phy->u_phy.dev,
> > > > > +                     "Timeout waiting for PHY to stabilize on enable (HSIC)\n");
> > > >
> > > > Should return error (return value of utmi_wait_register) here?
> > > >
> > >
> > > Noted.
> > >
> > > > > +
> > > > > +     return 0;
> > > > > +}
> > > > > +
> > > > > +static int uhsic_phy_power_off(struct tegra_usb_phy *phy)
> > > > > +{
> > > > > +     void __iomem *base = phy->regs;
> > > > > +     u32 val;
> > > > > +
> > > > > +     set_phcd(phy, true);
> > > > > +
> > > > > +     val = tegra_hsic_readl(phy, UHSIC_PADS_CFG1);
> > > > > +     val |= (UHSIC_PD_BG | UHSIC_PD_TX | UHSIC_PD_TRK | UHSIC_PD_RX |
> > > > > +             UHSIC_PD_ZI | UHSIC_RPD_DATA | UHSIC_RPD_STROBE);
> > > > > +     tegra_hsic_writel(phy, UHSIC_PADS_CFG1, val);
> > > > > +
> > > > > +     val = readl_relaxed(base + USB_SUSP_CTRL);
> > > > > +     val |= UHSIC_RESET;
> > > > > +     writel_relaxed(val, base + USB_SUSP_CTRL);
> > > > > +
> > > > > +     udelay(30);
> > > > > +
> > > > > +     val = readl_relaxed(base + USB_SUSP_CTRL);
> > > > > +     val &= ~UHSIC_PHY_ENABLE;
> > > > > +     writel_relaxed(val, base + USB_SUSP_CTRL);
> > > > > +
> > > > > +     return 0;
> > > > > +}
> > > > > +
> > > > >  static int tegra_usb_phy_power_on(struct tegra_usb_phy *phy)
> > > > >  {
> > > > >       int err = 0;
> > > > > @@ -828,6 +1042,10 @@ static int tegra_usb_phy_power_on(struct tegra_usb_phy *phy)
> > > > >               err = ulpi_phy_power_on(phy);
> > > > >               break;
> > > > >
> > > > > +     case USBPHY_INTERFACE_MODE_HSIC:
> > > > > +             err = uhsic_phy_power_on(phy);
> > > > > +             break;
> > > > > +
> > > > >       default:
> > > > >               break;
> > > > >       }
> > > > > @@ -859,6 +1077,10 @@ static int tegra_usb_phy_power_off(struct tegra_usb_phy *phy)
> > > > >               err = ulpi_phy_power_off(phy);
> > > > >               break;
> > > > >
> > > > > +     case USBPHY_INTERFACE_MODE_HSIC:
> > > > > +             err = uhsic_phy_power_off(phy);
> > > > > +             break;
> > > > > +
> > > > >       default:
> > > > >               break;
> > > > >       }
> > > > > @@ -1256,6 +1478,8 @@ static const struct tegra_phy_soc_config tegra20_soc_config = {
> > > > >       .requires_usbmode_setup = false,
> > > > >       .requires_extra_tuning_parameters = false,
> > > > >       .requires_pmc_ao_power_up = false,
> > > > > +     .uhsic_registers_shift = 0,
> > > > > +     .uhsic_tx_rtune = 0, /* 40 ohm */
> > > > >  };
> > > > >
> > > > >  static const struct tegra_phy_soc_config tegra30_soc_config = {
> > > > > @@ -1264,6 +1488,8 @@ static const struct tegra_phy_soc_config tegra30_soc_config = {
> > > > >       .requires_usbmode_setup = true,
> > > > >       .requires_extra_tuning_parameters = true,
> > > > >       .requires_pmc_ao_power_up = true,
> > > > > +     .uhsic_registers_shift = 0x400,
> > > > > +     .uhsic_tx_rtune = 8,  /* 50 ohm */
> > > > >  };
> > > > >
> > > > >  static const struct of_device_id tegra_usb_phy_id_table[] = {
> > > > > @@ -1360,6 +1586,7 @@ static int tegra_usb_phy_probe(struct platform_device *pdev)
> > > > >       tegra_phy->phy_type = of_usb_get_phy_mode(np);
> > > > >       switch (tegra_phy->phy_type) {
> > > > >       case USBPHY_INTERFACE_MODE_UTMI:
> > > > > +     case USBPHY_INTERFACE_MODE_HSIC:
> > > > >               err = utmi_phy_probe(tegra_phy, pdev);
> > > > >               if (err)
> > > > >                       return err;
> > > > > diff --git a/include/linux/usb/tegra_usb_phy.h b/include/linux/usb/tegra_usb_phy.h
> > > > > index e394f4880b7e..4e79f1c2173a 100644
> > > > > --- a/include/linux/usb/tegra_usb_phy.h
> > > > > +++ b/include/linux/usb/tegra_usb_phy.h
> > > > > @@ -24,6 +24,9 @@ struct gpio_desc;
> > > > >   * requires_extra_tuning_parameters: true if xcvr_hsslew, hssquelch_level
> > > > >   *      and hsdiscon_level should be set for adequate signal quality
> > > > >   * requires_pmc_ao_power_up: true if USB AO is powered down by default
> > > > > + * uhsic_registers_shift: for Tegra30+ where HSIC registers were shifted
> > > > > + *      comparing to Tegra20 by 0x400, since Tegra20 has no UTMIP on PHY2
> > > > > + * uhsic_tx_rtune: fine tuned 50 Ohm termination resistor for NMOS/PMOS driver
> > > > >   */
> > > > >
> > > > >  struct tegra_phy_soc_config {
> > > > > @@ -32,6 +35,8 @@ struct tegra_phy_soc_config {
> > > > >       bool requires_usbmode_setup;
> > > > >       bool requires_extra_tuning_parameters;
> > > > >       bool requires_pmc_ao_power_up;
> > > > > +     u32 uhsic_registers_shift;
> > > >
> > > > I would rather call this 'uhsic_registers_offset'. Shift first brings to mind bitshifts, and we often have fields in these config structs for bit shifts called '*_shift'.
> > > >
> > >
> > > Yes, offset seems more appropriate.
> > >
> > > > > +     u32 uhsic_tx_rtune;
> > > > >  };
> > > > >
> > > > >  struct tegra_utmip_config {
> > > > >
> > >
> > > Mikko, thank you for your review, since HSIC patches already were
> > > picked into linux/next I will prepare a few followup patches to
> > > address some of uncertainties.
> >
> > Thank you!
> >
> >
> >