| 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: <20250610105812.jepct55ic5snglno@umbar.lan>
Date: Tue, 10 Jun 2025 13:58:12 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: Krishna Kurapati <krishna.kurapati@....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 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?
> +}
> +
> +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
>
Powered by blists - more mailing lists