| Message ID | 20241007023900.3924763-2-aryan.srivastava@alliedtelesis.co.nz |
|---|---|
| State | New |
| Headers | show |
| Series | i2c: octeon: Add block-mode r/w | expand |
Hi Aryan,
kernel test robot noticed the following build warnings:
[auto build test WARNING on andi-shyti/i2c/i2c-host]
[also build test WARNING on linus/master v6.12-rc2 next-20241009]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Aryan-Srivastava/i2c-octeon-refactor-common-i2c-operations/20241007-104113
base: https://git.kernel.org/pub/scm/linux/kernel/git/andi.shyti/linux.git i2c/i2c-host
patch link: https://lore.kernel.org/r/20241007023900.3924763-2-aryan.srivastava%40alliedtelesis.co.nz
patch subject: [PATCH v9 1/2] i2c: octeon: refactor common i2c operations
config: x86_64-allmodconfig (https://download.01.org/0day-ci/archive/20241010/202410100921.WVORo97f-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241010/202410100921.WVORo97f-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410100921.WVORo97f-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/i2c/busses/i2c-octeon-core.c:517:3: warning: variable 'cmd' is uninitialized when used here [-Wuninitialized]
517 | cmd |= SW_TWSI_OP_10_IA;
| ^~~
drivers/i2c/busses/i2c-octeon-core.c:514:9: note: initialize the variable 'cmd' to silence this warning
514 | u64 cmd;
| ^
| = 0
1 warning generated.
Kconfig warnings: (for reference only)
WARNING: unmet direct dependencies detected for MODVERSIONS
Depends on [n]: MODULES [=y] && !COMPILE_TEST [=y]
Selected by [y]:
- RANDSTRUCT_FULL [=y] && (CC_HAS_RANDSTRUCT [=y] || GCC_PLUGINS [=n]) && MODULES [=y]
vim +/cmd +517 drivers/i2c/busses/i2c-octeon-core.c
ad83665b4687f5 Jan Glauber 2016-08-24 509
dcdb10ccc89293 Aryan Srivastava 2024-10-07 510 /* Generic consideration for extended internal addresses in i2c hlc r/w ops */
dcdb10ccc89293 Aryan Srivastava 2024-10-07 511 static bool octeon_i2c_hlc_ext(struct octeon_i2c *i2c, struct i2c_msg msg, u64 *cmd_in, u64 *ext)
dcdb10ccc89293 Aryan Srivastava 2024-10-07 512 {
dcdb10ccc89293 Aryan Srivastava 2024-10-07 513 bool set_ext = false;
dcdb10ccc89293 Aryan Srivastava 2024-10-07 514 u64 cmd;
ad83665b4687f5 Jan Glauber 2016-08-24 515
dcdb10ccc89293 Aryan Srivastava 2024-10-07 516 if (msg.flags & I2C_M_TEN)
ad83665b4687f5 Jan Glauber 2016-08-24 @517 cmd |= SW_TWSI_OP_10_IA;
ad83665b4687f5 Jan Glauber 2016-08-24 518 else
ad83665b4687f5 Jan Glauber 2016-08-24 519 cmd |= SW_TWSI_OP_7_IA;
ad83665b4687f5 Jan Glauber 2016-08-24 520
dcdb10ccc89293 Aryan Srivastava 2024-10-07 521 if (msg.len == 2) {
dcdb10ccc89293 Aryan Srivastava 2024-10-07 522 cmd |= SW_TWSI_EIA;
dcdb10ccc89293 Aryan Srivastava 2024-10-07 523 *ext = (u64)msg.buf[0] << SW_TWSI_IA_SHIFT;
dcdb10ccc89293 Aryan Srivastava 2024-10-07 524 cmd |= (u64)msg.buf[1] << SW_TWSI_IA_SHIFT;
dcdb10ccc89293 Aryan Srivastava 2024-10-07 525 set_ext = true;
dcdb10ccc89293 Aryan Srivastava 2024-10-07 526 } else {
dcdb10ccc89293 Aryan Srivastava 2024-10-07 527 cmd |= (u64)msg.buf[0] << SW_TWSI_IA_SHIFT;
dcdb10ccc89293 Aryan Srivastava 2024-10-07 528 }
dcdb10ccc89293 Aryan Srivastava 2024-10-07 529
dcdb10ccc89293 Aryan Srivastava 2024-10-07 530 *cmd_in |= cmd;
dcdb10ccc89293 Aryan Srivastava 2024-10-07 531 return set_ext;
dcdb10ccc89293 Aryan Srivastava 2024-10-07 532 }
dcdb10ccc89293 Aryan Srivastava 2024-10-07 533
diff --git a/drivers/i2c/busses/i2c-octeon-core.c b/drivers/i2c/busses/i2c-octeon-core.c index 16cc34a0526e..3195477cfc63 100644 --- a/drivers/i2c/busses/i2c-octeon-core.c +++ b/drivers/i2c/busses/i2c-octeon-core.c @@ -498,6 +498,50 @@ static int octeon_i2c_hlc_write(struct octeon_i2c *i2c, struct i2c_msg *msgs) return ret; } +/* Process hlc transaction */ +static int octeon_i2c_hlc_cmd_send(struct octeon_i2c *i2c, u64 cmd) +{ + octeon_i2c_hlc_int_clear(i2c); + octeon_i2c_writeq_flush(cmd, i2c->twsi_base + OCTEON_REG_SW_TWSI(i2c)); + + return octeon_i2c_hlc_wait(i2c); +} + +/* Generic consideration for extended internal addresses in i2c hlc r/w ops */ +static bool octeon_i2c_hlc_ext(struct octeon_i2c *i2c, struct i2c_msg msg, u64 *cmd_in, u64 *ext) +{ + bool set_ext = false; + u64 cmd; + + if (msg.flags & I2C_M_TEN) + cmd |= SW_TWSI_OP_10_IA; + else + cmd |= SW_TWSI_OP_7_IA; + + if (msg.len == 2) { + cmd |= SW_TWSI_EIA; + *ext = (u64)msg.buf[0] << SW_TWSI_IA_SHIFT; + cmd |= (u64)msg.buf[1] << SW_TWSI_IA_SHIFT; + set_ext = true; + } else { + cmd |= (u64)msg.buf[0] << SW_TWSI_IA_SHIFT; + } + + *cmd_in |= cmd; + return set_ext; +} + +/* Construct and send i2c transaction core cmd for read ops */ +static int octeon_i2c_hlc_read_cmd(struct octeon_i2c *i2c, struct i2c_msg msg, u64 cmd) +{ + u64 ext = 0; + + if (octeon_i2c_hlc_ext(i2c, msg, &cmd, &ext)) + octeon_i2c_writeq_flush(ext, i2c->twsi_base + OCTEON_REG_SW_TWSI_EXT(i2c)); + + return octeon_i2c_hlc_cmd_send(i2c, cmd); +} + /* high-level-controller composite write+read, msg0=addr, msg1=data */ static int octeon_i2c_hlc_comp_read(struct octeon_i2c *i2c, struct i2c_msg *msgs) { @@ -512,26 +556,8 @@ static int octeon_i2c_hlc_comp_read(struct octeon_i2c *i2c, struct i2c_msg *msgs /* A */ cmd |= (u64)(msgs[0].addr & 0x7full) << SW_TWSI_ADDR_SHIFT; - if (msgs[0].flags & I2C_M_TEN) - cmd |= SW_TWSI_OP_10_IA; - else - cmd |= SW_TWSI_OP_7_IA; - - if (msgs[0].len == 2) { - u64 ext = 0; - - cmd |= SW_TWSI_EIA; - ext = (u64)msgs[0].buf[0] << SW_TWSI_IA_SHIFT; - cmd |= (u64)msgs[0].buf[1] << SW_TWSI_IA_SHIFT; - octeon_i2c_writeq_flush(ext, i2c->twsi_base + OCTEON_REG_SW_TWSI_EXT(i2c)); - } else { - cmd |= (u64)msgs[0].buf[0] << SW_TWSI_IA_SHIFT; - } - - octeon_i2c_hlc_int_clear(i2c); - octeon_i2c_writeq_flush(cmd, i2c->twsi_base + OCTEON_REG_SW_TWSI(i2c)); - - ret = octeon_i2c_hlc_wait(i2c); + /* Send core command */ + ret = octeon_i2c_hlc_read_cmd(i2c, msgs[0], cmd); if (ret) goto err; @@ -567,19 +593,8 @@ static int octeon_i2c_hlc_comp_write(struct octeon_i2c *i2c, struct i2c_msg *msg /* A */ cmd |= (u64)(msgs[0].addr & 0x7full) << SW_TWSI_ADDR_SHIFT; - if (msgs[0].flags & I2C_M_TEN) - cmd |= SW_TWSI_OP_10_IA; - else - cmd |= SW_TWSI_OP_7_IA; - - if (msgs[0].len == 2) { - cmd |= SW_TWSI_EIA; - ext |= (u64)msgs[0].buf[0] << SW_TWSI_IA_SHIFT; - set_ext = true; - cmd |= (u64)msgs[0].buf[1] << SW_TWSI_IA_SHIFT; - } else { - cmd |= (u64)msgs[0].buf[0] << SW_TWSI_IA_SHIFT; - } + /* Set parameters for extended message (if required) */ + set_ext = octeon_i2c_hlc_ext(i2c, msgs[0], &cmd, &ext); for (i = 0, j = msgs[1].len - 1; i < msgs[1].len && i < 4; i++, j--) cmd |= (u64)msgs[1].buf[j] << (8 * i); @@ -592,10 +607,7 @@ static int octeon_i2c_hlc_comp_write(struct octeon_i2c *i2c, struct i2c_msg *msg if (set_ext) octeon_i2c_writeq_flush(ext, i2c->twsi_base + OCTEON_REG_SW_TWSI_EXT(i2c)); - octeon_i2c_hlc_int_clear(i2c); - octeon_i2c_writeq_flush(cmd, i2c->twsi_base + OCTEON_REG_SW_TWSI(i2c)); - - ret = octeon_i2c_hlc_wait(i2c); + ret = octeon_i2c_hlc_cmd_send(i2c, cmd); if (ret) goto err;
Refactor the current implementation of the high-level composite read and write operations in preparation of the addition of block-mode read/write operations. The sending of the i2c command is generic and will apply for both the block-mode and non-block-mode ops. Extract this from the current hlc ops, and place into a generic function, octeon_i2c_hlc_cmd_send. The considerations made for extended addresses in the command construction are almost common for all cases, extract these into octeon_i2c_hlc_ext. There is one difference between the extended read and write cases. When performing extended read or writes the SW_TWSI_EXT must be written with an extended internal address, but the data field is only filled in the write case (read back in read case). This results in the original code block for the read case immediately writing this register, while the write case fills in any data and then writes the register. To create a common block of code for both processes remove the SW_TWSI_EXT write from within the code block and instead in it's place a variable is set, set_ext, which is returned and used as a condition to do the register write, in the read command case. There are parts of the commands construction which are common (only in the read case), extract this and place into generic function octeon_i2c_hlc_read_cmd. This function also reads the return from octeon_i2c_hlc_ext and completes the write to SW_TWSI_EXT if required. The write commands cannot be made entirely into common code as there are distinct differences in the block mode and non-block-mode process. Particularly the writing of data into the buffer. Signed-off-by: Aryan Srivastava <aryan.srivastava@alliedtelesis.co.nz> --- drivers/i2c/busses/i2c-octeon-core.c | 86 ++++++++++++++++------------ 1 file changed, 49 insertions(+), 37 deletions(-)