Re: [PATCH V6 1/4] interconnect: qcom: Add multidev EPSS L3 support

[Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>]

On 25.11.2024 6:45 PM, Raviteja Laggyshetty wrote:
> EPSS on SA8775P has two instances which requires creation of two device
> nodes with different compatible and device data because of unique
> icc node id and name limitation in interconnect framework.
> Add multidevice support to osm-l3 code to get unique node id from IDA
> and node name is made unique by appending node address.
> 
> Signed-off-by: Raviteja Laggyshetty <quic_rlaggysh@quicinc.com>
> ---

[...]

> +	ret = of_property_read_reg(pdev->dev.of_node, 0, &addr, NULL);
> +	if (ret)
> +		return ret;
> +
>  	qp->base = devm_platform_ioremap_resource(pdev, 0);
>  	if (IS_ERR(qp->base))
>  		return PTR_ERR(qp->base);
> @@ -242,8 +262,13 @@ static int qcom_osm_l3_probe(struct platform_device *pdev)
>  
>  	icc_provider_init(provider);
>  
> +	/* Allocate unique id for qnodes */
> +	for (i = 0; i < num_nodes; i++)
> +		qnodes[i]->id = ida_alloc_min(&osm_l3_id, OSM_L3_NODE_ID_START, GFP_KERNEL);

As I've said in my previous emails, this is a framework-level problem.

Up until now we've simply silently ignored the possibility of an
interconnect provider having more than one instance, as conveniently
most previous SoCs had a bunch of distinct bus masters.

Currently, debugfs-client.c relies on the node names being unique.
Keeping them as such is also useful for having a sane sysfs/debugfs
interface. But it's not always feasible, and a hierarchical approach
(like in pmdomain) may be a better fit.

Then, node->id is used for creating links, and we unfortunately cannot
assume that both src and dst are within the same provider.
I'm not a fan of these IDs being hardcoded, but there are some drivers
that rely on that, which itself is also a bit unfortunate..


If Mike (who introduced debugfs-client and is probably the main user)
doesn't object to a small ABI break (which is "fine" with a debugfs
driver that requires editing the source code to be compiled), we could
add a property within icc_provider like `bool dynamic_ids` and have an
ICC-global IDA that would take care of any conflicts.

Provider drivers whose consumers don't already rely on programmatical
use of hardcoded IDs *and* don't have cross-provider links could then
enable that flag and have the node IDs and names set like you did in
this patch. This also sounds very useful for icc-clk.

Konrad