Re: [PATCH v8 2/7] interconnect: Add generic interconnect driver for Exynos SoCs

From: Krzysztof Kozlowski
Date: Wed Nov 04 2020 - 07:37:36 EST


On Wed, Nov 04, 2020 at 11:36:52AM +0100, 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 described by specifying in the 'interconnects' property
> the interconnect consumer path for each interconnect provider DT node.
>
> 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 hard coded 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 samsung,data-clk-ratio DT property is used to specify the ratio
> of the interconect bandwidth to the minimum data clock frequency
> for each bus.
>
> Due to unspecified relative probing order, -EPROBE_DEFER may be
> propagated to ensure that the parent is probed before its children.
>
> Signed-off-by: Artur Świgoń <a.swigon@xxxxxxxxxxx>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@xxxxxxxxxxx>
> ---
> Changes for v8:
> - renamed drivers/interconnect/exynos to drivers/interconnect/samsung,
> - added missing driver sync_state callback assignment.
>
> 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/samsung/Kconfig | 13 +++
> drivers/interconnect/samsung/Makefile | 4 +
> drivers/interconnect/samsung/exynos.c | 199 ++++++++++++++++++++++++++++++++++
> 5 files changed, 218 insertions(+)
> create mode 100644 drivers/interconnect/samsung/Kconfig
> create mode 100644 drivers/interconnect/samsung/Makefile
> create mode 100644 drivers/interconnect/samsung/exynos.c
>
> diff --git a/drivers/interconnect/Kconfig b/drivers/interconnect/Kconfig
> index 5b7204e..d637a89 100644
> --- a/drivers/interconnect/Kconfig
> +++ b/drivers/interconnect/Kconfig
> @@ -13,5 +13,6 @@ if INTERCONNECT
>
> source "drivers/interconnect/imx/Kconfig"
> source "drivers/interconnect/qcom/Kconfig"
> +source "drivers/interconnect/samsung/Kconfig"
>
> endif
> diff --git a/drivers/interconnect/Makefile b/drivers/interconnect/Makefile
> index d203520..c2f9e9d 100644
> --- a/drivers/interconnect/Makefile
> +++ b/drivers/interconnect/Makefile
> @@ -6,3 +6,4 @@ icc-core-objs := core.o bulk.o
> obj-$(CONFIG_INTERCONNECT) += icc-core.o
> obj-$(CONFIG_INTERCONNECT_IMX) += imx/
> obj-$(CONFIG_INTERCONNECT_QCOM) += qcom/
> +obj-$(CONFIG_INTERCONNECT_SAMSUNG) += samsung/
> \ No newline at end of file

This needs a fix.

> diff --git a/drivers/interconnect/samsung/Kconfig b/drivers/interconnect/samsung/Kconfig
> new file mode 100644
> index 0000000..508ed64
> --- /dev/null
> +++ b/drivers/interconnect/samsung/Kconfig
> @@ -0,0 +1,13 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +config INTERCONNECT_SAMSUNG
> + bool "Samsung interconnect drivers"

"Samsung SoC interconnect drivers"

> + depends on ARCH_EXYNOS || COMPILE_TEST

Don't the depend on INTERCONNECT?

> + help
> + Interconnect drivers for Samsung SoCs.
> +
> +

One line break

> +config INTERCONNECT_EXYNOS
> + tristate "Exynos generic interconnect driver"
> + depends on INTERCONNECT_SAMSUNG

How about:
default y if ARCH_EXYNOS

> + help
> + Generic interconnect driver for Exynos SoCs.
> diff --git a/drivers/interconnect/samsung/Makefile b/drivers/interconnect/samsung/Makefile
> new file mode 100644
> index 0000000..e19d1df
> --- /dev/null
> +++ b/drivers/interconnect/samsung/Makefile
> @@ -0,0 +1,4 @@
> +# SPDX-License-Identifier: GPL-2.0
> +exynos-interconnect-objs := exynos.o

What is this line for?

Best regards,
Krzysztof


> +
> +obj-$(CONFIG_INTERCONNECT_EXYNOS) += exynos-interconnect.o
> diff --git a/drivers/interconnect/samsung/exynos.c b/drivers/interconnect/samsung/exynos.c
> new file mode 100644
> index 0000000..6559d8c