[PATCH 0/9] USB31DRD phy updates for Google Tensor gs101 (orientation & DWC3 rpm)

[PATCH 0/9] USB31DRD phy updates for Google Tensor gs101 (orientation & DWC3 rpm)

[André Draszik]

Hi,

This series enables USB3 Type-C lane orientation detection and
configuration on platforms that support this (Google gs101), and it
also allows the DWC3 core to enter runtime suspend even when UDC is
active.

For lane orientation, this driver now optionally (based on DT)
subscribes to the TCPC's lane orientation notifier and remembers the
orientation to later be used during phy_init().

To enable DWC3 runtime suspend, the gadget needs to inform the core via
dwc3_gadget_interrupt() with event type == DWC3_DEVICE_EVENT_DISCONNECT
of a cable disconnect. For that to allow to happen, this driver
therefore needs to stop forcing the Vbus and bvalid signals to active
and instead change their state based on actual conditions. The same
TCPC notifier is used to detect this, and program the hardware
accordingly.

That signal state is based on advice given by Thinh in
https://lore.kernel.org/all/20240813230625.jgkatqstyhcmpezv@synopsys.com/

Both changes together now allow cable orientation detection to work, as
the DWC3 will now call phy_exit() on cable disconnect, and we can
reprogram the lane mux in phy_init().

On top of that, there are some small related cleanup patches.

Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
André Draszik (9):
      dt-bindings: phy: samsung,usb3-drd-phy: align to universal style
      dt-bindings: phy: samsung,usb3-drd-phy: add optional orientation-switch
      dt-bindings: phy: samsung,usb3-drd-phy: gs101: require Type-C properties
      phy: exynos5-usbdrd: convert to dev_err_probe
      phy: exynos5-usbdrd: fix EDS distribution tuning (gs101)
      phy: exynos5-usbdrd: gs101: ensure power is gated to SS phy in phy_exit()
      phy: exynos5-usbdrd: gs101: configure SS lanes based on orientation
      phy: exynos5-usbdrd: subscribe to orientation notifier if required
      phy: exynos5-usbdrd: allow DWC3 runtime suspend with UDC bound (E850+)

 .../bindings/phy/samsung,usb3-drd-phy.yaml         |  20 ++
 drivers/phy/samsung/Kconfig                        |   1 +
 drivers/phy/samsung/phy-exynos5-usbdrd.c           | 222 ++++++++++++++++-----
 3 files changed, 197 insertions(+), 46 deletions(-)
---
base-commit: ed9a4ad6e5bd3a443e81446476718abebee47e82
change-id: 20241127-gs101-phy-lanes-orientation-phy-29d20c6d84d2

Best regards,
-- 
André Draszik <andre.draszik@linaro.org>

[PATCH 1/9] dt-bindings: phy: samsung,usb3-drd-phy: align to universal style

[André Draszik]

In [1], Rob pointed out that we should really be separating properties
with blank lines in between, which is universal style. Only where
properties are booleans, empty lines are not required.

Do so.

Link: https://lore.kernel.org/all/20240711212359.GA3023490-robh@kernel.org/ [1]
Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
 .../devicetree/bindings/phy/samsung,usb3-drd-phy.yaml     | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml b/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
index 16321cdd4919..1f8b35917b11 100644
--- a/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
@@ -83,14 +83,19 @@ properties:
 
   pll-supply:
     description: Power supply for the USB PLL.
+
   dvdd-usb20-supply:
     description: DVDD power supply for the USB 2.0 phy.
+
   vddh-usb20-supply:
     description: VDDh power supply for the USB 2.0 phy.
+
   vdd33-usb20-supply:
     description: 3.3V power supply for the USB 2.0 phy.
+
   vdda-usbdp-supply:
     description: VDDa power supply for the USB DP phy.
+
   vddh-usbdp-supply:
     description: VDDh power supply for the USB DP phy.
 
@@ -117,6 +122,7 @@ allOf:
             - description: Gate of control interface AXI clock
             - description: Gate of control interface APB clock
             - description: Gate of SCL APB clock
+
         clock-names:
           items:
             - const: phy
@@ -124,10 +130,13 @@ allOf:
             - const: ctrl_aclk
             - const: ctrl_pclk
             - const: scl_pclk
+
         reg:
           minItems: 3
+
         reg-names:
           minItems: 3
+
       required:
         - reg-names
         - pll-supply
@@ -149,6 +158,7 @@ allOf:
         clocks:
           minItems: 5
           maxItems: 5
+
         clock-names:
           items:
             - const: phy
@@ -156,8 +166,10 @@ allOf:
             - const: phy_utmi
             - const: phy_pipe
             - const: itp
+
         reg:
           maxItems: 1
+
         reg-names:
           maxItems: 1
 
@@ -174,12 +186,15 @@ allOf:
         clocks:
           minItems: 2
           maxItems: 2
+
         clock-names:
           items:
             - const: phy
             - const: ref
+
         reg:
           maxItems: 1
+
         reg-names:
           maxItems: 1
 

-- 
2.47.0.338.g60cca15819-goog

[PATCH 2/9] dt-bindings: phy: samsung,usb3-drd-phy: add optional orientation-switch

[André Draszik]

orientation-switch is the standard declaration to inform the Type-C mux
layer that a remote-endpoint is capable of processing orientation
change messages.

Add as an optional since not all versions of this phy currently support
or even need the orientation-switch.

Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
 Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml b/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
index 1f8b35917b11..6c17a44718aa 100644
--- a/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
@@ -51,6 +51,9 @@ properties:
   "#phy-cells":
     const: 1
 
+  orientation-switch:
+    $ref: /schemas/usb/usb-switch.yaml#/properties/orientation-switch
+
   port:
     $ref: /schemas/graph.yaml#/properties/port
     description:

-- 
2.47.0.338.g60cca15819-goog

[PATCH 3/9] dt-bindings: phy: samsung,usb3-drd-phy: gs101: require Type-C properties

[André Draszik]

The USB PHY on gs101 needs to be configured based on the orientation of
the connector. For that the DTS needs a link between the phy's port and
a TCPCi, and we'll need to inform the phy driver that it should handle
the orientation (register a handler).

Update the schema to enforce that by requiring the orientation-switch
and port properties (on gs101 only).

Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
 Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml b/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
index 6c17a44718aa..0659fc1ce13e 100644
--- a/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
@@ -142,6 +142,8 @@ allOf:
 
       required:
         - reg-names
+        - orientation-switch
+        - port
         - pll-supply
         - dvdd-usb20-supply
         - vddh-usb20-supply

-- 
2.47.0.338.g60cca15819-goog

[PATCH 4/9] phy: exynos5-usbdrd: convert to dev_err_probe

[André Draszik]

dev_err_probe() exists to simplify code.

Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
 drivers/phy/samsung/phy-exynos5-usbdrd.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/phy/samsung/phy-exynos5-usbdrd.c b/drivers/phy/samsung/phy-exynos5-usbdrd.c
index c421b495eb0f..ceae4b47cece 100644
--- a/drivers/phy/samsung/phy-exynos5-usbdrd.c
+++ b/drivers/phy/samsung/phy-exynos5-usbdrd.c
@@ -1725,10 +1725,9 @@ static int exynos5_usbdrd_phy_probe(struct platform_device *pdev)
 
 	reg_pmu = syscon_regmap_lookup_by_phandle(dev->of_node,
 						   "samsung,pmu-syscon");
-	if (IS_ERR(reg_pmu)) {
-		dev_err(dev, "Failed to lookup PMU regmap\n");
-		return PTR_ERR(reg_pmu);
-	}
+	if (IS_ERR(reg_pmu))
+		return dev_err_probe(dev, PTR_ERR(reg_pmu),
+				     "Failed to lookup PMU regmap\n");
 
 	/*
 	 * Exynos5420 SoC has multiple channels for USB 3.0 PHY, with
@@ -1759,10 +1758,9 @@ static int exynos5_usbdrd_phy_probe(struct platform_device *pdev)
 	for (i = 0; i < EXYNOS5_DRDPHYS_NUM; i++) {
 		struct phy *phy = devm_phy_create(dev, NULL, drv_data->phy_ops);
 
-		if (IS_ERR(phy)) {
-			dev_err(dev, "Failed to create usbdrd_phy phy\n");
-			return PTR_ERR(phy);
-		}
+		if (IS_ERR(phy))
+			return dev_err_probe(dev, PTR_ERR(phy),
+					     "Failed to create usbdrd_phy phy\n");
 
 		phy_drd->phys[i].phy = phy;
 		phy_drd->phys[i].index = i;
@@ -1786,10 +1784,9 @@ static int exynos5_usbdrd_phy_probe(struct platform_device *pdev)
 
 	phy_provider = devm_of_phy_provider_register(dev,
 						     exynos5_usbdrd_phy_xlate);
-	if (IS_ERR(phy_provider)) {
-		dev_err(phy_drd->dev, "Failed to register phy provider\n");
-		return PTR_ERR(phy_provider);
-	}
+	if (IS_ERR(phy_provider))
+		return dev_err_probe(phy_drd->dev, PTR_ERR(phy_provider),
+				     "Failed to register phy provider\n");
 
 	return 0;
 }

-- 
2.47.0.338.g60cca15819-goog

[PATCH 5/9] phy: exynos5-usbdrd: fix EDS distribution tuning (gs101)

[André Draszik]

This code's intention is to configure lane0 and lane2 tunings, but for
lane2 there is a typo and it ends up tuning something else.

Fix the typo, as it doesn't appear to make sense to apply different
tunings for lane0 vs lane2.

The same typo appears to exist in the bootloader, hence we restore the
original value in the typo'd registers as well. This can be removed
once / if the bootloader is updated.

Note that this is incorrect in the downstream driver as well - the
values had been copied from there.

Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
 drivers/phy/samsung/phy-exynos5-usbdrd.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/samsung/phy-exynos5-usbdrd.c b/drivers/phy/samsung/phy-exynos5-usbdrd.c
index ceae4b47cece..2a724d362c2d 100644
--- a/drivers/phy/samsung/phy-exynos5-usbdrd.c
+++ b/drivers/phy/samsung/phy-exynos5-usbdrd.c
@@ -1510,8 +1510,11 @@ static const struct exynos5_usbdrd_phy_tuning gs101_tunes_pipe3_preinit[] = {
 	PHY_TUNING_ENTRY_PMA(0x09e0, -1, 0x00),
 	PHY_TUNING_ENTRY_PMA(0x09e4, -1, 0x36),
 	PHY_TUNING_ENTRY_PMA(0x1e7c, -1, 0x06),
-	PHY_TUNING_ENTRY_PMA(0x1e90, -1, 0x00),
-	PHY_TUNING_ENTRY_PMA(0x1e94, -1, 0x36),
+	PHY_TUNING_ENTRY_PMA(0x19e0, -1, 0x00),
+	PHY_TUNING_ENTRY_PMA(0x19e4, -1, 0x36),
+	/* fix bootloader bug */
+	PHY_TUNING_ENTRY_PMA(0x1e90, -1, 0x02),
+	PHY_TUNING_ENTRY_PMA(0x1e94, -1, 0x0b),
 	/* improve LVCC */
 	PHY_TUNING_ENTRY_PMA(0x08f0, -1, 0x30),
 	PHY_TUNING_ENTRY_PMA(0x18f0, -1, 0x30),

-- 
2.47.0.338.g60cca15819-goog

[PATCH 6/9] phy: exynos5-usbdrd: gs101: ensure power is gated to SS phy in phy_exit()

[André Draszik]

We currently don't gate the power to the SS phy in phy_exit().

Shuffle the code slightly to ensure the power is gated to the SS phy as
well.

Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
 drivers/phy/samsung/phy-exynos5-usbdrd.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/phy/samsung/phy-exynos5-usbdrd.c b/drivers/phy/samsung/phy-exynos5-usbdrd.c
index 2a724d362c2d..c1ce6fdeef31 100644
--- a/drivers/phy/samsung/phy-exynos5-usbdrd.c
+++ b/drivers/phy/samsung/phy-exynos5-usbdrd.c
@@ -1296,14 +1296,17 @@ static int exynos5_usbdrd_gs101_phy_exit(struct phy *phy)
 	struct exynos5_usbdrd_phy *phy_drd = to_usbdrd_phy(inst);
 	int ret;
 
+	if (inst->phy_cfg->id == EXYNOS5_DRDPHY_UTMI) {
+		ret = exynos850_usbdrd_phy_exit(phy);
+		if (ret)
+			return ret;
+	}
+
+	exynos5_usbdrd_phy_isol(inst, true);
+
 	if (inst->phy_cfg->id != EXYNOS5_DRDPHY_UTMI)
 		return 0;
 
-	ret = exynos850_usbdrd_phy_exit(phy);
-	if (ret)
-		return ret;
-
-	exynos5_usbdrd_phy_isol(inst, true);
 	return regulator_bulk_disable(phy_drd->drv_data->n_regulators,
 				      phy_drd->regulators);
 }

-- 
2.47.0.338.g60cca15819-goog

[PATCH 7/9] phy: exynos5-usbdrd: gs101: configure SS lanes based on orientation

[André Draszik]

USB SS lanes need to be configured based the connector orientation -
at most two lanes will be in use for USB (and the remaining two for
alternate modes like DP).

For the USB link to come up in SS, the lane configuration registers
have to be programmed accordingly.

While we still need a way to be notified of the actual connector
orientation and then reprogram the registers accordingly (at the moment
the configuration happens just once during phy_init() and never again),
we can prepare the code doing the configuration to take the orientation
into account.

Do so.

Note: the mutex is needed to synchronize this with the upcoming
connector orientation callback.

Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
 drivers/phy/samsung/phy-exynos5-usbdrd.c | 72 ++++++++++++++++++++++----------
 1 file changed, 51 insertions(+), 21 deletions(-)

diff --git a/drivers/phy/samsung/phy-exynos5-usbdrd.c b/drivers/phy/samsung/phy-exynos5-usbdrd.c
index c1ce6fdeef31..1a34e9b4618a 100644
--- a/drivers/phy/samsung/phy-exynos5-usbdrd.c
+++ b/drivers/phy/samsung/phy-exynos5-usbdrd.c
@@ -23,6 +23,7 @@
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/soc/samsung/exynos-regs-pmu.h>
+#include <linux/usb/typec_mux.h>
 
 /* Exynos USB PHY registers */
 #define EXYNOS5_FSEL_9MHZ6		0x0
@@ -209,6 +210,10 @@
 
 #define EXYNOS9_PMA_USBDP_CMN_REG00B8		0x02e0
 #define CMN_REG00B8_LANE_MUX_SEL_DP		GENMASK(3, 0)
+#define CMN_REG00B8_LANE_MUX_SEL_DP_LANE3	BIT(3)
+#define CMN_REG00B8_LANE_MUX_SEL_DP_LANE2	BIT(2)
+#define CMN_REG00B8_LANE_MUX_SEL_DP_LANE1	BIT(1)
+#define CMN_REG00B8_LANE_MUX_SEL_DP_LANE0	BIT(0)
 
 #define EXYNOS9_PMA_USBDP_CMN_REG01C0		0x0700
 #define CMN_REG01C0_ANA_LCPLL_LOCK_DONE		BIT(7)
