| Message ID | 20220817054321.6519-17-farbere@amazon.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Variety of fixes and new features for mr75203 driver | expand |
On Wed, Aug 17, 2022 at 05:43:21AM +0000, Eliav Farber wrote: > This change adds debugfs to read and write TS coefficients - g, h, j and > cal5. > > The coefficients can vary between product and product, so to calibrate > them it can be very useful to to be able to modify them on the fly. > > e.g. > > cat /sys/kernel/debug/940f23d0000.pvt/ts_coeff_cal5 > 4096 > > echo 83000 > sys/kernel/debug/940f23d0000.pvt/ts_coeff_g > What happens if you write 0 into all those attributes, or 0xffffffff ? Guenter > Signed-off-by: Eliav Farber <farbere@amazon.com> > --- > drivers/hwmon/mr75203.c | 196 ++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 196 insertions(+) > > diff --git a/drivers/hwmon/mr75203.c b/drivers/hwmon/mr75203.c > index 2898565afaab..ce34a44237e8 100644 > --- a/drivers/hwmon/mr75203.c > +++ b/drivers/hwmon/mr75203.c > @@ -9,6 +9,7 @@ > */ > #include <linux/bits.h> > #include <linux/clk.h> > +#include <linux/debugfs.h> > #include <linux/hwmon.h> > #include <linux/module.h> > #include <linux/mod_devicetable.h> > @@ -127,6 +128,7 @@ struct pvt_device { > struct clk *clk; > struct reset_control *rst; > struct voltage_device *vd; > + struct dentry *dbgfs_dir; > u32 t_num; > u32 p_num; > u32 v_num; > @@ -139,6 +141,198 @@ struct pvt_device { > u8 vm_ch_total; > }; > > +static ssize_t pvt_ts_coeff_h_read(struct file *file, > + char __user *user_buf, > + size_t count, loff_t *ppos) > +{ > + struct pvt_device *pvt = file->private_data; > + char buf[16]; > + unsigned int len; > + > + len = sprintf(buf, "%u\n", pvt->ts_coeff_h); > + > + return simple_read_from_buffer(user_buf, count, ppos, buf, len); > +} > + > +static ssize_t pvt_ts_coeff_h_write(struct file *file, > + const char __user *user_buf, > + size_t count, loff_t *ppos) > +{ > + struct pvt_device *pvt = file->private_data; > + int ret; > + u32 coeff; > + > + ret = kstrtou32_from_user(user_buf, count, 0, &coeff); > + if (ret) > + return ret; > + > + pvt->ts_coeff_h = coeff; > + > + return count; > +} > + > +static const struct file_operations pvt_ts_coeff_h_fops = { > + .read = pvt_ts_coeff_h_read, > + .write = pvt_ts_coeff_h_write, > + .open = simple_open, > + .owner = THIS_MODULE, > + .llseek = default_llseek, > +}; > + > +static ssize_t pvt_ts_coeff_g_read(struct file *file, > + char __user *user_buf, > + size_t count, loff_t *ppos) > +{ > + struct pvt_device *pvt = file->private_data; > + char buf[16]; > + unsigned int len; > + > + len = sprintf(buf, "%u\n", pvt->ts_coeff_g); > + > + return simple_read_from_buffer(user_buf, count, ppos, buf, len); > +} > + > +static ssize_t pvt_ts_coeff_g_write(struct file *file, > + const char __user *user_buf, > + size_t count, loff_t *ppos) > +{ > + struct pvt_device *pvt = file->private_data; > + int ret; > + u32 coeff; > + > + ret = kstrtou32_from_user(user_buf, count, 0, &coeff); > + if (ret) > + return ret; > + > + pvt->ts_coeff_g = coeff; > + > + return count; > +} > + > +static const struct file_operations pvt_ts_coeff_g_fops = { > + .read = pvt_ts_coeff_g_read, > + .write = pvt_ts_coeff_g_write, > + .open = simple_open, > + .owner = THIS_MODULE, > + .llseek = default_llseek, > +}; > + > +static ssize_t pvt_ts_coeff_j_read(struct file *file, > + char __user *user_buf, > + size_t count, loff_t *ppos) > +{ > + struct pvt_device *pvt = file->private_data; > + char buf[16]; > + unsigned int len; > + > + len = sprintf(buf, "%d\n", pvt->ts_coeff_j); > + > + return simple_read_from_buffer(user_buf, count, ppos, buf, len); > +} > + > +static ssize_t pvt_ts_coeff_j_write(struct file *file, > + const char __user *user_buf, > + size_t count, loff_t *ppos) > +{ > + struct pvt_device *pvt = file->private_data; > + int ret; > + s32 coeff; > + > + ret = kstrtos32_from_user(user_buf, count, 0, &coeff); > + if (ret) > + return ret; > + > + pvt->ts_coeff_j = coeff; > + > + return count; > +} > + > +static const struct file_operations pvt_ts_coeff_j_fops = { > + .read = pvt_ts_coeff_j_read, > + .write = pvt_ts_coeff_j_write, > + .open = simple_open, > + .owner = THIS_MODULE, > + .llseek = default_llseek, > +}; > + > +static ssize_t pvt_ts_coeff_cal5_read(struct file *file, > + char __user *user_buf, > + size_t count, loff_t *ppos) > +{ > + struct pvt_device *pvt = file->private_data; > + char buf[16]; > + unsigned int len; > + > + len = sprintf(buf, "%u\n", pvt->ts_coeff_cal5); > + > + return simple_read_from_buffer(user_buf, count, ppos, buf, len); > +} > + > +static ssize_t pvt_ts_coeff_cal5_write(struct file *file, > + const char __user *user_buf, > + size_t count, loff_t *ppos) > +{ > + struct pvt_device *pvt = file->private_data; > + int ret; > + u32 coeff; > + > + ret = kstrtou32_from_user(user_buf, count, 0, &coeff); > + if (ret) > + return ret; > + > + if (coeff == 0) > + return -EINVAL; > + > + pvt->ts_coeff_cal5 = coeff; > + > + return count; > +} > + > +static const struct file_operations pvt_ts_coeff_cal5_fops = { > + .read = pvt_ts_coeff_cal5_read, > + .write = pvt_ts_coeff_cal5_write, > + .open = simple_open, > + .owner = THIS_MODULE, > + .llseek = default_llseek, > +}; > + > +static void devm_pvt_ts_dbgfs_remove(void *data) > +{ > + struct pvt_device *pvt = (struct pvt_device *)data; > + > + debugfs_remove_recursive(pvt->dbgfs_dir); > + pvt->dbgfs_dir = NULL; > +} > + > +static int pvt_ts_dbgfs_create(struct pvt_device *pvt, struct device *dev) > +{ > + int ret; > + > + pvt->dbgfs_dir = debugfs_create_dir(dev_name(dev), NULL); > + if (!pvt->dbgfs_dir) { > + dev_err(dev, "Failed to create dbgfs_dir\n"); > + return -EINVAL; > + } > + > + debugfs_create_file("ts_coeff_h", 0644, pvt->dbgfs_dir, pvt, > + &pvt_ts_coeff_h_fops); > + debugfs_create_file("ts_coeff_g", 0644, pvt->dbgfs_dir, pvt, > + &pvt_ts_coeff_g_fops); > + debugfs_create_file("ts_coeff_j", 0644, pvt->dbgfs_dir, pvt, > + &pvt_ts_coeff_j_fops); > + debugfs_create_file("ts_coeff_cal5", 0644, pvt->dbgfs_dir, pvt, > + &pvt_ts_coeff_cal5_fops); > + > + ret = devm_add_action_or_reset(dev, devm_pvt_ts_dbgfs_remove, pvt); > + if (ret) { > + dev_err(dev, "failed to add action to remove pvt dbgfs (%d)\n", > + ret); > + return ret; > + } > + > + return 0; > +} > + > static umode_t pvt_is_visible(const void *data, enum hwmon_sensor_types type, > u32 attr, int channel) > { > @@ -655,6 +849,8 @@ static int mr75203_probe(struct platform_device *pdev) > dev_dbg(dev, "ts-coeff: h = %u, g = %u, j = %d, cal5 = %u\n", > pvt->ts_coeff_h, pvt->ts_coeff_g, pvt->ts_coeff_j, > pvt->ts_coeff_cal5); > + > + pvt_ts_dbgfs_create(pvt, dev); > } > > if (pd_num) {
On 8/19/2022 2:11 AM, Guenter Roeck wrote: > On Wed, Aug 17, 2022 at 05:43:21AM +0000, Eliav Farber wrote: >> This change adds debugfs to read and write TS coefficients - g, h, j and >> cal5. >> >> The coefficients can vary between product and product, so to calibrate >> them it can be very useful to to be able to modify them on the fly. >> >> e.g. >> >> cat /sys/kernel/debug/940f23d0000.pvt/ts_coeff_cal5 >> 4096 >> >> echo 83000 > sys/kernel/debug/940f23d0000.pvt/ts_coeff_g >> > > What happens if you write 0 into all those attributes, or 0xffffffff ? The driver equation is: T = G + H * (n / cal5 - 0.5) + J * F So I added protection for cal5 not being 0. Besides that there is no limitation on what these values can be. I can't really think of any other logical limitation I can apply. -- Regards, Eliav
On Mon, Aug 22, 2022 at 04:59:43PM +0300, Farber, Eliav wrote: > On 8/19/2022 2:11 AM, Guenter Roeck wrote: > > On Wed, Aug 17, 2022 at 05:43:21AM +0000, Eliav Farber wrote: > > > This change adds debugfs to read and write TS coefficients - g, h, j and > > > cal5. > > > > > > The coefficients can vary between product and product, so to calibrate > > > them it can be very useful to to be able to modify them on the fly. > > > > > > e.g. > > > > > > cat /sys/kernel/debug/940f23d0000.pvt/ts_coeff_cal5 > > > 4096 > > > > > > echo 83000 > sys/kernel/debug/940f23d0000.pvt/ts_coeff_g > > > > > > > What happens if you write 0 into all those attributes, or 0xffffffff ? > The driver equation is: > T = G + H * (n / cal5 - 0.5) + J * F > So I added protection for cal5 not being 0. > Besides that there is no limitation on what these values can be. > I can't really think of any other logical limitation I can apply. > There needs to be an overflow protection. I am quite sure that 0xffffffff would result in overflows and thus in quite random reported values. Thanks, Guenter > -- > Regards, Eliav
On 8/22/2022 7:28 PM, Guenter Roeck wrote: > There needs to be an overflow protection. I am quite sure that 0xffffffff > would result in overflows and thus in quite random reported values. Added overflow protection. Will be part of v3. -- Thanks, Eliav
diff --git a/drivers/hwmon/mr75203.c b/drivers/hwmon/mr75203.c index 2898565afaab..ce34a44237e8 100644 --- a/drivers/hwmon/mr75203.c +++ b/drivers/hwmon/mr75203.c @@ -9,6 +9,7 @@ */ #include <linux/bits.h> #include <linux/clk.h> +#include <linux/debugfs.h> #include <linux/hwmon.h> #include <linux/module.h> #include <linux/mod_devicetable.h> @@ -127,6 +128,7 @@ struct pvt_device { struct clk *clk; struct reset_control *rst; struct voltage_device *vd; + struct dentry *dbgfs_dir; u32 t_num; u32 p_num; u32 v_num; @@ -139,6 +141,198 @@ struct pvt_device { u8 vm_ch_total; }; +static ssize_t pvt_ts_coeff_h_read(struct file *file, + char __user *user_buf, + size_t count, loff_t *ppos) +{ + struct pvt_device *pvt = file->private_data; + char buf[16]; + unsigned int len; + + len = sprintf(buf, "%u\n", pvt->ts_coeff_h); + + return simple_read_from_buffer(user_buf, count, ppos, buf, len); +} + +static ssize_t pvt_ts_coeff_h_write(struct file *file, + const char __user *user_buf, + size_t count, loff_t *ppos) +{ + struct pvt_device *pvt = file->private_data; + int ret; + u32 coeff; + + ret = kstrtou32_from_user(user_buf, count, 0, &coeff); + if (ret) + return ret; + + pvt->ts_coeff_h = coeff; + + return count; +} + +static const struct file_operations pvt_ts_coeff_h_fops = { + .read = pvt_ts_coeff_h_read, + .write = pvt_ts_coeff_h_write, + .open = simple_open, + .owner = THIS_MODULE, + .llseek = default_llseek, +}; + +static ssize_t pvt_ts_coeff_g_read(struct file *file, + char __user *user_buf, + size_t count, loff_t *ppos) +{ + struct pvt_device *pvt = file->private_data; + char buf[16]; + unsigned int len; + + len = sprintf(buf, "%u\n", pvt->ts_coeff_g); + + return simple_read_from_buffer(user_buf, count, ppos, buf, len); +} + +static ssize_t pvt_ts_coeff_g_write(struct file *file, + const char __user *user_buf, + size_t count, loff_t *ppos) +{ + struct pvt_device *pvt = file->private_data; + int ret; + u32 coeff; + + ret = kstrtou32_from_user(user_buf, count, 0, &coeff); + if (ret) + return ret; + + pvt->ts_coeff_g = coeff; + + return count; +} + +static const struct file_operations pvt_ts_coeff_g_fops = { + .read = pvt_ts_coeff_g_read, + .write = pvt_ts_coeff_g_write, + .open = simple_open, + .owner = THIS_MODULE, + .llseek = default_llseek, +}; + +static ssize_t pvt_ts_coeff_j_read(struct file *file, + char __user *user_buf, + size_t count, loff_t *ppos) +{ + struct pvt_device *pvt = file->private_data; + char buf[16]; + unsigned int len; + + len = sprintf(buf, "%d\n", pvt->ts_coeff_j); + + return simple_read_from_buffer(user_buf, count, ppos, buf, len); +} + +static ssize_t pvt_ts_coeff_j_write(struct file *file, + const char __user *user_buf, + size_t count, loff_t *ppos) +{ + struct pvt_device *pvt = file->private_data; + int ret; + s32 coeff; + + ret = kstrtos32_from_user(user_buf, count, 0, &coeff); + if (ret) + return ret; + + pvt->ts_coeff_j = coeff; + + return count; +} + +static const struct file_operations pvt_ts_coeff_j_fops = { + .read = pvt_ts_coeff_j_read, + .write = pvt_ts_coeff_j_write, + .open = simple_open, + .owner = THIS_MODULE, + .llseek = default_llseek, +}; + +static ssize_t pvt_ts_coeff_cal5_read(struct file *file, + char __user *user_buf, + size_t count, loff_t *ppos) +{ + struct pvt_device *pvt = file->private_data; + char buf[16]; + unsigned int len; + + len = sprintf(buf, "%u\n", pvt->ts_coeff_cal5); + + return simple_read_from_buffer(user_buf, count, ppos, buf, len); +} + +static ssize_t pvt_ts_coeff_cal5_write(struct file *file, + const char __user *user_buf, + size_t count, loff_t *ppos) +{ + struct pvt_device *pvt = file->private_data; + int ret; + u32 coeff; + + ret = kstrtou32_from_user(user_buf, count, 0, &coeff); + if (ret) + return ret; + + if (coeff == 0) + return -EINVAL; + + pvt->ts_coeff_cal5 = coeff; + + return count; +} + +static const struct file_operations pvt_ts_coeff_cal5_fops = { + .read = pvt_ts_coeff_cal5_read, + .write = pvt_ts_coeff_cal5_write, + .open = simple_open, + .owner = THIS_MODULE, + .llseek = default_llseek, +}; + +static void devm_pvt_ts_dbgfs_remove(void *data) +{ + struct pvt_device *pvt = (struct pvt_device *)data; + + debugfs_remove_recursive(pvt->dbgfs_dir); + pvt->dbgfs_dir = NULL; +} + +static int pvt_ts_dbgfs_create(struct pvt_device *pvt, struct device *dev) +{ + int ret; + + pvt->dbgfs_dir = debugfs_create_dir(dev_name(dev), NULL); + if (!pvt->dbgfs_dir) { + dev_err(dev, "Failed to create dbgfs_dir\n"); + return -EINVAL; + } + + debugfs_create_file("ts_coeff_h", 0644, pvt->dbgfs_dir, pvt, + &pvt_ts_coeff_h_fops); + debugfs_create_file("ts_coeff_g", 0644, pvt->dbgfs_dir, pvt, + &pvt_ts_coeff_g_fops); + debugfs_create_file("ts_coeff_j", 0644, pvt->dbgfs_dir, pvt, + &pvt_ts_coeff_j_fops); + debugfs_create_file("ts_coeff_cal5", 0644, pvt->dbgfs_dir, pvt, + &pvt_ts_coeff_cal5_fops); + + ret = devm_add_action_or_reset(dev, devm_pvt_ts_dbgfs_remove, pvt); + if (ret) { + dev_err(dev, "failed to add action to remove pvt dbgfs (%d)\n", + ret); + return ret; + } + + return 0; +} + static umode_t pvt_is_visible(const void *data, enum hwmon_sensor_types type, u32 attr, int channel) { @@ -655,6 +849,8 @@ static int mr75203_probe(struct platform_device *pdev) dev_dbg(dev, "ts-coeff: h = %u, g = %u, j = %d, cal5 = %u\n", pvt->ts_coeff_h, pvt->ts_coeff_g, pvt->ts_coeff_j, pvt->ts_coeff_cal5); + + pvt_ts_dbgfs_create(pvt, dev); } if (pd_num) {
This change adds debugfs to read and write TS coefficients - g, h, j and cal5. The coefficients can vary between product and product, so to calibrate them it can be very useful to to be able to modify them on the fly. e.g. cat /sys/kernel/debug/940f23d0000.pvt/ts_coeff_cal5 4096 echo 83000 > sys/kernel/debug/940f23d0000.pvt/ts_coeff_g Signed-off-by: Eliav Farber <farbere@amazon.com> --- drivers/hwmon/mr75203.c | 196 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 196 insertions(+)