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

[Tyrel Datwyler <tyreld@linux.ibm.com>]

On 11/29/21 5:06 PM, Nathan Lynch wrote:
> 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"
> 

Fair enough. Seems I misremembered the nature of the original problem and should
have looked back at the commit to completely jog my memory.