[PATCH 00/10] ARM: qcom: enable USB3 (DWC3) controller on MSM8974 platform

[PATCH 00/10] ARM: qcom: enable USB3 (DWC3) controller on MSM8974 platform

[Dmitry Baryshkov]

MSM8974 platform uses three different USB hosts: SS DWC3 and two HS
ChipIdea (normal and HSIC). Enhance qcom-ipq806x-usb driver to support
HS and SS PHYs used for the DWC3 controller and enable USB 3.0 host on
the MSM8974 platform.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
Dmitry Baryshkov (10):
      dt-bindings: usb: qcom,snps-dwc3: document the MSM8974 controller
      dt-bindings: phy: qcom,ipq806x-usb-phy-hs: document the MSM8974 HS PHY
      dt-bindings: phy: qcom,ipq806x-usb-phy-ss: add MSM8974 SS PHY
      phy: qcom-ipq806x-usb: add MSM8974 HS PHY support
      phy: qcom-ipq806x-usb: fix the SS PHY TEST_POWERDOWN bit
      phy: qcom-ipq806x-usb: fix the PHY_PARAM_CTRL1 field masks
      phy: qcom-ipq806x-usb: parse the tx-deamp-3_5db property as documented
      phy: qcom-ipq806x-usb: support the MSM8974 SS PHY parameters
      ARM: dts: qcom: msm8974: add the USB3 controller and its PHYs
      ARM: dts: qcom: apq8074-dragonboard: enable the USB3 host

 .../bindings/phy/qcom,ipq806x-usb-phy-hs.yaml      |   6 +-
 .../bindings/phy/qcom,ipq806x-usb-phy-ss.yaml      |   4 +-
 .../devicetree/bindings/usb/qcom,snps-dwc3.yaml    |  19 ++++
 .../arm/boot/dts/qcom/qcom-apq8074-dragonboard.dts |  61 ++++++++++++
 arch/arm/boot/dts/qcom/qcom-msm8974.dtsi           |  47 +++++++++
 drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c        | 107 +++++++++++++++++----
 6 files changed, 221 insertions(+), 23 deletions(-)
---
base-commit: 6b8c8af514d739d0335f5579b585e02babe8a727
change-id: 20260810-msm8974-usb3-8591395891b9
prerequisite-change-id: 20260810-msm8974-onboard-usb-98d293645270:v1
prerequisite-patch-id: c26539fbb2d694a92439cb1e726212918553475c
prerequisite-patch-id: 4ad3db6c6ac190b38705ed8876802392c8d7d339
prerequisite-patch-id: 3478affb2dfff9e7ab10d3021889e8b18457533f
prerequisite-patch-id: b646868cd0435307c98f9f5befcc6b19f21ec0d7
prerequisite-patch-id: b98cf68875fdf3b81fdc383e1b69a3fcc35bf986
prerequisite-patch-id: 4af06591492ba21e53d415f18de59488bfc3c2d4
prerequisite-patch-id: 5177cabe40273cd97110295567ff2cccbaad02df
prerequisite-patch-id: 1d2ed9fb3f5d99c6ea828eb5aec1412f4b46467a

Best regards,
--  
With best wishes
Dmitry

[PATCH 01/10] dt-bindings: usb: qcom,snps-dwc3: document the MSM8974 controller

[Dmitry Baryshkov]

Add the qcom,msm8974-dwc3 compatible. The controller uses the same four
clocks as IPQ8074. Unlike other controllers, it has only 3 IRQs routed:
core, power event and HS PHY.

Assisted-by: Claude:claude-fable-5
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 .../devicetree/bindings/usb/qcom,snps-dwc3.yaml       | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
index ea60f7220afe..9f91c8711e02 100644
--- a/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
@@ -40,6 +40,7 @@ properties:
           - qcom,maili-dwc3
           - qcom,milos-dwc3
           - qcom,msm8953-dwc3
+          - qcom,msm8974-dwc3
           - qcom,msm8994-dwc3
           - qcom,msm8996-dwc3
           - qcom,msm8998-dwc3
@@ -268,6 +269,7 @@ allOf:
           contains:
             enum:
               - qcom,ipq8074-dwc3
+              - qcom,msm8974-dwc3
               - qcom,qdu1000-dwc3
     then:
       properties:
@@ -453,6 +455,23 @@ allOf:
             - const: qusb2_phy
             - const: ss_phy_irq
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,msm8974-dwc3
+    then:
+      properties:
+        interrupts:
+          minItems: 3
+          maxItems: 3
+        interrupt-names:
+          items:
+            - const: dwc_usb3
+            - const: pwr_event
+            - const: hs_phy_irq
+
   - if:
       properties:
         compatible:

-- 
2.47.3

[PATCH 02/10] dt-bindings: phy: qcom,ipq806x-usb-phy-hs: document the MSM8974 HS PHY

[Dmitry Baryshkov]

The MSM8974 DWC3 wrapper contains the same generation of Synopsys 28nm
HS PHY as IPQ806x, controlled through the QSCRATCH registers, but the
binding only covers IPQ806x, leaving the MSM8974 controller to run on
bootloader-configured PHY state.

Add the qcom,msm8974-usb-phy-hs compatible.

Assisted-by: Claude:claude-fable-5
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 Documentation/devicetree/bindings/phy/qcom,ipq806x-usb-phy-hs.yaml | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/qcom,ipq806x-usb-phy-hs.yaml b/Documentation/devicetree/bindings/phy/qcom,ipq806x-usb-phy-hs.yaml
index 35296c588e78..de6f94997d5e 100644
--- a/Documentation/devicetree/bindings/phy/qcom,ipq806x-usb-phy-hs.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,ipq806x-usb-phy-hs.yaml
@@ -4,7 +4,7 @@
 $id: http://devicetree.org/schemas/phy/qcom,ipq806x-usb-phy-hs.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Qualcomm ipq806x usb DWC3 HS PHY CONTROLLER
+title: Qualcomm ipq806x/msm8974 usb DWC3 HS PHY CONTROLLER
 
 maintainers:
   - Ansuel Smith <ansuelsmth@gmail.com>
@@ -16,7 +16,9 @@ description:
 
 properties:
   compatible:
-    const: qcom,ipq806x-usb-phy-hs
+    enum:
+      - qcom,ipq806x-usb-phy-hs
+      - qcom,msm8974-usb-phy-hs
 
   "#phy-cells":
     const: 0

-- 
2.47.3

[PATCH 03/10] dt-bindings: phy: qcom,ipq806x-usb-phy-ss: add MSM8974 SS PHY

[Dmitry Baryshkov]

The MSM8974 DWC3 wrapper contains the same QSCRATCH-controlled
SuperSpeed PHY as IPQ806x (SS_PHY_CTRL at qscratch + 0x30 with an
identical bit layout, CR protocol port for the lane registers). The
SoC-specific params differ from those specified for IPQ806x, so add new
compatible for the PHY on MSM8974.

Assisted-by: Claude:claude-fable-5
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 Documentation/devicetree/bindings/phy/qcom,ipq806x-usb-phy-ss.yaml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/phy/qcom,ipq806x-usb-phy-ss.yaml b/Documentation/devicetree/bindings/phy/qcom,ipq806x-usb-phy-ss.yaml
index 6cf5c6c06072..29cb43cf7348 100644
--- a/Documentation/devicetree/bindings/phy/qcom,ipq806x-usb-phy-ss.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,ipq806x-usb-phy-ss.yaml
@@ -16,7 +16,9 @@ description:
 
 properties:
   compatible:
-    const: qcom,ipq806x-usb-phy-ss
+    oneOf:
+      - const: qcom,ipq806x-usb-phy-ss
+      - const: qcom,msm8974-usb-phy-ss
 
   "#phy-cells":
     const: 0

-- 
2.47.3

Re: [PATCH 03/10] dt-bindings: phy: qcom,ipq806x-usb-phy-ss: add MSM8974 SS PHY

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The schema unconditionally applies IPQ806x hardware defaults to the newly introduced MSM8974 variant, contradicting the commit message claim that the parameters differ.
--

--- Patch [3]: [PATCH 03/10] dt-bindings: phy: qcom,ipq806x-usb-phy-ss: add MSM8974 SS PHY ---
commit c3a8fb6b674551444c17885768e8da57eb5bdf84
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

dt-bindings: phy: qcom,ipq806x-usb-phy-ss: add MSM8974 SS PHY

This commit adds the msm8974 compatible string to the ipq806x dwc3
superspeed phy device tree binding. It notes that while the hardware layout
is identical, the parameters differ from those specified for ipq806x.

