| Message ID | 203a7cd37dd95a0127cc06de14986721ab621e93.1587408025.git.grantpeltier93@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | hwmon: (pmbus) add debugfs features for Gen 2 Renesas digital multiphase | expand |
On Mon, Apr 20, 2020 at 02:03:36PM -0500, Grant Peltier wrote: > Add debugfs endpoints to support features of 2nd generation Renesas > digital multiphase voltage regulators that are not compatible with > sysfs. > > The read_black_box endpoint allows users to read the contents of a > RAM segment used to record fault conditions within Gen 2 devices. > > The write_config endpoint allows users to write configuration hex > files to Gen 2 devices which modify how they operate. > > Signed-off-by: Grant Peltier <grantpeltier93@gmail.com> > --- > drivers/hwmon/pmbus/isl68137.c | 490 ++++++++++++++++++++++++++++++++- > 1 file changed, 487 insertions(+), 3 deletions(-) > > diff --git a/drivers/hwmon/pmbus/isl68137.c b/drivers/hwmon/pmbus/isl68137.c > index 0c622711ef7e..24af016729a3 100644 > --- a/drivers/hwmon/pmbus/isl68137.c > +++ b/drivers/hwmon/pmbus/isl68137.c > @@ -7,10 +7,12 @@ > * > */ > > +#include <linux/debugfs.h> > #include <linux/err.h> > #include <linux/hwmon-sysfs.h> > #include <linux/i2c.h> > #include <linux/init.h> > +#include <linux/jiffies.h> > #include <linux/kernel.h> > #include <linux/module.h> > #include <linux/string.h> > @@ -18,8 +20,22 @@ > > #include "pmbus.h" > > -#define ISL68137_VOUT_AVS 0x30 > -#define RAA_DMPVR2_READ_VMON 0xc8 > +#define ISL68137_VOUT_AVS 0x30 > +#define RAA_DMPVR2_DMA_FIX 0xc5 > +#define RAA_DMPVR2_DMA_SEQ 0xc6 > +#define RAA_DMPVR2_DMA_ADDR 0xc7 > +#define RAA_DMPVR2_READ_VMON 0xc8 > +#define RAA_DMPVR2_BB_BASE_ADDR 0xef80 > +#define RAA_DMPVR2_BB_WSIZE 4 > +#define RAA_DMPVR2_BB_WCNT 32 > +#define RAA_DMPVR2_BB_BUF_SIZE 288 > +#define RAA_DMPVR2_NVM_CNT_ADDR 0x00c2 > +#define RAA_DMPVR2_PRGM_STATUS_ADDR 0x0707 > +#define RAA_DMPVR2_BANK0_STATUS_ADDR 0x0709 > +#define RAA_DMPVR2_BANK1_STATUS_ADDR 0x070a > +#define RAA_DMPVR2_CFG_MAX_SLOT 16 > +#define RAA_DMPVR2_CFG_HEAD_LEN 290 > +#define RAA_DMPVR2_CFG_SLOT_LEN 358 > > enum chips { > isl68137, > @@ -71,6 +87,21 @@ enum variants { > raa_dmpvr2_hv, > }; > > +enum { > + RAA_DMPVR2_DEBUGFS_CFG_W = 0, > + RAA_DMPVR2_DEBUGFS_BB_R, > + RAA_DMPVR2_DEBUGFS_NUM_ENTRIES, > +}; > + > +struct raa_dmpvr2_ctrl { > + enum chips part; > + struct i2c_client *client; > + int debugfs_entries[RAA_DMPVR2_DEBUGFS_NUM_ENTRIES]; > +}; > + > +#define to_ctrl(x, y) container_of((x), struct raa_dmpvr2_ctrl, \ > + debugfs_entries[(y)]) > + > static ssize_t isl68137_avs_enable_show_page(struct i2c_client *client, > int page, > char *buf) > @@ -157,6 +188,425 @@ static const struct attribute_group *isl68137_attribute_groups[] = { > NULL, > }; > > +/** > + * Non-standard SMBus read function to account for I2C controllers that > + * do not support SMBus block reads. Reads 5 bytes from client (length + 4 data > + * bytes) > + */ Normally this is emulated for such controllers. I don't recall seeing such a need before. The code below duplicates similar code in i2c_smbus_xfer_emulated(), which is much more sophisticated. Are you sure this is needed ? Can you point me to an affected controller ? > +static s32 raa_smbus_read40(const struct i2c_client *client, u8 command, > + unsigned char *data) > +{ > + int status; > + unsigned char msgbuf[1]; > + struct i2c_msg msg[2] = { > + { > + .addr = client->addr, > + .flags = client->flags, > + .len = 1, > + .buf = msgbuf, > + }, > + { > + .addr = client->addr, > + .flags = client->flags | I2C_M_RD, > + .len = 5, > + .buf = data, > + }, > + }; > + > + msgbuf[0] = command; > + status = i2c_transfer(client->adapter, msg, 2); > + if (status != 2) > + return status; i2c_transfer() can return 1 if only one of the two messages was sent. > + return 0; > +} > + > +/** > + * Helper function required since linux SMBus implementation does not currently > + * (v5.6) support the SMBus 3.0 "Read 32" protocol > + */ > +static s32 raa_dmpvr2_smbus_read32(const struct i2c_client *client, u8 command, > + unsigned char *data) > +{ > + int status; > + unsigned char msgbuf[1]; > + struct i2c_msg msg[2] = { > + { > + .addr = client->addr, > + .flags = client->flags, > + .len = 1, > + .buf = msgbuf, > + }, > + { > + .addr = client->addr, > + .flags = client->flags | I2C_M_RD, > + .len = 4, > + .buf = data, > + }, > + }; > + > + msgbuf[0] = command; > + status = i2c_transfer(client->adapter, msg, 2); > + if (status != 2) > + return status; > + return 0; > +} Maybe it would be worthwhile to consider implementing it ? > + > +/** > + * Helper function required since linux SMBus implementation does not currently > + * (v5.6) support the SMBus 3.0 "Write 32" protocol > + */ > +static s32 raa_dmpvr2_smbus_write32(const struct i2c_client *client, > + u8 command, u32 value) > +{ > + int status; > + unsigned char msgbuf[5]; > + struct i2c_msg msg[1] = { > + { > + .addr = client->addr, > + .flags = client->flags, > + .len = 5, > + .buf = msgbuf, > + }, > + }; > + msgbuf[0] = command; > + msgbuf[1] = value & 0x0FF; > + msgbuf[2] = (value >> 8) & 0x0FF; > + msgbuf[3] = (value >> 16) & 0x0FF; > + msgbuf[4] = (value >> 24) & 0x0FF; > + status = i2c_transfer(client->adapter, msg, 1); > + if (status != 1) > + return status; > + return 0; > +} > + > +static ssize_t raa_dmpvr2_read_black_box(struct raa_dmpvr2_ctrl *ctrl, > + char __user *buf, size_t len, > + loff_t *ppos) > +{ > + int i, j; > + u16 addr = RAA_DMPVR2_BB_BASE_ADDR; > + unsigned char data[RAA_DMPVR2_BB_WSIZE] = { 0 }; > + char out[RAA_DMPVR2_BB_BUF_SIZE] = { 0 }; > + char *optr = out; > + > + i2c_smbus_write_word_data(ctrl->client, RAA_DMPVR2_DMA_ADDR, addr); > + for (i = 0; i < RAA_DMPVR2_BB_WCNT; i++) { > + raa_dmpvr2_smbus_read32(ctrl->client, RAA_DMPVR2_DMA_SEQ, > + data); > + for (j = 0; j < RAA_DMPVR2_BB_WSIZE; j++) > + optr += snprintf(optr, sizeof(out), "%02X", data[j]); > + *optr = '\n'; > + optr++; > + } > + > + return simple_read_from_buffer(buf, len, ppos, &out, sizeof(out)); > +} > + > +struct raa_dmpvr_cfg_cmd { > + u8 cmd; > + u8 len; > + u8 data[4]; > +}; > + > +struct raa_dmpvr2_cfg { > + u8 dev_id[4]; > + u8 dev_rev[4]; > + int slot_cnt; > + int cmd_cnt; > + struct raa_dmpvr_cfg_cmd *cmds; > + u8 crc[4]; > +}; > + > +/** > + * Helper function to handle hex string to byte conversions > + */ > +static int raa_dmpvr2_hextou8(char *buf, u8 *res) > +{ > + char s[3]; > + > + s[0] = buf[0]; > + s[1] = buf[1]; > + s[2] = '\0'; > + return kstrtou8(s, 16, res); > +} > + > +static int raa_dmpvr2_parse_cfg(char *buf, struct raa_dmpvr2_cfg *cfg) > +{ > + const int lsta = 2; > + const int csta = 6; > + const int dsta = 8; > + char *cptr, *line; > + int lcnt = 1; > + int ccnt = 0; > + int i, j, ret; > + u8 b; > + > + // Ensure there is enough memory for the file > + for (i = 0; i < strlen(buf); i++) { > + if (buf[i] == '\n' && i < strlen(buf)-2) { > + lcnt++; > + if (buf[i+1] != '4' || buf[i+2] != '9') > + ccnt++; > + } > + } > + cfg->slot_cnt = (lcnt-RAA_DMPVR2_CFG_HEAD_LEN)/RAA_DMPVR2_CFG_SLOT_LEN; > + if (cfg->slot_cnt < 1 || cfg->slot_cnt > RAA_DMPVR2_CFG_MAX_SLOT) > + return -EINVAL; > + cfg->cmd_cnt = ccnt; > + cfg->cmds = kmalloc_array(ccnt, sizeof(struct raa_dmpvr_cfg_cmd), > + GFP_KERNEL); > + if (!cfg->cmds) > + return -ENOMEM; > + > + // Parse header > + for (i = 0; i < 2; i++) { > + line = strsep(&buf, "\n"); > + cptr = line + dsta; > + for (j = 3; j >= 0; j--) { > + ret = raa_dmpvr2_hextou8(cptr, &b); > + if (ret) > + goto parse_err; > + if (i == 0) > + cfg->dev_id[j] = b; > + else > + cfg->dev_rev[j] = b; > + cptr += 2; > + } > + } > + > + // Parse cmds > + i = 0; > + while (line != NULL && strlen(line) > (dsta + 2)) { > + if (strncmp(line, "49", 2) != 0) { > + ret = raa_dmpvr2_hextou8(line+lsta, &b); > + if (ret) > + goto parse_err; > + cfg->cmds[i].len = b - 3; > + ret = raa_dmpvr2_hextou8(line+csta, &b); > + if (ret) > + goto parse_err; > + cfg->cmds[i].cmd = b; > + for (j = 0; j < cfg->cmds[i].len; j++) { > + cptr = line + dsta + (2 * j); > + ret = raa_dmpvr2_hextou8(cptr, &b); > + if (ret) > + goto parse_err; > + cfg->cmds[i].data[j] = b; > + } > + i++; > + } > + line = strsep(&buf, "\n"); > + } > + return 0; > + > +parse_err: > + kfree(cfg->cmds); > + return ret; > +} > + > +static int raa_dmpvr2_verify_device(struct raa_dmpvr2_ctrl *ctrl, > + struct raa_dmpvr2_cfg *cfg) > +{ > + u8 dev_id[5]; > + u8 dev_rev[5]; > + int status; > + > + status = raa_smbus_read40(ctrl->client, PMBUS_IC_DEVICE_ID, dev_id); > + if (status) > + return status; > + > + status = raa_smbus_read40(ctrl->client, PMBUS_IC_DEVICE_REV, dev_rev); > + if (status) > + return status; > + > + return memcmp(cfg->dev_id, dev_id+1, 4) > + | memcmp(cfg->dev_rev+3, dev_rev+4, 1); > +} > + > +static int raa_dmpvr2_check_cfg(struct raa_dmpvr2_ctrl *ctrl, > + struct raa_dmpvr2_cfg *cfg) > +{ > + u8 data[4]; > + > + i2c_smbus_write_word_data(ctrl->client, RAA_DMPVR2_DMA_ADDR, > + RAA_DMPVR2_NVM_CNT_ADDR); > + raa_dmpvr2_smbus_read32(ctrl->client, RAA_DMPVR2_DMA_SEQ, data); > + if (cfg->slot_cnt > data[0]) > + return -EINVAL; > + return 0; > +} > + > +static int raa_dmpvr2_send_cfg(struct raa_dmpvr2_ctrl *ctrl, > + struct raa_dmpvr2_cfg *cfg) > +{ > + int i, status; > + u16 word; > + u32 dbl_word; > + > + for (i = 0; i < cfg->cmd_cnt; i++) { > + if (cfg->cmds[i].len == 2) { > + word = cfg->cmds[i].data[0] > + | (cfg->cmds[i].data[1] << 8); > + status = i2c_smbus_write_word_data(ctrl->client, > + cfg->cmds[i].cmd, > + word); > + if (status < 0) > + return status; > + } else if (cfg->cmds[i].len == 4) { > + dbl_word = cfg->cmds[i].data[0] > + | (cfg->cmds[i].data[1] << 8) > + | (cfg->cmds[i].data[2] << 16) > + | (cfg->cmds[i].data[3] << 24); > + status = raa_dmpvr2_smbus_write32(ctrl->client, > + cfg->cmds[i].cmd, > + dbl_word); > + if (status < 0) > + return status; > + } else { > + return -EINVAL; > + } > + } > + > + return 0; > +} > + > +static int raa_dmpvr2_cfg_write_result(struct raa_dmpvr2_ctrl *ctrl, > + struct raa_dmpvr2_cfg *cfg) > +{ > + u8 data[4] = {0}; > + u8 data1[4]; > + u8 *dptr; > + unsigned long start; > + int i, j, status; > + > + // Check programmer status > + start = jiffies; > + i2c_smbus_write_word_data(ctrl->client, RAA_DMPVR2_DMA_ADDR, > + RAA_DMPVR2_PRGM_STATUS_ADDR); > + while (data[0] == 0 && !time_after(jiffies, start + HZ + HZ)) { > + raa_dmpvr2_smbus_read32(ctrl->client, RAA_DMPVR2_DMA_FIX, > + data); > + } > + if (data[0] != 1) > + return -ETIME; > + > + // Check bank statuses > + i2c_smbus_write_word_data(ctrl->client, RAA_DMPVR2_DMA_ADDR, > + RAA_DMPVR2_BANK0_STATUS_ADDR); > + raa_dmpvr2_smbus_read32(ctrl->client, RAA_DMPVR2_DMA_FIX, data); > + i2c_smbus_write_word_data(ctrl->client, RAA_DMPVR2_DMA_ADDR, > + RAA_DMPVR2_BANK1_STATUS_ADDR); > + raa_dmpvr2_smbus_read32(ctrl->client, RAA_DMPVR2_DMA_FIX, data1); > + > + for (i = 0; i < cfg->slot_cnt; i++) { > + if (i < 8) { > + j = i; > + dptr = data; > + } else { > + j = i - 8; > + dptr = data1; > + } > + status = (dptr[j/2] >> (4 * (j % 2))) & 0x0F; > + if (status != 1) > + return -EIO; > + } > + > + return 0; > +} > + > +static ssize_t raa_dmpvr2_write_cfg(struct raa_dmpvr2_ctrl *ctrl, > + const char __user *buf, size_t len, > + loff_t *ppos) > +{ > + char *cbuf; > + int ret, status; > + struct raa_dmpvr2_cfg *cfg; > + > + cfg = kmalloc(sizeof(*cfg), GFP_KERNEL); > + if (!cfg) > + return -ENOMEM; > + > + cbuf = kmalloc(len, GFP_KERNEL); > + if (!cbuf) { > + status = -ENOMEM; > + goto buf_err; > + } > + ret = simple_write_to_buffer(cbuf, len, ppos, buf, len); > + > + // Parse file > + status = raa_dmpvr2_parse_cfg(cbuf, cfg); > + if (status) > + goto parse_err; > + // Verify device and file IDs/revs match > + status = raa_dmpvr2_verify_device(ctrl, cfg); > + if (status) > + goto dev_err; > + // Verify enough of NVM slots available > + status = raa_dmpvr2_check_cfg(ctrl, cfg); > + if (status) > + goto dev_err; > + // Write CFG to device > + status = raa_dmpvr2_send_cfg(ctrl, cfg); > + if (status) > + goto dev_err; > + // Verify programming success > + status = raa_dmpvr2_cfg_write_result(ctrl, cfg); > + if (status) > + goto dev_err; > + // Free memory > + kfree(cbuf); > + kfree(cfg->cmds); > + kfree(cfg); > + return ret; > + > + // Handle Errors > +dev_err: > + kfree(cfg->cmds); > +parse_err: > + kfree(cbuf); > +buf_err: > + kfree(cfg); > + return status; > +} > + > +static ssize_t raa_dmpvr2_debugfs_read(struct file *file, char __user *buf, > + size_t len, loff_t *ppos) > +{ > + int *idxp = file->private_data; > + int idx = *idxp; > + struct raa_dmpvr2_ctrl *ctrl = to_ctrl(idxp, idx); > + > + switch (idx) { > + case RAA_DMPVR2_DEBUGFS_BB_R: > + return raa_dmpvr2_read_black_box(ctrl, buf, len, ppos); > + default: > + return -EINVAL; > + } > +} > + > +static ssize_t raa_dmpvr2_debugfs_write(struct file *file, > + const char __user *buf, size_t len, > + loff_t *ppos) > +{ > + int *idxp = file->private_data; > + int idx = *idxp; > + struct raa_dmpvr2_ctrl *ctrl = to_ctrl(idxp, idx); > + > + switch (idx) { > + case RAA_DMPVR2_DEBUGFS_CFG_W: > + return raa_dmpvr2_write_cfg(ctrl, buf, len, ppos); > + default: > + return -EINVAL; > + } > +} > + > +static const struct file_operations raa_dmpvr2_debugfs_fops = { > + .llseek = noop_llseek, > + .read = raa_dmpvr2_debugfs_read, > + .write = raa_dmpvr2_debugfs_write, > + .open = simple_open, > +}; > + > static int raa_dmpvr2_read_word_data(struct i2c_client *client, int page, > int phase, int reg) > { > @@ -220,7 +670,11 @@ static struct pmbus_driver_info raa_dmpvr_info = { > static int isl68137_probe(struct i2c_client *client, > const struct i2c_device_id *id) > { > + int i, ret; > struct pmbus_driver_info *info; > + struct dentry *debugfs; > + struct dentry *debug_dir; > + struct raa_dmpvr2_ctrl *ctrl; > > info = devm_kzalloc(&client->dev, sizeof(*info), GFP_KERNEL); > if (!info) > @@ -262,7 +716,37 @@ static int isl68137_probe(struct i2c_client *client, > return -ENODEV; > } > > - return pmbus_do_probe(client, id, info); > + // No debugfs features for Gen 1 > + if (id->driver_data == raa_dmpvr1_2rail) > + return pmbus_do_probe(client, id, info); > + > + ret = pmbus_do_probe(client, id, info); > + if (ret != 0) > + return ret; > + > + ctrl = devm_kzalloc(&client->dev, sizeof(*ctrl), GFP_KERNEL); > + if (!ctrl) > + return 0; > + > + ctrl->client = client; > + debugfs = pmbus_get_debugfs_dir(client); > + if (!debugfs) > + return 0; > + > + debug_dir = debugfs_create_dir(client->name, debugfs); > + if (!debug_dir) > + return 0; > + > + for (i = 0; i < RAA_DMPVR2_DEBUGFS_NUM_ENTRIES; i++) > + ctrl->debugfs_entries[i] = i; > + > + debugfs_create_file("write_config", 0220, debug_dir, > + &ctrl->debugfs_entries[RAA_DMPVR2_DEBUGFS_CFG_W], > + &raa_dmpvr2_debugfs_fops); > + debugfs_create_file("read_black_box", 0440, debug_dir, > + &ctrl->debugfs_entries[RAA_DMPVR2_DEBUGFS_BB_R], > + &raa_dmpvr2_debugfs_fops); > + return 0; > } > > static const struct i2c_device_id raa_dmpvr_id[] = { > -- > 2.20.1 >
On Tue, Apr 21, 2020 at 11:58:51AM -0700, Guenter Roeck wrote: > Normally this is emulated for such controllers. I don't recall seeing > such a need before. The code below duplicates similar code in > i2c_smbus_xfer_emulated(), which is much more sophisticated. > Are you sure this is needed ? Can you point me to an affected > controller ? > > > +static s32 raa_smbus_read40(const struct i2c_client *client, u8 command, > > + unsigned char *data) > > +{ > > + int status; > > + unsigned char msgbuf[1]; > > + struct i2c_msg msg[2] = { > > + { > > + .addr = client->addr, > > + .flags = client->flags, > > + .len = 1, > > + .buf = msgbuf, > > + }, > > + { > > + .addr = client->addr, > > + .flags = client->flags | I2C_M_RD, > > + .len = 5, > > + .buf = data, > > + }, > > + }; > > + > > + msgbuf[0] = command; > > + status = i2c_transfer(client->adapter, msg, 2); > > + if (status != 2) > > + return status; > > i2c_transfer() can return 1 if only one of the two messages was sent. > > > + return 0; > > +} I have been using BCM2835 for most of my testing. I originally tried using i2c_smbus_read_block_data() but that was returning errors. However, from your email, I went back and tried i2c_smbus_read_i2c_block_data() and that appears to be working so I will switch to that instead. > > + > > +/** > > + * Helper function required since linux SMBus implementation does not currently > > + * (v5.6) support the SMBus 3.0 "Read 32" protocol > > + */ > > +static s32 raa_dmpvr2_smbus_read32(const struct i2c_client *client, u8 command, > > + unsigned char *data) > > +{ > > + int status; > > + unsigned char msgbuf[1]; > > + struct i2c_msg msg[2] = { > > + { > > + .addr = client->addr, > > + .flags = client->flags, > > + .len = 1, > > + .buf = msgbuf, > > + }, > > + { > > + .addr = client->addr, > > + .flags = client->flags | I2C_M_RD, > > + .len = 4, > > + .buf = data, > > + }, > > + }; > > + > > + msgbuf[0] = command; > > + status = i2c_transfer(client->adapter, msg, 2); > > + if (status != 2) > > + return status; > > + return 0; > > +} > > Maybe it would be worthwhile to consider implementing it ? > I could add these functions to i2c-core-smbus instead if that is desired. However, I am unsure if it would be proper to only partially update the SMBus implementation to match the 3.0 spec. Is there perhaps a better forum to discuss adding all of the 3.0 changes?
On Tue, Apr 21, 2020 at 04:49:17PM -0500, Grant Peltier wrote: > On Tue, Apr 21, 2020 at 11:58:51AM -0700, Guenter Roeck wrote: > > Normally this is emulated for such controllers. I don't recall seeing > > such a need before. The code below duplicates similar code in > > i2c_smbus_xfer_emulated(), which is much more sophisticated. > > Are you sure this is needed ? Can you point me to an affected > > controller ? > > > > > +static s32 raa_smbus_read40(const struct i2c_client *client, u8 command, > > > + unsigned char *data) > > > +{ > > > + int status; > > > + unsigned char msgbuf[1]; > > > + struct i2c_msg msg[2] = { > > > + { > > > + .addr = client->addr, > > > + .flags = client->flags, > > > + .len = 1, > > > + .buf = msgbuf, > > > + }, > > > + { > > > + .addr = client->addr, > > > + .flags = client->flags | I2C_M_RD, > > > + .len = 5, > > > + .buf = data, > > > + }, > > > + }; > > > + > > > + msgbuf[0] = command; > > > + status = i2c_transfer(client->adapter, msg, 2); > > > + if (status != 2) > > > + return status; > > > > i2c_transfer() can return 1 if only one of the two messages was sent. > > > > > + return 0; > > > +} > I have been using BCM2835 for most of my testing. I originally tried using > i2c_smbus_read_block_data() but that was returning errors. However, from your > email, I went back and tried i2c_smbus_read_i2c_block_data() and that appears > to be working so I will switch to that instead. > This is odd, since the function should do a SMBus block read. Did you pass a buffer that was too small, by any chance ? > > > + > > > +/** > > > + * Helper function required since linux SMBus implementation does not currently > > > + * (v5.6) support the SMBus 3.0 "Read 32" protocol > > > + */ > > > +static s32 raa_dmpvr2_smbus_read32(const struct i2c_client *client, u8 command, > > > + unsigned char *data) > > > +{ > > > + int status; > > > + unsigned char msgbuf[1]; > > > + struct i2c_msg msg[2] = { > > > + { > > > + .addr = client->addr, > > > + .flags = client->flags, > > > + .len = 1, > > > + .buf = msgbuf, > > > + }, > > > + { > > > + .addr = client->addr, > > > + .flags = client->flags | I2C_M_RD, > > > + .len = 4, > > > + .buf = data, > > > + }, > > > + }; > > > + > > > + msgbuf[0] = command; > > > + status = i2c_transfer(client->adapter, msg, 2); > > > + if (status != 2) > > > + return status; > > > + return 0; > > > +} > > > > Maybe it would be worthwhile to consider implementing it ? > > > I could add these functions to i2c-core-smbus instead if that is desired. > However, I am unsure if it would be proper to only partially update the SMBus > implementation to match the 3.0 spec. Is there perhaps a better forum to > discuss adding all of the 3.0 changes? > Never mind, just keep it local. I don't like it too much, but getting patches accepted into the i2c core might take several releases, and it would be unfair to impose that on you. Guenter
On Tue, Apr 21, 2020 at 07:04:04PM -0700, Guenter Roeck wrote: > On Tue, Apr 21, 2020 at 04:49:17PM -0500, Grant Peltier wrote: > > On Tue, Apr 21, 2020 at 11:58:51AM -0700, Guenter Roeck wrote: > > > Normally this is emulated for such controllers. I don't recall seeing > > > such a need before. The code below duplicates similar code in > > > i2c_smbus_xfer_emulated(), which is much more sophisticated. > > > Are you sure this is needed ? Can you point me to an affected > > > controller ? > > > > > > > +static s32 raa_smbus_read40(const struct i2c_client *client, u8 command, > > > > + unsigned char *data) > > > > +{ > > > > + int status; > > > > + unsigned char msgbuf[1]; > > > > + struct i2c_msg msg[2] = { > > > > + { > > > > + .addr = client->addr, > > > > + .flags = client->flags, > > > > + .len = 1, > > > > + .buf = msgbuf, > > > > + }, > > > > + { > > > > + .addr = client->addr, > > > > + .flags = client->flags | I2C_M_RD, > > > > + .len = 5, > > > > + .buf = data, > > > > + }, > > > > + }; > > > > + > > > > + msgbuf[0] = command; > > > > + status = i2c_transfer(client->adapter, msg, 2); > > > > + if (status != 2) > > > > + return status; > > > > > > i2c_transfer() can return 1 if only one of the two messages was sent. > > > > > > > + return 0; > > > > +} > > I have been using BCM2835 for most of my testing. I originally tried using > > i2c_smbus_read_block_data() but that was returning errors. However, from your > > email, I went back and tried i2c_smbus_read_i2c_block_data() and that appears > > to be working so I will switch to that instead. > > > This is odd, since the function should do a SMBus block read. Did you pass a > buffer that was too small, by any chance ? I tried with a variety of buffer sizes from 4 (data size) to 32 (max block size) and it always returned an error. I believe that i2c_smbus_read_block_data() attempts to do a legitimate SMBus block read which depends on the controller interpretting and reading the correct number of bytes as signaled from the client. This is in line with the docstring for the function and the fact that the BCM2835 responds with false (0) from i2c_check_functionality() for I2C_FUNC_SMBUS_READ_BLOCK_DATA. On the other hand, i2c_smbus_read_i2c_block_data() appears to do a fixed length read similar to the function that I wrote above. Thanks, Grant
diff --git a/drivers/hwmon/pmbus/isl68137.c b/drivers/hwmon/pmbus/isl68137.c index 0c622711ef7e..24af016729a3 100644 --- a/drivers/hwmon/pmbus/isl68137.c +++ b/drivers/hwmon/pmbus/isl68137.c @@ -7,10 +7,12 @@ * */ +#include <linux/debugfs.h> #include <linux/err.h> #include <linux/hwmon-sysfs.h> #include <linux/i2c.h> #include <linux/init.h> +#include <linux/jiffies.h> #include <linux/kernel.h> #include <linux/module.h> #include <linux/string.h> @@ -18,8 +20,22 @@ #include "pmbus.h" -#define ISL68137_VOUT_AVS 0x30 -#define RAA_DMPVR2_READ_VMON 0xc8 +#define ISL68137_VOUT_AVS 0x30 +#define RAA_DMPVR2_DMA_FIX 0xc5 +#define RAA_DMPVR2_DMA_SEQ 0xc6 +#define RAA_DMPVR2_DMA_ADDR 0xc7 +#define RAA_DMPVR2_READ_VMON 0xc8 +#define RAA_DMPVR2_BB_BASE_ADDR 0xef80 +#define RAA_DMPVR2_BB_WSIZE 4 +#define RAA_DMPVR2_BB_WCNT 32 +#define RAA_DMPVR2_BB_BUF_SIZE 288 +#define RAA_DMPVR2_NVM_CNT_ADDR 0x00c2 +#define RAA_DMPVR2_PRGM_STATUS_ADDR 0x0707 +#define RAA_DMPVR2_BANK0_STATUS_ADDR 0x0709 +#define RAA_DMPVR2_BANK1_STATUS_ADDR 0x070a +#define RAA_DMPVR2_CFG_MAX_SLOT 16 +#define RAA_DMPVR2_CFG_HEAD_LEN 290 +#define RAA_DMPVR2_CFG_SLOT_LEN 358 enum chips { isl68137, @@ -71,6 +87,21 @@ enum variants { raa_dmpvr2_hv, }; +enum { + RAA_DMPVR2_DEBUGFS_CFG_W = 0, + RAA_DMPVR2_DEBUGFS_BB_R, + RAA_DMPVR2_DEBUGFS_NUM_ENTRIES, +}; + +struct raa_dmpvr2_ctrl { + enum chips part; + struct i2c_client *client; + int debugfs_entries[RAA_DMPVR2_DEBUGFS_NUM_ENTRIES]; +}; + +#define to_ctrl(x, y) container_of((x), struct raa_dmpvr2_ctrl, \ + debugfs_entries[(y)]) + static ssize_t isl68137_avs_enable_show_page(struct i2c_client *client, int page, char *buf) @@ -157,6 +188,425 @@ static const struct attribute_group *isl68137_attribute_groups[] = { NULL, }; +/** + * Non-standard SMBus read function to account for I2C controllers that + * do not support SMBus block reads. Reads 5 bytes from client (length + 4 data + * bytes) + */ +static s32 raa_smbus_read40(const struct i2c_client *client, u8 command, + unsigned char *data) +{ + int status; + unsigned char msgbuf[1]; + struct i2c_msg msg[2] = { + { + .addr = client->addr, + .flags = client->flags, + .len = 1, + .buf = msgbuf, + }, + { + .addr = client->addr, + .flags = client->flags | I2C_M_RD, + .len = 5, + .buf = data, + }, + }; + + msgbuf[0] = command; + status = i2c_transfer(client->adapter, msg, 2); + if (status != 2) + return status; + return 0; +} + +/** + * Helper function required since linux SMBus implementation does not currently + * (v5.6) support the SMBus 3.0 "Read 32" protocol + */ +static s32 raa_dmpvr2_smbus_read32(const struct i2c_client *client, u8 command, + unsigned char *data) +{ + int status; + unsigned char msgbuf[1]; + struct i2c_msg msg[2] = { + { + .addr = client->addr, + .flags = client->flags, + .len = 1, + .buf = msgbuf, + }, + { + .addr = client->addr, + .flags = client->flags | I2C_M_RD, + .len = 4, + .buf = data, + }, + }; + + msgbuf[0] = command; + status = i2c_transfer(client->adapter, msg, 2); + if (status != 2) + return status; + return 0; +} + +/** + * Helper function required since linux SMBus implementation does not currently + * (v5.6) support the SMBus 3.0 "Write 32" protocol + */ +static s32 raa_dmpvr2_smbus_write32(const struct i2c_client *client, + u8 command, u32 value) +{ + int status; + unsigned char msgbuf[5]; + struct i2c_msg msg[1] = { + { + .addr = client->addr, + .flags = client->flags, + .len = 5, + .buf = msgbuf, + }, + }; + msgbuf[0] = command; + msgbuf[1] = value & 0x0FF; + msgbuf[2] = (value >> 8) & 0x0FF; + msgbuf[3] = (value >> 16) & 0x0FF; + msgbuf[4] = (value >> 24) & 0x0FF; + status = i2c_transfer(client->adapter, msg, 1); + if (status != 1) + return status; + return 0; +} + +static ssize_t raa_dmpvr2_read_black_box(struct raa_dmpvr2_ctrl *ctrl, + char __user *buf, size_t len, + loff_t *ppos) +{ + int i, j; + u16 addr = RAA_DMPVR2_BB_BASE_ADDR; + unsigned char data[RAA_DMPVR2_BB_WSIZE] = { 0 }; + char out[RAA_DMPVR2_BB_BUF_SIZE] = { 0 }; + char *optr = out; + + i2c_smbus_write_word_data(ctrl->client, RAA_DMPVR2_DMA_ADDR, addr); + for (i = 0; i < RAA_DMPVR2_BB_WCNT; i++) { + raa_dmpvr2_smbus_read32(ctrl->client, RAA_DMPVR2_DMA_SEQ, + data); + for (j = 0; j < RAA_DMPVR2_BB_WSIZE; j++) + optr += snprintf(optr, sizeof(out), "%02X", data[j]); + *optr = '\n'; + optr++; + } + + return simple_read_from_buffer(buf, len, ppos, &out, sizeof(out)); +} + +struct raa_dmpvr_cfg_cmd { + u8 cmd; + u8 len; + u8 data[4]; +}; + +struct raa_dmpvr2_cfg { + u8 dev_id[4]; + u8 dev_rev[4]; + int slot_cnt; + int cmd_cnt; + struct raa_dmpvr_cfg_cmd *cmds; + u8 crc[4]; +}; + +/** + * Helper function to handle hex string to byte conversions + */ +static int raa_dmpvr2_hextou8(char *buf, u8 *res) +{ + char s[3]; + + s[0] = buf[0]; + s[1] = buf[1]; + s[2] = '\0'; + return kstrtou8(s, 16, res); +} + +static int raa_dmpvr2_parse_cfg(char *buf, struct raa_dmpvr2_cfg *cfg) +{ + const int lsta = 2; + const int csta = 6; + const int dsta = 8; + char *cptr, *line; + int lcnt = 1; + int ccnt = 0; + int i, j, ret; + u8 b; + + // Ensure there is enough memory for the file + for (i = 0; i < strlen(buf); i++) { + if (buf[i] == '\n' && i < strlen(buf)-2) { + lcnt++; + if (buf[i+1] != '4' || buf[i+2] != '9') + ccnt++; + } + } + cfg->slot_cnt = (lcnt-RAA_DMPVR2_CFG_HEAD_LEN)/RAA_DMPVR2_CFG_SLOT_LEN; + if (cfg->slot_cnt < 1 || cfg->slot_cnt > RAA_DMPVR2_CFG_MAX_SLOT) + return -EINVAL; + cfg->cmd_cnt = ccnt; + cfg->cmds = kmalloc_array(ccnt, sizeof(struct raa_dmpvr_cfg_cmd), + GFP_KERNEL); + if (!cfg->cmds) + return -ENOMEM; + + // Parse header + for (i = 0; i < 2; i++) { + line = strsep(&buf, "\n"); + cptr = line + dsta; + for (j = 3; j >= 0; j--) { + ret = raa_dmpvr2_hextou8(cptr, &b); + if (ret) + goto parse_err; + if (i == 0) + cfg->dev_id[j] = b; + else + cfg->dev_rev[j] = b; + cptr += 2; + } + } + + // Parse cmds + i = 0; + while (line != NULL && strlen(line) > (dsta + 2)) { + if (strncmp(line, "49", 2) != 0) { + ret = raa_dmpvr2_hextou8(line+lsta, &b); + if (ret) + goto parse_err; + cfg->cmds[i].len = b - 3; + ret = raa_dmpvr2_hextou8(line+csta, &b); + if (ret) + goto parse_err; + cfg->cmds[i].cmd = b; + for (j = 0; j < cfg->cmds[i].len; j++) { + cptr = line + dsta + (2 * j); + ret = raa_dmpvr2_hextou8(cptr, &b); + if (ret) + goto parse_err; + cfg->cmds[i].data[j] = b; + } + i++; + } + line = strsep(&buf, "\n"); + } + return 0; + +parse_err: + kfree(cfg->cmds); + return ret; +} + +static int raa_dmpvr2_verify_device(struct raa_dmpvr2_ctrl *ctrl, + struct raa_dmpvr2_cfg *cfg) +{ + u8 dev_id[5]; + u8 dev_rev[5]; + int status; + + status = raa_smbus_read40(ctrl->client, PMBUS_IC_DEVICE_ID, dev_id); + if (status) + return status; + + status = raa_smbus_read40(ctrl->client, PMBUS_IC_DEVICE_REV, dev_rev); + if (status) + return status; + + return memcmp(cfg->dev_id, dev_id+1, 4) + | memcmp(cfg->dev_rev+3, dev_rev+4, 1); +} + +static int raa_dmpvr2_check_cfg(struct raa_dmpvr2_ctrl *ctrl, + struct raa_dmpvr2_cfg *cfg) +{ + u8 data[4]; + + i2c_smbus_write_word_data(ctrl->client, RAA_DMPVR2_DMA_ADDR, + RAA_DMPVR2_NVM_CNT_ADDR); + raa_dmpvr2_smbus_read32(ctrl->client, RAA_DMPVR2_DMA_SEQ, data); + if (cfg->slot_cnt > data[0]) + return -EINVAL; + return 0; +} + +static int raa_dmpvr2_send_cfg(struct raa_dmpvr2_ctrl *ctrl, + struct raa_dmpvr2_cfg *cfg) +{ + int i, status; + u16 word; + u32 dbl_word; + + for (i = 0; i < cfg->cmd_cnt; i++) { + if (cfg->cmds[i].len == 2) { + word = cfg->cmds[i].data[0] + | (cfg->cmds[i].data[1] << 8); + status = i2c_smbus_write_word_data(ctrl->client, + cfg->cmds[i].cmd, + word); + if (status < 0) + return status; + } else if (cfg->cmds[i].len == 4) { + dbl_word = cfg->cmds[i].data[0] + | (cfg->cmds[i].data[1] << 8) + | (cfg->cmds[i].data[2] << 16) + | (cfg->cmds[i].data[3] << 24); + status = raa_dmpvr2_smbus_write32(ctrl->client, + cfg->cmds[i].cmd, + dbl_word); + if (status < 0) + return status; + } else { + return -EINVAL; + } + } + + return 0; +} + +static int raa_dmpvr2_cfg_write_result(struct raa_dmpvr2_ctrl *ctrl, + struct raa_dmpvr2_cfg *cfg) +{ + u8 data[4] = {0}; + u8 data1[4]; + u8 *dptr; + unsigned long start; + int i, j, status; + + // Check programmer status + start = jiffies; + i2c_smbus_write_word_data(ctrl->client, RAA_DMPVR2_DMA_ADDR, + RAA_DMPVR2_PRGM_STATUS_ADDR); + while (data[0] == 0 && !time_after(jiffies, start + HZ + HZ)) { + raa_dmpvr2_smbus_read32(ctrl->client, RAA_DMPVR2_DMA_FIX, + data); + } + if (data[0] != 1) + return -ETIME; + + // Check bank statuses + i2c_smbus_write_word_data(ctrl->client, RAA_DMPVR2_DMA_ADDR, + RAA_DMPVR2_BANK0_STATUS_ADDR); + raa_dmpvr2_smbus_read32(ctrl->client, RAA_DMPVR2_DMA_FIX, data); + i2c_smbus_write_word_data(ctrl->client, RAA_DMPVR2_DMA_ADDR, + RAA_DMPVR2_BANK1_STATUS_ADDR); + raa_dmpvr2_smbus_read32(ctrl->client, RAA_DMPVR2_DMA_FIX, data1); + + for (i = 0; i < cfg->slot_cnt; i++) { + if (i < 8) { + j = i; + dptr = data; + } else { + j = i - 8; + dptr = data1; + } + status = (dptr[j/2] >> (4 * (j % 2))) & 0x0F; + if (status != 1) + return -EIO; + } + + return 0; +} + +static ssize_t raa_dmpvr2_write_cfg(struct raa_dmpvr2_ctrl *ctrl, + const char __user *buf, size_t len, + loff_t *ppos) +{ + char *cbuf; + int ret, status; + struct raa_dmpvr2_cfg *cfg; + + cfg = kmalloc(sizeof(*cfg), GFP_KERNEL); + if (!cfg) + return -ENOMEM; + + cbuf = kmalloc(len, GFP_KERNEL); + if (!cbuf) { + status = -ENOMEM; + goto buf_err; + } + ret = simple_write_to_buffer(cbuf, len, ppos, buf, len); + + // Parse file + status = raa_dmpvr2_parse_cfg(cbuf, cfg); + if (status) + goto parse_err; + // Verify device and file IDs/revs match + status = raa_dmpvr2_verify_device(ctrl, cfg); + if (status) + goto dev_err; + // Verify enough of NVM slots available + status = raa_dmpvr2_check_cfg(ctrl, cfg); + if (status) + goto dev_err; + // Write CFG to device + status = raa_dmpvr2_send_cfg(ctrl, cfg); + if (status) + goto dev_err; + // Verify programming success + status = raa_dmpvr2_cfg_write_result(ctrl, cfg); + if (status) + goto dev_err; + // Free memory + kfree(cbuf); + kfree(cfg->cmds); + kfree(cfg); + return ret; + + // Handle Errors +dev_err: + kfree(cfg->cmds); +parse_err: + kfree(cbuf); +buf_err: + kfree(cfg); + return status; +} + +static ssize_t raa_dmpvr2_debugfs_read(struct file *file, char __user *buf, + size_t len, loff_t *ppos) +{ + int *idxp = file->private_data; + int idx = *idxp; + struct raa_dmpvr2_ctrl *ctrl = to_ctrl(idxp, idx); + + switch (idx) { + case RAA_DMPVR2_DEBUGFS_BB_R: + return raa_dmpvr2_read_black_box(ctrl, buf, len, ppos); + default: + return -EINVAL; + } +} + +static ssize_t raa_dmpvr2_debugfs_write(struct file *file, + const char __user *buf, size_t len, + loff_t *ppos) +{ + int *idxp = file->private_data; + int idx = *idxp; + struct raa_dmpvr2_ctrl *ctrl = to_ctrl(idxp, idx); + + switch (idx) { + case RAA_DMPVR2_DEBUGFS_CFG_W: + return raa_dmpvr2_write_cfg(ctrl, buf, len, ppos); + default: + return -EINVAL; + } +} + +static const struct file_operations raa_dmpvr2_debugfs_fops = { + .llseek = noop_llseek, + .read = raa_dmpvr2_debugfs_read, + .write = raa_dmpvr2_debugfs_write, + .open = simple_open, +}; + static int raa_dmpvr2_read_word_data(struct i2c_client *client, int page, int phase, int reg) { @@ -220,7 +670,11 @@ static struct pmbus_driver_info raa_dmpvr_info = { static int isl68137_probe(struct i2c_client *client, const struct i2c_device_id *id) { + int i, ret; struct pmbus_driver_info *info; + struct dentry *debugfs; + struct dentry *debug_dir; + struct raa_dmpvr2_ctrl *ctrl; info = devm_kzalloc(&client->dev, sizeof(*info), GFP_KERNEL); if (!info) @@ -262,7 +716,37 @@ static int isl68137_probe(struct i2c_client *client, return -ENODEV; } - return pmbus_do_probe(client, id, info); + // No debugfs features for Gen 1 + if (id->driver_data == raa_dmpvr1_2rail) + return pmbus_do_probe(client, id, info); + + ret = pmbus_do_probe(client, id, info); + if (ret != 0) + return ret; + + ctrl = devm_kzalloc(&client->dev, sizeof(*ctrl), GFP_KERNEL); + if (!ctrl) + return 0; + + ctrl->client = client; + debugfs = pmbus_get_debugfs_dir(client); + if (!debugfs) + return 0; + + debug_dir = debugfs_create_dir(client->name, debugfs); + if (!debug_dir) + return 0; + + for (i = 0; i < RAA_DMPVR2_DEBUGFS_NUM_ENTRIES; i++) + ctrl->debugfs_entries[i] = i; + + debugfs_create_file("write_config", 0220, debug_dir, + &ctrl->debugfs_entries[RAA_DMPVR2_DEBUGFS_CFG_W], + &raa_dmpvr2_debugfs_fops); + debugfs_create_file("read_black_box", 0440, debug_dir, + &ctrl->debugfs_entries[RAA_DMPVR2_DEBUGFS_BB_R], + &raa_dmpvr2_debugfs_fops); + return 0; } static const struct i2c_device_id raa_dmpvr_id[] = {
Add debugfs endpoints to support features of 2nd generation Renesas digital multiphase voltage regulators that are not compatible with sysfs. The read_black_box endpoint allows users to read the contents of a RAM segment used to record fault conditions within Gen 2 devices. The write_config endpoint allows users to write configuration hex files to Gen 2 devices which modify how they operate. Signed-off-by: Grant Peltier <grantpeltier93@gmail.com> --- drivers/hwmon/pmbus/isl68137.c | 490 ++++++++++++++++++++++++++++++++- 1 file changed, 487 insertions(+), 3 deletions(-)