[PATCH v2 02/13] dt-bindings: phy: Add binding for QCS615 standalone QMP DP PHY

Xiangxu Yin posted 13 patches 2 months, 2 weeks ago
There is a newer version of this series
[PATCH v2 02/13] dt-bindings: phy: Add binding for QCS615 standalone QMP DP PHY
Posted by Xiangxu Yin 2 months, 2 weeks ago
Introduce device tree binding documentation for the Qualcomm QMP DP PHY
on QCS615 SoCs. This PHY supports DisplayPort functionality and is
designed to operate independently from the USB3 PHY.

Unlike combo PHYs found on other platforms, the QCS615 DP PHY is
standalone and does not support USB/DP multiplexing. The binding
describes the required clocks, resets, TCSR configuration, and clock/PHY
cells for proper integration.

Signed-off-by: Xiangxu Yin <xiangxu.yin@oss.qualcomm.com>
---
 .../bindings/phy/qcom,qcs615-qmp-dp-phy.yaml       | 111 +++++++++++++++++++++
 1 file changed, 111 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/qcom,qcs615-qmp-dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,qcs615-qmp-dp-phy.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..17e37c1df7b61dc2f7aa35ee106fd94ee2829c5f
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/qcom,qcs615-qmp-dp-phy.yaml
@@ -0,0 +1,111 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/qcom,qcs615-qmp-dp-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm QMP PHY controller (DP, QCS615)
+
+maintainers:
+  - Vinod Koul <vkoul@kernel.org>
+
+description:
+  The QMP DP PHY controller supports DisplayPort physical layer functionality
+  on Qualcomm QCS615 SoCs. This PHY is independent from USB3 PHY and does not
+  support combo mode.
+
+properties:
+  compatible:
+    enum:
+      - qcom,qcs615-qmp-dp-phy
+
+  reg:
+    maxItems: 4
+
+  clocks:
+    maxItems: 2
+
+  clock-names:
+    items:
+      - const: cfg_ahb
+      - const: ref
+
+  clock-output-names:
+    maxItems: 2
+    description:
+      Names of the clocks provided by the PHY.
+
+  qcom,tcsr-reg:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    items:
+      - items:
+          - description: phandle to TCSR hardware block
+          - description: offset of the DP PHY moode register
+    description:
+      DP PHY moode register present in the TCSR
+
+  resets:
+    maxItems: 1
+
+  reset-names:
+    items:
+      - const: phy
+
+  vdda-phy-supply: true
+
+  vdda-pll-supply: true
+
+  "#clock-cells":
+    const: 1
+    description:
+      See include/dt-bindings/phy/phy-qcom-qmp.h
+
+  "#phy-cells":
+    const: 1
+    description:
+      See include/dt-bindings/phy/phy-qcom-qmp.h
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - clock-output-names
+  - qcom,tcsr-reg
+  - resets
+  - reset-names
+  - vdda-phy-supply
+  - vdda-pll-supply
+  - "#clock-cells"
+  - "#phy-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,qcs615-gcc.h>
+    #include <dt-bindings/clock/qcom,rpmh.h>
+
+    phy@88e9000 {
+      compatible = "qcom,qcs615-qmp-dp-phy";
+      reg = <0x088e9000 0x200>,
+            <0x088e9400 0x10c>,
+            <0x088e9800 0x10c>,
+            <0x088e9c00 0x200>;
+
+      clocks = <&gcc GCC_AHB2PHY_WEST_CLK>,
+               <&gcc GCC_USB3_SEC_CLKREF_CLK>;
+      clock-names = "cfg_ahb", "ref";
+      clock-output-names = "dp_phy_link_clk", "dp_phy_vco_div_clk";
+
+      qcom,tcsr-reg = <&tcsr 0xb24c>;
+
+      resets = <&gcc GCC_USB3_DP_PHY_SEC_BCR>;
+      reset-names = "phy";
+
+      vdda-phy-supply = <&vreg_l11a>;
+      vdda-pll-supply = <&vreg_l5a>;
+
+      #clock-cells = <1>;
+      #phy-cells = <1>;
+    };

-- 
2.34.1
Re: [PATCH v2 02/13] dt-bindings: phy: Add binding for QCS615 standalone QMP DP PHY
Posted by Krzysztof Kozlowski 2 months, 2 weeks ago
On 22/07/2025 09:22, Xiangxu Yin wrote:
> Introduce device tree binding documentation for the Qualcomm QMP DP PHY
> on QCS615 SoCs. This PHY supports DisplayPort functionality and is
> designed to operate independently from the USB3 PHY.

A nit, subject: drop second/last, redundant "binding for". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

> 
> Unlike combo PHYs found on other platforms, the QCS615 DP PHY is
> standalone and does not support USB/DP multiplexing. The binding
> describes the required clocks, resets, TCSR configuration, and clock/PHY
> cells for proper integration.
> 
> Signed-off-by: Xiangxu Yin <xiangxu.yin@oss.qualcomm.com>
> ---
>  .../bindings/phy/qcom,qcs615-qmp-dp-phy.yaml       | 111 +++++++++++++++++++++
>  1 file changed, 111 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/qcom,qcs615-qmp-dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,qcs615-qmp-dp-phy.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..17e37c1df7b61dc2f7aa35ee106fd94ee2829c5f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom,qcs615-qmp-dp-phy.yaml
> @@ -0,0 +1,111 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/qcom,qcs615-qmp-dp-phy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm QMP PHY controller (DP, QCS615)

That's too vague title. You are not adding here Qualcomm QMP PHY
controllers.

> +
> +maintainers:
> +  - Vinod Koul <vkoul@kernel.org>

Hm? Why?

> +
> +description:
> +  The QMP DP PHY controller supports DisplayPort physical layer functionality
> +  on Qualcomm QCS615 SoCs. This PHY is independent from USB3 PHY and does not
> +  support combo mode.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,qcs615-qmp-dp-phy
> +
> +  reg:
> +    maxItems: 4

I don't understand what you are doing here. Why previous patch evolved
into this? Where is any reasoning for that in the changelog? You said:

"- Add new binding qcom,qcs615-qmp-dp-phy.yaml for QCS615 standalone DP
[Krzysztof]"

but you must say WHY you are doing things...

Anyway, missing constraints. Look at other Qualcomm bindings.

> +
> +  clocks:
> +    maxItems: 2
> +
> +  clock-names:
> +    items:
> +      - const: cfg_ahb
> +      - const: ref
> +
> +  clock-output-names:
> +    maxItems: 2
> +    description:
> +      Names of the clocks provided by the PHY.

Drop description, redundant. It cannot be anything else.

> +
> +  qcom,tcsr-reg:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    items:
> +      - items:
> +          - description: phandle to TCSR hardware block
> +          - description: offset of the DP PHY moode register
> +    description:
> +      DP PHY moode register present in the TCSR
> +
> +  resets:
> +    maxItems: 1
> +
> +  reset-names:
> +    items:
> +      - const: phy

Drop reset-names, useless.

> +
> +  vdda-phy-supply: true
> +
> +  vdda-pll-supply: true
> +
> +  "#clock-cells":
> +    const: 1
> +    description:
> +      See include/dt-bindings/phy/phy-qcom-qmp.h
> +
> +  "#phy-cells":
> +    const: 1
> +    description:
> +      See include/dt-bindings/phy/phy-qcom-qmp.h
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - clock-output-names
> +  - qcom,tcsr-reg
> +  - resets
> +  - reset-names
> +  - vdda-phy-supply
> +  - vdda-pll-supply
> +  - "#clock-cells"
> +  - "#phy-cells"
> +
Why introducing completely different order? See existing binding and DTS
coding style.

Best regards,
Krzysztof
Re: [PATCH v2 02/13] dt-bindings: phy: Add binding for QCS615 standalone QMP DP PHY
Posted by Xiangxu Yin 1 month, 3 weeks ago
On 7/22/2025 5:18 PM, Krzysztof Kozlowski wrote:
> On 22/07/2025 09:22, Xiangxu Yin wrote:
>> Introduce device tree binding documentation for the Qualcomm QMP DP PHY
>> on QCS615 SoCs. This PHY supports DisplayPort functionality and is
>> designed to operate independently from the USB3 PHY.
> A nit, subject: drop second/last, redundant "binding for". The
> "dt-bindings" prefix is already stating that these are bindings.
> See also:
> https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

Ok, will update subject in next patch.

>> Unlike combo PHYs found on other platforms, the QCS615 DP PHY is
>> standalone and does not support USB/DP multiplexing. The binding
>> describes the required clocks, resets, TCSR configuration, and clock/PHY
>> cells for proper integration.
>>
>> Signed-off-by: Xiangxu Yin <xiangxu.yin@oss.qualcomm.com>
>> ---
>>  .../bindings/phy/qcom,qcs615-qmp-dp-phy.yaml       | 111 +++++++++++++++++++++
>>  1 file changed, 111 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/qcom,qcs615-qmp-dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,qcs615-qmp-dp-phy.yaml
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..17e37c1df7b61dc2f7aa35ee106fd94ee2829c5f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/qcom,qcs615-qmp-dp-phy.yaml
>> @@ -0,0 +1,111 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/phy/qcom,qcs615-qmp-dp-phy.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm QMP PHY controller (DP, QCS615)
> That's too vague title. You are not adding here Qualcomm QMP PHY
> controllers.

Will update to 'Qualcomm QMP USB3-DP PHY controller (DP, QCS615)' in next patch.

>
>> +
>> +maintainers:
>> +  - Vinod Koul <vkoul@kernel.org>
> Hm? Why?


I referred to the definitions in qcom,msm8998-qmp-usb3-phy.yaml and qcom,sc8280xp-qmp-usb43dp-phy.yaml.

I also found that Vinod Koul is listed as the maintainer of the GENERIC PHY FRAMEWORK in linux-next/MAINTAINERS.

Did I misunderstand anything here?


>> +
>> +description:
>> +  The QMP DP PHY controller supports DisplayPort physical layer functionality
>> +  on Qualcomm QCS615 SoCs. This PHY is independent from USB3 PHY and does not
>> +  support combo mode.
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - qcom,qcs615-qmp-dp-phy
>> +
>> +  reg:
>> +    maxItems: 4
> I don't understand what you are doing here. Why previous patch evolved
> into this? Where is any reasoning for that in the changelog? You said:
>
> "- Add new binding qcom,qcs615-qmp-dp-phy.yaml for QCS615 standalone DP
> [Krzysztof]"
>
> but you must say WHY you are doing things...
>
> Anyway, missing constraints. Look at other Qualcomm bindings.


I misunderstood your earlier comment in '20241129-add-displayport-support-for-qcs615-platform-v1-2-09a4338d93ef@quicinc.com' and mistakenly included "[Krzysztof]" in the commit message as if it were a quote. 

Apologies for the confusion and will drop this part in cover letter in next patch.

In next patch, the address regions will be consolidated into a single range, similar to other QMP PHYs, and |maxItems| will be updated to |1|.


>
>> +
>> +  clocks:
>> +    maxItems: 2
>> +
>> +  clock-names:
>> +    items:
>> +      - const: cfg_ahb
>> +      - const: ref
>> +
>> +  clock-output-names:
>> +    maxItems: 2
>> +    description:
>> +      Names of the clocks provided by the PHY.
> Drop description, redundant. It cannot be anything else.


Ok, will drop it.


>
>> +
>> +  qcom,tcsr-reg:
>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>> +    items:
>> +      - items:
>> +          - description: phandle to TCSR hardware block
>> +          - description: offset of the DP PHY moode register
>> +    description:
>> +      DP PHY moode register present in the TCSR
>> +
>> +  resets:
>> +    maxItems: 1
>> +
>> +  reset-names:
>> +    items:
>> +      - const: phy
> Drop reset-names, useless.


In next patch, this config will be updated to USB or DP PHY binding with two resets: 'phy_phy' for USB and 'dp_phy' for DP.
Shall I keep 'reset-names' prop here?

>
>> +
>> +  vdda-phy-supply: true
>> +
>> +  vdda-pll-supply: true
>> +
>> +  "#clock-cells":
>> +    const: 1
>> +    description:
>> +      See include/dt-bindings/phy/phy-qcom-qmp.h
>> +
>> +  "#phy-cells":
>> +    const: 1
>> +    description:
>> +      See include/dt-bindings/phy/phy-qcom-qmp.h
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - clocks
>> +  - clock-names
>> +  - clock-output-names
>> +  - qcom,tcsr-reg
>> +  - resets
>> +  - reset-names
>> +  - vdda-phy-supply
>> +  - vdda-pll-supply
>> +  - "#clock-cells"
>> +  - "#phy-cells"
>> +
> Why introducing completely different order? See existing binding and DTS
> coding style.
>
> Best regards,
> Krzysztof


Ok, will update to follow the existing order.

compatible
reg
clocks
clock-names
resets
reset-names
vdda-phy-supply
vdda-pll-supply
#clock-cells
#phy-cells
qcom,tcsr-reg

Re: [PATCH v2 02/13] dt-bindings: phy: Add binding for QCS615 standalone QMP DP PHY
Posted by Dmitry Baryshkov 2 months, 2 weeks ago
On Tue, Jul 22, 2025 at 03:22:03PM +0800, Xiangxu Yin wrote:
> Introduce device tree binding documentation for the Qualcomm QMP DP PHY
> on QCS615 SoCs. This PHY supports DisplayPort functionality and is
> designed to operate independently from the USB3 PHY.
> 
> Unlike combo PHYs found on other platforms, the QCS615 DP PHY is
> standalone and does not support USB/DP multiplexing. The binding
> describes the required clocks, resets, TCSR configuration, and clock/PHY
> cells for proper integration.

Simply put: no, this is not correct. Even if you go to the SM6150 block
diagram, it points out that DP uses the USB3 PHY, not a separate DP PHY.

I thought that we have discussed it beforehand.

I can quote my comment from the previous thread:

>> No. It means replacing extending existing entries with bigger reg and
>> #phy-cells = <1>. The driver must keep working with old node definitions
>> as is to ensure backwards compatibility. New nodes should make it
>> register two PHYs (USB3 and DP). On the driver side modify generic code
>> paths, all platforms supported by the driver should be able to support
>> USB3+DP combination.

Looking at the hardware memory maps:

MSM8998: USB3 PHY regs at 0xc010000, DP PHY regs at 0xc011000
SDM660: USB3 PHY regs at 0xc010000, DP PHY regs at 0xc011000
QCM2290: USB3 PHY regs at 0x1615000, DP PHY regs at 0x1616000
SM6115: USB3 PHY regs at 0x1615000, DP PHY regs at 0x1616000

Now:
SM6150: USB3 PHY regs at 0x88e6000
        USB3 PHY regs at 0x88e8000, DP PHY regs at 0x88e9000

I do not know, why msm-4.14 didn't describe second USB3 PHY. Maybe you
can comment on it.

But based on that list, the only special case that we need to handle is
the first USB3 PHY, which doesn't have a corresponding DP PHY block. But
it will be handled anyway by the code that implements support for the
existing DT entries. All other hardware blocks are combo USB+DP PHYs.

Having all of that in mind, please, for v3 patchset implement USB+DP
support in the phy-qcom-qmp-usbc driver and add the following logic
that also was requested in v1 review:

>> Not quite. Both USB3 and DP drivers should be calling power_on / _off.
>> If USB3 is on, powering on DP PHY should fail. Vice versa, if DP is on,
>> powering on USB should fail.

> 
> Signed-off-by: Xiangxu Yin <xiangxu.yin@oss.qualcomm.com>
> ---
>  .../bindings/phy/qcom,qcs615-qmp-dp-phy.yaml       | 111 +++++++++++++++++++++
>  1 file changed, 111 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/qcom,qcs615-qmp-dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,qcs615-qmp-dp-phy.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..17e37c1df7b61dc2f7aa35ee106fd94ee2829c5f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom,qcs615-qmp-dp-phy.yaml
> @@ -0,0 +1,111 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/qcom,qcs615-qmp-dp-phy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm QMP PHY controller (DP, QCS615)
> +
> +maintainers:
> +  - Vinod Koul <vkoul@kernel.org>
> +
> +description:
> +  The QMP DP PHY controller supports DisplayPort physical layer functionality
> +  on Qualcomm QCS615 SoCs. This PHY is independent from USB3 PHY and does not
> +  support combo mode.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,qcs615-qmp-dp-phy
> +
> +  reg:
> +    maxItems: 4
> +
> +  clocks:
> +    maxItems: 2
> +
> +  clock-names:
> +    items:
> +      - const: cfg_ahb
> +      - const: ref
> +
> +  clock-output-names:
> +    maxItems: 2
> +    description:
> +      Names of the clocks provided by the PHY.
> +
> +  qcom,tcsr-reg:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    items:
> +      - items:
> +          - description: phandle to TCSR hardware block
> +          - description: offset of the DP PHY moode register
> +    description:
> +      DP PHY moode register present in the TCSR
> +
> +  resets:
> +    maxItems: 1
> +
> +  reset-names:
> +    items:
> +      - const: phy
> +
> +  vdda-phy-supply: true
> +
> +  vdda-pll-supply: true
> +
> +  "#clock-cells":
> +    const: 1
> +    description:
> +      See include/dt-bindings/phy/phy-qcom-qmp.h
> +
> +  "#phy-cells":
> +    const: 1
> +    description:
> +      See include/dt-bindings/phy/phy-qcom-qmp.h
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - clock-output-names
> +  - qcom,tcsr-reg
> +  - resets
> +  - reset-names
> +  - vdda-phy-supply
> +  - vdda-pll-supply
> +  - "#clock-cells"
> +  - "#phy-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/qcom,qcs615-gcc.h>
> +    #include <dt-bindings/clock/qcom,rpmh.h>
> +
> +    phy@88e9000 {
> +      compatible = "qcom,qcs615-qmp-dp-phy";
> +      reg = <0x088e9000 0x200>,
> +            <0x088e9400 0x10c>,
> +            <0x088e9800 0x10c>,
> +            <0x088e9c00 0x200>;
> +
> +      clocks = <&gcc GCC_AHB2PHY_WEST_CLK>,
> +               <&gcc GCC_USB3_SEC_CLKREF_CLK>;
> +      clock-names = "cfg_ahb", "ref";
> +      clock-output-names = "dp_phy_link_clk", "dp_phy_vco_div_clk";
> +
> +      qcom,tcsr-reg = <&tcsr 0xb24c>;
> +
> +      resets = <&gcc GCC_USB3_DP_PHY_SEC_BCR>;
> +      reset-names = "phy";
> +
> +      vdda-phy-supply = <&vreg_l11a>;
> +      vdda-pll-supply = <&vreg_l5a>;
> +
> +      #clock-cells = <1>;
> +      #phy-cells = <1>;
> +    };
> 
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry
Re: [PATCH v2 02/13] dt-bindings: phy: Add binding for QCS615 standalone QMP DP PHY
Posted by Xiangxu Yin 2 months, 2 weeks ago
On 7/22/2025 4:38 PM, Dmitry Baryshkov wrote:
> On Tue, Jul 22, 2025 at 03:22:03PM +0800, Xiangxu Yin wrote:
>> Introduce device tree binding documentation for the Qualcomm QMP DP PHY
>> on QCS615 SoCs. This PHY supports DisplayPort functionality and is
>> designed to operate independently from the USB3 PHY.
>>
>> Unlike combo PHYs found on other platforms, the QCS615 DP PHY is
>> standalone and does not support USB/DP multiplexing. The binding
>> describes the required clocks, resets, TCSR configuration, and clock/PHY
>> cells for proper integration.
> Simply put: no, this is not correct. Even if you go to the SM6150 block
> diagram, it points out that DP uses the USB3 PHY, not a separate DP PHY.
>
> I thought that we have discussed it beforehand.
>
> I can quote my comment from the previous thread:
>
>>> No. It means replacing extending existing entries with bigger reg and
>>> #phy-cells = <1>. The driver must keep working with old node definitions
>>> as is to ensure backwards compatibility. New nodes should make it
>>> register two PHYs (USB3 and DP). On the driver side modify generic code
>>> paths, all platforms supported by the driver should be able to support
>>> USB3+DP combination.
> Looking at the hardware memory maps:
>
> MSM8998: USB3 PHY regs at 0xc010000, DP PHY regs at 0xc011000
> SDM660: USB3 PHY regs at 0xc010000, DP PHY regs at 0xc011000
> QCM2290: USB3 PHY regs at 0x1615000, DP PHY regs at 0x1616000
> SM6115: USB3 PHY regs at 0x1615000, DP PHY regs at 0x1616000
>
> Now:
> SM6150: USB3 PHY regs at 0x88e6000
>         USB3 PHY regs at 0x88e8000, DP PHY regs at 0x88e9000
>
> I do not know, why msm-4.14 didn't describe second USB3 PHY. Maybe you
> can comment on it.
>
> But based on that list, the only special case that we need to handle is
> the first USB3 PHY, which doesn't have a corresponding DP PHY block. But
> it will be handled anyway by the code that implements support for the
> existing DT entries. All other hardware blocks are combo USB+DP PHYs.
>
> Having all of that in mind, please, for v3 patchset implement USB+DP
> support in the phy-qcom-qmp-usbc driver and add the following logic
> that also was requested in v1 review:
>
>>> Not quite. Both USB3 and DP drivers should be calling power_on / _off.
>>> If USB3 is on, powering on DP PHY should fail. Vice versa, if DP is on,
>>> powering on USB should fail.
> I think our understanding might not be fully aligned. 
> Perhaps this is because I didn’t accurately update the mutual exclusion relationships and test results for the different PHYs. 
> Let me clarify my latest findings and explain why I believe these are separate PHYs that require mutual exclusion via TCSR.
>
> 1. About the TCSR DP_PHYMODE Registers
>
> MSM8998/SDM660:
> 	Only one TCSR_USB3_DP_PHYMODE register at 0x1FCB248.
> QCM2290/SM6115:
> 	TCSR_USB3_0_DP_PHYMODE at 0x3CB248
> 	TCSR_USB3_1_DP_PHYMODE at 0x3CB24C
> SM6150:
> 	TCSR_USB3_0_DP_PHYMODE at 0x1FCB248
> 	TCSR_USB3_1_DP_PHYMODE at 0x1FCB24C
> Even though MSM8998, SDM660, QCM2290, and SM6115 all have one USB3 PHY and one DP PHY, the TCSR DP_PHYMODE register configuration is different on each platform.
>
> Additionally, I found some interesting register documentation for QCM2290/SM6115:
> 	TCSR_USB3_0_DP_PHYMODE: “In kamorta this one is for mobile usb. DP not supported.”
> 	TCSR_USB3_1_DP_PHYMODE: “DP mode supported for Auto usb in kamorta.”
> I think the reason for having two different TCSR registers is to allow both the USB3.0 and DP PHYs to be useds at the same time in certain product configurations.
>
> 2. SM6150 Test Results
> When TCSR_DP_PHYMODE_0 is switched to DP, the USB3 primary PHY cannot work, and the DP PHY is also not functional (possibly due to clock lack or other configuration mismatch with this TCSR setting).
> When TCSR_DP_PHYMODE_1 is switched to DP, both the USB3 primary PHY and the DP PHY work normally.
> I think "why msm-4.14 didn't describe second USB3 PHY", because TCSR_DP_PHYMODE_1 always works in DP mode.
> https://android.googlesource.com/kernel/msm/+/af03eef7d4c3cbd1fe26c67d4f1915b05d0c1488/drivers/gpu/drm/msm/dp/dp_catalog_v200.c
>
> Based on these info, I believe these are separate PHYs, and only the TCSR DP_PHYMODE registers determine which USB3/DP PHYs are paired or mutually exclusive. This is why I have maintained separate private data for each PHY and implemented Power on mutex control via TCSR, rather than using a qmp_combo-like structure.
>
> Given the above, do you think we still need to force USB and DP to be strictly bound together like a combo PHY?
>
>> Signed-off-by: Xiangxu Yin <xiangxu.yin@oss.qualcomm.com>
>> ---
>>  .../bindings/phy/qcom,qcs615-qmp-dp-phy.yaml       | 111 +++++++++++++++++++++
>>  1 file changed, 111 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/qcom,qcs615-qmp-dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,qcs615-qmp-dp-phy.yaml
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..17e37c1df7b61dc2f7aa35ee106fd94ee2829c5f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/qcom,qcs615-qmp-dp-phy.yaml
>> @@ -0,0 +1,111 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/phy/qcom,qcs615-qmp-dp-phy.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm QMP PHY controller (DP, QCS615)
>> +
>> +maintainers:
>> +  - Vinod Koul <vkoul@kernel.org>
>> +
>> +description:
>> +  The QMP DP PHY controller supports DisplayPort physical layer functionality
>> +  on Qualcomm QCS615 SoCs. This PHY is independent from USB3 PHY and does not
>> +  support combo mode.
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - qcom,qcs615-qmp-dp-phy
>> +
>> +  reg:
>> +    maxItems: 4
>> +
>> +  clocks:
>> +    maxItems: 2
>> +
>> +  clock-names:
>> +    items:
>> +      - const: cfg_ahb
>> +      - const: ref
>> +
>> +  clock-output-names:
>> +    maxItems: 2
>> +    description:
>> +      Names of the clocks provided by the PHY.
>> +
>> +  qcom,tcsr-reg:
>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>> +    items:
>> +      - items:
>> +          - description: phandle to TCSR hardware block
>> +          - description: offset of the DP PHY moode register
>> +    description:
>> +      DP PHY moode register present in the TCSR
>> +
>> +  resets:
>> +    maxItems: 1
>> +
>> +  reset-names:
>> +    items:
>> +      - const: phy
>> +
>> +  vdda-phy-supply: true
>> +
>> +  vdda-pll-supply: true
>> +
>> +  "#clock-cells":
>> +    const: 1
>> +    description:
>> +      See include/dt-bindings/phy/phy-qcom-qmp.h
>> +
>> +  "#phy-cells":
>> +    const: 1
>> +    description:
>> +      See include/dt-bindings/phy/phy-qcom-qmp.h
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - clocks
>> +  - clock-names
>> +  - clock-output-names
>> +  - qcom,tcsr-reg
>> +  - resets
>> +  - reset-names
>> +  - vdda-phy-supply
>> +  - vdda-pll-supply
>> +  - "#clock-cells"
>> +  - "#phy-cells"
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/clock/qcom,qcs615-gcc.h>
>> +    #include <dt-bindings/clock/qcom,rpmh.h>
>> +
>> +    phy@88e9000 {
>> +      compatible = "qcom,qcs615-qmp-dp-phy";
>> +      reg = <0x088e9000 0x200>,
>> +            <0x088e9400 0x10c>,
>> +            <0x088e9800 0x10c>,
>> +            <0x088e9c00 0x200>;
>> +
>> +      clocks = <&gcc GCC_AHB2PHY_WEST_CLK>,
>> +               <&gcc GCC_USB3_SEC_CLKREF_CLK>;
>> +      clock-names = "cfg_ahb", "ref";
>> +      clock-output-names = "dp_phy_link_clk", "dp_phy_vco_div_clk";
>> +
>> +      qcom,tcsr-reg = <&tcsr 0xb24c>;
>> +
>> +      resets = <&gcc GCC_USB3_DP_PHY_SEC_BCR>;
>> +      reset-names = "phy";
>> +
>> +      vdda-phy-supply = <&vreg_l11a>;
>> +      vdda-pll-supply = <&vreg_l5a>;
>> +
>> +      #clock-cells = <1>;
>> +      #phy-cells = <1>;
>> +    };
>>
>> -- 
>> 2.34.1
>>
Re: [PATCH v2 02/13] dt-bindings: phy: Add binding for QCS615 standalone QMP DP PHY
Posted by Dmitry Baryshkov 2 months, 2 weeks ago
On Tue, Jul 22, 2025 at 08:05:06PM +0800, Xiangxu Yin wrote:
> 
> On 7/22/2025 4:38 PM, Dmitry Baryshkov wrote:
> > On Tue, Jul 22, 2025 at 03:22:03PM +0800, Xiangxu Yin wrote:
> >> Introduce device tree binding documentation for the Qualcomm QMP DP PHY
> >> on QCS615 SoCs. This PHY supports DisplayPort functionality and is
> >> designed to operate independently from the USB3 PHY.
> >>
> >> Unlike combo PHYs found on other platforms, the QCS615 DP PHY is
> >> standalone and does not support USB/DP multiplexing. The binding
> >> describes the required clocks, resets, TCSR configuration, and clock/PHY
> >> cells for proper integration.
> > Simply put: no, this is not correct. Even if you go to the SM6150 block
> > diagram, it points out that DP uses the USB3 PHY, not a separate DP PHY.
> >
> > I thought that we have discussed it beforehand.
> >
> > I can quote my comment from the previous thread:
> >
> >>> No. It means replacing extending existing entries with bigger reg and
> >>> #phy-cells = <1>. The driver must keep working with old node definitions
> >>> as is to ensure backwards compatibility. New nodes should make it
> >>> register two PHYs (USB3 and DP). On the driver side modify generic code
> >>> paths, all platforms supported by the driver should be able to support
> >>> USB3+DP combination.
> > Looking at the hardware memory maps:
> >
> > MSM8998: USB3 PHY regs at 0xc010000, DP PHY regs at 0xc011000
> > SDM660: USB3 PHY regs at 0xc010000, DP PHY regs at 0xc011000
> > QCM2290: USB3 PHY regs at 0x1615000, DP PHY regs at 0x1616000
> > SM6115: USB3 PHY regs at 0x1615000, DP PHY regs at 0x1616000
> >
> > Now:
> > SM6150: USB3 PHY regs at 0x88e6000
> >         USB3 PHY regs at 0x88e8000, DP PHY regs at 0x88e9000
> >
> > I do not know, why msm-4.14 didn't describe second USB3 PHY. Maybe you
> > can comment on it.
> >
> > But based on that list, the only special case that we need to handle is
> > the first USB3 PHY, which doesn't have a corresponding DP PHY block. But
> > it will be handled anyway by the code that implements support for the
> > existing DT entries. All other hardware blocks are combo USB+DP PHYs.
> >
> > Having all of that in mind, please, for v3 patchset implement USB+DP
> > support in the phy-qcom-qmp-usbc driver and add the following logic
> > that also was requested in v1 review:
> >
> >>> Not quite. Both USB3 and DP drivers should be calling power_on / _off.
> >>> If USB3 is on, powering on DP PHY should fail. Vice versa, if DP is on,
> >>> powering on USB should fail.
> > I think our understanding might not be fully aligned. 

I did not write this. Please fix your mailer to quote messages properly.
As you are using Thunderbird, I'm not sure where the issue comes from.

Also please fix it to wrap your responses somwhere logically.

> > Perhaps this is because I didn’t accurately update the mutual exclusion relationships and test results for the different PHYs. 
> > Let me clarify my latest findings and explain why I believe these are separate PHYs that require mutual exclusion via TCSR.
> >
> > 1. About the TCSR DP_PHYMODE Registers
> >
> > MSM8998/SDM660:
> > 	Only one TCSR_USB3_DP_PHYMODE register at 0x1FCB248.
> > QCM2290/SM6115:
> > 	TCSR_USB3_0_DP_PHYMODE at 0x3CB248
> > 	TCSR_USB3_1_DP_PHYMODE at 0x3CB24C
> > SM6150:
> > 	TCSR_USB3_0_DP_PHYMODE at 0x1FCB248
> > 	TCSR_USB3_1_DP_PHYMODE at 0x1FCB24C

SM6150 has two different sets of output pins, so the first register
covers first set of SS lanes (which are routed to the documented SS
PHY), the second register covers the second set of SS lanes (which are
routed to the DP and secondary USB PHY).

I can only assume that the same configuration was supposed to be
applicable to QCM2290 / SM6115, but was later removed / disabled, while
the registers were kept in the TCSR block.

> > Even though MSM8998, SDM660, QCM2290, and SM6115 all have one USB3 PHY and one DP PHY, the TCSR DP_PHYMODE register configuration is different on each platform.
> >
> > Additionally, I found some interesting register documentation for QCM2290/SM6115:
> > 	TCSR_USB3_0_DP_PHYMODE: “In kamorta this one is for mobile usb. DP not supported.”
> > 	TCSR_USB3_1_DP_PHYMODE: “DP mode supported for Auto usb in kamorta.”
> > I think the reason for having two different TCSR registers is to allow both the USB3.0 and DP PHYs to be useds at the same time in certain product configurations.

Sure. One for the first PHY (USB), one for the second PHY (USB+DP).
If you check the memory map, you will find the second VLS CLAMP register
for the second USB PHY.

> >
> > 2. SM6150 Test Results
> > When TCSR_DP_PHYMODE_0 is switched to DP, the USB3 primary PHY cannot work, and the DP PHY is also not functional (possibly due to clock lack or other configuration mismatch with this TCSR setting).
> > When TCSR_DP_PHYMODE_1 is switched to DP, both the USB3 primary PHY and the DP PHY work normally.
> > I think "why msm-4.14 didn't describe second USB3 PHY", because TCSR_DP_PHYMODE_1 always works in DP mode.
> > https://android.googlesource.com/kernel/msm/+/af03eef7d4c3cbd1fe26c67d4f1915b05d0c1488/drivers/gpu/drm/msm/dp/dp_catalog_v200.c

Here it still programs the TCSR register.

> >
> > Based on these info, I believe these are separate PHYs, and only the
> > TCSR DP_PHYMODE registers determine which USB3/DP PHYs are paired or
> > mutually exclusive. This is why I have maintained separate private
> > data for each PHY and implemented Power on mutex control via TCSR,
> > rather than using a qmp_combo-like structure.

Still, no. Check the block diagram of SM6150.

> >
> > Given the above, do you think we still need to force USB and DP to be strictly bound together like a combo PHY?

Yes.

> >
> >> Signed-off-by: Xiangxu Yin <xiangxu.yin@oss.qualcomm.com>
> >> ---
> >>  .../bindings/phy/qcom,qcs615-qmp-dp-phy.yaml       | 111 +++++++++++++++++++++
> >>  1 file changed, 111 insertions(+)
> >>

-- 
With best wishes
Dmitry
Re: [PATCH v2 02/13] dt-bindings: phy: Add binding for QCS615 standalone QMP DP PHY
Posted by Xiangxu Yin 2 months, 1 week ago
On 7/22/2025 8:41 PM, Dmitry Baryshkov wrote:
> On Tue, Jul 22, 2025 at 08:05:06PM +0800, Xiangxu Yin wrote:
>> On 7/22/2025 4:38 PM, Dmitry Baryshkov wrote:
>>> On Tue, Jul 22, 2025 at 03:22:03PM +0800, Xiangxu Yin wrote:
>>>> Introduce device tree binding documentation for the Qualcomm QMP DP PHY
>>>> on QCS615 SoCs. This PHY supports DisplayPort functionality and is
>>>> designed to operate independently from the USB3 PHY.
>>>>
>>>> Unlike combo PHYs found on other platforms, the QCS615 DP PHY is
>>>> standalone and does not support USB/DP multiplexing. The binding
>>>> describes the required clocks, resets, TCSR configuration, and clock/PHY
>>>> cells for proper integration.
>>> Simply put: no, this is not correct. Even if you go to the SM6150 block
>>> diagram, it points out that DP uses the USB3 PHY, not a separate DP PHY.
>>>
>>> I thought that we have discussed it beforehand.
>>>
>>> I can quote my comment from the previous thread:
>>>
>>>>> No. It means replacing extending existing entries with bigger reg and
>>>>> #phy-cells = <1>. The driver must keep working with old node definitions
>>>>> as is to ensure backwards compatibility. New nodes should make it
>>>>> register two PHYs (USB3 and DP). On the driver side modify generic code
>>>>> paths, all platforms supported by the driver should be able to support
>>>>> USB3+DP combination.
>>> Looking at the hardware memory maps:
>>>
>>> MSM8998: USB3 PHY regs at 0xc010000, DP PHY regs at 0xc011000
>>> SDM660: USB3 PHY regs at 0xc010000, DP PHY regs at 0xc011000
>>> QCM2290: USB3 PHY regs at 0x1615000, DP PHY regs at 0x1616000
>>> SM6115: USB3 PHY regs at 0x1615000, DP PHY regs at 0x1616000
>>>
>>> Now:
>>> SM6150: USB3 PHY regs at 0x88e6000
>>>         USB3 PHY regs at 0x88e8000, DP PHY regs at 0x88e9000
>>>
>>> I do not know, why msm-4.14 didn't describe second USB3 PHY. Maybe you
>>> can comment on it.
>>>
>>> But based on that list, the only special case that we need to handle is
>>> the first USB3 PHY, which doesn't have a corresponding DP PHY block. But
>>> it will be handled anyway by the code that implements support for the
>>> existing DT entries. All other hardware blocks are combo USB+DP PHYs.
>>>
>>> Having all of that in mind, please, for v3 patchset implement USB+DP
>>> support in the phy-qcom-qmp-usbc driver and add the following logic
>>> that also was requested in v1 review:
>>>
>>>>> Not quite. Both USB3 and DP drivers should be calling power_on / _off.
>>>>> If USB3 is on, powering on DP PHY should fail. Vice versa, if DP is on,
>>>>> powering on USB should fail.
>>> I think our understanding might not be fully aligned. 
> I did not write this. Please fix your mailer to quote messages properly.
> As you are using Thunderbird, I'm not sure where the issue comes from.
>
> Also please fix it to wrap your responses somwhere logically.
>
>>> Perhaps this is because I didn’t accurately update the mutual exclusion relationships and test results for the different PHYs. 
>>> Let me clarify my latest findings and explain why I believe these are separate PHYs that require mutual exclusion via TCSR.
>>>
>>> 1. About the TCSR DP_PHYMODE Registers
>>>
>>> MSM8998/SDM660:
>>> 	Only one TCSR_USB3_DP_PHYMODE register at 0x1FCB248.
>>> QCM2290/SM6115:
>>> 	TCSR_USB3_0_DP_PHYMODE at 0x3CB248
>>> 	TCSR_USB3_1_DP_PHYMODE at 0x3CB24C
>>> SM6150:
>>> 	TCSR_USB3_0_DP_PHYMODE at 0x1FCB248
>>> 	TCSR_USB3_1_DP_PHYMODE at 0x1FCB24C
> SM6150 has two different sets of output pins, so the first register
> covers first set of SS lanes (which are routed to the documented SS
> PHY), the second register covers the second set of SS lanes (which are
> routed to the DP and secondary USB PHY).
>
> I can only assume that the same configuration was supposed to be
> applicable to QCM2290 / SM6115, but was later removed / disabled, while
> the registers were kept in the TCSR block.
>
>>> Even though MSM8998, SDM660, QCM2290, and SM6115 all have one USB3 PHY and one DP PHY, the TCSR DP_PHYMODE register configuration is different on each platform.
>>>
>>> Additionally, I found some interesting register documentation for QCM2290/SM6115:
>>> 	TCSR_USB3_0_DP_PHYMODE: “In kamorta this one is for mobile usb. DP not supported.”
>>> 	TCSR_USB3_1_DP_PHYMODE: “DP mode supported for Auto usb in kamorta.”
>>> I think the reason for having two different TCSR registers is to allow both the USB3.0 and DP PHYs to be useds at the same time in certain product configurations.
> Sure. One for the first PHY (USB), one for the second PHY (USB+DP).
> If you check the memory map, you will find the second VLS CLAMP register
> for the second USB PHY.
>
>>> 2. SM6150 Test Results
>>> When TCSR_DP_PHYMODE_0 is switched to DP, the USB3 primary PHY cannot work, and the DP PHY is also not functional (possibly due to clock lack or other configuration mismatch with this TCSR setting).
>>> When TCSR_DP_PHYMODE_1 is switched to DP, both the USB3 primary PHY and the DP PHY work normally.
>>> I think "why msm-4.14 didn't describe second USB3 PHY", because TCSR_DP_PHYMODE_1 always works in DP mode.
>>> https://android.googlesource.com/kernel/msm/+/af03eef7d4c3cbd1fe26c67d4f1915b05d0c1488/drivers/gpu/drm/msm/dp/dp_catalog_v200.c
> Here it still programs the TCSR register.
>
>>> Based on these info, I believe these are separate PHYs, and only the
>>> TCSR DP_PHYMODE registers determine which USB3/DP PHYs are paired or
>>> mutually exclusive. This is why I have maintained separate private
>>> data for each PHY and implemented Power on mutex control via TCSR,
>>> rather than using a qmp_combo-like structure.
> Still, no. Check the block diagram of SM6150.
>
>>> Given the above, do you think we still need to force USB and DP to be strictly bound together like a combo PHY?
> Yes.

I checked the related PHY series and block diagrams again.

PRI and SEC go to different nodes based on the SoC design, and there are two types of configurations: USB3-only and USB3+DP pairing.

Before proceed the v3 patchset, I’d like to double-confirm whether the following structure is what you expect:

usb_qmpphy_1: phy@88e6000 {
    compatible = "qcom,sm6150-qmp-usb3-prim-phy"; <== rename to PRIM
    ...
    qcom,tcsr-reg = <&tcsr 0xb244>, <&tcsr 0xb248>;
    qcom,tcsr-names = "vls_clamp", "dp_phy_mode";
    
    #clock-cells = <1>;
    #phy-cells = <1>;
    ...
};

usb_qmpphy_2: phy@88e8000 {
    compatible = "qcom,sm6150-qmp-usb3dp-sec-phy"; <== SEC SS, use usb3dp to indicate DP capability

    reg = <0x0 0x088e8000 0x0 0x2000>; <== SS2 base address and offset define in driver config

    clocks = <&gcc GCC_AHB2PHY_WEST_CLK>,
            <&gcc GCC_USB3_SEC_CLKREF_CLK>; <== This SoC has no USB3.0 SEC SS clk
    clock-names = "cfg_ahb",
                "ref";
    clock-output-names = "dp_phy_link_clk",
                    "dp_phy_vco_div_clk";
                    
    resets = <&gcc GCC_USB3PHY_PHY_SEC_BCR >,
         <&gcc GCC_USB3_DP_PHY_SEC_BCR>;
    reset-names = "phy", "phy_phy";

    qcom,tcsr-reg = <&tcsr 0xbff0>, <&tcsr 0xb24c>;
    qcom,tcsr-names = "vls_clamp", "dp_phy_mode"; <== added for backward compatibility with legacy configs that only had vls_clamp

    #clock-cells = <1>;
    #phy-cells = <1>;

    status = "disabled";
};

>
>>>> Signed-off-by: Xiangxu Yin <xiangxu.yin@oss.qualcomm.com>
>>>> ---
>>>>  .../bindings/phy/qcom,qcs615-qmp-dp-phy.yaml       | 111 +++++++++++++++++++++
>>>>  1 file changed, 111 insertions(+)
>>>>
Re: [PATCH v2 02/13] dt-bindings: phy: Add binding for QCS615 standalone QMP DP PHY
Posted by Dmitry Baryshkov 2 months, 1 week ago
On Wed, Jul 30, 2025 at 04:53:16PM +0800, Xiangxu Yin wrote:
> 
> On 7/22/2025 8:41 PM, Dmitry Baryshkov wrote:
> > On Tue, Jul 22, 2025 at 08:05:06PM +0800, Xiangxu Yin wrote:
> >> On 7/22/2025 4:38 PM, Dmitry Baryshkov wrote:
> >>> On Tue, Jul 22, 2025 at 03:22:03PM +0800, Xiangxu Yin wrote:
> >>>> Introduce device tree binding documentation for the Qualcomm QMP DP PHY
> >>>> on QCS615 SoCs. This PHY supports DisplayPort functionality and is
> >>>> designed to operate independently from the USB3 PHY.
> >>>>
> >>>> Unlike combo PHYs found on other platforms, the QCS615 DP PHY is
> >>>> standalone and does not support USB/DP multiplexing. The binding
> >>>> describes the required clocks, resets, TCSR configuration, and clock/PHY
> >>>> cells for proper integration.
> >>> Simply put: no, this is not correct. Even if you go to the SM6150 block
> >>> diagram, it points out that DP uses the USB3 PHY, not a separate DP PHY.
> >>>
> >>> I thought that we have discussed it beforehand.
> >>>
> >>> I can quote my comment from the previous thread:
> >>>
> >>>>> No. It means replacing extending existing entries with bigger reg and
> >>>>> #phy-cells = <1>. The driver must keep working with old node definitions
> >>>>> as is to ensure backwards compatibility. New nodes should make it
> >>>>> register two PHYs (USB3 and DP). On the driver side modify generic code
> >>>>> paths, all platforms supported by the driver should be able to support
> >>>>> USB3+DP combination.
> >>> Looking at the hardware memory maps:
> >>>
> >>> MSM8998: USB3 PHY regs at 0xc010000, DP PHY regs at 0xc011000
> >>> SDM660: USB3 PHY regs at 0xc010000, DP PHY regs at 0xc011000
> >>> QCM2290: USB3 PHY regs at 0x1615000, DP PHY regs at 0x1616000
> >>> SM6115: USB3 PHY regs at 0x1615000, DP PHY regs at 0x1616000
> >>>
> >>> Now:
> >>> SM6150: USB3 PHY regs at 0x88e6000
> >>>         USB3 PHY regs at 0x88e8000, DP PHY regs at 0x88e9000
> >>>
> >>> I do not know, why msm-4.14 didn't describe second USB3 PHY. Maybe you
> >>> can comment on it.
> >>>
> >>> But based on that list, the only special case that we need to handle is
> >>> the first USB3 PHY, which doesn't have a corresponding DP PHY block. But
> >>> it will be handled anyway by the code that implements support for the
> >>> existing DT entries. All other hardware blocks are combo USB+DP PHYs.
> >>>
> >>> Having all of that in mind, please, for v3 patchset implement USB+DP
> >>> support in the phy-qcom-qmp-usbc driver and add the following logic
> >>> that also was requested in v1 review:
> >>>
> >>>>> Not quite. Both USB3 and DP drivers should be calling power_on / _off.
> >>>>> If USB3 is on, powering on DP PHY should fail. Vice versa, if DP is on,
> >>>>> powering on USB should fail.
> >>> I think our understanding might not be fully aligned. 
> > I did not write this. Please fix your mailer to quote messages properly.
> > As you are using Thunderbird, I'm not sure where the issue comes from.
> >
> > Also please fix it to wrap your responses somwhere logically.
> >
> >>> Perhaps this is because I didn’t accurately update the mutual exclusion relationships and test results for the different PHYs. 
> >>> Let me clarify my latest findings and explain why I believe these are separate PHYs that require mutual exclusion via TCSR.
> >>>
> >>> 1. About the TCSR DP_PHYMODE Registers
> >>>
> >>> MSM8998/SDM660:
> >>> 	Only one TCSR_USB3_DP_PHYMODE register at 0x1FCB248.
> >>> QCM2290/SM6115:
> >>> 	TCSR_USB3_0_DP_PHYMODE at 0x3CB248
> >>> 	TCSR_USB3_1_DP_PHYMODE at 0x3CB24C
> >>> SM6150:
> >>> 	TCSR_USB3_0_DP_PHYMODE at 0x1FCB248
> >>> 	TCSR_USB3_1_DP_PHYMODE at 0x1FCB24C
> > SM6150 has two different sets of output pins, so the first register
> > covers first set of SS lanes (which are routed to the documented SS
> > PHY), the second register covers the second set of SS lanes (which are
> > routed to the DP and secondary USB PHY).
> >
> > I can only assume that the same configuration was supposed to be
> > applicable to QCM2290 / SM6115, but was later removed / disabled, while
> > the registers were kept in the TCSR block.
> >
> >>> Even though MSM8998, SDM660, QCM2290, and SM6115 all have one USB3 PHY and one DP PHY, the TCSR DP_PHYMODE register configuration is different on each platform.
> >>>
> >>> Additionally, I found some interesting register documentation for QCM2290/SM6115:
> >>> 	TCSR_USB3_0_DP_PHYMODE: “In kamorta this one is for mobile usb. DP not supported.”
> >>> 	TCSR_USB3_1_DP_PHYMODE: “DP mode supported for Auto usb in kamorta.”
> >>> I think the reason for having two different TCSR registers is to allow both the USB3.0 and DP PHYs to be useds at the same time in certain product configurations.
> > Sure. One for the first PHY (USB), one for the second PHY (USB+DP).
> > If you check the memory map, you will find the second VLS CLAMP register
> > for the second USB PHY.
> >
> >>> 2. SM6150 Test Results
> >>> When TCSR_DP_PHYMODE_0 is switched to DP, the USB3 primary PHY cannot work, and the DP PHY is also not functional (possibly due to clock lack or other configuration mismatch with this TCSR setting).
> >>> When TCSR_DP_PHYMODE_1 is switched to DP, both the USB3 primary PHY and the DP PHY work normally.
> >>> I think "why msm-4.14 didn't describe second USB3 PHY", because TCSR_DP_PHYMODE_1 always works in DP mode.
> >>> https://android.googlesource.com/kernel/msm/+/af03eef7d4c3cbd1fe26c67d4f1915b05d0c1488/drivers/gpu/drm/msm/dp/dp_catalog_v200.c
> > Here it still programs the TCSR register.
> >
> >>> Based on these info, I believe these are separate PHYs, and only the
> >>> TCSR DP_PHYMODE registers determine which USB3/DP PHYs are paired or
> >>> mutually exclusive. This is why I have maintained separate private
> >>> data for each PHY and implemented Power on mutex control via TCSR,
> >>> rather than using a qmp_combo-like structure.
> > Still, no. Check the block diagram of SM6150.
> >
> >>> Given the above, do you think we still need to force USB and DP to be strictly bound together like a combo PHY?
> > Yes.
> 
> I checked the related PHY series and block diagrams again.
> 
> PRI and SEC go to different nodes based on the SoC design, and there are two types of configurations: USB3-only and USB3+DP pairing.
> 
> Before proceed the v3 patchset, I’d like to double-confirm whether the following structure is what you expect:
> 
> usb_qmpphy_1: phy@88e6000 {
>     compatible = "qcom,sm6150-qmp-usb3-prim-phy"; <== rename to PRIM

No, we already have a compatible name and DT schema for this device.

>     ...
>     qcom,tcsr-reg = <&tcsr 0xb244>, <&tcsr 0xb248>;
>     qcom,tcsr-names = "vls_clamp", "dp_phy_mode";

No need for qcom,tcsr-names. Second TCSR register should be optional in
the driver.

>     
>     #clock-cells = <1>;
>     #phy-cells = <1>;

#clock-cells = <0>;
#phy-cells = <0>;

>     ...
> };
> 
> usb_qmpphy_2: phy@88e8000 {
>     compatible = "qcom,sm6150-qmp-usb3dp-sec-phy"; <== SEC SS, use usb3dp to indicate DP capability

qcom,sm6150-qmp-usb3-dp-phy

> 
>     reg = <0x0 0x088e8000 0x0 0x2000>; <== SS2 base address and offset define in driver config
> 
>     clocks = <&gcc GCC_AHB2PHY_WEST_CLK>,
>             <&gcc GCC_USB3_SEC_CLKREF_CLK>; <== This SoC has no USB3.0 SEC SS clk
>     clock-names = "cfg_ahb",
>                 "ref";
>     clock-output-names = "dp_phy_link_clk",
>                     "dp_phy_vco_div_clk";

No need to, the driver can generate names on its own.

>                     
>     resets = <&gcc GCC_USB3PHY_PHY_SEC_BCR >,
>          <&gcc GCC_USB3_DP_PHY_SEC_BCR>;
>     reset-names = "phy", "phy_phy";

"phy_phy", "dp_phy". Is there no GCC_USB3_PHY_SEC_BCR?

> 
>     qcom,tcsr-reg = <&tcsr 0xbff0>, <&tcsr 0xb24c>;
>     qcom,tcsr-names = "vls_clamp", "dp_phy_mode"; <== added for backward compatibility with legacy configs that only had vls_clamp

No need for qcom,tcsr-names, correct otherwise.

> 
>     #clock-cells = <1>;
>     #phy-cells = <1>;
> 
>     status = "disabled";
> };
> 
> >
> >>>> Signed-off-by: Xiangxu Yin <xiangxu.yin@oss.qualcomm.com>
> >>>> ---
> >>>>  .../bindings/phy/qcom,qcs615-qmp-dp-phy.yaml       | 111 +++++++++++++++++++++
> >>>>  1 file changed, 111 insertions(+)
> >>>>

