Re: [PATCH 05/11] dt-bindings: clock: qcom: ipq5332-gcc: Describe clock inputs per compatible

[Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>]

On 9/24/2026 4:28 PM, Krzysztof Kozlowski wrote:
> On 24/09/2026 11:15, Kathiravan Thirumoorthy wrote:
>> On 9/23/2026 2:17 PM, Krzysztof Kozlowski wrote:
>>> On 21/09/2026 08:19, Kathiravan Thirumoorthy wrote:
>>>> The IPQ5332 and IPQ5424 GCC blocks use different clock input orderings.
>>>> Document the clock inputs in compatible-specific schema constraints instead
>>>> of using a common clock description.
>>>>
>>>> Also add the 375 MHz PPE clock generated by the CMN PLL to the IPQ5424
>>>> clock input list.
>>>>
>>>> This keeps the binding aligned with the clock ordering used by the
>>>> existing DTS and GCC driver implementations.
>>>>
>>>> Signed-off-by: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
>>>> ---
>>>>    .../bindings/clock/qcom,ipq5332-gcc.yaml           | 26 +++++++++++++---------
>>>>    1 file changed, 16 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/clock/qcom,ipq5332-gcc.yaml b/Documentation/devicetree/bindings/clock/qcom,ipq5332-gcc.yaml
>>>> index 1230183fc0a9..fb39dffca988 100644
>>>> --- a/Documentation/devicetree/bindings/clock/qcom,ipq5332-gcc.yaml
>>>> +++ b/Documentation/devicetree/bindings/clock/qcom,ipq5332-gcc.yaml
>>>> @@ -25,14 +25,7 @@ properties:
>>>>    
>>>>      clocks:
>>>>        minItems: 5
>>>> -    items:
>>>> -      - description: Board XO clock source
>>>> -      - description: Sleep clock source
>>>> -      - description: PCIE 2lane PHY pipe clock source
>>>> -      - description: PCIE 2lane x1 PHY pipe clock source (For second lane)
>>>> -      - description: USB PCIE wrapper pipe clock source
>>>> -      - description: PCIE 2-lane PHY2 pipe clock source
>>>> -      - description: PCIE 2-lane PHY3 pipe clock source
>>> You are changing the order which is ABI break. Describe the actual
>>> problem to justify such change. Are you saying this was broken since
>>> beginning or what exactly is not working here? The driver was added by
>>> Qualcomm in 2024, so how did it even work? It's not some 10 yo legacy
>>> where we did some mistakes in binding while converting from TXT.
>>>
>>> Also, I do not get why the list cannot be shared.
>> When the bindings was posted, list is combined for both IPQ5332 and
>> IPQ5424. But the driver wasn't updated and it was following the below
>> order, so was DTS, which is not compatible the DT binding description.
>>
>> enum {
>>           DT_XO,
>>           DT_SLEEP_CLK,
>>           DT_PCIE30_PHY0_PIPE_CLK,
>>           DT_PCIE30_PHY1_PIPE_CLK,
>>           DT_PCIE30_PHY2_PIPE_CLK,
>>           DT_PCIE30_PHY3_PIPE_CLK,
>>           DT_USB_PCIE_WRAPPER_PIPE_CLK,
>>           DT_CMN_PLL_NSS_375M_CLK,
>> };
>>
>> So I'm splitting the description between IPQ5332 and IPQ5424 to matching
>> with the DTS and the driver description.
> So we asked back in 2024 to have the common part of the list. I even did
> the effort to dig it:
>
> https://lore.kernel.org/all/ruti7hmkxkayzmqfbme6rw6j2vbhlx4ul4ptcckwamcpd4cyfx@owufhxwpvlcj/
>
> Authors implemented it partially, broken drivers. Now you basically
> revert what we asked, which is BTW logical option. It's logical and
> reasonable, but leaves bitter feeling.
>
> I have feeling there is no accountability and no responsibility of the
> IPQ at all.
>
> No, that's not going to work. You cannot come 1.5 year later and revert
> what reviewer asked you, even if issue was just by mistake in the first
> place. And you should not even try to do that.

I understand the concern, and I agree this should have been caught much
earlier.

I only discovered the mismatch while working on the IPQ5424 clock
additions. My intent here is resolve the inconsistency that currently
exists between the binding and implementation.

>
> NAK

I'm happy to explore an alternative approach if updating the binding is
not considered acceptable.

>
> Best regards,
> Krzysztof