1. Patchew
  2. linux
  3. [PATCH V8 0/7] Add EPSS L3 provider support on SA8775P SoC
  4. View patch

[PATCH V8 2/7] interconnect: core: Add dynamic id allocation support

Raviteja Laggyshetty posted 7 patches 1 year ago
There is a newer version of this series
[PATCH V8 2/7] interconnect: core: Add dynamic id allocation support
Posted by Raviteja Laggyshetty 1 year ago 
The current interconnect framework relies on static IDs for node
creation and registration, which limits topologies with multiple
instances of the same interconnect provider. To address this, update
the interconnect framework APIs icc_node_create() and icc_link_create()
APIs to dynamically allocate IDs for interconnect nodes during creation.
This change removes the dependency on static IDs, allowing multiple
instances of the same hardware, such as EPSS L3.

Signed-off-by: Raviteja Laggyshetty <quic_rlaggysh@quicinc.com>
---
 drivers/interconnect/core.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
index 9d5404a07e8a..40700246f1b6 100644
--- a/drivers/interconnect/core.c
+++ b/drivers/interconnect/core.c
@@ -20,6 +20,8 @@
 
 #include "internal.h"
 
+#define ICC_DYN_ID_START 10000
+
 #define CREATE_TRACE_POINTS
 #include "trace.h"
 
@@ -826,7 +828,12 @@ static struct icc_node *icc_node_create_nolock(int id)
 	if (!node)
 		return ERR_PTR(-ENOMEM);
 
