[Skiboot] [PATCH 4/6] hw/npu2, platform: Add NPU2 platform device detection callback

[Andrew Donnellan]

On 21/08/18 16:48, Alistair Popple wrote:
>> Move the existing I2C-based presence detection for OpenCAPI devices on
>> Zaius/ZZ into common code, which we use by default for platforms which do
>> not define a callback. Clean up the use of the ibm,npu-link-type property,
>> which will now be exposed solely for debugging and not consumed internally.
> 
> <snip>
> 
>> +/*
>> + * A default presence detection implementation for platforms like ZZ and Zaius
>> + * that don't provide their own. Assumes all devices found will be OpenCAPI.
>> + */
>> +static void i2c_presence_detect(struct npu2 *npu)
>> +{
>> +	struct npu2_dev *dev;
>> +	for (int i = 0; i < npu->total_devices; i++) {
>> +		dev = &npu->devices[i];
>> +		if (platform.ocapi->force_presence ||
>> +		    _i2c_presence_detect(dev))
>> +			dev->type = NPU2_DEV_TYPE_OPENCAPI;
>> +		else
>> +			dev->type = NPU2_DEV_TYPE_UNKNOWN;
> 
> What are the chances of i2c detection working on unknown platforms? We should
> really only default to something if there is a reasonable expectation that it
> will work correctly. If not we should default to setting everything to
> NPU2_DEV_TYPE_UNKNOWN and have the ZZ and Zaius platforms set
> i2c_presence_detect() as their npu2_device_detect callback.

If it's an unknown platform it won't have platform.ocapi defined.

...aaaaand we don't null-check that in this path even though we 
dereference it. Will fix.

I see your point and maybe I'll export it in npu2.h so we can reference 
it as a platform callback.

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