Message ID | 20170906210041.22533-3-cbostic@linux.vnet.ibm.com |
---|---|
State | Changes Requested, archived |
Headers | show |
Series | Add user space accessibility for ucd9000 stats | expand |
On Wed, 2017-09-06 at 16:00 -0500, Christopher Bostic wrote: > Expose via files in debugfs the gpiN_fault fields as well as > the entire contents of the mfr_status register. Allows user > space to pull this data for error logging etc... > > > Signed-off-by: Christopher Bostic <cbostic@linux.vnet.ibm.com> > --- > v7 - Patch order changed from #3 to #2 and as a result needed > to add the ucd9000_remove definition here. > v6 - Remove chip idx values on remove so that we can retain the > same indexes regardless how many times we're bound/unbound. > - Remove type 90160 check when adding debugfs. > v5 - Add unique debugfs dir ID for each ucd9000 sysfs device. > - Change branching return to a !! method. > - Clean up line breaks, other white space. > - Add comments on assumptions made regarding ucd90160 versus other > types. > v2 - Remove mfr_status directory in debugfs and place the files > up in the parent ucd9000 directory. > --- > drivers/hwmon/pmbus/ucd9000.c | 166 +++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 165 insertions(+), 1 deletion(-) > > diff --git a/drivers/hwmon/pmbus/ucd9000.c b/drivers/hwmon/pmbus/ucd9000.c > index 9a82e88..26870e6 100644 > --- a/drivers/hwmon/pmbus/ucd9000.c > +++ b/drivers/hwmon/pmbus/ucd9000.c > @@ -19,6 +19,7 @@ > * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. > */ > > +#include <linux/debugfs.h> > #include <linux/kernel.h> > #include <linux/module.h> > #include <linux/init.h> > @@ -27,6 +28,7 @@ > #include <linux/i2c.h> > #include <linux/i2c/pmbus.h> > #include <linux/gpio.h> > +#include <linux/idr.h> > #include "pmbus.h" > > enum chips { ucd9000, ucd90120, ucd90124, ucd90160, ucd9090, ucd90910 }; > @@ -35,6 +37,7 @@ > #define UCD9000_NUM_PAGES 0xd6 > #define UCD9000_FAN_CONFIG_INDEX 0xe7 > #define UCD9000_FAN_CONFIG 0xe8 > +#define UCD9000_MFR_STATUS 0xf3 > #define UCD9000_GPIO_SELECT 0xfa > #define UCD9000_GPIO_CONFIG 0xfb > #define UCD9000_DEVICE_ID 0xfd > @@ -56,17 +59,29 @@ > #define UCD9000_MON_VOLTAGE_HW 4 > > #define UCD9000_NUM_FAN 4 > +#define UCD9000_NAME_SIZE 24 > > #define UCD9000_GPIO_NAME_LEN 16 > #define UCD90160_NUM_GPIOS 26 > +#define UCD90160_GPI_COUNT 8 > +#define UCD90160_GPI_FAULT_BASE 16 > + > +static DEFINE_IDA(ucd9000_ida); > > struct ucd9000_data { > u8 fan_data[UCD9000_NUM_FAN][I2C_SMBUS_BLOCK_MAX]; > struct pmbus_driver_info info; > struct gpio_chip gpio; > + struct dentry *debugfs; > + int idx; > }; > #define to_ucd9000_data(_info) container_of(_info, struct ucd9000_data, info) > > +struct ucd9000_debugfs_entry { > + struct i2c_client *client; > + u8 index; > +}; > + > static int ucd9000_get_fan_config(struct i2c_client *client, int fan) > { > int fan_config = 0; > @@ -294,6 +309,141 @@ static int ucd9000_gpio_direction_output(struct gpio_chip *gc, > return ucd9000_gpio_set_direction(gc, offset, UCD9000_GPIO_OUTPUT, val); > } > > +static struct dentry *ucd9000_debugfs_dir; I thought we were fixing this up? > + > +#if IS_ENABLED(CONFIG_DEBUG_FS) > +static int ucd9000_get_mfr_status(struct i2c_client *client, u32 *buffer) > +{ > + int ret; > + > + ret = pmbus_set_page(client, 0); > + if (ret < 0) { > + dev_err(&client->dev, "pmbus_set_page failed. rc:%d\n", ret); > + > + return ret; > + } > + > + /* > + * Warning: > + * > + * Though not currently supported this will cause stack corruption for > + * ucd90240! Command reference, page 81: > + * > + * With the ucd90120 and ucd90124 devices, this command [MFR_STATUS] > + * is 2 bytes long (bits 0-15). With the ucd90240 this command is 5 > + * bytes long. With all other devices, it is 4 bytes long. > + */ > + return i2c_smbus_read_block_data(client, UCD9000_MFR_STATUS, > + (u8 *)buffer); > +} > + > +static int ucd9000_debugfs_get_mfr_status_bit(void *data, u64 *val) > +{ > + struct ucd9000_debugfs_entry *entry = data; > + struct i2c_client *client = entry->client; > + int nr = entry->index; > + u32 buffer; > + int ret; > + > + ret = ucd9000_get_mfr_status(client, &buffer); > + if (ret < 0) { > + dev_err(&client->dev, "Failed to read mfr status. rc:%d\n", > + ret); > + > + return ret; > + } > + > + *val = !!(ret & BIT(nr)); > + > + return 0; > +} > + > +DEFINE_DEBUGFS_ATTRIBUTE(ucd9000_debugfs_ops_mfr_status_bit, > + ucd9000_debugfs_get_mfr_status_bit, NULL, "%1lld\n"); > + > +static int ucd9000_debugfs_get_mfr_status_word(void *data, u64 *val) > +{ > + struct ucd9000_debugfs_entry *entry = data; > + struct i2c_client *client = entry->client; > + u32 buffer; > + int ret; > + > + ret = ucd9000_get_mfr_status(client, &buffer); > + if (ret < 0) { > + dev_err(&client->dev, "Failed to read mfr status. rc:%d\n", > + ret); > + > + return ret; > + } > + > + *val = ret; > + > + return 0; > +} > +DEFINE_DEBUGFS_ATTRIBUTE(ucd9000_debugfs_ops_mfr_status_word, > + ucd9000_debugfs_get_mfr_status_word, NULL, "%08llx\n"); > + > +static int ucd9000_init_debugfs(struct i2c_client *client, > + struct ucd9000_data *data) > +{ > + struct ucd9000_debugfs_entry *entries; > + char name[UCD9000_NAME_SIZE]; > + int i; > + > + data->idx = ida_simple_get(&ucd9000_ida, 0, INT_MAX, GFP_KERNEL); > + scnprintf(name, UCD9000_NAME_SIZE, "ucd9000.%d", data->idx); > + ucd9000_debugfs_dir = debugfs_create_dir(name, NULL); We should be storing the dentry in the context struct. We're now uniquely naming the directory, but we're still storing the dentry in static storage. The code as it stands will still stomp over the top of existing devices on probe, and it cannot be cleaned up properly. In fact, we'll cleanup the wrong entry depending on the order of probe/remove across the devices. It's not just fixed by uniquely naming the directory. Andrew > + if (IS_ERR(ucd9000_debugfs_dir)) { > + dev_warn(&client->dev, "Failed to create debugfs dir: %p\n", > + ucd9000_debugfs_dir); > + ucd9000_debugfs_dir = NULL; > + ida_simple_remove(&ucd9000_ida, data->idx); > + return 0; > + } > + > + /* > + * Warning: > + * > + * Makes assumption we're on a ucd90160 type! entries will be different > + * sizes for other types. > + */ > + entries = devm_kzalloc(&client->dev, sizeof(*entries) * > + (UCD90160_GPI_COUNT + 1) * 10, GFP_KERNEL); > + if (!entries) { > + ida_simple_remove(&ucd9000_ida, data->idx); > + return -ENOMEM; > + } > + > + /* > + * Warning: > + * > + * This makes the assumption we're probing a ucd90160 type and how the > + * GPI information is organized. Needs to account for all other > + * ucd9000 varieties. > + */ > + for (i = 0; i < UCD90160_GPI_COUNT; i++) { > + entries[i].client = client; > + entries[i].index = UCD90160_GPI_FAULT_BASE + i; > + scnprintf(name, UCD9000_NAME_SIZE, "gpi%d_alarm", i+1); > + debugfs_create_file(name, 0444, ucd9000_debugfs_dir, > + &entries[i], > + &ucd9000_debugfs_ops_mfr_status_bit); > + } > + entries[i].client = client; > + scnprintf(name, UCD9000_NAME_SIZE, "mfr_status"); > + debugfs_create_file(name, 0444, ucd9000_debugfs_dir, &entries[i], > + &ucd9000_debugfs_ops_mfr_status_word); > + > + return 0; > +} > +#else > +static int ucd9000_init_debugfs(struct i2c_client *client, > + struct ucd9000_data *data) > +{ > + return 0; > +} > +#endif /* IS_ENABLED(CONFIG_DEBUG_FS) */ > + > static int ucd9000_probe(struct i2c_client *client, > const struct i2c_device_id *id) > { > @@ -433,16 +583,30 @@ static int ucd9000_probe(struct i2c_client *client, > return ret; > } > > + ret = ucd9000_init_debugfs(client, data); > + if (ret < 0) > + dev_warn(&client->dev, "Failed to register debugfs: %d\n", ret); > + > return pmbus_do_probe(client, mid, info); > } > > +static int ucd9000_remove(struct i2c_client *client) > +{ > + struct ucd9000_data *data > + = to_ucd9000_data(pmbus_get_driver_info(client)); > + > + ida_simple_remove(&ucd9000_ida, data->idx); > + debugfs_remove_recursive(ucd9000_debugfs_dir); > + return pmbus_do_remove(client); > +} > + > /* This is the driver that will be inserted */ > static struct i2c_driver ucd9000_driver = { > .driver = { > .name = "ucd9000", > }, > .probe = ucd9000_probe, > - .remove = pmbus_do_remove, > + .remove = ucd9000_remove, > .id_table = ucd9000_id, > }; >
On 9/6/17 8:17 PM, Andrew Jeffery wrote: > On Wed, 2017-09-06 at 16:00 -0500, Christopher Bostic wrote: >> Expose via files in debugfs the gpiN_fault fields as well as >> the entire contents of the mfr_status register. Allows user >> space to pull this data for error logging etc... >> >>> Signed-off-by: Christopher Bostic <cbostic@linux.vnet.ibm.com> >> --- >> v7 - Patch order changed from #3 to #2 and as a result needed >> to add the ucd9000_remove definition here. >> v6 - Remove chip idx values on remove so that we can retain the >> same indexes regardless how many times we're bound/unbound. >> - Remove type 90160 check when adding debugfs. >> v5 - Add unique debugfs dir ID for each ucd9000 sysfs device. >> - Change branching return to a !! method. >> - Clean up line breaks, other white space. >> - Add comments on assumptions made regarding ucd90160 versus other >> types. >> v2 - Remove mfr_status directory in debugfs and place the files >> up in the parent ucd9000 directory. >> --- >> drivers/hwmon/pmbus/ucd9000.c | 166 +++++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 165 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/hwmon/pmbus/ucd9000.c b/drivers/hwmon/pmbus/ucd9000.c >> index 9a82e88..26870e6 100644 >> --- a/drivers/hwmon/pmbus/ucd9000.c >> +++ b/drivers/hwmon/pmbus/ucd9000.c >> @@ -19,6 +19,7 @@ >> * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. >> */ >> >> +#include <linux/debugfs.h> >> #include <linux/kernel.h> >> #include <linux/module.h> >> #include <linux/init.h> >> @@ -27,6 +28,7 @@ >> #include <linux/i2c.h> >> #include <linux/i2c/pmbus.h> >> #include <linux/gpio.h> >> +#include <linux/idr.h> >> #include "pmbus.h" >> >> enum chips { ucd9000, ucd90120, ucd90124, ucd90160, ucd9090, ucd90910 }; >> @@ -35,6 +37,7 @@ >> #define UCD9000_NUM_PAGES 0xd6 >> #define UCD9000_FAN_CONFIG_INDEX 0xe7 >> #define UCD9000_FAN_CONFIG 0xe8 >> +#define UCD9000_MFR_STATUS 0xf3 >> #define UCD9000_GPIO_SELECT 0xfa >> #define UCD9000_GPIO_CONFIG 0xfb >> #define UCD9000_DEVICE_ID 0xfd >> @@ -56,17 +59,29 @@ >> #define UCD9000_MON_VOLTAGE_HW 4 >> >> #define UCD9000_NUM_FAN 4 >> +#define UCD9000_NAME_SIZE 24 >> >> #define UCD9000_GPIO_NAME_LEN 16 >> #define UCD90160_NUM_GPIOS 26 >> +#define UCD90160_GPI_COUNT 8 >> +#define UCD90160_GPI_FAULT_BASE 16 >> + >> +static DEFINE_IDA(ucd9000_ida); >> >> struct ucd9000_data { >> u8 fan_data[UCD9000_NUM_FAN][I2C_SMBUS_BLOCK_MAX]; >> struct pmbus_driver_info info; >> struct gpio_chip gpio; >> + struct dentry *debugfs; >> + int idx; >> }; >> #define to_ucd9000_data(_info) container_of(_info, struct ucd9000_data, info) >> >> +struct ucd9000_debugfs_entry { >> + struct i2c_client *client; >> + u8 index; >> +}; >> + >> static int ucd9000_get_fan_config(struct i2c_client *client, int fan) >> { >> int fan_config = 0; >> @@ -294,6 +309,141 @@ static int ucd9000_gpio_direction_output(struct gpio_chip *gc, >> return ucd9000_gpio_set_direction(gc, offset, UCD9000_GPIO_OUTPUT, val); >> } >> >> +static struct dentry *ucd9000_debugfs_dir; > I thought we were fixing this up? > >> + >> +#if IS_ENABLED(CONFIG_DEBUG_FS) >> +static int ucd9000_get_mfr_status(struct i2c_client *client, u32 *buffer) >> +{ >> + int ret; >> + >> + ret = pmbus_set_page(client, 0); >> + if (ret < 0) { >> + dev_err(&client->dev, "pmbus_set_page failed. rc:%d\n", ret); >> + >> + return ret; >> + } >> + >> + /* >> + * Warning: >> + * >> + * Though not currently supported this will cause stack corruption for >> + * ucd90240! Command reference, page 81: >> + * >> + * With the ucd90120 and ucd90124 devices, this command [MFR_STATUS] >> + * is 2 bytes long (bits 0-15). With the ucd90240 this command is 5 >> + * bytes long. With all other devices, it is 4 bytes long. >> + */ >> + return i2c_smbus_read_block_data(client, UCD9000_MFR_STATUS, >> + (u8 *)buffer); >> +} >> + >> +static int ucd9000_debugfs_get_mfr_status_bit(void *data, u64 *val) >> +{ >> + struct ucd9000_debugfs_entry *entry = data; >> + struct i2c_client *client = entry->client; >> + int nr = entry->index; >> + u32 buffer; >> + int ret; >> + >> + ret = ucd9000_get_mfr_status(client, &buffer); >> + if (ret < 0) { >> + dev_err(&client->dev, "Failed to read mfr status. rc:%d\n", >> + ret); >> + >> + return ret; >> + } >> + >> + *val = !!(ret & BIT(nr)); >> + >> + return 0; >> +} >> + >> +DEFINE_DEBUGFS_ATTRIBUTE(ucd9000_debugfs_ops_mfr_status_bit, >> + ucd9000_debugfs_get_mfr_status_bit, NULL, "%1lld\n"); >> + >> +static int ucd9000_debugfs_get_mfr_status_word(void *data, u64 *val) >> +{ >> + struct ucd9000_debugfs_entry *entry = data; >> + struct i2c_client *client = entry->client; >> + u32 buffer; >> + int ret; >> + >> + ret = ucd9000_get_mfr_status(client, &buffer); >> + if (ret < 0) { >> + dev_err(&client->dev, "Failed to read mfr status. rc:%d\n", >> + ret); >> + >> + return ret; >> + } >> + >> + *val = ret; >> + >> + return 0; >> +} >> +DEFINE_DEBUGFS_ATTRIBUTE(ucd9000_debugfs_ops_mfr_status_word, >> + ucd9000_debugfs_get_mfr_status_word, NULL, "%08llx\n"); >> + >> +static int ucd9000_init_debugfs(struct i2c_client *client, >> + struct ucd9000_data *data) >> +{ >> + struct ucd9000_debugfs_entry *entries; >> + char name[UCD9000_NAME_SIZE]; >> + int i; >> + >> + data->idx = ida_simple_get(&ucd9000_ida, 0, INT_MAX, GFP_KERNEL); >> + scnprintf(name, UCD9000_NAME_SIZE, "ucd9000.%d", data->idx); >> + ucd9000_debugfs_dir = debugfs_create_dir(name, NULL); > We should be storing the dentry in the context struct. We're now uniquely > naming the directory, but we're still storing the dentry in static storage. The > code as it stands will still stomp over the top of existing devices on probe, > and it cannot be cleaned up properly. In fact, we'll cleanup the wrong entry > depending on the order of probe/remove across the devices. It's not just fixed > by uniquely naming the directory. OK, will correct for next set. Thanks, Chris > > Andrew > >> + if (IS_ERR(ucd9000_debugfs_dir)) { >> + dev_warn(&client->dev, "Failed to create debugfs dir: %p\n", >> + ucd9000_debugfs_dir); >> + ucd9000_debugfs_dir = NULL; >> + ida_simple_remove(&ucd9000_ida, data->idx); >> + return 0; >> + } >> + >> + /* >> + * Warning: >> + * >> + * Makes assumption we're on a ucd90160 type! entries will be different >> + * sizes for other types. >> + */ >> + entries = devm_kzalloc(&client->dev, sizeof(*entries) * >> + (UCD90160_GPI_COUNT + 1) * 10, GFP_KERNEL); >> + if (!entries) { >> + ida_simple_remove(&ucd9000_ida, data->idx); >> + return -ENOMEM; >> + } >> + >> + /* >> + * Warning: >> + * >> + * This makes the assumption we're probing a ucd90160 type and how the >> + * GPI information is organized. Needs to account for all other >> + * ucd9000 varieties. >> + */ >> + for (i = 0; i < UCD90160_GPI_COUNT; i++) { >> + entries[i].client = client; >> + entries[i].index = UCD90160_GPI_FAULT_BASE + i; >> + scnprintf(name, UCD9000_NAME_SIZE, "gpi%d_alarm", i+1); >> + debugfs_create_file(name, 0444, ucd9000_debugfs_dir, >> + &entries[i], >> + &ucd9000_debugfs_ops_mfr_status_bit); >> + } >> + entries[i].client = client; >> + scnprintf(name, UCD9000_NAME_SIZE, "mfr_status"); >> + debugfs_create_file(name, 0444, ucd9000_debugfs_dir, &entries[i], >> + &ucd9000_debugfs_ops_mfr_status_word); >> + >> + return 0; >> +} >> +#else >> +static int ucd9000_init_debugfs(struct i2c_client *client, >> + struct ucd9000_data *data) >> +{ >> + return 0; >> +} >> +#endif /* IS_ENABLED(CONFIG_DEBUG_FS) */ >> + >> static int ucd9000_probe(struct i2c_client *client, >> const struct i2c_device_id *id) >> { >> @@ -433,16 +583,30 @@ static int ucd9000_probe(struct i2c_client *client, >> return ret; >> } >> >> + ret = ucd9000_init_debugfs(client, data); >> + if (ret < 0) >> + dev_warn(&client->dev, "Failed to register debugfs: %d\n", ret); >> + >> return pmbus_do_probe(client, mid, info); >> } >> >> +static int ucd9000_remove(struct i2c_client *client) >> +{ >> + struct ucd9000_data *data >> + = to_ucd9000_data(pmbus_get_driver_info(client)); >> + >> + ida_simple_remove(&ucd9000_ida, data->idx); >> + debugfs_remove_recursive(ucd9000_debugfs_dir); >> + return pmbus_do_remove(client); >> +} >> + >> /* This is the driver that will be inserted */ >> static struct i2c_driver ucd9000_driver = { >> .driver = { >> .name = "ucd9000", >> }, >> .probe = ucd9000_probe, >> - .remove = pmbus_do_remove, >> + .remove = ucd9000_remove, >> .id_table = ucd9000_id, >> }; >>
diff --git a/drivers/hwmon/pmbus/ucd9000.c b/drivers/hwmon/pmbus/ucd9000.c index 9a82e88..26870e6 100644 --- a/drivers/hwmon/pmbus/ucd9000.c +++ b/drivers/hwmon/pmbus/ucd9000.c @@ -19,6 +19,7 @@ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. */ +#include <linux/debugfs.h> #include <linux/kernel.h> #include <linux/module.h> #include <linux/init.h> @@ -27,6 +28,7 @@ #include <linux/i2c.h> #include <linux/i2c/pmbus.h> #include <linux/gpio.h> +#include <linux/idr.h> #include "pmbus.h" enum chips { ucd9000, ucd90120, ucd90124, ucd90160, ucd9090, ucd90910 }; @@ -35,6 +37,7 @@ #define UCD9000_NUM_PAGES 0xd6 #define UCD9000_FAN_CONFIG_INDEX 0xe7 #define UCD9000_FAN_CONFIG 0xe8 +#define UCD9000_MFR_STATUS 0xf3 #define UCD9000_GPIO_SELECT 0xfa #define UCD9000_GPIO_CONFIG 0xfb #define UCD9000_DEVICE_ID 0xfd @@ -56,17 +59,29 @@ #define UCD9000_MON_VOLTAGE_HW 4 #define UCD9000_NUM_FAN 4 +#define UCD9000_NAME_SIZE 24 #define UCD9000_GPIO_NAME_LEN 16 #define UCD90160_NUM_GPIOS 26 +#define UCD90160_GPI_COUNT 8 +#define UCD90160_GPI_FAULT_BASE 16 + +static DEFINE_IDA(ucd9000_ida); struct ucd9000_data { u8 fan_data[UCD9000_NUM_FAN][I2C_SMBUS_BLOCK_MAX]; struct pmbus_driver_info info; struct gpio_chip gpio; + struct dentry *debugfs; + int idx; }; #define to_ucd9000_data(_info) container_of(_info, struct ucd9000_data, info) +struct ucd9000_debugfs_entry { + struct i2c_client *client; + u8 index; +}; + static int ucd9000_get_fan_config(struct i2c_client *client, int fan) { int fan_config = 0; @@ -294,6 +309,141 @@ static int ucd9000_gpio_direction_output(struct gpio_chip *gc, return ucd9000_gpio_set_direction(gc, offset, UCD9000_GPIO_OUTPUT, val); } +static struct dentry *ucd9000_debugfs_dir; + +#if IS_ENABLED(CONFIG_DEBUG_FS) +static int ucd9000_get_mfr_status(struct i2c_client *client, u32 *buffer) +{ + int ret; + + ret = pmbus_set_page(client, 0); + if (ret < 0) { + dev_err(&client->dev, "pmbus_set_page failed. rc:%d\n", ret); + + return ret; + } + + /* + * Warning: + * + * Though not currently supported this will cause stack corruption for + * ucd90240! Command reference, page 81: + * + * With the ucd90120 and ucd90124 devices, this command [MFR_STATUS] + * is 2 bytes long (bits 0-15). With the ucd90240 this command is 5 + * bytes long. With all other devices, it is 4 bytes long. + */ + return i2c_smbus_read_block_data(client, UCD9000_MFR_STATUS, + (u8 *)buffer); +} + +static int ucd9000_debugfs_get_mfr_status_bit(void *data, u64 *val) +{ + struct ucd9000_debugfs_entry *entry = data; + struct i2c_client *client = entry->client; + int nr = entry->index; + u32 buffer; + int ret; + + ret = ucd9000_get_mfr_status(client, &buffer); + if (ret < 0) { + dev_err(&client->dev, "Failed to read mfr status. rc:%d\n", + ret); + + return ret; + } + + *val = !!(ret & BIT(nr)); + + return 0; +} + +DEFINE_DEBUGFS_ATTRIBUTE(ucd9000_debugfs_ops_mfr_status_bit, + ucd9000_debugfs_get_mfr_status_bit, NULL, "%1lld\n"); + +static int ucd9000_debugfs_get_mfr_status_word(void *data, u64 *val) +{ + struct ucd9000_debugfs_entry *entry = data; + struct i2c_client *client = entry->client; + u32 buffer; + int ret; + + ret = ucd9000_get_mfr_status(client, &buffer); + if (ret < 0) { + dev_err(&client->dev, "Failed to read mfr status. rc:%d\n", + ret); + + return ret; + } + + *val = ret; + + return 0; +} +DEFINE_DEBUGFS_ATTRIBUTE(ucd9000_debugfs_ops_mfr_status_word, + ucd9000_debugfs_get_mfr_status_word, NULL, "%08llx\n"); + +static int ucd9000_init_debugfs(struct i2c_client *client, + struct ucd9000_data *data) +{ + struct ucd9000_debugfs_entry *entries; + char name[UCD9000_NAME_SIZE]; + int i; + + data->idx = ida_simple_get(&ucd9000_ida, 0, INT_MAX, GFP_KERNEL); + scnprintf(name, UCD9000_NAME_SIZE, "ucd9000.%d", data->idx); + ucd9000_debugfs_dir = debugfs_create_dir(name, NULL); + if (IS_ERR(ucd9000_debugfs_dir)) { + dev_warn(&client->dev, "Failed to create debugfs dir: %p\n", + ucd9000_debugfs_dir); + ucd9000_debugfs_dir = NULL; + ida_simple_remove(&ucd9000_ida, data->idx); + return 0; + } + + /* + * Warning: + * + * Makes assumption we're on a ucd90160 type! entries will be different + * sizes for other types. + */ + entries = devm_kzalloc(&client->dev, sizeof(*entries) * + (UCD90160_GPI_COUNT + 1) * 10, GFP_KERNEL); + if (!entries) { + ida_simple_remove(&ucd9000_ida, data->idx); + return -ENOMEM; + } + + /* + * Warning: + * + * This makes the assumption we're probing a ucd90160 type and how the + * GPI information is organized. Needs to account for all other + * ucd9000 varieties. + */ + for (i = 0; i < UCD90160_GPI_COUNT; i++) { + entries[i].client = client; + entries[i].index = UCD90160_GPI_FAULT_BASE + i; + scnprintf(name, UCD9000_NAME_SIZE, "gpi%d_alarm", i+1); + debugfs_create_file(name, 0444, ucd9000_debugfs_dir, + &entries[i], + &ucd9000_debugfs_ops_mfr_status_bit); + } + entries[i].client = client; + scnprintf(name, UCD9000_NAME_SIZE, "mfr_status"); + debugfs_create_file(name, 0444, ucd9000_debugfs_dir, &entries[i], + &ucd9000_debugfs_ops_mfr_status_word); + + return 0; +} +#else +static int ucd9000_init_debugfs(struct i2c_client *client, + struct ucd9000_data *data) +{ + return 0; +} +#endif /* IS_ENABLED(CONFIG_DEBUG_FS) */ + static int ucd9000_probe(struct i2c_client *client, const struct i2c_device_id *id) { @@ -433,16 +583,30 @@ static int ucd9000_probe(struct i2c_client *client, return ret; } + ret = ucd9000_init_debugfs(client, data); + if (ret < 0) + dev_warn(&client->dev, "Failed to register debugfs: %d\n", ret); + return pmbus_do_probe(client, mid, info); } +static int ucd9000_remove(struct i2c_client *client) +{ + struct ucd9000_data *data + = to_ucd9000_data(pmbus_get_driver_info(client)); + + ida_simple_remove(&ucd9000_ida, data->idx); + debugfs_remove_recursive(ucd9000_debugfs_dir); + return pmbus_do_remove(client); +} + /* This is the driver that will be inserted */ static struct i2c_driver ucd9000_driver = { .driver = { .name = "ucd9000", }, .probe = ucd9000_probe, - .remove = pmbus_do_remove, + .remove = ucd9000_remove, .id_table = ucd9000_id, };
Expose via files in debugfs the gpiN_fault fields as well as the entire contents of the mfr_status register. Allows user space to pull this data for error logging etc... Signed-off-by: Christopher Bostic <cbostic@linux.vnet.ibm.com> --- v7 - Patch order changed from #3 to #2 and as a result needed to add the ucd9000_remove definition here. v6 - Remove chip idx values on remove so that we can retain the same indexes regardless how many times we're bound/unbound. - Remove type 90160 check when adding debugfs. v5 - Add unique debugfs dir ID for each ucd9000 sysfs device. - Change branching return to a !! method. - Clean up line breaks, other white space. - Add comments on assumptions made regarding ucd90160 versus other types. v2 - Remove mfr_status directory in debugfs and place the files up in the parent ucd9000 directory. --- drivers/hwmon/pmbus/ucd9000.c | 166 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 165 insertions(+), 1 deletion(-)