Re: [PATCH v4 1/2] dt-bindings: qcom,snps-dwc3: Add property indicating presence of eUSB2 phy

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The `qcom,has-eusb2-phy` property introduces a redundant boolean flag that duplicates existing hardware topology information, and its description incorrectly encodes software behavior.
- [Medium] The driver implementation for the newly added device tree property is missing from the patch series, despite claims in the commit message.
--

--- Patch [1]: [PATCH v4 1/2] dt-bindings: qcom,snps-dwc3: Add property indicating presence of eUSB2 phy ---
commit 9b14e7faf16aedf430d06c05d4c7f31252c58f4c
Author: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>

dt-bindings: qcom,snps-dwc3: Add property indicating presence of eUSB2 phy

Add property "qcom,has-eusb2-phy" to indicate whether the SoC has eUSB2 phy
or not. This is used to modify dp/dm interrupts during host mode suspend.

> diff --git a/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
> index 8201656b41ed7..0c6a839be82fd 100644
> --- a/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
> +++ b/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
> @@ -152,6 +152,13 @@ properties:
>        HS/FS/LS modes are supported.
>      type: boolean
>  
> +  qcom,has-eusb2-phy:
> +    description:
> +      If present, it indicates that the controller is working with eUSB2
> +      phy. This property is used to modify dp/dm interrupt configurations
> +      while entering suspend.
> +    type: boolean

[Severity: Medium]
Is this new property strictly necessary to describe the hardware topology?

The hardware topology appears to already be fully described. The standard
phys property points to a PHY node which has its own specific compatible
string. Could the driver deduce the PHY type by querying the linked PHY node
or by using the controller's SoC-specific compatible string, rather than
relying on a custom boolean flag?

Also, the description explicitly mentions software policy ("used to modify
dp/dm interrupt configurations while entering suspend"). Should this be
updated to describe the hardware rather than how the OS driver behaves?

[Severity: Medium]
The commit message mentions that this property "is used to modify dp/dm
interrupts during host mode suspend", but the driver implementation for this
feature seems to be missing from the patch.

Should the driver changes in drivers/usb/ that actually parse and utilize
this property be included in the same series to allow for a complete review?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-eusb2-interrupt-handling-v5-v4-0-d4f993925608@oss.qualcomm.com?part=1