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(-)

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;
 
-- 
2.46.0

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  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki