[Skiboot] [PATCH v2 8/8] Add Swift platform

[Reza Arbab]

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. :)

>>+		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.

>>+#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.

>Also, this looks like some GPUs only have 2 links between them and some 
>have 3, is that correct?

That is correct.

-- 
Reza Arbab