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

On 11/29/21 5:06 PM, Nathan Lynch wrote:

Tyrel Datwyler [off-list ref] writes:

quoted

On 11/29/21 12:58 AM, Mahesh Salgaonkar wrote:

quoted

-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.