On 9/25/2023 8:39 AM, Ilpo Järvinen wrote:
> On Fri, 22 Sep 2023, Jithu Joseph wrote:
>
...
>>
>> - activate.rsvd = 0;
>> activate.delay = IFS_THREAD_WAIT;
>> activate.sigmce = 0;
>> - activate.start = 0;
>> - activate.stop = ifsd->valid_chunks - 1;
>> + to_start = 0;
>> + to_stop = ifsd->valid_chunks - 1;
>> +
>> + if (ifsd->generation) {
>> + activate.gen2.start = to_start;
>> + activate.gen2.stop = to_stop;
>> + } else {
>> + activate.gen0.start = to_start;
>> + activate.gen0.stop = to_stop;
>> + }
>
> Is it okay to not do activate.gen0.rsvd = 0 anymore? If you know it is, it
> would be nice to record that fact into the changelog so that it can be
> found in the history.
I did test on a gen0 to check if there is a problem due to this (and it seemed fine).
I will make a note in changelog as you suggest
>
>>
>> timeout = jiffies + HZ / 2;
>> retries = MAX_IFS_RETRIES;
>>
>> - while (activate.start <= activate.stop) {
>> + while (to_start <= to_stop) {
>> if (time_after(jiffies, timeout)) {
>> status.error_code = IFS_SW_TIMEOUT;
>> break;
>> @@ -196,13 +205,14 @@ static void ifs_test_core(int cpu, struct device *dev)
>>
>> status.data = msrvals[1];
>>
>> - trace_ifs_status(cpu, activate, status);
>> + trace_ifs_status(cpu, to_start, to_stop, status.data);
>>
>> /* Some cases can be retried, give up for others */
>> if (!can_restart(status))
>> break;
>>
>> - if (status.chunk_num == activate.start) {
>> + status_chunk = ifsd->generation ? status.gen2.chunk_num : status.gen0.chunk_num;
>> + if (status_chunk == to_start) {
>> /* Check for forward progress */
>> if (--retries == 0) {
>> if (status.error_code == IFS_NO_ERROR)
>> @@ -211,7 +221,9 @@ static void ifs_test_core(int cpu, struct device *dev)
>> }
>> } else {
>> retries = MAX_IFS_RETRIES;
>> - activate.start = status.chunk_num;
>> + ifsd->generation ? (activate.gen2.start = status_chunk) :
>> + (activate.gen0.start = status_chunk);
>
> The alignment of the second line is still not correct but now I notice how
> the left-hand side is hidden within those expressions. Just do a normal if
> instead so that it is simpler to understand, please.
In v1 the second line was kept 1 space to the right of previous line. In v2 I kept
them at the same indent, since your original comment was Misaliged.
I will change these two lines to
if (ifsd->generation)
activate.gen2.start = status_chunk;
else
activate.gen0.start = status_chunk
Jithu
