[PATCH v2 0/4] usb: dwc3: Modify role-switching QC drd usb controllers

[PATCH v2 0/4] usb: dwc3: Modify role-switching QC drd usb controllers

[Krishna Kurapati]

Currently on QC targets, the conndone/disconnect events in device mode are
generated by controller when software writes to QSCRATCH registers in qcom
glue layer rather than the vbus line being routed to dwc3 core IP for it
to recognize and generate these events. We need to set UTMI_OTG_VBUS_VALID
bit of QSCRATCH_HS_PHY_CTRL register to generate a connection done event
and clear it to generate a disconnect event during cable removal or mode
switch is done

When the disconnect is not generated upon cable removal, the "connected"
flag of dwc3 is left marked as "true" and it blocks suspend routines and
for that to happen upon cable removal, the cable disconnect notification
from usb_role_switch to DWC3 core driver needs to reach DWC3 Qualcomm glue
driver for it generate the event.

Currently, the way DWC3 core and Qualcomm glue driver is designed, there
is no mechanism through which the DWC3 core can notify the Qualcomm glue
layer of any role changes which it receives from usb_role_switch. To
register these glue callbacks at probe time, for enabling core to notify
glue layer, the legacy Qualcomm driver has no way to find out when the
child driver probe was successful since it does not check for the same
during of_platform_populate.

For flattened implementation of the glue driver, register callbacks for
core to invoke and notify glue layer of role switch notifications.

Set-Role and Run_stop notifier callbacks have been added to inform glue
of changes in role and any modifications UDC might be performing on the
controller. These callbacks allow us to modify qscratch accordingly and
generate disconnect/connect events to facilitate suspend entry and proper
enumeration.

The series only allows autosuspend to be used but still relies on user
enabling it from userspace (echo auto > a600000.usb/power/control).

Tests done:
1. Enumeration in device mode:
After creating symlinks to ffs.adb and writing to UDC node, ADB is up and
working in a stable way.

2. When none is written to UDC, device enters suspend.

3. When cable is removed, cable disconnect notification comes and when
qscratch registers are cleared properly, it is generating disconnect event

4. Device enters suspend upon removing cable (host and device mode).

5. In host mode, when autosuspend is enabled from userspace for controller,
xhci, roothubs and connected peripheral, the controller
enters runtime suspend.

6. Upon removing cable in host mode, setmode brings back usb to device
mode (which is default setting), it enters suspend as cable is still
disconnected.

7. When in host mode, if we enter runtime suspend with wakeup enabled,
clicking on buttons of headset are resuming the controller.

While at it, remove glue's extcon handling. Let the DTs being flattened use
role-switch/typec frameworks instead of extcon. DTs using "linux,extcon-usb-
gpio" can use "usb-conn-gpio" if they are having USB-B connectors. If they
are using Type-c like in case of msm8996-xiaomi-common.dtsi which gets
extcon informtation from TI based typec control chip, while flattening the
DT, role switch mechanism needs to be used instead of extcon.

This series has been tested on SM8450 QRD.
There will be a separate patch sent for flattening usb dt node.

changes in v2:
Rebased on top of usb-next.
Removed glue's extcon handling and made use of in-core handling.

Link to v1:
https://lore.kernel.org/all/20231017131851.8299-1-quic_kriskura@quicinc.com/

Krishna Kurapati (4):
  usb: dwc3: core: Introduce glue callbacks for flattened
    implementations
  usb: dwc3: qcom: Implement glue callbacks to facilitate runtime
    suspend
  usb: dwc3: qcom: Facilitate autosuspend during host mode
  usb: dwc3: qcom: Remove extcon functionality from glue

 drivers/usb/dwc3/core.c      |   1 +
 drivers/usb/dwc3/core.h      |  26 +++++
 drivers/usb/dwc3/drd.c       |   1 +
 drivers/usb/dwc3/dwc3-qcom.c | 219 +++++++++++++++++++----------------
 drivers/usb/dwc3/gadget.c    |   1 +
 5 files changed, 150 insertions(+), 98 deletions(-)

-- 
2.34.1

[PATCH v2 1/4] usb: dwc3: core: Introduce glue callbacks for flattened implementations

[Krishna Kurapati]

In certain situations like role switching, the glue layers need to be
informed of these events, so that they can take any necessary action.
But in non-flattened implementations, the glue drivers have no data on
when the core driver probe was successful post invoking of_platform_
populate. Now that the core driver supports flattened implementations
as well, introduce vendor callbacks that can be passed on from glue to
core before invoking dwc3_core_probe.

Introduce callbacks to notify glue layer of role_switch and run_stop
changes. These can be used by flattened implementation of Qualcomm
glue layer to generate connect/disconnect events in controller during
cable connect and run stop modifications by udc in device mode.

Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
---
 drivers/usb/dwc3/core.c   |  1 +
 drivers/usb/dwc3/core.h   | 26 ++++++++++++++++++++++++++
 drivers/usb/dwc3/drd.c    |  1 +
 drivers/usb/dwc3/gadget.c |  1 +
 4 files changed, 29 insertions(+)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 2bc775a747f2..c01b15e3710f 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -2351,6 +2351,7 @@ static int dwc3_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	dwc->dev = &pdev->dev;
+	dwc->glue_ops = NULL;
 
 	probe_data.dwc = dwc;
 	probe_data.res = res;
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index d5b985fa12f4..a803884be4ed 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -992,6 +992,17 @@ struct dwc3_scratchpad_array {
 	__le64	dma_adr[DWC3_MAX_HIBER_SCRATCHBUFS];
 };
 
+/*
+ * struct dwc3_glue_ops - The ops indicate the notifications that
+ *				need to be passed on to glue layer
+ * @notify_set_role: Notify glue of role switch notifications
+ * @notify_run_stop: Notify run stop enable/disable information to glue
+ */
+struct dwc3_glue_ops {
+	void	(*notify_set_role)(struct dwc3 *dwc, enum usb_role role);
+	void	(*notify_run_stop)(struct dwc3 *dwc, bool is_on);
+};
+
 /**
  * struct dwc3 - representation of our controller
  * @drd_work: workqueue used for role swapping
@@ -1168,6 +1179,7 @@ struct dwc3_scratchpad_array {
  * @wakeup_pending_funcs: Indicates whether any interface has requested for
  *			 function wakeup in bitmap format where bit position
  *			 represents interface_id.
+ * @glue_ops: Vendor callbacks for flattened device implementations.
  */
 struct dwc3 {
 	struct work_struct	drd_work;
@@ -1400,6 +1412,8 @@ struct dwc3 {
 	struct dentry		*debug_root;
 	u32			gsbuscfg0_reqinfo;
 	u32			wakeup_pending_funcs;
+
+	struct dwc3_glue_ops	*glue_ops;
 };
 
 #define INCRX_BURST_MODE 0
@@ -1614,6 +1628,18 @@ void dwc3_event_buffers_cleanup(struct dwc3 *dwc);
 int dwc3_core_soft_reset(struct dwc3 *dwc);
 void dwc3_enable_susphy(struct dwc3 *dwc, bool enable);
 
+static inline void dwc3_notify_set_role(struct dwc3 *dwc, enum usb_role role)
+{
+	if (dwc->glue_ops && dwc->glue_ops->notify_set_role)
+		dwc->glue_ops->notify_set_role(dwc, role);
+}
+
+static inline void dwc3_notify_run_stop(struct dwc3 *dwc, bool is_on)
+{
+	if (dwc->glue_ops && dwc->glue_ops->notify_run_stop)
+		dwc->glue_ops->notify_run_stop(dwc, is_on);
+}
+
 #if IS_ENABLED(CONFIG_USB_DWC3_HOST) || IS_ENABLED(CONFIG_USB_DWC3_DUAL_ROLE)
 int dwc3_host_init(struct dwc3 *dwc);
 void dwc3_host_exit(struct dwc3 *dwc);
diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
index 7977860932b1..408551768a95 100644
--- a/drivers/usb/dwc3/drd.c
+++ b/drivers/usb/dwc3/drd.c
@@ -464,6 +464,7 @@ static int dwc3_usb_role_switch_set(struct usb_role_switch *sw,
 		break;
 	}
 
+	dwc3_notify_set_role(dwc, role);
 	dwc3_set_mode(dwc, mode);
 	return 0;
 }
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 321361288935..73bed11bccaf 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2641,6 +2641,7 @@ static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on)
 	if (saved_config)
 		dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
 
