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