mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v7] phy: Add USB3 PHY support to Google Tensor SoC USB PHY driver
@ 2026-09-18 22:25 RD Babiera
  2026-09-21 18:29 ` Neill Kapron
  0 siblings, 1 reply; 3+ messages in thread
From: RD Babiera @ 2026-09-18 22:25 UTC (permalink / raw)
  To: vkoul, peter.griffin, andre.draszik, tudor.ambarus, p.zabel,
	neil.armstrong
  Cc: badhri, nkapron, linux-arm-kernel, linux-samsung-soc, linux-phy,
	linux-kernel, rdbabiera

Add USB3 PHY support for the Google Tensor G5 USB PHY driver.
This patch adds functionality for the usb3_tca register, usb3 clock,
and usb3 reset as defined in google,lga-usb-phy.yaml. Kconfig now lists
USB SuperSpeed support.

Refactor the probe sequence to initialize the USB2 and USB3 PHYs, and then
initialize clocks and resets for both PHYs afterwards.

Refactor set_vbus_valid to reduce duplicated code.

Implement USB3 phy_ops for phy_init, phy_exit, and phy_power_on.
combo_phy_state enum is added to track PHY bringup state across
PHY API calls. google_usb_set_orientation can reprogram the TCA when the
PHY is ready.

google_usb_set_orientation now calls pm_runtime_get_if_active to
guarantee register access. Setting orientation and pm_runtime management
are now handled within phy_mutex.

Signed-off-by: RD Babiera <rdbabiera@google.com>
---
Changes since v1:
* Removed mix of goto-based and scope-based cleanup from usb3 phy_init
* Removed unused usb3_core resource from probe
* Added combo_phy_state enum to interally track ComboPHY bringup state
  to allow google_usb_set_orientation() to change TCA orientation.
* Modify Kconfig documentation to reflect SuperSpeed support

Changes since v2:
* google_usb3_phy_init now sets USBDP_TOP_CFG_REG_PMGT_REF_CLK_REQ_N
  to false if phy_init fails elsewhere.
* google_usb3_phy_init errors are now handled via DEFINE_FREE structures.
  This affects set_pmgt_ref_clk_req_n, clk_bulk_prepare_enable, and
  reset_control_bulk_deassert.
* google_usb2_phy_init also handles undoing clk_bulk_prepare_enable via
  DEFINE_FREE structure.
* google_usb3_phy_power_on allows program_tca_locked in the
  COMBO_PHY_TCA_READY state. Waiting for PoR=>NC is only performed once.
* Note: there are checkpatch errors for the DEFINE_FREE macros resulting
  in "ERROR: trailing statements should be on next line". Other cases of
  DEFINE_FREE where the line limit would otherwise exceed 100 columns
  have the indentation done the same way.

Changes since v3:
* set_pmgt_ref_clk_req_n(false) in google_usb3_phy_exit() comes after
  reset assertion and clock disable to match phy_init() sequence.
* program_tca_locked in usb3 power_on() now requires a valid orientation
  to match google_usb_set_orientation requirements.

Changes since v4:
* google_usb_set_orientation sets gphy->orientation while holding
  phy_mutex.
* google_usb_set_orientation now holds a pm_runtime reference before
  checking for suspend, and calls put_autosuspend prior to normal
  exit.

Changes since v5:
* google_usb_set_orientation replaces pm_runtime_suspended check with
  pm_runtime_get_if_active call.

Changes since v6:
* google_usb_set_orientation uses pm_runtime_put() instead of
  pm_runtime_put_autosuspend().
* Added newline to all dev_* logs
* xa_ack sw poll changed to 100ms, and xa_timeout_val is programmed
  to match during usb3 phy init
* google_usb3_phy_power_on returns -EINVAL when PHY is idle and
  only performs PoR->NC wait in COMBO_PHY_INIT_DONE
---
 drivers/phy/Kconfig          |   2 +-
 drivers/phy/phy-google-usb.c | 434 +++++++++++++++++++++++++++++++----
 2 files changed, 393 insertions(+), 43 deletions(-)

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 19f3b7d12b7d..d2d401129af7 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -100,7 +100,7 @@ config PHY_GOOGLE_USB
 	  the G5 generation (Laguna). This driver provides the PHY interfaces
 	  to interact with the SNPS eUSB2 and USB 3.2/DisplayPort Combo PHY,
 	  both of which are integrated with the DWC3 USB DRD controller.
-	  This driver currently supports USB high-speed.
+	  This driver currently supports USB high-speed and SuperSpeed.
 
 config USB_LGM_PHY
 	tristate "INTEL Lightning Mountain USB PHY Driver"
diff --git a/drivers/phy/phy-google-usb.c b/drivers/phy/phy-google-usb.c
index ab20bc20f19e..f2716125ae92 100644
--- a/drivers/phy/phy-google-usb.c
+++ b/drivers/phy/phy-google-usb.c
@@ -20,6 +20,7 @@
 #include <linux/reset.h>
 #include <linux/usb/typec_mux.h>
 
+/* USB_CFG_CSR */
 #define USBCS_USB2PHY_CFG19_OFFSET 0x0
 #define USBCS_USB2PHY_CFG19_PHY_CFG_PLL_FB_DIV GENMASK(19, 8)
 
@@ -28,11 +29,44 @@
 #define USBCS_USB2PHY_CFG21_REF_FREQ_SEL GENMASK(15, 13)
 #define USBCS_USB2PHY_CFG21_PHY_TX_DIG_BYPASS_SEL BIT(19)
 
+/* USBDP_TOP */
 #define USBCS_PHY_CFG1_OFFSET 0x28
+#define USBCS_PHY_CFG1_PHY0_MPLLA_SSC_EN BIT(1)
+#define USBCS_PHY_CFG1_PHY0_SRAM_BYPASS_MODE GENMASK(11, 10)
+#define SRAM_BYPASS_MODE_BYPASS_FIRMWARE BIT(0)
+#define SRAM_BYPASS_MODE_BYPASS_CONTEXT BIT(1)
 #define USBCS_PHY_CFG1_SYS_VBUSVALID BIT(17)
 
+#define USBDP_TOP_CFG_REG_OFFSET 0x44
+#define USBDP_TOP_CFG_REG_PMGT_REF_CLK_REQ_N BIT(0)
+
+#define PHY_POWER_CONFIG_REG1_OFFSET 0x48
+#define PHY_POWER_CONFIG_REG1_PG_MODE_EN BIT(1)
+#define PHY_POWER_CONFIG_REG1_UPCS_PIPE_CONFIG GENMASK(31, 14)
+#define UPCS_PIPE_CONFIG_ISO_CPM BIT(5)
+#define UPCS_PIPE_CONFIG_PG_MODE_STATIC BIT(6)
+#define UPCS_PIPE_CONFIG_LANE_RESET_NO_PG_EXIT BIT(9)
+
+/* USB3_TCA */
+#define TCA_INTR_STS_OFFSET 0x8
+#define TCA_INTR_STS_XA_ACT_EVT BIT(0)
+#define TCA_TCPC_OFFSET 0x14
+#define TCA_TCPC_MUX_CONTROL GENMASK(2, 0)
+#define TCA_TCPC_MUX_CONTROL_USB_ONLY 0x1
+#define TCA_TCPC_CONNECTOR_ORIENTATION BIT(3)
+#define TCA_TCPC_VALID BIT(4)
+#define TCA_CTRLSYNCMODE_CFG1_OFFSET 0x24
+#define TCA_CTRLSYNCMODE_CFG1_XA_TIMEOUT_VAL GENMASK(19, 0)
+#define TCA_CTRLSYNCMODE_CFG1_XA_TIMEOUT_VAL_100MS 0x1e85
+#define TCA_PSTATE_0_OFFSET 0x50
+#define TCA_PSTATE_0_UPCS_LANE0_PHYSTATUS BIT(8)
+
+#define GPHY_TCA_DELAY_US 10
+#define GPHY_TCA_TIMEOUT_US 100000
+
 enum google_usb_phy_id {
 	GOOGLE_USB2_PHY,
+	GOOGLE_USB3_PHY,
 	GOOGLE_USB_PHY_NUM,
 };
 
@@ -46,53 +80,199 @@ struct google_usb_phy_instance {
 	struct reset_control_bulk_data *rsts;
 };
 
+struct google_usb_phy_config {
+	const char * const *clk_names;
+	unsigned int num_clks;
+	const char * const *rst_names;
+	unsigned int num_rsts;
+};
+
+static const char * const u2phy_clk_names[] = {
+	"usb2",
+	"usb2_apb",
+};
+static const char * const u3phy_clk_names[] = {
+	"usb3"
+};
+static const char * const u2phy_rst_names[] = {
+	"usb2",
+	"usb2_apb",
+};
+static const char * const u3phy_rst_names[] = {
+	"usb3"
+};
+
+static const struct google_usb_phy_config phy_configs[GOOGLE_USB_PHY_NUM] = {
+	[GOOGLE_USB2_PHY] = {
+		.clk_names = u2phy_clk_names,
+		.num_clks = ARRAY_SIZE(u2phy_clk_names),
+		.rst_names = u2phy_rst_names,
+		.num_rsts = ARRAY_SIZE(u2phy_rst_names),
+	},
+	[GOOGLE_USB3_PHY] = {
+		.clk_names = u3phy_clk_names,
+		.num_clks = ARRAY_SIZE(u3phy_clk_names),
+		.rst_names = u3phy_rst_names,
+		.num_rsts = ARRAY_SIZE(u3phy_rst_names),
+	},
+};
+
+static inline void google_usb_phy_clk_disable(struct google_usb_phy_instance *inst)
+{
+	clk_bulk_disable_unprepare(inst->num_clks, inst->clks);
+}
+DEFINE_FREE(inst_clk_disable, struct google_usb_phy_instance *,
+	    if (_T) google_usb_phy_clk_disable(_T))
+
+static inline void google_usb_phy_rst_disable(struct google_usb_phy_instance *inst)
+{
+	reset_control_bulk_assert(inst->num_rsts, inst->rsts);
+}
+DEFINE_FREE(inst_rst_disable, struct google_usb_phy_instance *,
+	    if (_T) google_usb_phy_rst_disable(_T))
+
+/*
+ * combo_phy_state
+ *	COMBO_PHY_IDLE: The ComboPHY has been torn down and USB3 has not completed
+ *			bringup
+ *	COMBO_PHY_INIT_DONE: The ComboPHY bringup sequence is complete.
+ *	COMBO_PHY_TCA_READY: The PoR => NC transition is complete, and the TCA can be
+ *			     moved into USB.
+ */
+enum combo_phy_state {
+	COMBO_PHY_IDLE,
+	COMBO_PHY_INIT_DONE,
+	COMBO_PHY_TCA_READY,
+};
+
 struct google_usb_phy {
 	struct device *dev;
 	struct regmap *usb_cfg_regmap;
 	unsigned int usb2_cfg_offset;
 	void __iomem *usbdp_top_base;
+	void __iomem *usb3_tca_base;
 	struct google_usb_phy_instance *insts;
 	/*
 	 * Protect phy registers from concurrent access, specifically via
-	 * google_usb_set_orientation callback.
+	 * google_usb_set_orientation callback. phy_mutex also protects
+	 * concurrent access to phy_state.
 	 */
 	struct mutex phy_mutex;
 	struct typec_switch_dev *sw;
 	enum typec_orientation orientation;
+	enum combo_phy_state phy_state;
 };
 
 static void set_vbus_valid(struct google_usb_phy *gphy)
 {
 	u32 reg;
 
-	if (gphy->orientation == TYPEC_ORIENTATION_NONE) {
-		reg = readl(gphy->usbdp_top_base + USBCS_PHY_CFG1_OFFSET);
+	reg = readl(gphy->usbdp_top_base + USBCS_PHY_CFG1_OFFSET);
+	if (gphy->orientation == TYPEC_ORIENTATION_NONE)
 		reg &= ~USBCS_PHY_CFG1_SYS_VBUSVALID;
-		writel(reg, gphy->usbdp_top_base + USBCS_PHY_CFG1_OFFSET);
-	} else {
-		reg = readl(gphy->usbdp_top_base + USBCS_PHY_CFG1_OFFSET);
+	else
 		reg |= USBCS_PHY_CFG1_SYS_VBUSVALID;
-		writel(reg, gphy->usbdp_top_base + USBCS_PHY_CFG1_OFFSET);
-	}
+	writel(reg, gphy->usbdp_top_base + USBCS_PHY_CFG1_OFFSET);
+}
+
+static void set_sram_bypass(struct google_usb_phy *gphy, u32 bypass)
+{
+	u32 reg;
+
+	reg = readl(gphy->usbdp_top_base + USBCS_PHY_CFG1_OFFSET);
+	reg &= ~USBCS_PHY_CFG1_PHY0_SRAM_BYPASS_MODE;
+	reg |= FIELD_PREP(USBCS_PHY_CFG1_PHY0_SRAM_BYPASS_MODE, bypass);
+	writel(reg, gphy->usbdp_top_base + USBCS_PHY_CFG1_OFFSET);
+}
+
+static void set_pmgt_ref_clk_req_n(struct google_usb_phy *gphy, bool resume)
+{
+	u32 reg;
+
+	reg = readl(gphy->usbdp_top_base + USBDP_TOP_CFG_REG_OFFSET);
+	if (resume)
+		reg |= USBDP_TOP_CFG_REG_PMGT_REF_CLK_REQ_N;
+	else
+		reg &= ~USBDP_TOP_CFG_REG_PMGT_REF_CLK_REQ_N;
+	writel(reg, gphy->usbdp_top_base + USBDP_TOP_CFG_REG_OFFSET);
+}
+
+static inline void disable_pmgt_ref_clk_req_n(struct google_usb_phy *gphy)
+{
+	set_pmgt_ref_clk_req_n(gphy, false);
+}
+DEFINE_FREE(pmgt_ref_clk_req_n, struct google_usb_phy *, if (_T) disable_pmgt_ref_clk_req_n(_T))
+
+static int wait_tca_xa_ack(struct google_usb_phy *gphy)
+{
+	int ret;
+	u32 reg;
+
+	ret = readl_poll_timeout(gphy->usb3_tca_base + TCA_INTR_STS_OFFSET,
+				 reg, !!(reg & TCA_INTR_STS_XA_ACT_EVT),
+				 GPHY_TCA_DELAY_US, GPHY_TCA_TIMEOUT_US);
+	if (ret)
+		dev_err(gphy->dev, "tca xa_ack timeout, ret=%d\n", ret);
+
+	return ret;
+}
+
+static int program_tca_locked(struct google_usb_phy *gphy)
+	   __must_hold(&gphy->phy_mutex)
+{
+	int ret;
+	u32 reg;
+
+	reg = readl(gphy->usb3_tca_base + TCA_INTR_STS_OFFSET);
+	writel(reg, gphy->usb3_tca_base + TCA_INTR_STS_OFFSET);
+
+	reg = readl(gphy->usb3_tca_base + TCA_TCPC_OFFSET);
+	reg &= ~TCA_TCPC_MUX_CONTROL;
+	reg |= FIELD_PREP(TCA_TCPC_MUX_CONTROL, TCA_TCPC_MUX_CONTROL_USB_ONLY);
+	if (gphy->orientation == TYPEC_ORIENTATION_REVERSE)
+		reg |= TCA_TCPC_CONNECTOR_ORIENTATION;
+	else
+		reg &= ~TCA_TCPC_CONNECTOR_ORIENTATION;
+	reg |= TCA_TCPC_VALID;
+	writel(reg, gphy->usb3_tca_base + TCA_TCPC_OFFSET);
+
+	ret = wait_tca_xa_ack(gphy);
+	dev_dbg(gphy->dev, "TCA switch %s, mux %lu, orientation %s\n",
+		ret ? "failed" : "success",
+		FIELD_GET(TCA_TCPC_MUX_CONTROL, reg),
+		FIELD_GET(TCA_TCPC_CONNECTOR_ORIENTATION, reg) ? "reverse" : "normal");
+
+	reg = readl(gphy->usb3_tca_base + TCA_INTR_STS_OFFSET);
+	writel(reg, gphy->usb3_tca_base + TCA_INTR_STS_OFFSET);
+
+	return ret;
 }
 
 static int google_usb_set_orientation(struct typec_switch_dev *sw,
 				      enum typec_orientation orientation)
 {
 	struct google_usb_phy *gphy = typec_switch_get_drvdata(sw);
+	int ret = 0;
 
 	dev_dbg(gphy->dev, "set orientation %d\n", orientation);
 
-	gphy->orientation = orientation;
+	guard(mutex)(&gphy->phy_mutex);
 
-	if (pm_runtime_suspended(gphy->dev))
-		return 0;
+	gphy->orientation = orientation;
 
-	guard(mutex)(&gphy->phy_mutex);
+	if (IS_ENABLED(CONFIG_PM)) {
+		if (pm_runtime_get_if_active(gphy->dev) <= 0)
+			return 0;
+	}
 
 	set_vbus_valid(gphy);
 
-	return 0;
+	if (gphy->phy_state == COMBO_PHY_TCA_READY && orientation != TYPEC_ORIENTATION_NONE)
+		ret = program_tca_locked(gphy);
+
+	pm_runtime_put(gphy->dev);
+
+	return ret;
 }
 
 static int google_usb2_phy_init(struct phy *_phy)
@@ -122,17 +302,18 @@ static int google_usb2_phy_init(struct phy *_phy)
 	ret = clk_bulk_prepare_enable(inst->num_clks, inst->clks);
 	if (ret)
 		return ret;
+	struct google_usb_phy_instance *clk_dev __free(inst_clk_disable) = inst;
 
 	ret = reset_control_bulk_deassert(inst->num_rsts, inst->rsts);
-	if (ret) {
-		clk_bulk_disable_unprepare(inst->num_clks, inst->clks);
+	if (ret)
 		return ret;
-	}
 
 	regmap_read(gphy->usb_cfg_regmap, gphy->usb2_cfg_offset + USBCS_USB2PHY_CFG21_OFFSET, &reg);
 	reg |= USBCS_USB2PHY_CFG21_PHY_ENABLE;
 	regmap_write(gphy->usb_cfg_regmap, gphy->usb2_cfg_offset + USBCS_USB2PHY_CFG21_OFFSET, reg);
 
+	retain_and_null_ptr(clk_dev);
+
 	return 0;
 }
 
@@ -161,6 +342,128 @@ static const struct phy_ops google_usb2_phy_ops = {
 	.exit		= google_usb2_phy_exit,
 };
 
+static int google_usb3_phy_init(struct phy *_phy)
+{
+	struct google_usb_phy_instance *inst = phy_get_drvdata(_phy);
+	struct google_usb_phy *gphy = inst->parent;
+	int ret = 0;
+	u32 reg;
+
+	dev_dbg(gphy->dev, "initializing usb3 phy\n");
+
+	guard(mutex)(&gphy->phy_mutex);
+
+	if (gphy->phy_state != COMBO_PHY_IDLE) {
+		dev_warn(gphy->dev, "usb3 phy init called when combo phy state is not idle\n");
+		return 0;
+	}
+
+	reg = readl(gphy->usb3_tca_base + TCA_CTRLSYNCMODE_CFG1_OFFSET);
+	reg &= ~TCA_CTRLSYNCMODE_CFG1_XA_TIMEOUT_VAL;
+	reg |= FIELD_PREP(TCA_CTRLSYNCMODE_CFG1_XA_TIMEOUT_VAL,
+			  TCA_CTRLSYNCMODE_CFG1_XA_TIMEOUT_VAL_100MS);
+	writel(reg, gphy->usb3_tca_base + TCA_CTRLSYNCMODE_CFG1_OFFSET);
+
+	reg = readl(gphy->usbdp_top_base + PHY_POWER_CONFIG_REG1_OFFSET);
+	reg |= PHY_POWER_CONFIG_REG1_PG_MODE_EN;
+	reg &= ~PHY_POWER_CONFIG_REG1_UPCS_PIPE_CONFIG;
+	reg |= FIELD_PREP(PHY_POWER_CONFIG_REG1_UPCS_PIPE_CONFIG,
+			  (UPCS_PIPE_CONFIG_ISO_CPM |
+			   UPCS_PIPE_CONFIG_PG_MODE_STATIC |
+			   UPCS_PIPE_CONFIG_LANE_RESET_NO_PG_EXIT));
+	writel(reg, gphy->usbdp_top_base + PHY_POWER_CONFIG_REG1_OFFSET);
+
+	set_vbus_valid(gphy);
+
+	reg = readl(gphy->usbdp_top_base + USBCS_PHY_CFG1_OFFSET);
+	reg |= USBCS_PHY_CFG1_PHY0_MPLLA_SSC_EN;
+	writel(reg, gphy->usbdp_top_base + USBCS_PHY_CFG1_OFFSET);
+
+	set_sram_bypass(gphy, SRAM_BYPASS_MODE_BYPASS_FIRMWARE |
+			SRAM_BYPASS_MODE_BYPASS_CONTEXT);
+	set_pmgt_ref_clk_req_n(gphy, true);
+	struct google_usb_phy *pmgt_ref_clk_req_dev __free(pmgt_ref_clk_req_n) = gphy;
+
+	ret = clk_bulk_prepare_enable(inst->num_clks, inst->clks);
+	if (ret)
+		return ret;
+	struct google_usb_phy_instance *clk_dev __free(inst_clk_disable) = inst;
+
+	ret = reset_control_bulk_deassert(inst->num_rsts, inst->rsts);
+	if (ret)
+		return ret;
+	struct google_usb_phy_instance *rst_dev __free(inst_rst_disable) = inst;
+
+	ret = readl_poll_timeout(gphy->usb3_tca_base + TCA_PSTATE_0_OFFSET,
+				 reg, !(reg & TCA_PSTATE_0_UPCS_LANE0_PHYSTATUS),
+				 GPHY_TCA_DELAY_US, GPHY_TCA_TIMEOUT_US);
+	if (ret) {
+		dev_err(gphy->dev, "wait for lane0 phystatus timed out\n");
+		return ret;
+	}
+
+	gphy->phy_state = COMBO_PHY_INIT_DONE;
+
+	retain_and_null_ptr(rst_dev);
+	retain_and_null_ptr(clk_dev);
+	retain_and_null_ptr(pmgt_ref_clk_req_dev);
+
+	return 0;
+}
+
+static int google_usb3_phy_exit(struct phy *_phy)
+{
+	struct google_usb_phy_instance *inst = phy_get_drvdata(_phy);
+	struct google_usb_phy *gphy = inst->parent;
+
+	dev_dbg(gphy->dev, "exiting usb3 phy\n");
+
+	guard(mutex)(&gphy->phy_mutex);
+
+	reset_control_bulk_assert(inst->num_rsts, inst->rsts);
+	clk_bulk_disable_unprepare(inst->num_clks, inst->clks);
+	set_pmgt_ref_clk_req_n(gphy, false);
+
+	gphy->phy_state = COMBO_PHY_IDLE;
+
+	return 0;
+}
+
+static int google_usb3_phy_power_on(struct phy *_phy)
+{
+	struct google_usb_phy_instance *inst = phy_get_drvdata(_phy);
+	struct google_usb_phy *gphy = inst->parent;
+	int ret;
+
+	dev_dbg(gphy->dev, "power on usb3 phy\n");
+
+	guard(mutex)(&gphy->phy_mutex);
+
+	if (gphy->phy_state == COMBO_PHY_IDLE)
+		return -EINVAL;
+
+	if (gphy->phy_state == COMBO_PHY_INIT_DONE) {
+		/* Wait for PoR -> NC transitions*/
+		ret = wait_tca_xa_ack(gphy);
+		if (ret) {
+			dev_err(gphy->dev, "PoR->NC transition timeout\n");
+			return ret;
+		}
+		gphy->phy_state = COMBO_PHY_TCA_READY;
+	}
+
+	if (gphy->orientation != TYPEC_ORIENTATION_NONE)
+		return program_tca_locked(gphy);
+
+	return 0;
+}
+
+static const struct phy_ops google_usb3_phy_ops = {
+	.init		= google_usb3_phy_init,
+	.exit		= google_usb3_phy_exit,
+	.power_on	= google_usb3_phy_power_on,
+};
+
 static struct phy *google_usb_phy_xlate(struct device *dev,
 					const struct of_phandle_args *args)
 {
@@ -173,14 +476,61 @@ static struct phy *google_usb_phy_xlate(struct device *dev,
 	return gphy->insts[args->args[0]].phy;
 }
 
+static int google_usb_phy_parse_clocks(struct google_usb_phy *gphy)
+{
+	struct device *dev = gphy->dev;
+	int id, i, ret;
+
+	for (id = 0; id < GOOGLE_USB_PHY_NUM; id++) {
+		const struct google_usb_phy_config *cfg = &phy_configs[id];
+		struct google_usb_phy_instance *inst = &gphy->insts[id];
+
+		inst->num_clks = cfg->num_clks;
+		inst->clks = devm_kcalloc(dev, inst->num_clks, sizeof(*inst->clks), GFP_KERNEL);
+		if (!inst->clks)
+			return -ENOMEM;
+
+		for (i = 0; i < inst->num_clks; i++)
+			inst->clks[i].id = cfg->clk_names[i];
+
+		ret = devm_clk_bulk_get(dev, inst->num_clks, inst->clks);
+		if (ret)
+			return dev_err_probe(dev, ret, "failed to get phy%d clks\n", id);
+	}
+
+	return 0;
+}
+
+static int google_usb_phy_parse_resets(struct google_usb_phy *gphy)
+{
+	struct device *dev = gphy->dev;
+	int id, i, ret;
+
+	for (id = 0; id < GOOGLE_USB_PHY_NUM; id++) {
+		const struct google_usb_phy_config *cfg = &phy_configs[id];
+		struct google_usb_phy_instance *inst = &gphy->insts[id];
+
+		inst->num_rsts = cfg->num_rsts;
+		inst->rsts = devm_kcalloc(dev, inst->num_rsts, sizeof(*inst->rsts), GFP_KERNEL);
+		if (!inst->rsts)
+			return -ENOMEM;
+
+		for (i = 0; i < inst->num_rsts; i++)
+			inst->rsts[i].id = cfg->rst_names[i];
+		ret = devm_reset_control_bulk_get_exclusive(dev, inst->num_rsts, inst->rsts);
+		if (ret)
+			return dev_err_probe(dev, ret, "failed to get phy%d resets\n", id);
+	}
+
+	return 0;
+}
+
 static int google_usb_phy_probe(struct platform_device *pdev)
 {
 	struct typec_switch_desc sw_desc = { };
-	struct google_usb_phy_instance *inst;
 	struct phy_provider *phy_provider;
 	struct device *dev = &pdev->dev;
 	struct google_usb_phy *gphy;
-	struct phy *phy;
 	u32 args[1];
 	int ret;
 
@@ -212,39 +562,39 @@ static int google_usb_phy_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, PTR_ERR(gphy->usbdp_top_base),
 				    "invalid usbdp top\n");
 
+	gphy->usb3_tca_base = devm_platform_ioremap_resource_byname(pdev,
+								    "usb3_tca");
+	if (IS_ERR(gphy->usb3_tca_base))
+		return dev_err_probe(dev, PTR_ERR(gphy->usb3_tca_base),
+				    "invalid usb3 tca\n");
+
 	gphy->insts = devm_kcalloc(dev, GOOGLE_USB_PHY_NUM, sizeof(*gphy->insts), GFP_KERNEL);
 	if (!gphy->insts)
 		return -ENOMEM;
 
-	inst = &gphy->insts[GOOGLE_USB2_PHY];
-	inst->parent = gphy;
-	inst->index = GOOGLE_USB2_PHY;
-	phy = devm_phy_create(dev, NULL, &google_usb2_phy_ops);
-	if (IS_ERR(phy))
-		return dev_err_probe(dev, PTR_ERR(phy),
+	gphy->insts[GOOGLE_USB2_PHY].phy = devm_phy_create(dev, NULL, &google_usb2_phy_ops);
+	gphy->insts[GOOGLE_USB2_PHY].index = GOOGLE_USB2_PHY;
+	gphy->insts[GOOGLE_USB2_PHY].parent = gphy;
+	if (IS_ERR(gphy->insts[GOOGLE_USB2_PHY].phy))
+		return dev_err_probe(dev, PTR_ERR(gphy->insts[GOOGLE_USB2_PHY].phy),
 				     "failed to create usb2 phy instance\n");
-	inst->phy = phy;
-	phy_set_drvdata(phy, inst);
+	phy_set_drvdata(gphy->insts[GOOGLE_USB2_PHY].phy, &gphy->insts[GOOGLE_USB2_PHY]);
 
-	inst->num_clks = 2;
-	inst->clks = devm_kcalloc(dev, inst->num_clks, sizeof(*inst->clks), GFP_KERNEL);
-	if (!inst->clks)
-		return -ENOMEM;
-	inst->clks[0].id = "usb2";
-	inst->clks[1].id = "usb2_apb";
-	ret = devm_clk_bulk_get(dev, inst->num_clks, inst->clks);
+	gphy->insts[GOOGLE_USB3_PHY].phy = devm_phy_create(dev, NULL, &google_usb3_phy_ops);
+	gphy->insts[GOOGLE_USB3_PHY].index = GOOGLE_USB3_PHY;
+	gphy->insts[GOOGLE_USB3_PHY].parent = gphy;
+	if (IS_ERR(gphy->insts[GOOGLE_USB3_PHY].phy))
+		return dev_err_probe(dev, PTR_ERR(gphy->insts[GOOGLE_USB3_PHY].phy),
+				     "failed to create usb3 phy instance\n");
+	phy_set_drvdata(gphy->insts[GOOGLE_USB3_PHY].phy, &gphy->insts[GOOGLE_USB3_PHY]);
+
+	ret = google_usb_phy_parse_clocks(gphy);
 	if (ret)
-		return dev_err_probe(dev, ret, "failed to get u2 phy clks\n");
+		return ret;
 
-	inst->num_rsts = 2;
-	inst->rsts = devm_kcalloc(dev, inst->num_rsts, sizeof(*inst->rsts), GFP_KERNEL);
-	if (!inst->rsts)
-		return -ENOMEM;
-	inst->rsts[0].id = "usb2";
-	inst->rsts[1].id = "usb2_apb";
-	ret = devm_reset_control_bulk_get_exclusive(dev, inst->num_rsts, inst->rsts);
+	ret = google_usb_phy_parse_resets(gphy);
 	if (ret)
-		return dev_err_probe(dev, ret, "failed to get u2 phy resets\n");
+		return ret;
 
 	phy_provider = devm_of_phy_provider_register(dev, google_usb_phy_xlate);
 	if (IS_ERR(phy_provider))

base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
-- 
2.55.0.1082.g2b9226bbc0-goog


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v7] phy: Add USB3 PHY support to Google Tensor SoC USB PHY driver
  2026-09-18 22:25 [PATCH v7] phy: Add USB3 PHY support to Google Tensor SoC USB PHY driver RD Babiera
@ 2026-09-21 18:29 ` Neill Kapron
  2026-09-25 19:40   ` RD Babiera
  0 siblings, 1 reply; 3+ messages in thread
From: Neill Kapron @ 2026-09-21 18:29 UTC (permalink / raw)
  To: RD Babiera
  Cc: vkoul, peter.griffin, andre.draszik, tudor.ambarus, p.zabel,
	neil.armstrong, badhri, linux-arm-kernel, linux-samsung-soc,
	linux-phy, linux-kernel

Hi RD,

Thanks for sending v7. I've reviewed the changes and identified a few
functional issues, and a couple minor items as seen below:


On Fri, Sep 18, 2026 at 10:25:14PM +0000, RD Babiera wrote:
> Add USB3 PHY support for the Google Tensor G5 USB PHY driver.
...
> --- a/drivers/phy/phy-google-usb.c
> +++ b/drivers/phy/phy-google-usb.c
> @@ -20,6 +20,7 @@
>  #include <linux/reset.h>
>  #include <linux/usb/typec_mux.h>

The driver is now using readl_poll_timeout() and
pm_runtime_get_if_active(), we should be including linux/iopoll.h and
linux/pm_runtime.h explicitly.

> +#define TCA_CTRLSYNCMODE_CFG1_XA_TIMEOUT_VAL_100MS 0x1e85
> +#define TCA_PSTATE_0_OFFSET 0x50
> +#define TCA_PSTATE_0_UPCS_LANE0_PHYSTATUS BIT(8)
> +
> +#define GPHY_TCA_DELAY_US 10
> +#define GPHY_TCA_TIMEOUT_US 100000

With the addition of TCA_CTRLSYNCMODE_CFG1_XA_TIMEOUT_VAL_100MS, we
should consider bumping GPHY_TCA_TIMEOUT_US to be slightly larger (e.g.
110000us) to ensure the hardware timeout is guaranteed to expire before
the software poll timeout.

> +static const char * const u2phy_clk_names[] = {
> +	"usb2",
> +	"usb2_apb",
> +};
> +static const char * const u3phy_clk_names[] = {
> +	"usb3"
> +};
> +static const char * const u2phy_rst_names[] = {
> +	"usb2",
> +	"usb2_apb",
> +};
> +static const char * const u3phy_rst_names[] = {
> +	"usb3"
> +};

nit: checkpatch.pl --strict flags missing blank lines between these
array declarations (and the inline helper functions + DEFINE__FREE
macros below).

> +
> +static const struct google_usb_phy_config phy_configs[GOOGLE_USB_PHY_NUM] = {
> +	[GOOGLE_USB2_PHY] = {
> +		.clk_names = u2phy_clk_names,
> +		.num_clks = ARRAY_SIZE(u2phy_clk_names),
> +		.rst_names = u2phy_rst_names,
> +		.num_rsts = ARRAY_SIZE(u2phy_rst_names),
> +	},
> +	[GOOGLE_USB3_PHY] = {
> +		.clk_names = u3phy_clk_names,
> +		.num_clks = ARRAY_SIZE(u3phy_clk_names),
> +		.rst_names = u3phy_rst_names,
> +		.num_rsts = ARRAY_SIZE(u3phy_rst_names),
> +	},
> +};
> +
> +static inline void google_usb_phy_clk_disable(struct google_usb_phy_instance *inst)
> +{
> +	clk_bulk_disable_unprepare(inst->num_clks, inst->clks);
> +}
> +DEFINE_FREE(inst_clk_disable, struct google_usb_phy_instance *,
> +	    if (_T) google_usb_phy_clk_disable(_T))
> +
> +static inline void google_usb_phy_rst_disable(struct google_usb_phy_instance *inst)
> +{
> +	reset_control_bulk_assert(inst->num_rsts, inst->rsts);
> +}
> +DEFINE_FREE(inst_rst_disable, struct google_usb_phy_instance *,
> +	    if (_T) google_usb_phy_rst_disable(_T))
> +
...
>   
>  static int google_usb_set_orientation(struct typec_switch_dev *sw,
>  				      enum typec_orientation orientation)
>  {
>  	struct google_usb_phy *gphy = typec_switch_get_drvdata(sw);
> +	int ret = 0;
>  
>  	dev_dbg(gphy->dev, "set orientation %d\n", orientation);
>  
> -	gphy->orientation = orientation;
> +	guard(mutex)(&gphy->phy_mutex);
>  
> -	if (pm_runtime_suspended(gphy->dev))
> -		return 0;
> +	gphy->orientation = orientation;
>  
> -	guard(mutex)(&gphy->phy_mutex);
> +	if (IS_ENABLED(CONFIG_PM)) {
> +		if (pm_runtime_get_if_active(gphy->dev) <= 0)
> +			return 0;
> +	}
>  
>  	set_vbus_valid(gphy);
>  
> -	return 0;
> +	if (gphy->phy_state == COMBO_PHY_TCA_READY && orientation != TYPEC_ORIENTATION_NONE)
> +		ret = program_tca_locked(gphy);
> +
> +	pm_runtime_put(gphy->dev);
> +
> +	return ret;
>  }

Previously, sashiko recommended moving to pm_runtime_get_if_active(),
which was done in v6. However I think this may have changed the behavior
of google_usb_set_orientation() and potentially introduced a regression
due to the pre-existing ordering of calls in gooogle_usb_phy_probe(),
causing this function to always take the early 'return 0' path.

In google_usb_phy_probe(), we call devm_phy_create() prior to calling
pm_runtime_enable(dev).

In drivers/phy/phy-core.c, devm_phy_create() calls phy_create(), which
has the following check:

    if (pm_runtime_enabled(dev)) {
        pm_runtime_enable(&phy->dev);
        pm_runtime_no_callbacks(&phy->dev);
    }

Therefore, the phy device never has pm_runtime_enabled, causing this
call to pm_runtime_get_if_active() to always return 0, and the function
exits prior to calling `set_vbus_valid()`.

I think moving the pm_runtime_enable(dev) call prior to
devm_phy_create() will resolve the issue, but we should audit power
managment in this driver to verify.

> 
> +static int google_usb3_phy_init(struct phy *_phy)
> +{
> +	struct google_usb_phy_instance *inst = phy_get_drvdata(_phy);
> +	struct google_usb_phy *gphy = inst->parent;
> +	int ret = 0;
> +	u32 reg;
> +
> +	dev_dbg(gphy->dev, "initializing usb3 phy\n");
> +
> +	guard(mutex)(&gphy->phy_mutex);
> +
> +	if (gphy->phy_state != COMBO_PHY_IDLE) {
> +		dev_warn(gphy->dev, "usb3 phy init called when combo phy state is not idle\n");
> +		return 0;
> +	}
> +
> +	reg = readl(gphy->usb3_tca_base + TCA_CTRLSYNCMODE_CFG1_OFFSET);
> +	reg &= ~TCA_CTRLSYNCMODE_CFG1_XA_TIMEOUT_VAL;
> +	reg |= FIELD_PREP(TCA_CTRLSYNCMODE_CFG1_XA_TIMEOUT_VAL,
> +			  TCA_CTRLSYNCMODE_CFG1_XA_TIMEOUT_VAL_100MS);
> +	writel(reg, gphy->usb3_tca_base + TCA_CTRLSYNCMODE_CFG1_OFFSET);

I think this introduces a regression between v6 and v7, as usb3_tca_base
may be accessed prior to the 'usb3' clock being enabled, and
furthermore, the call to reset_control_bulk_deassert() will clear this
value.

Therefore, I think we need to this after the call to
reset_control_bulk_deassert().

> +
> +	reg = readl(gphy->usbdp_top_base + PHY_POWER_CONFIG_REG1_OFFSET);
> +	reg |= PHY_POWER_CONFIG_REG1_PG_MODE_EN;
> +	reg &= ~PHY_POWER_CONFIG_REG1_UPCS_PIPE_CONFIG;
> +	reg |= FIELD_PREP(PHY_POWER_CONFIG_REG1_UPCS_PIPE_CONFIG,
> +			  (UPCS_PIPE_CONFIG_ISO_CPM |
> +			   UPCS_PIPE_CONFIG_PG_MODE_STATIC |
> +			   UPCS_PIPE_CONFIG_LANE_RESET_NO_PG_EXIT));
> +	writel(reg, gphy->usbdp_top_base + PHY_POWER_CONFIG_REG1_OFFSET);
> +
> +	set_vbus_valid(gphy);
> +
> +	reg = readl(gphy->usbdp_top_base + USBCS_PHY_CFG1_OFFSET);
> +	reg |= USBCS_PHY_CFG1_PHY0_MPLLA_SSC_EN;
> +	writel(reg, gphy->usbdp_top_base + USBCS_PHY_CFG1_OFFSET);
> +
> +	set_sram_bypass(gphy, SRAM_BYPASS_MODE_BYPASS_FIRMWARE |
> +			SRAM_BYPASS_MODE_BYPASS_CONTEXT);
> +	set_pmgt_ref_clk_req_n(gphy, true);
> +	struct google_usb_phy *pmgt_ref_clk_req_dev __free(pmgt_ref_clk_req_n) = gphy;
> +
> +	ret = clk_bulk_prepare_enable(inst->num_clks, inst->clks);
> +	if (ret)
> +		return ret;
> +	struct google_usb_phy_instance *clk_dev __free(inst_clk_disable) = inst;
> +
> +	ret = reset_control_bulk_deassert(inst->num_rsts, inst->rsts);
> +	if (ret)
> +		return ret;
> +	struct google_usb_phy_instance *rst_dev __free(inst_rst_disable) = inst;
> +
> +	ret = readl_poll_timeout(gphy->usb3_tca_base + TCA_PSTATE_0_OFFSET,
> +				 reg, !(reg & TCA_PSTATE_0_UPCS_LANE0_PHYSTATUS),
> +				 GPHY_TCA_DELAY_US, GPHY_TCA_TIMEOUT_US);
> +	if (ret) {
> +		dev_err(gphy->dev, "wait for lane0 phystatus timed out\n");
> +		return ret;
> +	}
> +
> +	gphy->phy_state = COMBO_PHY_INIT_DONE;
> +
> +	retain_and_null_ptr(rst_dev);
> +	retain_and_null_ptr(clk_dev);
> +	retain_and_null_ptr(pmgt_ref_clk_req_dev);
> +
> +	return 0;
> +}
> +
> 
>

Thanks,
Neill

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v7] phy: Add USB3 PHY support to Google Tensor SoC USB PHY driver
  2026-09-21 18:29 ` Neill Kapron
