mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Bryan O'Donoghue <bod@kernel.org>
To: Atanas Filipov <atanas.filipov@oss.qualcomm.com>,
	Georgi Djakov <djakov@kernel.org>,
	linux-arm-msm@vger.kernel.org, linux-pm@vger.kernel.org
Cc: 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>,
	Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
Subject: Re: [PATCH v2 2/3] interconnect: qcom: Add CAMNOC interconnect provider driver
Date: Tue, 29 Sep 2026 09:36:50 +0100	[thread overview]
Message-ID: <ea7958f2-44bb-477e-b06a-44153489f396@kernel.org> (raw)
In-Reply-To: <20260819093211.1436275-3-atanas.filipov@oss.qualcomm.com>

On 19/08/2026 10:32, 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>
> ---
>   drivers/interconnect/qcom/Kconfig       |  10 +
>   drivers/interconnect/qcom/Makefile      |   2 +
>   drivers/interconnect/qcom/qcom-camnoc.c | 279 ++++++++++++++++++++++++
>   3 files changed, 291 insertions(+)
>   create mode 100644 drivers/interconnect/qcom/qcom-camnoc.c
> 
> diff --git a/drivers/interconnect/qcom/Kconfig b/drivers/interconnect/qcom/Kconfig
> index 5b8a094ca4ed..9d9d238019a0 100644
> --- a/drivers/interconnect/qcom/Kconfig
> +++ b/drivers/interconnect/qcom/Kconfig
> @@ -135,6 +135,16 @@ config INTERCONNECT_QCOM_NORD
>   	  the RPMh hardware and communicates via Bus Clock Manager (BCM)
>   	  through the Resource State Coordinator (RSC).
> 
> +config INTERCONNECT_QCOM_CAMNOC
> +	tristate "Qualcomm CAMNOC interconnect driver"
> +	depends on INTERCONNECT_QCOM || COMPILE_TEST
> +	depends on ARM64 || COMPILE_TEST
> +	help
> +	  Say y here to support the Camera Network-on-Chip (CAMNOC)
> +	  interconnect driver for Qualcomm SoCs. It aggregates bandwidth
> +	  requests from camera sub-devices (IFE, JPEG, BPS, etc.) and
> +	  scales CAM_CC_CAMNOC_AXI_CLK accordingly.
> +
>   config INTERCONNECT_QCOM_OSM_L3
>   	tristate "Qualcomm OSM L3 interconnect driver"
>   	depends on INTERCONNECT_QCOM || COMPILE_TEST
> diff --git a/drivers/interconnect/qcom/Makefile b/drivers/interconnect/qcom/Makefile
> index 1c7d410b40cc..52a5998ae6a8 100644
> --- a/drivers/interconnect/qcom/Makefile
> +++ b/drivers/interconnect/qcom/Makefile
> @@ -18,6 +18,7 @@ qnoc-msm8974-objs			:= msm8974.o
>   qnoc-msm8976-objs			:= msm8976.o
>   qnoc-msm8996-objs			:= msm8996.o
>   qnoc-nord-objs				:= nord.o
> +icc-camnoc-objs				:= qcom-camnoc.o
>   icc-osm-l3-objs				:= osm-l3.o
>   qnoc-qcm2290-objs			:= qcm2290.o
>   qnoc-qcs404-objs			:= qcs404.o
> @@ -66,6 +67,7 @@ obj-$(CONFIG_INTERCONNECT_QCOM_MSM8974) += qnoc-msm8974.o
>   obj-$(CONFIG_INTERCONNECT_QCOM_MSM8976) += qnoc-msm8976.o
>   obj-$(CONFIG_INTERCONNECT_QCOM_MSM8996) += qnoc-msm8996.o
>   obj-$(CONFIG_INTERCONNECT_QCOM_NORD) += qnoc-nord.o
> +obj-$(CONFIG_INTERCONNECT_QCOM_CAMNOC) += icc-camnoc.o
>   obj-$(CONFIG_INTERCONNECT_QCOM_OSM_L3) += icc-osm-l3.o
>   obj-$(CONFIG_INTERCONNECT_QCOM_QCM2290) += qnoc-qcm2290.o
>   obj-$(CONFIG_INTERCONNECT_QCOM_QCS404) += qnoc-qcs404.o
> diff --git a/drivers/interconnect/qcom/qcom-camnoc.c b/drivers/interconnect/qcom/qcom-camnoc.c
> new file mode 100644
> index 000000000000..9a703b052b2d
> --- /dev/null
> +++ b/drivers/interconnect/qcom/qcom-camnoc.c
> @@ -0,0 +1,279 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + *
> + * Qualcomm Camera NOC (CAMNOC) interconnect provider.
> + *
> + * CAMNOC is the internal AXI interconnect within the Qualcomm camera
> + * subsystem. Multiple camera sub-devices (IFE, JPEG, BPS, etc.) share
> + * CAM_CC_CAMNOC_AXI_CLK. This driver acts as an ICC provider so that
> + * each sub-device can independently vote for bandwidth; the ICC core
> + * aggregates the votes (max of peak_bw across all consumers) and this
> + * driver translates the result into a clk_set_rate() call, avoiding
> + * the last-writer-wins race that occurs with direct clk_set_rate().
> + *
> + * Consumers express their required clock rate directly as peak_bw in
> + * kBps (e.g. 400000 for 400 MHz).  The driver converts kBps → Hz:
> + *   rate_hz = peak_bw_kBps * 1000
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/interconnect-provider.h>
> +#include <linux/interconnect.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +
> +#include <dt-bindings/interconnect/qcom,camnoc.h>
> +
> +#define to_camnoc_provider(_p) \
> +	container_of(_p, struct qcom_camnoc_icc_provider, provider)
> +
> +struct qcom_camnoc_icc_provider {
> +	struct clk *clk;
> +	struct icc_provider provider;
> +};
> +
> +struct qcom_camnoc_node {
> +	const char *name;
> +	u16 buswidth;
> +};
> +
> +/* IFE */
> +static const struct qcom_camnoc_node camnoc_master_ife_0 = {
> +	.name = "master-camnoc-ife-0", .buswidth = 16
> +};
> +
> +static const struct qcom_camnoc_node camnoc_master_ife_0_rdi = {
> +	.name = "master-camnoc-ife-0-rdi", .buswidth = 16
> +};
> +
> +static const struct qcom_camnoc_node camnoc_master_ife_1 = {
> +	.name = "master-camnoc-ife-1", .buswidth = 16
> +};
> +
> +static const struct qcom_camnoc_node camnoc_master_ife_1_rdi = {
> +	.name = "master-camnoc-ife-1-rdi", .buswidth = 16
> +};
> +
> +static const struct qcom_camnoc_node camnoc_master_ife_lite_0 = {
> +	.name = "master-camnoc-ife-lite-0", .buswidth = 8
> +};
> +
> +static const struct qcom_camnoc_node camnoc_master_ife_lite_1 = {
> +	.name = "master-camnoc-ife-lite-1", .buswidth = 8
> +};
> +
> +static const struct qcom_camnoc_node camnoc_master_ife_lite_2 = {
> +	.name = "master-camnoc-ife-lite-2", .buswidth = 8
> +};
> +
> +static const struct qcom_camnoc_node camnoc_master_ife_lite_3 = {
> +	.name = "master-camnoc-ife-lite-3", .buswidth = 8
> +};
> +
> +static const struct qcom_camnoc_node camnoc_master_ife_lite_4 = {
> +	.name = "master-camnoc-ife-lite-4", .buswidth = 8
> +};
> +
> +/* BPS */
> +static const struct qcom_camnoc_node camnoc_master_bps = {
> +	.name = "master-camnoc-bps", .buswidth = 16
> +};
> +
> +/* IPE */
> +static const struct qcom_camnoc_node camnoc_master_ipe_0 = {
> +	.name = "master-camnoc-ipe-0", .buswidth = 16
> +};
> +
> +static const struct qcom_camnoc_node camnoc_master_ipe_1 = {
> +	.name = "master-camnoc-ipe-1", .buswidth = 16
> +};
> +
> +/* JPEG */
> +static const struct qcom_camnoc_node camnoc_master_jpeg = {
> +	.name = "master-camnoc-jpeg", .buswidth = 16
> +};
> +
> +static const struct qcom_camnoc_node camnoc_master_jpeg_dma = {
> +	.name = "master-camnoc-jpeg-dma", .buswidth = 16
> +};
> +
> +/* CDM */
> +static const struct qcom_camnoc_node camnoc_master_cdm = {
> +	.name = "master-camnoc-cdm", .buswidth = 4
> +};
> +
> +/* FD */
> +static const struct qcom_camnoc_node camnoc_master_fd = {
> +	.name = "master-camnoc-fd", .buswidth = 16
> +};
> +
> +/* SBI */
> +static const struct qcom_camnoc_node camnoc_master_sbi = {
> +	.name = "master-camnoc-sbi", .buswidth = 16
> +};
> +
> +/* ICP */
> +static const struct qcom_camnoc_node camnoc_master_icp = {
> +	.name = "master-camnoc-icp", .buswidth = 8
> +};
> +
> +/* Slave */
> +static const struct qcom_camnoc_node camnoc_slave_axi = {
> +	.name = "slave-camnoc-axi", .buswidth = 32
> +};
> +
> +static const struct qcom_camnoc_node * const camnoc_nodes[] = {
> +	[MASTER_CAMNOC_IFE_0]      = &camnoc_master_ife_0,
> +	[MASTER_CAMNOC_IFE_0_RDI]  = &camnoc_master_ife_0_rdi,
> +	[MASTER_CAMNOC_IFE_1]      = &camnoc_master_ife_1,
> +	[MASTER_CAMNOC_IFE_1_RDI]  = &camnoc_master_ife_1_rdi,
> +	[MASTER_CAMNOC_IFE_LITE_0] = &camnoc_master_ife_lite_0,
> +	[MASTER_CAMNOC_IFE_LITE_1] = &camnoc_master_ife_lite_1,
> +	[MASTER_CAMNOC_IFE_LITE_2] = &camnoc_master_ife_lite_2,
> +	[MASTER_CAMNOC_IFE_LITE_3] = &camnoc_master_ife_lite_3,
> +	[MASTER_CAMNOC_IFE_LITE_4] = &camnoc_master_ife_lite_4,
> +	[MASTER_CAMNOC_BPS]        = &camnoc_master_bps,
> +	[MASTER_CAMNOC_IPE_0]      = &camnoc_master_ipe_0,
> +	[MASTER_CAMNOC_IPE_1]      = &camnoc_master_ipe_1,
> +	[MASTER_CAMNOC_JPEG]       = &camnoc_master_jpeg,
> +	[MASTER_CAMNOC_JPEG_DMA]   = &camnoc_master_jpeg_dma,
> +	[MASTER_CAMNOC_CDM]        = &camnoc_master_cdm,
> +	[MASTER_CAMNOC_FD]         = &camnoc_master_fd,
> +	[MASTER_CAMNOC_SBI]        = &camnoc_master_sbi,
> +	[MASTER_CAMNOC_ICP]        = &camnoc_master_icp,
> +	[SLAVE_CAMNOC_AXI]         = &camnoc_slave_axi,
> +};

