[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e95634ba-b186-4fee-b93b-1ac4662fd2c4@quicinc.com>
Date: Wed, 27 Mar 2024 17:05:43 +0530
From: Odelu Kukatla <quic_okukatla@...cinc.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
Bjorn Andersson
<andersson@...nel.org>,
Konrad Dybcio <konrad.dybcio@...aro.org>,
"Georgi
Djakov" <djakov@...nel.org>, Rob Herring <robh@...nel.org>,
"Krzysztof
Kozlowski" <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley
<conor+dt@...nel.org>
CC: Kees Cook <keescook@...omium.org>, <cros-qcom-dts-watchers@...omium.org>,
"Gustavo A . R . Silva" <gustavoars@...nel.org>,
<linux-arm-msm@...r.kernel.org>, <linux-pm@...r.kernel.org>,
<devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<linux-hardening@...r.kernel.org>, <quic_rlaggysh@...cinc.com>,
<quic_mdtipton@...cinc.com>
Subject: Re: [PATCH v4 3/4] dt-bindings: interconnect: add clock property to
enable QOS on SC7280
On 3/26/2024 1:00 PM, Krzysztof Kozlowski wrote:
> On 25/03/2024 19:16, Odelu Kukatla wrote:
>> Added clock property to enable clocks required for accessing
>> qos registers.
>>
>> Signed-off-by: Odelu Kukatla <quic_okukatla@...cinc.com>
>> ---
>> .../bindings/interconnect/qcom,sc7280-rpmh.yaml | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/interconnect/qcom,sc7280-rpmh.yaml b/Documentation/devicetree/bindings/interconnect/qcom,sc7280-rpmh.yaml
>> index b135597d9489..950ecdd5252e 100644
>> --- a/Documentation/devicetree/bindings/interconnect/qcom,sc7280-rpmh.yaml
>> +++ b/Documentation/devicetree/bindings/interconnect/qcom,sc7280-rpmh.yaml
>> @@ -35,6 +35,10 @@ properties:
>> reg:
>> maxItems: 1
>>
>> + clocks:
>> + minItems: 1
>> + maxItems: 2
>
> Why is this flexible? Nothing in commit msg explains that. I gave the
> same talk twice, gave there examples, yet it is not enough...
>
Clocks property is optional, and can be either 1 or 2 or none.
I think "minItems: 1" should be removed. If no clock property is mentioned in node that means no clock is required to be enabled for QoS configuration.
I will add back specific number of clocks based on interconnect compatible similar to what i did in v3.
> https://elixir.bootlin.com/linux/v6.8/source/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml#L132
>
>> +
>> required:
>> - compatible
>>
>> @@ -57,6 +61,7 @@ unevaluatedProperties: false
>>
>> examples:
>> - |
>> + #include <dt-bindings/clock/qcom,gcc-sc7280.h>
>> interconnect {
>> compatible = "qcom,sc7280-clk-virt";
>> #interconnect-cells = <2>;
>> @@ -69,3 +74,12 @@ examples:
>> #interconnect-cells = <2>;
>> qcom,bcm-voters = <&apps_bcm_voter>;
>
> If all devices have clocks, then you could add them here. It seems not
> all of them have clocks...
>
> Best regards,
> Krzysztof
>
Thanks,
Odelu
Powered by blists - more mailing lists