linux-hwmon.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] hwmon (occ): Store error condition for rate-limited polls
@ 2019-04-16 15:43 Eddie James
  2019-04-16 15:43 ` [PATCH 2/3] hwmon (occ): Prevent sysfs error attribute from returning error Eddie James
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Eddie James @ 2019-04-16 15:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-hwmon, linux, jdelvare, andrew, mine260309, Eddie James

The OCC driver limits the rate of sending poll commands to the OCC. If a
user reads a hwmon entry after a poll response resulted in an error and
is rate-limited, the error is invisible to the user. Fix this by storing
the last error and returning that in the rate-limited case.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 drivers/hwmon/occ/common.c | 4 ++++
 drivers/hwmon/occ/common.h | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/occ/common.c b/drivers/hwmon/occ/common.c
index 9d197e9..13a6290 100644
--- a/drivers/hwmon/occ/common.c
+++ b/drivers/hwmon/occ/common.c
@@ -141,6 +141,7 @@ static int occ_poll(struct occ *occ)
 	/* mutex should already be locked if necessary */
 	rc = occ->send_cmd(occ, cmd);
 	if (rc) {
+		occ->last_error = rc;
 		if (occ->error_count++ > OCC_ERROR_COUNT_THRESHOLD)
 			occ->error = rc;
 
@@ -149,6 +150,7 @@ static int occ_poll(struct occ *occ)
 
 	/* clear error since communication was successful */
 	occ->error_count = 0;
+	occ->last_error = 0;
 	occ->error = 0;
 
 	/* check for safe state */
@@ -210,6 +212,8 @@ int occ_update_response(struct occ *occ)
 	if (time_after(jiffies, occ->last_update + OCC_UPDATE_FREQUENCY)) {
 		rc = occ_poll(occ);
 		occ->last_update = jiffies;
+	} else {
+		rc = occ->last_error;
 	}
 
 	mutex_unlock(&occ->lock);
diff --git a/drivers/hwmon/occ/common.h b/drivers/hwmon/occ/common.h
index ed2cf42..fc13f3c 100644
--- a/drivers/hwmon/occ/common.h
+++ b/drivers/hwmon/occ/common.h
@@ -106,7 +106,8 @@ struct occ {
 	struct attribute_group group;
 	const struct attribute_group *groups[2];
 
-	int error;                      /* latest transfer error */
+	int error;                      /* final transfer error after retry */
+	int last_error;			/* latest transfer error */
 	unsigned int error_count;       /* number of xfr errors observed */
 	unsigned long last_safe;        /* time OCC entered "safe" state */
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/3] hwmon (occ): Prevent sysfs error attribute from returning error
  2019-04-16 15:43 [PATCH 1/3] hwmon (occ): Store error condition for rate-limited polls Eddie James
@ 2019-04-16 15:43 ` Eddie James
  2019-04-16 22:14   ` Guenter Roeck
  2019-04-16 15:43 ` [PATCH 3/3] hwmon (occ): Add more details to Kconfig help text Eddie James
  2019-04-16 22:12 ` [PATCH 1/3] hwmon (occ): Store error condition for rate-limited polls Guenter Roeck
  2 siblings, 1 reply; 6+ messages in thread
From: Eddie James @ 2019-04-16 15:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-hwmon, linux, jdelvare, andrew, mine260309, Eddie James

The error sysfs attribute returns the stored error state of the OCC and
doesn't depend on the OCC poll response. Therefore, split the error
attribute into it's own function to avoid failing out of the function if
the poll response fails.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 drivers/hwmon/occ/sysfs.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/hwmon/occ/sysfs.c b/drivers/hwmon/occ/sysfs.c
index 1cb1e65..c73be07 100644
--- a/drivers/hwmon/occ/sysfs.c
+++ b/drivers/hwmon/occ/sysfs.c
@@ -63,9 +63,6 @@ static ssize_t occ_sysfs_show(struct device *dev,
 		else
 			val = 1;
 		break;
-	case 8:
-		val = occ->error;
-		break;
 	default:
 		return -EINVAL;
 	}
@@ -73,6 +70,16 @@ static ssize_t occ_sysfs_show(struct device *dev,
 	return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
 }
 
+static ssize_t occ_error_show(struct device *dev,
+			      struct device_attribute *attr, char *buf)
+{
+	struct occ *occ = dev_get_drvdata(dev);
+
+	occ_update_response(occ);
+
+	return snprintf(buf, PAGE_SIZE - 1, "%d\n", occ->error);
+}
+
 static SENSOR_DEVICE_ATTR(occ_master, 0444, occ_sysfs_show, NULL, 0);
 static SENSOR_DEVICE_ATTR(occ_active, 0444, occ_sysfs_show, NULL, 1);
 static SENSOR_DEVICE_ATTR(occ_dvfs_overtemp, 0444, occ_sysfs_show, NULL, 2);
@@ -81,7 +88,7 @@ static SENSOR_DEVICE_ATTR(occ_mem_throttle, 0444, occ_sysfs_show, NULL, 4);
 static SENSOR_DEVICE_ATTR(occ_quick_pwr_drop, 0444, occ_sysfs_show, NULL, 5);
 static SENSOR_DEVICE_ATTR(occ_state, 0444, occ_sysfs_show, NULL, 6);
 static SENSOR_DEVICE_ATTR(occs_present, 0444, occ_sysfs_show, NULL, 7);
-static SENSOR_DEVICE_ATTR(occ_error, 0444, occ_sysfs_show, NULL, 8);
+static DEVICE_ATTR_RO(occ_error);
 
 static struct attribute *occ_attributes[] = {
 	&sensor_dev_attr_occ_master.dev_attr.attr,
@@ -92,7 +99,7 @@ static struct attribute *occ_attributes[] = {
 	&sensor_dev_attr_occ_quick_pwr_drop.dev_attr.attr,
 	&sensor_dev_attr_occ_state.dev_attr.attr,
 	&sensor_dev_attr_occs_present.dev_attr.attr,
-	&sensor_dev_attr_occ_error.dev_attr.attr,
+	&dev_attr_occ_error.attr,
 	NULL
 };
 
@@ -156,7 +163,7 @@ void occ_sysfs_poll_done(struct occ *occ)
 	}
 
 	if (occ->error && occ->error != occ->prev_error) {
-		name = sensor_dev_attr_occ_error.dev_attr.attr.name;
+		name = dev_attr_occ_error.attr.name;
 		sysfs_notify(&occ->bus_dev->kobj, NULL, name);
 	}
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/3] hwmon (occ): Add more details to Kconfig help text
  2019-04-16 15:43 [PATCH 1/3] hwmon (occ): Store error condition for rate-limited polls Eddie James
  2019-04-16 15:43 ` [PATCH 2/3] hwmon (occ): Prevent sysfs error attribute from returning error Eddie James
