Drop the clock rate suffix from the NSS Clock Controller clock names for
PPE and NSS clocks. A generic name allows for easier extension of support
to additional SoCs that utilize same hardware design.
Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
---
.../devicetree/bindings/clock/qcom,ipq9574-nsscc.yaml | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/clock/qcom,ipq9574-nsscc.yaml b/Documentation/devicetree/bindings/clock/qcom,ipq9574-nsscc.yaml
index 17252b6ea3be..b9ca69172adc 100644
--- a/Documentation/devicetree/bindings/clock/qcom,ipq9574-nsscc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,ipq9574-nsscc.yaml
@@ -25,8 +25,8 @@ properties:
clocks:
items:
- description: Board XO source
- - description: CMN_PLL NSS 1200MHz (Bias PLL cc) clock source
- - description: CMN_PLL PPE 353MHz (Bias PLL ubi nc) clock source
+ - description: CMN_PLL NSS (Bias PLL cc) clock source
+ - description: CMN_PLL PPE (Bias PLL ubi nc) clock source
- description: GCC GPLL0 OUT AUX clock source
- description: Uniphy0 NSS Rx clock source
- description: Uniphy0 NSS Tx clock source
@@ -42,8 +42,8 @@ properties:
clock-names:
items:
- const: xo
- - const: nss_1200
- - const: ppe_353
+ - const: nss
+ - const: ppe
- const: gpll0_out
- const: uniphy0_rx
- const: uniphy0_tx
@@ -82,8 +82,8 @@ examples:
<&uniphy 5>,
<&gcc GCC_NSSCC_CLK>;
clock-names = "xo",
- "nss_1200",
- "ppe_353",
+ "nss",
+ "ppe",
"gpll0_out",
"uniphy0_rx",
"uniphy0_tx",
--
2.34.1On Thu, Jul 10, 2025 at 08:28:13PM +0800, Luo Jie wrote: > Drop the clock rate suffix from the NSS Clock Controller clock names for > PPE and NSS clocks. A generic name allows for easier extension of support > to additional SoCs that utilize same hardware design. This is an ABI change. You must state that here and provide a reason the change is okay (assuming it is). Otherwise, you are stuck with the name even if not optimal. > > Signed-off-by: Luo Jie <quic_luoj@quicinc.com> > --- > .../devicetree/bindings/clock/qcom,ipq9574-nsscc.yaml | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/Documentation/devicetree/bindings/clock/qcom,ipq9574-nsscc.yaml b/Documentation/devicetree/bindings/clock/qcom,ipq9574-nsscc.yaml > index 17252b6ea3be..b9ca69172adc 100644 > --- a/Documentation/devicetree/bindings/clock/qcom,ipq9574-nsscc.yaml > +++ b/Documentation/devicetree/bindings/clock/qcom,ipq9574-nsscc.yaml > @@ -25,8 +25,8 @@ properties: > clocks: > items: > - description: Board XO source > - - description: CMN_PLL NSS 1200MHz (Bias PLL cc) clock source > - - description: CMN_PLL PPE 353MHz (Bias PLL ubi nc) clock source > + - description: CMN_PLL NSS (Bias PLL cc) clock source > + - description: CMN_PLL PPE (Bias PLL ubi nc) clock source > - description: GCC GPLL0 OUT AUX clock source > - description: Uniphy0 NSS Rx clock source > - description: Uniphy0 NSS Tx clock source > @@ -42,8 +42,8 @@ properties: > clock-names: > items: > - const: xo > - - const: nss_1200 > - - const: ppe_353 > + - const: nss > + - const: ppe > - const: gpll0_out > - const: uniphy0_rx > - const: uniphy0_tx > @@ -82,8 +82,8 @@ examples: > <&uniphy 5>, > <&gcc GCC_NSSCC_CLK>; > clock-names = "xo", > - "nss_1200", > - "ppe_353", > + "nss", > + "ppe", > "gpll0_out", > "uniphy0_rx", > "uniphy0_tx", > > -- > 2.34.1 >
On 7/11/25 12:54 AM, Rob Herring wrote: > On Thu, Jul 10, 2025 at 08:28:13PM +0800, Luo Jie wrote: >> Drop the clock rate suffix from the NSS Clock Controller clock names for >> PPE and NSS clocks. A generic name allows for easier extension of support >> to additional SoCs that utilize same hardware design. > > This is an ABI change. You must state that here and provide a reason the > change is okay (assuming it is). Otherwise, you are stuck with the name > even if not optimal. The reason here seems to be simplifying the YAML.. which is not a good reason really.. I would instead suggest keeping the clocks list as-is for ipq9574 (this existing case), whereas improving it for any new additions Konrad
On 7/11/2025 8:15 PM, Konrad Dybcio wrote: > On 7/11/25 12:54 AM, Rob Herring wrote: >> On Thu, Jul 10, 2025 at 08:28:13PM +0800, Luo Jie wrote: >>> Drop the clock rate suffix from the NSS Clock Controller clock names for >>> PPE and NSS clocks. A generic name allows for easier extension of support >>> to additional SoCs that utilize same hardware design. >> >> This is an ABI change. You must state that here and provide a reason the >> change is okay (assuming it is). Otherwise, you are stuck with the name >> even if not optimal. > > The reason here seems to be simplifying the YAML.. which is not a good > reason really.. > > I would instead suggest keeping the clocks list as-is for ipq9574 (this > existing case), whereas improving it for any new additions > > Konrad Thanks Rob and Konrad for the comments. "nss_1200" and "nss" refer to the same clock pin on different SoC. As per Krzystof's previous comment on V2, including the frequency as a suffix in the clock name is not required, since only the frequencies vary across different IPQ SoCs, while the source clock pins for 'PPE' and 'NSS' clocks are the same. Hence this ABI change was deemed necessary. By removing the frequency suffix, the device tree bindings becomes more flexible and easier to extend for supporting new hardware variants in the future. Impact due to this ABI change: The NSS clock controller node is only enabled for the IPQ9574 DTS. In this patch series, the corresponding DTS changes for IPQ9574 are also included to align with this ABI change. Please let me know if further clarification or adjustments are needed.
On 7/18/25 11:12 AM, Luo Jie wrote:
>
>
> On 7/11/2025 8:15 PM, Konrad Dybcio wrote:
>> On 7/11/25 12:54 AM, Rob Herring wrote:
>>> On Thu, Jul 10, 2025 at 08:28:13PM +0800, Luo Jie wrote:
>>>> Drop the clock rate suffix from the NSS Clock Controller clock names for
>>>> PPE and NSS clocks. A generic name allows for easier extension of support
>>>> to additional SoCs that utilize same hardware design.
>>>
>>> This is an ABI change. You must state that here and provide a reason the
>>> change is okay (assuming it is). Otherwise, you are stuck with the name
>>> even if not optimal.
>>
>> The reason here seems to be simplifying the YAML.. which is not a good
>> reason really..
>>
>> I would instead suggest keeping the clocks list as-is for ipq9574 (this
>> existing case), whereas improving it for any new additions
>>
>> Konrad
>
> Thanks Rob and Konrad for the comments.
>
> "nss_1200" and "nss" refer to the same clock pin on different SoC.
> As per Krzystof's previous comment on V2, including the frequency
> as a suffix in the clock name is not required, since only the
> frequencies vary across different IPQ SoCs, while the source clock
> pins for 'PPE' and 'NSS' clocks are the same. Hence this ABI change
> was deemed necessary.
>
> By removing the frequency suffix, the device tree bindings becomes
> more flexible and easier to extend for supporting new hardware
> variants in the future.
>
> Impact due to this ABI change: The NSS clock controller node is only
> enabled for the IPQ9574 DTS. In this patch series, the corresponding
> DTS changes for IPQ9574 are also included to align with this ABI
> change.
The point of an ABI is to keep exposing the same interface without
any change requirements, i.e. if a customer ships the DT from
torvalds/master in firmware and is not willing to update it, they
can no longer update the kernel without a workaround.
> Please let me know if further clarification or adjustments are needed.
What we're asking for is that you don't alter the name on the
existing platform, but use a no-suffix version for the ones you
introduce going forward
i.e. (pseudo-YAML)
if:
properties:
compatible:
- const: qcom,ipq9574-nsscc
then:
properties:
clock-names:
items:
- clockname_1200
else:
properties:
clock-names:
items:
- clockname # no suffix
Konrad
On 7/18/2025 5:28 PM, Konrad Dybcio wrote: > On 7/18/25 11:12 AM, Luo Jie wrote: >> >> >> On 7/11/2025 8:15 PM, Konrad Dybcio wrote: >>> On 7/11/25 12:54 AM, Rob Herring wrote: >>>> On Thu, Jul 10, 2025 at 08:28:13PM +0800, Luo Jie wrote: >>>>> Drop the clock rate suffix from the NSS Clock Controller clock names for >>>>> PPE and NSS clocks. A generic name allows for easier extension of support >>>>> to additional SoCs that utilize same hardware design. >>>> >>>> This is an ABI change. You must state that here and provide a reason the >>>> change is okay (assuming it is). Otherwise, you are stuck with the name >>>> even if not optimal. >>> >>> The reason here seems to be simplifying the YAML.. which is not a good >>> reason really.. >>> >>> I would instead suggest keeping the clocks list as-is for ipq9574 (this >>> existing case), whereas improving it for any new additions >>> >>> Konrad >> >> Thanks Rob and Konrad for the comments. >> >> "nss_1200" and "nss" refer to the same clock pin on different SoC. >> As per Krzystof's previous comment on V2, including the frequency >> as a suffix in the clock name is not required, since only the >> frequencies vary across different IPQ SoCs, while the source clock >> pins for 'PPE' and 'NSS' clocks are the same. Hence this ABI change >> was deemed necessary. >> >> By removing the frequency suffix, the device tree bindings becomes >> more flexible and easier to extend for supporting new hardware >> variants in the future. >> >> Impact due to this ABI change: The NSS clock controller node is only >> enabled for the IPQ9574 DTS. In this patch series, the corresponding >> DTS changes for IPQ9574 are also included to align with this ABI >> change. > > The point of an ABI is to keep exposing the same interface without > any change requirements, i.e. if a customer ships the DT from > torvalds/master in firmware and is not willing to update it, they > can no longer update the kernel without a workaround. > >> Please let me know if further clarification or adjustments are needed. > > What we're asking for is that you don't alter the name on the > existing platform, but use a no-suffix version for the ones you > introduce going forward > > i.e. (pseudo-YAML) > > if: > properties: > compatible: > - const: qcom,ipq9574-nsscc > then: > properties: > clock-names: > items: > - clockname_1200 > else: > properties: > clock-names: > items: > - clockname # no suffix > > Konrad We had adopted this proposal in version 2 previously, but as noted in the discussion linked below, Krzysztof had suggested to avoid using the clock rate in the clock names when defining the constraints for them. However I do agree that we should keep the interface for IPQ9574 unchanged and instead use a generic clock name to support the newer SoCs. https://lore.kernel.org/all/20250701-optimistic-esoteric-swallow-d93fc6@krzk-bin/ Request Krzysztof to provide his comments as well, on whether we can follow your suggested approach to avoid breaking ABI for IPQ9574.
On 7/18/25 5:51 PM, Luo Jie wrote: > > > On 7/18/2025 5:28 PM, Konrad Dybcio wrote: >> On 7/18/25 11:12 AM, Luo Jie wrote: >>> >>> >>> On 7/11/2025 8:15 PM, Konrad Dybcio wrote: >>>> On 7/11/25 12:54 AM, Rob Herring wrote: >>>>> On Thu, Jul 10, 2025 at 08:28:13PM +0800, Luo Jie wrote: >>>>>> Drop the clock rate suffix from the NSS Clock Controller clock names for >>>>>> PPE and NSS clocks. A generic name allows for easier extension of support >>>>>> to additional SoCs that utilize same hardware design. >>>>> >>>>> This is an ABI change. You must state that here and provide a reason the >>>>> change is okay (assuming it is). Otherwise, you are stuck with the name >>>>> even if not optimal. >>>> >>>> The reason here seems to be simplifying the YAML.. which is not a good >>>> reason really.. >>>> >>>> I would instead suggest keeping the clocks list as-is for ipq9574 (this >>>> existing case), whereas improving it for any new additions >>>> >>>> Konrad >>> >>> Thanks Rob and Konrad for the comments. >>> >>> "nss_1200" and "nss" refer to the same clock pin on different SoC. >>> As per Krzystof's previous comment on V2, including the frequency >>> as a suffix in the clock name is not required, since only the >>> frequencies vary across different IPQ SoCs, while the source clock >>> pins for 'PPE' and 'NSS' clocks are the same. Hence this ABI change >>> was deemed necessary. >>> >>> By removing the frequency suffix, the device tree bindings becomes >>> more flexible and easier to extend for supporting new hardware >>> variants in the future. >>> >>> Impact due to this ABI change: The NSS clock controller node is only >>> enabled for the IPQ9574 DTS. In this patch series, the corresponding >>> DTS changes for IPQ9574 are also included to align with this ABI >>> change. >> >> The point of an ABI is to keep exposing the same interface without >> any change requirements, i.e. if a customer ships the DT from >> torvalds/master in firmware and is not willing to update it, they >> can no longer update the kernel without a workaround. >> >>> Please let me know if further clarification or adjustments are needed. >> >> What we're asking for is that you don't alter the name on the >> existing platform, but use a no-suffix version for the ones you >> introduce going forward >> >> i.e. (pseudo-YAML) >> >> if: >> properties: >> compatible: >> - const: qcom,ipq9574-nsscc >> then: >> properties: >> clock-names: >> items: >> - clockname_1200 >> else: >> properties: >> clock-names: >> items: >> - clockname # no suffix >> >> Konrad > > We had adopted this proposal in version 2 previously, but as noted in > the discussion linked below, Krzysztof had suggested to avoid using the > clock rate in the clock names when defining the constraints for them. > However I do agree that we should keep the interface for IPQ9574 > unchanged and instead use a generic clock name to support the newer > SoCs. > > https://lore.kernel.org/all/20250701-optimistic-esoteric-swallow-d93fc6@krzk-bin/ > > Request Krzysztof to provide his comments as well, on whether we can > follow your suggested approach to avoid breaking ABI for IPQ9574. Krzysztof, should the bindings be improved-through-breaking, or should there simply be a new YAML with un-suffixed entries, where new platforms would be added down the line? Konrad
On 29/07/2025 15:53, Konrad Dybcio wrote: >> >> We had adopted this proposal in version 2 previously, but as noted in >> the discussion linked below, Krzysztof had suggested to avoid using the >> clock rate in the clock names when defining the constraints for them. >> However I do agree that we should keep the interface for IPQ9574 >> unchanged and instead use a generic clock name to support the newer >> SoCs. >> >> https://lore.kernel.org/all/20250701-optimistic-esoteric-swallow-d93fc6@krzk-bin/ >> >> Request Krzysztof to provide his comments as well, on whether we can >> follow your suggested approach to avoid breaking ABI for IPQ9574. > > Krzysztof, should the bindings be improved-through-breaking, or should Unfortunately not, you should not change them for such reason. > there simply be a new YAML with un-suffixed entries, where new platforms > would be added down the line? Either new binding file or here with allOf:if:then differences per variant. Depends on readability. Best regards, Krzysztof
On 7/29/2025 9:57 PM, Krzysztof Kozlowski wrote: > On 29/07/2025 15:53, Konrad Dybcio wrote: >>> >>> We had adopted this proposal in version 2 previously, but as noted in >>> the discussion linked below, Krzysztof had suggested to avoid using the >>> clock rate in the clock names when defining the constraints for them. >>> However I do agree that we should keep the interface for IPQ9574 >>> unchanged and instead use a generic clock name to support the newer >>> SoCs. >>> >>> https://lore.kernel.org/all/20250701-optimistic-esoteric-swallow-d93fc6@krzk-bin/ >>> >>> Request Krzysztof to provide his comments as well, on whether we can >>> follow your suggested approach to avoid breaking ABI for IPQ9574. >> >> Krzysztof, should the bindings be improved-through-breaking, or should > > > Unfortunately not, you should not change them for such reason. > >> there simply be a new YAML with un-suffixed entries, where new platforms >> would be added down the line? > > > Either new binding file or here with allOf:if:then differences per > variant. Depends on readability. > OK. Thank you for the clarification. > > Best regards, > Krzysztof
© 2016 - 2025 Red Hat, Inc.