Devicetree
 help / color / mirror / Atom feed
From: "Gjorgji Rosikopulos (Consultant)" <gjorgji.rosikopulos@oss.qualcomm.com>
To: Krzysztof Kozlowski <krzk@kernel.org>,
	Atanas Filipov <atanas.filipov@oss.qualcomm.com>
Cc: Georgi Djakov <djakov@kernel.org>,
	linux-arm-msm@vger.kernel.org, linux-pm@vger.kernel.org,
	Bjorn Andersson <andersson@kernel.org>,
	Konrad Dybcio <konradybcio@kernel.org>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Subject: Re: [PATCH v2 2/3] interconnect: qcom: Add CAMNOC interconnect provider driver
Date: Wed, 9 Sep 2026 15:58:21 +0300	[thread overview]
Message-ID: <4fc98d9e-5013-45b4-8fea-d0d6a8feea45@oss.qualcomm.com> (raw)
In-Reply-To: <20260827-adept-traditional-earwig-e2c8c6@quoll>

Hi Krzysztof,

Thanks for the review.

On 8/27/2026 12:25 PM, Krzysztof Kozlowski wrote:
> On Wed, Aug 19, 2026 at 12:32:10PM +0300, Atanas Filipov wrote:
>> Add an ICC provider for the Qualcomm Camera NOC (CAMNOC). Multiple
>> camera sub-devices share CAM_CC_CAMNOC_AXI_CLK; direct clk_set_rate()
>> calls from each consumer cause a last-writer-wins race.
>>
>> Each consumer obtains an ICC path to SLAVE_CAMNOC_AXI and votes via
>> icc_set_bw(). The ICC core aggregates votes (max peak_bw) and the
>> set() callback converts the result to clk_set_rate(). Consumers
>> encode the required frequency as peak_bw in kBps.
>>
>> Signed-off-by: Atanas Filipov <atanas.filipov@oss.qualcomm.com>
>> ---

<snip>

>> +static int qcom_camnoc_get_bw(struct icc_node *node, u32 *avg, u32 *peak)
>> +{
>> +	*avg = 0;
>> +	*peak = 0;
>> +
>> +	return 0;
>> +}
>> +
>> +static int qcom_camnoc_set(struct icc_node *src, struct icc_node *dst)
>> +{
>> +	struct qcom_camnoc_icc_provider *cp =
>> +		to_camnoc_provider(src->provider);
>> +	unsigned long rate;
>> +
>> +	/*
>> +	 * peak_bw is the aggregated max across all consumers (kBps).
>> +	 * Consumers encode the required clock frequency directly as kBps,
>> +	 * so the conversion is simply: rate_hz = peak_bw * 1000.
>> +	 * A vote of 0 means no requirement; leave the clock at its minimum.
>> +	 */
>> +	rate = icc_units_to_bps(dst->peak_bw);
>> +
>> +	return clk_set_rate(cp->clk, rate);
> 
> This driver does nothing to the ICC hardware, no votes. It only
> translates kbps to Hz. This is not the purpose of ICC drivers.
> 
> Looks heavily incomplete.

Maybe those series should be marked as RFC. The basic idea is to manage clocks as interconnect providers,
similar to this change: https://lore.kernel.org/all/20230512001334.2983048-3-dmitry.baryshkov@linaro.org/

There is requirement to maintain higher clock rate on source clocks and some place to aggregate them.
One example is camnoc axi clock which is used by jpeg, camss, ope and maybe other drivers.
The interconnect has already functionality inplace to maintain higher
bw per request which can be used for maintaining higher clock frequency from requested drivers.

One option is to use already available icc-clk provider, but we have to add many icc providers
instead one provider with many ports.
If you have an idea or other way of maintaining higher clock freq we can explore that as well.

Thanks,
~Gjorgji

  parent reply	other threads:[~2026-09-09 12:58 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-19  9:32 [PATCH v2 0/3] Add Qualcomm CAMNOC ICC provider Atanas Filipov
2026-08-19  9:32 ` [PATCH v2 1/3] dt-bindings: interconnect: Add Qualcomm CAMNOC ICC binding Atanas Filipov
2026-08-27  9:23   ` Krzysztof Kozlowski
2026-08-27  9:26     ` Krzysztof Kozlowski
2026-08-19  9:32 ` [PATCH v2 2/3] interconnect: qcom: Add CAMNOC interconnect provider driver Atanas Filipov
2026-08-19  9:41   ` sashiko-bot
2026-08-27  9:25   ` Krzysztof Kozlowski
2026-08-27  9:28     ` Krzysztof Kozlowski
2026-09-09 12:58     ` Gjorgji Rosikopulos (Consultant) [this message]
2026-08-19  9:32 ` [PATCH v2 3/3] arm64: dts: qcom: sm8250: Add CAMNOC ICC provider node Atanas Filipov

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=4fc98d9e-5013-45b4-8fea-d0d6a8feea45@oss.qualcomm.com \
    --to=gjorgji.rosikopulos@oss.qualcomm.com \
    --cc=andersson@kernel.org \
    --cc=atanas.filipov@oss.qualcomm.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=djakov@kernel.org \
    --cc=dmitry.baryshkov@oss.qualcomm.com \
    --cc=konradybcio@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=krzk@kernel.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=robh@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