diff mbox series

[v6,1/2] i2c: octeon: refactor hlc r/w operations

Message ID 20240613025412.3848629-2-aryan.srivastava@alliedtelesis.co.nz
State Superseded
Headers show
Series [v6,1/2] i2c: octeon: refactor hlc r/w operations | expand

Commit Message

Aryan Srivastava June 13, 2024, 2:54 a.m. UTC
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 R/W. 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 common for all r/w cases, extract these into
octeon_i2c_hlc_ext.

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.

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

Comments

Markus Elfring June 13, 2024, 6:32 a.m. UTC | #1
> Refactor the current implementation of the high-level composite read and
> write operations in preparation of the addition of block-mode read/write
> operations.
…

* I find that a cover letter can be helpful also for the presented small patch series.

* How do you think about to replace any abbreviations in summary phrases?
  - HLC
  - r/w


Regards,
Markus
Aryan Srivastava June 20, 2024, 1:05 a.m. UTC | #2
Hi Markus,
On Thu, 2024-06-13 at 08:32 +0200, Markus Elfring wrote:
> > Refactor the current implementation of the high-level composite
> > read and
> > write operations in preparation of the addition of block-mode
> > read/write
> > operations.
> …
> 
> * I find that a cover letter can be helpful also for the presented
> small patch series.
> 
I did make one, but I am not sure what happened to it. The link on the
lore.kernel thread for it is broken, I might have generated it
incorrectly? Hopefully the next patch series will have intact.
> * How do you think about to replace any abbreviations in summary
> phrases?
>   - HLC
>   - r/w
> 
Done.
> 
> Regards,
> Markus

Thanks,
Aryan
diff mbox series

Patch

diff --git a/drivers/i2c/busses/i2c-octeon-core.c b/drivers/i2c/busses/i2c-octeon-core.c
index 845eda70b8ca..6772359ca6c8 100644
--- a/drivers/i2c/busses/i2c-octeon-core.c
+++ b/drivers/i2c/busses/i2c-octeon-core.c
@@ -485,6 +485,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 + 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 + 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)
 {
@@ -499,26 +543,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 + 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 + SW_TWSI(i2c));
-
-	ret = octeon_i2c_hlc_wait(i2c);
+	/* Send core command */
+	ret = octeon_i2c_hlc_cmd(i2c, msgs[0], cmd);
 	if (ret)
 		goto err;
 
@@ -554,19 +580,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);
@@ -579,10 +594,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 + SW_TWSI_EXT(i2c));
 
-	octeon_i2c_hlc_int_clear(i2c);
-	octeon_i2c_writeq_flush(cmd, i2c->twsi_base + SW_TWSI(i2c));
-
-	ret = octeon_i2c_hlc_wait(i2c);
+	ret = octeon_i2c_hlc_cmd_send(i2c, cmd);
 	if (ret)
 		goto err;