@@ -383,11 +388,13 @@ struct exynos5_usbdrd_phy_drvdata {
  * @clks: clocks for register access
  * @core_clks: core clocks for phy (ref, pipe3, utmi+, ITP, etc. as required)
  * @drv_data: pointer to SoC level driver data structure
+ * @phy_mutex: mutex protecting phy_init/exit & TCPC callbacks
  * @phys: array for 'EXYNOS5_DRDPHYS_NUM' number of PHY
  *	    instances each with its 'phy' and 'phy_cfg'.
  * @extrefclk: frequency select settings when using 'separate
  *	       reference clocks' for SS and HS operations
  * @regulators: regulators for phy
+ * @orientation: TypeC connector orientation - normal or flipped
  */
 struct exynos5_usbdrd_phy {
 	struct device *dev;
@@ -397,6 +404,7 @@ struct exynos5_usbdrd_phy {
 	struct clk_bulk_data *clks;
 	struct clk_bulk_data *core_clks;
 	const struct exynos5_usbdrd_phy_drvdata *drv_data;
+	struct mutex phy_mutex;
 	struct phy_usb_instance {
 		struct phy *phy;
 		u32 index;
@@ -406,6 +414,8 @@ struct exynos5_usbdrd_phy {
 	} phys[EXYNOS5_DRDPHYS_NUM];
 	u32 extrefclk;
 	struct regulator_bulk_data *regulators;
+
+	enum typec_orientation orientation;
 };
 
 static inline
@@ -647,22 +657,38 @@ exynos5_usbdrd_usbdp_g2_v4_pma_lane_mux_sel(struct exynos5_usbdrd_phy *phy_drd)
 	/* lane configuration: USB on all lanes */
 	reg = readl(regs_base + EXYNOS9_PMA_USBDP_CMN_REG00B8);
 	reg &= ~CMN_REG00B8_LANE_MUX_SEL_DP;
-	writel(reg, regs_base + EXYNOS9_PMA_USBDP_CMN_REG00B8);
-
 	/*
-	 * FIXME: below code supports one connector orientation only. It needs
-	 * updating once we can receive connector events.
+	 * USB on lanes 0 & 1 in normal mode, or 2 & 3 if reversed, DP on the
+	 * other ones.
 	 */
+	reg |= FIELD_PREP(CMN_REG00B8_LANE_MUX_SEL_DP,
+			  ((phy_drd->orientation == TYPEC_ORIENTATION_NORMAL)
+			   ? (CMN_REG00B8_LANE_MUX_SEL_DP_LANE3
+			      | CMN_REG00B8_LANE_MUX_SEL_DP_LANE2)
+			   : (CMN_REG00B8_LANE_MUX_SEL_DP_LANE1
+			      | CMN_REG00B8_LANE_MUX_SEL_DP_LANE0)));
+	writel(reg, regs_base + EXYNOS9_PMA_USBDP_CMN_REG00B8);
+
 	/* override of TX receiver detector and comparator: lane 1 */
 	reg = readl(regs_base + EXYNOS9_PMA_USBDP_TRSV_REG0413);
-	reg &= ~TRSV_REG0413_OVRD_LN1_TX_RXD_COMP_EN;
-	reg &= ~TRSV_REG0413_OVRD_LN1_TX_RXD_EN;
+	if (phy_drd->orientation == TYPEC_ORIENTATION_NORMAL) {
+		reg &= ~TRSV_REG0413_OVRD_LN1_TX_RXD_COMP_EN;
+		reg &= ~TRSV_REG0413_OVRD_LN1_TX_RXD_EN;
+	} else {
+		reg |= TRSV_REG0413_OVRD_LN1_TX_RXD_COMP_EN;
+		reg |= TRSV_REG0413_OVRD_LN1_TX_RXD_EN;
+	}
 	writel(reg, regs_base + EXYNOS9_PMA_USBDP_TRSV_REG0413);
 
 	/* lane 3 */
 	reg = readl(regs_base + EXYNOS9_PMA_USBDP_TRSV_REG0813);
-	reg |= TRSV_REG0813_OVRD_LN3_TX_RXD_COMP_EN;
-	reg |= TRSV_REG0813_OVRD_LN3_TX_RXD_EN;
+	if (phy_drd->orientation == TYPEC_ORIENTATION_NORMAL) {
+		reg |= TRSV_REG0813_OVRD_LN3_TX_RXD_COMP_EN;
+		reg |= TRSV_REG0813_OVRD_LN3_TX_RXD_EN;
+	} else {
+		reg &= ~TRSV_REG0813_OVRD_LN3_TX_RXD_COMP_EN;
+		reg &= ~TRSV_REG0813_OVRD_LN3_TX_RXD_EN;
+	}
 	writel(reg, regs_base + EXYNOS9_PMA_USBDP_TRSV_REG0813);
 }
 
@@ -700,21 +726,18 @@ exynos5_usbdrd_usbdp_g2_v4_pma_check_cdr_lock(struct exynos5_usbdrd_phy *phy_drd
 	int err;
 
 	err = readl_poll_timeout(
-			phy_drd->reg_pma + EXYNOS9_PMA_USBDP_TRSV_REG03C3,
-			reg, (reg & locked) == locked, sleep_us, timeout_us);
-	if (!err)
-		return;
-
-	dev_err(phy_drd->dev,
-		"timed out waiting for CDR lock (l0): %#.8x, retrying\n", reg);
-
-	/* based on cable orientation, this might be on the other phy port */
-	err = readl_poll_timeout(
-			phy_drd->reg_pma + EXYNOS9_PMA_USBDP_TRSV_REG07C3,
+			/* lane depends on cable orientation */
+			(phy_drd->reg_pma
+			 + ((phy_drd->orientation == TYPEC_ORIENTATION_NORMAL)
+			    ? EXYNOS9_PMA_USBDP_TRSV_REG03C3
+			    : EXYNOS9_PMA_USBDP_TRSV_REG07C3)),
 			reg, (reg & locked) == locked, sleep_us, timeout_us);
 	if (err)
 		dev_err(phy_drd->dev,
-			"timed out waiting for CDR lock (l2): %#.8x\n", reg);
+			"timed out waiting for CDR(l%d) lock: %#.8x\n",
+			((phy_drd->orientation == TYPEC_ORIENTATION_NORMAL)
+			 ? 0
+			 : 2), reg);
 }
 
 static void exynos5_usbdrd_utmi_init(struct exynos5_usbdrd_phy *phy_drd)
@@ -1184,7 +1207,8 @@ static int exynos850_usbdrd_phy_init(struct phy *phy)
 		return ret;
 
 	/* UTMI or PIPE3 specific init */
-	inst->phy_cfg->phy_init(phy_drd);
+	scoped_guard(mutex, &phy_drd->phy_mutex)
+		inst->phy_cfg->phy_init(phy_drd);
 
 	clk_bulk_disable_unprepare(phy_drd->drv_data->n_clks, phy_drd->clks);
 
@@ -1203,6 +1227,8 @@ static int exynos850_usbdrd_phy_exit(struct phy *phy)
 	if (ret)
 		return ret;
 
+	guard(mutex)(&phy_drd->phy_mutex);
+
 	/* Set PHY clock and control HS PHY */
 	reg = readl(regs_base + EXYNOS850_DRD_UTMI);
 	reg &= ~(UTMI_DP_PULLDOWN | UTMI_DM_PULLDOWN);
@@ -1701,6 +1727,10 @@ static int exynos5_usbdrd_phy_probe(struct platform_device *pdev)
 		return -EINVAL;
 	phy_drd->drv_data = drv_data;
 
+	ret = devm_mutex_init(dev, &phy_drd->phy_mutex);
+	if (ret)
+		return ret;
+
 	if (of_property_present(dev->of_node, "reg-names")) {
 		void __iomem *reg;
 

-- 
2.47.0.338.g60cca15819-goog

[PATCH 8/9] phy: exynos5-usbdrd: subscribe to orientation notifier if required

[André Draszik]

gs101's SS phy needs to be configured differently based on the
connector orientation, as the SS link can only be established if the
mux is configured correctly.

The code to handle programming of the mux is in place already, this commit
now adds the missing pieces to subscribe to the Type-C orientation
switch event.

Note that for this all to work we rely on the USB controller
re-initialising us. It should invoke our .exit() upon cable unplug, and
during cable plug we'll receive the orientation event after which we
expect our .init() to be called.

Above reinitialisation happens if the DWC3 controller can enter runtime
suspend automatically. For the DWC3 driver, this is an opt-in:
    echo auto > /sys/devices/.../11110000.usb/power/control
Once done, things work as long as the UDC is not bound as otherwise it
stays busy because it doesn't cancel / stop outstanding TRBs. For now
we have to manually unbind the UDC in that case:
     echo "" > sys/kernel/config/usb_gadget/.../UDC

Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
 drivers/phy/samsung/Kconfig              |  1 +
 drivers/phy/samsung/phy-exynos5-usbdrd.c | 60 ++++++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+)

diff --git a/drivers/phy/samsung/Kconfig b/drivers/phy/samsung/Kconfig
index f10afa3d7ff5..fc7bd1088576 100644
--- a/drivers/phy/samsung/Kconfig
+++ b/drivers/phy/samsung/Kconfig
@@ -80,6 +80,7 @@ config PHY_EXYNOS5_USBDRD
 	tristate "Exynos5 SoC series USB DRD PHY driver"
 	depends on (ARCH_EXYNOS && OF) || COMPILE_TEST
 	depends on HAS_IOMEM
+	depends on TYPEC || (TYPEC=n && COMPILE_TEST)
 	depends on USB_DWC3_EXYNOS
 	select GENERIC_PHY
 	select MFD_SYSCON
diff --git a/drivers/phy/samsung/phy-exynos5-usbdrd.c b/drivers/phy/samsung/phy-exynos5-usbdrd.c
index 1a34e9b4618a..2010d25ee817 100644
--- a/drivers/phy/samsung/phy-exynos5-usbdrd.c
+++ b/drivers/phy/samsung/phy-exynos5-usbdrd.c
@@ -394,6 +394,7 @@ struct exynos5_usbdrd_phy_drvdata {
  * @extrefclk: frequency select settings when using 'separate
  *	       reference clocks' for SS and HS operations
  * @regulators: regulators for phy
+ * @sw: TypeC orientation switch handle
  * @orientation: TypeC connector orientation - normal or flipped
  */
 struct exynos5_usbdrd_phy {
@@ -415,6 +416,7 @@ struct exynos5_usbdrd_phy {
 	u32 extrefclk;
 	struct regulator_bulk_data *regulators;
 
+	struct typec_switch_dev *sw;
 	enum typec_orientation orientation;
 };
 
@@ -1400,6 +1402,60 @@ static int exynos5_usbdrd_phy_clk_handle(struct exynos5_usbdrd_phy *phy_drd)
 	return 0;
 }
 
+#if IS_ENABLED(CONFIG_TYPEC)
+static int exynos5_usbdrd_orien_sw_set(struct typec_switch_dev *sw,
+				       enum typec_orientation orientation)
+{
+	struct exynos5_usbdrd_phy *phy_drd = typec_switch_get_drvdata(sw);
+
+	scoped_guard(mutex, &phy_drd->phy_mutex)
+		phy_drd->orientation = orientation;
+
+	return 0;
+}
+
+static void exynos5_usbdrd_orien_switch_unregister(void *data)
+{
+	struct exynos5_usbdrd_phy *phy_drd = data;
+
+	typec_switch_unregister(phy_drd->sw);
+}
+
+static int exynos5_usbdrd_setup_notifiers(struct exynos5_usbdrd_phy *phy_drd)
+{
+	int ret;
+
+	phy_drd->orientation = (enum typec_orientation)-1;
+	if (device_property_present(phy_drd->dev, "orientation-switch")) {
+		struct typec_switch_desc sw_desc = { };
+
+		sw_desc.drvdata = phy_drd;
+		sw_desc.fwnode = dev_fwnode(phy_drd->dev);
+		sw_desc.set = exynos5_usbdrd_orien_sw_set;
+
+		phy_drd->sw = typec_switch_register(phy_drd->dev, &sw_desc);
+		if (IS_ERR(phy_drd->sw))
+			return dev_err_probe(phy_drd->dev,
+					     PTR_ERR(phy_drd->sw),
+					     "Failed to register TypeC orientation switch\n");
+
+		ret = devm_add_action_or_reset(phy_drd->dev,
+					       exynos5_usbdrd_orien_switch_unregister,
+					       phy_drd);
+		if (ret)
+			return dev_err_probe(phy_drd->dev, ret,
+					     "Failed to register TypeC orientation devm action\n");
+	}
+
+	return 0;
+}
+#else /* CONFIG_TYPEC */
+static int exynos5_usbdrd_setup_notifiers(struct exynos5_usbdrd_phy *phy_drd)
+{
+	return 0;
+}
+#endif /* CONFIG_TYPEC */
+
 static const struct exynos5_usbdrd_phy_config phy_cfg_exynos5[] = {
 	{
 		.id		= EXYNOS5_DRDPHY_UTMI,
@@ -1789,6 +1845,10 @@ static int exynos5_usbdrd_phy_probe(struct platform_device *pdev)
 	if (ret)
 		return dev_err_probe(dev, ret, "failed to get regulators\n");
 
+	ret = exynos5_usbdrd_setup_notifiers(phy_drd);
+	if (ret)
+		return ret;
+
 	dev_vdbg(dev, "Creating usbdrd_phy phy\n");
 
 	for (i = 0; i < EXYNOS5_DRDPHYS_NUM; i++) {

-- 
2.47.0.338.g60cca15819-goog

[PATCH 9/9] phy: exynos5-usbdrd: allow DWC3 runtime suspend with UDC bound (E850+)

[André Draszik]

To make USB runtime suspend work when a UDC has been bound, the phy
needs to inform the USBDRD controller (DWC3) that Vbus and bvalid are
gone, so that it can in turn raise the respective gadget interrupt with
event == DWC3_DEVICE_EVENT_DISCONNECT, which will cause the USB stack
to clean up, allowing DWC3 to enter runtime suspend.

On e850 and gs101 this isn't working, as the respective signals are not
directly connected, and instead this driver uses override bits in the
PHY IP to set those signals. It currently forcefully sets them to 'on',
so the above mentioned interrupt will not be raised, preventing runtime
suspend.

To detect that state, update this driver to act on the TCPC's
orientation signal - when orientation == NONE, Vbus is gone and we can
clear the respective bits. Similarly, for other orientation values we
re-enable them.

This makes runtime suspend work on platforms with a TCPC (like Pixel6),
while keeping compatibility with platforms without (e850-96).

With runtime suspend working, USB-C cable orientation detection now
also fully works on such platforms, and the link comes up as Superspeed
as expected irrespective of the cable orientation and whether UDC /
gadget are configured and active.

Signed-off-by: André Draszik <andre.draszik@linaro.org>
---
 drivers/phy/samsung/phy-exynos5-usbdrd.c | 53 +++++++++++++++++++++++++++-----
 1 file changed, 45 insertions(+), 8 deletions(-)

diff --git a/drivers/phy/samsung/phy-exynos5-usbdrd.c b/drivers/phy/samsung/phy-exynos5-usbdrd.c
index 2010d25ee817..4882158df1a5 100644
--- a/drivers/phy/samsung/phy-exynos5-usbdrd.c
+++ b/drivers/phy/samsung/phy-exynos5-usbdrd.c
@@ -1136,13 +1136,15 @@ static void exynos850_usbdrd_utmi_init(struct exynos5_usbdrd_phy *phy_drd)
 	reg |= LINKCTRL_BUS_FILTER_BYPASS(0xf);
 	writel(reg, regs_base + EXYNOS850_DRD_LINKCTRL);
 
-	reg = readl(regs_base + EXYNOS850_DRD_UTMI);
-	reg |= UTMI_FORCE_BVALID | UTMI_FORCE_VBUSVALID;
-	writel(reg, regs_base + EXYNOS850_DRD_UTMI);
-
-	reg = readl(regs_base + EXYNOS850_DRD_HSP);
-	reg |= HSP_VBUSVLDEXT | HSP_VBUSVLDEXTSEL;
-	writel(reg, regs_base + EXYNOS850_DRD_HSP);
+	if (!phy_drd->sw) {
+		reg = readl(regs_base + EXYNOS850_DRD_UTMI);
+		reg |= UTMI_FORCE_BVALID | UTMI_FORCE_VBUSVALID;
+		writel(reg, regs_base + EXYNOS850_DRD_UTMI);
+
+		reg = readl(regs_base + EXYNOS850_DRD_HSP);
+		reg |= HSP_VBUSVLDEXT | HSP_VBUSVLDEXTSEL;
+		writel(reg, regs_base + EXYNOS850_DRD_HSP);
+	}
 
 	reg = readl(regs_base + EXYNOS850_DRD_SSPPLLCTL);
 	reg &= ~SSPPLLCTL_FSEL;
@@ -1407,9 +1409,44 @@ static int exynos5_usbdrd_orien_sw_set(struct typec_switch_dev *sw,
 				       enum typec_orientation orientation)
 {
 	struct exynos5_usbdrd_phy *phy_drd = typec_switch_get_drvdata(sw);
+	int ret;
+
+	if (phy_drd->orientation == orientation)
+		return 0;
+
+	ret = clk_bulk_prepare_enable(phy_drd->drv_data->n_clks, phy_drd->clks);
+	if (ret) {
+		dev_err(phy_drd->dev, "Failed to enable PHY clocks(s)\n");
+		return ret;
+	}
+
+	scoped_guard(mutex, &phy_drd->phy_mutex) {
+		void __iomem * const regs_base = phy_drd->reg_phy;
+		unsigned int reg;
+
+		if (orientation == TYPEC_ORIENTATION_NONE) {
+			reg = readl(regs_base + EXYNOS850_DRD_UTMI);
+			reg &= ~(UTMI_FORCE_VBUSVALID | UTMI_FORCE_BVALID);
+			writel(reg, regs_base +  EXYNOS850_DRD_UTMI);
+
+			reg = readl(regs_base + EXYNOS850_DRD_HSP);
+			reg |= HSP_VBUSVLDEXTSEL;
+			reg &= ~HSP_VBUSVLDEXT;
+			writel(reg, regs_base + EXYNOS850_DRD_HSP);
+		} else {
+			reg = readl(regs_base + EXYNOS850_DRD_UTMI);
+			reg |= UTMI_FORCE_VBUSVALID | UTMI_FORCE_BVALID;
+			writel(reg, regs_base +  EXYNOS850_DRD_UTMI);
+
+			reg = readl(regs_base + EXYNOS850_DRD_HSP);
+			reg |= HSP_VBUSVLDEXTSEL | HSP_VBUSVLDEXT;
+			writel(reg, regs_base + EXYNOS850_DRD_HSP);
+		}
 
-	scoped_guard(mutex, &phy_drd->phy_mutex)
 		phy_drd->orientation = orientation;
+	}
+
+	clk_bulk_disable(phy_drd->drv_data->n_clks, phy_drd->clks);
 
 	return 0;
 }

-- 
2.47.0.338.g60cca15819-goog

Re: [PATCH 3/9] dt-bindings: phy: samsung,usb3-drd-phy: gs101: require Type-C properties

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1401 bytes --]

On Wed, Nov 27, 2024 at 10:58:13AM +0000, André Draszik wrote:
> The USB PHY on gs101 needs to be configured based on the orientation of
> the connector. For that the DTS needs a link between the phy's port and
> a TCPCi, and we'll need to inform the phy driver that it should handle
> the orientation (register a handler).
> 
> Update the schema to enforce that by requiring the orientation-switch
> and port properties (on gs101 only).
> 
> Signed-off-by: André Draszik <andre.draszik@linaro.org>

What is your driver doing if these are not provided? New required
properties are an ABI break after all and I don't see a mention of how
you're handling it here.

> ---
>  Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml b/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
> index 6c17a44718aa..0659fc1ce13e 100644
> --- a/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
> @@ -142,6 +142,8 @@ allOf:
>  
>        required:
>          - reg-names
> +        - orientation-switch
> +        - port
>          - pll-supply
>          - dvdd-usb20-supply
>          - vddh-usb20-supply
> 
> -- 
> 2.47.0.338.g60cca15819-goog
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 2/9] dt-bindings: phy: samsung,usb3-drd-phy: add optional orientation-switch

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 465 bytes --]

On Wed, Nov 27, 2024 at 10:58:12AM +0000, André Draszik wrote:
> orientation-switch is the standard declaration to inform the Type-C mux
> layer that a remote-endpoint is capable of processing orientation
> change messages.
> 
> Add as an optional since not all versions of this phy currently support
> or even need the orientation-switch.
> 
> Signed-off-by: André Draszik <andre.draszik@linaro.org>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 2/9] dt-bindings: phy: samsung,usb3-drd-phy: add optional orientation-switch

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 649 bytes --]

