Hi Reinette,
On 6/14/24 13:37, Reinette Chatre wrote:
> Hi Babu,
>
> On 6/5/24 3:45 PM, Babu Moger wrote:
>> In an effort to support MBM and MBA tests for AMD, renaming for variable
>> and functions to generic names. For Intel, the memory controller is called
>
> Changelog usually starts with some context and then problem to be solved.
> What
> the patch does follows that. Maybe just something like:
>
> For Intel, the memory controller is called Integrated Memory
> Controller (IMC). For AMD, it is called Unified Memory Controller (UMC).
> Change the names of variables and functions from imc to mc in preparation
> for support of MBM and MBA tests for AMD.
> No functional change.
Sure.
>
>> Integrated Memory Controllers (IMC). For AMD, it is called Unified
>> Memory Controller (UMC).
>>
>> Change the names of variables and functions from imc (Integrated memory
>> controller) to mc(Memory Controller). No functional change.
>>
>> Signed-off-by: Babu Moger <babu.moger@xxxxxxx>
>> ---
>
> ...
>
>> @@ -349,10 +350,10 @@ static void do_imc_mem_bw_test(void)
>> *
>> * Return: = 0 on success. < 0 on failure.
>> */
>> -static int get_mem_bw_imc(const char *bw_report, float *bw_imc)
>> +static int get_mem_bw_mc(const char *bw_report, float *bw_mc)
>
> The name of the function is expected to be changed in the function comments
> also.
Ok.
>
>> {
>> float reads, writes, of_mul_read, of_mul_write;
>> - int imc;
>> + int mc;
>> /* Start all iMC counters to log values (both read and write) */
>
> Was this intended to be MC?
Yes.
>
>> reads = 0, writes = 0, of_mul_read = 1, of_mul_write = 1;
>> @@ -361,21 +362,21 @@ static int get_mem_bw_imc(const char *bw_report,
>> float *bw_imc)
>> * Get results which are stored in struct type imc_counter_config
>> * Take overflow into consideration before calculating total
>> bandwidth.
>> */
>
> In below snippet imc_counter_config is renamed to mc_counter_config yet the
> comment above was not changed to match.
Will correct it.
>
>> - for (imc = 0; imc < imcs; imc++) {
>> - struct imc_counter_config *r =
>> - &imc_counters_config[imc][READ];
>> - struct imc_counter_config *w =
>> - &imc_counters_config[imc][WRITE];
>> + for (mc = 0; mc < mcs; mc++) {
>> + struct mc_counter_config *r =
>> + &mc_counters_config[mc][READ];
>> + struct mc_counter_config *w =
>> + &mc_counters_config[mc][WRITE];
>>
>
> I noticed a couple of misses by just looking at this patch. You can
> use grep to ensure that this patch does what you intend.
Sure.--
Thanks
Babu Moger
