Re: [PATCH v3 1/4] dt-bindings: clock: qcom: Add QREF regulator supplies for glymur

[Krzysztof Kozlowski]

On 19/05/2026 13:25, Manivannan Sadhasivam wrote:
> On Mon, May 18, 2026 at 12:26:22AM -0700, Qiang Yu wrote:
>> On Mon, May 18, 2026 at 09:00:33AM +0200, Krzysztof Kozlowski wrote:
>>> On 18/05/2026 05:50, Qiang Yu wrote:
>>>> On Sun, May 17, 2026 at 10:27:39AM +0200, Krzysztof Kozlowski wrote:
>>>>> On 17/05/2026 07:39, Qiang Yu wrote:
>>>>>> On Thu, May 14, 2026 at 12:22:17PM +0200, Krzysztof Kozlowski wrote:
>>>>>>> On Wed, May 06, 2026 at 01:43:51AM -0700, Qiang Yu wrote:
>>>>>>>> Add regulator supply properties for the Glymur TCSR QREF/REFGEN blocks
>>>>>>>> required by clkref clocks.
>>>>>>>>
>>>>>>>> The vdda-qreftx*, vdda-qrefrpt*, and vdda-qrefrx* supplies map to common
>>>>>>>> QREF TX/RPT/RX components, while SoC-specific topology and instance count
>>>>>>>> differ. Document them here for qcom,glymur-tcsr.
>>>>>>>>
>>>>>>>> Signed-off-by: Qiang Yu <qiang.yu@xxxxxxxxxxxxxxxx>
>>>>>>>> ---
>>>>>>>>  .../bindings/clock/qcom,sm8550-tcsr.yaml           | 57 ++++++++++++++++++++++
>>>>>>>>  1 file changed, 57 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8550-tcsr.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8550-tcsr.yaml
>>>>>>>> index 1ccdf4b0f5dd..57921cb63230 100644
>>>>>>>> --- a/Documentation/devicetree/bindings/clock/qcom,sm8550-tcsr.yaml
>>>>>>>> +++ b/Documentation/devicetree/bindings/clock/qcom,sm8550-tcsr.yaml
>>>>>>>> @@ -51,6 +51,63 @@ properties:
>>>>>>>>    '#reset-cells':
>>>>>>>>      const: 1
>>>>>>>>  
>>>>>>>> +  vdda-refgen-0p9-supply: true
>>>>>>>> +  vdda-refgen-1p2-supply: true
>>>>>>>> +  vdda-qrefrx0-0p9-supply: true
>>>>>>>> +  vdda-qrefrx1-0p9-supply: true
>>>>>>>> +  vdda-qrefrx2-0p9-supply: true
>>>>>>>> +  vdda-qrefrx4-0p9-supply: true
>>>>>>>> +  vdda-qrefrx5-0p9-supply: true
>>>>>>>> +  vdda-qreftx0-0p9-supply: true
>>>>>>>> +  vdda-qreftx0-1p2-supply: true
>>>>>>>> +  vdda-qreftx1-0p9-supply: true
>>>>>>>> +  vdda-qrefrpt0-0p9-supply: true
>>>>>>>> +  vdda-qrefrpt1-0p9-supply: true
>>>>>>>> +  vdda-qrefrpt2-0p9-supply: true
>>>>>>>> +  vdda-qrefrpt3-0p9-supply: true
>>>>>>>> +  vdda-qrefrpt4-0p9-supply: true
>>>>>>>
>>>>>>> Either I do not understand your previous explanation:
>>>>>>> CXO -> TX0 -> RPT0 -> RPT1 -> RPT2 -> RX2 -> PCIe4_PHY
>>>>>>>
>>>>>>> or this is still wrong. There is no TCSR here, so this proves nothing.
>>>>>>> If TCSR is TX0, then you do not have five of them...
>>>>>>>
>>>>>>> My previous comment stay - you are not describing the actual hardware
>>>>>>> here.
>>>>>>>
>>>>>> The CXO network "-> TX0 -> RPT0 -> RPT1 -> RPT2 -> RX2 ->" is referred to
>>>>>> as the QREF block, and each component is controlled by the tcsr_clkref_en
>>>>>> registers.
>>>>>
>>>>> Still no clue what this -> relation is. Again, describe the hardware.
>>>>>
>>>>>>
>>>>>> If a PHY receives its reference clock from QREF, it will have a clkref_en
>>>>>> register. However, this register may be located in different regions
>>>>>> depending on the target. On glymur it resides in TCSR, so I added these
>>>>>> LDOs QREF required in tcsr yaml.
>>>>> Registers are not described as supplies.
>>>>
>>>> I'm not descirbing register as supply.
>>>>
>>>>     tx0-0p9/1p2  rpt0-0p9   rpt1-0p9    rpt2-0p9    rx2-0p9
>>>>        |             |           |        |           |
>>>>        |             |           |        |           |
>>>> CXO -> TX0 -------> RPT0 ------> RPT1 -> RPT2 -----> RX2 -> PCIe4_PHY
>>>>        |             |           |        |           |
>>>>        |             |           |        |           |
>>>>        ---------------------------------------------------tcsr_clkref_en
>>>>
>>>> These components(TX/RTP/RX) can be disabled/enabled by tcsr_clkref_en
>>>> register, and they require power supplies.
>>>
>>> So I told you more than once - none of these are supplies to the TCSR.
>>> You clearly misunderstand what a supply is.
>>>
>>
>> The TCSR binding here describes the tcsr_clkref_en clock gate, not the
>> TCSR register block itself. The clock gate controls whether the reference
>> clock is forwarded through the QREF chain to the PHY.
>>
>> The QREF components (TX/RPT/RX) sit between the clock gate and the PHY.
>> They require LDO supplies to operate, and those supplies must be enabled
>> before the clock gate is asserted and disabled after it is deasserted.
>> This enable/disable sequencing is the responsibility of the clock gate
>> driver, not the PHY driver.
>>
>> Since the supplies are managed as part of the clock gate operation, they
>> are modeled as properties of the clock gate node. The node happens to live
>> in TCSR on glymur, but the supplies describe what the clock gate needs to
>> do its job, not what TCSR itself needs.
>>
> 
> Just to add a bit more context:
> 
> The QREF block supplies the reference clock to the PHY IPs. But the digital
> logic (register interface) to control this QREF block lives inside TCSR in some
> SoCs like Glymur. But AFAIK, the analog QREF circuitry is not inside TCSR, but
> somewhere near to PHYs.
> 
> Also, QREF needs its own LDOs to operate and supply reference clocks to PHYs.
> Initially, we tried to add these QREF supplies to PHY node itself. But that was
> pushed back by Johan [1]. His argument was that since these LDOs power QREFs,
> not the PHY IPs, these supplies should not be added to the PHY nodes. And since
> we do not have a dedicated QREF DT node due to the fact that the QREF registers
> gets moved between various IPs based on the available space in the RTL. (It used
> to live in GCC, but now it is in TCSR and in the future it could be in some
> other IPs. Unfortunately, we cannot control this design)
> 
> So he suggested to add these supplies to TCSR node which acts as a control
> interface to QREF, even though it is not an accurate hw representation either.
> 
> And this patchset is based on that feedback only.
> 
> But your argument is also valid that these supplies are not supplying the TCSR
> block in hw, but just the QREF analog circuitry living close to PHY.
> 
> We are open to suggestions here as we do not know what is the accurate hardware
> description for these supplies/QREF.

As I understood these are real supplies to QREF which on these SoCs is
part of TCSR, so in general it is fine. Should be in its own binding
file, though.

Best regards,
Krzysztof