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....@oss.qualcomm.com>
>>>>>>>> ---
>>>>>>>> .../bindings/phy/qcom,qcs615-qmp-dp-phy.yaml | 111
>>>>>>>> +++++++++++++++++++++
>>>>>>>> 1 file changed, 111 insertions(+)
>>>>>>>>
>
>
