2024-06-13 02:54:44

by Aryan Srivastava

[permalink] [raw]
Subject: [PATCH v6 1/2] i2c: octeon: refactor hlc r/w operations

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 <[email protected]>
---
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 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;

--
2.43.2



2024-06-13 06:33:54

by Markus Elfring

[permalink] [raw]
Subject: Re: [PATCH v6 1/2] i2c: octeon: refactor hlc r/w operations

> 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

2024-06-13 23:55:47

by Andi Shyti

[permalink] [raw]
Subject: Re: [PATCH v6 1/2] i2c: octeon: refactor hlc r/w operations

Hi Aryan,

> +/* 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));

I think this check here is the only logical change I see. Right?

If so, can you please describe in the log why you made this
change?

Thanks,
Andi

> + return octeon_i2c_hlc_cmd_send(i2c, cmd);
> +}