Tyrel Datwyler <tyr...@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"