-- 
With best wishes
Dmitry
Re: [PATCH v2 02/13] dt-bindings: phy: Add binding for QCS615 standalone QMP DP PHY
Posted by Xiangxu Yin 2 months, 1 week ago
On 7/31/2025 2:35 AM, Dmitry Baryshkov wrote:
> On Wed, Jul 30, 2025 at 04:53:16PM +0800, Xiangxu Yin wrote:
>> On 7/22/2025 8:41 PM, Dmitry Baryshkov wrote:
>>> On Tue, Jul 22, 2025 at 08:05:06PM +0800, Xiangxu Yin wrote:
>>>> On 7/22/2025 4:38 PM, Dmitry Baryshkov wrote:
>>>>> On Tue, Jul 22, 2025 at 03:22:03PM +0800, Xiangxu Yin wrote:
>>>>>> Introduce device tree binding documentation for the Qualcomm QMP DP PHY
>>>>>> on QCS615 SoCs. This PHY supports DisplayPort functionality and is
>>>>>> designed to operate independently from the USB3 PHY.
>>>>>>
>>>>>> Unlike combo PHYs found on other platforms, the QCS615 DP PHY is
>>>>>> standalone and does not support USB/DP multiplexing. The binding
>>>>>> describes the required clocks, resets, TCSR configuration, and clock/PHY
>>>>>> cells for proper integration.
>>>>> Simply put: no, this is not correct. Even if you go to the SM6150 block
>>>>> diagram, it points out that DP uses the USB3 PHY, not a separate DP PHY.
>>>>>
>>>>> I thought that we have discussed it beforehand.
>>>>>
>>>>> I can quote my comment from the previous thread:
>>>>>
>>>>>>> No. It means replacing extending existing entries with bigger reg and
>>>>>>> #phy-cells = <1>. The driver must keep working with old node definitions
>>>>>>> as is to ensure backwards compatibility. New nodes should make it
>>>>>>> register two PHYs (USB3 and DP). On the driver side modify generic code
>>>>>>> paths, all platforms supported by the driver should be able to support
>>>>>>> USB3+DP combination.
>>>>> Looking at the hardware memory maps:
>>>>>
>>>>> MSM8998: USB3 PHY regs at 0xc010000, DP PHY regs at 0xc011000
>>>>> SDM660: USB3 PHY regs at 0xc010000, DP PHY regs at 0xc011000
>>>>> QCM2290: USB3 PHY regs at 0x1615000, DP PHY regs at 0x1616000
>>>>> SM6115: USB3 PHY regs at 0x1615000, DP PHY regs at 0x1616000
>>>>>
>>>>> Now:
>>>>> SM6150: USB3 PHY regs at 0x88e6000
>>>>>         USB3 PHY regs at 0x88e8000, DP PHY regs at 0x88e9000
>>>>>
>>>>> I do not know, why msm-4.14 didn't describe second USB3 PHY. Maybe you
>>>>> can comment on it.
>>>>>
>>>>> But based on that list, the only special case that we need to handle is
>>>>> the first USB3 PHY, which doesn't have a corresponding DP PHY block. But
>>>>> it will be handled anyway by the code that implements support for the
>>>>> existing DT entries. All other hardware blocks are combo USB+DP PHYs.
>>>>>
>>>>> Having all of that in mind, please, for v3 patchset implement USB+DP
>>>>> support in the phy-qcom-qmp-usbc driver and add the following logic
>>>>> that also was requested in v1 review:
>>>>>
>>>>>>> Not quite. Both USB3 and DP drivers should be calling power_on / _off.
>>>>>>> If USB3 is on, powering on DP PHY should fail. Vice versa, if DP is on,
>>>>>>> powering on USB should fail.
>>>>> I think our understanding might not be fully aligned. 
>>> I did not write this. Please fix your mailer to quote messages properly.
>>> As you are using Thunderbird, I'm not sure where the issue comes from.
>>>
>>> Also please fix it to wrap your responses somwhere logically.
>>>
>>>>> Perhaps this is because I didn’t accurately update the mutual exclusion relationships and test results for the different PHYs. 
>>>>> Let me clarify my latest findings and explain why I believe these are separate PHYs that require mutual exclusion via TCSR.
>>>>>
>>>>> 1. About the TCSR DP_PHYMODE Registers
>>>>>
>>>>> MSM8998/SDM660:
>>>>> 	Only one TCSR_USB3_DP_PHYMODE register at 0x1FCB248.
>>>>> QCM2290/SM6115:
>>>>> 	TCSR_USB3_0_DP_PHYMODE at 0x3CB248
>>>>> 	TCSR_USB3_1_DP_PHYMODE at 0x3CB24C
>>>>> SM6150:
>>>>> 	TCSR_USB3_0_DP_PHYMODE at 0x1FCB248
>>>>> 	TCSR_USB3_1_DP_PHYMODE at 0x1FCB24C
>>> SM6150 has two different sets of output pins, so the first register
>>> covers first set of SS lanes (which are routed to the documented SS
>>> PHY), the second register covers the second set of SS lanes (which are
>>> routed to the DP and secondary USB PHY).
>>>
>>> I can only assume that the same configuration was supposed to be
>>> applicable to QCM2290 / SM6115, but was later removed / disabled, while
>>> the registers were kept in the TCSR block.
>>>
>>>>> Even though MSM8998, SDM660, QCM2290, and SM6115 all have one USB3 PHY and one DP PHY, the TCSR DP_PHYMODE register configuration is different on each platform.
>>>>>
>>>>> Additionally, I found some interesting register documentation for QCM2290/SM6115:
>>>>> 	TCSR_USB3_0_DP_PHYMODE: “In kamorta this one is for mobile usb. DP not supported.”
>>>>> 	TCSR_USB3_1_DP_PHYMODE: “DP mode supported for Auto usb in kamorta.”
>>>>> I think the reason for having two different TCSR registers is to allow both the USB3.0 and DP PHYs to be useds at the same time in certain product configurations.
>>> Sure. One for the first PHY (USB), one for the second PHY (USB+DP).
>>> If you check the memory map, you will find the second VLS CLAMP register
>>> for the second USB PHY.
>>>
>>>>> 2. SM6150 Test Results
>>>>> When TCSR_DP_PHYMODE_0 is switched to DP, the USB3 primary PHY cannot work, and the DP PHY is also not functional (possibly due to clock lack or other configuration mismatch with this TCSR setting).
>>>>> When TCSR_DP_PHYMODE_1 is switched to DP, both the USB3 primary PHY and the DP PHY work normally.
>>>>> I think "why msm-4.14 didn't describe second USB3 PHY", because TCSR_DP_PHYMODE_1 always works in DP mode.
>>>>> https://android.googlesource.com/kernel/msm/+/af03eef7d4c3cbd1fe26c67d4f1915b05d0c1488/drivers/gpu/drm/msm/dp/dp_catalog_v200.c
>>> Here it still programs the TCSR register.
>>>
>>>>> Based on these info, I believe these are separate PHYs, and only the
>>>>> TCSR DP_PHYMODE registers determine which USB3/DP PHYs are paired or
>>>>> mutually exclusive. This is why I have maintained separate private
>>>>> data for each PHY and implemented Power on mutex control via TCSR,
>>>>> rather than using a qmp_combo-like structure.
>>> Still, no. Check the block diagram of SM6150.
>>>
>>>>> Given the above, do you think we still need to force USB and DP to be strictly bound together like a combo PHY?
>>> Yes.
>> I checked the related PHY series and block diagrams again.
>>
>> PRI and SEC go to different nodes based on the SoC design, and there are two types of configurations: USB3-only and USB3+DP pairing.
>>
>> Before proceed the v3 patchset, I’d like to double-confirm whether the following structure is what you expect:
>>
>> usb_qmpphy_1: phy@88e6000 {
>>     compatible = "qcom,sm6150-qmp-usb3-prim-phy"; <== rename to PRIM
> No, we already have a compatible name and DT schema for this device.
Then current compatible name is "qcom,qcs615-qmp-usb3-phy" and shall we need update to "qcom,sm6150-qmp-usb3-phy"?
>
>>     ...
>>     qcom,tcsr-reg = <&tcsr 0xb244>, <&tcsr 0xb248>;
>>     qcom,tcsr-names = "vls_clamp", "dp_phy_mode";
> No need for qcom,tcsr-names. Second TCSR register should be optional in
> the driver.
Ok.
>
>>     
>>     #clock-cells = <1>;
>>     #phy-cells = <1>;
> #clock-cells = <0>;
> #phy-cells = <0>;
>
>>     ...
>> };
>>
>> usb_qmpphy_2: phy@88e8000 {
>>     compatible = "qcom,sm6150-qmp-usb3dp-sec-phy"; <== SEC SS, use usb3dp to indicate DP capability
> qcom,sm6150-qmp-usb3-dp-phy
Ok, but for this part, shall we update dt-binding in "qcom,msm8998-qmp-usb3-phy.yaml" or create a new one?
>
>>     reg = <0x0 0x088e8000 0x0 0x2000>; <== SS2 base address and offset define in driver config
>>
>>     clocks = <&gcc GCC_AHB2PHY_WEST_CLK>,
>>             <&gcc GCC_USB3_SEC_CLKREF_CLK>; <== This SoC has no USB3.0 SEC SS clk
>>     clock-names = "cfg_ahb",
>>                 "ref";
>>     clock-output-names = "dp_phy_link_clk",
>>                     "dp_phy_vco_div_clk";
> No need to, the driver can generate names on its own.
Ok.
>
>>                     
>>     resets = <&gcc GCC_USB3PHY_PHY_SEC_BCR >,
>>          <&gcc GCC_USB3_DP_PHY_SEC_BCR>;
>>     reset-names = "phy", "phy_phy";
> "phy_phy", "dp_phy". Is there no GCC_USB3_PHY_SEC_BCR?
There are only GCC_USB2_PHY_SEC_BCR and GCC_USB3PHY_PHY_SEC_BCR, no GCC_USB3_PHY_SEC_BCR.
>>     qcom,tcsr-reg = <&tcsr 0xbff0>, <&tcsr 0xb24c>;
>>     qcom,tcsr-names = "vls_clamp", "dp_phy_mode"; <== added for backward compatibility with legacy configs that only had vls_clamp
> No need for qcom,tcsr-names, correct otherwise.
>
>>     #clock-cells = <1>;
>>     #phy-cells = <1>;
>>
>>     status = "disabled";
>> };
>>
>>>>>> Signed-off-by: Xiangxu Yin <xiangxu.yin@oss.qualcomm.com>
>>>>>> ---
>>>>>>  .../bindings/phy/qcom,qcs615-qmp-dp-phy.yaml       | 111 +++++++++++++++++++++
>>>>>>  1 file changed, 111 insertions(+)
>>>>>>
Re: [PATCH v2 02/13] dt-bindings: phy: Add binding for QCS615 standalone QMP DP PHY
Posted by Dmitry Baryshkov 2 months ago
On 31/07/2025 08:06, Xiangxu Yin wrote:
> 
> On 7/31/2025 2:35 AM, Dmitry Baryshkov wrote:
>> On Wed, Jul 30, 2025 at 04:53:16PM +0800, Xiangxu Yin wrote:
>>> On 7/22/2025 8:41 PM, Dmitry Baryshkov wrote:
>>>> On Tue, Jul 22, 2025 at 08:05:06PM +0800, Xiangxu Yin wrote:
>>>>> On 7/22/2025 4:38 PM, Dmitry Baryshkov wrote:
>>>>>> On Tue, Jul 22, 2025 at 03:22:03PM +0800, Xiangxu Yin wrote:
>>>>>>> Introduce device tree binding documentation for the Qualcomm QMP DP PHY
>>>>>>> on QCS615 SoCs. This PHY supports DisplayPort functionality and is
>>>>>>> designed to operate independently from the USB3 PHY.
>>>>>>>
>>>>>>> Unlike combo PHYs found on other platforms, the QCS615 DP PHY is
>>>>>>> standalone and does not support USB/DP multiplexing. The binding
>>>>>>> describes the required clocks, resets, TCSR configuration, and clock/PHY
>>>>>>> cells for proper integration.
>>>>>> Simply put: no, this is not correct. Even if you go to the SM6150 block
>>>>>> diagram, it points out that DP uses the USB3 PHY, not a separate DP PHY.
>>>>>>
>>>>>> I thought that we have discussed it beforehand.
>>>>>>
>>>>>> I can quote my comment from the previous thread:
>>>>>>
>>>>>>>> No. It means replacing extending existing entries with bigger reg and
>>>>>>>> #phy-cells = <1>. The driver must keep working with old node definitions
>>>>>>>> as is to ensure backwards compatibility. New nodes should make it
>>>>>>>> register two PHYs (USB3 and DP). On the driver side modify generic code
>>>>>>>> paths, all platforms supported by the driver should be able to support
>>>>>>>> USB3+DP combination.
>>>>>> Looking at the hardware memory maps:
>>>>>>
>>>>>> MSM8998: USB3 PHY regs at 0xc010000, DP PHY regs at 0xc011000
>>>>>> SDM660: USB3 PHY regs at 0xc010000, DP PHY regs at 0xc011000
>>>>>> QCM2290: USB3 PHY regs at 0x1615000, DP PHY regs at 0x1616000
>>>>>> SM6115: USB3 PHY regs at 0x1615000, DP PHY regs at 0x1616000
>>>>>>
>>>>>> Now:
>>>>>> SM6150: USB3 PHY regs at 0x88e6000
>>>>>>          USB3 PHY regs at 0x88e8000, DP PHY regs at 0x88e9000
>>>>>>
>>>>>> I do not know, why msm-4.14 didn't describe second USB3 PHY. Maybe you
>>>>>> can comment on it.
>>>>>>
>>>>>> But based on that list, the only special case that we need to handle is
>>>>>> the first USB3 PHY, which doesn't have a corresponding DP PHY block. But
>>>>>> it will be handled anyway by the code that implements support for the
>>>>>> existing DT entries. All other hardware blocks are combo USB+DP PHYs.
>>>>>>
>>>>>> Having all of that in mind, please, for v3 patchset implement USB+DP
>>>>>> support in the phy-qcom-qmp-usbc driver and add the following logic
>>>>>> that also was requested in v1 review:
>>>>>>
>>>>>>>> Not quite. Both USB3 and DP drivers should be calling power_on / _off.
>>>>>>>> If USB3 is on, powering on DP PHY should fail. Vice versa, if DP is on,
>>>>>>>> powering on USB should fail.
>>>>>> I think our understanding might not be fully aligned.
>>>> I did not write this. Please fix your mailer to quote messages properly.
>>>> As you are using Thunderbird, I'm not sure where the issue comes from.
>>>>
>>>> Also please fix it to wrap your responses somwhere logically.
>>>>
>>>>>> Perhaps this is because I didn’t accurately update the mutual exclusion relationships and test results for the different PHYs.
>>>>>> Let me clarify my latest findings and explain why I believe these are separate PHYs that require mutual exclusion via TCSR.
>>>>>>
>>>>>> 1. About the TCSR DP_PHYMODE Registers
>>>>>>
>>>>>> MSM8998/SDM660:
>>>>>> 	Only one TCSR_USB3_DP_PHYMODE register at 0x1FCB248.
>>>>>> QCM2290/SM6115:
>>>>>> 	TCSR_USB3_0_DP_PHYMODE at 0x3CB248
>>>>>> 	TCSR_USB3_1_DP_PHYMODE at 0x3CB24C
>>>>>> SM6150:
>>>>>> 	TCSR_USB3_0_DP_PHYMODE at 0x1FCB248
>>>>>> 	TCSR_USB3_1_DP_PHYMODE at 0x1FCB24C
>>>> SM6150 has two different sets of output pins, so the first register
>>>> covers first set of SS lanes (which are routed to the documented SS
>>>> PHY), the second register covers the second set of SS lanes (which are
>>>> routed to the DP and secondary USB PHY).
>>>>
>>>> I can only assume that the same configuration was supposed to be
>>>> applicable to QCM2290 / SM6115, but was later removed / disabled, while
>>>> the registers were kept in the TCSR block.
>>>>
>>>>>> Even though MSM8998, SDM660, QCM2290, and SM6115 all have one USB3 PHY and one DP PHY, the TCSR DP_PHYMODE register configuration is different on each platform.
>>>>>>
>>>>>> Additionally, I found some interesting register documentation for QCM2290/SM6115:
>>>>>> 	TCSR_USB3_0_DP_PHYMODE: “In kamorta this one is for mobile usb. DP not supported.”
>>>>>> 	TCSR_USB3_1_DP_PHYMODE: “DP mode supported for Auto usb in kamorta.”
>>>>>> I think the reason for having two different TCSR registers is to allow both the USB3.0 and DP PHYs to be useds at the same time in certain product configurations.
>>>> Sure. One for the first PHY (USB), one for the second PHY (USB+DP).
>>>> If you check the memory map, you will find the second VLS CLAMP register
>>>> for the second USB PHY.
>>>>
>>>>>> 2. SM6150 Test Results
>>>>>> When TCSR_DP_PHYMODE_0 is switched to DP, the USB3 primary PHY cannot work, and the DP PHY is also not functional (possibly due to clock lack or other configuration mismatch with this TCSR setting).
>>>>>> When TCSR_DP_PHYMODE_1 is switched to DP, both the USB3 primary PHY and the DP PHY work normally.
>>>>>> I think "why msm-4.14 didn't describe second USB3 PHY", because TCSR_DP_PHYMODE_1 always works in DP mode.
>>>>>> https://android.googlesource.com/kernel/msm/+/af03eef7d4c3cbd1fe26c67d4f1915b05d0c1488/drivers/gpu/drm/msm/dp/dp_catalog_v200.c
>>>> Here it still programs the TCSR register.
>>>>
>>>>>> Based on these info, I believe these are separate PHYs, and only the
>>>>>> TCSR DP_PHYMODE registers determine which USB3/DP PHYs are paired or
>>>>>> mutually exclusive. This is why I have maintained separate private
>>>>>> data for each PHY and implemented Power on mutex control via TCSR,
>>>>>> rather than using a qmp_combo-like structure.
>>>> Still, no. Check the block diagram of SM6150.
>>>>
>>>>>> Given the above, do you think we still need to force USB and DP to be strictly bound together like a combo PHY?
>>>> Yes.
>>> I checked the related PHY series and block diagrams again.
>>>
>>> PRI and SEC go to different nodes based on the SoC design, and there are two types of configurations: USB3-only and USB3+DP pairing.
>>>
>>> Before proceed the v3 patchset, I’d like to double-confirm whether the following structure is what you expect:
>>>
>>> usb_qmpphy_1: phy@88e6000 {
>>>      compatible = "qcom,sm6150-qmp-usb3-prim-phy"; <== rename to PRIM
>> No, we already have a compatible name and DT schema for this device.
> Then current compatible name is "qcom,qcs615-qmp-usb3-phy" and shall we need update to "qcom,sm6150-qmp-usb3-phy"?

Why? You _already_ have a compatible string. You don't need to change it 
just to follow the SoC name.

>>
>>>      ...
>>>      qcom,tcsr-reg = <&tcsr 0xb244>, <&tcsr 0xb248>;
>>>      qcom,tcsr-names = "vls_clamp", "dp_phy_mode";
>> No need for qcom,tcsr-names. Second TCSR register should be optional in
>> the driver.
> Ok.
>>
>>>      
>>>      #clock-cells = <1>;
>>>      #phy-cells = <1>;
>> #clock-cells = <0>;
>> #phy-cells = <0>;
>>
>>>      ...
>>> };
>>>
>>> usb_qmpphy_2: phy@88e8000 {
>>>      compatible = "qcom,sm6150-qmp-usb3dp-sec-phy"; <== SEC SS, use usb3dp to indicate DP capability
>> qcom,sm6150-qmp-usb3-dp-phy
> Ok, but for this part, shall we update dt-binding in "qcom,msm8998-qmp-usb3-phy.yaml" or create a new one?

I think (I might be wrong here) new ones is a better fit. We'll migrate 
the rest of PHYs to new bindings later on.

>>
>>>      reg = <0x0 0x088e8000 0x0 0x2000>; <== SS2 base address and offset define in driver config
>>>
>>>      clocks = <&gcc GCC_AHB2PHY_WEST_CLK>,
>>>              <&gcc GCC_USB3_SEC_CLKREF_CLK>; <== This SoC has no USB3.0 SEC SS clk
>>>      clock-names = "cfg_ahb",
>>>                  "ref";
>>>      clock-output-names = "dp_phy_link_clk",
>>>                      "dp_phy_vco_div_clk";
>> No need to, the driver can generate names on its own.
> Ok.
>>
>>>                      
>>>      resets = <&gcc GCC_USB3PHY_PHY_SEC_BCR >,
>>>           <&gcc GCC_USB3_DP_PHY_SEC_BCR>;
>>>      reset-names = "phy", "phy_phy";
>> "phy_phy", "dp_phy". Is there no GCC_USB3_PHY_SEC_BCR?
> There are only GCC_USB2_PHY_SEC_BCR and GCC_USB3PHY_PHY_SEC_BCR, no GCC_USB3_PHY_SEC_BCR.

Ack.

>>>      qcom,tcsr-reg = <&tcsr 0xbff0>, <&tcsr 0xb24c>;
>>>      qcom,tcsr-names = "vls_clamp", "dp_phy_mode"; <== added for backward compatibility with legacy configs that only had vls_clamp
>> No need for qcom,tcsr-names, correct otherwise.
>>
>>>      #clock-cells = <1>;
>>>      #phy-cells = <1>;
>>>
>>>      status = "disabled";
>>> };
>>>
>>>>>>> Signed-off-by: Xiangxu Yin <xiangxu.yin@oss.qualcomm.com>
>>>>>>> ---
>>>>>>>   .../bindings/phy/qcom,qcs615-qmp-dp-phy.yaml       | 111 +++++++++++++++++++++
>>>>>>>   1 file changed, 111 insertions(+)
>>>>>>>


-- 
With best wishes
Dmitry
Re: [PATCH v2 02/13] dt-bindings: phy: Add binding for QCS615 standalone QMP DP PHY
Posted by Xiangxu Yin 2 months ago
On 8/1/2025 1:13 AM, Dmitry Baryshkov wrote:
> On 31/07/2025 08:06, Xiangxu Yin wrote:
>>
>> On 7/31/2025 2:35 AM, Dmitry Baryshkov wrote:
>>> On Wed, Jul 30, 2025 at 04:53:16PM +0800, Xiangxu Yin wrote:
>>>> On 7/22/2025 8:41 PM, Dmitry Baryshkov wrote:
>>>>> On Tue, Jul 22, 2025 at 08:05:06PM +0800, Xiangxu Yin wrote:
>>>>>> On 7/22/2025 4:38 PM, Dmitry Baryshkov wrote:
>>>>>>> On Tue, Jul 22, 2025 at 03:22:03PM +0800, Xiangxu Yin wrote:
>>>>>>>> Introduce device tree binding documentation for the Qualcomm QMP DP PHY
>>>>>>>> on QCS615 SoCs. This PHY supports DisplayPort functionality and is
>>>>>>>> designed to operate independently from the USB3 PHY.
>>>>>>>>
>>>>>>>> Unlike combo PHYs found on other platforms, the QCS615 DP PHY is
>>>>>>>> standalone and does not support USB/DP multiplexing. The binding
>>>>>>>> describes the required clocks, resets, TCSR configuration, and clock/PHY
>>>>>>>> cells for proper integration.
>>>>>>> Simply put: no, this is not correct. Even if you go to the SM6150 block
>>>>>>> diagram, it points out that DP uses the USB3 PHY, not a separate DP PHY.
>>>>>>>
>>>>>>> I thought that we have discussed it beforehand.
>>>>>>>
>>>>>>> I can quote my comment from the previous thread:
>>>>>>>
>>>>>>>>> No. It means replacing extending existing entries with bigger reg and
>>>>>>>>> #phy-cells = <1>. The driver must keep working with old node definitions
>>>>>>>>> as is to ensure backwards compatibility. New nodes should make it
>>>>>>>>> register two PHYs (USB3 and DP). On the driver side modify generic code
>>>>>>>>> paths, all platforms supported by the driver should be able to support
>>>>>>>>> USB3+DP combination.
>>>>>>> Looking at the hardware memory maps:
>>>>>>>
>>>>>>> MSM8998: USB3 PHY regs at 0xc010000, DP PHY regs at 0xc011000
>>>>>>> SDM660: USB3 PHY regs at 0xc010000, DP PHY regs at 0xc011000
>>>>>>> QCM2290: USB3 PHY regs at 0x1615000, DP PHY regs at 0x1616000
>>>>>>> SM6115: USB3 PHY regs at 0x1615000, DP PHY regs at 0x1616000
>>>>>>>
>>>>>>> Now:
>>>>>>> SM6150: USB3 PHY regs at 0x88e6000
>>>>>>>          USB3 PHY regs at 0x88e8000, DP PHY regs at 0x88e9000
>>>>>>>
>>>>>>> I do not know, why msm-4.14 didn't describe second USB3 PHY. Maybe you
>>>>>>> can comment on it.
>>>>>>>
>>>>>>> But based on that list, the only special case that we need to handle is
>>>>>>> the first USB3 PHY, which doesn't have a corresponding DP PHY block. But
>>>>>>> it will be handled anyway by the code that implements support for the
>>>>>>> existing DT entries. All other hardware blocks are combo USB+DP PHYs.
>>>>>>>
>>>>>>> Having all of that in mind, please, for v3 patchset implement USB+DP
>>>>>>> support in the phy-qcom-qmp-usbc driver and add the following logic
>>>>>>> that also was requested in v1 review:
>>>>>>>
>>>>>>>>> Not quite. Both USB3 and DP drivers should be calling power_on / _off.
>>>>>>>>> If USB3 is on, powering on DP PHY should fail. Vice versa, if DP is on,
>>>>>>>>> powering on USB should fail.
>>>>>>> I think our understanding might not be fully aligned.
>>>>> I did not write this. Please fix your mailer to quote messages properly.
>>>>> As you are using Thunderbird, I'm not sure where the issue comes from.
>>>>>
>>>>> Also please fix it to wrap your responses somwhere logically.
>>>>>
>>>>>>> Perhaps this is because I didn’t accurately update the mutual exclusion relationships and test results for the different PHYs.
>>>>>>> Let me clarify my latest findings and explain why I believe these are separate PHYs that require mutual exclusion via TCSR.
>>>>>>>
>>>>>>> 1. About the TCSR DP_PHYMODE Registers
>>>>>>>
>>>>>>> MSM8998/SDM660:
>>>>>>>     Only one TCSR_USB3_DP_PHYMODE register at 0x1FCB248.
>>>>>>> QCM2290/SM6115:
>>>>>>>     TCSR_USB3_0_DP_PHYMODE at 0x3CB248
>>>>>>>     TCSR_USB3_1_DP_PHYMODE at 0x3CB24C
>>>>>>> SM6150:
>>>>>>>     TCSR_USB3_0_DP_PHYMODE at 0x1FCB248
>>>>>>>     TCSR_USB3_1_DP_PHYMODE at 0x1FCB24C
>>>>> SM6150 has two different sets of output pins, so the first register
>>>>> covers first set of SS lanes (which are routed to the documented SS
>>>>> PHY), the second register covers the second set of SS lanes (which are
>>>>> routed to the DP and secondary USB PHY).
>>>>>
>>>>> I can only assume that the same configuration was supposed to be
>>>>> applicable to QCM2290 / SM6115, but was later removed / disabled, while
>>>>> the registers were kept in the TCSR block.
>>>>>
>>>>>>> Even though MSM8998, SDM660, QCM2290, and SM6115 all have one USB3 PHY and one DP PHY, the TCSR DP_PHYMODE register configuration is different on each platform.
>>>>>>>
>>>>>>> Additionally, I found some interesting register documentation for QCM2290/SM6115:
>>>>>>>     TCSR_USB3_0_DP_PHYMODE: “In kamorta this one is for mobile usb. DP not supported.”
>>>>>>>     TCSR_USB3_1_DP_PHYMODE: “DP mode supported for Auto usb in kamorta.”
>>>>>>> I think the reason for having two different TCSR registers is to allow both the USB3.0 and DP PHYs to be useds at the same time in certain product configurations.
>>>>> Sure. One for the first PHY (USB), one for the second PHY (USB+DP).
>>>>> If you check the memory map, you will find the second VLS CLAMP register
>>>>> for the second USB PHY.
>>>>>
>>>>>>> 2. SM6150 Test Results
>>>>>>> When TCSR_DP_PHYMODE_0 is switched to DP, the USB3 primary PHY cannot work, and the DP PHY is also not functional (possibly due to clock lack or other configuration mismatch with this TCSR setting).
>>>>>>> When TCSR_DP_PHYMODE_1 is switched to DP, both the USB3 primary PHY and the DP PHY work normally.
>>>>>>> I think "why msm-4.14 didn't describe second USB3 PHY", because TCSR_DP_PHYMODE_1 always works in DP mode.
>>>>>>> https://android.googlesource.com/kernel/msm/+/af03eef7d4c3cbd1fe26c67d4f1915b05d0c1488/drivers/gpu/drm/msm/dp/dp_catalog_v200.c
>>>>> Here it still programs the TCSR register.
>>>>>
>>>>>>> Based on these info, I believe these are separate PHYs, and only the
>>>>>>> TCSR DP_PHYMODE registers determine which USB3/DP PHYs are paired or
>>>>>>> mutually exclusive. This is why I have maintained separate private
>>>>>>> data for each PHY and implemented Power on mutex control via TCSR,
>>>>>>> rather than using a qmp_combo-like structure.
>>>>> Still, no. Check the block diagram of SM6150.
>>>>>
>>>>>>> Given the above, do you think we still need to force USB and DP to be strictly bound together like a combo PHY?
>>>>> Yes.
>>>> I checked the related PHY series and block diagrams again.
>>>>
>>>> PRI and SEC go to different nodes based on the SoC design, and there are two types of configurations: USB3-only and USB3+DP pairing.
>>>>
>>>> Before proceed the v3 patchset, I’d like to double-confirm whether the following structure is what you expect:
>>>>
>>>> usb_qmpphy_1: phy@88e6000 {
>>>>      compatible = "qcom,sm6150-qmp-usb3-prim-phy"; <== rename to PRIM
>>> No, we already have a compatible name and DT schema for this device.
>> Then current compatible name is "qcom,qcs615-qmp-usb3-phy" and shall we need update to "qcom,sm6150-qmp-usb3-phy"?
>
> Why? You _already_ have a compatible string. You don't need to change it just to follow the SoC name. 
>
Ok, but just to confirm — in this case, the USB3-DP PHY would use "qcom,sm6150-qmp-usb3-dp-phy" while the USB3-only PHY still uses "qcom,qcs615-qmp-usb3-phy"?

Since both PHYs are on the same SoC, would it make sense to keep the naming consistent and use "qcom,qcs615-..." for both? 

>>>
>>>>      ...
>>>>      qcom,tcsr-reg = <&tcsr 0xb244>, <&tcsr 0xb248>;
>>>>      qcom,tcsr-names = "vls_clamp", "dp_phy_mode";
>>> No need for qcom,tcsr-names. Second TCSR register should be optional in
>>> the driver.
>> Ok.
>>>
>>>>           #clock-cells = <1>;
>>>>      #phy-cells = <1>;
>>> #clock-cells = <0>;
>>> #phy-cells = <0>;
>>>
>>>>      ...
>>>> };
>>>>
>>>> usb_qmpphy_2: phy@88e8000 {
>>>>      compatible = "qcom,sm6150-qmp-usb3dp-sec-phy"; <== SEC SS, use usb3dp to indicate DP capability
>>> qcom,sm6150-qmp-usb3-dp-phy
>> Ok, but for this part, shall we update dt-binding in "qcom,msm8998-qmp-usb3-phy.yaml" or create a new one?
>
> I think (I might be wrong here) new ones is a better fit. We'll migrate the rest of PHYs to new bindings later on. 
>
Ok, I’ll keep the USB3-DP PHY node definition in a separate YAML file in v3.
>>>
>>>>      reg = <0x0 0x088e8000 0x0 0x2000>; <== SS2 base address and offset define in driver config
>>>>
>>>>      clocks = <&gcc GCC_AHB2PHY_WEST_CLK>,
>>>>              <&gcc GCC_USB3_SEC_CLKREF_CLK>; <== This SoC has no USB3.0 SEC SS clk
>>>>      clock-names = "cfg_ahb",
>>>>                  "ref";
>>>>      clock-output-names = "dp_phy_link_clk",
>>>>                      "dp_phy_vco_div_clk";
>>> No need to, the driver can generate names on its own.
>> Ok.
>>>
>>>>                           resets = <&gcc GCC_USB3PHY_PHY_SEC_BCR >,
>>>>           <&gcc GCC_USB3_DP_PHY_SEC_BCR>;
>>>>      reset-names = "phy", "phy_phy";
>>> "phy_phy", "dp_phy". Is there no GCC_USB3_PHY_SEC_BCR?
>> There are only GCC_USB2_PHY_SEC_BCR and GCC_USB3PHY_PHY_SEC_BCR, no GCC_USB3_PHY_SEC_BCR.
>
> Ack.
>
>>>>      qcom,tcsr-reg = <&tcsr 0xbff0>, <&tcsr 0xb24c>;
>>>>      qcom,tcsr-names = "vls_clamp", "dp_phy_mode"; <== added for backward compatibility with legacy configs that only had vls_clamp
>>> No need for qcom,tcsr-names, correct otherwise.
>>>
>>>>      #clock-cells = <1>;
>>>>      #phy-cells = <1>;
>>>>
>>>>      status = "disabled";
>>>> };
>>>>
>>>>>>>> Signed-off-by: Xiangxu Yin <xiangxu.yin@oss.qualcomm.com>
>>>>>>>> ---
>>>>>>>>   .../bindings/phy/qcom,qcs615-qmp-dp-phy.yaml       | 111 +++++++++++++++++++++
>>>>>>>>   1 file changed, 111 insertions(+)
>>>>>>>>
>
>
Re: [PATCH v2 02/13] dt-bindings: phy: Add binding for QCS615 standalone QMP DP PHY
Posted by Dmitry Baryshkov 2 months ago
On Fri, Aug 01, 2025 at 11:57:50AM +0800, Xiangxu Yin wrote:
> 
> On 8/1/2025 1:13 AM, Dmitry Baryshkov wrote:
> > On 31/07/2025 08:06, Xiangxu Yin wrote:
> >>
> >> On 7/31/2025 2:35 AM, Dmitry Baryshkov wrote:
> >>> On Wed, Jul 30, 2025 at 04:53:16PM +0800, Xiangxu Yin wrote:
> >>>> On 7/22/2025 8:41 PM, Dmitry Baryshkov wrote:
> >>>>> On Tue, Jul 22, 2025 at 08:05:06PM +0800, Xiangxu Yin wrote:
> >>>>>> On 7/22/2025 4:38 PM, Dmitry Baryshkov wrote:
> >>>>>>> On Tue, Jul 22, 2025 at 03:22:03PM +0800, Xiangxu Yin wrote:
> >>>>>>>> Introduce device tree binding documentation for the Qualcomm QMP DP PHY
> >>>>>>>> on QCS615 SoCs. This PHY supports DisplayPort functionality and is
> >>>>>>>> designed to operate independently from the USB3 PHY.
> >>>>>>>>
> >>>>>>>> Unlike combo PHYs found on other platforms, the QCS615 DP PHY is
> >>>>>>>> standalone and does not support USB/DP multiplexing. The binding
> >>>>>>>> describes the required clocks, resets, TCSR configuration, and clock/PHY
> >>>>>>>> cells for proper integration.
> >>>>>>> Simply put: no, this is not correct. Even if you go to the SM6150 block
> >>>>>>> diagram, it points out that DP uses the USB3 PHY, not a separate DP PHY.
> >>>>>>>
> >>>>>>> I thought that we have discussed it beforehand.
> >>>>>>>
> >>>>>>> I can quote my comment from the previous thread:
> >>>>>>>
> >>>>>>>>> No. It means replacing extending existing entries with bigger reg and
> >>>>>>>>> #phy-cells = <1>. The driver must keep working with old node definitions
> >>>>>>>>> as is to ensure backwards compatibility. New nodes should make it
> >>>>>>>>> register two PHYs (USB3 and DP). On the driver side modify generic code
> >>>>>>>>> paths, all platforms supported by the driver should be able to support
> >>>>>>>>> USB3+DP combination.
> >>>>>>> Looking at the hardware memory maps:
> >>>>>>>
> >>>>>>> MSM8998: USB3 PHY regs at 0xc010000, DP PHY regs at 0xc011000
> >>>>>>> SDM660: USB3 PHY regs at 0xc010000, DP PHY regs at 0xc011000
> >>>>>>> QCM2290: USB3 PHY regs at 0x1615000, DP PHY regs at 0x1616000
> >>>>>>> SM6115: USB3 PHY regs at 0x1615000, DP PHY regs at 0x1616000
> >>>>>>>
> >>>>>>> Now:
> >>>>>>> SM6150: USB3 PHY regs at 0x88e6000
> >>>>>>>          USB3 PHY regs at 0x88e8000, DP PHY regs at 0x88e9000
> >>>>>>>
> >>>>>>> I do not know, why msm-4.14 didn't describe second USB3 PHY. Maybe you
> >>>>>>> can comment on it.
> >>>>>>>
> >>>>>>> But based on that list, the only special case that we need to handle is
> >>>>>>> the first USB3 PHY, which doesn't have a corresponding DP PHY block. But
> >>>>>>> it will be handled anyway by the code that implements support for the
> >>>>>>> existing DT entries. All other hardware blocks are combo USB+DP PHYs.
> >>>>>>>
> >>>>>>> Having all of that in mind, please, for v3 patchset implement USB+DP
> >>>>>>> support in the phy-qcom-qmp-usbc driver and add the following logic
> >>>>>>> that also was requested in v1 review:
> >>>>>>>
> >>>>>>>>> Not quite. Both USB3 and DP drivers should be calling power_on / _off.
> >>>>>>>>> If USB3 is on, powering on DP PHY should fail. Vice versa, if DP is on,
> >>>>>>>>> powering on USB should fail.
> >>>>>>> I think our understanding might not be fully aligned.
> >>>>> I did not write this. Please fix your mailer to quote messages properly.
> >>>>> As you are using Thunderbird, I'm not sure where the issue comes from.
> >>>>>
> >>>>> Also please fix it to wrap your responses somwhere logically.
> >>>>>
> >>>>>>> Perhaps this is because I didn’t accurately update the mutual exclusion relationships and test results for the different PHYs.
> >>>>>>> Let me clarify my latest findings and explain why I believe these are separate PHYs that require mutual exclusion via TCSR.
> >>>>>>>
> >>>>>>> 1. About the TCSR DP_PHYMODE Registers
> >>>>>>>
> >>>>>>> MSM8998/SDM660:
> >>>>>>>     Only one TCSR_USB3_DP_PHYMODE register at 0x1FCB248.
> >>>>>>> QCM2290/SM6115:
> >>>>>>>     TCSR_USB3_0_DP_PHYMODE at 0x3CB248
> >>>>>>>     TCSR_USB3_1_DP_PHYMODE at 0x3CB24C
> >>>>>>> SM6150:
> >>>>>>>     TCSR_USB3_0_DP_PHYMODE at 0x1FCB248
> >>>>>>>     TCSR_USB3_1_DP_PHYMODE at 0x1FCB24C
> >>>>> SM6150 has two different sets of output pins, so the first register
> >>>>> covers first set of SS lanes (which are routed to the documented SS
> >>>>> PHY), the second register covers the second set of SS lanes (which are
> >>>>> routed to the DP and secondary USB PHY).
> >>>>>
> >>>>> I can only assume that the same configuration was supposed to be
> >>>>> applicable to QCM2290 / SM6115, but was later removed / disabled, while
> >>>>> the registers were kept in the TCSR block.
> >>>>>
> >>>>>>> Even though MSM8998, SDM660, QCM2290, and SM6115 all have one USB3 PHY and one DP PHY, the TCSR DP_PHYMODE register configuration is different on each platform.
> >>>>>>>
> >>>>>>> Additionally, I found some interesting register documentation for QCM2290/SM6115:
> >>>>>>>     TCSR_USB3_0_DP_PHYMODE: “In kamorta this one is for mobile usb. DP not supported.”
> >>>>>>>     TCSR_USB3_1_DP_PHYMODE: “DP mode supported for Auto usb in kamorta.”
> >>>>>>> I think the reason for having two different TCSR registers is to allow both the USB3.0 and DP PHYs to be useds at the same time in certain product configurations.
> >>>>> Sure. One for the first PHY (USB), one for the second PHY (USB+DP).
> >>>>> If you check the memory map, you will find the second VLS CLAMP register
> >>>>> for the second USB PHY.
> >>>>>
> >>>>>>> 2. SM6150 Test Results
> >>>>>>> When TCSR_DP_PHYMODE_0 is switched to DP, the USB3 primary PHY cannot work, and the DP PHY is also not functional (possibly due to clock lack or other configuration mismatch with this TCSR setting).
> >>>>>>> When TCSR_DP_PHYMODE_1 is switched to DP, both the USB3 primary PHY and the DP PHY work normally.
> >>>>>>> I think "why msm-4.14 didn't describe second USB3 PHY", because TCSR_DP_PHYMODE_1 always works in DP mode.
> >>>>>>> https://android.googlesource.com/kernel/msm/+/af03eef7d4c3cbd1fe26c67d4f1915b05d0c1488/drivers/gpu/drm/msm/dp/dp_catalog_v200.c
> >>>>> Here it still programs the TCSR register.
> >>>>>
> >>>>>>> Based on these info, I believe these are separate PHYs, and only the
> >>>>>>> TCSR DP_PHYMODE registers determine which USB3/DP PHYs are paired or
> >>>>>>> mutually exclusive. This is why I have maintained separate private
> >>>>>>> data for each PHY and implemented Power on mutex control via TCSR,
> >>>>>>> rather than using a qmp_combo-like structure.
> >>>>> Still, no. Check the block diagram of SM6150.
> >>>>>
> >>>>>>> Given the above, do you think we still need to force USB and DP to be strictly bound together like a combo PHY?
> >>>>> Yes.
> >>>> I checked the related PHY series and block diagrams again.
> >>>>
> >>>> PRI and SEC go to different nodes based on the SoC design, and there are two types of configurations: USB3-only and USB3+DP pairing.
> >>>>
> >>>> Before proceed the v3 patchset, I’d like to double-confirm whether the following structure is what you expect:
> >>>>
> >>>> usb_qmpphy_1: phy@88e6000 {
> >>>>      compatible = "qcom,sm6150-qmp-usb3-prim-phy"; <== rename to PRIM
> >>> No, we already have a compatible name and DT schema for this device.
> >> Then current compatible name is "qcom,qcs615-qmp-usb3-phy" and shall we need update to "qcom,sm6150-qmp-usb3-phy"?
> >
> > Why? You _already_ have a compatible string. You don't need to change it just to follow the SoC name. 
> >
> Ok, but just to confirm — in this case, the USB3-DP PHY would use "qcom,sm6150-qmp-usb3-dp-phy" while the USB3-only PHY still uses "qcom,qcs615-qmp-usb3-phy"?
> 
> Since both PHYs are on the same SoC, would it make sense to keep the naming consistent and use "qcom,qcs615-..." for both? 

Either way is fine with me.

> 

-- 
With best wishes
Dmitry
Re: [PATCH v2 02/13] dt-bindings: phy: Add binding for QCS615 standalone QMP DP PHY
Posted by Xiangxu Yin 2 months ago
On 8/1/2025 3:30 PM, Dmitry Baryshkov wrote:
> On Fri, Aug 01, 2025 at 11:57:50AM +0800, Xiangxu Yin wrote:
>> On 8/1/2025 1:13 AM, Dmitry Baryshkov wrote:
>>> On 31/07/2025 08:06, Xiangxu Yin wrote:
>>>> On 7/31/2025 2:35 AM, Dmitry Baryshkov wrote:
>>>>> On Wed, Jul 30, 2025 at 04:53:16PM +0800, Xiangxu Yin wrote:
>>>>>> On 7/22/2025 8:41 PM, Dmitry Baryshkov wrote:
>>>>>>> On Tue, Jul 22, 2025 at 08:05:06PM +0800, Xiangxu Yin wrote:
>>>>>>>> On 7/22/2025 4:38 PM, Dmitry Baryshkov wrote:
>>>>>>>>> On Tue, Jul 22, 2025 at 03:22:03PM +0800, Xiangxu Yin wrote:
>>>>>>>>>> Introduce device tree binding documentation for the Qualcomm QMP DP PHY
>>>>>>>>>> on QCS615 SoCs. This PHY supports DisplayPort functionality and is
>>>>>>>>>> designed to operate independently from the USB3 PHY.
>>>>>>>>>>
>>>>>>>>>> Unlike combo PHYs found on other platforms, the QCS615 DP PHY is
>>>>>>>>>> standalone and does not support USB/DP multiplexing. The binding
>>>>>>>>>> describes the required clocks, resets, TCSR configuration, and clock/PHY
>>>>>>>>>> cells for proper integration.
>>>>>>>>> Simply put: no, this is not correct. Even if you go to the SM6150 block
>>>>>>>>> diagram, it points out that DP uses the USB3 PHY, not a separate DP PHY.
>>>>>>>>>
>>>>>>>>> I thought that we have discussed it beforehand.
>>>>>>>>>
>>>>>>>>> I can quote my comment from the previous thread:
>>>>>>>>>
>>>>>>>>>>> No. It means replacing extending existing entries with bigger reg and
>>>>>>>>>>> #phy-cells = <1>. The driver must keep working with old node definitions
>>>>>>>>>>> as is to ensure backwards compatibility. New nodes should make it
>>>>>>>>>>> register two PHYs (USB3 and DP). On the driver side modify generic code
>>>>>>>>>>> paths, all platforms supported by the driver should be able to support
>>>>>>>>>>> USB3+DP combination.
>>>>>>>>> Looking at the hardware memory maps:
>>>>>>>>>
>>>>>>>>> MSM8998: USB3 PHY regs at 0xc010000, DP PHY regs at 0xc011000
>>>>>>>>> SDM660: USB3 PHY regs at 0xc010000, DP PHY regs at 0xc011000
>>>>>>>>> QCM2290: USB3 PHY regs at 0x1615000, DP PHY regs at 0x1616000
>>>>>>>>> SM6115: USB3 PHY regs at 0x1615000, DP PHY regs at 0x1616000
>>>>>>>>>
>>>>>>>>> Now:
>>>>>>>>> SM6150: USB3 PHY regs at 0x88e6000
>>>>>>>>>          USB3 PHY regs at 0x88e8000, DP PHY regs at 0x88e9000
>>>>>>>>>
>>>>>>>>> I do not know, why msm-4.14 didn't describe second USB3 PHY. Maybe you
>>>>>>>>> can comment on it.
>>>>>>>>>
>>>>>>>>> But based on that list, the only special case that we need to handle is
>>>>>>>>> the first USB3 PHY, which doesn't have a corresponding DP PHY block. But
>>>>>>>>> it will be handled anyway by the code that implements support for the
>>>>>>>>> existing DT entries. All other hardware blocks are combo USB+DP PHYs.
>>>>>>>>>
>>>>>>>>> Having all of that in mind, please, for v3 patchset implement USB+DP
>>>>>>>>> support in the phy-qcom-qmp-usbc driver and add the following logic
>>>>>>>>> that also was requested in v1 review:
>>>>>>>>>
>>>>>>>>>>> Not quite. Both USB3 and DP drivers should be calling power_on / _off.
>>>>>>>>>>> If USB3 is on, powering on DP PHY should fail. Vice versa, if DP is on,
>>>>>>>>>>> powering on USB should fail.
>>>>>>>>> I think our understanding might not be fully aligned.
>>>>>>> I did not write this. Please fix your mailer to quote messages properly.
>>>>>>> As you are using Thunderbird, I'm not sure where the issue comes from.
>>>>>>>
>>>>>>> Also please fix it to wrap your responses somwhere logically.
>>>>>>>
>>>>>>>>> Perhaps this is because I didn’t accurately update the mutual exclusion relationships and test results for the different PHYs.
>>>>>>>>> Let me clarify my latest findings and explain why I believe these are separate PHYs that require mutual exclusion via TCSR.
>>>>>>>>>
>>>>>>>>> 1. About the TCSR DP_PHYMODE Registers
>>>>>>>>>
>>>>>>>>> MSM8998/SDM660:
>>>>>>>>>     Only one TCSR_USB3_DP_PHYMODE register at 0x1FCB248.
>>>>>>>>> QCM2290/SM6115:
>>>>>>>>>     TCSR_USB3_0_DP_PHYMODE at 0x3CB248
>>>>>>>>>     TCSR_USB3_1_DP_PHYMODE at 0x3CB24C
>>>>>>>>> SM6150:
>>>>>>>>>     TCSR_USB3_0_DP_PHYMODE at 0x1FCB248
>>>>>>>>>     TCSR_USB3_1_DP_PHYMODE at 0x1FCB24C
>>>>>>> SM6150 has two different sets of output pins, so the first register
>>>>>>> covers first set of SS lanes (which are routed to the documented SS
>>>>>>> PHY), the second register covers the second set of SS lanes (which are
>>>>>>> routed to the DP and secondary USB PHY).
>>>>>>>
>>>>>>> I can only assume that the same configuration was supposed to be
>>>>>>> applicable to QCM2290 / SM6115, but was later removed / disabled, while
>>>>>>> the registers were kept in the TCSR block.
>>>>>>>
>>>>>>>>> Even though MSM8998, SDM660, QCM2290, and SM6115 all have one USB3 PHY and one DP PHY, the TCSR DP_PHYMODE register configuration is different on each platform.
>>>>>>>>>
>>>>>>>>> Additionally, I found some interesting register documentation for QCM2290/SM6115:
>>>>>>>>>     TCSR_USB3_0_DP_PHYMODE: “In kamorta this one is for mobile usb. DP not supported.”
>>>>>>>>>     TCSR_USB3_1_DP_PHYMODE: “DP mode supported for Auto usb in kamorta.”
>>>>>>>>> I think the reason for having two different TCSR registers is to allow both the USB3.0 and DP PHYs to be useds at the same time in certain product configurations.
>>>>>>> Sure. One for the first PHY (USB), one for the second PHY (USB+DP).
>>>>>>> If you check the memory map, you will find the second VLS CLAMP register
>>>>>>> for the second USB PHY.
>>>>>>>
>>>>>>>>> 2. SM6150 Test Results
>>>>>>>>> When TCSR_DP_PHYMODE_0 is switched to DP, the USB3 primary PHY cannot work, and the DP PHY is also not functional (possibly due to clock lack or other configuration mismatch with this TCSR setting).
>>>>>>>>> When TCSR_DP_PHYMODE_1 is switched to DP, both the USB3 primary PHY and the DP PHY work normally.
>>>>>>>>> I think "why msm-4.14 didn't describe second USB3 PHY", because TCSR_DP_PHYMODE_1 always works in DP mode.
>>>>>>>>> https://android.googlesource.com/kernel/msm/+/af03eef7d4c3cbd1fe26c67d4f1915b05d0c1488/drivers/gpu/drm/msm/dp/dp_catalog_v200.c
>>>>>>> Here it still programs the TCSR register.
>>>>>>>
>>>>>>>>> Based on these info, I believe these are separate PHYs, and only the
>>>>>>>>> TCSR DP_PHYMODE registers determine which USB3/DP PHYs are paired or
>>>>>>>>> mutually exclusive. This is why I have maintained separate private
>>>>>>>>> data for each PHY and implemented Power on mutex control via TCSR,
>>>>>>>>> rather than using a qmp_combo-like structure.
>>>>>>> Still, no. Check the block diagram of SM6150.
>>>>>>>
>>>>>>>>> Given the above, do you think we still need to force USB and DP to be strictly bound together like a combo PHY?
>>>>>>> Yes.
>>>>>> I checked the related PHY series and block diagrams again.
>>>>>>
>>>>>> PRI and SEC go to different nodes based on the SoC design, and there are two types of configurations: USB3-only and USB3+DP pairing.
>>>>>>
>>>>>> Before proceed the v3 patchset, I’d like to double-confirm whether the following structure is what you expect:
>>>>>>
>>>>>> usb_qmpphy_1: phy@88e6000 {
>>>>>>      compatible = "qcom,sm6150-qmp-usb3-prim-phy"; <== rename to PRIM
>>>>> No, we already have a compatible name and DT schema for this device.
>>>> Then current compatible name is "qcom,qcs615-qmp-usb3-phy" and shall we need update to "qcom,sm6150-qmp-usb3-phy"?
>>> Why? You _already_ have a compatible string. You don't need to change it just to follow the SoC name. 
>>>
>> Ok, but just to confirm — in this case, the USB3-DP PHY would use "qcom,sm6150-qmp-usb3-dp-phy" while the USB3-only PHY still uses "qcom,qcs615-qmp-usb3-phy"?
>>
>> Since both PHYs are on the same SoC, would it make sense to keep the naming consistent and use "qcom,qcs615-..." for both? 
> Either way is fine with me.
Ok, then I’ll use |"qcom,qcs615-qmp-usb3-dp-phy"| for the USB3-DP PHY in the v3 patch.
>