On 17/02/2023 22:28, Konrad Dybcio wrote:
On 17.02.2023 21:26, Dmitry Baryshkov wrote:
On Fri, 17 Feb 2023 at 21:53, Konrad Dybcio <konrad.dybcio@xxxxxxxxxx> wrote:
On 17.02.2023 20:27, Konrad Dybcio wrote:
On 17.02.2023 11:46, Konrad Dybcio wrote:
A0NoC only does bus scaling through RPM votes and does not have any
ICC clocks. Describe this.
Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxx>
---
This is bad, as devm_clk_get_bulk{"", _optional} doesn't
check if num_clocks makes sense and passes "-1" down the
devres alloc chain..
I'll rework this for the next revision by simply assigning
the common "bus", "bus_a" set everywhere instead of relying
on it being there by default..
Or maybe I shouldn't, as that will require redefining the array
over and over again.. Perhaps just passing <&xo_board>, <&xo_board>
to a0noc's "bus", "bus_a", similar to what's been done on SDM630's
GNoC would be less messy?
What about simply skipping a call to devm_clk_get if num_bus_clocks is negative?
I tested that locally before reporting the mistake on my side and
while it works, I just consider it.. ugly, because:
num_clocks =
0 => use the externally specified num_/clocks (logical)
=0 => use the default 2
<0 => consider there's zero
..but maybe that's just me.. if you don't find it ugly, I may just
go with that.
Would 'lesser ugliness' count? Maybe add a define 'ICC_RPM_NO_CLOCKS =
-1'? I think that spawning default bus & bus_a everywhere would be worse.
When this driver is rewritten in Rust, we'll have a clear distinction
between None (meaning default two clocks) and Some(0) (meaning no
clocks). Wait, does that sound even more ugly?
--
With best wishes
Dmitry
