linux-kernel - Re: [PATCH 1/3] dt-bindings: interconnect: add clocks property to enable QoS on qcs8300

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <bf8fbe4b-d89e-424f-8445-0da2f80422c1@kernel.org>
Date: Sat, 29 Nov 2025 10:33:34 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Odelu Kukatla <odelu.kukatla@....qualcomm.com>,
 Georgi Djakov <djakov@...nel.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Bjorn Andersson <andersson@...nel.org>,
 Konrad Dybcio <konradybcio@...nel.org>
Cc: Raviteja Laggyshetty <raviteja.laggyshetty@....qualcomm.com>,
 Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>,
 Bartosz Golaszewski <bartosz.golaszewski@...aro.org>,
 linux-arm-msm@...r.kernel.org, linux-pm@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 Mike Tipton <mike.tipton@....qualcomm.com>
Subject: Re: [PATCH 1/3] dt-bindings: interconnect: add clocks property to
 enable QoS on qcs8300

On 28/11/2025 16:01, Odelu Kukatla wrote:
> Add 'clocks' property to enable QoS configuration. This property
> enables the necessary clocks for QoS configuration.
> 
> QoS configuration is essential for ensuring that latency sensitive
> components such as CPUs and multimedia engines receive prioritized
> access to memory and interconnect resources. This helps to manage
> bandwidth and latency across subsystems, improving system responsiveness
> and performance in concurrent workloads.

I don't see how clocks property help here at all. Are you getting clock
rates in the driver of some other clocks to make QoS decisions?

> 
> Both 'reg' and 'clocks' properties are optional. If either is missing,

No! They are not. How they can be optional in the hardware? How SoC can
have for ONE GIVEN device optional reg, meaning one board with the same
Soc has the IO address space but other board with the same SoC does not
have it.

> QoS configuration will be skipped. This behavior is controlled by the
> 'qos_requires_clocks' flag in the driver, which ensures that QoS
> configuration is bypassed when required clocks are not defined.

This suggests that - driver is not helping. Please describe the
hardware, not your drivers.

> 
> Signed-off-by: Odelu Kukatla <odelu.kukatla@....qualcomm.com>
> ---
>  .../interconnect/qcom,qcs8300-rpmh.yaml       | 53 ++++++++++++++++---
>  1 file changed, 47 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/interconnect/qcom,qcs8300-rpmh.yaml b/Documentation/devicetree/bindings/interconnect/qcom,qcs8300-rpmh.yaml
> index e9f528d6d9a8..594e835d1845 100644
> --- a/Documentation/devicetree/bindings/interconnect/qcom,qcs8300-rpmh.yaml
> +++ b/Documentation/devicetree/bindings/interconnect/qcom,qcs8300-rpmh.yaml
> @@ -35,6 +35,10 @@ properties:
>    reg:
>      maxItems: 1
>  
> +  clocks:
> +    minItems: 1
> +    maxItems: 4
> +
>  required:
>    - compatible
>  
> @@ -45,14 +49,39 @@ allOf:
>          compatible:
>            contains:
>              enum:
> -              - qcom,qcs8300-clk-virt
> -              - qcom,qcs8300-mc-virt
> +              - qcom,qcs8300-aggre1-noc
>      then:
>        properties:
> -        reg: false
> -    else:
> -      required:
> -        - reg

Why do you remove this? You cannot make random changes.

> +        clocks:
> +          items:
> +            - description: aggre UFS PHY AXI clock
> +            - description: aggre QUP PRIM AXI clock
> +            - description: aggre USB2 PRIM AXI clock
> +            - description: aggre USB3 PRIM AXI clock
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,qcs8300-aggre2-noc
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: RPMH CC IPA clock
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,qcs8300-gem-noc
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: GCC DDRSS GPU AXI clock

So all devices have clocks now? You made the schema less strict now,
removed pieces of it and you add more relaxed code telling every device
has a clock.

And none of this is explained in the commit msg.

>  
>  unevaluatedProperties: false
>  
> @@ -63,6 +92,7 @@ examples:
>          reg = <0x9100000 0xf7080>;
>          #interconnect-cells = <2>;
>          qcom,bcm-voters = <&apps_bcm_voter>;
> +        clocks = <&gcc_ddrss_gpu_axi_clk>;
>      };
>  
>      clk_virt: interconnect-0 {
> @@ -70,3 +100,14 @@ examples:
>          #interconnect-cells = <2>;
>          qcom,bcm-voters = <&apps_bcm_voter>;
>      };
> +
> +    aggre1_noc: interconnect@...0000 {

No need for new example, it is the same as previous.

> +        compatible = "qcom,qcs8300-aggre1-noc";
> +        reg = <0x016c0000 0x17080>;
> +        #interconnect-cells = <2>;
> +        qcom,bcm-voters = <&apps_bcm_voter>;
> +        clocks = <&gcc_aggre_ufs_phy_axi_clk>,
> +                 <&gcc_aggre_noc_qupv3_axi_clk>,
> +                 <&gcc_aggre_usb2_prim_axi_clk>,
> +                 <&gcc_aggre_usb3_prim_axi_clk>;
> +    };


Best regards,
Krzysztof