Re: [PATCH v3 6/7] phy: qcom-qmp-pcie: add support for ipq9574 gen3x2 PHY

[mr.nuke.me@gmail.com]

On 4/30/24 1:31 AM, Varadarajan Narayanan wrote:
> On Mon, Apr 29, 2024 at 01:55:32PM +0300, Dmitry Baryshkov wrote:
>> On Mon, 29 Apr 2024 at 09:20, Varadarajan Narayanan
>> <quic_varada@quicinc.com> wrote:
>>>
>>> On Wed, Apr 17, 2024 at 12:50:49AM +0300, Dmitry Baryshkov wrote:
>>>> On Wed, 17 Apr 2024 at 00:25, Alex G. <mr.nuke.me@gmail.com> wrote:
>>>>>
>>>>> Hi Dmitry,
>>>>>
>>>>> On 4/15/24 16:25, mr.nuke.me@gmail.com wrote:
>>>>>>
>>>>>>
>>>>>> On 4/15/24 15:10, Dmitry Baryshkov wrote:
>>>>>>> On Mon, 15 Apr 2024 at 21:23, Alexandru Gagniuc <mr.nuke.me@gmail.com>
>>>>>>> wrote:
>>>>>>>>
>>>>>>>> Add support for the gen3x2 PCIe PHY on IPQ9574, ported form downstream
>>>>>>>> 5.4 kernel. Only the serdes and pcs_misc tables are new, the others
>>>>>>>> being reused from IPQ8074 and IPQ6018 PHYs.
>>>>>>>>
>>>>>>>> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
>>>>>>>> ---
>>>>>>>>    drivers/phy/qualcomm/phy-qcom-qmp-pcie.c      | 136 +++++++++++++++++-
>>>>>>>>    .../phy/qualcomm/phy-qcom-qmp-pcs-pcie-v5.h   |  14 ++
>>>>>>>>    2 files changed, 149 insertions(+), 1 deletion(-)
>>>>>>>>
>>>>>>>
>>>>>>> [skipped]
>>>>>>>
>>>>>>>> @@ -2448,7 +2542,7 @@ static inline void qphy_clrbits(void __iomem
>>>>>>>> *base, u32 offset, u32 val)
>>>>>>>>
>>>>>>>>    /* list of clocks required by phy */
>>>>>>>>    static const char * const qmp_pciephy_clk_l[] = {
>>>>>>>> -       "aux", "cfg_ahb", "ref", "refgen", "rchng", "phy_aux",
>>>>>>>> +       "aux", "cfg_ahb", "ref", "refgen", "rchng", "phy_aux",
>>>>>>>> "anoc", "snoc"
>>>>>>>
>>>>>>> Are the NoC clocks really necessary to drive the PHY? I think they are
>>>>>>> usually connected to the controllers, not the PHYs.
>>>>>>
>>>>>> The system will hang if these clocks are not enabled. They are also
>>>>>> attached to the PHY in the QCA 5.4 downstream kernel.
>>>>
>>>> Interesting.
>>>> I see that Varadarajan is converting these clocks into interconnects.
>>>> Maybe it's better to wait for those patches to land and use
>>>> interconnects instead. I think it would better suit the
>>>> infrastructure.
>>>>
>>>> Varadarajan, could you please comment, are these interconnects
>>>> connected to the PHY too or just to the PCIe controller?
>>>
>>> Sorry for the late response. Missed this e-mail.
>>>
>>> These 2 clks are related to AXI port clk on Aggnoc/SNOC, not
>>> directly connected to PCIE wrapper, but it should be enabled to
>>> generate pcie traffic.
>>
>> So, are they required for the PHY or are they required for the PCIe
>> controller only?
> 
> These 2 clks are required for PCIe controller only.
> PCIE controller need these clks to send/receive axi pkts.

Very interesting information, thank you!

Dmitry, In light of this information do you want me to move these clocks 
out of the PHY and into the PCIe controller?

Alex

> Thanks
> Varada
> 
>>>>> They are named "anoc_lane", and "snoc_lane" in the downstream kernel.
>>>>> Would you like me to use these names instead?
>>>>
>>>> I'm fine either way.
>>>>
>>
>>
>>
>> --
>> With best wishes
>> Dmitry