Re: [PATCH v5 2/4] soc: qcom: ice: Add OPP-based clock scaling support for ICE
From: Abhinaba Rakshit <hidden>
Date: 2026-02-20 11:15:46
Also in:
linux-arm-msm, linux-crypto, linux-scsi, lkml
On Fri, Feb 20, 2026 at 10:42:58AM +0100, Konrad Dybcio wrote:
On 2/20/26 8:33 AM, Abhinaba Rakshit wrote:quoted
On Thu, Feb 19, 2026 at 03:20:31PM +0100, Konrad Dybcio wrote:quoted
On 2/18/26 8:02 PM, Abhinaba Rakshit wrote:quoted
On Mon, Feb 16, 2026 at 01:18:57PM +0100, Konrad Dybcio wrote:quoted
On 2/13/26 8:02 AM, Abhinaba Rakshit wrote:quoted
On Thu, Feb 12, 2026 at 12:30:00PM +0100, Konrad Dybcio wrote:quoted
On 2/11/26 10:47 AM, Abhinaba Rakshit wrote:quoted
Register optional operation-points-v2 table for ICE device and aquire its minimum and maximum frequency during ICE device probe.[...]quoted
quoted
quoted
However, my main concern was for the corner cases, where: (target_freq > max && ROUND_CEIL) and (target_freq < min && ROUND_FLOOR) In both the cases, the OPP APIs will fail and the clock remains unchanged.I would argue that's expected behavior, if the requested rate can not be achieved, the "set_rate"-like function should failquoted
Hence, I added the checks to make the API as generic/robust as possible.AFAICT we generally set storage_ctrl_rate == ice_clk_rate with some slight play, but the latter never goes above the FMAX of the former For the second case, I'm not sure it's valid. For "find lowest rate" I would expect find_freq_*ceil*(rate=0). For other cases of scale-down I would expect that we want to keep the clock at >= (or ideally == )storage_ctrl_clk anyway so I'm not sure _floor() is usefulClear, I guess, the idea is to ensure ice-clk <= storage-clk in case of scale_up and ice-clk >= storage-clk in case of scale_down.I don't quite understand the first case (ice <= storage for scale_up), could you please elaborate?Here I basically mean to say is that, as you mentioned "we generally set storage_ctrl_rate == ice_clk_rate, but latter never goes above the FMAX of the former". I guess, the ideal way to handle this is to ensure using _floor when we want to scale_up. This ensures the ice_clk does not vote for more that what storage_ctrl is running on.Right, but what I was asking specifically is why we don't want that to happen
I would argue saying that, having ice_clk higher than storage_ctrl_clk does not makes sense, as it will not improve the throughput since the controller clock rate will still be a bottle-neck and it will surely drain more power.
quoted
Also, this avoids the corner case, where target_freq provided is higher that the supporter rates (descriped in ICE OPP-table) for ICE, using _ceil makes no sense.This is potentially a valid concern, do we have cases of storage_clk > ice_clk?
As of now, on the UFS storages (targets KLMT) I dont see the storage_clk (FMAX) > ice_clk (FMAX). They are mostly equal. However, I am not sure, about all the other targets and cannot call the same will persist on the upcommings as well. Abhinaba Rakshit