Thread (16 messages) 16 messages, 3 authors, 2026-02-24

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 fail
quoted
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 useful
Clear, 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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help