[Skiboot] [PATCH v2 6/6] platforms/astbmc/witherspoon: Implement OpenCAPI support

[Andrew Donnellan]

On 30/08/18 02:55, Frederic Barrat wrote:
>> +    if (gpu1_present) {
>> +        if (state & (1 << 1)) {
>> +            prlog(PR_DEBUG, "PLAT: Chip %d GPU#1 is OpenCAPI\n",
>> +                  chip->id);
>> +            dev = find_link_index(npu, 4);
>> +            if (dev)
>> +                dev->type = NPU2_DEV_TYPE_OPENCAPI;
>> +            dev = find_link_index(npu, 5);
>> +            if (dev)
>> +                /* dev->type = NPU2_DEV_TYPE_OPENCAPI; */
>> +                /* We current don't support using the second link */
>> +                dev->type = NPU2_DEV_TYPE_UNKNOWN;
>> +        } else {
>> +            prlog(PR_DEBUG, "PLAT: Chip %d GPU#1 is NVLink\n",
>> +                  chip->id);
>> +            dev = find_link_index(npu, 3);
>> +            if (dev)
>> +                dev->type = NPU2_DEV_TYPE_NVLINK;
>> +            dev = find_link_index(npu, 4);
>> +            if (dev)
>> +                dev->type = NPU2_DEV_TYPE_NVLINK;
>> +            dev = find_link_index(npu, 5);
>> +            if (dev)
>> +                dev->type = NPU2_DEV_TYPE_NVLINK;
>> +        }
>> +    }
> 
> Do we need some sort of set_link_type() helper function?

Hmm, that might make things look a beat neater... let me think about it.

> 
> 
>> +    return;
>> +
>> +i2c_failed:
>> +    prlog(PR_ERR, "PLAT: NPU device type detection failed, rc=%d\n", 
>> rc);
>> +    return;
>> +}
>> +
>> +const struct platform_ocapi witherspoon_ocapi = {
>> +       .i2c_engine          = 1,
>> +       .i2c_port            = 4,
>> +       .odl_phy_swap        = false,
>> +       .i2c_reset_addr      = 0x20,
>> +       .i2c_reset_brick2    = 1 << 0,
>> +       .i2c_reset_brick3    = 1 << 0,
>> +       .i2c_reset_brick4    = 1 << 1,
>> +       .i2c_reset_brick5    = 1 << 1,
> 
> A comment explaining why it's not as wrong as it looks at first could be 
> useful :-) It's in the commit message, but will be lost to whoever reads 
> this.

ACK.

> 
> Overall the full series look pretty good to me!

Thanks!


Andrew

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan at au1.ibm.com  IBM Australia Limited