Re: [PATCH] usb: dwc3: Update nominal max votes for qcom usb

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] APPS_USB_AVG_BW is incorrectly defined without the MBps_to_icc() conversion macro, passing a severely undersized value to the interconnect framework.

Pre-existing issues:
- [High] If pm_runtime_resume_and_get() fails in dwc3_qcom_remove(), the driver returns early, skipping core unregistration and causing massive resource leaks and potential panics.
--

commit 68d35d27d1f598a1e07e782bd412e6f30d3c6fbf
Author: Akash Kumar <akash.kumar@oss.qualcomm.com>

usb: dwc3: Update nominal max votes for qcom usb

This commit increases nominal max bandwidth votes to improve DDR performance
and handle multiple USB audio use cases without glitches. It updates the
peak interconnect bandwidth from 2500 to 5500 and increases the apps USB
average and peak bandwidth allocations.

> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index f43f73ac36ff1..aa43a03f3c7fc 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -46,9 +46,9 @@
>  #define USB_MEMORY_AVG_HS_BW MBps_to_icc(240)
>  #define USB_MEMORY_PEAK_HS_BW MBps_to_icc(700)
>  #define USB_MEMORY_AVG_SS_BW  MBps_to_icc(1000)
> -#define USB_MEMORY_PEAK_SS_BW MBps_to_icc(2500)
> -#define APPS_USB_AVG_BW 0
> -#define APPS_USB_PEAK_BW MBps_to_icc(40)
> +#define USB_MEMORY_PEAK_SS_BW MBps_to_icc(5500)
> +#define APPS_USB_AVG_BW 40

[Severity: High]
Should this value be wrapped in the MBps_to_icc macro like the other
bandwidth definitions?

Since icc_set_bw expects values in kBps, defining APPS_USB_AVG_BW as
just 40 without the conversion macro appears to vote for 40 kBps instead
of the intended 40 MBps. Does this unintentionally starve the apps-usb
path instead of improving performance?

> +#define APPS_USB_PEAK_BW MBps_to_icc(80)
>  
>  /* Qualcomm SoCs with multiport support has up to 4 ports */
>  #define DWC3_QCOM_MAX_PORTS	4


This isn't a bug introduced by this patch, but looking at driver unbind
in dwc3_qcom_remove:

drivers/usb/dwc3/dwc3-qcom.c:dwc3_qcom_remove() {
    ...
    if (pm_runtime_resume_and_get(qcom->dev) < 0)
        return;
    ...
}

[Severity: High]
If pm_runtime_resume_and_get fails, does this early return skip
dwc3_core_remove and core unregistration entirely?

Since devres-managed memory for the qcom and dwc structs will still be
freed when the remove callback completes, will leaving debugfs files
active and workqueues (like vbus_draw_work) running result in a
use-after-free if they are accessed later?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611081831.2743464-1-akash.kumar@oss.qualcomm.com?part=1