Re: [PATCH v5 2/4] soc: qcom: ice: Add OPP-based clock scaling support for ICE
From: Konrad Dybcio <hidden>
Date: 2026-02-16 12:19:04
Also in:
linux-arm-msm, linux-devicetree, linux-scsi, lkml
On 2/13/26 8:02 AM, Abhinaba Rakshit wrote:
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
+ if (!ice->has_opp) + return -EOPNOTSUPP; + + /* Clamp the freq to max if target_freq is beyond supported frequencies */ + if (ice->max_freq && target_freq >= ice->max_freq) { + ice_freq = ice->max_freq; + goto scale_clock; + } + + /* Clamp the freq to min if target_freq is below supported frequencies */ + if (ice->min_freq && target_freq <= ice->min_freq) { + ice_freq = ice->min_freq; + goto scale_clock; + }The OPP framework won't let you overclock the ICE if this is what these checks are about. Plus the clk framework will perform rounding for you tooRight, maybe I can just add a check for 0 freq just to ensure the export API is not miss used. Something shown below: if (!target_freq) return -EINVAL; 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
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
Please let me know, your thoughts.quoted
quoted
+ + switch (flags) {Are you going to use these flags? Currently they're dead codeI agree, currently they are not used. However, since its an export API, I want to keep the rounding FLAGS support as it a common to have rounding flags in clock scaling APIs, and to support any future use-cases as well.
I think you have a bit of a misconception - yes, this is an export API and
should be designed with the consumers in mind, but then it's consumed by
in-tree modules only ("what's not on the list doesn't exist"), so it's actually
generally *discouraged* (with varying levels of emphasis) to add any code that
is not immediately useful, as these functions can be updated at any point in
time down the line
Konrad