From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mx0b-0031df01.pphosted.com (mx0b-0031df01.pphosted.com [205.220.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 3E3E137F010 for ; Tue, 21 Apr 2026 06:06:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=205.220.180.131 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776751575; cv=none; b=JdMWZ70kMvDftMjf+VO1jrTw3IPmTlFIUQG6GwgxlHbaE1LUljgtU6hSsxDC9icY2eRMGjQD+z9r8OCGpXWHgFMTu9IxBaMtmIP24xzSAb9gBi7mNa/CXx9crUmxcqn/7Pga4/gW0gzgwxutVrkrUf8eOx7+0cthYYA8fjs9Z4c= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776751575; c=relaxed/simple; bh=0ChHJjneCdtwjKLIkSPHeSzmtX7Fdl0q36FnIQ9gTXI=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=GSZT+31sMGBZJon5POdDxFJLa9DEsueX8QKeHo7g3QKfMgmOdgbvTLrjlONbl2KVioIbatxIP5Vb9L4Dsz81TLPhi3TBIioZKHsBImxg0ILAm8p7mVTbIjYVsyDbYf6qOVPQvQYqbl8kUWy0kubdR18RGZIKNjlWPVEf1VJmlug= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=oss.qualcomm.com; spf=pass smtp.mailfrom=oss.qualcomm.com; dkim=pass (2048-bit key) header.d=qualcomm.com header.i=@qualcomm.com header.b=EhQzvH9B; dkim=pass (2048-bit key) header.d=oss.qualcomm.com header.i=@oss.qualcomm.com header.b=gYwoqFl2; arc=none smtp.client-ip=205.220.180.131 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=oss.qualcomm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=oss.qualcomm.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=qualcomm.com header.i=@qualcomm.com header.b="EhQzvH9B"; dkim=pass (2048-bit key) header.d=oss.qualcomm.com header.i=@oss.qualcomm.com header.b="gYwoqFl2" Received: from pps.filterd (m0279869.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.18.1.11/8.18.1.11) with ESMTP id 63L64SlV3257286 for ; Tue, 21 Apr 2026 06:06:13 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=qualcomm.com; h= cc:content-transfer-encoding:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to; s=qcppdkim1; bh= x73AA1IG6HcWfqgG+vCPoGL693vFztSF6NnjQyhqMhA=; b=EhQzvH9BLXCA+sro HE7OMfeuv3edT7f+p3PdC9vw23JDqfmM22U2ayM/Pzlyj3LUkipwWTabhn2Sy5IQ MQYfs8sEl6+yg1jcRlXr2IxY8g2cXrK2eFgeKboztrDn7eT8BlpAt9r2pp9Qm3qv 7drDyAVAJlyQlLyxnAC/E4eAjkEGQuCx9s6Fiq6l69VeK3ntvXp9/O0q1BR2o7jc OsWmyAcXMxK7r7rZYSb4NehRKayE7pTAlrg0xHVoUs0HK+QkMDQHlNJv5g02r+lf OjnWoDCANF0AjO/svREY+KVv6d3k35jDzSbUM7HZGeOHK+o73HwsqjtjHzKmw+w6 Y/g9xA== Received: from mail-pg1-f199.google.com (mail-pg1-f199.google.com [209.85.215.199]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 4dp3na808m-1 (version=TLSv1.3 cipher=TLS_AES_128_GCM_SHA256 bits=128 verify=NOT) for ; Tue, 21 Apr 2026 06:06:12 +0000 (GMT) Received: by mail-pg1-f199.google.com with SMTP id 41be03b00d2f7-c76c2bb3149so1807270a12.1 for ; Mon, 20 Apr 2026 23:06:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oss.qualcomm.com; s=google; t=1776751572; x=1777356372; darn=vger.kernel.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=x73AA1IG6HcWfqgG+vCPoGL693vFztSF6NnjQyhqMhA=; b=gYwoqFl2MKC0iJru9WAD2ccLYYTctl8QIq/ULAEieLWeiT3fNcwDAVwhXNL6FFUGAm Wbh5TB5jiJz4fpqqN8oJYWttV4G+tQjJOSY4j07FIZwUaZmNt+jCiezifqM7rS4WiRW+ REsjHBDASzO+KtyVlj5yci0W+6DzlwqTEzI5oRDSmqNFh76o7I0NYHqxkvDAy7HOEC/5 0E64yLBNEs13BNCDjrTJovolgOR9YV2pNOB3mgnV/cQC8TDiSjXg6mLvlmH45fZJnGUJ n+JXw477B369vWpJHofPRZkvObwsJnIaMIgxnhdYIqOMHv5eKclvsT7edRS7ATveKplz zRjA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1776751572; x=1777356372; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=x73AA1IG6HcWfqgG+vCPoGL693vFztSF6NnjQyhqMhA=; b=J7JTU1wEQ1Jfug3ZNybHvEMfFrbelB8Xv8xrQXafFoiPsuSdnVjQOwE/2nly0p0rJo VZ/eRWjhL02mmstOsjyMziuft2HIemAXutXJRofzZnin+I78FoWEPU0mM01+P3HNNHkh sFnFH3A/qz/q0KasWZoImp6/w2XDRO055ptyF5GoQnxuTbohI536slTsGkyWUU6nSATG VmfaRhxfV44WKUR1xuREdzsMYCEtrkz6S+Uk+wzAWL6JZIgSggbuplQNsfWEta8Z2XK/ OXbpd+DeUNM4qmlE20OkeD1UJp+DaAD7SSDNXXfuthtnvi9UoTAnomJkH5C0aJ2kck6g fnsQ== X-Forwarded-Encrypted: i=1; AFNElJ8PGm434kZKYxLu2O6tEZDEtLickb1scTTEPmdBUEoUwrNOtQiNL0sm+y3R5gJTG42FEZkuu+qHv1CrhIs=@vger.kernel.org X-Gm-Message-State: AOJu0YxlpfEnDp3eCgJo1AMROWcPSyvNIIxLbLAw2GR059F1/q/itoGX J8iLkHn16ATekmgai5IFsn6zfao/deUm5r2Cu5Dj2A5HjPZVqbyTqQnsdHLY3Vt8NTW8s2yhZE9 RTYWNwLi2B2l48LEpZ+dmEdj3v1NoK41n18cRH3y4LQOdModDynQbsUeQTYZ71PPtqr0= X-Gm-Gg: AeBDieuJvYm3IkLFNZaddA35e6Ce4i50oliOxflPZfKJ8wDql6w3GBaXF7xP5WoX39K 0BMoXdkc9D4JCGQ8HA3gr3IC8qQFP7VBclLQyriz8xedCQgCwrJeHdQ9W9/6REr84fOFUz65KEk Tl8ddQB6iA0h/gCJXsqtkZf2CPPqVJ42bPkPMi5qA4RzdvttA5cU5h4FPUYrXTAAQtTAQ2pOMHs F2wmiT6anBeyDuK2Ov1NXqpv+5PvlhmpRt1fS/LTBP+Xg+h9X6vp9WeKu2cvygoWh+0kKWi2+w2 dXABeWyrAcQOhQF5P8SRFOAxewym/yw2XAlU/bXaRikbbtgZyT5APw/+n59qjHzoyTq+qxrE3go vCXfxTie4+ctjcrDBVxpl3Q8P5DhOk3pY5zVaBPpQAQg1KgBvlylm47Nyr1M6Zeno9t+clw== X-Received: by 2002:a05:6a00:4511:b0:82f:7762:3eb2 with SMTP id d2e1a72fcca58-82f8b507fdamr12071096b3a.17.1776751571729; Mon, 20 Apr 2026 23:06:11 -0700 (PDT) X-Received: by 2002:a05:6a00:4511:b0:82f:7762:3eb2 with SMTP id d2e1a72fcca58-82f8b507fdamr12071065b3a.17.1776751571216; Mon, 20 Apr 2026 23:06:11 -0700 (PDT) Received: from hu-arakshit-hyd.qualcomm.com ([202.46.22.19]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-82f932dac13sm12018462b3a.35.2026.04.20.23.06.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 20 Apr 2026 23:06:10 -0700 (PDT) Date: Tue, 21 Apr 2026 11:36:02 +0530 From: Abhinaba Rakshit To: Harshal Dev Cc: Bjorn Andersson , Konrad Dybcio , Manivannan Sadhasivam , "James E.J. Bottomley" , "Martin K. Petersen" , Adrian Hunter , Ulf Hansson , Neeraj Soni , Kuldeep Singh , Rob Herring , Krzysztof Kozlowski , Conor Dooley , linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org, linux-mmc@vger.kernel.org, devicetree@vger.kernel.org Subject: Re: [PATCH v8 1/5] soc: qcom: ice: Add OPP-based clock scaling support for ICE Message-ID: References: <20260409-enable-ice-clock-scaling-v8-0-ca1129798606@oss.qualcomm.com> <20260409-enable-ice-clock-scaling-v8-1-ca1129798606@oss.qualcomm.com> <4528374d-8175-4a1c-859f-23ddf2bbef52@oss.qualcomm.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <4528374d-8175-4a1c-859f-23ddf2bbef52@oss.qualcomm.com> X-Proofpoint-GUID: pITwuiG8t_H_tiuSnSLLwLOzVePlsdzs X-Proofpoint-ORIG-GUID: pITwuiG8t_H_tiuSnSLLwLOzVePlsdzs X-Proofpoint-Spam-Details-Enc: AW1haW4tMjYwNDIxMDA1NyBTYWx0ZWRfX6q5yzFgi7HhF 93NFtdHmA3rbXlEJSO9+oWxxh6q7vgbdAcy1VLNB/czrDuLd1gtMlX1encZluI0JQdvQlQRPlm9 YyFP7YeTnVKagy9D1MF+phK4JrckKZvPR6dpVz/Raz4lfTQyWf6VWEWKLOYA+GYF+1Lk9hdTnoD 18z86x+3MCHX75RWyOiBfixQBUbYbXc5jCubbMjj3clAdbbauYDIkIS1t8/kJ27CV/2b7uo0che 0tgpOMW6U2yivYPdOMO5/aq4NMr9t+rGfUjnSVf9rFxxiZHMFhumKIGsvFTrkZxr3aOhTli9YD/ Noo6KKsfaDJsuHeeJBN4fylAp0Zld/hLa/4s0K0JI4O4NBUooeHD6lWGM1ERcbX4CpT5nFVfDo6 Dkyu/ZHOhUpcaBAcaHJnBRQnDLCQoTicgqcmpyfBl/FPD+rYX41PIgI/eYGcqdsxSruwow5gEEz uFxgHXIxfASOG3UCfug== X-Authority-Analysis: v=2.4 cv=O9cJeh9W c=1 sm=1 tr=0 ts=69e713d4 cx=c_pps a=Oh5Dbbf/trHjhBongsHeRQ==:117 a=fChuTYTh2wq5r3m49p7fHw==:17 a=IkcTkHD0fZMA:10 a=A5OVakUREuEA:10 a=s4-Qcg_JpJYA:10 a=VkNPw1HP01LnGYTKEx00:22 a=u7WPNUs3qKkmUXheDGA7:22 a=_glEPmIy2e8OvE2BGh3C:22 a=EUspDBNiAAAA:8 a=XGHNUSocF6D1puBBiEoA:9 a=3ZKOabzyN94A:10 a=QEXdDO2ut3YA:10 a=_Vgx9l1VpLgwpw_dHYaR:22 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.293,Aquarius:18.0.1143,Hydra:6.1.51,FMLib:17.12.100.49 definitions=2026-04-21_01,2026-04-20_02,2025-10-01_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 adultscore=0 clxscore=1015 spamscore=0 impostorscore=0 malwarescore=0 lowpriorityscore=0 priorityscore=1501 suspectscore=0 bulkscore=0 phishscore=0 classifier=typeunknown authscore=0 authtc= authcc= route=outbound adjust=0 reason=mlx scancount=1 engine=8.22.0-2604070000 definitions=main-2604210057 On Fri, Apr 17, 2026 at 06:55:14PM +0530, Harshal Dev wrote: > > > On 4/9/2026 5:14 PM, Abhinaba Rakshit wrote: > > Register optional operation-points-v2 table for ICE device > > during device probe. Attach the OPP-table with only the ICE > > core clock. Since, dtbinding is on a trasition phase to include > > iface clock and clock-names, attaching the opp-table to core clock > > remains options such that it does not cause probe failures. > > > > Introduce clock scaling API qcom_ice_scale_clk which scale ICE > > core clock based on the target frequency provided and if a valid > > OPP-table is registered. Use round_ceil passed to decide on the > > rounding of the clock freq against OPP-table. Clock scaling is > > disabled when a valid OPP-table is not registered. > > > > This ensures when an ICE-device specific OPP table is available, > > use the PM OPP framework to manage frequency scaling and maintain > > proper power-domain constraints. > > > > Also, ensure to drop the votes in suspend to prevent power/thermal > > retention. Subsequently restore the frequency in resume from > > core_clk_freq which stores the last ICE core clock operating frequency. > > > > Signed-off-by: Abhinaba Rakshit > > --- > > drivers/soc/qcom/ice.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++ > > include/soc/qcom/ice.h | 2 ++ > > 2 files changed, 94 insertions(+) > > > > diff --git a/drivers/soc/qcom/ice.c b/drivers/soc/qcom/ice.c > > index bf4ab2d9e5c0360d8fe6135cc35f93b6b09e7a0e..9e869e6abc6300c7608b4d9a18e7f3e80c93f5e7 100644 > > --- a/drivers/soc/qcom/ice.c > > +++ b/drivers/soc/qcom/ice.c > > @@ -16,6 +16,7 @@ > > [..] > > > @@ -742,6 +800,40 @@ static int qcom_ice_probe(struct platform_device *pdev) > > if (IS_ERR(engine)) > > return PTR_ERR(engine); > > > > + /* qcom_ice_create() may return NULL if scm calls are not available */ > > + if (!engine) > > + return -EOPNOTSUPP; > > + > > + err = devm_pm_opp_set_clkname(&pdev->dev, "core"); > > + if (err && err != -ENOENT) { > > + dev_err(&pdev->dev, "Unable to set core clkname to OPP-table\n"); > > + return err; > > + } > > + > > + /* OPP table is optional */ > > + err = devm_pm_opp_of_add_table(&pdev->dev); > > + if (err && err != -ENODEV) { > > + dev_err(&pdev->dev, "Invalid OPP table in Device tree\n"); > > + return err; > > + } > > + > > + /* > > + * The OPP table is optional. devm_pm_opp_of_add_table() returns > > + * -ENODEV when no OPP table is present in DT, which is not treated > > + * as an error. Therefore, track successful OPP registration only > > + * when the return value is 0. > > + */ > > + engine->has_opp = (err == 0); > > + if (!engine->has_opp) > > + dev_info(&pdev->dev, "ICE OPP table is not registered, please update your DT\n"); > > + > > + /* > > + * Store the core clock rate for suspend resume cycles, > > + * against OPP aware DVFS operations. core_clk_freq will > > + * have a valid value only for non-legacy bindings. > > + */ > > + engine->core_clk_freq = clk_get_rate(engine->core_clk); > > + > > When you are calling 4-5 functions in a function, it's probably time to define another > function to keep things simple. Maybe qcom_ice_attach_opp_table(). > > Also, I still have issues with engine->has_opp = (err == 0), mostly because I don't > see this style used at other placed in the kernel. I would still suggest that you > make it simpler, but I won't hard-request it. > > /* The same explanatory comment as before */ > if (err == -ENODEV) > engine->has_opp = false; > dev_info(...); > else > engine->has_opp = true; > > With these optional suggestions, feel free to add: > > Reviewed-by: Harshal Dev Thanks for the review. Regarding the points you mentioned: At the moment, not much is happening here beyond registering the OPP table and caching the core clock rate. This is executed once per device probe, and the logic is fairly localized to the ICE probe path. Because of that, it didn’t feel like its reusable or demands its own helper. That said, I do see your point about modularity. If this logic grows further, or if we end up needing the same sequence elsewhere, I agree it would make sense to factor it out into a separate function at that time. Regarding engine->has_opp = (err == 0), I understand your concern. However, I intend to keep it this way, as it keeps the flow consise avoiding much of if-else branching and also serves the purpose. Abhinaba Rakshit