lkml.org 
[lkml]   [2020]   [Nov]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    SubjectRe: [PATCH v7 2/6] interconnect: Add generic interconnect driver for Exynos SoCs
    From
    Date
    Hi Sylwester,

    On 10/30/20 9:51 PM, Sylwester Nawrocki wrote:
    > This patch adds a generic interconnect driver for Exynos SoCs in order
    > to provide interconnect functionality for each "samsung,exynos-bus"
    > compatible device.
    >
    > The SoC topology is a graph (or more specifically, a tree) and its
    > edges are specified using the 'samsung,interconnect-parent' in the

    samsung,interconnect-parent -> interconnects?


    > DT. Due to unspecified relative probing order, -EPROBE_DEFER may be
    > propagated to ensure that the parent is probed before its children.
    >
    > Each bus is now an interconnect provider and an interconnect node as
    > well (cf. Documentation/interconnect/interconnect.rst), i.e. every bus
    > registers itself as a node. Node IDs are not hardcoded but rather
    > assigned dynamically at runtime. This approach allows for using this
    > driver with various Exynos SoCs.
    >
    > Frequencies requested via the interconnect API for a given node are
    > propagated to devfreq using dev_pm_qos_update_request(). Please note
    > that it is not an error when CONFIG_INTERCONNECT is 'n', in which
    > case all interconnect API functions are no-op.
    >
    > The bus-width DT property is to determine the interconnect data
    > width and traslate requested bandwidth to clock frequency for each
    > bus.
    >
    > Signed-off-by: Artur Świgoń <a.swigon@samsung.com>
    > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
    > ---
    > Changes for v7:
    > - adjusted to the DT property changes: "interconnects" instead
    > of "samsung,interconnect-parent", "samsung,data-clk-ratio"
    > instead of "bus-width",
    > - adaptation to of_icc_get_from_provider() function changes
    > in v5.10-rc1.
    >
    > Changes for v6:
    > - corrected of_node dereferencing in exynos_icc_get_parent()
    > function,
    > - corrected initialization of icc_node->name so as to avoid
    > direct of_node->name dereferencing,
    > - added parsing of bus-width DT property.
    >
    > Changes for v5:
    > - adjust to renamed exynos,interconnect-parent-node property,
    > - use automatically generated platform device id as the interconect
    > node id instead of a now unavailable devfreq->id field,
    > - add icc_ prefix to some variables to make the code more self-commenting,
    > - use icc_nodes_remove() instead of icc_node_del() + icc_node_destroy(),
    > - adjust to exynos,interconnect-parent-node property rename to
    > samsung,interconnect-parent,
    > - converted to a separate platform driver in drivers/interconnect.
    >
    > ---
    > drivers/interconnect/Kconfig | 1 +
    > drivers/interconnect/Makefile | 1 +
    > drivers/interconnect/exynos/Kconfig | 6 ++
    > drivers/interconnect/exynos/Makefile | 4 +
    > drivers/interconnect/exynos/exynos.c | 198 +++++++++++++++++++++++++++++++++++
    > 5 files changed, 210 insertions(+)
    > create mode 100644 drivers/interconnect/exynos/Kconfig
    > create mode 100644 drivers/interconnect/exynos/Makefile
    > create mode 100644 drivers/interconnect/exynos/exynos.c
    >
    > diff --git a/drivers/interconnect/Kconfig b/drivers/interconnect/Kconfig
    > index 5b7204e..eca6eda 100644
    > --- a/drivers/interconnect/Kconfig
    > +++ b/drivers/interconnect/Kconfig
    > @@ -11,6 +11,7 @@ menuconfig INTERCONNECT
    >
    > if INTERCONNECT
    >
    > +source "drivers/interconnect/exynos/Kconfig"
    > source "drivers/interconnect/imx/Kconfig"
    > source "drivers/interconnect/qcom/Kconfig"
    >
    > diff --git a/drivers/interconnect/Makefile b/drivers/interconnect/Makefile
    > index d203520..665538d 100644
    > --- a/drivers/interconnect/Makefile
    > +++ b/drivers/interconnect/Makefile
    > @@ -4,5 +4,6 @@ CFLAGS_core.o := -I$(src)
    > icc-core-objs := core.o bulk.o
    >
    > obj-$(CONFIG_INTERCONNECT) += icc-core.o
    > +obj-$(CONFIG_INTERCONNECT_EXYNOS) += exynos/
    > obj-$(CONFIG_INTERCONNECT_IMX) += imx/
    > obj-$(CONFIG_INTERCONNECT_QCOM) += qcom/
    > diff --git a/drivers/interconnect/exynos/Kconfig b/drivers/interconnect/exynos/Kconfig
    > new file mode 100644
    > index 0000000..e51e52e
    > --- /dev/null
    > +++ b/drivers/interconnect/exynos/Kconfig
    > @@ -0,0 +1,6 @@
    > +# SPDX-License-Identifier: GPL-2.0-only
    > +config INTERCONNECT_EXYNOS
    > + tristate "Exynos generic interconnect driver"
    > + depends on ARCH_EXYNOS || COMPILE_TEST
    > + help
    > + Generic interconnect driver for Exynos SoCs.
    > diff --git a/drivers/interconnect/exynos/Makefile b/drivers/interconnect/exynos/Makefile
    > new file mode 100644
    > index 0000000..e19d1df
    > --- /dev/null
    > +++ b/drivers/interconnect/exynos/Makefile
    > @@ -0,0 +1,4 @@
    > +# SPDX-License-Identifier: GPL-2.0
    > +exynos-interconnect-objs := exynos.o
    > +
    > +obj-$(CONFIG_INTERCONNECT_EXYNOS) += exynos-interconnect.o
    > diff --git a/drivers/interconnect/exynos/exynos.c b/drivers/interconnect/exynos/exynos.c
    > new file mode 100644
    > index 0000000..772d1fc
    > --- /dev/null
    > +++ b/drivers/interconnect/exynos/exynos.c
    > @@ -0,0 +1,198 @@
    > +// SPDX-License-Identifier: GPL-2.0-only
    > +/*
    > + * Exynos generic interconnect provider driver
    > + *
    > + * Copyright (c) 2020 Samsung Electronics Co., Ltd.
    > + *
    > + * Authors: Artur Świgoń <a.swigon@samsung.com>
    > + * Sylwester Nawrocki <s.nawrocki@samsung.com>
    > + */
    > +#include <linux/device.h>
    > +#include <linux/interconnect-provider.h>
    > +#include <linux/module.h>
    > +#include <linux/of.h>
    > +#include <linux/platform_device.h>
    > +#include <linux/pm_qos.h>
    > +#include <linux/slab.h>
    > +
    > +#define EXYNOS_ICC_DEFAULT_BUS_CLK_RATIO 8
    > +
    > +struct exynos_icc_priv {
    > + struct device *dev;
    > +
    > + /* One interconnect node per provider */
    > + struct icc_provider provider;
    > + struct icc_node *node;
    > +
    > + struct dev_pm_qos_request qos_req;
    > + u32 bus_clk_ratio;
    > +};
    > +
    > +static struct icc_node *exynos_icc_get_parent(struct device_node *np)
    > +{
    > + struct of_phandle_args args;
    > + struct icc_node_data *icc_node_data;
    > + struct icc_node *icc_node;
    > + int num, ret;
    > +
    > + num = of_count_phandle_with_args(np, "interconnects",
    > + "#interconnect-cells");
    > + if (num < 1)
    > + return NULL; /* parent nodes are optional */
    > +
    > + /* Get the interconnect target node */
    > + ret = of_parse_phandle_with_args(np, "interconnects",
    > + "#interconnect-cells", 0, &args);
    > + if (ret < 0)
    > + return ERR_PTR(ret);
    > +
    > + icc_node_data = of_icc_get_from_provider(&args);
    > + of_node_put(args.np);
    > +
    > + if (IS_ERR(icc_node_data))
    > + return ERR_CAST(icc_node_data);
    > +
    > + icc_node = icc_node_data->node;
    > + kfree(icc_node_data);
    > +
    > + return icc_node;
    > +}

    I have a question about exynos_icc_get_parent().
    As I checked, this function returns the only one icc_node
    as parent node. But, bus_display dt node in the exynos4412.dtsi
    specifies the two interconnect node as following with bus_leftbus, bus_dmc,

    When I checked the return value of exynos_icc_get_parent()
    during probing for bus_display device, exynos_icc_get_parent() function
    only returns 'bus_leftbus' icc_node. Do you need to add two phandle
    of icc node?

    diff --git a/arch/arm/boot/dts/exynos4412.dtsi b/arch/arm/boot/dts/exynos4412.dtsi
    index d07739ec8740..9e4045ceb6ab 100644
    --- a/arch/arm/boot/dts/exynos4412.dtsi
    +++ b/arch/arm/boot/dts/exynos4412.dtsi
    @@ -472,7 +472,7 @@
    clocks = <&clock CLK_ACLK160>;
    clock-names = "bus";
    operating-points-v2 = <&bus_display_opp_table>;
    interconnects = <&bus_leftbus &bus_dmc>;
    #interconnect-cells = <0>;
    status = "disabled";
    };

    > +
    > +static int exynos_generic_icc_set(struct icc_node *src, struct icc_node *dst)
    > +{
    > + struct exynos_icc_priv *src_priv = src->data, *dst_priv = dst->data;
    > + s32 src_freq = max(src->avg_bw, src->peak_bw) / src_priv->bus_clk_ratio;
    > + s32 dst_freq = max(dst->avg_bw, dst->peak_bw) / dst_priv->bus_clk_ratio;
    > + int ret;
    > +
    > + ret = dev_pm_qos_update_request(&src_priv->qos_req, src_freq);
    > + if (ret < 0) {
    > + dev_err(src_priv->dev, "failed to update PM QoS of %s (src)\n",
    > + src->name);
    > + return ret;
    > + }
    > +
    > + ret = dev_pm_qos_update_request(&dst_priv->qos_req, dst_freq);
    > + if (ret < 0) {
    > + dev_err(dst_priv->dev, "failed to update PM QoS of %s (dst)\n",
    > + dst->name);
    > + return ret;
    > + }
    > +
    > + return 0;
    > +}
    > +
    > +static struct icc_node *exynos_generic_icc_xlate(struct of_phandle_args *spec,
    > + void *data)
    > +{
    > + struct exynos_icc_priv *priv = data;
    > +
    > + if (spec->np != priv->dev->parent->of_node)
    > + return ERR_PTR(-EINVAL);
    > +
    > + return priv->node;
    > +}
    > +
    > +static int exynos_generic_icc_remove(struct platform_device *pdev)
    > +{
    > + struct exynos_icc_priv *priv = platform_get_drvdata(pdev);
    > + struct icc_node *parent_node, *node = priv->node;
    > +
    > + parent_node = exynos_icc_get_parent(priv->dev->parent->of_node);
    > + if (parent_node && !IS_ERR(parent_node))
    > + icc_link_destroy(node, parent_node);
    > +
    > + icc_nodes_remove(&priv->provider);
    > + icc_provider_del(&priv->provider);
    > +
    > + return 0;
    > +}
    > +
    > +static int exynos_generic_icc_probe(struct platform_device *pdev)
    > +{
    > + struct device *bus_dev = pdev->dev.parent;
    > + struct exynos_icc_priv *priv;
    > + struct icc_provider *provider;
    > + struct icc_node *icc_node, *icc_parent_node;
    > + int ret;
    > +
    > + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
    > + if (!priv)
    > + return -ENOMEM;
    > +
    > + priv->dev = &pdev->dev;
    > + platform_set_drvdata(pdev, priv);
    > +
    > + provider = &priv->provider;
    > +
    > + provider->set = exynos_generic_icc_set;
    > + provider->aggregate = icc_std_aggregate;
    > + provider->xlate = exynos_generic_icc_xlate;
    > + provider->dev = bus_dev;
    > + provider->inter_set = true;
    > + provider->data = priv;
    > +
    > + ret = icc_provider_add(provider);
    > + if (ret < 0)
    > + return ret;
    > +
    > + icc_node = icc_node_create(pdev->id);
    > + if (IS_ERR(icc_node)) {
    > + ret = PTR_ERR(icc_node);
    > + goto err_prov_del;
    > + }
    > +
    > + priv->node = icc_node;
    > + icc_node->name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%pOFn",
    > + bus_dev->of_node);
    > + if (of_property_read_u32(bus_dev->of_node, "samsung,data-clock-ratio",
    > + &priv->bus_clk_ratio))
    > + priv->bus_clk_ratio = EXYNOS_ICC_DEFAULT_BUS_CLK_RATIO;
    > +
    > + /*
    > + * Register a PM QoS request for the parent (devfreq) device.
    > + */
    > + ret = dev_pm_qos_add_request(bus_dev, &priv->qos_req,
    > + DEV_PM_QOS_MIN_FREQUENCY, 0);
    > + if (ret < 0)
    > + goto err_node_del;
    > +
    > + icc_node->data = priv;
    > + icc_node_add(icc_node, provider);
    > +
    > + icc_parent_node = exynos_icc_get_parent(bus_dev->of_node);
    > + if (IS_ERR(icc_parent_node)) {
    > + ret = PTR_ERR(icc_parent_node);
    > + goto err_pmqos_del;
    > + }
    > + if (icc_parent_node) {
    > + ret = icc_link_create(icc_node, icc_parent_node->id);
    > + if (ret < 0)
    > + goto err_pmqos_del;
    > + }
    > +
    > + return 0;
    > +
    > +err_pmqos_del:
    > + dev_pm_qos_remove_request(&priv->qos_req);
    > +err_node_del:
    > + icc_nodes_remove(provider);
    > +err_prov_del:
    > + icc_provider_del(provider);
    > + return ret;
    > +}
    > +
    > +static struct platform_driver exynos_generic_icc_driver = {
    > + .driver = {
    > + .name = "exynos-generic-icc",
    > + },
    > + .probe = exynos_generic_icc_probe,
    > + .remove = exynos_generic_icc_remove,
    > +};
    > +module_platform_driver(exynos_generic_icc_driver);
    > +
    > +MODULE_DESCRIPTION("Exynos generic interconnect driver");
    > +MODULE_AUTHOR("Artur Świgoń <a.swigon@samsung.com>");
    > +MODULE_AUTHOR("Sylwester Nawrocki <s.nawrocki@samsung.com>");
    > +MODULE_LICENSE("GPL v2");
    > +MODULE_ALIAS("platform:exynos-generic-icc");
    >


    --
    Best Regards,
    Chanwoo Choi
    Samsung Electronics

    \
     
     \ /
      Last update: 2020-11-03 10:24    [W:3.591 / U:1.096 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site