hwmon: (pmbus): ibm-cffps: Add debugfs entry for input history
cover hwmon: (pmbus): ibm-cffps: Add debugfs entry for input history
From: "Edward A. James" <[email protected]>
Add a debugfs entry to dump the binary from the power supply input history. To do this, I needed to export the pmbus device debugfs directory from pmbus core.
Edward A. James (2):
hwmon: (pmbus): Export pmbus device debugfs directory entry
hwmon: (pmbus): cffps: Add input_history debugfs file
drivers/hwmon/pmbus/ibm-cffps.c | 97 +++++++++++++++++++++++++++++++++++++++- drivers/hwmon/pmbus/pmbus.h | 1 + drivers/hwmon/pmbus/pmbus_core.c | 7 +++ 3 files changed, 104 insertions(+), 1 deletion(-)
Signature · 1 line
1.8.3.1
PATCH 1/2 hwmon: (pmbus): Export pmbus device debugfs directory entry
From: "Edward A. James" <[email protected]>
Pmbus client drivers, if they want to use debugfs, should use the same root directory as the pmbus debugfs entries are using. Therefore, export the device dentry for the pmbus client.
Signed-off-by Edward A. James <[email protected]>
--- drivers/hwmon/pmbus/pmbus.h | 1 + drivers/hwmon/pmbus/pmbus_core.c | 7 +++++++ 2 files changed, 8 insertions(+)
drivers/hwmon/pmbus/pmbus.h +1 -0
@@ -452,5 +452,6 @@ int pmbus_do_probe(struct i2c_client *client, const struct i2c_device_id *id,int pmbus_do_remove (struct i2c_client *client);const struct pmbus_driver_info *pmbus_get_driver_info (struct i2c_client *client); +struct dentry *pmbus_get_debugfs_dir (struct i2c_client *client);#endif /* PMBUS_H */
drivers/hwmon/pmbus/pmbus_core.c +7 -0
@@ -2371,6 +2371,13 @@ int pmbus_do_remove(struct i2c_client *client) }EXPORT_SYMBOL_GPL (pmbus_do_remove); +struct dentry *pmbus_get_debugfs_dir (struct i2c_client *client) +{ +struct pmbus_data *data =i2c_get_clientdata (client); + +return data->debugfs; +} +static int __init pmbus_core_init (void ) { pmbus_debugfs_dir =debugfs_create_dir ("pmbus" ,NULL );
Signature · 1 line
1.8.3.1
Re: [PATCH linux dev-4.10 1/2] hwmon: (pmbus): Export pmbus device debugfs directory entry
Reviewed-by Matt Spinler [email protected]
Quoted 41 lines · Eddie James
From: "Edward A. James" <[email protected]>
Pmbus client drivers, if they want to use debugfs, should use the same root directory as the pmbus debugfs entries are using. Therefore, export the device dentry for the pmbus client.
Signed-off-by Edward A. James <[email protected]>--- drivers/hwmon/pmbus/pmbus.h | 1 + drivers/hwmon/pmbus/pmbus_core.c | 7 +++++++ 2 files changed, 8 insertions(+)drivers/hwmon/pmbus/pmbus.h +1
-0@@ -452,5 +452,6 @@ int pmbus_do_probe(struct i2c_client *client, const struct i2c_device_id *id,int pmbus_do_remove (struct i2c_client *client);const struct pmbus_driver_info *pmbus_get_driver_info (struct i2c_client *client); +struct dentry *pmbus_get_debugfs_dir (struct i2c_client *client);#endif /* PMBUS_H */ drivers/hwmon/pmbus/pmbus_core.c +7
-0@@ -2371,6 +2371,13 @@ int pmbus_do_remove(struct i2c_client *client) }EXPORT_SYMBOL_GPL (pmbus_do_remove); +struct dentry *pmbus_get_debugfs_dir (struct i2c_client *client) +{ +struct pmbus_data *data =i2c_get_clientdata (client); + +return data->debugfs; +} +static int __init pmbus_core_init (void ) { pmbus_debugfs_dir =debugfs_create_dir ("pmbus" ,NULL );
Re: [PATCH linux dev-4.10 1/2] hwmon: (pmbus): Export pmbus device debugfs directory entry
In reply to Eddie JamesQuoted 7 lines · Eddie James
From: "Edward A. James" <[email protected]>
Pmbus client drivers, if they want to use debugfs, should use the same root directory as the pmbus debugfs entries are using. Therefore, export the device dentry for the pmbus client.
Signed-off-by Edward A. James <[email protected]>
I prefer you send this upstream and we cherry-pick it back after it gets applied. I can't see it being controversial too.
Cheers,
Andrew
Quoted 34 lines
--- drivers/hwmon/pmbus/pmbus.h | 1 + drivers/hwmon/pmbus/pmbus_core.c | 7 +++++++ 2 files changed, 8 insertions(+)drivers/hwmon/pmbus/pmbus.h +1
-0@@ -452,5 +452,6 @@ int pmbus_do_probe(struct i2c_client *client, const struct i2c_device_id *id,int pmbus_do_remove (struct i2c_client *client);const struct pmbus_driver_info *pmbus_get_driver_info (struct i2c_client *client); +struct dentry *pmbus_get_debugfs_dir (struct i2c_client *client);#endif /* PMBUS_H */ drivers/hwmon/pmbus/pmbus_core.c +7
-0@@ -2371,6 +2371,13 @@ int pmbus_do_remove(struct i2c_client *client) }EXPORT_SYMBOL_GPL (pmbus_do_remove); +struct dentry *pmbus_get_debugfs_dir (struct i2c_client *client) +{ +struct pmbus_data *data =i2c_get_clientdata (client); + +return data->debugfs; +} +static int __init pmbus_core_init (void ) { pmbus_debugfs_dir =debugfs_create_dir ("pmbus" ,NULL );
PATCH 2/2 hwmon: (pmbus): cffps: Add input_history debugfs file
In reply to Eddie JamesFrom: "Edward A. James" <[email protected]>
Provide binary data of the powers supply input history to debugfs.
Signed-off-by Edward A. James <[email protected]>
--- drivers/hwmon/pmbus/ibm-cffps.c | 97 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 96 insertions(+), 1 deletion(-)
drivers/hwmon/pmbus/ibm-cffps.c +96 -1
@@ -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[] = {
Signature · 1 line
1.8.3.1
Re: [PATCH linux dev-4.10 2/2] hwmon: (pmbus): cffps: Add input_history debugfs file
Reviewed-by Matt Spinler [email protected]
Quoted 138 lines · Eddie James
From: "Edward A. James" <[email protected]>
Provide binary data of the powers supply input history to debugfs.
Signed-off-by Edward A. James <[email protected]>--- drivers/hwmon/pmbus/ibm-cffps.c | 97 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 96 insertions(+), 1 deletion(-)drivers/hwmon/pmbus/ibm-cffps.c +96
-1@@ -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[] = {
Re: [PATCH linux dev-4.10 2/2] hwmon: (pmbus): cffps: Add input_history debugfs file
In reply to Eddie JamesQuoted 3 lines · Eddie James
From: "Edward A. James" <[email protected]>
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.
Quoted 113 lines
Signed-off-by Edward A. James <[email protected]>--- drivers/hwmon/pmbus/ibm-cffps.c | 97 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 96 insertions(+), 1 deletion(-)drivers/hwmon/pmbus/ibm-cffps.c +77
-1@@ -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;
drivers/hwmon/pmbus/ibm-cffps.c static int ibm_cffps_read_word_data(struct i2c_client *client, int page, · line 208
Quoted 10 lines
drivers/hwmon/pmbus/ibm-cffps.c +10
-0+ +/* 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.
drivers/hwmon/pmbus/ibm-cffps.c static int ibm_cffps_read_word_data(struct i2c_client *client, int page, · line 220
Quoted 12 lines
drivers/hwmon/pmbus/ibm-cffps.c +9
-0+ + 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