[PATCH 0/3] Add support for multiport controller

[PATCH 0/3] Add support for multiport controller

[Harsh Agarwal]

Currently the DWC3 driver supports only single port controller which 
requires at most two PHYs ie HS and SS PHYs. There are SoCs that has
DWC3 controller with multiple ports that can operate in host mode. Some of
the port supports both SS+HS and other port supports only HS mode.

This change refactors the PHY logic to support multiport controller. The 
implementation has been tested with Generic PHYs as well.

For any multiport controller we would define a new node "multiport" inside
dwc3 and then add subsequent "mport" nodes inside it for individual ports
that it supports. Now each individual "mport" node defines their own PHYs.

e.g.
Consider a Dual port controller where each port supports HS+SS 

multiport {
	mp_1: mport@1 {
		usb-phy = <usb2_phy0>, <usb3_phy0>;
        /* Can define Generic PHYs also */  
	};	
	mp_2: mport@2 {
		usb-phy = <usb2_phy1>, <usb3_phy1>;
	};	

Harsh Agarwal (3):
  dt-bindings: usb: dwc3: Add support for multiport related properties
  usb: phy: Add devm_of_usb_get_phy_by_phandle
  usb: dwc3: Refactor PHY logic to support Multiport Controller

 .../devicetree/bindings/usb/snps,dwc3.yaml         |  55 +++
 drivers/usb/dwc3/core.c                            | 400 +++++++++++++++------
 drivers/usb/dwc3/core.h                            |  12 +-
 drivers/usb/dwc3/drd.c                             |  16 +-
 drivers/usb/dwc3/gadget.c                          |   4 +-
 drivers/usb/phy/phy.c                              |  34 ++
 include/linux/usb/phy.h                            |   8 +
 7 files changed, 402 insertions(+), 127 deletions(-)

-- 
2.7.4

[PATCH 1/3] dt-bindings: usb: dwc3: Add support for multiport related properties

[Harsh Agarwal]

Added support for multiport, mport, num-ssphy and num-hsphy
properties. These properties are used to support devices having
a multiport controller.

Signed-off-by: Harsh Agarwal <quic_harshq@quicinc.com>
---
 .../devicetree/bindings/usb/snps,dwc3.yaml         | 55 ++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
index f4471f8..9d916c72 100644
--- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
@@ -341,6 +341,34 @@ properties:
       This port is used with the 'usb-role-switch' property  to connect the
       dwc3 to type C connector.
 
+  multiport:
+    description:
+      If a single USB controller supports multiple ports, then it's referred to as
+      a multiport controller. Each port of the multiport controller can support
+      either High Speed or Super Speed or both and have their own PHY phandles. Each
+      port is represented by "mport" node and all the "mport" nodes are grouped
+      together inside the "multiport" node where individual "mport" node defines the
+      PHYs supported by that port.
+    required:
+      - mport
+
+  num_usb2_phy:
+    description: Total number of HS-PHYs defined by the multiport controller.
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  num_usb3_phy:
+    description: Total number of SS-PHYs defined by the multiport controller.
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  mport:
+    description: Each mport node represents one port of the multiport controller.
+    oneOf:
+       - required:
+         - usb-phy
+       - required:
+          - phys
+          - phy-names
+
 unevaluatedProperties: false
 
 required:
@@ -369,4 +397,31 @@ examples:
       snps,dis_u2_susphy_quirk;
       snps,dis_enblslpm_quirk;
     };
+  - |
+    usb@4a000000 {
+      compatible = "snps,dwc3";
+      reg = <0x4a000000 0xcfff>;
+      interrupts = <0 92 4>;
+
+      multiport {
+
+        MP_1: mport@1 {
+          usb-phy = <&usb2_phy0>, <&usb3_phy0>;
+          /* Can define Generic PHYs also */
+        };
+
+        MP_2: mport@2 {
+          usb-phy = <&usb2_phy1>, <&usb3_phy1>;
+        };
+
+        MP_3: mport@3 {
+          usb-phy = <&usb2_phy2>;
+        };
+
+        MP_4: mport@4 {
+          usb-phy = <&usb2_phy3>;
+        };
+
+      };
+    };
 ...
-- 
2.7.4

[PATCH 2/3] usb: phy: Add devm_of_usb_get_phy_by_phandle

[Harsh Agarwal]

Adding support for devm_of_usb_get_phy_by_phandle which allows
us to get PHY phandles of a device declared inside lookup_node.

Signed-off-by: Harsh Agarwal <quic_harshq@quicinc.com>
---
 drivers/usb/phy/phy.c   | 34 ++++++++++++++++++++++++++++++++++
 include/linux/usb/phy.h |  8 ++++++++
 2 files changed, 42 insertions(+)

diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c
index 1b24492..0843757 100644
--- a/drivers/usb/phy/phy.c
+++ b/drivers/usb/phy/phy.c
@@ -615,6 +615,40 @@ struct usb_phy *devm_usb_get_phy_by_phandle(struct device *dev,
 EXPORT_SYMBOL_GPL(devm_usb_get_phy_by_phandle);
 
 /**
+ * devm_of_usb_get_phy_by_phandle - find the USB PHY by phandle in lookup_node
+ * @dev: device that requests this phy
+ * @phandle: name of the property holding the phy phandle value
+ * @index: the index of the phy
+ * @lookup_node: The node to search for PHY phandles.
+ *
+ * Returns the phy driver associated with the given phandle value,
+ * after getting a refcount to it, -ENODEV if there is no such phy or
+ * -EPROBE_DEFER if there is a phandle to the phy, but the device is
+ * not yet loaded. While at that, it also associates the device with
+ * the phy using devres. On driver detach, release function is invoked
+ * on the devres data, then, devres data is freed.
+ *
+ * For use by USB host and peripheral drivers.
+ */
+struct usb_phy *devm_of_usb_get_phy_by_phandle(struct device *dev,
+	const char *phandle, u8 index, struct device_node *lookup_node)
+{
+	struct device_node *node;
+	struct usb_phy	*phy;
+
+	node = of_parse_phandle(lookup_node, phandle, index);
+	if (!node) {
+		dev_dbg(dev, "failed to get %s phandle in %pOF node\n", phandle,
+			dev->of_node);
+		return ERR_PTR(-ENODEV);
+	}
+	phy = devm_usb_get_phy_by_node(dev, node, NULL);
+	of_node_put(node);
+	return phy;
+}
+EXPORT_SYMBOL_GPL(devm_of_usb_get_phy_by_phandle);
+
+/**
  * devm_usb_put_phy - release the USB PHY
  * @dev: device that wants to release this phy
  * @phy: the phy returned by devm_usb_get_phy()
diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h
index e4de6bc..2581c72 100644
--- a/include/linux/usb/phy.h
+++ b/include/linux/usb/phy.h
@@ -220,6 +220,8 @@ extern struct usb_phy *devm_usb_get_phy(struct device *dev,
 	enum usb_phy_type type);
 extern struct usb_phy *devm_usb_get_phy_by_phandle(struct device *dev,
 	const char *phandle, u8 index);
+extern struct usb_phy *devm_of_usb_get_phy_by_phandle(struct device *dev,
+	const char *phandle, u8 index, struct device_node *lookup_node);
 extern struct usb_phy *devm_usb_get_phy_by_node(struct device *dev,
 	struct device_node *node, struct notifier_block *nb);
 extern void usb_put_phy(struct usb_phy *);
@@ -249,6 +251,12 @@ static inline struct usb_phy *devm_usb_get_phy_by_phandle(struct device *dev,
 	return ERR_PTR(-ENXIO);
 }
 
+extern inline struct usb_phy *devm_of_usb_get_phy_by_phandle(struct device *dev,
+	const char *phandle, u8 index, struct device_node *lookup_node)
+{
+	return ERR_PTR(-ENXIO);
+}
+
 static inline struct usb_phy *devm_usb_get_phy_by_node(struct device *dev,
 	struct device_node *node, struct notifier_block *nb)
 {
-- 
2.7.4

[PATCH 3/3] usb: dwc3: Refactor PHY logic to support Multiport Controller

[Harsh Agarwal]

Currently the DWC3 driver supports only single port controller
which requires at most 2 PHYs ie HS and SS PHYs.

But some SOCs have a "multiport" USB DWC3 controller where a
single controller supports multiple ports and each port have
their own PHYs. Refactor PHY logic to support the same.

Signed-off-by: Harsh Agarwal <quic_harshq@quicinc.com>
---
 drivers/usb/dwc3/core.c   | 400 +++++++++++++++++++++++++++++++++-------------
 drivers/usb/dwc3/core.h   |  12 +-
 drivers/usb/dwc3/drd.c    |  16 +-
 drivers/usb/dwc3/gadget.c |   4 +-
 4 files changed, 305 insertions(+), 127 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 5734219..5cc799e 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -120,7 +120,7 @@ static void __dwc3_set_mode(struct work_struct *work)
 {
 	struct dwc3 *dwc = work_to_dwc(work);
 	unsigned long flags;
-	int ret;
+	int i, ret;
 	u32 reg;
 
 	mutex_lock(&dwc->mutex);
@@ -189,10 +189,13 @@ static void __dwc3_set_mode(struct work_struct *work)
 		if (ret) {
 			dev_err(dwc->dev, "failed to initialize host\n");
 		} else {
-			if (dwc->usb2_phy)
-				otg_set_vbus(dwc->usb2_phy->otg, true);
-			phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST);
-			phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_HOST);
+			for (i = 0; i < dwc->num_usb2_phy; i++) {
+				if (dwc->usb2_phy[i])
+					otg_set_vbus(dwc->usb2_phy[i]->otg, true);
+				phy_set_mode(dwc->usb2_generic_phy[i], PHY_MODE_USB_HOST);
+			}
+			for (i = 0; i < dwc->num_usb3_phy; i++)
+				phy_set_mode(dwc->usb3_generic_phy[i], PHY_MODE_USB_HOST);
 			if (dwc->dis_split_quirk) {
 				reg = dwc3_readl(dwc->regs, DWC3_GUCTL3);
 				reg |= DWC3_GUCTL3_SPLITDISABLE;
@@ -205,10 +208,10 @@ static void __dwc3_set_mode(struct work_struct *work)
 
 		dwc3_event_buffers_setup(dwc);
 
-		if (dwc->usb2_phy)
-			otg_set_vbus(dwc->usb2_phy->otg, false);
-		phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_DEVICE);
-		phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_DEVICE);
+		if (dwc->usb2_phy[0])
+			otg_set_vbus(dwc->usb2_phy[0]->otg, false);
+		phy_set_mode(dwc->usb2_generic_phy[0], PHY_MODE_USB_DEVICE);
+		phy_set_mode(dwc->usb3_generic_phy[0], PHY_MODE_USB_DEVICE);
 
 		ret = dwc3_gadget_init(dwc);
 		if (ret)
@@ -656,6 +659,7 @@ static int dwc3_core_ulpi_init(struct dwc3 *dwc)
  */
 static int dwc3_phy_setup(struct dwc3 *dwc)
 {
+	int i;
 	unsigned int hw_mode;
 	u32 reg;
 
@@ -716,7 +720,8 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
 	if (dwc->dis_del_phy_power_chg_quirk)
 		reg &= ~DWC3_GUSB3PIPECTL_DEPOCHANGE;
 
-	dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg);
+	for (i = 0; i < dwc->num_usb3_phy; i++)
+		dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(i), reg);
 
 	reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
 
@@ -730,7 +735,8 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
 		} else if (dwc->hsphy_interface &&
 				!strncmp(dwc->hsphy_interface, "ulpi", 4)) {
 			reg |= DWC3_GUSB2PHYCFG_ULPI_UTMI;
-			dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
+			for (i = 0; i < dwc->num_usb2_phy; i++)
+				dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
 		} else {
 			/* Relying on default value. */
 			if (!(reg & DWC3_GUSB2PHYCFG_ULPI_UTMI))
@@ -787,7 +793,8 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
 	if (dwc->dis_u2_freeclk_exists_quirk)
 		reg &= ~DWC3_GUSB2PHYCFG_U2_FREECLK_EXISTS;
 
-	dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
+	for (i = 0; i < dwc->num_usb2_phy; i++)
+		dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
 
 	return 0;
 }
@@ -826,17 +833,23 @@ static void dwc3_clk_disable(struct dwc3 *dwc)
 
 static void dwc3_core_exit(struct dwc3 *dwc)
 {
+	int i;
 	dwc3_event_buffers_cleanup(dwc);
 
-	usb_phy_shutdown(dwc->usb2_phy);
-	usb_phy_shutdown(dwc->usb3_phy);
-	phy_exit(dwc->usb2_generic_phy);
-	phy_exit(dwc->usb3_generic_phy);
+	for (i = 0; i < dwc->num_usb2_phy; i++) {
+		usb_phy_shutdown(dwc->usb2_phy[i]);
+		usb_phy_set_suspend(dwc->usb2_phy[i], 1);
+		phy_exit(dwc->usb2_generic_phy[i]);
+		phy_power_off(dwc->usb2_generic_phy[i]);
+	}
+
+	for (i = 0; i < dwc->num_usb3_phy; i++) {
+		usb_phy_shutdown(dwc->usb3_phy[i]);
+		usb_phy_set_suspend(dwc->usb3_phy[i], 1);
+		phy_exit(dwc->usb3_generic_phy[i]);
+		phy_power_off(dwc->usb3_generic_phy[i]);
+	}
 
-	usb_phy_set_suspend(dwc->usb2_phy, 1);
-	usb_phy_set_suspend(dwc->usb3_phy, 1);
-	phy_power_off(dwc->usb2_generic_phy);
-	phy_power_off(dwc->usb3_generic_phy);
 	dwc3_clk_disable(dwc);
 	reset_control_assert(dwc->reset);
 }
@@ -1039,7 +1052,7 @@ static int dwc3_core_init(struct dwc3 *dwc)
 {
 	unsigned int		hw_mode;
 	u32			reg;
-	int			ret;
+	int			ret, i;
 
 	hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
 
@@ -1067,16 +1080,24 @@ static int dwc3_core_init(struct dwc3 *dwc)
 		dwc->phys_ready = true;
 	}
 
-	usb_phy_init(dwc->usb2_phy);
-	usb_phy_init(dwc->usb3_phy);
-	ret = phy_init(dwc->usb2_generic_phy);
-	if (ret < 0)
-		goto err0a;
+	for (i = 0; i < dwc->num_usb2_phy; i++)
+		usb_phy_init(dwc->usb2_phy[i]);
+	for (i = 0; i < dwc->num_usb3_phy; i++)
+		usb_phy_init(dwc->usb3_phy[i]);
 
-	ret = phy_init(dwc->usb3_generic_phy);
-	if (ret < 0) {
-		phy_exit(dwc->usb2_generic_phy);
-		goto err0a;
+	for (i = 0; i < dwc->num_usb2_phy; i++) {
+		ret = phy_init(dwc->usb2_generic_phy[i]);
+		if (ret < 0)
+			goto err0a;
+	}
+
+	for (i = 0; i < dwc->num_usb3_phy; i++) {
+		ret = phy_init(dwc->usb3_generic_phy[i]);
+		if (ret < 0) {
+			for (i = 0; i < dwc->num_usb2_phy; i++)
+				phy_exit(dwc->usb2_generic_phy[i]);
+			goto err0a;
+		}
 	}
 
 	ret = dwc3_core_soft_reset(dwc);
@@ -1086,15 +1107,19 @@ static int dwc3_core_init(struct dwc3 *dwc)
 	if (hw_mode == DWC3_GHWPARAMS0_MODE_DRD &&
 	    !DWC3_VER_IS_WITHIN(DWC3, ANY, 194A)) {
 		if (!dwc->dis_u3_susphy_quirk) {
-			reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0));
-			reg |= DWC3_GUSB3PIPECTL_SUSPHY;
-			dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg);
+			for (i = 0; i < dwc->num_usb3_phy; i++) {
+				reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(i));
+				reg |= DWC3_GUSB3PIPECTL_SUSPHY;
+				dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(i), reg);
+			}
 		}
 
 		if (!dwc->dis_u2_susphy_quirk) {
-			reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
-			reg |= DWC3_GUSB2PHYCFG_SUSPHY;
-			dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
+			for (i = 0; i < dwc->num_usb2_phy; i++) {
+				reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(i));
+				reg |= DWC3_GUSB2PHYCFG_SUSPHY;
+				dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
+			}
 		}
 	}
 
@@ -1113,15 +1138,19 @@ static int dwc3_core_init(struct dwc3 *dwc)
 
 	dwc3_set_incr_burst_type(dwc);
 
-	usb_phy_set_suspend(dwc->usb2_phy, 0);
-	usb_phy_set_suspend(dwc->usb3_phy, 0);
-	ret = phy_power_on(dwc->usb2_generic_phy);
-	if (ret < 0)
-		goto err2;
+	for (i = 0; i < dwc->num_usb2_phy; i++) {
+		usb_phy_set_suspend(dwc->usb2_phy[i], 0);
+		ret = phy_power_on(dwc->usb2_generic_phy[i]);
+		if (ret < 0)
+			goto err2;
+	}
 
-	ret = phy_power_on(dwc->usb3_generic_phy);
-	if (ret < 0)
-		goto err3;
+	for (i = 0; i < dwc->num_usb3_phy; i++) {
+		usb_phy_set_suspend(dwc->usb3_phy[i], 0);
+		ret = phy_power_on(dwc->usb3_generic_phy[i]);
+		if (ret < 0)
+			goto err3;
+	}
 
 	ret = dwc3_event_buffers_setup(dwc);
 	if (ret) {
@@ -1229,20 +1258,29 @@ static int dwc3_core_init(struct dwc3 *dwc)
 	return 0;
 
 err4:
-	phy_power_off(dwc->usb3_generic_phy);
+	for (i = 0; i < dwc->num_usb3_phy; i++)
+		phy_power_off(dwc->usb3_generic_phy[i]);
 
 err3:
-	phy_power_off(dwc->usb2_generic_phy);
+	for (i = 0; i < dwc->num_usb2_phy; i++)
+		phy_power_off(dwc->usb2_generic_phy[i]);
 
 err2:
-	usb_phy_set_suspend(dwc->usb2_phy, 1);
-	usb_phy_set_suspend(dwc->usb3_phy, 1);
+	for (i = 0; i < dwc->num_usb2_phy; i++)
+		usb_phy_set_suspend(dwc->usb2_phy[i], 1);
+	for (i = 0; i < dwc->num_usb3_phy; i++)
+		usb_phy_set_suspend(dwc->usb3_phy[i], 1);
 
 err1:
-	usb_phy_shutdown(dwc->usb2_phy);
-	usb_phy_shutdown(dwc->usb3_phy);
-	phy_exit(dwc->usb2_generic_phy);
-	phy_exit(dwc->usb3_generic_phy);
+	for (i = 0; i < dwc->num_usb2_phy; i++) {
+		usb_phy_shutdown(dwc->usb2_phy[i]);
+		phy_exit(dwc->usb2_generic_phy[i]);
+	}
+
+	for (i = 0; i < dwc->num_usb3_phy; i++) {
+		usb_phy_shutdown(dwc->usb3_phy[i]);
+		phy_exit(dwc->usb3_generic_phy[i]);
+	}
 
 err0a:
 	dwc3_ulpi_exit(dwc);
@@ -1251,53 +1289,172 @@ static int dwc3_core_init(struct dwc3 *dwc)
 	return ret;
 }
 
-static int dwc3_core_get_phy(struct dwc3 *dwc)
+static struct usb_phy *dwc3_core_get_phy_by_handle_with_node(struct device *dev,
+	const char *phandle, u8 index, struct device_node *lookup_node)
+{
+	struct device_node *node;
+	struct usb_phy	*phy;
+
+	node = of_parse_phandle(lookup_node, phandle, index);
+	if (!node) {
+		dev_err(dev, "failed to get %s phandle in %pOF node\n", phandle,
+			dev->of_node);
+		return ERR_PTR(-ENODEV);
+	}
+	phy = devm_usb_get_phy_by_node(dev, node, NULL);
+	of_node_put(node);
+	return phy;
+}
+
+static int dwc3_count_phys(struct dwc3 *dwc, struct device_node *lookup_node)
+{
+	int count;
+
+	count = of_count_phandle_with_args(lookup_node, "phys", NULL);
+
+	if (count == -ENOENT)
+		count = of_count_phandle_with_args(lookup_node, "usb-phy", NULL);
+
+	if (count == 1) {
+		dwc->num_usb2_phy++;
+	} else if (count == 2) {
+		dwc->num_usb2_phy++;
+		dwc->num_usb3_phy++;
+	} else {
+		return count;
+	}
+	return 0;
+}
+
+static int dwc3_extract_num_phys(struct dwc3 *dwc)
+{
+	struct device_node	*ports, *port;
+	int			ret;
+
+	/* Find if any "multiport" child is present inside DWC3*/
+	for_each_available_child_of_node(dwc->dev->of_node, ports) {
+		if (!strcmp(ports->name, "multiport"))
+			break;
+	}
+	if (!ports) {
+		dwc->num_usb2_phy = 1;
+		dwc->num_usb3_phy = 1;
+	} else {
+		for_each_available_child_of_node(ports, port) {
+			ret  = dwc3_count_phys(dwc, port);
+			if (ret)
+				return ret;
+		}
+	}
+	dev_info(dwc->dev, "Num of HS and SS PHY are %u %u\n", dwc->num_usb2_phy,
+									dwc->num_usb3_phy);
+
+	dwc->usb2_phy = devm_kzalloc(dwc->dev,
+		sizeof(*dwc->usb2_phy) * dwc->num_usb2_phy, GFP_KERNEL);
+	if (!dwc->usb2_phy)
+		return -ENOMEM;
+
+	dwc->usb3_phy = devm_kzalloc(dwc->dev,
+		sizeof(*dwc->usb3_phy) * dwc->num_usb3_phy, GFP_KERNEL);
+	if (!dwc->usb3_phy)
+		return -ENOMEM;
+
+	dwc->usb2_generic_phy = devm_kzalloc(dwc->dev,
+		sizeof(*dwc->usb2_generic_phy) * dwc->num_usb2_phy, GFP_KERNEL);
+	if (!dwc->usb2_generic_phy)
+		return -ENOMEM;
+
+	dwc->usb3_generic_phy = devm_kzalloc(dwc->dev,
+		sizeof(*dwc->usb3_generic_phy) * dwc->num_usb3_phy, GFP_KERNEL);
+	if (!dwc->usb3_generic_phy)
+		return -ENOMEM;
+
+	return 0;
+}
+
+static int dwc3_core_get_phy_by_node(struct dwc3 *dwc,
+		struct device_node *lookup_node, int i)
 {
 	struct device		*dev = dwc->dev;
-	struct device_node	*node = dev->of_node;
-	int ret;
+	int			ret;
 
-	if (node) {
-		dwc->usb2_phy = devm_usb_get_phy_by_phandle(dev, "usb-phy", 0);
-		dwc->usb3_phy = devm_usb_get_phy_by_phandle(dev, "usb-phy", 1);
+	if (lookup_node) {
+		dwc->usb2_phy[i] = devm_of_usb_get_phy_by_phandle(dev,
+								"usb-phy", 0, lookup_node);
+		dwc->usb3_phy[i] = devm_of_usb_get_phy_by_phandle(dev,
+								"usb-phy", 1, lookup_node);
 	} else {
-		dwc->usb2_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
-		dwc->usb3_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3);
+		dwc->usb2_phy[i] = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
+		dwc->usb3_phy[i] = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3);
 	}
 
-	if (IS_ERR(dwc->usb2_phy)) {
-		ret = PTR_ERR(dwc->usb2_phy);
+	if (IS_ERR(dwc->usb2_phy[i])) {
+		ret = PTR_ERR(dwc->usb2_phy[i]);
 		if (ret == -ENXIO || ret == -ENODEV)
-			dwc->usb2_phy = NULL;
+			dwc->usb2_phy[i] = NULL;
 		else
 			return dev_err_probe(dev, ret, "no usb2 phy configured\n");
 	}
 
-	if (IS_ERR(dwc->usb3_phy)) {
-		ret = PTR_ERR(dwc->usb3_phy);
+	if (IS_ERR(dwc->usb3_phy[i])) {
+		ret = PTR_ERR(dwc->usb3_phy[i]);
 		if (ret == -ENXIO || ret == -ENODEV)
-			dwc->usb3_phy = NULL;
+			dwc->usb3_phy[i] = NULL;
 		else
 			return dev_err_probe(dev, ret, "no usb3 phy configured\n");
 	}
 
-	dwc->usb2_generic_phy = devm_phy_get(dev, "usb2-phy");
-	if (IS_ERR(dwc->usb2_generic_phy)) {
-		ret = PTR_ERR(dwc->usb2_generic_phy);
-		if (ret == -ENOSYS || ret == -ENODEV)
-			dwc->usb2_generic_phy = NULL;
+	dwc->usb2_generic_phy[i] = devm_of_phy_get(dev, lookup_node, "usb2-phy");
+	if (IS_ERR(dwc->usb2_generic_phy[i])) {
+		ret = PTR_ERR(dwc->usb2_generic_phy[i]);
+		if (ret == -ENODEV)
+			dwc->usb2_generic_phy[i] = NULL;
 		else
 			return dev_err_probe(dev, ret, "no usb2 phy configured\n");
 	}
 
-	dwc->usb3_generic_phy = devm_phy_get(dev, "usb3-phy");
-	if (IS_ERR(dwc->usb3_generic_phy)) {
-		ret = PTR_ERR(dwc->usb3_generic_phy);
-		if (ret == -ENOSYS || ret == -ENODEV)
-			dwc->usb3_generic_phy = NULL;
+	dwc->usb3_generic_phy[i] = devm_of_phy_get(dev, lookup_node, "usb3-phy");
+	if (IS_ERR(dwc->usb3_generic_phy[i])) {
+		ret = PTR_ERR(dwc->usb3_generic_phy[i]);
+		if (ret == -ENODEV)
+			dwc->usb3_generic_phy[i] = NULL;
 		else
 			return dev_err_probe(dev, ret, "no usb3 phy configured\n");
 	}
+	return 0;
+}
+
+static int dwc3_core_get_phy(struct dwc3 *dwc)
+{
+	struct device		*dev = dwc->dev;
+	struct device_node	*node = dev->of_node;
+	struct device_node	*ports, *port;
+	int ret, i = 0;
+
+	ret = dwc3_extract_num_phys(dwc);
+	if (ret) {
+		dev_err(dwc->dev, "Unable to extract number of PHYs\n");
+		return ret;
+	}
+
+	/* Find if any "multiport" child is present inside DWC3*/
+	for_each_available_child_of_node(node, ports) {
+		if (!strcmp(ports->name, "multiport"))
+			break;
+	}
+
+	if (!ports) {
+		ret = dwc3_core_get_phy_by_node(dwc, node, 0);
+		if (ret)
+			return ret;
+	} else {
+		for_each_available_child_of_node(ports, port) {
+			ret = dwc3_core_get_phy_by_node(dwc, port, i);
+			if (ret)
+				return ret;
+			i++;
+		}
+	}
 
 	return 0;
 }
@@ -1305,16 +1462,16 @@ static int dwc3_core_get_phy(struct dwc3 *dwc)
 static int dwc3_core_init_mode(struct dwc3 *dwc)
 {
 	struct device *dev = dwc->dev;
-	int ret;
+	int i, ret;
 
 	switch (dwc->dr_mode) {
 	case USB_DR_MODE_PERIPHERAL:
 		dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_DEVICE);
 
-		if (dwc->usb2_phy)
-			otg_set_vbus(dwc->usb2_phy->otg, false);
-		phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_DEVICE);
-		phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_DEVICE);
+		if (dwc->usb2_phy[0])
+			otg_set_vbus(dwc->usb2_phy[0]->otg, false);
+		phy_set_mode(dwc->usb2_generic_phy[0], PHY_MODE_USB_DEVICE);
+		phy_set_mode(dwc->usb3_generic_phy[0], PHY_MODE_USB_DEVICE);
 
 		ret = dwc3_gadget_init(dwc);
 		if (ret)
@@ -1323,10 +1480,15 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
 	case USB_DR_MODE_HOST:
 		dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_HOST);
 
-		if (dwc->usb2_phy)
-			otg_set_vbus(dwc->usb2_phy->otg, true);
-		phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST);
-		phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_HOST);
+		for (i = 0; i < dwc->num_usb3_phy; i++) {
+			if (dwc->usb2_phy[i])
+				otg_set_vbus(dwc->usb2_phy[i]->otg, true);
+			phy_set_mode(dwc->usb2_generic_phy[i], PHY_MODE_USB_HOST);
+		}
+
+
+		for (i = 0; i < dwc->num_usb3_phy; i++)
+			phy_set_mode(dwc->usb3_generic_phy[i], PHY_MODE_USB_HOST);
 
 		ret = dwc3_host_init(dwc);
 		if (ret)
@@ -1674,7 +1836,7 @@ static int dwc3_probe(struct platform_device *pdev)
 	struct resource		*res, dwc_res;
 	struct dwc3		*dwc;
 
-	int			ret;
+	int			ret, i;
 
 	void __iomem		*regs;
 
@@ -1839,15 +2001,18 @@ static int dwc3_probe(struct platform_device *pdev)
 	dwc3_debugfs_exit(dwc);
 	dwc3_event_buffers_cleanup(dwc);
 
-	usb_phy_shutdown(dwc->usb2_phy);
-	usb_phy_shutdown(dwc->usb3_phy);
-	phy_exit(dwc->usb2_generic_phy);
-	phy_exit(dwc->usb3_generic_phy);
-
-	usb_phy_set_suspend(dwc->usb2_phy, 1);
-	usb_phy_set_suspend(dwc->usb3_phy, 1);
-	phy_power_off(dwc->usb2_generic_phy);
-	phy_power_off(dwc->usb3_generic_phy);
+	for (i = 0; i < dwc->num_usb2_phy; i++) {
+		usb_phy_shutdown(dwc->usb2_phy[i]);
+		usb_phy_set_suspend(dwc->usb2_phy[i], 1);
+		phy_exit(dwc->usb2_generic_phy[i]);
+		phy_power_off(dwc->usb2_generic_phy[i]);
+	}
+	for (i = 0; i < dwc->num_usb3_phy; i++) {
+		usb_phy_shutdown(dwc->usb3_phy[i]);
+		usb_phy_set_suspend(dwc->usb3_phy[i], 1);
+		phy_exit(dwc->usb3_generic_phy[i]);
+		phy_power_off(dwc->usb3_generic_phy[i]);
+	}
 
 	dwc3_ulpi_exit(dwc);
 
