Re: [PATCH v2 2/3] interconnect: qcom: Add CAMNOC interconnect provider driver
From: Gjorgji Rosikopulos (Consultant)
Date: Wed Sep 09 2026 - 09:23:12 EST
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@xxxxxxxxxxxxxxxx>
>> ---
<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@xxxxxxxxxx/
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