Message ID | 1509656381-22755-3-git-send-email-eajames@linux.vnet.ibm.com |
---|---|
State | Rejected, archived |
Headers | show |
Series | hwmon: (pmbus): ibm-cffps: Add debugfs entry for input history | expand |
Reviewed-by: Matt Spinler mspinler@linux.vnet.ibm.com On 11/2/2017 3:59 PM, Eddie James wrote: > From: "Edward A. James" <eajames@us.ibm.com> > > Provide binary data of the powers supply input history to debugfs. > > Signed-off-by: Edward A. James <eajames@us.ibm.com> > --- > drivers/hwmon/pmbus/ibm-cffps.c | 97 ++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 96 insertions(+), 1 deletion(-) > > diff --git a/drivers/hwmon/pmbus/ibm-cffps.c b/drivers/hwmon/pmbus/ibm-cffps.c > index 1a6294c..0570c12 100644 > --- a/drivers/hwmon/pmbus/ibm-cffps.c > +++ b/drivers/hwmon/pmbus/ibm-cffps.c > @@ -7,13 +7,19 @@ > * (at your option) any later version. > */ > > +#include <linux/debugfs.h> > #include <linux/device.h> > +#include <linux/fs.h> > #include <linux/i2c.h> > #include <linux/jiffies.h> > #include <linux/module.h> > +#include <linux/mutex.h> > > #include "pmbus.h" > > +#define CFFPS_INPUT_HISTORY_CMD 0xD6 > +#define CFFPS_INPUT_HISTORY_SIZE 100 > + > /* STATUS_MFR_SPECIFIC bits */ > #define CFFPS_MFR_FAN_FAULT BIT(0) > #define CFFPS_MFR_THERMAL_FAULT BIT(1) > @@ -24,6 +30,70 @@ > #define CFFPS_MFR_VAUX_FAULT BIT(6) > #define CFFPS_MFR_CURRENT_SHARE_WARNING BIT(7) > > +struct ibm_cffps { > + struct i2c_client *client; > + > + struct mutex update_lock; > + unsigned long last_updated; > + > + u8 byte_count; > + u8 input_history[CFFPS_INPUT_HISTORY_SIZE]; > +}; > + > +static ssize_t ibm_cffps_read_input_history(struct file *file, > + char __user *buf, size_t count, > + loff_t *ppos) > +{ > + int rc; > + struct ibm_cffps *psu = file->private_data; > + u8 msgbuf0[1] = { CFFPS_INPUT_HISTORY_CMD }; > + u8 msgbuf1[CFFPS_INPUT_HISTORY_SIZE + 1] = { 0 }; > + struct i2c_msg msg[2] = { > + { > + .addr = psu->client->addr, > + .flags = psu->client->flags, > + .len = 1, > + .buf = msgbuf0, > + }, { > + .addr = psu->client->addr, > + .flags = psu->client->flags | I2C_M_RD, > + .len = CFFPS_INPUT_HISTORY_SIZE + 1, > + .buf = msgbuf1, > + }, > + }; > + > + if (!*ppos) { > + mutex_lock(&psu->update_lock); > + if (time_after(jiffies, psu->last_updated + HZ)) { > + /* > + * Use a raw i2c transfer, since we need more bytes > + * than Linux I2C supports through smbus xfr (only 32). > + */ > + rc = i2c_transfer(psu->client->adapter, msg, 2); > + if (rc < 0) { > + mutex_unlock(&psu->update_lock); > + return rc; > + } > + > + psu->byte_count = msgbuf1[0]; > + memcpy(psu->input_history, &msgbuf1[1], > + CFFPS_INPUT_HISTORY_SIZE); > + psu->last_updated = jiffies; > + } > + > + mutex_unlock(&psu->update_lock); > + } > + > + return simple_read_from_buffer(buf, count, ppos, psu->input_history, > + psu->byte_count); > +} > + > +static const struct file_operations ibm_cffps_fops = { > + .llseek = noop_llseek, > + .read = ibm_cffps_read_input_history, > + .open = simple_open, > +}; > + > static int ibm_cffps_read_byte_data(struct i2c_client *client, int page, > int reg) > { > @@ -119,7 +189,32 @@ static int ibm_cffps_read_word_data(struct i2c_client *client, int page, > static int ibm_cffps_probe(struct i2c_client *client, > const struct i2c_device_id *id) > { > - return pmbus_do_probe(client, id, &ibm_cffps_info); > + int rc; > + struct dentry *debugfs; > + struct ibm_cffps *psu; > + > + rc = pmbus_do_probe(client, id, &ibm_cffps_info); > + if (rc) > + return rc; > + > + /* Don't fail the probe if we can't create debugfs */ > + psu = devm_kzalloc(&client->dev, sizeof(*psu), GFP_KERNEL); > + if (!psu) > + return 0; > + > + psu->client = client; > + mutex_init(&psu->update_lock); > + if (jiffies > HZ) > + psu->last_updated = jiffies - HZ; > + > + debugfs = pmbus_get_debugfs_dir(client); > + if (!debugfs) > + return 0; > + > + debugfs_create_file("ibm_cffps_input_history", 0444, debugfs, psu, > + &ibm_cffps_fops); > + > + return 0; > } > > static const struct i2c_device_id ibm_cffps_id[] = { <html> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> </head> <body text="#000000" bgcolor="#FFFFFF"> <p>Reviewed-by: Matt Spinler <a class="moz-txt-link-abbreviated" href="mailto:mspinler@linux.vnet.ibm.com">mspinler@linux.vnet.ibm.com</a></p> <br> <div class="moz-cite-prefix">On 11/2/2017 3:59 PM, Eddie James wrote:<br> </div> <blockquote type="cite" cite="mid:1509656381-22755-3-git-send-email-eajames@linux.vnet.ibm.com"> <pre wrap="">From: "Edward A. James" <a class="moz-txt-link-rfc2396E" href="mailto:eajames@us.ibm.com"><eajames@us.ibm.com></a> Provide binary data of the powers supply input history to debugfs. Signed-off-by: Edward A. James <a class="moz-txt-link-rfc2396E" href="mailto:eajames@us.ibm.com"><eajames@us.ibm.com></a> --- drivers/hwmon/pmbus/ibm-cffps.c | 97 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 96 insertions(+), 1 deletion(-) diff --git a/drivers/hwmon/pmbus/ibm-cffps.c b/drivers/hwmon/pmbus/ibm-cffps.c index 1a6294c..0570c12 100644 --- a/drivers/hwmon/pmbus/ibm-cffps.c +++ b/drivers/hwmon/pmbus/ibm-cffps.c @@ -7,13 +7,19 @@ * (at your option) any later version. */ +#include <linux/debugfs.h> #include <linux/device.h> +#include <linux/fs.h> #include <linux/i2c.h> #include <linux/jiffies.h> #include <linux/module.h> +#include <linux/mutex.h> #include "pmbus.h" +#define CFFPS_INPUT_HISTORY_CMD 0xD6 +#define CFFPS_INPUT_HISTORY_SIZE 100 + /* STATUS_MFR_SPECIFIC bits */ #define CFFPS_MFR_FAN_FAULT BIT(0) #define CFFPS_MFR_THERMAL_FAULT BIT(1) @@ -24,6 +30,70 @@ #define CFFPS_MFR_VAUX_FAULT BIT(6) #define CFFPS_MFR_CURRENT_SHARE_WARNING BIT(7) +struct ibm_cffps { + struct i2c_client *client; + + struct mutex update_lock; + unsigned long last_updated; + + u8 byte_count; + u8 input_history[CFFPS_INPUT_HISTORY_SIZE]; +}; + +static ssize_t ibm_cffps_read_input_history(struct file *file, + char __user *buf, size_t count, + loff_t *ppos) +{ + int rc; + struct ibm_cffps *psu = file->private_data; + u8 msgbuf0[1] = { CFFPS_INPUT_HISTORY_CMD }; + u8 msgbuf1[CFFPS_INPUT_HISTORY_SIZE + 1] = { 0 }; + struct i2c_msg msg[2] = { + { + .addr = psu->client->addr, + .flags = psu->client->flags, + .len = 1, + .buf = msgbuf0, + }, { + .addr = psu->client->addr, + .flags = psu->client->flags | I2C_M_RD, + .len = CFFPS_INPUT_HISTORY_SIZE + 1, + .buf = msgbuf1, + }, + }; + + if (!*ppos) { + mutex_lock(&psu->update_lock); + if (time_after(jiffies, psu->last_updated + HZ)) { + /* + * Use a raw i2c transfer, since we need more bytes + * than Linux I2C supports through smbus xfr (only 32). + */ + rc = i2c_transfer(psu->client->adapter, msg, 2); + if (rc < 0) { + mutex_unlock(&psu->update_lock); + return rc; + } + + psu->byte_count = msgbuf1[0]; + memcpy(psu->input_history, &msgbuf1[1], + CFFPS_INPUT_HISTORY_SIZE); + psu->last_updated = jiffies; + } + + mutex_unlock(&psu->update_lock); + } + + return simple_read_from_buffer(buf, count, ppos, psu->input_history, + psu->byte_count); +} + +static const struct file_operations ibm_cffps_fops = { + .llseek = noop_llseek, + .read = ibm_cffps_read_input_history, + .open = simple_open, +}; + static int ibm_cffps_read_byte_data(struct i2c_client *client, int page, int reg) { @@ -119,7 +189,32 @@ static int ibm_cffps_read_word_data(struct i2c_client *client, int page, static int ibm_cffps_probe(struct i2c_client *client, const struct i2c_device_id *id) { - return pmbus_do_probe(client, id, &ibm_cffps_info); + int rc; + struct dentry *debugfs; + struct ibm_cffps *psu; + + rc = pmbus_do_probe(client, id, &ibm_cffps_info); + if (rc) + return rc; + + /* Don't fail the probe if we can't create debugfs */ + psu = devm_kzalloc(&client->dev, sizeof(*psu), GFP_KERNEL); + if (!psu) + return 0; + + psu->client = client; + mutex_init(&psu->update_lock); + if (jiffies > HZ) + psu->last_updated = jiffies - HZ; + + debugfs = pmbus_get_debugfs_dir(client); + if (!debugfs) + return 0; + + debugfs_create_file("ibm_cffps_input_history", 0444, debugfs, psu, + &ibm_cffps_fops); + + return 0; } static const struct i2c_device_id ibm_cffps_id[] = { </pre> </blockquote> <br> </body> </html>
On Thu, 2017-11-02 at 15:59 -0500, Eddie James wrote: > From: "Edward A. James" <eajames@us.ibm.com> > > Provide binary data of the powers supply input history to debugfs. Please improve this commit message to discuss the finer points of the approach you took. For instance, getting the history off of the PSU involves some annoying quasi-duplication of core functions with the ibm_cffps_read_input_history() implementation. > > Signed-off-by: Edward A. James <eajames@us.ibm.com> > --- > drivers/hwmon/pmbus/ibm-cffps.c | 97 ++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 96 insertions(+), 1 deletion(-) > > diff --git a/drivers/hwmon/pmbus/ibm-cffps.c b/drivers/hwmon/pmbus/ibm-cffps.c > index 1a6294c..0570c12 100644 > --- a/drivers/hwmon/pmbus/ibm-cffps.c > +++ b/drivers/hwmon/pmbus/ibm-cffps.c > @@ -7,13 +7,19 @@ > * (at your option) any later version. > */ > > +#include <linux/debugfs.h> > #include <linux/device.h> > +#include <linux/fs.h> > #include <linux/i2c.h> > #include <linux/jiffies.h> > #include <linux/module.h> > +#include <linux/mutex.h> > > #include "pmbus.h" > > +#define CFFPS_INPUT_HISTORY_CMD 0xD6 > +#define CFFPS_INPUT_HISTORY_SIZE 100 > + > /* STATUS_MFR_SPECIFIC bits */ > #define CFFPS_MFR_FAN_FAULT BIT(0) > #define CFFPS_MFR_THERMAL_FAULT BIT(1) > @@ -24,6 +30,70 @@ > #define CFFPS_MFR_VAUX_FAULT BIT(6) > #define CFFPS_MFR_CURRENT_SHARE_WARNING BIT(7) > > +struct ibm_cffps { > + struct i2c_client *client; > + > + struct mutex update_lock; > + unsigned long last_updated; > + > + u8 byte_count; > + u8 input_history[CFFPS_INPUT_HISTORY_SIZE]; > +}; > + > +static ssize_t ibm_cffps_read_input_history(struct file *file, > + char __user *buf, size_t count, > + loff_t *ppos) > +{ > + int rc; > + struct ibm_cffps *psu = file->private_data; > + u8 msgbuf0[1] = { CFFPS_INPUT_HISTORY_CMD }; > + u8 msgbuf1[CFFPS_INPUT_HISTORY_SIZE + 1] = { 0 }; > + struct i2c_msg msg[2] = { > + { > + .addr = psu->client->addr, > + .flags = psu->client->flags, > + .len = 1, > + .buf = msgbuf0, > + }, { > + .addr = psu->client->addr, > + .flags = psu->client->flags | I2C_M_RD, > + .len = CFFPS_INPUT_HISTORY_SIZE + 1, > + .buf = msgbuf1, > + }, > + }; > + > + if (!*ppos) { > + mutex_lock(&psu->update_lock); > + if (time_after(jiffies, psu->last_updated + HZ)) { > + /* > + * Use a raw i2c transfer, since we need more bytes > + * than Linux I2C supports through smbus xfr (only 32). > + */ > + rc = i2c_transfer(psu->client->adapter, msg, 2); > + if (rc < 0) { > + mutex_unlock(&psu->update_lock); > + return rc; > + } > + > + psu->byte_count = msgbuf1[0]; > + memcpy(psu->input_history, &msgbuf1[1], > + CFFPS_INPUT_HISTORY_SIZE); > + psu->last_updated = jiffies; > + } > + > + mutex_unlock(&psu->update_lock); > + } > + > + return simple_read_from_buffer(buf, count, ppos, psu->input_history, > + psu->byte_count); > +} > + > +static const struct file_operations ibm_cffps_fops = { > + .llseek = noop_llseek, > + .read = ibm_cffps_read_input_history, > + .open = simple_open, > +}; > + > static int ibm_cffps_read_byte_data(struct i2c_client *client, int page, > int reg) > { > @@ -119,7 +189,32 @@ static int ibm_cffps_read_word_data(struct i2c_client *client, int page, > static int ibm_cffps_probe(struct i2c_client *client, > const struct i2c_device_id *id) > { > - return pmbus_do_probe(client, id, &ibm_cffps_info); > + int rc; > + struct dentry *debugfs; > + struct ibm_cffps *psu; > + > + rc = pmbus_do_probe(client, id, &ibm_cffps_info); > + if (rc) > + return rc; > + > + /* Don't fail the probe if we can't create debugfs */ > + psu = devm_kzalloc(&client->dev, sizeof(*psu), GFP_KERNEL); > + if (!psu) > + return 0; > + > + psu->client = client; > + mutex_init(&psu->update_lock); > + if (jiffies > HZ) > + psu->last_updated = jiffies - HZ; I think we can ditch the test: jiffies is an unsigned long and unsigned underflow is defined as wrapping. If jiffies <= HZ, then jiffies - HZ becomes (jiffies - HZ) + ULONG_MAX + 1, which can be rewritten as ULONG_MAX - HZ + jiffies + 1 to keep your sanity in an unsigned world. Then because you're using time_after() in the read() function, the wrapping is taken care of in the jiffies <= HZ case (and naturally the jiffies > HZ case is catered for), and we're home. > + > + debugfs = pmbus_get_debugfs_dir(client); > + if (!debugfs) > + return 0; > + > + debugfs_create_file("ibm_cffps_input_history", 0444, debugfs, psu, > + &ibm_cffps_fops); > + > + return 0; > } > > static const struct i2c_device_id ibm_cffps_id[] = { Please send this patch upstream as well. We'll cherry-pick it back after it's applied. Cheers, Andrew
diff --git a/drivers/hwmon/pmbus/ibm-cffps.c b/drivers/hwmon/pmbus/ibm-cffps.c index 1a6294c..0570c12 100644 --- a/drivers/hwmon/pmbus/ibm-cffps.c +++ b/drivers/hwmon/pmbus/ibm-cffps.c @@ -7,13 +7,19 @@ * (at your option) any later version. */ +#include <linux/debugfs.h> #include <linux/device.h> +#include <linux/fs.h> #include <linux/i2c.h> #include <linux/jiffies.h> #include <linux/module.h> +#include <linux/mutex.h> #include "pmbus.h" +#define CFFPS_INPUT_HISTORY_CMD 0xD6 +#define CFFPS_INPUT_HISTORY_SIZE 100 + /* STATUS_MFR_SPECIFIC bits */ #define CFFPS_MFR_FAN_FAULT BIT(0) #define CFFPS_MFR_THERMAL_FAULT BIT(1) @@ -24,6 +30,70 @@ #define CFFPS_MFR_VAUX_FAULT BIT(6) #define CFFPS_MFR_CURRENT_SHARE_WARNING BIT(7) +struct ibm_cffps { + struct i2c_client *client; + + struct mutex update_lock; + unsigned long last_updated; + + u8 byte_count; + u8 input_history[CFFPS_INPUT_HISTORY_SIZE]; +}; + +static ssize_t ibm_cffps_read_input_history(struct file *file, + char __user *buf, size_t count, + loff_t *ppos) +{ + int rc; + struct ibm_cffps *psu = file->private_data; + u8 msgbuf0[1] = { CFFPS_INPUT_HISTORY_CMD }; + u8 msgbuf1[CFFPS_INPUT_HISTORY_SIZE + 1] = { 0 }; + struct i2c_msg msg[2] = { + { + .addr = psu->client->addr, + .flags = psu->client->flags, + .len = 1, + .buf = msgbuf0, + }, { + .addr = psu->client->addr, + .flags = psu->client->flags | I2C_M_RD, + .len = CFFPS_INPUT_HISTORY_SIZE + 1, + .buf = msgbuf1, + }, + }; + + if (!*ppos) { + mutex_lock(&psu->update_lock); + if (time_after(jiffies, psu->last_updated + HZ)) { + /* + * Use a raw i2c transfer, since we need more bytes + * than Linux I2C supports through smbus xfr (only 32). + */ + rc = i2c_transfer(psu->client->adapter, msg, 2); + if (rc < 0) { + mutex_unlock(&psu->update_lock); + return rc; + } + + psu->byte_count = msgbuf1[0]; + memcpy(psu->input_history, &msgbuf1[1], + CFFPS_INPUT_HISTORY_SIZE); + psu->last_updated = jiffies; + } + + mutex_unlock(&psu->update_lock); + } + + return simple_read_from_buffer(buf, count, ppos, psu->input_history, + psu->byte_count); +} + +static const struct file_operations ibm_cffps_fops = { + .llseek = noop_llseek, + .read = ibm_cffps_read_input_history, + .open = simple_open, +}; + static int ibm_cffps_read_byte_data(struct i2c_client *client, int page, int reg) { @@ -119,7 +189,32 @@ static int ibm_cffps_read_word_data(struct i2c_client *client, int page, static int ibm_cffps_probe(struct i2c_client *client, const struct i2c_device_id *id) { - return pmbus_do_probe(client, id, &ibm_cffps_info); + int rc; + struct dentry *debugfs; + struct ibm_cffps *psu; + + rc = pmbus_do_probe(client, id, &ibm_cffps_info); + if (rc) + return rc; + + /* Don't fail the probe if we can't create debugfs */ + psu = devm_kzalloc(&client->dev, sizeof(*psu), GFP_KERNEL); + if (!psu) + return 0; + + psu->client = client; + mutex_init(&psu->update_lock); + if (jiffies > HZ) + psu->last_updated = jiffies - HZ; + + debugfs = pmbus_get_debugfs_dir(client); + if (!debugfs) + return 0; + + debugfs_create_file("ibm_cffps_input_history", 0444, debugfs, psu, + &ibm_cffps_fops); + + return 0; } static const struct i2c_device_id ibm_cffps_id[] = {