Re: [PATCH 1/9] dt-bindings: usb: qcom,snps-dwc3: Add ability to select UTMI as reference clock

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Addition of a bespoke boolean property `qcom,select-utmi-as-ref-clk` to dictate software behavior instead of describing hardware.
- [Medium] The commit message references a non-existent deprecated DT property (`snps,quirk-ref-clk-period-ns`) and contains typos.
--

--- Patch [1]: [PATCH 1/9] dt-bindings: usb: qcom,snps-dwc3: Add ability to select UTMI as reference clock ---
commit bbfd7925c484a6ab77f42b767c8fcd756476cd44
Author: George Moussalem <george.moussalem@outlook.com>

dt-bindings: usb: qcom,snps-dwc3: Add ability to select UTMI as reference clock

This commit introduces a new device tree property to select the UTMI clock
as the reference clock for DWC3. It aims to support chipsets that have a
reference clock rate different from the default 19.2 MHz.

> In the flattened snsp-dwc3 model, it is currently not possible to pass a

[Severity: Low]
This isn't a bug, but there is a typo in the text above ("snsp-dwc3" instead
of "snps-dwc3").

> The existing DT property 'snps,quirk-ref-clk-period-ns' has been
> deprecated.

[Severity: Medium]
Is this property name accurate?

Searching the tree reveals no property named snps,quirk-ref-clk-period-ns.

Could this be referencing snps,ref-clock-period-ns, which is the property
defined in snps,dwc3-common.yaml?

> diff --git a/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
> index ea60f7220afe3..aa263dfd42a1c 100644
> --- a/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
> +++ b/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
> @@ -155,6 +155,15 @@ properties:
>        HS/FS/LS modes are supported.
>      type: boolean
>  
> +  qcom,select-utmi-as-ref-clk:
> +    description:
> +      If present, pass the UTMI clock as the reference clock to the DWC3 core to

[Severity: Medium]
Does this property dictate driver software behavior rather than describing
the hardware?

The hardware's connection to the UTMI clock is already fully described via
the clocks property (e.g., mock_utmi).

Can the driver deduce the need to pass this clock based on the SoC's
compatible string, or by checking the clock's presence, rather than relying
on a new bespoke boolean property?

> +      use its clock rate to calculate the reference clock period and frame
> +      length adjustment in GUCTL and GFLADJ registers. This is needed when these
> +      values based on the standard clock rate deviate from the hardware default
> +      values. If not set, the hardware default values are used.
> +    type: boolean
> +
>    wakeup-source: true
>  
>  required:

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