From: Konrad Dybcio <konrad.dybcio@linaro.org>
To: Georgi Djakov <djakov@kernel.org>, Andy Gross <agross@kernel.org>,
Bjorn Andersson <andersson@kernel.org>,
Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: 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 v7 6/9] interconnect: qcom: rpm: Handle interface clocks
Date: Tue, 21 Mar 2023 15:23:02 +0100 [thread overview]
Message-ID: <9cd49e80-d13b-6d16-0469-4be3fdbd1762@linaro.org> (raw)
In-Reply-To: <32bcb19f-db74-8660-7270-305e81363376@kernel.org>
On 21.03.2023 14:56, Georgi Djakov wrote:
> Hi Konrad,
>
> Thank you for working on this and sorry about jumping a bit late into
> the discussion.
>
> On 8.03.23 23:40, Konrad Dybcio wrote:
>> Some (but not all) providers (or their specific nodes) require
>> specific clocks to be turned on before they can be accessed. Failure
>> to ensure that results in a seemingly random system crash (which
>> would usually happen at boot with the interconnect driver built-in),
>> resulting in the platform not booting up properly.
>
> These "interface" clocks seem to be used only to program QoS for the
> respective ip block (eg ufs). So if we don't program QoS, there should
> be no crashes, right?
Correct.
>
> I believe that in downstream they defer setting QoS until the first
> non-zero bandwidth request because of drivers that probe asynchronously
> or there is some firmware booting involved (IPA maybe).
Hmm.. that would make sense.
> And bad stuff
> might happen if we touch the clock while the firmware is still booting.
> So setting the QoS on the first non-zero bandwidth request might not be
> a bad idea.
Sounds like a plan, I don't think setting QoS parameters on buses
that are not in use does anything (citation needed).
Such nodes should probably be also excluded from sync_state
> by implementing get_bw() to return 0 bandwidth.
I agree, setting the QoS parameters in sync_state (so, without
their dependencies fully met) sounds like a recipe for disaster,
while getting rid of QoS in sync_state at all would leave the nodes
that are not explicitly referenced in the device tree unconfigured.
Konrad
>
> BR,
> Georgi
>
>>
>> Limit the number of bus_clocks to 2 (which is the maximum that SMD
>> RPM interconnect supports anyway) and handle non-scaling clocks
>> separately. Update MSM8996 and SDM660 drivers to make sure they do
>> not regress with this change.
>>
>> This unfortunately has to be done in one patch to prevent either
>> compile errors or broken bisect.
>>
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>> ---
>> drivers/interconnect/qcom/icc-rpm.c | 52 ++++++++++++++++++++++++++++++-------
>> drivers/interconnect/qcom/icc-rpm.h | 14 ++++++++--
>> drivers/interconnect/qcom/msm8996.c | 22 +++++++---------
>> drivers/interconnect/qcom/sdm660.c | 16 +++++-------
>> 4 files changed, 70 insertions(+), 34 deletions(-)
>>
> [..]
>
next prev parent reply other threads:[~2023-03-21 14:23 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-08 21:40 [PATCH v7 0/9] The great interconnecification fixation Konrad Dybcio
2023-03-08 21:40 ` [PATCH v7 1/9] interconnect: qcom: rpm: make QoS INVALID default Konrad Dybcio
2023-03-11 13:52 ` Dmitry Baryshkov
2023-03-08 21:40 ` [PATCH v7 2/9] interconnect: qcom: rpm: Add support for specifying channel num Konrad Dybcio
2023-03-11 13:54 ` Dmitry Baryshkov
2023-03-21 14:06 ` Georgi Djakov
2023-03-21 14:09 ` Konrad Dybcio
2023-03-21 14:21 ` Georgi Djakov
2023-03-21 14:23 ` Konrad Dybcio
2023-03-21 14:49 ` Georgi Djakov
2023-03-21 17:33 ` Konrad Dybcio
2023-03-08 21:40 ` [PATCH v7 3/9] interconnect: qcom: Sort kerneldoc entries Konrad Dybcio
2023-03-11 13:54 ` Dmitry Baryshkov
2023-03-08 21:40 ` [PATCH v7 4/9] interconnect: qcom: rpm: Rename icc desc clocks to bus_blocks Konrad Dybcio
2023-03-08 21:40 ` [PATCH v7 5/9] interconnect: qcom: rpm: Rename icc provider num_clocks to num_bus_clocks Konrad Dybcio
2023-03-08 21:40 ` [PATCH v7 6/9] interconnect: qcom: rpm: Handle interface clocks Konrad Dybcio
2023-03-10 14:21 ` Bryan O'Donoghue
2023-03-10 14:26 ` Konrad Dybcio
2023-03-10 16:47 ` Bryan O'Donoghue
2023-03-10 18:05 ` Konrad Dybcio
2023-03-11 0:16 ` Bryan O'Donoghue
2023-03-11 0:54 ` Konrad Dybcio
2023-03-11 12:11 ` Bryan O'Donoghue
2023-03-11 12:36 ` Konrad Dybcio
2023-03-11 13:32 ` Dmitry Baryshkov
2023-03-11 14:01 ` Dmitry Baryshkov
2023-03-11 14:29 ` Bryan O'Donoghue
2023-03-11 14:35 ` Dmitry Baryshkov
2023-03-11 14:38 ` Bryan O'Donoghue
2023-03-11 15:26 ` Dmitry Baryshkov
2023-03-21 13:58 ` Georgi Djakov
2023-03-21 14:11 ` Konrad Dybcio
2023-03-21 14:38 ` Georgi Djakov
2023-03-21 17:38 ` Georgi Djakov
2023-03-21 13:56 ` Georgi Djakov
2023-03-21 14:23 ` Konrad Dybcio [this message]
2023-03-08 21:40 ` [PATCH v7 7/9] interconnect: qcom: icc-rpm: Enforce 2 or 0 bus clocks Konrad Dybcio
2023-03-08 21:40 ` [PATCH v7 8/9] interconnect: qcom: rpm: Don't use clk_get_optional for bus clocks anymore Konrad Dybcio
2023-03-08 21:40 ` [PATCH v7 9/9] interconnect: qcom: msm8996: Promote to core_initcall Konrad Dybcio
2023-03-10 14:23 ` Bryan O'Donoghue
2023-03-10 14:27 ` 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=9cd49e80-d13b-6d16-0469-4be3fdbd1762@linaro.org \
--to=konrad.dybcio@linaro.org \
--cc=agross@kernel.org \
--cc=andersson@kernel.org \
--cc=bryan.odonoghue@linaro.org \
--cc=djakov@kernel.org \
--cc=dmitry.baryshkov@linaro.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
/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;
as well as URLs for NNTP newsgroup(s).