On Wed, Nov 27, 2024 at 04:00:59PM +0000, Conor Dooley wrote:
> On Wed, Nov 27, 2024 at 10:58:12AM +0000, André Draszik wrote:
> > orientation-switch is the standard declaration to inform the Type-C mux
> > layer that a remote-endpoint is capable of processing orientation
> > change messages.
> > 
> > Add as an optional since not all versions of this phy currently support
> > or even need the orientation-switch.
> > 
> > Signed-off-by: André Draszik <andre.draszik@linaro.org>
> 
> Acked-by: Conor Dooley <conor.dooley@microchip.com>

Actually, this patch unconditionally adds the property to the binding.
Is it valid for !gs101?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 3/9] dt-bindings: phy: samsung,usb3-drd-phy: gs101: require Type-C properties

[André Draszik]

Hi Conor,

On Wed, 2024-11-27 at 16:00 +0000, Conor Dooley wrote:
> On Wed, Nov 27, 2024 at 10:58:13AM +0000, André Draszik wrote:
> > The USB PHY on gs101 needs to be configured based on the orientation of
> > the connector. For that the DTS needs a link between the phy's port and
> > a TCPCi, and we'll need to inform the phy driver that it should handle
> > the orientation (register a handler).
> > 
> > Update the schema to enforce that by requiring the orientation-switch
> > and port properties (on gs101 only).
> > 
> > Signed-off-by: André Draszik <andre.draszik@linaro.org>
> 
> What is your driver doing if these are not provided? New required
> properties are an ABI break after all and I don't see a mention of how
> you're handling it here.

This is hooked-in in patch 8 of this series in
exynos5_usbdrd_setup_notifiers(). The new behaviour is gated off

    if (device_property_present(phy_drd->dev, "orientation-switch")) {
        ...

Without that property (i.e. old DTS or !gs101), the driver will behave as
before (meaning for gs101 it will work in SS mode in one orientation only).

Does that address your concern?

Cheers,
Andre'

Re: [PATCH 2/9] dt-bindings: phy: samsung,usb3-drd-phy: add optional orientation-switch

[André Draszik]

Hi Conor,

Thanks for your review.

On Wed, 2024-11-27 at 16:02 +0000, Conor Dooley wrote:
> On Wed, Nov 27, 2024 at 04:00:59PM +0000, Conor Dooley wrote:
> > On Wed, Nov 27, 2024 at 10:58:12AM +0000, André Draszik wrote:
> > > orientation-switch is the standard declaration to inform the Type-C
> > > mux
> > > layer that a remote-endpoint is capable of processing orientation
> > > change messages.
> > > 
> > > Add as an optional since not all versions of this phy currently
> > > support
> > > or even need the orientation-switch.
> > > 
> > > Signed-off-by: André Draszik <andre.draszik@linaro.org>
> > 
> > Acked-by: Conor Dooley <conor.dooley@microchip.com>
> 
> Actually, this patch unconditionally adds the property to the binding.
> Is it valid for !gs101?

Good point. I've only made it required for gs101 in the next patch, but as-
is, somebody could still add it (in error) in !gs101 cases.

I guess it's best to squash patches 3 and 4 in that case, and update it to
disallow for !gs101.

Thanks,
Andre'

Re: [PATCH 3/9] dt-bindings: phy: samsung,usb3-drd-phy: gs101: require Type-C properties

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1351 bytes --]

On Wed, Nov 27, 2024 at 04:24:08PM +0000, André Draszik wrote:
> Hi Conor,
> 
> On Wed, 2024-11-27 at 16:00 +0000, Conor Dooley wrote:
> > On Wed, Nov 27, 2024 at 10:58:13AM +0000, André Draszik wrote:
> > > The USB PHY on gs101 needs to be configured based on the orientation of
> > > the connector. For that the DTS needs a link between the phy's port and
> > > a TCPCi, and we'll need to inform the phy driver that it should handle
> > > the orientation (register a handler).
> > > 
> > > Update the schema to enforce that by requiring the orientation-switch
> > > and port properties (on gs101 only).
> > > 
> > > Signed-off-by: André Draszik <andre.draszik@linaro.org>
> > 
> > What is your driver doing if these are not provided? New required
> > properties are an ABI break after all and I don't see a mention of how
> > you're handling it here.
> 
> This is hooked-in in patch 8 of this series in
> exynos5_usbdrd_setup_notifiers(). The new behaviour is gated off
> 
>     if (device_property_present(phy_drd->dev, "orientation-switch")) {
>         ...
> 
> Without that property (i.e. old DTS or !gs101), the driver will behave as
> before (meaning for gs101 it will work in SS mode in one orientation only).
> 
> Does that address your concern?

Aye, but it'd be good to call that out in your commit message.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 2/9] dt-bindings: phy: samsung,usb3-drd-phy: add optional orientation-switch

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1166 bytes --]

On Wed, Nov 27, 2024 at 04:27:09PM +0000, André Draszik wrote:
> Hi Conor,
> 
> Thanks for your review.
> 
> On Wed, 2024-11-27 at 16:02 +0000, Conor Dooley wrote:
> > On Wed, Nov 27, 2024 at 04:00:59PM +0000, Conor Dooley wrote:
> > > On Wed, Nov 27, 2024 at 10:58:12AM +0000, André Draszik wrote:
> > > > orientation-switch is the standard declaration to inform the Type-C
> > > > mux
> > > > layer that a remote-endpoint is capable of processing orientation
> > > > change messages.
> > > > 
> > > > Add as an optional since not all versions of this phy currently
> > > > support
> > > > or even need the orientation-switch.
> > > > 
> > > > Signed-off-by: André Draszik <andre.draszik@linaro.org>
> > > 
> > > Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > 
> > Actually, this patch unconditionally adds the property to the binding.
> > Is it valid for !gs101?
> 
> Good point. I've only made it required for gs101 in the next patch, but as-
> is, somebody could still add it (in error) in !gs101 cases.

> I guess it's best to squash patches 3 and 4 in that case, and update it to
> disallow for !gs101.

SGTM, go ahead.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 4/9] phy: exynos5-usbdrd: convert to dev_err_probe

[Krzysztof Kozlowski]

On 27/11/2024 11:58, André Draszik wrote:
> dev_err_probe() exists to simplify code.
> 
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
> ---
>  drivers/phy/samsung/phy-exynos5-usbdrd.c | 21 +++++++++------------
>  1 file changed, 9 insertions(+), 12 deletions(-)


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 6/9] phy: exynos5-usbdrd: gs101: ensure power is gated to SS phy in phy_exit()

[Krzysztof Kozlowski]

On 27/11/2024 11:58, André Draszik wrote:
> We currently don't gate the power to the SS phy in phy_exit().
> 
> Shuffle the code slightly to ensure the power is gated to the SS phy as
> well.
> 
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
> ---


I think this should be actually a fix with cc-stable.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

Re: [PATCH 6/9] phy: exynos5-usbdrd: gs101: ensure power is gated to SS phy in phy_exit()

[André Draszik]

Hi Krzysztof,

On Wed, 2024-11-27 at 20:42 +0100, Krzysztof Kozlowski wrote:
> On 27/11/2024 11:58, André Draszik wrote:
> > We currently don't gate the power to the SS phy in phy_exit().
> > 
> > Shuffle the code slightly to ensure the power is gated to the SS phy as
> > well.
> > 
> > Signed-off-by: André Draszik <andre.draszik@linaro.org>
> > ---
> 
> 
> I think this should be actually a fix with cc-stable.

Will do in v2.

> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Thanks for the review!

Cheers,
Andre'

Re: [PATCH 3/9] dt-bindings: phy: samsung,usb3-drd-phy: gs101: require Type-C properties

[Peter Griffin]

Hi André,

On Wed, 27 Nov 2024 at 10:58, André Draszik <andre.draszik@linaro.org> wrote:
>
> The USB PHY on gs101 needs to be configured based on the orientation of
> the connector. For that the DTS needs a link between the phy's port and
> a TCPCi, and we'll need to inform the phy driver that it should handle
> the orientation (register a handler).
>
> Update the schema to enforce that by requiring the orientation-switch
> and port properties (on gs101 only).
>
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
> ---

Reviewed-by: Peter Griffin <peter.griffin@linaro.org>

regards,

Peter

Re: [PATCH 4/9] phy: exynos5-usbdrd: convert to dev_err_probe

[Peter Griffin]

Hi André,

On Wed, 27 Nov 2024 at 10:58, André Draszik <andre.draszik@linaro.org> wrote:
>
> dev_err_probe() exists to simplify code.
>
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
> ---

Reviewed-by: Peter Griffin <peter.griffin@linaro.org>

regards.,

Peter

Re: [PATCH 1/9] dt-bindings: phy: samsung,usb3-drd-phy: align to universal style

[Peter Griffin]

Hi André,

On Wed, 27 Nov 2024 at 10:58, André Draszik <andre.draszik@linaro.org> wrote:
>
> In [1], Rob pointed out that we should really be separating properties
> with blank lines in between, which is universal style. Only where
> properties are booleans, empty lines are not required.
>
> Do so.
>
> Link: https://lore.kernel.org/all/20240711212359.GA3023490-robh@kernel.org/ [1]
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
> ---

Reviewed-by: Peter Griffin <peter.griffin@linaro.org>

regards.,

Peter

Re: [PATCH 5/9] phy: exynos5-usbdrd: fix EDS distribution tuning (gs101)

[Peter Griffin]

Hi André,

On Wed, 27 Nov 2024 at 10:58, André Draszik <andre.draszik@linaro.org> wrote:
>
> This code's intention is to configure lane0 and lane2 tunings, but for
> lane2 there is a typo and it ends up tuning something else.
>
> Fix the typo, as it doesn't appear to make sense to apply different
> tunings for lane0 vs lane2.
>
> The same typo appears to exist in the bootloader, hence we restore the
> original value in the typo'd registers as well. This can be removed
> once / if the bootloader is updated.
>
> Note that this is incorrect in the downstream driver as well - the
> values had been copied from there.
>
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
> ---

Reviewed-by: Peter Griffin <peter.griffin@linaro.org>
Tested-by: Peter Griffin <peter.griffin@linaro.org>

Validated that a super-speed USB link is established between Pixel 6
phone and my laptop with this patch applied and that adb works.

regards,

Peter

Re: [PATCH 6/9] phy: exynos5-usbdrd: gs101: ensure power is gated to SS phy in phy_exit()

[Peter Griffin]

Hi André,

On Wed, 27 Nov 2024 at 10:58, André Draszik <andre.draszik@linaro.org> wrote:
>
> We currently don't gate the power to the SS phy in phy_exit().
>
> Shuffle the code slightly to ensure the power is gated to the SS phy as
> well.
>
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
> ---

Reviewed-by: Peter Griffin <peter.griffin@linaro.org>

regards,

Peter

Re: [PATCH 7/9] phy: exynos5-usbdrd: gs101: configure SS lanes based on orientation

[Peter Griffin]

Hi André,

On Wed, 27 Nov 2024 at 10:58, André Draszik <andre.draszik@linaro.org> wrote:
>
> USB SS lanes need to be configured based the connector orientation -

