linux-kernel - Re: [PATCH 8/8] ARM: dts: msm: Use Operation Points V2 for UFS 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: <9aab1765-224e-4f84-9366-a1b46e5260b7@quicinc.com>
Date: Mon, 20 Jan 2025 20:12:39 +0800
From: Ziqi Chen <quic_ziqichen@...cinc.com>
To: Krzysztof Kozlowski <krzk@...nel.org>, <quic_cang@...cinc.com>,
        <bvanassche@....org>, <mani@...nel.org>, <beanhuo@...ron.com>,
        <avri.altman@....com>, <junwoo80.lee@...sung.com>,
        <martin.petersen@...cle.com>, <quic_nguyenb@...cinc.com>,
        <quic_nitirawa@...cinc.com>, <quic_rampraka@...cinc.com>
CC: <linux-scsi@...r.kernel.org>, Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konradybcio@...nel.org>, Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>,
        "open list:ARM/QUALCOMM SUPPORT" <linux-arm-msm@...r.kernel.org>,
        "open
 list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
	<devicetree@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 8/8] ARM: dts: msm: Use Operation Points V2 for UFS on
 SM8650

Hi Krzysztof,

Thanks for review and comments~

As Neil has submitted a similar patch:
https://lore.kernel.org/all/20250115-topic-sm8x50-upstream-dt-icc-update-v1-10-eaa8b10e2af7@linaro.org/ 

I will withdraw this patch in next version.

-Ziqi

On 1/16/2025 5:22 PM, Krzysztof Kozlowski wrote:
> On 16/01/2025 10:11, Ziqi Chen wrote:
>> Use Operation Points V2 for UFS on SM8650 so that multi-level
>> clock/gear scaling can be possible.
>>
>> Co-developed-by: Can Guo <quic_cang@...cinc.com>
>> Signed-off-by: Can Guo <quic_cang@...cinc.com>
>> Signed-off-by: Ziqi Chen <quic_ziqichen@...cinc.com>
> 
> Please don't send downstream code directly, but fix it first. Actually -
> rework it 100%.
> 
> Please use subject prefixes matching the subsystem. You can get them for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching. For bindings, the preferred subjects are
> explained here:
> https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters
> >> ---
>>   arch/arm64/boot/dts/qcom/sm8650.dtsi | 51 +++++++++++++++++++++++-----
>>   1 file changed, 43 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>> index 01ac3769ffa6..5466f1217f64 100644
>> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>> @@ -2557,18 +2557,11 @@ ufs_mem_hc: ufs@...4000 {
>>   				      "tx_lane0_sync_clk",
>>   				      "rx_lane0_sync_clk",
>>   				      "rx_lane1_sync_clk";
>> -			freq-table-hz = <100000000 403000000>,
>> -					<0 0>,
>> -					<0 0>,
>> -					<100000000 403000000>,
>> -					<100000000 403000000>,
>> -					<0 0>,
>> -					<0 0>,
>> -					<0 0>;
>>   
>>   			resets = <&gcc GCC_UFS_PHY_BCR>;
>>   			reset-names = "rst";
>>   
>> +			operating-points-v2 = <&ufs_opp_table>;
>>   			interconnects = <&aggre1_noc MASTER_UFS_MEM QCOM_ICC_TAG_ALWAYS
>>   					 &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
>>   					<&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ALWAYS
>> @@ -2590,6 +2583,48 @@ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
>>   			#reset-cells = <1>;
>>   
>>   			status = "disabled";
>> +
>> +			ufs_opp_table: opp-table {
>> +					   compatible = "operating-points-v2";
> 
> 
> Messed indentation.
> 
>> +					   // LOW_SVS
> 
> 
> Drop
> 
>> +					   opp-100000000 {
>> +							   opp-hz = /bits/ 64 <100000000>,
>> +									   /bits/ 64 <0>,
> 
> Messed alignment.
> 
>> +									   /bits/ 64 <0>,
>> +									   /bits/ 64 <100000000>,
>> +									   /bits/ 64 <0>,
>> +									   /bits/ 64 <0>,
>> +									   /bits/ 64 <0>,
> 
> 
> 
> 
> Best regards,
> Krzysztof