@ 2019-04-16 15:43 ` Eddie James
  2019-04-16 22:15   ` Guenter Roeck
  2019-04-16 22:12 ` [PATCH 1/3] hwmon (occ): Store error condition for rate-limited polls Guenter Roeck
  2 siblings, 1 reply; 6+ messages in thread
From: Eddie James @ 2019-04-16 15:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-hwmon, linux, jdelvare, andrew, mine260309, Eddie James

From: Eddie James <eajames@us.ibm.com>

The help text needs to spell out how the driver runs on a BMC, as it
previously seemed to indicate it ran on a POWER processor.

Signed-off-by: Eddie James <eajames@us.ibm.com>
---
 drivers/hwmon/occ/Kconfig | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/hwmon/occ/Kconfig b/drivers/hwmon/occ/Kconfig
index 2a3869f..1658634 100644
--- a/drivers/hwmon/occ/Kconfig
+++ b/drivers/hwmon/occ/Kconfig
@@ -9,8 +9,10 @@ config SENSORS_OCC_P8_I2C
 	select SENSORS_OCC
 	help
 	 This option enables support for monitoring sensors provided by the
-	 On-Chip Controller (OCC) on a POWER8 processor. Communications with
-	 the OCC are established through I2C bus.
+	 On-Chip Controller (OCC) on a POWER8 processor. However, this driver
+	 can only run on a baseboard management controller (BMC) connected to
+	 the P8, not the POWER processor itself. Communications with the OCC are
+	 established through I2C bus.
 
 	 This driver can also be built as a module. If so, the module will be
 	 called occ-p8-hwmon.
@@ -22,8 +24,10 @@ config SENSORS_OCC_P9_SBE
 	select SENSORS_OCC
 	help
 	 This option enables support for monitoring sensors provided by the
-	 On-Chip Controller (OCC) on a POWER9 processor. Communications with
-	 the OCC are established through SBE fifo on an FSI bus.
+	 On-Chip Controller (OCC) on a POWER9 processor. However, this driver
+	 can only run on a baseboard management controller (BMC) connected to
+	 the P9, not the POWER processor itself. Communications with the OCC are
+	 established through SBE fifo on an FSI bus.
 
 	 This driver can also be built as a module. If so, the module will be
 	 called occ-p9-hwmon.
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] hwmon (occ): Store error condition for rate-limited polls
  2019-04-16 15:43 [PATCH 1/3] hwmon (occ): Store error condition for rate-limited polls Eddie James
  2019-04-16 15:43 ` [PATCH 2/3] hwmon (occ): Prevent sysfs error attribute from returning error Eddie James
  2019-04-16 15:43 ` [PATCH 3/3] hwmon (occ): Add more details to Kconfig help text Eddie James
@ 2019-04-16 22:12 ` Guenter Roeck
  2 siblings, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2019-04-16 22:12 UTC (permalink / raw)
  To: Eddie James; +Cc: linux-kernel, linux-hwmon, jdelvare, andrew, mine260309