small nit: "based the" -> "based on the"

> at most two lanes will be in use for USB (and the remaining two for
> alternate modes like DP).
>
> For the USB link to come up in SS, the lane configuration registers
> have to be programmed accordingly.
>
> While we still need a way to be notified of the actual connector
> orientation and then reprogram the registers accordingly (at the moment
> the configuration happens just once during phy_init() and never again),
> we can prepare the code doing the configuration to take the orientation
> into account.
>
> Do so.
>
> Note: the mutex is needed to synchronize this with the upcoming
> connector orientation callback.
>
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
> ---
>  drivers/phy/samsung/phy-exynos5-usbdrd.c | 72 ++++++++++++++++++++++----------
>  1 file changed, 51 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/phy/samsung/phy-exynos5-usbdrd.c b/drivers/phy/samsung/phy-exynos5-usbdrd.c
> index c1ce6fdeef31..1a34e9b4618a 100644
> --- a/drivers/phy/samsung/phy-exynos5-usbdrd.c
> +++ b/drivers/phy/samsung/phy-exynos5-usbdrd.c
> @@ -23,6 +23,7 @@
>  #include <linux/regmap.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/soc/samsung/exynos-regs-pmu.h>
> +#include <linux/usb/typec_mux.h>
>
>  /* Exynos USB PHY registers */
>  #define EXYNOS5_FSEL_9MHZ6             0x0
> @@ -209,6 +210,10 @@
>
>  #define EXYNOS9_PMA_USBDP_CMN_REG00B8          0x02e0
>  #define CMN_REG00B8_LANE_MUX_SEL_DP            GENMASK(3, 0)
> +#define CMN_REG00B8_LANE_MUX_SEL_DP_LANE3      BIT(3)
> +#define CMN_REG00B8_LANE_MUX_SEL_DP_LANE2      BIT(2)
> +#define CMN_REG00B8_LANE_MUX_SEL_DP_LANE1      BIT(1)
> +#define CMN_REG00B8_LANE_MUX_SEL_DP_LANE0      BIT(0)
>
>  #define EXYNOS9_PMA_USBDP_CMN_REG01C0          0x0700
>  #define CMN_REG01C0_ANA_LCPLL_LOCK_DONE                BIT(7)
> @@ -383,11 +388,13 @@ struct exynos5_usbdrd_phy_drvdata {
>   * @clks: clocks for register access
>   * @core_clks: core clocks for phy (ref, pipe3, utmi+, ITP, etc. as required)
>   * @drv_data: pointer to SoC level driver data structure
> + * @phy_mutex: mutex protecting phy_init/exit & TCPC callbacks
>   * @phys: array for 'EXYNOS5_DRDPHYS_NUM' number of PHY
>   *         instances each with its 'phy' and 'phy_cfg'.
>   * @extrefclk: frequency select settings when using 'separate
>   *            reference clocks' for SS and HS operations
>   * @regulators: regulators for phy
> + * @orientation: TypeC connector orientation - normal or flipped
>   */
>  struct exynos5_usbdrd_phy {
>         struct device *dev;
> @@ -397,6 +404,7 @@ struct exynos5_usbdrd_phy {
>         struct clk_bulk_data *clks;
>         struct clk_bulk_data *core_clks;
>         const struct exynos5_usbdrd_phy_drvdata *drv_data;
> +       struct mutex phy_mutex;
>         struct phy_usb_instance {
>                 struct phy *phy;
>                 u32 index;
> @@ -406,6 +414,8 @@ struct exynos5_usbdrd_phy {
>         } phys[EXYNOS5_DRDPHYS_NUM];
>         u32 extrefclk;
>         struct regulator_bulk_data *regulators;
> +
> +       enum typec_orientation orientation;

typec_orientation is defined in #include <linux/usb/typec.h> so it
should be added to the includes

Apart from that

Reviewed-by: Peter Griffin <peter.griffin@linaro.org>
Tested-by: Peter Griffin <peter.griffin@linaro.org>

Validated that a super-speed USB link is established between Pixel 6
phone and my laptop with this patch applied and that adb works.

regards,

Peter

Re: [PATCH 8/9] phy: exynos5-usbdrd: subscribe to orientation notifier if required

[Vinod Koul]

On 27-11-24, 10:58, André Draszik wrote:
> gs101's SS phy needs to be configured differently based on the
> connector orientation, as the SS link can only be established if the
> mux is configured correctly.
> 
> The code to handle programming of the mux is in place already, this commit
> now adds the missing pieces to subscribe to the Type-C orientation
> switch event.
> 
> Note that for this all to work we rely on the USB controller
> re-initialising us. It should invoke our .exit() upon cable unplug, and
> during cable plug we'll receive the orientation event after which we
> expect our .init() to be called.
> 
> Above reinitialisation happens if the DWC3 controller can enter runtime
> suspend automatically. For the DWC3 driver, this is an opt-in:
>     echo auto > /sys/devices/.../11110000.usb/power/control
> Once done, things work as long as the UDC is not bound as otherwise it
> stays busy because it doesn't cancel / stop outstanding TRBs. For now
> we have to manually unbind the UDC in that case:
>      echo "" > sys/kernel/config/usb_gadget/.../UDC
> 
> Signed-off-by: André Draszik <andre.draszik@linaro.org>
> ---
>  drivers/phy/samsung/Kconfig              |  1 +
>  drivers/phy/samsung/phy-exynos5-usbdrd.c | 60 ++++++++++++++++++++++++++++++++
>  2 files changed, 61 insertions(+)
> 
> diff --git a/drivers/phy/samsung/Kconfig b/drivers/phy/samsung/Kconfig
> index f10afa3d7ff5..fc7bd1088576 100644
> --- a/drivers/phy/samsung/Kconfig
> +++ b/drivers/phy/samsung/Kconfig
> @@ -80,6 +80,7 @@ config PHY_EXYNOS5_USBDRD
>  	tristate "Exynos5 SoC series USB DRD PHY driver"
>  	depends on (ARCH_EXYNOS && OF) || COMPILE_TEST
>  	depends on HAS_IOMEM
> +	depends on TYPEC || (TYPEC=n && COMPILE_TEST)
>  	depends on USB_DWC3_EXYNOS
>  	select GENERIC_PHY
>  	select MFD_SYSCON
> diff --git a/drivers/phy/samsung/phy-exynos5-usbdrd.c b/drivers/phy/samsung/phy-exynos5-usbdrd.c
> index 1a34e9b4618a..2010d25ee817 100644
> --- a/drivers/phy/samsung/phy-exynos5-usbdrd.c
> +++ b/drivers/phy/samsung/phy-exynos5-usbdrd.c
> @@ -394,6 +394,7 @@ struct exynos5_usbdrd_phy_drvdata {
>   * @extrefclk: frequency select settings when using 'separate
>   *	       reference clocks' for SS and HS operations
>   * @regulators: regulators for phy
> + * @sw: TypeC orientation switch handle
>   * @orientation: TypeC connector orientation - normal or flipped
>   */
>  struct exynos5_usbdrd_phy {
> @@ -415,6 +416,7 @@ struct exynos5_usbdrd_phy {
>  	u32 extrefclk;
>  	struct regulator_bulk_data *regulators;
>  
> +	struct typec_switch_dev *sw;
>  	enum typec_orientation orientation;
>  };
>  
> @@ -1400,6 +1402,60 @@ static int exynos5_usbdrd_phy_clk_handle(struct exynos5_usbdrd_phy *phy_drd)
>  	return 0;
>  }
>  
> +#if IS_ENABLED(CONFIG_TYPEC)
> +static int exynos5_usbdrd_orien_sw_set(struct typec_switch_dev *sw,
> +				       enum typec_orientation orientation)
> +{
> +	struct exynos5_usbdrd_phy *phy_drd = typec_switch_get_drvdata(sw);
> +
> +	scoped_guard(mutex, &phy_drd->phy_mutex)
> +		phy_drd->orientation = orientation;
> +
> +	return 0;
> +}
> +
> +static void exynos5_usbdrd_orien_switch_unregister(void *data)
> +{
> +	struct exynos5_usbdrd_phy *phy_drd = data;
> +
> +	typec_switch_unregister(phy_drd->sw);
> +}
> +
> +static int exynos5_usbdrd_setup_notifiers(struct exynos5_usbdrd_phy *phy_drd)
> +{
> +	int ret;
> +
> +	phy_drd->orientation = (enum typec_orientation)-1;

Should this be TYPEC_ORIENTATION_NONE?

> +	if (device_property_present(phy_drd->dev, "orientation-switch")) {
> +		struct typec_switch_desc sw_desc = { };
> +
> +		sw_desc.drvdata = phy_drd;
> +		sw_desc.fwnode = dev_fwnode(phy_drd->dev);
> +		sw_desc.set = exynos5_usbdrd_orien_sw_set;
> +
> +		phy_drd->sw = typec_switch_register(phy_drd->dev, &sw_desc);
> +		if (IS_ERR(phy_drd->sw))
> +			return dev_err_probe(phy_drd->dev,
> +					     PTR_ERR(phy_drd->sw),
> +					     "Failed to register TypeC orientation switch\n");
> +
> +		ret = devm_add_action_or_reset(phy_drd->dev,
> +					       exynos5_usbdrd_orien_switch_unregister,
> +					       phy_drd);
> +		if (ret)
> +			return dev_err_probe(phy_drd->dev, ret,
> +					     "Failed to register TypeC orientation devm action\n");
> +	}
> +
> +	return 0;
> +}
> +#else /* CONFIG_TYPEC */
> +static int exynos5_usbdrd_setup_notifiers(struct exynos5_usbdrd_phy *phy_drd)
> +{
> +	return 0;
> +}
> +#endif /* CONFIG_TYPEC */
> +
>  static const struct exynos5_usbdrd_phy_config phy_cfg_exynos5[] = {
>  	{
>  		.id		= EXYNOS5_DRDPHY_UTMI,
> @@ -1789,6 +1845,10 @@ static int exynos5_usbdrd_phy_probe(struct platform_device *pdev)
>  	if (ret)
>  		return dev_err_probe(dev, ret, "failed to get regulators\n");
>  
> +	ret = exynos5_usbdrd_setup_notifiers(phy_drd);
> +	if (ret)
> +		return ret;
> +
>  	dev_vdbg(dev, "Creating usbdrd_phy phy\n");
>  
>  	for (i = 0; i < EXYNOS5_DRDPHYS_NUM; i++) {
> 
> -- 
> 2.47.0.338.g60cca15819-goog

-- 
~Vinod

Re: [PATCH 8/9] phy: exynos5-usbdrd: subscribe to orientation notifier if required

[André Draszik]

Hi Vinod,

Thanks for your review.

On Wed, 2024-12-04 at 19:40 +0530, Vinod Koul wrote:
> On 27-11-24, 10:58, André Draszik wrote:
> > gs101's SS phy needs to be configured differently based on the
> > connector orientation, as the SS link can only be established if the
> > mux is configured correctly.
> > 
> > The code to handle programming of the mux is in place already, this commit
> > now adds the missing pieces to subscribe to the Type-C orientation
> > switch event.
> > 
> > Note that for this all to work we rely on the USB controller
> > re-initialising us. It should invoke our .exit() upon cable unplug, and
> > during cable plug we'll receive the orientation event after which we
> > expect our .init() to be called.
> > 
> > Above reinitialisation happens if the DWC3 controller can enter runtime
> > suspend automatically. For the DWC3 driver, this is an opt-in:
> >     echo auto > /sys/devices/.../11110000.usb/power/control
> > Once done, things work as long as the UDC is not bound as otherwise it
> > stays busy because it doesn't cancel / stop outstanding TRBs. For now
> > we have to manually unbind the UDC in that case:
> >      echo "" > sys/kernel/config/usb_gadget/.../UDC
> > 
> > Signed-off-by: André Draszik <andre.draszik@linaro.org>
> > ---
> >  drivers/phy/samsung/Kconfig              |  1 +
> >  drivers/phy/samsung/phy-exynos5-usbdrd.c | 60 ++++++++++++++++++++++++++++++++
> >  2 files changed, 61 insertions(+)
> > 
> > diff --git a/drivers/phy/samsung/Kconfig b/drivers/phy/samsung/Kconfig
> > index f10afa3d7ff5..fc7bd1088576 100644
> > --- a/drivers/phy/samsung/Kconfig
> > +++ b/drivers/phy/samsung/Kconfig
> > @@ -80,6 +80,7 @@ config PHY_EXYNOS5_USBDRD
> >  	tristate "Exynos5 SoC series USB DRD PHY driver"
> >  	depends on (ARCH_EXYNOS && OF) || COMPILE_TEST
> >  	depends on HAS_IOMEM
> > +	depends on TYPEC || (TYPEC=n && COMPILE_TEST)
> >  	depends on USB_DWC3_EXYNOS
> >  	select GENERIC_PHY
> >  	select MFD_SYSCON
> > diff --git a/drivers/phy/samsung/phy-exynos5-usbdrd.c b/drivers/phy/samsung/phy-exynos5-usbdrd.c
> > index 1a34e9b4618a..2010d25ee817 100644
> > --- a/drivers/phy/samsung/phy-exynos5-usbdrd.c
> > +++ b/drivers/phy/samsung/phy-exynos5-usbdrd.c
> > @@ -394,6 +394,7 @@ struct exynos5_usbdrd_phy_drvdata {
> >   * @extrefclk: frequency select settings when using 'separate
> >   *	       reference clocks' for SS and HS operations
> >   * @regulators: regulators for phy
> > + * @sw: TypeC orientation switch handle
> >   * @orientation: TypeC connector orientation - normal or flipped
> >   */
> >  struct exynos5_usbdrd_phy {
> > @@ -415,6 +416,7 @@ struct exynos5_usbdrd_phy {
> >  	u32 extrefclk;
> >  	struct regulator_bulk_data *regulators;
> >  
> > +	struct typec_switch_dev *sw;
> >  	enum typec_orientation orientation;
> >  };
> >  
> > @@ -1400,6 +1402,60 @@ static int exynos5_usbdrd_phy_clk_handle(struct exynos5_usbdrd_phy *phy_drd)
> >  	return 0;
> >  }
> >  
> > +#if IS_ENABLED(CONFIG_TYPEC)
> > +static int exynos5_usbdrd_orien_sw_set(struct typec_switch_dev *sw,
> > +				       enum typec_orientation orientation)
> > +{
> > +	struct exynos5_usbdrd_phy *phy_drd = typec_switch_get_drvdata(sw);
> > +
> > +	scoped_guard(mutex, &phy_drd->phy_mutex)
> > +		phy_drd->orientation = orientation;
> > +
> > +	return 0;
> > +}
> > +
> > +static void exynos5_usbdrd_orien_switch_unregister(void *data)
> > +{
> > +	struct exynos5_usbdrd_phy *phy_drd = data;
> > +
> > +	typec_switch_unregister(phy_drd->sw);
> > +}
> > +
> > +static int exynos5_usbdrd_setup_notifiers(struct exynos5_usbdrd_phy *phy_drd)
> > +{
> > +	int ret;
> > +
> > +	phy_drd->orientation = (enum typec_orientation)-1;
> 
> Should this be TYPEC_ORIENTATION_NONE?

It's -1 so that exynos5_usbdrd_orien_sw_set() (patch 9) can
check against the current value after driver load.

That said, it should probably be moved into patch 9 where it
actually matters, or even be removed altogether, in combination
with that check in exynos5_usbdrd_orien_sw_set() from patch 9.

I had added the check because some (not all) other phy drivers
do that, but looking further at that, I believe it's not
actually necessary, exynos5_usbdrd_orien_sw_set() can execute
unconditionally.

Will remove this and related code in next version.

Cheers,
Andre'