Re: [PATCH] interconnect: Do not skip aggregation for disabled paths

[adhudase]

On 2020-07-21 17:37, Georgi Djakov wrote:
> When an interconnect path is being disabled, currently we don't 
> aggregate
> the requests for it afterwards. But the re-aggregation step shouldn't 
> be
> skipped, as it may leave the nodes with outdated bandwidth data. This
> outdated data may actually keep the path still enabled and prevent the
> device from going into lower power states.
>
> Reported-by: Atul Dhudase <adhudase@xxxxxxxxxxxxxx>
> Fixes: 7d374b209083 ("interconnect: Add helpers for enabling/disabling 
> a path")
> Signed-off-by: Georgi Djakov <georgi.djakov@xxxxxxxxxx>
> ---

Tested-by: Atul Dhudase <adhudase@xxxxxxxxxxxxxx>
Reviewed-by: Atul Dhudase <adhudase@xxxxxxxxxxxxxx>

>  drivers/interconnect/core.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
> index 37d5ec970cc1..5174dcb31ab7 100644
> --- a/drivers/interconnect/core.c
> +++ b/drivers/interconnect/core.c
> @@ -243,6 +243,7 @@ static int aggregate_requests(struct icc_node 
> *node)
>  {
>  	struct icc_provider *p = node->provider;
>  	struct icc_req *r;
> +	u32 avg_bw, peak_bw;
>
>  	node->avg_bw = 0;
>  	node->peak_bw = 0;
> @@ -251,9 +252,14 @@ static int aggregate_requests(struct icc_node 
> *node)
>  		p->pre_aggregate(node);
>
>  	hlist_for_each_entry(r, &node->req_list, req_node) {
> -		if (!r->enabled)
> -			continue;
> -		p->aggregate(node, r->tag, r->avg_bw, r->peak_bw,
> +		if (r->enabled) {
> +			avg_bw = r->avg_bw;
> +			peak_bw = r->peak_bw;
> +		} else {
> +			avg_bw = 0;
> +			peak_bw = 0;
> +		}
> +		p->aggregate(node, r->tag, avg_bw, peak_bw,
>  			     &node->avg_bw, &node->peak_bw);
>  	}