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 | 80 +++++++++++++++++++++++++++++++-----
1 file changed, 70 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index a7eaefaeec4d..5195267cd34d 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -83,6 +83,8 @@ struct dwc3_qcom {
bool pm_suspended;
struct icc_path *icc_path_ddr;
struct icc_path *icc_path_apps;
+
+ enum usb_role current_role;
};
#define to_dwc3_qcom(d) container_of((d), struct dwc3_qcom, dwc)
@@ -111,10 +113,6 @@ static inline void dwc3_qcom_clrbits(void __iomem *base, u32 offset, u32 val)
readl(base + offset);
}
-/*
- * TODO: Make the in-core role switching code invoke dwc3_qcom_vbus_override_enable(),
- * validate that the in-core extcon support is functional
- */
static void dwc3_qcom_vbus_override_enable(struct dwc3_qcom *qcom, bool enable)
{
if (enable) {
@@ -560,6 +558,57 @@ 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);
+
+ /*
+ * Current role changes via usb_role_switch_set_role callback protected
+ * internally by mutex lock.
+ */
+ 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 = {
+ .pre_set_role = dwc3_qcom_set_role_notifier,
+ .pre_run_stop = dwc3_qcom_run_stop_notifier,
+};
+
static int dwc3_qcom_probe(struct platform_device *pdev)
{
struct dwc3_probe_data probe_data = {};
@@ -636,6 +685,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 ((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;
@@ -650,12 +716,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);
-
wakeup_source = of_property_read_bool(dev->of_node, "wakeup-source");
device_init_wakeup(&pdev->dev, wakeup_source);
--
2.34.1On 8/6/25 11:58 AM, 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 | 80 +++++++++++++++++++++++++++++++-----
> 1 file changed, 70 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index a7eaefaeec4d..5195267cd34d 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -83,6 +83,8 @@ struct dwc3_qcom {
> bool pm_suspended;
> struct icc_path *icc_path_ddr;
> struct icc_path *icc_path_apps;
> +
> + enum usb_role current_role;
> };
>
> #define to_dwc3_qcom(d) container_of((d), struct dwc3_qcom, dwc)
> @@ -111,10 +113,6 @@ static inline void dwc3_qcom_clrbits(void __iomem *base, u32 offset, u32 val)
> readl(base + offset);
> }
>
> -/*
> - * TODO: Make the in-core role switching code invoke dwc3_qcom_vbus_override_enable(),
> - * validate that the in-core extcon support is functional
> - */
> static void dwc3_qcom_vbus_override_enable(struct dwc3_qcom *qcom, bool enable)
> {
> if (enable) {
> @@ -560,6 +558,57 @@ 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) {
no need for the "< 0":
"""
Return 0 if the runtime PM usage counter of @dev has been
incremented or a negative error code otherwise.
"""
> + dev_dbg(qcom->dev, "Failed to resume device\n");
This probably belongs in the suspend/resume calls themselves
> + return;
> + }
> +
> + if (qcom->current_role == USB_ROLE_DEVICE &&
> + next_role != USB_ROLE_DEVICE)
The second part is unnecessary because the first if-condition in this
function ensures it
> + dwc3_qcom_vbus_override_enable(qcom, false);
> + else if ((qcom->current_role != USB_ROLE_DEVICE) &&
> + (next_role == USB_ROLE_DEVICE))
similarly here
meaning this can become
dwc3_qcom_vbus_override_enable(qcom, next_role == USB_ROLE_DEVICE)
(I'm not sure if it's easier to read, up to you)
> + dwc3_qcom_vbus_override_enable(qcom, true);
> +
> + pm_runtime_mark_last_busy(qcom->dev);
> + pm_runtime_put_sync(qcom->dev);
> +
> + /*
> + * Current role changes via usb_role_switch_set_role callback protected
> + * internally by mutex lock.
> + */
> + 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);
this argument logically becomes true, always
> + pm_runtime_mark_last_busy(qcom->dev);
> +}
> +
> +struct dwc3_glue_ops dwc3_qcom_glue_ops = {
> + .pre_set_role = dwc3_qcom_set_role_notifier,
> + .pre_run_stop = dwc3_qcom_run_stop_notifier,
> +};
> +
> static int dwc3_qcom_probe(struct platform_device *pdev)
> {
> struct dwc3_probe_data probe_data = {};
> @@ -636,6 +685,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 ((device_property_read_bool(dev, "usb-role-switch")) &&
> + (usb_get_role_switch_default_mode(dev) == USB_DR_MODE_HOST))
currently this will never be true on any qcom dt ("role-switch-default-mode" is
not present anywhere)
> + qcom->current_role = USB_ROLE_HOST;
> + else
> + qcom->current_role = USB_ROLE_DEVICE;
> + }
> +
> + qcom->dwc.glue_ops = &dwc3_qcom_glue_ops;
> +
Konrad
On 8/6/2025 4:02 PM, Konrad Dybcio wrote:
> On 8/6/25 11:58 AM, Krishna Kurapati wrote:
[...]
>> +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) {
>
> no need for the "< 0":
>
> """
> Return 0 if the runtime PM usage counter of @dev has been
> incremented or a negative error code otherwise.
> """
>
ACK. Will remove the "<" condition here.
>> + dev_dbg(qcom->dev, "Failed to resume device\n");
>
> This probably belongs in the suspend/resume calls themselves
>
I think today, resume fails only if "clk_bulk_prepare_enable" fails.
Would like to keep this log here for now.
>> + return;
>> + }
>> +
>> + if (qcom->current_role == USB_ROLE_DEVICE &&
>> + next_role != USB_ROLE_DEVICE)
>
> The second part is unnecessary because the first if-condition in this
> function ensures it
ACK.
>> + dwc3_qcom_vbus_override_enable(qcom, false);
>> + else if ((qcom->current_role != USB_ROLE_DEVICE) &&
>> + (next_role == USB_ROLE_DEVICE))
>
> similarly here
>
> meaning this can become
>
> dwc3_qcom_vbus_override_enable(qcom, next_role == USB_ROLE_DEVICE)
>
> (I'm not sure if it's easier to read, up to you)
>
Will keep the if-else check as is for now.
>> + dwc3_qcom_vbus_override_enable(qcom, true);
>> +
>> + pm_runtime_mark_last_busy(qcom->dev);
>> + pm_runtime_put_sync(qcom->dev);
>> +
>> + /*
>> + * Current role changes via usb_role_switch_set_role callback protected
>> + * internally by mutex lock.
>> + */
>> + 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);
>
> this argument logically becomes true, always
ACK. Will just pass true here in v4.
>
>> + pm_runtime_mark_last_busy(qcom->dev);
>> +}
>> +
>> +struct dwc3_glue_ops dwc3_qcom_glue_ops = {
>> + .pre_set_role = dwc3_qcom_set_role_notifier,
>> + .pre_run_stop = dwc3_qcom_run_stop_notifier,
>> +};
>> +
>> static int dwc3_qcom_probe(struct platform_device *pdev)
>> {
>> struct dwc3_probe_data probe_data = {};
>> @@ -636,6 +685,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 ((device_property_read_bool(dev, "usb-role-switch")) &&
>> + (usb_get_role_switch_default_mode(dev) == USB_DR_MODE_HOST))
>
> currently this will never be true on any qcom dt ("role-switch-default-mode" is
> not present anywhere)
Agree. But I wrote for the sake of covering all cases and just in case
anyone uses this property tomorrow.
Regards,
Krishna,
On 8/7/25 7:17 AM, Krishna Kurapati wrote:
>
>
> On 8/6/2025 4:02 PM, Konrad Dybcio wrote:
>> On 8/6/25 11:58 AM, Krishna Kurapati wrote:
>
[...]
>>> + 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 ((device_property_read_bool(dev, "usb-role-switch")) &&
>>> + (usb_get_role_switch_default_mode(dev) == USB_DR_MODE_HOST))
>>
>> currently this will never be true on any qcom dt ("role-switch-default-mode" is
>> not present anywhere)
>
> Agree. But I wrote for the sake of covering all cases and just in case anyone uses this property tomorrow.
This is fine, just wanted to make sure this is intended
Konrad
© 2016 - 2025 Red Hat, Inc.