Re: [PATCH 3/9] usb: dwc3: qcom: Add support for selecting UTMI as reference clock

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The error code from `devm_clk_get_optional` is overwritten and ignored, failing to abort the probe on errors like `-EPROBE_DEFER`.
- [Medium] Use of `devm_clk_get_optional` instead of `devm_clk_get` for a conditionally required clock.
--

--- Patch [3]: [PATCH 3/9] usb: dwc3: qcom: Add support for selecting UTMI as reference clock ---
commit 62ea0cca7668740a78a41a22a25751f5e998df7d
Author: George Moussalem <george.moussalem@outlook.com>

usb: dwc3: qcom: Add support for selecting UTMI as reference clock

This commit adds support for selecting the UTMI clock as the reference clock
for the DWC3 core on certain Qualcomm chipsets. It reads a device tree
property to determine if the UTMI clock should be used and passes it to the
core driver via the probe data.

> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c

[ ... ]

> @@ -698,6 +700,18 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>  			qcom->current_role = USB_ROLE_DEVICE;
>  	}
>  
> +	select_utmi_as_ref_clk = device_property_read_bool(dev,
> +					"qcom,select-utmi-as-ref-clk");
> +	if (select_utmi_as_ref_clk) {

[Severity: Medium]
Since the UTMI clock is required to derive the reference clock period when
"qcom,select-utmi-as-ref-clk" is set, should this use devm_clk_get() instead?

If the "mock_utmi" clock is missing from the Device Tree,
devm_clk_get_optional() will return NULL rather than an error pointer.
Because IS_ERR(NULL) is false, the driver will silently continue with a NULL
clock, which might lead to the same non-functional USB operation mentioned in
the commit message.

> +		ref_clk = devm_clk_get_optional(dev, "mock_utmi");
> +		if (IS_ERR(ref_clk)) {
> +			ret = PTR_ERR(ref_clk);

[Severity: High]
Does this missing error handling silently swallow probe deferrals?

If devm_clk_get_optional() returns an error like -EPROBE_DEFER, the error
code is stored in 'ret' and a warning is logged, but execution continues.
The 'ret' variable will then be overwritten by the return value of
dwc3_core_probe() shortly after:

dwc3_qcom_probe() {
    ...
    ret = dwc3_core_probe(&probe_data);
    if (ret) {
        ret = dev_err_probe(dev, ret, "failed to register DWC3 Core\n");
        goto clk_disable;
    }
    ...
}

Should this error path abort the probe with a goto clk_disable?

> +			dev_warn(dev, "failed to get ref clock: %d\n", ret);
> +		} else {
> +			probe_data.ref_clk = ref_clk;
> +		}
> +	}
> +
>  	qcom->dwc.glue_ops = &dwc3_qcom_glue_ops;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260825-ipq-flatten-usb-v1-0-5c1f3170bbe9@outlook.com?part=3