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....@linaro.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

Reply via email to