Re: [PATCH v6 5/8] clk: qcom: tcsrcc-glymur: Add Mahua QREF regulator support

[Konrad Dybcio]

On 6/29/26 3:32 PM, Qiang Yu wrote:
> On Mon, Jun 29, 2026 at 12:30:37PM +0200, Konrad Dybcio wrote:
>> On 6/26/26 5:53 AM, Qiang Yu wrote:
>>> On Mon, Jun 22, 2026 at 03:03:39PM +0200, Konrad Dybcio wrote:
>>>> On 6/22/26 2:49 PM, Qiang Yu wrote:
>>>>> On Mon, Jun 22, 2026 at 01:35:45PM +0200, Konrad Dybcio wrote:
>>>>>> On 6/22/26 7:11 AM, Qiang Yu wrote:
>>>>>>> Mahua is based on Glymur but uses a different QREF topology, requiring
>>>>>>> distinct regulator lists and clock descriptors for its PCIe clock
>>>>>>> references.
>>>>>>>
>>>>>>> Add mahua-specific regulator arrays and clk descriptor table, and use
>>>>>>> match_data to select the correct descriptor table per compatible string at
>>>>>>> probe time.
>>>>>>>
>>>>>>> Signed-off-by: Qiang Yu <qiang.yu@xxxxxxxxxxxxxxxx>
>>>>>>> ---
>>>>
>>>> [...]
>>>>
>>>>>> You're also missing PCIe_1_CLKREF_EN (+0x48) (for PCIe5)
>>>>>> which goes through CXO1_>TX->RPT0->RPT1->RPT2->RX2
>>>>>
>>>>> I have removed PCIe_1_CLKREF_EN in dts node because PCIe5 PHY doesn't
>>>>> require QREF. So I didn't provide its structure here.
>>>>
>>>> I don't quite get what you mean. I see that it is there in the graph
>>>
>>> I double-checked with the HW team — QREF is not required for CXO0. In TCSR
>>> HPG, I think the QREF components you are seeing are in an old version
>>> diagram, but it says the QREF for CXO0 will be removed. In the updated
>>> diagram, PCIe5, USB2_0, USB2_1, USB4_0 and USB4_1 PHYs get refclk directly
>>> from CXO0.
>>>
>>> In theory, we don't need to vote clkref_en and QREF LDOs for these PHY
>>> instances, and I got confirmation from the PCIe HW team for PCIe5 PHY.
>>>
>>> I also compared the description of the TCSR_CLKREF_EN register between
>>> glymur and mahua on ipcatlog. Unfortunately, only
>>> TCSR_USB4_1_CLKREF_EN_USB4_ENABLE is marked as "not in use". I believe
>>> this is because these registers are not documented well in ipcatlog.
>>>
>>> So I think part of the regulator lists you commented for USB instances is
>>> not required, and their dts node also can to be changed to use CXO as
>>> refclk instead of TCSR_CLKREF_EN. But the mapping between the USB instance
>>> names in the QREF diagram and the TCSR_CLKREF_EN registers is not clear —
>>> I cannot find TCSR_USB4_0_CLKREF_EN_USB4_ENABLE, but there is a USB4_0 PHY
>>> in the diagram. Can we leave the USB regulator list as NULL or we just
>>> add them as it is harmless?
>>
>> I believe the internal discussions we had indeed point to the TX/RPT
>> elements having been removed somewhere in the chip dev process, so
>> the regulators can go, in my understanding.
>>
>> As for the refclk toggles themselves, my understanding is that they are
>> still meaningful. Although, we should be able to just smoketest that - set
>> all of these registers to 0 and see if the related IPs still work normally
>>
> 
> As internal discussion, the TCSR_USBn_CLKREF_EN to digram instance mapping
> is:
> 
> TCSR_USB2_1_CLKREF_EN -> usb2_hs
> TCSR_USB2_2_CLKREF_EN -> usb2_hs3
> TCSR_USB2_3_CLKREF_EN -> usb2_hs6
> TCSR_USB2_4_CLKREF_EN -> usb2_2
> TCSR_USB3_0_CLKREF_EN -> usb3_ss3
> TCSR_USB3_1_CLKREF_EN -> usb3_ss4
> TCSR_USB4_1_CLKREF_EN -> usb4_1
> TCSR_USB4_2_CLKREF_EN -> usb4_2
> 
> I will remove regulator list for TCSR_USB2_1_CLKREF_EN and
> TCSR_USB4_1_CLKREF_EN. For the registers, we can leave them in the clk
> array, the driver can still vote CXO as parent of tcsr_clkref_en and the
> register setting in enable/disable is just a no-op if they are really not
> required.

sgtm

Konrad