The Networking Subsystem (NSS) clock controller acts as both a clock
provider and an interconnect provider. The #interconnect-cells property
is mandatory in the Device Tree Source (DTS) to ensure that client
drivers, such as the PPE driver, can correctly acquire ICC clocks from
the NSS ICC provider.
Although this property is already present in the NSS CC node of the DTS
for CMN PLL for IPQ9574 SoC which is currently supported, it was previously
omitted from the list of required properties in the bindings documentation.
Adding this as a required property is not expected to break the ABI for
currently supported SoC.
Marking #interconnect-cells as required to comply with Device Tree (DT)
binding requirements for interconnect providers.
Fixes: 28300ecedce4 ("dt-bindings: clock: Add ipq9574 NSSCC clock and reset definitions")
Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
---
Documentation/devicetree/bindings/clock/qcom,ipq9574-nsscc.yaml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/clock/qcom,ipq9574-nsscc.yaml b/Documentation/devicetree/bindings/clock/qcom,ipq9574-nsscc.yaml
index 17252b6ea3be..fc604279114f 100644
--- a/Documentation/devicetree/bindings/clock/qcom,ipq9574-nsscc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,ipq9574-nsscc.yaml
@@ -57,6 +57,7 @@ required:
- compatible
- clocks
- clock-names
+ - '#interconnect-cells'
allOf:
- $ref: qcom,gcc.yaml#
@@ -94,5 +95,6 @@ examples:
"bus";
#clock-cells = <1>;
#reset-cells = <1>;
+ #interconnect-cells = <1>;
};
...
--
2.34.1On Tue, Sep 09, 2025 at 09:39:11PM +0800, Luo Jie wrote: > The Networking Subsystem (NSS) clock controller acts as both a clock > provider and an interconnect provider. The #interconnect-cells property > is mandatory in the Device Tree Source (DTS) to ensure that client > drivers, such as the PPE driver, can correctly acquire ICC clocks from > the NSS ICC provider. > > Although this property is already present in the NSS CC node of the DTS > for CMN PLL for IPQ9574 SoC which is currently supported, it was previously > omitted from the list of required properties in the bindings documentation. > Adding this as a required property is not expected to break the ABI for > currently supported SoC. > > Marking #interconnect-cells as required to comply with Device Tree (DT) > binding requirements for interconnect providers. DT bindings do not require interconnect-cells, so that's not a correct reason. Drop them from required properties. Best regards, Krzysztof
On 9/12/25 9:04 AM, Krzysztof Kozlowski wrote: > On Tue, Sep 09, 2025 at 09:39:11PM +0800, Luo Jie wrote: >> The Networking Subsystem (NSS) clock controller acts as both a clock >> provider and an interconnect provider. The #interconnect-cells property >> is mandatory in the Device Tree Source (DTS) to ensure that client >> drivers, such as the PPE driver, can correctly acquire ICC clocks from >> the NSS ICC provider. >> >> Although this property is already present in the NSS CC node of the DTS >> for CMN PLL for IPQ9574 SoC which is currently supported, it was previously >> omitted from the list of required properties in the bindings documentation. >> Adding this as a required property is not expected to break the ABI for >> currently supported SoC. >> >> Marking #interconnect-cells as required to comply with Device Tree (DT) >> binding requirements for interconnect providers. > > DT bindings do not require interconnect-cells, so that's not a correct > reason. Drop them from required properties. "Mark #interconnect-cells as required to allow consuming the provided interconnect endpoints"? Konrad
On 12/09/2025 11:13, Konrad Dybcio wrote: > On 9/12/25 9:04 AM, Krzysztof Kozlowski wrote: >> On Tue, Sep 09, 2025 at 09:39:11PM +0800, Luo Jie wrote: >>> The Networking Subsystem (NSS) clock controller acts as both a clock >>> provider and an interconnect provider. The #interconnect-cells property >>> is mandatory in the Device Tree Source (DTS) to ensure that client >>> drivers, such as the PPE driver, can correctly acquire ICC clocks from >>> the NSS ICC provider. >>> >>> Although this property is already present in the NSS CC node of the DTS >>> for CMN PLL for IPQ9574 SoC which is currently supported, it was previously >>> omitted from the list of required properties in the bindings documentation. >>> Adding this as a required property is not expected to break the ABI for >>> currently supported SoC. >>> >>> Marking #interconnect-cells as required to comply with Device Tree (DT) >>> binding requirements for interconnect providers. >> >> DT bindings do not require interconnect-cells, so that's not a correct >> reason. Drop them from required properties. > > "Mark #interconnect-cells as required to allow consuming the provided > interconnect endpoints"? The point is they do not have to be required. Best regards, Krzysztof
On 9/12/2025 5:16 PM, Krzysztof Kozlowski wrote: > On 12/09/2025 11:13, Konrad Dybcio wrote: >> On 9/12/25 9:04 AM, Krzysztof Kozlowski wrote: >>> On Tue, Sep 09, 2025 at 09:39:11PM +0800, Luo Jie wrote: >>>> The Networking Subsystem (NSS) clock controller acts as both a clock >>>> provider and an interconnect provider. The #interconnect-cells property >>>> is mandatory in the Device Tree Source (DTS) to ensure that client >>>> drivers, such as the PPE driver, can correctly acquire ICC clocks from >>>> the NSS ICC provider. >>>> >>>> Although this property is already present in the NSS CC node of the DTS >>>> for CMN PLL for IPQ9574 SoC which is currently supported, it was previously >>>> omitted from the list of required properties in the bindings documentation. >>>> Adding this as a required property is not expected to break the ABI for >>>> currently supported SoC. >>>> >>>> Marking #interconnect-cells as required to comply with Device Tree (DT) >>>> binding requirements for interconnect providers. >>> >>> DT bindings do not require interconnect-cells, so that's not a correct >>> reason. Drop them from required properties. >> >> "Mark #interconnect-cells as required to allow consuming the provided >> interconnect endpoints"? > > > The point is they do not have to be required. The reason for adding this property as required is to enforce the DTS to define this important resource correctly. If this property is missed from the DTS, the client driver such as PPE driver will not be able to initialize correctly. This is necessary irrespective of whether these clocks are enabled by bootloader or not. The IPQ9574 SoC DTS defines this property even though the property was not marked as mandatory in the bindings, and hence the PPE driver is working. By now marking it as required, we can enforce that DTS files going forward for newer SoC (IPQ5424 and later) are properly defining this resource. This prevents any DTS misconfiguration and improves bindings validation as new SoCs are introduced. > > Best regards, > Krzysztof
On 16/09/2025 16:03, Luo Jie wrote: > > > On 9/12/2025 5:16 PM, Krzysztof Kozlowski wrote: >> On 12/09/2025 11:13, Konrad Dybcio wrote: >>> On 9/12/25 9:04 AM, Krzysztof Kozlowski wrote: >>>> On Tue, Sep 09, 2025 at 09:39:11PM +0800, Luo Jie wrote: >>>>> The Networking Subsystem (NSS) clock controller acts as both a clock >>>>> provider and an interconnect provider. The #interconnect-cells property >>>>> is mandatory in the Device Tree Source (DTS) to ensure that client >>>>> drivers, such as the PPE driver, can correctly acquire ICC clocks from >>>>> the NSS ICC provider. >>>>> >>>>> Although this property is already present in the NSS CC node of the DTS >>>>> for CMN PLL for IPQ9574 SoC which is currently supported, it was previously >>>>> omitted from the list of required properties in the bindings documentation. >>>>> Adding this as a required property is not expected to break the ABI for >>>>> currently supported SoC. >>>>> >>>>> Marking #interconnect-cells as required to comply with Device Tree (DT) >>>>> binding requirements for interconnect providers. >>>> >>>> DT bindings do not require interconnect-cells, so that's not a correct >>>> reason. Drop them from required properties. >>> >>> "Mark #interconnect-cells as required to allow consuming the provided >>> interconnect endpoints"? >> >> >> The point is they do not have to be required. > > The reason for adding this property as required is to enforce > the DTS to define this important resource correctly. If this property > is missed from the DTS, the client driver such as PPE driver will not > be able to initialize correctly. This is necessary irrespective of > whether these clocks are enabled by bootloader or not. The IPQ9574 SoC > DTS defines this property even though the property was not marked as > mandatory in the bindings, and hence the PPE driver is working. > > By now marking it as required, we can enforce that DTS files going > forward for newer SoC (IPQ5424 and later) are properly defining this > resource. This prevents any DTS misconfiguration and improves bindings > validation as new SoCs are introduced. So you explain to the DT maintainer how the DT works. Well, thank you, everyday I can learn something. You wasted a lot of our (multiple maintainers) time in the past, so I will just NAK your patches instead of wasting time again. Best regards, Krzysztof
On 9/17/2025 8:35 AM, Krzysztof Kozlowski wrote: > On 16/09/2025 16:03, Luo Jie wrote: >> >> >> On 9/12/2025 5:16 PM, Krzysztof Kozlowski wrote: >>> On 12/09/2025 11:13, Konrad Dybcio wrote: >>>> On 9/12/25 9:04 AM, Krzysztof Kozlowski wrote: >>>>> On Tue, Sep 09, 2025 at 09:39:11PM +0800, Luo Jie wrote: >>>>>> The Networking Subsystem (NSS) clock controller acts as both a clock >>>>>> provider and an interconnect provider. The #interconnect-cells property >>>>>> is mandatory in the Device Tree Source (DTS) to ensure that client >>>>>> drivers, such as the PPE driver, can correctly acquire ICC clocks from >>>>>> the NSS ICC provider. >>>>>> >>>>>> Although this property is already present in the NSS CC node of the DTS >>>>>> for CMN PLL for IPQ9574 SoC which is currently supported, it was previously >>>>>> omitted from the list of required properties in the bindings documentation. >>>>>> Adding this as a required property is not expected to break the ABI for >>>>>> currently supported SoC. >>>>>> >>>>>> Marking #interconnect-cells as required to comply with Device Tree (DT) >>>>>> binding requirements for interconnect providers. >>>>> >>>>> DT bindings do not require interconnect-cells, so that's not a correct >>>>> reason. Drop them from required properties. >>>> >>>> "Mark #interconnect-cells as required to allow consuming the provided >>>> interconnect endpoints"? >>> >>> >>> The point is they do not have to be required. >> >> The reason for adding this property as required is to enforce >> the DTS to define this important resource correctly. If this property >> is missed from the DTS, the client driver such as PPE driver will not >> be able to initialize correctly. This is necessary irrespective of >> whether these clocks are enabled by bootloader or not. The IPQ9574 SoC >> DTS defines this property even though the property was not marked as >> mandatory in the bindings, and hence the PPE driver is working. >> >> By now marking it as required, we can enforce that DTS files going >> forward for newer SoC (IPQ5424 and later) are properly defining this >> resource. This prevents any DTS misconfiguration and improves bindings >> validation as new SoCs are introduced. > > So you explain to the DT maintainer how the DT works. Well, thank you, > everyday I can learn something. > > You wasted a lot of our (multiple maintainers) time in the past, so I > will just NAK your patches instead of wasting time again. > > Best regards, > Krzysztof My sincere apologies for the misunderstanding and inconvenience my previous response has caused. I can assure you that my intention was never to describe the DT subsystem working, and am sorry that it has come out as such. I am committed to learning and continuously improving the quality of my contributions, and co-operating with reviewers and maintainers by following their feedback. I will update the patch accordingly to remove this property marking as required. Thank you very much for your support.
On 9/12/25 11:13 AM, Konrad Dybcio wrote: > On 9/12/25 9:04 AM, Krzysztof Kozlowski wrote: >> On Tue, Sep 09, 2025 at 09:39:11PM +0800, Luo Jie wrote: >>> The Networking Subsystem (NSS) clock controller acts as both a clock >>> provider and an interconnect provider. The #interconnect-cells property >>> is mandatory in the Device Tree Source (DTS) to ensure that client >>> drivers, such as the PPE driver, can correctly acquire ICC clocks from >>> the NSS ICC provider. >>> >>> Although this property is already present in the NSS CC node of the DTS >>> for CMN PLL for IPQ9574 SoC which is currently supported, it was previously >>> omitted from the list of required properties in the bindings documentation. >>> Adding this as a required property is not expected to break the ABI for >>> currently supported SoC. >>> >>> Marking #interconnect-cells as required to comply with Device Tree (DT) >>> binding requirements for interconnect providers. >> >> DT bindings do not require interconnect-cells, so that's not a correct >> reason. Drop them from required properties. > > "Mark #interconnect-cells as required to allow consuming the provided > interconnect endpoints"? "which are in turn necessary for the SoC to function" Konrad
On 12/09/2025 11:13, Konrad Dybcio wrote: > On 9/12/25 11:13 AM, Konrad Dybcio wrote: >> On 9/12/25 9:04 AM, Krzysztof Kozlowski wrote: >>> On Tue, Sep 09, 2025 at 09:39:11PM +0800, Luo Jie wrote: >>>> The Networking Subsystem (NSS) clock controller acts as both a clock >>>> provider and an interconnect provider. The #interconnect-cells property >>>> is mandatory in the Device Tree Source (DTS) to ensure that client >>>> drivers, such as the PPE driver, can correctly acquire ICC clocks from >>>> the NSS ICC provider. >>>> >>>> Although this property is already present in the NSS CC node of the DTS >>>> for CMN PLL for IPQ9574 SoC which is currently supported, it was previously >>>> omitted from the list of required properties in the bindings documentation. >>>> Adding this as a required property is not expected to break the ABI for >>>> currently supported SoC. >>>> >>>> Marking #interconnect-cells as required to comply with Device Tree (DT) >>>> binding requirements for interconnect providers. >>> >>> DT bindings do not require interconnect-cells, so that's not a correct >>> reason. Drop them from required properties. >> >> "Mark #interconnect-cells as required to allow consuming the provided >> interconnect endpoints"? > > "which are in turn necessary for the SoC to function" If this never worked and code was buggy, never booted, was sent incomplete and in junk state, then sure. Say like that. :) But I have a feeling code was working okayish... Best regards, Krzysztof
On 9/12/25 11:17 AM, Krzysztof Kozlowski wrote: > On 12/09/2025 11:13, Konrad Dybcio wrote: >> On 9/12/25 11:13 AM, Konrad Dybcio wrote: >>> On 9/12/25 9:04 AM, Krzysztof Kozlowski wrote: >>>> On Tue, Sep 09, 2025 at 09:39:11PM +0800, Luo Jie wrote: >>>>> The Networking Subsystem (NSS) clock controller acts as both a clock >>>>> provider and an interconnect provider. The #interconnect-cells property >>>>> is mandatory in the Device Tree Source (DTS) to ensure that client >>>>> drivers, such as the PPE driver, can correctly acquire ICC clocks from >>>>> the NSS ICC provider. >>>>> >>>>> Although this property is already present in the NSS CC node of the DTS >>>>> for CMN PLL for IPQ9574 SoC which is currently supported, it was previously >>>>> omitted from the list of required properties in the bindings documentation. >>>>> Adding this as a required property is not expected to break the ABI for >>>>> currently supported SoC. >>>>> >>>>> Marking #interconnect-cells as required to comply with Device Tree (DT) >>>>> binding requirements for interconnect providers. >>>> >>>> DT bindings do not require interconnect-cells, so that's not a correct >>>> reason. Drop them from required properties. >>> >>> "Mark #interconnect-cells as required to allow consuming the provided >>> interconnect endpoints"? >> >> "which are in turn necessary for the SoC to function" > > If this never worked and code was buggy, never booted, was sent > incomplete and in junk state, then sure. Say like that. :) > > But I have a feeling code was working okayish... If Linux is unaware of resources, it can't turn them off/on, so it was only working courtesy of the previous boot stages messing with them. Konrad
On 12/09/2025 11:21, Konrad Dybcio wrote: > On 9/12/25 11:17 AM, Krzysztof Kozlowski wrote: >> On 12/09/2025 11:13, Konrad Dybcio wrote: >>> On 9/12/25 11:13 AM, Konrad Dybcio wrote: >>>> On 9/12/25 9:04 AM, Krzysztof Kozlowski wrote: >>>>> On Tue, Sep 09, 2025 at 09:39:11PM +0800, Luo Jie wrote: >>>>>> The Networking Subsystem (NSS) clock controller acts as both a clock >>>>>> provider and an interconnect provider. The #interconnect-cells property >>>>>> is mandatory in the Device Tree Source (DTS) to ensure that client >>>>>> drivers, such as the PPE driver, can correctly acquire ICC clocks from >>>>>> the NSS ICC provider. >>>>>> >>>>>> Although this property is already present in the NSS CC node of the DTS >>>>>> for CMN PLL for IPQ9574 SoC which is currently supported, it was previously >>>>>> omitted from the list of required properties in the bindings documentation. >>>>>> Adding this as a required property is not expected to break the ABI for >>>>>> currently supported SoC. >>>>>> >>>>>> Marking #interconnect-cells as required to comply with Device Tree (DT) >>>>>> binding requirements for interconnect providers. >>>>> >>>>> DT bindings do not require interconnect-cells, so that's not a correct >>>>> reason. Drop them from required properties. >>>> >>>> "Mark #interconnect-cells as required to allow consuming the provided >>>> interconnect endpoints"? >>> >>> "which are in turn necessary for the SoC to function" >> >> If this never worked and code was buggy, never booted, was sent >> incomplete and in junk state, then sure. Say like that. :) >> >> But I have a feeling code was working okayish... > > If Linux is unaware of resources, it can't turn them off/on, so it was > only working courtesy of the previous boot stages messing with them. Which is fine and present in all other cases/drivers/devices. Entire Linux in many places relies on bootloader and that is not a "work by coincidence". Another thing is if you keep backwards compatibility in the driver but want to enforce DTS to care about these resources, but that is not explained here, I think. Best regards, Krzysztof
On 9/12/25 11:27 AM, Krzysztof Kozlowski wrote: > On 12/09/2025 11:21, Konrad Dybcio wrote: >> On 9/12/25 11:17 AM, Krzysztof Kozlowski wrote: >>> On 12/09/2025 11:13, Konrad Dybcio wrote: >>>> On 9/12/25 11:13 AM, Konrad Dybcio wrote: >>>>> On 9/12/25 9:04 AM, Krzysztof Kozlowski wrote: >>>>>> On Tue, Sep 09, 2025 at 09:39:11PM +0800, Luo Jie wrote: >>>>>>> The Networking Subsystem (NSS) clock controller acts as both a clock >>>>>>> provider and an interconnect provider. The #interconnect-cells property >>>>>>> is mandatory in the Device Tree Source (DTS) to ensure that client >>>>>>> drivers, such as the PPE driver, can correctly acquire ICC clocks from >>>>>>> the NSS ICC provider. >>>>>>> >>>>>>> Although this property is already present in the NSS CC node of the DTS >>>>>>> for CMN PLL for IPQ9574 SoC which is currently supported, it was previously >>>>>>> omitted from the list of required properties in the bindings documentation. >>>>>>> Adding this as a required property is not expected to break the ABI for >>>>>>> currently supported SoC. >>>>>>> >>>>>>> Marking #interconnect-cells as required to comply with Device Tree (DT) >>>>>>> binding requirements for interconnect providers. >>>>>> >>>>>> DT bindings do not require interconnect-cells, so that's not a correct >>>>>> reason. Drop them from required properties. >>>>> >>>>> "Mark #interconnect-cells as required to allow consuming the provided >>>>> interconnect endpoints"? >>>> >>>> "which are in turn necessary for the SoC to function" >>> >>> If this never worked and code was buggy, never booted, was sent >>> incomplete and in junk state, then sure. Say like that. :) >>> >>> But I have a feeling code was working okayish... >> >> If Linux is unaware of resources, it can't turn them off/on, so it was >> only working courtesy of the previous boot stages messing with them. > > > Which is fine and present in all other cases/drivers/devices. Entire > Linux in many places relies on bootloader and that is not a "work by > coincidence". > > Another thing is if you keep backwards compatibility in the driver but > want to enforce DTS to care about these resources, but that is not > explained here, I think. I don't feel like arguing axiology today ;) But I see your point and I won't object to either outcome, so long as the property is *allowed* As a sidenote the IPQ SoCs have a rather thin layer of fw Konrad
© 2016 - 2025 Red Hat, Inc.