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