This will fill up quickly given the number of entries. This should be 
moved to a separate file.

> +
> +#define CAMNOC_NUM_NODES	ARRAY_SIZE(camnoc_nodes)
> +
> +static int qcom_camnoc_get_bw(struct icc_node *node, u32 *avg, u32 *peak)
> +{
> +	*avg = 0;
> +	*peak = 0;
> +
> +	return 0;
> +}

What's going on here ? This sets everything to zero always ?

> +
> +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);
> +}
> +
> +static void qcom_camnoc_remove(struct platform_device *pdev)
> +{
> +	struct qcom_camnoc_icc_provider *cp = platform_get_drvdata(pdev);
> +
> +	icc_provider_deregister(&cp->provider);
> +	icc_nodes_remove(&cp->provider);
> +}
> +
> +static int qcom_camnoc_probe(struct platform_device *pdev)
> +{
> +	struct qcom_camnoc_icc_provider *cp;
> +	struct icc_onecell_data *data;
> +	struct icc_provider *provider;
> +	struct icc_node *node;
> +	unsigned int i;
> +	int ret;
> +
> +	cp = devm_kzalloc(&pdev->dev, sizeof(*cp), GFP_KERNEL);
> +	if (!cp)
> +		return -ENOMEM;
> +
> +	cp->clk = devm_clk_get(&pdev->dev, "camnoc_axi");
> +	if (IS_ERR(cp->clk))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(cp->clk),
> +				     "failed to get camnoc_axi clock\n");
> +
> +	data = devm_kzalloc(&pdev->dev,
> +			    struct_size(data, nodes, CAMNOC_NUM_NODES),
> +			    GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	data->num_nodes = CAMNOC_NUM_NODES;
> +
> +	provider = &cp->provider;
> +	provider->dev = &pdev->dev;
> +	provider->get_bw = qcom_camnoc_get_bw;
> +	provider->set = qcom_camnoc_set;
> +	provider->aggregate = icc_std_aggregate;
> +	provider->xlate = of_icc_xlate_onecell;
> +	provider->data = data;
> +
> +	icc_provider_init(provider);
> +
> +	for (i = 0; i < CAMNOC_NUM_NODES; i++) {
> +		node = icc_node_create_dyn();
> +		if (IS_ERR(node)) {
> +			ret = PTR_ERR(node);
> +			goto err_remove;
> +		}
> +
> +		ret = icc_node_set_name(node, provider, camnoc_nodes[i]->name);
> +		if (ret) {
> +			icc_node_destroy(node->id);
> +			goto err_remove;
> +		}
> +
> +		node->data = (void *)camnoc_nodes[i];
> +		icc_node_add(node, provider);
> +		data->nodes[i] = node;
> +	}
> +
> +	/* Link every master to the single CAMNOC AXI slave. */
> +	for (i = 0; i < CAMNOC_NUM_NODES - 1; i++) {
> +		if (camnoc_nodes[i] == &camnoc_slave_axi)
> +			continue;
> +		ret = icc_link_nodes(data->nodes[i],
> +				     &data->nodes[SLAVE_CAMNOC_AXI]);
> +		if (ret)
> +			goto err_remove;
> +	}
> +
> +	ret = icc_provider_register(provider);
> +	if (ret)
> +		goto err_remove;
> +
> +	platform_set_drvdata(pdev, cp);
> +
> +	return 0;
> +
> +err_remove:
> +	icc_nodes_remove(provider);
> +	return ret;
> +}
> +
> +static const struct of_device_id qcom_camnoc_of_match[] = {
> +	{ .compatible = "qcom,sm8250-cam-virt" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, qcom_camnoc_of_match);
> +
> +static struct platform_driver qcom_camnoc_driver = {
> +	.probe  = qcom_camnoc_probe,
> +	.remove = qcom_camnoc_remove,
> +	.driver = {
> +		.name           = "qcom-camnoc-icc",
> +		.of_match_table = qcom_camnoc_of_match,
> +		.sync_state     = icc_sync_state,
> +	},
> +};
> +module_platform_driver(qcom_camnoc_driver);
> +
> +MODULE_DESCRIPTION("Qualcomm CAMNOC interconnect driver");
> +MODULE_LICENSE("GPL");
> --
> 2.34.1
> 
> 


  parent reply	other threads:[~2026-09-29  8:36 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <w3u_oVZRxmZkAlC5RmxuB0v5k970jezsVHDZvryQ3oYkStqmsp79LjODiA0mYkNl1jkRya-7Ncvk8KGrE1vnUQ==@protonmail.internalid>
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-27  9:25     ` Krzysztof Kozlowski
2026-08-27  9:28       ` Krzysztof Kozlowski
2026-09-09 12:58       ` Gjorgji Rosikopulos (Consultant)
2026-09-21 14:17         ` Konrad Dybcio
2026-09-21 15:05           ` Atanas Filipov
2026-09-21 15:17           ` Gjorgji Rosikopulos (Consultant)
2026-09-29  8:36     ` Bryan O'Donoghue [this message]
2026-09-30  8:31       ` Gjorgji Rosikopulos (Consultant)
2026-08-19  9:32   ` [PATCH v2 3/3] arm64: dts: qcom: sm8250: Add CAMNOC ICC provider node Atanas Filipov
2026-09-29  8:39   ` [PATCH v2 0/3] Add Qualcomm CAMNOC ICC provider Bryan O'Donoghue
2026-09-29 11:43     ` Gjorgji Rosikopulos (Consultant)

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=ea7958f2-44bb-477e-b06a-44153489f396@kernel.org \
    --to=bod@kernel.org \
    --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=gjorgji.rosikopulos@oss.qualcomm.com \
    --cc=konradybcio@kernel.org \
    --cc=krzk+dt@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

all inboxes | Powered by JetHome®