Skip to contentSkip to conversation

hwmon: (ucd9000) Add syfs access files for chip specific faults

PATCH In review Christopher Bostic 2 participants 3 messages Aug 12, 2017 – Aug 16, 2017 Hardware monitoring

lore
Share & tools
lore

Christopher Bostic

author
#0 lore raw

PATCH v2 1/1 hwmon: (ucd9000) Add syfs access files for chip specific faults

Create sysfs files for mfr specific fault indicators. Add sysfs files for GPIO configuration access. Link driver into the gpio device driver.

Signed-off-by Christopher Bostic <[email protected]>

---
v2 - Removed clear_faults file - redundant since all other sysfs
core accesses result in an automatic clear fault.
- Removed local status_word and status_vout register dumps and use the new pmbus core status facilities instead.
- Rename gpi<x>_fault to gpi<x>_alarm to better match core naming conventions.
- Add full register dump for mfr_status.
- Add gpio chip to device structure and use gpio interfaces.

---
 drivers/hwmon/pmbus/ucd9000.c | 268 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 266 insertions(+), 2 deletions(-)
drivers/hwmon/pmbus/ucd9000.c +266 -2
index 3e3aa95..ecd3e1a 100644
@@ -26,6 +26,9 @@
 #include <linux/slab.h>
 #include <linux/i2c.h>
 #include <linux/i2c/pmbus.h>
+#include <linux/sysfs.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/gpio.h>
 #include "pmbus.h"
 
 enum chips { ucd9000, ucd90120, ucd90124, ucd90160, ucd9090, ucd90910 };

@@ -34,6 +37,10 @@
 #define UCD9000_NUM_PAGES		0xd6
 #define UCD9000_FAN_CONFIG_INDEX	0xe7
 #define UCD9000_FAN_CONFIG		0xe8
+#define UCD9000_LOGGED_FAULTS		0xea
+#define UCD9000_MFR_STATUS		0xf3
+#define UCD9000_GPIO_SELECT		0xfa
+#define UCD9000_GPIO_CONFIG		0xfb
 #define UCD9000_DEVICE_ID		0xfd
 
 #define UCD9000_MON_TYPE(x)	(((x) >> 5) & 0x07)

@@ -46,9 +53,14 @@
 
 #define UCD9000_NUM_FAN		4
 
+#define UCD9000_FULL_REG	BIT(8)
+
+#define UCD90160_NUM_GPIOS	26
+
 struct ucd9000_data {
 	u8 fan_data[UCD9000_NUM_FAN][I2C_SMBUS_BLOCK_MAX];
 	struct pmbus_driver_info info;
+	struct gpio_chip gpio;
 };
 #define to_ucd9000_data(_info) container_of(_info, struct ucd9000_data, info)
 

@@ -119,6 +131,225 @@ static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg)
 };
 MODULE_DEVICE_TABLE(i2c, ucd9000_id);
 
+static void ucd9000_gpio_set(struct gpio_chip *gc, unsigned int offset,
+			int value)
+{
+	struct i2c_client *client  = gpiochip_get_data(gc);
+	int ret;
+
+	ret = i2c_smbus_write_byte_data(client, offset, value);
+	if (ret)
+		dev_err(&client->dev, "Failed to set gpio select %d\n", value);
+}
+
+static int ucd9000_gpio_get(struct gpio_chip *gc, unsigned int offset)
+{
+	struct i2c_client *client  = gpiochip_get_data(gc);
+
+	return i2c_smbus_read_byte_data(client, offset);
+}
+
+static ssize_t ucd9000_clear_logged_faults(struct device *dev,
+				struct device_attribute *attr, const char *buf,
+				size_t count)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	int ret;
+
+	ret = i2c_smbus_write_byte_data(client, UCD9000_LOGGED_FAULTS, 0);
+	if (ret) {
+		dev_err(&client->dev, "Failed to clear logged faults\n");
+		return ret;
+	}
+
+	return count;
+}
+
+static ssize_t ucd9000_mfr_status(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
+	bool full_reg = !!(sensor_attr->index & UCD9000_FULL_REG);
+	struct i2c_client *client = to_i2c_client(dev);
+	int nr = sensor_attr->index, ret;
+	u32 block_buffer;
+
+	ret = pmbus_set_page(client, 0);
+	if (ret < 0) {
+		dev_err(&client->dev, "i2c_smbus_write failed. rc:%d\n", ret);
+		return ret;
+	}
+
+	ret = i2c_smbus_read_block_data(client, UCD9000_MFR_STATUS,
+					(u8 *)&block_buffer);
+	if (ret < 0) {
+		dev_err(&client->dev,
+			"Failed to read mfr status. rc:%d\n", ret);
+		return ret;
+	}
+
+	if (full_reg)
+		return sprintf(buf, "%08x\n", ret);
+	else
+		return sprintf(buf, "%1d\n", ret & BIT(nr) ? 1 : 0);
+}
+
+static ssize_t ucd9000_gpi_config(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
+	struct i2c_client *client = to_i2c_client(dev);
+	struct ucd9000_data *data
+		= to_ucd9000_data(pmbus_get_driver_info(client));
+	int nr = (sensor_attr->index & 0xf00) >> 8;
+	int reg = sensor_attr->index & 0xff, ret;
+
+
+	ucd9000_gpio_set(&data->gpio, UCD9000_GPIO_SELECT, reg);
+	ret = ucd9000_gpio_get(&data->gpio, UCD9000_GPIO_CONFIG);
+	if (ret < 0) {
+		dev_err(&client->dev,
+			"Failed to read gpi%02d config\n", reg);
+	}
+
+	return sprintf(buf, "%1d\n", ret & BIT(nr) ? 1 : 0);
+}
+
+static DEVICE_ATTR(clear_logged_faults, 0200,
+			  NULL, ucd9000_clear_logged_faults);
+
+/* MFR status */
+static SENSOR_DEVICE_ATTR(gpi8_alarm, 0444, ucd9000_mfr_status, NULL, 23);
+static SENSOR_DEVICE_ATTR(gpi7_alarm, 0444, ucd9000_mfr_status, NULL, 22);
+static SENSOR_DEVICE_ATTR(gpi6_alarm, 0444, ucd9000_mfr_status, NULL, 21);
+static SENSOR_DEVICE_ATTR(gpi5_alarm, 0444, ucd9000_mfr_status, NULL, 20);
+static SENSOR_DEVICE_ATTR(gpi4_alarm, 0444, ucd9000_mfr_status, NULL, 19);
+static SENSOR_DEVICE_ATTR(gpi3_alarm, 0444, ucd9000_mfr_status, NULL, 18);
+static SENSOR_DEVICE_ATTR(gpi2_alarm, 0444, ucd9000_mfr_status, NULL, 17);
+static SENSOR_DEVICE_ATTR(gpi1_alarm, 0444, ucd9000_mfr_status, NULL, 16);
+static SENSOR_DEVICE_ATTR(new_logged_fault_detail, 0444,
+			ucd9000_mfr_status, NULL, 12);
+static SENSOR_DEVICE_ATTR(system_watchdog_timeout, 0444,
+			ucd9000_mfr_status, NULL, 11);
+static SENSOR_DEVICE_ATTR(store_default_all_error, 0444,
+			ucd9000_mfr_status, NULL, 10);
+static SENSOR_DEVICE_ATTR(store_default_all_done, 0444,
+			ucd9000_mfr_status, NULL, 9);
+static SENSOR_DEVICE_ATTR(watchdog_timeout, 0444,
+			ucd9000_mfr_status, NULL, 8);
+static SENSOR_DEVICE_ATTR(invalid_logs, 0444,
+			ucd9000_mfr_status, NULL, 7);
+static SENSOR_DEVICE_ATTR(logged_fault_detail, 0444,
+			ucd9000_mfr_status, NULL, 6);
+static SENSOR_DEVICE_ATTR(resequence_error, 0444, ucd9000_mfr_status, NULL, 5);
+static SENSOR_DEVICE_ATTR(pkgid_mismatch, 0444, ucd9000_mfr_status, NULL, 4);
+static SENSOR_DEVICE_ATTR(hardcoded_parms, 0444, ucd9000_mfr_status, NULL, 3);
+static SENSOR_DEVICE_ATTR(seq_off_timeout, 0444, ucd9000_mfr_status, NULL, 2);
+static SENSOR_DEVICE_ATTR(seq_on_timeout, 0444, ucd9000_mfr_status, NULL, 1);
+static SENSOR_DEVICE_ATTR(mfr_status, 0444,
+			ucd9000_mfr_status, NULL, UCD9000_FULL_REG);
+
+/* GPIO config */
+static SENSOR_DEVICE_ATTR(gpio8_enab, 0444, ucd9000_gpi_config, NULL, 8);
+static SENSOR_DEVICE_ATTR(gpio8_out_enab, 0444, ucd9000_gpi_config,
+			NULL, 0x0108);
+static SENSOR_DEVICE_ATTR(gpio8_out_val, 0444, ucd9000_gpi_config,
+			NULL, 0x0208);
+static SENSOR_DEVICE_ATTR(gpio8_stat, 0444, ucd9000_gpi_config,
+			NULL, 0x0308);
+static SENSOR_DEVICE_ATTR(gpio9_enab, 0444, ucd9000_gpi_config, NULL, 9);
+static SENSOR_DEVICE_ATTR(gpio9_out_enab, 0444, ucd9000_gpi_config,
+			NULL, 0x0109);
+static SENSOR_DEVICE_ATTR(gpio9_out_val, 0444, ucd9000_gpi_config,
+			NULL, 0x0209);
+static SENSOR_DEVICE_ATTR(gpio9_stat, 0444, ucd9000_gpi_config,
+			NULL, 0x0309);
+static SENSOR_DEVICE_ATTR(gpio10_enab, 0444, ucd9000_gpi_config, NULL, 10);
+static SENSOR_DEVICE_ATTR(gpio10_out_enab, 0444, ucd9000_gpi_config,
+			NULL, 0x010a);
+static SENSOR_DEVICE_ATTR(gpio10_out_val, 0444, ucd9000_gpi_config,
+			NULL, 0x020a);
+static SENSOR_DEVICE_ATTR(gpio10_stat, 0444, ucd9000_gpi_config,
+			NULL, 0x030a);
+static SENSOR_DEVICE_ATTR(gpio11_enab, 0444, ucd9000_gpi_config, NULL, 11);
+static SENSOR_DEVICE_ATTR(gpio11_out_enab, 0444, ucd9000_gpi_config,
+			NULL, 0x010b);
+static SENSOR_DEVICE_ATTR(gpio11_out_val, 0444, ucd9000_gpi_config,
+			NULL, 0x020b);
+static SENSOR_DEVICE_ATTR(gpio11_stat, 0444, ucd9000_gpi_config,
+			NULL, 0x030b);
+static SENSOR_DEVICE_ATTR(gpio14_enab, 0444, ucd9000_gpi_config, NULL, 14);
+static SENSOR_DEVICE_ATTR(gpio14_out_enab, 0444, ucd9000_gpi_config,
+			NULL, 0x010e);
+static SENSOR_DEVICE_ATTR(gpio14_out_val, 0444, ucd9000_gpi_config,
+			NULL, 0x020e);
+static SENSOR_DEVICE_ATTR(gpio14_stat, 0444, ucd9000_gpi_config,
+			NULL, 0x030e);
+static SENSOR_DEVICE_ATTR(gpio17_enab, 0444, ucd9000_gpi_config, NULL, 17);
+static SENSOR_DEVICE_ATTR(gpio17_out_enab, 0444, ucd9000_gpi_config,
+			NULL, 0x0111);
+static SENSOR_DEVICE_ATTR(gpio17_out_val, 0444, ucd9000_gpi_config,
+			NULL, 0x0211);
+static SENSOR_DEVICE_ATTR(gpio17_stat, 0444, ucd9000_gpi_config,
+			NULL, 0x0311);
+
+
+static struct attribute *ucd9000_attributes[] = {
+	&dev_attr_clear_logged_faults.attr,
+	&sensor_dev_attr_gpi8_alarm.dev_attr.attr,
+	&sensor_dev_attr_gpi7_alarm.dev_attr.attr,
+	&sensor_dev_attr_gpi6_alarm.dev_attr.attr,
+	&sensor_dev_attr_gpi5_alarm.dev_attr.attr,
+	&sensor_dev_attr_gpi4_alarm.dev_attr.attr,
+	&sensor_dev_attr_gpi3_alarm.dev_attr.attr,
+	&sensor_dev_attr_gpi2_alarm.dev_attr.attr,
+	&sensor_dev_attr_gpi1_alarm.dev_attr.attr,
+	&sensor_dev_attr_new_logged_fault_detail.dev_attr.attr,
+	&sensor_dev_attr_system_watchdog_timeout.dev_attr.attr,
+	&sensor_dev_attr_store_default_all_error.dev_attr.attr,
+	&sensor_dev_attr_store_default_all_done.dev_attr.attr,
+	&sensor_dev_attr_watchdog_timeout.dev_attr.attr,
+	&sensor_dev_attr_invalid_logs.dev_attr.attr,
+	&sensor_dev_attr_logged_fault_detail.dev_attr.attr,
+	&sensor_dev_attr_resequence_error.dev_attr.attr,
+	&sensor_dev_attr_pkgid_mismatch.dev_attr.attr,
+	&sensor_dev_attr_hardcoded_parms.dev_attr.attr,
+	&sensor_dev_attr_seq_off_timeout.dev_attr.attr,
+	&sensor_dev_attr_seq_on_timeout.dev_attr.attr,
+	&sensor_dev_attr_mfr_status.dev_attr.attr,
+	&sensor_dev_attr_gpio8_enab.dev_attr.attr,
+	&sensor_dev_attr_gpio8_out_enab.dev_attr.attr,
+	&sensor_dev_attr_gpio8_out_val.dev_attr.attr,
+	&sensor_dev_attr_gpio8_stat.dev_attr.attr,
+	&sensor_dev_attr_gpio9_enab.dev_attr.attr,
+	&sensor_dev_attr_gpio9_out_enab.dev_attr.attr,
+	&sensor_dev_attr_gpio9_out_val.dev_attr.attr,
+	&sensor_dev_attr_gpio9_stat.dev_attr.attr,
+	&sensor_dev_attr_gpio10_enab.dev_attr.attr,
+	&sensor_dev_attr_gpio10_out_enab.dev_attr.attr,
+	&sensor_dev_attr_gpio10_out_val.dev_attr.attr,
+	&sensor_dev_attr_gpio10_stat.dev_attr.attr,
+	&sensor_dev_attr_gpio11_enab.dev_attr.attr,
+	&sensor_dev_attr_gpio11_out_enab.dev_attr.attr,
+	&sensor_dev_attr_gpio11_out_val.dev_attr.attr,
+	&sensor_dev_attr_gpio11_stat.dev_attr.attr,
+	&sensor_dev_attr_gpio14_enab.dev_attr.attr,
+	&sensor_dev_attr_gpio14_out_enab.dev_attr.attr,
+	&sensor_dev_attr_gpio14_out_val.dev_attr.attr,
+	&sensor_dev_attr_gpio14_stat.dev_attr.attr,
+	&sensor_dev_attr_gpio17_enab.dev_attr.attr,
+	&sensor_dev_attr_gpio17_out_enab.dev_attr.attr,
+	&sensor_dev_attr_gpio17_out_val.dev_attr.attr,
+	&sensor_dev_attr_gpio17_stat.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group ucd9000_attr_group = {
+	.attrs = ucd9000_attributes,
+};
+
 static int ucd9000_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {

@@ -227,7 +458,40 @@ static int ucd9000_probe(struct i2c_client *client,
 		  | PMBUS_HAVE_FAN34 | PMBUS_HAVE_STATUS_FAN34;
 	}
 
-	return pmbus_do_probe(client, mid, info);
+	info->debugfs = true;
+
+	data->gpio.label = "gpio-ucd9000";
+	data->gpio.get = ucd9000_gpio_get;
+	data->gpio.set = ucd9000_gpio_set;
+	data->gpio.can_sleep = 1;
+	data->gpio.base = -1;
+	if (mid->driver_data == ucd90160)
+		data->gpio.ngpio = UCD90160_NUM_GPIOS;
+	data->gpio.parent = &client->dev;
+	data->gpio.owner = THIS_MODULE;
+
+	ret = devm_gpiochip_add_data(&client->dev, &data->gpio, client);
+	if (ret) {
+		/* Use data->gpio.dev as a flag for freeing gpiochip */
+		data->gpio.parent = NULL;
+		dev_warn(&client->dev, "Count not add gpiochip\n");
+		return ret;
+	}
+
+	dev_info(&client->dev, "gpios %i...%i\n", data->gpio.base,
+		data->gpio.base + data->gpio.ngpio - 1);
+
+	ret = sysfs_create_group(&client->dev.kobj, &ucd9000_attr_group);
+	if (ret < 0)
+		return ret;
+
+	return  pmbus_do_probe(client, mid, info);
+}
+
+static int ucd9000_remove(struct i2c_client *client)
+{
+	sysfs_remove_group(&client->dev.kobj, &ucd9000_attr_group);
+	return pmbus_do_remove(client);
 }
 
 /* This is the driver that will be inserted */

@@ -236,7 +500,7 @@ static int ucd9000_probe(struct i2c_client *client,
 		.name = "ucd9000",
 	},
 	.probe = ucd9000_probe,
-	.remove = pmbus_do_remove,
+	.remove = ucd9000_remove,
 	.id_table = ucd9000_id,
 };