@ 2026-09-25 19:40   ` RD Babiera
  0 siblings, 0 replies; 3+ messages in thread
From: RD Babiera @ 2026-09-25 19:40 UTC (permalink / raw)
  To: Neill Kapron
  Cc: vkoul, peter.griffin, andre.draszik, tudor.ambarus, p.zabel,
	neil.armstrong, badhri, linux-arm-kernel, linux-samsung-soc,
	linux-phy, linux-kernel

Hi Neill, thanks for the thorough review. Will send v8 out shortly.

On Mon, Sep 21, 2026 at 11:29 AM Neill Kapron <nkapron@google.com> wrote:
> The driver is now using readl_poll_timeout() and
> pm_runtime_get_if_active(), we should be including linux/iopoll.h and
> linux/pm_runtime.h explicitly.

Acknowledged.

> > +#define TCA_CTRLSYNCMODE_CFG1_XA_TIMEOUT_VAL_100MS 0x1e85
> > +#define TCA_PSTATE_0_OFFSET 0x50
> > +#define TCA_PSTATE_0_UPCS_LANE0_PHYSTATUS BIT(8)
> > +
> > +#define GPHY_TCA_DELAY_US 10
> > +#define GPHY_TCA_TIMEOUT_US 100000
>
> With the addition of TCA_CTRLSYNCMODE_CFG1_XA_TIMEOUT_VAL_100MS, we
> should consider bumping GPHY_TCA_TIMEOUT_US to be slightly larger (e.g.
> 110000us) to ensure the hardware timeout is guaranteed to expire before
> the software poll timeout.

Will change here.

> > +static const char * const u2phy_clk_names[] = {
> > +     "usb2",
> > +     "usb2_apb",
> > +};
> > +static const char * const u3phy_clk_names[] = {
> > +     "usb3"
> > +};
> > +static const char * const u2phy_rst_names[] = {
> > +     "usb2",
> > +     "usb2_apb",
> > +};
> > +static const char * const u3phy_rst_names[] = {
> > +     "usb3"
> > +};
>
> nit: checkpatch.pl --strict flags missing blank lines between these
> array declarations (and the inline helper functions + DEFINE__FREE
> macros below).

Will check.

> >  static int google_usb_set_orientation(struct typec_switch_dev *sw,
> >                                     enum typec_orientation orientation)
> >  {
> >       struct google_usb_phy *gphy = typec_switch_get_drvdata(sw);
> > +     int ret = 0;
> >
> >       dev_dbg(gphy->dev, "set orientation %d\n", orientation);
> >
> > -     gphy->orientation = orientation;
> > +     guard(mutex)(&gphy->phy_mutex);
> >
> > -     if (pm_runtime_suspended(gphy->dev))
> > -             return 0;
> > +     gphy->orientation = orientation;
> >
> > -     guard(mutex)(&gphy->phy_mutex);
> > +     if (IS_ENABLED(CONFIG_PM)) {
> > +             if (pm_runtime_get_if_active(gphy->dev) <= 0)
> > +                     return 0;
> > +     }
> >
> >       set_vbus_valid(gphy);
> >
> > -     return 0;
> > +     if (gphy->phy_state == COMBO_PHY_TCA_READY && orientation != TYPEC_ORIENTATION_NONE)
> > +             ret = program_tca_locked(gphy);
> > +
> > +     pm_runtime_put(gphy->dev);
> > +
> > +     return ret;
> >  }
>
> Previously, sashiko recommended moving to pm_runtime_get_if_active(),
> which was done in v6. However I think this may have changed the behavior
> of google_usb_set_orientation() and potentially introduced a regression
> due to the pre-existing ordering of calls in gooogle_usb_phy_probe(),
> causing this function to always take the early 'return 0' path.
>
> In google_usb_phy_probe(), we call devm_phy_create() prior to calling
> pm_runtime_enable(dev).
>
> In drivers/phy/phy-core.c, devm_phy_create() calls phy_create(), which
> has the following check:
>
>     if (pm_runtime_enabled(dev)) {
>         pm_runtime_enable(&phy->dev);
>         pm_runtime_no_callbacks(&phy->dev);
>     }
>
> Therefore, the phy device never has pm_runtime_enabled, causing this
> call to pm_runtime_get_if_active() to always return 0, and the function
> exits prior to calling `set_vbus_valid()`.
>
> I think moving the pm_runtime_enable(dev) call prior to
> devm_phy_create() will resolve the issue, but we should audit power
> managment in this driver to verify.

The check here is called on gphy->dev as opposed to phy->dev. fw_devlink uses
FW_DEVLINK_FLAGS_RPM by default, so the dwc3 consumer controller holds
DL_FLAG_PM_RUNTIME on the phy platform device. Every resume call on
the dwc3 controller results in the supplier resuming as well under this
model, so the runtime_get() call here passes.

> I think this introduces a regression between v6 and v7, as usb3_tca_base
> may be accessed prior to the 'usb3' clock being enabled, and
> furthermore, the call to reset_control_bulk_deassert() will clear this
> value.
>
> Therefore, I think we need to this after the call to
> reset_control_bulk_deassert().

usb3_tca_base is powered by a different clock as opposed to the 'usb3'
one, so the register access will be safe as long as the platform driver's
power domain is on. Register reads in either order both result in the
same behavior during testing, but I would argue that writing before
the call to reset_control_bulk_deassert() ensures that the parameter
is applied on PoR.

Best,
RD

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-09-25 19:40 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-18 22:25 [PATCH v7] phy: Add USB3 PHY support to Google Tensor SoC USB PHY driver RD Babiera
2026-09-21 18:29 ` Neill Kapron
2026-09-25 19:40   ` RD Babiera

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®