Re: [PATCH v4 1/7] interconnect: Add generic on-chip interconnect API

From: Georgi Djakov
Date: Wed Jun 06 2018 - 14:09:37 EST


Hi Evan,

On 06/06/2018 05:59 PM, Georgi Djakov wrote:
>>> +
>>> +/**
>>> + * icc_node_create() - create a node
>>> + * @id: node id
>>> + *
>>> + * Return: icc_node pointer on success, or ERR_PTR() on error
>>> + */
>>> +struct icc_node *icc_node_create(int id)
>>> +{
>>> + struct icc_node *node;
>>> +
>>> + /* check if node already exists */
>>> + node = node_find(id);
>>> + if (node)
>>> + return node;
>>
>> This is probably going to do more harm than good once icc_node_delete comes
>> in, since it almost certainly indicates a programmer error or ID collision,
>> and will likely result in a double free. We should probably fail with
>> EEXIST instead.
>
> In the current approach we create the nodes one by one, and the linked
> nodes are created when they are referenced. The other way around would
> be to create first all the nodes and then populate the links to avoid
> the "chicken and egg" problem.
>

Just to elaborate a bit more on that: We can't actually register all the
nodes in advance, as we might have multiple interconnect providers
probing in different order. Each provider may have nodes linking to
nodes belonging to other providers (not probed yet). That's why we
create the nodes on the first reference and then, when the actual
provider driver is probed, the rest of the node data is filled.

Thanks,
Georgi