Signature · 1 line
1.8.2.2
#1 lore raw

Hi Chris,

Comments inline based on some skimming of the datasheet and command reference.

Quoted 63 lines · Christopher Bostic

Create sysfs files for mfr specific fault indicators. Add sysfs files for GPIO configuration access. Link driver into the gpio device driver.

Quoted 1 line
Signed-off-by Christopher Bostic <[email protected]>

---
v2 - Removed clear_faults file - redundant since all other sysfs core accesses result in an automatic clear fault.
   - Removed local status_word and status_vout register dumps and use the new pmbus core status facilities instead.
   - Rename gpi<x>_fault to gpi<x>_alarm to better match core naming conventions.
   - Add full register dump for mfr_status.
   - Add gpio chip to device structure and use gpio interfaces.
---
 drivers/hwmon/pmbus/ucd9000.c | 268 +++++++++++++++++++++++++++++++++++++++++-

 1 file changed, 266 insertions(+), 2 deletions(-)

drivers/hwmon/pmbus/ucd9000.c +3 -0
index 3e3aa95..ecd3e1a 100644
@@ -26,6 +26,9 @@
  #include <linux/slab.h>
  #include <linux/i2c.h>
  #include <linux/i2c/pmbus.h>
+#include <linux/sysfs.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/gpio.h>
  #include "pmbus.h"


 enum chips { ucd9000, ucd90120, ucd90124, ucd90160, ucd9090, ucd90910 };

drivers/hwmon/pmbus/ucd9000.c +0 -0
@@ -34,6 +37,10 @@
Quoted 8 lines

 #define UCD9000_NUM_PAGES 0xd6
 #define UCD9000_FAN_CONFIG_INDEX 0xe7
 #define UCD9000_FAN_CONFIG 0xe8

drivers/hwmon/pmbus/ucd9000.c +4 -0
+#define UCD9000_LOGGED_FAULTS		0xea
+#define UCD9000_MFR_STATUS		0xf3
+#define UCD9000_GPIO_SELECT		0xfa
+#define UCD9000_GPIO_CONFIG		0xfb

 #define UCD9000_DEVICE_ID 0xfd

Quoted 1 line

 #define UCD9000_MON_TYPE(x) (((x) >> 5) & 0x07)

drivers/hwmon/pmbus/ucd9000.c +0 -0
@@ -46,9 +53,14 @@

Quoted 1 line

 #define UCD9000_NUM_FAN 4

Quoted 1 line

+#define UCD9000_FULL_REG BIT(8)

+

Quoted 1 line

+#define UCD90160_NUM_GPIOS 26

+
 struct ucd9000_data {

drivers/hwmon/pmbus/ucd9000.c line 63

Quoted 3 lines

  u8 fan_data[UCD9000_NUM_FAN][I2C_SMBUS_BLOCK_MAX]; struct pmbus_driver_info info;

drivers/hwmon/pmbus/ucd9000.c +1 -0
+	struct gpio_chip gpio;

 };
 #define to_ucd9000_data(_info) container_of(_info, struct ucd9000_data, info)

drivers/hwmon/pmbus/ucd9000.c +0 -0
@@ -119,6 +131,225 @@ static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg)
  };
  MODULE_DEVICE_TABLE(i2c, ucd9000_id);


+static void ucd9000_gpio_set(struct gpio_chip *gc, unsigned int offset,

Quoted 6 lines

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 135

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			int value)

+{

Review

Quoted 2 lines
(quoted diff) +2 -0
+	struct i2c_client *client  = gpiochip_get_data(gc);
+	int ret;

+

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 140

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+	ret = i2c_smbus_write_byte_data(client, offset, value);

Why are we using i2c_smbus_write_byte_data()? Why aren't we using the pmbus_*() helpers? Is it because we don't have to set the page? Might be worth a comment if so.

Regardless this doesn't seem right - offset is in the GPIO number space relative to the base for the GPIO chip. i2c_smbus_write_byte_data() uses the second argument as the command - so we'll be writing 'value' to registers we don't intend?

Shouldn't this be writing GPIO_SELECT and then GPIO_CONFIG? Also as noted in the datasheet don't we need a double-write of GPIO_CONFIG to avoid storing enabled output state on STORE_DEFAULT_ALL?

Quoted 9 lines
Quoted 2 lines
drivers/hwmon/pmbus/ucd9000.c +2 -0
+	if (ret)
+		dev_err(&client->dev, "Failed to set gpio select %d\n", value);
drivers/hwmon/pmbus/ucd9000.c +4 -0
+}
+
+static int ucd9000_gpio_get(struct gpio_chip *gc, unsigned int offset)
+{

Review

Quoted 1 line
(quoted diff) +1 -0
+	struct i2c_client *client  = gpiochip_get_data(gc);

+

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 149

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+	return i2c_smbus_read_byte_data(client, offset);

Similar comments here as for the _set() case.

Quoted 102 lines
drivers/hwmon/pmbus/ucd9000.c +3 -0
+}
+
+static ssize_t ucd9000_clear_logged_faults(struct device *dev,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 154

Quoted 2 lines
drivers/hwmon/pmbus/ucd9000.c +2 -0
+				struct device_attribute *attr, const char *buf,
+				size_t count)

+{

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 157

Quoted 2 lines
drivers/hwmon/pmbus/ucd9000.c +2 -0
+	struct i2c_client *client = to_i2c_client(dev);
+	int ret;

+

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 163

Quoted 5 lines
drivers/hwmon/pmbus/ucd9000.c +5 -0
+	ret = i2c_smbus_write_byte_data(client, UCD9000_LOGGED_FAULTS, 0);
+	if (ret) {
+		dev_err(&client->dev, "Failed to clear logged faults\n");
+		return ret;
+	}

+

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+	return count;
drivers/hwmon/pmbus/ucd9000.c +3 -0
+}
+
+static ssize_t ucd9000_mfr_status(struct device *dev,

Review

Quoted 2 lines
(quoted diff) +2 -0
+				struct device_attribute *attr,
+				char *buf)

+{

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 176

Quoted 5 lines
drivers/hwmon/pmbus/ucd9000.c +5 -0
+	struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
+	bool full_reg = !!(sensor_attr->index & UCD9000_FULL_REG);
+	struct i2c_client *client = to_i2c_client(dev);
+	int nr = sensor_attr->index, ret;
+	u32 block_buffer;

+

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 182

Quoted 5 lines
drivers/hwmon/pmbus/ucd9000.c +5 -0
+	ret = pmbus_set_page(client, 0);
+	if (ret < 0) {
+		dev_err(&client->dev, "i2c_smbus_write failed. rc:%d\n", ret);
+		return ret;
+	}

+

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 190

Quoted 7 lines
drivers/hwmon/pmbus/ucd9000.c +7 -0
+	ret = i2c_smbus_read_block_data(client, UCD9000_MFR_STATUS,
+					(u8 *)&block_buffer);
+	if (ret < 0) {
+		dev_err(&client->dev,
+			"Failed to read mfr status. rc:%d\n", ret);
+		return ret;
+	}

+

Quoted 4 lines
drivers/hwmon/pmbus/ucd9000.c +4 -0
+	if (full_reg)
+		return sprintf(buf, "%08x\n", ret);
+	else
+		return sprintf(buf, "%1d\n", ret & BIT(nr) ? 1 : 0);
drivers/hwmon/pmbus/ucd9000.c +3 -0
+}
+
+static ssize_t ucd9000_gpi_config(struct device *dev,

Review

Quoted 2 lines
(quoted diff) +2 -0
+				struct device_attribute *attr,
+				char *buf)

+{

Quoted 6 lines
drivers/hwmon/pmbus/ucd9000.c +6 -0
+	struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
+	struct i2c_client *client = to_i2c_client(dev);
+	struct ucd9000_data *data
+		= to_ucd9000_data(pmbus_get_driver_info(client));
+	int nr = (sensor_attr->index & 0xf00) >> 8;
+	int reg = sensor_attr->index & 0xff, ret;
drivers/hwmon/pmbus/ucd9000.c +2 -0
+
+

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 215

Quoted 6 lines
drivers/hwmon/pmbus/ucd9000.c +6 -0
+	ucd9000_gpio_set(&data->gpio, UCD9000_GPIO_SELECT, reg);
+	ret = ucd9000_gpio_get(&data->gpio, UCD9000_GPIO_CONFIG);
+	if (ret < 0) {
+		dev_err(&client->dev,
+			"Failed to read gpi%02d config\n", reg);
+	}

+

Quoted 1 line
(quoted diff) +1 -0
+	return sprintf(buf, "%1d\n", ret & BIT(nr) ? 1 : 0);
drivers/hwmon/pmbus/ucd9000.c +3 -0
+}
+
+static DEVICE_ATTR(clear_logged_faults, 0200,
Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			  NULL, ucd9000_clear_logged_faults);
drivers/hwmon/pmbus/ucd9000.c +11 -0
+
+/* MFR status */
+static SENSOR_DEVICE_ATTR(gpi8_alarm, 0444, ucd9000_mfr_status, NULL, 23);
+static SENSOR_DEVICE_ATTR(gpi7_alarm, 0444, ucd9000_mfr_status, NULL, 22);
+static SENSOR_DEVICE_ATTR(gpi6_alarm, 0444, ucd9000_mfr_status, NULL, 21);
+static SENSOR_DEVICE_ATTR(gpi5_alarm, 0444, ucd9000_mfr_status, NULL, 20);
+static SENSOR_DEVICE_ATTR(gpi4_alarm, 0444, ucd9000_mfr_status, NULL, 19);
+static SENSOR_DEVICE_ATTR(gpi3_alarm, 0444, ucd9000_mfr_status, NULL, 18);
+static SENSOR_DEVICE_ATTR(gpi2_alarm, 0444, ucd9000_mfr_status, NULL, 17);
+static SENSOR_DEVICE_ATTR(gpi1_alarm, 0444, ucd9000_mfr_status, NULL, 16);
+static SENSOR_DEVICE_ATTR(new_logged_fault_detail, 0444,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 233

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			ucd9000_mfr_status, NULL, 12);

+static SENSOR_DEVICE_ATTR(system_watchdog_timeout, 0444,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 235

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			ucd9000_mfr_status, NULL, 11);

+static SENSOR_DEVICE_ATTR(store_default_all_error, 0444,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 237

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			ucd9000_mfr_status, NULL, 10);

+static SENSOR_DEVICE_ATTR(store_default_all_done, 0444,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 239

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			ucd9000_mfr_status, NULL, 9);

+static SENSOR_DEVICE_ATTR(watchdog_timeout, 0444,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 241

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			ucd9000_mfr_status, NULL, 8);

+static SENSOR_DEVICE_ATTR(invalid_logs, 0444,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 243

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			ucd9000_mfr_status, NULL, 7);

+static SENSOR_DEVICE_ATTR(logged_fault_detail, 0444,

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			ucd9000_mfr_status, NULL, 6);
drivers/hwmon/pmbus/ucd9000.c +6 -0
+static SENSOR_DEVICE_ATTR(resequence_error, 0444, ucd9000_mfr_status, NULL, 5);
+static SENSOR_DEVICE_ATTR(pkgid_mismatch, 0444, ucd9000_mfr_status, NULL, 4);
+static SENSOR_DEVICE_ATTR(hardcoded_parms, 0444, ucd9000_mfr_status, NULL, 3);
+static SENSOR_DEVICE_ATTR(seq_off_timeout, 0444, ucd9000_mfr_status, NULL, 2);
+static SENSOR_DEVICE_ATTR(seq_on_timeout, 0444, ucd9000_mfr_status, NULL, 1);
+static SENSOR_DEVICE_ATTR(mfr_status, 0444,
Quoted 1 line

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 252

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			ucd9000_mfr_status, NULL, UCD9000_FULL_REG);

Are these for debug purposes? I can imagine the *_alarm attributes would be useful in normal circumstances, but are the others?

Quoted 5 lines
drivers/hwmon/pmbus/ucd9000.c +4 -0
+
+/* GPIO config */
+static SENSOR_DEVICE_ATTR(gpio8_enab, 0444, ucd9000_gpi_config, NULL, 8);
+static SENSOR_DEVICE_ATTR(gpio8_out_enab, 0444, ucd9000_gpi_config,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 257

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			NULL, 0x0108);

I would think these should be eliminated by using the gpiochip. If they're not, can you please explain why?

Quoted 102 lines

+static SENSOR_DEVICE_ATTR(gpio8_out_val, 0444, ucd9000_gpi_config,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 259

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			NULL, 0x0208);

+static SENSOR_DEVICE_ATTR(gpio8_stat, 0444, ucd9000_gpi_config,

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			NULL, 0x0308);
drivers/hwmon/pmbus/ucd9000.c +2 -0
+static SENSOR_DEVICE_ATTR(gpio9_enab, 0444, ucd9000_gpi_config, NULL, 9);
+static SENSOR_DEVICE_ATTR(gpio9_out_enab, 0444, ucd9000_gpi_config,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 264

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			NULL, 0x0109);

+static SENSOR_DEVICE_ATTR(gpio9_out_val, 0444, ucd9000_gpi_config,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 266

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			NULL, 0x0209);

+static SENSOR_DEVICE_ATTR(gpio9_stat, 0444, ucd9000_gpi_config,

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			NULL, 0x0309);
drivers/hwmon/pmbus/ucd9000.c +2 -0
+static SENSOR_DEVICE_ATTR(gpio10_enab, 0444, ucd9000_gpi_config, NULL, 10);
+static SENSOR_DEVICE_ATTR(gpio10_out_enab, 0444, ucd9000_gpi_config,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 271

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			NULL, 0x010a);

+static SENSOR_DEVICE_ATTR(gpio10_out_val, 0444, ucd9000_gpi_config,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 273

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			NULL, 0x020a);

+static SENSOR_DEVICE_ATTR(gpio10_stat, 0444, ucd9000_gpi_config,

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			NULL, 0x030a);
drivers/hwmon/pmbus/ucd9000.c +2 -0
+static SENSOR_DEVICE_ATTR(gpio11_enab, 0444, ucd9000_gpi_config, NULL, 11);
+static SENSOR_DEVICE_ATTR(gpio11_out_enab, 0444, ucd9000_gpi_config,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 278

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			NULL, 0x010b);

+static SENSOR_DEVICE_ATTR(gpio11_out_val, 0444, ucd9000_gpi_config,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 280

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			NULL, 0x020b);

+static SENSOR_DEVICE_ATTR(gpio11_stat, 0444, ucd9000_gpi_config,

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			NULL, 0x030b);
drivers/hwmon/pmbus/ucd9000.c +2 -0
+static SENSOR_DEVICE_ATTR(gpio14_enab, 0444, ucd9000_gpi_config, NULL, 14);
+static SENSOR_DEVICE_ATTR(gpio14_out_enab, 0444, ucd9000_gpi_config,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 285

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			NULL, 0x010e);

+static SENSOR_DEVICE_ATTR(gpio14_out_val, 0444, ucd9000_gpi_config,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 287

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			NULL, 0x020e);

+static SENSOR_DEVICE_ATTR(gpio14_stat, 0444, ucd9000_gpi_config,

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			NULL, 0x030e);
drivers/hwmon/pmbus/ucd9000.c +2 -0
+static SENSOR_DEVICE_ATTR(gpio17_enab, 0444, ucd9000_gpi_config, NULL, 17);
+static SENSOR_DEVICE_ATTR(gpio17_out_enab, 0444, ucd9000_gpi_config,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 292

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			NULL, 0x0111);

+static SENSOR_DEVICE_ATTR(gpio17_out_val, 0444, ucd9000_gpi_config,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 294

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			NULL, 0x0211);

