Re: [PATCH] powerpc/rtas: Introduce rtas_get_sensor_nonblocking() for pci hotplug driver.

[Nathan Lynch <nathanl@linux.ibm.com>]

Tyrel Datwyler <tyreld@linux.ibm.com> writes:
> On 11/29/21 12:58 AM, Mahesh Salgaonkar wrote:
>> -int rtas_get_sensor_fast(int sensor, int index, int *state)
>> +static int
>> +__rtas_get_sensor(int sensor, int index, int *state, bool warn_on)
>>  {
>>  	int token = rtas_token("get-sensor-state");
>>  	int rc;
>> @@ -618,14 +619,26 @@ int rtas_get_sensor_fast(int sensor, int index, int *state)
>>  		return -ENOENT;
>> 
>>  	rc = rtas_call(token, 2, 2, state, sensor, index);
>> -	WARN_ON(rc == RTAS_BUSY || (rc >= RTAS_EXTENDED_DELAY_MIN &&
>> -				    rc <= RTAS_EXTENDED_DELAY_MAX));
>> +	WARN_ON(warn_on &&
>> +		(rc == RTAS_BUSY || (rc >= RTAS_EXTENDED_DELAY_MIN &&
>> +				    rc <= RTAS_EXTENDED_DELAY_MAX)));
>
> The whole point of rtas_get_sensor_fast() is that on busy we will just let it
> error out because we don't want to wait. I'm not sure I see the point of the
> spurious WARN_ONs anytime we hit a BUSY or DELAY return code. Maybe converting
> that to a pr_debug() might be better and save expanding the API with a _fast and
> _nonblocking variant that do the same thing minus one surpressing a
> WARN_ON splat.

There is a subset of sensors that are specified to not ever return busy
or delay statuses. rtas_get_sensor_fast() is meant to be used with
those, and it would be an error to use it on a sensor not in that set.
So the WARN_ON() is appropriate IMO; if it triggers it indicates either
a misuse of the API or a firmware bug. See commit 1c2cb594441d
"powerpc/rtas: Introduce rtas_get_sensor_fast() for IRQ handlers"