[Skiboot] [PATCH 06/13] hw/npu2: Rework npu2_add_interrupt_map() and skip non-NVLink devices

[Alexey Kardashevskiy]

On 14/12/2018 16:30, Andrew Donnellan wrote:
> On 14/12/18 4:24 pm, Alexey Kardashevskiy wrote:
>>>           map[i + 4] = icsp; /* interrupt-parent */
>>> -        map[i + 5] = p->base_lsi + (index * 2) + 1; /* NDL No-Stall
>>> Event */
>>> +        map[i + 5] = p->base_lsi + (dev->brick_index * 2) + 1; /*
>>> NDL No-Stall Event */
>>
>>
>> The comment in hw/npu2-common.c says "May be overridden by platform
>> presence detection" about dev->brick_index initialization. And
>> brick_index is initialized from link_index which is (as I figured out
>> recently) not exactly the index into npu2::devices[]. So probably
>> switching from npu2::devices[]'s index to dev->brick_index is the right
>> move but please make it a separate patch for bisectability.
> 
> If you really want it in a separate patch I can do that but it's a very
> minor change, I will at least document it in the commit


I'd like to know if this was tested (but you say it was not)  or  see it
clear in the code and the commit log that what you are doing is correct.

I can only see that brick_index==index most of the times but not always
and nothing really puts any light on when/why it is !=, explaining this
in the commit log of this patch would be useful. I'd even suggest making
it a comment in the chunk above.


-- 
Alexey