[Skiboot] [PATCH 01/11] core: add a platform op to read sensors

[Stewart Smith]

Cedric Le Goater <clg at fr.ibm.com> writes:
> On 02/12/2015 08:42 AM, Stewart Smith wrote:
>> Cédric Le Goater <clg at fr.ibm.com> writes:
>>> +static int64_t opal_sensor_read(uint32_t sensor_hndl, int token,
>>> +		uint32_t *sensor_data)
>>> +{
>>> +	if (platform.sensor_read)
>>> +		return platform.sensor_read(sensor_hndl, token, sensor_data);
>>> +
>>> +	return OPAL_UNSUPPORTED;
>>> +}
>> 
>> It seems that previous behavior is to return OPAL_PARAMETER when
>> OPAL_SENSOR_READ is called with an unsupported sensor handle (at least
>> from my reading of parse_sensor_id in hw/fsp/fsp-sensor.c)
>
> yes but OPAL_UNSUPPORTED seemed to be an improvement in that case. The return
> value of opal_sensor_read() is only tested against OPAL_ASYNC_COMPLETION,
> so I don't think we are breaking anything.

Maybe split it into two patches, one explicitly replicating existing
behavior and the other making the (slight) modification? (along with
documentation and justifying why this doesn't break existing systems).

>> It would also be great if you document the OPAL_SENSOR_READ API while
>> you're at it too.
>
> OK. will do.
>
>> It seems that on non-fsp systems you would have gotten OPAL_PARAMETER
>> back as kernel should have been calling OPAL_CHECK_TOKEN first.
>
> This is not the case. Something to fix in the kernel. I will look into it.

Excellent. Look forward to seeing the patches!

(I've been trying to document if OPAL_CHECK_TOKEN should be used in
doc/opal-api )