@@ -1929,6 +2094,7 @@ static int dwc3_core_init_for_resume(struct dwc3 *dwc)
 
 static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
 {
+	int i;
 	unsigned long	flags;
 	u32 reg;
 
@@ -1951,17 +2117,21 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
 		/* Let controller to suspend HSPHY before PHY driver suspends */
 		if (dwc->dis_u2_susphy_quirk ||
 		    dwc->dis_enblslpm_quirk) {
-			reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
-			reg |=  DWC3_GUSB2PHYCFG_ENBLSLPM |
-				DWC3_GUSB2PHYCFG_SUSPHY;
-			dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
-
-			/* Give some time for USB2 PHY to suspend */
-			usleep_range(5000, 6000);
+			for (i = 0; i < dwc->num_usb2_phy; i++) {
+				reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(i));
+				reg |=  DWC3_GUSB2PHYCFG_ENBLSLPM |
+					DWC3_GUSB2PHYCFG_SUSPHY;
+				dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
+
+				/* Give some time for USB2 PHY to suspend */
+				usleep_range(5000, 6000);
+			}
 		}
 
-		phy_pm_runtime_put_sync(dwc->usb2_generic_phy);
-		phy_pm_runtime_put_sync(dwc->usb3_generic_phy);
+		for (i = 0; i < dwc->num_usb2_phy; i++)
+			phy_pm_runtime_put_sync(dwc->usb2_generic_phy[i]);
+		for (i = 0; i < dwc->num_usb3_phy; i++)
+			phy_pm_runtime_put_sync(dwc->usb3_generic_phy[i]);
 		break;
 	case DWC3_GCTL_PRTCAP_OTG:
 		/* do nothing during runtime_suspend */
@@ -1989,7 +2159,7 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
 static int dwc3_resume_common(struct dwc3 *dwc, pm_message_t msg)
 {
 	unsigned long	flags;
-	int		ret;
+	int		i, ret;
 	u32		reg;
 
 	switch (dwc->current_dr_role) {
@@ -2012,17 +2182,21 @@ static int dwc3_resume_common(struct dwc3 *dwc, pm_message_t msg)
 			break;
 		}
 		/* Restore GUSB2PHYCFG bits that were modified in suspend */
-		reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
-		if (dwc->dis_u2_susphy_quirk)
-			reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
+		for (i = 0; i < dwc->num_usb2_phy; i++) {
+			reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(i));
+			if (dwc->dis_u2_susphy_quirk)
+				reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
 
-		if (dwc->dis_enblslpm_quirk)
-			reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;
+			if (dwc->dis_enblslpm_quirk)
+				reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;
 
-		dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
+			dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
+		}
 
-		phy_pm_runtime_get_sync(dwc->usb2_generic_phy);
-		phy_pm_runtime_get_sync(dwc->usb3_generic_phy);
+		for (i = 0; i < dwc->num_usb2_phy; i++)
+			phy_pm_runtime_get_sync(dwc->usb2_generic_phy[i]);
+		for (i = 0; i < dwc->num_usb3_phy; i++)
+			phy_pm_runtime_get_sync(dwc->usb3_generic_phy[i]);
 		break;
 	case DWC3_GCTL_PRTCAP_OTG:
 		/* nothing to do on runtime_resume */
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 81c486b..c169bf1 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -1020,6 +1020,8 @@ struct dwc3_scratchpad_array {
  * @usb_psy: pointer to power supply interface.
  * @usb2_phy: pointer to USB2 PHY
  * @usb3_phy: pointer to USB3 PHY
+ * @num_usb2_phy: Number of HS ports controlled by the core
+ * @num_dsphy: Number of SS ports controlled by the core
  * @usb2_generic_phy: pointer to USB2 PHY
  * @usb3_generic_phy: pointer to USB3 PHY
  * @phys_ready: flag to indicate that PHYs are ready
@@ -1147,11 +1149,13 @@ struct dwc3 {
 
 	struct reset_control	*reset;
 
-	struct usb_phy		*usb2_phy;
-	struct usb_phy		*usb3_phy;
+	struct usb_phy		**usb2_phy;
+	struct usb_phy		**usb3_phy;
+	u32			num_usb2_phy;
+	u32			num_usb3_phy;
 
-	struct phy		*usb2_generic_phy;
-	struct phy		*usb3_generic_phy;
+	struct phy		**usb2_generic_phy;
+	struct phy		**usb3_generic_phy;
 
 	bool			phys_ready;
 
diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
index 039bf24..404643f 100644
--- a/drivers/usb/dwc3/drd.c
+++ b/drivers/usb/dwc3/drd.c
@@ -384,10 +384,10 @@ void dwc3_otg_update(struct dwc3 *dwc, bool ignore_idstatus)
 		if (ret) {
 			dev_err(dwc->dev, "failed to initialize host\n");
 		} else {
-			if (dwc->usb2_phy)
-				otg_set_vbus(dwc->usb2_phy->otg, true);
-			if (dwc->usb2_generic_phy)
-				phy_set_mode(dwc->usb2_generic_phy,
+			if (dwc->usb2_phy[0])
+				otg_set_vbus(dwc->usb2_phy[0]->otg, true);
+			if (dwc->usb2_generic_phy[0])
+				phy_set_mode(dwc->usb2_generic_phy[0],
 					     PHY_MODE_USB_HOST);
 		}
 		break;
@@ -398,10 +398,10 @@ void dwc3_otg_update(struct dwc3 *dwc, bool ignore_idstatus)
 		dwc3_event_buffers_setup(dwc);
 		spin_unlock_irqrestore(&dwc->lock, flags);
 
-		if (dwc->usb2_phy)
-			otg_set_vbus(dwc->usb2_phy->otg, false);
-		if (dwc->usb2_generic_phy)
-			phy_set_mode(dwc->usb2_generic_phy,
+		if (dwc->usb2_phy[0])
+			otg_set_vbus(dwc->usb2_phy[0]->otg, false);
+		if (dwc->usb2_generic_phy[0])
+			phy_set_mode(dwc->usb2_generic_phy[0],
 				     PHY_MODE_USB_DEVICE);
 		ret = dwc3_gadget_init(dwc);
 		if (ret)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 00427d1..e3b2a17 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2872,8 +2872,8 @@ static int dwc3_gadget_vbus_draw(struct usb_gadget *g, unsigned int mA)
 	union power_supply_propval	val = {0};
 	int				ret;
 
-	if (dwc->usb2_phy)
-		return usb_phy_set_power(dwc->usb2_phy, mA);
+	if (dwc->usb2_phy[0])
+		return usb_phy_set_power(dwc->usb2_phy[0], mA);
 
 	if (!dwc->usb_psy)
 		return -EOPNOTSUPP;
-- 
2.7.4

Re: [PATCH 2/3] usb: phy: Add devm_of_usb_get_phy_by_phandle

[kernel test robot]

Hi Harsh,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on usb/usb-testing]
[also build test WARNING on robh/for-next v5.18 next-20220531]
[cannot apply to balbi-usb/testing/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Harsh-Agarwal/Add-support-for-multiport-controller/20220531-162337
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: csky-defconfig (https://download.01.org/0day-ci/archive/20220531/202205311838.eFoNSZzZ-lkp@intel.com/config)
compiler: csky-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/7af1d0e696ec2ddf574df10e3bf3799299b8061a
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Harsh-Agarwal/Add-support-for-multiport-controller/20220531-162337
        git checkout 7af1d0e696ec2ddf574df10e3bf3799299b8061a
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=csky SHELL=/bin/bash drivers/usb/phy/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from include/linux/usb/otg.h:14,
                    from include/linux/usb/of.h:12,
                    from drivers/usb/phy/of.c:9:
>> include/linux/usb/phy.h:257:16: warning: 'ERR_PTR' is static but used in inline function 'devm_of_usb_get_phy_by_phandle' which is not static
     257 |         return ERR_PTR(-ENXIO);
         |                ^~~~~~~


vim +257 include/linux/usb/phy.h

   253	
   254	extern inline struct usb_phy *devm_of_usb_get_phy_by_phandle(struct device *dev,
   255		const char *phandle, u8 index, struct device_node *lookup_node)
   256	{
 > 257		return ERR_PTR(-ENXIO);
   258	}
   259	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH 3/3] usb: dwc3: Refactor PHY logic to support Multiport Controller

[Pavan Kondeti]

Hi Harsh,

On Tue, May 31, 2022 at 01:50:17PM +0530, Harsh Agarwal wrote:
> Currently the DWC3 driver supports only single port controller
> which requires at most 2 PHYs ie HS and SS PHYs.
> 
> But some SOCs have a "multiport" USB DWC3 controller where a
> single controller supports multiple ports and each port have
> their own PHYs. Refactor PHY logic to support the same.
> 
> Signed-off-by: Harsh Agarwal <quic_harshq@quicinc.com>
> ---
>  drivers/usb/dwc3/core.c   | 400 +++++++++++++++++++++++++++++++++-------------
>  drivers/usb/dwc3/core.h   |  12 +-
>  drivers/usb/dwc3/drd.c    |  16 +-
>  drivers/usb/dwc3/gadget.c |   4 +-
>  4 files changed, 305 insertions(+), 127 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 5734219..5cc799e 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -120,7 +120,7 @@ static void __dwc3_set_mode(struct work_struct *work)
>  {
>  	struct dwc3 *dwc = work_to_dwc(work);
>  	unsigned long flags;
> -	int ret;
> +	int i, ret;
>  	u32 reg;
>  

<snip>

> -static int dwc3_core_get_phy(struct dwc3 *dwc)
> +static struct usb_phy *dwc3_core_get_phy_by_handle_with_node(struct device *dev,
> +	const char *phandle, u8 index, struct device_node *lookup_node)
> +{
> +	struct device_node *node;
> +	struct usb_phy	*phy;
> +
> +	node = of_parse_phandle(lookup_node, phandle, index);
> +	if (!node) {
> +		dev_err(dev, "failed to get %s phandle in %pOF node\n", phandle,
> +			dev->of_node);
> +		return ERR_PTR(-ENODEV);
> +	}
> +	phy = devm_usb_get_phy_by_node(dev, node, NULL);
> +	of_node_put(node);
> +	return phy;
> +}

Remove this function definition, since we moved this to PHY library.

> +
> +static int dwc3_count_phys(struct dwc3 *dwc, struct device_node *lookup_node)
> +{
> +	int count;
> +
> +	count = of_count_phandle_with_args(lookup_node, "phys", NULL);
> +
> +	if (count == -ENOENT)
> +		count = of_count_phandle_with_args(lookup_node, "usb-phy", NULL);
> +
> +	if (count == 1) {
> +		dwc->num_usb2_phy++;
> +	} else if (count == 2) {
> +		dwc->num_usb2_phy++;
> +		dwc->num_usb3_phy++;
> +	} else {
> +		return count;
> +	}
> +	return 0;
> +}
> +
> +static int dwc3_extract_num_phys(struct dwc3 *dwc)
> +{
> +	struct device_node	*ports, *port;
> +	int			ret;
> +
> +	/* Find if any "multiport" child is present inside DWC3*/
> +	for_each_available_child_of_node(dwc->dev->of_node, ports) {
> +		if (!strcmp(ports->name, "multiport"))
> +			break;
> +	}
> +	if (!ports) {
> +		dwc->num_usb2_phy = 1;
> +		dwc->num_usb3_phy = 1;
> +	} else {
> +		for_each_available_child_of_node(ports, port) {
> +			ret  = dwc3_count_phys(dwc, port);
> +			if (ret)
> +				return ret;

If you break the loop, you have to call of_node_put() explicitly.

> +		}
> +	}
> +	dev_info(dwc->dev, "Num of HS and SS PHY are %u %u\n", dwc->num_usb2_phy,
> +									dwc->num_usb3_phy);

If I declare the multiport node don't specify any phys for any of the ports,
we end up coming here with dwc->num_usb2_phy = dwc->num_usb3_phy = 0. That is
a problem since we access dwc->usb2_phy[0] directly. Can we bail out in that
case?

> +
> +	dwc->usb2_phy = devm_kzalloc(dwc->dev,
> +		sizeof(*dwc->usb2_phy) * dwc->num_usb2_phy, GFP_KERNEL);
> +	if (!dwc->usb2_phy)
> +		return -ENOMEM;
> +
> +	dwc->usb3_phy = devm_kzalloc(dwc->dev,
> +		sizeof(*dwc->usb3_phy) * dwc->num_usb3_phy, GFP_KERNEL);
> +	if (!dwc->usb3_phy)
> +		return -ENOMEM;
> +
> +	dwc->usb2_generic_phy = devm_kzalloc(dwc->dev,
> +		sizeof(*dwc->usb2_generic_phy) * dwc->num_usb2_phy, GFP_KERNEL);
> +	if (!dwc->usb2_generic_phy)
> +		return -ENOMEM;
> +
> +	dwc->usb3_generic_phy = devm_kzalloc(dwc->dev,
> +		sizeof(*dwc->usb3_generic_phy) * dwc->num_usb3_phy, GFP_KERNEL);
> +	if (!dwc->usb3_generic_phy)
> +		return -ENOMEM;
> +
> +	return 0;
> +}
> +
> +static int dwc3_core_get_phy_by_node(struct dwc3 *dwc,
> +		struct device_node *lookup_node, int i)
>  {
>  	struct device		*dev = dwc->dev;
> -	struct device_node	*node = dev->of_node;
> -	int ret;
> +	int			ret;
>  
> -	if (node) {
> -		dwc->usb2_phy = devm_usb_get_phy_by_phandle(dev, "usb-phy", 0);
> -		dwc->usb3_phy = devm_usb_get_phy_by_phandle(dev, "usb-phy", 1);
> +	if (lookup_node) {
> +		dwc->usb2_phy[i] = devm_of_usb_get_phy_by_phandle(dev,
> +								"usb-phy", 0, lookup_node);
> +		dwc->usb3_phy[i] = devm_of_usb_get_phy_by_phandle(dev,
> +								"usb-phy", 1, lookup_node);
>  	} else {
> -		dwc->usb2_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
> -		dwc->usb3_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3);
> +		dwc->usb2_phy[i] = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
> +		dwc->usb3_phy[i] = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3);
>  	}
>  
> -	if (IS_ERR(dwc->usb2_phy)) {
> -		ret = PTR_ERR(dwc->usb2_phy);
> +	if (IS_ERR(dwc->usb2_phy[i])) {
> +		ret = PTR_ERR(dwc->usb2_phy[i]);
>  		if (ret == -ENXIO || ret == -ENODEV)
> -			dwc->usb2_phy = NULL;
> +			dwc->usb2_phy[i] = NULL;
>  		else
>  			return dev_err_probe(dev, ret, "no usb2 phy configured\n");
>  	}
>  
> -	if (IS_ERR(dwc->usb3_phy)) {
> -		ret = PTR_ERR(dwc->usb3_phy);
> +	if (IS_ERR(dwc->usb3_phy[i])) {
> +		ret = PTR_ERR(dwc->usb3_phy[i]);
>  		if (ret == -ENXIO || ret == -ENODEV)
> -			dwc->usb3_phy = NULL;
> +			dwc->usb3_phy[i] = NULL;
>  		else
>  			return dev_err_probe(dev, ret, "no usb3 phy configured\n");
>  	}
>  
> -	dwc->usb2_generic_phy = devm_phy_get(dev, "usb2-phy");
> -	if (IS_ERR(dwc->usb2_generic_phy)) {
> -		ret = PTR_ERR(dwc->usb2_generic_phy);
> -		if (ret == -ENOSYS || ret == -ENODEV)
> -			dwc->usb2_generic_phy = NULL;
> +	dwc->usb2_generic_phy[i] = devm_of_phy_get(dev, lookup_node, "usb2-phy");
> +	if (IS_ERR(dwc->usb2_generic_phy[i])) {
> +		ret = PTR_ERR(dwc->usb2_generic_phy[i]);
> +		if (ret == -ENODEV)
> +			dwc->usb2_generic_phy[i] = NULL;
>  		else
>  			return dev_err_probe(dev, ret, "no usb2 phy configured\n");
>  	}
>  
> -	dwc->usb3_generic_phy = devm_phy_get(dev, "usb3-phy");
> -	if (IS_ERR(dwc->usb3_generic_phy)) {
> -		ret = PTR_ERR(dwc->usb3_generic_phy);
> -		if (ret == -ENOSYS || ret == -ENODEV)
> -			dwc->usb3_generic_phy = NULL;
> +	dwc->usb3_generic_phy[i] = devm_of_phy_get(dev, lookup_node, "usb3-phy");
> +	if (IS_ERR(dwc->usb3_generic_phy[i])) {
> +		ret = PTR_ERR(dwc->usb3_generic_phy[i]);
> +		if (ret == -ENODEV)
> +			dwc->usb3_generic_phy[i] = NULL;
>  		else
>  			return dev_err_probe(dev, ret, "no usb3 phy configured\n");
>  	}
> +	return 0;
> +}
> +
> +static int dwc3_core_get_phy(struct dwc3 *dwc)
> +{
> +	struct device		*dev = dwc->dev;
> +	struct device_node	*node = dev->of_node;
> +	struct device_node	*ports, *port;
> +	int ret, i = 0;
> +
> +	ret = dwc3_extract_num_phys(dwc);
> +	if (ret) {
> +		dev_err(dwc->dev, "Unable to extract number of PHYs\n");
> +		return ret;
> +	}
> +
> +	/* Find if any "multiport" child is present inside DWC3*/
> +	for_each_available_child_of_node(node, ports) {
> +		if (!strcmp(ports->name, "multiport"))
> +			break;
> +	}
> +
> +	if (!ports) {
> +		ret = dwc3_core_get_phy_by_node(dwc, node, 0);
> +		if (ret)
> +			return ret;
> +	} else {
> +		for_each_available_child_of_node(ports, port) {
> +			ret = dwc3_core_get_phy_by_node(dwc, port, i);
> +			if (ret)
> +				return ret;
> +			i++;
> +		}
> +	}
>  
How does this work actually for a case where I have 4 ports with hs_phy=4 and
ss_phy=2. Here we end up calling i = {0, 1, 2, 3} and accessing un-allocated
ss phy instances in dwc3_core_get_phy_by_node(). you should also handle the
case where first two ports are HS only and last two ports are both HS and SS.

Thanks,
Pavan

Re: [PATCH 1/3] dt-bindings: usb: dwc3: Add support for multiport related properties

[Rob Herring]

On Tue, 31 May 2022 13:50:15 +0530, Harsh Agarwal wrote:
> Added support for multiport, mport, num-ssphy and num-hsphy
> properties. These properties are used to support devices having
> a multiport controller.
> 
> Signed-off-by: Harsh Agarwal <quic_harshq@quicinc.com>
> ---
>  .../devicetree/bindings/usb/snps,dwc3.yaml         | 55 ++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/usb/snps,dwc3.yaml:366:8: [warning] wrong indentation: expected 6 but found 7 (indentation)
./Documentation/devicetree/bindings/usb/snps,dwc3.yaml:367:10: [warning] wrong indentation: expected 11 but found 9 (indentation)
./Documentation/devicetree/bindings/usb/snps,dwc3.yaml:369:11: [warning] wrong indentation: expected 11 but found 10 (indentation)

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/usb/snps,dwc3.example.dts:86.27-89.15: Warning (unit_address_vs_reg): /example-2/usb@4a000000/multiport/mport@1: node has a unit name, but no reg or ranges property
Documentation/devicetree/bindings/usb/snps,dwc3.example.dts:91.27-93.15: Warning (unit_address_vs_reg): /example-2/usb@4a000000/multiport/mport@2: node has a unit name, but no reg or ranges property
Documentation/devicetree/bindings/usb/snps,dwc3.example.dts:95.27-97.15: Warning (unit_address_vs_reg): /example-2/usb@4a000000/multiport/mport@3: node has a unit name, but no reg or ranges property
Documentation/devicetree/bindings/usb/snps,dwc3.example.dts:99.27-101.15: Warning (unit_address_vs_reg): /example-2/usb@4a000000/multiport/mport@4: node has a unit name, but no reg or ranges property
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/snps,dwc3.example.dtb: usb@4a000000: multiport: 'mport' is a required property
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/snps,dwc3.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH 3/3] usb: dwc3: Refactor PHY logic to support Multiport Controller

[Andrew Halaney]

On Tue, May 31, 2022 at 01:50:17PM +0530, Harsh Agarwal wrote:
> Currently the DWC3 driver supports only single port controller
> which requires at most 2 PHYs ie HS and SS PHYs.
> 
> But some SOCs have a "multiport" USB DWC3 controller where a
> single controller supports multiple ports and each port have
> their own PHYs. Refactor PHY logic to support the same.
> 
> Signed-off-by: Harsh Agarwal <quic_harshq@quicinc.com>
> ---
>  drivers/usb/dwc3/core.c   | 400 +++++++++++++++++++++++++++++++++-------------
>  drivers/usb/dwc3/core.h   |  12 +-
>  drivers/usb/dwc3/drd.c    |  16 +-
>  drivers/usb/dwc3/gadget.c |   4 +-
>  4 files changed, 305 insertions(+), 127 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 5734219..5cc799e 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -120,7 +120,7 @@ static void __dwc3_set_mode(struct work_struct *work)
>  {
>  	struct dwc3 *dwc = work_to_dwc(work);
>  	unsigned long flags;
> -	int ret;
> +	int i, ret;
>  	u32 reg;
>  
>  	mutex_lock(&dwc->mutex);
> @@ -189,10 +189,13 @@ static void __dwc3_set_mode(struct work_struct *work)
>  		if (ret) {
>  			dev_err(dwc->dev, "failed to initialize host\n");
>  		} else {
> -			if (dwc->usb2_phy)
> -				otg_set_vbus(dwc->usb2_phy->otg, true);
> -			phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST);
> -			phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_HOST);
> +			for (i = 0; i < dwc->num_usb2_phy; i++) {
> +				if (dwc->usb2_phy[i])
> +					otg_set_vbus(dwc->usb2_phy[i]->otg, true);
> +				phy_set_mode(dwc->usb2_generic_phy[i], PHY_MODE_USB_HOST);
> +			}
> +			for (i = 0; i < dwc->num_usb3_phy; i++)
> +				phy_set_mode(dwc->usb3_generic_phy[i], PHY_MODE_USB_HOST);
>  			if (dwc->dis_split_quirk) {
>  				reg = dwc3_readl(dwc->regs, DWC3_GUCTL3);
>  				reg |= DWC3_GUCTL3_SPLITDISABLE;
> @@ -205,10 +208,10 @@ static void __dwc3_set_mode(struct work_struct *work)
>  
>  		dwc3_event_buffers_setup(dwc);
>  
> -		if (dwc->usb2_phy)
> -			otg_set_vbus(dwc->usb2_phy->otg, false);
> -		phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_DEVICE);
> -		phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_DEVICE);
> +		if (dwc->usb2_phy[0])
> +			otg_set_vbus(dwc->usb2_phy[0]->otg, false);
> +		phy_set_mode(dwc->usb2_generic_phy[0], PHY_MODE_USB_DEVICE);
> +		phy_set_mode(dwc->usb3_generic_phy[0], PHY_MODE_USB_DEVICE);
>  
>  		ret = dwc3_gadget_init(dwc);
>  		if (ret)
> @@ -656,6 +659,7 @@ static int dwc3_core_ulpi_init(struct dwc3 *dwc)
>   */
>  static int dwc3_phy_setup(struct dwc3 *dwc)
>  {
> +	int i;
>  	unsigned int hw_mode;
>  	u32 reg;
>  
> @@ -716,7 +720,8 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
>  	if (dwc->dis_del_phy_power_chg_quirk)
>  		reg &= ~DWC3_GUSB3PIPECTL_DEPOCHANGE;
>  
> -	dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg);
> +	for (i = 0; i < dwc->num_usb3_phy; i++)
> +		dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(i), reg);
>  
>  	reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
>  
> @@ -730,7 +735,8 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
>  		} else if (dwc->hsphy_interface &&
>  				!strncmp(dwc->hsphy_interface, "ulpi", 4)) {
>  			reg |= DWC3_GUSB2PHYCFG_ULPI_UTMI;
> -			dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
> +			for (i = 0; i < dwc->num_usb2_phy; i++)
> +				dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
>  		} else {
>  			/* Relying on default value. */
>  			if (!(reg & DWC3_GUSB2PHYCFG_ULPI_UTMI))
> @@ -787,7 +793,8 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
>  	if (dwc->dis_u2_freeclk_exists_quirk)
>  		reg &= ~DWC3_GUSB2PHYCFG_U2_FREECLK_EXISTS;
>  
> -	dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
> +	for (i = 0; i < dwc->num_usb2_phy; i++)
> +		dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
>  
>  	return 0;
>  }
> @@ -826,17 +833,23 @@ static void dwc3_clk_disable(struct dwc3 *dwc)
>  
>  static void dwc3_core_exit(struct dwc3 *dwc)
>  {
> +	int i;
>  	dwc3_event_buffers_cleanup(dwc);
>  
> -	usb_phy_shutdown(dwc->usb2_phy);
> -	usb_phy_shutdown(dwc->usb3_phy);
> -	phy_exit(dwc->usb2_generic_phy);
> -	phy_exit(dwc->usb3_generic_phy);
> +	for (i = 0; i < dwc->num_usb2_phy; i++) {
> +		usb_phy_shutdown(dwc->usb2_phy[i]);
> +		usb_phy_set_suspend(dwc->usb2_phy[i], 1);
> +		phy_exit(dwc->usb2_generic_phy[i]);
> +		phy_power_off(dwc->usb2_generic_phy[i]);
> +	}
> +
> +	for (i = 0; i < dwc->num_usb3_phy; i++) {
> +		usb_phy_shutdown(dwc->usb3_phy[i]);
> +		usb_phy_set_suspend(dwc->usb3_phy[i], 1);
> +		phy_exit(dwc->usb3_generic_phy[i]);
> +		phy_power_off(dwc->usb3_generic_phy[i]);
> +	}
>  
> -	usb_phy_set_suspend(dwc->usb2_phy, 1);
> -	usb_phy_set_suspend(dwc->usb3_phy, 1);
> -	phy_power_off(dwc->usb2_generic_phy);
> -	phy_power_off(dwc->usb3_generic_phy);
>  	dwc3_clk_disable(dwc);
>  	reset_control_assert(dwc->reset);
>  }
> @@ -1039,7 +1052,7 @@ static int dwc3_core_init(struct dwc3 *dwc)
>  {
>  	unsigned int		hw_mode;
>  	u32			reg;
> -	int			ret;
> +	int			ret, i;
>  
>  	hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
>  
> @@ -1067,16 +1080,24 @@ static int dwc3_core_init(struct dwc3 *dwc)
>  		dwc->phys_ready = true;
>  	}
>  
> -	usb_phy_init(dwc->usb2_phy);
> -	usb_phy_init(dwc->usb3_phy);
> -	ret = phy_init(dwc->usb2_generic_phy);
> -	if (ret < 0)
> -		goto err0a;
> +	for (i = 0; i < dwc->num_usb2_phy; i++)
> +		usb_phy_init(dwc->usb2_phy[i]);
> +	for (i = 0; i < dwc->num_usb3_phy; i++)
> +		usb_phy_init(dwc->usb3_phy[i]);
>  
> -	ret = phy_init(dwc->usb3_generic_phy);
> -	if (ret < 0) {
> -		phy_exit(dwc->usb2_generic_phy);
> -		goto err0a;
> +	for (i = 0; i < dwc->num_usb2_phy; i++) {
> +		ret = phy_init(dwc->usb2_generic_phy[i]);
> +		if (ret < 0)
> +			goto err0a;

Should we clean up the prior phys that did not fail? There's a couple
loops where that question stands in this patch.

> +	}
> +
> +	for (i = 0; i < dwc->num_usb3_phy; i++) {
> +		ret = phy_init(dwc->usb3_generic_phy[i]);
> +		if (ret < 0) {
> +			for (i = 0; i < dwc->num_usb2_phy; i++)
> +				phy_exit(dwc->usb2_generic_phy[i]);
> +			goto err0a;
> +		}
>  	}
>  
>  	ret = dwc3_core_soft_reset(dwc);
> @@ -1086,15 +1107,19 @@ static int dwc3_core_init(struct dwc3 *dwc)
>  	if (hw_mode == DWC3_GHWPARAMS0_MODE_DRD &&
>  	    !DWC3_VER_IS_WITHIN(DWC3, ANY, 194A)) {
>  		if (!dwc->dis_u3_susphy_quirk) {
> -			reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0));
> -			reg |= DWC3_GUSB3PIPECTL_SUSPHY;
> -			dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg);
> +			for (i = 0; i < dwc->num_usb3_phy; i++) {
> +				reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(i));
> +				reg |= DWC3_GUSB3PIPECTL_SUSPHY;
> +				dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(i), reg);
> +			}
>  		}
>  
>  		if (!dwc->dis_u2_susphy_quirk) {
> -			reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
> -			reg |= DWC3_GUSB2PHYCFG_SUSPHY;
> -			dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
> +			for (i = 0; i < dwc->num_usb2_phy; i++) {
> +				reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(i));
> +				reg |= DWC3_GUSB2PHYCFG_SUSPHY;
> +				dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
> +			}
>  		}
>  	}
>  
> @@ -1113,15 +1138,19 @@ static int dwc3_core_init(struct dwc3 *dwc)
>  
>  	dwc3_set_incr_burst_type(dwc);
>  
> -	usb_phy_set_suspend(dwc->usb2_phy, 0);
> -	usb_phy_set_suspend(dwc->usb3_phy, 0);
> -	ret = phy_power_on(dwc->usb2_generic_phy);
> -	if (ret < 0)
> -		goto err2;
> +	for (i = 0; i < dwc->num_usb2_phy; i++) {
> +		usb_phy_set_suspend(dwc->usb2_phy[i], 0);
> +		ret = phy_power_on(dwc->usb2_generic_phy[i]);
> +		if (ret < 0)
> +			goto err2;
> +	}
>  
> -	ret = phy_power_on(dwc->usb3_generic_phy);
> -	if (ret < 0)
> -		goto err3;
> +	for (i = 0; i < dwc->num_usb3_phy; i++) {
> +		usb_phy_set_suspend(dwc->usb3_phy[i], 0);
> +		ret = phy_power_on(dwc->usb3_generic_phy[i]);
> +		if (ret < 0)
> +			goto err3;
> +	}
>  
>  	ret = dwc3_event_buffers_setup(dwc);
>  	if (ret) {
> @@ -1229,20 +1258,29 @@ static int dwc3_core_init(struct dwc3 *dwc)
>  	return 0;
>  
>  err4:
> -	phy_power_off(dwc->usb3_generic_phy);
> +	for (i = 0; i < dwc->num_usb3_phy; i++)
> +		phy_power_off(dwc->usb3_generic_phy[i]);
>  
>  err3:
> -	phy_power_off(dwc->usb2_generic_phy);
> +	for (i = 0; i < dwc->num_usb2_phy; i++)
> +		phy_power_off(dwc->usb2_generic_phy[i]);
>  
>  err2:
> -	usb_phy_set_suspend(dwc->usb2_phy, 1);
> -	usb_phy_set_suspend(dwc->usb3_phy, 1);
> +	for (i = 0; i < dwc->num_usb2_phy; i++)
> +		usb_phy_set_suspend(dwc->usb2_phy[i], 1);
> +	for (i = 0; i < dwc->num_usb3_phy; i++)
> +		usb_phy_set_suspend(dwc->usb3_phy[i], 1);
>  
>  err1:
> -	usb_phy_shutdown(dwc->usb2_phy);
> -	usb_phy_shutdown(dwc->usb3_phy);
> -	phy_exit(dwc->usb2_generic_phy);
> -	phy_exit(dwc->usb3_generic_phy);
> +	for (i = 0; i < dwc->num_usb2_phy; i++) {
> +		usb_phy_shutdown(dwc->usb2_phy[i]);
> +		phy_exit(dwc->usb2_generic_phy[i]);
> +	}
> +
> +	for (i = 0; i < dwc->num_usb3_phy; i++) {
> +		usb_phy_shutdown(dwc->usb3_phy[i]);
> +		phy_exit(dwc->usb3_generic_phy[i]);
> +	}
>  
>  err0a:
>  	dwc3_ulpi_exit(dwc);
> @@ -1251,53 +1289,172 @@ static int dwc3_core_init(struct dwc3 *dwc)
>  	return ret;
>  }
>  
> -static int dwc3_core_get_phy(struct dwc3 *dwc)
> +static struct usb_phy *dwc3_core_get_phy_by_handle_with_node(struct device *dev,
> +	const char *phandle, u8 index, struct device_node *lookup_node)
> +{
> +	struct device_node *node;
> +	struct usb_phy	*phy;
> +
> +	node = of_parse_phandle(lookup_node, phandle, index);
> +	if (!node) {
> +		dev_err(dev, "failed to get %s phandle in %pOF node\n", phandle,
> +			dev->of_node);
> +		return ERR_PTR(-ENODEV);
> +	}
> +	phy = devm_usb_get_phy_by_node(dev, node, NULL);
> +	of_node_put(node);
> +	return phy;
> +}
> +
> +static int dwc3_count_phys(struct dwc3 *dwc, struct device_node *lookup_node)
> +{
> +	int count;
> +
> +	count = of_count_phandle_with_args(lookup_node, "phys", NULL);
> +
> +	if (count == -ENOENT)
> +		count = of_count_phandle_with_args(lookup_node, "usb-phy", NULL);
> +
> +	if (count == 1) {
> +		dwc->num_usb2_phy++;
> +	} else if (count == 2) {
> +		dwc->num_usb2_phy++;
> +		dwc->num_usb3_phy++;
> +	} else {
> +		return count;
> +	}
> +	return 0;
> +}
> +
> +static int dwc3_extract_num_phys(struct dwc3 *dwc)
> +{
> +	struct device_node	*ports, *port;
> +	int			ret;
> +
> +	/* Find if any "multiport" child is present inside DWC3*/

Nit, space after DWC3

> +	for_each_available_child_of_node(dwc->dev->of_node, ports) {
> +		if (!strcmp(ports->name, "multiport"))
> +			break;
> +	}
> +	if (!ports) {
> +		dwc->num_usb2_phy = 1;
> +		dwc->num_usb3_phy = 1;
> +	} else {
> +		for_each_available_child_of_node(ports, port) {
> +			ret  = dwc3_count_phys(dwc, port);
> +			if (ret)
> +				return ret;
> +		}
> +	}
> +	dev_info(dwc->dev, "Num of HS and SS PHY are %u %u\n", dwc->num_usb2_phy,
> +									dwc->num_usb3_phy);
> +
> +	dwc->usb2_phy = devm_kzalloc(dwc->dev,
> +		sizeof(*dwc->usb2_phy) * dwc->num_usb2_phy, GFP_KERNEL);
> +	if (!dwc->usb2_phy)
> +		return -ENOMEM;
> +
> +	dwc->usb3_phy = devm_kzalloc(dwc->dev,
> +		sizeof(*dwc->usb3_phy) * dwc->num_usb3_phy, GFP_KERNEL);
> +	if (!dwc->usb3_phy)
> +		return -ENOMEM;
> +
> +	dwc->usb2_generic_phy = devm_kzalloc(dwc->dev,
> +		sizeof(*dwc->usb2_generic_phy) * dwc->num_usb2_phy, GFP_KERNEL);
> +	if (!dwc->usb2_generic_phy)
> +		return -ENOMEM;
> +
> +	dwc->usb3_generic_phy = devm_kzalloc(dwc->dev,
> +		sizeof(*dwc->usb3_generic_phy) * dwc->num_usb3_phy, GFP_KERNEL);
> +	if (!dwc->usb3_generic_phy)
> +		return -ENOMEM;
> +
> +	return 0;
> +}
> +
> +static int dwc3_core_get_phy_by_node(struct dwc3 *dwc,
> +		struct device_node *lookup_node, int i)
>  {
>  	struct device		*dev = dwc->dev;
> -	struct device_node	*node = dev->of_node;
> -	int ret;
> +	int			ret;
>  
> -	if (node) {
> -		dwc->usb2_phy = devm_usb_get_phy_by_phandle(dev, "usb-phy", 0);
> -		dwc->usb3_phy = devm_usb_get_phy_by_phandle(dev, "usb-phy", 1);
> +	if (lookup_node) {
> +		dwc->usb2_phy[i] = devm_of_usb_get_phy_by_phandle(dev,
> +								"usb-phy", 0, lookup_node);
> +		dwc->usb3_phy[i] = devm_of_usb_get_phy_by_phandle(dev,
> +								"usb-phy", 1, lookup_node);
>  	} else {
> -		dwc->usb2_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
> -		dwc->usb3_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3);
> +		dwc->usb2_phy[i] = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
> +		dwc->usb3_phy[i] = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3);
>  	}
>  
> -	if (IS_ERR(dwc->usb2_phy)) {
> -		ret = PTR_ERR(dwc->usb2_phy);
> +	if (IS_ERR(dwc->usb2_phy[i])) {
> +		ret = PTR_ERR(dwc->usb2_phy[i]);
>  		if (ret == -ENXIO || ret == -ENODEV)
> -			dwc->usb2_phy = NULL;
> +			dwc->usb2_phy[i] = NULL;
>  		else
>  			return dev_err_probe(dev, ret, "no usb2 phy configured\n");
>  	}
>  
> -	if (IS_ERR(dwc->usb3_phy)) {
> -		ret = PTR_ERR(dwc->usb3_phy);
> +	if (IS_ERR(dwc->usb3_phy[i])) {
> +		ret = PTR_ERR(dwc->usb3_phy[i]);
>  		if (ret == -ENXIO || ret == -ENODEV)
> -			dwc->usb3_phy = NULL;
> +			dwc->usb3_phy[i] = NULL;
>  		else
>  			return dev_err_probe(dev, ret, "no usb3 phy configured\n");
>  	}
>  
> -	dwc->usb2_generic_phy = devm_phy_get(dev, "usb2-phy");
> -	if (IS_ERR(dwc->usb2_generic_phy)) {
> -		ret = PTR_ERR(dwc->usb2_generic_phy);
> -		if (ret == -ENOSYS || ret == -ENODEV)
> -			dwc->usb2_generic_phy = NULL;
> +	dwc->usb2_generic_phy[i] = devm_of_phy_get(dev, lookup_node, "usb2-phy");
> +	if (IS_ERR(dwc->usb2_generic_phy[i])) {
> +		ret = PTR_ERR(dwc->usb2_generic_phy[i]);
> +		if (ret == -ENODEV)

I think this can be -ENOSYS if !CONFIG_GENERIC_PHY

> +			dwc->usb2_generic_phy[i] = NULL;
>  		else
>  			return dev_err_probe(dev, ret, "no usb2 phy configured\n");
>  	}
>  
> -	dwc->usb3_generic_phy = devm_phy_get(dev, "usb3-phy");
> -	if (IS_ERR(dwc->usb3_generic_phy)) {
> -		ret = PTR_ERR(dwc->usb3_generic_phy);
> -		if (ret == -ENOSYS || ret == -ENODEV)
> -			dwc->usb3_generic_phy = NULL;
> +	dwc->usb3_generic_phy[i] = devm_of_phy_get(dev, lookup_node, "usb3-phy");
> +	if (IS_ERR(dwc->usb3_generic_phy[i])) {
> +		ret = PTR_ERR(dwc->usb3_generic_phy[i]);
> +		if (ret == -ENODEV)
> +			dwc->usb3_generic_phy[i] = NULL;
>  		else
>  			return dev_err_probe(dev, ret, "no usb3 phy configured\n");
>  	}
> +	return 0;
> +}
> +
> +static int dwc3_core_get_phy(struct dwc3 *dwc)
> +{
> +	struct device		*dev = dwc->dev;
> +	struct device_node	*node = dev->of_node;
> +	struct device_node	*ports, *port;
> +	int ret, i = 0;
> +
> +	ret = dwc3_extract_num_phys(dwc);
> +	if (ret) {
> +		dev_err(dwc->dev, "Unable to extract number of PHYs\n");
> +		return ret;
> +	}
> +
> +	/* Find if any "multiport" child is present inside DWC3*/

Nit, space after DWC3

> +	for_each_available_child_of_node(node, ports) {
> +		if (!strcmp(ports->name, "multiport"))
> +			break;
> +	}
> +
> +	if (!ports) {
> +		ret = dwc3_core_get_phy_by_node(dwc, node, 0);
> +		if (ret)
> +			return ret;
> +	} else {
> +		for_each_available_child_of_node(ports, port) {
> +			ret = dwc3_core_get_phy_by_node(dwc, port, i);
> +			if (ret)
> +				return ret;
> +			i++;
> +		}
> +	}
>  
>  	return 0;
>  }
> @@ -1305,16 +1462,16 @@ static int dwc3_core_get_phy(struct dwc3 *dwc)
>  static int dwc3_core_init_mode(struct dwc3 *dwc)
>  {
>  	struct device *dev = dwc->dev;
> -	int ret;
> +	int i, ret;
>  
>  	switch (dwc->dr_mode) {
>  	case USB_DR_MODE_PERIPHERAL:
>  		dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_DEVICE);
>  
> -		if (dwc->usb2_phy)
> -			otg_set_vbus(dwc->usb2_phy->otg, false);
> -		phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_DEVICE);
> -		phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_DEVICE);
> +		if (dwc->usb2_phy[0])
> +			otg_set_vbus(dwc->usb2_phy[0]->otg, false);
> +		phy_set_mode(dwc->usb2_generic_phy[0], PHY_MODE_USB_DEVICE);
> +		phy_set_mode(dwc->usb3_generic_phy[0], PHY_MODE_USB_DEVICE);
>  
>  		ret = dwc3_gadget_init(dwc);
>  		if (ret)
> @@ -1323,10 +1480,15 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
>  	case USB_DR_MODE_HOST:
>  		dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_HOST);
>  
> -		if (dwc->usb2_phy)
> -			otg_set_vbus(dwc->usb2_phy->otg, true);
> -		phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST);
> -		phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_HOST);
> +		for (i = 0; i < dwc->num_usb3_phy; i++) {
> +			if (dwc->usb2_phy[i])
> +				otg_set_vbus(dwc->usb2_phy[i]->otg, true);
> +			phy_set_mode(dwc->usb2_generic_phy[i], PHY_MODE_USB_HOST);
> +		}
> +
> +
> +		for (i = 0; i < dwc->num_usb3_phy; i++)
> +			phy_set_mode(dwc->usb3_generic_phy[i], PHY_MODE_USB_HOST);
>  
>  		ret = dwc3_host_init(dwc);
>  		if (ret)
> @@ -1674,7 +1836,7 @@ static int dwc3_probe(struct platform_device *pdev)
>  	struct resource		*res, dwc_res;
>  	struct dwc3		*dwc;
>  
> -	int			ret;
> +	int			ret, i;
>  
>  	void __iomem		*regs;
>  
> @@ -1839,15 +2001,18 @@ static int dwc3_probe(struct platform_device *pdev)
>  	dwc3_debugfs_exit(dwc);
>  	dwc3_event_buffers_cleanup(dwc);
>  
> -	usb_phy_shutdown(dwc->usb2_phy);
> -	usb_phy_shutdown(dwc->usb3_phy);
> -	phy_exit(dwc->usb2_generic_phy);
> -	phy_exit(dwc->usb3_generic_phy);
> -
> -	usb_phy_set_suspend(dwc->usb2_phy, 1);
> -	usb_phy_set_suspend(dwc->usb3_phy, 1);
> -	phy_power_off(dwc->usb2_generic_phy);
> -	phy_power_off(dwc->usb3_generic_phy);
> +	for (i = 0; i < dwc->num_usb2_phy; i++) {
> +		usb_phy_shutdown(dwc->usb2_phy[i]);
> +		usb_phy_set_suspend(dwc->usb2_phy[i], 1);
> +		phy_exit(dwc->usb2_generic_phy[i]);
> +		phy_power_off(dwc->usb2_generic_phy[i]);
> +	}
> +	for (i = 0; i < dwc->num_usb3_phy; i++) {
> +		usb_phy_shutdown(dwc->usb3_phy[i]);
> +		usb_phy_set_suspend(dwc->usb3_phy[i], 1);
> +		phy_exit(dwc->usb3_generic_phy[i]);
> +		phy_power_off(dwc->usb3_generic_phy[i]);
> +	}
>  
>  	dwc3_ulpi_exit(dwc);
>  
> @@ -1929,6 +2094,7 @@ static int dwc3_core_init_for_resume(struct dwc3 *dwc)
>  
>  static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
>  {
> +	int i;
>  	unsigned long	flags;
>  	u32 reg;
>  
> @@ -1951,17 +2117,21 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
>  		/* Let controller to suspend HSPHY before PHY driver suspends */
>  		if (dwc->dis_u2_susphy_quirk ||
>  		    dwc->dis_enblslpm_quirk) {
> -			reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
> -			reg |=  DWC3_GUSB2PHYCFG_ENBLSLPM |
> -				DWC3_GUSB2PHYCFG_SUSPHY;
> -			dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
> -
> -			/* Give some time for USB2 PHY to suspend */
> -			usleep_range(5000, 6000);
> +			for (i = 0; i < dwc->num_usb2_phy; i++) {
> +				reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(i));
> +				reg |=  DWC3_GUSB2PHYCFG_ENBLSLPM |
> +					DWC3_GUSB2PHYCFG_SUSPHY;
> +				dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
> +
> +				/* Give some time for USB2 PHY to suspend */
> +				usleep_range(5000, 6000);
> +			}
>  		}
>  
> -		phy_pm_runtime_put_sync(dwc->usb2_generic_phy);
> -		phy_pm_runtime_put_sync(dwc->usb3_generic_phy);
> +		for (i = 0; i < dwc->num_usb2_phy; i++)
> +			phy_pm_runtime_put_sync(dwc->usb2_generic_phy[i]);
> +		for (i = 0; i < dwc->num_usb3_phy; i++)
> +			phy_pm_runtime_put_sync(dwc->usb3_generic_phy[i]);
>  		break;
>  	case DWC3_GCTL_PRTCAP_OTG:
>  		/* do nothing during runtime_suspend */
> @@ -1989,7 +2159,7 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
>  static int dwc3_resume_common(struct dwc3 *dwc, pm_message_t msg)
>  {
>  	unsigned long	flags;
> -	int		ret;
> +	int		i, ret;
>  	u32		reg;
>  
>  	switch (dwc->current_dr_role) {
> @@ -2012,17 +2182,21 @@ static int dwc3_resume_common(struct dwc3 *dwc, pm_message_t msg)
>  			break;
>  		}
>  		/* Restore GUSB2PHYCFG bits that were modified in suspend */
> -		reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
> -		if (dwc->dis_u2_susphy_quirk)
> -			reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
> +		for (i = 0; i < dwc->num_usb2_phy; i++) {
> +			reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(i));
> +			if (dwc->dis_u2_susphy_quirk)
> +				reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
>  
> -		if (dwc->dis_enblslpm_quirk)
> -			reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;
> +			if (dwc->dis_enblslpm_quirk)
> +				reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;
>  
> -		dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
> +			dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
> +		}
>  
> -		phy_pm_runtime_get_sync(dwc->usb2_generic_phy);
> -		phy_pm_runtime_get_sync(dwc->usb3_generic_phy);
> +		for (i = 0; i < dwc->num_usb2_phy; i++)
> +			phy_pm_runtime_get_sync(dwc->usb2_generic_phy[i]);
> +		for (i = 0; i < dwc->num_usb3_phy; i++)
> +			phy_pm_runtime_get_sync(dwc->usb3_generic_phy[i]);
>  		break;
>  	case DWC3_GCTL_PRTCAP_OTG:
>  		/* nothing to do on runtime_resume */
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 81c486b..c169bf1 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -1020,6 +1020,8 @@ struct dwc3_scratchpad_array {
>   * @usb_psy: pointer to power supply interface.
>   * @usb2_phy: pointer to USB2 PHY
>   * @usb3_phy: pointer to USB3 PHY
> + * @num_usb2_phy: Number of HS ports controlled by the core
> + * @num_dsphy: Number of SS ports controlled by the core

s/num_dsphy/num_usb3_phy/

>   * @usb2_generic_phy: pointer to USB2 PHY
>   * @usb3_generic_phy: pointer to USB3 PHY
>   * @phys_ready: flag to indicate that PHYs are ready
> @@ -1147,11 +1149,13 @@ struct dwc3 {
>  
>  	struct reset_control	*reset;
>  
> -	struct usb_phy		*usb2_phy;
> -	struct usb_phy		*usb3_phy;
> +	struct usb_phy		**usb2_phy;
> +	struct usb_phy		**usb3_phy;
> +	u32			num_usb2_phy;
> +	u32			num_usb3_phy;
>  
> -	struct phy		*usb2_generic_phy;
> -	struct phy		*usb3_generic_phy;
> +	struct phy		**usb2_generic_phy;
> +	struct phy		**usb3_generic_phy;
>  
>  	bool			phys_ready;
>  
> diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
> index 039bf24..404643f 100644
> --- a/drivers/usb/dwc3/drd.c
> +++ b/drivers/usb/dwc3/drd.c
> @@ -384,10 +384,10 @@ void dwc3_otg_update(struct dwc3 *dwc, bool ignore_idstatus)
>  		if (ret) {
>  			dev_err(dwc->dev, "failed to initialize host\n");
>  		} else {
> -			if (dwc->usb2_phy)
> -				otg_set_vbus(dwc->usb2_phy->otg, true);
> -			if (dwc->usb2_generic_phy)
> -				phy_set_mode(dwc->usb2_generic_phy,
> +			if (dwc->usb2_phy[0])
> +				otg_set_vbus(dwc->usb2_phy[0]->otg, true);
> +			if (dwc->usb2_generic_phy[0])
> +				phy_set_mode(dwc->usb2_generic_phy[0],
>  					     PHY_MODE_USB_HOST);
>  		}
>  		break;
> @@ -398,10 +398,10 @@ void dwc3_otg_update(struct dwc3 *dwc, bool ignore_idstatus)
>  		dwc3_event_buffers_setup(dwc);
>  		spin_unlock_irqrestore(&dwc->lock, flags);
>  
> -		if (dwc->usb2_phy)
> -			otg_set_vbus(dwc->usb2_phy->otg, false);
> -		if (dwc->usb2_generic_phy)
> -			phy_set_mode(dwc->usb2_generic_phy,
> +		if (dwc->usb2_phy[0])
> +			otg_set_vbus(dwc->usb2_phy[0]->otg, false);
> +		if (dwc->usb2_generic_phy[0])
> +			phy_set_mode(dwc->usb2_generic_phy[0],
>  				     PHY_MODE_USB_DEVICE);
>  		ret = dwc3_gadget_init(dwc);
>  		if (ret)
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 00427d1..e3b2a17 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -2872,8 +2872,8 @@ static int dwc3_gadget_vbus_draw(struct usb_gadget *g, unsigned int mA)
>  	union power_supply_propval	val = {0};
>  	int				ret;
>  
> -	if (dwc->usb2_phy)
> -		return usb_phy_set_power(dwc->usb2_phy, mA);
> +	if (dwc->usb2_phy[0])
> +		return usb_phy_set_power(dwc->usb2_phy[0], mA);
>  
>  	if (!dwc->usb_psy)
>  		return -EOPNOTSUPP;
> -- 
> 2.7.4
> 

Re: [PATCH 3/3] usb: dwc3: Refactor PHY logic to support Multiport Controller

[Harsh Agarwal]

On 6/2/2022 1:23 AM, Andrew Halaney wrote:
> On Tue, May 31, 2022 at 01:50:17PM +0530, Harsh Agarwal wrote:
>> Currently the DWC3 driver supports only single port controller
>> which requires at most 2 PHYs ie HS and SS PHYs.
>>
>> But some SOCs have a "multiport" USB DWC3 controller where a
>> single controller supports multiple ports and each port have
>> their own PHYs. Refactor PHY logic to support the same.
>>
>> Signed-off-by: Harsh Agarwal <quic_harshq@quicinc.com>
>> ---
>>   drivers/usb/dwc3/core.c   | 400 +++++++++++++++++++++++++++++++++-------------
>>   drivers/usb/dwc3/core.h   |  12 +-
>>   drivers/usb/dwc3/drd.c    |  16 +-
>>   drivers/usb/dwc3/gadget.c |   4 +-
>>   4 files changed, 305 insertions(+), 127 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 5734219..5cc799e 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -120,7 +120,7 @@ static void __dwc3_set_mode(struct work_struct *work)
>>   {
>>   	struct dwc3 *dwc = work_to_dwc(work);
>>   	unsigned long flags;
>> -	int ret;
>> +	int i, ret;
>>   	u32 reg;
>>   
>>   	mutex_lock(&dwc->mutex);
>> @@ -189,10 +189,13 @@ static void __dwc3_set_mode(struct work_struct *work)
>>   		if (ret) {
>>   			dev_err(dwc->dev, "failed to initialize host\n");
>>   		} else {
>> -			if (dwc->usb2_phy)
>> -				otg_set_vbus(dwc->usb2_phy->otg, true);
>> -			phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST);
>> -			phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_HOST);
>> +			for (i = 0; i < dwc->num_usb2_phy; i++) {
>> +				if (dwc->usb2_phy[i])
>> +					otg_set_vbus(dwc->usb2_phy[i]->otg, true);
>> +				phy_set_mode(dwc->usb2_generic_phy[i], PHY_MODE_USB_HOST);
>> +			}
>> +			for (i = 0; i < dwc->num_usb3_phy; i++)
>> +				phy_set_mode(dwc->usb3_generic_phy[i], PHY_MODE_USB_HOST);
>>   			if (dwc->dis_split_quirk) {
>>   				reg = dwc3_readl(dwc->regs, DWC3_GUCTL3);
>>   				reg |= DWC3_GUCTL3_SPLITDISABLE;
>> @@ -205,10 +208,10 @@ static void __dwc3_set_mode(struct work_struct *work)
>>   
>>   		dwc3_event_buffers_setup(dwc);
>>   
>> -		if (dwc->usb2_phy)
>> -			otg_set_vbus(dwc->usb2_phy->otg, false);
>> -		phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_DEVICE);
>> -		phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_DEVICE);
>> +		if (dwc->usb2_phy[0])
>> +			otg_set_vbus(dwc->usb2_phy[0]->otg, false);
>> +		phy_set_mode(dwc->usb2_generic_phy[0], PHY_MODE_USB_DEVICE);
>> +		phy_set_mode(dwc->usb3_generic_phy[0], PHY_MODE_USB_DEVICE);
>>   
>>   		ret = dwc3_gadget_init(dwc);
>>   		if (ret)
>> @@ -656,6 +659,7 @@ static int dwc3_core_ulpi_init(struct dwc3 *dwc)
>>    */
>>   static int dwc3_phy_setup(struct dwc3 *dwc)
>>   {
>> +	int i;
>>   	unsigned int hw_mode;
>>   	u32 reg;
>>   
>> @@ -716,7 +720,8 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
>>   	if (dwc->dis_del_phy_power_chg_quirk)
>>   		reg &= ~DWC3_GUSB3PIPECTL_DEPOCHANGE;
>>   
>> -	dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg);
>> +	for (i = 0; i < dwc->num_usb3_phy; i++)
>> +		dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(i), reg);
>>   
>>   	reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
>>   
>> @@ -730,7 +735,8 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
>>   		} else if (dwc->hsphy_interface &&
>>   				!strncmp(dwc->hsphy_interface, "ulpi", 4)) {
>>   			reg |= DWC3_GUSB2PHYCFG_ULPI_UTMI;
>> -			dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>> +			for (i = 0; i < dwc->num_usb2_phy; i++)
>> +				dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
>>   		} else {
>>   			/* Relying on default value. */
>>   			if (!(reg & DWC3_GUSB2PHYCFG_ULPI_UTMI))
>> @@ -787,7 +793,8 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
>>   	if (dwc->dis_u2_freeclk_exists_quirk)
>>   		reg &= ~DWC3_GUSB2PHYCFG_U2_FREECLK_EXISTS;
>>   
>> -	dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>> +	for (i = 0; i < dwc->num_usb2_phy; i++)
>> +		dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
>>   
>>   	return 0;
>>   }
>> @@ -826,17 +833,23 @@ static void dwc3_clk_disable(struct dwc3 *dwc)
>>   
>>   static void dwc3_core_exit(struct dwc3 *dwc)
>>   {
>> +	int i;
>>   	dwc3_event_buffers_cleanup(dwc);
>>   
>> -	usb_phy_shutdown(dwc->usb2_phy);
>> -	usb_phy_shutdown(dwc->usb3_phy);
>> -	phy_exit(dwc->usb2_generic_phy);
>> -	phy_exit(dwc->usb3_generic_phy);
>> +	for (i = 0; i < dwc->num_usb2_phy; i++) {
>> +		usb_phy_shutdown(dwc->usb2_phy[i]);
>> +		usb_phy_set_suspend(dwc->usb2_phy[i], 1);
>> +		phy_exit(dwc->usb2_generic_phy[i]);
>> +		phy_power_off(dwc->usb2_generic_phy[i]);
>> +	}
>> +
>> +	for (i = 0; i < dwc->num_usb3_phy; i++) {
>> +		usb_phy_shutdown(dwc->usb3_phy[i]);
>> +		usb_phy_set_suspend(dwc->usb3_phy[i], 1);
>> +		phy_exit(dwc->usb3_generic_phy[i]);
>> +		phy_power_off(dwc->usb3_generic_phy[i]);
>> +	}
>>   
>> -	usb_phy_set_suspend(dwc->usb2_phy, 1);
>> -	usb_phy_set_suspend(dwc->usb3_phy, 1);
>> -	phy_power_off(dwc->usb2_generic_phy);
>> -	phy_power_off(dwc->usb3_generic_phy);
>>   	dwc3_clk_disable(dwc);
>>   	reset_control_assert(dwc->reset);
>>   }
>> @@ -1039,7 +1052,7 @@ static int dwc3_core_init(struct dwc3 *dwc)
>>   {
>>   	unsigned int		hw_mode;
>>   	u32			reg;
>> -	int			ret;
>> +	int			ret, i;
>>   
>>   	hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
>>   
>> @@ -1067,16 +1080,24 @@ static int dwc3_core_init(struct dwc3 *dwc)
>>   		dwc->phys_ready = true;
>>   	}
>>   
>> -	usb_phy_init(dwc->usb2_phy);
>> -	usb_phy_init(dwc->usb3_phy);
>> -	ret = phy_init(dwc->usb2_generic_phy);
>> -	if (ret < 0)
>> -		goto err0a;
>> +	for (i = 0; i < dwc->num_usb2_phy; i++)
>> +		usb_phy_init(dwc->usb2_phy[i]);
>> +	for (i = 0; i < dwc->num_usb3_phy; i++)
>> +		usb_phy_init(dwc->usb3_phy[i]);
>>   
>> -	ret = phy_init(dwc->usb3_generic_phy);
>> -	if (ret < 0) {
>> -		phy_exit(dwc->usb2_generic_phy);
>> -		goto err0a;
>> +	for (i = 0; i < dwc->num_usb2_phy; i++) {
>> +		ret = phy_init(dwc->usb2_generic_phy[i]);
>> +		if (ret < 0)
>> +			goto err0a;
> Should we clean up the prior phys that did not fail? There's a couple
> loops where that question stands in this patch.
I am just keeping the Generic PHY implementation as is.
Earlier also we were cleaning up the HS PHY incase
SS PHY fails, so keeping the same for Multiport.
For you please point out if you find any major concern here?

>> +	}
>> +
>> +	for (i = 0; i < dwc->num_usb3_phy; i++) {
>> +		ret = phy_init(dwc->usb3_generic_phy[i]);
>> +		if (ret < 0) {
>> +			for (i = 0; i < dwc->num_usb2_phy; i++)
>> +				phy_exit(dwc->usb2_generic_phy[i]);
>> +			goto err0a;
>> +		}
>>   	}
>>   
>>   	ret = dwc3_core_soft_reset(dwc);
>> @@ -1086,15 +1107,19 @@ static int dwc3_core_init(struct dwc3 *dwc)
>>   	if (hw_mode == DWC3_GHWPARAMS0_MODE_DRD &&
>>   	    !DWC3_VER_IS_WITHIN(DWC3, ANY, 194A)) {
>>   		if (!dwc->dis_u3_susphy_quirk) {
>> -			reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0));
>> -			reg |= DWC3_GUSB3PIPECTL_SUSPHY;
>> -			dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg);
>> +			for (i = 0; i < dwc->num_usb3_phy; i++) {
>> +				reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(i));
>> +				reg |= DWC3_GUSB3PIPECTL_SUSPHY;
>> +				dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(i), reg);
>> +			}
>>   		}
>>   
>>   		if (!dwc->dis_u2_susphy_quirk) {
>> -			reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
>> -			reg |= DWC3_GUSB2PHYCFG_SUSPHY;
>> -			dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>> +			for (i = 0; i < dwc->num_usb2_phy; i++) {
>> +				reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(i));
>> +				reg |= DWC3_GUSB2PHYCFG_SUSPHY;
>> +				dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
>> +			}
>>   		}
>>   	}
>>   
>> @@ -1113,15 +1138,19 @@ static int dwc3_core_init(struct dwc3 *dwc)
>>   
>>   	dwc3_set_incr_burst_type(dwc);
>>   
>> -	usb_phy_set_suspend(dwc->usb2_phy, 0);
>> -	usb_phy_set_suspend(dwc->usb3_phy, 0);
>> -	ret = phy_power_on(dwc->usb2_generic_phy);
>> -	if (ret < 0)
>> -		goto err2;
>> +	for (i = 0; i < dwc->num_usb2_phy; i++) {
>> +		usb_phy_set_suspend(dwc->usb2_phy[i], 0);
>> +		ret = phy_power_on(dwc->usb2_generic_phy[i]);
>> +		if (ret < 0)
>> +			goto err2;
>> +	}
>>   
>> -	ret = phy_power_on(dwc->usb3_generic_phy);
>> -	if (ret < 0)
>> -		goto err3;
>> +	for (i = 0; i < dwc->num_usb3_phy; i++) {
>> +		usb_phy_set_suspend(dwc->usb3_phy[i], 0);
>> +		ret = phy_power_on(dwc->usb3_generic_phy[i]);
>> +		if (ret < 0)
>> +			goto err3;
>> +	}
>>   
>>   	ret = dwc3_event_buffers_setup(dwc);
>>   	if (ret) {
>> @@ -1229,20 +1258,29 @@ static int dwc3_core_init(struct dwc3 *dwc)
>>   	return 0;
>>   
>>   err4:
>> -	phy_power_off(dwc->usb3_generic_phy);
>> +	for (i = 0; i < dwc->num_usb3_phy; i++)
>> +		phy_power_off(dwc->usb3_generic_phy[i]);
>>   
>>   err3:
>> -	phy_power_off(dwc->usb2_generic_phy);
>> +	for (i = 0; i < dwc->num_usb2_phy; i++)
>> +		phy_power_off(dwc->usb2_generic_phy[i]);
>>   
>>   err2:
>> -	usb_phy_set_suspend(dwc->usb2_phy, 1);
>> -	usb_phy_set_suspend(dwc->usb3_phy, 1);
>> +	for (i = 0; i < dwc->num_usb2_phy; i++)
>> +		usb_phy_set_suspend(dwc->usb2_phy[i], 1);
>> +	for (i = 0; i < dwc->num_usb3_phy; i++)
>> +		usb_phy_set_suspend(dwc->usb3_phy[i], 1);
>>   
>>   err1:
>> -	usb_phy_shutdown(dwc->usb2_phy);
>> -	usb_phy_shutdown(dwc->usb3_phy);
>> -	phy_exit(dwc->usb2_generic_phy);
>> -	phy_exit(dwc->usb3_generic_phy);
>> +	for (i = 0; i < dwc->num_usb2_phy; i++) {
>> +		usb_phy_shutdown(dwc->usb2_phy[i]);
>> +		phy_exit(dwc->usb2_generic_phy[i]);
>> +	}
>> +
>> +	for (i = 0; i < dwc->num_usb3_phy; i++) {
>> +		usb_phy_shutdown(dwc->usb3_phy[i]);
>> +		phy_exit(dwc->usb3_generic_phy[i]);
>> +	}
>>   
>>   err0a:
>>   	dwc3_ulpi_exit(dwc);
>> @@ -1251,53 +1289,172 @@ static int dwc3_core_init(struct dwc3 *dwc)
>>   	return ret;
>>   }
>>   
>> -static int dwc3_core_get_phy(struct dwc3 *dwc)
>> +static struct usb_phy *dwc3_core_get_phy_by_handle_with_node(struct device *dev,
>> +	const char *phandle, u8 index, struct device_node *lookup_node)
>> +{
>> +	struct device_node *node;
>> +	struct usb_phy	*phy;
>> +
>> +	node = of_parse_phandle(lookup_node, phandle, index);
>> +	if (!node) {
>> +		dev_err(dev, "failed to get %s phandle in %pOF node\n", phandle,
>> +			dev->of_node);
>> +		return ERR_PTR(-ENODEV);
>> +	}
>> +	phy = devm_usb_get_phy_by_node(dev, node, NULL);
>> +	of_node_put(node);
>> +	return phy;
>> +}
>> +
>> +static int dwc3_count_phys(struct dwc3 *dwc, struct device_node *lookup_node)
>> +{
>> +	int count;
>> +
>> +	count = of_count_phandle_with_args(lookup_node, "phys", NULL);
>> +
>> +	if (count == -ENOENT)
>> +		count = of_count_phandle_with_args(lookup_node, "usb-phy", NULL);
>> +
>> +	if (count == 1) {
>> +		dwc->num_usb2_phy++;
>> +	} else if (count == 2) {
>> +		dwc->num_usb2_phy++;
>> +		dwc->num_usb3_phy++;
>> +	} else {
>> +		return count;
>> +	}
>> +	return 0;
>> +}
>> +
>> +static int dwc3_extract_num_phys(struct dwc3 *dwc)
>> +{
>> +	struct device_node	*ports, *port;
>> +	int			ret;
>> +
>> +	/* Find if any "multiport" child is present inside DWC3*/
> Nit, space after DWC3
Done
>
>> +	for_each_available_child_of_node(dwc->dev->of_node, ports) {
>> +		if (!strcmp(ports->name, "multiport"))
>> +			break;
>> +	}
>> +	if (!ports) {
>> +		dwc->num_usb2_phy = 1;
>> +		dwc->num_usb3_phy = 1;
>> +	} else {
>> +		for_each_available_child_of_node(ports, port) {
>> +			ret  = dwc3_count_phys(dwc, port);
>> +			if (ret)
>> +				return ret;
>> +		}
>> +	}
>> +	dev_info(dwc->dev, "Num of HS and SS PHY are %u %u\n", dwc->num_usb2_phy,
>> +									dwc->num_usb3_phy);
>> +
>> +	dwc->usb2_phy = devm_kzalloc(dwc->dev,
>> +		sizeof(*dwc->usb2_phy) * dwc->num_usb2_phy, GFP_KERNEL);
>> +	if (!dwc->usb2_phy)
>> +		return -ENOMEM;
>> +
>> +	dwc->usb3_phy = devm_kzalloc(dwc->dev,
>> +		sizeof(*dwc->usb3_phy) * dwc->num_usb3_phy, GFP_KERNEL);
>> +	if (!dwc->usb3_phy)
>> +		return -ENOMEM;
>> +
>> +	dwc->usb2_generic_phy = devm_kzalloc(dwc->dev,
>> +		sizeof(*dwc->usb2_generic_phy) * dwc->num_usb2_phy, GFP_KERNEL);
>> +	if (!dwc->usb2_generic_phy)
>> +		return -ENOMEM;
>> +
>> +	dwc->usb3_generic_phy = devm_kzalloc(dwc->dev,
>> +		sizeof(*dwc->usb3_generic_phy) * dwc->num_usb3_phy, GFP_KERNEL);
>> +	if (!dwc->usb3_generic_phy)
>> +		return -ENOMEM;
>> +
>> +	return 0;
>> +}
>> +
>> +static int dwc3_core_get_phy_by_node(struct dwc3 *dwc,
>> +		struct device_node *lookup_node, int i)
>>   {
>>   	struct device		*dev = dwc->dev;
>> -	struct device_node	*node = dev->of_node;
>> -	int ret;
>> +	int			ret;
>>   
>> -	if (node) {
>> -		dwc->usb2_phy = devm_usb_get_phy_by_phandle(dev, "usb-phy", 0);
>> -		dwc->usb3_phy = devm_usb_get_phy_by_phandle(dev, "usb-phy", 1);
>> +	if (lookup_node) {
>> +		dwc->usb2_phy[i] = devm_of_usb_get_phy_by_phandle(dev,
>> +								"usb-phy", 0, lookup_node);
>> +		dwc->usb3_phy[i] = devm_of_usb_get_phy_by_phandle(dev,
>> +								"usb-phy", 1, lookup_node);
>>   	} else {
>> -		dwc->usb2_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
>> -		dwc->usb3_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3);
>> +		dwc->usb2_phy[i] = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
>> +		dwc->usb3_phy[i] = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3);
>>   	}
>>   
>> -	if (IS_ERR(dwc->usb2_phy)) {
>> -		ret = PTR_ERR(dwc->usb2_phy);
>> +	if (IS_ERR(dwc->usb2_phy[i])) {
>> +		ret = PTR_ERR(dwc->usb2_phy[i]);
>>   		if (ret == -ENXIO || ret == -ENODEV)
>> -			dwc->usb2_phy = NULL;
>> +			dwc->usb2_phy[i] = NULL;
>>   		else
>>   			return dev_err_probe(dev, ret, "no usb2 phy configured\n");
>>   	}
>>   
>> -	if (IS_ERR(dwc->usb3_phy)) {
>> -		ret = PTR_ERR(dwc->usb3_phy);
>> +	if (IS_ERR(dwc->usb3_phy[i])) {
>> +		ret = PTR_ERR(dwc->usb3_phy[i]);
>>   		if (ret == -ENXIO || ret == -ENODEV)
>> -			dwc->usb3_phy = NULL;
>> +			dwc->usb3_phy[i] = NULL;
>>   		else
>>   			return dev_err_probe(dev, ret, "no usb3 phy configured\n");
>>   	}
>>   
>> -	dwc->usb2_generic_phy = devm_phy_get(dev, "usb2-phy");
>> -	if (IS_ERR(dwc->usb2_generic_phy)) {
>> -		ret = PTR_ERR(dwc->usb2_generic_phy);
>> -		if (ret == -ENOSYS || ret == -ENODEV)
>> -			dwc->usb2_generic_phy = NULL;
>> +	dwc->usb2_generic_phy[i] = devm_of_phy_get(dev, lookup_node, "usb2-phy");
>> +	if (IS_ERR(dwc->usb2_generic_phy[i])) {
>> +		ret = PTR_ERR(dwc->usb2_generic_phy[i]);
>> +		if (ret == -ENODEV)
> I think this can be -ENOSYS if !CONFIG_GENERIC_PHY
For now, I have added it back
>
>> +			dwc->usb2_generic_phy[i] = NULL;
>>   		else
>>   			return dev_err_probe(dev, ret, "no usb2 phy configured\n");
>>   	}
>>   
>> -	dwc->usb3_generic_phy = devm_phy_get(dev, "usb3-phy");
>> -	if (IS_ERR(dwc->usb3_generic_phy)) {
>> -		ret = PTR_ERR(dwc->usb3_generic_phy);
>> -		if (ret == -ENOSYS || ret == -ENODEV)
>> -			dwc->usb3_generic_phy = NULL;
>> +	dwc->usb3_generic_phy[i] = devm_of_phy_get(dev, lookup_node, "usb3-phy");
>> +	if (IS_ERR(dwc->usb3_generic_phy[i])) {
>> +		ret = PTR_ERR(dwc->usb3_generic_phy[i]);
>> +		if (ret == -ENODEV)
>> +			dwc->usb3_generic_phy[i] = NULL;
>>   		else
>>   			return dev_err_probe(dev, ret, "no usb3 phy configured\n");
>>   	}
>> +	return 0;
>> +}
>> +
>> +static int dwc3_core_get_phy(struct dwc3 *dwc)
>> +{
>> +	struct device		*dev = dwc->dev;
>> +	struct device_node	*node = dev->of_node;
>> +	struct device_node	*ports, *port;
>> +	int ret, i = 0;
>> +
>> +	ret = dwc3_extract_num_phys(dwc);
>> +	if (ret) {
>> +		dev_err(dwc->dev, "Unable to extract number of PHYs\n");
>> +		return ret;
>> +	}
>> +
>> +	/* Find if any "multiport" child is present inside DWC3*/
> Nit, space after DWC3
Done
>
>> +	for_each_available_child_of_node(node, ports) {
>> +		if (!strcmp(ports->name, "multiport"))
>> +			break;
>> +	}
>> +
>> +	if (!ports) {
>> +		ret = dwc3_core_get_phy_by_node(dwc, node, 0);
>> +		if (ret)
>> +			return ret;
>> +	} else {
>> +		for_each_available_child_of_node(ports, port) {
>> +			ret = dwc3_core_get_phy_by_node(dwc, port, i);
>> +			if (ret)
>> +				return ret;
>> +			i++;
>> +		}
>> +	}
>>   
>>   	return 0;
>>   }
>> @@ -1305,16 +1462,16 @@ static int dwc3_core_get_phy(struct dwc3 *dwc)
>>   static int dwc3_core_init_mode(struct dwc3 *dwc)
>>   {
>>   	struct device *dev = dwc->dev;
>> -	int ret;
>> +	int i, ret;
>>   
>>   	switch (dwc->dr_mode) {
>>   	case USB_DR_MODE_PERIPHERAL:
>>   		dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_DEVICE);
>>   
>> -		if (dwc->usb2_phy)
>> -			otg_set_vbus(dwc->usb2_phy->otg, false);
>> -		phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_DEVICE);
>> -		phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_DEVICE);
>> +		if (dwc->usb2_phy[0])
>> +			otg_set_vbus(dwc->usb2_phy[0]->otg, false);
>> +		phy_set_mode(dwc->usb2_generic_phy[0], PHY_MODE_USB_DEVICE);
>> +		phy_set_mode(dwc->usb3_generic_phy[0], PHY_MODE_USB_DEVICE);
>>   
>>   		ret = dwc3_gadget_init(dwc);
>>   		if (ret)
>> @@ -1323,10 +1480,15 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
>>   	case USB_DR_MODE_HOST:
>>   		dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_HOST);
>>   
>> -		if (dwc->usb2_phy)
>> -			otg_set_vbus(dwc->usb2_phy->otg, true);
>> -		phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST);
>> -		phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_HOST);
>> +		for (i = 0; i < dwc->num_usb3_phy; i++) {
>> +			if (dwc->usb2_phy[i])
>> +				otg_set_vbus(dwc->usb2_phy[i]->otg, true);
>> +			phy_set_mode(dwc->usb2_generic_phy[i], PHY_MODE_USB_HOST);
>> +		}
>> +
>> +
>> +		for (i = 0; i < dwc->num_usb3_phy; i++)
>> +			phy_set_mode(dwc->usb3_generic_phy[i], PHY_MODE_USB_HOST);
>>   
>>   		ret = dwc3_host_init(dwc);
>>   		if (ret)
>> @@ -1674,7 +1836,7 @@ static int dwc3_probe(struct platform_device *pdev)
>>   	struct resource		*res, dwc_res;
>>   	struct dwc3		*dwc;
>>   
>> -	int			ret;
>> +	int			ret, i;
>>   
>>   	void __iomem		*regs;
>>   
>> @@ -1839,15 +2001,18 @@ static int dwc3_probe(struct platform_device *pdev)
>>   	dwc3_debugfs_exit(dwc);
>>   	dwc3_event_buffers_cleanup(dwc);
>>   
>> -	usb_phy_shutdown(dwc->usb2_phy);
>> -	usb_phy_shutdown(dwc->usb3_phy);
>> -	phy_exit(dwc->usb2_generic_phy);
>> -	phy_exit(dwc->usb3_generic_phy);
>> -
>> -	usb_phy_set_suspend(dwc->usb2_phy, 1);
>> -	usb_phy_set_suspend(dwc->usb3_phy, 1);
>> -	phy_power_off(dwc->usb2_generic_phy);
>> -	phy_power_off(dwc->usb3_generic_phy);
>> +	for (i = 0; i < dwc->num_usb2_phy; i++) {
>> +		usb_phy_shutdown(dwc->usb2_phy[i]);
>> +		usb_phy_set_suspend(dwc->usb2_phy[i], 1);
>> +		phy_exit(dwc->usb2_generic_phy[i]);
>> +		phy_power_off(dwc->usb2_generic_phy[i]);
>> +	}
>> +	for (i = 0; i < dwc->num_usb3_phy; i++) {
>> +		usb_phy_shutdown(dwc->usb3_phy[i]);
>> +		usb_phy_set_suspend(dwc->usb3_phy[i], 1);
>> +		phy_exit(dwc->usb3_generic_phy[i]);
>> +		phy_power_off(dwc->usb3_generic_phy[i]);
>> +	}
>>   
>>   	dwc3_ulpi_exit(dwc);
>>   
>> @@ -1929,6 +2094,7 @@ static int dwc3_core_init_for_resume(struct dwc3 *dwc)
>>   
>>   static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
>>   {
>> +	int i;
>>   	unsigned long	flags;
>>   	u32 reg;
>>   
>> @@ -1951,17 +2117,21 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
>>   		/* Let controller to suspend HSPHY before PHY driver suspends */
>>   		if (dwc->dis_u2_susphy_quirk ||
>>   		    dwc->dis_enblslpm_quirk) {
>> -			reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
>> -			reg |=  DWC3_GUSB2PHYCFG_ENBLSLPM |
>> -				DWC3_GUSB2PHYCFG_SUSPHY;
>> -			dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>> -
>> -			/* Give some time for USB2 PHY to suspend */
>> -			usleep_range(5000, 6000);
>> +			for (i = 0; i < dwc->num_usb2_phy; i++) {
>> +				reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(i));
>> +				reg |=  DWC3_GUSB2PHYCFG_ENBLSLPM |
>> +					DWC3_GUSB2PHYCFG_SUSPHY;
>> +				dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
>> +
>> +				/* Give some time for USB2 PHY to suspend */
>> +				usleep_range(5000, 6000);
>> +			}
>>   		}
>>   
>> -		phy_pm_runtime_put_sync(dwc->usb2_generic_phy);
>> -		phy_pm_runtime_put_sync(dwc->usb3_generic_phy);
>> +		for (i = 0; i < dwc->num_usb2_phy; i++)
>> +			phy_pm_runtime_put_sync(dwc->usb2_generic_phy[i]);
>> +		for (i = 0; i < dwc->num_usb3_phy; i++)
>> +			phy_pm_runtime_put_sync(dwc->usb3_generic_phy[i]);
>>   		break;
>>   	case DWC3_GCTL_PRTCAP_OTG:
>>   		/* do nothing during runtime_suspend */
>> @@ -1989,7 +2159,7 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
>>   static int dwc3_resume_common(struct dwc3 *dwc, pm_message_t msg)
>>   {
>>   	unsigned long	flags;
>> -	int		ret;
>> +	int		i, ret;
>>   	u32		reg;
>>   
>>   	switch (dwc->current_dr_role) {
>> @@ -2012,17 +2182,21 @@ static int dwc3_resume_common(struct dwc3 *dwc, pm_message_t msg)
>>   			break;
>>   		}
>>   		/* Restore GUSB2PHYCFG bits that were modified in suspend */
>> -		reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
>> -		if (dwc->dis_u2_susphy_quirk)
>> -			reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
>> +		for (i = 0; i < dwc->num_usb2_phy; i++) {
>> +			reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(i));
>> +			if (dwc->dis_u2_susphy_quirk)
>> +				reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
>>   
>> -		if (dwc->dis_enblslpm_quirk)
>> -			reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;
>> +			if (dwc->dis_enblslpm_quirk)
>> +				reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;
>>   
>> -		dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>> +			dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
>> +		}
>>   
>> -		phy_pm_runtime_get_sync(dwc->usb2_generic_phy);
>> -		phy_pm_runtime_get_sync(dwc->usb3_generic_phy);
>> +		for (i = 0; i < dwc->num_usb2_phy; i++)
>> +			phy_pm_runtime_get_sync(dwc->usb2_generic_phy[i]);
>> +		for (i = 0; i < dwc->num_usb3_phy; i++)
>> +			phy_pm_runtime_get_sync(dwc->usb3_generic_phy[i]);
>>   		break;
>>   	case DWC3_GCTL_PRTCAP_OTG:
>>   		/* nothing to do on runtime_resume */
>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>> index 81c486b..c169bf1 100644
>> --- a/drivers/usb/dwc3/core.h
>> +++ b/drivers/usb/dwc3/core.h
>> @@ -1020,6 +1020,8 @@ struct dwc3_scratchpad_array {
>>    * @usb_psy: pointer to power supply interface.
>>    * @usb2_phy: pointer to USB2 PHY
>>    * @usb3_phy: pointer to USB3 PHY
>> + * @num_usb2_phy: Number of HS ports controlled by the core
>> + * @num_dsphy: Number of SS ports controlled by the core
> s/num_dsphy/num_usb3_phy/
Done
>
>>    * @usb2_generic_phy: pointer to USB2 PHY
>>    * @usb3_generic_phy: pointer to USB3 PHY
>>    * @phys_ready: flag to indicate that PHYs are ready
>> @@ -1147,11 +1149,13 @@ struct dwc3 {
>>   
>>   	struct reset_control	*reset;
>>   
>> -	struct usb_phy		*usb2_phy;
>> -	struct usb_phy		*usb3_phy;
>> +	struct usb_phy		**usb2_phy;
>> +	struct usb_phy		**usb3_phy;
>> +	u32			num_usb2_phy;
>> +	u32			num_usb3_phy;
>>   
>> -	struct phy		*usb2_generic_phy;
>> -	struct phy		*usb3_generic_phy;
>> +	struct phy		**usb2_generic_phy;
>> +	struct phy		**usb3_generic_phy;
>>   
>>   	bool			phys_ready;
>>   
>> diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
>> index 039bf24..404643f 100644
>> --- a/drivers/usb/dwc3/drd.c
>> +++ b/drivers/usb/dwc3/drd.c
>> @@ -384,10 +384,10 @@ void dwc3_otg_update(struct dwc3 *dwc, bool ignore_idstatus)
>>   		if (ret) {
>>   			dev_err(dwc->dev, "failed to initialize host\n");
>>   		} else {
>> -			if (dwc->usb2_phy)
>> -				otg_set_vbus(dwc->usb2_phy->otg, true);
>> -			if (dwc->usb2_generic_phy)
>> -				phy_set_mode(dwc->usb2_generic_phy,
>> +			if (dwc->usb2_phy[0])
>> +				otg_set_vbus(dwc->usb2_phy[0]->otg, true);
>> +			if (dwc->usb2_generic_phy[0])
>> +				phy_set_mode(dwc->usb2_generic_phy[0],
>>   					     PHY_MODE_USB_HOST);
>>   		}
>>   		break;
>> @@ -398,10 +398,10 @@ void dwc3_otg_update(struct dwc3 *dwc, bool ignore_idstatus)
>>   		dwc3_event_buffers_setup(dwc);
>>   		spin_unlock_irqrestore(&dwc->lock, flags);
>>   
>> -		if (dwc->usb2_phy)
>> -			otg_set_vbus(dwc->usb2_phy->otg, false);
>> -		if (dwc->usb2_generic_phy)
>> -			phy_set_mode(dwc->usb2_generic_phy,
>> +		if (dwc->usb2_phy[0])
>> +			otg_set_vbus(dwc->usb2_phy[0]->otg, false);
>> +		if (dwc->usb2_generic_phy[0])
>> +			phy_set_mode(dwc->usb2_generic_phy[0],
>>   				     PHY_MODE_USB_DEVICE);
>>   		ret = dwc3_gadget_init(dwc);
>>   		if (ret)
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index 00427d1..e3b2a17 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -2872,8 +2872,8 @@ static int dwc3_gadget_vbus_draw(struct usb_gadget *g, unsigned int mA)
>>   	union power_supply_propval	val = {0};
>>   	int				ret;
>>   
>> -	if (dwc->usb2_phy)
>> -		return usb_phy_set_power(dwc->usb2_phy, mA);
>> +	if (dwc->usb2_phy[0])
>> +		return usb_phy_set_power(dwc->usb2_phy[0], mA);
>>   
>>   	if (!dwc->usb_psy)
>>   		return -EOPNOTSUPP;
>> -- 
>> 2.7.4
>>

Re: [PATCH 3/3] usb: dwc3: Refactor PHY logic to support Multiport Controller

[Harsh Agarwal]

On 5/31/2022 5:56 PM, Pavan Kondeti wrote:
> Hi Harsh,
>
> On Tue, May 31, 2022 at 01:50:17PM +0530, Harsh Agarwal wrote:
>> Currently the DWC3 driver supports only single port controller
>> which requires at most 2 PHYs ie HS and SS PHYs.
>>
>> But some SOCs have a "multiport" USB DWC3 controller where a
>> single controller supports multiple ports and each port have
>> their own PHYs. Refactor PHY logic to support the same.
>>
>> Signed-off-by: Harsh Agarwal <quic_harshq@quicinc.com>
>> ---
>>   drivers/usb/dwc3/core.c   | 400 +++++++++++++++++++++++++++++++++-------------
>>   drivers/usb/dwc3/core.h   |  12 +-
>>   drivers/usb/dwc3/drd.c    |  16 +-
>>   drivers/usb/dwc3/gadget.c |   4 +-
>>   4 files changed, 305 insertions(+), 127 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 5734219..5cc799e 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -120,7 +120,7 @@ static void __dwc3_set_mode(struct work_struct *work)
>>   {
>>   	struct dwc3 *dwc = work_to_dwc(work);
>>   	unsigned long flags;
>> -	int ret;
>> +	int i, ret;
>>   	u32 reg;
>>   
> <snip>
>
>> -static int dwc3_core_get_phy(struct dwc3 *dwc)
>> +static struct usb_phy *dwc3_core_get_phy_by_handle_with_node(struct device *dev,
>> +	const char *phandle, u8 index, struct device_node *lookup_node)
>> +{
>> +	struct device_node *node;
>> +	struct usb_phy	*phy;
>> +
>> +	node = of_parse_phandle(lookup_node, phandle, index);
>> +	if (!node) {
>> +		dev_err(dev, "failed to get %s phandle in %pOF node\n", phandle,
>> +			dev->of_node);
>> +		return ERR_PTR(-ENODEV);
>> +	}
>> +	phy = devm_usb_get_phy_by_node(dev, node, NULL);
>> +	of_node_put(node);
>> +	return phy;
>> +}
> Remove this function definition, since we moved this to PHY library.
Got it removed in the latest PS v2
>
>> +
>> +static int dwc3_count_phys(struct dwc3 *dwc, struct device_node *lookup_node)
>> +{
>> +	int count;
>> +
>> +	count = of_count_phandle_with_args(lookup_node, "phys", NULL);
>> +
>> +	if (count == -ENOENT)
>> +		count = of_count_phandle_with_args(lookup_node, "usb-phy", NULL);
>> +
>> +	if (count == 1) {
>> +		dwc->num_usb2_phy++;
>> +	} else if (count == 2) {
>> +		dwc->num_usb2_phy++;
>> +		dwc->num_usb3_phy++;
>> +	} else {
>> +		return count;
>> +	}
>> +	return 0;
>> +}
>> +
>> +static int dwc3_extract_num_phys(struct dwc3 *dwc)
>> +{
>> +	struct device_node	*ports, *port;
>> +	int			ret;
>> +
>> +	/* Find if any "multiport" child is present inside DWC3*/
>> +	for_each_available_child_of_node(dwc->dev->of_node, ports) {
>> +		if (!strcmp(ports->name, "multiport"))
>> +			break;
>> +	}
>> +	if (!ports) {
>> +		dwc->num_usb2_phy = 1;
>> +		dwc->num_usb3_phy = 1;
>> +	} else {
>> +		for_each_available_child_of_node(ports, port) {
>> +			ret  = dwc3_count_phys(dwc, port);
>> +			if (ret)
>> +				return ret;
> If you break the loop, you have to call of_node_put() explicitly.
Done. P;ease check v2
>
>> +		}
>> +	}
>> +	dev_info(dwc->dev, "Num of HS and SS PHY are %u %u\n", dwc->num_usb2_phy,
>> +									dwc->num_usb3_phy);
> If I declare the multiport node don't specify any phys for any of the ports,
> we end up coming here with dwc->num_usb2_phy = dwc->num_usb3_phy = 0. That is
> a problem since we access dwc->usb2_phy[0] directly. Can we bail out in that
> case?
We have mandated to use either usb-phy or Generic phy inside the child nodes
of multiport.
>
>> +
>> +	dwc->usb2_phy = devm_kzalloc(dwc->dev,
>> +		sizeof(*dwc->usb2_phy) * dwc->num_usb2_phy, GFP_KERNEL);
>> +	if (!dwc->usb2_phy)
>> +		return -ENOMEM;
>> +
>> +	dwc->usb3_phy = devm_kzalloc(dwc->dev,
>> +		sizeof(*dwc->usb3_phy) * dwc->num_usb3_phy, GFP_KERNEL);
>> +	if (!dwc->usb3_phy)
>> +		return -ENOMEM;
>> +
>> +	dwc->usb2_generic_phy = devm_kzalloc(dwc->dev,
>> +		sizeof(*dwc->usb2_generic_phy) * dwc->num_usb2_phy, GFP_KERNEL);
>> +	if (!dwc->usb2_generic_phy)
>> +		return -ENOMEM;
>> +
>> +	dwc->usb3_generic_phy = devm_kzalloc(dwc->dev,
>> +		sizeof(*dwc->usb3_generic_phy) * dwc->num_usb3_phy, GFP_KERNEL);
>> +	if (!dwc->usb3_generic_phy)
>> +		return -ENOMEM;
>> +
>> +	return 0;
>> +}
>> +
>> +static int dwc3_core_get_phy_by_node(struct dwc3 *dwc,
>> +		struct device_node *lookup_node, int i)
>>   {
>>   	struct device		*dev = dwc->dev;
>> -	struct device_node	*node = dev->of_node;
>> -	int ret;
>> +	int			ret;
>>   
>> -	if (node) {
>> -		dwc->usb2_phy = devm_usb_get_phy_by_phandle(dev, "usb-phy", 0);
>> -		dwc->usb3_phy = devm_usb_get_phy_by_phandle(dev, "usb-phy", 1);
>> +	if (lookup_node) {
>> +		dwc->usb2_phy[i] = devm_of_usb_get_phy_by_phandle(dev,
>> +								"usb-phy", 0, lookup_node);
>> +		dwc->usb3_phy[i] = devm_of_usb_get_phy_by_phandle(dev,
>> +								"usb-phy", 1, lookup_node);
>>   	} else {
>> -		dwc->usb2_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
>> -		dwc->usb3_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3);
>> +		dwc->usb2_phy[i] = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
>> +		dwc->usb3_phy[i] = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3);
>>   	}
>>   
>> -	if (IS_ERR(dwc->usb2_phy)) {
>> -		ret = PTR_ERR(dwc->usb2_phy);
>> +	if (IS_ERR(dwc->usb2_phy[i])) {
>> +		ret = PTR_ERR(dwc->usb2_phy[i]);
>>   		if (ret == -ENXIO || ret == -ENODEV)
>> -			dwc->usb2_phy = NULL;
>> +			dwc->usb2_phy[i] = NULL;
>>   		else
>>   			return dev_err_probe(dev, ret, "no usb2 phy configured\n");
>>   	}
>>   
>> -	if (IS_ERR(dwc->usb3_phy)) {
>> -		ret = PTR_ERR(dwc->usb3_phy);
>> +	if (IS_ERR(dwc->usb3_phy[i])) {
>> +		ret = PTR_ERR(dwc->usb3_phy[i]);
>>   		if (ret == -ENXIO || ret == -ENODEV)
>> -			dwc->usb3_phy = NULL;
>> +			dwc->usb3_phy[i] = NULL;
>>   		else
>>   			return dev_err_probe(dev, ret, "no usb3 phy configured\n");
>>   	}
>>   
>> -	dwc->usb2_generic_phy = devm_phy_get(dev, "usb2-phy");
>> -	if (IS_ERR(dwc->usb2_generic_phy)) {
>> -		ret = PTR_ERR(dwc->usb2_generic_phy);
>> -		if (ret == -ENOSYS || ret == -ENODEV)
>> -			dwc->usb2_generic_phy = NULL;
>> +	dwc->usb2_generic_phy[i] = devm_of_phy_get(dev, lookup_node, "usb2-phy");
>> +	if (IS_ERR(dwc->usb2_generic_phy[i])) {
>> +		ret = PTR_ERR(dwc->usb2_generic_phy[i]);
>> +		if (ret == -ENODEV)
>> +			dwc->usb2_generic_phy[i] = NULL;
>>   		else
>>   			return dev_err_probe(dev, ret, "no usb2 phy configured\n");
>>   	}
>>   
>> -	dwc->usb3_generic_phy = devm_phy_get(dev, "usb3-phy");
>> -	if (IS_ERR(dwc->usb3_generic_phy)) {
>> -		ret = PTR_ERR(dwc->usb3_generic_phy);
>> -		if (ret == -ENOSYS || ret == -ENODEV)
>> -			dwc->usb3_generic_phy = NULL;
>> +	dwc->usb3_generic_phy[i] = devm_of_phy_get(dev, lookup_node, "usb3-phy");
>> +	if (IS_ERR(dwc->usb3_generic_phy[i])) {
>> +		ret = PTR_ERR(dwc->usb3_generic_phy[i]);
>> +		if (ret == -ENODEV)
>> +			dwc->usb3_generic_phy[i] = NULL;
>>   		else
>>   			return dev_err_probe(dev, ret, "no usb3 phy configured\n");
>>   	}
>> +	return 0;
>> +}
>> +
>> +static int dwc3_core_get_phy(struct dwc3 *dwc)
>> +{
>> +	struct device		*dev = dwc->dev;
>> +	struct device_node	*node = dev->of_node;
>> +	struct device_node	*ports, *port;
>> +	int ret, i = 0;
>> +
>> +	ret = dwc3_extract_num_phys(dwc);
>> +	if (ret) {
>> +		dev_err(dwc->dev, "Unable to extract number of PHYs\n");
>> +		return ret;
>> +	}
>> +
>> +	/* Find if any "multiport" child is present inside DWC3*/
>> +	for_each_available_child_of_node(node, ports) {
>> +		if (!strcmp(ports->name, "multiport"))
>> +			break;
>> +	}
>> +
>> +	if (!ports) {
>> +		ret = dwc3_core_get_phy_by_node(dwc, node, 0);
>> +		if (ret)
>> +			return ret;
>> +	} else {
>> +		for_each_available_child_of_node(ports, port) {
>> +			ret = dwc3_core_get_phy_by_node(dwc, port, i);
>> +			if (ret)
>> +				return ret;
>> +			i++;
>> +		}
>> +	}
>>   
> How does this work actually for a case where I have 4 ports with hs_phy=4 and
> ss_phy=2. Here we end up calling i = {0, 1, 2, 3} and accessing un-allocated
> ss phy instances in dwc3_core_get_phy_by_node(). you should also handle the
> case where first two ports are HS only and last two ports are both HS and SS.
Thanks for this endcase. Got it rectified in the new PS v2.
Yes lets suppose a Quadport controller having the first two
ports as HS only and last two ports as SS+HS capable.
Then proper indexing for SS PHY should also be taken care of.
Even though SS phy belongs to 3rd port, it's index will start with 0.
This is because for each SS PHY we have to properly map to
USBPIPECTL, PORTSC registers.
>
> Thanks,
> Pavan

Re: [PATCH 3/3] usb: dwc3: Refactor PHY logic to support Multiport Controller

[Andrew Halaney]

On Sun, Jun 05, 2022 at 11:51:38PM +0530, Harsh Agarwal wrote:
> 
> On 6/2/2022 1:23 AM, Andrew Halaney wrote:
> > On Tue, May 31, 2022 at 01:50:17PM +0530, Harsh Agarwal wrote:
> > > Currently the DWC3 driver supports only single port controller
> > > which requires at most 2 PHYs ie HS and SS PHYs.
> > > 
> > > But some SOCs have a "multiport" USB DWC3 controller where a
> > > single controller supports multiple ports and each port have
> > > their own PHYs. Refactor PHY logic to support the same.
> > > 
> > > Signed-off-by: Harsh Agarwal <quic_harshq@quicinc.com>
> > > ---
> > >   drivers/usb/dwc3/core.c   | 400 +++++++++++++++++++++++++++++++++-------------
> > >   drivers/usb/dwc3/core.h   |  12 +-
> > >   drivers/usb/dwc3/drd.c    |  16 +-
> > >   drivers/usb/dwc3/gadget.c |   4 +-
> > >   4 files changed, 305 insertions(+), 127 deletions(-)
> > > 
> > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > > index 5734219..5cc799e 100644
> > > --- a/drivers/usb/dwc3/core.c
> > > +++ b/drivers/usb/dwc3/core.c
> > > @@ -120,7 +120,7 @@ static void __dwc3_set_mode(struct work_struct *work)
> > >   {
> > >   	struct dwc3 *dwc = work_to_dwc(work);
> > >   	unsigned long flags;
> > > -	int ret;
> > > +	int i, ret;
> > >   	u32 reg;
> > >   	mutex_lock(&dwc->mutex);
> > > @@ -189,10 +189,13 @@ static void __dwc3_set_mode(struct work_struct *work)
> > >   		if (ret) {
> > >   			dev_err(dwc->dev, "failed to initialize host\n");
> > >   		} else {
> > > -			if (dwc->usb2_phy)
> > > -				otg_set_vbus(dwc->usb2_phy->otg, true);
> > > -			phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST);
> > > -			phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_HOST);
> > > +			for (i = 0; i < dwc->num_usb2_phy; i++) {
> > > +				if (dwc->usb2_phy[i])
> > > +					otg_set_vbus(dwc->usb2_phy[i]->otg, true);
> > > +				phy_set_mode(dwc->usb2_generic_phy[i], PHY_MODE_USB_HOST);
> > > +			}
> > > +			for (i = 0; i < dwc->num_usb3_phy; i++)
> > > +				phy_set_mode(dwc->usb3_generic_phy[i], PHY_MODE_USB_HOST);
> > >   			if (dwc->dis_split_quirk) {
> > >   				reg = dwc3_readl(dwc->regs, DWC3_GUCTL3);
> > >   				reg |= DWC3_GUCTL3_SPLITDISABLE;
> > > @@ -205,10 +208,10 @@ static void __dwc3_set_mode(struct work_struct *work)
> > >   		dwc3_event_buffers_setup(dwc);
> > > -		if (dwc->usb2_phy)
> > > -			otg_set_vbus(dwc->usb2_phy->otg, false);
> > > -		phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_DEVICE);
> > > -		phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_DEVICE);
> > > +		if (dwc->usb2_phy[0])
> > > +			otg_set_vbus(dwc->usb2_phy[0]->otg, false);
> > > +		phy_set_mode(dwc->usb2_generic_phy[0], PHY_MODE_USB_DEVICE);
> > > +		phy_set_mode(dwc->usb3_generic_phy[0], PHY_MODE_USB_DEVICE);
> > >   		ret = dwc3_gadget_init(dwc);
> > >   		if (ret)
> > > @@ -656,6 +659,7 @@ static int dwc3_core_ulpi_init(struct dwc3 *dwc)
> > >    */
> > >   static int dwc3_phy_setup(struct dwc3 *dwc)
> > >   {
> > > +	int i;
> > >   	unsigned int hw_mode;
> > >   	u32 reg;
> > > @@ -716,7 +720,8 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
> > >   	if (dwc->dis_del_phy_power_chg_quirk)
> > >   		reg &= ~DWC3_GUSB3PIPECTL_DEPOCHANGE;
> > > -	dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg);
> > > +	for (i = 0; i < dwc->num_usb3_phy; i++)
> > > +		dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(i), reg);
> > >   	reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
> > > @@ -730,7 +735,8 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
> > >   		} else if (dwc->hsphy_interface &&
> > >   				!strncmp(dwc->hsphy_interface, "ulpi", 4)) {
> > >   			reg |= DWC3_GUSB2PHYCFG_ULPI_UTMI;
> > > -			dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
> > > +			for (i = 0; i < dwc->num_usb2_phy; i++)
> > > +				dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
> > >   		} else {
> > >   			/* Relying on default value. */
> > >   			if (!(reg & DWC3_GUSB2PHYCFG_ULPI_UTMI))
> > > @@ -787,7 +793,8 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
> > >   	if (dwc->dis_u2_freeclk_exists_quirk)
> > >   		reg &= ~DWC3_GUSB2PHYCFG_U2_FREECLK_EXISTS;
> > > -	dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
> > > +	for (i = 0; i < dwc->num_usb2_phy; i++)
> > > +		dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
> > >   	return 0;
> > >   }
> > > @@ -826,17 +833,23 @@ static void dwc3_clk_disable(struct dwc3 *dwc)
> > >   static void dwc3_core_exit(struct dwc3 *dwc)
> > >   {
> > > +	int i;
> > >   	dwc3_event_buffers_cleanup(dwc);
> > > -	usb_phy_shutdown(dwc->usb2_phy);
> > > -	usb_phy_shutdown(dwc->usb3_phy);
> > > -	phy_exit(dwc->usb2_generic_phy);
> > > -	phy_exit(dwc->usb3_generic_phy);
> > > +	for (i = 0; i < dwc->num_usb2_phy; i++) {
> > > +		usb_phy_shutdown(dwc->usb2_phy[i]);
> > > +		usb_phy_set_suspend(dwc->usb2_phy[i], 1);
> > > +		phy_exit(dwc->usb2_generic_phy[i]);
> > > +		phy_power_off(dwc->usb2_generic_phy[i]);
> > > +	}
> > > +
> > > +	for (i = 0; i < dwc->num_usb3_phy; i++) {
> > > +		usb_phy_shutdown(dwc->usb3_phy[i]);
> > > +		usb_phy_set_suspend(dwc->usb3_phy[i], 1);
> > > +		phy_exit(dwc->usb3_generic_phy[i]);
> > > +		phy_power_off(dwc->usb3_generic_phy[i]);
> > > +	}
> > > -	usb_phy_set_suspend(dwc->usb2_phy, 1);
> > > -	usb_phy_set_suspend(dwc->usb3_phy, 1);
> > > -	phy_power_off(dwc->usb2_generic_phy);
> > > -	phy_power_off(dwc->usb3_generic_phy);
> > >   	dwc3_clk_disable(dwc);
> > >   	reset_control_assert(dwc->reset);
> > >   }
> > > @@ -1039,7 +1052,7 @@ static int dwc3_core_init(struct dwc3 *dwc)
> > >   {
> > >   	unsigned int		hw_mode;
> > >   	u32			reg;
> > > -	int			ret;
> > > +	int			ret, i;
> > >   	hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
> > > @@ -1067,16 +1080,24 @@ static int dwc3_core_init(struct dwc3 *dwc)
> > >   		dwc->phys_ready = true;
> > >   	}
> > > -	usb_phy_init(dwc->usb2_phy);
> > > -	usb_phy_init(dwc->usb3_phy);
> > > -	ret = phy_init(dwc->usb2_generic_phy);
> > > -	if (ret < 0)
> > > -		goto err0a;
> > > +	for (i = 0; i < dwc->num_usb2_phy; i++)
> > > +		usb_phy_init(dwc->usb2_phy[i]);
> > > +	for (i = 0; i < dwc->num_usb3_phy; i++)
> > > +		usb_phy_init(dwc->usb3_phy[i]);
> > > -	ret = phy_init(dwc->usb3_generic_phy);
> > > -	if (ret < 0) {
> > > -		phy_exit(dwc->usb2_generic_phy);
> > > -		goto err0a;
> > > +	for (i = 0; i < dwc->num_usb2_phy; i++) {
> > > +		ret = phy_init(dwc->usb2_generic_phy[i]);
> > > +		if (ret < 0)
> > > +			goto err0a;
> > Should we clean up the prior phys that did not fail? There's a couple
> > loops where that question stands in this patch.
> I am just keeping the Generic PHY implementation as is.
> Earlier also we were cleaning up the HS PHY incase
> SS PHY fails, so keeping the same for Multiport.
> For you please point out if you find any major concern here?

Earlier, there was only one usb2_generic_phy/usb3_generic_phy, so if
phy_init(usb2_generic_phy) returned an error, it wasn't initialized, and
it did not need a matching call to phy_exit(usb2_generic_phy).

That is being called though ifthe superspeed phy fails since the
highspeed one did successfully init, so things are cleaned up in the
old cases.

My point now is that, say with 4 highspeed phys, the 3rd one fails.
i.e. phy_init(usb2_generic_phy[i]) where i == 2. In that case we don't
ever call phy_exit() on the first two phys. I wouldn't say it is a
major concern, but cleanup here is not fully done.

You could do something like this to do clean it all up, kind of annoying
though:

	for (i = 0; i < dwc->num_usb2_phy; i++) {
		ret = phy_init(dwc->usb2_generic_phy[i]);
		if (ret < 0) {
			/* clean up prior phys we initialized */
			for (j = 0; j < i; j++)
				phy_exit(usb2_generic_phy[j]);
			goto err0a;
		}
	}

Thanks,
Andrew

> 
> > > +	}
> > > +
> > > +	for (i = 0; i < dwc->num_usb3_phy; i++) {
> > > +		ret = phy_init(dwc->usb3_generic_phy[i]);
> > > +		if (ret < 0) {
> > > +			for (i = 0; i < dwc->num_usb2_phy; i++)
> > > +				phy_exit(dwc->usb2_generic_phy[i]);
> > > +			goto err0a;
> > > +		}
> > >   	}
> > >   	ret = dwc3_core_soft_reset(dwc);
> > > @@ -1086,15 +1107,19 @@ static int dwc3_core_init(struct dwc3 *dwc)
> > >   	if (hw_mode == DWC3_GHWPARAMS0_MODE_DRD &&
> > >   	    !DWC3_VER_IS_WITHIN(DWC3, ANY, 194A)) {
> > >   		if (!dwc->dis_u3_susphy_quirk) {
> > > -			reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0));
> > > -			reg |= DWC3_GUSB3PIPECTL_SUSPHY;
> > > -			dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg);
> > > +			for (i = 0; i < dwc->num_usb3_phy; i++) {
> > > +				reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(i));
> > > +				reg |= DWC3_GUSB3PIPECTL_SUSPHY;
> > > +				dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(i), reg);
> > > +			}
> > >   		}
> > >   		if (!dwc->dis_u2_susphy_quirk) {
> > > -			reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
> > > -			reg |= DWC3_GUSB2PHYCFG_SUSPHY;
> > > -			dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
> > > +			for (i = 0; i < dwc->num_usb2_phy; i++) {
> > > +				reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(i));
> > > +				reg |= DWC3_GUSB2PHYCFG_SUSPHY;
> > > +				dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
> > > +			}
> > >   		}
> > >   	}
> > > @@ -1113,15 +1138,19 @@ static int dwc3_core_init(struct dwc3 *dwc)
> > >   	dwc3_set_incr_burst_type(dwc);
> > > -	usb_phy_set_suspend(dwc->usb2_phy, 0);
> > > -	usb_phy_set_suspend(dwc->usb3_phy, 0);
> > > -	ret = phy_power_on(dwc->usb2_generic_phy);
> > > -	if (ret < 0)
> > > -		goto err2;
> > > +	for (i = 0; i < dwc->num_usb2_phy; i++) {
> > > +		usb_phy_set_suspend(dwc->usb2_phy[i], 0);
> > > +		ret = phy_power_on(dwc->usb2_generic_phy[i]);
> > > +		if (ret < 0)
> > > +			goto err2;
> > > +	}
> > > -	ret = phy_power_on(dwc->usb3_generic_phy);
> > > -	if (ret < 0)
> > > -		goto err3;
> > > +	for (i = 0; i < dwc->num_usb3_phy; i++) {
> > > +		usb_phy_set_suspend(dwc->usb3_phy[i], 0);
> > > +		ret = phy_power_on(dwc->usb3_generic_phy[i]);
> > > +		if (ret < 0)
> > > +			goto err3;
> > > +	}
> > >   	ret = dwc3_event_buffers_setup(dwc);
> > >   	if (ret) {
> > > @@ -1229,20 +1258,29 @@ static int dwc3_core_init(struct dwc3 *dwc)
> > >   	return 0;
> > >   err4:
> > > -	phy_power_off(dwc->usb3_generic_phy);
> > > +	for (i = 0; i < dwc->num_usb3_phy; i++)
> > > +		phy_power_off(dwc->usb3_generic_phy[i]);
> > >   err3:
> > > -	phy_power_off(dwc->usb2_generic_phy);
> > > +	for (i = 0; i < dwc->num_usb2_phy; i++)
> > > +		phy_power_off(dwc->usb2_generic_phy[i]);
> > >   err2:
> > > -	usb_phy_set_suspend(dwc->usb2_phy, 1);
> > > -	usb_phy_set_suspend(dwc->usb3_phy, 1);
> > > +	for (i = 0; i < dwc->num_usb2_phy; i++)
> > > +		usb_phy_set_suspend(dwc->usb2_phy[i], 1);
> > > +	for (i = 0; i < dwc->num_usb3_phy; i++)
> > > +		usb_phy_set_suspend(dwc->usb3_phy[i], 1);
> > >   err1:
> > > -	usb_phy_shutdown(dwc->usb2_phy);
> > > -	usb_phy_shutdown(dwc->usb3_phy);
> > > -	phy_exit(dwc->usb2_generic_phy);
> > > -	phy_exit(dwc->usb3_generic_phy);
> > > +	for (i = 0; i < dwc->num_usb2_phy; i++) {
> > > +		usb_phy_shutdown(dwc->usb2_phy[i]);
> > > +		phy_exit(dwc->usb2_generic_phy[i]);
> > > +	}
> > > +
> > > +	for (i = 0; i < dwc->num_usb3_phy; i++) {
> > > +		usb_phy_shutdown(dwc->usb3_phy[i]);
> > > +		phy_exit(dwc->usb3_generic_phy[i]);
> > > +	}
> > >   err0a:
> > >   	dwc3_ulpi_exit(dwc);
> > > @@ -1251,53 +1289,172 @@ static int dwc3_core_init(struct dwc3 *dwc)
> > >   	return ret;
> > >   }
> > > -static int dwc3_core_get_phy(struct dwc3 *dwc)
> > > +static struct usb_phy *dwc3_core_get_phy_by_handle_with_node(struct device *dev,
> > > +	const char *phandle, u8 index, struct device_node *lookup_node)
> > > +{
> > > +	struct device_node *node;
> > > +	struct usb_phy	*phy;
> > > +
> > > +	node = of_parse_phandle(lookup_node, phandle, index);
> > > +	if (!node) {
> > > +		dev_err(dev, "failed to get %s phandle in %pOF node\n", phandle,
> > > +			dev->of_node);
> > > +		return ERR_PTR(-ENODEV);
> > > +	}
> > > +	phy = devm_usb_get_phy_by_node(dev, node, NULL);
> > > +	of_node_put(node);
> > > +	return phy;
> > > +}
> > > +
> > > +static int dwc3_count_phys(struct dwc3 *dwc, struct device_node *lookup_node)
> > > +{
> > > +	int count;
> > > +
> > > +	count = of_count_phandle_with_args(lookup_node, "phys", NULL);
> > > +
> > > +	if (count == -ENOENT)
> > > +		count = of_count_phandle_with_args(lookup_node, "usb-phy", NULL);
> > > +
> > > +	if (count == 1) {
> > > +		dwc->num_usb2_phy++;
> > > +	} else if (count == 2) {
> > > +		dwc->num_usb2_phy++;
> > > +		dwc->num_usb3_phy++;
> > > +	} else {
> > > +		return count;
> > > +	}
> > > +	return 0;
> > > +}
> > > +
> > > +static int dwc3_extract_num_phys(struct dwc3 *dwc)
> > > +{
> > > +	struct device_node	*ports, *port;
> > > +	int			ret;
> > > +
> > > +	/* Find if any "multiport" child is present inside DWC3*/
> > Nit, space after DWC3
> Done
> > 
> > > +	for_each_available_child_of_node(dwc->dev->of_node, ports) {
> > > +		if (!strcmp(ports->name, "multiport"))
> > > +			break;
> > > +	}
> > > +	if (!ports) {
> > > +		dwc->num_usb2_phy = 1;
> > > +		dwc->num_usb3_phy = 1;
> > > +	} else {
> > > +		for_each_available_child_of_node(ports, port) {
> > > +			ret  = dwc3_count_phys(dwc, port);
> > > +			if (ret)
> > > +				return ret;
> > > +		}
> > > +	}
> > > +	dev_info(dwc->dev, "Num of HS and SS PHY are %u %u\n", dwc->num_usb2_phy,
> > > +									dwc->num_usb3_phy);
> > > +
> > > +	dwc->usb2_phy = devm_kzalloc(dwc->dev,
> > > +		sizeof(*dwc->usb2_phy) * dwc->num_usb2_phy, GFP_KERNEL);
> > > +	if (!dwc->usb2_phy)
> > > +		return -ENOMEM;
> > > +
> > > +	dwc->usb3_phy = devm_kzalloc(dwc->dev,
> > > +		sizeof(*dwc->usb3_phy) * dwc->num_usb3_phy, GFP_KERNEL);
> > > +	if (!dwc->usb3_phy)
> > > +		return -ENOMEM;
> > > +
> > > +	dwc->usb2_generic_phy = devm_kzalloc(dwc->dev,
> > > +		sizeof(*dwc->usb2_generic_phy) * dwc->num_usb2_phy, GFP_KERNEL);
> > > +	if (!dwc->usb2_generic_phy)
> > > +		return -ENOMEM;
> > > +
> > > +	dwc->usb3_generic_phy = devm_kzalloc(dwc->dev,
> > > +		sizeof(*dwc->usb3_generic_phy) * dwc->num_usb3_phy, GFP_KERNEL);
> > > +	if (!dwc->usb3_generic_phy)
> > > +		return -ENOMEM;
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static int dwc3_core_get_phy_by_node(struct dwc3 *dwc,
> > > +		struct device_node *lookup_node, int i)
> > >   {
> > >   	struct device		*dev = dwc->dev;
> > > -	struct device_node	*node = dev->of_node;
> > > -	int ret;
> > > +	int			ret;
> > > -	if (node) {
> > > -		dwc->usb2_phy = devm_usb_get_phy_by_phandle(dev, "usb-phy", 0);
> > > -		dwc->usb3_phy = devm_usb_get_phy_by_phandle(dev, "usb-phy", 1);
> > > +	if (lookup_node) {
> > > +		dwc->usb2_phy[i] = devm_of_usb_get_phy_by_phandle(dev,
> > > +								"usb-phy", 0, lookup_node);
> > > +		dwc->usb3_phy[i] = devm_of_usb_get_phy_by_phandle(dev,
> > > +								"usb-phy", 1, lookup_node);
> > >   	} else {
> > > -		dwc->usb2_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
> > > -		dwc->usb3_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3);
> > > +		dwc->usb2_phy[i] = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
> > > +		dwc->usb3_phy[i] = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3);
> > >   	}
> > > -	if (IS_ERR(dwc->usb2_phy)) {
> > > -		ret = PTR_ERR(dwc->usb2_phy);
> > > +	if (IS_ERR(dwc->usb2_phy[i])) {
> > > +		ret = PTR_ERR(dwc->usb2_phy[i]);
> > >   		if (ret == -ENXIO || ret == -ENODEV)
> > > -			dwc->usb2_phy = NULL;
> > > +			dwc->usb2_phy[i] = NULL;
> > >   		else
> > >   			return dev_err_probe(dev, ret, "no usb2 phy configured\n");
> > >   	}
> > > -	if (IS_ERR(dwc->usb3_phy)) {
> > > -		ret = PTR_ERR(dwc->usb3_phy);
> > > +	if (IS_ERR(dwc->usb3_phy[i])) {
> > > +		ret = PTR_ERR(dwc->usb3_phy[i]);
> > >   		if (ret == -ENXIO || ret == -ENODEV)
> > > -			dwc->usb3_phy = NULL;
> > > +			dwc->usb3_phy[i] = NULL;
> > >   		else
> > >   			return dev_err_probe(dev, ret, "no usb3 phy configured\n");
> > >   	}
> > > -	dwc->usb2_generic_phy = devm_phy_get(dev, "usb2-phy");
> > > -	if (IS_ERR(dwc->usb2_generic_phy)) {
> > > -		ret = PTR_ERR(dwc->usb2_generic_phy);
> > > -		if (ret == -ENOSYS || ret == -ENODEV)
> > > -			dwc->usb2_generic_phy = NULL;
> > > +	dwc->usb2_generic_phy[i] = devm_of_phy_get(dev, lookup_node, "usb2-phy");
> > > +	if (IS_ERR(dwc->usb2_generic_phy[i])) {
> > > +		ret = PTR_ERR(dwc->usb2_generic_phy[i]);
> > > +		if (ret == -ENODEV)
> > I think this can be -ENOSYS if !CONFIG_GENERIC_PHY
> For now, I have added it back
> > 
> > > +			dwc->usb2_generic_phy[i] = NULL;
> > >   		else
> > >   			return dev_err_probe(dev, ret, "no usb2 phy configured\n");
> > >   	}
> > > -	dwc->usb3_generic_phy = devm_phy_get(dev, "usb3-phy");
> > > -	if (IS_ERR(dwc->usb3_generic_phy)) {
> > > -		ret = PTR_ERR(dwc->usb3_generic_phy);
> > > -		if (ret == -ENOSYS || ret == -ENODEV)
> > > -			dwc->usb3_generic_phy = NULL;
> > > +	dwc->usb3_generic_phy[i] = devm_of_phy_get(dev, lookup_node, "usb3-phy");
> > > +	if (IS_ERR(dwc->usb3_generic_phy[i])) {
> > > +		ret = PTR_ERR(dwc->usb3_generic_phy[i]);
> > > +		if (ret == -ENODEV)
> > > +			dwc->usb3_generic_phy[i] = NULL;
> > >   		else
> > >   			return dev_err_probe(dev, ret, "no usb3 phy configured\n");
> > >   	}
> > > +	return 0;
> > > +}
> > > +
> > > +static int dwc3_core_get_phy(struct dwc3 *dwc)
> > > +{
> > > +	struct device		*dev = dwc->dev;
> > > +	struct device_node	*node = dev->of_node;
> > > +	struct device_node	*ports, *port;
> > > +	int ret, i = 0;
> > > +
> > > +	ret = dwc3_extract_num_phys(dwc);
> > > +	if (ret) {
> > > +		dev_err(dwc->dev, "Unable to extract number of PHYs\n");
> > > +		return ret;
> > > +	}
> > > +
> > > +	/* Find if any "multiport" child is present inside DWC3*/
> > Nit, space after DWC3
> Done
> > 
> > > +	for_each_available_child_of_node(node, ports) {
> > > +		if (!strcmp(ports->name, "multiport"))
> > > +			break;
> > > +	}
> > > +
> > > +	if (!ports) {
> > > +		ret = dwc3_core_get_phy_by_node(dwc, node, 0);
> > > +		if (ret)
> > > +			return ret;
> > > +	} else {
> > > +		for_each_available_child_of_node(ports, port) {
> > > +			ret = dwc3_core_get_phy_by_node(dwc, port, i);
> > > +			if (ret)
> > > +				return ret;
> > > +			i++;
> > > +		}
> > > +	}
> > >   	return 0;
> > >   }
> > > @@ -1305,16 +1462,16 @@ static int dwc3_core_get_phy(struct dwc3 *dwc)
> > >   static int dwc3_core_init_mode(struct dwc3 *dwc)
> > >   {
> > >   	struct device *dev = dwc->dev;
> > > -	int ret;
> > > +	int i, ret;
> > >   	switch (dwc->dr_mode) {
> > >   	case USB_DR_MODE_PERIPHERAL:
> > >   		dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_DEVICE);
> > > -		if (dwc->usb2_phy)
> > > -			otg_set_vbus(dwc->usb2_phy->otg, false);
> > > -		phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_DEVICE);
> > > -		phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_DEVICE);
> > > +		if (dwc->usb2_phy[0])
> > > +			otg_set_vbus(dwc->usb2_phy[0]->otg, false);
> > > +		phy_set_mode(dwc->usb2_generic_phy[0], PHY_MODE_USB_DEVICE);
> > > +		phy_set_mode(dwc->usb3_generic_phy[0], PHY_MODE_USB_DEVICE);
> > >   		ret = dwc3_gadget_init(dwc);
> > >   		if (ret)
> > > @@ -1323,10 +1480,15 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
> > >   	case USB_DR_MODE_HOST:
> > >   		dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_HOST);
> > > -		if (dwc->usb2_phy)
> > > -			otg_set_vbus(dwc->usb2_phy->otg, true);
> > > -		phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST);
> > > -		phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_HOST);
> > > +		for (i = 0; i < dwc->num_usb3_phy; i++) {
> > > +			if (dwc->usb2_phy[i])
> > > +				otg_set_vbus(dwc->usb2_phy[i]->otg, true);
> > > +			phy_set_mode(dwc->usb2_generic_phy[i], PHY_MODE_USB_HOST);
> > > +		}
> > > +
> > > +
> > > +		for (i = 0; i < dwc->num_usb3_phy; i++)
> > > +			phy_set_mode(dwc->usb3_generic_phy[i], PHY_MODE_USB_HOST);
> > >   		ret = dwc3_host_init(dwc);
> > >   		if (ret)
> > > @@ -1674,7 +1836,7 @@ static int dwc3_probe(struct platform_device *pdev)
> > >   	struct resource		*res, dwc_res;
> > >   	struct dwc3		*dwc;
> > > -	int			ret;
> > > +	int			ret, i;
> > >   	void __iomem		*regs;
> > > @@ -1839,15 +2001,18 @@ static int dwc3_probe(struct platform_device *pdev)
> > >   	dwc3_debugfs_exit(dwc);
> > >   	dwc3_event_buffers_cleanup(dwc);
> > > -	usb_phy_shutdown(dwc->usb2_phy);
> > > -	usb_phy_shutdown(dwc->usb3_phy);
> > > -	phy_exit(dwc->usb2_generic_phy);
> > > -	phy_exit(dwc->usb3_generic_phy);
> > > -
> > > -	usb_phy_set_suspend(dwc->usb2_phy, 1);
> > > -	usb_phy_set_suspend(dwc->usb3_phy, 1);
> > > -	phy_power_off(dwc->usb2_generic_phy);
> > > -	phy_power_off(dwc->usb3_generic_phy);
> > > +	for (i = 0; i < dwc->num_usb2_phy; i++) {
> > > +		usb_phy_shutdown(dwc->usb2_phy[i]);
> > > +		usb_phy_set_suspend(dwc->usb2_phy[i], 1);
> > > +		phy_exit(dwc->usb2_generic_phy[i]);
> > > +		phy_power_off(dwc->usb2_generic_phy[i]);
> > > +	}
> > > +	for (i = 0; i < dwc->num_usb3_phy; i++) {
> > > +		usb_phy_shutdown(dwc->usb3_phy[i]);
> > > +		usb_phy_set_suspend(dwc->usb3_phy[i], 1);
> > > +		phy_exit(dwc->usb3_generic_phy[i]);
> > > +		phy_power_off(dwc->usb3_generic_phy[i]);
> > > +	}
> > >   	dwc3_ulpi_exit(dwc);
> > > @@ -1929,6 +2094,7 @@ static int dwc3_core_init_for_resume(struct dwc3 *dwc)
> > >   static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
> > >   {
> > > +	int i;
> > >   	unsigned long	flags;
> > >   	u32 reg;
> > > @@ -1951,17 +2117,21 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
> > >   		/* Let controller to suspend HSPHY before PHY driver suspends */
> > >   		if (dwc->dis_u2_susphy_quirk ||
> > >   		    dwc->dis_enblslpm_quirk) {
> > > -			reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
> > > -			reg |=  DWC3_GUSB2PHYCFG_ENBLSLPM |
> > > -				DWC3_GUSB2PHYCFG_SUSPHY;
> > > -			dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
> > > -
> > > -			/* Give some time for USB2 PHY to suspend */
> > > -			usleep_range(5000, 6000);
> > > +			for (i = 0; i < dwc->num_usb2_phy; i++) {
> > > +				reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(i));
> > > +				reg |=  DWC3_GUSB2PHYCFG_ENBLSLPM |
> > > +					DWC3_GUSB2PHYCFG_SUSPHY;
> > > +				dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
> > > +
> > > +				/* Give some time for USB2 PHY to suspend */
> > > +				usleep_range(5000, 6000);
> > > +			}
> > >   		}
> > > -		phy_pm_runtime_put_sync(dwc->usb2_generic_phy);
> > > -		phy_pm_runtime_put_sync(dwc->usb3_generic_phy);
> > > +		for (i = 0; i < dwc->num_usb2_phy; i++)
> > > +			phy_pm_runtime_put_sync(dwc->usb2_generic_phy[i]);
> > > +		for (i = 0; i < dwc->num_usb3_phy; i++)
> > > +			phy_pm_runtime_put_sync(dwc->usb3_generic_phy[i]);
> > >   		break;
> > >   	case DWC3_GCTL_PRTCAP_OTG:
> > >   		/* do nothing during runtime_suspend */
> > > @@ -1989,7 +2159,7 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
> > >   static int dwc3_resume_common(struct dwc3 *dwc, pm_message_t msg)
> > >   {
> > >   	unsigned long	flags;
> > > -	int		ret;
> > > +	int		i, ret;
> > >   	u32		reg;
> > >   	switch (dwc->current_dr_role) {
> > > @@ -2012,17 +2182,21 @@ static int dwc3_resume_common(struct dwc3 *dwc, pm_message_t msg)
> > >   			break;
> > >   		}
> > >   		/* Restore GUSB2PHYCFG bits that were modified in suspend */
> > > -		reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
> > > -		if (dwc->dis_u2_susphy_quirk)
> > > -			reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
> > > +		for (i = 0; i < dwc->num_usb2_phy; i++) {
> > > +			reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(i));
> > > +			if (dwc->dis_u2_susphy_quirk)
> > > +				reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
> > > -		if (dwc->dis_enblslpm_quirk)
> > > -			reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;
> > > +			if (dwc->dis_enblslpm_quirk)
> > > +				reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;
> > > -		dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
> > > +			dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
> > > +		}
> > > -		phy_pm_runtime_get_sync(dwc->usb2_generic_phy);
> > > -		phy_pm_runtime_get_sync(dwc->usb3_generic_phy);
> > > +		for (i = 0; i < dwc->num_usb2_phy; i++)
> > > +			phy_pm_runtime_get_sync(dwc->usb2_generic_phy[i]);
> > > +		for (i = 0; i < dwc->num_usb3_phy; i++)
> > > +			phy_pm_runtime_get_sync(dwc->usb3_generic_phy[i]);
> > >   		break;
> > >   	case DWC3_GCTL_PRTCAP_OTG:
> > >   		/* nothing to do on runtime_resume */
> > > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> > > index 81c486b..c169bf1 100644
> > > --- a/drivers/usb/dwc3/core.h
> > > +++ b/drivers/usb/dwc3/core.h
> > > @@ -1020,6 +1020,8 @@ struct dwc3_scratchpad_array {
> > >    * @usb_psy: pointer to power supply interface.
> > >    * @usb2_phy: pointer to USB2 PHY
> > >    * @usb3_phy: pointer to USB3 PHY
> > > + * @num_usb2_phy: Number of HS ports controlled by the core
> > > + * @num_dsphy: Number of SS ports controlled by the core
> > s/num_dsphy/num_usb3_phy/
> Done
> > 
> > >    * @usb2_generic_phy: pointer to USB2 PHY
> > >    * @usb3_generic_phy: pointer to USB3 PHY
> > >    * @phys_ready: flag to indicate that PHYs are ready
> > > @@ -1147,11 +1149,13 @@ struct dwc3 {
> > >   	struct reset_control	*reset;
> > > -	struct usb_phy		*usb2_phy;
> > > -	struct usb_phy		*usb3_phy;
> > > +	struct usb_phy		**usb2_phy;
> > > +	struct usb_phy		**usb3_phy;
> > > +	u32			num_usb2_phy;
> > > +	u32			num_usb3_phy;
> > > -	struct phy		*usb2_generic_phy;
> > > -	struct phy		*usb3_generic_phy;
> > > +	struct phy		**usb2_generic_phy;
> > > +	struct phy		**usb3_generic_phy;
> > >   	bool			phys_ready;
> > > diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
> > > index 039bf24..404643f 100644
> > > --- a/drivers/usb/dwc3/drd.c
> > > +++ b/drivers/usb/dwc3/drd.c
> > > @@ -384,10 +384,10 @@ void dwc3_otg_update(struct dwc3 *dwc, bool ignore_idstatus)
> > >   		if (ret) {
> > >   			dev_err(dwc->dev, "failed to initialize host\n");
> > >   		} else {
> > > -			if (dwc->usb2_phy)
> > > -				otg_set_vbus(dwc->usb2_phy->otg, true);
> > > -			if (dwc->usb2_generic_phy)
> > > -				phy_set_mode(dwc->usb2_generic_phy,
> > > +			if (dwc->usb2_phy[0])
> > > +				otg_set_vbus(dwc->usb2_phy[0]->otg, true);
> > > +			if (dwc->usb2_generic_phy[0])
> > > +				phy_set_mode(dwc->usb2_generic_phy[0],
> > >   					     PHY_MODE_USB_HOST);
> > >   		}
> > >   		break;
> > > @@ -398,10 +398,10 @@ void dwc3_otg_update(struct dwc3 *dwc, bool ignore_idstatus)
> > >   		dwc3_event_buffers_setup(dwc);
> > >   		spin_unlock_irqrestore(&dwc->lock, flags);
> > > -		if (dwc->usb2_phy)
> > > -			otg_set_vbus(dwc->usb2_phy->otg, false);
> > > -		if (dwc->usb2_generic_phy)
> > > -			phy_set_mode(dwc->usb2_generic_phy,
> > > +		if (dwc->usb2_phy[0])
> > > +			otg_set_vbus(dwc->usb2_phy[0]->otg, false);
> > > +		if (dwc->usb2_generic_phy[0])
> > > +			phy_set_mode(dwc->usb2_generic_phy[0],
> > >   				     PHY_MODE_USB_DEVICE);
> > >   		ret = dwc3_gadget_init(dwc);
> > >   		if (ret)
> > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > > index 00427d1..e3b2a17 100644
> > > --- a/drivers/usb/dwc3/gadget.c
> > > +++ b/drivers/usb/dwc3/gadget.c
> > > @@ -2872,8 +2872,8 @@ static int dwc3_gadget_vbus_draw(struct usb_gadget *g, unsigned int mA)
> > >   	union power_supply_propval	val = {0};
> > >   	int				ret;
> > > -	if (dwc->usb2_phy)
> > > -		return usb_phy_set_power(dwc->usb2_phy, mA);
> > > +	if (dwc->usb2_phy[0])
> > > +		return usb_phy_set_power(dwc->usb2_phy[0], mA);
> > >   	if (!dwc->usb_psy)
> > >   		return -EOPNOTSUPP;
> > > -- 
> > > 2.7.4
> > > 
> 

Re: [PATCH 3/3] usb: dwc3: Refactor PHY logic to support Multiport Controller

[Harsh Agarwal]

On 6/6/2022 6:34 PM, Andrew Halaney wrote:
> On Sun, Jun 05, 2022 at 11:51:38PM +0530, Harsh Agarwal wrote:
>> On 6/2/2022 1:23 AM, Andrew Halaney wrote:
>>> On Tue, May 31, 2022 at 01:50:17PM +0530, Harsh Agarwal wrote:
>>>> Currently the DWC3 driver supports only single port controller
>>>> which requires at most 2 PHYs ie HS and SS PHYs.
>>>>
>>>> But some SOCs have a "multiport" USB DWC3 controller where a
>>>> single controller supports multiple ports and each port have
>>>> their own PHYs. Refactor PHY logic to support the same.
>>>>
>>>> Signed-off-by: Harsh Agarwal <quic_harshq@quicinc.com>
>>>> ---
>>>>    drivers/usb/dwc3/core.c   | 400 +++++++++++++++++++++++++++++++++-------------
>>>>    drivers/usb/dwc3/core.h   |  12 +-
>>>>    drivers/usb/dwc3/drd.c    |  16 +-
>>>>    drivers/usb/dwc3/gadget.c |   4 +-
>>>>    4 files changed, 305 insertions(+), 127 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>>> index 5734219..5cc799e 100644
>>>> --- a/drivers/usb/dwc3/core.c
>>>> +++ b/drivers/usb/dwc3/core.c
>>>> @@ -120,7 +120,7 @@ static void __dwc3_set_mode(struct work_struct *work)
>>>>    {
>>>>    	struct dwc3 *dwc = work_to_dwc(work);
>>>>    	unsigned long flags;
>>>> -	int ret;
>>>> +	int i, ret;
>>>>    	u32 reg;
>>>>    	mutex_lock(&dwc->mutex);
>>>> @@ -189,10 +189,13 @@ static void __dwc3_set_mode(struct work_struct *work)
>>>>    		if (ret) {
>>>>    			dev_err(dwc->dev, "failed to initialize host\n");
>>>>    		} else {
>>>> -			if (dwc->usb2_phy)
>>>> -				otg_set_vbus(dwc->usb2_phy->otg, true);
>>>> -			phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST);
>>>> -			phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_HOST);
>>>> +			for (i = 0; i < dwc->num_usb2_phy; i++) {
>>>> +				if (dwc->usb2_phy[i])
>>>> +					otg_set_vbus(dwc->usb2_phy[i]->otg, true);
>>>> +				phy_set_mode(dwc->usb2_generic_phy[i], PHY_MODE_USB_HOST);
>>>> +			}
>>>> +			for (i = 0; i < dwc->num_usb3_phy; i++)
>>>> +				phy_set_mode(dwc->usb3_generic_phy[i], PHY_MODE_USB_HOST);
>>>>    			if (dwc->dis_split_quirk) {
>>>>    				reg = dwc3_readl(dwc->regs, DWC3_GUCTL3);
>>>>    				reg |= DWC3_GUCTL3_SPLITDISABLE;
>>>> @@ -205,10 +208,10 @@ static void __dwc3_set_mode(struct work_struct *work)
>>>>    		dwc3_event_buffers_setup(dwc);
>>>> -		if (dwc->usb2_phy)
>>>> -			otg_set_vbus(dwc->usb2_phy->otg, false);
>>>> -		phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_DEVICE);
>>>> -		phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_DEVICE);
>>>> +		if (dwc->usb2_phy[0])
>>>> +			otg_set_vbus(dwc->usb2_phy[0]->otg, false);
>>>> +		phy_set_mode(dwc->usb2_generic_phy[0], PHY_MODE_USB_DEVICE);
>>>> +		phy_set_mode(dwc->usb3_generic_phy[0], PHY_MODE_USB_DEVICE);
>>>>    		ret = dwc3_gadget_init(dwc);
>>>>    		if (ret)
>>>> @@ -656,6 +659,7 @@ static int dwc3_core_ulpi_init(struct dwc3 *dwc)
>>>>     */
>>>>    static int dwc3_phy_setup(struct dwc3 *dwc)
>>>>    {
>>>> +	int i;
>>>>    	unsigned int hw_mode;
>>>>    	u32 reg;
>>>> @@ -716,7 +720,8 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
>>>>    	if (dwc->dis_del_phy_power_chg_quirk)
>>>>    		reg &= ~DWC3_GUSB3PIPECTL_DEPOCHANGE;
>>>> -	dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg);
>>>> +	for (i = 0; i < dwc->num_usb3_phy; i++)
>>>> +		dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(i), reg);
>>>>    	reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
>>>> @@ -730,7 +735,8 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
>>>>    		} else if (dwc->hsphy_interface &&
>>>>    				!strncmp(dwc->hsphy_interface, "ulpi", 4)) {
>>>>    			reg |= DWC3_GUSB2PHYCFG_ULPI_UTMI;
>>>> -			dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>>>> +			for (i = 0; i < dwc->num_usb2_phy; i++)
>>>> +				dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
>>>>    		} else {
>>>>    			/* Relying on default value. */
>>>>    			if (!(reg & DWC3_GUSB2PHYCFG_ULPI_UTMI))
>>>> @@ -787,7 +793,8 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
>>>>    	if (dwc->dis_u2_freeclk_exists_quirk)
>>>>    		reg &= ~DWC3_GUSB2PHYCFG_U2_FREECLK_EXISTS;
>>>> -	dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>>>> +	for (i = 0; i < dwc->num_usb2_phy; i++)
>>>> +		dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
>>>>    	return 0;
>>>>    }
>>>> @@ -826,17 +833,23 @@ static void dwc3_clk_disable(struct dwc3 *dwc)
>>>>    static void dwc3_core_exit(struct dwc3 *dwc)
>>>>    {
>>>> +	int i;
>>>>    	dwc3_event_buffers_cleanup(dwc);
>>>> -	usb_phy_shutdown(dwc->usb2_phy);
>>>> -	usb_phy_shutdown(dwc->usb3_phy);
>>>> -	phy_exit(dwc->usb2_generic_phy);
>>>> -	phy_exit(dwc->usb3_generic_phy);
>>>> +	for (i = 0; i < dwc->num_usb2_phy; i++) {
>>>> +		usb_phy_shutdown(dwc->usb2_phy[i]);
>>>> +		usb_phy_set_suspend(dwc->usb2_phy[i], 1);
>>>> +		phy_exit(dwc->usb2_generic_phy[i]);
>>>> +		phy_power_off(dwc->usb2_generic_phy[i]);
>>>> +	}
>>>> +
>>>> +	for (i = 0; i < dwc->num_usb3_phy; i++) {
>>>> +		usb_phy_shutdown(dwc->usb3_phy[i]);
>>>> +		usb_phy_set_suspend(dwc->usb3_phy[i], 1);
>>>> +		phy_exit(dwc->usb3_generic_phy[i]);
>>>> +		phy_power_off(dwc->usb3_generic_phy[i]);
>>>> +	}
>>>> -	usb_phy_set_suspend(dwc->usb2_phy, 1);
>>>> -	usb_phy_set_suspend(dwc->usb3_phy, 1);
>>>> -	phy_power_off(dwc->usb2_generic_phy);
>>>> -	phy_power_off(dwc->usb3_generic_phy);
>>>>    	dwc3_clk_disable(dwc);
>>>>    	reset_control_assert(dwc->reset);
>>>>    }
>>>> @@ -1039,7 +1052,7 @@ static int dwc3_core_init(struct dwc3 *dwc)
>>>>    {
>>>>    	unsigned int		hw_mode;
>>>>    	u32			reg;
>>>> -	int			ret;
>>>> +	int			ret, i;
>>>>    	hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
>>>> @@ -1067,16 +1080,24 @@ static int dwc3_core_init(struct dwc3 *dwc)
>>>>    		dwc->phys_ready = true;
>>>>    	}
>>>> -	usb_phy_init(dwc->usb2_phy);
>>>> -	usb_phy_init(dwc->usb3_phy);
>>>> -	ret = phy_init(dwc->usb2_generic_phy);
>>>> -	if (ret < 0)
>>>> -		goto err0a;
>>>> +	for (i = 0; i < dwc->num_usb2_phy; i++)
>>>> +		usb_phy_init(dwc->usb2_phy[i]);
>>>> +	for (i = 0; i < dwc->num_usb3_phy; i++)
>>>> +		usb_phy_init(dwc->usb3_phy[i]);
>>>> -	ret = phy_init(dwc->usb3_generic_phy);
>>>> -	if (ret < 0) {
>>>> -		phy_exit(dwc->usb2_generic_phy);
>>>> -		goto err0a;
>>>> +	for (i = 0; i < dwc->num_usb2_phy; i++) {
>>>> +		ret = phy_init(dwc->usb2_generic_phy[i]);
>>>> +		if (ret < 0)
>>>> +			goto err0a;
>>> Should we clean up the prior phys that did not fail? There's a couple
>>> loops where that question stands in this patch.
>> I am just keeping the Generic PHY implementation as is.
>> Earlier also we were cleaning up the HS PHY incase
>> SS PHY fails, so keeping the same for Multiport.
>> For you please point out if you find any major concern here?
> Earlier, there was only one usb2_generic_phy/usb3_generic_phy, so if
> phy_init(usb2_generic_phy) returned an error, it wasn't initialized, and
> it did not need a matching call to phy_exit(usb2_generic_phy).
>
> That is being called though ifthe superspeed phy fails since the
> highspeed one did successfully init, so things are cleaned up in the
> old cases.
>
> My point now is that, say with 4 highspeed phys, the 3rd one fails.
> i.e. phy_init(usb2_generic_phy[i]) where i == 2. In that case we don't
> ever call phy_exit() on the first two phys. I wouldn't say it is a
> major concern, but cleanup here is not fully done.
>
> You could do something like this to do clean it all up, kind of annoying
> though:
>
> 	for (i = 0; i < dwc->num_usb2_phy; i++) {
> 		ret = phy_init(dwc->usb2_generic_phy[i]);
> 		if (ret < 0) {
> 			/* clean up prior phys we initialized */
> 			for (j = 0; j < i; j++)
> 				phy_exit(usb2_generic_phy[j]);
> 			goto err0a;
> 		}
> 	}
>
> Thanks,
> Andrew
yes agree with your analysis.
Also, if any SSPHY init fails, then clear already initialized SSPHYs and
HSPHYs.

I am thinking instead of adding all this logic, we can just do "goto err1"
instead of "goto err0a".

Similar approach if usb_phy_init fails ( Non-Generic phys)
>
>>>> +	}
>>>> +
>>>> +	for (i = 0; i < dwc->num_usb3_phy; i++) {
>>>> +		ret = phy_init(dwc->usb3_generic_phy[i]);
>>>> +		if (ret < 0) {
>>>> +			for (i = 0; i < dwc->num_usb2_phy; i++)
>>>> +				phy_exit(dwc->usb2_generic_phy[i]);
>>>> +			goto err0a;
>>>> +		}
>>>>    	}
>>>>    	ret = dwc3_core_soft_reset(dwc);
>>>> @@ -1086,15 +1107,19 @@ static int dwc3_core_init(struct dwc3 *dwc)
>>>>    	if (hw_mode == DWC3_GHWPARAMS0_MODE_DRD &&
>>>>    	    !DWC3_VER_IS_WITHIN(DWC3, ANY, 194A)) {
>>>>    		if (!dwc->dis_u3_susphy_quirk) {
>>>> -			reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0));
>>>> -			reg |= DWC3_GUSB3PIPECTL_SUSPHY;
>>>> -			dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg);
>>>> +			for (i = 0; i < dwc->num_usb3_phy; i++) {
>>>> +				reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(i));
>>>> +				reg |= DWC3_GUSB3PIPECTL_SUSPHY;
>>>> +				dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(i), reg);
>>>> +			}
>>>>    		}
>>>>    		if (!dwc->dis_u2_susphy_quirk) {
>>>> -			reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
>>>> -			reg |= DWC3_GUSB2PHYCFG_SUSPHY;
>>>> -			dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>>>> +			for (i = 0; i < dwc->num_usb2_phy; i++) {
>>>> +				reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(i));
>>>> +				reg |= DWC3_GUSB2PHYCFG_SUSPHY;
>>>> +				dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
>>>> +			}
>>>>    		}
>>>>    	}
>>>> @@ -1113,15 +1138,19 @@ static int dwc3_core_init(struct dwc3 *dwc)
>>>>    	dwc3_set_incr_burst_type(dwc);
>>>> -	usb_phy_set_suspend(dwc->usb2_phy, 0);
>>>> -	usb_phy_set_suspend(dwc->usb3_phy, 0);
>>>> -	ret = phy_power_on(dwc->usb2_generic_phy);
>>>> -	if (ret < 0)
>>>> -		goto err2;
>>>> +	for (i = 0; i < dwc->num_usb2_phy; i++) {
>>>> +		usb_phy_set_suspend(dwc->usb2_phy[i], 0);
>>>> +		ret = phy_power_on(dwc->usb2_generic_phy[i]);
>>>> +		if (ret < 0)
>>>> +			goto err2;
>>>> +	}
>>>> -	ret = phy_power_on(dwc->usb3_generic_phy);
>>>> -	if (ret < 0)
>>>> -		goto err3;
>>>> +	for (i = 0; i < dwc->num_usb3_phy; i++) {
>>>> +		usb_phy_set_suspend(dwc->usb3_phy[i], 0);
>>>> +		ret = phy_power_on(dwc->usb3_generic_phy[i]);
>>>> +		if (ret < 0)
>>>> +			goto err3;
>>>> +	}
>>>>    	ret = dwc3_event_buffers_setup(dwc);
>>>>    	if (ret) {
>>>> @@ -1229,20 +1258,29 @@ static int dwc3_core_init(struct dwc3 *dwc)
>>>>    	return 0;
>>>>    err4:
>>>> -	phy_power_off(dwc->usb3_generic_phy);
>>>> +	for (i = 0; i < dwc->num_usb3_phy; i++)
>>>> +		phy_power_off(dwc->usb3_generic_phy[i]);
>>>>    err3:
>>>> -	phy_power_off(dwc->usb2_generic_phy);
>>>> +	for (i = 0; i < dwc->num_usb2_phy; i++)
>>>> +		phy_power_off(dwc->usb2_generic_phy[i]);
>>>>    err2:
>>>> -	usb_phy_set_suspend(dwc->usb2_phy, 1);
>>>> -	usb_phy_set_suspend(dwc->usb3_phy, 1);
>>>> +	for (i = 0; i < dwc->num_usb2_phy; i++)
>>>> +		usb_phy_set_suspend(dwc->usb2_phy[i], 1);
>>>> +	for (i = 0; i < dwc->num_usb3_phy; i++)
>>>> +		usb_phy_set_suspend(dwc->usb3_phy[i], 1);
>>>>    err1:
>>>> -	usb_phy_shutdown(dwc->usb2_phy);
>>>> -	usb_phy_shutdown(dwc->usb3_phy);
>>>> -	phy_exit(dwc->usb2_generic_phy);
>>>> -	phy_exit(dwc->usb3_generic_phy);
>>>> +	for (i = 0; i < dwc->num_usb2_phy; i++) {
>>>> +		usb_phy_shutdown(dwc->usb2_phy[i]);
>>>> +		phy_exit(dwc->usb2_generic_phy[i]);
>>>> +	}
>>>> +
>>>> +	for (i = 0; i < dwc->num_usb3_phy; i++) {
>>>> +		usb_phy_shutdown(dwc->usb3_phy[i]);
>>>> +		phy_exit(dwc->usb3_generic_phy[i]);
>>>> +	}
>>>>    err0a:
>>>>    	dwc3_ulpi_exit(dwc);
>>>> @@ -1251,53 +1289,172 @@ static int dwc3_core_init(struct dwc3 *dwc)
>>>>    	return ret;
>>>>    }
>>>> -static int dwc3_core_get_phy(struct dwc3 *dwc)
>>>> +static struct usb_phy *dwc3_core_get_phy_by_handle_with_node(struct device *dev,
>>>> +	const char *phandle, u8 index, struct device_node *lookup_node)
>>>> +{
>>>> +	struct device_node *node;
>>>> +	struct usb_phy	*phy;
>>>> +
>>>> +	node = of_parse_phandle(lookup_node, phandle, index);
>>>> +	if (!node) {
>>>> +		dev_err(dev, "failed to get %s phandle in %pOF node\n", phandle,
>>>> +			dev->of_node);
>>>> +		return ERR_PTR(-ENODEV);
>>>> +	}
>>>> +	phy = devm_usb_get_phy_by_node(dev, node, NULL);
>>>> +	of_node_put(node);
>>>> +	return phy;
>>>> +}
>>>> +
>>>> +static int dwc3_count_phys(struct dwc3 *dwc, struct device_node *lookup_node)
>>>> +{
>>>> +	int count;
>>>> +
>>>> +	count = of_count_phandle_with_args(lookup_node, "phys", NULL);
>>>> +
>>>> +	if (count == -ENOENT)
>>>> +		count = of_count_phandle_with_args(lookup_node, "usb-phy", NULL);
>>>> +
>>>> +	if (count == 1) {
>>>> +		dwc->num_usb2_phy++;
>>>> +	} else if (count == 2) {
>>>> +		dwc->num_usb2_phy++;
>>>> +		dwc->num_usb3_phy++;
>>>> +	} else {
>>>> +		return count;
>>>> +	}
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static int dwc3_extract_num_phys(struct dwc3 *dwc)
>>>> +{
>>>> +	struct device_node	*ports, *port;
>>>> +	int			ret;
>>>> +
>>>> +	/* Find if any "multiport" child is present inside DWC3*/
>>> Nit, space after DWC3
>> Done
>>>> +	for_each_available_child_of_node(dwc->dev->of_node, ports) {
>>>> +		if (!strcmp(ports->name, "multiport"))
>>>> +			break;
>>>> +	}
>>>> +	if (!ports) {
>>>> +		dwc->num_usb2_phy = 1;
>>>> +		dwc->num_usb3_phy = 1;
>>>> +	} else {
>>>> +		for_each_available_child_of_node(ports, port) {
>>>> +			ret  = dwc3_count_phys(dwc, port);
>>>> +			if (ret)
>>>> +				return ret;
>>>> +		}
>>>> +	}
>>>> +	dev_info(dwc->dev, "Num of HS and SS PHY are %u %u\n", dwc->num_usb2_phy,
>>>> +									dwc->num_usb3_phy);
>>>> +
>>>> +	dwc->usb2_phy = devm_kzalloc(dwc->dev,
>>>> +		sizeof(*dwc->usb2_phy) * dwc->num_usb2_phy, GFP_KERNEL);
>>>> +	if (!dwc->usb2_phy)
>>>> +		return -ENOMEM;
>>>> +
>>>> +	dwc->usb3_phy = devm_kzalloc(dwc->dev,
>>>> +		sizeof(*dwc->usb3_phy) * dwc->num_usb3_phy, GFP_KERNEL);
>>>> +	if (!dwc->usb3_phy)
>>>> +		return -ENOMEM;
>>>> +
>>>> +	dwc->usb2_generic_phy = devm_kzalloc(dwc->dev,
>>>> +		sizeof(*dwc->usb2_generic_phy) * dwc->num_usb2_phy, GFP_KERNEL);
>>>> +	if (!dwc->usb2_generic_phy)
>>>> +		return -ENOMEM;
>>>> +
>>>> +	dwc->usb3_generic_phy = devm_kzalloc(dwc->dev,
>>>> +		sizeof(*dwc->usb3_generic_phy) * dwc->num_usb3_phy, GFP_KERNEL);
>>>> +	if (!dwc->usb3_generic_phy)
>>>> +		return -ENOMEM;
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static int dwc3_core_get_phy_by_node(struct dwc3 *dwc,
>>>> +		struct device_node *lookup_node, int i)
>>>>    {
>>>>    	struct device		*dev = dwc->dev;
>>>> -	struct device_node	*node = dev->of_node;
>>>> -	int ret;
>>>> +	int			ret;
>>>> -	if (node) {
>>>> -		dwc->usb2_phy = devm_usb_get_phy_by_phandle(dev, "usb-phy", 0);
>>>> -		dwc->usb3_phy = devm_usb_get_phy_by_phandle(dev, "usb-phy", 1);
>>>> +	if (lookup_node) {
>>>> +		dwc->usb2_phy[i] = devm_of_usb_get_phy_by_phandle(dev,
>>>> +								"usb-phy", 0, lookup_node);
>>>> +		dwc->usb3_phy[i] = devm_of_usb_get_phy_by_phandle(dev,
>>>> +								"usb-phy", 1, lookup_node);
>>>>    	} else {
>>>> -		dwc->usb2_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
>>>> -		dwc->usb3_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3);
>>>> +		dwc->usb2_phy[i] = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
>>>> +		dwc->usb3_phy[i] = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3);
>>>>    	}
>>>> -	if (IS_ERR(dwc->usb2_phy)) {
>>>> -		ret = PTR_ERR(dwc->usb2_phy);
>>>> +	if (IS_ERR(dwc->usb2_phy[i])) {
>>>> +		ret = PTR_ERR(dwc->usb2_phy[i]);
>>>>    		if (ret == -ENXIO || ret == -ENODEV)
>>>> -			dwc->usb2_phy = NULL;
>>>> +			dwc->usb2_phy[i] = NULL;
>>>>    		else
>>>>    			return dev_err_probe(dev, ret, "no usb2 phy configured\n");
>>>>    	}
>>>> -	if (IS_ERR(dwc->usb3_phy)) {
>>>> -		ret = PTR_ERR(dwc->usb3_phy);
>>>> +	if (IS_ERR(dwc->usb3_phy[i])) {
>>>> +		ret = PTR_ERR(dwc->usb3_phy[i]);
>>>>    		if (ret == -ENXIO || ret == -ENODEV)
>>>> -			dwc->usb3_phy = NULL;
>>>> +			dwc->usb3_phy[i] = NULL;
>>>>    		else
>>>>    			return dev_err_probe(dev, ret, "no usb3 phy configured\n");
>>>>    	}
>>>> -	dwc->usb2_generic_phy = devm_phy_get(dev, "usb2-phy");
>>>> -	if (IS_ERR(dwc->usb2_generic_phy)) {
>>>> -		ret = PTR_ERR(dwc->usb2_generic_phy);
>>>> -		if (ret == -ENOSYS || ret == -ENODEV)
>>>> -			dwc->usb2_generic_phy = NULL;
>>>> +	dwc->usb2_generic_phy[i] = devm_of_phy_get(dev, lookup_node, "usb2-phy");
>>>> +	if (IS_ERR(dwc->usb2_generic_phy[i])) {
>>>> +		ret = PTR_ERR(dwc->usb2_generic_phy[i]);
>>>> +		if (ret == -ENODEV)
>>> I think this can be -ENOSYS if !CONFIG_GENERIC_PHY
>> For now, I have added it back
>>>> +			dwc->usb2_generic_phy[i] = NULL;
>>>>    		else
>>>>    			return dev_err_probe(dev, ret, "no usb2 phy configured\n");
>>>>    	}
>>>> -	dwc->usb3_generic_phy = devm_phy_get(dev, "usb3-phy");
>>>> -	if (IS_ERR(dwc->usb3_generic_phy)) {
>>>> -		ret = PTR_ERR(dwc->usb3_generic_phy);
>>>> -		if (ret == -ENOSYS || ret == -ENODEV)
>>>> -			dwc->usb3_generic_phy = NULL;
>>>> +	dwc->usb3_generic_phy[i] = devm_of_phy_get(dev, lookup_node, "usb3-phy");
>>>> +	if (IS_ERR(dwc->usb3_generic_phy[i])) {
>>>> +		ret = PTR_ERR(dwc->usb3_generic_phy[i]);
>>>> +		if (ret == -ENODEV)
>>>> +			dwc->usb3_generic_phy[i] = NULL;
>>>>    		else
>>>>    			return dev_err_probe(dev, ret, "no usb3 phy configured\n");
>>>>    	}
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static int dwc3_core_get_phy(struct dwc3 *dwc)
>>>> +{
>>>> +	struct device		*dev = dwc->dev;
>>>> +	struct device_node	*node = dev->of_node;
>>>> +	struct device_node	*ports, *port;
>>>> +	int ret, i = 0;
>>>> +
>>>> +	ret = dwc3_extract_num_phys(dwc);
>>>> +	if (ret) {
>>>> +		dev_err(dwc->dev, "Unable to extract number of PHYs\n");
>>>> +		return ret;
>>>> +	}
>>>> +
>>>> +	/* Find if any "multiport" child is present inside DWC3*/
>>> Nit, space after DWC3
>> Done
>>>> +	for_each_available_child_of_node(node, ports) {
>>>> +		if (!strcmp(ports->name, "multiport"))
>>>> +			break;
>>>> +	}
>>>> +
>>>> +	if (!ports) {
>>>> +		ret = dwc3_core_get_phy_by_node(dwc, node, 0);
>>>> +		if (ret)
>>>> +			return ret;
>>>> +	} else {
>>>> +		for_each_available_child_of_node(ports, port) {
>>>> +			ret = dwc3_core_get_phy_by_node(dwc, port, i);
>>>> +			if (ret)
>>>> +				return ret;
>>>> +			i++;
>>>> +		}
>>>> +	}
>>>>    	return 0;
>>>>    }
>>>> @@ -1305,16 +1462,16 @@ static int dwc3_core_get_phy(struct dwc3 *dwc)
>>>>    static int dwc3_core_init_mode(struct dwc3 *dwc)
>>>>    {
>>>>    	struct device *dev = dwc->dev;
>>>> -	int ret;
>>>> +	int i, ret;
>>>>    	switch (dwc->dr_mode) {
>>>>    	case USB_DR_MODE_PERIPHERAL:
>>>>    		dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_DEVICE);
>>>> -		if (dwc->usb2_phy)
>>>> -			otg_set_vbus(dwc->usb2_phy->otg, false);
>>>> -		phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_DEVICE);
>>>> -		phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_DEVICE);
>>>> +		if (dwc->usb2_phy[0])
>>>> +			otg_set_vbus(dwc->usb2_phy[0]->otg, false);
>>>> +		phy_set_mode(dwc->usb2_generic_phy[0], PHY_MODE_USB_DEVICE);
>>>> +		phy_set_mode(dwc->usb3_generic_phy[0], PHY_MODE_USB_DEVICE);
>>>>    		ret = dwc3_gadget_init(dwc);
>>>>    		if (ret)
>>>> @@ -1323,10 +1480,15 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
>>>>    	case USB_DR_MODE_HOST:
>>>>    		dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_HOST);
>>>> -		if (dwc->usb2_phy)
>>>> -			otg_set_vbus(dwc->usb2_phy->otg, true);
>>>> -		phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST);
>>>> -		phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_HOST);
>>>> +		for (i = 0; i < dwc->num_usb3_phy; i++) {
>>>> +			if (dwc->usb2_phy[i])
>>>> +				otg_set_vbus(dwc->usb2_phy[i]->otg, true);
>>>> +			phy_set_mode(dwc->usb2_generic_phy[i], PHY_MODE_USB_HOST);
>>>> +		}
>>>> +
>>>> +
>>>> +		for (i = 0; i < dwc->num_usb3_phy; i++)
>>>> +			phy_set_mode(dwc->usb3_generic_phy[i], PHY_MODE_USB_HOST);
>>>>    		ret = dwc3_host_init(dwc);
>>>>    		if (ret)
>>>> @@ -1674,7 +1836,7 @@ static int dwc3_probe(struct platform_device *pdev)
>>>>    	struct resource		*res, dwc_res;
>>>>    	struct dwc3		*dwc;
>>>> -	int			ret;
>>>> +	int			ret, i;
>>>>    	void __iomem		*regs;
>>>> @@ -1839,15 +2001,18 @@ static int dwc3_probe(struct platform_device *pdev)
>>>>    	dwc3_debugfs_exit(dwc);
>>>>    	dwc3_event_buffers_cleanup(dwc);
>>>> -	usb_phy_shutdown(dwc->usb2_phy);
>>>> -	usb_phy_shutdown(dwc->usb3_phy);
>>>> -	phy_exit(dwc->usb2_generic_phy);
>>>> -	phy_exit(dwc->usb3_generic_phy);
>>>> -
>>>> -	usb_phy_set_suspend(dwc->usb2_phy, 1);
>>>> -	usb_phy_set_suspend(dwc->usb3_phy, 1);
>>>> -	phy_power_off(dwc->usb2_generic_phy);
>>>> -	phy_power_off(dwc->usb3_generic_phy);
>>>> +	for (i = 0; i < dwc->num_usb2_phy; i++) {
>>>> +		usb_phy_shutdown(dwc->usb2_phy[i]);
>>>> +		usb_phy_set_suspend(dwc->usb2_phy[i], 1);
>>>> +		phy_exit(dwc->usb2_generic_phy[i]);
>>>> +		phy_power_off(dwc->usb2_generic_phy[i]);
>>>> +	}
>>>> +	for (i = 0; i < dwc->num_usb3_phy; i++) {
>>>> +		usb_phy_shutdown(dwc->usb3_phy[i]);
>>>> +		usb_phy_set_suspend(dwc->usb3_phy[i], 1);
>>>> +		phy_exit(dwc->usb3_generic_phy[i]);
>>>> +		phy_power_off(dwc->usb3_generic_phy[i]);
>>>> +	}
>>>>    	dwc3_ulpi_exit(dwc);
>>>> @@ -1929,6 +2094,7 @@ static int dwc3_core_init_for_resume(struct dwc3 *dwc)
>>>>    static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
>>>>    {
>>>> +	int i;
>>>>    	unsigned long	flags;
>>>>    	u32 reg;
>>>> @@ -1951,17 +2117,21 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
>>>>    		/* Let controller to suspend HSPHY before PHY driver suspends */
>>>>    		if (dwc->dis_u2_susphy_quirk ||
>>>>    		    dwc->dis_enblslpm_quirk) {
>>>> -			reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
>>>> -			reg |=  DWC3_GUSB2PHYCFG_ENBLSLPM |
>>>> -				DWC3_GUSB2PHYCFG_SUSPHY;
>>>> -			dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>>>> -
>>>> -			/* Give some time for USB2 PHY to suspend */
>>>> -			usleep_range(5000, 6000);
>>>> +			for (i = 0; i < dwc->num_usb2_phy; i++) {
>>>> +				reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(i));
>>>> +				reg |=  DWC3_GUSB2PHYCFG_ENBLSLPM |
>>>> +					DWC3_GUSB2PHYCFG_SUSPHY;
>>>> +				dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
>>>> +
>>>> +				/* Give some time for USB2 PHY to suspend */
>>>> +				usleep_range(5000, 6000);
>>>> +			}
>>>>    		}
>>>> -		phy_pm_runtime_put_sync(dwc->usb2_generic_phy);
>>>> -		phy_pm_runtime_put_sync(dwc->usb3_generic_phy);
>>>> +		for (i = 0; i < dwc->num_usb2_phy; i++)
>>>> +			phy_pm_runtime_put_sync(dwc->usb2_generic_phy[i]);
>>>> +		for (i = 0; i < dwc->num_usb3_phy; i++)
>>>> +			phy_pm_runtime_put_sync(dwc->usb3_generic_phy[i]);
>>>>    		break;
>>>>    	case DWC3_GCTL_PRTCAP_OTG:
>>>>    		/* do nothing during runtime_suspend */
>>>> @@ -1989,7 +2159,7 @@ static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
>>>>    static int dwc3_resume_common(struct dwc3 *dwc, pm_message_t msg)
>>>>    {
>>>>    	unsigned long	flags;
>>>> -	int		ret;
>>>> +	int		i, ret;
>>>>    	u32		reg;
>>>>    	switch (dwc->current_dr_role) {
>>>> @@ -2012,17 +2182,21 @@ static int dwc3_resume_common(struct dwc3 *dwc, pm_message_t msg)
>>>>    			break;
>>>>    		}
>>>>    		/* Restore GUSB2PHYCFG bits that were modified in suspend */
>>>> -		reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
>>>> -		if (dwc->dis_u2_susphy_quirk)
>>>> -			reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
>>>> +		for (i = 0; i < dwc->num_usb2_phy; i++) {
>>>> +			reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(i));
>>>> +			if (dwc->dis_u2_susphy_quirk)
>>>> +				reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
>>>> -		if (dwc->dis_enblslpm_quirk)
>>>> -			reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;
>>>> +			if (dwc->dis_enblslpm_quirk)
>>>> +				reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;
>>>> -		dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>>>> +			dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
>>>> +		}
>>>> -		phy_pm_runtime_get_sync(dwc->usb2_generic_phy);
>>>> -		phy_pm_runtime_get_sync(dwc->usb3_generic_phy);
>>>> +		for (i = 0; i < dwc->num_usb2_phy; i++)
>>>> +			phy_pm_runtime_get_sync(dwc->usb2_generic_phy[i]);
>>>> +		for (i = 0; i < dwc->num_usb3_phy; i++)
>>>> +			phy_pm_runtime_get_sync(dwc->usb3_generic_phy[i]);
>>>>    		break;
>>>>    	case DWC3_GCTL_PRTCAP_OTG:
>>>>    		/* nothing to do on runtime_resume */
>>>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>>>> index 81c486b..c169bf1 100644
>>>> --- a/drivers/usb/dwc3/core.h
>>>> +++ b/drivers/usb/dwc3/core.h
>>>> @@ -1020,6 +1020,8 @@ struct dwc3_scratchpad_array {
>>>>     * @usb_psy: pointer to power supply interface.
>>>>     * @usb2_phy: pointer to USB2 PHY
>>>>     * @usb3_phy: pointer to USB3 PHY
>>>> + * @num_usb2_phy: Number of HS ports controlled by the core
>>>> + * @num_dsphy: Number of SS ports controlled by the core
>>> s/num_dsphy/num_usb3_phy/
>> Done
>>>>     * @usb2_generic_phy: pointer to USB2 PHY
>>>>     * @usb3_generic_phy: pointer to USB3 PHY
>>>>     * @phys_ready: flag to indicate that PHYs are ready
>>>> @@ -1147,11 +1149,13 @@ struct dwc3 {
>>>>    	struct reset_control	*reset;
>>>> -	struct usb_phy		*usb2_phy;
>>>> -	struct usb_phy		*usb3_phy;
>>>> +	struct usb_phy		**usb2_phy;
>>>> +	struct usb_phy		**usb3_phy;
>>>> +	u32			num_usb2_phy;
>>>> +	u32			num_usb3_phy;
>>>> -	struct phy		*usb2_generic_phy;
>>>> -	struct phy		*usb3_generic_phy;
>>>> +	struct phy		**usb2_generic_phy;
>>>> +	struct phy		**usb3_generic_phy;
>>>>    	bool			phys_ready;
>>>> diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
>>>> index 039bf24..404643f 100644
>>>> --- a/drivers/usb/dwc3/drd.c
>>>> +++ b/drivers/usb/dwc3/drd.c
>>>> @@ -384,10 +384,10 @@ void dwc3_otg_update(struct dwc3 *dwc, bool ignore_idstatus)
>>>>    		if (ret) {
>>>>    			dev_err(dwc->dev, "failed to initialize host\n");
>>>>    		} else {
>>>> -			if (dwc->usb2_phy)
>>>> -				otg_set_vbus(dwc->usb2_phy->otg, true);
>>>> -			if (dwc->usb2_generic_phy)
>>>> -				phy_set_mode(dwc->usb2_generic_phy,
>>>> +			if (dwc->usb2_phy[0])
>>>> +				otg_set_vbus(dwc->usb2_phy[0]->otg, true);
>>>> +			if (dwc->usb2_generic_phy[0])
>>>> +				phy_set_mode(dwc->usb2_generic_phy[0],
>>>>    					     PHY_MODE_USB_HOST);
>>>>    		}
>>>>    		break;
>>>> @@ -398,10 +398,10 @@ void dwc3_otg_update(struct dwc3 *dwc, bool ignore_idstatus)
>>>>    		dwc3_event_buffers_setup(dwc);
>>>>    		spin_unlock_irqrestore(&dwc->lock, flags);
>>>> -		if (dwc->usb2_phy)
>>>> -			otg_set_vbus(dwc->usb2_phy->otg, false);
>>>> -		if (dwc->usb2_generic_phy)
>>>> -			phy_set_mode(dwc->usb2_generic_phy,
>>>> +		if (dwc->usb2_phy[0])
>>>> +			otg_set_vbus(dwc->usb2_phy[0]->otg, false);
>>>> +		if (dwc->usb2_generic_phy[0])
>>>> +			phy_set_mode(dwc->usb2_generic_phy[0],
>>>>    				     PHY_MODE_USB_DEVICE);
>>>>    		ret = dwc3_gadget_init(dwc);
>>>>    		if (ret)
>>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>>>> index 00427d1..e3b2a17 100644
>>>> --- a/drivers/usb/dwc3/gadget.c
>>>> +++ b/drivers/usb/dwc3/gadget.c
>>>> @@ -2872,8 +2872,8 @@ static int dwc3_gadget_vbus_draw(struct usb_gadget *g, unsigned int mA)
>>>>    	union power_supply_propval	val = {0};
>>>>    	int				ret;
>>>> -	if (dwc->usb2_phy)
>>>> -		return usb_phy_set_power(dwc->usb2_phy, mA);
>>>> +	if (dwc->usb2_phy[0])
>>>> +		return usb_phy_set_power(dwc->usb2_phy[0], mA);
>>>>    	if (!dwc->usb_psy)
>>>>    		return -EOPNOTSUPP;
>>>> -- 
>>>> 2.7.4
>>>>

Re: [PATCH 3/3] usb: dwc3: Refactor PHY logic to support Multiport Controller

[Harsh Agarwal]

On 6/8/2022 7:59 PM, Harsh Agarwal wrote:
>
> On 6/6/2022 6:34 PM, Andrew Halaney wrote:
>> On Sun, Jun 05, 2022 at 11:51:38PM +0530, Harsh Agarwal wrote:
>>> On 6/2/2022 1:23 AM, Andrew Halaney wrote:
>>>> On Tue, May 31, 2022 at 01:50:17PM +0530, Harsh Agarwal wrote:
>>>>> Currently the DWC3 driver supports only single port controller
>>>>> which requires at most 2 PHYs ie HS and SS PHYs.
>>>>>
>>>>> But some SOCs have a "multiport" USB DWC3 controller where a
>>>>> single controller supports multiple ports and each port have
>>>>> their own PHYs. Refactor PHY logic to support the same.
>>>>>
>>>>> Signed-off-by: Harsh Agarwal <quic_harshq@quicinc.com>
>>>>> ---
>>>>>    drivers/usb/dwc3/core.c   | 400 
>>>>> +++++++++++++++++++++++++++++++++-------------
>>>>>    drivers/usb/dwc3/core.h   |  12 +-
>>>>>    drivers/usb/dwc3/drd.c    |  16 +-
>>>>>    drivers/usb/dwc3/gadget.c |   4 +-
>>>>>    4 files changed, 305 insertions(+), 127 deletions(-)
>>>>>
>>>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>>>> index 5734219..5cc799e 100644
>>>>> --- a/drivers/usb/dwc3/core.c
>>>>> +++ b/drivers/usb/dwc3/core.c
>>>>> @@ -120,7 +120,7 @@ static void __dwc3_set_mode(struct work_struct 
>>>>> *work)
>>>>>    {
>>>>>        struct dwc3 *dwc = work_to_dwc(work);
>>>>>        unsigned long flags;
>>>>> -    int ret;
>>>>> +    int i, ret;
>>>>>        u32 reg;
>>>>>        mutex_lock(&dwc->mutex);
>>>>> @@ -189,10 +189,13 @@ static void __dwc3_set_mode(struct 
>>>>> work_struct *work)
>>>>>            if (ret) {
>>>>>                dev_err(dwc->dev, "failed to initialize host\n");
>>>>>            } else {
>>>>> -            if (dwc->usb2_phy)
>>>>> -                otg_set_vbus(dwc->usb2_phy->otg, true);
>>>>> -            phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST);
>>>>> -            phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_HOST);
>>>>> +            for (i = 0; i < dwc->num_usb2_phy; i++) {
>>>>> +                if (dwc->usb2_phy[i])
>>>>> + otg_set_vbus(dwc->usb2_phy[i]->otg, true);
>>>>> +                phy_set_mode(dwc->usb2_generic_phy[i], 
>>>>> PHY_MODE_USB_HOST);
>>>>> +            }
>>>>> +            for (i = 0; i < dwc->num_usb3_phy; i++)
>>>>> +                phy_set_mode(dwc->usb3_generic_phy[i], 
>>>>> PHY_MODE_USB_HOST);
>>>>>                if (dwc->dis_split_quirk) {
>>>>>                    reg = dwc3_readl(dwc->regs, DWC3_GUCTL3);
>>>>>                    reg |= DWC3_GUCTL3_SPLITDISABLE;
>>>>> @@ -205,10 +208,10 @@ static void __dwc3_set_mode(struct 
>>>>> work_struct *work)
>>>>>            dwc3_event_buffers_setup(dwc);
>>>>> -        if (dwc->usb2_phy)
>>>>> -            otg_set_vbus(dwc->usb2_phy->otg, false);
>>>>> -        phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_DEVICE);
>>>>> -        phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_DEVICE);
>>>>> +        if (dwc->usb2_phy[0])
>>>>> +            otg_set_vbus(dwc->usb2_phy[0]->otg, false);
>>>>> +        phy_set_mode(dwc->usb2_generic_phy[0], PHY_MODE_USB_DEVICE);
>>>>> +        phy_set_mode(dwc->usb3_generic_phy[0], PHY_MODE_USB_DEVICE);
>>>>>            ret = dwc3_gadget_init(dwc);
>>>>>            if (ret)
>>>>> @@ -656,6 +659,7 @@ static int dwc3_core_ulpi_init(struct dwc3 *dwc)
>>>>>     */
>>>>>    static int dwc3_phy_setup(struct dwc3 *dwc)
>>>>>    {
>>>>> +    int i;
>>>>>        unsigned int hw_mode;
>>>>>        u32 reg;
>>>>> @@ -716,7 +720,8 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
>>>>>        if (dwc->dis_del_phy_power_chg_quirk)
>>>>>            reg &= ~DWC3_GUSB3PIPECTL_DEPOCHANGE;
>>>>> -    dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg);
>>>>> +    for (i = 0; i < dwc->num_usb3_phy; i++)
>>>>> +        dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(i), reg);
>>>>>        reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
>>>>> @@ -730,7 +735,8 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
>>>>>            } else if (dwc->hsphy_interface &&
>>>>>                    !strncmp(dwc->hsphy_interface, "ulpi", 4)) {
>>>>>                reg |= DWC3_GUSB2PHYCFG_ULPI_UTMI;
>>>>> -            dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>>>>> +            for (i = 0; i < dwc->num_usb2_phy; i++)
>>>>> +                dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
>>>>>            } else {
>>>>>                /* Relying on default value. */
>>>>>                if (!(reg & DWC3_GUSB2PHYCFG_ULPI_UTMI))
>>>>> @@ -787,7 +793,8 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
>>>>>        if (dwc->dis_u2_freeclk_exists_quirk)
>>>>>            reg &= ~DWC3_GUSB2PHYCFG_U2_FREECLK_EXISTS;
>>>>> -    dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>>>>> +    for (i = 0; i < dwc->num_usb2_phy; i++)
>>>>> +        dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
>>>>>        return 0;
>>>>>    }
>>>>> @@ -826,17 +833,23 @@ static void dwc3_clk_disable(struct dwc3 *dwc)
>>>>>    static void dwc3_core_exit(struct dwc3 *dwc)
>>>>>    {
>>>>> +    int i;
>>>>>        dwc3_event_buffers_cleanup(dwc);
>>>>> -    usb_phy_shutdown(dwc->usb2_phy);
>>>>> -    usb_phy_shutdown(dwc->usb3_phy);
>>>>> -    phy_exit(dwc->usb2_generic_phy);
>>>>> -    phy_exit(dwc->usb3_generic_phy);
>>>>> +    for (i = 0; i < dwc->num_usb2_phy; i++) {
>>>>> +        usb_phy_shutdown(dwc->usb2_phy[i]);
>>>>> +        usb_phy_set_suspend(dwc->usb2_phy[i], 1);
>>>>> +        phy_exit(dwc->usb2_generic_phy[i]);
>>>>> +        phy_power_off(dwc->usb2_generic_phy[i]);
>>>>> +    }
>>>>> +
>>>>> +    for (i = 0; i < dwc->num_usb3_phy; i++) {
>>>>> +        usb_phy_shutdown(dwc->usb3_phy[i]);
>>>>> +        usb_phy_set_suspend(dwc->usb3_phy[i], 1);
>>>>> +        phy_exit(dwc->usb3_generic_phy[i]);
>>>>> +        phy_power_off(dwc->usb3_generic_phy[i]);
>>>>> +    }
>>>>> -    usb_phy_set_suspend(dwc->usb2_phy, 1);
>>>>> -    usb_phy_set_suspend(dwc->usb3_phy, 1);
>>>>> -    phy_power_off(dwc->usb2_generic_phy);
>>>>> -    phy_power_off(dwc->usb3_generic_phy);
>>>>>        dwc3_clk_disable(dwc);
>>>>>        reset_control_assert(dwc->reset);
>>>>>    }
>>>>> @@ -1039,7 +1052,7 @@ static int dwc3_core_init(struct dwc3 *dwc)
>>>>>    {
>>>>>        unsigned int        hw_mode;
>>>>>        u32            reg;
>>>>> -    int            ret;
>>>>> +    int            ret, i;
>>>>>        hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
>>>>> @@ -1067,16 +1080,24 @@ static int dwc3_core_init(struct dwc3 *dwc)
>>>>>            dwc->phys_ready = true;
>>>>>        }
>>>>> -    usb_phy_init(dwc->usb2_phy);
>>>>> -    usb_phy_init(dwc->usb3_phy);
>>>>> -    ret = phy_init(dwc->usb2_generic_phy);
>>>>> -    if (ret < 0)
>>>>> -        goto err0a;
>>>>> +    for (i = 0; i < dwc->num_usb2_phy; i++)
>>>>> +        usb_phy_init(dwc->usb2_phy[i]);
>>>>> +    for (i = 0; i < dwc->num_usb3_phy; i++)
>>>>> +        usb_phy_init(dwc->usb3_phy[i]);
>>>>> -    ret = phy_init(dwc->usb3_generic_phy);
>>>>> -    if (ret < 0) {
>>>>> -        phy_exit(dwc->usb2_generic_phy);
>>>>> -        goto err0a;
>>>>> +    for (i = 0; i < dwc->num_usb2_phy; i++) {
>>>>> +        ret = phy_init(dwc->usb2_generic_phy[i]);
>>>>> +        if (ret < 0)
>>>>> +            goto err0a;
>>>> Should we clean up the prior phys that did not fail? There's a couple
>>>> loops where that question stands in this patch.
>>> I am just keeping the Generic PHY implementation as is.
>>> Earlier also we were cleaning up the HS PHY incase
>>> SS PHY fails, so keeping the same for Multiport.
>>> For you please point out if you find any major concern here?
>> Earlier, there was only one usb2_generic_phy/usb3_generic_phy, so if
>> phy_init(usb2_generic_phy) returned an error, it wasn't initialized, and
>> it did not need a matching call to phy_exit(usb2_generic_phy).
>>
>> That is being called though ifthe superspeed phy fails since the
>> highspeed one did successfully init, so things are cleaned up in the
>> old cases.
>>
>> My point now is that, say with 4 highspeed phys, the 3rd one fails.
>> i.e. phy_init(usb2_generic_phy[i]) where i == 2. In that case we don't
>> ever call phy_exit() on the first two phys. I wouldn't say it is a
>> major concern, but cleanup here is not fully done.
>>
>> You could do something like this to do clean it all up, kind of annoying
>> though:
>>
>>     for (i = 0; i < dwc->num_usb2_phy; i++) {
>>         ret = phy_init(dwc->usb2_generic_phy[i]);
>>         if (ret < 0) {
>>             /* clean up prior phys we initialized */
>>             for (j = 0; j < i; j++)
>>                 phy_exit(usb2_generic_phy[j]);
>>             goto err0a;
>>         }
>>     }
>>
>> Thanks,
>> Andrew
> yes agree with your analysis.
> Also, if any SSPHY init fails, then clear already initialized SSPHYs and
> HSPHYs.
>
> I am thinking instead of adding all this logic, we can just do "goto 
> err1"
> instead of "goto err0a".
>
> Similar approach if usb_phy_init fails ( Non-Generic phys)

For now in v3, incase PHY init fails, then I will add loops to call
shutdown/exit API on only those PHYs which has been
properly initialized before as suggested before. This is just to
avoid calling unnecessary shudown/exit on all the PHYs.

If someone feels going to "err1"  should not be an issue
then will incorporate that later.

>>
>>>>> +    }
>>>>> +
>>>>> +    for (i = 0; i < dwc->num_usb3_phy; i++) {
>>>>> +        ret = phy_init(dwc->usb3_generic_phy[i]);
>>>>> +        if (ret < 0) {
>>>>> +            for (i = 0; i < dwc->num_usb2_phy; i++)
>>>>> +                phy_exit(dwc->usb2_generic_phy[i]);
>>>>> +            goto err0a;
>>>>> +        }
>>>>>        }
>>>>>        ret = dwc3_core_soft_reset(dwc);
>>>>> @@ -1086,15 +1107,19 @@ static int dwc3_core_init(struct dwc3 *dwc)
>>>>>        if (hw_mode == DWC3_GHWPARAMS0_MODE_DRD &&
>>>>>            !DWC3_VER_IS_WITHIN(DWC3, ANY, 194A)) {
>>>>>            if (!dwc->dis_u3_susphy_quirk) {
>>>>> -            reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0));
>>>>> -            reg |= DWC3_GUSB3PIPECTL_SUSPHY;
>>>>> -            dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg);
>>>>> +            for (i = 0; i < dwc->num_usb3_phy; i++) {
>>>>> +                reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(i));
>>>>> +                reg |= DWC3_GUSB3PIPECTL_SUSPHY;
>>>>> +                dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(i), reg);
>>>>> +            }
>>>>>            }
>>>>>            if (!dwc->dis_u2_susphy_quirk) {
>>>>> -            reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
>>>>> -            reg |= DWC3_GUSB2PHYCFG_SUSPHY;
>>>>> -            dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>>>>> +            for (i = 0; i < dwc->num_usb2_phy; i++) {
>>>>> +                reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(i));
>>>>> +                reg |= DWC3_GUSB2PHYCFG_SUSPHY;
>>>>> +                dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
>>>>> +            }
>>>>>            }
>>>>>        }
>>>>> @@ -1113,15 +1138,19 @@ static int dwc3_core_init(struct dwc3 *dwc)
>>>>>        dwc3_set_incr_burst_type(dwc);
>>>>> -    usb_phy_set_suspend(dwc->usb2_phy, 0);
>>>>> -    usb_phy_set_suspend(dwc->usb3_phy, 0);
>>>>> -    ret = phy_power_on(dwc->usb2_generic_phy);
>>>>> -    if (ret < 0)
>>>>> -        goto err2;
>>>>> +    for (i = 0; i < dwc->num_usb2_phy; i++) {
>>>>> +        usb_phy_set_suspend(dwc->usb2_phy[i], 0);
>>>>> +        ret = phy_power_on(dwc->usb2_generic_phy[i]);
>>>>> +        if (ret < 0)
>>>>> +            goto err2;
>>>>> +    }
>>>>> -    ret = phy_power_on(dwc->usb3_generic_phy);
>>>>> -    if (ret < 0)
>>>>> -        goto err3;
>>>>> +    for (i = 0; i < dwc->num_usb3_phy; i++) {
>>>>> +        usb_phy_set_suspend(dwc->usb3_phy[i], 0);
>>>>> +        ret = phy_power_on(dwc->usb3_generic_phy[i]);
>>>>> +        if (ret < 0)
>>>>> +            goto err3;
>>>>> +    }
>>>>>        ret = dwc3_event_buffers_setup(dwc);
>>>>>        if (ret) {
>>>>> @@ -1229,20 +1258,29 @@ static int dwc3_core_init(struct dwc3 *dwc)
>>>>>        return 0;
>>>>>    err4:
>>>>> -    phy_power_off(dwc->usb3_generic_phy);
>>>>> +    for (i = 0; i < dwc->num_usb3_phy; i++)
>>>>> +        phy_power_off(dwc->usb3_generic_phy[i]);
>>>>>    err3:
>>>>> -    phy_power_off(dwc->usb2_generic_phy);
>>>>> +    for (i = 0; i < dwc->num_usb2_phy; i++)
>>>>> +        phy_power_off(dwc->usb2_generic_phy[i]);
>>>>>    err2:
>>>>> -    usb_phy_set_suspend(dwc->usb2_phy, 1);
>>>>> -    usb_phy_set_suspend(dwc->usb3_phy, 1);
>>>>> +    for (i = 0; i < dwc->num_usb2_phy; i++)
>>>>> +        usb_phy_set_suspend(dwc->usb2_phy[i], 1);
>>>>> +    for (i = 0; i < dwc->num_usb3_phy; i++)
>>>>> +        usb_phy_set_suspend(dwc->usb3_phy[i], 1);
>>>>>    err1:
>>>>> -    usb_phy_shutdown(dwc->usb2_phy);
>>>>> -    usb_phy_shutdown(dwc->usb3_phy);
>>>>> -    phy_exit(dwc->usb2_generic_phy);
>>>>> -    phy_exit(dwc->usb3_generic_phy);
>>>>> +    for (i = 0; i < dwc->num_usb2_phy; i++) {
>>>>> +        usb_phy_shutdown(dwc->usb2_phy[i]);
>>>>> +        phy_exit(dwc->usb2_generic_phy[i]);
>>>>> +    }
>>>>> +
>>>>> +    for (i = 0; i < dwc->num_usb3_phy; i++) {
>>>>> +        usb_phy_shutdown(dwc->usb3_phy[i]);
>>>>> +        phy_exit(dwc->usb3_generic_phy[i]);
>>>>> +    }
>>>>>    err0a:
>>>>>        dwc3_ulpi_exit(dwc);
>>>>> @@ -1251,53 +1289,172 @@ static int dwc3_core_init(struct dwc3 *dwc)
>>>>>        return ret;
>>>>>    }
>>>>> -static int dwc3_core_get_phy(struct dwc3 *dwc)
>>>>> +static struct usb_phy 
>>>>> *dwc3_core_get_phy_by_handle_with_node(struct device *dev,
>>>>> +    const char *phandle, u8 index, struct device_node *lookup_node)
>>>>> +{
>>>>> +    struct device_node *node;
>>>>> +    struct usb_phy    *phy;
>>>>> +
>>>>> +    node = of_parse_phandle(lookup_node, phandle, index);
>>>>> +    if (!node) {
>>>>> +        dev_err(dev, "failed to get %s phandle in %pOF node\n", 
>>>>> phandle,
>>>>> +            dev->of_node);
>>>>> +        return ERR_PTR(-ENODEV);
>>>>> +    }
>>>>> +    phy = devm_usb_get_phy_by_node(dev, node, NULL);
>>>>> +    of_node_put(node);
>>>>> +    return phy;
>>>>> +}
>>>>> +
>>>>> +static int dwc3_count_phys(struct dwc3 *dwc, struct device_node 
>>>>> *lookup_node)
>>>>> +{
>>>>> +    int count;
>>>>> +
>>>>> +    count = of_count_phandle_with_args(lookup_node, "phys", NULL);
>>>>> +
>>>>> +    if (count == -ENOENT)
>>>>> +        count = of_count_phandle_with_args(lookup_node, 
>>>>> "usb-phy", NULL);
>>>>> +
>>>>> +    if (count == 1) {
>>>>> +        dwc->num_usb2_phy++;
>>>>> +    } else if (count == 2) {
>>>>> +        dwc->num_usb2_phy++;
>>>>> +        dwc->num_usb3_phy++;
>>>>> +    } else {
>>>>> +        return count;
>>>>> +    }
>>>>> +    return 0;
>>>>> +}
>>>>> +
>>>>> +static int dwc3_extract_num_phys(struct dwc3 *dwc)
>>>>> +{
>>>>> +    struct device_node    *ports, *port;
>>>>> +    int            ret;
>>>>> +
>>>>> +    /* Find if any "multiport" child is present inside DWC3*/
>>>> Nit, space after DWC3
>>> Done
>>>>> + for_each_available_child_of_node(dwc->dev->of_node, ports) {
>>>>> +        if (!strcmp(ports->name, "multiport"))
>>>>> +            break;
>>>>> +    }
>>>>> +    if (!ports) {
>>>>> +        dwc->num_usb2_phy = 1;
>>>>> +        dwc->num_usb3_phy = 1;
>>>>> +    } else {
>>>>> +        for_each_available_child_of_node(ports, port) {
>>>>> +            ret  = dwc3_count_phys(dwc, port);
>>>>> +            if (ret)
>>>>> +                return ret;
>>>>> +        }
>>>>> +    }
>>>>> +    dev_info(dwc->dev, "Num of HS and SS PHY are %u %u\n", 
>>>>> dwc->num_usb2_phy,
>>>>> + dwc->num_usb3_phy);
>>>>> +
>>>>> +    dwc->usb2_phy = devm_kzalloc(dwc->dev,
>>>>> +        sizeof(*dwc->usb2_phy) * dwc->num_usb2_phy, GFP_KERNEL);
>>>>> +    if (!dwc->usb2_phy)
>>>>> +        return -ENOMEM;
>>>>> +
>>>>> +    dwc->usb3_phy = devm_kzalloc(dwc->dev,
>>>>> +        sizeof(*dwc->usb3_phy) * dwc->num_usb3_phy, GFP_KERNEL);
>>>>> +    if (!dwc->usb3_phy)
>>>>> +        return -ENOMEM;
>>>>> +
>>>>> +    dwc->usb2_generic_phy = devm_kzalloc(dwc->dev,
>>>>> +        sizeof(*dwc->usb2_generic_phy) * dwc->num_usb2_phy, 
>>>>> GFP_KERNEL);
>>>>> +    if (!dwc->usb2_generic_phy)
>>>>> +        return -ENOMEM;
>>>>> +
>>>>> +    dwc->usb3_generic_phy = devm_kzalloc(dwc->dev,
>>>>> +        sizeof(*dwc->usb3_generic_phy) * dwc->num_usb3_phy, 
>>>>> GFP_KERNEL);
>>>>> +    if (!dwc->usb3_generic_phy)
>>>>> +        return -ENOMEM;
>>>>> +
>>>>> +    return 0;
>>>>> +}
>>>>> +
>>>>> +static int dwc3_core_get_phy_by_node(struct dwc3 *dwc,
>>>>> +        struct device_node *lookup_node, int i)
>>>>>    {
>>>>>        struct device        *dev = dwc->dev;
>>>>> -    struct device_node    *node = dev->of_node;
>>>>> -    int ret;
>>>>> +    int            ret;
>>>>> -    if (node) {
>>>>> -        dwc->usb2_phy = devm_usb_get_phy_by_phandle(dev, 
>>>>> "usb-phy", 0);
>>>>> -        dwc->usb3_phy = devm_usb_get_phy_by_phandle(dev, 
>>>>> "usb-phy", 1);
>>>>> +    if (lookup_node) {
>>>>> +        dwc->usb2_phy[i] = devm_of_usb_get_phy_by_phandle(dev,
>>>>> +                                "usb-phy", 0, lookup_node);
>>>>> +        dwc->usb3_phy[i] = devm_of_usb_get_phy_by_phandle(dev,
>>>>> +                                "usb-phy", 1, lookup_node);
>>>>>        } else {
>>>>> -        dwc->usb2_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
>>>>> -        dwc->usb3_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3);
>>>>> +        dwc->usb2_phy[i] = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
>>>>> +        dwc->usb3_phy[i] = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3);
>>>>>        }
>>>>> -    if (IS_ERR(dwc->usb2_phy)) {
>>>>> -        ret = PTR_ERR(dwc->usb2_phy);
>>>>> +    if (IS_ERR(dwc->usb2_phy[i])) {
>>>>> +        ret = PTR_ERR(dwc->usb2_phy[i]);
>>>>>            if (ret == -ENXIO || ret == -ENODEV)
>>>>> -            dwc->usb2_phy = NULL;
>>>>> +            dwc->usb2_phy[i] = NULL;
>>>>>            else
>>>>>                return dev_err_probe(dev, ret, "no usb2 phy 
>>>>> configured\n");
>>>>>        }
>>>>> -    if (IS_ERR(dwc->usb3_phy)) {
>>>>> -        ret = PTR_ERR(dwc->usb3_phy);
>>>>> +    if (IS_ERR(dwc->usb3_phy[i])) {
>>>>> +        ret = PTR_ERR(dwc->usb3_phy[i]);
>>>>>            if (ret == -ENXIO || ret == -ENODEV)
>>>>> -            dwc->usb3_phy = NULL;
>>>>> +            dwc->usb3_phy[i] = NULL;
>>>>>            else
>>>>>                return dev_err_probe(dev, ret, "no usb3 phy 
>>>>> configured\n");
>>>>>        }
>>>>> -    dwc->usb2_generic_phy = devm_phy_get(dev, "usb2-phy");
>>>>> -    if (IS_ERR(dwc->usb2_generic_phy)) {
>>>>> -        ret = PTR_ERR(dwc->usb2_generic_phy);
>>>>> -        if (ret == -ENOSYS || ret == -ENODEV)
>>>>> -            dwc->usb2_generic_phy = NULL;
>>>>> +    dwc->usb2_generic_phy[i] = devm_of_phy_get(dev, lookup_node, 
>>>>> "usb2-phy");
>>>>> +    if (IS_ERR(dwc->usb2_generic_phy[i])) {
>>>>> +        ret = PTR_ERR(dwc->usb2_generic_phy[i]);
>>>>> +        if (ret == -ENODEV)
>>>> I think this can be -ENOSYS if !CONFIG_GENERIC_PHY
>>> For now, I have added it back
>>>>> + dwc->usb2_generic_phy[i] = NULL;
>>>>>            else
>>>>>                return dev_err_probe(dev, ret, "no usb2 phy 
>>>>> configured\n");
>>>>>        }
>>>>> -    dwc->usb3_generic_phy = devm_phy_get(dev, "usb3-phy");
>>>>> -    if (IS_ERR(dwc->usb3_generic_phy)) {
>>>>> -        ret = PTR_ERR(dwc->usb3_generic_phy);
>>>>> -        if (ret == -ENOSYS || ret == -ENODEV)
>>>>> -            dwc->usb3_generic_phy = NULL;
>>>>> +    dwc->usb3_generic_phy[i] = devm_of_phy_get(dev, lookup_node, 
>>>>> "usb3-phy");
>>>>> +    if (IS_ERR(dwc->usb3_generic_phy[i])) {
>>>>> +        ret = PTR_ERR(dwc->usb3_generic_phy[i]);
>>>>> +        if (ret == -ENODEV)
>>>>> +            dwc->usb3_generic_phy[i] = NULL;
>>>>>            else
>>>>>                return dev_err_probe(dev, ret, "no usb3 phy 
>>>>> configured\n");
>>>>>        }
>>>>> +    return 0;
>>>>> +}
>>>>> +
>>>>> +static int dwc3_core_get_phy(struct dwc3 *dwc)
>>>>> +{
>>>>> +    struct device        *dev = dwc->dev;
>>>>> +    struct device_node    *node = dev->of_node;
>>>>> +    struct device_node    *ports, *port;
>>>>> +    int ret, i = 0;
>>>>> +
>>>>> +    ret = dwc3_extract_num_phys(dwc);
>>>>> +    if (ret) {
>>>>> +        dev_err(dwc->dev, "Unable to extract number of PHYs\n");
>>>>> +        return ret;
>>>>> +    }
>>>>> +
>>>>> +    /* Find if any "multiport" child is present inside DWC3*/
>>>> Nit, space after DWC3
>>> Done
>>>>> + for_each_available_child_of_node(node, ports) {
>>>>> +        if (!strcmp(ports->name, "multiport"))
>>>>> +            break;
>>>>> +    }
>>>>> +
>>>>> +    if (!ports) {
>>>>> +        ret = dwc3_core_get_phy_by_node(dwc, node, 0);
>>>>> +        if (ret)
>>>>> +            return ret;
>>>>> +    } else {
>>>>> +        for_each_available_child_of_node(ports, port) {
>>>>> +            ret = dwc3_core_get_phy_by_node(dwc, port, i);
>>>>> +            if (ret)
>>>>> +                return ret;
>>>>> +            i++;
>>>>> +        }
>>>>> +    }
>>>>>        return 0;
>>>>>    }
>>>>> @@ -1305,16 +1462,16 @@ static int dwc3_core_get_phy(struct dwc3 
>>>>> *dwc)
>>>>>    static int dwc3_core_init_mode(struct dwc3 *dwc)
>>>>>    {
>>>>>        struct device *dev = dwc->dev;
>>>>> -    int ret;
>>>>> +    int i, ret;
>>>>>        switch (dwc->dr_mode) {
>>>>>        case USB_DR_MODE_PERIPHERAL:
>>>>>            dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_DEVICE);
>>>>> -        if (dwc->usb2_phy)
>>>>> -            otg_set_vbus(dwc->usb2_phy->otg, false);
>>>>> -        phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_DEVICE);
>>>>> -        phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_DEVICE);
>>>>> +        if (dwc->usb2_phy[0])
>>>>> +            otg_set_vbus(dwc->usb2_phy[0]->otg, false);
>>>>> +        phy_set_mode(dwc->usb2_generic_phy[0], PHY_MODE_USB_DEVICE);
>>>>> +        phy_set_mode(dwc->usb3_generic_phy[0], PHY_MODE_USB_DEVICE);
>>>>>            ret = dwc3_gadget_init(dwc);
>>>>>            if (ret)
>>>>> @@ -1323,10 +1480,15 @@ static int dwc3_core_init_mode(struct dwc3 
>>>>> *dwc)
>>>>>        case USB_DR_MODE_HOST:
>>>>>            dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_HOST);
>>>>> -        if (dwc->usb2_phy)
>>>>> -            otg_set_vbus(dwc->usb2_phy->otg, true);
>>>>> -        phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST);
>>>>> -        phy_set_mode(dwc->usb3_generic_phy, PHY_MODE_USB_HOST);
>>>>> +        for (i = 0; i < dwc->num_usb3_phy; i++) {
>>>>> +            if (dwc->usb2_phy[i])
>>>>> +                otg_set_vbus(dwc->usb2_phy[i]->otg, true);
>>>>> +            phy_set_mode(dwc->usb2_generic_phy[i], 
>>>>> PHY_MODE_USB_HOST);
>>>>> +        }
>>>>> +
>>>>> +
>>>>> +        for (i = 0; i < dwc->num_usb3_phy; i++)
>>>>> +            phy_set_mode(dwc->usb3_generic_phy[i], 
>>>>> PHY_MODE_USB_HOST);
>>>>>            ret = dwc3_host_init(dwc);
>>>>>            if (ret)
>>>>> @@ -1674,7 +1836,7 @@ static int dwc3_probe(struct platform_device 
>>>>> *pdev)
>>>>>        struct resource        *res, dwc_res;
>>>>>        struct dwc3        *dwc;
>>>>> -    int            ret;
>>>>> +    int            ret, i;
>>>>>        void __iomem        *regs;
>>>>> @@ -1839,15 +2001,18 @@ static int dwc3_probe(struct 
>>>>> platform_device *pdev)
>>>>>        dwc3_debugfs_exit(dwc);
>>>>>        dwc3_event_buffers_cleanup(dwc);
>>>>> -    usb_phy_shutdown(dwc->usb2_phy);
>>>>> -    usb_phy_shutdown(dwc->usb3_phy);
>>>>> -    phy_exit(dwc->usb2_generic_phy);
>>>>> -    phy_exit(dwc->usb3_generic_phy);
>>>>> -
>>>>> -    usb_phy_set_suspend(dwc->usb2_phy, 1);
>>>>> -    usb_phy_set_suspend(dwc->usb3_phy, 1);
>>>>> -    phy_power_off(dwc->usb2_generic_phy);
>>>>> -    phy_power_off(dwc->usb3_generic_phy);
>>>>> +    for (i = 0; i < dwc->num_usb2_phy; i++) {
>>>>> +        usb_phy_shutdown(dwc->usb2_phy[i]);
>>>>> +        usb_phy_set_suspend(dwc->usb2_phy[i], 1);
>>>>> +        phy_exit(dwc->usb2_generic_phy[i]);
>>>>> +        phy_power_off(dwc->usb2_generic_phy[i]);
>>>>> +    }
>>>>> +    for (i = 0; i < dwc->num_usb3_phy; i++) {
>>>>> +        usb_phy_shutdown(dwc->usb3_phy[i]);
>>>>> +        usb_phy_set_suspend(dwc->usb3_phy[i], 1);
>>>>> +        phy_exit(dwc->usb3_generic_phy[i]);
>>>>> +        phy_power_off(dwc->usb3_generic_phy[i]);
>>>>> +    }
>>>>>        dwc3_ulpi_exit(dwc);
>>>>> @@ -1929,6 +2094,7 @@ static int dwc3_core_init_for_resume(struct 
>>>>> dwc3 *dwc)
>>>>>    static int dwc3_suspend_common(struct dwc3 *dwc, pm_message_t msg)
>>>>>    {
>>>>> +    int i;
>>>>>        unsigned long    flags;
>>>>>        u32 reg;
>>>>> @@ -1951,17 +2117,21 @@ static int dwc3_suspend_common(struct dwc3 
>>>>> *dwc, pm_message_t msg)
>>>>>            /* Let controller to suspend HSPHY before PHY driver 
>>>>> suspends */
>>>>>            if (dwc->dis_u2_susphy_quirk ||
>>>>>                dwc->dis_enblslpm_quirk) {
>>>>> -            reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
>>>>> -            reg |=  DWC3_GUSB2PHYCFG_ENBLSLPM |
>>>>> -                DWC3_GUSB2PHYCFG_SUSPHY;
>>>>> -            dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>>>>> -
>>>>> -            /* Give some time for USB2 PHY to suspend */
>>>>> -            usleep_range(5000, 6000);
>>>>> +            for (i = 0; i < dwc->num_usb2_phy; i++) {
>>>>> +                reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(i));
>>>>> +                reg |=  DWC3_GUSB2PHYCFG_ENBLSLPM |
>>>>> +                    DWC3_GUSB2PHYCFG_SUSPHY;
>>>>> +                dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
>>>>> +
>>>>> +                /* Give some time for USB2 PHY to suspend */
>>>>> +                usleep_range(5000, 6000);
>>>>> +            }
>>>>>            }
>>>>> - phy_pm_runtime_put_sync(dwc->usb2_generic_phy);
>>>>> - phy_pm_runtime_put_sync(dwc->usb3_generic_phy);
>>>>> +        for (i = 0; i < dwc->num_usb2_phy; i++)
>>>>> + phy_pm_runtime_put_sync(dwc->usb2_generic_phy[i]);
>>>>> +        for (i = 0; i < dwc->num_usb3_phy; i++)
>>>>> + phy_pm_runtime_put_sync(dwc->usb3_generic_phy[i]);
>>>>>            break;
>>>>>        case DWC3_GCTL_PRTCAP_OTG:
>>>>>            /* do nothing during runtime_suspend */
>>>>> @@ -1989,7 +2159,7 @@ static int dwc3_suspend_common(struct dwc3 
>>>>> *dwc, pm_message_t msg)
>>>>>    static int dwc3_resume_common(struct dwc3 *dwc, pm_message_t msg)
>>>>>    {
>>>>>        unsigned long    flags;
>>>>> -    int        ret;
>>>>> +    int        i, ret;
>>>>>        u32        reg;
>>>>>        switch (dwc->current_dr_role) {
>>>>> @@ -2012,17 +2182,21 @@ static int dwc3_resume_common(struct dwc3 
>>>>> *dwc, pm_message_t msg)
>>>>>                break;
>>>>>            }
>>>>>            /* Restore GUSB2PHYCFG bits that were modified in 
>>>>> suspend */
>>>>> -        reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
>>>>> -        if (dwc->dis_u2_susphy_quirk)
>>>>> -            reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
>>>>> +        for (i = 0; i < dwc->num_usb2_phy; i++) {
>>>>> +            reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(i));
>>>>> +            if (dwc->dis_u2_susphy_quirk)
>>>>> +                reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
>>>>> -        if (dwc->dis_enblslpm_quirk)
>>>>> -            reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;
>>>>> +            if (dwc->dis_enblslpm_quirk)
>>>>> +                reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;
>>>>> -        dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>>>>> +            dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
>>>>> +        }
>>>>> - phy_pm_runtime_get_sync(dwc->usb2_generic_phy);
>>>>> - phy_pm_runtime_get_sync(dwc->usb3_generic_phy);
>>>>> +        for (i = 0; i < dwc->num_usb2_phy; i++)
>>>>> + phy_pm_runtime_get_sync(dwc->usb2_generic_phy[i]);
>>>>> +        for (i = 0; i < dwc->num_usb3_phy; i++)
>>>>> + phy_pm_runtime_get_sync(dwc->usb3_generic_phy[i]);
>>>>>            break;
>>>>>        case DWC3_GCTL_PRTCAP_OTG:
>>>>>            /* nothing to do on runtime_resume */
>>>>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>>>>> index 81c486b..c169bf1 100644
>>>>> --- a/drivers/usb/dwc3/core.h
>>>>> +++ b/drivers/usb/dwc3/core.h
>>>>> @@ -1020,6 +1020,8 @@ struct dwc3_scratchpad_array {
>>>>>     * @usb_psy: pointer to power supply interface.
>>>>>     * @usb2_phy: pointer to USB2 PHY
>>>>>     * @usb3_phy: pointer to USB3 PHY
>>>>> + * @num_usb2_phy: Number of HS ports controlled by the core
>>>>> + * @num_dsphy: Number of SS ports controlled by the core
>>>> s/num_dsphy/num_usb3_phy/
>>> Done
>>>>>     * @usb2_generic_phy: pointer to USB2 PHY
>>>>>     * @usb3_generic_phy: pointer to USB3 PHY
>>>>>     * @phys_ready: flag to indicate that PHYs are ready
>>>>> @@ -1147,11 +1149,13 @@ struct dwc3 {
>>>>>        struct reset_control    *reset;
>>>>> -    struct usb_phy        *usb2_phy;
>>>>> -    struct usb_phy        *usb3_phy;
>>>>> +    struct usb_phy        **usb2_phy;
>>>>> +    struct usb_phy        **usb3_phy;
>>>>> +    u32            num_usb2_phy;
>>>>> +    u32            num_usb3_phy;
>>>>> -    struct phy        *usb2_generic_phy;
>>>>> -    struct phy        *usb3_generic_phy;
>>>>> +    struct phy        **usb2_generic_phy;
>>>>> +    struct phy        **usb3_generic_phy;
>>>>>        bool            phys_ready;
>>>>> diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
>>>>> index 039bf24..404643f 100644
>>>>> --- a/drivers/usb/dwc3/drd.c
>>>>> +++ b/drivers/usb/dwc3/drd.c
>>>>> @@ -384,10 +384,10 @@ void dwc3_otg_update(struct dwc3 *dwc, bool 
>>>>> ignore_idstatus)
>>>>>            if (ret) {
>>>>>                dev_err(dwc->dev, "failed to initialize host\n");
>>>>>            } else {
>>>>> -            if (dwc->usb2_phy)
>>>>> -                otg_set_vbus(dwc->usb2_phy->otg, true);
>>>>> -            if (dwc->usb2_generic_phy)
>>>>> -                phy_set_mode(dwc->usb2_generic_phy,
>>>>> +            if (dwc->usb2_phy[0])
>>>>> +                otg_set_vbus(dwc->usb2_phy[0]->otg, true);
>>>>> +            if (dwc->usb2_generic_phy[0])
>>>>> +                phy_set_mode(dwc->usb2_generic_phy[0],
>>>>>                             PHY_MODE_USB_HOST);
>>>>>            }
>>>>>            break;
>>>>> @@ -398,10 +398,10 @@ void dwc3_otg_update(struct dwc3 *dwc, bool 
>>>>> ignore_idstatus)
>>>>>            dwc3_event_buffers_setup(dwc);
>>>>>            spin_unlock_irqrestore(&dwc->lock, flags);
>>>>> -        if (dwc->usb2_phy)
>>>>> -            otg_set_vbus(dwc->usb2_phy->otg, false);
>>>>> -        if (dwc->usb2_generic_phy)
>>>>> -            phy_set_mode(dwc->usb2_generic_phy,
>>>>> +        if (dwc->usb2_phy[0])
>>>>> +            otg_set_vbus(dwc->usb2_phy[0]->otg, false);
>>>>> +        if (dwc->usb2_generic_phy[0])
>>>>> +            phy_set_mode(dwc->usb2_generic_phy[0],
>>>>>                         PHY_MODE_USB_DEVICE);
>>>>>            ret = dwc3_gadget_init(dwc);
>>>>>            if (ret)
>>>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>>>>> index 00427d1..e3b2a17 100644
>>>>> --- a/drivers/usb/dwc3/gadget.c
>>>>> +++ b/drivers/usb/dwc3/gadget.c
>>>>> @@ -2872,8 +2872,8 @@ static int dwc3_gadget_vbus_draw(struct 
>>>>> usb_gadget *g, unsigned int mA)
>>>>>        union power_supply_propval    val = {0};
>>>>>        int                ret;
>>>>> -    if (dwc->usb2_phy)
>>>>> -        return usb_phy_set_power(dwc->usb2_phy, mA);
>>>>> +    if (dwc->usb2_phy[0])
>>>>> +        return usb_phy_set_power(dwc->usb2_phy[0], mA);
>>>>>        if (!dwc->usb_psy)
>>>>>            return -EOPNOTSUPP;
>>>>> -- 
>>>>> 2.7.4
>>>>>