On Tue, Apr 16, 2019 at 03:43:48PM +0000, Eddie James wrote:
> The OCC driver limits the rate of sending poll commands to the OCC. If a
> user reads a hwmon entry after a poll response resulted in an error and
> is rate-limited, the error is invisible to the user. Fix this by storing
> the last error and returning that in the rate-limited case.
> 
> Signed-off-by: Eddie James <eajames@linux.ibm.com>

Applied.

Thanks,
Guenter

> ---
>  drivers/hwmon/occ/common.c | 4 ++++
>  drivers/hwmon/occ/common.h | 3 ++-
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/occ/common.c b/drivers/hwmon/occ/common.c
> index 9d197e9..13a6290 100644
> --- a/drivers/hwmon/occ/common.c
> +++ b/drivers/hwmon/occ/common.c
> @@ -141,6 +141,7 @@ static int occ_poll(struct occ *occ)
>  	/* mutex should already be locked if necessary */
>  	rc = occ->send_cmd(occ, cmd);
>  	if (rc) {
> +		occ->last_error = rc;
>  		if (occ->error_count++ > OCC_ERROR_COUNT_THRESHOLD)
>  			occ->error = rc;
>  
> @@ -149,6 +150,7 @@ static int occ_poll(struct occ *occ)
>  
>  	/* clear error since communication was successful */
>  	occ->error_count = 0;
> +	occ->last_error = 0;
>  	occ->error = 0;
>  
>  	/* check for safe state */
> @@ -210,6 +212,8 @@ int occ_update_response(struct occ *occ)
>  	if (time_after(jiffies, occ->last_update + OCC_UPDATE_FREQUENCY)) {
>  		rc = occ_poll(occ);
>  		occ->last_update = jiffies;
> +	} else {
> +		rc = occ->last_error;
>  	}
>  
>  	mutex_unlock(&occ->lock);
> diff --git a/drivers/hwmon/occ/common.h b/drivers/hwmon/occ/common.h
> index ed2cf42..fc13f3c 100644
> --- a/drivers/hwmon/occ/common.h
> +++ b/drivers/hwmon/occ/common.h
> @@ -106,7 +106,8 @@ struct occ {
>  	struct attribute_group group;
>  	const struct attribute_group *groups[2];
>  
> -	int error;                      /* latest transfer error */
> +	int error;                      /* final transfer error after retry */
> +	int last_error;			/* latest transfer error */
>  	unsigned int error_count;       /* number of xfr errors observed */
>  	unsigned long last_safe;        /* time OCC entered "safe" state */
>  

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/3] hwmon (occ): Prevent sysfs error attribute from returning error
  2019-04-16 15:43 ` [PATCH 2/3] hwmon (occ): Prevent sysfs error attribute from returning error Eddie James
@ 2019-04-16 22:14   ` Guenter Roeck
  0 siblings, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2019-04-16 22:14 UTC (permalink / raw)
  To: Eddie James; +Cc: linux-kernel, linux-hwmon, jdelvare, andrew, mine260309

On Tue, Apr 16, 2019 at 03:43:49PM +0000, Eddie James wrote:
> The error sysfs attribute returns the stored error state of the OCC and
> doesn't depend on the OCC poll response. Therefore, split the error
> attribute into it's own function to avoid failing out of the function if
> the poll response fails.
> 
> Signed-off-by: Eddie James <eajames@linux.ibm.com>

Applied.

Thanks,
Guenter

> ---
>  drivers/hwmon/occ/sysfs.c | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/hwmon/occ/sysfs.c b/drivers/hwmon/occ/sysfs.c
> index 1cb1e65..c73be07 100644
> --- a/drivers/hwmon/occ/sysfs.c
> +++ b/drivers/hwmon/occ/sysfs.c
> @@ -63,9 +63,6 @@ static ssize_t occ_sysfs_show(struct device *dev,
>  		else
>  			val = 1;
>  		break;
> -	case 8:
> -		val = occ->error;
> -		break;
>  	default:
>  		return -EINVAL;
>  	}
> @@ -73,6 +70,16 @@ static ssize_t occ_sysfs_show(struct device *dev,
>  	return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
>  }
>  
> +static ssize_t occ_error_show(struct device *dev,
> +			      struct device_attribute *attr, char *buf)
> +{
> +	struct occ *occ = dev_get_drvdata(dev);
> +
> +	occ_update_response(occ);
> +
> +	return snprintf(buf, PAGE_SIZE - 1, "%d\n", occ->error);
> +}
> +
>  static SENSOR_DEVICE_ATTR(occ_master, 0444, occ_sysfs_show, NULL, 0);
>  static SENSOR_DEVICE_ATTR(occ_active, 0444, occ_sysfs_show, NULL, 1);
>  static SENSOR_DEVICE_ATTR(occ_dvfs_overtemp, 0444, occ_sysfs_show, NULL, 2);
> @@ -81,7 +88,7 @@ static SENSOR_DEVICE_ATTR(occ_mem_throttle, 0444, occ_sysfs_show, NULL, 4);
>  static SENSOR_DEVICE_ATTR(occ_quick_pwr_drop, 0444, occ_sysfs_show, NULL, 5);
>  static SENSOR_DEVICE_ATTR(occ_state, 0444, occ_sysfs_show, NULL, 6);
>  static SENSOR_DEVICE_ATTR(occs_present, 0444, occ_sysfs_show, NULL, 7);
> -static SENSOR_DEVICE_ATTR(occ_error, 0444, occ_sysfs_show, NULL, 8);
> +static DEVICE_ATTR_RO(occ_error);
>  
>  static struct attribute *occ_attributes[] = {
>  	&sensor_dev_attr_occ_master.dev_attr.attr,
> @@ -92,7 +99,7 @@ static struct attribute *occ_attributes[] = {
>  	&sensor_dev_attr_occ_quick_pwr_drop.dev_attr.attr,
>  	&sensor_dev_attr_occ_state.dev_attr.attr,
>  	&sensor_dev_attr_occs_present.dev_attr.attr,
> -	&sensor_dev_attr_occ_error.dev_attr.attr,
> +	&dev_attr_occ_error.attr,
>  	NULL
>  };
>  
> @@ -156,7 +163,7 @@ void occ_sysfs_poll_done(struct occ *occ)
>  	}
>  
>  	if (occ->error && occ->error != occ->prev_error) {
> -		name = sensor_dev_attr_occ_error.dev_attr.attr.name;
> +		name = dev_attr_occ_error.attr.name;
>  		sysfs_notify(&occ->bus_dev->kobj, NULL, name);
>  	}
>  

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/3] hwmon (occ): Add more details to Kconfig help text
  2019-04-16 15:43 ` [PATCH 3/3] hwmon (occ): Add more details to Kconfig help text Eddie James
@ 2019-04-16 22:15   ` Guenter Roeck
  0 siblings, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2019-04-16 22:15 UTC (permalink / raw)
  To: Eddie James
  Cc: linux-kernel, linux-hwmon, jdelvare, andrew, mine260309, Eddie James

