| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <aaeafb33-9897-4f47-b4f8-4b92885c3795@oss.qualcomm.com>
Date: Tue, 10 Jun 2025 22:06:24 +0530
From: Krishna Kurapati <krishna.kurapati@....qualcomm.com>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
Cc: Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Bjorn Andersson <bjorn.andersson@....qualcomm.com>,
linux-arm-msm@...r.kernel.org, linux-usb@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/4] usb: dwc3: qcom: Implement glue callbacks to
facilitate runtime suspend
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@....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
>>
Powered by blists - more mailing lists