Re: [PATCH v7 2/6] interconnect: Add generic interconnect driver for Exynos SoCs

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");