On 12.12.2019 09:29, Georgi Djakov wrote:

Hi Leonard,

Thank you for your continuous work on the patches and sorry for not reviewing
this earlier.
On 11/14/19 22:09, Leonard Crestez wrote:

quoted

This adds support for i.MX SoC family to interconnect framework.

Platform drivers can describe the interconnect graph and several
adjustment knobs where icc node bandwidth is converted to a
DEV_PM_QOS_MIN_FREQUENCY request.

The interconnect provider is probed through the main NOC device and
other adjustable nodes on the same graph are found from a
fsl,scalable-nodes phandle array property.

Signed-off-by: Alexandre Bailon <redacted>
Signed-off-by: Leonard Crestez <redacted>

quoted

+static int imx_icc_node_init_qos(struct icc_provider *provider,
+				 struct icc_node *node)
+{
+	struct imx_icc_node *node_data = node->data;
+	struct device *dev = provider->dev;
+	struct device_node *dn = NULL;
+	struct platform_device *pdev;
+	int i, count;
+	u32 node_id;
+	int ret;
+
+	count = of_property_count_u32_elems(dev->of_node,
+					    "fsl,scalable-node-ids");
+	if (count < 0) {
+		dev_err(dev, "Failed to parse fsl,scalable-node-ids: %d\n",
+			count);
+		return count;
+	}
+
+	for (i = 0; i < count; i++) {
+		ret = of_property_read_u32_index(dev->of_node,
+						 "fsl,scalable-node-ids",
+						 i, &node_id);
+
+		if (ret < 0) {
+			dev_err(dev, "Failed to parse fsl,scalable-node-ids[%d]: %d\n",
+				i, ret);
+			return ret;
+		}
+		if (node_id != node->id)
+			continue;
+
+		dn = of_parse_phandle(dev->of_node, "fsl,scalable-nodes", i);

Why is this needed? I would expect that the interconnect provider driver already
knows which nodes are scalable based on the platform compatible string.
Then maybe this driver should create devfreq devices for each node that is scalable?

The scalable nodes are independent devfreq instances which are probed 
through their own DT compat strings. It's even possible to reload 
imx8m-ddrc (the driver scaling the dram controller) at runtime.

The most common solution to fetch other devices on DT systems is via 
phandles and fsl,scalable-nodes is a phandle array. Since the provider 
is platform-specific and knows the topology of the soc it could even use 
of_find_node_by_path but that seems very messy. It's also quite brittle, 
I've seen several bugs caused by DT node renaming.

This support for arbitrary "scalable nodes" might be excessively generic 
and strict DT compatibility might be difficult to maintain if too much 
is exposed. Changing per-soc driver data is otherwise easy.

In vendor tree we only ever scale the main NOC and DDRC anyway so 
equivalent functionality could be achieved with a single "fsl,ddrc" 
phandle property on the noc.

Support for scaling peripheral buses could be implemented by adding 
additional properties like "fsl,display-nic". Such a feature would need 
careful measurement on real hardware anyway.

--
Regards,
Leonard

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel