[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8b9f06ae-f711-2e71-1e36-1f8519b52e8d@linaro.org>
Date: Thu, 22 Oct 2020 16:04:09 +0300
From: Georgi Djakov <georgi.djakov@...aro.org>
To: Jun Nie <jun.nie@...aro.org>
Cc: devicetree@...r.kernel.org, bjorn.andersson@...aro.org,
agross@...nel.org, linux-pm@...r.kernel.org,
linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org,
robh@...nel.org, shawn.guo@...aro.org
Subject: Re: [PATCH 1/5] interconnect: qcom: Consolidate interconnect RPM
support
Hi Jun,
Thanks for working on this.
On 9/30/20 11:16, Jun Nie wrote:
> Add RPM based interconnect driver implements the set and aggregate
> functionalities that translates bandwidth requests into RPM messages.
> These modules provide a common set of functionalities for all
> Qualcomm RPM based interconnect providers and should help reduce code
> duplication when adding new providers.
>
> Signed-off-by: Jun Nie <jun.nie@...aro.org>
> ---
> drivers/interconnect/qcom/Makefile | 3 +-
> drivers/interconnect/qcom/icc-rpm.c | 194 ++++++++++++++++++++++
> drivers/interconnect/qcom/icc-rpm.h | 73 +++++++++
> drivers/interconnect/qcom/msm8916.c | 241 ++--------------------------
> 4 files changed, 279 insertions(+), 232 deletions(-)
> create mode 100644 drivers/interconnect/qcom/icc-rpm.c
> create mode 100644 drivers/interconnect/qcom/icc-rpm.h
>
> diff --git a/drivers/interconnect/qcom/Makefile b/drivers/interconnect/qcom/Makefile
> index 1702ece67dc5..f5e803489de0 100644
> --- a/drivers/interconnect/qcom/Makefile
> +++ b/drivers/interconnect/qcom/Makefile
> @@ -9,7 +9,7 @@ icc-rpmh-obj := icc-rpmh.o
> qnoc-sc7180-objs := sc7180.o
> qnoc-sdm845-objs := sdm845.o
> qnoc-sm8150-objs := sm8150.o
> -icc-smd-rpm-objs := smd-rpm.o
> +icc-smd-rpm-objs := smd-rpm.o icc-rpm.o
>
> obj-$(CONFIG_INTERCONNECT_QCOM_BCM_VOTER) += icc-bcm-voter.o
> obj-$(CONFIG_INTERCONNECT_QCOM_MSM8916) += qnoc-msm8916.o
> @@ -21,3 +21,4 @@ obj-$(CONFIG_INTERCONNECT_QCOM_SC7180) += qnoc-sc7180.o
> obj-$(CONFIG_INTERCONNECT_QCOM_SDM845) += qnoc-sdm845.o
> obj-$(CONFIG_INTERCONNECT_QCOM_SM8150) += qnoc-sm8150.o
> obj-$(CONFIG_INTERCONNECT_QCOM_SMD_RPM) += icc-smd-rpm.o
> +obj-$(CONFIG_INTERCONNECT_QCOM_SMD_RPM) += icc-smd-rpm.o
Duplicate line.
> diff --git a/drivers/interconnect/qcom/icc-rpm.c b/drivers/interconnect/qcom/icc-rpm.c
> new file mode 100644
> index 000000000000..4ddcb6cd4b84
> --- /dev/null
> +++ b/drivers/interconnect/qcom/icc-rpm.c
[..]
> +int qnoc_probe(struct platform_device *pdev, size_t cd_size, int cd_num,
> + const struct clk_bulk_data *cd)
> +{
> + struct device *dev = &pdev->dev;
> + const struct qcom_icc_desc *desc;
> + struct icc_onecell_data *data;
> + struct icc_provider *provider;
> + struct qcom_icc_node **qnodes;
> + struct qcom_icc_provider *qp;
> + struct icc_node *node;
> + size_t num_nodes, i;
> + int ret;
> +
> + /* wait for the RPM proxy */
> + if (!qcom_icc_rpm_smd_available())
> + return -EPROBE_DEFER;
> +
> + desc = of_device_get_match_data(dev);
> + if (!desc)
> + return -EINVAL;
> +
> + qnodes = desc->nodes;
> + num_nodes = desc->num_nodes;
> +
> + qp = devm_kzalloc(dev, sizeof(*qp), GFP_KERNEL);
> + if (!qp)
> + return -ENOMEM;
> +
> + data = devm_kzalloc(dev, struct_size(data, nodes, num_nodes),
> + GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + qp->bus_clks = devm_kmemdup(dev, cd, cd_size,
> + GFP_KERNEL);
> + if (!qp->bus_clks)
> + return -ENOMEM;
> +
> + qp->num_clks = cd_num;
> + ret = devm_clk_bulk_get(dev, qp->num_clks, qp->bus_clks);
> + if (ret)
> + return ret;
> +
> + ret = clk_bulk_prepare_enable(qp->num_clks, qp->bus_clks);
> + if (ret)
> + return ret;
> +
> + provider = &qp->provider;
> + INIT_LIST_HEAD(&provider->nodes);
> + provider->dev = dev;
> + provider->set = qcom_icc_set;
> + provider->aggregate = icc_std_aggregate;
> + provider->xlate = of_icc_xlate_onecell;
> + provider->data = data;
> +
> + ret = icc_provider_add(provider);
> + if (ret) {
> + dev_err(dev, "error adding interconnect provider: %d\n", ret);
> + clk_bulk_disable_unprepare(qp->num_clks, qp->bus_clks);
> + return ret;
> + }
> +
> + for (i = 0; i < num_nodes; i++) {
> + size_t j;
> +
> + node = icc_node_create(qnodes[i]->id);
> + if (IS_ERR(node)) {
> + ret = PTR_ERR(node);
> + goto err;
> + }
> +
> + node->name = qnodes[i]->name;
> + node->data = qnodes[i];
> + icc_node_add(node, provider);
> +
> + dev_dbg(dev, "registered node %s\n", node->name);
Please drop this line.
> +
> + /* populate links */
Please drop this comment.
> + for (j = 0; j < qnodes[i]->num_links; j++)
> + icc_link_create(node, qnodes[i]->links[j]);
> +
> + data->nodes[i] = node;
> + }
> + data->num_nodes = num_nodes;
> +
> + platform_set_drvdata(pdev, qp);
> +
> + return 0;
> +err:
> + icc_nodes_remove(provider);
> + clk_bulk_disable_unprepare(qp->num_clks, qp->bus_clks);
> + icc_provider_del(provider);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(qnoc_probe);
> +
> +int qnoc_remove(struct platform_device *pdev)
> +{
> + struct qcom_icc_provider *qp = platform_get_drvdata(pdev);
> +
> + icc_nodes_remove(&qp->provider);
> + clk_bulk_disable_unprepare(qp->num_clks, qp->bus_clks);
> + return icc_provider_del(&qp->provider);
> +}
> +EXPORT_SYMBOL(qnoc_remove);
> diff --git a/drivers/interconnect/qcom/icc-rpm.h b/drivers/interconnect/qcom/icc-rpm.h
> new file mode 100644
> index 000000000000..b75bb59d79b7
> --- /dev/null
> +++ b/drivers/interconnect/qcom/icc-rpm.h
> @@ -0,0 +1,73 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2020 Linaro Ltd
> + */
> +
> +#ifndef __DRIVERS_INTERCONNECT_QCOM_SMD_RPM_COMMON_H
> +#define __DRIVERS_INTERCONNECT_QCOM_SMD_RPM_COMMON_H
Would be nice if this matches with path and filename:
__DRIVERS_INTERCONNECT_QCOM_ICC_RPM_H
All the rest looks good.
Thanks,
Georgi
Powered by blists - more mailing lists