diff mbox series

[v2,1/1] hwmon (occ): Add temp sensor value check

Message ID 1555524209-26997-1-git-send-email-a.amelkin@yadro.com
State Accepted, archived
Headers show
Series [v2,1/1] hwmon (occ): Add temp sensor value check | expand

Commit Message

Alexander Amelkin April 17, 2019, 6:03 p.m. UTC
From: Alexander Soldatov <a.soldatov@yadro.com>

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.

Signed-off-by: Alexander Soldatov <a.soldatov@yadro.com>
Signed-off-by: Alexander Amelkin <a.amelkin@yadro.com>
Reviewed-by: Alexander Amelkin <a.amelkin@yadro.com>
Cc: Edward A. James <eajames@us.ibm.com>
Cc: Joel Stanley <joel@jms.id.au>
---
 drivers/hwmon/occ/common.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Eddie James April 17, 2019, 6:35 p.m. UTC | #1
On 4/17/19 1:03 PM, Alexander Amelkin wrote:
> From: Alexander Soldatov <a.soldatov@yadro.com>
>
> 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.


Thanks Alexander and Guenter.


Reviewed-by: Eddie James <eajames@linux.ibm.com>


>
> Signed-off-by: Alexander Soldatov <a.soldatov@yadro.com>
> Signed-off-by: Alexander Amelkin <a.amelkin@yadro.com>
> Reviewed-by: Alexander Amelkin <a.amelkin@yadro.com>
> Cc: Edward A. James <eajames@us.ibm.com>
> Cc: Joel Stanley <joel@jms.id.au>
> ---
>   drivers/hwmon/occ/common.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/drivers/hwmon/occ/common.c b/drivers/hwmon/occ/common.c
> index 4679acb..825631c 100644
> --- a/drivers/hwmon/occ/common.c
> +++ b/drivers/hwmon/occ/common.c
> @@ -235,6 +235,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:
Guenter Roeck April 17, 2019, 7:36 p.m. UTC | #2
On Wed, Apr 17, 2019 at 01:35:41PM -0500, Eddie James wrote:
> 
> On 4/17/19 1:03 PM, Alexander Amelkin wrote:
> >From: Alexander Soldatov <a.soldatov@yadro.com>
> >
> >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.
> 
> 
> Thanks Alexander and Guenter.
> 
> 
> Reviewed-by: Eddie James <eajames@linux.ibm.com>
> 

Applied.

Thanks,
Guenter

> 
> >
> >Signed-off-by: Alexander Soldatov <a.soldatov@yadro.com>
> >Signed-off-by: Alexander Amelkin <a.amelkin@yadro.com>
> >Reviewed-by: Alexander Amelkin <a.amelkin@yadro.com>
> >Cc: Edward A. James <eajames@us.ibm.com>
> >Cc: Joel Stanley <joel@jms.id.au>
> >---
> >  drivers/hwmon/occ/common.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> >diff --git a/drivers/hwmon/occ/common.c b/drivers/hwmon/occ/common.c
> >index 4679acb..825631c 100644
> >--- a/drivers/hwmon/occ/common.c
> >+++ b/drivers/hwmon/occ/common.c
> >@@ -235,6 +235,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:
>
Andrew Jeffery April 18, 2019, 6:13 a.m. UTC | #3
On Thu, 18 Apr 2019, at 05:07, Guenter Roeck wrote:
> On Wed, Apr 17, 2019 at 01:35:41PM -0500, Eddie James wrote:
> > 
> > On 4/17/19 1:03 PM, Alexander Amelkin wrote:
> > >From: Alexander Soldatov <a.soldatov@yadro.com>
> > >
> > >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.
> > 
> > 
> > Thanks Alexander and Guenter.
> > 
> > 
> > Reviewed-by: Eddie James <eajames@linux.ibm.com>
> > 
> 
> Applied.

I've backported and applied this to dev-5.0.

Cheers,

Andrew

> 
> Thanks,
> Guenter
> 
> > 
> > >
> > >Signed-off-by: Alexander Soldatov <a.soldatov@yadro.com>
> > >Signed-off-by: Alexander Amelkin <a.amelkin@yadro.com>
> > >Reviewed-by: Alexander Amelkin <a.amelkin@yadro.com>
> > >Cc: Edward A. James <eajames@us.ibm.com>
> > >Cc: Joel Stanley <joel@jms.id.au>
> > >---
> > >  drivers/hwmon/occ/common.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > >diff --git a/drivers/hwmon/occ/common.c b/drivers/hwmon/occ/common.c
> > >index 4679acb..825631c 100644
> > >--- a/drivers/hwmon/occ/common.c
> > >+++ b/drivers/hwmon/occ/common.c
> > >@@ -235,6 +235,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:
> > 
>
diff mbox series

Patch

diff --git a/drivers/hwmon/occ/common.c b/drivers/hwmon/occ/common.c
index 4679acb..825631c 100644
--- a/drivers/hwmon/occ/common.c
+++ b/drivers/hwmon/occ/common.c
@@ -235,6 +235,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: