Received-SPF: pass (google.com: domain of linux-kernel+bounces-120759-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <e95634ba-b186-4fee-b93b-1ac4662fd2c4@quicinc.com>
Date: Wed, 27 Mar 2024 17:05:43 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v4 3/4] dt-bindings: interconnect: add clock property to
 enable QOS on SC7280
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
        Bjorn Andersson
	<andersson@kernel.org>,
        Konrad Dybcio <konrad.dybcio@linaro.org>,
        "Georgi
 Djakov" <djakov@kernel.org>, Rob Herring <robh@kernel.org>,
        "Krzysztof
 Kozlowski" <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley
	<conor+dt@kernel.org>
CC: Kees Cook <keescook@chromium.org>, <cros-qcom-dts-watchers@chromium.org>,
        "Gustavo A . R . Silva" <gustavoars@kernel.org>,
        <linux-arm-msm@vger.kernel.org>, <linux-pm@vger.kernel.org>,
        <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <linux-hardening@vger.kernel.org>, <quic_rlaggysh@quicinc.com>,
        <quic_mdtipton@quicinc.com>
References: <20240325181628.9407-1-quic_okukatla@quicinc.com>
 <20240325181628.9407-4-quic_okukatla@quicinc.com>
 <a259fa95-bfc3-4959-a159-8683df473e66@linaro.org>
Content-Language: en-US
From: Odelu Kukatla <quic_okukatla@quicinc.com>
In-Reply-To: <a259fa95-bfc3-4959-a159-8683df473e66@linaro.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit


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@quicinc.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