[Skiboot] [PATCH v2 8/8] Add Swift platform
Alexey Kardashevskiy
aik at ozlabs.ru
Mon Jul 15 15:20:34 AEST 2019
On 11/07/2019 02:33, Reza Arbab wrote:
> On Wed, Jul 10, 2019 at 12:26:59PM +1000, Alexey Kardashevskiy wrote:
>> On 09/07/2019 07:07, Reza Arbab wrote:
>>> Signed-off-by: Reza Arbab <arbab at linux.ibm.com>
>>
>> No commit log at all?
>>
>> What is that "swift"? How many/which model/DD cpus, npus, gpus, any
>> mention of any spec, anything here or in the swift.c header comment
>> will do.
>
> My bad. I'll add a description of the system.
>
> In my defense I'll just offer that the first commit of witherspoon.c has
> nothing but an ASCII art portrait of Reese Witherspoon. :)
but you did not do even that!!! ;)
>
>>> + dt_add_property_u64(dev->dn, "ibm,link-speed", 25000000000ull);
>>> + dt_add_property_cells(dev->dn, "nvidia,link-speed", 9);
>>
>> I have always been curious - what are these "25000000000ull" and "9"
>> and where do they come from and can we have them defined? Thanks,
>
> There are references elsewhere, like hdata/spira.c:
>
> /* ibm,link-speed is in bps and nvidia,link-speed is ~magic~ */
>
> and doc/device-tree/nvlink.rst:
>
> ; Denotes the speed the link is running at:
> ; 0x3 == 20 Gbps, 0x8 = 25.78125 Gbps, 0x9 == 25.00000 Gbps
>
> I'll add some #defines to make this explicit and use one of them instead
> of open coding 9.
oh great, please do. I came across systems with 8 and 9 and it took me
some time to realize that these numbers do matter.
>
>>> +#define DN(g) devs[g]->nvlink.gpu->dn
>>
>> This one does not seem very useful, can be open coded.
>
> Yeah, it only gets used once. I'll drop it.
>
>>> + add_peers_prop(0, G(3), G(3),
>>> + G(2), G(2), G(2),
>>> + G(1), G(1), G(1),
>>> + N(0), N(0), N(0), N(0));
>>
>> 2, 3, 3, 4 items per line and the next one 1, 3, 3, 1, 4 - what is the
>> point in such grouping? There must be something about the code
>> readability but I just do not see it :)
>
> I thought it might help to put consecutive links going to the same place
> on the same line. No worries, I'll just use two lines instead, one line
> for links to other GPUs, one line for links to the NPU.
I do not mind either way, I was just wondering if it is something like
"these are grouped into yetanotherbricks" or some other new and weird
things. Thanks,
>> Also, this looks like some GPUs only have 2 links between them and
>> some have 3, is that correct?
>
> That is correct.
>
--
Alexey
More information about the Skiboot
mailing list