> diff --git a/Documentation/devicetree/bindings/phy/qcom,ipq806x-usb-phy-ss.yaml b/Documentation/devicetree/bindings/phy/qcom,ipq806x-usb-phy-ss.yaml
> index 6cf5c6c06072c..29cb43cf73481 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,ipq806x-usb-phy-ss.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,ipq806x-usb-phy-ss.yaml
> @@ -16,7 +16,9 @@ description:
>  
>  properties:
>    compatible:
> -    const: qcom,ipq806x-usb-phy-ss
> +    oneOf:
> +      - const: qcom,ipq806x-usb-phy-ss
> +      - const: qcom,msm8974-usb-phy-ss

[Severity: Low]
The commit message states that the hardware-specific parameters differ from
those specified for ipq806x. However, will dt-schema validation now
incorrectly interpret omitted optional properties on msm8974 nodes as having
the ipq806x defaults?

For example, the schema currently defines global defaults:

  qcom,rx-eq:
    $ref: /schemas/types.yaml#/definitions/uint32
    description: Override value for rx_eq.
    default: 4

Should there be a conditional block to ensure these defaults are scoped
strictly to the qcom,ipq806x-usb-phy-ss compatible, or should the differing
msm8974 defaults be explicitly specified?

>  
>    "#phy-cells":
>      const: 0

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810-msm8974-usb3-v1-0-1a47e1185524@oss.qualcomm.com?part=3

Re: [PATCH 03/10] dt-bindings: phy: qcom,ipq806x-usb-phy-ss: add MSM8974 SS PHY

[Rob Herring (Arm)]

On Mon, 10 Aug 2026 14:42:33 +0300, Dmitry Baryshkov wrote:
> The MSM8974 DWC3 wrapper contains the same QSCRATCH-controlled
> SuperSpeed PHY as IPQ806x (SS_PHY_CTRL at qscratch + 0x30 with an
> identical bit layout, CR protocol port for the lane registers). The
> SoC-specific params differ from those specified for IPQ806x, so add new
> compatible for the PHY on MSM8974.
> 
> Assisted-by: Claude:claude-fable-5
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
>  Documentation/devicetree/bindings/phy/qcom,ipq806x-usb-phy-ss.yaml | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/phy/qcom,ipq806x-usb-phy-ss.yaml: properties:compatible:oneOf: [{'const': 'qcom,ipq806x-usb-phy-ss'}, {'const': 'qcom,msm8974-usb-phy-ss'}] should not be valid under {'items': {'propertyNames': {'const': 'const'}, 'required': ['const']}}
	hint: Use 'enum' rather than 'oneOf' + 'const' entries
	from schema $id: http://devicetree.org/meta-schemas/keywords.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.kernel.org/project/devicetree/patch/20260810-msm8974-usb3-v1-3-1a47e1185524@oss.qualcomm.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

[PATCH 04/10] phy: qcom-ipq806x-usb: add MSM8974 HS PHY support

[Dmitry Baryshkov]

The MSM8974 DWC3 wrapper embeds the same generation of QSCRATCH
controlled Synopsys 28nm HS PHY as IPQ806x, but the driver only knows
the IPQ806x variants, so on MSM8974 the controller runs on whatever
PHY state the bootloader left behind.

Add the qcom,msm8974-usb-phy-hs compatible with its own init
sequence: unlike the IPQ806x one, it must not overwrite the whole
PHY_CTRL register, as the power-on values of the reference frequency
selection straps have to be preserved. Only enable the UTMI clock and
the DP/DM MPM clamps, signal VBUS valid towards the link, and disable
retention.

Assisted-by: Claude:claude-fable-5
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c | 35 +++++++++++++++++++++++++++--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c b/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
index f22c0000479f..21b59d4ca8d2 100644
--- a/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
+++ b/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
@@ -17,6 +17,7 @@
 #define HSUSB_PHY_CTRL_REG		(0x10)
 
 /* PHY_CTRL_REG */
+#define HSUSB_CLAMP_MPM_DPSE_DMSE_EN_N	BIT(26)
 #define HSUSB_CTRL_DMSEHV_CLAMP		BIT(24)
 #define HSUSB_CTRL_USB2_SUSPEND		BIT(23)
 #define HSUSB_CTRL_UTMI_CLK_EN		BIT(21)
@@ -27,9 +28,13 @@
 #define HSUSB_CTRL_ID_HV_CLAMP		BIT(9)
 #define HSUSB_CTRL_OTGSESSVLD_CLAMP	BIT(8)
 #define HSUSB_CTRL_CLAMP_EN		BIT(7)
+#define HSUSB_CTRL_CLAMP_MPM_DPSE_DMSE	BIT(26)
 #define HSUSB_CTRL_RETENABLEN		BIT(1)
 #define HSUSB_CTRL_POR			BIT(0)
 
+/* PHY parameter override register (eye diagram tuning) */
+#define HSUSB_PARAMETER_OVERRIDE_X_REG	(0x14)
+
 /* QSCRATCH_GENERAL_CFG */
 #define HSUSB_GCFG_XHCI_REV		BIT(2)
 
@@ -115,6 +120,8 @@
 #define LATCH_SLEEP				40
 #define LATCH_TIMEOUT				100
 
+struct phy_drvdata;
+
 struct usb_phy {
 	void __iomem		*base;
 	struct device		*dev;
@@ -123,11 +130,14 @@ struct usb_phy {
 	u32			rx_eq;
 	u32			tx_deamp_3_5db;
 	u32			mpll;
+	const struct phy_drvdata *drvdata;
 };
 
 struct phy_drvdata {
 	struct phy_ops	ops;
 	u32		clk_rate;
+	u32		hs_tune_val;
+	u32		fsel;
 };
 
 /**
@@ -273,7 +283,8 @@ static int qcom_ipq806x_usb_hs_phy_init(struct phy *phy)
 		HSUSB_CTRL_RETENABLEN  | HSUSB_CTRL_COMMONONN |
 		HSUSB_CTRL_OTGSESSVLD_CLAMP | HSUSB_CTRL_ID_HV_CLAMP |
 		HSUSB_CTRL_UTMI_OTG_VBUS_VALID | HSUSB_CTRL_UTMI_CLK_EN |
-		HSUSB_CTRL_CLAMP_EN | 0x70;
+		HSUSB_CTRL_CLAMP_EN | HSUSB_CLAMP_MPM_DPSE_DMSE_EN_N |
+		phy_dwc3->drvdata->fsel;
 
 	/* use core clock if external reference is not present */
 	if (!phy_dwc3->xo_clk)
@@ -282,9 +293,14 @@ static int qcom_ipq806x_usb_hs_phy_init(struct phy *phy)
 	writel(val, phy_dwc3->base + HSUSB_PHY_CTRL_REG);
 	usleep_range(2000, 2200);
 
-	/* Disable (bypass) VBUS and ID filters */
+	/* Set XHCI_REV bit (2) to 1 - XHCI version 1.0 */
 	writel(HSUSB_GCFG_XHCI_REV, phy_dwc3->base + QSCRATCH_GENERAL_CFG);
 
+	if (phy_dwc3->drvdata->hs_tune_val)
+		usb_phy_write_readback(phy_dwc3, HSUSB_PARAMETER_OVERRIDE_X_REG,
+				       0x03ffffff,
+				       phy_dwc3->drvdata->hs_tune_val);
+
 	return 0;
 }
 
@@ -458,6 +474,18 @@ static const struct phy_drvdata qcom_ipq806x_usb_hs_drvdata = {
 		.owner		= THIS_MODULE,
 	},
 	.clk_rate = 60000000,