+static SENSOR_DEVICE_ATTR(gpio17_stat, 0444, ucd9000_gpi_config,

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			NULL, 0x0311);
drivers/hwmon/pmbus/ucd9000.c +3 -0
+
+
+static struct attribute *ucd9000_attributes[] = {
Quoted 47 lines
drivers/hwmon/pmbus/ucd9000.c +47 -0
+	&dev_attr_clear_logged_faults.attr,
+	&sensor_dev_attr_gpi8_alarm.dev_attr.attr,
+	&sensor_dev_attr_gpi7_alarm.dev_attr.attr,
+	&sensor_dev_attr_gpi6_alarm.dev_attr.attr,
+	&sensor_dev_attr_gpi5_alarm.dev_attr.attr,
+	&sensor_dev_attr_gpi4_alarm.dev_attr.attr,
+	&sensor_dev_attr_gpi3_alarm.dev_attr.attr,
+	&sensor_dev_attr_gpi2_alarm.dev_attr.attr,
+	&sensor_dev_attr_gpi1_alarm.dev_attr.attr,
+	&sensor_dev_attr_new_logged_fault_detail.dev_attr.attr,
+	&sensor_dev_attr_system_watchdog_timeout.dev_attr.attr,
+	&sensor_dev_attr_store_default_all_error.dev_attr.attr,
+	&sensor_dev_attr_store_default_all_done.dev_attr.attr,
+	&sensor_dev_attr_watchdog_timeout.dev_attr.attr,
+	&sensor_dev_attr_invalid_logs.dev_attr.attr,
+	&sensor_dev_attr_logged_fault_detail.dev_attr.attr,
+	&sensor_dev_attr_resequence_error.dev_attr.attr,
+	&sensor_dev_attr_pkgid_mismatch.dev_attr.attr,
+	&sensor_dev_attr_hardcoded_parms.dev_attr.attr,
+	&sensor_dev_attr_seq_off_timeout.dev_attr.attr,
+	&sensor_dev_attr_seq_on_timeout.dev_attr.attr,
+	&sensor_dev_attr_mfr_status.dev_attr.attr,
+	&sensor_dev_attr_gpio8_enab.dev_attr.attr,
+	&sensor_dev_attr_gpio8_out_enab.dev_attr.attr,
+	&sensor_dev_attr_gpio8_out_val.dev_attr.attr,
+	&sensor_dev_attr_gpio8_stat.dev_attr.attr,
+	&sensor_dev_attr_gpio9_enab.dev_attr.attr,
+	&sensor_dev_attr_gpio9_out_enab.dev_attr.attr,
+	&sensor_dev_attr_gpio9_out_val.dev_attr.attr,
+	&sensor_dev_attr_gpio9_stat.dev_attr.attr,
+	&sensor_dev_attr_gpio10_enab.dev_attr.attr,
+	&sensor_dev_attr_gpio10_out_enab.dev_attr.attr,
+	&sensor_dev_attr_gpio10_out_val.dev_attr.attr,
+	&sensor_dev_attr_gpio10_stat.dev_attr.attr,
+	&sensor_dev_attr_gpio11_enab.dev_attr.attr,
+	&sensor_dev_attr_gpio11_out_enab.dev_attr.attr,
+	&sensor_dev_attr_gpio11_out_val.dev_attr.attr,
+	&sensor_dev_attr_gpio11_stat.dev_attr.attr,
+	&sensor_dev_attr_gpio14_enab.dev_attr.attr,
+	&sensor_dev_attr_gpio14_out_enab.dev_attr.attr,
+	&sensor_dev_attr_gpio14_out_val.dev_attr.attr,
+	&sensor_dev_attr_gpio14_stat.dev_attr.attr,
+	&sensor_dev_attr_gpio17_enab.dev_attr.attr,
+	&sensor_dev_attr_gpio17_out_enab.dev_attr.attr,
+	&sensor_dev_attr_gpio17_out_val.dev_attr.attr,
+	&sensor_dev_attr_gpio17_stat.dev_attr.attr,
+	NULL
drivers/hwmon/pmbus/ucd9000.c +3 -0
+};
+
+static const struct attribute_group ucd9000_attr_group = {
Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+	.attrs = ucd9000_attributes,
drivers/hwmon/pmbus/ucd9000.c +2 -0
+};
+

 static int ucd9000_probe(struct i2c_client *client,

Quoted 1 line

   const struct i2c_device_id *id)

 {

drivers/hwmon/pmbus/ucd9000.c +0 -0
@@ -227,7 +458,40 @@ static int ucd9000_probe(struct i2c_client *client,
 >  		  | PMBUS_HAVE_FAN34 | PMBUS_HAVE_STATUS_FAN34;
 >  	}

Quoted 2 lines

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_probe(struct i2c_client *client, · line 461

Quoted 2 lines
drivers/hwmon/pmbus/ucd9000.c +1 -1
-	return pmbus_do_probe(client, mid, info);
+	info->debugfs = true;

This causes a compilation failure:

  CC      drivers/hwmon/pmbus/ucd9000.o
drivers/hwmon/pmbus/ucd9000.c: In function ‘ucd9000_probe’:
drivers/hwmon/pmbus/ucd9000.c:461:6: error: ‘struct pmbus_driver_info’ has no member named ‘debugfs’ info->debugfs = true;
      ^~
scripts/Makefile.build:294: recipe for target 'drivers/hwmon/pmbus/ucd9000.o' failed

From the pmbus/debugfs implementation applied to the tree it looks like we can just drop this.

Quoted 11 lines

+

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_probe(struct i2c_client *client, · line 471

Quoted 9 lines
drivers/hwmon/pmbus/ucd9000.c +9 -0
+	data->gpio.label = "gpio-ucd9000";
+	data->gpio.get = ucd9000_gpio_get;
+	data->gpio.set = ucd9000_gpio_set;
+	data->gpio.can_sleep = 1;
+	data->gpio.base = -1;
+	if (mid->driver_data == ucd90160)
+		data->gpio.ngpio = UCD90160_NUM_GPIOS;
+	data->gpio.parent = &client->dev;
+	data->gpio.owner = THIS_MODULE;

+

Quoted 3 lines

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_probe(struct i2c_client *client, · line 475

Quoted 3 lines
drivers/hwmon/pmbus/ucd9000.c +3 -0
+	ret = devm_gpiochip_add_data(&client->dev, &data->gpio, client);
+	if (ret) {
+		/* Use data->gpio.dev as a flag for freeing gpiochip */

What is this comment about?

Quoted 30 lines

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_probe(struct i2c_client *client, · line 479

Quoted 4 lines
drivers/hwmon/pmbus/ucd9000.c +4 -0
+		data->gpio.parent = NULL;
+		dev_warn(&client->dev, "Count not add gpiochip\n");
+		return ret;
+	}

+

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_probe(struct i2c_client *client, · line 482

Quoted 2 lines
drivers/hwmon/pmbus/ucd9000.c +2 -0
+	dev_info(&client->dev, "gpios %i...%i\n", data->gpio.base,
+		data->gpio.base + data->gpio.ngpio - 1);

+

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_probe(struct i2c_client *client, · line 486

Quoted 3 lines
drivers/hwmon/pmbus/ucd9000.c +3 -0
+	ret = sysfs_create_group(&client->dev.kobj, &ucd9000_attr_group);
+	if (ret < 0)
+		return ret;

+

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+	return  pmbus_do_probe(client, mid, info);
drivers/hwmon/pmbus/ucd9000.c +4 -0
+}
+
+static int ucd9000_remove(struct i2c_client *client)
+{

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_probe(struct i2c_client *client, · line 494

Quoted 2 lines
drivers/hwmon/pmbus/ucd9000.c +2 -0
+	sysfs_remove_group(&client->dev.kobj, &ucd9000_attr_group);
+	return pmbus_do_remove(client);

 }

 /* This is the driver that will be inserted */

drivers/hwmon/pmbus/ucd9000.c +0 -0
@@ -236,7 +500,7 @@ static int ucd9000_probe(struct i2c_client *client,
 >  		.name = "ucd9000",
 >  	},
 >  	.probe = ucd9000_probe,
 > -	.remove = pmbus_do_remove,
 > +	.remove = ucd9000_remove,
 >  	.id_table = ucd9000_id,
  };
Quoted 1 line

Cheers,

Andrew

author
#2 lore raw
Quoted 71 lines · Andrew Jeffery

Hi Chris,

Comments inline based on some skimming of the datasheet and command reference.

Quoted 65 lines · Christopher Bostic

Create sysfs files for mfr specific fault indicators. Add sysfs files for GPIO configuration access. Link driver into the gpio device driver.

Quoted 1 line
Signed-off-by Christopher Bostic <[email protected]>

---
v2 - Removed clear_faults file - redundant since all other sysfs
core accesses result in an automatic clear fault.
- Removed local status_word and status_vout register dumps and use the new pmbus core status facilities instead.
- Rename gpi<x>_fault to gpi<x>_alarm to better match core naming conventions.
- Add full register dump for mfr_status.
- Add gpio chip to device structure and use gpio interfaces.

---
  drivers/hwmon/pmbus/ucd9000.c | 268 +++++++++++++++++++++++++++++++++++++++++-
  1 file changed, 266 insertions(+), 2 deletions(-)
drivers/hwmon/pmbus/ucd9000.c +3 -0
index 3e3aa95..ecd3e1a 100644
@@ -26,6 +26,9 @@
  #include <linux/slab.h>
  #include <linux/i2c.h>
  #include <linux/i2c/pmbus.h>
+#include <linux/sysfs.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/gpio.h>
  #include "pmbus.h"
 
  enum chips { ucd9000, ucd90120, ucd90124, ucd90160, ucd9090, ucd90910 };

@@ -34,6 +37,10 @@
Quoted 10 lines
  #define UCD9000_NUM_PAGES		0xd6
  #define UCD9000_FAN_CONFIG_INDEX	0xe7
  #define UCD9000_FAN_CONFIG		0xe8
drivers/hwmon/pmbus/ucd9000.c +4 -0
+#define UCD9000_LOGGED_FAULTS		0xea
+#define UCD9000_MFR_STATUS		0xf3
+#define UCD9000_GPIO_SELECT		0xfa
+#define UCD9000_GPIO_CONFIG		0xfb
  #define UCD9000_DEVICE_ID		0xfd
 
  #define UCD9000_MON_TYPE(x)	(((x) >> 5) & 0x07)
drivers/hwmon/pmbus/ucd9000.c +0 -0
@@ -46,9 +53,14 @@
Quoted 3 lines
  #define UCD9000_NUM_FAN		4

+#define UCD9000_FULL_REG BIT(8)

+

Quoted 1 line

+#define UCD90160_NUM_GPIOS 26

+
struct ucd9000_data {

drivers/hwmon/pmbus/ucd9000.c line 63

Quoted 3 lines
  	u8 fan_data[UCD9000_NUM_FAN][I2C_SMBUS_BLOCK_MAX];
  	struct pmbus_driver_info info;
drivers/hwmon/pmbus/ucd9000.c +1 -0
+	struct gpio_chip gpio;

};
#define to_ucd9000_data(_info) container_of(_info, struct ucd9000_data, info)

drivers/hwmon/pmbus/ucd9000.c +2 -0
@@ -119,6 +131,225 @@ static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg)
  };
  MODULE_DEVICE_TABLE(i2c, ucd9000_id);
 
+static void ucd9000_gpio_set(struct gpio_chip *gc, unsigned int offset,
 > +			int value)
+{
Quoted 7 lines
Quoted 4 lines
drivers/hwmon/pmbus/ucd9000.c +1 -0
 > +	struct i2c_client *client  = gpiochip_get_data(gc);
 > +	int ret;
+

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 140

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+	ret = i2c_smbus_write_byte_data(client, offset, value);

Why are we using i2c_smbus_write_byte_data()? Why aren't we using the pmbus_*() helpers? Is it because we don't have to set the page? Might be worth a comment if so.

Hi Andrew,

Yes because we don't need don't need to set the page. Will add a comment.

Quoted 5 lines

Regardless this doesn't seem right - offset is in the GPIO number space relative to the base for the GPIO chip. i2c_smbus_write_byte_data() uses the second argument as the command - so we'll be writing 'value' to registers we don't intend?

Using the get/set methods of struct gpio_chip in this way was wrong on my part. I wasn't considering as you point out that the number space is relative to the base.

Quoted 1 line

Shouldn't this be writing GPIO_SELECT and then GPIO_CONFIG?

That was the intent. As written it wasn't clear. Will correct.

Quoted 3 lines

Also as noted in
the datasheet don't we need a double-write of GPIO_CONFIG to avoid storing enabled output state on STORE_DEFAULT_ALL?

The need is to be able to read out the gpio config for various pin ID's. If I understand the spec then I would need to:

1.  Write (FBh) GPIO_CONFIG to enable
2.  Read (FBh) GPIO_CONFIG to retrieve the pin ID info
3.  Write (FBh) GPIO_CONFIG to disable.

Would you agree?

Quoted 10 lines
Quoted 9 lines
Quoted 2 lines
drivers/hwmon/pmbus/ucd9000.c +2 -0
+	if (ret)
+		dev_err(&client->dev, "Failed to set gpio select %d\n", value);
drivers/hwmon/pmbus/ucd9000.c +4 -0
+}
+
+static int ucd9000_gpio_get(struct gpio_chip *gc, unsigned int offset)
+{

Review

Quoted 1 line
(quoted diff) +1 -0
+	struct i2c_client *client  = gpiochip_get_data(gc);

+

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 149

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+	return i2c_smbus_read_byte_data(client, offset);

Similar comments here as for the _set() case.

Noted.

Quoted 102 lines
Quoted 102 lines
drivers/hwmon/pmbus/ucd9000.c +3 -0
+}
+
+static ssize_t ucd9000_clear_logged_faults(struct device *dev,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 154

Quoted 2 lines
drivers/hwmon/pmbus/ucd9000.c +2 -0
+				struct device_attribute *attr, const char *buf,
+				size_t count)

+{

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 157

Quoted 2 lines
drivers/hwmon/pmbus/ucd9000.c +2 -0
+	struct i2c_client *client = to_i2c_client(dev);
+	int ret;

+

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 163

Quoted 5 lines
drivers/hwmon/pmbus/ucd9000.c +5 -0
+	ret = i2c_smbus_write_byte_data(client, UCD9000_LOGGED_FAULTS, 0);
+	if (ret) {
+		dev_err(&client->dev, "Failed to clear logged faults\n");
+		return ret;
+	}

+

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+	return count;
drivers/hwmon/pmbus/ucd9000.c +3 -0
+}
+
+static ssize_t ucd9000_mfr_status(struct device *dev,

Review

Quoted 2 lines
(quoted diff) +2 -0
+				struct device_attribute *attr,
+				char *buf)

+{

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 176

Quoted 5 lines
drivers/hwmon/pmbus/ucd9000.c +5 -0
+	struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
+	bool full_reg = !!(sensor_attr->index & UCD9000_FULL_REG);
+	struct i2c_client *client = to_i2c_client(dev);
+	int nr = sensor_attr->index, ret;
+	u32 block_buffer;

+

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 182

Quoted 5 lines
drivers/hwmon/pmbus/ucd9000.c +5 -0
+	ret = pmbus_set_page(client, 0);
+	if (ret < 0) {
+		dev_err(&client->dev, "i2c_smbus_write failed. rc:%d\n", ret);
+		return ret;
+	}

+

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 190

Quoted 7 lines
drivers/hwmon/pmbus/ucd9000.c +7 -0
+	ret = i2c_smbus_read_block_data(client, UCD9000_MFR_STATUS,
+					(u8 *)&block_buffer);
+	if (ret < 0) {
+		dev_err(&client->dev,
+			"Failed to read mfr status. rc:%d\n", ret);
+		return ret;
+	}

+

Quoted 4 lines
drivers/hwmon/pmbus/ucd9000.c +4 -0
+	if (full_reg)
+		return sprintf(buf, "%08x\n", ret);
+	else
+		return sprintf(buf, "%1d\n", ret & BIT(nr) ? 1 : 0);
drivers/hwmon/pmbus/ucd9000.c +3 -0
+}
+
+static ssize_t ucd9000_gpi_config(struct device *dev,

Review

Quoted 2 lines
(quoted diff) +2 -0
+				struct device_attribute *attr,
+				char *buf)

+{

Quoted 6 lines
drivers/hwmon/pmbus/ucd9000.c +6 -0
+	struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
+	struct i2c_client *client = to_i2c_client(dev);
+	struct ucd9000_data *data
+		= to_ucd9000_data(pmbus_get_driver_info(client));
+	int nr = (sensor_attr->index & 0xf00) >> 8;
+	int reg = sensor_attr->index & 0xff, ret;
drivers/hwmon/pmbus/ucd9000.c +2 -0
+
+

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 215

Quoted 6 lines
drivers/hwmon/pmbus/ucd9000.c +6 -0
+	ucd9000_gpio_set(&data->gpio, UCD9000_GPIO_SELECT, reg);
+	ret = ucd9000_gpio_get(&data->gpio, UCD9000_GPIO_CONFIG);
+	if (ret < 0) {
+		dev_err(&client->dev,
+			"Failed to read gpi%02d config\n", reg);
+	}

+

Quoted 1 line
(quoted diff) +1 -0
+	return sprintf(buf, "%1d\n", ret & BIT(nr) ? 1 : 0);
drivers/hwmon/pmbus/ucd9000.c +3 -0
+}
+
+static DEVICE_ATTR(clear_logged_faults, 0200,
Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			  NULL, ucd9000_clear_logged_faults);
drivers/hwmon/pmbus/ucd9000.c +11 -0
+
+/* MFR status */
+static SENSOR_DEVICE_ATTR(gpi8_alarm, 0444, ucd9000_mfr_status, NULL, 23);
+static SENSOR_DEVICE_ATTR(gpi7_alarm, 0444, ucd9000_mfr_status, NULL, 22);
+static SENSOR_DEVICE_ATTR(gpi6_alarm, 0444, ucd9000_mfr_status, NULL, 21);
+static SENSOR_DEVICE_ATTR(gpi5_alarm, 0444, ucd9000_mfr_status, NULL, 20);
+static SENSOR_DEVICE_ATTR(gpi4_alarm, 0444, ucd9000_mfr_status, NULL, 19);
+static SENSOR_DEVICE_ATTR(gpi3_alarm, 0444, ucd9000_mfr_status, NULL, 18);
+static SENSOR_DEVICE_ATTR(gpi2_alarm, 0444, ucd9000_mfr_status, NULL, 17);
+static SENSOR_DEVICE_ATTR(gpi1_alarm, 0444, ucd9000_mfr_status, NULL, 16);
+static SENSOR_DEVICE_ATTR(new_logged_fault_detail, 0444,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 233

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			ucd9000_mfr_status, NULL, 12);

+static SENSOR_DEVICE_ATTR(system_watchdog_timeout, 0444,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 235

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			ucd9000_mfr_status, NULL, 11);

+static SENSOR_DEVICE_ATTR(store_default_all_error, 0444,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 237

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			ucd9000_mfr_status, NULL, 10);

+static SENSOR_DEVICE_ATTR(store_default_all_done, 0444,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 239

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			ucd9000_mfr_status, NULL, 9);

+static SENSOR_DEVICE_ATTR(watchdog_timeout, 0444,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 241

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			ucd9000_mfr_status, NULL, 8);

+static SENSOR_DEVICE_ATTR(invalid_logs, 0444,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 243

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			ucd9000_mfr_status, NULL, 7);

+static SENSOR_DEVICE_ATTR(logged_fault_detail, 0444,

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			ucd9000_mfr_status, NULL, 6);
drivers/hwmon/pmbus/ucd9000.c +6 -0
+static SENSOR_DEVICE_ATTR(resequence_error, 0444, ucd9000_mfr_status, NULL, 5);
+static SENSOR_DEVICE_ATTR(pkgid_mismatch, 0444, ucd9000_mfr_status, NULL, 4);
+static SENSOR_DEVICE_ATTR(hardcoded_parms, 0444, ucd9000_mfr_status, NULL, 3);
+static SENSOR_DEVICE_ATTR(seq_off_timeout, 0444, ucd9000_mfr_status, NULL, 2);
+static SENSOR_DEVICE_ATTR(seq_on_timeout, 0444, ucd9000_mfr_status, NULL, 1);
+static SENSOR_DEVICE_ATTR(mfr_status, 0444,
Quoted 3 lines
Quoted 1 line

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 252

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			ucd9000_mfr_status, NULL, UCD9000_FULL_REG);

Are these for debug purposes? I can imagine the *_alarm attributes would be useful in normal circumstances, but are the others?

This is for general status gathering of the ucd90160.. Discussed with the application user and they're ok with removing these so I will take them out.

Quoted 7 lines
Quoted 5 lines
drivers/hwmon/pmbus/ucd9000.c +4 -0
+
+/* GPIO config */
+static SENSOR_DEVICE_ATTR(gpio8_enab, 0444, ucd9000_gpi_config, NULL, 8);
+static SENSOR_DEVICE_ATTR(gpio8_out_enab, 0444, ucd9000_gpi_config,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 257

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			NULL, 0x0108);

I would think these should be eliminated by using the gpiochip. If they're not, can you please explain why?

Are you referring to having user space code use the /sys/class/gpio/gpioX interface to retrieve these pin states?

Currently the user space code is looking for these ucd9000 driver files as listed here and that would drive some mods on their end. Not saying that's a good reason to keep as it is but its still a factor. Definitely for first community submission that would be something to address.

Quoted 106 lines
Quoted 99 lines

+static SENSOR_DEVICE_ATTR(gpio8_out_val, 0444, ucd9000_gpi_config,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 259

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			NULL, 0x0208);

+static SENSOR_DEVICE_ATTR(gpio8_stat, 0444, ucd9000_gpi_config,

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			NULL, 0x0308);
drivers/hwmon/pmbus/ucd9000.c +2 -0
+static SENSOR_DEVICE_ATTR(gpio9_enab, 0444, ucd9000_gpi_config, NULL, 9);
+static SENSOR_DEVICE_ATTR(gpio9_out_enab, 0444, ucd9000_gpi_config,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 264

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			NULL, 0x0109);

+static SENSOR_DEVICE_ATTR(gpio9_out_val, 0444, ucd9000_gpi_config,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 266

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			NULL, 0x0209);

+static SENSOR_DEVICE_ATTR(gpio9_stat, 0444, ucd9000_gpi_config,

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			NULL, 0x0309);
drivers/hwmon/pmbus/ucd9000.c +2 -0
+static SENSOR_DEVICE_ATTR(gpio10_enab, 0444, ucd9000_gpi_config, NULL, 10);
+static SENSOR_DEVICE_ATTR(gpio10_out_enab, 0444, ucd9000_gpi_config,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 271

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			NULL, 0x010a);

+static SENSOR_DEVICE_ATTR(gpio10_out_val, 0444, ucd9000_gpi_config,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 273

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			NULL, 0x020a);

+static SENSOR_DEVICE_ATTR(gpio10_stat, 0444, ucd9000_gpi_config,

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			NULL, 0x030a);
drivers/hwmon/pmbus/ucd9000.c +2 -0
+static SENSOR_DEVICE_ATTR(gpio11_enab, 0444, ucd9000_gpi_config, NULL, 11);
+static SENSOR_DEVICE_ATTR(gpio11_out_enab, 0444, ucd9000_gpi_config,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 278

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			NULL, 0x010b);

+static SENSOR_DEVICE_ATTR(gpio11_out_val, 0444, ucd9000_gpi_config,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 280

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			NULL, 0x020b);

+static SENSOR_DEVICE_ATTR(gpio11_stat, 0444, ucd9000_gpi_config,

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			NULL, 0x030b);
drivers/hwmon/pmbus/ucd9000.c +2 -0
+static SENSOR_DEVICE_ATTR(gpio14_enab, 0444, ucd9000_gpi_config, NULL, 14);
+static SENSOR_DEVICE_ATTR(gpio14_out_enab, 0444, ucd9000_gpi_config,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 285

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			NULL, 0x010e);

+static SENSOR_DEVICE_ATTR(gpio14_out_val, 0444, ucd9000_gpi_config,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 287

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			NULL, 0x020e);

+static SENSOR_DEVICE_ATTR(gpio14_stat, 0444, ucd9000_gpi_config,

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			NULL, 0x030e);
drivers/hwmon/pmbus/ucd9000.c +2 -0
+static SENSOR_DEVICE_ATTR(gpio17_enab, 0444, ucd9000_gpi_config, NULL, 17);
+static SENSOR_DEVICE_ATTR(gpio17_out_enab, 0444, ucd9000_gpi_config,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 292

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			NULL, 0x0111);

+static SENSOR_DEVICE_ATTR(gpio17_out_val, 0444, ucd9000_gpi_config,

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_read_byte_data(struct i2c_client *client, int page, int reg) · line 294

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			NULL, 0x0211);

+static SENSOR_DEVICE_ATTR(gpio17_stat, 0444, ucd9000_gpi_config,

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+			NULL, 0x0311);
drivers/hwmon/pmbus/ucd9000.c +3 -0
+
+
+static struct attribute *ucd9000_attributes[] = {
Quoted 47 lines
drivers/hwmon/pmbus/ucd9000.c +47 -0
+	&dev_attr_clear_logged_faults.attr,
+	&sensor_dev_attr_gpi8_alarm.dev_attr.attr,
+	&sensor_dev_attr_gpi7_alarm.dev_attr.attr,
+	&sensor_dev_attr_gpi6_alarm.dev_attr.attr,
+	&sensor_dev_attr_gpi5_alarm.dev_attr.attr,
+	&sensor_dev_attr_gpi4_alarm.dev_attr.attr,
+	&sensor_dev_attr_gpi3_alarm.dev_attr.attr,
+	&sensor_dev_attr_gpi2_alarm.dev_attr.attr,
+	&sensor_dev_attr_gpi1_alarm.dev_attr.attr,
+	&sensor_dev_attr_new_logged_fault_detail.dev_attr.attr,
+	&sensor_dev_attr_system_watchdog_timeout.dev_attr.attr,
+	&sensor_dev_attr_store_default_all_error.dev_attr.attr,
+	&sensor_dev_attr_store_default_all_done.dev_attr.attr,
+	&sensor_dev_attr_watchdog_timeout.dev_attr.attr,
+	&sensor_dev_attr_invalid_logs.dev_attr.attr,
+	&sensor_dev_attr_logged_fault_detail.dev_attr.attr,
+	&sensor_dev_attr_resequence_error.dev_attr.attr,
+	&sensor_dev_attr_pkgid_mismatch.dev_attr.attr,
+	&sensor_dev_attr_hardcoded_parms.dev_attr.attr,
+	&sensor_dev_attr_seq_off_timeout.dev_attr.attr,
+	&sensor_dev_attr_seq_on_timeout.dev_attr.attr,
+	&sensor_dev_attr_mfr_status.dev_attr.attr,
+	&sensor_dev_attr_gpio8_enab.dev_attr.attr,
+	&sensor_dev_attr_gpio8_out_enab.dev_attr.attr,
+	&sensor_dev_attr_gpio8_out_val.dev_attr.attr,
+	&sensor_dev_attr_gpio8_stat.dev_attr.attr,
+	&sensor_dev_attr_gpio9_enab.dev_attr.attr,
+	&sensor_dev_attr_gpio9_out_enab.dev_attr.attr,
+	&sensor_dev_attr_gpio9_out_val.dev_attr.attr,
+	&sensor_dev_attr_gpio9_stat.dev_attr.attr,
+	&sensor_dev_attr_gpio10_enab.dev_attr.attr,
+	&sensor_dev_attr_gpio10_out_enab.dev_attr.attr,
+	&sensor_dev_attr_gpio10_out_val.dev_attr.attr,
+	&sensor_dev_attr_gpio10_stat.dev_attr.attr,
+	&sensor_dev_attr_gpio11_enab.dev_attr.attr,
+	&sensor_dev_attr_gpio11_out_enab.dev_attr.attr,
+	&sensor_dev_attr_gpio11_out_val.dev_attr.attr,
+	&sensor_dev_attr_gpio11_stat.dev_attr.attr,
+	&sensor_dev_attr_gpio14_enab.dev_attr.attr,
+	&sensor_dev_attr_gpio14_out_enab.dev_attr.attr,
+	&sensor_dev_attr_gpio14_out_val.dev_attr.attr,
+	&sensor_dev_attr_gpio14_stat.dev_attr.attr,
+	&sensor_dev_attr_gpio17_enab.dev_attr.attr,
+	&sensor_dev_attr_gpio17_out_enab.dev_attr.attr,
+	&sensor_dev_attr_gpio17_out_val.dev_attr.attr,
+	&sensor_dev_attr_gpio17_stat.dev_attr.attr,
+	NULL
drivers/hwmon/pmbus/ucd9000.c +3 -0
+};
+
+static const struct attribute_group ucd9000_attr_group = {
Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+	.attrs = ucd9000_attributes,
drivers/hwmon/pmbus/ucd9000.c +2 -0
+};
+
  static int ucd9000_probe(struct i2c_client *client,
Quoted 1 line

const struct i2c_device_id *id)

{

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_probe(struct i2c_client *client, · line 462

Quoted 5 lines
drivers/hwmon/pmbus/ucd9000.c +0 -0
@@ -227,7 +458,40 @@ static int ucd9000_probe(struct i2c_client *client,
 >   		  | PMBUS_HAVE_FAN34 | PMBUS_HAVE_STATUS_FAN34;
 >   	}
 
 > -	return pmbus_do_probe(client, mid, info);
 > +	info->debugfs = true;

This causes a compilation failure:

Quoted 8 lines

CC drivers/hwmon/pmbus/ucd9000.o
drivers/hwmon/pmbus/ucd9000.c: In function ‘ucd9000_probe’:
drivers/hwmon/pmbus/ucd9000.c:461:6: error: ‘struct pmbus_driver_info’ has no member named ‘debugfs’

   info->debugfs = true;
       ^~

scripts/Makefile.build:294: recipe for target 'drivers/hwmon/pmbus/ucd9000.o' failed
From the pmbus/debugfs implementation applied to the tree it looks like we can
just drop this.

Yes, this was a vestige of a previous pmbus core update revision. Will fix.

Quoted 11 lines
Quoted 11 lines
drivers/hwmon/pmbus/ucd9000.c +1 -0
+
 > +	data->gpio.label = "gpio-ucd9000";
 > +	data->gpio.get = ucd9000_gpio_get;

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_probe(struct i2c_client *client, · line 471

Quoted 7 lines
drivers/hwmon/pmbus/ucd9000.c +7 -0
+	data->gpio.set = ucd9000_gpio_set;
+	data->gpio.can_sleep = 1;
+	data->gpio.base = -1;
+	if (mid->driver_data == ucd90160)
+		data->gpio.ngpio = UCD90160_NUM_GPIOS;
+	data->gpio.parent = &client->dev;
+	data->gpio.owner = THIS_MODULE;

+

Quoted 4 lines
Quoted 3 lines

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_probe(struct i2c_client *client, · line 475

Quoted 3 lines
drivers/hwmon/pmbus/ucd9000.c +3 -0
+	ret = devm_gpiochip_add_data(&client->dev, &data->gpio, client);
+	if (ret) {
+		/* Use data->gpio.dev as a flag for freeing gpiochip */

What is this comment about?

Was a note for something I was to follow up on. Will remove.

Thanks,
Chris

Quoted 35 lines
Quoted 25 lines

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_probe(struct i2c_client *client, · line 479

Quoted 5 lines
drivers/hwmon/pmbus/ucd9000.c +4 -0
+		data->gpio.parent = NULL;
+		dev_warn(&client->dev, "Count not add gpiochip\n");
+		return ret;
+	}

+

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_probe(struct i2c_client *client, · line 482

Quoted 2 lines
drivers/hwmon/pmbus/ucd9000.c +2 -0
+	dev_info(&client->dev, "gpios %i...%i\n", data->gpio.base,
+		data->gpio.base + data->gpio.ngpio - 1);

+

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_probe(struct i2c_client *client, · line 486

Quoted 3 lines
drivers/hwmon/pmbus/ucd9000.c +3 -0
+	ret = sysfs_create_group(&client->dev.kobj, &ucd9000_attr_group);
+	if (ret < 0)
+		return ret;

+

Quoted 1 line
drivers/hwmon/pmbus/ucd9000.c +1 -0
+	return  pmbus_do_probe(client, mid, info);
drivers/hwmon/pmbus/ucd9000.c +4 -0
+}
+
+static int ucd9000_remove(struct i2c_client *client)
+{

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_probe(struct i2c_client *client, · line 494

Quoted 2 lines
drivers/hwmon/pmbus/ucd9000.c +2 -0
+	sysfs_remove_group(&client->dev.kobj, &ucd9000_attr_group);
+	return pmbus_do_remove(client);

}

/* This is the driver that will be inserted */

drivers/hwmon/pmbus/ucd9000.c static int ucd9000_probe(struct i2c_client *client, · line 506

Quoted 7 lines
drivers/hwmon/pmbus/ucd9000.c +0 -0
@@ -236,7 +500,7 @@ static int ucd9000_probe(struct i2c_client *client,
 >   		.name = "ucd9000",
 >   	},
 >   	.probe = ucd9000_probe,
 > -	.remove = pmbus_do_remove,
 > +	.remove = ucd9000_remove,
 >   	.id_table = ucd9000_id,
  };

Cheers,

Andrew