Re: [PATCH 3/8] phy: qcom: qmp-usbc: Add DP phy mode support on QCS615

[Xiangxu Yin <quic_xiangxuy@xxxxxxxxxxx>]

On 12/12/2024 3:15 AM, Dmitry Baryshkov wrote:
> On Wed, Dec 11, 2024 at 08:50:02PM +0800, Xiangxu Yin wrote:
>>
>>
>> On 12/11/2024 5:46 PM, Dmitry Baryshkov wrote:
>>> On Wed, Dec 11, 2024 at 08:46:16AM +0800, Xiangxu Yin wrote:
>>>>
>>>>
>>>> On 12/10/2024 11:09 PM, Dmitry Baryshkov wrote:
>>>>> On Thu, Dec 05, 2024 at 08:31:24PM +0200, Dmitry Baryshkov wrote:
>>>>>> On Thu, Dec 05, 2024 at 09:26:47PM +0800, Xiangxu Yin wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 11/29/2024 10:33 PM, Dmitry Baryshkov wrote:
>>>>>>>> On Fri, 29 Nov 2024 at 09:59, Xiangxu Yin <quic_xiangxuy@xxxxxxxxxxx> wrote:
>>>>>>>>>
>>>>>>>>> Extended DP support for QCS615 USB or DP phy. Differentiated between
>>>>>>>>> USBC and DP PHY using the match table’s type, dynamically generating
>>>>>>>>> different types of cfg and layout attributes during initialization based
>>>>>>>>> on this type. Static variables are stored in cfg, while parsed values
>>>>>>>>> are organized into the layout structure.
>>>>>>>>
>>>>>>>> We didn't have an understanding / conclusion whether
>>>>>>>> qcom,usb-ssphy-qmp-usb3-or-dp PHYs are actually a single device / PHY
>>>>>>>> or two PHYs being placed next to each other. Could you please start
>>>>>>>> your commit message by explaining it? Or even better, make that a part
>>>>>>>> of the cover letter for a new series touching just the USBC PHY
>>>>>>>> driver. DP changes don't have anything in common with the PHY changes,
>>>>>>>> so you can split the series into two.
>>>>>>>>
>>>>>>> Before implement DP extension, we have discussed with abhinav and krishna about whether use combo, usbc or separate phy.
>>>>>>
>>>>>> What is "DP extension"?
>>>>>>
>>>> I'm sorry confusion casued by my description. It's means extend DP implemnt for USBC phy driver.
>>>>>>>
>>>>>>> We identified that DP and USB share some common controls for phy_mode and orientation.
>>>>>>> Specifically, 'TCSR_USB3_0_DP_PHYMODE' controls who must use the lanes - USB or DP,
>>>>>>> while PERIPH_SS_USB0_USB3PHY_PCS_MISC_TYPEC_CTRL controls the orientation.
>>>>>>> It would be more efficient for a single driver to manage these controls. 
>>>>>>
>>>>>> The question is about the hardware, not about the driver.
>>>>>>
>>>>>>> Additionally, this PHY does not support Alt Mode, and the two control registers are located in separate address spaces. 
>>>>>>> Therefore, even though the orientation for DP on this platform is always normal and connected to the video output board, 
>>>>>>> we still decided to base it on the USBC extension.
>>>>>>
>>>>>> Could you please clarify, do usb3-or-dp PHYs support DP-over-USB-C? I
>>>>>> thought that usbc-or-dp platforms support that, but they don't
>>>>>> support DP+USB pin configuration. Note, the question is broader than
>>>>>> just QCS615, it covers the PHY type itself.
>>>>>>
>>>>>> Also, is TCSR configuration read/write or read-only? Are we supposed to
>>>>>> set the register from OS or are we supposed to read it and thus detemine
>>>>>> the PHY mode?
>>>>>
>>>>> Any updates on these two topics?
>>>>>
>>>> Still confirming detail info with HW & design team.
>>>> I’ll update the information that has been confirmed so far.
>>>> This phy support DP-over-USB-C,but it's not support alt-mode which 2 lane work for DP, other 2 lane work for USB.
>>>> TCSR phy mode is read/write reg and we can read for determine phy mode.
>>>
>>> Ok, thanks for the explanation. From my point of view:
>>>
>>> - Implement the DP PHY to be a part of the same driver. Each device
>>>   supported by the usbc driver should get both PHYs.
>>>
>>> - Make sure not to break the ABI: #phy-cells = <0> should still work and
>>>   return USB PHY, keeping backwards compatibility. Newer devices or
>>>   upgraded DT for old devices should return USB PHY for <... 0> and DP
>>>   PHY for <... 1>.
>>>
>> Yes, currently we have implemented like your description,
>> Each deivce shoud get both PHYs, DP PHY for <... 1> and USB PHY for <... 0>.
> 
> Please note the backwards compatibility clause.
> 
For the USB node, we kept the same implementation as the original function interface, and the devicetree node definition also remains unchanged.
In subsequent patches, I will follow Krzysztof’s suggestion to use a separate DT-binding to describe the DP PHY configuration, 
without making changes to the USB devicetree and DT-binding implementation.
>>> - I'm not shure how to handle the USB and DP coexistence, especially in
>>>   your case of the USB-or-DP PHY.
>>>
>> For coexistence process:
>>
>> When we start implement DP part, usb driver team said only need config TCSR phy mode and orientation during switch in USB-C port.
>> Based on your previous comments avout SW_PWRDN, I'm confirming with the USB team whether SW_REST/SWPWRDN/START_CTRL registers might affect DP.
> 
> Thanks!
> 
>> Anyway, even though the original SoC design supports DP or USB over Type-C，
>> but on QCS615 ADP AIR platform, there are only four USB-A port which works with 'qcs615-qmp-usb3-phy' driver, and no USB-C port.
>> DP port is mappped from usb pin to the video out sub-board.
>> so we are unable to verify the switching case between DP and USB devices under USB-C.
> 
> That's also fine. We will get to that point once MSM8998 / SDM660
> get USB-C support (the only current blocker is the support for the
> TYPEC block of the PMI8998).
> 
I can't access MSM8998 / SDM660 documents now, but I have confirmed detail info about USB & DP phy design for sm6150.