+	.fsel = 0x70,
+};
+
+static const struct phy_drvdata qcom_msm8974_usb_hs_drvdata = {
+	.ops = {
+		.init		= qcom_ipq806x_usb_hs_phy_init,
+		.exit		= qcom_ipq806x_usb_hs_phy_exit,
+		.owner		= THIS_MODULE,
+	},
+	.clk_rate = 60000000,
+	.hs_tune_val = 0x00d191a4,
+	.fsel = 0x30,
 };
 
 static const struct phy_drvdata qcom_ipq806x_usb_ss_drvdata = {
@@ -474,6 +502,8 @@ static const struct of_device_id qcom_ipq806x_usb_phy_table[] = {
 	  .data = &qcom_ipq806x_usb_hs_drvdata },
 	{ .compatible = "qcom,ipq806x-usb-phy-ss",
 	  .data = &qcom_ipq806x_usb_ss_drvdata },
+	{ .compatible = "qcom,msm8974-usb-phy-hs",
+	  .data = &qcom_msm8974_usb_hs_drvdata },
 	{ /* Sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, qcom_ipq806x_usb_phy_table);
@@ -492,6 +522,7 @@ static int qcom_ipq806x_usb_phy_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	data = of_device_get_match_data(&pdev->dev);
+	phy_dwc3->drvdata = data;
 
 	phy_dwc3->dev = &pdev->dev;
 

-- 
2.47.3

Re: [PATCH 04/10] phy: qcom-ipq806x-usb: add MSM8974 HS PHY support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The MSM8974 initialization sequence reuses the shared IPQ806x sequence instead of providing its own, directly contradicting the commit message and unconditionally overwriting the PHY_CTRL register.
- [High] Setting the active-low HSUSB_CLAMP_MPM_DPSE_DMSE_EN_N bit disables the DP/DM MPM clamps, contradicting the commit message.
- [Low] Duplicate and redundant macro definitions for BIT(26) in the PHY_CTRL register.
--

--- Patch [4]: [PATCH 04/10] phy: qcom-ipq806x-usb: add MSM8974 HS PHY support ---
commit 90246a8322244753151c5fb6777bd8d442742a72
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

phy: qcom-ipq806x-usb: add MSM8974 HS PHY support

This commit adds support for the MSM8974 DWC3 wrapper by introducing a new
compatible string and driver data. It claims to implement a dedicated init
sequence for MSM8974 to preserve reference frequency selection straps.

> Add the qcom,msm8974-usb-phy-hs compatible with its own init
> sequence: unlike the IPQ806x one, it must not overwrite the whole
> PHY_CTRL register, as the power-on values of the reference frequency
> selection straps have to be preserved. Only enable the UTMI clock and
> the DP/DM MPM clamps, signal VBUS valid towards the link, and disable
> retention.

> diff --git a/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c b/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
> --- a/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
> +++ b/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
> @@ -17,6 +17,7 @@
>  #define HSUSB_PHY_CTRL_REG		(0x10)
>  
>  /* PHY_CTRL_REG */
> +#define HSUSB_CLAMP_MPM_DPSE_DMSE_EN_N	BIT(26)
>  #define HSUSB_CTRL_DMSEHV_CLAMP		BIT(24)
>  #define HSUSB_CTRL_USB2_SUSPEND		BIT(23)
>  #define HSUSB_CTRL_UTMI_CLK_EN		BIT(21)
[ ... ]
> @@ -27,9 +28,13 @@
>  #define HSUSB_CTRL_ID_HV_CLAMP		BIT(9)
>  #define HSUSB_CTRL_OTGSESSVLD_CLAMP	BIT(8)
>  #define HSUSB_CTRL_CLAMP_EN		BIT(7)
> +#define HSUSB_CTRL_CLAMP_MPM_DPSE_DMSE	BIT(26)

[Severity: Low]
This isn't a bug, but does this bit need to be defined twice? It was just
defined as HSUSB_CLAMP_MPM_DPSE_DMSE_EN_N a few lines above.

[ ... ]
> @@ -273,7 +283,8 @@ static int qcom_ipq806x_usb_hs_phy_init(struct phy *phy)
>  		HSUSB_CTRL_RETENABLEN  | HSUSB_CTRL_COMMONONN |
>  		HSUSB_CTRL_OTGSESSVLD_CLAMP | HSUSB_CTRL_ID_HV_CLAMP |
>  		HSUSB_CTRL_UTMI_OTG_VBUS_VALID | HSUSB_CTRL_UTMI_CLK_EN |
> -		HSUSB_CTRL_CLAMP_EN | 0x70;
> +		HSUSB_CTRL_CLAMP_EN | HSUSB_CLAMP_MPM_DPSE_DMSE_EN_N |

[Severity: High]
The commit message mentions we should "enable the DP/DM MPM clamps". Since
this bit has an _EN_N suffix, standardly indicating it is active-low, does
setting it to 1 here actually disable the clamps instead of enabling them?

> +		phy_dwc3->drvdata->fsel;
>  
>  	/* use core clock if external reference is not present */
>  	if (!phy_dwc3->xo_clk)
> @@ -282,9 +293,14 @@ static int qcom_ipq806x_usb_hs_phy_init(struct phy *phy)
>  	writel(val, phy_dwc3->base + HSUSB_PHY_CTRL_REG);

[Severity: High]
The commit message states that the MSM8974 init sequence "must not
overwrite the whole PHY_CTRL register". Does unconditionally writing to
this register with writel() break the preservation of the power-on values
for the reference frequency selection straps?

[ ... ]
> @@ -458,6 +474,18 @@ static const struct phy_drvdata qcom_ipq806x_usb_hs_drvdata = {
>  		.owner		= THIS_MODULE,
>  	},
>  	.clk_rate = 60000000,
> +	.fsel = 0x70,
> +};
> +
> +static const struct phy_drvdata qcom_msm8974_usb_hs_drvdata = {
> +	.ops = {
> +		.init		= qcom_ipq806x_usb_hs_phy_init,

[Severity: High]
The commit message indicates that MSM8974 was intended to have "its own init
sequence", but here it is assigned the shared qcom_ipq806x_usb_hs_phy_init
function. Should this point to a dedicated initialization function for
MSM8974 to avoid the writel() register overwrite mentioned above?

> +		.exit		= qcom_ipq806x_usb_hs_phy_exit,
> +		.owner		= THIS_MODULE,
> +	},

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810-msm8974-usb3-v1-0-1a47e1185524@oss.qualcomm.com?part=4

[PATCH 05/10] phy: qcom-ipq806x-usb: fix the SS PHY TEST_POWERDOWN bit

[Dmitry Baryshkov]

The SS PHY exit sequence tries to put the PHY into retention via the
TEST_POWERDOWN bit of SS_PHY_CTRL, but defines that bit as BIT(27). In
the DWC3 QSCRATCH SS_PHY_CTRL register TEST_POWERDOWN is bit 26 (see
the downstream phy-msm-ssusb driver for the same IP generation), so the
sequence pokes an unrelated bit and the PHY is never powered down.

Fixes: ef19b117b834 ("phy: qualcomm: add qcom ipq806x dwc usb phy driver")
Assisted-by: Claude:claude-fable-5
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c b/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
index 21b59d4ca8d2..cf77e0a66e20 100644
--- a/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
+++ b/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
@@ -51,7 +51,7 @@
 
 /* PHY_CTRL_REG */
 #define SSUSB_CTRL_REF_USE_PAD		BIT(28)
-#define SSUSB_CTRL_TEST_POWERDOWN	BIT(27)
+#define SSUSB_CTRL_TEST_POWERDOWN	BIT(26)
 #define SSUSB_CTRL_LANE0_PWR_PRESENT	BIT(24)
 #define SSUSB_CTRL_SS_PHY_EN		BIT(8)
 #define SSUSB_CTRL_SS_PHY_RESET		BIT(7)

-- 
2.47.3

Re: [PATCH 05/10] phy: qcom-ipq806x-usb: fix the SS PHY TEST_POWERDOWN bit

[sashiko-bot]

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the patch corrects the bit offset for `SSUSB_CTRL_TEST_POWERDOWN` while failing to actually fix the PHY powerdown issue due to a pre-existing copy-paste error that clears the bit instead of setting it. Furthermore, a naive correction of this error will expose a missing cleanup in the init sequence.
--

--- Patch [5]: [PATCH 05/10] phy: qcom-ipq806x-usb: fix the SS PHY TEST_POWERDOWN bit ---
commit c8781c769e4ee40df09815570d904fe2655dc82f
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

phy: qcom-ipq806x-usb: fix the SS PHY TEST_POWERDOWN bit
    
The SS PHY exit sequence tries to put the PHY into retention via the
TEST_POWERDOWN bit of SS_PHY_CTRL, but defines that bit as BIT(27). In
the DWC3 QSCRATCH SS_PHY_CTRL register TEST_POWERDOWN is bit 26 (see
the downstream phy-msm-ssusb driver for the same IP generation), so the
sequence pokes an unrelated bit and the PHY is never powered down.

> diff --git a/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c b/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
> index 21b59d4ca8d2a..cf77e0a66e20d 100644
> --- a/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
> +++ b/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
> @@ -51,7 +51,7 @@
>  
>  /* PHY_CTRL_REG */
>  #define SSUSB_CTRL_REF_USE_PAD		BIT(28)
> -#define SSUSB_CTRL_TEST_POWERDOWN	BIT(27)
> +#define SSUSB_CTRL_TEST_POWERDOWN	BIT(26)

[Severity: High]
This isn't a bug introduced by this patch, but while correcting the bit
offset for SSUSB_CTRL_TEST_POWERDOWN, the PHY powerdown issue remains
because the exit sequence clears the bit instead of setting it.

Looking at qcom_ipq806x_usb_ss_phy_exit() in 
drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c, it calls:

	usb_phy_write_readback(phy_dwc3, SSUSB_PHY_CTRL_REG,
			       SSUSB_CTRL_TEST_POWERDOWN, 0x0);

The usb_phy_write_readback() function clears the masked bits when passed 0x0:

	tmp &= ~mask;		/* retain other bits */
	write_val = tmp | val;

Should the third call in qcom_ipq806x_usb_ss_phy_exit() pass
SSUSB_CTRL_TEST_POWERDOWN as the value to actually set the bit?

Furthermore, if the exit sequence is fixed to set the bit, will the PHY
fail to exit retention on resume? In qcom_ipq806x_usb_ss_phy_init(), the
register is read, modified, and written back without clearing the
TEST_POWERDOWN bit:

	data = readl(phy_dwc3->base + SSUSB_PHY_CTRL_REG);
	writel(data | SSUSB_CTRL_SS_PHY_RESET,
	       phy_dwc3->base + SSUSB_PHY_CTRL_REG);

Since data is never updated to clear SSUSB_CTRL_TEST_POWERDOWN, would it
stay set and prevent the PHY from exiting retention mode?

>  #define SSUSB_CTRL_LANE0_PWR_PRESENT	BIT(24)
>  #define SSUSB_CTRL_SS_PHY_EN		BIT(8)
>  #define SSUSB_CTRL_SS_PHY_RESET		BIT(7)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810-msm8974-usb3-v1-0-1a47e1185524@oss.qualcomm.com?part=5

[PATCH 06/10] phy: qcom-ipq806x-usb: fix the PHY_PARAM_CTRL1 field masks

[Dmitry Baryshkov]

Each of the PHY_PARAM_CTRL1 field masks extends one bit below its
documented field: TX_FULL_SWING [26:20] is coded as GENMASK(26, 19),
TX_DEEMPH_6DB [19:14] as GENMASK(19, 13), TX_DEEMPH_3_5DB [13:8] as
GENMASK(13, 7) and LOS_BIAS [7:3] as GENMASK(7, 2). Also, the field
described as LOS_BIAS in reality is called LOS_LEVEL. FIELD_PREP against
these masks places every value one bit short of its field, so the
programmed swing/de-emphasis/LOS parameters land shifted and the
neighbouring fields are corrupted; the masked write-readback in the SS
PHY init reports 'write: ... to QSCRATCH: 4 FAILED'.

Align the masks with the documented field positions.

Fixes: ef19b117b834 ("phy: qualcomm: add qcom ipq806x dwc usb phy driver")
Assisted-by: Claude:claude-fable-5
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c b/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
index cf77e0a66e20..3d3e9d6da298 100644
--- a/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
+++ b/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
@@ -68,15 +68,17 @@
 #define SSPHY_MPLL_VALUE		0
 
 /* QSCRATCH PHY_PARAM_CTRL1 fields */
-#define PHY_PARAM_CTRL1_TX_FULL_SWING_MASK	GENMASK(26, 19)
-#define PHY_PARAM_CTRL1_TX_DEEMPH_6DB_MASK	GENMASK(19, 13)
-#define PHY_PARAM_CTRL1_TX_DEEMPH_3_5DB_MASK	GENMASK(13, 7)
-#define PHY_PARAM_CTRL1_LOS_BIAS_MASK		GENMASK(7, 2)
+#define PHY_PARAM_CTRL1_TX_FULL_SWING_MASK	GENMASK(26, 20)
+#define PHY_PARAM_CTRL1_TX_DEEMPH_6DB_MASK	GENMASK(19, 14)
+#define PHY_PARAM_CTRL1_TX_DEEMPH_3_5DB_MASK	GENMASK(13, 8)
+#define PHY_PARAM_CTRL1_LOS_LEVEL_MASK		GENMASK(7, 3)
+#define PHY_PARAM_CTRL1_LOS_BIAS_MASK		GENMASK(2, 0)
 
 #define PHY_PARAM_CTRL1_MASK				\
 		(PHY_PARAM_CTRL1_TX_FULL_SWING_MASK |	\
 		 PHY_PARAM_CTRL1_TX_DEEMPH_6DB_MASK |	\
 		 PHY_PARAM_CTRL1_TX_DEEMPH_3_5DB_MASK |	\
+		 PHY_PARAM_CTRL1_LOS_LEVEL_MASK | \
 		 PHY_PARAM_CTRL1_LOS_BIAS_MASK)
 
 #define PHY_PARAM_CTRL1_TX_FULL_SWING(x)	\
@@ -85,6 +87,8 @@
 		FIELD_PREP(PHY_PARAM_CTRL1_TX_DEEMPH_6DB_MASK, (x))
 #define PHY_PARAM_CTRL1_TX_DEEMPH_3_5DB(x)	\
 		FIELD_PREP(PHY_PARAM_CTRL1_TX_DEEMPH_3_5DB_MASK, x)
+#define PHY_PARAM_CTRL1_LOS_LEVEL(x)	\
+		FIELD_PREP(PHY_PARAM_CTRL1_LOS_LEVEL_MASK, (x))
 #define PHY_PARAM_CTRL1_LOS_BIAS(x)	\
 		FIELD_PREP(PHY_PARAM_CTRL1_LOS_BIAS_MASK, (x))
 
@@ -436,7 +440,8 @@ static int qcom_ipq806x_usb_ss_phy_init(struct phy *phy)
 	data |= PHY_PARAM_CTRL1_TX_FULL_SWING(0x6e) |
 		PHY_PARAM_CTRL1_TX_DEEMPH_6DB(0x20) |
 		PHY_PARAM_CTRL1_TX_DEEMPH_3_5DB(phy_dwc3->tx_deamp_3_5db) |
-		PHY_PARAM_CTRL1_LOS_BIAS(0x9);
+		PHY_PARAM_CTRL1_LOS_LEVEL(0x9) |
+		PHY_PARAM_CTRL1_LOS_BIAS(0x0);
 
 	usb_phy_write_readback(phy_dwc3, SSUSB_PHY_PARAM_CTRL_1,
 			       PHY_PARAM_CTRL1_MASK, data);

-- 
2.47.3

Re: [PATCH 06/10] phy: qcom-ipq806x-usb: fix the PHY_PARAM_CTRL1 field masks

[sashiko-bot]

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

Pre-existing issues:
- [High] The patch fixes overlapping off-by-one field masks for PHY_PARAM_CTRL1, but fails to address identical off-by-one overlaps in RX_OVRD_IN_HI and TX_OVRD_DRV_LO register masks which cause active initialization bugs.
- [High] Missing clock cleanup on error paths in qcom_ipq806x_usb_ss_phy_init().
- [High] TEST_POWERDOWN is incorrectly cleared instead of being set during PHY exit.
- [High] Missing -EPROBE_DEFER handling for the optional xo_clk.
- [Medium] qcom_ipq806x_usb_ss_phy_init() incorrectly passes a fully assembled register value to usb_phy_write_readback(), causing spurious readback verification failures.
--

--- Patch [6]: [PATCH 06/10] phy: qcom-ipq806x-usb: fix the PHY_PARAM_CTRL1 field masks ---
commit cc9548cb27cc0af73d58ff5dfbb1d43ff53c1b2b
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

phy: qcom-ipq806x-usb: fix the PHY_PARAM_CTRL1 field masks

This patch aligns the PHY_PARAM_CTRL1 field masks with their documented 
positions to prevent values from landing shifted. The previous off-by-one 
masks corrupted neighboring fields and caused spurious write-readback 
failures during PHY initialization.

> diff --git a/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c b/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
> index cf77e0a66e20d..3d3e9d6da298c 100644
> --- a/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
> +++ b/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
> @@ -68,15 +68,17 @@
>  #define SSPHY_MPLL_VALUE		0
>  
>  /* QSCRATCH PHY_PARAM_CTRL1 fields */
> -#define PHY_PARAM_CTRL1_TX_FULL_SWING_MASK	GENMASK(26, 19)
> -#define PHY_PARAM_CTRL1_TX_DEEMPH_6DB_MASK	GENMASK(19, 13)
> -#define PHY_PARAM_CTRL1_TX_DEEMPH_3_5DB_MASK	GENMASK(13, 7)
> -#define PHY_PARAM_CTRL1_LOS_BIAS_MASK		GENMASK(7, 2)
> +#define PHY_PARAM_CTRL1_TX_FULL_SWING_MASK	GENMASK(26, 20)
> +#define PHY_PARAM_CTRL1_TX_DEEMPH_6DB_MASK	GENMASK(19, 14)
> +#define PHY_PARAM_CTRL1_TX_DEEMPH_3_5DB_MASK	GENMASK(13, 8)
> +#define PHY_PARAM_CTRL1_LOS_LEVEL_MASK		GENMASK(7, 3)
> +#define PHY_PARAM_CTRL1_LOS_BIAS_MASK		GENMASK(2, 0)
>  
>  #define PHY_PARAM_CTRL1_MASK				\
>  		(PHY_PARAM_CTRL1_TX_FULL_SWING_MASK |	\
>  		 PHY_PARAM_CTRL1_TX_DEEMPH_6DB_MASK |	\
>  		 PHY_PARAM_CTRL1_TX_DEEMPH_3_5DB_MASK |	\
> +		 PHY_PARAM_CTRL1_LOS_LEVEL_MASK | \
>  		 PHY_PARAM_CTRL1_LOS_BIAS_MASK)

[Severity: High]
This is a pre-existing issue, but while fixing the overlapping field masks for 
PHY_PARAM_CTRL1, are there identical overlaps in the RX_OVRD_IN_HI and 
TX_OVRD_DRV_LO masks that break the initialization logic?

For example:

#define RX_OVRD_IN_HI_RX_EQ_MASK		GENMASK(10, 7)
#define RX_OVRD_IN_HI_RX_EQ_EN_OVRD		BIT(7)

Because these masks overlap at bit 7, does qcom_ipq806x_usb_ss_phy_init() 
silently disable the override right after enabling it?

	data |= RX_OVRD_IN_HI_RX_EQ_EN_OVRD;
	data &= ~RX_OVRD_IN_HI_RX_EQ_MASK;
	data |= RX_OVRD_IN_HI_RX_EQ(phy_dwc3->rx_eq);

[ ... ]

> @@ -436,7 +440,8 @@ static int qcom_ipq806x_usb_ss_phy_init(struct phy *phy)
>  	data |= PHY_PARAM_CTRL1_TX_FULL_SWING(0x6e) |
>  		PHY_PARAM_CTRL1_TX_DEEMPH_6DB(0x20) |
>  		PHY_PARAM_CTRL1_TX_DEEMPH_3_5DB(phy_dwc3->tx_deamp_3_5db) |
> -		PHY_PARAM_CTRL1_LOS_BIAS(0x9);
> +		PHY_PARAM_CTRL1_LOS_LEVEL(0x9) |
> +		PHY_PARAM_CTRL1_LOS_BIAS(0x0);
>  
>  	usb_phy_write_readback(phy_dwc3, SSUSB_PHY_PARAM_CTRL_1,
>  			       PHY_PARAM_CTRL1_MASK, data);

[Severity: Medium]
This isn't a bug introduced by this patch, but does this call to 
usb_phy_write_readback() always fail spuriously on probe?

It looks like we are passing a fully assembled register value (`data`) as `val`
instead of passing just the masked bits:

	data = readl(phy_dwc3->base + SSUSB_PHY_PARAM_CTRL_1);
	data &= ~PHY_PARAM_CTRL1_MASK;
	data |= PHY_PARAM_CTRL1_TX_FULL_SWING(0x6e) | ...
	usb_phy_write_readback(phy_dwc3, SSUSB_PHY_PARAM_CTRL_1,
			       PHY_PARAM_CTRL1_MASK, data);

Inside usb_phy_write_readback(), it uses the provided `mask` when checking `val`:

	/* Read back to see if val was written */
	tmp = readl(phy_dwc3->base + offset);
	tmp &= mask;		/* clear other bits */

	if (tmp != val)
		dev_err(...);

Since `data` contains unmasked bits from the initial readl() and `tmp` masks
them out, won't this check unconditionally fail if any unmasked bits are non-zero?

[Severity: High]
This is a pre-existing issue, but does qcom_ipq806x_usb_ss_phy_init() leak 
the clock references if it fails? 

When a hardware read/write fails, the error path returns without unwinding 
the enabled clocks:

err_phy_trans:
	return ret;

Since the PHY core doesn't call phy_exit() when phy_init() fails, it seems 
xo_clk and ref_clk remain permanently prepared and enabled?

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but does the PHY exit sequence in 
qcom_ipq806x_usb_ss_phy_exit() disable PHY retention instead of enabling it?

The comment indicates we want to set TEST_POWERDOWN:

	/* 3. Set TEST_POWERED_DOWN in PHY_CTRL_REG to enable PHY retention */
	...
	usb_phy_write_readback(phy_dwc3, SSUSB_PHY_CTRL_REG,
			       SSUSB_CTRL_TEST_POWERDOWN, 0x0);

Since 0x0 is passed as the `val`, usb_phy_write_readback() clears the 
SSUSB_CTRL_TEST_POWERDOWN bit rather than setting it. Should this pass 
SSUSB_CTRL_TEST_POWERDOWN as the value?

[ ... ]

[Severity: High]
This is a pre-existing issue, but does qcom_ipq806x_usb_phy_probe() handle 
probe deferral correctly for the optional xo_clk?

	phy_dwc3->xo_clk = devm_clk_get(phy_dwc3->dev, "xo");
	if (IS_ERR(phy_dwc3->xo_clk)) {
		dev_dbg(phy_dwc3->dev, "cannot get TCXO clock\n");
		phy_dwc3->xo_clk = NULL;
	}

devm_clk_get() returns -EPROBE_DEFER when the clock is not yet ready. By 
checking IS_ERR and masking all errors to NULL, wouldn't the driver fail to 
defer its probe and silently operate without the TCXO clock if it probes 
before the clock provider is ready?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810-msm8974-usb3-v1-0-1a47e1185524@oss.qualcomm.com?part=6

[PATCH 07/10] phy: qcom-ipq806x-usb: parse the tx-deamp-3_5db property as documented

[Dmitry Baryshkov]

The driver reads the transmit de-emphasis override from
qcom,tx-deamp_3_5db while the binding documents qcom,tx-deamp-3_5db,
so a device tree following the binding is silently ignored and the
default is used instead. No in-tree device tree uses either spelling.

Parse the property under its documented name.

Fixes: ef19b117b834 ("phy: qualcomm: add qcom ipq806x dwc usb phy driver")
Assisted-by: Claude:claude-fable-5
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c b/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
index 3d3e9d6da298..86e8e869a743 100644
--- a/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
+++ b/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
@@ -561,7 +561,7 @@ static int qcom_ipq806x_usb_phy_probe(struct platform_device *pdev)
 				     &phy_dwc3->rx_eq))
 		phy_dwc3->rx_eq = SSPHY_RX_EQ_VALUE;
 
