From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
To: Konrad Dybcio <konrad.dybcio@linaro.org>
Cc: Andy Gross <agross@kernel.org>,
Bjorn Andersson <andersson@kernel.org>,
Georgi Djakov <djakov@kernel.org>,
Evan Green <evgreen@chromium.org>, Jun Nie <jun.nie@linaro.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Brian Masney <masneyb@onstation.org>,
Yassine Oudjana <y.oudjana@protonmail.com>,
Georgi Djakov <georgi.djakov@linaro.org>,
Bryan O'Donoghue <bryan.odonoghue@linaro.org>,
linux-arm-msm@vger.kernel.org, linux-pm@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 08/10] interconnect: qcom: msm8996: Specify no bus clock scaling on A0NoC
Date: Sat, 18 Feb 2023 00:19:34 +0200 [thread overview]
Message-ID: <ad231bd5-4308-4a69-86c5-5431eba2807d@linaro.org> (raw)
In-Reply-To: <5a2f502a-6530-dc8c-a81e-3d2a33964366@linaro.org>
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@linaro.org> 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@linaro.org>
>>>>> ---
>>>> 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
next prev parent reply other threads:[~2023-02-17 22:19 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-17 10:46 [PATCH v5 00/10] The great interconnecification fixation Konrad Dybcio
2023-02-17 10:46 ` [PATCH v5 01/10] interconnect: qcom: rpm: make QoS INVALID default, separate out driver data Konrad Dybcio
2023-02-23 13:01 ` Bryan O'Donoghue
2023-02-17 10:46 ` [PATCH v5 02/10] interconnect: qcom: rpm: Add support for specifying channel num Konrad Dybcio
2023-02-17 10:46 ` [PATCH v5 03/10] interconnect: qcom: Sort kerneldoc entries Konrad Dybcio
2023-02-17 10:46 ` [PATCH v5 04/10] interconnect: qcom: rpm: Rename icc desc clocks to bus_blocks Konrad Dybcio
2023-02-17 10:46 ` [PATCH v5 05/10] interconnect: qcom: rpm: Rename icc provider num_clocks to num_bus_clocks Konrad Dybcio
2023-02-17 10:46 ` [PATCH v5 06/10] interconnect: qcom: rpm: Handle interface clocks Konrad Dybcio
2023-02-17 10:46 ` [PATCH v5 07/10] interconnect: qcom: icc-rpm: Allow negative num_bus_clocks Konrad Dybcio
2023-02-17 10:46 ` [PATCH v5 08/10] interconnect: qcom: msm8996: Specify no bus clock scaling on A0NoC Konrad Dybcio
2023-02-17 19:27 ` Konrad Dybcio
2023-02-17 19:53 ` Konrad Dybcio
2023-02-17 20:26 ` Dmitry Baryshkov
2023-02-17 20:28 ` Konrad Dybcio
2023-02-17 22:19 ` Dmitry Baryshkov [this message]
2023-02-23 14:07 ` Bjorn Andersson
2023-02-23 14:17 ` Konrad Dybcio
2023-02-17 10:46 ` [PATCH v5 09/10] interconnect: qcom: rpm: Don't use clk_get_optional for bus clocks anymore Konrad Dybcio
2023-02-17 10:46 ` [PATCH v5 10/10] interconnect: qcom: msm8996: Promote to core_initcall Konrad Dybcio
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ad231bd5-4308-4a69-86c5-5431eba2807d@linaro.org \
--to=dmitry.baryshkov@linaro.org \
--cc=agross@kernel.org \
--cc=andersson@kernel.org \
--cc=bryan.odonoghue@linaro.org \
--cc=djakov@kernel.org \
--cc=evgreen@chromium.org \
--cc=georgi.djakov@linaro.org \
--cc=gregkh@linuxfoundation.org \
--cc=jun.nie@linaro.org \
--cc=konrad.dybcio@linaro.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=masneyb@onstation.org \
--cc=y.oudjana@protonmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox