[PATCH v3] powernv/sensor: Handle OPAL_WRONG_STATE error return

[PATCH v3] powernv/sensor: Handle OPAL_WRONG_STATE error return

[Vipin K Parashar]

OPAL returns OPAL_WRONG_STATE upon failing to provide
sensor data due to core sleeping/offline. Added check
for OPAL_WRONG_STATE rerurn code with sensor read failure.
Also added a log message indicating sensor data being
queried for sleeping/offline core.

Signed-off-by: Vipin K Parashar <vipin@linux.vnet.ibm.com>
---
Changes in v3:
 - Added a new case for OPAL_WRONG_STATE in sensor read
   along with a log message indicating sleeping/offline core
   causing read fail.

 arch/powerpc/platforms/powernv/opal-sensor.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/powerpc/platforms/powernv/opal-sensor.c b/arch/powerpc/platforms/powernv/opal-sensor.c
index 308efd1..fb6d6bb 100644
--- a/arch/powerpc/platforms/powernv/opal-sensor.c
+++ b/arch/powerpc/platforms/powernv/opal-sensor.c
@@ -64,6 +64,12 @@ int opal_get_sensor_data(u32 sensor_hndl, u32 *sensor_data)
 		*sensor_data = be32_to_cpu(data);
 		break;
 
+	case OPAL_WRONG_STATE:
+		pr_notice("%s: Sensor data read failure due to "
+				"core sleeping/offline\n", __func__);
+		ret = -EIO;
+		break;
+
 	default:
 		ret = opal_error_code(ret);
 		break;
-- 
2.7.4

Re: [PATCH v3] powernv/sensor: Handle OPAL_WRONG_STATE error return

[Vaibhav Jain]

Hi Vipin,

Minor spell fix in the patch description:

Vipin K Parashar <vipin@linux.vnet.ibm.com> writes:
> OPAL returns OPAL_WRONG_STATE upon failing to provide
> sensor data due to core sleeping/offline. Added check
> for OPAL_WRONG_STATE rerurn code with sensor read failure.
s/rerurn/return

> Also added a log message indicating sensor data being
> queried for sleeping/offline core.
s/for/for a/

-- 
Vaibhav Jain <vaibhav@linux.vnet.ibm.com>
Linux Technology Center, IBM India Pvt. Ltd.

Re: [PATCH v3] powernv/sensor: Handle OPAL_WRONG_STATE error return

[Michael Ellerman]

Vipin K Parashar <vipin@linux.vnet.ibm.com> writes:

> OPAL returns OPAL_WRONG_STATE upon failing to provide
> sensor data due to core sleeping/offline. Added check
> for OPAL_WRONG_STATE rerurn code with sensor read failure.
> Also added a log message indicating sensor data being
> queried for sleeping/offline core.
>
> Signed-off-by: Vipin K Parashar <vipin@linux.vnet.ibm.com>
> ---
> Changes in v3:
>  - Added a new case for OPAL_WRONG_STATE in sensor read
>    along with a log message indicating sleeping/offline core
>    causing read fail.
>
>  arch/powerpc/platforms/powernv/opal-sensor.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/arch/powerpc/platforms/powernv/opal-sensor.c b/arch/powerpc/platforms/powernv/opal-sensor.c
> index 308efd1..fb6d6bb 100644
> --- a/arch/powerpc/platforms/powernv/opal-sensor.c
> +++ b/arch/powerpc/platforms/powernv/opal-sensor.c
> @@ -64,6 +64,12 @@ int opal_get_sensor_data(u32 sensor_hndl, u32 *sensor_data)
>  		*sensor_data = be32_to_cpu(data);
>  		break;
>  
> +	case OPAL_WRONG_STATE:
> +		pr_notice("%s: Sensor data read failure due to "
> +				"core sleeping/offline\n", __func__);

I don't think it should print.

It's not the users fault, or anything they can prevent. It's a
mis-feature (aka. bug) in the driver that it queries sensors for offline
CPUs. At least it should be ratelimited.

I thought the entire motivation for the patch in the first place was
that we were spamming the console with messages?

cheers

Re: [PATCH v3] powernv/sensor: Handle OPAL_WRONG_STATE error return

[Vipin K Parashar]

On Monday 06 March 2017 03:03 PM, Michael Ellerman wrote:
> Vipin K Parashar <vipin@linux.vnet.ibm.com> writes:
>
>> OPAL returns OPAL_WRONG_STATE upon failing to provide
>> sensor data due to core sleeping/offline. Added check
>> for OPAL_WRONG_STATE rerurn code with sensor read failure.
>> Also added a log message indicating sensor data being
>> queried for sleeping/offline core.
>>
>> Signed-off-by: Vipin K Parashar <vipin@linux.vnet.ibm.com>
>> ---
>> Changes in v3:
>>   - Added a new case for OPAL_WRONG_STATE in sensor read
>>     along with a log message indicating sleeping/offline core
>>     causing read fail.
>>
>>   arch/powerpc/platforms/powernv/opal-sensor.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/arch/powerpc/platforms/powernv/opal-sensor.c b/arch/powerpc/platforms/powernv/opal-sensor.c
>> index 308efd1..fb6d6bb 100644
>> --- a/arch/powerpc/platforms/powernv/opal-sensor.c
>> +++ b/arch/powerpc/platforms/powernv/opal-sensor.c
>> @@ -64,6 +64,12 @@ int opal_get_sensor_data(u32 sensor_hndl, u32 *sensor_data)
>>   		*sensor_data = be32_to_cpu(data);
>>   		break;
>>   
>> +	case OPAL_WRONG_STATE:
>> +		pr_notice("%s: Sensor data read failure due to "
>> +				"core sleeping/offline\n", __func__);
> I don't think it should print.
>
> It's not the users fault, or anything they can prevent. It's a
> mis-feature (aka. bug) in the driver that it queries sensors for offline
> CPUs. At least it should be ratelimited.
>
> I thought the entire motivation for the patch in the first place was
> that we were spamming the console with messages?

Yes. Correct.
To avoid flooding console, logged message using pr_notice.
I think, it was suggested in previous reviews to log a message before
returning -EIO.
Shall i directly return -EIO without any log message ?

> cheers
>