-	if (device_property_read_u32(&pdev->dev, "qcom,tx-deamp_3_5db",
+	if (device_property_read_u32(&pdev->dev, "qcom,tx-deamp-3_5db",
 				     &phy_dwc3->tx_deamp_3_5db))
 		phy_dwc3->tx_deamp_3_5db = SSPHY_TX_DEEMPH_3_5DB;
 

-- 
2.47.3

Re: [PATCH 07/10] phy: qcom-ipq806x-usb: parse the tx-deamp-3_5db property as documented

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] The patch breaks backward compatibility with existing device trees by replacing the `qcom,tx-deamp_3_5db` property with `qcom,tx-deamp-3_5db` without providing a fallback. The commit message incorrectly claims that no in-tree device tree uses either spelling, but `arch/arm/boot/dts/qcom/qcom-ipq8064-v2.0.dtsi` actively sets `qcom,tx-deamp_3_5db = <32>;`. Consequently, the driver will silently fail to read this property from existing deployed DTBs and fall back to the default value, leading to an unintended hardware configuration change.
--

--- Patch [7]: [PATCH 07/10] phy: qcom-ipq806x-usb: parse the tx-deamp-3_5db property as documented ---
commit 0d730340df80865649e4381722ca35e1603f1cad
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

phy: qcom-ipq806x-usb: parse the tx-deamp-3_5db property as documented

This commit updates the driver to parse the "qcom,tx-deamp-3_5db" property,
matching the device tree binding documentation. It replaces the old
"qcom,tx-deamp_3_5db" spelling.

> The driver reads the transmit de-emphasis override from
> qcom,tx-deamp_3_5db while the binding documents qcom,tx-deamp-3_5db,
> so a device tree following the binding is silently ignored and the
> default is used instead. No in-tree device tree uses either spelling.

Is the statement about in-tree device trees accurate? Looking at the source,
arch/arm/boot/dts/qcom/qcom-ipq8064-v2.0.dtsi actively sets
"qcom,tx-deamp_3_5db = <32>;".

> diff --git a/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c b/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
> index 3d3e9d6da298c..86e8e869a743f 100644
> --- a/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
> +++ b/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
> @@ -561,7 +561,7 @@ static int qcom_ipq806x_usb_phy_probe(struct platform_device *pdev)
>  				     &phy_dwc3->rx_eq))
>  		phy_dwc3->rx_eq = SSPHY_RX_EQ_VALUE;
>  
> -	if (device_property_read_u32(&pdev->dev, "qcom,tx-deamp_3_5db",
> +	if (device_property_read_u32(&pdev->dev, "qcom,tx-deamp-3_5db",

[Severity: Critical]
Does replacing this property without a fallback introduce a regression for
backward compatibility with existing device trees?

Since device tree bindings are a stable kernel ABI, systems booting with an
older DTB (or the current in-tree DTB) might silently fail to read this
property and fall back to the default value instead of the configured value.

Should qcom_ipq806x_usb_phy_probe() maintain support for the legacy
underscore property as a fallback?

>  				     &phy_dwc3->tx_deamp_3_5db))
>  		phy_dwc3->tx_deamp_3_5db = SSPHY_TX_DEEMPH_3_5DB;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810-msm8974-usb3-v1-0-1a47e1185524@oss.qualcomm.com?part=7

[PATCH 08/10] phy: qcom-ipq806x-usb: support the MSM8974 SS PHY parameters

[Dmitry Baryshkov]

For the MSM8974 SS PHY instance the vendor kernels program analog
parameters different from those used for IPQ806x.  With the IPQ806x
values the masked write-readback of PHY_PARAM_CTRL1 fails and the lane
runs with wrong launch amplitudes.

Add platform data for the MSM8974 compatible.

Assisted-by: Claude:claude-fable-5
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c | 57 ++++++++++++++++++++++-------
 1 file changed, 44 insertions(+), 13 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c b/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
index 86e8e869a743..35abe9ae0542 100644
--- a/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
+++ b/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
@@ -142,6 +142,11 @@ struct phy_drvdata {
 	u32		clk_rate;
 	u32		hs_tune_val;
 	u32		fsel;
+	u32		tx_amplitude;
+	u32		tx_full_swing;
+	u32		los_level;
+	u32		los_bias;
+	bool		set_deemph_6db;
 };
 
 /**
@@ -322,7 +327,7 @@ static int qcom_ipq806x_usb_ss_phy_init(struct phy *phy)
 {
 	struct usb_phy *phy_dwc3 = phy_get_drvdata(phy);
 	int ret;
-	u32 data;
+	u32 data, mask;
 
 	ret = clk_prepare_enable(phy_dwc3->xo_clk);
 	if (ret)
@@ -414,7 +419,7 @@ static int qcom_ipq806x_usb_ss_phy_init(struct phy *phy)
 	data &= ~TX_OVRD_DRV_LO_PREEMPH_MASK;
 	data |= TX_OVRD_DRV_LO_PREEMPH(phy_dwc3->tx_deamp_3_5db);
 	data &= ~TX_OVRD_DRV_LO_AMPLITUDE_MASK;
-	data |= 0x6E;
+	data |= phy_dwc3->drvdata->tx_amplitude;
 	data |= TX_OVRD_DRV_LO_EN;
 	ret = usb_ss_write_phycreg(phy_dwc3,
 				   SSPHY_CTRL_TX_OVRD_DRV_LO(0), data);
@@ -433,18 +438,22 @@ static int qcom_ipq806x_usb_ss_phy_init(struct phy *phy)
 	 * TX_DEEMPH_3_5DB [13:8] set based on SoC version
 	 * LOS_BIAS [7:3] to 9
 	 */
-	data = readl(phy_dwc3->base + SSUSB_PHY_PARAM_CTRL_1);
-
-	data &= ~PHY_PARAM_CTRL1_MASK;
-
-	data |= PHY_PARAM_CTRL1_TX_FULL_SWING(0x6e) |
-		PHY_PARAM_CTRL1_TX_DEEMPH_6DB(0x20) |
-		PHY_PARAM_CTRL1_TX_DEEMPH_3_5DB(phy_dwc3->tx_deamp_3_5db) |
-		PHY_PARAM_CTRL1_LOS_LEVEL(0x9) |
-		PHY_PARAM_CTRL1_LOS_BIAS(0x0);
+	mask = PHY_PARAM_CTRL1_TX_FULL_SWING_MASK |
+	       PHY_PARAM_CTRL1_TX_DEEMPH_3_5DB_MASK |
+	       PHY_PARAM_CTRL1_LOS_LEVEL_MASK |
+	       PHY_PARAM_CTRL1_LOS_BIAS_MASK;
+
+	data = PHY_PARAM_CTRL1_TX_FULL_SWING(phy_dwc3->drvdata->tx_full_swing) |
+	       PHY_PARAM_CTRL1_TX_DEEMPH_3_5DB(phy_dwc3->tx_deamp_3_5db) |
+	       PHY_PARAM_CTRL1_LOS_LEVEL(phy_dwc3->drvdata->los_level) |
+	       PHY_PARAM_CTRL1_LOS_BIAS(phy_dwc3->drvdata->los_bias);
+
+	if (phy_dwc3->drvdata->set_deemph_6db) {
+		mask |= PHY_PARAM_CTRL1_TX_DEEMPH_6DB_MASK;
+		data |= PHY_PARAM_CTRL1_TX_DEEMPH_6DB(0x20);
+	}
 
-	usb_phy_write_readback(phy_dwc3, SSUSB_PHY_PARAM_CTRL_1,
-			       PHY_PARAM_CTRL1_MASK, data);
+	usb_phy_write_readback(phy_dwc3, SSUSB_PHY_PARAM_CTRL_1, mask, data);
 
 err_phy_trans:
 	return ret;
@@ -500,6 +509,26 @@ static const struct phy_drvdata qcom_ipq806x_usb_ss_drvdata = {
 		.owner		= THIS_MODULE,
 	},
 	.clk_rate = 125000000,
+	.tx_amplitude = 0x6e,
+	.tx_full_swing = 0x6e,
+	.los_level = 0x9,
+	.los_bias = 0x0,
+	.set_deemph_6db = true,
+};
+
+/* MSM8974: values from the vendor phy-msm-ssusb driver */
+static const struct phy_drvdata qcom_msm8974_usb_ss_drvdata = {
+	.ops = {
+		.init		= qcom_ipq806x_usb_ss_phy_init,
+		.exit		= qcom_ipq806x_usb_ss_phy_exit,
+		.owner		= THIS_MODULE,
+	},
+	.clk_rate = 125000000,
+	.tx_amplitude = 0x7f,
+	.tx_full_swing = 0x7f,
+	.los_level = 0x9,
+	.los_bias = 0x5,
+	.set_deemph_6db = false,
 };
 
 static const struct of_device_id qcom_ipq806x_usb_phy_table[] = {
@@ -509,6 +538,8 @@ static const struct of_device_id qcom_ipq806x_usb_phy_table[] = {
 	  .data = &qcom_ipq806x_usb_ss_drvdata },
 	{ .compatible = "qcom,msm8974-usb-phy-hs",
 	  .data = &qcom_msm8974_usb_hs_drvdata },
+	{ .compatible = "qcom,msm8974-usb-phy-ss",
+	  .data = &qcom_msm8974_usb_ss_drvdata },
 	{ /* Sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, qcom_ipq806x_usb_phy_table);

-- 
2.47.3

Re: [PATCH 08/10] phy: qcom-ipq806x-usb: support the MSM8974 SS PHY parameters

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Unconditional modification of shared IPQ806x HS PHY initialization introduces a hardware regression and contradicts the commit message.
- [Low] The PHY_PARAM_CTRL1_MASK macro is left as unused dead code.
--

--- Patch [8]: [PATCH 08/10] phy: qcom-ipq806x-usb: support the MSM8974 SS PHY parameters ---
commit ba82cb741a57672fd8113542520d510104c224d2
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

phy: qcom-ipq806x-usb: support the MSM8974 SS PHY parameters

This commit adds platform data for the MSM8974 compatible in the IPQ806x USB
PHY driver. It modifies qcom_ipq806x_usb_ss_phy_init() to use dynamic SS PHY
parameters instead of hardcoding IPQ806x-specific values. This ensures the
correct launch amplitudes are used for MSM8974.

[Severity: High]
Does this code unconditionally apply the active-low clamp disable bit?

drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c:qcom_ipq806x_usb_hs_phy_init() {
	val = HSUSB_CTRL_DPSEHV_CLAMP | HSUSB_CTRL_DMSEHV_CLAMP |
		HSUSB_CTRL_RETENABLEN  | HSUSB_CTRL_COMMONONN |
		HSUSB_CTRL_OTGSESSVLD_CLAMP | HSUSB_CTRL_ID_HV_CLAMP |
		HSUSB_CTRL_UTMI_OTG_VBUS_VALID | HSUSB_CTRL_UTMI_CLK_EN |
		HSUSB_CTRL_CLAMP_EN | HSUSB_CLAMP_MPM_DPSE_DMSE_EN_N |
		phy_dwc3->drvdata->fsel;
    ...
	writel(val, phy_dwc3->base + HSUSB_PHY_CTRL_REG);
    ...
}

This completely overwrites the PHY_CTRL register for all platforms. Can this
introduce a hardware regression for older IPQ806x hardware by sending an
unintended active-low clamp-disable bit?

Also, does this contradict the commit message's claim that MSM8974 must not
overwrite the whole PHY_CTRL register?

Finally, are there redundant, duplicated bit definitions for BIT(26) in the
same file?

drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c {
    ...
#define HSUSB_CLAMP_MPM_DPSE_DMSE_EN_N	BIT(26)
    ...
}

> diff --git a/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c b/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
> index 86e8e869a743f..35abe9ae05429 100644
> --- a/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
> +++ b/drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c
[ ... ]
> @@ -433,18 +438,22 @@ static int qcom_ipq806x_usb_ss_phy_init(struct phy *phy)
>  	 * TX_DEEMPH_3_5DB [13:8] set based on SoC version
>  	 * LOS_BIAS [7:3] to 9
>  	 */
> -	data = readl(phy_dwc3->base + SSUSB_PHY_PARAM_CTRL_1);
> -
> -	data &= ~PHY_PARAM_CTRL1_MASK;

[Severity: Low]
Is the PHY_PARAM_CTRL1_MASK macro left as unused dead code now that its usage
has been removed here?

> -
> -	data |= PHY_PARAM_CTRL1_TX_FULL_SWING(0x6e) |
> -		PHY_PARAM_CTRL1_TX_DEEMPH_6DB(0x20) |
> -		PHY_PARAM_CTRL1_TX_DEEMPH_3_5DB(phy_dwc3->tx_deamp_3_5db) |
> -		PHY_PARAM_CTRL1_LOS_LEVEL(0x9) |
> -		PHY_PARAM_CTRL1_LOS_BIAS(0x0);
> +	mask = PHY_PARAM_CTRL1_TX_FULL_SWING_MASK |
> +	       PHY_PARAM_CTRL1_TX_DEEMPH_3_5DB_MASK |
> +	       PHY_PARAM_CTRL1_LOS_LEVEL_MASK |
> +	       PHY_PARAM_CTRL1_LOS_BIAS_MASK;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810-msm8974-usb3-v1-0-1a47e1185524@oss.qualcomm.com?part=8

[PATCH 09/10] ARM: dts: qcom: msm8974: add the USB3 controller and its PHYs

[Dmitry Baryshkov]

The primary USB port of MSM8974 - a Synopsys DWC3 behind the standard
Qualcomm wrapper - is not described, so boards can only use the two
ChipIdea controllers.

Describe the wrapper and the DWC3 core together with both of its
QSCRATCH-controlled PHYs: the HS PHY at qscratch + 0x00 and the
SuperSpeed PHY at qscratch + 0x30 (compatible with the ipq806x SS PHY).
The HS PHY reference is the 60 MHz mock UTMI clock plus an RPM vote
keeping the XO alive; the SS PHY takes its reference from the pads,
driven by the PMIC differential XO buffer (diff_clk), matching the
ref_clk the vendor kernel hands to the DWC3 wrapper.

Both PHYs must be described and initialized even for high-speed-only
use: leaving the SS PHY dead does not just cap the speed, it confuses
the upstream-facing state machine of at least the TUSB8040A hub on the
APQ8074 dragonboard, which then connects but never answers enumeration
in any speed configuration.

Assisted-by: Claude:claude-fable-5
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 arch/arm/boot/dts/qcom/qcom-msm8974.dtsi | 47 ++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/arch/arm/boot/dts/qcom/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom/qcom-msm8974.dtsi
index 06c0af0aa317..bac3c8772e0a 100644
--- a/arch/arm/boot/dts/qcom/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom/qcom-msm8974.dtsi
@@ -713,6 +713,53 @@ blsp2_i2c6: i2c@f9968000 {
 			#size-cells = <0>;
 		};
 
+		usb3: usb@f9200000 {
+			compatible = "qcom,msm8974-dwc3", "qcom,snps-dwc3";
+			reg = <0xf9200000 0xfc000>;
+			interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 180 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "dwc_usb3", "pwr_event",
+					  "hs_phy_irq";
+			clocks = <&gcc GCC_SYS_NOC_USB3_AXI_CLK>,
+				 <&gcc GCC_USB30_MASTER_CLK>,
+				 <&gcc GCC_USB30_SLEEP_CLK>,
+				 <&gcc GCC_USB30_MOCK_UTMI_CLK>;
+			clock-names = "cfg_noc", "core", "sleep", "mock_utmi";
+			assigned-clocks = <&gcc GCC_USB30_MOCK_UTMI_CLK>,
+					  <&gcc GCC_USB30_MASTER_CLK>;
+			assigned-clock-rates = <60000000>, <125000000>;
+			resets = <&gcc GCC_USB_30_BCR>;
+			phys = <&usb3_hs_phy>, <&usb3_ss_phy>;
+			phy-names = "usb2-phy", "usb3-phy";
+			snps,dis_u2_susphy_quirk;
+			snps,dis_enblslpm_quirk;
+			status = "disabled";
+		};
+
+		usb3_hs_phy: phy@f92f8800 {
+			compatible = "qcom,msm8974-usb-phy-hs";
+			reg = <0xf92f8800 0x30>;
+			clocks = <&gcc GCC_USB30_MOCK_UTMI_CLK>,
+				 <&rpmcc RPM_SMD_XO_CLK_SRC>;
+			clock-names = "ref", "xo";
+			#phy-cells = <0>;
+			status = "disabled";
+		};
+
+		usb3_ss_phy: phy@f92f8830 {
+			compatible = "qcom,msm8974-usb-phy-ss",
+				     "qcom,ipq806x-usb-phy-ss";
+			reg = <0xf92f8830 0x30>;
+			clocks = <&gcc GCC_USB30_MASTER_CLK>,
+				 <&rpmcc RPM_SMD_DIFF_CLK>;
+			clock-names = "ref", "xo";
+			#phy-cells = <0>;
+			qcom,rx-eq = <3>;
+			qcom,tx-deamp-3_5db = <22>;
+			status = "disabled";
+		};
+
 		usb: usb@f9a55000 {
 			compatible = "qcom,ci-hdrc";
 			reg = <0xf9a55000 0x200>,

-- 
2.47.3

Re: [PATCH 09/10] ARM: dts: qcom: msm8974: add the USB3 controller and its PHYs

[sashiko-bot]

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

New issues:
- [Low] DT Schema violation: the `compatible` property for `usb3_ss_phy` uses an array of two strings instead of a single string.

Pre-existing issues:
- [High] Data race on physical PHY control register between two uncoordinated drivers.
--

--- Patch [9]: [PATCH 09/10] ARM: dts: qcom: msm8974: add the USB3 controller and its PHYs ---
commit 688cb2c7a84a066e54c52b8eab7f37e81311d8fb
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

ARM: dts: qcom: msm8974: add the USB3 controller and its PHYs

This commit describes the primary USB3 controller (DWC3) and its High-Speed
and SuperSpeed PHYs for the MSM8974 SoC. It initializes both PHYs to ensure
proper upstream enumeration.

> diff --git a/arch/arm/boot/dts/qcom/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom/qcom-msm8974.dtsi
> index 06c0af0aa3176..bac3c8772e0a2 100644
> --- a/arch/arm/boot/dts/qcom/qcom-msm8974.dtsi
> +++ b/arch/arm/boot/dts/qcom/qcom-msm8974.dtsi

[ ... ]

> @@ -713,6 +713,53 @@ blsp2_i2c6: i2c@f9968000 {
>  			#size-cells = <0>;
>  		};
>  
> +		usb3: usb@f9200000 {
> +			compatible = "qcom,msm8974-dwc3", "qcom,snps-dwc3";
> +			reg = <0xf9200000 0xfc000>;

[Severity: High]
This isn't a bug introduced by this patch, but does mapping this region
expose a data race on the physical PHY control register?

The dwc3-qcom wrapper driver maps this region, calculating qscratch_base
as 0xf92f8800, and performs read-modify-write operations on offset 0x30
(physical 0xf92f8830) during role switches or gadget start/stop.

Simultaneously, the newly bound phy-qcom-ipq806x-usb driver maps the
exact same physical address bypassing exclusivity in the usb3_ss_phy
node below.

[ ... ]

> +		usb3_ss_phy: phy@f92f8830 {
> +			compatible = "qcom,msm8974-usb-phy-ss",
> +				     "qcom,ipq806x-usb-phy-ss";

[Severity: Low]
Will this compatible array cause make dtbs_check validation to fail?

The DT binding schema qcom,ipq806x-usb-phy-ss.yaml strictly defines the
compatible property as either "qcom,ipq806x-usb-phy-ss" or
"qcom,msm8974-usb-phy-ss". 

Providing a string array of both violates this definition.

> +			reg = <0xf92f8830 0x30>;

[Severity: High]
This is a pre-existing issue, but returning to the mapping overlap, the
SuperSpeed PHY driver explicitly maps the exact same physical address
(0xf92f8830) here and modifies its offset 0x00 without coordination.

Since both drivers perform uncoordinated read-modify-write operations
(sometimes with large delays like msleep(30) mid-RMW in the PHY driver),
can concurrent operations clobber the physical register state?

This could lose bits like SSUSB_CTRL_SS_PHY_EN or LANE0_PWR_PRESENT,
corrupting the power state and breaking USB enumeration during concurrent
plug/unplug events and runtime PM transitions.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810-msm8974-usb3-v1-0-1a47e1185524@oss.qualcomm.com?part=9

[PATCH 10/10] ARM: dts: qcom: apq8074-dragonboard: enable the USB3 host

[Dmitry Baryshkov]

The USB3 port of the dragonboard drives the onboard TUSB8040A hub with
the AX88179 GbE controller and the PL2775 USB-SATA bridge behind it,
but the controller is disabled and nothing describes the devices, so
their reset lines float and their supplies are unmanaged.

Enable the DWC3 controller in host mode together with both of its
PHYs, and describe the onboard chain as device nodes under the
controller: the TUSB8040A hub pair (with the EEPROM-programmed
0451:8041/0451:8043 IDs this board reports), the AX88179 on the DN2
port and the PL2775 on the DN0 port, each with its PM8941 reset GPIO,
all powered from the always-on 3.3V carrier board rail, modeled as a
fixed regulator.

The SuperSpeed PHY is required even for the hub's USB 2.0 half to
respond: with the SS PHY left uninitialized the hub connects but never
answers enumeration at any speed.

Assisted-by: Claude:claude-fable-5
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 .../arm/boot/dts/qcom/qcom-apq8074-dragonboard.dts | 61 ++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/arch/arm/boot/dts/qcom/qcom-apq8074-dragonboard.dts b/arch/arm/boot/dts/qcom/qcom-apq8074-dragonboard.dts
index d3ae6c6a6f83..083fd8fa0aea 100644
--- a/arch/arm/boot/dts/qcom/qcom-apq8074-dragonboard.dts
+++ b/arch/arm/boot/dts/qcom/qcom-apq8074-dragonboard.dts
@@ -50,6 +50,15 @@ mpss_region: mpss@ac00000 {
 		};
 	};
 
+	pwr_c_3p3: regulator-pwr-c-3p3 {
+		compatible = "regulator-fixed";
+		regulator-name = "pwr_c_3p3";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-always-on;
+		regulator-boot-on;
+	};
+
 	vreg_boost: vreg-boost {
 		compatible = "regulator-fixed";
 
@@ -477,3 +486,55 @@ &usb_hs2_phy {
 	extcon = <&smbb>;
 	qcom,init-seq = /bits/ 8 <0x1 0x63>;
 };
+
+&usb3 {
+	dr_mode = "host";
+
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	status = "okay";
+
+	/* TI TUSB8040A four port hub */
+	hub_2_0: hub@1 {
+		compatible = "usb451,8043";
+		reg = <1>;
+		peer-hub = <&hub_3_0>;
+		reset-gpios = <&pm8941_gpios 6 GPIO_ACTIVE_LOW>;
+		vdd-supply = <&pwr_c_3p3>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		/* Prolific PL2775 USB-SATA bridge on the DN0 port */
+		device@1 {
+			compatible = "usb67b,2775";
+			reg = <1>;
+			reset-gpios = <&pm8941_gpios 10 GPIO_ACTIVE_LOW>;
+			vdd-supply = <&pwr_c_3p3>;
+		};
+
+		/* ASIX AX88179 USB-GbE controller on the DN2 port */
+		ethernet@3 {
+			compatible = "usbb95,1790";
+			reg = <3>;
+			reset-gpios = <&pm8941_gpios 9 GPIO_ACTIVE_LOW>;
+			vdd-supply = <&pwr_c_3p3>;
+		};
+	};
+
+	hub_3_0: hub@2 {
+		compatible = "usb451,8041";
+		reg = <2>;
+		peer-hub = <&hub_2_0>;
+		reset-gpios = <&pm8941_gpios 6 GPIO_ACTIVE_LOW>;
+		vdd-supply = <&pwr_c_3p3>;
+	};
+};
+
+&usb3_hs_phy {
+	status = "okay";
+};
+
+&usb3_ss_phy {
+	status = "okay";
+};

-- 
2.47.3

Re: [PATCH 10/10] ARM: dts: qcom: apq8074-dragonboard: enable the USB3 host

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] The `reset-gpios` and `vdd-supply` properties for the USB hubs and peripherals will be completely ignored because their compatible strings are not supported by the kernel's onboard USB dev driver.
- [High] `qcom_ipq806x_usb_hs_phy_init()` overwrites the entire `HSUSB_PHY_CTRL_REG`, destroying bootloader-configured frequency straps for MSM8974.
- [High] SuperSpeed USB devices (AX88179 and PL2775) are incorrectly nested under the HighSpeed hub (`hub_2_0`) instead of the SuperSpeed hub (`hub_3_0`).
- [High] The SuperSpeed hub (`hub_3_0`) is assigned an invalid port address (`reg = <2>`), breaking DT node mapping for both root hubs.
--

--- Patch [10]: [PATCH 10/10] ARM: dts: qcom: apq8074-dragonboard: enable the USB3 host ---
commit 316b8b1bbfd2cabe66fe0293f9ed61e3a48ec197
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

ARM: dts: qcom: apq8074-dragonboard: enable the USB3 host

The USB3 port of the dragonboard drives the onboard TUSB8040A hub with
the AX88179 GbE controller and the PL2775 USB-SATA bridge behind it.
This commit enables the DWC3 controller in host mode along with its PHYs
and describes the onboard chain as device nodes under the controller.

> diff --git a/arch/arm/boot/dts/qcom/qcom-apq8074-dragonboard.dts b/arch/arm/boot/dts/qcom/qcom-apq8074-dragonboard.dts
> --- a/arch/arm/boot/dts/qcom/qcom-apq8074-dragonboard.dts
> +++ b/arch/arm/boot/dts/qcom/qcom-apq8074-dragonboard.dts

[ ... ]

> +	/* TI TUSB8040A four port hub */
> +	hub_2_0: hub@1 {
> +		compatible = "usb451,8043";

[Severity: High]
Will the reset-gpios and vdd-supply properties be ignored here?

The new compatible strings (usb451,8043, usb451,8041, usb67b,2775,
usbb95,1790) do not appear to be supported by the onboard_usb_dev driver's
onboard_dev_match array. The generic USB core parses these nodes but does
not natively handle reset-gpios.

Could this cause the reset lines to float and power sequencing to be
unmanaged, preventing proper enumeration?

> +		reg = <1>;
> +		peer-hub = <&hub_3_0>;
> +		reset-gpios = <&pm8941_gpios 6 GPIO_ACTIVE_LOW>;
> +		vdd-supply = <&pwr_c_3p3>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		/* Prolific PL2775 USB-SATA bridge on the DN0 port */
> +		device@1 {
> +			compatible = "usb67b,2775";

[Severity: High]
Are the SuperSpeed USB devices (AX88179 and PL2775) nested under the
incorrect hub here?

Since these are SuperSpeed capable devices, they will enumerate on the
SuperSpeed bus. By placing them under the HighSpeed hub (hub_2_0) instead
of the SuperSpeed hub (hub_3_0), the USB core won't be able to match the
enumerated device to a DT child.

Will this cause device-specific DT properties like reset-gpios to be
silently ignored during USB enumeration?

> +			reg = <1>;
> +			reset-gpios = <&pm8941_gpios 10 GPIO_ACTIVE_LOW>;
> +			vdd-supply = <&pwr_c_3p3>;
> +		};
> +
> +		/* ASIX AX88179 USB-GbE controller on the DN2 port */
> +		ethernet@3 {
> +			compatible = "usbb95,1790";
> +			reg = <3>;
> +			reset-gpios = <&pm8941_gpios 9 GPIO_ACTIVE_LOW>;
> +			vdd-supply = <&pwr_c_3p3>;
> +		};
> +	};
> +
> +	hub_3_0: hub@2 {
> +		compatible = "usb451,8041";
> +		reg = <2>;

[Severity: High]
Is this port address valid for the SuperSpeed hub?

The APQ8074 DWC3 controller has a single physical port, meaning both root hubs
should expose exactly one port (port 1). Because this uses reg = <2>, the
USB 3.0 root hub will look for port 1 and instead find hub_2_0 (which has
reg = <1>).

Does this leave hub_3_0 orphaned and break DT node mapping?

> +		peer-hub = <&hub_2_0>;
> +		reset-gpios = <&pm8941_gpios 6 GPIO_ACTIVE_LOW>;
> +		vdd-supply = <&pwr_c_3p3>;
> +	};
> +};

A related issue appears to exist in the PHY initialization sequence for this
hardware:

drivers/phy/qualcomm/phy-qcom-ipq806x-usb.c:qcom_ipq806x_usb_hs_phy_init() {
    ...
    writel(val, phy_dwc3->base + HSUSB_PHY_CTRL_REG);
    ...
}

[Severity: High]
Does qcom_ipq806x_usb_hs_phy_init() overwrite the entire HSUSB_PHY_CTRL_REG?

If the MSM8974 HS PHY probe reuses this function, calling writel() without a
prior readl() will destroy the bootloader-configured reference frequency
straps. This seems to contradict the requirement to preserve these strap
bits.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810-msm8974-usb3-v1-0-1a47e1185524@oss.qualcomm.com?part=10