RE: [PATCH V4 3/7] i2c: qup: Add V2 tags support
Hi Ivan,
-Original Message-
From: Ivan T. Ivanov [mailto:iiva...@mm-sol.com]
Sent: Monday, July 20, 2015 3:14 PM
To: Sricharan R
Cc: devicet...@vger.kernel.org; linux-arm-...@vger.kernel.org;
ga...@codeaurora.org; linux-ker...@vger.kernel.org; linux-
i...@vger.kernel.org; agr...@codeaurora.org; dmaeng...@vger.kernel.org;
linux-arm-ker...@lists.infradead.org
Subject: Re: [PATCH V4 3/7] i2c: qup: Add V2 tags support
Hi Sricharan,
On Thu, 2015-07-09 at 08:55 +0530, Sricharan R wrote:
QUP from version 2.1.1 onwards, supports a new format of i2c command
tags. Tag codes instructs the controller to perform a operation like
read/write. This new tagging version supports bam dma and transfers of
more than 256 bytes without 'stop'
in between. Adding the support for the same.
IIRC, more than 256 bytes in message is supported only in BAM(DMA) mode,
if this is true, please be more explicit in commit message.
More than 256 byte read transfers are possible in V2 mode and not possible
only
in V1 mode. So qup_i2c_quirks should be populated only for V1 mode and V2
mode
during probe.
You haven't tried to read more than 256 bytes with this patch, right? See
qup_i2c_quirks ;-)
Ya I have tested for 256 bytes transfer in my previous series, not on
this one. My bad. Should have error'ed out with qup_i2c_quirks in place.
Anyways will not populate qup_i2c_quirks for v2 mode.
struct qup_i2c_dev {
struct device*dev;
void __iomem*base;
@@ -117,6 +138,7 @@ struct qup_i2c_dev {
int in_blk_sz;
unsigned longone_byte_t;
+ struct qup_i2c_blockblk;
struct i2c_msg*msg;
/* Current posion in user message buffer */ @@ -126,6 +148,14
@@ struct qup_i2c_dev {
/* QUP core errors */
u32 qup_err;
+ int use_v2_tags;
+
+ int (*qup_i2c_write_one)(struct qup_i2c_dev *qup,
+ struct i2c_msg *msg);
+
+ int (*qup_i2c_read_one)(struct qup_i2c_dev *qup,
+ struct i2c_msg *msg);
+
Do we really need additional level of indirection?
We have separate struct i2c_algorithm, then we have common
qup_i2c_read/write methods and then we have different read/write sub
functions. I don't think 3-4 lines code reuse deserve increased complexity.
Infact I was thinking this way as well. But anyways wanted to get reviewed
and see which was better. Will change this.
snip
+static void qup_i2c_get_blk_data(struct qup_i2c_dev *qup,
+ struct i2c_msg *msg) {
This is more like set_blk_metadata. Second argument could fit line above.
Ok. Will change name and indentation.
+ memset(qup-blk, 0, sizeof(qup-blk));
+
+ if (!qup-use_v2_tags) {
+ if (!(msg-flags I2C_M_RD))
+ qup-blk.tx_tag_len = 1;
+ return;
+ }
+
+ qup-blk.data_len = msg-len;
+ qup-blk.count = (msg-len + QUP_READ_LIMIT - 1) /
+ QUP_READ_LIMIT;
+
+ /* 4 bytes for first block and 2 writes for rest */
+ qup-blk.tx_tag_len = 4 + (qup-blk.count - 1) * 2;
+
+ /* There are 2 tag bytes that are read in to fifo for every block */
+ if (msg-flags I2C_M_RD)
+ qup-blk.rx_tag_len = qup-blk.count * 2; }
+
snip
+static int qup_i2c_get_tags(u8 *tags, struct qup_i2c_dev *qup,
+ struct i2c_msg
+*msg) {
This is more like set_tags.
Ok, will change.
+ u16 addr = (msg-addr 1) | ((msg-flags I2C_M_RD) ==
I2C_M_RD);
+ int len = 0;
+ int data_len;
+
+ if (qup-blk.pos == 0) {
+ tags[len++] = QUP_TAG_V2_START;
+ tags[len++] = addr 0xff;
+
+ if (msg-flags I2C_M_TEN)
+ tags[len++] = addr 8;
+ }
+
+ /* Send _STOP commands for the last block */
+ if (qup-blk.pos == (qup-blk.count - 1)) {
+ if (msg-flags I2C_M_RD)
+ tags[len++] = QUP_TAG_V2_DATARD_STOP;
+ else
+ tags[len++] = QUP_TAG_V2_DATAWR_STOP;
+ } else {
+ if (msg-flags I2C_M_RD)
+ tags[len++] = QUP_TAG_V2_DATARD;
+ else
+ tags[len++] = QUP_TAG_V2_DATAWR;
+ }
+
+ data_len = qup_i2c_get_data_len(qup);
+
+ /* 0 implies 256 bytes */
+ if (data_len == QUP_READ_LIMIT)
+ tags[len++] = 0;
+ else
+ tags[len++] = data_len;
+
+ return len;
+}
+
Regards,
Sricharan
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org
Re: [PATCH V4 3/7] i2c: qup: Add V2 tags support
Hi Sricharan,
On Thu, 2015-07-09 at 08:55 +0530, Sricharan R wrote:
QUP from version 2.1.1 onwards, supports a new format of
i2c command tags. Tag codes instructs the controller to
perform a operation like read/write. This new tagging version
supports bam dma and transfers of more than 256 bytes without 'stop'
in between. Adding the support for the same.
IIRC, more than 256 bytes in message is supported only in BAM(DMA)
mode, if this is true, please be more explicit in commit message.
You haven't tried to read more than 256 bytes with this
patch, right? See qup_i2c_quirks ;-)
struct qup_i2c_dev {
struct device*dev;
void __iomem*base;
@@ -117,6 +138,7 @@ struct qup_i2c_dev {
int in_blk_sz;
unsigned longone_byte_t;
+ struct qup_i2c_blockblk;
struct i2c_msg*msg;
/* Current posion in user message buffer */
@@ -126,6 +148,14 @@ struct qup_i2c_dev {
/* QUP core errors */
u32 qup_err;
+ int use_v2_tags;
+
+ int (*qup_i2c_write_one)(struct qup_i2c_dev *qup,
+ struct i2c_msg *msg);
+
+ int (*qup_i2c_read_one)(struct qup_i2c_dev *qup,
+ struct i2c_msg *msg);
+
Do we really need additional level of indirection?
We have separate struct i2c_algorithm, then we have common
qup_i2c_read/write methods and then we have different
read/write sub functions. I don't think 3-4 lines code reuse
deserve increased complexity.
snip
+static void qup_i2c_get_blk_data(struct qup_i2c_dev *qup,
+ struct i2c_msg *msg)
+{
This is more like set_blk_metadata. Second argument could fit line above.
+ memset(qup-blk, 0, sizeof(qup-blk));
+
+ if (!qup-use_v2_tags) {
+ if (!(msg-flags I2C_M_RD))
+ qup-blk.tx_tag_len = 1;
+ return;
+ }
+
+ qup-blk.data_len = msg-len;
+ qup-blk.count = (msg-len + QUP_READ_LIMIT - 1) / QUP_READ_LIMIT;
+
+ /* 4 bytes for first block and 2 writes for rest */
+ qup-blk.tx_tag_len = 4 + (qup-blk.count - 1) * 2;
+
+ /* There are 2 tag bytes that are read in to fifo for every block */
+ if (msg-flags I2C_M_RD)
+ qup-blk.rx_tag_len = qup-blk.count * 2;
+}
+
snip
+static int qup_i2c_get_tags(u8 *tags, struct qup_i2c_dev *qup,
+ struct i2c_msg *msg)
+{
This is more like set_tags.
+ u16 addr = (msg-addr 1) | ((msg-flags I2C_M_RD) == I2C_M_RD);
+ int len = 0;
+ int data_len;
+
+ if (qup-blk.pos == 0) {
+ tags[len++] = QUP_TAG_V2_START;
+ tags[len++] = addr 0xff;
+
+ if (msg-flags I2C_M_TEN)
+ tags[len++] = addr 8;
+ }
+
+ /* Send _STOP commands for the last block */
+ if (qup-blk.pos == (qup-blk.count - 1)) {
+ if (msg-flags I2C_M_RD)
+ tags[len++] = QUP_TAG_V2_DATARD_STOP;
+ else
+ tags[len++] = QUP_TAG_V2_DATAWR_STOP;
+ } else {
+ if (msg-flags I2C_M_RD)
+ tags[len++] = QUP_TAG_V2_DATARD;
+ else
+ tags[len++] = QUP_TAG_V2_DATAWR;
+ }
+
+ data_len = qup_i2c_get_data_len(qup);
+
+ /* 0 implies 256 bytes */
+ if (data_len == QUP_READ_LIMIT)
+ tags[len++] = 0;
+ else
+ tags[len++] = data_len;
+
+ return len;
+}
+
Regards,
Ivan
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V4 3/7] i2c: qup: Add V2 tags support
QUP from version 2.1.1 onwards, supports a new format of
i2c command tags. Tag codes instructs the controller to
perform a operation like read/write. This new tagging version
supports bam dma and transfers of more than 256 bytes without 'stop'
in between. Adding the support for the same.
For each block a data_write/read tag and data_len tag is added to
the output fifo. For the final block of data write_stop/read_stop
tag is used.
Signed-off-by: Andy Gross agr...@codeaurora.org
Signed-off-by: Sricharan R sricha...@codeaurora.org
---
drivers/i2c/busses/i2c-qup.c | 330 ++-
1 file changed, 323 insertions(+), 7 deletions(-)
diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
index 131dc28..a4e20d9 100644
--- a/drivers/i2c/busses/i2c-qup.c
+++ b/drivers/i2c/busses/i2c-qup.c
@@ -43,6 +43,8 @@
#define QUP_I2C_CLK_CTL0x400
#define QUP_I2C_STATUS 0x404
+#define QUP_I2C_MASTER_GEN 0x408
+
/* QUP States and reset values */
#define QUP_RESET_STATE0
#define QUP_RUN_STATE 1
@@ -69,6 +71,8 @@
#define QUP_CLOCK_AUTO_GATEBIT(13)
#define I2C_MINI_CORE (2 8)
#define I2C_N_VAL 15
+#define I2C_N_VAL_V2 7
+
/* Most significant word offset in FIFO port */
#define QUP_MSW_SHIFT (I2C_N_VAL + 1)
@@ -79,6 +83,7 @@
#define QUP_PACK_ENBIT(15)
#define QUP_REPACK_EN (QUP_UNPACK_EN | QUP_PACK_EN)
+#define QUP_V2_TAGS_EN 1
#define QUP_OUTPUT_BLOCK_SIZE(x)(((x) 0) 0x03)
#define QUP_OUTPUT_FIFO_SIZE(x)(((x) 2) 0x07)
@@ -91,6 +96,13 @@
#define QUP_TAG_STOP (3 8)
#define QUP_TAG_REC(4 8)
+/* QUP v2 tags */
+#define QUP_TAG_V2_START 0x81
+#define QUP_TAG_V2_DATAWR 0x82
+#define QUP_TAG_V2_DATAWR_STOP 0x83
+#define QUP_TAG_V2_DATARD 0x85
+#define QUP_TAG_V2_DATARD_STOP 0x87
+
/* Status, Error flags */
#define I2C_STATUS_WR_BUFFER_FULL BIT(0)
#define I2C_STATUS_BUS_ACTIVE BIT(8)
@@ -102,6 +114,15 @@
#define RESET_BIT 0x0
#define ONE_BYTE 0x1
+struct qup_i2c_block {
+ int count;
+ int pos;
+ int tx_tag_len;
+ int rx_tag_len;
+ int data_len;
+ u8 tags[6];
+};
+
struct qup_i2c_dev {
struct device *dev;
void __iomem*base;
@@ -117,6 +138,7 @@ struct qup_i2c_dev {
int in_blk_sz;
unsigned long one_byte_t;
+ struct qup_i2c_blockblk;
struct i2c_msg *msg;
/* Current posion in user message buffer */
@@ -126,6 +148,14 @@ struct qup_i2c_dev {
/* QUP core errors */
u32 qup_err;
+ int use_v2_tags;
+
+ int (*qup_i2c_write_one)(struct qup_i2c_dev *qup,
+struct i2c_msg *msg);
+
+ int (*qup_i2c_read_one)(struct qup_i2c_dev *qup,
+ struct i2c_msg *msg);
+
struct completion xfer;
};
@@ -266,7 +296,7 @@ static int qup_i2c_wait_ready(struct qup_i2c_dev *qup, int
op, bool val,
static void qup_i2c_set_write_mode(struct qup_i2c_dev *qup, struct i2c_msg
*msg)
{
/* Number of entries to shift out, including the start */
- int total = msg-len + 1;
+ int total = msg-len + qup-blk.tx_tag_len;
if (total qup-out_fifo_sz) {
/* FIFO mode */
@@ -324,6 +354,134 @@ static int qup_i2c_issue_write(struct qup_i2c_dev *qup,
struct i2c_msg *msg)
return ret;
}
+static void qup_i2c_get_blk_data(struct qup_i2c_dev *qup,
+struct i2c_msg *msg)
+{
+ memset(qup-blk, 0, sizeof(qup-blk));
+
+ if (!qup-use_v2_tags) {
+ if (!(msg-flags I2C_M_RD))
+ qup-blk.tx_tag_len = 1;
+ return;
+ }
+
+ qup-blk.data_len = msg-len;
+ qup-blk.count = (msg-len + QUP_READ_LIMIT - 1) / QUP_READ_LIMIT;
+
+ /* 4 bytes for first block and 2 writes for rest */
+ qup-blk.tx_tag_len = 4 + (qup-blk.count - 1) * 2;
+
+ /* There are 2 tag bytes that are read in to fifo for every block */
+ if (msg-flags I2C_M_RD)
+ qup-blk.rx_tag_len = qup-blk.count * 2;
+}
+
+static int qup_i2c_send_data(struct qup_i2c_dev *qup, int tlen, u8 *tbuf,
+int dlen, u8 *dbuf)
+{
+ u32 val = 0, idx = 0, pos = 0, i = 0, t;
+ int len = tlen + dlen;
+ u8 *buf = tbuf;
+ int ret = 0;
+
+ while (len 0) {
+ ret = qup_i2c_wait_ready(qup, QUP_OUT_FULL,
+RESET_BIT, 4 * ONE_BYTE);
+ if (ret) {
+ dev_err(qup-dev, timeout for fifo out full);
+ return ret;
+ }
+
+