+	dwc3_notify_run_stop(dwc, is_on);
 	reg = dwc3_readl(dwc->regs, DWC3_DCTL);
 	if (is_on) {
 		if (DWC3_VER_IS_WITHIN(DWC3, ANY, 187A)) {
-- 
2.34.1

[PATCH v2 2/4] usb: dwc3: qcom: Implement glue callbacks to facilitate runtime suspend

[Krishna Kurapati]

On Qualcomm DWC3 dual-role controllers, the conndone/disconnect events in
device mode are generated by controller when software writes to QSCRATCH
registers in Qualcomm Glue layer rather than the vbus line being routed to
dwc3 core IP for it to recognize and generate these events.

UTMI_OTG_VBUS_VALID  bit of QSCRATCH_HS_PHY_CTRL register needs to be set
to generate a connection done event and to be cleared for the controller to
generate a disconnect event during cable removal. When the disconnect is
not generated upon cable removal, the "connected" flag of dwc3 is left
marked as "true" and it blocks suspend routines and for that to happen upon
cable removal, the cable disconnect notification coming in via set_role
call need to be provided to the Qualcomm glue layer as well.

Currently, the way DWC3 core and Qualcomm legacy glue driver are designed,
there is no mechanism through which the DWC3 core can notify the Qualcomm
glue layer of any role changes which it receives via role switch. To
register these glue callbacks at probe time, for enabling core to notify
glue layer, the legacy Qualcomm driver has no way to find out when the
child driver probe was successful since it does not check for the same
during of_platform_populate.

Hence implement the following glue callbacks for flattened Qualcomm glue
driver:

1. set_role: To pass role switching information from drd layer to glue.
This information is needed to identify NONE/DEVICE mode switch and modify
QSCRATCH to generate connect-done event on device mode entry and disconnect
event on cable removal in device mode.

2. run_stop: When booting up in device mode, if autouspend is enabled and
userspace doesn't write UDC on boot, controller enters autosuspend. After
this, if the userspace writes to UDC in the future, run_stop notifier is
required to enable UTMI_OTG_VBUS_VALID of QSCRATCH so that connect done
event is generated after run_stop(1) is done to finish enumeration.

Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
---
 drivers/usb/dwc3/dwc3-qcom.c | 82 ++++++++++++++++++++++++++++++++----
 1 file changed, 73 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index ca7e1c02773a..d40b52e2ba01 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -89,6 +89,12 @@ struct dwc3_qcom {
 	bool			pm_suspended;
 	struct icc_path		*icc_path_ddr;
 	struct icc_path		*icc_path_apps;
+
+	/*
+	 * Current role changes via usb_role_switch_set_role callback protected
+	 * internally by mutex lock.
+	 */
+	enum usb_role		current_role;
 };
 
 #define to_dwc3_qcom(d) container_of((d), struct dwc3_qcom, dwc)
@@ -118,9 +124,9 @@ static inline void dwc3_qcom_clrbits(void __iomem *base, u32 offset, u32 val)
 }
 
 /*
- * TODO: Make the in-core role switching code invoke dwc3_qcom_vbus_override_enable(),
- * validate that the in-core extcon support is functional, and drop extcon
- * handling from the glue
+ * TODO: Validate that the in-core extcon support is functional, and drop
+ * extcon handling from the glue. Make in-core extcon invoke
+ * dwc3_qcom_vbus_override_enable()
  */
 static void dwc3_qcom_vbus_override_enable(struct dwc3_qcom *qcom, bool enable)
 {
@@ -641,6 +647,53 @@ static int dwc3_qcom_setup_irq(struct dwc3_qcom *qcom, struct platform_device *p
 	return 0;
 }
 
+static void dwc3_qcom_set_role_notifier(struct dwc3 *dwc, enum usb_role next_role)
+{
+	struct dwc3_qcom *qcom = to_dwc3_qcom(dwc);
+
+	if (qcom->current_role == next_role)
+		return;
+
+	if (pm_runtime_resume_and_get(qcom->dev) < 0) {
+		dev_dbg(qcom->dev, "Failed to resume device\n");
+		return;
+	}
+
+	if (qcom->current_role == USB_ROLE_DEVICE &&
+	    next_role != USB_ROLE_DEVICE)
+		dwc3_qcom_vbus_override_enable(qcom, false);
+	else if ((qcom->current_role != USB_ROLE_DEVICE) &&
+		 (next_role == USB_ROLE_DEVICE))
+		dwc3_qcom_vbus_override_enable(qcom, true);
+
+	pm_runtime_mark_last_busy(qcom->dev);
+	pm_runtime_put_sync(qcom->dev);
+
+	qcom->current_role = next_role;
+}
+
+static void dwc3_qcom_run_stop_notifier(struct dwc3 *dwc, bool is_on)
+{
+	struct dwc3_qcom *qcom = to_dwc3_qcom(dwc);
+
+	/*
+	 * When autosuspend is enabled and controller goes to suspend
+	 * after removing UDC from userspace, the next UDC write needs
+	 * setting of QSCRATCH VBUS_VALID to "1" to generate a connect
+	 * done event.
+	 */
+	if (!is_on)
+		return;
+
+	dwc3_qcom_vbus_override_enable(qcom, is_on);
+	pm_runtime_mark_last_busy(qcom->dev);
+}
+
+struct dwc3_glue_ops dwc3_qcom_glue_ops = {
+	.notify_set_role	= dwc3_qcom_set_role_notifier,
+	.notify_run_stop	= dwc3_qcom_run_stop_notifier,
+};
+
 static int dwc3_qcom_probe(struct platform_device *pdev)
 {
 	struct dwc3_probe_data	probe_data = {};
@@ -717,6 +770,23 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
 	if (ignore_pipe_clk)
 		dwc3_qcom_select_utmi_clk(qcom);
 
+	qcom->mode = usb_get_dr_mode(dev);
+
+	if (qcom->mode == USB_DR_MODE_HOST) {
+		qcom->current_role = USB_ROLE_HOST;
+	} else if (qcom->mode == USB_DR_MODE_PERIPHERAL) {
+		qcom->current_role = USB_ROLE_DEVICE;
+		dwc3_qcom_vbus_override_enable(qcom, true);
+	} else if (qcom->mode == USB_DR_MODE_OTG) {
+		if ((device_property_read_bool(dev, "usb-role-switch")) &&
+		    (usb_get_role_switch_default_mode(dev) == USB_DR_MODE_HOST))
+			qcom->current_role = USB_ROLE_HOST;
+		else
+			qcom->current_role = USB_ROLE_DEVICE;
+	}
+
+	qcom->dwc.glue_ops = &dwc3_qcom_glue_ops;
+
 	qcom->dwc.dev = dev;
 	probe_data.dwc = &qcom->dwc;
 	probe_data.res = &res;
@@ -731,12 +801,6 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
 	if (ret)
 		goto remove_core;
 
-	qcom->mode = usb_get_dr_mode(dev);
-
-	/* enable vbus override for device mode */
-	if (qcom->mode != USB_DR_MODE_HOST)
-		dwc3_qcom_vbus_override_enable(qcom, true);
-
 	/* register extcon to override sw_vbus on Vbus change later */
 	ret = dwc3_qcom_register_extcon(qcom);
 	if (ret)
-- 
2.34.1

[PATCH v2 3/4] usb: dwc3: qcom: Facilitate autosuspend during host mode

[Krishna Kurapati]

When in host mode, it is intended that the controller goes to suspend
state to save power and wait for interrupts from connected peripheral
to wake it up. This is particularly used in cases where a HID or Audio
device is connected. In such scenarios, the usb controller can enter
auto suspend and resume action after getting interrupts from the
connected device.

Allow autosuspend for and xhci device and allow userspace to decide
whether to enable this functionality.

a) Register to usb-core notifications in set_role vendor callback to
identify when root hubs are being created. Configure them to
use_autosuspend.

b) Identify usb core notifications where the HCD is being added and
enable autosuspend for that particular xhci device.

Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
---
 drivers/usb/dwc3/dwc3-qcom.c | 62 ++++++++++++++++++++++++++++++++----
 1 file changed, 56 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index d40b52e2ba01..17bbd5a06c08 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -95,6 +95,8 @@ struct dwc3_qcom {
 	 * internally by mutex lock.
 	 */
 	enum usb_role		current_role;
+
+	struct notifier_block	xhci_nb;
 };
 
 #define to_dwc3_qcom(d) container_of((d), struct dwc3_qcom, dwc)
@@ -647,6 +649,39 @@ static int dwc3_qcom_setup_irq(struct dwc3_qcom *qcom, struct platform_device *p
 	return 0;
 }
 
+static int dwc3_xhci_event_notifier(struct notifier_block *nb,
+				    unsigned long event, void *ptr)
+{
+	struct dwc3_qcom  *qcom	= container_of(nb, struct dwc3_qcom, xhci_nb);
+	struct dwc3	  *dwc	= &qcom->dwc;
+	struct usb_bus	  *ubus	= ptr;
+	struct usb_hcd	  *hcd;
+
+	if (!dwc->xhci)
+		goto done;
+
+	hcd = platform_get_drvdata(dwc->xhci);
+	if (!hcd)
+		goto done;
+
+	if (event != USB_BUS_ADD)
+		goto done;
+
+	if (strcmp(dev_name(ubus->sysdev), dev_name(dwc->sysdev)) != 0)
+		goto done;
+
+	if (event == USB_BUS_ADD) {
+		/*
+		 * Identify instant of creation of primary hcd and
+		 * mark xhci as autosuspend capable at this point.
+		 */
+		pm_runtime_use_autosuspend(&dwc->xhci->dev);
+	}
+
+done:
+	return NOTIFY_DONE;
+}
+
 static void dwc3_qcom_set_role_notifier(struct dwc3 *dwc, enum usb_role next_role)
 {
 	struct dwc3_qcom *qcom = to_dwc3_qcom(dwc);
@@ -659,12 +694,22 @@ static void dwc3_qcom_set_role_notifier(struct dwc3 *dwc, enum usb_role next_rol
 		return;
 	}
 
-	if (qcom->current_role == USB_ROLE_DEVICE &&
-	    next_role != USB_ROLE_DEVICE)
+	if (qcom->current_role == USB_ROLE_NONE) {
+		if (next_role == USB_ROLE_DEVICE) {
+			dwc3_qcom_vbus_override_enable(qcom, true);
+		} else if (next_role == USB_ROLE_HOST) {
+			qcom->xhci_nb.notifier_call = dwc3_xhci_event_notifier;
+			usb_register_notify(&qcom->xhci_nb);
+		}
+	} else if (qcom->current_role == USB_ROLE_DEVICE &&
+		   next_role != USB_ROLE_DEVICE) {
 		dwc3_qcom_vbus_override_enable(qcom, false);
-	else if ((qcom->current_role != USB_ROLE_DEVICE) &&
-		 (next_role == USB_ROLE_DEVICE))
-		dwc3_qcom_vbus_override_enable(qcom, true);
+	} else if (qcom->current_role == USB_ROLE_HOST) {
+		if (next_role == USB_ROLE_NONE)
+			usb_unregister_notify(&qcom->xhci_nb);
+		else if (next_role == USB_ROLE_DEVICE)
+			dwc3_qcom_vbus_override_enable(qcom, true);
+	}
 
 	pm_runtime_mark_last_busy(qcom->dev);
 	pm_runtime_put_sync(qcom->dev);
@@ -774,6 +819,8 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
 
 	if (qcom->mode == USB_DR_MODE_HOST) {
 		qcom->current_role = USB_ROLE_HOST;
+		qcom->xhci_nb.notifier_call = dwc3_xhci_event_notifier;
+		usb_register_notify(&qcom->xhci_nb);
 	} else if (qcom->mode == USB_DR_MODE_PERIPHERAL) {
 		qcom->current_role = USB_ROLE_DEVICE;
 		dwc3_qcom_vbus_override_enable(qcom, true);
@@ -794,7 +841,7 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
 	ret = dwc3_core_probe(&probe_data);
 	if (ret)  {
 		ret = dev_err_probe(dev, ret, "failed to register DWC3 Core\n");
-		goto clk_disable;
+		goto unregister_notify;
 	}
 
 	ret = dwc3_qcom_interconnect_init(qcom);
@@ -817,6 +864,9 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
 	dwc3_qcom_interconnect_exit(qcom);
 remove_core:
 	dwc3_core_remove(&qcom->dwc);
+unregister_notify:
+	if (qcom->mode == USB_DR_MODE_HOST)
+		usb_unregister_notify(&qcom->xhci_nb);
 clk_disable:
 	clk_bulk_disable_unprepare(qcom->num_clocks, qcom->clks);
 
-- 
2.34.1

[PATCH v2 4/4] usb: dwc3: qcom: Remove extcon functionality from glue

[Krishna Kurapati]

Deprecate usage of extcon functionality from the glue driver. Now
that the glue driver is a flattened implementation, all existing
DTs would eventually move to new bindings. While doing so let them
make use of role-switch/ typec frameworks to provide role data
rather than using extcon.

On upstream, summary of targets/platforms using extcon is as follows:

1. MSM8916 and MSM8939 use Chipidea controller, hence the changes have no
effect on them.

2. Of the other extcon users, most of them use "linux,extcon-usb-gpio"
driver which relies on id/vbus gpios to inform role changes. This can be
transitioned to role switch based driver (usb-conn-gpio) while flattening
those platforms to move away from extcon and rely on role
switching.

3. The one target that uses dwc3 controller and extcon and is not based
on reading gpios is "arch/arm64/boot/dts/qcom/msm8996-xiaomi-common.dtsi".
This platform uses TI chip to provide extcon. If usb on this platform is
being flattneed, then effort should be put in to define a usb-c-connector
device in DT and make use of role switch functionality in TUSB320L driver.

Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
---
 drivers/usb/dwc3/dwc3-qcom.c | 91 ------------------------------------
 1 file changed, 91 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index 17bbd5a06c08..1a73a7797d41 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -79,11 +79,6 @@ struct dwc3_qcom {
 	struct dwc3_qcom_port	ports[DWC3_QCOM_MAX_PORTS];
 	u8			num_ports;
 
-	struct extcon_dev	*edev;
-	struct extcon_dev	*host_edev;
-	struct notifier_block	vbus_nb;
-	struct notifier_block	host_nb;
-
 	enum usb_dr_mode	mode;
 	bool			is_suspended;
 	bool			pm_suspended;
@@ -125,11 +120,6 @@ static inline void dwc3_qcom_clrbits(void __iomem *base, u32 offset, u32 val)
 	readl(base + offset);
 }
 
-/*
- * TODO: Validate that the in-core extcon support is functional, and drop
- * extcon handling from the glue. Make in-core extcon invoke
- * dwc3_qcom_vbus_override_enable()
- */
 static void dwc3_qcom_vbus_override_enable(struct dwc3_qcom *qcom, bool enable)
 {
 	if (enable) {
@@ -145,80 +135,6 @@ static void dwc3_qcom_vbus_override_enable(struct dwc3_qcom *qcom, bool enable)
 	}
 }
 
-static int dwc3_qcom_vbus_notifier(struct notifier_block *nb,
-				   unsigned long event, void *ptr)
-{
-	struct dwc3_qcom *qcom = container_of(nb, struct dwc3_qcom, vbus_nb);
-
-	/* enable vbus override for device mode */
-	dwc3_qcom_vbus_override_enable(qcom, event);
-	qcom->mode = event ? USB_DR_MODE_PERIPHERAL : USB_DR_MODE_HOST;
-
-	return NOTIFY_DONE;
-}
-
-static int dwc3_qcom_host_notifier(struct notifier_block *nb,
-				   unsigned long event, void *ptr)
-{
-	struct dwc3_qcom *qcom = container_of(nb, struct dwc3_qcom, host_nb);
-
-	/* disable vbus override in host mode */
-	dwc3_qcom_vbus_override_enable(qcom, !event);
-	qcom->mode = event ? USB_DR_MODE_HOST : USB_DR_MODE_PERIPHERAL;
-
-	return NOTIFY_DONE;
-}
-
-static int dwc3_qcom_register_extcon(struct dwc3_qcom *qcom)
-{
-	struct device		*dev = qcom->dev;
-	struct extcon_dev	*host_edev;
-	int			ret;
-
-	if (!of_property_present(dev->of_node, "extcon"))
-		return 0;
-
-	qcom->edev = extcon_get_edev_by_phandle(dev, 0);
-	if (IS_ERR(qcom->edev))
-		return dev_err_probe(dev, PTR_ERR(qcom->edev),
-				     "Failed to get extcon\n");
-
-	qcom->vbus_nb.notifier_call = dwc3_qcom_vbus_notifier;
-
-	qcom->host_edev = extcon_get_edev_by_phandle(dev, 1);
-	if (IS_ERR(qcom->host_edev))
-		qcom->host_edev = NULL;
-
-	ret = devm_extcon_register_notifier(dev, qcom->edev, EXTCON_USB,
-					    &qcom->vbus_nb);
-	if (ret < 0) {
-		dev_err(dev, "VBUS notifier register failed\n");
-		return ret;
-	}
-
-	if (qcom->host_edev)
-		host_edev = qcom->host_edev;
-	else
-		host_edev = qcom->edev;
-
-	qcom->host_nb.notifier_call = dwc3_qcom_host_notifier;
-	ret = devm_extcon_register_notifier(dev, host_edev, EXTCON_USB_HOST,
-					    &qcom->host_nb);
-	if (ret < 0) {
-		dev_err(dev, "Host notifier register failed\n");
-		return ret;
-	}
-
-	/* Update initial VBUS override based on extcon state */
-	if (extcon_get_state(qcom->edev, EXTCON_USB) ||
-	    !extcon_get_state(host_edev, EXTCON_USB_HOST))
-		dwc3_qcom_vbus_notifier(&qcom->vbus_nb, true, qcom->edev);
-	else
-		dwc3_qcom_vbus_notifier(&qcom->vbus_nb, false, qcom->edev);
-
-	return 0;
-}
-
 static int dwc3_qcom_interconnect_enable(struct dwc3_qcom *qcom)
 {
 	int ret;
@@ -848,11 +764,6 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
 	if (ret)
 		goto remove_core;
 
-	/* register extcon to override sw_vbus on Vbus change later */
-	ret = dwc3_qcom_register_extcon(qcom);
-	if (ret)
-		goto interconnect_exit;
-
 	wakeup_source = of_property_read_bool(dev->of_node, "wakeup-source");
 	device_init_wakeup(&pdev->dev, wakeup_source);
 
@@ -860,8 +771,6 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
 
 	return 0;
 
-interconnect_exit:
-	dwc3_qcom_interconnect_exit(qcom);
 remove_core:
 	dwc3_core_remove(&qcom->dwc);
 unregister_notify:
-- 
2.34.1

Re: [PATCH v2 0/4] usb: dwc3: Modify role-switching QC drd usb controllers

[Krishna Kurapati]

On 6/10/2025 2:43 PM, Krishna Kurapati wrote:

[...]

> 
> changes in v2:
> Rebased on top of usb-next.

One correction. The series is based on top of (acked commit):

https://lore.kernel.org/all/20250604060019.2174029-1-krishna.kurapati@oss.qualcomm.com/

Sorry for the above mistake.

Regards,
Krishna.

> Removed glue's extcon handling and made use of in-core handling.
> 
> Link to v1:
> https://lore.kernel.org/all/20231017131851.8299-1-quic_kriskura@quicinc.com/
> 
> Krishna Kurapati (4):
>    usb: dwc3: core: Introduce glue callbacks for flattened
>      implementations
>    usb: dwc3: qcom: Implement glue callbacks to facilitate runtime
>      suspend
>    usb: dwc3: qcom: Facilitate autosuspend during host mode
>    usb: dwc3: qcom: Remove extcon functionality from glue
> 
>   drivers/usb/dwc3/core.c      |   1 +
>   drivers/usb/dwc3/core.h      |  26 +++++
>   drivers/usb/dwc3/drd.c       |   1 +
>   drivers/usb/dwc3/dwc3-qcom.c | 219 +++++++++++++++++++----------------
>   drivers/usb/dwc3/gadget.c    |   1 +
>   5 files changed, 150 insertions(+), 98 deletions(-)
> 

Re: [PATCH v2 2/4] usb: dwc3: qcom: Implement glue callbacks to facilitate runtime suspend

[Dmitry Baryshkov]

On Tue, Jun 10, 2025 at 02:43:55PM +0530, Krishna Kurapati wrote:
> On Qualcomm DWC3 dual-role controllers, the conndone/disconnect events in
> device mode are generated by controller when software writes to QSCRATCH
> registers in Qualcomm Glue layer rather than the vbus line being routed to
> dwc3 core IP for it to recognize and generate these events.
> 
> UTMI_OTG_VBUS_VALID  bit of QSCRATCH_HS_PHY_CTRL register needs to be set
> to generate a connection done event and to be cleared for the controller to
> generate a disconnect event during cable removal. When the disconnect is
> not generated upon cable removal, the "connected" flag of dwc3 is left
> marked as "true" and it blocks suspend routines and for that to happen upon
> cable removal, the cable disconnect notification coming in via set_role
> call need to be provided to the Qualcomm glue layer as well.
> 
> Currently, the way DWC3 core and Qualcomm legacy glue driver are designed,
> there is no mechanism through which the DWC3 core can notify the Qualcomm
> glue layer of any role changes which it receives via role switch. To
> register these glue callbacks at probe time, for enabling core to notify
> glue layer, the legacy Qualcomm driver has no way to find out when the
> child driver probe was successful since it does not check for the same
> during of_platform_populate.
> 
> Hence implement the following glue callbacks for flattened Qualcomm glue
> driver:
> 
> 1. set_role: To pass role switching information from drd layer to glue.
> This information is needed to identify NONE/DEVICE mode switch and modify
> QSCRATCH to generate connect-done event on device mode entry and disconnect
> event on cable removal in device mode.
> 
> 2. run_stop: When booting up in device mode, if autouspend is enabled and
> userspace doesn't write UDC on boot, controller enters autosuspend. After
> this, if the userspace writes to UDC in the future, run_stop notifier is
> required to enable UTMI_OTG_VBUS_VALID of QSCRATCH so that connect done
> event is generated after run_stop(1) is done to finish enumeration.
> 
> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> ---
>  drivers/usb/dwc3/dwc3-qcom.c | 82 ++++++++++++++++++++++++++++++++----
>  1 file changed, 73 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index ca7e1c02773a..d40b52e2ba01 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -89,6 +89,12 @@ struct dwc3_qcom {
>  	bool			pm_suspended;
>  	struct icc_path		*icc_path_ddr;
>  	struct icc_path		*icc_path_apps;
> +
> +	/*
> +	 * Current role changes via usb_role_switch_set_role callback protected
> +	 * internally by mutex lock.
> +	 */
> +	enum usb_role		current_role;
>  };
>  
>  #define to_dwc3_qcom(d) container_of((d), struct dwc3_qcom, dwc)
> @@ -118,9 +124,9 @@ static inline void dwc3_qcom_clrbits(void __iomem *base, u32 offset, u32 val)
>  }
>  
>  /*
> - * TODO: Make the in-core role switching code invoke dwc3_qcom_vbus_override_enable(),
> - * validate that the in-core extcon support is functional, and drop extcon
> - * handling from the glue
> + * TODO: Validate that the in-core extcon support is functional, and drop
> + * extcon handling from the glue. Make in-core extcon invoke
> + * dwc3_qcom_vbus_override_enable()
>   */
>  static void dwc3_qcom_vbus_override_enable(struct dwc3_qcom *qcom, bool enable)
>  {
> @@ -641,6 +647,53 @@ static int dwc3_qcom_setup_irq(struct dwc3_qcom *qcom, struct platform_device *p
>  	return 0;
>  }
>  
> +static void dwc3_qcom_set_role_notifier(struct dwc3 *dwc, enum usb_role next_role)
> +{
> +	struct dwc3_qcom *qcom = to_dwc3_qcom(dwc);
> +
> +	if (qcom->current_role == next_role)
> +		return;
> +
> +	if (pm_runtime_resume_and_get(qcom->dev) < 0) {
> +		dev_dbg(qcom->dev, "Failed to resume device\n");
> +		return;
> +	}
> +
> +	if (qcom->current_role == USB_ROLE_DEVICE &&
> +	    next_role != USB_ROLE_DEVICE)
> +		dwc3_qcom_vbus_override_enable(qcom, false);
> +	else if ((qcom->current_role != USB_ROLE_DEVICE) &&
> +		 (next_role == USB_ROLE_DEVICE))
> +		dwc3_qcom_vbus_override_enable(qcom, true);
> +
> +	pm_runtime_mark_last_busy(qcom->dev);
> +	pm_runtime_put_sync(qcom->dev);
> +
> +	qcom->current_role = next_role;

How is it protected by the mutex? Which mutex?

> +}
> +
> +static void dwc3_qcom_run_stop_notifier(struct dwc3 *dwc, bool is_on)
> +{
> +	struct dwc3_qcom *qcom = to_dwc3_qcom(dwc);
> +
> +	/*
> +	 * When autosuspend is enabled and controller goes to suspend
> +	 * after removing UDC from userspace, the next UDC write needs
> +	 * setting of QSCRATCH VBUS_VALID to "1" to generate a connect
> +	 * done event.
> +	 */
> +	if (!is_on)
> +		return;
> +
> +	dwc3_qcom_vbus_override_enable(qcom, is_on);
> +	pm_runtime_mark_last_busy(qcom->dev);
> +}
> +
> +struct dwc3_glue_ops dwc3_qcom_glue_ops = {
> +	.notify_set_role	= dwc3_qcom_set_role_notifier,
> +	.notify_run_stop	= dwc3_qcom_run_stop_notifier,
> +};
> +
>  static int dwc3_qcom_probe(struct platform_device *pdev)
>  {
>  	struct dwc3_probe_data	probe_data = {};
> @@ -717,6 +770,23 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>  	if (ignore_pipe_clk)
>  		dwc3_qcom_select_utmi_clk(qcom);
>  
> +	qcom->mode = usb_get_dr_mode(dev);
> +
> +	if (qcom->mode == USB_DR_MODE_HOST) {
> +		qcom->current_role = USB_ROLE_HOST;
> +	} else if (qcom->mode == USB_DR_MODE_PERIPHERAL) {
> +		qcom->current_role = USB_ROLE_DEVICE;
> +		dwc3_qcom_vbus_override_enable(qcom, true);
> +	} else if (qcom->mode == USB_DR_MODE_OTG) {

Just 'else', otherwise you are not going to implement the default case
correctly (per usb-drd.yaml we should default to OTG).

> +		if ((device_property_read_bool(dev, "usb-role-switch")) &&
> +		    (usb_get_role_switch_default_mode(dev) == USB_DR_MODE_HOST))
> +			qcom->current_role = USB_ROLE_HOST;
> +		else
> +			qcom->current_role = USB_ROLE_DEVICE;
> +	}
> +
> +	qcom->dwc.glue_ops = &dwc3_qcom_glue_ops;
> +
>  	qcom->dwc.dev = dev;
>  	probe_data.dwc = &qcom->dwc;
>  	probe_data.res = &res;
> @@ -731,12 +801,6 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto remove_core;
>  
> -	qcom->mode = usb_get_dr_mode(dev);
> -
> -	/* enable vbus override for device mode */
> -	if (qcom->mode != USB_DR_MODE_HOST)
> -		dwc3_qcom_vbus_override_enable(qcom, true);
> -
>  	/* register extcon to override sw_vbus on Vbus change later */
>  	ret = dwc3_qcom_register_extcon(qcom);
>  	if (ret)
> -- 
> 2.34.1
> 

Re: [PATCH v2 3/4] usb: dwc3: qcom: Facilitate autosuspend during host mode

[Dmitry Baryshkov]

On Tue, Jun 10, 2025 at 02:43:56PM +0530, Krishna Kurapati wrote:
> When in host mode, it is intended that the controller goes to suspend
> state to save power and wait for interrupts from connected peripheral
> to wake it up. This is particularly used in cases where a HID or Audio
> device is connected. In such scenarios, the usb controller can enter
> auto suspend and resume action after getting interrupts from the
> connected device.
> 
> Allow autosuspend for and xhci device and allow userspace to decide
> whether to enable this functionality.
> 
> a) Register to usb-core notifications in set_role vendor callback to
> identify when root hubs are being created. Configure them to
> use_autosuspend.
> 
> b) Identify usb core notifications where the HCD is being added and
> enable autosuspend for that particular xhci device.
> 
> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> ---
>  drivers/usb/dwc3/dwc3-qcom.c | 62 ++++++++++++++++++++++++++++++++----
>  1 file changed, 56 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index d40b52e2ba01..17bbd5a06c08 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -95,6 +95,8 @@ struct dwc3_qcom {
>  	 * internally by mutex lock.
>  	 */
>  	enum usb_role		current_role;
> +
> +	struct notifier_block	xhci_nb;
>  };
>  
>  #define to_dwc3_qcom(d) container_of((d), struct dwc3_qcom, dwc)
> @@ -647,6 +649,39 @@ static int dwc3_qcom_setup_irq(struct dwc3_qcom *qcom, struct platform_device *p
>  	return 0;
>  }
>  
> +static int dwc3_xhci_event_notifier(struct notifier_block *nb,
> +				    unsigned long event, void *ptr)
> +{
> +	struct dwc3_qcom  *qcom	= container_of(nb, struct dwc3_qcom, xhci_nb);
> +	struct dwc3	  *dwc	= &qcom->dwc;
> +	struct usb_bus	  *ubus	= ptr;
> +	struct usb_hcd	  *hcd;
> +
> +	if (!dwc->xhci)
> +		goto done;
> +
> +	hcd = platform_get_drvdata(dwc->xhci);
> +	if (!hcd)
> +		goto done;
> +
> +	if (event != USB_BUS_ADD)
> +		goto done;
> +
> +	if (strcmp(dev_name(ubus->sysdev), dev_name(dwc->sysdev)) != 0)
> +		goto done;
> +
> +	if (event == USB_BUS_ADD) {
> +		/*
> +		 * Identify instant of creation of primary hcd and
> +		 * mark xhci as autosuspend capable at this point.
> +		 */
> +		pm_runtime_use_autosuspend(&dwc->xhci->dev);

This feels like an overkill, using notifiers to set autosuspend flag.
Please extend platform data and/or other static code in order to set the
flag on the created xHCI devices.

> +	}
> +
> +done:
> +	return NOTIFY_DONE;
> +}
> +
>  static void dwc3_qcom_set_role_notifier(struct dwc3 *dwc, enum usb_role next_role)
>  {
>  	struct dwc3_qcom *qcom = to_dwc3_qcom(dwc);
> @@ -659,12 +694,22 @@ static void dwc3_qcom_set_role_notifier(struct dwc3 *dwc, enum usb_role next_rol
>  		return;
>  	}
>  
> -	if (qcom->current_role == USB_ROLE_DEVICE &&
> -	    next_role != USB_ROLE_DEVICE)
> +	if (qcom->current_role == USB_ROLE_NONE) {
> +		if (next_role == USB_ROLE_DEVICE) {
> +			dwc3_qcom_vbus_override_enable(qcom, true);
> +		} else if (next_role == USB_ROLE_HOST) {
> +			qcom->xhci_nb.notifier_call = dwc3_xhci_event_notifier;
> +			usb_register_notify(&qcom->xhci_nb);
> +		}
> +	} else if (qcom->current_role == USB_ROLE_DEVICE &&
> +		   next_role != USB_ROLE_DEVICE) {
>  		dwc3_qcom_vbus_override_enable(qcom, false);
> -	else if ((qcom->current_role != USB_ROLE_DEVICE) &&
> -		 (next_role == USB_ROLE_DEVICE))
> -		dwc3_qcom_vbus_override_enable(qcom, true);
> +	} else if (qcom->current_role == USB_ROLE_HOST) {
> +		if (next_role == USB_ROLE_NONE)
> +			usb_unregister_notify(&qcom->xhci_nb);
> +		else if (next_role == USB_ROLE_DEVICE)
> +			dwc3_qcom_vbus_override_enable(qcom, true);
> +	}
>  
>  	pm_runtime_mark_last_busy(qcom->dev);
>  	pm_runtime_put_sync(qcom->dev);
> @@ -774,6 +819,8 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>  
>  	if (qcom->mode == USB_DR_MODE_HOST) {
>  		qcom->current_role = USB_ROLE_HOST;
> +		qcom->xhci_nb.notifier_call = dwc3_xhci_event_notifier;
> +		usb_register_notify(&qcom->xhci_nb);
>  	} else if (qcom->mode == USB_DR_MODE_PERIPHERAL) {
>  		qcom->current_role = USB_ROLE_DEVICE;
>  		dwc3_qcom_vbus_override_enable(qcom, true);
> @@ -794,7 +841,7 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>  	ret = dwc3_core_probe(&probe_data);
>  	if (ret)  {
>  		ret = dev_err_probe(dev, ret, "failed to register DWC3 Core\n");
> -		goto clk_disable;
> +		goto unregister_notify;
>  	}
>  
>  	ret = dwc3_qcom_interconnect_init(qcom);
> @@ -817,6 +864,9 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>  	dwc3_qcom_interconnect_exit(qcom);
>  remove_core:
>  	dwc3_core_remove(&qcom->dwc);
> +unregister_notify:
> +	if (qcom->mode == USB_DR_MODE_HOST)
> +		usb_unregister_notify(&qcom->xhci_nb);
>  clk_disable:
>  	clk_bulk_disable_unprepare(qcom->num_clocks, qcom->clks);
>  
> -- 
> 2.34.1
> 

Re: [PATCH v2 4/4] usb: dwc3: qcom: Remove extcon functionality from glue

[Dmitry Baryshkov]

On Tue, Jun 10, 2025 at 02:43:57PM +0530, Krishna Kurapati wrote:
> Deprecate usage of extcon functionality from the glue driver. Now
> that the glue driver is a flattened implementation, all existing
> DTs would eventually move to new bindings. While doing so let them
> make use of role-switch/ typec frameworks to provide role data
> rather than using extcon.
> 
> On upstream, summary of targets/platforms using extcon is as follows:
> 
> 1. MSM8916 and MSM8939 use Chipidea controller, hence the changes have no
> effect on them.
> 
> 2. Of the other extcon users, most of them use "linux,extcon-usb-gpio"
> driver which relies on id/vbus gpios to inform role changes. This can be
> transitioned to role switch based driver (usb-conn-gpio) while flattening
> those platforms to move away from extcon and rely on role
> switching.
> 
> 3. The one target that uses dwc3 controller and extcon and is not based
> on reading gpios is "arch/arm64/boot/dts/qcom/msm8996-xiaomi-common.dtsi".
> This platform uses TI chip to provide extcon. If usb on this platform is
> being flattneed, then effort should be put in to define a usb-c-connector
> device in DT and make use of role switch functionality in TUSB320L driver.
> 
> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> ---
>  drivers/usb/dwc3/dwc3-qcom.c | 91 ------------------------------------
>  1 file changed, 91 deletions(-)

I'd say, this should be the patch 1 in the series.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

Re: [PATCH v2 2/4] usb: dwc3: qcom: Implement glue callbacks to facilitate runtime suspend

[Krishna Kurapati]

On 6/10/2025 4:28 PM, Dmitry Baryshkov wrote:
> On Tue, Jun 10, 2025 at 02:43:55PM +0530, Krishna Kurapati wrote:
>> On Qualcomm DWC3 dual-role controllers, the conndone/disconnect events in
>> device mode are generated by controller when software writes to QSCRATCH
>> registers in Qualcomm Glue layer rather than the vbus line being routed to
>> dwc3 core IP for it to recognize and generate these events.
>>
>> UTMI_OTG_VBUS_VALID  bit of QSCRATCH_HS_PHY_CTRL register needs to be set
>> to generate a connection done event and to be cleared for the controller to
>> generate a disconnect event during cable removal. When the disconnect is
>> not generated upon cable removal, the "connected" flag of dwc3 is left
>> marked as "true" and it blocks suspend routines and for that to happen upon
>> cable removal, the cable disconnect notification coming in via set_role
>> call need to be provided to the Qualcomm glue layer as well.
>>
>> Currently, the way DWC3 core and Qualcomm legacy glue driver are designed,
>> there is no mechanism through which the DWC3 core can notify the Qualcomm
>> glue layer of any role changes which it receives via role switch. To
>> register these glue callbacks at probe time, for enabling core to notify
>> glue layer, the legacy Qualcomm driver has no way to find out when the
>> child driver probe was successful since it does not check for the same
>> during of_platform_populate.
>>
>> Hence implement the following glue callbacks for flattened Qualcomm glue
>> driver:
>>
>> 1. set_role: To pass role switching information from drd layer to glue.
>> This information is needed to identify NONE/DEVICE mode switch and modify
>> QSCRATCH to generate connect-done event on device mode entry and disconnect
>> event on cable removal in device mode.
>>
>> 2. run_stop: When booting up in device mode, if autouspend is enabled and
>> userspace doesn't write UDC on boot, controller enters autosuspend. After
>> this, if the userspace writes to UDC in the future, run_stop notifier is
>> required to enable UTMI_OTG_VBUS_VALID of QSCRATCH so that connect done
>> event is generated after run_stop(1) is done to finish enumeration.
>>
>> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
>> ---
>>   drivers/usb/dwc3/dwc3-qcom.c | 82 ++++++++++++++++++++++++++++++++----
>>   1 file changed, 73 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
>> index ca7e1c02773a..d40b52e2ba01 100644
>> --- a/drivers/usb/dwc3/dwc3-qcom.c
>> +++ b/drivers/usb/dwc3/dwc3-qcom.c
>> @@ -89,6 +89,12 @@ struct dwc3_qcom {
>>   	bool			pm_suspended;
>>   	struct icc_path		*icc_path_ddr;
>>   	struct icc_path		*icc_path_apps;
>> +
>> +	/*
>> +	 * Current role changes via usb_role_switch_set_role callback protected
>> +	 * internally by mutex lock.
>> +	 */
>> +	enum usb_role		current_role;
>>   };
>>   
>>   #define to_dwc3_qcom(d) container_of((d), struct dwc3_qcom, dwc)
>> @@ -118,9 +124,9 @@ static inline void dwc3_qcom_clrbits(void __iomem *base, u32 offset, u32 val)
>>   }
>>   
>>   /*
>> - * TODO: Make the in-core role switching code invoke dwc3_qcom_vbus_override_enable(),
>> - * validate that the in-core extcon support is functional, and drop extcon
>> - * handling from the glue
>> + * TODO: Validate that the in-core extcon support is functional, and drop
>> + * extcon handling from the glue. Make in-core extcon invoke
>> + * dwc3_qcom_vbus_override_enable()
>>    */
>>   static void dwc3_qcom_vbus_override_enable(struct dwc3_qcom *qcom, bool enable)
>>   {
>> @@ -641,6 +647,53 @@ static int dwc3_qcom_setup_irq(struct dwc3_qcom *qcom, struct platform_device *p
>>   	return 0;
>>   }
>>   
>> +static void dwc3_qcom_set_role_notifier(struct dwc3 *dwc, enum usb_role next_role)
>> +{
>> +	struct dwc3_qcom *qcom = to_dwc3_qcom(dwc);
>> +
>> +	if (qcom->current_role == next_role)
>> +		return;
>> +
>> +	if (pm_runtime_resume_and_get(qcom->dev) < 0) {
>> +		dev_dbg(qcom->dev, "Failed to resume device\n");
>> +		return;
>> +	}
>> +
>> +	if (qcom->current_role == USB_ROLE_DEVICE &&
>> +	    next_role != USB_ROLE_DEVICE)
>> +		dwc3_qcom_vbus_override_enable(qcom, false);
>> +	else if ((qcom->current_role != USB_ROLE_DEVICE) &&
>> +		 (next_role == USB_ROLE_DEVICE))
>> +		dwc3_qcom_vbus_override_enable(qcom, true);
>> +
>> +	pm_runtime_mark_last_busy(qcom->dev);
>> +	pm_runtime_put_sync(qcom->dev);
>> +
>> +	qcom->current_role = next_role;
> 
> How is it protected by the mutex? Which mutex?
> 

I see a mutex lock in usb_role_switch_set_role() that invokes set role. 
I think that should be sufficient here.

>> +}
>> +
>> +static void dwc3_qcom_run_stop_notifier(struct dwc3 *dwc, bool is_on)
>> +{
>> +	struct dwc3_qcom *qcom = to_dwc3_qcom(dwc);
>> +
>> +	/*
>> +	 * When autosuspend is enabled and controller goes to suspend
>> +	 * after removing UDC from userspace, the next UDC write needs
>> +	 * setting of QSCRATCH VBUS_VALID to "1" to generate a connect
>> +	 * done event.
>> +	 */
>> +	if (!is_on)
>> +		return;
>> +
>> +	dwc3_qcom_vbus_override_enable(qcom, is_on);
>> +	pm_runtime_mark_last_busy(qcom->dev);
>> +}
>> +
>> +struct dwc3_glue_ops dwc3_qcom_glue_ops = {
>> +	.notify_set_role	= dwc3_qcom_set_role_notifier,
>> +	.notify_run_stop	= dwc3_qcom_run_stop_notifier,
>> +};
>> +
>>   static int dwc3_qcom_probe(struct platform_device *pdev)
>>   {
>>   	struct dwc3_probe_data	probe_data = {};
>> @@ -717,6 +770,23 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>>   	if (ignore_pipe_clk)
>>   		dwc3_qcom_select_utmi_clk(qcom);
>>   
>> +	qcom->mode = usb_get_dr_mode(dev);
>> +
>> +	if (qcom->mode == USB_DR_MODE_HOST) {
>> +		qcom->current_role = USB_ROLE_HOST;
>> +	} else if (qcom->mode == USB_DR_MODE_PERIPHERAL) {
>> +		qcom->current_role = USB_ROLE_DEVICE;
>> +		dwc3_qcom_vbus_override_enable(qcom, true);
>> +	} else if (qcom->mode == USB_DR_MODE_OTG) {
> 
> Just 'else', otherwise you are not going to implement the default case
> correctly (per usb-drd.yaml we should default to OTG).
> 

ACK. Will modify it in v3.

Regards,
Krishna,

>> +		if ((device_property_read_bool(dev, "usb-role-switch")) &&
>> +		    (usb_get_role_switch_default_mode(dev) == USB_DR_MODE_HOST))
>> +			qcom->current_role = USB_ROLE_HOST;
>> +		else
>> +			qcom->current_role = USB_ROLE_DEVICE;
>> +	}
>> +
>> +	qcom->dwc.glue_ops = &dwc3_qcom_glue_ops;
>> +
>>   	qcom->dwc.dev = dev;
>>   	probe_data.dwc = &qcom->dwc;
>>   	probe_data.res = &res;
>> @@ -731,12 +801,6 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>>   	if (ret)
>>   		goto remove_core;
>>   
>> -	qcom->mode = usb_get_dr_mode(dev);
>> -
>> -	/* enable vbus override for device mode */
>> -	if (qcom->mode != USB_DR_MODE_HOST)
>> -		dwc3_qcom_vbus_override_enable(qcom, true);
>> -
>>   	/* register extcon to override sw_vbus on Vbus change later */
>>   	ret = dwc3_qcom_register_extcon(qcom);
>>   	if (ret)
>> -- 
>> 2.34.1
>>

Re: [PATCH v2 3/4] usb: dwc3: qcom: Facilitate autosuspend during host mode

[Krishna Kurapati]

On 6/10/2025 4:30 PM, Dmitry Baryshkov wrote:
> On Tue, Jun 10, 2025 at 02:43:56PM +0530, Krishna Kurapati wrote:
>> When in host mode, it is intended that the controller goes to suspend
>> state to save power and wait for interrupts from connected peripheral
>> to wake it up. This is particularly used in cases where a HID or Audio
>> device is connected. In such scenarios, the usb controller can enter
>> auto suspend and resume action after getting interrupts from the
>> connected device.
>>
>> Allow autosuspend for and xhci device and allow userspace to decide
>> whether to enable this functionality.
>>
>> a) Register to usb-core notifications in set_role vendor callback to
>> identify when root hubs are being created. Configure them to
>> use_autosuspend.
>>
>> b) Identify usb core notifications where the HCD is being added and
>> enable autosuspend for that particular xhci device.
>>
>> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
>> ---
>>   drivers/usb/dwc3/dwc3-qcom.c | 62 ++++++++++++++++++++++++++++++++----
>>   1 file changed, 56 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
>> index d40b52e2ba01..17bbd5a06c08 100644
>> --- a/drivers/usb/dwc3/dwc3-qcom.c
>> +++ b/drivers/usb/dwc3/dwc3-qcom.c
>> @@ -95,6 +95,8 @@ struct dwc3_qcom {
>>   	 * internally by mutex lock.
>>   	 */
>>   	enum usb_role		current_role;
>> +
>> +	struct notifier_block	xhci_nb;
>>   };
>>   
>>   #define to_dwc3_qcom(d) container_of((d), struct dwc3_qcom, dwc)
>> @@ -647,6 +649,39 @@ static int dwc3_qcom_setup_irq(struct dwc3_qcom *qcom, struct platform_device *p
>>   	return 0;
>>   }
>>   
>> +static int dwc3_xhci_event_notifier(struct notifier_block *nb,
>> +				    unsigned long event, void *ptr)
>> +{
>> +	struct dwc3_qcom  *qcom	= container_of(nb, struct dwc3_qcom, xhci_nb);
>> +	struct dwc3	  *dwc	= &qcom->dwc;
>> +	struct usb_bus	  *ubus	= ptr;
>> +	struct usb_hcd	  *hcd;
>> +
>> +	if (!dwc->xhci)
>> +		goto done;
>> +
>> +	hcd = platform_get_drvdata(dwc->xhci);
>> +	if (!hcd)
>> +		goto done;
>> +
>> +	if (event != USB_BUS_ADD)
>> +		goto done;
>> +
>> +	if (strcmp(dev_name(ubus->sysdev), dev_name(dwc->sysdev)) != 0)
>> +		goto done;
>> +
>> +	if (event == USB_BUS_ADD) {
>> +		/*
>> +		 * Identify instant of creation of primary hcd and
>> +		 * mark xhci as autosuspend capable at this point.
>> +		 */
>> +		pm_runtime_use_autosuspend(&dwc->xhci->dev);
> 
> This feels like an overkill, using notifiers to set autosuspend flag.
> Please extend platform data and/or other static code in order to set the
> flag on the created xHCI devices.
> 

Do you mean modifying pdev of xhci from dwc3/host.c ? Or adding the 
use_autosuspend call in xhci-plat.c ?

I thought adding this notifier would be a better way to identify when 
the xhci probe is in progress instead of touching pdev of xhci device 
from dwc3 layer.

Regards,
Krishna,

>> +	}
>> +
>> +done:
>> +	return NOTIFY_DONE;
>> +}
>> +
>>   static void dwc3_qcom_set_role_notifier(struct dwc3 *dwc, enum usb_role next_role)
>>   {
>>   	struct dwc3_qcom *qcom = to_dwc3_qcom(dwc);
>> @@ -659,12 +694,22 @@ static void dwc3_qcom_set_role_notifier(struct dwc3 *dwc, enum usb_role next_rol
>>   		return;
>>   	}
>>   
>> -	if (qcom->current_role == USB_ROLE_DEVICE &&
>> -	    next_role != USB_ROLE_DEVICE)
>> +	if (qcom->current_role == USB_ROLE_NONE) {
>> +		if (next_role == USB_ROLE_DEVICE) {
>> +			dwc3_qcom_vbus_override_enable(qcom, true);
>> +		} else if (next_role == USB_ROLE_HOST) {
>> +			qcom->xhci_nb.notifier_call = dwc3_xhci_event_notifier;
>> +			usb_register_notify(&qcom->xhci_nb);
>> +		}
>> +	} else if (qcom->current_role == USB_ROLE_DEVICE &&
>> +		   next_role != USB_ROLE_DEVICE) {
>>   		dwc3_qcom_vbus_override_enable(qcom, false);
>> -	else if ((qcom->current_role != USB_ROLE_DEVICE) &&
>> -		 (next_role == USB_ROLE_DEVICE))
>> -		dwc3_qcom_vbus_override_enable(qcom, true);
>> +	} else if (qcom->current_role == USB_ROLE_HOST) {
>> +		if (next_role == USB_ROLE_NONE)
>> +			usb_unregister_notify(&qcom->xhci_nb);
>> +		else if (next_role == USB_ROLE_DEVICE)
>> +			dwc3_qcom_vbus_override_enable(qcom, true);
>> +	}
>>   
>>   	pm_runtime_mark_last_busy(qcom->dev);
>>   	pm_runtime_put_sync(qcom->dev);
>> @@ -774,6 +819,8 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>>   
>>   	if (qcom->mode == USB_DR_MODE_HOST) {
>>   		qcom->current_role = USB_ROLE_HOST;
>> +		qcom->xhci_nb.notifier_call = dwc3_xhci_event_notifier;
>> +		usb_register_notify(&qcom->xhci_nb);
>>   	} else if (qcom->mode == USB_DR_MODE_PERIPHERAL) {
>>   		qcom->current_role = USB_ROLE_DEVICE;
>>   		dwc3_qcom_vbus_override_enable(qcom, true);
>> @@ -794,7 +841,7 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>>   	ret = dwc3_core_probe(&probe_data);
>>   	if (ret)  {
>>   		ret = dev_err_probe(dev, ret, "failed to register DWC3 Core\n");
>> -		goto clk_disable;
>> +		goto unregister_notify;
>>   	}
>>   
>>   	ret = dwc3_qcom_interconnect_init(qcom);
>> @@ -817,6 +864,9 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>>   	dwc3_qcom_interconnect_exit(qcom);
>>   remove_core:
>>   	dwc3_core_remove(&qcom->dwc);
>> +unregister_notify:
>> +	if (qcom->mode == USB_DR_MODE_HOST)
>> +		usb_unregister_notify(&qcom->xhci_nb);
>>   clk_disable:
>>   	clk_bulk_disable_unprepare(qcom->num_clocks, qcom->clks);
>>   
>> -- 
>> 2.34.1
>>

Re: [PATCH v2 2/4] usb: dwc3: qcom: Implement glue callbacks to facilitate runtime suspend

[Dmitry Baryshkov]

On Tue, Jun 10, 2025 at 10:06:24PM +0530, Krishna Kurapati wrote:
> 
> 
> On 6/10/2025 4:28 PM, Dmitry Baryshkov wrote:
> > On Tue, Jun 10, 2025 at 02:43:55PM +0530, Krishna Kurapati wrote:
> > > On Qualcomm DWC3 dual-role controllers, the conndone/disconnect events in
> > > device mode are generated by controller when software writes to QSCRATCH
> > > registers in Qualcomm Glue layer rather than the vbus line being routed to
> > > dwc3 core IP for it to recognize and generate these events.
> > > 
> > > UTMI_OTG_VBUS_VALID  bit of QSCRATCH_HS_PHY_CTRL register needs to be set
> > > to generate a connection done event and to be cleared for the controller to
> > > generate a disconnect event during cable removal. When the disconnect is
> > > not generated upon cable removal, the "connected" flag of dwc3 is left
> > > marked as "true" and it blocks suspend routines and for that to happen upon
> > > cable removal, the cable disconnect notification coming in via set_role
> > > call need to be provided to the Qualcomm glue layer as well.
> > > 
> > > Currently, the way DWC3 core and Qualcomm legacy glue driver are designed,
> > > there is no mechanism through which the DWC3 core can notify the Qualcomm
> > > glue layer of any role changes which it receives via role switch. To
> > > register these glue callbacks at probe time, for enabling core to notify
> > > glue layer, the legacy Qualcomm driver has no way to find out when the
> > > child driver probe was successful since it does not check for the same
> > > during of_platform_populate.
> > > 
> > > Hence implement the following glue callbacks for flattened Qualcomm glue
> > > driver:
> > > 
> > > 1. set_role: To pass role switching information from drd layer to glue.
> > > This information is needed to identify NONE/DEVICE mode switch and modify
> > > QSCRATCH to generate connect-done event on device mode entry and disconnect
> > > event on cable removal in device mode.
> > > 
> > > 2. run_stop: When booting up in device mode, if autouspend is enabled and
> > > userspace doesn't write UDC on boot, controller enters autosuspend. After
> > > this, if the userspace writes to UDC in the future, run_stop notifier is
> > > required to enable UTMI_OTG_VBUS_VALID of QSCRATCH so that connect done
> > > event is generated after run_stop(1) is done to finish enumeration.
> > > 
> > > Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> > > ---
> > >   drivers/usb/dwc3/dwc3-qcom.c | 82 ++++++++++++++++++++++++++++++++----
> > >   1 file changed, 73 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> > > index ca7e1c02773a..d40b52e2ba01 100644
> > > --- a/drivers/usb/dwc3/dwc3-qcom.c
> > > +++ b/drivers/usb/dwc3/dwc3-qcom.c
> > > @@ -89,6 +89,12 @@ struct dwc3_qcom {
> > >   	bool			pm_suspended;
> > >   	struct icc_path		*icc_path_ddr;
> > >   	struct icc_path		*icc_path_apps;
> > > +
> > > +	/*
> > > +	 * Current role changes via usb_role_switch_set_role callback protected
> > > +	 * internally by mutex lock.
> > > +	 */
> > > +	enum usb_role		current_role;
> > >   };
> > >   #define to_dwc3_qcom(d) container_of((d), struct dwc3_qcom, dwc)
> > > @@ -118,9 +124,9 @@ static inline void dwc3_qcom_clrbits(void __iomem *base, u32 offset, u32 val)
> > >   }
> > >   /*
> > > - * TODO: Make the in-core role switching code invoke dwc3_qcom_vbus_override_enable(),
> > > - * validate that the in-core extcon support is functional, and drop extcon
> > > - * handling from the glue
> > > + * TODO: Validate that the in-core extcon support is functional, and drop
> > > + * extcon handling from the glue. Make in-core extcon invoke
> > > + * dwc3_qcom_vbus_override_enable()
> > >    */
> > >   static void dwc3_qcom_vbus_override_enable(struct dwc3_qcom *qcom, bool enable)
> > >   {
> > > @@ -641,6 +647,53 @@ static int dwc3_qcom_setup_irq(struct dwc3_qcom *qcom, struct platform_device *p
> > >   	return 0;
> > >   }
> > > +static void dwc3_qcom_set_role_notifier(struct dwc3 *dwc, enum usb_role next_role)
> > > +{
> > > +	struct dwc3_qcom *qcom = to_dwc3_qcom(dwc);
> > > +
> > > +	if (qcom->current_role == next_role)
> > > +		return;
> > > +
> > > +	if (pm_runtime_resume_and_get(qcom->dev) < 0) {
> > > +		dev_dbg(qcom->dev, "Failed to resume device\n");
> > > +		return;
> > > +	}
> > > +
> > > +	if (qcom->current_role == USB_ROLE_DEVICE &&
> > > +	    next_role != USB_ROLE_DEVICE)
> > > +		dwc3_qcom_vbus_override_enable(qcom, false);
> > > +	else if ((qcom->current_role != USB_ROLE_DEVICE) &&
> > > +		 (next_role == USB_ROLE_DEVICE))
> > > +		dwc3_qcom_vbus_override_enable(qcom, true);
> > > +
> > > +	pm_runtime_mark_last_busy(qcom->dev);
> > > +	pm_runtime_put_sync(qcom->dev);
> > > +
> > > +	qcom->current_role = next_role;
> > 
> > How is it protected by the mutex? Which mutex?
> > 
> 
> I see a mutex lock in usb_role_switch_set_role() that invokes set role. I
> think that should be sufficient here.

Please add a comment to the source code.

-- 
With best wishes
Dmitry

Re: [PATCH v2 3/4] usb: dwc3: qcom: Facilitate autosuspend during host mode

[Dmitry Baryshkov]

On Tue, Jun 10, 2025 at 10:10:33PM +0530, Krishna Kurapati wrote:
> 
> 
> On 6/10/2025 4:30 PM, Dmitry Baryshkov wrote:
> > On Tue, Jun 10, 2025 at 02:43:56PM +0530, Krishna Kurapati wrote:
> > > When in host mode, it is intended that the controller goes to suspend
> > > state to save power and wait for interrupts from connected peripheral
> > > to wake it up. This is particularly used in cases where a HID or Audio
> > > device is connected. In such scenarios, the usb controller can enter
> > > auto suspend and resume action after getting interrupts from the
> > > connected device.
> > > 
> > > Allow autosuspend for and xhci device and allow userspace to decide
> > > whether to enable this functionality.
> > > 
> > > a) Register to usb-core notifications in set_role vendor callback to
> > > identify when root hubs are being created. Configure them to
> > > use_autosuspend.
> > > 
> > > b) Identify usb core notifications where the HCD is being added and
> > > enable autosuspend for that particular xhci device.
> > > 
> > > Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> > > ---
> > >   drivers/usb/dwc3/dwc3-qcom.c | 62 ++++++++++++++++++++++++++++++++----
> > >   1 file changed, 56 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> > > index d40b52e2ba01..17bbd5a06c08 100644
> > > --- a/drivers/usb/dwc3/dwc3-qcom.c
> > > +++ b/drivers/usb/dwc3/dwc3-qcom.c
> > > @@ -95,6 +95,8 @@ struct dwc3_qcom {
> > >   	 * internally by mutex lock.
> > >   	 */
> > >   	enum usb_role		current_role;
> > > +
> > > +	struct notifier_block	xhci_nb;
> > >   };
> > >   #define to_dwc3_qcom(d) container_of((d), struct dwc3_qcom, dwc)
> > > @@ -647,6 +649,39 @@ static int dwc3_qcom_setup_irq(struct dwc3_qcom *qcom, struct platform_device *p
> > >   	return 0;
> > >   }
> > > +static int dwc3_xhci_event_notifier(struct notifier_block *nb,
> > > +				    unsigned long event, void *ptr)
> > > +{
> > > +	struct dwc3_qcom  *qcom	= container_of(nb, struct dwc3_qcom, xhci_nb);
> > > +	struct dwc3	  *dwc	= &qcom->dwc;
> > > +	struct usb_bus	  *ubus	= ptr;
> > > +	struct usb_hcd	  *hcd;
> > > +
> > > +	if (!dwc->xhci)
> > > +		goto done;
> > > +
> > > +	hcd = platform_get_drvdata(dwc->xhci);
> > > +	if (!hcd)
> > > +		goto done;
> > > +
> > > +	if (event != USB_BUS_ADD)
> > > +		goto done;
> > > +
> > > +	if (strcmp(dev_name(ubus->sysdev), dev_name(dwc->sysdev)) != 0)
> > > +		goto done;
> > > +
> > > +	if (event == USB_BUS_ADD) {
> > > +		/*
> > > +		 * Identify instant of creation of primary hcd and
> > > +		 * mark xhci as autosuspend capable at this point.
> > > +		 */
> > > +		pm_runtime_use_autosuspend(&dwc->xhci->dev);
> > 
> > This feels like an overkill, using notifiers to set autosuspend flag.
> > Please extend platform data and/or other static code in order to set the
> > flag on the created xHCI devices.
> > 
> 
> Do you mean modifying pdev of xhci from dwc3/host.c ? Or adding the
> use_autosuspend call in xhci-plat.c ?

The latter one.

> 
> I thought adding this notifier would be a better way to identify when the
> xhci probe is in progress instead of touching pdev of xhci device from dwc3
> layer.
> 
> Regards,
> Krishna,

-- 
With best wishes
Dmitry

Re: [PATCH v2 2/4] usb: dwc3: qcom: Implement glue callbacks to facilitate runtime suspend

[Krishna Kurapati]

On 6/10/2025 10:53 PM, Dmitry Baryshkov wrote:
> On Tue, Jun 10, 2025 at 10:06:24PM +0530, Krishna Kurapati wrote:
>>
>>
>> On 6/10/2025 4:28 PM, Dmitry Baryshkov wrote:
>>> On Tue, Jun 10, 2025 at 02:43:55PM +0530, Krishna Kurapati wrote:
>>>> On Qualcomm DWC3 dual-role controllers, the conndone/disconnect events in
>>>> device mode are generated by controller when software writes to QSCRATCH
>>>> registers in Qualcomm Glue layer rather than the vbus line being routed to
>>>> dwc3 core IP for it to recognize and generate these events.
>>>>
>>>> UTMI_OTG_VBUS_VALID  bit of QSCRATCH_HS_PHY_CTRL register needs to be set
>>>> to generate a connection done event and to be cleared for the controller to
>>>> generate a disconnect event during cable removal. When the disconnect is
>>>> not generated upon cable removal, the "connected" flag of dwc3 is left
>>>> marked as "true" and it blocks suspend routines and for that to happen upon
>>>> cable removal, the cable disconnect notification coming in via set_role
>>>> call need to be provided to the Qualcomm glue layer as well.
>>>>
>>>> Currently, the way DWC3 core and Qualcomm legacy glue driver are designed,
>>>> there is no mechanism through which the DWC3 core can notify the Qualcomm
>>>> glue layer of any role changes which it receives via role switch. To
>>>> register these glue callbacks at probe time, for enabling core to notify
>>>> glue layer, the legacy Qualcomm driver has no way to find out when the
>>>> child driver probe was successful since it does not check for the same
>>>> during of_platform_populate.
>>>>
>>>> Hence implement the following glue callbacks for flattened Qualcomm glue
>>>> driver:
>>>>
>>>> 1. set_role: To pass role switching information from drd layer to glue.
>>>> This information is needed to identify NONE/DEVICE mode switch and modify
>>>> QSCRATCH to generate connect-done event on device mode entry and disconnect
>>>> event on cable removal in device mode.
>>>>
>>>> 2. run_stop: When booting up in device mode, if autouspend is enabled and
>>>> userspace doesn't write UDC on boot, controller enters autosuspend. After
>>>> this, if the userspace writes to UDC in the future, run_stop notifier is
>>>> required to enable UTMI_OTG_VBUS_VALID of QSCRATCH so that connect done
>>>> event is generated after run_stop(1) is done to finish enumeration.
>>>>
>>>> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
>>>> ---
>>>>    drivers/usb/dwc3/dwc3-qcom.c | 82 ++++++++++++++++++++++++++++++++----
>>>>    1 file changed, 73 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
>>>> index ca7e1c02773a..d40b52e2ba01 100644
>>>> --- a/drivers/usb/dwc3/dwc3-qcom.c
>>>> +++ b/drivers/usb/dwc3/dwc3-qcom.c
>>>> @@ -89,6 +89,12 @@ struct dwc3_qcom {
>>>>    	bool			pm_suspended;
>>>>    	struct icc_path		*icc_path_ddr;
>>>>    	struct icc_path		*icc_path_apps;
>>>> +
>>>> +	/*
>>>> +	 * Current role changes via usb_role_switch_set_role callback protected
>>>> +	 * internally by mutex lock.
>>>> +	 */
>>>> +	enum usb_role		current_role;
>>>>    };
>>>>    #define to_dwc3_qcom(d) container_of((d), struct dwc3_qcom, dwc)
>>>> @@ -118,9 +124,9 @@ static inline void dwc3_qcom_clrbits(void __iomem *base, u32 offset, u32 val)
>>>>    }
>>>>    /*
>>>> - * TODO: Make the in-core role switching code invoke dwc3_qcom_vbus_override_enable(),
>>>> - * validate that the in-core extcon support is functional, and drop extcon
>>>> - * handling from the glue
>>>> + * TODO: Validate that the in-core extcon support is functional, and drop
>>>> + * extcon handling from the glue. Make in-core extcon invoke
>>>> + * dwc3_qcom_vbus_override_enable()
>>>>     */
>>>>    static void dwc3_qcom_vbus_override_enable(struct dwc3_qcom *qcom, bool enable)
>>>>    {
>>>> @@ -641,6 +647,53 @@ static int dwc3_qcom_setup_irq(struct dwc3_qcom *qcom, struct platform_device *p
>>>>    	return 0;
>>>>    }
>>>> +static void dwc3_qcom_set_role_notifier(struct dwc3 *dwc, enum usb_role next_role)
>>>> +{
>>>> +	struct dwc3_qcom *qcom = to_dwc3_qcom(dwc);
>>>> +
>>>> +	if (qcom->current_role == next_role)
>>>> +		return;
>>>> +
>>>> +	if (pm_runtime_resume_and_get(qcom->dev) < 0) {
>>>> +		dev_dbg(qcom->dev, "Failed to resume device\n");
>>>> +		return;
>>>> +	}
>>>> +
>>>> +	if (qcom->current_role == USB_ROLE_DEVICE &&
>>>> +	    next_role != USB_ROLE_DEVICE)
>>>> +		dwc3_qcom_vbus_override_enable(qcom, false);
>>>> +	else if ((qcom->current_role != USB_ROLE_DEVICE) &&
>>>> +		 (next_role == USB_ROLE_DEVICE))
>>>> +		dwc3_qcom_vbus_override_enable(qcom, true);
>>>> +
>>>> +	pm_runtime_mark_last_busy(qcom->dev);
>>>> +	pm_runtime_put_sync(qcom->dev);
>>>> +
>>>> +	qcom->current_role = next_role;
>>>
>>> How is it protected by the mutex? Which mutex?
>>>
>>
>> I see a mutex lock in usb_role_switch_set_role() that invokes set role. I
>> think that should be sufficient here.
> 
> Please add a comment to the source code.
> 

Hi Dmitry,

  I added the comment at the declaration of variable in structure. Will 
move it to the function.

  Thank you for the review.

Regards,
Krishna,

Re: [PATCH v2 1/4] usb: dwc3: core: Introduce glue callbacks for flattened implementations

[Thinh Nguyen]

Hi,

On Tue, Jun 10, 2025, Krishna Kurapati wrote:
> In certain situations like role switching, the glue layers need to be
> informed of these events, so that they can take any necessary action.
> But in non-flattened implementations, the glue drivers have no data on
> when the core driver probe was successful post invoking of_platform_
> populate. Now that the core driver supports flattened implementations
> as well, introduce vendor callbacks that can be passed on from glue to
> core before invoking dwc3_core_probe.
> 
> Introduce callbacks to notify glue layer of role_switch and run_stop
> changes. These can be used by flattened implementation of Qualcomm
> glue layer to generate connect/disconnect events in controller during
> cable connect and run stop modifications by udc in device mode.
> 
> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> ---
>  drivers/usb/dwc3/core.c   |  1 +
>  drivers/usb/dwc3/core.h   | 26 ++++++++++++++++++++++++++
>  drivers/usb/dwc3/drd.c    |  1 +
>  drivers/usb/dwc3/gadget.c |  1 +
>  4 files changed, 29 insertions(+)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 2bc775a747f2..c01b15e3710f 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -2351,6 +2351,7 @@ static int dwc3_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  
>  	dwc->dev = &pdev->dev;
> +	dwc->glue_ops = NULL;
>  
>  	probe_data.dwc = dwc;
>  	probe_data.res = res;
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index d5b985fa12f4..a803884be4ed 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -992,6 +992,17 @@ struct dwc3_scratchpad_array {
>  	__le64	dma_adr[DWC3_MAX_HIBER_SCRATCHBUFS];
>  };
>  
> +/*
> + * struct dwc3_glue_ops - The ops indicate the notifications that
> + *				need to be passed on to glue layer
> + * @notify_set_role: Notify glue of role switch notifications
> + * @notify_run_stop: Notify run stop enable/disable information to glue
> + */
> +struct dwc3_glue_ops {
> +	void	(*notify_set_role)(struct dwc3 *dwc, enum usb_role role);
> +	void	(*notify_run_stop)(struct dwc3 *dwc, bool is_on);

These are not to simply to "notify". They are callbacks right? Perhaps
prefix them pre_* or post_* instead of notify_*.

> +};
> +

(Appologies for the delays, I'll get back on reviewing more of this
series. I've been flooded with other tasks these last few days).

BR,
Thinh

Re: [PATCH v2 1/4] usb: dwc3: core: Introduce glue callbacks for flattened implementations

[Thinh Nguyen]

On Tue, Jun 10, 2025, Krishna Kurapati wrote:
> In certain situations like role switching, the glue layers need to be
> informed of these events, so that they can take any necessary action.
> But in non-flattened implementations, the glue drivers have no data on
> when the core driver probe was successful post invoking of_platform_
> populate. Now that the core driver supports flattened implementations
> as well, introduce vendor callbacks that can be passed on from glue to
> core before invoking dwc3_core_probe.
> 
> Introduce callbacks to notify glue layer of role_switch and run_stop
> changes. These can be used by flattened implementation of Qualcomm
> glue layer to generate connect/disconnect events in controller during
> cable connect and run stop modifications by udc in device mode.
> 
> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> ---
>  drivers/usb/dwc3/core.c   |  1 +
>  drivers/usb/dwc3/core.h   | 26 ++++++++++++++++++++++++++
>  drivers/usb/dwc3/drd.c    |  1 +
>  drivers/usb/dwc3/gadget.c |  1 +
>  4 files changed, 29 insertions(+)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 2bc775a747f2..c01b15e3710f 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -2351,6 +2351,7 @@ static int dwc3_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  
>  	dwc->dev = &pdev->dev;
> +	dwc->glue_ops = NULL;
>  
>  	probe_data.dwc = dwc;
>  	probe_data.res = res;
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index d5b985fa12f4..a803884be4ed 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -992,6 +992,17 @@ struct dwc3_scratchpad_array {
>  	__le64	dma_adr[DWC3_MAX_HIBER_SCRATCHBUFS];
>  };
>  
> +/*

Let's keep consistent with the doc style /**

> + * struct dwc3_glue_ops - The ops indicate the notifications that
> + *				need to be passed on to glue layer
> + * @notify_set_role: Notify glue of role switch notifications
> + * @notify_run_stop: Notify run stop enable/disable information to glue
> + */
> +struct dwc3_glue_ops {
> +	void	(*notify_set_role)(struct dwc3 *dwc, enum usb_role role);
> +	void	(*notify_run_stop)(struct dwc3 *dwc, bool is_on);

Use pre_ or prep_ prefix instead of notify_ indicating callbacks for
glue driver to perform updates before set_role or run_stop.

> +};
> +
>  /**
>   * struct dwc3 - representation of our controller
>   * @drd_work: workqueue used for role swapping
> @@ -1168,6 +1179,7 @@ struct dwc3_scratchpad_array {
>   * @wakeup_pending_funcs: Indicates whether any interface has requested for
>   *			 function wakeup in bitmap format where bit position
>   *			 represents interface_id.
> + * @glue_ops: Vendor callbacks for flattened device implementations.
>   */
>  struct dwc3 {
>  	struct work_struct	drd_work;
> @@ -1400,6 +1412,8 @@ struct dwc3 {
>  	struct dentry		*debug_root;
>  	u32			gsbuscfg0_reqinfo;
>  	u32			wakeup_pending_funcs;
> +
> +	struct dwc3_glue_ops	*glue_ops;

Use const, and move this closer on top. Perhaps below gadget_driver.

>  };
>  
>  #define INCRX_BURST_MODE 0
> @@ -1614,6 +1628,18 @@ void dwc3_event_buffers_cleanup(struct dwc3 *dwc);
>  int dwc3_core_soft_reset(struct dwc3 *dwc);
>  void dwc3_enable_susphy(struct dwc3 *dwc, bool enable);
>  
> +static inline void dwc3_notify_set_role(struct dwc3 *dwc, enum usb_role role)
> +{
> +	if (dwc->glue_ops && dwc->glue_ops->notify_set_role)
> +		dwc->glue_ops->notify_set_role(dwc, role);
> +}
> +
> +static inline void dwc3_notify_run_stop(struct dwc3 *dwc, bool is_on)
> +{
> +	if (dwc->glue_ops && dwc->glue_ops->notify_run_stop)
> +		dwc->glue_ops->notify_run_stop(dwc, is_on);
> +}
> +
>  #if IS_ENABLED(CONFIG_USB_DWC3_HOST) || IS_ENABLED(CONFIG_USB_DWC3_DUAL_ROLE)
>  int dwc3_host_init(struct dwc3 *dwc);
>  void dwc3_host_exit(struct dwc3 *dwc);
> diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
> index 7977860932b1..408551768a95 100644
> --- a/drivers/usb/dwc3/drd.c
> +++ b/drivers/usb/dwc3/drd.c
> @@ -464,6 +464,7 @@ static int dwc3_usb_role_switch_set(struct usb_role_switch *sw,
>  		break;
>  	}
>  
> +	dwc3_notify_set_role(dwc, role);

This should be done in __dwc3_set_mode(). Perhaps right before setting
PRTCAPDIR?

>  	dwc3_set_mode(dwc, mode);
>  	return 0;
>  }
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 321361288935..73bed11bccaf 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -2641,6 +2641,7 @@ static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on)
>  	if (saved_config)
>  		dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>  
> +	dwc3_notify_run_stop(dwc, is_on);

This should be done right before writing to DCTL.run_stop.

>  	reg = dwc3_readl(dwc->regs, DWC3_DCTL);
>  	if (is_on) {
>  		if (DWC3_VER_IS_WITHIN(DWC3, ANY, 187A)) {
> -- 
> 2.34.1
> 

BR,
Thinh

Re: [PATCH v2 2/4] usb: dwc3: qcom: Implement glue callbacks to facilitate runtime suspend

[Thinh Nguyen]

On Tue, Jun 10, 2025, Krishna Kurapati wrote:
> On Qualcomm DWC3 dual-role controllers, the conndone/disconnect events in
> device mode are generated by controller when software writes to QSCRATCH
> registers in Qualcomm Glue layer rather than the vbus line being routed to
> dwc3 core IP for it to recognize and generate these events.
> 
> UTMI_OTG_VBUS_VALID  bit of QSCRATCH_HS_PHY_CTRL register needs to be set
> to generate a connection done event and to be cleared for the controller to
> generate a disconnect event during cable removal. When the disconnect is
> not generated upon cable removal, the "connected" flag of dwc3 is left
> marked as "true" and it blocks suspend routines and for that to happen upon
> cable removal, the cable disconnect notification coming in via set_role
> call need to be provided to the Qualcomm glue layer as well.
> 
> Currently, the way DWC3 core and Qualcomm legacy glue driver are designed,
> there is no mechanism through which the DWC3 core can notify the Qualcomm
> glue layer of any role changes which it receives via role switch. To
> register these glue callbacks at probe time, for enabling core to notify
> glue layer, the legacy Qualcomm driver has no way to find out when the
> child driver probe was successful since it does not check for the same
> during of_platform_populate.
> 
> Hence implement the following glue callbacks for flattened Qualcomm glue
> driver:
> 
> 1. set_role: To pass role switching information from drd layer to glue.
> This information is needed to identify NONE/DEVICE mode switch and modify
> QSCRATCH to generate connect-done event on device mode entry and disconnect
> event on cable removal in device mode.
> 
> 2. run_stop: When booting up in device mode, if autouspend is enabled and
> userspace doesn't write UDC on boot, controller enters autosuspend. After
> this, if the userspace writes to UDC in the future, run_stop notifier is
> required to enable UTMI_OTG_VBUS_VALID of QSCRATCH so that connect done
> event is generated after run_stop(1) is done to finish enumeration.
> 
> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> ---
>  drivers/usb/dwc3/dwc3-qcom.c | 82 ++++++++++++++++++++++++++++++++----
>  1 file changed, 73 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index ca7e1c02773a..d40b52e2ba01 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -89,6 +89,12 @@ struct dwc3_qcom {
>  	bool			pm_suspended;
>  	struct icc_path		*icc_path_ddr;
>  	struct icc_path		*icc_path_apps;
> +
> +	/*
> +	 * Current role changes via usb_role_switch_set_role callback protected
> +	 * internally by mutex lock.
> +	 */
> +	enum usb_role		current_role;

Can we just track the current role through dwc3 core instead of an
addition field in the glue?

BR,
Thinh

>  };
>  

Re: [PATCH v2 3/4] usb: dwc3: qcom: Facilitate autosuspend during host mode

[Thinh Nguyen]

On Tue, Jun 10, 2025, Krishna Kurapati wrote:
> When in host mode, it is intended that the controller goes to suspend
> state to save power and wait for interrupts from connected peripheral
> to wake it up. This is particularly used in cases where a HID or Audio
> device is connected. In such scenarios, the usb controller can enter
> auto suspend and resume action after getting interrupts from the
> connected device.
> 
> Allow autosuspend for and xhci device and allow userspace to decide
> whether to enable this functionality.
> 
> a) Register to usb-core notifications in set_role vendor callback to
> identify when root hubs are being created. Configure them to
> use_autosuspend.
> 
> b) Identify usb core notifications where the HCD is being added and
> enable autosuspend for that particular xhci device.
> 
> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> ---
>  drivers/usb/dwc3/dwc3-qcom.c | 62 ++++++++++++++++++++++++++++++++----
>  1 file changed, 56 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index d40b52e2ba01..17bbd5a06c08 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -95,6 +95,8 @@ struct dwc3_qcom {
>  	 * internally by mutex lock.
>  	 */
>  	enum usb_role		current_role;
> +
> +	struct notifier_block	xhci_nb;
>  };
>  
>  #define to_dwc3_qcom(d) container_of((d), struct dwc3_qcom, dwc)
> @@ -647,6 +649,39 @@ static int dwc3_qcom_setup_irq(struct dwc3_qcom *qcom, struct platform_device *p
>  	return 0;
>  }
>  
> +static int dwc3_xhci_event_notifier(struct notifier_block *nb,
> +				    unsigned long event, void *ptr)
> +{
> +	struct dwc3_qcom  *qcom	= container_of(nb, struct dwc3_qcom, xhci_nb);
> +	struct dwc3	  *dwc	= &qcom->dwc;
> +	struct usb_bus	  *ubus	= ptr;
> +	struct usb_hcd	  *hcd;
> +
> +	if (!dwc->xhci)
> +		goto done;
> +
> +	hcd = platform_get_drvdata(dwc->xhci);
> +	if (!hcd)
> +		goto done;
> +
> +	if (event != USB_BUS_ADD)
> +		goto done;
> +
> +	if (strcmp(dev_name(ubus->sysdev), dev_name(dwc->sysdev)) != 0)

Can this be false? If possible, I'd like to avoid these pointers and
strcmp here.

> +		goto done;
> +
> +	if (event == USB_BUS_ADD) {

This condition is redundant when you have the check a few lines above.

> +		/*
> +		 * Identify instant of creation of primary hcd and
> +		 * mark xhci as autosuspend capable at this point.
> +		 */
> +		pm_runtime_use_autosuspend(&dwc->xhci->dev);
> +	}
> +
> +done:
> +	return NOTIFY_DONE;
> +}
> +
>  static void dwc3_qcom_set_role_notifier(struct dwc3 *dwc, enum usb_role next_role)
>  {
>  	struct dwc3_qcom *qcom = to_dwc3_qcom(dwc);
> @@ -659,12 +694,22 @@ static void dwc3_qcom_set_role_notifier(struct dwc3 *dwc, enum usb_role next_rol
>  		return;
>  	}
>  
> -	if (qcom->current_role == USB_ROLE_DEVICE &&
> -	    next_role != USB_ROLE_DEVICE)
> +	if (qcom->current_role == USB_ROLE_NONE) {
> +		if (next_role == USB_ROLE_DEVICE) {
> +			dwc3_qcom_vbus_override_enable(qcom, true);
> +		} else if (next_role == USB_ROLE_HOST) {
> +			qcom->xhci_nb.notifier_call = dwc3_xhci_event_notifier;
> +			usb_register_notify(&qcom->xhci_nb);
> +		}
> +	} else if (qcom->current_role == USB_ROLE_DEVICE &&
> +		   next_role != USB_ROLE_DEVICE) {
>  		dwc3_qcom_vbus_override_enable(qcom, false);
> -	else if ((qcom->current_role != USB_ROLE_DEVICE) &&
> -		 (next_role == USB_ROLE_DEVICE))
> -		dwc3_qcom_vbus_override_enable(qcom, true);
> +	} else if (qcom->current_role == USB_ROLE_HOST) {
> +		if (next_role == USB_ROLE_NONE)
> +			usb_unregister_notify(&qcom->xhci_nb);
> +		else if (next_role == USB_ROLE_DEVICE)
> +			dwc3_qcom_vbus_override_enable(qcom, true);

We don't unregister the notifier when switching from host to device?

> +	}
>  
>  	pm_runtime_mark_last_busy(qcom->dev);
>  	pm_runtime_put_sync(qcom->dev);
> @@ -774,6 +819,8 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>  
>  	if (qcom->mode == USB_DR_MODE_HOST) {
>  		qcom->current_role = USB_ROLE_HOST;
> +		qcom->xhci_nb.notifier_call = dwc3_xhci_event_notifier;
> +		usb_register_notify(&qcom->xhci_nb);
>  	} else if (qcom->mode == USB_DR_MODE_PERIPHERAL) {
>  		qcom->current_role = USB_ROLE_DEVICE;
>  		dwc3_qcom_vbus_override_enable(qcom, true);
> @@ -794,7 +841,7 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>  	ret = dwc3_core_probe(&probe_data);
>  	if (ret)  {
>  		ret = dev_err_probe(dev, ret, "failed to register DWC3 Core\n");
> -		goto clk_disable;
> +		goto unregister_notify;
>  	}
>  
>  	ret = dwc3_qcom_interconnect_init(qcom);
> @@ -817,6 +864,9 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>  	dwc3_qcom_interconnect_exit(qcom);
>  remove_core:
>  	dwc3_core_remove(&qcom->dwc);
> +unregister_notify:
> +	if (qcom->mode == USB_DR_MODE_HOST)
> +		usb_unregister_notify(&qcom->xhci_nb);
>  clk_disable:
>  	clk_bulk_disable_unprepare(qcom->num_clocks, qcom->clks);
>  
> -- 
> 2.34.1
> 

BR,
Thinh

Re: [PATCH v2 1/4] usb: dwc3: core: Introduce glue callbacks for flattened implementations

[Krishna Kurapati]

On 6/24/2025 4:54 AM, Thinh Nguyen wrote:
> On Tue, Jun 10, 2025, Krishna Kurapati wrote:
>> In certain situations like role switching, the glue layers need to be
>> informed of these events, so that they can take any necessary action.
>> But in non-flattened implementations, the glue drivers have no data on
>> when the core driver probe was successful post invoking of_platform_
>> populate. Now that the core driver supports flattened implementations
>> as well, introduce vendor callbacks that can be passed on from glue to
>> core before invoking dwc3_core_probe.
>>
>> Introduce callbacks to notify glue layer of role_switch and run_stop
>> changes. These can be used by flattened implementation of Qualcomm
>> glue layer to generate connect/disconnect events in controller during
>> cable connect and run stop modifications by udc in device mode.
>>
>> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
>> ---
>>   drivers/usb/dwc3/core.c   |  1 +
>>   drivers/usb/dwc3/core.h   | 26 ++++++++++++++++++++++++++
>>   drivers/usb/dwc3/drd.c    |  1 +
>>   drivers/usb/dwc3/gadget.c |  1 +
>>   4 files changed, 29 insertions(+)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 2bc775a747f2..c01b15e3710f 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -2351,6 +2351,7 @@ static int dwc3_probe(struct platform_device *pdev)
>>   		return -ENOMEM;
>>   
>>   	dwc->dev = &pdev->dev;
>> +	dwc->glue_ops = NULL;
>>   
>>   	probe_data.dwc = dwc;
>>   	probe_data.res = res;
>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>> index d5b985fa12f4..a803884be4ed 100644
>> --- a/drivers/usb/dwc3/core.h
>> +++ b/drivers/usb/dwc3/core.h
>> @@ -992,6 +992,17 @@ struct dwc3_scratchpad_array {
>>   	__le64	dma_adr[DWC3_MAX_HIBER_SCRATCHBUFS];
>>   };
>>   
>> +/*
> 
> Let's keep consistent with the doc style /**
> 

ACK.

>> + * struct dwc3_glue_ops - The ops indicate the notifications that
>> + *				need to be passed on to glue layer
>> + * @notify_set_role: Notify glue of role switch notifications
>> + * @notify_run_stop: Notify run stop enable/disable information to glue
>> + */
>> +struct dwc3_glue_ops {
>> +	void	(*notify_set_role)(struct dwc3 *dwc, enum usb_role role);
>> +	void	(*notify_run_stop)(struct dwc3 *dwc, bool is_on);
> 
> Use pre_ or prep_ prefix instead of notify_ indicating callbacks for
>glue driver to perform updates before set_role or run_stop.

ACK. Will change it accordingly.

> 
>> +};
>> +
>>   /**
>>    * struct dwc3 - representation of our controller
>>    * @drd_work: workqueue used for role swapping
>> @@ -1168,6 +1179,7 @@ struct dwc3_scratchpad_array {
>>    * @wakeup_pending_funcs: Indicates whether any interface has requested for
>>    *			 function wakeup in bitmap format where bit position
>>    *			 represents interface_id.
>> + * @glue_ops: Vendor callbacks for flattened device implementations.
>>    */
>>   struct dwc3 {
>>   	struct work_struct	drd_work;
>> @@ -1400,6 +1412,8 @@ struct dwc3 {
>>   	struct dentry		*debug_root;
>>   	u32			gsbuscfg0_reqinfo;
>>   	u32			wakeup_pending_funcs;
>> +
>> +	struct dwc3_glue_ops	*glue_ops;
> 
> Use const, and move this closer on top. Perhaps below gadget_driver.
> 

ACK.

>>   };
>>   
>>   #define INCRX_BURST_MODE 0
>> @@ -1614,6 +1628,18 @@ void dwc3_event_buffers_cleanup(struct dwc3 *dwc);
>>   int dwc3_core_soft_reset(struct dwc3 *dwc);
>>   void dwc3_enable_susphy(struct dwc3 *dwc, bool enable);
>>   
>> +static inline void dwc3_notify_set_role(struct dwc3 *dwc, enum usb_role role)
>> +{
>> +	if (dwc->glue_ops && dwc->glue_ops->notify_set_role)
>> +		dwc->glue_ops->notify_set_role(dwc, role);
>> +}
>> +
>> +static inline void dwc3_notify_run_stop(struct dwc3 *dwc, bool is_on)
>> +{
>> +	if (dwc->glue_ops && dwc->glue_ops->notify_run_stop)
>> +		dwc->glue_ops->notify_run_stop(dwc, is_on);
>> +}
>> +
>>   #if IS_ENABLED(CONFIG_USB_DWC3_HOST) || IS_ENABLED(CONFIG_USB_DWC3_DUAL_ROLE)
>>   int dwc3_host_init(struct dwc3 *dwc);
>>   void dwc3_host_exit(struct dwc3 *dwc);
>> diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
>> index 7977860932b1..408551768a95 100644
>> --- a/drivers/usb/dwc3/drd.c
>> +++ b/drivers/usb/dwc3/drd.c
>> @@ -464,6 +464,7 @@ static int dwc3_usb_role_switch_set(struct usb_role_switch *sw,
>>   		break;
>>   	}
>>   
>> +	dwc3_notify_set_role(dwc, role);
> 
> This should be done in __dwc3_set_mode(). Perhaps right before setting
> PRTCAPDIR?
> 

Qualcomm glue driver needs ROLE (device /host and none) information. Set 
mode gives device and host mode information. So added set_role callback 
to get cable disconnect information.

>>   	dwc3_set_mode(dwc, mode);
>>   	return 0;
>>   }
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index 321361288935..73bed11bccaf 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -2641,6 +2641,7 @@ static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on)
>>   	if (saved_config)
>>   		dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>>   
>> +	dwc3_notify_run_stop(dwc, is_on);
> 
> This should be done right before writing to DCTL.run_stop.
> 

ACK.

>>   	reg = dwc3_readl(dwc->regs, DWC3_DCTL);
>>   	if (is_on) {
>>   		if (DWC3_VER_IS_WITHIN(DWC3, ANY, 187A)) {
>> -- 
>> 2.34.1
>>
> 

Thanks for the review Thinh.

Regards,
Krishna,

Re: [PATCH v2 2/4] usb: dwc3: qcom: Implement glue callbacks to facilitate runtime suspend

[Krishna Kurapati]

On 6/24/2025 5:02 AM, Thinh Nguyen wrote:
> On Tue, Jun 10, 2025, Krishna Kurapati wrote:
>> On Qualcomm DWC3 dual-role controllers, the conndone/disconnect events in
>> device mode are generated by controller when software writes to QSCRATCH
>> registers in Qualcomm Glue layer rather than the vbus line being routed to
>> dwc3 core IP for it to recognize and generate these events.
>>
>> UTMI_OTG_VBUS_VALID  bit of QSCRATCH_HS_PHY_CTRL register needs to be set
>> to generate a connection done event and to be cleared for the controller to
>> generate a disconnect event during cable removal. When the disconnect is
>> not generated upon cable removal, the "connected" flag of dwc3 is left
>> marked as "true" and it blocks suspend routines and for that to happen upon
>> cable removal, the cable disconnect notification coming in via set_role
>> call need to be provided to the Qualcomm glue layer as well.
>>
>> Currently, the way DWC3 core and Qualcomm legacy glue driver are designed,
>> there is no mechanism through which the DWC3 core can notify the Qualcomm
>> glue layer of any role changes which it receives via role switch. To
>> register these glue callbacks at probe time, for enabling core to notify
>> glue layer, the legacy Qualcomm driver has no way to find out when the
>> child driver probe was successful since it does not check for the same
>> during of_platform_populate.
>>
>> Hence implement the following glue callbacks for flattened Qualcomm glue
>> driver:
>>
>> 1. set_role: To pass role switching information from drd layer to glue.
>> This information is needed to identify NONE/DEVICE mode switch and modify
>> QSCRATCH to generate connect-done event on device mode entry and disconnect
>> event on cable removal in device mode.
>>
>> 2. run_stop: When booting up in device mode, if autouspend is enabled and
>> userspace doesn't write UDC on boot, controller enters autosuspend. After
>> this, if the userspace writes to UDC in the future, run_stop notifier is
>> required to enable UTMI_OTG_VBUS_VALID of QSCRATCH so that connect done
>> event is generated after run_stop(1) is done to finish enumeration.
>>
>> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
>> ---
>>   drivers/usb/dwc3/dwc3-qcom.c | 82 ++++++++++++++++++++++++++++++++----
>>   1 file changed, 73 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
>> index ca7e1c02773a..d40b52e2ba01 100644
>> --- a/drivers/usb/dwc3/dwc3-qcom.c
>> +++ b/drivers/usb/dwc3/dwc3-qcom.c
>> @@ -89,6 +89,12 @@ struct dwc3_qcom {
>>   	bool			pm_suspended;
>>   	struct icc_path		*icc_path_ddr;
>>   	struct icc_path		*icc_path_apps;
>> +
>> +	/*
>> +	 * Current role changes via usb_role_switch_set_role callback protected
>> +	 * internally by mutex lock.
>> +	 */
>> +	enum usb_role		current_role;
> 
> Can we just track the current role through dwc3 core instead of an
> addition field in the glue?
> 

Core caches only mode. We need ROLE NONE to identify cable connect. So 
adding that in glue to keep track.

Regards,
Krishna,

Re: [PATCH v2 3/4] usb: dwc3: qcom: Facilitate autosuspend during host mode

[Krishna Kurapati]

On 6/24/2025 5:29 AM, Thinh Nguyen wrote:
> On Tue, Jun 10, 2025, Krishna Kurapati wrote:
>> When in host mode, it is intended that the controller goes to suspend
>> state to save power and wait for interrupts from connected peripheral
>> to wake it up. This is particularly used in cases where a HID or Audio
>> device is connected. In such scenarios, the usb controller can enter
>> auto suspend and resume action after getting interrupts from the
>> connected device.
>>
>> Allow autosuspend for and xhci device and allow userspace to decide
>> whether to enable this functionality.
>>
>> a) Register to usb-core notifications in set_role vendor callback to
>> identify when root hubs are being created. Configure them to
>> use_autosuspend.
>>
>> b) Identify usb core notifications where the HCD is being added and
>> enable autosuspend for that particular xhci device.
>>
>> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>


[...]

>> +static int dwc3_xhci_event_notifier(struct notifier_block *nb,
>> +				    unsigned long event, void *ptr)
>> +{
>> +	struct dwc3_qcom  *qcom	= container_of(nb, struct dwc3_qcom, xhci_nb);
>> +	struct dwc3	  *dwc	= &qcom->dwc;
>> +	struct usb_bus	  *ubus	= ptr;
>> +	struct usb_hcd	  *hcd;
>> +
>> +	if (!dwc->xhci)
>> +		goto done;
>> +
>> +	hcd = platform_get_drvdata(dwc->xhci);
>> +	if (!hcd)
>> +		goto done;
>> +
>> +	if (event != USB_BUS_ADD)
>> +		goto done;
>> +
>> +	if (strcmp(dev_name(ubus->sysdev), dev_name(dwc->sysdev)) != 0)
> 
> Can this be false? If possible, I'd like to avoid these pointers and
> strcmp here.
> 

Needed this to identify if the dwc3 pointer corresponds to this glue or 
not. This can be false.

BTW, Dmitry suggested to just do "runtime_use_autosuspend" inside probe 
of xhci-plat.c and remove this logic. Hope that would be fine ?

>> +		goto done;
>> +
>> +	if (event == USB_BUS_ADD) {
> 
> This condition is redundant when you have the check a few lines above.
> 

ACK.

>> +		/*
>> +		 * Identify instant of creation of primary hcd and
>> +		 * mark xhci as autosuspend capable at this point.
>> +		 */
>> +		pm_runtime_use_autosuspend(&dwc->xhci->dev);
>> +	}
>> +
>> +done:
>> +	return NOTIFY_DONE;
>> +}
>> +
>>   static void dwc3_qcom_set_role_notifier(struct dwc3 *dwc, enum usb_role next_role)
>>   {
>>   	struct dwc3_qcom *qcom = to_dwc3_qcom(dwc);
>> @@ -659,12 +694,22 @@ static void dwc3_qcom_set_role_notifier(struct dwc3 *dwc, enum usb_role next_rol
>>   		return;
>>   	}
>>   
>> -	if (qcom->current_role == USB_ROLE_DEVICE &&
>> -	    next_role != USB_ROLE_DEVICE)
>> +	if (qcom->current_role == USB_ROLE_NONE) {
>> +		if (next_role == USB_ROLE_DEVICE) {
>> +			dwc3_qcom_vbus_override_enable(qcom, true);
>> +		} else if (next_role == USB_ROLE_HOST) {
>> +			qcom->xhci_nb.notifier_call = dwc3_xhci_event_notifier;
>> +			usb_register_notify(&qcom->xhci_nb);
>> +		}
>> +	} else if (qcom->current_role == USB_ROLE_DEVICE &&
>> +		   next_role != USB_ROLE_DEVICE) {
>>   		dwc3_qcom_vbus_override_enable(qcom, false);
>> -	else if ((qcom->current_role != USB_ROLE_DEVICE) &&
>> -		 (next_role == USB_ROLE_DEVICE))
>> -		dwc3_qcom_vbus_override_enable(qcom, true);
>> +	} else if (qcom->current_role == USB_ROLE_HOST) {
>> +		if (next_role == USB_ROLE_NONE)
>> +			usb_unregister_notify(&qcom->xhci_nb);
>> +		else if (next_role == USB_ROLE_DEVICE)
>> +			dwc3_qcom_vbus_override_enable(qcom, true);
> 
> We don't unregister the notifier when switching from host to device?
> 

ACK. My bad, missed it.

Thanks for the review.

Regards,
Krishna,

Re: [PATCH v2 2/4] usb: dwc3: qcom: Implement glue callbacks to facilitate runtime suspend

[Thinh Nguyen]

On Tue, Jun 24, 2025, Krishna Kurapati wrote:
> 
> 
> On 6/24/2025 5:02 AM, Thinh Nguyen wrote:
> > On Tue, Jun 10, 2025, Krishna Kurapati wrote:
> > > On Qualcomm DWC3 dual-role controllers, the conndone/disconnect events in
> > > device mode are generated by controller when software writes to QSCRATCH
> > > registers in Qualcomm Glue layer rather than the vbus line being routed to
> > > dwc3 core IP for it to recognize and generate these events.
> > > 
> > > UTMI_OTG_VBUS_VALID  bit of QSCRATCH_HS_PHY_CTRL register needs to be set
> > > to generate a connection done event and to be cleared for the controller to
> > > generate a disconnect event during cable removal. When the disconnect is
> > > not generated upon cable removal, the "connected" flag of dwc3 is left
> > > marked as "true" and it blocks suspend routines and for that to happen upon
> > > cable removal, the cable disconnect notification coming in via set_role
> > > call need to be provided to the Qualcomm glue layer as well.
> > > 
> > > Currently, the way DWC3 core and Qualcomm legacy glue driver are designed,
> > > there is no mechanism through which the DWC3 core can notify the Qualcomm
> > > glue layer of any role changes which it receives via role switch. To
> > > register these glue callbacks at probe time, for enabling core to notify
> > > glue layer, the legacy Qualcomm driver has no way to find out when the
> > > child driver probe was successful since it does not check for the same
> > > during of_platform_populate.
> > > 
> > > Hence implement the following glue callbacks for flattened Qualcomm glue
> > > driver:
> > > 
> > > 1. set_role: To pass role switching information from drd layer to glue.
> > > This information is needed to identify NONE/DEVICE mode switch and modify
> > > QSCRATCH to generate connect-done event on device mode entry and disconnect
> > > event on cable removal in device mode.
> > > 
> > > 2. run_stop: When booting up in device mode, if autouspend is enabled and
> > > userspace doesn't write UDC on boot, controller enters autosuspend. After
> > > this, if the userspace writes to UDC in the future, run_stop notifier is
> > > required to enable UTMI_OTG_VBUS_VALID of QSCRATCH so that connect done
> > > event is generated after run_stop(1) is done to finish enumeration.
> > > 
> > > Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> > > ---
> > >   drivers/usb/dwc3/dwc3-qcom.c | 82 ++++++++++++++++++++++++++++++++----
> > >   1 file changed, 73 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> > > index ca7e1c02773a..d40b52e2ba01 100644
> > > --- a/drivers/usb/dwc3/dwc3-qcom.c
> > > +++ b/drivers/usb/dwc3/dwc3-qcom.c
> > > @@ -89,6 +89,12 @@ struct dwc3_qcom {
> > >   	bool			pm_suspended;
> > >   	struct icc_path		*icc_path_ddr;
> > >   	struct icc_path		*icc_path_apps;
> > > +
> > > +	/*
> > > +	 * Current role changes via usb_role_switch_set_role callback protected
> > > +	 * internally by mutex lock.
> > > +	 */
> > > +	enum usb_role		current_role;
> > 
> > Can we just track the current role through dwc3 core instead of an
> > addition field in the glue?
> > 
> 
> Core caches only mode. We need ROLE NONE to identify cable connect. So
> adding that in glue to keep track.
> 

The controller is always either host or device, not somewhere in
between. You're using ROLE_NONE to indicate connection, which is a
separate state.

I feel this should be tracked separately for clarity. The dwc3 also
tracks the connection state, can we use that?

BR,
Thinh

Re: [PATCH v2 2/4] usb: dwc3: qcom: Implement glue callbacks to facilitate runtime suspend

[Krishna Kurapati]

On 7/12/2025 4:59 AM, Thinh Nguyen wrote:
> On Tue, Jun 24, 2025, Krishna Kurapati wrote:
>>
>>
>> On 6/24/2025 5:02 AM, Thinh Nguyen wrote:
>>> On Tue, Jun 10, 2025, Krishna Kurapati wrote:
>>>> On Qualcomm DWC3 dual-role controllers, the conndone/disconnect events in
>>>> device mode are generated by controller when software writes to QSCRATCH
>>>> registers in Qualcomm Glue layer rather than the vbus line being routed to
>>>> dwc3 core IP for it to recognize and generate these events.
>>>>
>>>> UTMI_OTG_VBUS_VALID  bit of QSCRATCH_HS_PHY_CTRL register needs to be set
>>>> to generate a connection done event and to be cleared for the controller to
>>>> generate a disconnect event during cable removal. When the disconnect is
>>>> not generated upon cable removal, the "connected" flag of dwc3 is left
>>>> marked as "true" and it blocks suspend routines and for that to happen upon
>>>> cable removal, the cable disconnect notification coming in via set_role
>>>> call need to be provided to the Qualcomm glue layer as well.
>>>>
>>>> Currently, the way DWC3 core and Qualcomm legacy glue driver are designed,
>>>> there is no mechanism through which the DWC3 core can notify the Qualcomm
>>>> glue layer of any role changes which it receives via role switch. To
>>>> register these glue callbacks at probe time, for enabling core to notify
>>>> glue layer, the legacy Qualcomm driver has no way to find out when the
>>>> child driver probe was successful since it does not check for the same
>>>> during of_platform_populate.
>>>>
>>>> Hence implement the following glue callbacks for flattened Qualcomm glue
>>>> driver:
>>>>
>>>> 1. set_role: To pass role switching information from drd layer to glue.
>>>> This information is needed to identify NONE/DEVICE mode switch and modify
>>>> QSCRATCH to generate connect-done event on device mode entry and disconnect
>>>> event on cable removal in device mode.
>>>>
>>>> 2. run_stop: When booting up in device mode, if autouspend is enabled and
>>>> userspace doesn't write UDC on boot, controller enters autosuspend. After
>>>> this, if the userspace writes to UDC in the future, run_stop notifier is
>>>> required to enable UTMI_OTG_VBUS_VALID of QSCRATCH so that connect done
>>>> event is generated after run_stop(1) is done to finish enumeration.
>>>>
>>>> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
>>>> ---
>>>>    drivers/usb/dwc3/dwc3-qcom.c | 82 ++++++++++++++++++++++++++++++++----
>>>>    1 file changed, 73 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
>>>> index ca7e1c02773a..d40b52e2ba01 100644
>>>> --- a/drivers/usb/dwc3/dwc3-qcom.c
>>>> +++ b/drivers/usb/dwc3/dwc3-qcom.c
>>>> @@ -89,6 +89,12 @@ struct dwc3_qcom {
>>>>    	bool			pm_suspended;
>>>>    	struct icc_path		*icc_path_ddr;
>>>>    	struct icc_path		*icc_path_apps;
>>>> +
>>>> +	/*
>>>> +	 * Current role changes via usb_role_switch_set_role callback protected
>>>> +	 * internally by mutex lock.
>>>> +	 */
>>>> +	enum usb_role		current_role;
>>>
>>> Can we just track the current role through dwc3 core instead of an
>>> addition field in the glue?
>>>
>>
>> Core caches only mode. We need ROLE NONE to identify cable connect. So
>> adding that in glue to keep track.
>>
> 
> The controller is always either host or device, not somewhere in
> between. You're using ROLE_NONE to indicate connection, which is a
> separate state.
> 

Yes, but there is no flag that indicates that in dwc structure today. 
Also since only dwc3-qcom needs it at the moment, I added that role info 
in glue layer.

> I feel this should be tracked separately for clarity. The dwc3 also
> tracks the connection state, can we use that?
> 

Are you referring to the "connected" flag in DWC structure ? I see that 
it indicates connection status only in gadget mode.

Regards,
Krishna,

Re: [PATCH v2 2/4] usb: dwc3: qcom: Implement glue callbacks to facilitate runtime suspend

[Thinh Nguyen]

On Sun, Jul 13, 2025, Krishna Kurapati wrote:
> 
> 
> On 7/12/2025 4:59 AM, Thinh Nguyen wrote:
> > On Tue, Jun 24, 2025, Krishna Kurapati wrote:
> > > 
> > > 
> > > On 6/24/2025 5:02 AM, Thinh Nguyen wrote:
> > > > On Tue, Jun 10, 2025, Krishna Kurapati wrote:
> > > > > On Qualcomm DWC3 dual-role controllers, the conndone/disconnect events in
> > > > > device mode are generated by controller when software writes to QSCRATCH
> > > > > registers in Qualcomm Glue layer rather than the vbus line being routed to
> > > > > dwc3 core IP for it to recognize and generate these events.
> > > > > 
> > > > > UTMI_OTG_VBUS_VALID  bit of QSCRATCH_HS_PHY_CTRL register needs to be set
> > > > > to generate a connection done event and to be cleared for the controller to
> > > > > generate a disconnect event during cable removal. When the disconnect is
> > > > > not generated upon cable removal, the "connected" flag of dwc3 is left
> > > > > marked as "true" and it blocks suspend routines and for that to happen upon
> > > > > cable removal, the cable disconnect notification coming in via set_role
> > > > > call need to be provided to the Qualcomm glue layer as well.
> > > > > 
> > > > > Currently, the way DWC3 core and Qualcomm legacy glue driver are designed,
> > > > > there is no mechanism through which the DWC3 core can notify the Qualcomm
> > > > > glue layer of any role changes which it receives via role switch. To
> > > > > register these glue callbacks at probe time, for enabling core to notify
> > > > > glue layer, the legacy Qualcomm driver has no way to find out when the
> > > > > child driver probe was successful since it does not check for the same
> > > > > during of_platform_populate.
> > > > > 
> > > > > Hence implement the following glue callbacks for flattened Qualcomm glue
> > > > > driver:
> > > > > 
> > > > > 1. set_role: To pass role switching information from drd layer to glue.
> > > > > This information is needed to identify NONE/DEVICE mode switch and modify
> > > > > QSCRATCH to generate connect-done event on device mode entry and disconnect
> > > > > event on cable removal in device mode.
> > > > > 
> > > > > 2. run_stop: When booting up in device mode, if autouspend is enabled and
> > > > > userspace doesn't write UDC on boot, controller enters autosuspend. After
> > > > > this, if the userspace writes to UDC in the future, run_stop notifier is
> > > > > required to enable UTMI_OTG_VBUS_VALID of QSCRATCH so that connect done
> > > > > event is generated after run_stop(1) is done to finish enumeration.
> > > > > 
> > > > > Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> > > > > ---
> > > > >    drivers/usb/dwc3/dwc3-qcom.c | 82 ++++++++++++++++++++++++++++++++----
> > > > >    1 file changed, 73 insertions(+), 9 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> > > > > index ca7e1c02773a..d40b52e2ba01 100644
> > > > > --- a/drivers/usb/dwc3/dwc3-qcom.c
> > > > > +++ b/drivers/usb/dwc3/dwc3-qcom.c
> > > > > @@ -89,6 +89,12 @@ struct dwc3_qcom {
> > > > >    	bool			pm_suspended;
> > > > >    	struct icc_path		*icc_path_ddr;
> > > > >    	struct icc_path		*icc_path_apps;
> > > > > +
> > > > > +	/*
> > > > > +	 * Current role changes via usb_role_switch_set_role callback protected
> > > > > +	 * internally by mutex lock.
> > > > > +	 */
> > > > > +	enum usb_role		current_role;
> > > > 
> > > > Can we just track the current role through dwc3 core instead of an
> > > > addition field in the glue?
> > > > 
> > > 
> > > Core caches only mode. We need ROLE NONE to identify cable connect. So
> > > adding that in glue to keep track.
> > > 
> > 
> > The controller is always either host or device, not somewhere in
> > between. You're using ROLE_NONE to indicate connection, which is a
> > separate state.
> > 
> 
> Yes, but there is no flag that indicates that in dwc structure today. Also
> since only dwc3-qcom needs it at the moment, I added that role info in glue
> layer.

How are you using ROLE NONE? Do you send a role-switch call to "none" to
indicate disconnect? Let's not do that. Currently the dwc3 driver would
switch back to the default mode if "none" is selected, but this is not
well defined and implementation specific. It can be no-op and would not
violate the interface.

The role-switch interface should only be used for role-switching and not
connection/disconnection.

> 
> > I feel this should be tracked separately for clarity. The dwc3 also
> > tracks the connection state, can we use that?
> > 
> 
> Are you referring to the "connected" flag in DWC structure ? I see that it
> indicates connection status only in gadget mode.
> 

Yes, that flag is only for gadget.

Can you provide more info of the setup? Is there a type-c controller or
phy that can detect attach/deattach? Or it only propagates to the usb
controller?

BR,
Thinh

Re: [PATCH v2 2/4] usb: dwc3: qcom: Implement glue callbacks to facilitate runtime suspend

[Krishna Kurapati]

On 7/30/2025 6:53 AM, Thinh Nguyen wrote:
> On Sun, Jul 13, 2025, Krishna Kurapati wrote:
>>
>>
>> On 7/12/2025 4:59 AM, Thinh Nguyen wrote:
>>> On Tue, Jun 24, 2025, Krishna Kurapati wrote:
>>>>
>>>>
>>>> On 6/24/2025 5:02 AM, Thinh Nguyen wrote:
>>>>> On Tue, Jun 10, 2025, Krishna Kurapati wrote:
>>>>>> On Qualcomm DWC3 dual-role controllers, the conndone/disconnect events in
>>>>>> device mode are generated by controller when software writes to QSCRATCH
>>>>>> registers in Qualcomm Glue layer rather than the vbus line being routed to
>>>>>> dwc3 core IP for it to recognize and generate these events.
>>>>>>
>>>>>> UTMI_OTG_VBUS_VALID  bit of QSCRATCH_HS_PHY_CTRL register needs to be set
>>>>>> to generate a connection done event and to be cleared for the controller to
>>>>>> generate a disconnect event during cable removal. When the disconnect is
>>>>>> not generated upon cable removal, the "connected" flag of dwc3 is left
>>>>>> marked as "true" and it blocks suspend routines and for that to happen upon
>>>>>> cable removal, the cable disconnect notification coming in via set_role
>>>>>> call need to be provided to the Qualcomm glue layer as well.
>>>>>>
>>>>>> Currently, the way DWC3 core and Qualcomm legacy glue driver are designed,
>>>>>> there is no mechanism through which the DWC3 core can notify the Qualcomm
>>>>>> glue layer of any role changes which it receives via role switch. To
>>>>>> register these glue callbacks at probe time, for enabling core to notify
>>>>>> glue layer, the legacy Qualcomm driver has no way to find out when the
>>>>>> child driver probe was successful since it does not check for the same
>>>>>> during of_platform_populate.
>>>>>>
>>>>>> Hence implement the following glue callbacks for flattened Qualcomm glue
>>>>>> driver:
>>>>>>
>>>>>> 1. set_role: To pass role switching information from drd layer to glue.
>>>>>> This information is needed to identify NONE/DEVICE mode switch and modify
>>>>>> QSCRATCH to generate connect-done event on device mode entry and disconnect
>>>>>> event on cable removal in device mode.
>>>>>>
>>>>>> 2. run_stop: When booting up in device mode, if autouspend is enabled and
>>>>>> userspace doesn't write UDC on boot, controller enters autosuspend. After
>>>>>> this, if the userspace writes to UDC in the future, run_stop notifier is
>>>>>> required to enable UTMI_OTG_VBUS_VALID of QSCRATCH so that connect done
>>>>>> event is generated after run_stop(1) is done to finish enumeration.
>>>>>>
>>>>>> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
>>>>>> ---
>>>>>>     drivers/usb/dwc3/dwc3-qcom.c | 82 ++++++++++++++++++++++++++++++++----
>>>>>>     1 file changed, 73 insertions(+), 9 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
>>>>>> index ca7e1c02773a..d40b52e2ba01 100644
>>>>>> --- a/drivers/usb/dwc3/dwc3-qcom.c
>>>>>> +++ b/drivers/usb/dwc3/dwc3-qcom.c
>>>>>> @@ -89,6 +89,12 @@ struct dwc3_qcom {
>>>>>>     	bool			pm_suspended;
>>>>>>     	struct icc_path		*icc_path_ddr;
>>>>>>     	struct icc_path		*icc_path_apps;
>>>>>> +
>>>>>> +	/*
>>>>>> +	 * Current role changes via usb_role_switch_set_role callback protected
>>>>>> +	 * internally by mutex lock.
>>>>>> +	 */
>>>>>> +	enum usb_role		current_role;
>>>>>
>>>>> Can we just track the current role through dwc3 core instead of an
>>>>> addition field in the glue?
>>>>>
>>>>
>>>> Core caches only mode. We need ROLE NONE to identify cable connect. So
>>>> adding that in glue to keep track.
>>>>
>>>
>>> The controller is always either host or device, not somewhere in
>>> between. You're using ROLE_NONE to indicate connection, which is a
>>> separate state.
>>>
>>
>> Yes, but there is no flag that indicates that in dwc structure today. Also
>> since only dwc3-qcom needs it at the moment, I added that role info in glue
>> layer.
> 
> How are you using ROLE NONE? Do you send a role-switch call to "none" to
> indicate disconnect? Let's not do that. Currently the dwc3 driver would
> switch back to the default mode if "none" is selected, but this is not
> well defined and implementation specific. It can be no-op and would not
> violate the interface.
> 
> The role-switch interface should only be used for role-switching and not
> connection/disconnection.
> 
>>
>>> I feel this should be tracked separately for clarity. The dwc3 also
>>> tracks the connection state, can we use that?
>>>
>>
>> Are you referring to the "connected" flag in DWC structure ? I see that it
>> indicates connection status only in gadget mode.
>>
> 
> Yes, that flag is only for gadget.
> 
> Can you provide more info of the setup? Is there a type-c controller or
> phy that can detect attach/deattach? Or it only propagates to the usb
> controller?

My response didn't show up on lore since the mail client I used before 
sent the message in HTML format. So resending my response again.

Hi Thinh,

  Yes this is for cases where role switching is present (either with a 
Type-C controller, USB-conn-gpio, or a glink based role-switch).

  Actually the requirement is as follows:
  1. When in device mode, if we receive a cable disconnect, we need to 
clear OTG_VBUS_VALID bit of QSCRATCH register in glue address space.
  2. When cable is connected in device mode, we need to set the 
OTG_VBUS_VALID bit of QSCRATCH register in glue address space.
  3. When the runstop is set back after a suspend rotuine, then we need 
to set  OTG_VBUS_VALID bit of QSCRATCH register in glueaddress space.

  To take care of all the 3 scenarios above, the set_role and run_stop 
notifiers have been added.

  The role info propagates only from core to glue. It is not sent to the 
phy.

Regards,
Krishna,

Re: [PATCH v2 2/4] usb: dwc3: qcom: Implement glue callbacks to facilitate runtime suspend

[Thinh Nguyen]

On Wed, Jul 30, 2025, Krishna Kurapati wrote:
> 
> 
> On 7/30/2025 6:53 AM, Thinh Nguyen wrote:
> > On Sun, Jul 13, 2025, Krishna Kurapati wrote:
> > > 
> > > 
> > > On 7/12/2025 4:59 AM, Thinh Nguyen wrote:
> > > > On Tue, Jun 24, 2025, Krishna Kurapati wrote:
> > > > > 
> > > > > 
> > > > > On 6/24/2025 5:02 AM, Thinh Nguyen wrote:
> > > > > > On Tue, Jun 10, 2025, Krishna Kurapati wrote:
> > > > > > > On Qualcomm DWC3 dual-role controllers, the conndone/disconnect events in
> > > > > > > device mode are generated by controller when software writes to QSCRATCH
> > > > > > > registers in Qualcomm Glue layer rather than the vbus line being routed to
> > > > > > > dwc3 core IP for it to recognize and generate these events.
> > > > > > > 
> > > > > > > UTMI_OTG_VBUS_VALID  bit of QSCRATCH_HS_PHY_CTRL register needs to be set
> > > > > > > to generate a connection done event and to be cleared for the controller to
> > > > > > > generate a disconnect event during cable removal. When the disconnect is
> > > > > > > not generated upon cable removal, the "connected" flag of dwc3 is left
> > > > > > > marked as "true" and it blocks suspend routines and for that to happen upon
> > > > > > > cable removal, the cable disconnect notification coming in via set_role
> > > > > > > call need to be provided to the Qualcomm glue layer as well.
> > > > > > > 
> > > > > > > Currently, the way DWC3 core and Qualcomm legacy glue driver are designed,
> > > > > > > there is no mechanism through which the DWC3 core can notify the Qualcomm
> > > > > > > glue layer of any role changes which it receives via role switch. To
> > > > > > > register these glue callbacks at probe time, for enabling core to notify
> > > > > > > glue layer, the legacy Qualcomm driver has no way to find out when the
> > > > > > > child driver probe was successful since it does not check for the same
> > > > > > > during of_platform_populate.
> > > > > > > 
> > > > > > > Hence implement the following glue callbacks for flattened Qualcomm glue
> > > > > > > driver:
> > > > > > > 
> > > > > > > 1. set_role: To pass role switching information from drd layer to glue.
> > > > > > > This information is needed to identify NONE/DEVICE mode switch and modify
> > > > > > > QSCRATCH to generate connect-done event on device mode entry and disconnect
> > > > > > > event on cable removal in device mode.
> > > > > > > 
> > > > > > > 2. run_stop: When booting up in device mode, if autouspend is enabled and
> > > > > > > userspace doesn't write UDC on boot, controller enters autosuspend. After
> > > > > > > this, if the userspace writes to UDC in the future, run_stop notifier is
> > > > > > > required to enable UTMI_OTG_VBUS_VALID of QSCRATCH so that connect done
> > > > > > > event is generated after run_stop(1) is done to finish enumeration.
> > > > > > > 
> > > > > > > Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> > > > > > > ---
> > > > > > >     drivers/usb/dwc3/dwc3-qcom.c | 82 ++++++++++++++++++++++++++++++++----
> > > > > > >     1 file changed, 73 insertions(+), 9 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> > > > > > > index ca7e1c02773a..d40b52e2ba01 100644
> > > > > > > --- a/drivers/usb/dwc3/dwc3-qcom.c
> > > > > > > +++ b/drivers/usb/dwc3/dwc3-qcom.c
> > > > > > > @@ -89,6 +89,12 @@ struct dwc3_qcom {
> > > > > > >     	bool			pm_suspended;
> > > > > > >     	struct icc_path		*icc_path_ddr;
> > > > > > >     	struct icc_path		*icc_path_apps;
> > > > > > > +
> > > > > > > +	/*
> > > > > > > +	 * Current role changes via usb_role_switch_set_role callback protected
> > > > > > > +	 * internally by mutex lock.
> > > > > > > +	 */
> > > > > > > +	enum usb_role		current_role;
> > > > > > 
> > > > > > Can we just track the current role through dwc3 core instead of an
> > > > > > addition field in the glue?
> > > > > > 
> > > > > 
> > > > > Core caches only mode. We need ROLE NONE to identify cable connect. So
> > > > > adding that in glue to keep track.
> > > > > 
> > > > 
> > > > The controller is always either host or device, not somewhere in
> > > > between. You're using ROLE_NONE to indicate connection, which is a
> > > > separate state.
> > > > 
> > > 
> > > Yes, but there is no flag that indicates that in dwc structure today. Also
> > > since only dwc3-qcom needs it at the moment, I added that role info in glue
> > > layer.
> > 
> > How are you using ROLE NONE? Do you send a role-switch call to "none" to
> > indicate disconnect? Let's not do that. Currently the dwc3 driver would
> > switch back to the default mode if "none" is selected, but this is not
> > well defined and implementation specific. It can be no-op and would not
> > violate the interface.
> > 
> > The role-switch interface should only be used for role-switching and not
> > connection/disconnection.
> > 
> > > 
> > > > I feel this should be tracked separately for clarity. The dwc3 also
> > > > tracks the connection state, can we use that?
> > > > 
> > > 
> > > Are you referring to the "connected" flag in DWC structure ? I see that it
> > > indicates connection status only in gadget mode.
> > > 
> > 
> > Yes, that flag is only for gadget.
> > 
> > Can you provide more info of the setup? Is there a type-c controller or
> > phy that can detect attach/deattach? Or it only propagates to the usb
> > controller?
> 
> My response didn't show up on lore since the mail client I used before sent
> the message in HTML format. So resending my response again.
> 
> Hi Thinh,
> 
>  Yes this is for cases where role switching is present (either with a Type-C
> controller, USB-conn-gpio, or a glink based role-switch).
> 
>  Actually the requirement is as follows:
>  1. When in device mode, if we receive a cable disconnect, we need to clear
> OTG_VBUS_VALID bit of QSCRATCH register in glue address space.
>  2. When cable is connected in device mode, we need to set the
> OTG_VBUS_VALID bit of QSCRATCH register in glue address space.
>  3. When the runstop is set back after a suspend rotuine, then we need to
> set  OTG_VBUS_VALID bit of QSCRATCH register in glueaddress space.
> 
>  To take care of all the 3 scenarios above, the set_role and run_stop
> notifiers have been added.
> 
>  The role info propagates only from core to glue. It is not sent to the phy.
> 

When does ROLE NONE occur? Did you have the type-c driver set the role
switch to "none" indicate disconnect?

The vbus-valid is only for gadget, and you only care about the
OTG_VBUS_VALID right? Can we just use the dwc3->connected flag? Just
make sure that it's cleared on role-switch, which should be the case
because we always perform soft-disconnect on gadget unbind, and make
sure to set vbus-valid on pullup or gadget binding. Is there some
scenarios that dwc->connected does not cover?

Thanks,
Thinh

Re: [PATCH v2 2/4] usb: dwc3: qcom: Implement glue callbacks to facilitate runtime suspend

[Krishna Kurapati]

On 8/1/2025 6:31 AM, Thinh Nguyen wrote:
> On Wed, Jul 30, 2025, Krishna Kurapati wrote:
>>
>>
>> On 7/30/2025 6:53 AM, Thinh Nguyen wrote:
>>> On Sun, Jul 13, 2025, Krishna Kurapati wrote:
>>>>
>>>>
>>>> On 7/12/2025 4:59 AM, Thinh Nguyen wrote:
>>>>> On Tue, Jun 24, 2025, Krishna Kurapati wrote:
>>>>>>
>>>>>>
>>>>>> On 6/24/2025 5:02 AM, Thinh Nguyen wrote:
>>>>>>> On Tue, Jun 10, 2025, Krishna Kurapati wrote:
>>>>>>>> On Qualcomm DWC3 dual-role controllers, the conndone/disconnect events in
>>>>>>>> device mode are generated by controller when software writes to QSCRATCH
>>>>>>>> registers in Qualcomm Glue layer rather than the vbus line being routed to
>>>>>>>> dwc3 core IP for it to recognize and generate these events.
>>>>>>>>
>>>>>>>> UTMI_OTG_VBUS_VALID  bit of QSCRATCH_HS_PHY_CTRL register needs to be set
>>>>>>>> to generate a connection done event and to be cleared for the controller to
>>>>>>>> generate a disconnect event during cable removal. When the disconnect is
>>>>>>>> not generated upon cable removal, the "connected" flag of dwc3 is left
>>>>>>>> marked as "true" and it blocks suspend routines and for that to happen upon
>>>>>>>> cable removal, the cable disconnect notification coming in via set_role
>>>>>>>> call need to be provided to the Qualcomm glue layer as well.
>>>>>>>>
>>>>>>>> Currently, the way DWC3 core and Qualcomm legacy glue driver are designed,
>>>>>>>> there is no mechanism through which the DWC3 core can notify the Qualcomm
>>>>>>>> glue layer of any role changes which it receives via role switch. To
>>>>>>>> register these glue callbacks at probe time, for enabling core to notify
>>>>>>>> glue layer, the legacy Qualcomm driver has no way to find out when the
>>>>>>>> child driver probe was successful since it does not check for the same
>>>>>>>> during of_platform_populate.
>>>>>>>>
>>>>>>>> Hence implement the following glue callbacks for flattened Qualcomm glue
>>>>>>>> driver:
>>>>>>>>
>>>>>>>> 1. set_role: To pass role switching information from drd layer to glue.
>>>>>>>> This information is needed to identify NONE/DEVICE mode switch and modify
>>>>>>>> QSCRATCH to generate connect-done event on device mode entry and disconnect
>>>>>>>> event on cable removal in device mode.
>>>>>>>>
>>>>>>>> 2. run_stop: When booting up in device mode, if autouspend is enabled and
>>>>>>>> userspace doesn't write UDC on boot, controller enters autosuspend. After
>>>>>>>> this, if the userspace writes to UDC in the future, run_stop notifier is
>>>>>>>> required to enable UTMI_OTG_VBUS_VALID of QSCRATCH so that connect done
>>>>>>>> event is generated after run_stop(1) is done to finish enumeration.
>>>>>>>>
>>>>>>>> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
>>>>>>>> ---
>>>>>>>>      drivers/usb/dwc3/dwc3-qcom.c | 82 ++++++++++++++++++++++++++++++++----
>>>>>>>>      1 file changed, 73 insertions(+), 9 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
>>>>>>>> index ca7e1c02773a..d40b52e2ba01 100644
>>>>>>>> --- a/drivers/usb/dwc3/dwc3-qcom.c
>>>>>>>> +++ b/drivers/usb/dwc3/dwc3-qcom.c
>>>>>>>> @@ -89,6 +89,12 @@ struct dwc3_qcom {
>>>>>>>>      	bool			pm_suspended;
>>>>>>>>      	struct icc_path		*icc_path_ddr;
>>>>>>>>      	struct icc_path		*icc_path_apps;
>>>>>>>> +
>>>>>>>> +	/*
>>>>>>>> +	 * Current role changes via usb_role_switch_set_role callback protected
>>>>>>>> +	 * internally by mutex lock.
>>>>>>>> +	 */
>>>>>>>> +	enum usb_role		current_role;
>>>>>>>
>>>>>>> Can we just track the current role through dwc3 core instead of an
>>>>>>> addition field in the glue?
>>>>>>>
>>>>>>
>>>>>> Core caches only mode. We need ROLE NONE to identify cable connect. So
>>>>>> adding that in glue to keep track.
>>>>>>
>>>>>
>>>>> The controller is always either host or device, not somewhere in
>>>>> between. You're using ROLE_NONE to indicate connection, which is a
>>>>> separate state.
>>>>>
>>>>
>>>> Yes, but there is no flag that indicates that in dwc structure today. Also
>>>> since only dwc3-qcom needs it at the moment, I added that role info in glue
>>>> layer.
>>>
>>> How are you using ROLE NONE? Do you send a role-switch call to "none" to
>>> indicate disconnect? Let's not do that. Currently the dwc3 driver would
>>> switch back to the default mode if "none" is selected, but this is not
>>> well defined and implementation specific. It can be no-op and would not
>>> violate the interface.
>>>
>>> The role-switch interface should only be used for role-switching and not
>>> connection/disconnection.
>>>
>>>>
>>>>> I feel this should be tracked separately for clarity. The dwc3 also
>>>>> tracks the connection state, can we use that?
>>>>>
>>>>
>>>> Are you referring to the "connected" flag in DWC structure ? I see that it
>>>> indicates connection status only in gadget mode.
>>>>
>>>
>>> Yes, that flag is only for gadget.
>>>
>>> Can you provide more info of the setup? Is there a type-c controller or
>>> phy that can detect attach/deattach? Or it only propagates to the usb
>>> controller?
>>
>> My response didn't show up on lore since the mail client I used before sent
>> the message in HTML format. So resending my response again.
>>
>> Hi Thinh,
>>
>>   Yes this is for cases where role switching is present (either with a Type-C
>> controller, USB-conn-gpio, or a glink based role-switch).
>>
>>   Actually the requirement is as follows:
>>   1. When in device mode, if we receive a cable disconnect, we need to clear
>> OTG_VBUS_VALID bit of QSCRATCH register in glue address space.
>>   2. When cable is connected in device mode, we need to set the
>> OTG_VBUS_VALID bit of QSCRATCH register in glue address space.
>>   3. When the runstop is set back after a suspend rotuine, then we need to
>> set  OTG_VBUS_VALID bit of QSCRATCH register in glueaddress space.
>>
>>   To take care of all the 3 scenarios above, the set_role and run_stop
>> notifiers have been added.
>>
>>   The role info propagates only from core to glue. It is not sent to the phy.
>>
> 
> When does ROLE NONE occur? Did you have the type-c driver set the role
> switch to "none" indicate disconnect?
> 
> The vbus-valid is only for gadget, and you only care about the
> OTG_VBUS_VALID right? Can we just use the dwc3->connected flag? Just
> make sure that it's cleared on role-switch, which should be the case
> because we always perform soft-disconnect on gadget unbind, and make
> sure to set vbus-valid on pullup or gadget binding. Is there some
> scenarios that dwc->connected does not cover?
> 

Hi Thinh,

  In case of just a cable disconnect in device mode (and default dr mode 
is peripheral only), there would be no role switch. In that scenario, 
connected flag would stay "true" even after removing cable. In that 
case, we can generate disconnect interrupt only by clearing this 
VBUS_VALID bit and inturn the suspend will succeed. So wanted to use 
notification from set_role which would cover all cases:
1. cable disconnect/cable connect
2. Role switch from host->device and device->host

Regards,
Krishna,

Re: [PATCH v2 2/4] usb: dwc3: qcom: Implement glue callbacks to facilitate runtime suspend

[Thinh Nguyen]

On Tue, Aug 05, 2025, Krishna Kurapati wrote:
> 
> 
> On 8/1/2025 6:31 AM, Thinh Nguyen wrote:
> > On Wed, Jul 30, 2025, Krishna Kurapati wrote:
> > > 
> > > 
> > > On 7/30/2025 6:53 AM, Thinh Nguyen wrote:
> > > > On Sun, Jul 13, 2025, Krishna Kurapati wrote:
> > > > > 
> > > > > 
> > > > > On 7/12/2025 4:59 AM, Thinh Nguyen wrote:
> > > > > > On Tue, Jun 24, 2025, Krishna Kurapati wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > On 6/24/2025 5:02 AM, Thinh Nguyen wrote:
> > > > > > > > On Tue, Jun 10, 2025, Krishna Kurapati wrote:
> > > > > > > > > On Qualcomm DWC3 dual-role controllers, the conndone/disconnect events in
> > > > > > > > > device mode are generated by controller when software writes to QSCRATCH
> > > > > > > > > registers in Qualcomm Glue layer rather than the vbus line being routed to
> > > > > > > > > dwc3 core IP for it to recognize and generate these events.
> > > > > > > > > 
> > > > > > > > > UTMI_OTG_VBUS_VALID  bit of QSCRATCH_HS_PHY_CTRL register needs to be set
> > > > > > > > > to generate a connection done event and to be cleared for the controller to
> > > > > > > > > generate a disconnect event during cable removal. When the disconnect is
> > > > > > > > > not generated upon cable removal, the "connected" flag of dwc3 is left
> > > > > > > > > marked as "true" and it blocks suspend routines and for that to happen upon
> > > > > > > > > cable removal, the cable disconnect notification coming in via set_role
> > > > > > > > > call need to be provided to the Qualcomm glue layer as well.
> > > > > > > > > 
> > > > > > > > > Currently, the way DWC3 core and Qualcomm legacy glue driver are designed,
> > > > > > > > > there is no mechanism through which the DWC3 core can notify the Qualcomm
> > > > > > > > > glue layer of any role changes which it receives via role switch. To
> > > > > > > > > register these glue callbacks at probe time, for enabling core to notify
> > > > > > > > > glue layer, the legacy Qualcomm driver has no way to find out when the
> > > > > > > > > child driver probe was successful since it does not check for the same
> > > > > > > > > during of_platform_populate.
> > > > > > > > > 
> > > > > > > > > Hence implement the following glue callbacks for flattened Qualcomm glue
> > > > > > > > > driver:
> > > > > > > > > 
> > > > > > > > > 1. set_role: To pass role switching information from drd layer to glue.
> > > > > > > > > This information is needed to identify NONE/DEVICE mode switch and modify
> > > > > > > > > QSCRATCH to generate connect-done event on device mode entry and disconnect
> > > > > > > > > event on cable removal in device mode.
> > > > > > > > > 
> > > > > > > > > 2. run_stop: When booting up in device mode, if autouspend is enabled and
> > > > > > > > > userspace doesn't write UDC on boot, controller enters autosuspend. After
> > > > > > > > > this, if the userspace writes to UDC in the future, run_stop notifier is
> > > > > > > > > required to enable UTMI_OTG_VBUS_VALID of QSCRATCH so that connect done
> > > > > > > > > event is generated after run_stop(1) is done to finish enumeration.
> > > > > > > > > 
> > > > > > > > > Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> > > > > > > > > ---
> > > > > > > > >      drivers/usb/dwc3/dwc3-qcom.c | 82 ++++++++++++++++++++++++++++++++----
> > > > > > > > >      1 file changed, 73 insertions(+), 9 deletions(-)
> > > > > > > > > 
> > > > > > > > > diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> > > > > > > > > index ca7e1c02773a..d40b52e2ba01 100644
> > > > > > > > > --- a/drivers/usb/dwc3/dwc3-qcom.c
> > > > > > > > > +++ b/drivers/usb/dwc3/dwc3-qcom.c
> > > > > > > > > @@ -89,6 +89,12 @@ struct dwc3_qcom {
> > > > > > > > >      	bool			pm_suspended;
> > > > > > > > >      	struct icc_path		*icc_path_ddr;
> > > > > > > > >      	struct icc_path		*icc_path_apps;
> > > > > > > > > +
> > > > > > > > > +	/*
> > > > > > > > > +	 * Current role changes via usb_role_switch_set_role callback protected
> > > > > > > > > +	 * internally by mutex lock.
> > > > > > > > > +	 */
> > > > > > > > > +	enum usb_role		current_role;
> > > > > > > > 
> > > > > > > > Can we just track the current role through dwc3 core instead of an
> > > > > > > > addition field in the glue?
> > > > > > > > 
> > > > > > > 
> > > > > > > Core caches only mode. We need ROLE NONE to identify cable connect. So
> > > > > > > adding that in glue to keep track.
> > > > > > > 
> > > > > > 
> > > > > > The controller is always either host or device, not somewhere in
> > > > > > between. You're using ROLE_NONE to indicate connection, which is a
> > > > > > separate state.
> > > > > > 
> > > > > 
> > > > > Yes, but there is no flag that indicates that in dwc structure today. Also
> > > > > since only dwc3-qcom needs it at the moment, I added that role info in glue
> > > > > layer.
> > > > 
> > > > How are you using ROLE NONE? Do you send a role-switch call to "none" to
> > > > indicate disconnect? Let's not do that. Currently the dwc3 driver would
> > > > switch back to the default mode if "none" is selected, but this is not
> > > > well defined and implementation specific. It can be no-op and would not
> > > > violate the interface.
> > > > 
> > > > The role-switch interface should only be used for role-switching and not
> > > > connection/disconnection.
> > > > 
> > > > > 
> > > > > > I feel this should be tracked separately for clarity. The dwc3 also
> > > > > > tracks the connection state, can we use that?
> > > > > > 
> > > > > 
> > > > > Are you referring to the "connected" flag in DWC structure ? I see that it
> > > > > indicates connection status only in gadget mode.
> > > > > 
> > > > 
> > > > Yes, that flag is only for gadget.
> > > > 
> > > > Can you provide more info of the setup? Is there a type-c controller or
> > > > phy that can detect attach/deattach? Or it only propagates to the usb
> > > > controller?
> > > 
> > > My response didn't show up on lore since the mail client I used before sent
> > > the message in HTML format. So resending my response again.
> > > 
> > > Hi Thinh,
> > > 
> > >   Yes this is for cases where role switching is present (either with a Type-C
> > > controller, USB-conn-gpio, or a glink based role-switch).
> > > 
> > >   Actually the requirement is as follows:
> > >   1. When in device mode, if we receive a cable disconnect, we need to clear
> > > OTG_VBUS_VALID bit of QSCRATCH register in glue address space.
> > >   2. When cable is connected in device mode, we need to set the
> > > OTG_VBUS_VALID bit of QSCRATCH register in glue address space.
> > >   3. When the runstop is set back after a suspend rotuine, then we need to
> > > set  OTG_VBUS_VALID bit of QSCRATCH register in glueaddress space.
> > > 
> > >   To take care of all the 3 scenarios above, the set_role and run_stop
> > > notifiers have been added.
> > > 
> > >   The role info propagates only from core to glue. It is not sent to the phy.
> > > 
> > 
> > When does ROLE NONE occur? Did you have the type-c driver set the role
> > switch to "none" indicate disconnect?
> > 
> > The vbus-valid is only for gadget, and you only care about the
> > OTG_VBUS_VALID right? Can we just use the dwc3->connected flag? Just
> > make sure that it's cleared on role-switch, which should be the case
> > because we always perform soft-disconnect on gadget unbind, and make
> > sure to set vbus-valid on pullup or gadget binding. Is there some
> > scenarios that dwc->connected does not cover?
> > 
> 
> Hi Thinh,
> 
>  In case of just a cable disconnect in device mode (and default dr mode is
> peripheral only), there would be no role switch. In that scenario, connected
> flag would stay "true" even after removing cable. In that case, we can
> generate disconnect interrupt only by clearing this VBUS_VALID bit and
> inturn the suspend will succeed. So wanted to use notification from set_role
> which would cover all cases:
> 1. cable disconnect/cable connect
> 2. Role switch from host->device and device->host
> 

Ok. Thanks for the explanation. How everything tied together seems
awkward: The connector does a role-switch to "none" to trigger a
role-switch in dwc3, which then triggers a callback to dwc3-qcom to
clear vbus_valid, which then allows the controller to see a disconnect
event in dwc3. Is that right?

The connector driver should have a separate interface for
attach/deattach in addition to role-switch. But I don't think type-c
controller, USB-conn-gpio, and glink share the same way to handle
connection/disconnection.

Let's keep what you proposed, and keep in mind that ROLE NONE is for
selecting the default mode and not necessarily mean disconnection.

BR,
Thinh

Re: [PATCH v2 2/4] usb: dwc3: qcom: Implement glue callbacks to facilitate runtime suspend

[Krishna Kurapati]

On 8/6/2025 4:48 AM, Thinh Nguyen wrote:

[...]

>>>>>>>>>> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
>>>>>>>>>> index ca7e1c02773a..d40b52e2ba01 100644
>>>>>>>>>> --- a/drivers/usb/dwc3/dwc3-qcom.c
>>>>>>>>>> +++ b/drivers/usb/dwc3/dwc3-qcom.c
>>>>>>>>>> @@ -89,6 +89,12 @@ struct dwc3_qcom {
>>>>>>>>>>       	bool			pm_suspended;
>>>>>>>>>>       	struct icc_path		*icc_path_ddr;
>>>>>>>>>>       	struct icc_path		*icc_path_apps;
>>>>>>>>>> +
>>>>>>>>>> +	/*
>>>>>>>>>> +	 * Current role changes via usb_role_switch_set_role callback protected
>>>>>>>>>> +	 * internally by mutex lock.
>>>>>>>>>> +	 */
>>>>>>>>>> +	enum usb_role		current_role;
>>>>>>>>>
>>>>>>>>> Can we just track the current role through dwc3 core instead of an
>>>>>>>>> addition field in the glue?
>>>>>>>>>
>>>>>>>>
>>>>>>>> Core caches only mode. We need ROLE NONE to identify cable connect. So
>>>>>>>> adding that in glue to keep track.
>>>>>>>>
>>>>>>>
>>>>>>> The controller is always either host or device, not somewhere in
>>>>>>> between. You're using ROLE_NONE to indicate connection, which is a
>>>>>>> separate state.
>>>>>>>
>>>>>>
>>>>>> Yes, but there is no flag that indicates that in dwc structure today. Also
>>>>>> since only dwc3-qcom needs it at the moment, I added that role info in glue
>>>>>> layer.
>>>>>
>>>>> How are you using ROLE NONE? Do you send a role-switch call to "none" to
>>>>> indicate disconnect? Let's not do that. Currently the dwc3 driver would
>>>>> switch back to the default mode if "none" is selected, but this is not
>>>>> well defined and implementation specific. It can be no-op and would not
>>>>> violate the interface.
>>>>>
>>>>> The role-switch interface should only be used for role-switching and not
>>>>> connection/disconnection.
>>>>>
>>>>>>
>>>>>>> I feel this should be tracked separately for clarity. The dwc3 also
>>>>>>> tracks the connection state, can we use that?
>>>>>>>
>>>>>>
>>>>>> Are you referring to the "connected" flag in DWC structure ? I see that it
>>>>>> indicates connection status only in gadget mode.
>>>>>>
>>>>>
>>>>> Yes, that flag is only for gadget.
>>>>>
>>>>> Can you provide more info of the setup? Is there a type-c controller or
>>>>> phy that can detect attach/deattach? Or it only propagates to the usb
>>>>> controller?
>>>>
>>>> My response didn't show up on lore since the mail client I used before sent
>>>> the message in HTML format. So resending my response again.
>>>>
>>>> Hi Thinh,
>>>>
>>>>    Yes this is for cases where role switching is present (either with a Type-C
>>>> controller, USB-conn-gpio, or a glink based role-switch).
>>>>
>>>>    Actually the requirement is as follows:
>>>>    1. When in device mode, if we receive a cable disconnect, we need to clear
>>>> OTG_VBUS_VALID bit of QSCRATCH register in glue address space.
>>>>    2. When cable is connected in device mode, we need to set the
>>>> OTG_VBUS_VALID bit of QSCRATCH register in glue address space.
>>>>    3. When the runstop is set back after a suspend rotuine, then we need to
>>>> set  OTG_VBUS_VALID bit of QSCRATCH register in glueaddress space.
>>>>
>>>>    To take care of all the 3 scenarios above, the set_role and run_stop
>>>> notifiers have been added.
>>>>
>>>>    The role info propagates only from core to glue. It is not sent to the phy.
>>>>
>>>
>>> When does ROLE NONE occur? Did you have the type-c driver set the role
>>> switch to "none" indicate disconnect?
>>>
>>> The vbus-valid is only for gadget, and you only care about the
>>> OTG_VBUS_VALID right? Can we just use the dwc3->connected flag? Just
>>> make sure that it's cleared on role-switch, which should be the case
>>> because we always perform soft-disconnect on gadget unbind, and make
>>> sure to set vbus-valid on pullup or gadget binding. Is there some
>>> scenarios that dwc->connected does not cover?
>>>
>>
>> Hi Thinh,
>>
>>   In case of just a cable disconnect in device mode (and default dr mode is
>> peripheral only), there would be no role switch. In that scenario, connected
>> flag would stay "true" even after removing cable. In that case, we can
>> generate disconnect interrupt only by clearing this VBUS_VALID bit and
>> inturn the suspend will succeed. So wanted to use notification from set_role
>> which would cover all cases:
>> 1. cable disconnect/cable connect
>> 2. Role switch from host->device and device->host
>>
> 
> Ok. Thanks for the explanation. How everything tied together seems
> awkward: The connector does a role-switch to "none" to trigger a
> role-switch in dwc3, which then triggers a callback to dwc3-qcom to
> clear vbus_valid, which then allows the controller to see a disconnect
> event in dwc3. Is that right?
> 

Yes, that is right.

> The connector driver should have a separate interface for
> attach/deattach in addition to role-switch. But I don't think type-c
> controller, USB-conn-gpio, and glink share the same way to handle
> connection/disconnection.
> 

I think although ROLE_NONE is for selecting default mode, it still means 
the same in USB-conn-gpio / ucsi-glink and hd3ss3220 (not sure about 
typec-controller).

> Let's keep what you proposed, and keep in mind that ROLE NONE is for
> selecting the default mode and not necessarily mean disconnection.
> 

Sure. Will do the following in v3:
1. Instead of adding host notifiers, will add 
"pm_runtime_use_autosuspend" in xhci-plat.c
2. Fix up coding issues pointed by you in patch 1/4.

Let me know if this sounds fine.

Regards,
Krishna,

Re: [PATCH v2 2/4] usb: dwc3: qcom: Implement glue callbacks to facilitate runtime suspend

[Thinh Nguyen]

On Wed, Aug 06, 2025, Krishna Kurapati wrote:
> 
> 
> On 8/6/2025 4:48 AM, Thinh Nguyen wrote:
> 
> [...]
> 
> > > > > > > > > > > diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> > > > > > > > > > > index ca7e1c02773a..d40b52e2ba01 100644
> > > > > > > > > > > --- a/drivers/usb/dwc3/dwc3-qcom.c
> > > > > > > > > > > +++ b/drivers/usb/dwc3/dwc3-qcom.c
> > > > > > > > > > > @@ -89,6 +89,12 @@ struct dwc3_qcom {
> > > > > > > > > > >       	bool			pm_suspended;
> > > > > > > > > > >       	struct icc_path		*icc_path_ddr;
> > > > > > > > > > >       	struct icc_path		*icc_path_apps;
> > > > > > > > > > > +
> > > > > > > > > > > +	/*
> > > > > > > > > > > +	 * Current role changes via usb_role_switch_set_role callback protected
> > > > > > > > > > > +	 * internally by mutex lock.
> > > > > > > > > > > +	 */
> > > > > > > > > > > +	enum usb_role		current_role;
> > > > > > > > > > 
> > > > > > > > > > Can we just track the current role through dwc3 core instead of an
> > > > > > > > > > addition field in the glue?
> > > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > Core caches only mode. We need ROLE NONE to identify cable connect. So
> > > > > > > > > adding that in glue to keep track.
> > > > > > > > > 
> > > > > > > > 
> > > > > > > > The controller is always either host or device, not somewhere in
> > > > > > > > between. You're using ROLE_NONE to indicate connection, which is a
> > > > > > > > separate state.
> > > > > > > > 
> > > > > > > 
> > > > > > > Yes, but there is no flag that indicates that in dwc structure today. Also
> > > > > > > since only dwc3-qcom needs it at the moment, I added that role info in glue
> > > > > > > layer.
> > > > > > 
> > > > > > How are you using ROLE NONE? Do you send a role-switch call to "none" to
> > > > > > indicate disconnect? Let's not do that. Currently the dwc3 driver would
> > > > > > switch back to the default mode if "none" is selected, but this is not
> > > > > > well defined and implementation specific. It can be no-op and would not
> > > > > > violate the interface.
> > > > > > 
> > > > > > The role-switch interface should only be used for role-switching and not
> > > > > > connection/disconnection.
> > > > > > 
> > > > > > > 
> > > > > > > > I feel this should be tracked separately for clarity. The dwc3 also
> > > > > > > > tracks the connection state, can we use that?
> > > > > > > > 
> > > > > > > 
> > > > > > > Are you referring to the "connected" flag in DWC structure ? I see that it
> > > > > > > indicates connection status only in gadget mode.
> > > > > > > 
> > > > > > 
> > > > > > Yes, that flag is only for gadget.
> > > > > > 
> > > > > > Can you provide more info of the setup? Is there a type-c controller or
> > > > > > phy that can detect attach/deattach? Or it only propagates to the usb
> > > > > > controller?
> > > > > 
> > > > > My response didn't show up on lore since the mail client I used before sent
> > > > > the message in HTML format. So resending my response again.
> > > > > 
> > > > > Hi Thinh,
> > > > > 
> > > > >    Yes this is for cases where role switching is present (either with a Type-C
> > > > > controller, USB-conn-gpio, or a glink based role-switch).
> > > > > 
> > > > >    Actually the requirement is as follows:
> > > > >    1. When in device mode, if we receive a cable disconnect, we need to clear
> > > > > OTG_VBUS_VALID bit of QSCRATCH register in glue address space.
> > > > >    2. When cable is connected in device mode, we need to set the
> > > > > OTG_VBUS_VALID bit of QSCRATCH register in glue address space.
> > > > >    3. When the runstop is set back after a suspend rotuine, then we need to
> > > > > set  OTG_VBUS_VALID bit of QSCRATCH register in glueaddress space.
> > > > > 
> > > > >    To take care of all the 3 scenarios above, the set_role and run_stop
> > > > > notifiers have been added.
> > > > > 
> > > > >    The role info propagates only from core to glue. It is not sent to the phy.
> > > > > 
> > > > 
> > > > When does ROLE NONE occur? Did you have the type-c driver set the role
> > > > switch to "none" indicate disconnect?
> > > > 
> > > > The vbus-valid is only for gadget, and you only care about the
> > > > OTG_VBUS_VALID right? Can we just use the dwc3->connected flag? Just
> > > > make sure that it's cleared on role-switch, which should be the case
> > > > because we always perform soft-disconnect on gadget unbind, and make
> > > > sure to set vbus-valid on pullup or gadget binding. Is there some
> > > > scenarios that dwc->connected does not cover?
> > > > 
> > > 
> > > Hi Thinh,
> > > 
> > >   In case of just a cable disconnect in device mode (and default dr mode is
> > > peripheral only), there would be no role switch. In that scenario, connected
> > > flag would stay "true" even after removing cable. In that case, we can
> > > generate disconnect interrupt only by clearing this VBUS_VALID bit and
> > > inturn the suspend will succeed. So wanted to use notification from set_role
> > > which would cover all cases:
> > > 1. cable disconnect/cable connect
> > > 2. Role switch from host->device and device->host
> > > 
> > 
> > Ok. Thanks for the explanation. How everything tied together seems
> > awkward: The connector does a role-switch to "none" to trigger a
> > role-switch in dwc3, which then triggers a callback to dwc3-qcom to
> > clear vbus_valid, which then allows the controller to see a disconnect
> > event in dwc3. Is that right?
> > 
> 
> Yes, that is right.
> 
> > The connector driver should have a separate interface for
> > attach/deattach in addition to role-switch. But I don't think type-c
> > controller, USB-conn-gpio, and glink share the same way to handle
> > connection/disconnection.
> > 
> 
> I think although ROLE_NONE is for selecting default mode, it still means the
> same in USB-conn-gpio / ucsi-glink and hd3ss3220 (not sure about
> typec-controller).
> 
> > Let's keep what you proposed, and keep in mind that ROLE NONE is for
> > selecting the default mode and not necessarily mean disconnection.
> > 
> 
> Sure. Will do the following in v3:
> 1. Instead of adding host notifiers, will add "pm_runtime_use_autosuspend"
> in xhci-plat.c
> 2. Fix up coding issues pointed by you in patch 1/4.
> 
> Let me know if this sounds fine.
> 

Yes. Sounds good.

Thanks,
Thinh