Re: [PATCH v1] memory: tegra20: Add debug statistics

[Dmitry Osipenko <digetx@gmail.com>]

18.03.2021 19:20, Krzysztof Kozlowski пишет:
> On 18/03/2021 17:18, Dmitry Osipenko wrote:
>> 18.03.2021 18:23, Krzysztof Kozlowski пишет:
>>
>>>> +
>>>> +	/* collect memory controller utilization percent for each client */
>>>> +	for (i = 0; i < mc->soc->num_clients; i += 2) {
>>>> +		client0 = &mc->soc->clients[i];
>>>> +		client1 = &mc->soc->clients[i + 1];
>>>> +
>>>> +		if (i + 1 == mc->soc->num_clients)
>>>> +			client1 = NULL;
>>>> +
>>>> +		tegra20_mc_stat_events(mc, client0, client1,
>>>> +				       MC_STAT_CONTROL_FILTER_PRI_DISABLE,
>>>> +				       MC_STAT_CONTROL_PRI_EVENT_HP,
>>>> +				       MC_STAT_CONTROL_EVENT_QUALIFIED,
>>>> +				       &stats[i + 0].events,
>>>> +				       &stats[i + 1].events);
>>>> +	}
>>>> +
>>>> +	/* collect more info from active clients */
>>>> +	for (i = 0; i < mc->soc->num_clients; i++) {
>>>> +		clientb = mc->soc->num_clients;
>>>> +
>>>> +		for (client0 = NULL; i < mc->soc->num_clients; i++) {
>>>> +			if (stats[i].events) {
>>>> +				client0 = &mc->soc->clients[i];
>>>> +				clienta = i++;
>>>> +				break;
>>>> +			}
>>>> +		}
>>>
>>> Could all clients have 0 events ending with "clienta" being undefined?
>>> "client0" would be non-NULL because of loop before.
>>
>> As per 6.8.5.3 of C99 standard, the declaration of a for-loop "is
>> reached in the order of execution before the first evaluation of the
>> controlling expression".
>>
>> http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf
>>
>>>> +
>>>> +		for (client1 = NULL; i < mc->soc->num_clients; i++) {
>>>> +			if (stats[i].events) {
>>>> +				client1 = &mc->soc->clients[i];
>>>> +				clientb = i;
>>>> +				break;
>>>> +			}
>>>> +		}
>>>> +
>>>> +		if (!client0 && !client1)
>>>> +			break;
>>
>> this means that both client0 and client1 are set t0 NULL here if all
>> clients have 0 events.
>>
> 
> Yes, you're right. I missed the assignment client0=NULL in the for().

Good, thank you for the review! I'll prepare v2.