-	id = idr_alloc(&icc_idr, node, id, id + 1, GFP_KERNEL);
+	/* negative id indicates dynamic id allocation */
+	if (id < 0)
+		id = idr_alloc(&icc_idr, node, ICC_DYN_ID_START, 0, GFP_KERNEL);
+	else
+		id = idr_alloc(&icc_idr, node, id, id + 1, GFP_KERNEL);
+
 	if (id < 0) {
 		WARN(1, "%s: couldn't get idr\n", __func__);
 		kfree(node);
@@ -962,6 +969,10 @@ void icc_node_add(struct icc_node *node, struct icc_provider *provider)
 	node->avg_bw = node->init_avg;
 	node->peak_bw = node->init_peak;
 
+	if (node->id >= ICC_DYN_ID_START)
+		node->name = devm_kasprintf(provider->dev, GFP_KERNEL, "%s@%s",
+					    node->name, dev_name(provider->dev));
+
 	if (node->avg_bw || node->peak_bw) {
 		if (provider->pre_aggregate)
 			provider->pre_aggregate(node);
-- 
2.39.2
Re: [PATCH V8 2/7] interconnect: core: Add dynamic id allocation support
Posted by Dmitry Baryshkov 12 months ago 
On Wed, Feb 05, 2025 at 06:27:38PM +0000, Raviteja Laggyshetty wrote:
> The current interconnect framework relies on static IDs for node
> creation and registration, which limits topologies with multiple
> instances of the same interconnect provider. To address this, update
> the interconnect framework APIs icc_node_create() and icc_link_create()
> APIs to dynamically allocate IDs for interconnect nodes during creation.
> This change removes the dependency on static IDs, allowing multiple
> instances of the same hardware, such as EPSS L3.
> 
> Signed-off-by: Raviteja Laggyshetty <quic_rlaggysh@quicinc.com>
> ---
>  drivers/interconnect/core.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
> index 9d5404a07e8a..40700246f1b6 100644
> --- a/drivers/interconnect/core.c
> +++ b/drivers/interconnect/core.c
> @@ -20,6 +20,8 @@
>  
>  #include "internal.h"
>  
> +#define ICC_DYN_ID_START 10000
> +
>  #define CREATE_TRACE_POINTS
>  #include "trace.h"
>  
> @@ -826,7 +828,12 @@ static struct icc_node *icc_node_create_nolock(int id)
>  	if (!node)
>  		return ERR_PTR(-ENOMEM);
>  
> -	id = idr_alloc(&icc_idr, node, id, id + 1, GFP_KERNEL);
> +	/* negative id indicates dynamic id allocation */
> +	if (id < 0)

Nit: I think it might be better to add an explicit define for that and
to decline all other negatdive values. Please leave us some room for
future expansion.

> +		id = idr_alloc(&icc_idr, node, ICC_DYN_ID_START, 0, GFP_KERNEL);
> +	else
> +		id = idr_alloc(&icc_idr, node, id, id + 1, GFP_KERNEL);
> +
>  	if (id < 0) {
>  		WARN(1, "%s: couldn't get idr\n", __func__);
>  		kfree(node);
> @@ -962,6 +969,10 @@ void icc_node_add(struct icc_node *node, struct icc_provider *provider)
>  	node->avg_bw = node->init_avg;
>  	node->peak_bw = node->init_peak;
>  
> +	if (node->id >= ICC_DYN_ID_START)
> +		node->name = devm_kasprintf(provider->dev, GFP_KERNEL, "%s@%s",
> +					    node->name, dev_name(provider->dev));
> +
>  	if (node->avg_bw || node->peak_bw) {
>  		if (provider->pre_aggregate)
>  			provider->pre_aggregate(node);
> -- 
> 2.39.2
> 

-- 
With best wishes
Dmitry
Re: [PATCH V8 2/7] interconnect: core: Add dynamic id allocation support
Posted by Raviteja Laggyshetty 11 months, 4 weeks ago 

On 2/10/2025 4:20 PM, Dmitry Baryshkov wrote:
> On Wed, Feb 05, 2025 at 06:27:38PM +0000, Raviteja Laggyshetty wrote:
>> The current interconnect framework relies on static IDs for node
>> creation and registration, which limits topologies with multiple
>> instances of the same interconnect provider. To address this, update
>> the interconnect framework APIs icc_node_create() and icc_link_create()
>> APIs to dynamically allocate IDs for interconnect nodes during creation.
>> This change removes the dependency on static IDs, allowing multiple
>> instances of the same hardware, such as EPSS L3.
>>
>> Signed-off-by: Raviteja Laggyshetty <quic_rlaggysh@quicinc.com>
>> ---
>>  drivers/interconnect/core.c | 13 ++++++++++++-
>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
>> index 9d5404a07e8a..40700246f1b6 100644
>> --- a/drivers/interconnect/core.c
>> +++ b/drivers/interconnect/core.c
>> @@ -20,6 +20,8 @@
>>  
>>  #include "internal.h"
>>  
>> +#define ICC_DYN_ID_START 10000
>> +
>>  #define CREATE_TRACE_POINTS
>>  #include "trace.h"
>>  
>> @@ -826,7 +828,12 @@ static struct icc_node *icc_node_create_nolock(int id)
>>  	if (!node)
>>  		return ERR_PTR(-ENOMEM);
>>  
>> -	id = idr_alloc(&icc_idr, node, id, id + 1, GFP_KERNEL);
>> +	/* negative id indicates dynamic id allocation */
>> +	if (id < 0)
> 
> Nit: I think it might be better to add an explicit define for that and
> to decline all other negatdive values. Please leave us some room for
> future expansion.
> 
Do you mean to replace the value of ALLOC_DYN_ID from -1 to some
positive value like 100000 and to use it as initial ID for the nodes
requiring the dynamic allocation ? This explicit define can be used as
check for dynamic allocation and also as argument to idr_alloc min value
argument. Is my interpretation of the comment correct ?

>> +		id = idr_alloc(&icc_idr, node, ICC_DYN_ID_START, 0, GFP_KERNEL);
>> +	else
>> +		id = idr_alloc(&icc_idr, node, id, id + 1, GFP_KERNEL);
>> +
>>  	if (id < 0) {
>>  		WARN(1, "%s: couldn't get idr\n", __func__);
>>  		kfree(node);
>> @@ -962,6 +969,10 @@ void icc_node_add(struct icc_node *node, struct icc_provider *provider)
>>  	node->avg_bw = node->init_avg;
>>  	node->peak_bw = node->init_peak;
>>  
>> +	if (node->id >= ICC_DYN_ID_START)
>> +		node->name = devm_kasprintf(provider->dev, GFP_KERNEL, "%s@%s",
>> +					    node->name, dev_name(provider->dev));
>> +
>>  	if (node->avg_bw || node->peak_bw) {
>>  		if (provider->pre_aggregate)
>>  			provider->pre_aggregate(node);
>> -- 
>> 2.39.2
>>
>
Re: [PATCH V8 2/7] interconnect: core: Add dynamic id allocation support
Posted by Dmitry Baryshkov 11 months, 4 weeks ago 
On Sun, Feb 16, 2025 at 10:08:51PM +0530, Raviteja Laggyshetty wrote:
> 
> 
> On 2/10/2025 4:20 PM, Dmitry Baryshkov wrote:
> > On Wed, Feb 05, 2025 at 06:27:38PM +0000, Raviteja Laggyshetty wrote:
> >> The current interconnect framework relies on static IDs for node
> >> creation and registration, which limits topologies with multiple
> >> instances of the same interconnect provider. To address this, update
> >> the interconnect framework APIs icc_node_create() and icc_link_create()
> >> APIs to dynamically allocate IDs for interconnect nodes during creation.
> >> This change removes the dependency on static IDs, allowing multiple
> >> instances of the same hardware, such as EPSS L3.
> >>
> >> Signed-off-by: Raviteja Laggyshetty <quic_rlaggysh@quicinc.com>
> >> ---
> >>  drivers/interconnect/core.c | 13 ++++++++++++-
> >>  1 file changed, 12 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
> >> index 9d5404a07e8a..40700246f1b6 100644
> >> --- a/drivers/interconnect/core.c
> >> +++ b/drivers/interconnect/core.c
> >> @@ -20,6 +20,8 @@
> >>  
> >>  #include "internal.h"
> >>  
> >> +#define ICC_DYN_ID_START 10000
> >> +
> >>  #define CREATE_TRACE_POINTS
> >>  #include "trace.h"
> >>  
> >> @@ -826,7 +828,12 @@ static struct icc_node *icc_node_create_nolock(int id)
> >>  	if (!node)
> >>  		return ERR_PTR(-ENOMEM);
> >>  
> >> -	id = idr_alloc(&icc_idr, node, id, id + 1, GFP_KERNEL);
> >> +	/* negative id indicates dynamic id allocation */
> >> +	if (id < 0)
> > 
> > Nit: I think it might be better to add an explicit define for that and
> > to decline all other negatdive values. Please leave us some room for
> > future expansion.
> > 
> Do you mean to replace the value of ALLOC_DYN_ID from -1 to some
> positive value like 100000 and to use it as initial ID for the nodes
> requiring the dynamic allocation ? This explicit define can be used as
> check for dynamic allocation and also as argument to idr_alloc min value
> argument. Is my interpretation of the comment correct ?

No, it is not. I asked to add an explicit define for -1 in the ICC
framework and make icc_node_create_nolock() reject all other negative
values.

> 
> >> +		id = idr_alloc(&icc_idr, node, ICC_DYN_ID_START, 0, GFP_KERNEL);
> >> +	else
> >> +		id = idr_alloc(&icc_idr, node, id, id + 1, GFP_KERNEL);
> >> +
> >>  	if (id < 0) {
> >>  		WARN(1, "%s: couldn't get idr\n", __func__);
> >>  		kfree(node);
> >> @@ -962,6 +969,10 @@ void icc_node_add(struct icc_node *node, struct icc_provider *provider)
> >>  	node->avg_bw = node->init_avg;
> >>  	node->peak_bw = node->init_peak;
> >>  
> >> +	if (node->id >= ICC_DYN_ID_START)
> >> +		node->name = devm_kasprintf(provider->dev, GFP_KERNEL, "%s@%s",
> >> +					    node->name, dev_name(provider->dev));
> >> +
> >>  	if (node->avg_bw || node->peak_bw) {
> >>  		if (provider->pre_aggregate)
> >>  			provider->pre_aggregate(node);
> >> -- 
> >> 2.39.2
> >>
> > 
> 

-- 
With best wishes
Dmitry
Re: [PATCH V8 2/7] interconnect: core: Add dynamic id allocation support
Posted by Raviteja Laggyshetty 11 months, 3 weeks ago 

On 2/17/2025 6:32 AM, Dmitry Baryshkov wrote:
> On Sun, Feb 16, 2025 at 10:08:51PM +0530, Raviteja Laggyshetty wrote:
>>
>>
>> On 2/10/2025 4:20 PM, Dmitry Baryshkov wrote:
>>> On Wed, Feb 05, 2025 at 06:27:38PM +0000, Raviteja Laggyshetty wrote:
>>>> The current interconnect framework relies on static IDs for node
>>>> creation and registration, which limits topologies with multiple
>>>> instances of the same interconnect provider. To address this, update
>>>> the interconnect framework APIs icc_node_create() and icc_link_create()
>>>> APIs to dynamically allocate IDs for interconnect nodes during creation.
>>>> This change removes the dependency on static IDs, allowing multiple
>>>> instances of the same hardware, such as EPSS L3.
>>>>
>>>> Signed-off-by: Raviteja Laggyshetty <quic_rlaggysh@quicinc.com>
>>>> ---
>>>>  drivers/interconnect/core.c | 13 ++++++++++++-
>>>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
>>>> index 9d5404a07e8a..40700246f1b6 100644
>>>> --- a/drivers/interconnect/core.c
>>>> +++ b/drivers/interconnect/core.c
>>>> @@ -20,6 +20,8 @@
>>>>  
>>>>  #include "internal.h"
>>>>  
>>>> +#define ICC_DYN_ID_START 10000
>>>> +
>>>>  #define CREATE_TRACE_POINTS
>>>>  #include "trace.h"
>>>>  
>>>> @@ -826,7 +828,12 @@ static struct icc_node *icc_node_create_nolock(int id)
>>>>  	if (!node)
>>>>  		return ERR_PTR(-ENOMEM);
>>>>  
>>>> -	id = idr_alloc(&icc_idr, node, id, id + 1, GFP_KERNEL);
>>>> +	/* negative id indicates dynamic id allocation */
>>>> +	if (id < 0)
>>>
>>> Nit: I think it might be better to add an explicit define for that and
>>> to decline all other negatdive values. Please leave us some room for
>>> future expansion.
>>>
>> Do you mean to replace the value of ALLOC_DYN_ID from -1 to some
>> positive value like 100000 and to use it as initial ID for the nodes
>> requiring the dynamic allocation ? This explicit define can be used as
>> check for dynamic allocation and also as argument to idr_alloc min value
>> argument. Is my interpretation of the comment correct ?
> 
> No, it is not. I asked to add an explicit define for -1 in the ICC
> framework and make icc_node_create_nolock() reject all other negative
> values.

Understood, will make the change as suggested.
> 
>>
>>>> +		id = idr_alloc(&icc_idr, node, ICC_DYN_ID_START, 0, GFP_KERNEL);
>>>> +	else
>>>> +		id = idr_alloc(&icc_idr, node, id, id + 1, GFP_KERNEL);
>>>> +
>>>>  	if (id < 0) {
>>>>  		WARN(1, "%s: couldn't get idr\n", __func__);
>>>>  		kfree(node);
>>>> @@ -962,6 +969,10 @@ void icc_node_add(struct icc_node *node, struct icc_provider *provider)
>>>>  	node->avg_bw = node->init_avg;
>>>>  	node->peak_bw = node->init_peak;
>>>>  
>>>> +	if (node->id >= ICC_DYN_ID_START)
>>>> +		node->name = devm_kasprintf(provider->dev, GFP_KERNEL, "%s@%s",
>>>> +					    node->name, dev_name(provider->dev));
>>>> +
>>>>  	if (node->avg_bw || node->peak_bw) {
>>>>  		if (provider->pre_aggregate)
>>>>  			provider->pre_aggregate(node);
>>>> -- 
>>>> 2.39.2
>>>>
>>>
>>
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.