The 'usb-ssphy-qmp-usb3-or-dp PHY' on the current platform is essentially composed of three sub-PHYs, 
which can even be considered as three separate PHYs: USB3 primary PHY, USB3 secondary PHY, and USB3 DP PHY.

On the QCS615, the USB primary PHY is currently used to handle USB 3.0 communication for the previously mentioned four USB Type-A ports, 
while the USB3 secondary PHY and USB3 DP PHY are used for the output of the Type-C port,
but since the Type-C port is forcibly pin-to-pin configured to the video out board, the Type-C port will always configure as DP PHY.

The internal registers of these three PHYs are independent of each other, Neither their respective SWPWR_DN nor SWRST will affect the other two PHYs.
Additionally, there was a misunderstanding about the orientation previously.
The USB orientation setting only affects the current PHY and does not impact the DP PHY. The DP PHY is configured in the DP_PHY_CFG_1.

TSCR_PHY_MODE can specify which PHY outputs to the Type-C port, and the global reset will simultaneously reset the two associated PHYs. 
Therefore, the correct switching process is as follows.
When switching the inserted device:
	1.Identify the PHY type.
	2.Enable the regulator.
	3.Trigger a reset.
	4.Enable the clock.
	5.Configure PHY type related orientation
	6.switch the TCSR PHY mode.
	7.Configure the registers of PHY.
During release:
	1.Reset.
	2.Disable the clock.
	3.Disable the regulator.

Our current design overall complies with this process, but it lacks the configuration for DP_PHY_CFG_1.

Shall we continue the discussion to clarify remain comments of the USBC driver?

>> However, I'm also confirming whether anything other will affect USB and DP each other.
>