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@xxxxxxxxxx> > --- > 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