linux-kernel - Re: [PATCH V2] dt-bindings: clock: qcom: Remove required-opps from required list on SM8650

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: <14f57121-46f1-4dbe-92fd-e840705b771b@quicinc.com>
Date: Fri, 26 Jul 2024 20:00:42 +0530
From: Jagadeesh Kona <quic_jkona@...cinc.com>
To: Krzysztof Kozlowski <krzk@...nel.org>,
        Bjorn Andersson
	<andersson@...nel.org>,
        Michael Turquette <mturquette@...libre.com>,
        "Stephen
 Boyd" <sboyd@...nel.org>, Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski
	<krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Dmitry Baryshkov
	<dmitry.baryshkov@...aro.org>
CC: <linux-arm-msm@...r.kernel.org>, <linux-clk@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <devicetree@...r.kernel.org>,
        "Vladimir
 Zapolskiy" <vladimir.zapolskiy@...aro.org>,
        Bryan O'Donoghue
	<bryan.odonoghue@...aro.org>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        Taniya Das <quic_tdas@...cinc.com>,
        Satya Priya Kakitapalli
	<quic_skakitap@...cinc.com>,
        Imran Shaik <quic_imrashai@...cinc.com>,
        "Ajit
 Pandey" <quic_ajipan@...cinc.com>,
        kernel test robot <lkp@...el.com>
Subject: Re: [PATCH V2] dt-bindings: clock: qcom: Remove required-opps from
 required list on SM8650



On 7/24/2024 1:38 PM, Krzysztof Kozlowski wrote:
> On 20/07/2024 07:28, Jagadeesh Kona wrote:
>> On SM8650, the minimum voltage corner supported on MMCX from cmd-db is
>> sufficient for clock controllers to operate and there is no need to specify
>> the required-opps. Hence remove the required-opps property from the list of
>> required properties for SM8650 camcc and videocc bindings.
>>
>> This fixes:
>> arch/arm64/boot/dts/qcom/sm8650-mtp.dtb: clock-controller@...0000:
>> 'required-opps' is a required property
>>
>> arch/arm64/boot/dts/qcom/sm8650-mtp.dtb: clock-controller@...0000:
>> 'required-opps' is a required property
>>
>> Fixes: a6a61b9701d1 ("dt-bindings: clock: qcom: Add SM8650 video clock controller")
>> Fixes: 1ae3f0578e0e ("dt-bindings: clock: qcom: Add SM8650 camera clock controller")
>> Reported-by: kernel test robot <lkp@...el.com>
>> Closes: https://lore.kernel.org/oe-kbuild-all/202407070147.C9c3oTqS-lkp@intel.com/
>> Signed-off-by: Jagadeesh Kona <quic_jkona@...cinc.com>
>> ---
>> Changes in V2:
>>   - Made required: conditional and dropped required-opps from it only for SM8650 platform
>>   - Dropped Krzysztof Acked-by tag due to above changes
>>   - Link to V1: https://lore.kernel.org/all/20240708130836.19273-1-quic_jkona@quicinc.com/#r
>>
>> .../bindings/clock/qcom,sm8450-camcc.yaml     | 26 +++++++++++++------
>>   .../bindings/clock/qcom,sm8450-videocc.yaml   | 25 +++++++++++++-----
>>   2 files changed, 36 insertions(+), 15 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
>> index f58edfc10f4c..8698c801ed11 100644
>> --- a/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
>> +++ b/Documentation/devicetree/bindings/clock/qcom,sm8450-camcc.yaml
>> @@ -21,9 +21,6 @@ description: |
>>       include/dt-bindings/clock/qcom,sm8650-camcc.h
>>       include/dt-bindings/clock/qcom,x1e80100-camcc.h
>>   
>> -allOf:
>> -  - $ref: qcom,gcc.yaml#
>> -
>>   properties:
>>     compatible:
>>       enum:
>> @@ -53,11 +50,24 @@ properties:
>>     reg:
>>       maxItems: 1
>>   
>> -required:
> 
> You cannot remove required block.
> 
>> -  - compatible
>> -  - clocks
>> -  - power-domains
>> -  - required-opps
>> +allOf:
>> +  - $ref: qcom,gcc.yaml#
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: qcom,sm8650-camcc
>> +    then:
>> +      required:
>> +        - compatible
>> +        - clocks
>> +        - power-domains
>> +    else:
>> +      required:
>> +        - compatible
>> +        - clocks
>> +        - power-domains
>> +        - required-opps
>>   
>>   unevaluatedProperties: false
>>   
>> diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml
>> index b2792b4bb554..2e5a061f33d6 100644
>> --- a/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml
>> +++ b/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml
>> @@ -40,15 +40,26 @@ properties:
>>       description:
>>         A phandle to an OPP node describing required MMCX performance point.
>>   
>> -required:
> 
> No, you cannot remove required block.
> 
> To clarify: there is almost no single binding using your style. Even if
> there is one, then 99 others are using it differently. Do not implement
> things entirely different than everyone else. This is the same for C
> code you send upstream. No difference here...
> 

Thanks Krzysztof for the explanation.

Hi Dmitry,

As we discussed during SM8650 camcc and videocc changes, the MMCX rail's 
minimum voltage level from cmd-db is adequate for these clock 
controllers to operate on SM8650. So, we removed the 'required-opps' 
property from their DT nodes.

Although 'required-opps' will remain in the properties list, it’s not 
mandatory to be present in 'required:' list, as it is dependent on 
cmd-db minimum level. So, can I please go ahead and update these 
bindings to remove 'required-opps' from the 'required:' list, as done in 
v1 of this series.

It seems unconventional to make 'required:' conditional based on the 
platform type.

v1 link:- 
https://lore.kernel.org/all/rbej7rbjiwtgf4reiomtmlv3ef3ljfys5yfzypigrertylucu7@be3v65aeuimb/

Thanks,
Jagadeesh

> Best regards,
> Krzysztof
>