[PATCH linux dev-4.10 v2 0/2] drivers: hwmon: occ: sysfs_notify and presence detect

[PATCH linux dev-4.10 v2 0/2] drivers: hwmon: occ: sysfs_notify and presence detect

[Eddie James]

From: "Edward A. James" <eajames@us.ibm.com>

This patch set removes the error checking for the number of OCCs present.
Instead, export a new sysfs attribute to indicate the number of OCCs present
from the master poll response. In addition, add sysfs_notify both to the new
occs_present attribute and the throttling attributes.

Changes since v1:
 * just rebase

Edward A. James (2):
  drivers: hwmon: occ: Add occs_present attribute
  drivers: hwmon: occ: Add sysfs_notify to throttling attributes

 drivers/hwmon/occ/common.c | 49 ++++++++++++++++++++++++++++++++++------------
 drivers/hwmon/occ/common.h |  2 --
 drivers/hwmon/occ/p8_i2c.c |  4 ----
 drivers/hwmon/occ/p9_sbe.c |  4 ----
 4 files changed, 37 insertions(+), 22 deletions(-)

-- 
1.8.3.1

[PATCH linux dev-4.10 v2 1/2] drivers: hwmon: occ: Add occs_present attribute

[Eddie James]

From: "Edward A. James" <eajames@us.ibm.com>

Remove the error on detecting occs_present mismatch. Add a new sysfs
entry for occs_present field. Add sysfs_notify for this field, if it
changes.

Two reasons for this change:
1) Userspace app was having trouble handling this error code, since if you
bind the master OCC device first, you'll immediately get an error that we
haven't bound the same number of OCCs as the OCC reports are available.

2) This error checking for "number of OCC devices bound" isn't designed well
anyway, and makes more sense in userspace. The original intent was to implement
all of the OCC requirements in the driver, but we have an application running
on top that can pick up the slack, as it were. 

Signed-off-by: Edward A. James <eajames@us.ibm.com>
---
 drivers/hwmon/occ/common.c | 32 ++++++++++++++++++++------------
 drivers/hwmon/occ/common.h |  2 --
 drivers/hwmon/occ/p8_i2c.c |  4 ----
 drivers/hwmon/occ/p9_sbe.c |  4 ----
 4 files changed, 20 insertions(+), 22 deletions(-)

diff --git a/drivers/hwmon/occ/common.c b/drivers/hwmon/occ/common.c
index 34002fb..e8579b6 100644
--- a/drivers/hwmon/occ/common.c
+++ b/drivers/hwmon/occ/common.c
@@ -10,7 +10,7 @@
 #include <asm/unaligned.h>
 #include "common.h"
 
-#define OCC_NUM_STATUS_ATTRS		8
+#define OCC_NUM_STATUS_ATTRS		9
 
 #define OCC_STAT_MASTER			0x80
 #define OCC_STAT_ACTIVE			0x01
@@ -19,8 +19,6 @@
 #define OCC_EXT_STAT_MEM_THROTTLE	0x20
 #define OCC_EXT_STAT_QUICK_DROP		0x10
 
-atomic_t occ_num_occs = ATOMIC_INIT(0);
-
 struct temp_sensor_1 {
 	u16 sensor_id;
 	u16 value;
@@ -179,9 +177,11 @@ void occ_reset_error(struct occ *occ)
 int occ_poll(struct occ *occ)
 {
 	int rc, error = occ->error;
-	struct occ_poll_response_header *header;
+	struct occ_poll_response_header *header =
+		(struct occ_poll_response_header *)occ->resp.data;
 	u16 checksum = occ->poll_cmd_data + 1;
 	u8 cmd[8];
+	u8 occs_present = header->occs_present;
 
 	cmd[0] = 0;
 	cmd[1] = 0;
@@ -196,8 +196,6 @@ int occ_poll(struct occ *occ)
 	if (rc)
 		goto done;
 
-	header = (struct occ_poll_response_header *)occ->resp.data;
-
 	if (header->occ_state == OCC_STATE_SAFE) {
 		if (occ->last_safe) {
 			if (time_after(jiffies,
@@ -208,10 +206,10 @@ int occ_poll(struct occ *occ)
 	} else
 		occ->last_safe = 0;
 
-	if (header->status & OCC_STAT_MASTER) {
-		if (hweight8(header->occs_present) !=
-		    atomic_read(&occ_num_occs))
-			occ->error = -ENXIO;
+	if (occs_present != header->occs_present && occ->hwmon &&
+	    (header->status & OCC_STAT_MASTER)) {
+		sysfs_notify(&occ->bus_dev->kobj, NULL,
+			     occ->status_attrs[7].dev_attr.attr.name);
 	}
 
 done:
@@ -321,6 +319,12 @@ static ssize_t occ_show_status(struct device *dev,
 	case 6:
 		val = header->occ_state;
 		break;
+	case 7:
+		if (header->status & OCC_STAT_MASTER)
+			val = hweight8(header->occs_present);
+		else
+			val = 1;
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -1203,12 +1207,16 @@ int occ_create_status_attrs(struct occ *occ)
 		(struct sensor_device_attribute)SENSOR_ATTR(occ_status, 0444,
 							    occ_show_status,
 							    NULL, 6);
-
 	occ->status_attrs[7] =
+		(struct sensor_device_attribute)SENSOR_ATTR(occs_present, 0444,
+							    occ_show_status,
+							    NULL, 7);
+
+	occ->status_attrs[8] =
 		(struct sensor_device_attribute)SENSOR_ATTR(occ_error, 0444,
 							    occ_show_error,
 							    NULL, 0);
-	occ->error_attr_name = occ->status_attrs[7].dev_attr.attr.name;
+	occ->error_attr_name = occ->status_attrs[8].dev_attr.attr.name;
 
 	for (i = 0; i < OCC_NUM_STATUS_ATTRS; ++i) {
 		rc = device_create_file(dev, &occ->status_attrs[i].dev_attr);
diff --git a/drivers/hwmon/occ/common.h b/drivers/hwmon/occ/common.h
index acb50bc..1642fbd 100644
--- a/drivers/hwmon/occ/common.h
+++ b/drivers/hwmon/occ/common.h
@@ -140,8 +140,6 @@ struct occ {
 	((struct sensor_device_attribute_2)				\
 		SENSOR_ATTR_OCC(_name, _mode, _show, _store, _nr, _index))
 
-extern atomic_t occ_num_occs;
-
 void occ_parse_poll_response(struct occ *occ);
 void occ_reset_error(struct occ *occ);
 void occ_set_error(struct occ *occ, int error);
diff --git a/drivers/hwmon/occ/p8_i2c.c b/drivers/hwmon/occ/p8_i2c.c
index e1c6234..020a53b 100644
--- a/drivers/hwmon/occ/p8_i2c.c
+++ b/drivers/hwmon/occ/p8_i2c.c
@@ -248,8 +248,6 @@ static int p8_i2c_occ_probe(struct i2c_client *client,
 		return rc;
 	}
 
-	atomic_inc(&occ_num_occs);
-
 	return 0;
 }
 
@@ -259,8 +257,6 @@ static int p8_i2c_occ_remove(struct i2c_client *client)
 
 	occ_remove_status_attrs(occ);
 
-	atomic_dec(&occ_num_occs);
-
 	return 0;
 }
 
diff --git a/drivers/hwmon/occ/p9_sbe.c b/drivers/hwmon/occ/p9_sbe.c
index ac18422..a583c4b 100644
--- a/drivers/hwmon/occ/p9_sbe.c
+++ b/drivers/hwmon/occ/p9_sbe.c
@@ -176,8 +176,6 @@ static int p9_sbe_occ_probe(struct platform_device *pdev)
 	if (rc)
 		return rc;
 
-	atomic_inc(&occ_num_occs);
-
 	return rc;
 }
 
@@ -190,8 +188,6 @@ static int p9_sbe_occ_remove(struct platform_device *pdev)
 	p9_sbe_occ_close_client(p9_sbe_occ);
 	occ_remove_status_attrs(occ);
 
-	atomic_dec(&occ_num_occs);
-
 	return 0;
 }
 
-- 
1.8.3.1

[PATCH linux dev-4.10 v2 2/2] drivers: hwmon: occ: Add sysfs_notify to throttling attributes

[Eddie James]

From: "Edward A. James" <eajames@us.ibm.com>

Need to alert user space when we change throttling state.

Signed-off-by: Edward A. James <eajames@us.ibm.com>
---
 drivers/hwmon/occ/common.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/hwmon/occ/common.c b/drivers/hwmon/occ/common.c
index e8579b6..242ba40 100644
--- a/drivers/hwmon/occ/common.c
+++ b/drivers/hwmon/occ/common.c
@@ -182,6 +182,7 @@ int occ_poll(struct occ *occ)
 	u16 checksum = occ->poll_cmd_data + 1;
 	u8 cmd[8];
 	u8 occs_present = header->occs_present;
+	u8 ext_status = header->ext_status;
 
 	cmd[0] = 0;
 	cmd[1] = 0;
@@ -206,10 +207,26 @@ int occ_poll(struct occ *occ)
 	} else
 		occ->last_safe = 0;
 
-	if (occs_present != header->occs_present && occ->hwmon &&
-	    (header->status & OCC_STAT_MASTER)) {
-		sysfs_notify(&occ->bus_dev->kobj, NULL,
-			     occ->status_attrs[7].dev_attr.attr.name);
+	if (occ->hwmon) {
+		if (occs_present != header->occs_present &&
+		    (header->status & OCC_STAT_MASTER))
+			sysfs_notify(&occ->bus_dev->kobj, NULL,
+				     occ->status_attrs[7].dev_attr.attr.name);
+
+		if ((header->ext_status & OCC_EXT_STAT_DVFS_OT) != 
+		    (ext_status & OCC_EXT_STAT_DVFS_OT))
+			sysfs_notify(&occ->bus_dev->kobj, NULL,
+				     occ->status_attrs[2].dev_attr.attr.name);
+
+		if ((header->ext_status & OCC_EXT_STAT_DVFS_POWER) != 
+		    (ext_status & OCC_EXT_STAT_DVFS_POWER))
+			sysfs_notify(&occ->bus_dev->kobj, NULL,
+				     occ->status_attrs[3].dev_attr.attr.name);
+
+		if ((header->ext_status & OCC_EXT_STAT_MEM_THROTTLE) != 
+		    (ext_status & OCC_EXT_STAT_MEM_THROTTLE))
+			sysfs_notify(&occ->bus_dev->kobj, NULL,
+				     occ->status_attrs[4].dev_attr.attr.name);
 	}
 
 done:
-- 
1.8.3.1

Re: [PATCH linux dev-4.10 v2 0/2] drivers: hwmon: occ: sysfs_notify and presence detect

[Andrew Jeffery]

[-- Attachment #1: Type: text/plain, Size: 921 bytes --]

On Mon, 2017-10-16 at 11:29 -0500, Eddie James wrote:
> > From: "Edward A. James" <eajames@us.ibm.com>
> 
> This patch set removes the error checking for the number of OCCs present.
> Instead, export a new sysfs attribute to indicate the number of OCCs present
> from the master poll response. In addition, add sysfs_notify both to the new
> occs_present attribute and the throttling attributes.
> 
> Changes since v1:
>  * just rebase
> 
> Edward A. James (2):
>   drivers: hwmon: occ: Add occs_present attribute
>   drivers: hwmon: occ: Add sysfs_notify to throttling attributes

Applied to dev-4.10.

Cheers,

Andrew

> 
>  drivers/hwmon/occ/common.c | 49 ++++++++++++++++++++++++++++++++++------------
>  drivers/hwmon/occ/common.h |  2 --
>  drivers/hwmon/occ/p8_i2c.c |  4 ----
>  drivers/hwmon/occ/p9_sbe.c |  4 ----
>  4 files changed, 37 insertions(+), 22 deletions(-)
> 

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]