* [PATCH] i2c:octeon:Add block-mode r/w
@ 2023-07-05 23:45 Aryan Srivastava
2023-07-25 22:33 ` Andi Shyti
0 siblings, 1 reply; 17+ messages in thread
From: Aryan Srivastava @ 2023-07-05 23:45 UTC (permalink / raw
To: andi.shyti; +Cc: linux-i2c, linux-kernel, Aryan Srivastava
Add support for block mode read/write operations on
Thunderx chips.
When attempting r/w operations of greater then 8 bytes
block mode is used, instead of performing a series of
8 byte reads.
Signed-off-by: Aryan Srivastava <aryan.srivastava@alliedtelesis.co.nz>
---
drivers/i2c/busses/i2c-octeon-core.c | 162 ++++++++++++++++++++++-
drivers/i2c/busses/i2c-octeon-core.h | 14 ++
drivers/i2c/busses/i2c-thunderx-pcidrv.c | 4 +
3 files changed, 174 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/busses/i2c-octeon-core.c b/drivers/i2c/busses/i2c-octeon-core.c
index 845eda70b8ca..304db70cb1e0 100644
--- a/drivers/i2c/busses/i2c-octeon-core.c
+++ b/drivers/i2c/busses/i2c-octeon-core.c
@@ -130,6 +130,24 @@ static void octeon_i2c_hlc_disable(struct octeon_i2c *i2c)
octeon_i2c_ctl_write(i2c, TWSI_CTL_ENAB);
}
+static void octeon_i2c_block_enable(struct octeon_i2c *i2c)
+{
+ if (i2c->block_enabled || !TWSI_BLOCK_CTL(i2c))
+ return;
+
+ i2c->block_enabled = true;
+ octeon_i2c_writeq_flush(
+ TWSI_MODE_STRETCH | TWSI_MODE_BLOCK_MODE, i2c->twsi_base + TWSI_MODE(i2c));
+}
+
+static void octeon_i2c_block_disable(struct octeon_i2c *i2c)
+{
+ if (!i2c->block_enabled || !TWSI_BLOCK_CTL(i2c))
+ return;
+
+ i2c->block_enabled = false;
+ octeon_i2c_writeq_flush(TWSI_MODE_STRETCH, i2c->twsi_base + TWSI_MODE(i2c));
+}
/**
* octeon_i2c_hlc_wait - wait for an HLC operation to complete
* @i2c: The struct octeon_i2c
@@ -268,6 +286,7 @@ static int octeon_i2c_start(struct octeon_i2c *i2c)
u8 stat;
octeon_i2c_hlc_disable(i2c);
+ octeon_i2c_block_disable(i2c);
octeon_i2c_ctl_write(i2c, TWSI_CTL_ENAB | TWSI_CTL_STA);
ret = octeon_i2c_wait(i2c);
@@ -594,6 +613,128 @@ static int octeon_i2c_hlc_comp_write(struct octeon_i2c *i2c, struct i2c_msg *msg
return ret;
}
+/* high-level-controller composite block write+read, m[0]len<=2, m[1]len<=1024 */
+static int octeon_i2c_hlc_block_comp_read(struct octeon_i2c *i2c, struct i2c_msg *msgs)
+{
+ int i, j, len, ret = 0;
+ u64 cmd, rd;
+
+ octeon_i2c_hlc_enable(i2c);
+ octeon_i2c_block_enable(i2c);
+
+ len = msgs[1].len - 1;
+ /* Prepare core command */
+ cmd = SW_TWSI_V | SW_TWSI_R | SW_TWSI_SOVR;
+ /* SIZE */
+ octeon_i2c_writeq_flush((u64)(len), i2c->twsi_base + TWSI_BLOCK_CTL(i2c));
+ /* 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;
+ }
+
+ /* Send command to core (send data to FIFO) */
+ octeon_i2c_hlc_int_clear(i2c);
+ octeon_i2c_writeq_flush(cmd, i2c->twsi_base + SW_TWSI(i2c));
+
+ /* Wait for transaction to complete */
+ ret = octeon_i2c_hlc_wait(i2c);
+ if (ret)
+ goto err;
+
+ cmd = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
+ if ((cmd & SW_TWSI_R) == 0)
+ return octeon_i2c_check_status(i2c, false);
+
+ /* read data in FIFO */
+ octeon_i2c_writeq_flush(TWSI_BLOCK_STS_RESET_PTR, i2c->twsi_base + TWSI_BLOCK_STS(i2c));
+ for (i = 0; i < len; i += 8) {
+ rd = __raw_readq(i2c->twsi_base + TWSI_BLOCK_FIFO(i2c));
+ for (j = 7; j >= 0; j--)
+ msgs[1].buf[i + (7 - j)] = (rd >> (8 * j)) & 0xff;
+ }
+
+ octeon_i2c_block_disable(i2c);
+
+err:
+ return ret;
+}
+
+/* high-level-controller composite block write+write, m[0]len<=2, m[1]len<=1024 */
+static int octeon_i2c_hlc_block_comp_write(struct octeon_i2c *i2c, struct i2c_msg *msgs)
+{
+ bool set_ext = false;
+ int i, j, len, ret = 0;
+ u64 cmd, buf = 0, ext = 0;
+
+ octeon_i2c_hlc_enable(i2c);
+ octeon_i2c_block_enable(i2c);
+
+ len = msgs[1].len - 1;
+ /* Prepare core command */
+ cmd = SW_TWSI_V | SW_TWSI_SOVR;
+ /* SIZE */
+ octeon_i2c_writeq_flush((u64)(len), i2c->twsi_base + TWSI_BLOCK_CTL(i2c));
+ /* 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;
+ }
+
+ /* Write msg into FIFO buffer */
+ octeon_i2c_writeq_flush(TWSI_BLOCK_STS_RESET_PTR, i2c->twsi_base + TWSI_BLOCK_STS(i2c));
+ for (i = 0; i < len; i += 8) {
+ buf = 0;
+ for (j = 7; j >= 0; j--)
+ buf |= (msgs[1].buf[i + (7 - j)] << (8 * j));
+ octeon_i2c_writeq_flush(buf, i2c->twsi_base + TWSI_BLOCK_FIFO(i2c));
+ }
+ if (set_ext)
+ octeon_i2c_writeq_flush(ext, i2c->twsi_base + SW_TWSI_EXT(i2c));
+
+ /* Send command to core (send data in FIFO) */
+ octeon_i2c_hlc_int_clear(i2c);
+ octeon_i2c_writeq_flush(cmd, i2c->twsi_base + SW_TWSI(i2c));
+
+ /* Wait for transaction to complete */
+ ret = octeon_i2c_hlc_wait(i2c);
+ if (ret)
+ goto err;
+
+ cmd = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
+ if ((cmd & SW_TWSI_R) == 0)
+ return octeon_i2c_check_status(i2c, false);
+
+ octeon_i2c_block_disable(i2c);
+
+err:
+ return ret;
+}
+
/**
* octeon_i2c_xfer - The driver's master_xfer function
* @adap: Pointer to the i2c_adapter structure
@@ -619,13 +760,21 @@ int octeon_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
if ((msgs[0].flags & I2C_M_RD) == 0 &&
(msgs[1].flags & I2C_M_RECV_LEN) == 0 &&
msgs[0].len > 0 && msgs[0].len <= 2 &&
- msgs[1].len > 0 && msgs[1].len <= 8 &&
+ msgs[1].len > 0 &&
msgs[0].addr == msgs[1].addr) {
- if (msgs[1].flags & I2C_M_RD)
- ret = octeon_i2c_hlc_comp_read(i2c, msgs);
- else
- ret = octeon_i2c_hlc_comp_write(i2c, msgs);
- goto out;
+ if (msgs[1].len <= 8) {
+ if (msgs[1].flags & I2C_M_RD)
+ ret = octeon_i2c_hlc_comp_read(i2c, msgs);
+ else
+ ret = octeon_i2c_hlc_comp_write(i2c, msgs);
+ goto out;
+ } else if (msgs[1].len <= 1024 && TWSI_BLOCK_CTL(i2c)) {
+ if (msgs[1].flags & I2C_M_RD)
+ ret = octeon_i2c_hlc_block_comp_read(i2c, msgs);
+ else
+ ret = octeon_i2c_hlc_block_comp_write(i2c, msgs);
+ goto out;
+ }
}
}
@@ -720,6 +869,7 @@ int octeon_i2c_init_lowlevel(struct octeon_i2c *i2c)
/* toggle twice to force both teardowns */
octeon_i2c_hlc_enable(i2c);
octeon_i2c_hlc_disable(i2c);
+
return 0;
}
diff --git a/drivers/i2c/busses/i2c-octeon-core.h b/drivers/i2c/busses/i2c-octeon-core.h
index 9bb9f64fdda0..b889ecf9968d 100644
--- a/drivers/i2c/busses/i2c-octeon-core.h
+++ b/drivers/i2c/busses/i2c-octeon-core.h
@@ -85,6 +85,11 @@
#define TWSI_INT_SDA BIT_ULL(10)
#define TWSI_INT_SCL BIT_ULL(11)
+#define TWSI_MODE_STRETCH BIT_ULL(1)
+#define TWSI_MODE_BLOCK_MODE BIT_ULL(2)
+
+#define TWSI_BLOCK_STS_RESET_PTR BIT_ULL(0)
+#define TWSI_BLOCK_STS_BUSY BIT_ULL(1)
#define I2C_OCTEON_EVENT_WAIT 80 /* microseconds */
/* Register offsets */
@@ -92,11 +97,19 @@ struct octeon_i2c_reg_offset {
unsigned int sw_twsi;
unsigned int twsi_int;
unsigned int sw_twsi_ext;
+ unsigned int twsi_mode;
+ unsigned int twsi_block_ctl;
+ unsigned int twsi_block_sts;
+ unsigned int twsi_block_fifo;
};
#define SW_TWSI(x) (x->roff.sw_twsi)
#define TWSI_INT(x) (x->roff.twsi_int)
#define SW_TWSI_EXT(x) (x->roff.sw_twsi_ext)
+#define TWSI_MODE(x) (x->roff.twsi_mode)
+#define TWSI_BLOCK_CTL(x) (x->roff.twsi_block_ctl)
+#define TWSI_BLOCK_STS(x) (x->roff.twsi_block_sts)
+#define TWSI_BLOCK_FIFO(x) (x->roff.twsi_block_fifo)
struct octeon_i2c {
wait_queue_head_t queue;
@@ -110,6 +123,7 @@ struct octeon_i2c {
void __iomem *twsi_base;
struct device *dev;
bool hlc_enabled;
+ bool block_enabled;
bool broken_irq_mode;
bool broken_irq_check;
void (*int_enable)(struct octeon_i2c *);
diff --git a/drivers/i2c/busses/i2c-thunderx-pcidrv.c b/drivers/i2c/busses/i2c-thunderx-pcidrv.c
index a77cd86fe75e..abde98117d7e 100644
--- a/drivers/i2c/busses/i2c-thunderx-pcidrv.c
+++ b/drivers/i2c/busses/i2c-thunderx-pcidrv.c
@@ -165,6 +165,10 @@ static int thunder_i2c_probe_pci(struct pci_dev *pdev,
i2c->roff.sw_twsi = 0x1000;
i2c->roff.twsi_int = 0x1010;
i2c->roff.sw_twsi_ext = 0x1018;
+ i2c->roff.twsi_mode = 0x1038;
+ i2c->roff.twsi_block_ctl = 0x1048;
+ i2c->roff.twsi_block_sts = 0x1050;
+ i2c->roff.twsi_block_fifo = 0x1058;
i2c->dev = dev;
pci_set_drvdata(pdev, i2c);
--
2.41.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] i2c:octeon:Add block-mode r/w
2023-07-05 23:45 [PATCH] i2c:octeon:Add block-mode r/w Aryan Srivastava
@ 2023-07-25 22:33 ` Andi Shyti
2023-08-16 23:07 ` Aryan Srivastava
0 siblings, 1 reply; 17+ messages in thread
From: Andi Shyti @ 2023-07-25 22:33 UTC (permalink / raw
To: Aryan Srivastava; +Cc: linux-i2c, linux-kernel
Hi Aryan,
On Thu, Jul 06, 2023 at 11:45:00AM +1200, Aryan Srivastava wrote:
> Add support for block mode read/write operations on
> Thunderx chips.
>
> When attempting r/w operations of greater then 8 bytes
> block mode is used, instead of performing a series of
> 8 byte reads.
>
> Signed-off-by: Aryan Srivastava <aryan.srivastava@alliedtelesis.co.nz>
thanks for your patch, could you please run checkpatch.pl,
clean it up a little (e.g. comments like /* A */) and resend it?
Thanks,
Andi
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] i2c:octeon:Add block-mode r/w
2023-07-25 22:33 ` Andi Shyti
@ 2023-08-16 23:07 ` Aryan Srivastava
2023-09-03 12:34 ` Andi Shyti
0 siblings, 1 reply; 17+ messages in thread
From: Aryan Srivastava @ 2023-08-16 23:07 UTC (permalink / raw
To: andi.shyti; +Cc: aryan.srivastava, linux-i2c, linux-kernel
Add support for block mode read/write operations on
Thunderx chips.
When attempting r/w operations of greater then 8 bytes
block mode is used, instead of performing a series of
8 byte reads.
Signed-off-by: Aryan Srivastava <aryan.srivastava@alliedtelesis.co.nz>
---
Hi Andi,
I have re-ran the patch through checkpatch --strict and changed the
comments
Thanks,
Aryan
Changes in v2:
- comment style and formatting.
drivers/i2c/busses/i2c-octeon-core.c | 162 ++++++++++++++++++++++-
drivers/i2c/busses/i2c-octeon-core.h | 14 ++
drivers/i2c/busses/i2c-thunderx-pcidrv.c | 4 +
3 files changed, 174 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/busses/i2c-octeon-core.c b/drivers/i2c/busses/i2c-octeon-core.c
index 845eda70b8ca..27a7f61f575b 100644
--- a/drivers/i2c/busses/i2c-octeon-core.c
+++ b/drivers/i2c/busses/i2c-octeon-core.c
@@ -130,6 +130,24 @@ static void octeon_i2c_hlc_disable(struct octeon_i2c *i2c)
octeon_i2c_ctl_write(i2c, TWSI_CTL_ENAB);
}
+static void octeon_i2c_block_enable(struct octeon_i2c *i2c)
+{
+ if (i2c->block_enabled || !TWSI_BLOCK_CTL(i2c))
+ return;
+
+ i2c->block_enabled = true;
+ octeon_i2c_writeq_flush(TWSI_MODE_STRETCH
+ | TWSI_MODE_BLOCK_MODE, i2c->twsi_base + TWSI_MODE(i2c));
+}
+
+static void octeon_i2c_block_disable(struct octeon_i2c *i2c)
+{
+ if (!i2c->block_enabled || !TWSI_BLOCK_CTL(i2c))
+ return;
+
+ i2c->block_enabled = false;
+ octeon_i2c_writeq_flush(TWSI_MODE_STRETCH, i2c->twsi_base + TWSI_MODE(i2c));
+}
/**
* octeon_i2c_hlc_wait - wait for an HLC operation to complete
* @i2c: The struct octeon_i2c
@@ -268,6 +286,7 @@ static int octeon_i2c_start(struct octeon_i2c *i2c)
u8 stat;
octeon_i2c_hlc_disable(i2c);
+ octeon_i2c_block_disable(i2c);
octeon_i2c_ctl_write(i2c, TWSI_CTL_ENAB | TWSI_CTL_STA);
ret = octeon_i2c_wait(i2c);
@@ -594,6 +613,128 @@ static int octeon_i2c_hlc_comp_write(struct octeon_i2c *i2c, struct i2c_msg *msg
return ret;
}
+/* high-level-controller composite block write+read, m[0]len<=2, m[1]len<=1024 */
+static int octeon_i2c_hlc_block_comp_read(struct octeon_i2c *i2c, struct i2c_msg *msgs)
+{
+ int i, j, len, ret = 0;
+ u64 cmd, rd;
+
+ octeon_i2c_hlc_enable(i2c);
+ octeon_i2c_block_enable(i2c);
+
+ len = msgs[1].len - 1;
+ /* Prepare core command */
+ cmd = SW_TWSI_V | SW_TWSI_R | SW_TWSI_SOVR;
+ /* SIZE */
+ octeon_i2c_writeq_flush((u64)(len), i2c->twsi_base + TWSI_BLOCK_CTL(i2c));
+ /* ADDR */
+ 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;
+ }
+
+ /* Send command to core (send data to FIFO) */
+ octeon_i2c_hlc_int_clear(i2c);
+ octeon_i2c_writeq_flush(cmd, i2c->twsi_base + SW_TWSI(i2c));
+
+ /* Wait for transaction to complete */
+ ret = octeon_i2c_hlc_wait(i2c);
+ if (ret)
+ goto err;
+
+ cmd = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
+ if ((cmd & SW_TWSI_R) == 0)
+ return octeon_i2c_check_status(i2c, false);
+
+ /* read data in FIFO */
+ octeon_i2c_writeq_flush(TWSI_BLOCK_STS_RESET_PTR, i2c->twsi_base + TWSI_BLOCK_STS(i2c));
+ for (i = 0; i < len; i += 8) {
+ rd = __raw_readq(i2c->twsi_base + TWSI_BLOCK_FIFO(i2c));
+ for (j = 7; j >= 0; j--)
+ msgs[1].buf[i + (7 - j)] = (rd >> (8 * j)) & 0xff;
+ }
+
+ octeon_i2c_block_disable(i2c);
+
+err:
+ return ret;
+}
+
+/* high-level-controller composite block write+write, m[0]len<=2, m[1]len<=1024 */
+static int octeon_i2c_hlc_block_comp_write(struct octeon_i2c *i2c, struct i2c_msg *msgs)
+{
+ bool set_ext = false;
+ int i, j, len, ret = 0;
+ u64 cmd, buf = 0, ext = 0;
+
+ octeon_i2c_hlc_enable(i2c);
+ octeon_i2c_block_enable(i2c);
+
+ len = msgs[1].len - 1;
+ /* Prepare core command */
+ cmd = SW_TWSI_V | SW_TWSI_SOVR;
+ /* SIZE */
+ octeon_i2c_writeq_flush((u64)(len), i2c->twsi_base + TWSI_BLOCK_CTL(i2c));
+ /* ADDR */
+ 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;
+ }
+
+ /* Write msg into FIFO buffer */
+ octeon_i2c_writeq_flush(TWSI_BLOCK_STS_RESET_PTR, i2c->twsi_base + TWSI_BLOCK_STS(i2c));
+ for (i = 0; i < len; i += 8) {
+ buf = 0;
+ for (j = 7; j >= 0; j--)
+ buf |= (msgs[1].buf[i + (7 - j)] << (8 * j));
+ octeon_i2c_writeq_flush(buf, i2c->twsi_base + TWSI_BLOCK_FIFO(i2c));
+ }
+ if (set_ext)
+ octeon_i2c_writeq_flush(ext, i2c->twsi_base + SW_TWSI_EXT(i2c));
+
+ /* Send command to core (send data in FIFO) */
+ octeon_i2c_hlc_int_clear(i2c);
+ octeon_i2c_writeq_flush(cmd, i2c->twsi_base + SW_TWSI(i2c));
+
+ /* Wait for transaction to complete */
+ ret = octeon_i2c_hlc_wait(i2c);
+ if (ret)
+ goto err;
+
+ cmd = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
+ if ((cmd & SW_TWSI_R) == 0)
+ return octeon_i2c_check_status(i2c, false);
+
+ octeon_i2c_block_disable(i2c);
+
+err:
+ return ret;
+}
+
/**
* octeon_i2c_xfer - The driver's master_xfer function
* @adap: Pointer to the i2c_adapter structure
@@ -619,13 +760,21 @@ int octeon_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
if ((msgs[0].flags & I2C_M_RD) == 0 &&
(msgs[1].flags & I2C_M_RECV_LEN) == 0 &&
msgs[0].len > 0 && msgs[0].len <= 2 &&
- msgs[1].len > 0 && msgs[1].len <= 8 &&
+ msgs[1].len > 0 &&
msgs[0].addr == msgs[1].addr) {
- if (msgs[1].flags & I2C_M_RD)
- ret = octeon_i2c_hlc_comp_read(i2c, msgs);
- else
- ret = octeon_i2c_hlc_comp_write(i2c, msgs);
- goto out;
+ if (msgs[1].len <= 8) {
+ if (msgs[1].flags & I2C_M_RD)
+ ret = octeon_i2c_hlc_comp_read(i2c, msgs);
+ else
+ ret = octeon_i2c_hlc_comp_write(i2c, msgs);
+ goto out;
+ } else if (msgs[1].len <= 1024 && TWSI_BLOCK_CTL(i2c)) {
+ if (msgs[1].flags & I2C_M_RD)
+ ret = octeon_i2c_hlc_block_comp_read(i2c, msgs);
+ else
+ ret = octeon_i2c_hlc_block_comp_write(i2c, msgs);
+ goto out;
+ }
}
}
@@ -720,6 +869,7 @@ int octeon_i2c_init_lowlevel(struct octeon_i2c *i2c)
/* toggle twice to force both teardowns */
octeon_i2c_hlc_enable(i2c);
octeon_i2c_hlc_disable(i2c);
+
return 0;
}
diff --git a/drivers/i2c/busses/i2c-octeon-core.h b/drivers/i2c/busses/i2c-octeon-core.h
index 9bb9f64fdda0..b889ecf9968d 100644
--- a/drivers/i2c/busses/i2c-octeon-core.h
+++ b/drivers/i2c/busses/i2c-octeon-core.h
@@ -85,6 +85,11 @@
#define TWSI_INT_SDA BIT_ULL(10)
#define TWSI_INT_SCL BIT_ULL(11)
+#define TWSI_MODE_STRETCH BIT_ULL(1)
+#define TWSI_MODE_BLOCK_MODE BIT_ULL(2)
+
+#define TWSI_BLOCK_STS_RESET_PTR BIT_ULL(0)
+#define TWSI_BLOCK_STS_BUSY BIT_ULL(1)
#define I2C_OCTEON_EVENT_WAIT 80 /* microseconds */
/* Register offsets */
@@ -92,11 +97,19 @@ struct octeon_i2c_reg_offset {
unsigned int sw_twsi;
unsigned int twsi_int;
unsigned int sw_twsi_ext;
+ unsigned int twsi_mode;
+ unsigned int twsi_block_ctl;
+ unsigned int twsi_block_sts;
+ unsigned int twsi_block_fifo;
};
#define SW_TWSI(x) (x->roff.sw_twsi)
#define TWSI_INT(x) (x->roff.twsi_int)
#define SW_TWSI_EXT(x) (x->roff.sw_twsi_ext)
+#define TWSI_MODE(x) (x->roff.twsi_mode)
+#define TWSI_BLOCK_CTL(x) (x->roff.twsi_block_ctl)
+#define TWSI_BLOCK_STS(x) (x->roff.twsi_block_sts)
+#define TWSI_BLOCK_FIFO(x) (x->roff.twsi_block_fifo)
struct octeon_i2c {
wait_queue_head_t queue;
@@ -110,6 +123,7 @@ struct octeon_i2c {
void __iomem *twsi_base;
struct device *dev;
bool hlc_enabled;
+ bool block_enabled;
bool broken_irq_mode;
bool broken_irq_check;
void (*int_enable)(struct octeon_i2c *);
diff --git a/drivers/i2c/busses/i2c-thunderx-pcidrv.c b/drivers/i2c/busses/i2c-thunderx-pcidrv.c
index a77cd86fe75e..abde98117d7e 100644
--- a/drivers/i2c/busses/i2c-thunderx-pcidrv.c
+++ b/drivers/i2c/busses/i2c-thunderx-pcidrv.c
@@ -165,6 +165,10 @@ static int thunder_i2c_probe_pci(struct pci_dev *pdev,
i2c->roff.sw_twsi = 0x1000;
i2c->roff.twsi_int = 0x1010;
i2c->roff.sw_twsi_ext = 0x1018;
+ i2c->roff.twsi_mode = 0x1038;
+ i2c->roff.twsi_block_ctl = 0x1048;
+ i2c->roff.twsi_block_sts = 0x1050;
+ i2c->roff.twsi_block_fifo = 0x1058;
i2c->dev = dev;
pci_set_drvdata(pdev, i2c);
--
2.41.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] i2c:octeon:Add block-mode r/w
2023-08-16 23:07 ` Aryan Srivastava
@ 2023-09-03 12:34 ` Andi Shyti
2023-09-04 23:14 ` Aryan Srivastava
0 siblings, 1 reply; 17+ messages in thread
From: Andi Shyti @ 2023-09-03 12:34 UTC (permalink / raw
To: Aryan Srivastava; +Cc: linux-i2c, linux-kernel
Hi Aryan,
[...]
> I have re-ran the patch through checkpatch --strict and changed the
> comments
Thanks!
[...]
> +static void octeon_i2c_block_disable(struct octeon_i2c *i2c)
> +{
> + if (!i2c->block_enabled || !TWSI_BLOCK_CTL(i2c))
> + return;
> +
> + i2c->block_enabled = false;
> + octeon_i2c_writeq_flush(TWSI_MODE_STRETCH, i2c->twsi_base + TWSI_MODE(i2c));
> +}
can you leave a blank line here?
> /**
> * octeon_i2c_hlc_wait - wait for an HLC operation to complete
> * @i2c: The struct octeon_i2c
> @@ -268,6 +286,7 @@ static int octeon_i2c_start(struct octeon_i2c *i2c)
> u8 stat;
>
> octeon_i2c_hlc_disable(i2c);
> + octeon_i2c_block_disable(i2c);
>
> octeon_i2c_ctl_write(i2c, TWSI_CTL_ENAB | TWSI_CTL_STA);
> ret = octeon_i2c_wait(i2c);
> @@ -594,6 +613,128 @@ static int octeon_i2c_hlc_comp_write(struct octeon_i2c *i2c, struct i2c_msg *msg
> return ret;
> }
>
> +/* high-level-controller composite block write+read, m[0]len<=2, m[1]len<=1024 */
can you please improve the comment?
> +static int octeon_i2c_hlc_block_comp_read(struct octeon_i2c *i2c, struct i2c_msg *msgs)
> +{
> + int i, j, len, ret = 0;
> + u64 cmd, rd;
> +
> + octeon_i2c_hlc_enable(i2c);
> + octeon_i2c_block_enable(i2c);
> +
> + len = msgs[1].len - 1;
why -1?
> + /* Prepare core command */
> + cmd = SW_TWSI_V | SW_TWSI_R | SW_TWSI_SOVR;
cmd needs to be initialized to '0'.
> + /* SIZE */
> + octeon_i2c_writeq_flush((u64)(len), i2c->twsi_base + TWSI_BLOCK_CTL(i2c));
> + /* ADDR */
> + cmd |= (u64)(msgs[0].addr & 0x7full) << SW_TWSI_ADDR_SHIFT;
can you please write some more meaningful comments?
[...]
> + /* Wait for transaction to complete */
> + ret = octeon_i2c_hlc_wait(i2c);
> + if (ret)
> + goto err;
just return err, no point having a goto label here.
[...]
> +err:
> + return ret;
> +}
> +
> +/* high-level-controller composite block write+write, m[0]len<=2, m[1]len<=1024 */
> +static int octeon_i2c_hlc_block_comp_write(struct octeon_i2c *i2c, struct i2c_msg *msgs)
more or less same comments apply here as _comp_read()
[...]
> @@ -720,6 +869,7 @@ int octeon_i2c_init_lowlevel(struct octeon_i2c *i2c)
> /* toggle twice to force both teardowns */
> octeon_i2c_hlc_enable(i2c);
> octeon_i2c_hlc_disable(i2c);
> +
this change does not belong here
> return 0;
> }
>
[...]
> #define TWSI_INT_SDA BIT_ULL(10)
> #define TWSI_INT_SCL BIT_ULL(11)
>
> +#define TWSI_MODE_STRETCH BIT_ULL(1)
> +#define TWSI_MODE_BLOCK_MODE BIT_ULL(2)
> +
> +#define TWSI_BLOCK_STS_RESET_PTR BIT_ULL(0)
> +#define TWSI_BLOCK_STS_BUSY BIT_ULL(1)
The alignment here is a bit off.
Andi
> #define I2C_OCTEON_EVENT_WAIT 80 /* microseconds */
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] i2c:octeon:Add block-mode r/w
2023-09-03 12:34 ` Andi Shyti
@ 2023-09-04 23:14 ` Aryan Srivastava
2023-09-05 6:27 ` kernel test robot
` (2 more replies)
0 siblings, 3 replies; 17+ messages in thread
From: Aryan Srivastava @ 2023-09-04 23:14 UTC (permalink / raw
To: andi.shyti; +Cc: aryan.srivastava, linux-i2c, linux-kernel
Add support for block mode read/write operations on
Thunderx chips.
When attempting r/w operations of greater then 8 bytes
block mode is used, instead of performing a series of
8 byte reads.
Signed-off-by: Aryan Srivastava <aryan.srivastava@alliedtelesis.co.nz>
---
Changes in v2:
- comment style and formatting.
Changes in v3:
- comment style and formatting.
---
drivers/i2c/busses/i2c-octeon-core.c | 158 ++++++++++++++++++++++-
drivers/i2c/busses/i2c-octeon-core.h | 14 ++
drivers/i2c/busses/i2c-thunderx-pcidrv.c | 4 +
3 files changed, 170 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/busses/i2c-octeon-core.c b/drivers/i2c/busses/i2c-octeon-core.c
index 845eda70b8ca..940cc4647cb0 100644
--- a/drivers/i2c/busses/i2c-octeon-core.c
+++ b/drivers/i2c/busses/i2c-octeon-core.c
@@ -130,6 +130,25 @@ static void octeon_i2c_hlc_disable(struct octeon_i2c *i2c)
octeon_i2c_ctl_write(i2c, TWSI_CTL_ENAB);
}
+static void octeon_i2c_block_enable(struct octeon_i2c *i2c)
+{
+ if (i2c->block_enabled || !TWSI_BLOCK_CTL(i2c))
+ return;
+
+ i2c->block_enabled = true;
+ octeon_i2c_writeq_flush(TWSI_MODE_STRETCH
+ | TWSI_MODE_BLOCK_MODE, i2c->twsi_base + TWSI_MODE(i2c));
+}
+
+static void octeon_i2c_block_disable(struct octeon_i2c *i2c)
+{
+ if (!i2c->block_enabled || !TWSI_BLOCK_CTL(i2c))
+ return;
+
+ i2c->block_enabled = false;
+ octeon_i2c_writeq_flush(TWSI_MODE_STRETCH, i2c->twsi_base + TWSI_MODE(i2c));
+}
+
/**
* octeon_i2c_hlc_wait - wait for an HLC operation to complete
* @i2c: The struct octeon_i2c
@@ -268,6 +287,7 @@ static int octeon_i2c_start(struct octeon_i2c *i2c)
u8 stat;
octeon_i2c_hlc_disable(i2c);
+ octeon_i2c_block_disable(i2c);
octeon_i2c_ctl_write(i2c, TWSI_CTL_ENAB | TWSI_CTL_STA);
ret = octeon_i2c_wait(i2c);
@@ -594,6 +614,124 @@ static int octeon_i2c_hlc_comp_write(struct octeon_i2c *i2c, struct i2c_msg *msg
return ret;
}
+/* high-level-controller composite block write+read, msg0=addr, msg1=data */
+static int octeon_i2c_hlc_block_comp_read(struct octeon_i2c *i2c, struct i2c_msg *msgs)
+{
+ int i, j, len, ret = 0;
+ u64 cmd = 0, rd = 0;
+
+ octeon_i2c_hlc_enable(i2c);
+ octeon_i2c_block_enable(i2c);
+
+ /* Write (size - 1) into block control register */
+ len = msgs[1].len - 1;
+ octeon_i2c_writeq_flush((u64)(len), i2c->twsi_base + TWSI_BLOCK_CTL(i2c));
+
+ /* Prepare core command */
+ cmd = SW_TWSI_V | SW_TWSI_R | SW_TWSI_SOVR;
+ 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;
+ }
+
+ /* Send command to core (send data to FIFO) */
+ octeon_i2c_hlc_int_clear(i2c);
+ octeon_i2c_writeq_flush(cmd, i2c->twsi_base + SW_TWSI(i2c));
+
+ /* Wait for transaction to complete */
+ ret = octeon_i2c_hlc_wait(i2c);
+ if (ret)
+ return ret;
+
+ cmd = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
+ if ((cmd & SW_TWSI_R) == 0)
+ return octeon_i2c_check_status(i2c, false);
+
+ /* read data in FIFO */
+ octeon_i2c_writeq_flush(TWSI_BLOCK_STS_RESET_PTR, i2c->twsi_base + TWSI_BLOCK_STS(i2c));
+ for (i = 0; i < len; i += 8) {
+ rd = __raw_readq(i2c->twsi_base + TWSI_BLOCK_FIFO(i2c));
+ for (j = 7; j >= 0; j--)
+ msgs[1].buf[i + (7 - j)] = (rd >> (8 * j)) & 0xff;
+ }
+
+ octeon_i2c_block_disable(i2c);
+ return ret;
+}
+
+/* high-level-controller composite block write+write, m[0]len<=2, m[1]len<=1024 */
+static int octeon_i2c_hlc_block_comp_write(struct octeon_i2c *i2c, struct i2c_msg *msgs)
+{
+ bool set_ext = false;
+ int i, j, len, ret = 0;
+ u64 cmd, buf = 0, ext = 0;
+
+ octeon_i2c_hlc_enable(i2c);
+ octeon_i2c_block_enable(i2c);
+
+ /* Write (size - 1) into block control register */
+ len = msgs[1].len - 1;
+ octeon_i2c_writeq_flush((u64)(len), i2c->twsi_base + TWSI_BLOCK_CTL(i2c));
+
+ /* Prepare core command */
+ cmd = SW_TWSI_V | SW_TWSI_SOVR;
+ 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;
+ }
+
+ /* Write msg into FIFO buffer */
+ octeon_i2c_writeq_flush(TWSI_BLOCK_STS_RESET_PTR, i2c->twsi_base + TWSI_BLOCK_STS(i2c));
+ for (i = 0; i < len; i += 8) {
+ buf = 0;
+ for (j = 7; j >= 0; j--)
+ buf |= (msgs[1].buf[i + (7 - j)] << (8 * j));
+ octeon_i2c_writeq_flush(buf, i2c->twsi_base + TWSI_BLOCK_FIFO(i2c));
+ }
+ if (set_ext)
+ octeon_i2c_writeq_flush(ext, i2c->twsi_base + SW_TWSI_EXT(i2c));
+
+ /* Send command to core (send data in FIFO) */
+ octeon_i2c_hlc_int_clear(i2c);
+ octeon_i2c_writeq_flush(cmd, i2c->twsi_base + SW_TWSI(i2c));
+
+ /* Wait for transaction to complete */
+ ret = octeon_i2c_hlc_wait(i2c);
+ if (ret)
+ goto ret;
+
+ cmd = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
+ if ((cmd & SW_TWSI_R) == 0)
+ return octeon_i2c_check_status(i2c, false);
+
+ octeon_i2c_block_disable(i2c);
+ return ret;
+}
+
/**
* octeon_i2c_xfer - The driver's master_xfer function
* @adap: Pointer to the i2c_adapter structure
@@ -619,13 +757,21 @@ int octeon_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
if ((msgs[0].flags & I2C_M_RD) == 0 &&
(msgs[1].flags & I2C_M_RECV_LEN) == 0 &&
msgs[0].len > 0 && msgs[0].len <= 2 &&
- msgs[1].len > 0 && msgs[1].len <= 8 &&
+ msgs[1].len > 0 &&
msgs[0].addr == msgs[1].addr) {
- if (msgs[1].flags & I2C_M_RD)
- ret = octeon_i2c_hlc_comp_read(i2c, msgs);
- else
- ret = octeon_i2c_hlc_comp_write(i2c, msgs);
- goto out;
+ if (msgs[1].len <= 8) {
+ if (msgs[1].flags & I2C_M_RD)
+ ret = octeon_i2c_hlc_comp_read(i2c, msgs);
+ else
+ ret = octeon_i2c_hlc_comp_write(i2c, msgs);
+ goto out;
+ } else if (msgs[1].len <= 1024 && TWSI_BLOCK_CTL(i2c)) {
+ if (msgs[1].flags & I2C_M_RD)
+ ret = octeon_i2c_hlc_block_comp_read(i2c, msgs);
+ else
+ ret = octeon_i2c_hlc_block_comp_write(i2c, msgs);
+ goto out;
+ }
}
}
diff --git a/drivers/i2c/busses/i2c-octeon-core.h b/drivers/i2c/busses/i2c-octeon-core.h
index 9bb9f64fdda0..81fcf413c890 100644
--- a/drivers/i2c/busses/i2c-octeon-core.h
+++ b/drivers/i2c/busses/i2c-octeon-core.h
@@ -85,6 +85,11 @@
#define TWSI_INT_SDA BIT_ULL(10)
#define TWSI_INT_SCL BIT_ULL(11)
+#define TWSI_MODE_STRETCH BIT_ULL(1)
+#define TWSI_MODE_BLOCK_MODE BIT_ULL(2)
+
+#define TWSI_BLOCK_STS_RESET_PTR BIT_ULL(0)
+#define TWSI_BLOCK_STS_BUSY BIT_ULL(1)
#define I2C_OCTEON_EVENT_WAIT 80 /* microseconds */
/* Register offsets */
@@ -92,11 +97,19 @@ struct octeon_i2c_reg_offset {
unsigned int sw_twsi;
unsigned int twsi_int;
unsigned int sw_twsi_ext;
+ unsigned int twsi_mode;
+ unsigned int twsi_block_ctl;
+ unsigned int twsi_block_sts;
+ unsigned int twsi_block_fifo;
};
#define SW_TWSI(x) (x->roff.sw_twsi)
#define TWSI_INT(x) (x->roff.twsi_int)
#define SW_TWSI_EXT(x) (x->roff.sw_twsi_ext)
+#define TWSI_MODE(x) (x->roff.twsi_mode)
+#define TWSI_BLOCK_CTL(x) (x->roff.twsi_block_ctl)
+#define TWSI_BLOCK_STS(x) (x->roff.twsi_block_sts)
+#define TWSI_BLOCK_FIFO(x) (x->roff.twsi_block_fifo)
struct octeon_i2c {
wait_queue_head_t queue;
@@ -110,6 +123,7 @@ struct octeon_i2c {
void __iomem *twsi_base;
struct device *dev;
bool hlc_enabled;
+ bool block_enabled;
bool broken_irq_mode;
bool broken_irq_check;
void (*int_enable)(struct octeon_i2c *);
diff --git a/drivers/i2c/busses/i2c-thunderx-pcidrv.c b/drivers/i2c/busses/i2c-thunderx-pcidrv.c
index a77cd86fe75e..abde98117d7e 100644
--- a/drivers/i2c/busses/i2c-thunderx-pcidrv.c
+++ b/drivers/i2c/busses/i2c-thunderx-pcidrv.c
@@ -165,6 +165,10 @@ static int thunder_i2c_probe_pci(struct pci_dev *pdev,
i2c->roff.sw_twsi = 0x1000;
i2c->roff.twsi_int = 0x1010;
i2c->roff.sw_twsi_ext = 0x1018;
+ i2c->roff.twsi_mode = 0x1038;
+ i2c->roff.twsi_block_ctl = 0x1048;
+ i2c->roff.twsi_block_sts = 0x1050;
+ i2c->roff.twsi_block_fifo = 0x1058;
i2c->dev = dev;
pci_set_drvdata(pdev, i2c);
--
2.42.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] i2c:octeon:Add block-mode r/w
2023-09-04 23:14 ` Aryan Srivastava
@ 2023-09-05 6:27 ` kernel test robot
2023-09-05 7:52 ` kernel test robot
2023-09-05 10:22 ` Andi Shyti
2 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2023-09-05 6:27 UTC (permalink / raw
To: Aryan Srivastava, andi.shyti
Cc: oe-kbuild-all, aryan.srivastava, linux-i2c, linux-kernel
Hi Aryan,
kernel test robot noticed the following build errors:
[auto build test ERROR on wsa/i2c/for-next]
[also build test ERROR on linus/master v6.5 next-20230831]
[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-Add-block-mode-r-w/20230905-071739
base: https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
patch link: https://lore.kernel.org/r/20230904231439.485925-1-aryan.srivastava%40alliedtelesis.co.nz
patch subject: [PATCH] i2c:octeon:Add block-mode r/w
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20230905/202309051413.SkL3myiV-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230905/202309051413.SkL3myiV-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/202309051413.SkL3myiV-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/i2c/busses/i2c-octeon-core.c: In function 'octeon_i2c_hlc_block_comp_write':
>> drivers/i2c/busses/i2c-octeon-core.c:725:17: error: label 'ret' used but not defined
725 | goto ret;
| ^~~~
vim +/ret +725 drivers/i2c/busses/i2c-octeon-core.c
674
675 /* high-level-controller composite block write+write, m[0]len<=2, m[1]len<=1024 */
676 static int octeon_i2c_hlc_block_comp_write(struct octeon_i2c *i2c, struct i2c_msg *msgs)
677 {
678 bool set_ext = false;
679 int i, j, len, ret = 0;
680 u64 cmd, buf = 0, ext = 0;
681
682 octeon_i2c_hlc_enable(i2c);
683 octeon_i2c_block_enable(i2c);
684
685 /* Write (size - 1) into block control register */
686 len = msgs[1].len - 1;
687 octeon_i2c_writeq_flush((u64)(len), i2c->twsi_base + TWSI_BLOCK_CTL(i2c));
688
689 /* Prepare core command */
690 cmd = SW_TWSI_V | SW_TWSI_SOVR;
691 cmd |= (u64)(msgs[0].addr & 0x7full) << SW_TWSI_ADDR_SHIFT;
692
693 if (msgs[0].flags & I2C_M_TEN)
694 cmd |= SW_TWSI_OP_10_IA;
695 else
696 cmd |= SW_TWSI_OP_7_IA;
697
698 if (msgs[0].len == 2) {
699 cmd |= SW_TWSI_EIA;
700 ext |= (u64)msgs[0].buf[0] << SW_TWSI_IA_SHIFT;
701 set_ext = true;
702 cmd |= (u64)msgs[0].buf[1] << SW_TWSI_IA_SHIFT;
703 } else {
704 cmd |= (u64)msgs[0].buf[0] << SW_TWSI_IA_SHIFT;
705 }
706
707 /* Write msg into FIFO buffer */
708 octeon_i2c_writeq_flush(TWSI_BLOCK_STS_RESET_PTR, i2c->twsi_base + TWSI_BLOCK_STS(i2c));
709 for (i = 0; i < len; i += 8) {
710 buf = 0;
711 for (j = 7; j >= 0; j--)
712 buf |= (msgs[1].buf[i + (7 - j)] << (8 * j));
713 octeon_i2c_writeq_flush(buf, i2c->twsi_base + TWSI_BLOCK_FIFO(i2c));
714 }
715 if (set_ext)
716 octeon_i2c_writeq_flush(ext, i2c->twsi_base + SW_TWSI_EXT(i2c));
717
718 /* Send command to core (send data in FIFO) */
719 octeon_i2c_hlc_int_clear(i2c);
720 octeon_i2c_writeq_flush(cmd, i2c->twsi_base + SW_TWSI(i2c));
721
722 /* Wait for transaction to complete */
723 ret = octeon_i2c_hlc_wait(i2c);
724 if (ret)
> 725 goto ret;
726
727 cmd = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
728 if ((cmd & SW_TWSI_R) == 0)
729 return octeon_i2c_check_status(i2c, false);
730
731 octeon_i2c_block_disable(i2c);
732 return ret;
733 }
734
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] i2c:octeon:Add block-mode r/w
2023-09-04 23:14 ` Aryan Srivastava
2023-09-05 6:27 ` kernel test robot
@ 2023-09-05 7:52 ` kernel test robot
2023-09-05 10:22 ` Andi Shyti
2 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2023-09-05 7:52 UTC (permalink / raw
To: Aryan Srivastava, andi.shyti
Cc: llvm, oe-kbuild-all, aryan.srivastava, linux-i2c, linux-kernel
Hi Aryan,
kernel test robot noticed the following build errors:
[auto build test ERROR on wsa/i2c/for-next]
[also build test ERROR on linus/master v6.5 next-20230905]
[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-Add-block-mode-r-w/20230905-071739
base: https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
patch link: https://lore.kernel.org/r/20230904231439.485925-1-aryan.srivastava%40alliedtelesis.co.nz
patch subject: [PATCH] i2c:octeon:Add block-mode r/w
config: riscv-randconfig-001-20230905 (https://download.01.org/0day-ci/archive/20230905/202309051505.K5sogLFe-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230905/202309051505.K5sogLFe-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/202309051505.K5sogLFe-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/i2c/busses/i2c-octeon-core.c:725:8: error: use of undeclared label 'ret'
725 | goto ret;
| ^
1 error generated.
vim +/ret +725 drivers/i2c/busses/i2c-octeon-core.c
674
675 /* high-level-controller composite block write+write, m[0]len<=2, m[1]len<=1024 */
676 static int octeon_i2c_hlc_block_comp_write(struct octeon_i2c *i2c, struct i2c_msg *msgs)
677 {
678 bool set_ext = false;
679 int i, j, len, ret = 0;
680 u64 cmd, buf = 0, ext = 0;
681
682 octeon_i2c_hlc_enable(i2c);
683 octeon_i2c_block_enable(i2c);
684
685 /* Write (size - 1) into block control register */
686 len = msgs[1].len - 1;
687 octeon_i2c_writeq_flush((u64)(len), i2c->twsi_base + TWSI_BLOCK_CTL(i2c));
688
689 /* Prepare core command */
690 cmd = SW_TWSI_V | SW_TWSI_SOVR;
691 cmd |= (u64)(msgs[0].addr & 0x7full) << SW_TWSI_ADDR_SHIFT;
692
693 if (msgs[0].flags & I2C_M_TEN)
694 cmd |= SW_TWSI_OP_10_IA;
695 else
696 cmd |= SW_TWSI_OP_7_IA;
697
698 if (msgs[0].len == 2) {
699 cmd |= SW_TWSI_EIA;
700 ext |= (u64)msgs[0].buf[0] << SW_TWSI_IA_SHIFT;
701 set_ext = true;
702 cmd |= (u64)msgs[0].buf[1] << SW_TWSI_IA_SHIFT;
703 } else {
704 cmd |= (u64)msgs[0].buf[0] << SW_TWSI_IA_SHIFT;
705 }
706
707 /* Write msg into FIFO buffer */
708 octeon_i2c_writeq_flush(TWSI_BLOCK_STS_RESET_PTR, i2c->twsi_base + TWSI_BLOCK_STS(i2c));
709 for (i = 0; i < len; i += 8) {
710 buf = 0;
711 for (j = 7; j >= 0; j--)
712 buf |= (msgs[1].buf[i + (7 - j)] << (8 * j));
713 octeon_i2c_writeq_flush(buf, i2c->twsi_base + TWSI_BLOCK_FIFO(i2c));
714 }
715 if (set_ext)
716 octeon_i2c_writeq_flush(ext, i2c->twsi_base + SW_TWSI_EXT(i2c));
717
718 /* Send command to core (send data in FIFO) */
719 octeon_i2c_hlc_int_clear(i2c);
720 octeon_i2c_writeq_flush(cmd, i2c->twsi_base + SW_TWSI(i2c));
721
722 /* Wait for transaction to complete */
723 ret = octeon_i2c_hlc_wait(i2c);
724 if (ret)
> 725 goto ret;
726
727 cmd = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
728 if ((cmd & SW_TWSI_R) == 0)
729 return octeon_i2c_check_status(i2c, false);
730
731 octeon_i2c_block_disable(i2c);
732 return ret;
733 }
734
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] i2c:octeon:Add block-mode r/w
2023-09-04 23:14 ` Aryan Srivastava
2023-09-05 6:27 ` kernel test robot
2023-09-05 7:52 ` kernel test robot
@ 2023-09-05 10:22 ` Andi Shyti
2023-09-12 0:27 ` [PATCH] THUNDERX_I2C_BLOCK_MODE Aryan Srivastava
` (3 more replies)
2 siblings, 4 replies; 17+ messages in thread
From: Andi Shyti @ 2023-09-05 10:22 UTC (permalink / raw
To: Aryan Srivastava; +Cc: linux-i2c, linux-kernel
Hi Aryan,
In the title, please leave a space after the ':'
i2c: octeon: Add block-mode r/w
Please check with "git log drivers..." to see what's the rule in
a particular community.
I guess Wolfram can fix this, though, before pushing.
[...]
> +/* high-level-controller composite block write+read, msg0=addr, msg1=data */
I think this comment is fine and great to have it, but it's
missing a bit of clarity, can you please expand the concept?
> +static int octeon_i2c_hlc_block_comp_read(struct octeon_i2c *i2c, struct i2c_msg *msgs)
> +{
> + int i, j, len, ret = 0;
> + u64 cmd = 0, rd = 0;
can please you move rd, j inside the for loop? The basic common
sense is to have all variable declared in the innermost section
in order to avoid confusion.
It's a nitpick though, not a strong comment and, afaik, not a
real rule.
Same comment for the function below.
> +
> + octeon_i2c_hlc_enable(i2c);
> + octeon_i2c_block_enable(i2c);
> +
> + /* Write (size - 1) into block control register */
> + len = msgs[1].len - 1;
> + octeon_i2c_writeq_flush((u64)(len), i2c->twsi_base + TWSI_BLOCK_CTL(i2c));
> +
> + /* Prepare core command */
> + cmd = SW_TWSI_V | SW_TWSI_R | SW_TWSI_SOVR;
> + 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;
> + }
This first part is basically a copy/paste with the write()
function... can we put them together in a common function?
Can we put as much as we can in a single function?
> + /* Send command to core (send data to FIFO) */
> + octeon_i2c_hlc_int_clear(i2c);
> + octeon_i2c_writeq_flush(cmd, i2c->twsi_base + SW_TWSI(i2c));
> +
> + /* Wait for transaction to complete */
> + ret = octeon_i2c_hlc_wait(i2c);
> + if (ret)
> + return ret;
> +
> + cmd = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
> + if ((cmd & SW_TWSI_R) == 0)
> + return octeon_i2c_check_status(i2c, false);
> +
> + /* read data in FIFO */
> + octeon_i2c_writeq_flush(TWSI_BLOCK_STS_RESET_PTR, i2c->twsi_base + TWSI_BLOCK_STS(i2c));
> + for (i = 0; i < len; i += 8) {
> + rd = __raw_readq(i2c->twsi_base + TWSI_BLOCK_FIFO(i2c));
> + for (j = 7; j >= 0; j--)
is len always a multiple of 8?
> + msgs[1].buf[i + (7 - j)] = (rd >> (8 * j)) & 0xff;
> + }
> +
> + octeon_i2c_block_disable(i2c);
> + return ret;
> +}
[...]
> - msgs[1].len > 0 && msgs[1].len <= 8 &&
> + msgs[1].len > 0 &&
> msgs[0].addr == msgs[1].addr) {
> - if (msgs[1].flags & I2C_M_RD)
> - ret = octeon_i2c_hlc_comp_read(i2c, msgs);
> - else
> - ret = octeon_i2c_hlc_comp_write(i2c, msgs);
> - goto out;
> + if (msgs[1].len <= 8) {
> + if (msgs[1].flags & I2C_M_RD)
> + ret = octeon_i2c_hlc_comp_read(i2c, msgs);
> + else
> + ret = octeon_i2c_hlc_comp_write(i2c, msgs);
> + goto out;
> + } else if (msgs[1].len <= 1024 && TWSI_BLOCK_CTL(i2c)) {
> + if (msgs[1].flags & I2C_M_RD)
> + ret = octeon_i2c_hlc_block_comp_read(i2c, msgs);
> + else
> + ret = octeon_i2c_hlc_block_comp_write(i2c, msgs);
> + goto out;
> + }
the rest looks good...
Thanks,
Andi
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] THUNDERX_I2C_BLOCK_MODE
2023-09-05 10:22 ` Andi Shyti
@ 2023-09-12 0:27 ` Aryan Srivastava
2023-09-12 3:38 ` kernel test robot
2023-09-12 0:28 ` [PATCH] i2c:octeon:Add block-mode r/w Aryan Srivastava
` (2 subsequent siblings)
3 siblings, 1 reply; 17+ messages in thread
From: Aryan Srivastava @ 2023-09-12 0:27 UTC (permalink / raw
To: andi.shyti; +Cc: aryan.srivastava, linux-i2c, linux-kernel
i2c:octeon:Add block-mode r/w
Add support for block mode read/write operations on
Thunderx chips.
When attempting r/w operations of greater then 8 bytes
block mode is used, instead of performing a series of
8 byte reads.
Signed-off-by: Aryan Srivastava <aryan.srivastava@alliedtelesis.co.nz>
---
drivers/i2c/busses/i2c-octeon-core.c | 198 +++++++++++++++++++----
drivers/i2c/busses/i2c-octeon-core.h | 14 ++
drivers/i2c/busses/i2c-thunderx-pcidrv.c | 4 +
3 files changed, 186 insertions(+), 30 deletions(-)
diff --git a/drivers/i2c/busses/i2c-octeon-core.c b/drivers/i2c/busses/i2c-octeon-core.c
index 845eda70b8ca..4c704262d228 100644
--- a/drivers/i2c/busses/i2c-octeon-core.c
+++ b/drivers/i2c/busses/i2c-octeon-core.c
@@ -130,6 +130,25 @@ static void octeon_i2c_hlc_disable(struct octeon_i2c *i2c)
octeon_i2c_ctl_write(i2c, TWSI_CTL_ENAB);
}
+static void octeon_i2c_block_enable(struct octeon_i2c *i2c)
+{
+ if (i2c->block_enabled || !TWSI_BLOCK_CTL(i2c))
+ return;
+
+ i2c->block_enabled = true;
+ octeon_i2c_writeq_flush(TWSI_MODE_STRETCH
+ | TWSI_MODE_BLOCK_MODE, i2c->twsi_base + TWSI_MODE(i2c));
+}
+
+static void octeon_i2c_block_disable(struct octeon_i2c *i2c)
+{
+ if (!i2c->block_enabled || !TWSI_BLOCK_CTL(i2c))
+ return;
+
+ i2c->block_enabled = false;
+ octeon_i2c_writeq_flush(TWSI_MODE_STRETCH, i2c->twsi_base + TWSI_MODE(i2c));
+}
+
/**
* octeon_i2c_hlc_wait - wait for an HLC operation to complete
* @i2c: The struct octeon_i2c
@@ -268,6 +287,7 @@ static int octeon_i2c_start(struct octeon_i2c *i2c)
u8 stat;
octeon_i2c_hlc_disable(i2c);
+ octeon_i2c_block_disable(i2c);
octeon_i2c_ctl_write(i2c, TWSI_CTL_ENAB | TWSI_CTL_STA);
ret = octeon_i2c_wait(i2c);
@@ -485,6 +505,37 @@ 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);
+}
+
+/* Construct and send i2c transaction core cmd */
+static int octeon_i2c_hlc_cmd(struct octeon_i2c *i2c, struct i2c_msg msg, 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) {
+ u64 ext = 0;
+
+ cmd |= SW_TWSI_EIA;
+ ext = (u64)msg.buf[0] << SW_TWSI_IA_SHIFT;
+ cmd |= (u64)msg.buf[1] << SW_TWSI_IA_SHIFT;
+ octeon_i2c_writeq_flush(ext, i2c->twsi_base + SW_TWSI_EXT(i2c));
+ } else {
+ cmd |= (u64)msg.buf[0] << SW_TWSI_IA_SHIFT;
+ }
+
+ 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 +550,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;
@@ -579,10 +612,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;
@@ -594,6 +624,106 @@ static int octeon_i2c_hlc_comp_write(struct octeon_i2c *i2c, struct i2c_msg *msg
return ret;
}
+/**
+ * high-level-controller composite block write+read, msg0=addr, msg1=data
+ * Used in the case where the i2c xfer is for greater than 8 bytes of read data.
+ */
+static int octeon_i2c_hlc_block_comp_read(struct octeon_i2c *i2c, struct i2c_msg *msgs)
+{
+ int len, ret = 0;
+ u64 cmd = 0;
+
+ octeon_i2c_hlc_enable(i2c);
+ octeon_i2c_block_enable(i2c);
+
+ /* Write (size - 1) into block control register */
+ len = msgs[1].len - 1;
+ octeon_i2c_writeq_flush((u64)(len), i2c->twsi_base + TWSI_BLOCK_CTL(i2c));
+
+ /* Prepare core command */
+ cmd = SW_TWSI_V | SW_TWSI_R | SW_TWSI_SOVR;
+ cmd |= (u64)(msgs[0].addr & 0x7full) << SW_TWSI_ADDR_SHIFT;
+
+ /* Send core command */
+ ret = octeon_i2c_hlc_cmd(i2c, msgs[0], cmd);
+ if (ret)
+ return ret;
+
+ cmd = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
+ if ((cmd & SW_TWSI_R) == 0)
+ return octeon_i2c_check_status(i2c, false);
+
+ /* read data in FIFO */
+ octeon_i2c_writeq_flush(TWSI_BLOCK_STS_RESET_PTR, i2c->twsi_base + TWSI_BLOCK_STS(i2c));
+ for (int i = 0; i < len; i += 8) {
+ u64 rd = __raw_readq(i2c->twsi_base + TWSI_BLOCK_FIFO(i2c));
+ for (int j = 7; j >= 0; j--)
+ msgs[1].buf[i + (7 - j)] = (rd >> (8 * j)) & 0xff;
+ }
+
+ octeon_i2c_block_disable(i2c);
+ return ret;
+}
+
+/**
+ * high-level-controller composite block write+write, m[0]len<=2, m[1]len<=1024
+ * Used in the case where the i2c xfer is for greater than 8 bytes of write data.
+ */
+static int octeon_i2c_hlc_block_comp_write(struct octeon_i2c *i2c, struct i2c_msg *msgs)
+{
+ bool set_ext = false;
+ int i, j, len, ret = 0;
+ u64 cmd, ext = 0;
+
+ octeon_i2c_hlc_enable(i2c);
+ octeon_i2c_block_enable(i2c);
+
+ /* Write (size - 1) into block control register */
+ len = msgs[1].len - 1;
+ octeon_i2c_writeq_flush((u64)(len), i2c->twsi_base + TWSI_BLOCK_CTL(i2c));
+
+ /* Prepare core command */
+ cmd = SW_TWSI_V | SW_TWSI_SOVR;
+ 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;
+ }
+
+ /* Write msg into FIFO buffer */
+ octeon_i2c_writeq_flush(TWSI_BLOCK_STS_RESET_PTR, i2c->twsi_base + TWSI_BLOCK_STS(i2c));
+ for (i = 0; i < len; i += 8) {
+ u64 buf = 0;
+ for (j = 7; j >= 0; j--)
+ buf |= (msgs[1].buf[i + (7 - j)] << (8 * j));
+ octeon_i2c_writeq_flush(buf, i2c->twsi_base + TWSI_BLOCK_FIFO(i2c));
+ }
+ if (set_ext)
+ octeon_i2c_writeq_flush(ext, i2c->twsi_base + SW_TWSI_EXT(i2c));
+
+ /* Send command to core (send data in FIFO) */
+ ret = octeon_i2c_hlc_cmd_send(i2c, cmd);
+ if (ret)
+ return ret;
+
+ cmd = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
+ if ((cmd & SW_TWSI_R) == 0)
+ return octeon_i2c_check_status(i2c, false);
+
+ octeon_i2c_block_disable(i2c);
+ return ret;
+}
+
/**
* octeon_i2c_xfer - The driver's master_xfer function
* @adap: Pointer to the i2c_adapter structure
@@ -619,13 +749,21 @@ int octeon_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
if ((msgs[0].flags & I2C_M_RD) == 0 &&
(msgs[1].flags & I2C_M_RECV_LEN) == 0 &&
msgs[0].len > 0 && msgs[0].len <= 2 &&
- msgs[1].len > 0 && msgs[1].len <= 8 &&
+ msgs[1].len > 0 &&
msgs[0].addr == msgs[1].addr) {
- if (msgs[1].flags & I2C_M_RD)
- ret = octeon_i2c_hlc_comp_read(i2c, msgs);
- else
- ret = octeon_i2c_hlc_comp_write(i2c, msgs);
- goto out;
+ if (msgs[1].len <= 8) {
+ if (msgs[1].flags & I2C_M_RD)
+ ret = octeon_i2c_hlc_comp_read(i2c, msgs);
+ else
+ ret = octeon_i2c_hlc_comp_write(i2c, msgs);
+ goto out;
+ } else if (msgs[1].len <= 1024 && TWSI_BLOCK_CTL(i2c)) {
+ if (msgs[1].flags & I2C_M_RD)
+ ret = octeon_i2c_hlc_block_comp_read(i2c, msgs);
+ else
+ ret = octeon_i2c_hlc_block_comp_write(i2c, msgs);
+ goto out;
+ }
}
}
diff --git a/drivers/i2c/busses/i2c-octeon-core.h b/drivers/i2c/busses/i2c-octeon-core.h
index 9bb9f64fdda0..81fcf413c890 100644
--- a/drivers/i2c/busses/i2c-octeon-core.h
+++ b/drivers/i2c/busses/i2c-octeon-core.h
@@ -85,6 +85,11 @@
#define TWSI_INT_SDA BIT_ULL(10)
#define TWSI_INT_SCL BIT_ULL(11)
+#define TWSI_MODE_STRETCH BIT_ULL(1)
+#define TWSI_MODE_BLOCK_MODE BIT_ULL(2)
+
+#define TWSI_BLOCK_STS_RESET_PTR BIT_ULL(0)
+#define TWSI_BLOCK_STS_BUSY BIT_ULL(1)
#define I2C_OCTEON_EVENT_WAIT 80 /* microseconds */
/* Register offsets */
@@ -92,11 +97,19 @@ struct octeon_i2c_reg_offset {
unsigned int sw_twsi;
unsigned int twsi_int;
unsigned int sw_twsi_ext;
+ unsigned int twsi_mode;
+ unsigned int twsi_block_ctl;
+ unsigned int twsi_block_sts;
+ unsigned int twsi_block_fifo;
};
#define SW_TWSI(x) (x->roff.sw_twsi)
#define TWSI_INT(x) (x->roff.twsi_int)
#define SW_TWSI_EXT(x) (x->roff.sw_twsi_ext)
+#define TWSI_MODE(x) (x->roff.twsi_mode)
+#define TWSI_BLOCK_CTL(x) (x->roff.twsi_block_ctl)
+#define TWSI_BLOCK_STS(x) (x->roff.twsi_block_sts)
+#define TWSI_BLOCK_FIFO(x) (x->roff.twsi_block_fifo)
struct octeon_i2c {
wait_queue_head_t queue;
@@ -110,6 +123,7 @@ struct octeon_i2c {
void __iomem *twsi_base;
struct device *dev;
bool hlc_enabled;
+ bool block_enabled;
bool broken_irq_mode;
bool broken_irq_check;
void (*int_enable)(struct octeon_i2c *);
diff --git a/drivers/i2c/busses/i2c-thunderx-pcidrv.c b/drivers/i2c/busses/i2c-thunderx-pcidrv.c
index a77cd86fe75e..abde98117d7e 100644
--- a/drivers/i2c/busses/i2c-thunderx-pcidrv.c
+++ b/drivers/i2c/busses/i2c-thunderx-pcidrv.c
@@ -165,6 +165,10 @@ static int thunder_i2c_probe_pci(struct pci_dev *pdev,
i2c->roff.sw_twsi = 0x1000;
i2c->roff.twsi_int = 0x1010;
i2c->roff.sw_twsi_ext = 0x1018;
+ i2c->roff.twsi_mode = 0x1038;
+ i2c->roff.twsi_block_ctl = 0x1048;
+ i2c->roff.twsi_block_sts = 0x1050;
+ i2c->roff.twsi_block_fifo = 0x1058;
i2c->dev = dev;
pci_set_drvdata(pdev, i2c);
--
2.42.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] i2c:octeon:Add block-mode r/w
2023-09-05 10:22 ` Andi Shyti
2023-09-12 0:27 ` [PATCH] THUNDERX_I2C_BLOCK_MODE Aryan Srivastava
@ 2023-09-12 0:28 ` Aryan Srivastava
2023-09-12 1:16 ` [PATCH] i2c: octeon: Add " Aryan Srivastava
[not found] ` <9882daa4945914886b21642837816c2d99c027ac.camel@alliedtelesis.co.nz>
2024-04-15 0:52 ` [PATCH v4] i2c: octeon: Add " Aryan Srivastava
3 siblings, 1 reply; 17+ messages in thread
From: Aryan Srivastava @ 2023-09-12 0:28 UTC (permalink / raw
To: andi.shyti@kernel.org
Cc: Aryan Srivastava, linux-i2c@vger.kernel.org,
linux-kernel@vger.kernel.org
Hi, Andi,
On Tue, 2023-09-05 at 12:22 +0200, Andi Shyti wrote:
> Hi Aryan,
>
> In the title, please leave a space after the ':'
>
> i2c: octeon: Add block-mode r/w
>
> Please check with "git log drivers..." to see what's the rule in
> a particular community.
>
> I guess Wolfram can fix this, though, before pushing.
>
> [...]
>
Done
> > +/* high-level-controller composite block write+read, msg0=addr,
> > msg1=data */
>
> I think this comment is fine and great to have it, but it's
> missing a bit of clarity, can you please expand the concept?
>
Done, let me know if you want me to add more here.
> > +static int octeon_i2c_hlc_block_comp_read(struct octeon_i2c *i2c,
> > struct i2c_msg *msgs)
> > +{
> > + int i, j, len, ret = 0;
> > + u64 cmd = 0, rd = 0;
>
> can please you move rd, j inside the for loop? The basic common
> sense is to have all variable declared in the innermost section
> in order to avoid confusion.
>
> It's a nitpick though, not a strong comment and, afaik, not a
> real rule.
>
> Same comment for the function below.
>
Done, I agree they should be defined within loop. I was just trying to
match the other hlc r/w.
> > +
> > + octeon_i2c_hlc_enable(i2c);
> > + octeon_i2c_block_enable(i2c);
> > +
> > + /* Write (size - 1) into block control register */
> > + len = msgs[1].len - 1;
> > + octeon_i2c_writeq_flush((u64)(len), i2c->twsi_base +
> > TWSI_BLOCK_CTL(i2c));
> > +
> > + /* Prepare core command */
> > + cmd = SW_TWSI_V | SW_TWSI_R | SW_TWSI_SOVR;
> > + 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;
> > +No, but this doesnt really matter as the internal j loop will take
> > care of the remaining bytes.
> >
> > e.g. if the len is 9, then we will do
> > 0-7 in the first, then i = 8, which is < len, and then the internal
> > loop will do 8-17.
> > + 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;
> > + }
>
> This first part is basically a copy/paste with the write()
> function... can we put them together in a common function?
>
> Can we put as much as we can in a single function?
>
Done. Could not make a common for the write+writes, as they way we
insert data into buffers for writing are significantly different,
and don't occur after the core command send (i.e. the core command is
sent with/concurrently with the write data). Unlike the read, which
sends almost identical core commands and reads the buffer differently
afterwards. Though if would like it I could mangle together a function
for these as well.
> > + /* Send command to core (send data to FIFO) */
> > + octeon_i2c_hlc_int_clear(i2c);
> > + octeon_i2c_writeq_flush(cmd, i2c->twsi_base + SW_TWSI(i2c));
> > +
> > + /* Wait for transaction to complete */
> > + ret = octeon_i2c_hlc_wait(i2c);
> > + if (ret)
> > + return ret;
> > +
> > + cmd = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
> > + if ((cmd & SW_TWSI_R) == 0)
> > + return octeon_i2c_check_status(i2c, false);
> > +
> > + /* read data in FIFO */
> > + octeon_i2c_writeq_flush(TWSI_BLOCK_STS_RESET_PTR, i2c-
> > >twsi_base + TWSI_BLOCK_STS(i2c));
> > + for (i = 0; i < len; i += 8) {
> > + rd = __raw_readq(i2c->twsi_base +
> > TWSI_BLOCK_FIFO(i2c));
> > + for (j = 7; j >= 0; j--)
>
> is len always a multiple of 8?
>
> > + msgs[1].buf[i + (7 - j)] = (rd >> (8 * j)) &
> > 0xff;
> > + }
> > +
> > + octeon_i2c_block_disable(i2c);
> > + return ret;
> > +}
>
> [...]
No, but this doesnt really matter as the internal j loop will take care
of the remaining bytes.
e.g. if the len is 9, then we will do
0-7 in the first, then i = 8, which is < len, and then the internal
loop will do 8-17.
>
> > - msgs[1].len > 0 && msgs[1].len <= 8 &&
> > + msgs[1].len > 0 &&
> > msgs[0].addr == msgs[1].addr) {
> > - if (msgs[1].flags & I2C_M_RD)
> > - ret = octeon_i2c_hlc_comp_read(i2c,
> > msgs);
> > - else
> > - ret = octeon_i2c_hlc_comp_write(i2c,
> > msgs);
> > - goto out;
> > + if (msgs[1].len <= 8) {
> > + if (msgs[1].flags & I2C_M_RD)
> > + ret =
> > octeon_i2c_hlc_comp_read(i2c, msgs);
> > + else
> > + ret =
> > octeon_i2c_hlc_comp_write(i2c, msgs);
> > + goto out;
> > + } else if (msgs[1].len <= 1024 &&
> > TWSI_BLOCK_CTL(i2c)) {
> > + if (msgs[1].flags & I2C_M_RD)
> > + ret =
> > octeon_i2c_hlc_block_comp_read(i2c, msgs);
> > + else
> > + ret =
> > octeon_i2c_hlc_block_comp_write(i2c, msgs);
> > + goto out;
> > + }
>
> the rest looks good...
>
> Thanks,
> Andi
Thanks,
Aryan
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH] i2c: octeon: Add block-mode r/w
2023-09-12 0:28 ` [PATCH] i2c:octeon:Add block-mode r/w Aryan Srivastava
@ 2023-09-12 1:16 ` Aryan Srivastava
2024-01-16 1:38 ` Aryan Srivastava
0 siblings, 1 reply; 17+ messages in thread
From: Aryan Srivastava @ 2023-09-12 1:16 UTC (permalink / raw
To: aryan.srivastava; +Cc: andi.shyti, linux-i2c, linux-kernel
Add support for block mode read/write operations on
Thunderx chips.
When attempting r/w operations of greater then 8 bytes
block mode is used, instead of performing a series of
8 byte reads.
Signed-off-by: Aryan Srivastava <aryan.srivastava@alliedtelesis.co.nz>
---
Changes in v2:
- comment style and formatting.
Changes in v3:
- comment style and formatting.
Changes in v4:
- Refactoring common code.
- Additional comments.
---
drivers/i2c/busses/i2c-octeon-core.c | 196 +++++++++++++++++++----
drivers/i2c/busses/i2c-octeon-core.h | 14 ++
drivers/i2c/busses/i2c-thunderx-pcidrv.c | 4 +
3 files changed, 185 insertions(+), 29 deletions(-)
diff --git a/drivers/i2c/busses/i2c-octeon-core.c b/drivers/i2c/busses/i2c-octeon-core.c
index 845eda70b8ca..1ba4ac24b02a 100644
--- a/drivers/i2c/busses/i2c-octeon-core.c
+++ b/drivers/i2c/busses/i2c-octeon-core.c
@@ -130,6 +130,25 @@ static void octeon_i2c_hlc_disable(struct octeon_i2c *i2c)
octeon_i2c_ctl_write(i2c, TWSI_CTL_ENAB);
}
+static void octeon_i2c_block_enable(struct octeon_i2c *i2c)
+{
+ if (i2c->block_enabled || !TWSI_BLOCK_CTL(i2c))
+ return;
+
+ i2c->block_enabled = true;
+ octeon_i2c_writeq_flush(TWSI_MODE_STRETCH
+ | TWSI_MODE_BLOCK_MODE, i2c->twsi_base + TWSI_MODE(i2c));
+}
+
+static void octeon_i2c_block_disable(struct octeon_i2c *i2c)
+{
+ if (!i2c->block_enabled || !TWSI_BLOCK_CTL(i2c))
+ return;
+
+ i2c->block_enabled = false;
+ octeon_i2c_writeq_flush(TWSI_MODE_STRETCH, i2c->twsi_base + TWSI_MODE(i2c));
+}
+
/**
* octeon_i2c_hlc_wait - wait for an HLC operation to complete
* @i2c: The struct octeon_i2c
@@ -268,6 +287,7 @@ static int octeon_i2c_start(struct octeon_i2c *i2c)
u8 stat;
octeon_i2c_hlc_disable(i2c);
+ octeon_i2c_block_disable(i2c);
octeon_i2c_ctl_write(i2c, TWSI_CTL_ENAB | TWSI_CTL_STA);
ret = octeon_i2c_wait(i2c);
@@ -485,40 +505,53 @@ static int octeon_i2c_hlc_write(struct octeon_i2c *i2c, struct i2c_msg *msgs)
return ret;
}
-/* 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)
+/* Process hlc transaction */
+static int octeon_i2c_hlc_cmd_send(struct octeon_i2c *i2c, u64 cmd)
{
- int i, j, ret = 0;
- u64 cmd;
-
- octeon_i2c_hlc_enable(i2c);
+ octeon_i2c_hlc_int_clear(i2c);
+ octeon_i2c_writeq_flush(cmd, i2c->twsi_base + SW_TWSI(i2c));
- cmd = SW_TWSI_V | SW_TWSI_R | SW_TWSI_SOVR;
- /* SIZE */
- cmd |= (u64)(msgs[1].len - 1) << SW_TWSI_SIZE_SHIFT;
- /* A */
- cmd |= (u64)(msgs[0].addr & 0x7full) << SW_TWSI_ADDR_SHIFT;
+ return octeon_i2c_hlc_wait(i2c);
+}
- if (msgs[0].flags & I2C_M_TEN)
+/* Construct and send i2c transaction core cmd */
+static int octeon_i2c_hlc_cmd(struct octeon_i2c *i2c, struct i2c_msg msg, u64 cmd)
+{
+ if (msg.flags & I2C_M_TEN)
cmd |= SW_TWSI_OP_10_IA;
else
cmd |= SW_TWSI_OP_7_IA;
- if (msgs[0].len == 2) {
+ if (msg.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;
+ ext = (u64)msg.buf[0] << SW_TWSI_IA_SHIFT;
+ cmd |= (u64)msg.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;
+ cmd |= (u64)msg.buf[0] << SW_TWSI_IA_SHIFT;
}
- octeon_i2c_hlc_int_clear(i2c);
- octeon_i2c_writeq_flush(cmd, i2c->twsi_base + SW_TWSI(i2c));
+ return octeon_i2c_hlc_cmd_send(i2c, cmd);
+}
- ret = octeon_i2c_hlc_wait(i2c);
+/* 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)
+{
+ int i, j, ret = 0;
+ u64 cmd;
+
+ octeon_i2c_hlc_enable(i2c);
+
+ cmd = SW_TWSI_V | SW_TWSI_R | SW_TWSI_SOVR;
+ /* SIZE */
+ cmd |= (u64)(msgs[1].len - 1) << SW_TWSI_SIZE_SHIFT;
+ /* A */
+ cmd |= (u64)(msgs[0].addr & 0x7full) << SW_TWSI_ADDR_SHIFT;
+
+ /* Send core command */
+ ret = octeon_i2c_hlc_cmd(i2c, msgs[0], cmd);
if (ret)
goto err;
@@ -579,10 +612,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;
@@ -594,6 +624,106 @@ static int octeon_i2c_hlc_comp_write(struct octeon_i2c *i2c, struct i2c_msg *msg
return ret;
}
+/**
+ * high-level-controller composite block write+read, msg0=addr, msg1=data
+ * Used in the case where the i2c xfer is for greater than 8 bytes of read data.
+ */
+static int octeon_i2c_hlc_block_comp_read(struct octeon_i2c *i2c, struct i2c_msg *msgs)
+{
+ int len, ret = 0;
+ u64 cmd = 0;
+
+ octeon_i2c_hlc_enable(i2c);
+ octeon_i2c_block_enable(i2c);
+
+ /* Write (size - 1) into block control register */
+ len = msgs[1].len - 1;
+ octeon_i2c_writeq_flush((u64)(len), i2c->twsi_base + TWSI_BLOCK_CTL(i2c));
+
+ /* Prepare core command */
+ cmd = SW_TWSI_V | SW_TWSI_R | SW_TWSI_SOVR;
+ cmd |= (u64)(msgs[0].addr & 0x7full) << SW_TWSI_ADDR_SHIFT;
+
+ /* Send core command */
+ ret = octeon_i2c_hlc_cmd(i2c, msgs[0], cmd);
+ if (ret)
+ return ret;
+
+ cmd = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
+ if ((cmd & SW_TWSI_R) == 0)
+ return octeon_i2c_check_status(i2c, false);
+
+ /* read data in FIFO */
+ octeon_i2c_writeq_flush(TWSI_BLOCK_STS_RESET_PTR, i2c->twsi_base + TWSI_BLOCK_STS(i2c));
+ for (int i = 0; i < len; i += 8) {
+ u64 rd = __raw_readq(i2c->twsi_base + TWSI_BLOCK_FIFO(i2c));
+ for (int j = 7; j >= 0; j--)
+ msgs[1].buf[i + (7 - j)] = (rd >> (8 * j)) & 0xff;
+ }
+
+ octeon_i2c_block_disable(i2c);
+ return ret;
+}
+
+/**
+ * high-level-controller composite block write+write, m[0]len<=2, m[1]len<=1024
+ * Used in the case where the i2c xfer is for greater than 8 bytes of write data.
+ */
+static int octeon_i2c_hlc_block_comp_write(struct octeon_i2c *i2c, struct i2c_msg *msgs)
+{
+ bool set_ext = false;
+ int len, ret = 0;
+ u64 cmd, ext = 0;
+
+ octeon_i2c_hlc_enable(i2c);
+ octeon_i2c_block_enable(i2c);
+
+ /* Write (size - 1) into block control register */
+ len = msgs[1].len - 1;
+ octeon_i2c_writeq_flush((u64)(len), i2c->twsi_base + TWSI_BLOCK_CTL(i2c));
+
+ /* Prepare core command */
+ cmd = SW_TWSI_V | SW_TWSI_SOVR;
+ 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;
+ }
+
+ /* Write msg into FIFO buffer */
+ octeon_i2c_writeq_flush(TWSI_BLOCK_STS_RESET_PTR, i2c->twsi_base + TWSI_BLOCK_STS(i2c));
+ for (int i = 0; i < len; i += 8) {
+ u64 buf = 0;
+ for (int j = 7; j >= 0; j--)
+ buf |= (msgs[1].buf[i + (7 - j)] << (8 * j));
+ octeon_i2c_writeq_flush(buf, i2c->twsi_base + TWSI_BLOCK_FIFO(i2c));
+ }
+ if (set_ext)
+ octeon_i2c_writeq_flush(ext, i2c->twsi_base + SW_TWSI_EXT(i2c));
+
+ /* Send command to core (send data in FIFO) */
+ ret = octeon_i2c_hlc_cmd_send(i2c, cmd);
+ if (ret)
+ return ret;
+
+ cmd = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
+ if ((cmd & SW_TWSI_R) == 0)
+ return octeon_i2c_check_status(i2c, false);
+
+ octeon_i2c_block_disable(i2c);
+ return ret;
+}
+
/**
* octeon_i2c_xfer - The driver's master_xfer function
* @adap: Pointer to the i2c_adapter structure
@@ -619,13 +749,21 @@ int octeon_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
if ((msgs[0].flags & I2C_M_RD) == 0 &&
(msgs[1].flags & I2C_M_RECV_LEN) == 0 &&
msgs[0].len > 0 && msgs[0].len <= 2 &&
- msgs[1].len > 0 && msgs[1].len <= 8 &&
+ msgs[1].len > 0 &&
msgs[0].addr == msgs[1].addr) {
- if (msgs[1].flags & I2C_M_RD)
- ret = octeon_i2c_hlc_comp_read(i2c, msgs);
- else
- ret = octeon_i2c_hlc_comp_write(i2c, msgs);
- goto out;
+ if (msgs[1].len <= 8) {
+ if (msgs[1].flags & I2C_M_RD)
+ ret = octeon_i2c_hlc_comp_read(i2c, msgs);
+ else
+ ret = octeon_i2c_hlc_comp_write(i2c, msgs);
+ goto out;
+ } else if (msgs[1].len <= 1024 && TWSI_BLOCK_CTL(i2c)) {
+ if (msgs[1].flags & I2C_M_RD)
+ ret = octeon_i2c_hlc_block_comp_read(i2c, msgs);
+ else
+ ret = octeon_i2c_hlc_block_comp_write(i2c, msgs);
+ goto out;
+ }
}
}
diff --git a/drivers/i2c/busses/i2c-octeon-core.h b/drivers/i2c/busses/i2c-octeon-core.h
index 9bb9f64fdda0..81fcf413c890 100644
--- a/drivers/i2c/busses/i2c-octeon-core.h
+++ b/drivers/i2c/busses/i2c-octeon-core.h
@@ -85,6 +85,11 @@
#define TWSI_INT_SDA BIT_ULL(10)
#define TWSI_INT_SCL BIT_ULL(11)
+#define TWSI_MODE_STRETCH BIT_ULL(1)
+#define TWSI_MODE_BLOCK_MODE BIT_ULL(2)
+
+#define TWSI_BLOCK_STS_RESET_PTR BIT_ULL(0)
+#define TWSI_BLOCK_STS_BUSY BIT_ULL(1)
#define I2C_OCTEON_EVENT_WAIT 80 /* microseconds */
/* Register offsets */
@@ -92,11 +97,19 @@ struct octeon_i2c_reg_offset {
unsigned int sw_twsi;
unsigned int twsi_int;
unsigned int sw_twsi_ext;
+ unsigned int twsi_mode;
+ unsigned int twsi_block_ctl;
+ unsigned int twsi_block_sts;
+ unsigned int twsi_block_fifo;
};
#define SW_TWSI(x) (x->roff.sw_twsi)
#define TWSI_INT(x) (x->roff.twsi_int)
#define SW_TWSI_EXT(x) (x->roff.sw_twsi_ext)
+#define TWSI_MODE(x) (x->roff.twsi_mode)
+#define TWSI_BLOCK_CTL(x) (x->roff.twsi_block_ctl)
+#define TWSI_BLOCK_STS(x) (x->roff.twsi_block_sts)
+#define TWSI_BLOCK_FIFO(x) (x->roff.twsi_block_fifo)
struct octeon_i2c {
wait_queue_head_t queue;
@@ -110,6 +123,7 @@ struct octeon_i2c {
void __iomem *twsi_base;
struct device *dev;
bool hlc_enabled;
+ bool block_enabled;
bool broken_irq_mode;
bool broken_irq_check;
void (*int_enable)(struct octeon_i2c *);
diff --git a/drivers/i2c/busses/i2c-thunderx-pcidrv.c b/drivers/i2c/busses/i2c-thunderx-pcidrv.c
index a77cd86fe75e..abde98117d7e 100644
--- a/drivers/i2c/busses/i2c-thunderx-pcidrv.c
+++ b/drivers/i2c/busses/i2c-thunderx-pcidrv.c
@@ -165,6 +165,10 @@ static int thunder_i2c_probe_pci(struct pci_dev *pdev,
i2c->roff.sw_twsi = 0x1000;
i2c->roff.twsi_int = 0x1010;
i2c->roff.sw_twsi_ext = 0x1018;
+ i2c->roff.twsi_mode = 0x1038;
+ i2c->roff.twsi_block_ctl = 0x1048;
+ i2c->roff.twsi_block_sts = 0x1050;
+ i2c->roff.twsi_block_fifo = 0x1058;
i2c->dev = dev;
pci_set_drvdata(pdev, i2c);
--
2.42.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] THUNDERX_I2C_BLOCK_MODE
2023-09-12 0:27 ` [PATCH] THUNDERX_I2C_BLOCK_MODE Aryan Srivastava
@ 2023-09-12 3:38 ` kernel test robot
0 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2023-09-12 3:38 UTC (permalink / raw
To: Aryan Srivastava, andi.shyti
Cc: oe-kbuild-all, aryan.srivastava, linux-i2c, linux-kernel
Hi Aryan,
kernel test robot noticed the following build warnings:
[auto build test WARNING on wsa/i2c/for-next]
[also build test WARNING on linus/master v6.6-rc1 next-20230911]
[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/THUNDERX_I2C_BLOCK_MODE/20230912-084721
base: https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
patch link: https://lore.kernel.org/r/20230912002706.2393450-1-aryan.srivastava%40alliedtelesis.co.nz
patch subject: [PATCH] THUNDERX_I2C_BLOCK_MODE
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20230912/202309121146.BqM4z7k7-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230912/202309121146.BqM4z7k7-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/202309121146.BqM4z7k7-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/i2c/busses/i2c-octeon-core.c:632: warning: Function parameter or member 'i2c' not described in 'octeon_i2c_hlc_block_comp_read'
>> drivers/i2c/busses/i2c-octeon-core.c:632: warning: Function parameter or member 'msgs' not described in 'octeon_i2c_hlc_block_comp_read'
>> drivers/i2c/busses/i2c-octeon-core.c:632: warning: expecting prototype for high(). Prototype was for octeon_i2c_hlc_block_comp_read() instead
>> drivers/i2c/busses/i2c-octeon-core.c:673: warning: Function parameter or member 'i2c' not described in 'octeon_i2c_hlc_block_comp_write'
>> drivers/i2c/busses/i2c-octeon-core.c:673: warning: Function parameter or member 'msgs' not described in 'octeon_i2c_hlc_block_comp_write'
>> drivers/i2c/busses/i2c-octeon-core.c:673: warning: expecting prototype for high(). Prototype was for octeon_i2c_hlc_block_comp_write() instead
vim +632 drivers/i2c/busses/i2c-octeon-core.c
626
627 /**
628 * high-level-controller composite block write+read, msg0=addr, msg1=data
629 * Used in the case where the i2c xfer is for greater than 8 bytes of read data.
630 */
631 static int octeon_i2c_hlc_block_comp_read(struct octeon_i2c *i2c, struct i2c_msg *msgs)
> 632 {
633 int len, ret = 0;
634 u64 cmd = 0;
635
636 octeon_i2c_hlc_enable(i2c);
637 octeon_i2c_block_enable(i2c);
638
639 /* Write (size - 1) into block control register */
640 len = msgs[1].len - 1;
641 octeon_i2c_writeq_flush((u64)(len), i2c->twsi_base + TWSI_BLOCK_CTL(i2c));
642
643 /* Prepare core command */
644 cmd = SW_TWSI_V | SW_TWSI_R | SW_TWSI_SOVR;
645 cmd |= (u64)(msgs[0].addr & 0x7full) << SW_TWSI_ADDR_SHIFT;
646
647 /* Send core command */
648 ret = octeon_i2c_hlc_cmd(i2c, msgs[0], cmd);
649 if (ret)
650 return ret;
651
652 cmd = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
653 if ((cmd & SW_TWSI_R) == 0)
654 return octeon_i2c_check_status(i2c, false);
655
656 /* read data in FIFO */
657 octeon_i2c_writeq_flush(TWSI_BLOCK_STS_RESET_PTR, i2c->twsi_base + TWSI_BLOCK_STS(i2c));
658 for (int i = 0; i < len; i += 8) {
659 u64 rd = __raw_readq(i2c->twsi_base + TWSI_BLOCK_FIFO(i2c));
660 for (int j = 7; j >= 0; j--)
661 msgs[1].buf[i + (7 - j)] = (rd >> (8 * j)) & 0xff;
662 }
663
664 octeon_i2c_block_disable(i2c);
665 return ret;
666 }
667
668 /**
669 * high-level-controller composite block write+write, m[0]len<=2, m[1]len<=1024
670 * Used in the case where the i2c xfer is for greater than 8 bytes of write data.
671 */
672 static int octeon_i2c_hlc_block_comp_write(struct octeon_i2c *i2c, struct i2c_msg *msgs)
> 673 {
674 bool set_ext = false;
675 int i, j, len, ret = 0;
676 u64 cmd, ext = 0;
677
678 octeon_i2c_hlc_enable(i2c);
679 octeon_i2c_block_enable(i2c);
680
681 /* Write (size - 1) into block control register */
682 len = msgs[1].len - 1;
683 octeon_i2c_writeq_flush((u64)(len), i2c->twsi_base + TWSI_BLOCK_CTL(i2c));
684
685 /* Prepare core command */
686 cmd = SW_TWSI_V | SW_TWSI_SOVR;
687 cmd |= (u64)(msgs[0].addr & 0x7full) << SW_TWSI_ADDR_SHIFT;
688
689 if (msgs[0].flags & I2C_M_TEN)
690 cmd |= SW_TWSI_OP_10_IA;
691 else
692 cmd |= SW_TWSI_OP_7_IA;
693
694 if (msgs[0].len == 2) {
695 cmd |= SW_TWSI_EIA;
696 ext |= (u64)msgs[0].buf[0] << SW_TWSI_IA_SHIFT;
697 set_ext = true;
698 cmd |= (u64)msgs[0].buf[1] << SW_TWSI_IA_SHIFT;
699 } else {
700 cmd |= (u64)msgs[0].buf[0] << SW_TWSI_IA_SHIFT;
701 }
702
703 /* Write msg into FIFO buffer */
704 octeon_i2c_writeq_flush(TWSI_BLOCK_STS_RESET_PTR, i2c->twsi_base + TWSI_BLOCK_STS(i2c));
705 for (i = 0; i < len; i += 8) {
706 u64 buf = 0;
707 for (j = 7; j >= 0; j--)
708 buf |= (msgs[1].buf[i + (7 - j)] << (8 * j));
709 octeon_i2c_writeq_flush(buf, i2c->twsi_base + TWSI_BLOCK_FIFO(i2c));
710 }
711 if (set_ext)
712 octeon_i2c_writeq_flush(ext, i2c->twsi_base + SW_TWSI_EXT(i2c));
713
714 /* Send command to core (send data in FIFO) */
715 ret = octeon_i2c_hlc_cmd_send(i2c, cmd);
716 if (ret)
717 return ret;
718
719 cmd = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
720 if ((cmd & SW_TWSI_R) == 0)
721 return octeon_i2c_check_status(i2c, false);
722
723 octeon_i2c_block_disable(i2c);
724 return ret;
725 }
726
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] i2c:octeon:Add block-mode r/w
[not found] ` <9882daa4945914886b21642837816c2d99c027ac.camel@alliedtelesis.co.nz>
@ 2023-10-25 21:20 ` Aryan Srivastava
0 siblings, 0 replies; 17+ messages in thread
From: Aryan Srivastava @ 2023-10-25 21:20 UTC (permalink / raw
To: Aryan Srivastava
Cc: linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org,
andi.shyti@kernel.org
Hi Andi,
Did you have any more comments on my patch?
Thank you,
Aryan.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] i2c: octeon: Add block-mode r/w
2023-09-12 1:16 ` [PATCH] i2c: octeon: Add " Aryan Srivastava
@ 2024-01-16 1:38 ` Aryan Srivastava
0 siblings, 0 replies; 17+ messages in thread
From: Aryan Srivastava @ 2024-01-16 1:38 UTC (permalink / raw
To: aryan.srivastava; +Cc: andi.shyti, linux-i2c, linux-kernel
Hi Andi,
Did you have any more comments on my patch?
Thank you,
Aryan.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v4] i2c: octeon: Add block-mode r/w
2023-09-05 10:22 ` Andi Shyti
` (2 preceding siblings ...)
[not found] ` <9882daa4945914886b21642837816c2d99c027ac.camel@alliedtelesis.co.nz>
@ 2024-04-15 0:52 ` Aryan Srivastava
2024-04-15 21:59 ` Andi Shyti
3 siblings, 1 reply; 17+ messages in thread
From: Aryan Srivastava @ 2024-04-15 0:52 UTC (permalink / raw
To: andi.shyti; +Cc: aryan.srivastava, linux-i2c, linux-kernel
Add support for block mode read/write operations on
Thunderx chips.
When attempting r/w operations of greater then 8 bytes
block mode is used, instead of performing a series of
8 byte reads.
Signed-off-by: Aryan Srivastava <aryan.srivastava@alliedtelesis.co.nz>
---
Changes in v2:
- comment style and formatting.
Changes in v3:
- comment style and formatting.
Changes in v4:
- Refactoring common code.
- Additional comments.
---
drivers/i2c/busses/i2c-octeon-core.c | 196 +++++++++++++++++++----
drivers/i2c/busses/i2c-octeon-core.h | 14 ++
drivers/i2c/busses/i2c-thunderx-pcidrv.c | 4 +
3 files changed, 185 insertions(+), 29 deletions(-)
diff --git a/drivers/i2c/busses/i2c-octeon-core.c b/drivers/i2c/busses/i2c-octeon-core.c
index 845eda70b8ca..1ba4ac24b02a 100644
--- a/drivers/i2c/busses/i2c-octeon-core.c
+++ b/drivers/i2c/busses/i2c-octeon-core.c
@@ -130,6 +130,25 @@ static void octeon_i2c_hlc_disable(struct octeon_i2c *i2c)
octeon_i2c_ctl_write(i2c, TWSI_CTL_ENAB);
}
+static void octeon_i2c_block_enable(struct octeon_i2c *i2c)
+{
+ if (i2c->block_enabled || !TWSI_BLOCK_CTL(i2c))
+ return;
+
+ i2c->block_enabled = true;
+ octeon_i2c_writeq_flush(TWSI_MODE_STRETCH
+ | TWSI_MODE_BLOCK_MODE, i2c->twsi_base + TWSI_MODE(i2c));
+}
+
+static void octeon_i2c_block_disable(struct octeon_i2c *i2c)
+{
+ if (!i2c->block_enabled || !TWSI_BLOCK_CTL(i2c))
+ return;
+
+ i2c->block_enabled = false;
+ octeon_i2c_writeq_flush(TWSI_MODE_STRETCH, i2c->twsi_base + TWSI_MODE(i2c));
+}
+
/**
* octeon_i2c_hlc_wait - wait for an HLC operation to complete
* @i2c: The struct octeon_i2c
@@ -268,6 +287,7 @@ static int octeon_i2c_start(struct octeon_i2c *i2c)
u8 stat;
octeon_i2c_hlc_disable(i2c);
+ octeon_i2c_block_disable(i2c);
octeon_i2c_ctl_write(i2c, TWSI_CTL_ENAB | TWSI_CTL_STA);
ret = octeon_i2c_wait(i2c);
@@ -485,40 +505,53 @@ static int octeon_i2c_hlc_write(struct octeon_i2c *i2c, struct i2c_msg *msgs)
return ret;
}
-/* 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)
+/* Process hlc transaction */
+static int octeon_i2c_hlc_cmd_send(struct octeon_i2c *i2c, u64 cmd)
{
- int i, j, ret = 0;
- u64 cmd;
-
- octeon_i2c_hlc_enable(i2c);
+ octeon_i2c_hlc_int_clear(i2c);
+ octeon_i2c_writeq_flush(cmd, i2c->twsi_base + SW_TWSI(i2c));
- cmd = SW_TWSI_V | SW_TWSI_R | SW_TWSI_SOVR;
- /* SIZE */
- cmd |= (u64)(msgs[1].len - 1) << SW_TWSI_SIZE_SHIFT;
- /* A */
- cmd |= (u64)(msgs[0].addr & 0x7full) << SW_TWSI_ADDR_SHIFT;
+ return octeon_i2c_hlc_wait(i2c);
+}
- if (msgs[0].flags & I2C_M_TEN)
+/* Construct and send i2c transaction core cmd */
+static int octeon_i2c_hlc_cmd(struct octeon_i2c *i2c, struct i2c_msg msg, u64 cmd)
+{
+ if (msg.flags & I2C_M_TEN)
cmd |= SW_TWSI_OP_10_IA;
else
cmd |= SW_TWSI_OP_7_IA;
- if (msgs[0].len == 2) {
+ if (msg.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;
+ ext = (u64)msg.buf[0] << SW_TWSI_IA_SHIFT;
+ cmd |= (u64)msg.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;
+ cmd |= (u64)msg.buf[0] << SW_TWSI_IA_SHIFT;
}
- octeon_i2c_hlc_int_clear(i2c);
- octeon_i2c_writeq_flush(cmd, i2c->twsi_base + SW_TWSI(i2c));
+ return octeon_i2c_hlc_cmd_send(i2c, cmd);
+}
- ret = octeon_i2c_hlc_wait(i2c);
+/* 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)
+{
+ int i, j, ret = 0;
+ u64 cmd;
+
+ octeon_i2c_hlc_enable(i2c);
+
+ cmd = SW_TWSI_V | SW_TWSI_R | SW_TWSI_SOVR;
+ /* SIZE */
+ cmd |= (u64)(msgs[1].len - 1) << SW_TWSI_SIZE_SHIFT;
+ /* A */
+ cmd |= (u64)(msgs[0].addr & 0x7full) << SW_TWSI_ADDR_SHIFT;
+
+ /* Send core command */
+ ret = octeon_i2c_hlc_cmd(i2c, msgs[0], cmd);
if (ret)
goto err;
@@ -579,10 +612,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;
@@ -594,6 +624,106 @@ static int octeon_i2c_hlc_comp_write(struct octeon_i2c *i2c, struct i2c_msg *msg
return ret;
}
+/**
+ * high-level-controller composite block write+read, msg0=addr, msg1=data
+ * Used in the case where the i2c xfer is for greater than 8 bytes of read data.
+ */
+static int octeon_i2c_hlc_block_comp_read(struct octeon_i2c *i2c, struct i2c_msg *msgs)
+{
+ int len, ret = 0;
+ u64 cmd = 0;
+
+ octeon_i2c_hlc_enable(i2c);
+ octeon_i2c_block_enable(i2c);
+
+ /* Write (size - 1) into block control register */
+ len = msgs[1].len - 1;
+ octeon_i2c_writeq_flush((u64)(len), i2c->twsi_base + TWSI_BLOCK_CTL(i2c));
+
+ /* Prepare core command */
+ cmd = SW_TWSI_V | SW_TWSI_R | SW_TWSI_SOVR;
+ cmd |= (u64)(msgs[0].addr & 0x7full) << SW_TWSI_ADDR_SHIFT;
+
+ /* Send core command */
+ ret = octeon_i2c_hlc_cmd(i2c, msgs[0], cmd);
+ if (ret)
+ return ret;
+
+ cmd = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
+ if ((cmd & SW_TWSI_R) == 0)
+ return octeon_i2c_check_status(i2c, false);
+
+ /* read data in FIFO */
+ octeon_i2c_writeq_flush(TWSI_BLOCK_STS_RESET_PTR, i2c->twsi_base + TWSI_BLOCK_STS(i2c));
+ for (int i = 0; i < len; i += 8) {
+ u64 rd = __raw_readq(i2c->twsi_base + TWSI_BLOCK_FIFO(i2c));
+ for (int j = 7; j >= 0; j--)
+ msgs[1].buf[i + (7 - j)] = (rd >> (8 * j)) & 0xff;
+ }
+
+ octeon_i2c_block_disable(i2c);
+ return ret;
+}
+
+/**
+ * high-level-controller composite block write+write, m[0]len<=2, m[1]len<=1024
+ * Used in the case where the i2c xfer is for greater than 8 bytes of write data.
+ */
+static int octeon_i2c_hlc_block_comp_write(struct octeon_i2c *i2c, struct i2c_msg *msgs)
+{
+ bool set_ext = false;
+ int len, ret = 0;
+ u64 cmd, ext = 0;
+
+ octeon_i2c_hlc_enable(i2c);
+ octeon_i2c_block_enable(i2c);
+
+ /* Write (size - 1) into block control register */
+ len = msgs[1].len - 1;
+ octeon_i2c_writeq_flush((u64)(len), i2c->twsi_base + TWSI_BLOCK_CTL(i2c));
+
+ /* Prepare core command */
+ cmd = SW_TWSI_V | SW_TWSI_SOVR;
+ 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;
+ }
+
+ /* Write msg into FIFO buffer */
+ octeon_i2c_writeq_flush(TWSI_BLOCK_STS_RESET_PTR, i2c->twsi_base + TWSI_BLOCK_STS(i2c));
+ for (int i = 0; i < len; i += 8) {
+ u64 buf = 0;
+ for (int j = 7; j >= 0; j--)
+ buf |= (msgs[1].buf[i + (7 - j)] << (8 * j));
+ octeon_i2c_writeq_flush(buf, i2c->twsi_base + TWSI_BLOCK_FIFO(i2c));
+ }
+ if (set_ext)
+ octeon_i2c_writeq_flush(ext, i2c->twsi_base + SW_TWSI_EXT(i2c));
+
+ /* Send command to core (send data in FIFO) */
+ ret = octeon_i2c_hlc_cmd_send(i2c, cmd);
+ if (ret)
+ return ret;
+
+ cmd = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
+ if ((cmd & SW_TWSI_R) == 0)
+ return octeon_i2c_check_status(i2c, false);
+
+ octeon_i2c_block_disable(i2c);
+ return ret;
+}
+
/**
* octeon_i2c_xfer - The driver's master_xfer function
* @adap: Pointer to the i2c_adapter structure
@@ -619,13 +749,21 @@ int octeon_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
if ((msgs[0].flags & I2C_M_RD) == 0 &&
(msgs[1].flags & I2C_M_RECV_LEN) == 0 &&
msgs[0].len > 0 && msgs[0].len <= 2 &&
- msgs[1].len > 0 && msgs[1].len <= 8 &&
+ msgs[1].len > 0 &&
msgs[0].addr == msgs[1].addr) {
- if (msgs[1].flags & I2C_M_RD)
- ret = octeon_i2c_hlc_comp_read(i2c, msgs);
- else
- ret = octeon_i2c_hlc_comp_write(i2c, msgs);
- goto out;
+ if (msgs[1].len <= 8) {
+ if (msgs[1].flags & I2C_M_RD)
+ ret = octeon_i2c_hlc_comp_read(i2c, msgs);
+ else
+ ret = octeon_i2c_hlc_comp_write(i2c, msgs);
+ goto out;
+ } else if (msgs[1].len <= 1024 && TWSI_BLOCK_CTL(i2c)) {
+ if (msgs[1].flags & I2C_M_RD)
+ ret = octeon_i2c_hlc_block_comp_read(i2c, msgs);
+ else
+ ret = octeon_i2c_hlc_block_comp_write(i2c, msgs);
+ goto out;
+ }
}
}
diff --git a/drivers/i2c/busses/i2c-octeon-core.h b/drivers/i2c/busses/i2c-octeon-core.h
index 9bb9f64fdda0..81fcf413c890 100644
--- a/drivers/i2c/busses/i2c-octeon-core.h
+++ b/drivers/i2c/busses/i2c-octeon-core.h
@@ -85,6 +85,11 @@
#define TWSI_INT_SDA BIT_ULL(10)
#define TWSI_INT_SCL BIT_ULL(11)
+#define TWSI_MODE_STRETCH BIT_ULL(1)
+#define TWSI_MODE_BLOCK_MODE BIT_ULL(2)
+
+#define TWSI_BLOCK_STS_RESET_PTR BIT_ULL(0)
+#define TWSI_BLOCK_STS_BUSY BIT_ULL(1)
#define I2C_OCTEON_EVENT_WAIT 80 /* microseconds */
/* Register offsets */
@@ -92,11 +97,19 @@ struct octeon_i2c_reg_offset {
unsigned int sw_twsi;
unsigned int twsi_int;
unsigned int sw_twsi_ext;
+ unsigned int twsi_mode;
+ unsigned int twsi_block_ctl;
+ unsigned int twsi_block_sts;
+ unsigned int twsi_block_fifo;
};
#define SW_TWSI(x) (x->roff.sw_twsi)
#define TWSI_INT(x) (x->roff.twsi_int)
#define SW_TWSI_EXT(x) (x->roff.sw_twsi_ext)
+#define TWSI_MODE(x) (x->roff.twsi_mode)
+#define TWSI_BLOCK_CTL(x) (x->roff.twsi_block_ctl)
+#define TWSI_BLOCK_STS(x) (x->roff.twsi_block_sts)
+#define TWSI_BLOCK_FIFO(x) (x->roff.twsi_block_fifo)
struct octeon_i2c {
wait_queue_head_t queue;
@@ -110,6 +123,7 @@ struct octeon_i2c {
void __iomem *twsi_base;
struct device *dev;
bool hlc_enabled;
+ bool block_enabled;
bool broken_irq_mode;
bool broken_irq_check;
void (*int_enable)(struct octeon_i2c *);
diff --git a/drivers/i2c/busses/i2c-thunderx-pcidrv.c b/drivers/i2c/busses/i2c-thunderx-pcidrv.c
index a77cd86fe75e..abde98117d7e 100644
--- a/drivers/i2c/busses/i2c-thunderx-pcidrv.c
+++ b/drivers/i2c/busses/i2c-thunderx-pcidrv.c
@@ -165,6 +165,10 @@ static int thunder_i2c_probe_pci(struct pci_dev *pdev,
i2c->roff.sw_twsi = 0x1000;
i2c->roff.twsi_int = 0x1010;
i2c->roff.sw_twsi_ext = 0x1018;
+ i2c->roff.twsi_mode = 0x1038;
+ i2c->roff.twsi_block_ctl = 0x1048;
+ i2c->roff.twsi_block_sts = 0x1050;
+ i2c->roff.twsi_block_fifo = 0x1058;
i2c->dev = dev;
pci_set_drvdata(pdev, i2c);
--
2.43.2
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v4] i2c: octeon: Add block-mode r/w
2024-04-15 0:52 ` [PATCH v4] i2c: octeon: Add " Aryan Srivastava
@ 2024-04-15 21:59 ` Andi Shyti
2024-06-10 2:59 ` Aryan Srivastava
0 siblings, 1 reply; 17+ messages in thread
From: Andi Shyti @ 2024-04-15 21:59 UTC (permalink / raw
To: Aryan Srivastava; +Cc: linux-i2c, linux-kernel
Hi Aryan,
On Mon, Apr 15, 2024 at 12:52:13PM +1200, Aryan Srivastava wrote:
> Add support for block mode read/write operations on
> Thunderx chips.
>
> When attempting r/w operations of greater then 8 bytes
> block mode is used, instead of performing a series of
> 8 byte reads.
Can you please add some more description of your patch here.
How did you do it? Which modes have you added? What are these
modes doing and how they work?
The patch is not the easiest itself and with little description
is very challenging to review. Please make my life easier :-)
> Signed-off-by: Aryan Srivastava <aryan.srivastava@alliedtelesis.co.nz>
...
> +static void octeon_i2c_block_enable(struct octeon_i2c *i2c)
> +{
> + if (i2c->block_enabled || !TWSI_BLOCK_CTL(i2c))
does the block_ctl register stores the length of the message?
If it goes '0' does it mean that it's ready for a block transfer?
(same question for the disable function).
> + return;
> +
> + i2c->block_enabled = true;
> + octeon_i2c_writeq_flush(TWSI_MODE_STRETCH
> + | TWSI_MODE_BLOCK_MODE, i2c->twsi_base + TWSI_MODE(i2c));
> +}
...
> @@ -579,10 +612,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;
Can you put the octeon_i2c_hlc_comp_read/octeon_i2c_hlc_comp_write
refactoring in a different patch as a preparatory to this one?
It's easier to review.
Please, remember to keep patches logically separated in smaller
chunks.
>
> @@ -594,6 +624,106 @@ static int octeon_i2c_hlc_comp_write(struct octeon_i2c *i2c, struct i2c_msg *msg
> return ret;
> }
>
> +/**
> + * high-level-controller composite block write+read, msg0=addr, msg1=data
This message doesn't mean much. Please check the DOC formatting
and the other functions, as well.
Remember good comments are highly appreciated.
> + * Used in the case where the i2c xfer is for greater than 8 bytes of read data.
> + */
...
> + /* read data in FIFO */
> + octeon_i2c_writeq_flush(TWSI_BLOCK_STS_RESET_PTR, i2c->twsi_base + TWSI_BLOCK_STS(i2c));
> + for (int i = 0; i < len; i += 8) {
> + u64 rd = __raw_readq(i2c->twsi_base + TWSI_BLOCK_FIFO(i2c));
> + for (int j = 7; j >= 0; j--)
> + msgs[1].buf[i + (7 - j)] = (rd >> (8 * j)) & 0xff;
Looks good, but do you mind a comment here?
> + }
> +
> + octeon_i2c_block_disable(i2c);
> + return ret;
> +}
...
> + /* Write msg into FIFO buffer */
> + octeon_i2c_writeq_flush(TWSI_BLOCK_STS_RESET_PTR, i2c->twsi_base + TWSI_BLOCK_STS(i2c));
> + for (int i = 0; i < len; i += 8) {
> + u64 buf = 0;
> + for (int j = 7; j >= 0; j--)
> + buf |= (msgs[1].buf[i + (7 - j)] << (8 * j));
a comment here?
> + octeon_i2c_writeq_flush(buf, i2c->twsi_base + TWSI_BLOCK_FIFO(i2c));
> + }
> + if (set_ext)
> + octeon_i2c_writeq_flush(ext, i2c->twsi_base + SW_TWSI_EXT(i2c));
> +
> + /* Send command to core (send data in FIFO) */
> + ret = octeon_i2c_hlc_cmd_send(i2c, cmd);
> + if (ret)
> + return ret;
do we need to disable anything here?
Thanks for your patch,
Andi
> +
> + cmd = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
> + if ((cmd & SW_TWSI_R) == 0)
> + return octeon_i2c_check_status(i2c, false);
> +
> + octeon_i2c_block_disable(i2c);
> + return ret;
> +}
...
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4] i2c: octeon: Add block-mode r/w
2024-04-15 21:59 ` Andi Shyti
@ 2024-06-10 2:59 ` Aryan Srivastava
0 siblings, 0 replies; 17+ messages in thread
From: Aryan Srivastava @ 2024-06-10 2:59 UTC (permalink / raw
To: andi.shyti@kernel.org
Cc: linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org
Hi Andi,
Thanks for your comments.
On Mon, 2024-04-15 at 23:59 +0200, Andi Shyti wrote:
> Hi Aryan,
>
> On Mon, Apr 15, 2024 at 12:52:13PM +1200, Aryan Srivastava wrote:
> > Add support for block mode read/write operations on
> > Thunderx chips.
> >
> > When attempting r/w operations of greater then 8 bytes
> > block mode is used, instead of performing a series of
> > 8 byte reads.
>
> Can you please add some more description of your patch here.
>
> How did you do it? Which modes have you added? What are these
> modes doing and how they work?
>
> The patch is not the easiest itself and with little description
> is very challenging to review. Please make my life easier :-)
>
Done.
> > Signed-off-by: Aryan Srivastava
> > <aryan.srivastava@alliedtelesis.co.nz>
>
> ...
>
> > +static void octeon_i2c_block_enable(struct octeon_i2c *i2c)
> > +{
> > + if (i2c->block_enabled || !TWSI_BLOCK_CTL(i2c))
>
> does the block_ctl register stores the length of the message?
> If it goes '0' does it mean that it's ready for a block transfer?
> (same question for the disable function).
This is simply to check if the HW is capable for block transactions.
>
> > + return;
> > +
> > + i2c->block_enabled = true;
> > + octeon_i2c_writeq_flush(TWSI_MODE_STRETCH
> > + | TWSI_MODE_BLOCK_MODE, i2c->twsi_base +
> > TWSI_MODE(i2c));
> > +}
>
> ...
>
> > @@ -579,10 +612,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;
>
> Can you put the octeon_i2c_hlc_comp_read/octeon_i2c_hlc_comp_write
> refactoring in a different patch as a preparatory to this one?
> It's easier to review.
>
> Please, remember to keep patches logically separated in smaller
> chunks.
>
Done.
> >
> > @@ -594,6 +624,106 @@ static int octeon_i2c_hlc_comp_write(struct
> > octeon_i2c *i2c, struct i2c_msg *msg
> > return ret;
> > }
> >
> > +/**
> > + * high-level-controller composite block write+read, msg0=addr,
> > msg1=data
>
> This message doesn't mean much. Please check the DOC formatting
> and the other functions, as well.
>
> Remember good comments are highly appreciated.
>
Done.
> > + * Used in the case where the i2c xfer is for greater than 8 bytes
> > of read data.
> > + */
>
> ...
>
> > + /* read data in FIFO */
> > + octeon_i2c_writeq_flush(TWSI_BLOCK_STS_RESET_PTR, i2c-
> > >twsi_base + TWSI_BLOCK_STS(i2c));
> > + for (int i = 0; i < len; i += 8) {
> > + u64 rd = __raw_readq(i2c->twsi_base +
> > TWSI_BLOCK_FIFO(i2c));
> > + for (int j = 7; j >= 0; j--)
> > + msgs[1].buf[i + (7 - j)] = (rd >> (8 * j))
> > & 0xff;
>
> Looks good, but do you mind a comment here?
Done, also added explanation in commit.
>
> > + }
> > +
> > + octeon_i2c_block_disable(i2c);
> > + return ret;
> > +}
>
> ...
>
> > + /* Write msg into FIFO buffer */
> > + octeon_i2c_writeq_flush(TWSI_BLOCK_STS_RESET_PTR, i2c-
> > >twsi_base + TWSI_BLOCK_STS(i2c));
> > + for (int i = 0; i < len; i += 8) {
> > + u64 buf = 0;
> > + for (int j = 7; j >= 0; j--)
> > + buf |= (msgs[1].buf[i + (7 - j)] << (8 *
> > j));
>
> a comment here?
>
Done.
> > + octeon_i2c_writeq_flush(buf, i2c->twsi_base +
> > TWSI_BLOCK_FIFO(i2c));
> > + }
> > + if (set_ext)
> > + octeon_i2c_writeq_flush(ext, i2c->twsi_base +
> > SW_TWSI_EXT(i2c));
> > +
> > + /* Send command to core (send data in FIFO) */
> > + ret = octeon_i2c_hlc_cmd_send(i2c, cmd);
> > + if (ret)
> > + return ret;
>
> do we need to disable anything here?
There is a disable at the bottom of the function for block mode.
>
> Thanks for your patch,
> Andi
>
> > +
> > + cmd = __raw_readq(i2c->twsi_base + SW_TWSI(i2c));
> > + if ((cmd & SW_TWSI_R) == 0)
> > + return octeon_i2c_check_status(i2c, false);
> > +
> > + octeon_i2c_block_disable(i2c);
> > + return ret;
> > +}
>
> ...
Thanks,
Aryan.
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2024-06-10 2:59 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-05 23:45 [PATCH] i2c:octeon:Add block-mode r/w Aryan Srivastava
2023-07-25 22:33 ` Andi Shyti
2023-08-16 23:07 ` Aryan Srivastava
2023-09-03 12:34 ` Andi Shyti
2023-09-04 23:14 ` Aryan Srivastava
2023-09-05 6:27 ` kernel test robot
2023-09-05 7:52 ` kernel test robot
2023-09-05 10:22 ` Andi Shyti
2023-09-12 0:27 ` [PATCH] THUNDERX_I2C_BLOCK_MODE Aryan Srivastava
2023-09-12 3:38 ` kernel test robot
2023-09-12 0:28 ` [PATCH] i2c:octeon:Add block-mode r/w Aryan Srivastava
2023-09-12 1:16 ` [PATCH] i2c: octeon: Add " Aryan Srivastava
2024-01-16 1:38 ` Aryan Srivastava
[not found] ` <9882daa4945914886b21642837816c2d99c027ac.camel@alliedtelesis.co.nz>
2023-10-25 21:20 ` [PATCH] i2c:octeon:Add " Aryan Srivastava
2024-04-15 0:52 ` [PATCH v4] i2c: octeon: Add " Aryan Srivastava
2024-04-15 21:59 ` Andi Shyti
2024-06-10 2:59 ` Aryan Srivastava
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).