On 7/18/25 5:51 PM, Luo Jie wrote:

On 7/18/2025 5:28 PM, Konrad Dybcio wrote:

quoted

On 7/18/25 11:12 AM, Luo Jie wrote:

quoted

On 7/11/2025 8:15 PM, Konrad Dybcio wrote:

quoted

On 7/11/25 12:54 AM, Rob Herring wrote:

quoted

On Thu, Jul 10, 2025 at 08:28:13PM +0800, Luo Jie wrote:

quoted

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.

quoted

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/ (local)

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