On Tue, Apr 16, 2019 at 03:43:50PM +0000, Eddie James wrote:
> From: Eddie James <eajames@us.ibm.com>
> 
> The help text needs to spell out how the driver runs on a BMC, as it
> previously seemed to indicate it ran on a POWER processor.
> 
> Signed-off-by: Eddie James <eajames@us.ibm.com>

Applied.

Thanks,
Guenter

> ---
>  drivers/hwmon/occ/Kconfig | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/hwmon/occ/Kconfig b/drivers/hwmon/occ/Kconfig
> index 2a3869f..1658634 100644
> --- a/drivers/hwmon/occ/Kconfig
> +++ b/drivers/hwmon/occ/Kconfig
> @@ -9,8 +9,10 @@ config SENSORS_OCC_P8_I2C
>  	select SENSORS_OCC
>  	help
>  	 This option enables support for monitoring sensors provided by the
> -	 On-Chip Controller (OCC) on a POWER8 processor. Communications with
> -	 the OCC are established through I2C bus.
> +	 On-Chip Controller (OCC) on a POWER8 processor. However, this driver
> +	 can only run on a baseboard management controller (BMC) connected to
> +	 the P8, not the POWER processor itself. Communications with the OCC are
> +	 established through I2C bus.
>  
>  	 This driver can also be built as a module. If so, the module will be
>  	 called occ-p8-hwmon.
> @@ -22,8 +24,10 @@ config SENSORS_OCC_P9_SBE
>  	select SENSORS_OCC
>  	help
>  	 This option enables support for monitoring sensors provided by the
> -	 On-Chip Controller (OCC) on a POWER9 processor. Communications with
> -	 the OCC are established through SBE fifo on an FSI bus.
> +	 On-Chip Controller (OCC) on a POWER9 processor. However, this driver
> +	 can only run on a baseboard management controller (BMC) connected to
> +	 the P9, not the POWER processor itself. Communications with the OCC are
> +	 established through SBE fifo on an FSI bus.
>  
>  	 This driver can also be built as a module. If so, the module will be
>  	 called occ-p9-hwmon.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-04-16 22:15 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-16 15:43 [PATCH 1/3] hwmon (occ): Store error condition for rate-limited polls Eddie James
2019-04-16 15:43 ` [PATCH 2/3] hwmon (occ): Prevent sysfs error attribute from returning error Eddie James
2019-04-16 22:14   ` Guenter Roeck
2019-04-16 15:43 ` [PATCH 3/3] hwmon (occ): Add more details to Kconfig help text Eddie James
2019-04-16 22:15   ` Guenter Roeck
2019-04-16 22:12 ` [PATCH 1/3] hwmon (occ): Store error condition for rate-limited polls Guenter Roeck

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).