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 primarily refactors the Phy logic in core driver to allow
multiport support with Generic Phy's.
Chananges have been tested on QCOM SoC SA8295P which has 4 ports (2
are HS+SS capable and 2 are HS only capable).
Changes in current version:
Added DT support for first port of Teritiary USB controller on SA8540-Ride
Added support for reading port info from XHCI Extended Params registers.
Changes in RFC v4:
Added DT support for SA8295p.
Changes in RFC v3:
Incase any PHY init fails, then clear/exit the PHYs that
are already initialized.
Changes in RFC v2:
Changed dwc3_count_phys to return the number of PHY Phandles in the node.
This will be used now in dwc3_extract_num_phys to increment num_usb2_phy
and num_usb3_phy.
Added new parameter "ss_idx" in dwc3_core_get_phy_ny_node and changed its
structure such that the first half is for HS-PHY and second half is for
SS-PHY.
In dwc3_core_get_phy, for multiport controller, only if SS-PHY phandle is
present, pass proper SS_IDX else pass -1.
Link to RFC v4: https://lore.kernel.org/all/[email protected]/
Link to RFC v3: https://lore.kernel.org/all/[email protected]/#r
Link to RFC v2: https://lore.kernel.org/all/[email protected]/#r
Krishna Kurapati (8):
dt-bindings: usb: Add bindings for multiport properties on DWC3
controller
usb: dwc3: core: Access XHCI address space temporarily to read port
info
usb: dwc3: core: Skip setting event buffers for host only controllers
usb: dwc3: core: Refactor PHY logic to support Multiport Controller
usb: dwc3: qcom: Add multiport controller support for qcom wrapper
arm64: dts: qcom: sc8280xp: Add multiport controller node for SC8280
arm64: dts: qcom: sa8295p: Enable teritiary controller and its 4 USB
ports
arm64: dts: qcom: sa8540-ride: Enable first port of teritiary usb
controller
.../devicetree/bindings/usb/snps,dwc3.yaml | 13 +-
arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 47 +++
arch/arm64/boot/dts/qcom/sa8540p-ride.dts | 22 ++
arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 58 +++
drivers/usb/dwc3/core.c | 374 ++++++++++++++----
drivers/usb/dwc3/core.h | 20 +-
drivers/usb/dwc3/drd.c | 13 +-
drivers/usb/dwc3/dwc3-qcom.c | 28 +-
8 files changed, 473 insertions(+), 102 deletions(-)
--
2.39.0
Add bindings to indicate properties required to support multiport
on Snps Dwc3 controller.
Suggested-by: Bjorn Andersson <[email protected]>
Signed-off-by: Krishna Kurapati <[email protected]>
---
.../devicetree/bindings/usb/snps,dwc3.yaml | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
index be36956af53b..96701eb5a17c 100644
--- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
@@ -81,15 +81,16 @@ properties:
phys:
minItems: 1
- maxItems: 2
+ maxItems: 8
phy-names:
minItems: 1
- maxItems: 2
- items:
- enum:
- - usb2-phy
- - usb3-phy
+ maxItems: 8
+ oneOf:
+ - items:
+ enum: [ usb2-phy, usb3-phy ]
+ - items:
+ pattern: "^usb[23]-port[0-3]$"
power-domains:
description:
--
2.39.0
Currently host-only capable DWC3 controllers support Multiport. Temporarily
map XHCI address space for host-only controllers and parse XHCI Extended
Capabilities registers to read number of physical usb ports connected to the
multiport controller (presuming each port is at least HS capable) and extract
info on how many of these ports are Super Speed capable.
Signed-off-by: Krishna Kurapati <[email protected]>
---
drivers/usb/dwc3/core.c | 75 +++++++++++++++++++++++++++++++++++++++++
drivers/usb/dwc3/core.h | 9 +++++
2 files changed, 84 insertions(+)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 476b63618511..076c0f8a4441 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -37,6 +37,7 @@
#include "core.h"
#include "gadget.h"
#include "io.h"
+#include "../host/xhci.h"
#include "debug.h"
@@ -1750,6 +1751,65 @@ static struct extcon_dev *dwc3_get_extcon(struct dwc3 *dwc)
return edev;
}
+static int dwc3_read_port_info(struct dwc3 *dwc, struct resource *res)
+{
+ void __iomem *regs;
+ struct resource dwc_res;
+ u32 offset;
+ u32 temp;
+ u8 major_revision;
+ int ret = 0;
+
+ /*
+ * Remap xHCI address space to access XHCI ext cap regs,
+ * since it is needed to get port info.
+ */
+ dwc_res = *res;
+ dwc_res.start += 0;
+ dwc_res.end = dwc->xhci_resources[0].start +
+ DWC3_XHCI_REGS_END;
+
+ regs = ioremap(dwc_res.start, resource_size(&dwc_res));
+ if (IS_ERR(regs))
+ return PTR_ERR(regs);
+
+ offset = xhci_find_next_ext_cap(regs, 0,
+ XHCI_EXT_CAPS_PROTOCOL);
+ while (offset) {
+ temp = readl(regs + offset);
+ major_revision = XHCI_EXT_PORT_MAJOR(temp);
+
+ temp = readl(regs + offset + 0x08);
+ if (major_revision == 0x03) {
+ dwc->num_ss_ports += XHCI_EXT_PORT_COUNT(temp);
+ } else if (major_revision <= 0x02) {
+ dwc->num_ports += XHCI_EXT_PORT_COUNT(temp);
+ } else {
+ dev_err(dwc->dev, "port revision seems wrong\n");
+ ret = -EINVAL;
+ goto unmap_reg;
+ }
+
+ offset = xhci_find_next_ext_cap(regs, offset,
+ XHCI_EXT_CAPS_PROTOCOL);
+ }
+
+ temp = readl(regs + DWC3_XHCI_HCSPARAMS1);
+ if (HCS_MAX_PORTS(temp) != (dwc->num_ss_ports + dwc->num_ports)) {
+ dev_err(dwc->dev, "inconsistency in port info\n");
+ ret = -EINVAL;
+ goto unmap_reg;
+ }
+
+ dev_info(dwc->dev,
+ "num-ports: %d ss-capable: %d\n", dwc->num_ports, dwc->num_ss_ports);
+
+unmap_reg:
+ iounmap(regs);
+ return ret;
+}
+
static int dwc3_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -1757,6 +1817,7 @@ static int dwc3_probe(struct platform_device *pdev)
struct dwc3 *dwc;
int ret;
+ unsigned int hw_mode;
void __iomem *regs;
@@ -1880,6 +1941,20 @@ static int dwc3_probe(struct platform_device *pdev)
goto disable_clks;
}
+ /*
+ * Currently DWC3 controllers that are host-only capable
+ * support Multiport.
+ */
+ hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
+ if (hw_mode == DWC3_GHWPARAMS0_MODE_HOST) {
+ ret = dwc3_read_port_info(dwc, res);
+ if (ret)
+ goto disable_clks;
+ } else {
+ dwc->num_ports = 1;
+ dwc->num_ss_ports = 1;
+ }
+
spin_lock_init(&dwc->lock);
mutex_init(&dwc->mutex);
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 582ebd9cf9c2..74386d6a0277 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -35,6 +35,9 @@
#define DWC3_MSG_MAX 500
+/* XHCI Reg constants */
+#define DWC3_XHCI_HCSPARAMS1 0x04
+
/* Global constants */
#define DWC3_PULL_UP_TIMEOUT 500 /* ms */
#define DWC3_BOUNCE_SIZE 1024 /* size of a superspeed bulk */
@@ -1023,6 +1026,10 @@ struct dwc3_scratchpad_array {
* @usb_psy: pointer to power supply interface.
* @usb2_phy: pointer to USB2 PHY
* @usb3_phy: pointer to USB3 PHY
+ * @num_ports: Indicates the number of physical USB ports present on HW
+ * presuming each port is at least HS capable
+ * @num_ss_ports: Indicates the number of USB ports present on HW that are
+ * SS Capable
* @usb2_generic_phy: pointer to USB2 PHY
* @usb3_generic_phy: pointer to USB3 PHY
* @phys_ready: flag to indicate that PHYs are ready
@@ -1158,6 +1165,8 @@ struct dwc3 {
struct usb_phy *usb2_phy;
struct usb_phy *usb3_phy;
+ u32 num_ports;
+ u32 num_ss_ports;
struct phy *usb2_generic_phy;
struct phy *usb3_generic_phy;
--
2.39.0
Currently the DWC3 driver supports only single port controller
which requires at most one HS and one SS PHY.
But the DWC3 USB controller can be connected to multiple ports and
each port can have their own PHYs. Each port of the multiport
controller can either be HS+SS capable or HS only capable
Proper quantification of them is required to modify GUSB2PHYCFG
and GUSB3PIPECTL registers appropriately.
Add support for detecting, obtaining and configuring phy's supported
by a multiport controller and limit the max number of ports
supported to 4.
Signed-off-by: Harsh Agarwal <[email protected]>
Signed-off-by: Krishna Kurapati <[email protected]>
---
drivers/usb/dwc3/core.c | 279 +++++++++++++++++++++++++++++-----------
drivers/usb/dwc3/core.h | 11 +-
drivers/usb/dwc3/drd.c | 13 +-
3 files changed, 219 insertions(+), 84 deletions(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 1ca9fa40a66e..936f1cfd4770 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -122,6 +122,7 @@ static void __dwc3_set_mode(struct work_struct *work)
struct dwc3 *dwc = work_to_dwc(work);
unsigned long flags;
int ret;
+ int i;
u32 reg;
u32 desired_dr_role;
@@ -201,8 +202,10 @@ static void __dwc3_set_mode(struct work_struct *work)
} 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_ports; i++) {
+ phy_set_mode(dwc->usb2_generic_phy[i], PHY_MODE_USB_HOST);
+ 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;
@@ -217,8 +220,8 @@ static void __dwc3_set_mode(struct work_struct *work)
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);
+ 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)
@@ -660,22 +663,14 @@ static int dwc3_core_ulpi_init(struct dwc3 *dwc)
return ret;
}
-/**
- * dwc3_phy_setup - Configure USB PHY Interface of DWC3 Core
- * @dwc: Pointer to our controller context structure
- *
- * Returns 0 on success. The USB PHY interfaces are configured but not
- * initialized. The PHY interfaces and the PHYs get initialized together with
- * the core in dwc3_core_init.
- */
-static int dwc3_phy_setup(struct dwc3 *dwc)
+static int dwc3_ss_phy_setup(struct dwc3 *dwc, int index)
{
unsigned int hw_mode;
u32 reg;
hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
- reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(0));
+ reg = dwc3_readl(dwc->regs, DWC3_GUSB3PIPECTL(index));
/*
* Make sure UX_EXIT_PX is cleared as that causes issues with some
@@ -730,9 +725,19 @@ 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);
+ dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(index), reg);
+
+ return 0;
+}
+
+static int dwc3_hs_phy_setup(struct dwc3 *dwc, int index)
+{
+ unsigned int hw_mode;
+ u32 reg;
- reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
+ hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
+
+ reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(index));
/* Select the HS PHY interface */
switch (DWC3_GHWPARAMS3_HSPHY_IFC(dwc->hwparams.hwparams3)) {
@@ -744,7 +749,7 @@ 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);
+ dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(index), reg);
} else {
/* Relying on default value. */
if (!(reg & DWC3_GUSB2PHYCFG_ULPI_UTMI))
@@ -801,7 +806,35 @@ static int dwc3_phy_setup(struct dwc3 *dwc)
if (dwc->dis_u2_freeclk_exists_quirk || dwc->gfladj_refclk_lpm_sel)
reg &= ~DWC3_GUSB2PHYCFG_U2_FREECLK_EXISTS;
- dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
+ dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(index), reg);
+
+ return 0;
+}
+
+/**
+ * dwc3_phy_setup - Configure USB PHY Interface of DWC3 Core
+ * @dwc: Pointer to our controller context structure
+ *
+ * Returns 0 on success. The USB PHY interfaces are configured but not
+ * initialized. The PHY interfaces and the PHYs get initialized together with
+ * the core in dwc3_core_init.
+ */
+static int dwc3_phy_setup(struct dwc3 *dwc)
+{
+ int i;
+ int ret;
+
+ for (i = 0; i < dwc->num_ss_ports; i++) {
+ ret = dwc3_ss_phy_setup(dwc, i);
+ if (ret)
+ return ret;
+ }
+
+ for (i = 0; i < dwc->num_ports; i++) {
+ ret = dwc3_hs_phy_setup(dwc, i);
+ if (ret)
+ return ret;
+ }
return 0;
}
@@ -840,6 +873,7 @@ static void dwc3_clk_disable(struct dwc3 *dwc)
static void dwc3_core_exit(struct dwc3 *dwc)
{
+ int i;
unsigned int hw_mode;
hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
@@ -848,13 +882,18 @@ static void dwc3_core_exit(struct dwc3 *dwc)
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_ports; i++) {
+ phy_power_off(dwc->usb2_generic_phy[i]);
+ phy_power_off(dwc->usb3_generic_phy[i]);
+ }
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_ports; i++) {
+ phy_exit(dwc->usb2_generic_phy[i]);
+ phy_exit(dwc->usb3_generic_phy[i]);
+ }
dwc3_clk_disable(dwc);
reset_control_assert(dwc->reset);
@@ -1090,6 +1129,7 @@ static int dwc3_core_init(struct dwc3 *dwc)
unsigned int hw_mode;
u32 reg;
int ret;
+ int i, j;
hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
@@ -1124,14 +1164,27 @@ static int dwc3_core_init(struct dwc3 *dwc)
usb_phy_init(dwc->usb2_phy);
usb_phy_init(dwc->usb3_phy);
- ret = phy_init(dwc->usb2_generic_phy);
- if (ret < 0)
- goto err0a;
- ret = phy_init(dwc->usb3_generic_phy);
- if (ret < 0) {
- phy_exit(dwc->usb2_generic_phy);
- goto err0a;
+ for (i = 0; i < dwc->num_ports; i++) {
+ ret = phy_init(dwc->usb2_generic_phy[i]);
+ if (ret < 0) {
+ /* clean up prior initialized HS PHYs */
+ for (j = 0; j < i; j++)
+ phy_exit(dwc->usb2_generic_phy[j]);
+ goto err0a;
+ }
+ }
+
+ for (i = 0; i < dwc->num_ports; i++) {
+ ret = phy_init(dwc->usb3_generic_phy[i]);
+ if (ret < 0) {
+ /* clean up prior initialized SS PHYs */
+ for (j = 0; j < i; j++)
+ phy_exit(dwc->usb3_generic_phy[j]);
+ for (j = 0; j < dwc->num_ports; j++)
+ phy_exit(dwc->usb2_generic_phy[j]);
+ goto err0a;
+ }
}
ret = dwc3_core_soft_reset(dwc);
@@ -1141,15 +1194,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_ss_ports; 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_ports; i++) {
+ reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(i));
+ reg |= DWC3_GUSB2PHYCFG_SUSPHY;
+ dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(i), reg);
+ }
}
}
@@ -1173,13 +1230,24 @@ static int dwc3_core_init(struct dwc3 *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;
- ret = phy_power_on(dwc->usb3_generic_phy);
- if (ret < 0)
- goto err3;
+ for (i = 0; i < dwc->num_ports; i++) {
+ ret = phy_power_on(dwc->usb2_generic_phy[i]);
+ if (ret < 0) {
+ for (j = 0; j < i; j++)
+ phy_power_off(dwc->usb2_generic_phy[j]);
+ goto err2;
+ }
+ }
+
+ for (i = 0; i < dwc->num_ports; i++) {
+ ret = phy_power_on(dwc->usb3_generic_phy[i]);
+ if (ret < 0) {
+ for (j = 0; j < i; j++)
+ phy_power_off(dwc->usb3_generic_phy[j]);
+ goto err3;
+ }
+ }
if (hw_mode != DWC3_GHWPARAMS0_MODE_HOST) {
ret = dwc3_event_buffers_setup(dwc);
@@ -1304,10 +1372,12 @@ static int dwc3_core_init(struct dwc3 *dwc)
return 0;
err4:
- phy_power_off(dwc->usb3_generic_phy);
+ for (i = 0; i < dwc->num_ports; i++)
+ phy_power_off(dwc->usb3_generic_phy[i]);
err3:
- phy_power_off(dwc->usb2_generic_phy);
+ for (i = 0; i < dwc->num_ports; i++)
+ phy_power_off(dwc->usb2_generic_phy[i]);
err2:
usb_phy_set_suspend(dwc->usb2_phy, 1);
@@ -1316,8 +1386,11 @@ static int dwc3_core_init(struct dwc3 *dwc)
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_ports; i++) {
+ phy_exit(dwc->usb2_generic_phy[i]);
+ phy_exit(dwc->usb3_generic_phy[i]);
+ }
err0a:
dwc3_ulpi_exit(dwc);
@@ -1326,6 +1399,38 @@ static int dwc3_core_init(struct dwc3 *dwc)
return ret;
}
+static int dwc3_get_multiport_phys(struct dwc3 *dwc)
+{
+ int ret;
+ struct device *dev = dwc->dev;
+ int i;
+ char phy_name[11];
+
+ for (i = 0; i < dwc->num_ports; i++) {
+ sprintf(phy_name, "usb2-port%d", i);
+ dwc->usb2_generic_phy[i] = devm_phy_get(dev, phy_name);
+ if (IS_ERR(dwc->usb2_generic_phy[i])) {
+ ret = PTR_ERR(dwc->usb2_generic_phy[i]);
+ if (ret == -ENOSYS || ret == -ENODEV)
+ dwc->usb2_generic_phy[i] = NULL;
+ else
+ return dev_err_probe(dev, ret, "usb2 phy: %s not configured\n", phy_name);
+ }
+
+ sprintf(phy_name, "usb3-port%d", i);
+ dwc->usb3_generic_phy[i] = devm_phy_get(dev, phy_name);
+ if (IS_ERR(dwc->usb3_generic_phy[i])) {
+ ret = PTR_ERR(dwc->usb3_generic_phy[i]);
+ if (ret == -ENOSYS || ret == -ENODEV)
+ dwc->usb3_generic_phy[i] = NULL;
+ else
+ return dev_err_probe(dev, ret, "usb3 phy: %s not configured\n", phy_name);
+ }
+ }
+
+ return 0;
+}
+
static int dwc3_core_get_phy(struct dwc3 *dwc)
{
struct device *dev = dwc->dev;
@@ -1356,20 +1461,24 @@ static int dwc3_core_get_phy(struct dwc3 *dwc)
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 (dwc->num_ports > 1)
+ return dwc3_get_multiport_phys(dwc);
+
+
+ dwc->usb2_generic_phy[0] = devm_phy_get(dev, "usb2-phy");
+ if (IS_ERR(dwc->usb2_generic_phy[0])) {
+ ret = PTR_ERR(dwc->usb2_generic_phy[0]);
if (ret == -ENOSYS || ret == -ENODEV)
- dwc->usb2_generic_phy = NULL;
+ dwc->usb2_generic_phy[0] = 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);
+ dwc->usb3_generic_phy[0] = devm_phy_get(dev, "usb3-phy");
+ if (IS_ERR(dwc->usb3_generic_phy[0])) {
+ ret = PTR_ERR(dwc->usb3_generic_phy[0]);
if (ret == -ENOSYS || ret == -ENODEV)
- dwc->usb3_generic_phy = NULL;
+ dwc->usb3_generic_phy[0] = NULL;
else
return dev_err_probe(dev, ret, "no usb3 phy configured\n");
}
@@ -1381,6 +1490,7 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
{
struct device *dev = dwc->dev;
int ret;
+ int i;
switch (dwc->dr_mode) {
case USB_DR_MODE_PERIPHERAL:
@@ -1388,8 +1498,8 @@ static int dwc3_core_init_mode(struct dwc3 *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);
+ 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)
@@ -1400,8 +1510,10 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
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_ports; i++) {
+ phy_set_mode(dwc->usb2_generic_phy[i], PHY_MODE_USB_HOST);
+ phy_set_mode(dwc->usb3_generic_phy[i], PHY_MODE_USB_HOST);
+ }
ret = dwc3_host_init(dwc);
if (ret)
@@ -1823,6 +1935,7 @@ static int dwc3_probe(struct platform_device *pdev)
struct dwc3 *dwc;
int ret;
+ int i;
unsigned int hw_mode;
void __iomem *regs;
@@ -2020,13 +2133,19 @@ static int dwc3_probe(struct platform_device *pdev)
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_ports; i++) {
+ phy_power_off(dwc->usb2_generic_phy[i]);
+ phy_power_off(dwc->usb3_generic_phy[i]);
+ }
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_ports; i++) {
+ phy_exit(dwc->usb2_generic_phy[i]);
+ phy_exit(dwc->usb3_generic_phy[i]);
+ }
dwc3_ulpi_exit(dwc);
@@ -2108,6 +2227,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;
@@ -2128,17 +2248,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);
+ for (i = 0; i < dwc->num_ports; 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_ports; i++) {
+ phy_pm_runtime_put_sync(dwc->usb2_generic_phy[i]);
+ phy_pm_runtime_put_sync(dwc->usb3_generic_phy[i]);
+ }
break;
case DWC3_GCTL_PRTCAP_OTG:
/* do nothing during runtime_suspend */
@@ -2167,6 +2291,7 @@ static int dwc3_resume_common(struct dwc3 *dwc, pm_message_t msg)
{
unsigned long flags;
int ret;
+ int i;
u32 reg;
switch (dwc->current_dr_role) {
@@ -2187,17 +2312,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_ports; 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_ports; i++) {
+ phy_pm_runtime_get_sync(dwc->usb2_generic_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 74386d6a0277..e63e6b514d0d 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -35,6 +35,9 @@
#define DWC3_MSG_MAX 500
+/* Numer of ports supported by a multiport controller */
+#define MAX_PORTS_SUPPORTED 4
+
/* XHCI Reg constants */
#define DWC3_XHCI_HCSPARAMS1 0x04
@@ -1030,8 +1033,8 @@ struct dwc3_scratchpad_array {
* presuming each port is atleast HS capable
* @num_ss_ports: Indicates the number of USB ports present on HW that are
* SS Capable
- * @usb2_generic_phy: pointer to USB2 PHY
- * @usb3_generic_phy: pointer to USB3 PHY
+ * @usb2_generic_phy: pointer to array of USB2 PHY
+ * @usb3_generic_phy: pointer to array of USB3 PHY
* @phys_ready: flag to indicate that PHYs are ready
* @ulpi: pointer to ulpi interface
* @ulpi_ready: flag to indicate that ULPI is initialized
@@ -1167,8 +1170,8 @@ struct dwc3 {
u32 num_ports;
u32 num_ss_ports;
- struct phy *usb2_generic_phy;
- struct phy *usb3_generic_phy;
+ struct phy *usb2_generic_phy[MAX_PORTS_SUPPORTED];
+ struct phy *usb3_generic_phy[MAX_PORTS_SUPPORTED];
bool phys_ready;
diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
index 039bf241769a..b34260b627fe 100644
--- a/drivers/usb/dwc3/drd.c
+++ b/drivers/usb/dwc3/drd.c
@@ -328,6 +328,7 @@ static void dwc3_otg_device_exit(struct dwc3 *dwc)
void dwc3_otg_update(struct dwc3 *dwc, bool ignore_idstatus)
{
int ret;
+ int i;
u32 reg;
int id;
unsigned long flags;
@@ -386,9 +387,11 @@ void dwc3_otg_update(struct dwc3 *dwc, bool ignore_idstatus)
} 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,
- PHY_MODE_USB_HOST);
+ for (i = 0; i < dwc->num_ports; i++) {
+ if (dwc->usb2_generic_phy[i])
+ phy_set_mode(dwc->usb2_generic_phy[i],
+ PHY_MODE_USB_HOST);
+ }
}
break;
case DWC3_OTG_ROLE_DEVICE:
@@ -400,8 +403,8 @@ void dwc3_otg_update(struct dwc3 *dwc, bool ignore_idstatus)
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_generic_phy[0])
+ phy_set_mode(dwc->usb2_generic_phy[0],
PHY_MODE_USB_DEVICE);
ret = dwc3_gadget_init(dwc);
if (ret)
--
2.39.0
On some SoC's like SA8295P where the teritiary controller is host-only
capable, GEVTADDRHI/LO, GEVTSIZ, GEVTCOUNT registers are not accessible.
Trying to setup them up during core_init leads to a crash.
For DRD/Peripheral supported controllers, event buffer setup is done
again in gadget_pullup. Skip setup or cleanup of event buffers if
controller is host-only capable.
Signed-off-by: Krishna Kurapati <[email protected]>
---
drivers/usb/dwc3/core.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 076c0f8a4441..1ca9fa40a66e 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -840,7 +840,11 @@ static void dwc3_clk_disable(struct dwc3 *dwc)
static void dwc3_core_exit(struct dwc3 *dwc)
{
- dwc3_event_buffers_cleanup(dwc);
+ unsigned int hw_mode;
+
+ hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
+ if (hw_mode != DWC3_GHWPARAMS0_MODE_HOST)
+ dwc3_event_buffers_cleanup(dwc);
usb_phy_set_suspend(dwc->usb2_phy, 1);
usb_phy_set_suspend(dwc->usb3_phy, 1);
@@ -1177,10 +1181,12 @@ static int dwc3_core_init(struct dwc3 *dwc)
if (ret < 0)
goto err3;
- ret = dwc3_event_buffers_setup(dwc);
- if (ret) {
- dev_err(dwc->dev, "failed to setup event buffers\n");
- goto err4;
+ if (hw_mode != DWC3_GHWPARAMS0_MODE_HOST) {
+ ret = dwc3_event_buffers_setup(dwc);
+ if (ret) {
+ dev_err(dwc->dev, "failed to setup event buffers\n");
+ goto err4;
+ }
}
/*
@@ -2008,7 +2014,9 @@ static int dwc3_probe(struct platform_device *pdev)
err5:
dwc3_debugfs_exit(dwc);
- dwc3_event_buffers_cleanup(dwc);
+
+ if (hw_mode != DWC3_GHWPARAMS0_MODE_HOST)
+ dwc3_event_buffers_cleanup(dwc);
usb_phy_set_suspend(dwc->usb2_phy, 1);
usb_phy_set_suspend(dwc->usb3_phy, 1);
--
2.39.0
QCOM SoC SA8295P's teritiary quad port controller supports 2 HS+SS
ports and 2 HS only ports. Add support for configuring PWR_EVENT_IRQ's
for all the ports during suspend/resume.
Signed-off-by: Krishna Kurapati <[email protected]>
---
drivers/usb/dwc3/dwc3-qcom.c | 28 ++++++++++++++++++++++------
1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index 959fc925ca7c..0396f3e9bbe9 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -37,7 +37,10 @@
#define PIPE3_PHYSTATUS_SW BIT(3)
#define PIPE_UTMI_CLK_DIS BIT(8)
-#define PWR_EVNT_IRQ_STAT_REG 0x58
+#define PWR_EVNT_IRQ1_STAT_REG 0x58
+#define PWR_EVNT_IRQ2_STAT_REG 0x1dc
+#define PWR_EVNT_IRQ3_STAT_REG 0x228
+#define PWR_EVNT_IRQ4_STAT_REG 0x238
#define PWR_EVNT_LPM_IN_L2_MASK BIT(4)
#define PWR_EVNT_LPM_OUT_L2_MASK BIT(5)
@@ -93,6 +96,13 @@ struct dwc3_qcom {
struct icc_path *icc_path_apps;
};
+static u32 pwr_evnt_irq_stat_reg_offset[4] = {
+ PWR_EVNT_IRQ1_STAT_REG,
+ PWR_EVNT_IRQ2_STAT_REG,
+ PWR_EVNT_IRQ3_STAT_REG,
+ PWR_EVNT_IRQ4_STAT_REG,
+};
+
static inline void dwc3_qcom_setbits(void __iomem *base, u32 offset, u32 val)
{
u32 reg;
@@ -413,13 +423,16 @@ static int dwc3_qcom_suspend(struct dwc3_qcom *qcom, bool wakeup)
{
u32 val;
int i, ret;
+ struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
if (qcom->is_suspended)
return 0;
- val = readl(qcom->qscratch_base + PWR_EVNT_IRQ_STAT_REG);
- if (!(val & PWR_EVNT_LPM_IN_L2_MASK))
- dev_err(qcom->dev, "HS-PHY not in L2\n");
+ for (i = 0; i < dwc->num_ports; i++) {
+ val = readl(qcom->qscratch_base + pwr_evnt_irq_stat_reg_offset[i]);
+ if (!(val & PWR_EVNT_LPM_IN_L2_MASK))
+ dev_err(qcom->dev, "HS-PHY%d not in L2\n", i);
+ }
for (i = qcom->num_clocks - 1; i >= 0; i--)
clk_disable_unprepare(qcom->clks[i]);
@@ -446,6 +459,7 @@ static int dwc3_qcom_resume(struct dwc3_qcom *qcom, bool wakeup)
{
int ret;
int i;
+ struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
if (!qcom->is_suspended)
return 0;
@@ -467,8 +481,10 @@ static int dwc3_qcom_resume(struct dwc3_qcom *qcom, bool wakeup)
dev_warn(qcom->dev, "failed to enable interconnect: %d\n", ret);
/* Clear existing events from PHY related to L2 in/out */
- dwc3_qcom_setbits(qcom->qscratch_base, PWR_EVNT_IRQ_STAT_REG,
- PWR_EVNT_LPM_IN_L2_MASK | PWR_EVNT_LPM_OUT_L2_MASK);
+ for (i = 0; i < dwc->num_ports; i++)
+ dwc3_qcom_setbits(qcom->qscratch_base,
+ pwr_evnt_irq_stat_reg_offset[i],
+ PWR_EVNT_LPM_IN_L2_MASK | PWR_EVNT_LPM_OUT_L2_MASK);
qcom->is_suspended = false;
--
2.39.0
Enable teritiary controller for SA8295P (based on SC8280XP).
Add pinctrl support for usb ports to provide VBUS to connected peripherals.
Signed-off-by: Krishna Kurapati <[email protected]>
---
arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 47 ++++++++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
index fd253942e5e5..7e6061c43835 100644
--- a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
+++ b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
@@ -584,6 +584,19 @@ &usb_1_qmpphy {
status = "okay";
};
+&usb_2 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&usb2_en_state>,
+ <&usb3_en_state>,
+ <&usb4_en_state>,
+ <&usb5_en_state>;
+ status = "okay";
+};
+
+&usb_2_dwc3 {
+ dr_mode = "host";
+};
+
&usb_2_hsphy0 {
vdda-pll-supply = <&vreg_l5a>;
vdda18-supply = <&vreg_l7g>;
@@ -729,3 +742,37 @@ wake-n-pins {
};
};
};
+
+&pmm8540c_gpios {
+ usb2_en_state: usb2-en-state {
+ pins = "gpio9";
+ function = "normal";
+ output-high;
+ power-source = <0>;
+ };
+};
+
+&pmm8540e_gpios {
+ usb3_en_state: usb3-en-state {
+ pins = "gpio5";
+ function = "normal";
+ output-high;
+ power-source = <0>;
+ };
+};
+
+&pmm8540g_gpios {
+ usb4_en_state: usb4-en-state {
+ pins = "gpio5";
+ function = "normal";
+ output-high;
+ power-source = <0>;
+ };
+
+ usb5_en_state: usb5-en-state {
+ pins = "gpio9";
+ function = "normal";
+ output-high;
+ power-source = <0>;
+ };
+};
--
2.39.0
Enable first port of Quad port Teritiary USB controller for SA8540 Ride.
Signed-off-by: Krishna Kurapati <[email protected]>
---
arch/arm64/boot/dts/qcom/sa8540p-ride.dts | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts
index 3ccb5ffdb3ca..46e9ca9c666e 100644
--- a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts
+++ b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts
@@ -309,6 +309,19 @@ &usb_2_qmpphy0 {
status = "okay";
};
+&usb_2 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&usb2_en_state>;
+
+ status = "okay";
+};
+
+&usb_2_dwc3 {
+ dr_mode = "host";
+ phy-names = "usb2-port0", "usb3-port0";
+ phys = <&usb_2_hsphy0>, <&usb_2_qmpphy0>;
+};
+
&xo_board_clk {
clock-frequency = <38400000>;
};
@@ -401,4 +414,13 @@ wake-pins {
bias-pull-up;
};
};
+
+ usb2_en_state: usb2-en-state {
+ /* TS3USB221A USB2.0 mux select */
+ pins = "gpio24";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ output-low;
+ };
};
--
2.39.0
Add USB and DWC3 node for teritiary port of SC8280 along with multiport
IRQ's and phy's. This will be used as a base for SA8295P and SA8295-Ride
platforms.
Signed-off-by: Krishna Kurapati <[email protected]>
---
arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 58 ++++++++++++++++++++++++++
1 file changed, 58 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
index 0d02599d8867..47e1f1da63a4 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
@@ -3108,6 +3108,64 @@ usb_1_role_switch: endpoint {
};
};
+ usb_2: usb@a4f8800 {
+ compatible = "qcom,sc8280xp-dwc3", "qcom,dwc3";
+ reg = <0 0x0a4f8800 0 0x400>;
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ clocks = <&gcc GCC_CFG_NOC_USB3_MP_AXI_CLK>,
+ <&gcc GCC_USB30_MP_MASTER_CLK>,
+ <&gcc GCC_AGGRE_USB3_MP_AXI_CLK>,
+ <&gcc GCC_USB30_MP_SLEEP_CLK>,
+ <&gcc GCC_USB30_MP_MOCK_UTMI_CLK>,
+ <&gcc GCC_AGGRE_USB_NOC_AXI_CLK>,
+ <&gcc GCC_AGGRE_USB_NOC_NORTH_AXI_CLK>,
+ <&gcc GCC_AGGRE_USB_NOC_SOUTH_AXI_CLK>,
+ <&gcc GCC_SYS_NOC_USB_AXI_CLK>;
+ clock-names = "cfg_noc", "core", "iface", "sleep", "mock_utmi",
+ "noc_aggr", "noc_aggr_north", "noc_aggr_south", "noc_sys";
+
+ assigned-clocks = <&gcc GCC_USB30_MP_MOCK_UTMI_CLK>,
+ <&gcc GCC_USB30_MP_MASTER_CLK>;
+ assigned-clock-rates = <19200000>, <200000000>;
+
+ interrupts-extended = <&pdc 127 IRQ_TYPE_EDGE_RISING>,
+ <&pdc 126 IRQ_TYPE_EDGE_RISING>,
+ <&pdc 16 IRQ_TYPE_LEVEL_HIGH>;
+
+ interrupt-names = "dp_hs_phy_irq", "dm_hs_phy_irq",
+ "ss_phy_irq";
+
+ power-domains = <&gcc USB30_MP_GDSC>;
+
+ resets = <&gcc GCC_USB30_MP_BCR>;
+
+ interconnects = <&aggre1_noc MASTER_USB3_1 0 &mc_virt SLAVE_EBI1 0>,
+ <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_USB3_1 0>;
+ interconnect-names = "usb-ddr", "apps-usb";
+
+ required-opps = <&rpmhpd_opp_nom>;
+
+ status = "disabled";
+
+ usb_2_dwc3: usb@a400000 {
+ compatible = "snps,dwc3";
+ reg = <0 0x0a400000 0 0xcd00>;
+ interrupts = <GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>;
+ iommus = <&apps_smmu 0x800 0x0>;
+ phys = <&usb_2_hsphy0>, <&usb_2_qmpphy0>,
+ <&usb_2_hsphy1>, <&usb_2_qmpphy1>,
+ <&usb_2_hsphy2>,
+ <&usb_2_hsphy3>;
+ phy-names = "usb2-port0", "usb3-port0",
+ "usb2-port1", "usb3-port1",
+ "usb2-port2",
+ "usb2-port3";
+ };
+ };
+
mdss0: display-subsystem@ae00000 {
compatible = "qcom,sc8280xp-mdss";
reg = <0 0x0ae00000 0 0x1000>;
--
2.39.0
On 10/03/2023 17:34, Krishna Kurapati wrote:
> Add bindings to indicate properties required to support multiport
> on Snps Dwc3 controller.
>
> Suggested-by: Bjorn Andersson <[email protected]>
> Signed-off-by: Krishna Kurapati <[email protected]>
What happened with entire previous changelog? This is not v1 but v5 or
more? At least v4 was here:
https://lore.kernel.org/all/[email protected]/
Best regards,
Krzysztof
On 3/10/2023 10:11 PM, Krzysztof Kozlowski wrote:
> On 10/03/2023 17:34, Krishna Kurapati wrote:
>> Add bindings to indicate properties required to support multiport
>> on Snps Dwc3 controller.
>>
>> Suggested-by: Bjorn Andersson <[email protected]>
>> Signed-off-by: Krishna Kurapati <[email protected]>
>
> What happened with entire previous changelog? This is not v1 but v5 or
> more? At least v4 was here:
>
> https://lore.kernel.org/all/[email protected]/
>
> Best regards,
> Krzysztof
>
Hi Krzysztof,
Since I pushed a formal patch series, I mentioned PATCH in header
instead of "Patch v5". If the RFC v4 is to be followed by Patch-v5, I
can re-push the changes again with a proper header and fix my mistake.
The previous change log is mentioned in cover letter.
https://lore.kernel.org/all/[email protected]/
Regards,
Krishna,
On 10/03/2023 17:54, Krishna Kurapati PSSNV wrote:
>
>
> On 3/10/2023 10:11 PM, Krzysztof Kozlowski wrote:
>> On 10/03/2023 17:34, Krishna Kurapati wrote:
>>> Add bindings to indicate properties required to support multiport
>>> on Snps Dwc3 controller.
>>>
>>> Suggested-by: Bjorn Andersson <[email protected]>
>>> Signed-off-by: Krishna Kurapati <[email protected]>
>>
>> What happened with entire previous changelog? This is not v1 but v5 or
>> more? At least v4 was here:
>>
>> https://lore.kernel.org/all/[email protected]/
>>
>> Best regards,
>> Krzysztof
>>
> Hi Krzysztof,
>
> Since I pushed a formal patch series, I mentioned PATCH in header
> instead of "Patch v5". If the RFC v4 is to be followed by Patch-v5, I
> can re-push the changes again with a proper header and fix my mistake.
>
> The previous change log is mentioned in cover letter.
>
OK, for the future, first submission is the v1. This is fifth submission.
Best regards,
Krzysztof
On Fri, Mar 10, 2023, Krishna Kurapati wrote:
> Currently host-only capable DWC3 controllers support Multiport. Temporarily
> map XHCI address space for host-only controllers and parse XHCI Extended
> Capabilities registers to read number of physical usb ports connected to the
> multiport controller (presuming each port is at least HS capable) and extract
> info on how many of these ports are Super Speed capable.
>
> Signed-off-by: Krishna Kurapati <[email protected]>
> ---
> drivers/usb/dwc3/core.c | 75 +++++++++++++++++++++++++++++++++++++++++
> drivers/usb/dwc3/core.h | 9 +++++
> 2 files changed, 84 insertions(+)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 476b63618511..076c0f8a4441 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -37,6 +37,7 @@
> #include "core.h"
> #include "gadget.h"
> #include "io.h"
> +#include "../host/xhci.h"
I think better to duplicate some of the logic in dwc3 driver and avoid
any direct dependency with the xhci driver.
>
> #include "debug.h"
>
> @@ -1750,6 +1751,65 @@ static struct extcon_dev *dwc3_get_extcon(struct dwc3 *dwc)
> return edev;
> }
>
> +static int dwc3_read_port_info(struct dwc3 *dwc, struct resource *res)
> +{
> + void __iomem *regs;
> + struct resource dwc_res;
> + u32 offset;
> + u32 temp;
> + u8 major_revision;
> + int ret = 0;
> +
> + /*
> + * Remap xHCI address space to access XHCI ext cap regs,
> + * since it is needed to get port info.
> + */
> + dwc_res = *res;
> + dwc_res.start += 0;
> + dwc_res.end = dwc->xhci_resources[0].start +
> + DWC3_XHCI_REGS_END;
Isn't dwc->xhci_resources[0] already setup at this point? Can we use
dwc->xhci_resources[0] directly without copy the setting in dwc_res?
> +
> + regs = ioremap(dwc_res.start, resource_size(&dwc_res));
> + if (IS_ERR(regs))
> + return PTR_ERR(regs);
> +
> + offset = xhci_find_next_ext_cap(regs, 0,
> + XHCI_EXT_CAPS_PROTOCOL);
> + while (offset) {
> + temp = readl(regs + offset);
> + major_revision = XHCI_EXT_PORT_MAJOR(temp);
> +
> + temp = readl(regs + offset + 0x08);
> + if (major_revision == 0x03) {
> + dwc->num_ss_ports += XHCI_EXT_PORT_COUNT(temp);
> + } else if (major_revision <= 0x02) {
> + dwc->num_ports += XHCI_EXT_PORT_COUNT(temp);
> + } else {
> + dev_err(dwc->dev, "port revision seems wrong\n");
> + ret = -EINVAL;
> + goto unmap_reg;
> + }
> +
> + offset = xhci_find_next_ext_cap(regs, offset,
> + XHCI_EXT_CAPS_PROTOCOL);
> + }
> +
> + temp = readl(regs + DWC3_XHCI_HCSPARAMS1);
> + if (HCS_MAX_PORTS(temp) != (dwc->num_ss_ports + dwc->num_ports)) {
> + dev_err(dwc->dev, "inconsistency in port info\n");
> + ret = -EINVAL;
> + goto unmap_reg;
> + }
> +
> + dev_info(dwc->dev,
> + "num-ports: %d ss-capable: %d\n", dwc->num_ports, dwc->num_ss_ports);
The end user doesn't need to know this info. This should be a debug
message. Perhaps it can be a tracepoint if needed?
> +
> +unmap_reg:
> + iounmap(regs);
> + return ret;
> +}
> +
> static int dwc3_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -1757,6 +1817,7 @@ static int dwc3_probe(struct platform_device *pdev)
> struct dwc3 *dwc;
>
> int ret;
> + unsigned int hw_mode;
>
> void __iomem *regs;
>
> @@ -1880,6 +1941,20 @@ static int dwc3_probe(struct platform_device *pdev)
> goto disable_clks;
> }
>
> + /*
> + * Currently DWC3 controllers that are host-only capable
> + * support Multiport.
> + */
> + hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
> + if (hw_mode == DWC3_GHWPARAMS0_MODE_HOST) {
> + ret = dwc3_read_port_info(dwc, res);
> + if (ret)
> + goto disable_clks;
> + } else {
> + dwc->num_ports = 1;
> + dwc->num_ss_ports = 1;
> + }
> +
> spin_lock_init(&dwc->lock);
> mutex_init(&dwc->mutex);
>
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 582ebd9cf9c2..74386d6a0277 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -35,6 +35,9 @@
>
> #define DWC3_MSG_MAX 500
>
> +/* XHCI Reg constants */
> +#define DWC3_XHCI_HCSPARAMS1 0x04
> +
> /* Global constants */
> #define DWC3_PULL_UP_TIMEOUT 500 /* ms */
> #define DWC3_BOUNCE_SIZE 1024 /* size of a superspeed bulk */
> @@ -1023,6 +1026,10 @@ struct dwc3_scratchpad_array {
> * @usb_psy: pointer to power supply interface.
> * @usb2_phy: pointer to USB2 PHY
> * @usb3_phy: pointer to USB3 PHY
> + * @num_ports: Indicates the number of physical USB ports present on HW
> + * presuming each port is at least HS capable
This isn't the number of physical USB ports right? That's the number of
usb2 ports the controller is configured with right?. Perhaps we can use
num_usb2_ports and num_usb3_ports?
> + * @num_ss_ports: Indicates the number of USB ports present on HW that are
> + * SS Capable
> * @usb2_generic_phy: pointer to USB2 PHY
> * @usb3_generic_phy: pointer to USB3 PHY
> * @phys_ready: flag to indicate that PHYs are ready
> @@ -1158,6 +1165,8 @@ struct dwc3 {
> struct usb_phy *usb2_phy;
> struct usb_phy *usb3_phy;
>
> + u32 num_ports;
> + u32 num_ss_ports;
> struct phy *usb2_generic_phy;
> struct phy *usb3_generic_phy;
>
> --
> 2.39.0
>
Thanks,
Thinh
On 3/11/2023 5:25 AM, Thinh Nguyen wrote:
> On Fri, Mar 10, 2023, Krishna Kurapati wrote:
>> Currently host-only capable DWC3 controllers support Multiport. Temporarily
>> map XHCI address space for host-only controllers and parse XHCI Extended
>> Capabilities registers to read number of physical usb ports connected to the
>> multiport controller (presuming each port is at least HS capable) and extract
>> info on how many of these ports are Super Speed capable.
>>
>> Signed-off-by: Krishna Kurapati <[email protected]>
>> ---
>> drivers/usb/dwc3/core.c | 75 +++++++++++++++++++++++++++++++++++++++++
>> drivers/usb/dwc3/core.h | 9 +++++
>> 2 files changed, 84 insertions(+)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 476b63618511..076c0f8a4441 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -37,6 +37,7 @@
>> #include "core.h"
>> #include "gadget.h"
>> #include "io.h"
>> +#include "../host/xhci.h"
>
> I think better to duplicate some of the logic in dwc3 driver and avoid
> any direct dependency with the xhci driver.
>
>>
>> #include "debug.h"
>>
>> @@ -1750,6 +1751,65 @@ static struct extcon_dev *dwc3_get_extcon(struct dwc3 *dwc)
>> return edev;
>> }
>>
>> +static int dwc3_read_port_info(struct dwc3 *dwc, struct resource *res)
>> +{
>> + void __iomem *regs;
>> + struct resource dwc_res;
>> + u32 offset;
>> + u32 temp;
>> + u8 major_revision;
>> + int ret = 0;
>> +
>> + /*
>> + * Remap xHCI address space to access XHCI ext cap regs,
>> + * since it is needed to get port info.
>> + */
>> + dwc_res = *res;
>> + dwc_res.start += 0;
>> + dwc_res.end = dwc->xhci_resources[0].start +
>> + DWC3_XHCI_REGS_END;
>
> Isn't dwc->xhci_resources[0] already setup at this point? Can we use
> dwc->xhci_resources[0] directly without copy the setting in dwc_res?
>
>> +
>> + regs = ioremap(dwc_res.start, resource_size(&dwc_res));
>> + if (IS_ERR(regs))
>> + return PTR_ERR(regs);
>> +
>> + offset = xhci_find_next_ext_cap(regs, 0,
>> + XHCI_EXT_CAPS_PROTOCOL);
>> + while (offset) {
>> + temp = readl(regs + offset);
>> + major_revision = XHCI_EXT_PORT_MAJOR(temp);
>> +
>> + temp = readl(regs + offset + 0x08);
>> + if (major_revision == 0x03) {
>> + dwc->num_ss_ports += XHCI_EXT_PORT_COUNT(temp);
>> + } else if (major_revision <= 0x02) {
>> + dwc->num_ports += XHCI_EXT_PORT_COUNT(temp);
>> + } else {
>> + dev_err(dwc->dev, "port revision seems wrong\n");
>> + ret = -EINVAL;
>> + goto unmap_reg;
>> + }
>> +
>> + offset = xhci_find_next_ext_cap(regs, offset,
>> + XHCI_EXT_CAPS_PROTOCOL);
>> + }
>> +
>> + temp = readl(regs + DWC3_XHCI_HCSPARAMS1);
>> + if (HCS_MAX_PORTS(temp) != (dwc->num_ss_ports + dwc->num_ports)) {
>> + dev_err(dwc->dev, "inconsistency in port info\n");
>> + ret = -EINVAL;
>> + goto unmap_reg;
>> + }
>> +
>> + dev_info(dwc->dev,
>> + "num-ports: %d ss-capable: %d\n", dwc->num_ports, dwc->num_ss_ports);
>
> The end user doesn't need to know this info. This should be a debug
> message. Perhaps it can be a tracepoint if needed?
>
>> +
>> +unmap_reg:
>> + iounmap(regs);
>> + return ret;
>> +}
>> +
>> static int dwc3_probe(struct platform_device *pdev)
>> {
>> struct device *dev = &pdev->dev;
>> @@ -1757,6 +1817,7 @@ static int dwc3_probe(struct platform_device *pdev)
>> struct dwc3 *dwc;
>>
>> int ret;
>> + unsigned int hw_mode;
>>
>> void __iomem *regs;
>>
>> @@ -1880,6 +1941,20 @@ static int dwc3_probe(struct platform_device *pdev)
>> goto disable_clks;
>> }
>>
>> + /*
>> + * Currently DWC3 controllers that are host-only capable
>> + * support Multiport.
>> + */
>> + hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
>> + if (hw_mode == DWC3_GHWPARAMS0_MODE_HOST) {
>> + ret = dwc3_read_port_info(dwc, res);
>> + if (ret)
>> + goto disable_clks;
>> + } else {
>> + dwc->num_ports = 1;
>> + dwc->num_ss_ports = 1;
>> + }
>> +
>> spin_lock_init(&dwc->lock);
>> mutex_init(&dwc->mutex);
>>
>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>> index 582ebd9cf9c2..74386d6a0277 100644
>> --- a/drivers/usb/dwc3/core.h
>> +++ b/drivers/usb/dwc3/core.h
>> @@ -35,6 +35,9 @@
>>
>> #define DWC3_MSG_MAX 500
>>
>> +/* XHCI Reg constants */
>> +#define DWC3_XHCI_HCSPARAMS1 0x04
>> +
>> /* Global constants */
>> #define DWC3_PULL_UP_TIMEOUT 500 /* ms */
>> #define DWC3_BOUNCE_SIZE 1024 /* size of a superspeed bulk */
>> @@ -1023,6 +1026,10 @@ struct dwc3_scratchpad_array {
>> * @usb_psy: pointer to power supply interface.
>> * @usb2_phy: pointer to USB2 PHY
>> * @usb3_phy: pointer to USB3 PHY
>> + * @num_ports: Indicates the number of physical USB ports present on HW
>> + * presuming each port is at least HS capable
>
> This isn't the number of physical USB ports right? That's the number of
> usb2 ports the controller is configured with right?. Perhaps we can use
> num_usb2_ports and num_usb3_ports?
>
Hi Thinh,
Yes, naming this might have created a little confusion.
num_ports is supposed to indicate number of usb2 ports in the controller.
Incase of sa8295 (4 port controller with first two ports having ss
capability), num_ports would be 4 and num_ss_ports would be 2. (and not
6 as what num_ports usually sounds).
I can rename them accordingly in the next version and update the
description as well.
Regards,
Krishna,
>> + * @num_ss_ports: Indicates the number of USB ports present on HW that are
>> + * SS Capable
>> * @usb2_generic_phy: pointer to USB2 PHY
>> * @usb3_generic_phy: pointer to USB3 PHY
>> * @phys_ready: flag to indicate that PHYs are ready
>> @@ -1158,6 +1165,8 @@ struct dwc3 {
>> struct usb_phy *usb2_phy;
>> struct usb_phy *usb3_phy;
>>
>> + u32 num_ports;
>> + u32 num_ss_ports;
>> struct phy *usb2_generic_phy;
>> struct phy *usb3_generic_phy;
>>
>> --
>> 2.39.0
>>
>
> Thanks,
> Thinh
On Fri, 10 Mar 2023 22:04:13 +0530, Krishna Kurapati wrote:
> Add bindings to indicate properties required to support multiport
> on Snps Dwc3 controller.
>
> Suggested-by: Bjorn Andersson <[email protected]>
> Signed-off-by: Krishna Kurapati <[email protected]>
> ---
> .../devicetree/bindings/usb/snps,dwc3.yaml | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
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:90:5: [warning] wrong indentation: expected 6 but found 4 (indentation)
dtschema/dtc warnings/errors:
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
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 after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
On 3/11/2023 8:24 AM, Krishna Kurapati PSSNV wrote:
>
>
> On 3/11/2023 5:25 AM, Thinh Nguyen wrote:
>> On Fri, Mar 10, 2023, Krishna Kurapati wrote:
>>> Currently host-only capable DWC3 controllers support Multiport.
>>> Temporarily
>>> map XHCI address space for host-only controllers and parse XHCI Extended
>>> Capabilities registers to read number of physical usb ports connected
>>> to the
>>> multiport controller (presuming each port is at least HS capable) and
>>> extract
>>> info on how many of these ports are Super Speed capable.
>>>
>>> Signed-off-by: Krishna Kurapati <[email protected]>
>>> ---
>>> drivers/usb/dwc3/core.c | 75 +++++++++++++++++++++++++++++++++++++++++
>>> drivers/usb/dwc3/core.h | 9 +++++
>>> 2 files changed, 84 insertions(+)
>>>
>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>> index 476b63618511..076c0f8a4441 100644
>>> --- a/drivers/usb/dwc3/core.c
>>> +++ b/drivers/usb/dwc3/core.c
>>> @@ -37,6 +37,7 @@
>>> #include "core.h"
>>> #include "gadget.h"
>>> #include "io.h"
>>> +#include "../host/xhci.h"
>>
>> I think better to duplicate some of the logic in dwc3 driver and avoid
>> any direct dependency with the xhci driver.
>>
>>> #include "debug.h"
>>> @@ -1750,6 +1751,65 @@ static struct extcon_dev
>>> *dwc3_get_extcon(struct dwc3 *dwc)
>>> return edev;
>>> }
>>> +static int dwc3_read_port_info(struct dwc3 *dwc, struct resource *res)
>>> +{
>>> + void __iomem *regs;
>>> + struct resource dwc_res;
>>> + u32 offset;
>>> + u32 temp;
>>> + u8 major_revision;
>>> + int ret = 0;
>>> +
>>> + /*
>>> + * Remap xHCI address space to access XHCI ext cap regs,
>>> + * since it is needed to get port info.
>>> + */
>>> + dwc_res = *res;
>>> + dwc_res.start += 0;
>>> + dwc_res.end = dwc->xhci_resources[0].start +
>>> + DWC3_XHCI_REGS_END;
>>
>> Isn't dwc->xhci_resources[0] already setup at this point? Can we use
>> dwc->xhci_resources[0] directly without copy the setting in dwc_res?
>>
>>> +
>>> + regs = ioremap(dwc_res.start, resource_size(&dwc_res));
>>> + if (IS_ERR(regs))
>>> + return PTR_ERR(regs);
>>> +
>>> + offset = xhci_find_next_ext_cap(regs, 0,
>>> + XHCI_EXT_CAPS_PROTOCOL);
>>> + while (offset) {
>>> + temp = readl(regs + offset);
>>> + major_revision = XHCI_EXT_PORT_MAJOR(temp);
>>> +
>>> + temp = readl(regs + offset + 0x08);
>>> + if (major_revision == 0x03) {
>>> + dwc->num_ss_ports += XHCI_EXT_PORT_COUNT(temp);
>>> + } else if (major_revision <= 0x02) {
>>> + dwc->num_ports += XHCI_EXT_PORT_COUNT(temp);
>>> + } else {
>>> + dev_err(dwc->dev, "port revision seems wrong\n");
>>> + ret = -EINVAL;
>>> + goto unmap_reg;
>>> + }
>>> +
>>> + offset = xhci_find_next_ext_cap(regs, offset,
>>> + XHCI_EXT_CAPS_PROTOCOL);
>>> + }
>>> +
>>> + temp = readl(regs + DWC3_XHCI_HCSPARAMS1);
>>> + if (HCS_MAX_PORTS(temp) != (dwc->num_ss_ports + dwc->num_ports)) {
>>> + dev_err(dwc->dev, "inconsistency in port info\n");
>>> + ret = -EINVAL;
>>> + goto unmap_reg;
>>> + }
>>> +
>>> + dev_info(dwc->dev,
>>> + "num-ports: %d ss-capable: %d\n", dwc->num_ports,
>>> dwc->num_ss_ports);
>>
>> The end user doesn't need to know this info. This should be a debug
>> message. Perhaps it can be a tracepoint if needed?
>>
>>> +
>>> +unmap_reg:
>>> + iounmap(regs);
>>> + return ret;
>>> +}
>>> +
>>> static int dwc3_probe(struct platform_device *pdev)
>>> {
>>> struct device *dev = &pdev->dev;
>>> @@ -1757,6 +1817,7 @@ static int dwc3_probe(struct platform_device
>>> *pdev)
>>> struct dwc3 *dwc;
>>> int ret;
>>> + unsigned int hw_mode;
>>> void __iomem *regs;
>>> @@ -1880,6 +1941,20 @@ static int dwc3_probe(struct platform_device
>>> *pdev)
>>> goto disable_clks;
>>> }
>>> + /*
>>> + * Currently DWC3 controllers that are host-only capable
>>> + * support Multiport.
>>> + */
>>> + hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
>>> + if (hw_mode == DWC3_GHWPARAMS0_MODE_HOST) {
>>> + ret = dwc3_read_port_info(dwc, res);
>>> + if (ret)
>>> + goto disable_clks;
>>> + } else {
>>> + dwc->num_ports = 1;
>>> + dwc->num_ss_ports = 1;
>>> + }
>>> +
>>> spin_lock_init(&dwc->lock);
>>> mutex_init(&dwc->mutex);
>>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>>> index 582ebd9cf9c2..74386d6a0277 100644
>>> --- a/drivers/usb/dwc3/core.h
>>> +++ b/drivers/usb/dwc3/core.h
>>> @@ -35,6 +35,9 @@
>>> #define DWC3_MSG_MAX 500
>>> +/* XHCI Reg constants */
>>> +#define DWC3_XHCI_HCSPARAMS1 0x04
>>> +
>>> /* Global constants */
>>> #define DWC3_PULL_UP_TIMEOUT 500 /* ms */
>>> #define DWC3_BOUNCE_SIZE 1024 /* size of a superspeed bulk */
>>> @@ -1023,6 +1026,10 @@ struct dwc3_scratchpad_array {
>>> * @usb_psy: pointer to power supply interface.
>>> * @usb2_phy: pointer to USB2 PHY
>>> * @usb3_phy: pointer to USB3 PHY
>>> + * @num_ports: Indicates the number of physical USB ports present on HW
>>> + * presuming each port is at least HS capable
>>
>> This isn't the number of physical USB ports right? That's the number of
>> usb2 ports the controller is configured with right?. Perhaps we can use
>> num_usb2_ports and num_usb3_ports?
>>
> Hi Thinh,
>
> Yes, naming this might have created a little confusion.
> num_ports is supposed to indicate number of usb2 ports in the controller.
>
> Incase of sa8295 (4 port controller with first two ports having ss
> capability), num_ports would be 4 and num_ss_ports would be 2. (and not
> 6 as what num_ports usually sounds).
> I can rename them accordingly in the next version and update the
> description as well.
>
> Regards,
> Krishna,
>
Hi Thinh,
One reason I didn't mention something like num_hs_ports and sticked to
num_ports is because in core driver, wherever we need to do phy
operations like:
for (i = 0; i < num_ports; i++)
{
phy_set_mode(dwc->usb2_generic_phy[i], PHY_MODE_USB_HOST);
phy_set_mode(dwc->usb3_generic_phy[i], PHY_MODE_USB_HOST);
}
The intention is as follows:
If number of usb2 ports is 4, the loop can go from 0-3 and its fine.
If number of usb3-ports is 2, we don't know for sure, if the first 2
ports are SS capable or some other ports like (3 and 4) are SS capable.
So instead, I looped all phy operations around all usb2_generic_phy's
and usb3_generic_phy's. If they are NULL, we just bail out inside phy
operation.
While doing so, looping SS Phy operations around num_usb2_ports didn't
sound good. From code view, it would be like we are looping usb3_phy ops
around num_usb2_ports value (logically it is still correct as each port
is atleast HS capable). So to avoid this, I named the variable as
num_ports instead of num_usb2_ports
Regards,
Krishna,
>>> + * @num_ss_ports: Indicates the number of USB ports present on HW
>>> that are
>>> + * SS Capable
>>> * @usb2_generic_phy: pointer to USB2 PHY
>>> * @usb3_generic_phy: pointer to USB3 PHY
>>> * @phys_ready: flag to indicate that PHYs are ready
>>> @@ -1158,6 +1165,8 @@ struct dwc3 {
>>> struct usb_phy *usb2_phy;
>>> struct usb_phy *usb3_phy;
>>> + u32 num_ports;
>>> + u32 num_ss_ports;
>>> struct phy *usb2_generic_phy;
>>> struct phy *usb3_generic_phy;
>>> --
>>> 2.39.0
>>>
>>
>> Thanks,
>> Thinh
Hello!
On 3/10/23 7:34 PM, Krishna Kurapati wrote:
> Enable teritiary controller for SA8295P (based on SC8280XP).
Tertiary?
> Add pinctrl support for usb ports to provide VBUS to connected peripherals.
>
> Signed-off-by: Krishna Kurapati <[email protected]>
[...]
MBR, Sergey
On 3/10/23 7:34 PM, Krishna Kurapati wrote:
> On some SoC's like SA8295P where the teritiary controller is host-only
Likewise, tertiary. :-)
> capable, GEVTADDRHI/LO, GEVTSIZ, GEVTCOUNT registers are not accessible.
> Trying to setup them up during core_init leads to a crash.
>
> For DRD/Peripheral supported controllers, event buffer setup is done
> again in gadget_pullup. Skip setup or cleanup of event buffers if
> controller is host-only capable.
>
> Signed-off-by: Krishna Kurapati <[email protected]>
[...]
MBR, Sergey
On Sat, Mar 11, 2023 at 12:40 AM Krishna Kurapati
<[email protected]> wrote:
>
> On some SoC's like SA8295P where the teritiary controller is host-only
> capable, GEVTADDRHI/LO, GEVTSIZ, GEVTCOUNT registers are not accessible.
> Trying to setup them up during core_init leads to a crash.
>
> For DRD/Peripheral supported controllers, event buffer setup is done
> again in gadget_pullup. Skip setup or cleanup of event buffers if
> controller is host-only capable.
>
> Signed-off-by: Krishna Kurapati <[email protected]>
> ---
> drivers/usb/dwc3/core.c | 20 ++++++++++++++------
> 1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 076c0f8a4441..1ca9fa40a66e 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -840,7 +840,11 @@ static void dwc3_clk_disable(struct dwc3 *dwc)
>
> static void dwc3_core_exit(struct dwc3 *dwc)
> {
> - dwc3_event_buffers_cleanup(dwc);
> + unsigned int hw_mode;
> +
> + hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
> + if (hw_mode != DWC3_GHWPARAMS0_MODE_HOST)
> + dwc3_event_buffers_cleanup(dwc);
quick question about dwc3_event_buffers_cleanup, there are other
similar sites calling this function.
C symbol: dwc3_event_buffers_cleanup
File Function Line
0 core.h <global> 1546 void
dwc3_event_buffers_cleanup(struct dwc3 *dwc);
1 core.c __dwc3_set_mode 152 dwc3_event_buffers_cleanup(dwc);
2 core.c dwc3_event_buffers_cleanup 522 void
dwc3_event_buffers_cleanup(struct dwc3 *dwc)
3 core.c dwc3_core_exit 842 dwc3_event_buffers_cleanup(dwc);
4 core.c dwc3_probe 1936 dwc3_event_buffers_cleanup(dwc);
5 drd.c dwc3_otg_update 363 dwc3_event_buffers_cleanup(dwc);
6 drd.c dwc3_drd_exit 607 dwc3_event_buffers_cleanup(dwc);
For 1, 5, and 6, any need to take care of this situation?
>
> usb_phy_set_suspend(dwc->usb2_phy, 1);
> usb_phy_set_suspend(dwc->usb3_phy, 1);
> @@ -1177,10 +1181,12 @@ static int dwc3_core_init(struct dwc3 *dwc)
> if (ret < 0)
> goto err3;
>
> - ret = dwc3_event_buffers_setup(dwc);
> - if (ret) {
> - dev_err(dwc->dev, "failed to setup event buffers\n");
> - goto err4;
> + if (hw_mode != DWC3_GHWPARAMS0_MODE_HOST) {
> + ret = dwc3_event_buffers_setup(dwc);
> + if (ret) {
> + dev_err(dwc->dev, "failed to setup event buffers\n");
> + goto err4;
> + }
> }
>
> /*
> @@ -2008,7 +2014,9 @@ static int dwc3_probe(struct platform_device *pdev)
>
> err5:
> dwc3_debugfs_exit(dwc);
> - dwc3_event_buffers_cleanup(dwc);
> +
> + if (hw_mode != DWC3_GHWPARAMS0_MODE_HOST)
> + dwc3_event_buffers_cleanup(dwc);
>
> usb_phy_set_suspend(dwc->usb2_phy, 1);
> usb_phy_set_suspend(dwc->usb3_phy, 1);
> --
> 2.39.0
>
On 3/13/2023 7:37 AM, Dongliang Mu wrote:
> On Sat, Mar 11, 2023 at 12:40 AM Krishna Kurapati
> <[email protected]> wrote:
>>
>> On some SoC's like SA8295P where the teritiary controller is host-only
>> capable, GEVTADDRHI/LO, GEVTSIZ, GEVTCOUNT registers are not accessible.
>> Trying to setup them up during core_init leads to a crash.
>>
>> For DRD/Peripheral supported controllers, event buffer setup is done
>> again in gadget_pullup. Skip setup or cleanup of event buffers if
>> controller is host-only capable.
>>
>> Signed-off-by: Krishna Kurapati <[email protected]>
>> ---
>> drivers/usb/dwc3/core.c | 20 ++++++++++++++------
>> 1 file changed, 14 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 076c0f8a4441..1ca9fa40a66e 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -840,7 +840,11 @@ static void dwc3_clk_disable(struct dwc3 *dwc)
>>
>> static void dwc3_core_exit(struct dwc3 *dwc)
>> {
>> - dwc3_event_buffers_cleanup(dwc);
>> + unsigned int hw_mode;
>> +
>> + hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
>> + if (hw_mode != DWC3_GHWPARAMS0_MODE_HOST)
>> + dwc3_event_buffers_cleanup(dwc);
>
> quick question about dwc3_event_buffers_cleanup, there are other
> similar sites calling this function.
>
> C symbol: dwc3_event_buffers_cleanup
>
> File Function Line
> 0 core.h <global> 1546 void
> dwc3_event_buffers_cleanup(struct dwc3 *dwc);
> 1 core.c __dwc3_set_mode 152 dwc3_event_buffers_cleanup(dwc);
> 2 core.c dwc3_event_buffers_cleanup 522 void
> dwc3_event_buffers_cleanup(struct dwc3 *dwc)
> 3 core.c dwc3_core_exit 842 dwc3_event_buffers_cleanup(dwc);
> 4 core.c dwc3_probe 1936 dwc3_event_buffers_cleanup(dwc);
> 5 drd.c dwc3_otg_update 363 dwc3_event_buffers_cleanup(dwc);
> 6 drd.c dwc3_drd_exit 607 dwc3_event_buffers_cleanup(dwc);
>
> For 1, 5, and 6, any need to take care of this situation?
>
Hi Dongliang,
Thanks for the review.
In the other places mentioned like set_mode otg_update or drd_exit,
cleanup is called if we are in device mode and we want to exit that
mode. Since for MP, we have a host only controller those paths won't be
accessed I believe.
Regards,
Krishna,
>>
>> usb_phy_set_suspend(dwc->usb2_phy, 1);
>> usb_phy_set_suspend(dwc->usb3_phy, 1);
>> @@ -1177,10 +1181,12 @@ static int dwc3_core_init(struct dwc3 *dwc)
>> if (ret < 0)
>> goto err3;
>>
>> - ret = dwc3_event_buffers_setup(dwc);
>> - if (ret) {
>> - dev_err(dwc->dev, "failed to setup event buffers\n");
>> - goto err4;
>> + if (hw_mode != DWC3_GHWPARAMS0_MODE_HOST) {
>> + ret = dwc3_event_buffers_setup(dwc);
>> + if (ret) {
>> + dev_err(dwc->dev, "failed to setup event buffers\n");
>> + goto err4;
>> + }
>> }
>>
>> /*
>> @@ -2008,7 +2014,9 @@ static int dwc3_probe(struct platform_device *pdev)
>>
>> err5:
>> dwc3_debugfs_exit(dwc);
>> - dwc3_event_buffers_cleanup(dwc);
>> +
>> + if (hw_mode != DWC3_GHWPARAMS0_MODE_HOST)
>> + dwc3_event_buffers_cleanup(dwc);
>>
>> usb_phy_set_suspend(dwc->usb2_phy, 1);
>> usb_phy_set_suspend(dwc->usb3_phy, 1);
>> --
>> 2.39.0
>>
On Sat, Mar 11, 2023, Krishna Kurapati PSSNV wrote:
>
>
> On 3/11/2023 8:24 AM, Krishna Kurapati PSSNV wrote:
> >
> >
> > On 3/11/2023 5:25 AM, Thinh Nguyen wrote:
> > > On Fri, Mar 10, 2023, Krishna Kurapati wrote:
> > > > Currently host-only capable DWC3 controllers support Multiport.
> > > > Temporarily
> > > > map XHCI address space for host-only controllers and parse XHCI Extended
> > > > Capabilities registers to read number of physical usb ports
> > > > connected to the
> > > > multiport controller (presuming each port is at least HS
> > > > capable) and extract
> > > > info on how many of these ports are Super Speed capable.
> > > >
> > > > Signed-off-by: Krishna Kurapati <[email protected]>
> > > > ---
> > > > drivers/usb/dwc3/core.c | 75 +++++++++++++++++++++++++++++++++++++++++
> > > > drivers/usb/dwc3/core.h | 9 +++++
> > > > 2 files changed, 84 insertions(+)
> > > >
> > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > > > index 476b63618511..076c0f8a4441 100644
> > > > --- a/drivers/usb/dwc3/core.c
> > > > +++ b/drivers/usb/dwc3/core.c
> > > > @@ -37,6 +37,7 @@
> > > > #include "core.h"
> > > > #include "gadget.h"
> > > > #include "io.h"
> > > > +#include "../host/xhci.h"
> > >
> > > I think better to duplicate some of the logic in dwc3 driver and avoid
> > > any direct dependency with the xhci driver.
> > >
> > > > #include "debug.h"
> > > > @@ -1750,6 +1751,65 @@ static struct extcon_dev
> > > > *dwc3_get_extcon(struct dwc3 *dwc)
> > > > return edev;
> > > > }
> > > > +static int dwc3_read_port_info(struct dwc3 *dwc, struct resource *res)
> > > > +{
> > > > + void __iomem *regs;
> > > > + struct resource dwc_res;
> > > > + u32 offset;
> > > > + u32 temp;
> > > > + u8 major_revision;
> > > > + int ret = 0;
> > > > +
> > > > + /*
> > > > + * Remap xHCI address space to access XHCI ext cap regs,
> > > > + * since it is needed to get port info.
> > > > + */
> > > > + dwc_res = *res;
> > > > + dwc_res.start += 0;
> > > > + dwc_res.end = dwc->xhci_resources[0].start +
> > > > + DWC3_XHCI_REGS_END;
> > >
> > > Isn't dwc->xhci_resources[0] already setup at this point? Can we use
> > > dwc->xhci_resources[0] directly without copy the setting in dwc_res?
> > >
> > > > +
> > > > + regs = ioremap(dwc_res.start, resource_size(&dwc_res));
> > > > + if (IS_ERR(regs))
> > > > + return PTR_ERR(regs);
> > > > +
> > > > + offset = xhci_find_next_ext_cap(regs, 0,
> > > > + XHCI_EXT_CAPS_PROTOCOL);
> > > > + while (offset) {
> > > > + temp = readl(regs + offset);
> > > > + major_revision = XHCI_EXT_PORT_MAJOR(temp);
> > > > +
> > > > + temp = readl(regs + offset + 0x08);
> > > > + if (major_revision == 0x03) {
> > > > + dwc->num_ss_ports += XHCI_EXT_PORT_COUNT(temp);
> > > > + } else if (major_revision <= 0x02) {
> > > > + dwc->num_ports += XHCI_EXT_PORT_COUNT(temp);
> > > > + } else {
> > > > + dev_err(dwc->dev, "port revision seems wrong\n");
> > > > + ret = -EINVAL;
> > > > + goto unmap_reg;
> > > > + }
> > > > +
> > > > + offset = xhci_find_next_ext_cap(regs, offset,
> > > > + XHCI_EXT_CAPS_PROTOCOL);
> > > > + }
> > > > +
> > > > + temp = readl(regs + DWC3_XHCI_HCSPARAMS1);
> > > > + if (HCS_MAX_PORTS(temp) != (dwc->num_ss_ports + dwc->num_ports)) {
> > > > + dev_err(dwc->dev, "inconsistency in port info\n");
> > > > + ret = -EINVAL;
> > > > + goto unmap_reg;
> > > > + }
> > > > +
> > > > + dev_info(dwc->dev,
> > > > + "num-ports: %d ss-capable: %d\n", dwc->num_ports,
> > > > dwc->num_ss_ports);
> > >
> > > The end user doesn't need to know this info. This should be a debug
> > > message. Perhaps it can be a tracepoint if needed?
> > >
> > > > +
> > > > +unmap_reg:
> > > > + iounmap(regs);
> > > > + return ret;
> > > > +}
> > > > +
> > > > static int dwc3_probe(struct platform_device *pdev)
> > > > {
> > > > struct device *dev = &pdev->dev;
> > > > @@ -1757,6 +1817,7 @@ static int dwc3_probe(struct
> > > > platform_device *pdev)
> > > > struct dwc3 *dwc;
> > > > int ret;
> > > > + unsigned int hw_mode;
> > > > void __iomem *regs;
> > > > @@ -1880,6 +1941,20 @@ static int dwc3_probe(struct
> > > > platform_device *pdev)
> > > > goto disable_clks;
> > > > }
> > > > + /*
> > > > + * Currently DWC3 controllers that are host-only capable
> > > > + * support Multiport.
> > > > + */
> > > > + hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
> > > > + if (hw_mode == DWC3_GHWPARAMS0_MODE_HOST) {
> > > > + ret = dwc3_read_port_info(dwc, res);
> > > > + if (ret)
> > > > + goto disable_clks;
> > > > + } else {
> > > > + dwc->num_ports = 1;
> > > > + dwc->num_ss_ports = 1;
> > > > + }
> > > > +
> > > > spin_lock_init(&dwc->lock);
> > > > mutex_init(&dwc->mutex);
> > > > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> > > > index 582ebd9cf9c2..74386d6a0277 100644
> > > > --- a/drivers/usb/dwc3/core.h
> > > > +++ b/drivers/usb/dwc3/core.h
> > > > @@ -35,6 +35,9 @@
> > > > #define DWC3_MSG_MAX 500
> > > > +/* XHCI Reg constants */
> > > > +#define DWC3_XHCI_HCSPARAMS1 0x04
> > > > +
> > > > /* Global constants */
> > > > #define DWC3_PULL_UP_TIMEOUT 500 /* ms */
> > > > #define DWC3_BOUNCE_SIZE 1024 /* size of a superspeed bulk */
> > > > @@ -1023,6 +1026,10 @@ struct dwc3_scratchpad_array {
> > > > * @usb_psy: pointer to power supply interface.
> > > > * @usb2_phy: pointer to USB2 PHY
> > > > * @usb3_phy: pointer to USB3 PHY
> > > > + * @num_ports: Indicates the number of physical USB ports present on HW
> > > > + * presuming each port is at least HS capable
> > >
> > > This isn't the number of physical USB ports right? That's the number of
> > > usb2 ports the controller is configured with right?. Perhaps we can use
> > > num_usb2_ports and num_usb3_ports?
> > >
> > Hi Thinh,
> >
> > Yes, naming this might have created a little confusion.
> > num_ports is supposed to indicate number of usb2 ports in the controller.
> >
> > Incase of sa8295 (4 port controller with first two ports having ss
> > capability), num_ports would be 4 and num_ss_ports would be 2. (and not
> > 6 as what num_ports usually sounds).
> > I can rename them accordingly in the next version and update the
> > description as well.
> >
> > Regards,
> > Krishna,
> >
> Hi Thinh,
>
> One reason I didn't mention something like num_hs_ports and sticked to
> num_ports is because in core driver, wherever we need to do phy operations
> like:
>
> for (i = 0; i < num_ports; i++)
> {
> phy_set_mode(dwc->usb2_generic_phy[i], PHY_MODE_USB_HOST);
> phy_set_mode(dwc->usb3_generic_phy[i], PHY_MODE_USB_HOST);
> }
>
> The intention is as follows:
> If number of usb2 ports is 4, the loop can go from 0-3 and its fine.
> If number of usb3-ports is 2, we don't know for sure, if the first 2 ports
> are SS capable or some other ports like (3 and 4) are SS capable.
> So instead, I looped all phy operations around all usb2_generic_phy's and
> usb3_generic_phy's. If they are NULL, we just bail out inside phy operation.
>
> While doing so, looping SS Phy operations around num_usb2_ports didn't sound
> good. From code view, it would be like we are looping usb3_phy ops around
> num_usb2_ports value (logically it is still correct as each port is atleast
> HS capable). So to avoid this, I named the variable as num_ports instead of
> num_usb2_ports
>
Hi Krishna,
I think it's clearer if add this note along with using num_usb2_ports.
We just need to note this once and I think it's sufficient.
Thanks,
Thinh
Hi Krishna,
I'm unable to apply your patch series, it looks like patch 2 is malformed.
'git am' prints the following:
Applying: dt-bindings: usb: Add bindings for multiport properties on DWC3 controller
Applying: usb: dwc3: core: Access XHCI address space temporarily to read port info
error: corrupt patch at line 83
Patch failed at 0002 usb: dwc3: core: Access XHCI address space temporarily to read port info
Are you able to apply the series on your side?
Best,
Adrien
On 3/15/2023 2:02 AM, Adrien Thierry wrote:
> Hi Krishna,
>
> I'm unable to apply your patch series, it looks like patch 2 is malformed.
> 'git am' prints the following:
>
> Applying: dt-bindings: usb: Add bindings for multiport properties on DWC3 controller
> Applying: usb: dwc3: core: Access XHCI address space temporarily to read port info
> error: corrupt patch at line 83
> Patch failed at 0002 usb: dwc3: core: Access XHCI address space temporarily to read port info
>
> Are you able to apply the series on your side?
>
> Best,
>
> Adrien
>
Hi Adrien,
I rebased them last week before sending them out. Probably code got
updated causing conflicts. I will rebase them again this week and send
v6 addressing review comments.
Regards,
Krishna,