On 03.11.2020 10:37, Chanwoo Choi wrote:
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?
Yes, I will rephrase the whole commit message as it's a bit outdated now.
I've changed the sentence to: "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."
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
+++ b/drivers/interconnect/exynos/exynos.c
+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?
Yes, as we use the interconnect consumer bindings we need to specify a path, i.e. a <initiator, target> pair. When the provider node initializes it will link itself to that path. Currently the provider driver uses just the first phandle.
+++ 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"; };