2019-07-10 07:27:23

by Joel Stanley

[permalink] [raw]
Subject: [PATCH] hwmon (occ): Add temp sensor value check

From: Alexander Soldatov <[email protected]>

The occ driver supports two formats for the temp sensor value.

The OCC firmware for P8 supports only the first format, for which
no range checking or error processing is performed in the driver.
Inspecting the OCC sources for P8 reveals that OCC may send
a special value 0xFFFF to indicate that a sensor read timeout
has occurred, see

https://github.com/open-power/occ/blob/master_p8/src/occ/cmdh/cmdh_fsp_cmds.c#L395

That situation wasn't handled in the driver. This patch adds invalid
temp value check for the sensor data format 1 and handles it the same
way as it is done for the format 2, where EREMOTEIO is reported for
this case.

Fixes: 54076cb3b5ff ("hwmon (occ): Add sensor attributes and register hwmon device")
Signed-off-by: Alexander Soldatov <[email protected]>
Signed-off-by: Alexander Amelkin <[email protected]>
Reviewed-by: Alexander Amelkin <[email protected]>
Reviewed-by: Eddie James <[email protected]>
Signed-off-by: Joel Stanley <[email protected]>
---
drivers/hwmon/occ/common.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/drivers/hwmon/occ/common.c b/drivers/hwmon/occ/common.c
index cccf91742c1a..a7d2b16dd702 100644
--- a/drivers/hwmon/occ/common.c
+++ b/drivers/hwmon/occ/common.c
@@ -241,6 +241,12 @@ static ssize_t occ_show_temp_1(struct device *dev,
val = get_unaligned_be16(&temp->sensor_id);
break;
case 1:
+ /*
+ * If a sensor reading has expired and couldn't be refreshed,
+ * OCC returns 0xFFFF for that sensor.
+ */
+ if (temp->value == 0xFFFF)
+ return -EREMOTEIO;
val = get_unaligned_be16(&temp->value) * 1000;
break;
default:
--
2.20.1


2019-07-10 09:06:36

by Alexander Amelkin

[permalink] [raw]
Subject: Re: [PATCH] hwmon (occ): Add temp sensor value check

Thanks, Joel!

JFYI, Alexander Soldatov has left the YADRO team some time ago, so his e-mail @yadro.com isn't valid anymore.

Should anyone have any questions regarding this patch, feel free to email me.

With best regards,
Alexander Amelkin,
Leading BMC Software Engineer, YADRO
https://yadro.com

10.07.2019 10:26, Joel Stanley wrote:
> From: Alexander Soldatov <[email protected]>
>
> The occ driver supports two formats for the temp sensor value.
>
> The OCC firmware for P8 supports only the first format, for which
> no range checking or error processing is performed in the driver.
> Inspecting the OCC sources for P8 reveals that OCC may send
> a special value 0xFFFF to indicate that a sensor read timeout
> has occurred, see
>
> https://github.com/open-power/occ/blob/master_p8/src/occ/cmdh/cmdh_fsp_cmds.c#L395
>
> That situation wasn't handled in the driver. This patch adds invalid
> temp value check for the sensor data format 1 and handles it the same
> way as it is done for the format 2, where EREMOTEIO is reported for
> this case.
>
> Fixes: 54076cb3b5ff ("hwmon (occ): Add sensor attributes and register hwmon device")
> Signed-off-by: Alexander Soldatov <[email protected]>
> Signed-off-by: Alexander Amelkin <[email protected]>
> Reviewed-by: Alexander Amelkin <[email protected]>
> Reviewed-by: Eddie James <[email protected]>
> Signed-off-by: Joel Stanley <[email protected]>
> ---
> drivers/hwmon/occ/common.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/hwmon/occ/common.c b/drivers/hwmon/occ/common.c
> index cccf91742c1a..a7d2b16dd702 100644
> --- a/drivers/hwmon/occ/common.c
> +++ b/drivers/hwmon/occ/common.c
> @@ -241,6 +241,12 @@ static ssize_t occ_show_temp_1(struct device *dev,
> val = get_unaligned_be16(&temp->sensor_id);
> break;
> case 1:
> + /*
> + * If a sensor reading has expired and couldn't be refreshed,
> + * OCC returns 0xFFFF for that sensor.
> + */
> + if (temp->value == 0xFFFF)
> + return -EREMOTEIO;
> val = get_unaligned_be16(&temp->value) * 1000;
> break;
> default:


Attachments:
signature.asc (836.00 B)
OpenPGP digital signature

2019-07-10 20:58:24

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH] hwmon (occ): Add temp sensor value check

On Wed, Jul 10, 2019 at 04:56:06PM +0930, Joel Stanley wrote:
> From: Alexander Soldatov <[email protected]>
>
> The occ driver supports two formats for the temp sensor value.
>
> The OCC firmware for P8 supports only the first format, for which
> no range checking or error processing is performed in the driver.
> Inspecting the OCC sources for P8 reveals that OCC may send
> a special value 0xFFFF to indicate that a sensor read timeout
> has occurred, see
>
> https://github.com/open-power/occ/blob/master_p8/src/occ/cmdh/cmdh_fsp_cmds.c#L395
>
> That situation wasn't handled in the driver. This patch adds invalid
> temp value check for the sensor data format 1 and handles it the same
> way as it is done for the format 2, where EREMOTEIO is reported for
> this case.
>
> Fixes: 54076cb3b5ff ("hwmon (occ): Add sensor attributes and register hwmon device")
> Signed-off-by: Alexander Soldatov <[email protected]>
> Signed-off-by: Alexander Amelkin <[email protected]>
> Reviewed-by: Alexander Amelkin <[email protected]>
> Reviewed-by: Eddie James <[email protected]>
> Signed-off-by: Joel Stanley <[email protected]>

Applied.

Thanks,
Guenter

> ---
> drivers/hwmon/occ/common.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/hwmon/occ/common.c b/drivers/hwmon/occ/common.c
> index cccf91742c1a..a7d2b16dd702 100644
> --- a/drivers/hwmon/occ/common.c
> +++ b/drivers/hwmon/occ/common.c
> @@ -241,6 +241,12 @@ static ssize_t occ_show_temp_1(struct device *dev,
> val = get_unaligned_be16(&temp->sensor_id);
> break;
> case 1:
> + /*
> + * If a sensor reading has expired and couldn't be refreshed,
> + * OCC returns 0xFFFF for that sensor.
> + */
> + if (temp->value == 0xFFFF)
> + return -EREMOTEIO;
> val = get_unaligned_be16(&temp->value) * 1000;
> break;
> default: