From: Krzysztof Kozlowski <krzk@kernel.org>
To: Kathiravan Thirumoorthy
<kathiravan.thirumoorthy@oss.qualcomm.com>,
Bjorn Andersson <andersson@kernel.org>,
Stephen Boyd <sboyd@kernel.org>,
Brian Masney <bmasney+clk@redhat.com>,
Jerome Brunet <jbrunet+clk@baylibre.com>,
Rob Herring <robh@kernel.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Conor Dooley <conor+dt@kernel.org>,
Georgi Djakov <djakov@kernel.org>,
Konrad Dybcio <konradybcio@kernel.org>,
Dmitry Baryshkov <lumag@kernel.org>,
Abel Vesa <abelvesa@kernel.org>
Cc: linux-arm-msm@vger.kernel.org, linux-clk@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-pm@vger.kernel.org
Subject: Re: [PATCH 05/11] dt-bindings: clock: qcom: ipq5332-gcc: Describe clock inputs per compatible
Date: Thu, 12 Nov 2026 10:32:56 +0100 [thread overview]
Message-ID: <ed3c4e4b-7c6a-46ea-b07c-ba7ce6214830@kernel.org> (raw)
In-Reply-To: <e8dadc91-226d-4b05-8de1-4b22318fc7ea@oss.qualcomm.com>
On 12/11/2026 10:12, Kathiravan Thirumoorthy wrote:
>
> On 9/25/2026 12:09 PM, Kathiravan Thirumoorthy wrote:
>>
>> 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.
>
> Krzysztof, I understand the frustration behind this, and we will make
> sure issues like this are caught earlier going forward.
>
> Could you please advise us on the acceptable way forward for this series?
>
Unfortunately I don't know what to advise. I cannot Ack it, because
people look at history commits, take them out of context and use as
arguments in discussion ("oh look, here you acked such change, so I can
do the same").
Maintainer's feedback was basically only partially implemented and now
you want to revert that part which was implemented, basically opening
the gate to allow such patterns in the future. I don't know how you are
going to guarantee such issues will not happen in the future, so I guess
you need to live with broken platform. Please align with Qualcomm SoC
maintainers on this, because I will not ack it.
Best regards,
Krzysztof
next prev parent reply other threads:[~2026-11-12 9:33 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-21 6:19 [PATCH 00/11] Add a few minor enhancements to the Qualcomm IPQ GCC drivers Kathiravan Thirumoorthy
2026-09-21 6:19 ` [PATCH 01/11] clk: qcom: ipq5018: add 24MHz rate for USB UTMI clock Kathiravan Thirumoorthy
2026-09-21 12:03 ` Abel Vesa
2026-09-21 6:19 ` [PATCH 02/11] clk: qcom: ipq5332: " Kathiravan Thirumoorthy
2026-09-21 12:03 ` Abel Vesa
2026-09-21 6:19 ` [PATCH 03/11] clk: qcom: ipq5210: " Kathiravan Thirumoorthy
2026-09-21 12:03 ` Abel Vesa
2026-09-21 6:19 ` [PATCH 04/11] arm64: dts: qcom: ipq9574: Wire up USB3 PHY PIPE clock to GCC Kathiravan Thirumoorthy
2026-09-21 11:55 ` Abel Vesa
2026-09-21 6:19 ` [PATCH 05/11] dt-bindings: clock: qcom: ipq5332-gcc: Describe clock inputs per compatible Kathiravan Thirumoorthy
2026-09-23 8:47 ` Krzysztof Kozlowski
2026-09-24 9:15 ` Kathiravan Thirumoorthy
2026-09-24 10:58 ` Krzysztof Kozlowski
2026-09-25 6:39 ` Kathiravan Thirumoorthy
2026-11-12 9:12 ` Kathiravan Thirumoorthy
2026-11-12 9:32 ` Krzysztof Kozlowski [this message]
2026-09-21 6:19 ` [PATCH 06/11] arm64: dts: qcom: ipq5424: Wire up USB3 PHY PIPE clock to GCC Kathiravan Thirumoorthy
2026-09-21 12:04 ` Abel Vesa
2026-09-21 6:19 ` [PATCH 07/11] dt-bindings: qcom,ipq5424: add NSS MEMNOC clock and ICC IDs Kathiravan Thirumoorthy
2026-09-22 12:33 ` Georgi Djakov
2026-09-21 6:19 ` [PATCH 08/11] clk: qcom: gcc-ipq5424: add the MEMNOC clocks needed for the NSS block Kathiravan Thirumoorthy
2026-09-21 12:05 ` Abel Vesa
2026-09-21 6:19 ` [PATCH 09/11] arm64: dts: qcom: ipq5424: Wire up PPE clock to GCC Kathiravan Thirumoorthy
2026-09-21 12:06 ` Abel Vesa
2026-09-21 6:19 ` [PATCH 10/11] clk: qcom: gcc-ipq5210: add UNIPHY XPCS reset entries Kathiravan Thirumoorthy
2026-09-21 12:06 ` Abel Vesa
2026-09-21 6:19 ` [PATCH 11/11] clk: qcom: apss-ipq5424: fix 'ipa' typos Kathiravan Thirumoorthy
2026-09-21 12:07 ` Abel Vesa
2026-09-29 3:14 ` (subset) [PATCH 00/11] Add a few minor enhancements to the Qualcomm IPQ GCC drivers Bjorn Andersson
2026-09-29 17:27 ` Kathiravan Thirumoorthy
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ed3c4e4b-7c6a-46ea-b07c-ba7ce6214830@kernel.org \
--to=krzk@kernel.org \
--cc=abelvesa@kernel.org \
--cc=andersson@kernel.org \
--cc=bmasney+clk@redhat.com \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=djakov@kernel.org \
--cc=jbrunet+clk@baylibre.com \
--cc=kathiravan.thirumoorthy@oss.qualcomm.com \
--cc=konradybcio@kernel.org \
--cc=krzk+dt@kernel.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=lumag@kernel.org \
--cc=robh@kernel.org \
--cc=sboyd@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox