Re: [PATCH v5 08/10] interconnect: qcom: msm8996: Specify no bus clock scaling on A0NoC

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux