[12/12] i2c: qup: reorganization of driver code to remove polling for qup v2

Following are the major issues in current driver code

1. The current driver simply assumes the transfer completion
   whenever its gets any non-error interrupts and then simply do the
   polling of available/free bytes in FIFO.
2. The block mode is not working properly since no handling in
   being done for OUT_BLOCK_WRITE_REQ and IN_BLOCK_READ_READ.
3. An i2c transfer can contain multiple message and QUP v2
   supports reconfiguration during run in which the mode should be same
   for all the sub transfer. Currently the mode is being programmed
   before every sub transfer which is functionally wrong. If one message
   is less than FIFO length and other message is greater than FIFO
   length, then transfers will fail.

Because of above, i2c v2 transfers of size greater than 64 are failing
with following error message

	i2c_qup 78b6000.i2c: timeout for fifo out full

To make block mode working properly and move to use the interrupts
instead of polling, major code reorganization is required. Following
are the major changes done in this patch

1. Remove the polling of TX FIFO free space and RX FIFO available
   bytes and move to interrupts completely. QUP has QUP_MX_OUTPUT_DONE,
   QUP_MX_INPUT_DONE, OUT_BLOCK_WRITE_REQ and IN_BLOCK_READ_REQ
   interrupts to handle FIFO’s properly so check all these interrupts.
2. Determine the mode for transfer before starting by checking
   all the tx/rx data length in each message. The complete message can be
   transferred either in DMA mode or Programmed IO by FIFO/Block mode.
   in DMA mode, both tx and rx uses same mode but in PIO mode, the TX and
   RX can be in different mode.
3. During write, For FIFO mode, TX FIFO can be directly written
   without checking for FIFO space. For block mode, the QUP will generate
   OUT_BLOCK_WRITE_REQ interrupt whenever it has block size of available
   space.
4. During read, both TX and RX FIFO will be used. TX will be used
   for writing tags and RX will be used for receiving the data. In QUP,
   TX and RX can operate in separate mode so configure modes accordingly.
5. For read FIFO mode, wait for QUP_MX_INPUT_DONE interrupt which
   will be generated after all the bytes have been copied in RX FIFO. For
   read Block mode, QUP will generate IN_BLOCK_READ_REQ interrupts
   whenever it has block size of available data.
6. Split the transfer in chunk of one QUP block size(256 bytes)
   and schedule each block separately. QUP v2 supports reconfiguration
   during run in which QUP can transfer multiple blocks without issuing a
   stop events.
7. Port the SMBus block read support for new code changes.

Signed-off-by: Abhishek Sahu <absahu@codeaurora.org>
---
 drivers/i2c/busses/i2c-qup.c | 917 +++++++++++++++++++++++++------------------
 1 file changed, 533 insertions(+), 384 deletions(-)

Hi Abhishek,

On 2/3/2018 1:28 PM, Abhishek Sahu wrote:
> Following are the major issues in current driver code
> 
> 1. The current driver simply assumes the transfer completion
>    whenever its gets any non-error interrupts and then simply do the
>    polling of available/free bytes in FIFO.
> 2. The block mode is not working properly since no handling in
>    being done for OUT_BLOCK_WRITE_REQ and IN_BLOCK_READ_READ.
> 3. An i2c transfer can contain multiple message and QUP v2
>    supports reconfiguration during run in which the mode should be same
>    for all the sub transfer. Currently the mode is being programmed
>    before every sub transfer which is functionally wrong. If one message
>    is less than FIFO length and other message is greater than FIFO length,
     then transfers will fail.
> 
> Because of above, i2c v2 transfers of size greater than 64 are failing
> with following error message
> 
> 	i2c_qup 78b6000.i2c: timeout for fifo out full
> 
> To make block mode working properly and move to use the interrupts
> instead of polling, major code reorganization is required. Following
> are the major changes done in this patch
> 
> 1. Remove the polling of TX FIFO free space and RX FIFO available
>    bytes and move to interrupts completely. QUP has QUP_MX_OUTPUT_DONE,
>    QUP_MX_INPUT_DONE, OUT_BLOCK_WRITE_REQ and IN_BLOCK_READ_REQ
>    interrupts to handle FIFO’s properly so check all these interrupts.
> 2. Determine the mode for transfer before starting by checking
>    all the tx/rx data length in each message. The complete message can be
>    transferred either in DMA mode or Programmed IO by FIFO/Block mode.
>    in DMA mode, both tx and rx uses same mode but in PIO mode, the TX and
>    RX can be in different mode.
> 3. During write, For FIFO mode, TX FIFO can be directly written
>    without checking for FIFO space. For block mode, the QUP will generate
>    OUT_BLOCK_WRITE_REQ interrupt whenever it has block size of available
>    space.
> 4. During read, both TX and RX FIFO will be used. TX will be used
>    for writing tags and RX will be used for receiving the data. In QUP,
>    TX and RX can operate in separate mode so configure modes accordingly.
> 5. For read FIFO mode, wait for QUP_MX_INPUT_DONE interrupt which
>    will be generated after all the bytes have been copied in RX FIFO. For
>    read Block mode, QUP will generate IN_BLOCK_READ_REQ interrupts
>    whenever it has block size of available data.
> 6. Split the transfer in chunk of one QUP block size(256 bytes)
>    and schedule each block separately. QUP v2 supports reconfiguration
>    during run in which QUP can transfer multiple blocks without issuing a
>    stop events.
> 7. Port the SMBus block read support for new code changes.
> 
> Signed-off-by: Abhishek Sahu <absahu@codeaurora.org>
> ---
>  drivers/i2c/busses/i2c-qup.c | 917 +++++++++++++++++++++++++------------------
>  1 file changed, 533 insertions(+), 384 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
> index af6c21a..46736a1 100644
> --- a/drivers/i2c/busses/i2c-qup.c
> +++ b/drivers/i2c/busses/i2c-qup.c
> @@ -141,6 +141,15 @@
>  #define DEFAULT_CLK_FREQ 100000
>  #define DEFAULT_SRC_CLK 20000000
>  
> +/*
> + * Max tags length (start, stop and maximum 2 bytes address) for each QUP
> + * data transfer
> + */
> +#define QUP_MAX_TAGS_LEN		4
> +/* Max data length for each DATARD tags */
> +#define RECV_MAX_DATA_LEN		254
> +/* TAG length for DATA READ in RX FIFO  */
> +#define READ_RX_TAGS_LEN		2
>  /* MAX_OUTPUT_DONE_FLAG has been received */
>  #define QUP_BLK_EVENT_TX_IRQ_DONE		BIT(0)
>  /* MAX_INPUT_DONE_FLAG has been received */
> @@ -164,11 +173,24 @@
>   * tx_tag_len: tx tag length for current block
>   * rx_tag_len: rx tag length for current block
>   * data_len: remaining data length for current message
> + * cur_blk_len: data length for current block
>   * total_tx_len: total tx length including tag bytes for current QUP transfer
>   * total_rx_len: total rx length including tag bytes for current QUP transfer
> + * tx_fifo_data_pos: current byte number in TX FIFO word
>   * tx_fifo_free: number of free bytes in current QUP block write.
> + * rx_fifo_data_pos: current byte number in RX FIFO word
>   * fifo_available: number of available bytes in RX FIFO for current
>   *		   QUP block read
> + * tx_fifo_data: QUP TX FIFO write works on word basis (4 bytes). New byte write
> + *		 to TX FIFO will be appended in this data and will be written to
> + *		 TX FIFO when all the 4 bytes are available.
> + * rx_fifo_data: QUP RX FIFO read works on word basis (4 bytes). This will
> + *		 contains the 4 bytes of RX data.
> + * cur_data: pointer to tell cur data position for current message
> + * cur_tx_tags: pointer to tell cur position in tags
> + * tx_tags_sent: all tx tag bytes have been written in FIFO word
> + * send_last_word: for tx FIFO, last word send is pending in current block
> + * rx_tags_fetched: all the rx tag bytes have been fetched from rx fifo word
>   * is_tx_blk_mode: whether tx uses block or FIFO mode in case of non BAM xfer.
>   * is_rx_blk_mode: whether rx uses block or FIFO mode in case of non BAM xfer.
>   * tags: contains tx tag bytes for current QUP transfer
> @@ -179,10 +201,20 @@ struct qup_i2c_block {
>  	int		tx_tag_len;
>  	int		rx_tag_len;
>  	int		data_len;
> +	int		cur_blk_len;
>  	int		total_tx_len;
>  	int		total_rx_len;
> +	int		tx_fifo_data_pos;
>  	int		tx_fifo_free;
> +	int		rx_fifo_data_pos;
>  	int		fifo_available;
> +	u32		tx_fifo_data;
> +	u32		rx_fifo_data;
> +	u8		*cur_data;
> +	u8		*cur_tx_tags;
> +	bool		tx_tags_sent;
> +	bool		send_last_word;
> +	bool		rx_tags_fetched;
>  	bool		is_tx_blk_mode;
>  	bool		is_rx_blk_mode;
>  	u8		tags[6];
> @@ -214,6 +246,7 @@ struct qup_i2c_dev {
>  	int			out_blk_sz;
>  	int			in_blk_sz;
>  
> +	int			blk_xfer_limit;
>  	unsigned long		one_byte_t;
>  	unsigned long		xfer_timeout;
>  	struct qup_i2c_block	blk;
> @@ -228,10 +261,10 @@ struct qup_i2c_dev {
>  
>  	/* To check if this is the last msg */
>  	bool			is_last;
> -	bool			is_qup_v1;
> +	bool			is_smbus_read;
>  
>  	/* To configure when bus is in run state */
> -	int			config_run;
> +	u32			config_run;
>  
>  	/* dma parameters */
>  	bool			is_dma;
> @@ -245,6 +278,8 @@ struct qup_i2c_dev {
>  	unsigned int		dma_threshold;
>  	unsigned int		max_xfer_sg_len;
>  	unsigned int		tag_buf_pos;
> +	/* The threshold length above which block mode will be used */
> +	unsigned int		blk_mode_threshold;
>  	struct			dma_pool *dpool;
>  	struct			qup_i2c_tag start_tag;
>  	struct			qup_i2c_bam brx;
> @@ -309,9 +344,6 @@ static irqreturn_t qup_i2c_interrupt(int irq, void *dev)
>  		goto done;
>  	}
>  
> -	if (!qup->is_qup_v1)
> -		goto done;
> -
>  	if (opflags & QUP_OUT_SVC_FLAG) {
>  		writel(QUP_OUT_SVC_FLAG, qup->base + QUP_OPERATIONAL);
>  
> @@ -444,108 +476,6 @@ static int qup_i2c_bus_active(struct qup_i2c_dev *qup, int len)
>  	return ret;
>  }
>  
> -/**
> - * qup_i2c_wait_ready - wait for a give number of bytes in tx/rx path
> - * @qup: The qup_i2c_dev device
> - * @op: The bit/event to wait on
> - * @val: value of the bit to wait on, 0 or 1
> - * @len: The length the bytes to be transferred
> - */
> -static int qup_i2c_wait_ready(struct qup_i2c_dev *qup, int op, bool val,
> -			      int len)
> -{
> -	unsigned long timeout;
> -	u32 opflags;
> -	u32 status;
> -	u32 shift = __ffs(op);
> -	int ret = 0;
> -
> -	len *= qup->one_byte_t;
> -	/* timeout after a wait of twice the max time */
> -	timeout = jiffies + len * 4;
> -
> -	for (;;) {
> -		opflags = readl(qup->base + QUP_OPERATIONAL);
> -		status = readl(qup->base + QUP_I2C_STATUS);
> -
> -		if (((opflags & op) >> shift) == val) {
> -			if ((op == QUP_OUT_NOT_EMPTY) && qup->is_last) {
> -				if (!(status & I2C_STATUS_BUS_ACTIVE)) {
> -					ret = 0;
> -					goto done;
> -				}
> -			} else {
> -				ret = 0;
> -				goto done;
> -			}
> -		}
> -
> -		if (time_after(jiffies, timeout)) {
> -			ret = -ETIMEDOUT;
> -			goto done;
> -		}
> -		usleep_range(len, len * 2);
> -	}
> -
> -done:
> -	if (qup->bus_err || qup->qup_err)
> -		ret =  (qup->bus_err & QUP_I2C_NACK_FLAG) ? -ENXIO : -EIO;
> -
> -	return ret;
> -}
> -
> -static void qup_i2c_set_write_mode_v2(struct qup_i2c_dev *qup,
> -				      struct i2c_msg *msg)
> -{
> -	/* Number of entries to shift out, including the tags */
> -	int total = msg->len + qup->blk.tx_tag_len;
> -
> -	total |= qup->config_run;
> -
> -	if (total < qup->out_fifo_sz) {
> -		/* FIFO mode */
> -		writel(QUP_REPACK_EN, qup->base + QUP_IO_MODE);
> -		writel(total, qup->base + QUP_MX_WRITE_CNT);
> -	} else {
> -		/* BLOCK mode (transfer data on chunks) */
> -		writel(QUP_OUTPUT_BLK_MODE | QUP_REPACK_EN,
> -		       qup->base + QUP_IO_MODE);
> -		writel(total, qup->base + QUP_MX_OUTPUT_CNT);
> -	}
> -}
> -
> -static int check_for_fifo_space(struct qup_i2c_dev *qup)
> -{
> -	int ret;
> -
> -	ret = qup_i2c_change_state(qup, QUP_PAUSE_STATE);
> -	if (ret)
> -		goto out;
> -
> -	ret = qup_i2c_wait_ready(qup, QUP_OUT_FULL,
> -				 RESET_BIT, 4 * ONE_BYTE);
> -	if (ret) {
> -		/* Fifo is full. Drain out the fifo */
> -		ret = qup_i2c_change_state(qup, QUP_RUN_STATE);
> -		if (ret)
> -			goto out;
> -
> -		ret = qup_i2c_wait_ready(qup, QUP_OUT_NOT_EMPTY,
> -					 RESET_BIT, 256 * ONE_BYTE);
> -		if (ret) {
> -			dev_err(qup->dev, "timeout for fifo out full");
> -			goto out;
> -		}
> -
> -		ret = qup_i2c_change_state(qup, QUP_PAUSE_STATE);
> -		if (ret)
> -			goto out;
> -	}
> -
> -out:
> -	return ret;
> -}
> -
>  static void qup_i2c_write_tx_fifo_v1(struct qup_i2c_dev *qup)
>  {
>  	struct qup_i2c_block *blk = &qup->blk;
> @@ -591,60 +521,17 @@ static void qup_i2c_write_tx_fifo_v1(struct qup_i2c_dev *qup)
>  static void qup_i2c_set_blk_data(struct qup_i2c_dev *qup,
>  				 struct i2c_msg *msg)
>  {
> -	memset(&qup->blk, 0, sizeof(qup->blk));
> -
> +	qup->blk.pos = 0;
>  	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 = check_for_fifo_space(qup);
> -		if (ret)
> -			return ret;
> -
> -		t = (len >= 4) ? 4 : len;
> -
> -		while (idx < t) {
> -			if (!i && (pos >= tlen)) {
> -				buf = dbuf;
> -				pos = 0;
> -				i = 1;
> -			}
> -			val |= buf[pos++] << (idx++ * 8);
> -		}
> -
> -		writel(val, qup->base + QUP_OUT_FIFO_BASE);
> -		idx  = 0;
> -		val = 0;
> -		len -= 4;
> -	}
> -
> -	ret = qup_i2c_change_state(qup, QUP_RUN_STATE);
> -
> -	return ret;
> +	qup->blk.count = DIV_ROUND_UP(msg->len, qup->blk_xfer_limit);
>  }
>  
>  static int qup_i2c_get_data_len(struct qup_i2c_dev *qup)
>  {
>  	int data_len;
>  
> -	if (qup->blk.data_len > QUP_READ_LIMIT)
> -		data_len = QUP_READ_LIMIT;
> +	if (qup->blk.data_len > qup->blk_xfer_limit)
> +		data_len = qup->blk_xfer_limit;
>  	else
>  		data_len = qup->blk.data_len;
>  
> @@ -661,9 +548,9 @@ static int qup_i2c_set_tags_smb(u16 addr, u8 *tags, struct qup_i2c_dev *qup,
>  {
>  	int len = 0;
>  
> -	if (msg->len > 1) {
> +	if (qup->is_smbus_read) {
>  		tags[len++] = QUP_TAG_V2_DATARD_STOP;
> -		tags[len++] = qup_i2c_get_data_len(qup) - 1;
> +		tags[len++] = qup_i2c_get_data_len(qup);
>  	} else {
>  		tags[len++] = QUP_TAG_V2_START;
>  		tags[len++] = addr & 0xff;
> @@ -725,24 +612,6 @@ static int qup_i2c_set_tags(u8 *tags, struct qup_i2c_dev *qup,
>  	return len;
>  }
>  
> -static int qup_i2c_issue_xfer_v2(struct qup_i2c_dev *qup, struct i2c_msg *msg)
> -{
> -	int data_len = 0, tag_len, index;
> -	int ret;
> -
> -	tag_len = qup_i2c_set_tags(qup->blk.tags, qup, msg);
> -	index = msg->len - qup->blk.data_len;
> -
> -	/* only tags are written for read */
> -	if (!(msg->flags & I2C_M_RD))
> -		data_len = qup_i2c_get_data_len(qup);
> -
> -	ret = qup_i2c_send_data(qup, tag_len, qup->blk.tags,
> -				data_len, &msg->buf[index]);
> -	qup->blk.data_len -= data_len;
> -
> -	return ret;
> -}
>  
>  static void qup_i2c_bam_cb(void *data)
>  {
> @@ -809,6 +678,7 @@ static int qup_i2c_bam_make_desc(struct qup_i2c_dev *qup, struct i2c_msg *msg)
>  	u32 i = 0, tlen, tx_len = 0;
>  	u8 *tags;
>  
> +	qup->blk_xfer_limit = QUP_READ_LIMIT;
>  	qup_i2c_set_blk_data(qup, msg);
>  
>  	blocks = qup->blk.count;
> @@ -1057,7 +927,7 @@ static int qup_i2c_wait_for_complete(struct qup_i2c_dev *qup,
>  	unsigned long left;
>  	int ret = 0;
>  
> -	left = wait_for_completion_timeout(&qup->xfer, HZ);
> +	left = wait_for_completion_timeout(&qup->xfer, qup->xfer_timeout);
>  	if (!left) {
>  		writel(1, qup->base + QUP_SW_RESET);
>  		ret = -ETIMEDOUT;
> @@ -1069,65 +939,6 @@ static int qup_i2c_wait_for_complete(struct qup_i2c_dev *qup,
>  	return ret;
>  }
>  
> -static int qup_i2c_write_one_v2(struct qup_i2c_dev *qup, struct i2c_msg *msg)
> -{
> -	int ret = 0;
> -
> -	qup->msg = msg;
> -	qup->pos = 0;
> -	enable_irq(qup->irq);
> -	qup_i2c_set_blk_data(qup, msg);
> -	qup_i2c_set_write_mode_v2(qup, msg);
> -
> -	ret = qup_i2c_change_state(qup, QUP_RUN_STATE);
> -	if (ret)
> -		goto err;
> -
> -	writel(qup->clk_ctl, qup->base + QUP_I2C_CLK_CTL);
> -
> -	do {
> -		ret = qup_i2c_issue_xfer_v2(qup, msg);
> -		if (ret)
> -			goto err;
> -
> -		ret = qup_i2c_wait_for_complete(qup, msg);
> -		if (ret)
> -			goto err;
> -
> -		qup->blk.pos++;
> -	} while (qup->blk.pos < qup->blk.count);
> -
> -	ret = qup_i2c_wait_ready(qup, QUP_OUT_NOT_EMPTY, RESET_BIT, ONE_BYTE);
> -
> -err:
> -	disable_irq(qup->irq);
> -	qup->msg = NULL;
> -
> -	return ret;
> -}
> -
> -static void qup_i2c_set_read_mode_v2(struct qup_i2c_dev *qup, int len)
> -{
> -	int tx_len = qup->blk.tx_tag_len;
> -
> -	len += qup->blk.rx_tag_len;
> -	len |= qup->config_run;
> -	tx_len |= qup->config_run;
> -
> -	if (len < qup->in_fifo_sz) {
> -		/* FIFO mode */
> -		writel(QUP_REPACK_EN, qup->base + QUP_IO_MODE);
> -		writel(tx_len, qup->base + QUP_MX_WRITE_CNT);
> -		writel(len, qup->base + QUP_MX_READ_CNT);
> -	} else {
> -		/* BLOCK mode (transfer data on chunks) */
> -		writel(QUP_INPUT_BLK_MODE | QUP_REPACK_EN,
> -		       qup->base + QUP_IO_MODE);
> -		writel(tx_len, qup->base + QUP_MX_OUTPUT_CNT);
> -		writel(len, qup->base + QUP_MX_INPUT_CNT);
> -	}
> -}
> -
>  static void qup_i2c_read_rx_fifo_v1(struct qup_i2c_dev *qup)
>  {
>  	struct qup_i2c_block *blk = &qup->blk;
> @@ -1151,104 +962,6 @@ static void qup_i2c_read_rx_fifo_v1(struct qup_i2c_dev *qup)
>  		qup->cur_blk_events |= QUP_BLK_EVENT_RX_DATA_DONE;
>  }
>  
> -static int qup_i2c_read_fifo_v2(struct qup_i2c_dev *qup,
> -				struct i2c_msg *msg)
> -{
> -	u32 val;
> -	int idx, pos = 0, ret = 0, total, msg_offset = 0;
> -
> -	/*
> -	 * If the message length is already read in
> -	 * the first byte of the buffer, account for
> -	 * that by setting the offset
> -	 */
> -	if (qup_i2c_check_msg_len(msg) && (msg->len > 1))
> -		msg_offset = 1;
> -	total = qup_i2c_get_data_len(qup);
> -	total -= msg_offset;
> -
> -	/* 2 extra bytes for read tags */
> -	while (pos < (total + 2)) {
> -		/* Check that FIFO have data */
> -		ret = qup_i2c_wait_ready(qup, QUP_IN_NOT_EMPTY,
> -					 SET_BIT, 4 * ONE_BYTE);
> -		if (ret) {
> -			dev_err(qup->dev, "timeout for fifo not empty");
> -			return ret;
> -		}
> -		val = readl(qup->base + QUP_IN_FIFO_BASE);
> -
> -		for (idx = 0; idx < 4; idx++, val >>= 8, pos++) {
> -			/* first 2 bytes are tag bytes */
> -			if (pos < 2)
> -				continue;
> -
> -			if (pos >= (total + 2))
> -				goto out;
> -			msg->buf[qup->pos + msg_offset] = val & 0xff;
> -			qup->pos++;
> -		}
> -	}
> -
> -out:
> -	qup->blk.data_len -= total;
> -
> -	return ret;
> -}
> -
> -static int qup_i2c_read_one_v2(struct qup_i2c_dev *qup, struct i2c_msg *msg)
> -{
> -	int ret = 0;
> -
> -	qup->msg = msg;
> -	qup->pos  = 0;
> -	enable_irq(qup->irq);
> -	qup_i2c_set_blk_data(qup, msg);
> -	qup_i2c_set_read_mode_v2(qup, msg->len);
> -
> -	ret = qup_i2c_change_state(qup, QUP_RUN_STATE);
> -	if (ret)
> -		goto err;
> -
> -	writel(qup->clk_ctl, qup->base + QUP_I2C_CLK_CTL);
> -
> -	do {
> -		ret = qup_i2c_issue_xfer_v2(qup, msg);
> -		if (ret)
> -			goto err;
> -
> -		ret = qup_i2c_wait_for_complete(qup, msg);
> -		if (ret)
> -			goto err;
> -
> -		ret = qup_i2c_read_fifo_v2(qup, msg);
> -		if (ret)
> -			goto err;
> -
> -		qup->blk.pos++;
> -
> -		/* Handle SMBus block read length */
> -		if (qup_i2c_check_msg_len(msg) && (msg->len == 1)) {
> -			if (msg->buf[0] > I2C_SMBUS_BLOCK_MAX) {
> -				ret = -EPROTO;
> -				goto err;
> -			}
> -			msg->len += msg->buf[0];
> -			qup->pos = 0;
> -			qup_i2c_set_blk_data(qup, msg);
> -			/* set tag length for block read */
> -			qup->blk.tx_tag_len = 2;
> -			qup_i2c_set_read_mode_v2(qup, msg->buf[0]);
> -		}
> -	} while (qup->blk.pos < qup->blk.count);
> -
> -err:
> -	disable_irq(qup->irq);
> -	qup->msg = NULL;
> -
> -	return ret;
> -}
> -
>  static void qup_i2c_set_blk_event(struct qup_i2c_dev *qup, bool is_rx)
>  {
>  	qup->cur_blk_events = 0;
> @@ -1442,13 +1155,452 @@ static int qup_i2c_xfer(struct i2c_adapter *adap,
>  	return ret;
>  }
>  
 
  Since this is used for both FIFO and BLK mode, might be good to call it
  qup_i2c_set_event. Same in rest of the places and macros as well.
  But if this is going to be removed altogether, then great.

> +/*
> + * Function to configure registers related with reconfiguration during run
> + * and will be done before each I2C sub transfer.
> + */
> +static void qup_i2c_conf_count_v2(struct qup_i2c_dev *qup)
> +{
> +	struct qup_i2c_block *blk = &qup->blk;
> +	u32 qup_config = I2C_MINI_CORE | I2C_N_VAL_V2;
> +
> +	if (blk->is_tx_blk_mode)
> +		writel(qup->config_run | blk->total_tx_len,
> +		       qup->base + QUP_MX_OUTPUT_CNT);
> +	else
> +		writel(qup->config_run | blk->total_tx_len,
> +		       qup->base + QUP_MX_WRITE_CNT);
> +
> +	if (blk->total_rx_len) {
> +		if (blk->is_rx_blk_mode)
> +			writel(qup->config_run | blk->total_rx_len,
> +			       qup->base + QUP_MX_INPUT_CNT);
> +		else
> +			writel(qup->config_run | blk->total_rx_len,
> +			       qup->base + QUP_MX_READ_CNT);
> +	} else {
> +		qup_config |= QUP_NO_INPUT;
> +	}
> +
> +	writel(qup_config, qup->base + QUP_CONFIG);
> +}
> +
> +/*
> + * Function to configure registers related with transfer mode (FIFO/Block)
> + * before starting of i2c transfer and will be done only once in QUP RESET
> + * state.
> + */
> +static void qup_i2c_conf_mode_v2(struct qup_i2c_dev *qup)
> +{
> +	struct qup_i2c_block *blk = &qup->blk;
> +	u32 io_mode = QUP_REPACK_EN;
> +
> +	if (blk->is_tx_blk_mode) {
> +		io_mode |= QUP_OUTPUT_BLK_MODE;
> +		writel(0, qup->base + QUP_MX_WRITE_CNT);
> +	} else {
> +		writel(0, qup->base + QUP_MX_OUTPUT_CNT);
> +	}
> +
> +	if (blk->is_rx_blk_mode) {
> +		io_mode |= QUP_INPUT_BLK_MODE;
> +		writel(0, qup->base + QUP_MX_READ_CNT);
> +	} else {
> +		writel(0, qup->base + QUP_MX_INPUT_CNT);
> +	}
> +
> +	writel(io_mode, qup->base + QUP_IO_MODE);
> +}
> +
> +/*
> + * Function to clear required variables before starting of any QUP v2
> + * sub transfer
> + */
> +static void qup_i2c_clear_blk_v2(struct qup_i2c_block *blk)
> +{
> +	blk->send_last_word = false;
> +	blk->tx_tags_sent = false;
> +	blk->tx_fifo_data = 0;
> +	blk->tx_fifo_data_pos = 0;
> +	blk->tx_fifo_free = 0;
> +
> +	blk->rx_tags_fetched = false;
> +	blk->rx_fifo_data = 0;
> +	blk->rx_fifo_data_pos = 0;
> +	blk->fifo_available = 0;
> +}
> +
> +/*
> + * Function to receive data from RX FIFO for read message in QUP v2
> + * i2c transfer.
> + */
> +static void qup_i2c_recv_data(struct qup_i2c_dev *qup)
> +{
> +	struct qup_i2c_block *blk = &qup->blk;
> +	int j;
> +
> +	for (j = blk->rx_fifo_data_pos;
> +	     blk->cur_blk_len && blk->fifo_available;
> +	     blk->cur_blk_len--, blk->fifo_available--) {
> +		if (j == 0)
> +			blk->rx_fifo_data = readl(qup->base + QUP_IN_FIFO_BASE);
> +
> +		*(blk->cur_data++) = blk->rx_fifo_data;
> +		blk->rx_fifo_data >>= 8;
> +
> +		if (j == 3)
> +			j = 0;
> +		else
> +			j++;
> +	}
> +
> +	blk->rx_fifo_data_pos = j;
> +}
> +
> +/* Function to receive tags for read message in QUP v2 i2c transfer. */
> +static void qup_i2c_recv_tags(struct qup_i2c_dev *qup)
> +{
> +	struct qup_i2c_block *blk = &qup->blk;
> +
> +	blk->rx_fifo_data = readl(qup->base + QUP_IN_FIFO_BASE);
> +	blk->rx_fifo_data >>= blk->rx_tag_len  * 8;

  why cant it be simply ignored ?

> +	blk->rx_fifo_data_pos = blk->rx_tag_len;
> +	blk->fifo_available -= blk->rx_tag_len;
> +}
> +
> +/*
> + * This function reads the data and tags from RX FIFO. Since in read case, the
> + * tags will be preceded by received data bytes need to be written so
> + * 1. Check if rx_tags_fetched is false i.e. the start of QUP block so receive
> + *    all tag bytes and discard that.
> + * 2. Read the data from RX FIFO. When all the data bytes have been read then
> + *    mark the QUP_BLK_EVENT_RX_DATA_DONE in current block event and return.
> + */
> +static void qup_i2c_read_rx_fifo_v2(struct qup_i2c_dev *qup)
> +{
> +	struct qup_i2c_block *blk = &qup->blk;
> +
> +	if (!blk->rx_tags_fetched) {
> +		qup_i2c_recv_tags(qup);
> +		blk->rx_tags_fetched = true;
> +	}
> +
> +	qup_i2c_recv_data(qup);
> +	if (!blk->cur_blk_len)
> +		qup->cur_blk_events |= QUP_BLK_EVENT_RX_DATA_DONE;
> +}
> +
> +/*
> + * Function to write bytes in TX FIFO for write message in QUP v2 i2c
> + * transfer. QUP TX FIFO write works on word basis (4 bytes). New byte write to
> + * TX FIFO will be appended in this data tx_fifo_data and will be written to TX
> + * FIFO when all the 4 bytes are available.
> + */
> +static void
> +qup_i2c_write_blk_data(struct qup_i2c_dev *qup, u8 **data, unsigned int *len)
> +{
> +	struct qup_i2c_block *blk = &qup->blk;
> +	unsigned int j;
> +
> +	for (j = blk->tx_fifo_data_pos; *len && blk->tx_fifo_free;
> +	     (*len)--, blk->tx_fifo_free--) {
> +		blk->tx_fifo_data |= *(*data)++ << (j * 8);
> +		if (j == 3) {
> +			writel(blk->tx_fifo_data,
> +			       qup->base + QUP_OUT_FIFO_BASE);
> +			blk->tx_fifo_data = 0x0;
> +			j = 0;
> +		} else {
> +			j++;
> +		}
> +	}
> +
> +	blk->tx_fifo_data_pos = j;
> +}
> +
> +/* Function to transfer tags for read message in QUP v2 i2c transfer. */
> +static void qup_i2c_write_rx_tags_v2(struct qup_i2c_dev *qup)
> +{
> +	struct qup_i2c_block *blk = &qup->blk;
> +
> +	qup_i2c_write_blk_data(qup, &blk->cur_tx_tags, &blk->tx_tag_len);
> +	if (blk->tx_fifo_data_pos)
> +		writel(blk->tx_fifo_data, qup->base + QUP_OUT_FIFO_BASE);
> +
> +	qup->cur_blk_events |= QUP_BLK_EVENT_TX_DATA_DONE;
> +}
> +
> +/*
> + * This function writes the data and tags in TX FIFO. Since in write case, both
> + * tags and data need to be written and QUP write tags can have maximum 256 data
> + * length, so it follows simple internal state machine to manage it.
> + * 1. Check if tx_tags_sent is false i.e. the start of QUP block so write the
> + *    tags to TX FIFO.
> + * 2. Check if send_last_word is true. This will be set when last few data bytes
> + *    less than 4 bytes are reamining to be written in FIFO because of no FIFO
> + *    space. All this data bytes are available in tx_fifo_data so write this
> + *    in FIFO and mark the tx done.
> + * 3. Write the data to TX FIFO and check for cur_blk_len. If this is non zero
> + *    then more data is pending otherwise following 3 cases can be possible
> + *    a. if tx_fifo_data_pos is zero that means all the data bytes in this block
> + *       have been written in TX FIFO so mark the tx done.
> + *    b. tx_fifo_free is zero. In this case, last few bytes (less than 4
> + *       bytes) are copied to tx_fifo_data but couldn't be sent because of
> + *       FIFO full so make send_last_word true.
> + *    c. tx_fifo_free is non zero i.e tx FIFO is free so copy the remaining data
> + *       from tx_fifo_data to tx FIFO and mark the tx done. Since,
> + *       qup_i2c_write_blk_data do write in 4 bytes and FIFO space is in
> + *       multiple of 4 bytes so tx_fifo_free will be always greater than or
> + *       equal to 4 bytes.

       Comments b and c should be c and b as per the code below 

> + */
> +static void qup_i2c_write_tx_fifo_v2(struct qup_i2c_dev *qup)
> +{
> +	struct qup_i2c_block *blk = &qup->blk;
> +
> +	if (!blk->tx_tags_sent) {
> +		qup_i2c_write_blk_data(qup, &blk->cur_tx_tags,
> +				       &blk->tx_tag_len);
> +		blk->tx_tags_sent = true;
> +	}
> +
> +	if (blk->send_last_word)
> +		goto send_last_word;
> +
> +	qup_i2c_write_blk_data(qup, &blk->cur_data, &blk->cur_blk_len);

 ok, do understand, why is cur_blk_len zero and we still have pending bytes to be written ?

> +	if (!blk->cur_blk_len) {
> +		if (!blk->tx_fifo_data_pos)
> +			goto tx_data_done;
> +
> +		if (blk->tx_fifo_free)
> +			goto send_last_word;
> +
> +		blk->send_last_word = true;
> +	}
> +
> +	return;
> +
> +send_last_word:
> +	writel(blk->tx_fifo_data, qup->base + QUP_OUT_FIFO_BASE);
> +tx_data_done:
> +	qup->cur_blk_events |= QUP_BLK_EVENT_TX_DATA_DONE;

  Yes, as commented on the previous patch, if we can get rid of this 4 flags
  for completion in block/fifo mode, it would simply things a bit.

> +}
> +
> +/*
> + * Main transfer function which will be used for reading or writing i2c data.
> + * The QUP v2 supports reconfiguration during run in which multiple i2c sub
> + * transfers can be scheduled.
> + */
> +static int
> +qup_i2c_conf_xfer_v2(struct qup_i2c_dev *qup, bool is_rx, bool is_first,
> +		     bool change_pause_state)
> +{
> +	struct qup_i2c_block *blk = &qup->blk;
> +	struct i2c_msg *msg = qup->msg;
> +	int ret;
> +
> +	/*
> +	 * Check if its SMBus Block read for which the top level read will be
> +	 * done into 2 QUP reads. One with message length 1 while other one is
> +	 * with actual length.
> +	 */
> +	if (qup_i2c_check_msg_len(msg)) {
> +		if (qup->is_smbus_read) {
> +			/*
> +			 * If the message length is already read in
> +			 * the first byte of the buffer, account for
> +			 * that by setting the offset
> +			 */
> +			blk->cur_data += 1;
> +			is_first = false;
> +		} else {
> +			change_pause_state = false;
> +		}
> +	}
> +
> +	qup->config_run = is_first ? 0 : QUP_I2C_MX_CONFIG_DURING_RUN;
> +
> +	qup_i2c_clear_blk_v2(blk);
> +	qup_i2c_conf_count_v2(qup);
> +
> +	/* If it is first sub transfer, then configure i2c bus clocks */
> +	if (is_first) {
> +		ret = qup_i2c_change_state(qup, QUP_RUN_STATE);
> +		if (ret)
> +			return ret;
> +
> +		writel(qup->clk_ctl, qup->base + QUP_I2C_CLK_CTL);
> +
> +		ret = qup_i2c_change_state(qup, QUP_PAUSE_STATE);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	qup_i2c_set_blk_event(qup, is_rx);

  hmm, if the completion is changed as per the just INPUT/OUTPUT done
  then this blk event tracking can be removed.

> +	reinit_completion(&qup->xfer);
> +	enable_irq(qup->irq);
> +	/*
> +	 * In FIFO mode, tx FIFO can be written directly while in block mode the
> +	 * it will be written after getting OUT_BLOCK_WRITE_REQ interrupt
> +	 */
> +	if (!blk->is_tx_blk_mode) {
> +		blk->tx_fifo_free = qup->out_fifo_sz;
> +
> +		if (is_rx)
> +			qup_i2c_write_rx_tags_v2(qup);
> +		else
> +			qup_i2c_write_tx_fifo_v2(qup);
> +	}
> +
> +	ret = qup_i2c_change_state(qup, QUP_RUN_STATE);
> +	if (ret)
> +		goto err;
> +
> +	ret = qup_i2c_wait_for_complete(qup, msg);
> +	if (ret)
> +		goto err;
> +
> +	/* Move to pause state for all the transfers, except last one */
> +	if (change_pause_state) {
> +		ret = qup_i2c_change_state(qup, QUP_PAUSE_STATE);
> +		if (ret)
> +			goto err;
> +	}
> +
> +err:
> +	disable_irq(qup->irq);
> +	return ret;
> +}
> +
> +/*
> + * Function to transfer one read/write message in i2c transfer. It splits the
> + * message into multiple of blk_xfer_limit data length blocks and schedule each
> + * QUP block individually.
> + */
> +static int qup_i2c_xfer_v2_msg(struct qup_i2c_dev *qup, int msg_id, bool is_rx)
> +{
> +	int ret = 0;
> +	unsigned int data_len, i;
> +	struct i2c_msg *msg = qup->msg;
> +	struct qup_i2c_block *blk = &qup->blk;
> +	u8 *msg_buf = msg->buf;
> +
> +	qup->blk_xfer_limit = is_rx ? RECV_MAX_DATA_LEN : QUP_READ_LIMIT;
> +	qup_i2c_set_blk_data(qup, msg);
> +
> +	for (i = 0; i < blk->count; i++) {
> +		data_len =  qup_i2c_get_data_len(qup);
> +		blk->pos = i;
> +		blk->cur_tx_tags = blk->tags;
> +		blk->cur_blk_len = data_len;
> +		blk->tx_tag_len =
> +			qup_i2c_set_tags(blk->cur_tx_tags, qup, qup->msg);
> +
> +		blk->cur_data = msg_buf;
> +
> +		if (is_rx) {
> +			blk->total_tx_len = blk->tx_tag_len;
> +			blk->rx_tag_len = 2;
> +			blk->total_rx_len = blk->rx_tag_len + data_len;
> +		} else {
> +			blk->total_tx_len = blk->tx_tag_len + data_len;
> +			blk->total_rx_len = 0;
> +		}
> +
> +		ret = qup_i2c_conf_xfer_v2(qup, is_rx, !msg_id && !i,
> +					   !qup->is_last || i < blk->count - 1);
> +		if (ret)
> +			return ret;
> +
> +		/* Handle SMBus block read length */
> +		if (qup_i2c_check_msg_len(msg) && msg->len == 1 &&
> +		    !qup->is_smbus_read) {
> +			if (msg->buf[0] > I2C_SMBUS_BLOCK_MAX)
> +				return -EPROTO;
> +
> +			msg->len = msg->buf[0];
> +			qup->is_smbus_read = true;
> +			ret = qup_i2c_xfer_v2_msg(qup, msg_id, true);
> +			qup->is_smbus_read = false;
> +			if (ret)
> +				return ret;
> +
> +			msg->len += 1;
> +		}
> +
> +		msg_buf += data_len;
> +		blk->data_len -= qup->blk_xfer_limit;
> +	}
> +
> +	return ret;
> +}
> +
> +/*
> + * QUP v2 supports 3 modes
> + * Programmed IO using FIFO mode : Less than FIFO size
> + * Programmed IO using Block mode : Greater than FIFO size
> + * DMA using BAM : Appropriate for any transactio size but the address should be
> + *		   DMA applicable
> + *

 s/transactio/transaction

> + * This function determines the mode which will be used for this transfer. An
> + * i2c transfer contains multiple message. Following are the rules to determine
> + * the mode used.
> + * 1. Determine the tx and rx length for each message and maximum tx and rx
> + *    length for complete transfer
> + * 2. If tx or rx length is greater than DMA threshold than use the DMA mode.
> + * 3. In FIFO or block mode, TX and RX can operate in different mode so check
> + *    for maximum tx and rx length to determine mode.
> + */
> +static int
> +qup_i2c_determine_mode(struct qup_i2c_dev *qup, struct i2c_msg msgs[], int num)
   
   qup_i2c_determine_mode_v2
 
> +{
> +	int idx;
> +	bool no_dma = false;
> +	unsigned int max_tx_len = 0, max_rx_len = 0;
> +	unsigned int cur_tx_len, cur_rx_len;
> +	unsigned int total_rx_len = 0, total_tx_len = 0;
> +
> +	/* All i2c_msgs should be transferred using either dma or cpu */
> +	for (idx = 0; idx < num; idx++) {
> +		if (msgs[idx].len == 0)
> +			return -EINVAL;
> +
> +		if (msgs[idx].flags & I2C_M_RD) {
> +			cur_tx_len = 0;
> +			cur_rx_len = msgs[idx].len;
> +		} else {
> +			cur_tx_len = msgs[idx].len;
> +			cur_rx_len = 0;
> +		}
> +
> +		if (is_vmalloc_addr(msgs[idx].buf))
> +			no_dma = true;
> +
> +		max_tx_len = max(max_tx_len, cur_tx_len);
> +		max_rx_len = max(max_rx_len, cur_rx_len);
> +		total_rx_len += cur_rx_len;
> +		total_tx_len += cur_tx_len;
> +	}

 why is tag length for each block not being considered here ?

> +
> +	if (!no_dma && qup->is_dma &&

  why do we need is_dma and use_dma ?
  Now that you have removed the need for is_dma in rest of places,
  better to get rid of that fully.

> +	    (total_tx_len > qup->dma_threshold ||
> +	     total_rx_len > qup->dma_threshold)) {
> +		qup->use_dma = true;
> +	} else {
> +		qup->blk.is_tx_blk_mode =
> +			max_tx_len > qup->blk_mode_threshold ? true : false;
> +		qup->blk.is_rx_blk_mode =
> +			max_rx_len > qup->blk_mode_threshold ? true : false;
> +	}
> +
> +	return 0;
> +}
> +
>  static int qup_i2c_xfer_v2(struct i2c_adapter *adap,
>  			   struct i2c_msg msgs[],
>  			   int num)
>  {
>  	struct qup_i2c_dev *qup = i2c_get_adapdata(adap);
>  	int ret, idx = 0;
> -	unsigned int total_len = 0;
>  
>  	qup->bus_err = 0;
>  	qup->qup_err = 0;
> @@ -1457,6 +1609,10 @@ static int qup_i2c_xfer_v2(struct i2c_adapter *adap,
>  	if (ret < 0)
>  		goto out;
>  
> +	ret = qup_i2c_determine_mode(qup, msgs, num);
> +	if (ret)
> +		goto out;
> +
>  	writel(1, qup->base + QUP_SW_RESET);
>  	ret = qup_i2c_poll_state(qup, QUP_RESET_STATE);
>  	if (ret)
> @@ -1466,59 +1622,35 @@ static int qup_i2c_xfer_v2(struct i2c_adapter *adap,
>  	writel(I2C_MINI_CORE | I2C_N_VAL_V2, qup->base + QUP_CONFIG);
>  	writel(QUP_V2_TAGS_EN, qup->base + QUP_I2C_MASTER_GEN);
>  
> -	if ((qup->is_dma)) {
> -		/* All i2c_msgs should be transferred using either dma or cpu */
> +	if (qup_i2c_poll_state_i2c_master(qup)) {
> +		ret = -EIO;
> +		goto out;
> +	}
> +
> +	if (qup->use_dma) {
> +		reinit_completion(&qup->xfer);
> +		ret = qup_i2c_bam_xfer(adap, &msgs[0], num);
> +		qup->use_dma = false;
> +	} else {
> +		qup_i2c_conf_mode_v2(qup);
> +
>  		for (idx = 0; idx < num; idx++) {
> -			if (msgs[idx].len == 0) {
> -				ret = -EINVAL;
> -				goto out;
> -			}
> +			qup->msg = &msgs[idx];
> +			qup->is_last = idx == (num - 1);
>  
> -			if (is_vmalloc_addr(msgs[idx].buf))
> +			ret = qup_i2c_xfer_v2_msg(qup, idx,
> +					!!(msgs[idx].flags & I2C_M_RD));

  why !!() is required here ?

> +			if (ret)
>  				break;
> -
> -			total_len += msgs[idx].len;
>  		}
> -
> -		if (idx == num && total_len > qup->dma_threshold)
> -			qup->use_dma = true;
> +		qup->msg = NULL;
>  	}
>  
> -	idx = 0;
> -
> -	do {
> -		if (msgs[idx].len == 0) {
> -			ret = -EINVAL;
> -			goto out;
> -		}
> -
> -		if (qup_i2c_poll_state_i2c_master(qup)) {
> -			ret = -EIO;
> -			goto out;
> -		}
> -
> -		qup->is_last = (idx == (num - 1));
> -		if (idx)
> -			qup->config_run = QUP_I2C_MX_CONFIG_DURING_RUN;
> -		else
> -			qup->config_run = 0;
> -
> -		reinit_completion(&qup->xfer);
> -
> -		if (qup->use_dma) {
> -			ret = qup_i2c_bam_xfer(adap, &msgs[idx], num);
> -			qup->use_dma = false;
> -			break;
> -		} else {
> -			if (msgs[idx].flags & I2C_M_RD)
> -				ret = qup_i2c_read_one_v2(qup, &msgs[idx]);
> -			else
> -				ret = qup_i2c_write_one_v2(qup, &msgs[idx]);
> -		}
> -	} while ((idx++ < (num - 1)) && !ret);
> +	if (!ret)
> +		ret = qup_i2c_bus_active(qup, ONE_BYTE);
>  
>  	if (!ret)
> -		ret = qup_i2c_change_state(qup, QUP_RESET_STATE);
> +		qup_i2c_change_state(qup, QUP_RESET_STATE);
>  
>  	if (ret == 0)
>  		ret = num;
> @@ -1582,6 +1714,7 @@ static int qup_i2c_probe(struct platform_device *pdev)
>  	u32 src_clk_freq = DEFAULT_SRC_CLK;
>  	u32 clk_freq = DEFAULT_CLK_FREQ;
>  	int blocks;
> +	bool is_qup_v1;
>  
>  	qup = devm_kzalloc(&pdev->dev, sizeof(*qup), GFP_KERNEL);
>  	if (!qup)
> @@ -1600,12 +1733,10 @@ static int qup_i2c_probe(struct platform_device *pdev)
>  	if (of_device_is_compatible(pdev->dev.of_node, "qcom,i2c-qup-v1.1.1")) {
>  		qup->adap.algo = &qup_i2c_algo;
>  		qup->adap.quirks = &qup_i2c_quirks;
> -		qup->is_qup_v1 = true;
> -		qup->write_tx_fifo = qup_i2c_write_tx_fifo_v1;
> -		qup->read_rx_fifo = qup_i2c_read_rx_fifo_v1;
> -		qup->write_rx_tags = qup_i2c_write_rx_tags_v1;
> +		is_qup_v1 = true;
>  	} else {
>  		qup->adap.algo = &qup_i2c_algo_v2;
> +		is_qup_v1 = false;
>  		ret = qup_i2c_req_dma(qup);
>  
>  		if (ret == -EPROBE_DEFER)
> @@ -1731,14 +1862,31 @@ static int qup_i2c_probe(struct platform_device *pdev)
>  		ret = -EIO;
>  		goto fail;
>  	}
> -	qup->out_blk_sz = blk_sizes[size] / 2;
> +	qup->out_blk_sz = blk_sizes[size];
>  
>  	size = QUP_INPUT_BLOCK_SIZE(io_mode);
>  	if (size >= ARRAY_SIZE(blk_sizes)) {
>  		ret = -EIO;
>  		goto fail;
>  	}
> -	qup->in_blk_sz = blk_sizes[size] / 2;
> +	qup->in_blk_sz = blk_sizes[size];
> +
> +	if (is_qup_v1) {
> +		/*
> +		 * in QUP v1, QUP_CONFIG uses N as 15 i.e 16 bits constitutes a
> +		 * single transfer but the block size is in bytes so divide the
> +		 * in_blk_sz and out_blk_sz by 2
> +		 */
> +		qup->in_blk_sz /= 2;
> +		qup->out_blk_sz /= 2;
> +		qup->write_tx_fifo = qup_i2c_write_tx_fifo_v1;
> +		qup->read_rx_fifo = qup_i2c_read_rx_fifo_v1;
> +		qup->write_rx_tags = qup_i2c_write_rx_tags_v1;
> +	} else {
> +		qup->write_tx_fifo = qup_i2c_write_tx_fifo_v2;
> +		qup->read_rx_fifo = qup_i2c_read_rx_fifo_v2;
> +		qup->write_rx_tags = qup_i2c_write_rx_tags_v2;
> +	}
>  
>  	size = QUP_OUTPUT_FIFO_SIZE(io_mode);
>  	qup->out_fifo_sz = qup->out_blk_sz * (2 << size);
> @@ -1746,6 +1894,7 @@ static int qup_i2c_probe(struct platform_device *pdev)
>  	size = QUP_INPUT_FIFO_SIZE(io_mode);
>  	qup->in_fifo_sz = qup->in_blk_sz * (2 << size);
>  	qup->dma_threshold = min(qup->out_fifo_sz, qup->in_fifo_sz);
> +	qup->blk_mode_threshold = qup->dma_threshold - QUP_MAX_TAGS_LEN;
>  
>  	fs_div = ((src_clk_freq / clk_freq) / 2) - 3;
>  	hs_div = 3;
>

<snip>

>>  static void qup_i2c_set_blk_event(struct qup_i2c_dev *qup, bool 
>> is_rx)
>>  {
>>  	qup->cur_blk_events = 0;
>> @@ -1442,13 +1155,452 @@ static int qup_i2c_xfer(struct i2c_adapter 
>> *adap,
>>  	return ret;
>>  }
>> 
> 
>   Since this is used for both FIFO and BLK mode, might be good to call 
> it
>   qup_i2c_set_event. Same in rest of the places and macros as well.
>   But if this is going to be removed altogether, then great.
> 

   Sure. I will rename this and others.
   I will check for removing but as replied in Patch 11. We need to track 
for
   3 things in RX case. For TX case, OUTPUT_DONE should be sufficient.

>> +/*
>> + * Function to configure registers related with reconfiguration 
>> during run
>> + * and will be done before each I2C sub transfer.
>> + */
>> +static void qup_i2c_conf_count_v2(struct qup_i2c_dev *qup)
>> +{
>> +	struct qup_i2c_block *blk = &qup->blk;
>> +	u32 qup_config = I2C_MINI_CORE | I2C_N_VAL_V2;
>> +
>> +	if (blk->is_tx_blk_mode)
>> +		writel(qup->config_run | blk->total_tx_len,
>> +		       qup->base + QUP_MX_OUTPUT_CNT);
>> +	else
>> +		writel(qup->config_run | blk->total_tx_len,
>> +		       qup->base + QUP_MX_WRITE_CNT);
>> +
>> +	if (blk->total_rx_len) {
>> +		if (blk->is_rx_blk_mode)
>> +			writel(qup->config_run | blk->total_rx_len,
>> +			       qup->base + QUP_MX_INPUT_CNT);
>> +		else
>> +			writel(qup->config_run | blk->total_rx_len,
>> +			       qup->base + QUP_MX_READ_CNT);
>> +	} else {
>> +		qup_config |= QUP_NO_INPUT;
>> +	}
>> +
>> +	writel(qup_config, qup->base + QUP_CONFIG);
>> +}
>> +
>> +/*
>> + * Function to configure registers related with transfer mode 
>> (FIFO/Block)
>> + * before starting of i2c transfer and will be done only once in QUP 
>> RESET
>> + * state.
>> + */
>> +static void qup_i2c_conf_mode_v2(struct qup_i2c_dev *qup)
>> +{
>> +	struct qup_i2c_block *blk = &qup->blk;
>> +	u32 io_mode = QUP_REPACK_EN;
>> +
>> +	if (blk->is_tx_blk_mode) {
>> +		io_mode |= QUP_OUTPUT_BLK_MODE;
>> +		writel(0, qup->base + QUP_MX_WRITE_CNT);
>> +	} else {
>> +		writel(0, qup->base + QUP_MX_OUTPUT_CNT);
>> +	}
>> +
>> +	if (blk->is_rx_blk_mode) {
>> +		io_mode |= QUP_INPUT_BLK_MODE;
>> +		writel(0, qup->base + QUP_MX_READ_CNT);
>> +	} else {
>> +		writel(0, qup->base + QUP_MX_INPUT_CNT);
>> +	}
>> +
>> +	writel(io_mode, qup->base + QUP_IO_MODE);
>> +}
>> +
>> +/*
>> + * Function to clear required variables before starting of any QUP v2
>> + * sub transfer
>> + */
>> +static void qup_i2c_clear_blk_v2(struct qup_i2c_block *blk)
>> +{
>> +	blk->send_last_word = false;
>> +	blk->tx_tags_sent = false;
>> +	blk->tx_fifo_data = 0;
>> +	blk->tx_fifo_data_pos = 0;
>> +	blk->tx_fifo_free = 0;
>> +
>> +	blk->rx_tags_fetched = false;
>> +	blk->rx_fifo_data = 0;
>> +	blk->rx_fifo_data_pos = 0;
>> +	blk->fifo_available = 0;
>> +}
>> +
>> +/*
>> + * Function to receive data from RX FIFO for read message in QUP v2
>> + * i2c transfer.
>> + */
>> +static void qup_i2c_recv_data(struct qup_i2c_dev *qup)
>> +{
>> +	struct qup_i2c_block *blk = &qup->blk;
>> +	int j;
>> +
>> +	for (j = blk->rx_fifo_data_pos;
>> +	     blk->cur_blk_len && blk->fifo_available;
>> +	     blk->cur_blk_len--, blk->fifo_available--) {
>> +		if (j == 0)
>> +			blk->rx_fifo_data = readl(qup->base + QUP_IN_FIFO_BASE);
>> +
>> +		*(blk->cur_data++) = blk->rx_fifo_data;
>> +		blk->rx_fifo_data >>= 8;
>> +
>> +		if (j == 3)
>> +			j = 0;
>> +		else
>> +			j++;
>> +	}
>> +
>> +	blk->rx_fifo_data_pos = j;
>> +}
>> +
>> +/* Function to receive tags for read message in QUP v2 i2c transfer. 
>> */
>> +static void qup_i2c_recv_tags(struct qup_i2c_dev *qup)
>> +{
>> +	struct qup_i2c_block *blk = &qup->blk;
>> +
>> +	blk->rx_fifo_data = readl(qup->base + QUP_IN_FIFO_BASE);
>> +	blk->rx_fifo_data >>= blk->rx_tag_len  * 8;
> 
>   why cant it be simply ignored ?
> 

   We need to read the data from FIFO and increment the rx_fifo_data_pos
   and decrement the fifo_available. The tag bytes are being ignored.

>> +	blk->rx_fifo_data_pos = blk->rx_tag_len;
>> +	blk->fifo_available -= blk->rx_tag_len;
>> +}
>> +
>> +/*
>> + * This function reads the data and tags from RX FIFO. Since in read 
>> case, the
>> + * tags will be preceded by received data bytes need to be written so
>> + * 1. Check if rx_tags_fetched is false i.e. the start of QUP block 
>> so receive
>> + *    all tag bytes and discard that.
>> + * 2. Read the data from RX FIFO. When all the data bytes have been 
>> read then
>> + *    mark the QUP_BLK_EVENT_RX_DATA_DONE in current block event and 
>> return.
>> + */
>> +static void qup_i2c_read_rx_fifo_v2(struct qup_i2c_dev *qup)
>> +{
>> +	struct qup_i2c_block *blk = &qup->blk;
>> +
>> +	if (!blk->rx_tags_fetched) {
>> +		qup_i2c_recv_tags(qup);
>> +		blk->rx_tags_fetched = true;
>> +	}
>> +
>> +	qup_i2c_recv_data(qup);
>> +	if (!blk->cur_blk_len)
>> +		qup->cur_blk_events |= QUP_BLK_EVENT_RX_DATA_DONE;
>> +}
>> +
>> +/*
>> + * Function to write bytes in TX FIFO for write message in QUP v2 i2c
>> + * transfer. QUP TX FIFO write works on word basis (4 bytes). New 
>> byte write to
>> + * TX FIFO will be appended in this data tx_fifo_data and will be 
>> written to TX
>> + * FIFO when all the 4 bytes are available.
>> + */
>> +static void
>> +qup_i2c_write_blk_data(struct qup_i2c_dev *qup, u8 **data, unsigned 
>> int *len)
>> +{
>> +	struct qup_i2c_block *blk = &qup->blk;
>> +	unsigned int j;
>> +
>> +	for (j = blk->tx_fifo_data_pos; *len && blk->tx_fifo_free;
>> +	     (*len)--, blk->tx_fifo_free--) {
>> +		blk->tx_fifo_data |= *(*data)++ << (j * 8);
>> +		if (j == 3) {
>> +			writel(blk->tx_fifo_data,
>> +			       qup->base + QUP_OUT_FIFO_BASE);
>> +			blk->tx_fifo_data = 0x0;
>> +			j = 0;
>> +		} else {
>> +			j++;
>> +		}
>> +	}
>> +
>> +	blk->tx_fifo_data_pos = j;
>> +}
>> +
>> +/* Function to transfer tags for read message in QUP v2 i2c transfer. 
>> */
>> +static void qup_i2c_write_rx_tags_v2(struct qup_i2c_dev *qup)
>> +{
>> +	struct qup_i2c_block *blk = &qup->blk;
>> +
>> +	qup_i2c_write_blk_data(qup, &blk->cur_tx_tags, &blk->tx_tag_len);
>> +	if (blk->tx_fifo_data_pos)
>> +		writel(blk->tx_fifo_data, qup->base + QUP_OUT_FIFO_BASE);
>> +
>> +	qup->cur_blk_events |= QUP_BLK_EVENT_TX_DATA_DONE;
>> +}
>> +
>> +/*
>> + * This function writes the data and tags in TX FIFO. Since in write 
>> case, both
>> + * tags and data need to be written and QUP write tags can have 
>> maximum 256 data
>> + * length, so it follows simple internal state machine to manage it.
>> + * 1. Check if tx_tags_sent is false i.e. the start of QUP block so 
>> write the
>> + *    tags to TX FIFO.
>> + * 2. Check if send_last_word is true. This will be set when last few 
>> data bytes
>> + *    less than 4 bytes are reamining to be written in FIFO because 
>> of no FIFO
>> + *    space. All this data bytes are available in tx_fifo_data so 
>> write this
>> + *    in FIFO and mark the tx done.
>> + * 3. Write the data to TX FIFO and check for cur_blk_len. If this is 
>> non zero
>> + *    then more data is pending otherwise following 3 cases can be 
>> possible
>> + *    a. if tx_fifo_data_pos is zero that means all the data bytes in 
>> this block
>> + *       have been written in TX FIFO so mark the tx done.
>> + *    b. tx_fifo_free is zero. In this case, last few bytes (less 
>> than 4
>> + *       bytes) are copied to tx_fifo_data but couldn't be sent 
>> because of
>> + *       FIFO full so make send_last_word true.
>> + *    c. tx_fifo_free is non zero i.e tx FIFO is free so copy the 
>> remaining data
>> + *       from tx_fifo_data to tx FIFO and mark the tx done. Since,
>> + *       qup_i2c_write_blk_data do write in 4 bytes and FIFO space is 
>> in
>> + *       multiple of 4 bytes so tx_fifo_free will be always greater 
>> than or
>> + *       equal to 4 bytes.
> 
>        Comments b and c should be c and b as per the code below
> 

  I Will fix that.

>> + */
>> +static void qup_i2c_write_tx_fifo_v2(struct qup_i2c_dev *qup)
>> +{
>> +	struct qup_i2c_block *blk = &qup->blk;
>> +
>> +	if (!blk->tx_tags_sent) {
>> +		qup_i2c_write_blk_data(qup, &blk->cur_tx_tags,
>> +				       &blk->tx_tag_len);
>> +		blk->tx_tags_sent = true;
>> +	}
>> +
>> +	if (blk->send_last_word)
>> +		goto send_last_word;
>> +
>> +	qup_i2c_write_blk_data(qup, &blk->cur_data, &blk->cur_blk_len);
> 
>  ok, do understand, why is cur_blk_len zero and we still have pending
> bytes to be written ?
> 

  qup_i2c_write_blk_data will return if we don't have space
  in TX FIFO or these are last remaining bytes (less than 4).
  qup_i2c_write_blk_data will write to TX FIFO when the tx_fifo_data_pos
  is 3 i.e all 4 bytes.

  The following check is taking care of this case.
  If we have space available in TX FIFO then copy the data from 
tx_fifo_data
  otherwise mark send_last_word true so that when space will be
  available then this function will just sent those remaining bytes.


>> +	if (!blk->cur_blk_len) {
>> +		if (!blk->tx_fifo_data_pos)
>> +			goto tx_data_done;
>> +
>> +		if (blk->tx_fifo_free)
>> +			goto send_last_word;
>> +
>> +		blk->send_last_word = true;
>> +	}
>> +
>> +	return;
>> +
>> +send_last_word:
>> +	writel(blk->tx_fifo_data, qup->base + QUP_OUT_FIFO_BASE);
>> +tx_data_done:
>> +	qup->cur_blk_events |= QUP_BLK_EVENT_TX_DATA_DONE;
> 
>   Yes, as commented on the previous patch, if we can get rid of this 4 
> flags
>   for completion in block/fifo mode, it would simply things a bit.
> 

  It seems QUP_BLK_EVENT_TX_DATA_DONE can be removed.

>> +}
>> +
>> +/*
>> + * Main transfer function which will be used for reading or writing 
>> i2c data.
>> + * The QUP v2 supports reconfiguration during run in which multiple 
>> i2c sub
>> + * transfers can be scheduled.
>> + */
>> +static int
>> +qup_i2c_conf_xfer_v2(struct qup_i2c_dev *qup, bool is_rx, bool 
>> is_first,
>> +		     bool change_pause_state)
>> +{
>> +	struct qup_i2c_block *blk = &qup->blk;
>> +	struct i2c_msg *msg = qup->msg;
>> +	int ret;
>> +
>> +	/*
>> +	 * Check if its SMBus Block read for which the top level read will 
>> be
>> +	 * done into 2 QUP reads. One with message length 1 while other one 
>> is
>> +	 * with actual length.
>> +	 */
>> +	if (qup_i2c_check_msg_len(msg)) {
>> +		if (qup->is_smbus_read) {
>> +			/*
>> +			 * If the message length is already read in
>> +			 * the first byte of the buffer, account for
>> +			 * that by setting the offset
>> +			 */
>> +			blk->cur_data += 1;
>> +			is_first = false;
>> +		} else {
>> +			change_pause_state = false;
>> +		}
>> +	}
>> +
>> +	qup->config_run = is_first ? 0 : QUP_I2C_MX_CONFIG_DURING_RUN;
>> +
>> +	qup_i2c_clear_blk_v2(blk);
>> +	qup_i2c_conf_count_v2(qup);
>> +
>> +	/* If it is first sub transfer, then configure i2c bus clocks */
>> +	if (is_first) {
>> +		ret = qup_i2c_change_state(qup, QUP_RUN_STATE);
>> +		if (ret)
>> +			return ret;
>> +
>> +		writel(qup->clk_ctl, qup->base + QUP_I2C_CLK_CTL);
>> +
>> +		ret = qup_i2c_change_state(qup, QUP_PAUSE_STATE);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	qup_i2c_set_blk_event(qup, is_rx);
> 
>   hmm, if the completion is changed as per the just INPUT/OUTPUT done
>   then this blk event tracking can be removed.
> 

  I will check for RX case. If adding condition for I2C_M_RD and 
remaining
  bytes to be read is zero makes code more readable then I will change
  and remove this function.

>> +	reinit_completion(&qup->xfer);
>> +	enable_irq(qup->irq);
>> +	/*
>> +	 * In FIFO mode, tx FIFO can be written directly while in block mode 
>> the
>> +	 * it will be written after getting OUT_BLOCK_WRITE_REQ interrupt
>> +	 */
>> +	if (!blk->is_tx_blk_mode) {
>> +		blk->tx_fifo_free = qup->out_fifo_sz;
>> +
>> +		if (is_rx)
>> +			qup_i2c_write_rx_tags_v2(qup);
>> +		else
>> +			qup_i2c_write_tx_fifo_v2(qup);
>> +	}
>> +
>> +	ret = qup_i2c_change_state(qup, QUP_RUN_STATE);
>> +	if (ret)
>> +		goto err;
>> +
>> +	ret = qup_i2c_wait_for_complete(qup, msg);
>> +	if (ret)
>> +		goto err;
>> +
>> +	/* Move to pause state for all the transfers, except last one */
>> +	if (change_pause_state) {
>> +		ret = qup_i2c_change_state(qup, QUP_PAUSE_STATE);
>> +		if (ret)
>> +			goto err;
>> +	}
>> +
>> +err:
>> +	disable_irq(qup->irq);
>> +	return ret;
>> +}
>> +
>> +/*
>> + * Function to transfer one read/write message in i2c transfer. It 
>> splits the
>> + * message into multiple of blk_xfer_limit data length blocks and 
>> schedule each
>> + * QUP block individually.
>> + */
>> +static int qup_i2c_xfer_v2_msg(struct qup_i2c_dev *qup, int msg_id, 
>> bool is_rx)
>> +{
>> +	int ret = 0;
>> +	unsigned int data_len, i;
>> +	struct i2c_msg *msg = qup->msg;
>> +	struct qup_i2c_block *blk = &qup->blk;
>> +	u8 *msg_buf = msg->buf;
>> +
>> +	qup->blk_xfer_limit = is_rx ? RECV_MAX_DATA_LEN : QUP_READ_LIMIT;
>> +	qup_i2c_set_blk_data(qup, msg);
>> +
>> +	for (i = 0; i < blk->count; i++) {
>> +		data_len =  qup_i2c_get_data_len(qup);
>> +		blk->pos = i;
>> +		blk->cur_tx_tags = blk->tags;
>> +		blk->cur_blk_len = data_len;
>> +		blk->tx_tag_len =
>> +			qup_i2c_set_tags(blk->cur_tx_tags, qup, qup->msg);
>> +
>> +		blk->cur_data = msg_buf;
>> +
>> +		if (is_rx) {
>> +			blk->total_tx_len = blk->tx_tag_len;
>> +			blk->rx_tag_len = 2;
>> +			blk->total_rx_len = blk->rx_tag_len + data_len;
>> +		} else {
>> +			blk->total_tx_len = blk->tx_tag_len + data_len;
>> +			blk->total_rx_len = 0;
>> +		}
>> +
>> +		ret = qup_i2c_conf_xfer_v2(qup, is_rx, !msg_id && !i,
>> +					   !qup->is_last || i < blk->count - 1);
>> +		if (ret)
>> +			return ret;
>> +
>> +		/* Handle SMBus block read length */
>> +		if (qup_i2c_check_msg_len(msg) && msg->len == 1 &&
>> +		    !qup->is_smbus_read) {
>> +			if (msg->buf[0] > I2C_SMBUS_BLOCK_MAX)
>> +				return -EPROTO;
>> +
>> +			msg->len = msg->buf[0];
>> +			qup->is_smbus_read = true;
>> +			ret = qup_i2c_xfer_v2_msg(qup, msg_id, true);
>> +			qup->is_smbus_read = false;
>> +			if (ret)
>> +				return ret;
>> +
>> +			msg->len += 1;
>> +		}
>> +
>> +		msg_buf += data_len;
>> +		blk->data_len -= qup->blk_xfer_limit;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +/*
>> + * QUP v2 supports 3 modes
>> + * Programmed IO using FIFO mode : Less than FIFO size
>> + * Programmed IO using Block mode : Greater than FIFO size
>> + * DMA using BAM : Appropriate for any transactio size but the 
>> address should be
>> + *		   DMA applicable
>> + *
> 
>  s/transactio/transaction
> 

  I Will fix this and qup_i2c_determine_mode.

>> + * This function determines the mode which will be used for this 
>> transfer. An
>> + * i2c transfer contains multiple message. Following are the rules to 
>> determine
>> + * the mode used.
>> + * 1. Determine the tx and rx length for each message and maximum tx 
>> and rx
>> + *    length for complete transfer
>> + * 2. If tx or rx length is greater than DMA threshold than use the 
>> DMA mode.
>> + * 3. In FIFO or block mode, TX and RX can operate in different mode 
>> so check
>> + *    for maximum tx and rx length to determine mode.
>> + */
>> +static int
>> +qup_i2c_determine_mode(struct qup_i2c_dev *qup, struct i2c_msg 
>> msgs[], int num)
> 
>    qup_i2c_determine_mode_v2
> 
>> +{
>> +	int idx;
>> +	bool no_dma = false;
>> +	unsigned int max_tx_len = 0, max_rx_len = 0;
>> +	unsigned int cur_tx_len, cur_rx_len;
>> +	unsigned int total_rx_len = 0, total_tx_len = 0;
>> +
>> +	/* All i2c_msgs should be transferred using either dma or cpu */
>> +	for (idx = 0; idx < num; idx++) {
>> +		if (msgs[idx].len == 0)
>> +			return -EINVAL;
>> +
>> +		if (msgs[idx].flags & I2C_M_RD) {
>> +			cur_tx_len = 0;
>> +			cur_rx_len = msgs[idx].len;
>> +		} else {
>> +			cur_tx_len = msgs[idx].len;
>> +			cur_rx_len = 0;
>> +		}
>> +
>> +		if (is_vmalloc_addr(msgs[idx].buf))
>> +			no_dma = true;
>> +
>> +		max_tx_len = max(max_tx_len, cur_tx_len);
>> +		max_rx_len = max(max_rx_len, cur_rx_len);
>> +		total_rx_len += cur_rx_len;
>> +		total_tx_len += cur_tx_len;
>> +	}
> 
>  why is tag length for each block not being considered here ?

  That is to avoid unnecessary calculation.
  The role of this function is to just determine mode.
  DMA and block mode will work for any xfer length.
  but FIFO won't work for > 64.

  The max tag length already we have considered in blk_mode_threshold.
  in None of the case, it will give FIFO mode for xfer greater than
  64 bytes.

> 
>> +
>> +	if (!no_dma && qup->is_dma &&
> 
>   why do we need is_dma and use_dma ?
>   Now that you have removed the need for is_dma in rest of places,
>   better to get rid of that fully.
> 

  Both are doing separate things.
  use_dma will be false if any msgs[idx].buf is vmalloc address.
  qup->is_dma will be false if the dma channels are not defined
  in dtsi or in case of some DMA channel allocation failure.


>> +	    (total_tx_len > qup->dma_threshold ||
>> +	     total_rx_len > qup->dma_threshold)) {
>> +		qup->use_dma = true;
>> +	} else {
>> +		qup->blk.is_tx_blk_mode =
>> +			max_tx_len > qup->blk_mode_threshold ? true : false;
>> +		qup->blk.is_rx_blk_mode =
>> +			max_rx_len > qup->blk_mode_threshold ? true : false;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>  static int qup_i2c_xfer_v2(struct i2c_adapter *adap,
>>  			   struct i2c_msg msgs[],
>>  			   int num)
>>  {
>>  	struct qup_i2c_dev *qup = i2c_get_adapdata(adap);
>>  	int ret, idx = 0;
>> -	unsigned int total_len = 0;
>> 
>>  	qup->bus_err = 0;
>>  	qup->qup_err = 0;
>> @@ -1457,6 +1609,10 @@ static int qup_i2c_xfer_v2(struct i2c_adapter 
>> *adap,
>>  	if (ret < 0)
>>  		goto out;
>> 
>> +	ret = qup_i2c_determine_mode(qup, msgs, num);
>> +	if (ret)
>> +		goto out;
>> +
>>  	writel(1, qup->base + QUP_SW_RESET);
>>  	ret = qup_i2c_poll_state(qup, QUP_RESET_STATE);
>>  	if (ret)
>> @@ -1466,59 +1622,35 @@ static int qup_i2c_xfer_v2(struct i2c_adapter 
>> *adap,
>>  	writel(I2C_MINI_CORE | I2C_N_VAL_V2, qup->base + QUP_CONFIG);
>>  	writel(QUP_V2_TAGS_EN, qup->base + QUP_I2C_MASTER_GEN);
>> 
>> -	if ((qup->is_dma)) {
>> -		/* All i2c_msgs should be transferred using either dma or cpu */
>> +	if (qup_i2c_poll_state_i2c_master(qup)) {
>> +		ret = -EIO;
>> +		goto out;
>> +	}
>> +
>> +	if (qup->use_dma) {
>> +		reinit_completion(&qup->xfer);
>> +		ret = qup_i2c_bam_xfer(adap, &msgs[0], num);
>> +		qup->use_dma = false;
>> +	} else {
>> +		qup_i2c_conf_mode_v2(qup);
>> +
>>  		for (idx = 0; idx < num; idx++) {
>> -			if (msgs[idx].len == 0) {
>> -				ret = -EINVAL;
>> -				goto out;
>> -			}
>> +			qup->msg = &msgs[idx];
>> +			qup->is_last = idx == (num - 1);
>> 
>> -			if (is_vmalloc_addr(msgs[idx].buf))
>> +			ret = qup_i2c_xfer_v2_msg(qup, idx,
>> +					!!(msgs[idx].flags & I2C_M_RD));
> 
>   why !!() is required here ?
> 

  I made it to change for boolean.
  Same thing, we are doing in other places inside i2c base driver.

  Regards,
  Abhishek

On 2/3/2018 12:58 AM, Abhishek Sahu wrote:
> Following are the major issues in current driver code
> 
> 1. The current driver simply assumes the transfer completion
>     whenever its gets any non-error interrupts and then simply do the
>     polling of available/free bytes in FIFO.
> 2. The block mode is not working properly since no handling in
>     being done for OUT_BLOCK_WRITE_REQ and IN_BLOCK_READ_READ.
> 3. An i2c transfer can contain multiple message and QUP v2
>     supports reconfiguration during run in which the mode should be same
>     for all the sub transfer. Currently the mode is being programmed
>     before every sub transfer which is functionally wrong. If one message
>     is less than FIFO length and other message is greater than FIFO
>     length, then transfers will fail.
> 
> Because of above, i2c v2 transfers of size greater than 64 are failing
> with following error message
> 
> 	i2c_qup 78b6000.i2c: timeout for fifo out full
> 
> To make block mode working properly and move to use the interrupts
> instead of polling, major code reorganization is required. Following
> are the major changes done in this patch
> 
> 1. Remove the polling of TX FIFO free space and RX FIFO available
>     bytes and move to interrupts completely. QUP has QUP_MX_OUTPUT_DONE,
>     QUP_MX_INPUT_DONE, OUT_BLOCK_WRITE_REQ and IN_BLOCK_READ_REQ
>     interrupts to handle FIFO’s properly so check all these interrupts.
> 2. Determine the mode for transfer before starting by checking
>     all the tx/rx data length in each message. The complete message can be
>     transferred either in DMA mode or Programmed IO by FIFO/Block mode.
>     in DMA mode, both tx and rx uses same mode but in PIO mode, the TX and
>     RX can be in different mode.
> 3. During write, For FIFO mode, TX FIFO can be directly written
>     without checking for FIFO space. For block mode, the QUP will generate
>     OUT_BLOCK_WRITE_REQ interrupt whenever it has block size of available
>     space.
> 4. During read, both TX and RX FIFO will be used. TX will be used
>     for writing tags and RX will be used for receiving the data. In QUP,
>     TX and RX can operate in separate mode so configure modes accordingly.
> 5. For read FIFO mode, wait for QUP_MX_INPUT_DONE interrupt which
>     will be generated after all the bytes have been copied in RX FIFO. For
>     read Block mode, QUP will generate IN_BLOCK_READ_REQ interrupts
>     whenever it has block size of available data.
> 6. Split the transfer in chunk of one QUP block size(256 bytes)
>     and schedule each block separately. QUP v2 supports reconfiguration
>     during run in which QUP can transfer multiple blocks without issuing a
>     stop events.
> 7. Port the SMBus block read support for new code changes.
> 
> Signed-off-by: Abhishek Sahu <absahu@codeaurora.org>
> ---
>   drivers/i2c/busses/i2c-qup.c | 917 +++++++++++++++++++++++++------------------
>   1 file changed, 533 insertions(+), 384 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
> index af6c21a..46736a1 100644
> --- a/drivers/i2c/busses/i2c-qup.c
> +++ b/drivers/i2c/busses/i2c-qup.c
> @@ -141,6 +141,15 @@
>   #define DEFAULT_CLK_FREQ 100000
>   #define DEFAULT_SRC_CLK 20000000
>   
> +/*
> + * Max tags length (start, stop and maximum 2 bytes address) for each QUP
> + * data transfer
> + */
> +#define QUP_MAX_TAGS_LEN		4
> +/* Max data length for each DATARD tags */
> +#define RECV_MAX_DATA_LEN		254
> +/* TAG length for DATA READ in RX FIFO  */
> +#define READ_RX_TAGS_LEN		2
>   /* MAX_OUTPUT_DONE_FLAG has been received */
>   #define QUP_BLK_EVENT_TX_IRQ_DONE		BIT(0)
>   /* MAX_INPUT_DONE_FLAG has been received */
> @@ -164,11 +173,24 @@
>    * tx_tag_len: tx tag length for current block
>    * rx_tag_len: rx tag length for current block
>    * data_len: remaining data length for current message
> + * cur_blk_len: data length for current block
>    * total_tx_len: total tx length including tag bytes for current QUP transfer
>    * total_rx_len: total rx length including tag bytes for current QUP transfer
> + * tx_fifo_data_pos: current byte number in TX FIFO word
>    * tx_fifo_free: number of free bytes in current QUP block write.
> + * rx_fifo_data_pos: current byte number in RX FIFO word
>    * fifo_available: number of available bytes in RX FIFO for current
>    *		   QUP block read
> + * tx_fifo_data: QUP TX FIFO write works on word basis (4 bytes). New byte write
> + *		 to TX FIFO will be appended in this data and will be written to
> + *		 TX FIFO when all the 4 bytes are available.
> + * rx_fifo_data: QUP RX FIFO read works on word basis (4 bytes). This will
> + *		 contains the 4 bytes of RX data.
> + * cur_data: pointer to tell cur data position for current message
> + * cur_tx_tags: pointer to tell cur position in tags
> + * tx_tags_sent: all tx tag bytes have been written in FIFO word
> + * send_last_word: for tx FIFO, last word send is pending in current block
> + * rx_tags_fetched: all the rx tag bytes have been fetched from rx fifo word
>    * is_tx_blk_mode: whether tx uses block or FIFO mode in case of non BAM xfer.
>    * is_rx_blk_mode: whether rx uses block or FIFO mode in case of non BAM xfer.
>    * tags: contains tx tag bytes for current QUP transfer
> @@ -179,10 +201,20 @@ struct qup_i2c_block {
>   	int		tx_tag_len;
>   	int		rx_tag_len;
>   	int		data_len;
> +	int		cur_blk_len;
>   	int		total_tx_len;
>   	int		total_rx_len;
> +	int		tx_fifo_data_pos;
>   	int		tx_fifo_free;
> +	int		rx_fifo_data_pos;
>   	int		fifo_available;
> +	u32		tx_fifo_data;
> +	u32		rx_fifo_data;
> +	u8		*cur_data;
> +	u8		*cur_tx_tags;
> +	bool		tx_tags_sent;
> +	bool		send_last_word;
> +	bool		rx_tags_fetched;
>   	bool		is_tx_blk_mode;
>   	bool		is_rx_blk_mode;
>   	u8		tags[6];
> @@ -214,6 +246,7 @@ struct qup_i2c_dev {
>   	int			out_blk_sz;
>   	int			in_blk_sz;
>   
> +	int			blk_xfer_limit;
>   	unsigned long		one_byte_t;
>   	unsigned long		xfer_timeout;
>   	struct qup_i2c_block	blk;
> @@ -228,10 +261,10 @@ struct qup_i2c_dev {
>   
>   	/* To check if this is the last msg */
>   	bool			is_last;
> -	bool			is_qup_v1;
> +	bool			is_smbus_read;
>   
>   	/* To configure when bus is in run state */
> -	int			config_run;
> +	u32			config_run;
>   
>   	/* dma parameters */
>   	bool			is_dma;
> @@ -245,6 +278,8 @@ struct qup_i2c_dev {
>   	unsigned int		dma_threshold;
>   	unsigned int		max_xfer_sg_len;
>   	unsigned int		tag_buf_pos;
> +	/* The threshold length above which block mode will be used */
> +	unsigned int		blk_mode_threshold;
>   	struct			dma_pool *dpool;
>   	struct			qup_i2c_tag start_tag;
>   	struct			qup_i2c_bam brx;
> @@ -309,9 +344,6 @@ static irqreturn_t qup_i2c_interrupt(int irq, void *dev)
>   		goto done;
>   	}
>   
> -	if (!qup->is_qup_v1)
> -		goto done;
> -
>   	if (opflags & QUP_OUT_SVC_FLAG) {
>   		writel(QUP_OUT_SVC_FLAG, qup->base + QUP_OPERATIONAL);
>   
> @@ -444,108 +476,6 @@ static int qup_i2c_bus_active(struct qup_i2c_dev *qup, int len)
>   	return ret;
>   }
>   
> -/**
> - * qup_i2c_wait_ready - wait for a give number of bytes in tx/rx path
> - * @qup: The qup_i2c_dev device
> - * @op: The bit/event to wait on
> - * @val: value of the bit to wait on, 0 or 1
> - * @len: The length the bytes to be transferred
> - */
> -static int qup_i2c_wait_ready(struct qup_i2c_dev *qup, int op, bool val,
> -			      int len)
> -{
> -	unsigned long timeout;
> -	u32 opflags;
> -	u32 status;
> -	u32 shift = __ffs(op);
> -	int ret = 0;
> -
> -	len *= qup->one_byte_t;
> -	/* timeout after a wait of twice the max time */
> -	timeout = jiffies + len * 4;
> -
> -	for (;;) {
> -		opflags = readl(qup->base + QUP_OPERATIONAL);
> -		status = readl(qup->base + QUP_I2C_STATUS);
> -
> -		if (((opflags & op) >> shift) == val) {
> -			if ((op == QUP_OUT_NOT_EMPTY) && qup->is_last) {
> -				if (!(status & I2C_STATUS_BUS_ACTIVE)) {
> -					ret = 0;
> -					goto done;
> -				}
> -			} else {
> -				ret = 0;
> -				goto done;
> -			}
> -		}
> -
> -		if (time_after(jiffies, timeout)) {
> -			ret = -ETIMEDOUT;
> -			goto done;
> -		}
> -		usleep_range(len, len * 2);
> -	}
> -
> -done:
> -	if (qup->bus_err || qup->qup_err)
> -		ret =  (qup->bus_err & QUP_I2C_NACK_FLAG) ? -ENXIO : -EIO;
> -
> -	return ret;
> -}
> -
> -static void qup_i2c_set_write_mode_v2(struct qup_i2c_dev *qup,
> -				      struct i2c_msg *msg)
> -{
> -	/* Number of entries to shift out, including the tags */
> -	int total = msg->len + qup->blk.tx_tag_len;
> -
> -	total |= qup->config_run;
> -
> -	if (total < qup->out_fifo_sz) {
> -		/* FIFO mode */
> -		writel(QUP_REPACK_EN, qup->base + QUP_IO_MODE);
> -		writel(total, qup->base + QUP_MX_WRITE_CNT);
> -	} else {
> -		/* BLOCK mode (transfer data on chunks) */
> -		writel(QUP_OUTPUT_BLK_MODE | QUP_REPACK_EN,
> -		       qup->base + QUP_IO_MODE);
> -		writel(total, qup->base + QUP_MX_OUTPUT_CNT);
> -	}
> -}
> -
> -static int check_for_fifo_space(struct qup_i2c_dev *qup)
> -{
> -	int ret;
> -
> -	ret = qup_i2c_change_state(qup, QUP_PAUSE_STATE);
> -	if (ret)
> -		goto out;
> -
> -	ret = qup_i2c_wait_ready(qup, QUP_OUT_FULL,
> -				 RESET_BIT, 4 * ONE_BYTE);
> -	if (ret) {
> -		/* Fifo is full. Drain out the fifo */
> -		ret = qup_i2c_change_state(qup, QUP_RUN_STATE);
> -		if (ret)
> -			goto out;
> -
> -		ret = qup_i2c_wait_ready(qup, QUP_OUT_NOT_EMPTY,
> -					 RESET_BIT, 256 * ONE_BYTE);
> -		if (ret) {
> -			dev_err(qup->dev, "timeout for fifo out full");
> -			goto out;
> -		}
> -
> -		ret = qup_i2c_change_state(qup, QUP_PAUSE_STATE);
> -		if (ret)
> -			goto out;
> -	}
> -
> -out:
> -	return ret;
> -}
> -
>   static void qup_i2c_write_tx_fifo_v1(struct qup_i2c_dev *qup)
>   {
>   	struct qup_i2c_block *blk = &qup->blk;
> @@ -591,60 +521,17 @@ static void qup_i2c_write_tx_fifo_v1(struct qup_i2c_dev *qup)
>   static void qup_i2c_set_blk_data(struct qup_i2c_dev *qup,
>   				 struct i2c_msg *msg)
>   {
> -	memset(&qup->blk, 0, sizeof(qup->blk));
> -
> +	qup->blk.pos = 0;
>   	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 = check_for_fifo_space(qup);
> -		if (ret)
> -			return ret;
> -
> -		t = (len >= 4) ? 4 : len;
> -
> -		while (idx < t) {
> -			if (!i && (pos >= tlen)) {
> -				buf = dbuf;
> -				pos = 0;
> -				i = 1;
> -			}
> -			val |= buf[pos++] << (idx++ * 8);
> -		}
> -
> -		writel(val, qup->base + QUP_OUT_FIFO_BASE);
> -		idx  = 0;
> -		val = 0;
> -		len -= 4;
> -	}
> -
> -	ret = qup_i2c_change_state(qup, QUP_RUN_STATE);
> -
> -	return ret;
> +	qup->blk.count = DIV_ROUND_UP(msg->len, qup->blk_xfer_limit);
>   }
>   
>   static int qup_i2c_get_data_len(struct qup_i2c_dev *qup)
>   {
>   	int data_len;
>   
> -	if (qup->blk.data_len > QUP_READ_LIMIT)
> -		data_len = QUP_READ_LIMIT;
> +	if (qup->blk.data_len > qup->blk_xfer_limit)
> +		data_len = qup->blk_xfer_limit;
>   	else
>   		data_len = qup->blk.data_len;
>   
> @@ -661,9 +548,9 @@ static int qup_i2c_set_tags_smb(u16 addr, u8 *tags, struct qup_i2c_dev *qup,
>   {
>   	int len = 0;
>   
> -	if (msg->len > 1) {
> +	if (qup->is_smbus_read) {
>   		tags[len++] = QUP_TAG_V2_DATARD_STOP;
> -		tags[len++] = qup_i2c_get_data_len(qup) - 1;
> +		tags[len++] = qup_i2c_get_data_len(qup);
>   	} else {
>   		tags[len++] = QUP_TAG_V2_START;
>   		tags[len++] = addr & 0xff;
> @@ -725,24 +612,6 @@ static int qup_i2c_set_tags(u8 *tags, struct qup_i2c_dev *qup,
>   	return len;
>   }
>   
> -static int qup_i2c_issue_xfer_v2(struct qup_i2c_dev *qup, struct i2c_msg *msg)
> -{
> -	int data_len = 0, tag_len, index;
> -	int ret;
> -
> -	tag_len = qup_i2c_set_tags(qup->blk.tags, qup, msg);
> -	index = msg->len - qup->blk.data_len;
> -
> -	/* only tags are written for read */
> -	if (!(msg->flags & I2C_M_RD))
> -		data_len = qup_i2c_get_data_len(qup);
> -
> -	ret = qup_i2c_send_data(qup, tag_len, qup->blk.tags,
> -				data_len, &msg->buf[index]);
> -	qup->blk.data_len -= data_len;
> -
> -	return ret;
> -}
>   
>   static void qup_i2c_bam_cb(void *data)
>   {
> @@ -809,6 +678,7 @@ static int qup_i2c_bam_make_desc(struct qup_i2c_dev *qup, struct i2c_msg *msg)
>   	u32 i = 0, tlen, tx_len = 0;
>   	u8 *tags;
>   
> +	qup->blk_xfer_limit = QUP_READ_LIMIT;
>   	qup_i2c_set_blk_data(qup, msg);
>   
>   	blocks = qup->blk.count;
> @@ -1057,7 +927,7 @@ static int qup_i2c_wait_for_complete(struct qup_i2c_dev *qup,
>   	unsigned long left;
>   	int ret = 0;
>   
> -	left = wait_for_completion_timeout(&qup->xfer, HZ);
> +	left = wait_for_completion_timeout(&qup->xfer, qup->xfer_timeout);
>   	if (!left) {
>   		writel(1, qup->base + QUP_SW_RESET);
>   		ret = -ETIMEDOUT;
> @@ -1069,65 +939,6 @@ static int qup_i2c_wait_for_complete(struct qup_i2c_dev *qup,
>   	return ret;
>   }
>   
> -static int qup_i2c_write_one_v2(struct qup_i2c_dev *qup, struct i2c_msg *msg)
> -{
> -	int ret = 0;
> -
> -	qup->msg = msg;
> -	qup->pos = 0;
> -	enable_irq(qup->irq);
> -	qup_i2c_set_blk_data(qup, msg);
> -	qup_i2c_set_write_mode_v2(qup, msg);
> -
> -	ret = qup_i2c_change_state(qup, QUP_RUN_STATE);
> -	if (ret)
> -		goto err;
> -
> -	writel(qup->clk_ctl, qup->base + QUP_I2C_CLK_CTL);
> -
> -	do {
> -		ret = qup_i2c_issue_xfer_v2(qup, msg);
> -		if (ret)
> -			goto err;
> -
> -		ret = qup_i2c_wait_for_complete(qup, msg);
> -		if (ret)
> -			goto err;
> -
> -		qup->blk.pos++;
> -	} while (qup->blk.pos < qup->blk.count);
> -
> -	ret = qup_i2c_wait_ready(qup, QUP_OUT_NOT_EMPTY, RESET_BIT, ONE_BYTE);
> -
> -err:
> -	disable_irq(qup->irq);
> -	qup->msg = NULL;
> -
> -	return ret;
> -}
> -
> -static void qup_i2c_set_read_mode_v2(struct qup_i2c_dev *qup, int len)
> -{
> -	int tx_len = qup->blk.tx_tag_len;
> -
> -	len += qup->blk.rx_tag_len;
> -	len |= qup->config_run;
> -	tx_len |= qup->config_run;
> -
> -	if (len < qup->in_fifo_sz) {
> -		/* FIFO mode */
> -		writel(QUP_REPACK_EN, qup->base + QUP_IO_MODE);
> -		writel(tx_len, qup->base + QUP_MX_WRITE_CNT);
> -		writel(len, qup->base + QUP_MX_READ_CNT);
> -	} else {
> -		/* BLOCK mode (transfer data on chunks) */
> -		writel(QUP_INPUT_BLK_MODE | QUP_REPACK_EN,
> -		       qup->base + QUP_IO_MODE);
> -		writel(tx_len, qup->base + QUP_MX_OUTPUT_CNT);
> -		writel(len, qup->base + QUP_MX_INPUT_CNT);
> -	}
> -}
> -
>   static void qup_i2c_read_rx_fifo_v1(struct qup_i2c_dev *qup)
>   {
>   	struct qup_i2c_block *blk = &qup->blk;
> @@ -1151,104 +962,6 @@ static void qup_i2c_read_rx_fifo_v1(struct qup_i2c_dev *qup)
>   		qup->cur_blk_events |= QUP_BLK_EVENT_RX_DATA_DONE;
>   }
>   
> -static int qup_i2c_read_fifo_v2(struct qup_i2c_dev *qup,
> -				struct i2c_msg *msg)
> -{
> -	u32 val;
> -	int idx, pos = 0, ret = 0, total, msg_offset = 0;
> -
> -	/*
> -	 * If the message length is already read in
> -	 * the first byte of the buffer, account for
> -	 * that by setting the offset
> -	 */
> -	if (qup_i2c_check_msg_len(msg) && (msg->len > 1))
> -		msg_offset = 1;
> -	total = qup_i2c_get_data_len(qup);
> -	total -= msg_offset;
> -
> -	/* 2 extra bytes for read tags */
> -	while (pos < (total + 2)) {
> -		/* Check that FIFO have data */
> -		ret = qup_i2c_wait_ready(qup, QUP_IN_NOT_EMPTY,
> -					 SET_BIT, 4 * ONE_BYTE);
> -		if (ret) {
> -			dev_err(qup->dev, "timeout for fifo not empty");
> -			return ret;
> -		}
> -		val = readl(qup->base + QUP_IN_FIFO_BASE);
> -
> -		for (idx = 0; idx < 4; idx++, val >>= 8, pos++) {
> -			/* first 2 bytes are tag bytes */
> -			if (pos < 2)
> -				continue;
> -
> -			if (pos >= (total + 2))
> -				goto out;
> -			msg->buf[qup->pos + msg_offset] = val & 0xff;
> -			qup->pos++;
> -		}
> -	}
> -
> -out:
> -	qup->blk.data_len -= total;
> -
> -	return ret;
> -}
> -
> -static int qup_i2c_read_one_v2(struct qup_i2c_dev *qup, struct i2c_msg *msg)
> -{
> -	int ret = 0;
> -
> -	qup->msg = msg;
> -	qup->pos  = 0;
> -	enable_irq(qup->irq);
> -	qup_i2c_set_blk_data(qup, msg);
> -	qup_i2c_set_read_mode_v2(qup, msg->len);
> -
> -	ret = qup_i2c_change_state(qup, QUP_RUN_STATE);
> -	if (ret)
> -		goto err;
> -
> -	writel(qup->clk_ctl, qup->base + QUP_I2C_CLK_CTL);
> -
> -	do {
> -		ret = qup_i2c_issue_xfer_v2(qup, msg);
> -		if (ret)
> -			goto err;
> -
> -		ret = qup_i2c_wait_for_complete(qup, msg);
> -		if (ret)
> -			goto err;
> -
> -		ret = qup_i2c_read_fifo_v2(qup, msg);
> -		if (ret)
> -			goto err;
> -
> -		qup->blk.pos++;
> -
> -		/* Handle SMBus block read length */
> -		if (qup_i2c_check_msg_len(msg) && (msg->len == 1)) {
> -			if (msg->buf[0] > I2C_SMBUS_BLOCK_MAX) {
> -				ret = -EPROTO;
> -				goto err;
> -			}
> -			msg->len += msg->buf[0];
> -			qup->pos = 0;
> -			qup_i2c_set_blk_data(qup, msg);
> -			/* set tag length for block read */
> -			qup->blk.tx_tag_len = 2;
> -			qup_i2c_set_read_mode_v2(qup, msg->buf[0]);
> -		}
> -	} while (qup->blk.pos < qup->blk.count);
> -
> -err:
> -	disable_irq(qup->irq);
> -	qup->msg = NULL;
> -
> -	return ret;
> -}
> -
>   static void qup_i2c_set_blk_event(struct qup_i2c_dev *qup, bool is_rx)
>   {
>   	qup->cur_blk_events = 0;
> @@ -1442,13 +1155,452 @@ static int qup_i2c_xfer(struct i2c_adapter *adap,
>   	return ret;
>   }
>   
> +/*
> + * Function to configure registers related with reconfiguration during run
> + * and will be done before each I2C sub transfer.
> + */
Consider changing comment style to remove the word "Function" and state 
the operation in simple present tense.

"Function to configure ..." would be "Configure ..."

Same comment for all comments of this style below.

> +static void qup_i2c_conf_count_v2(struct qup_i2c_dev *qup)
> +{
> +	struct qup_i2c_block *blk = &qup->blk;
> +	u32 qup_config = I2C_MINI_CORE | I2C_N_VAL_V2;
> +
> +	if (blk->is_tx_blk_mode)
> +		writel(qup->config_run | blk->total_tx_len,
> +		       qup->base + QUP_MX_OUTPUT_CNT);
> +	else
> +		writel(qup->config_run | blk->total_tx_len,
> +		       qup->base + QUP_MX_WRITE_CNT);
> +
> +	if (blk->total_rx_len) {
> +		if (blk->is_rx_blk_mode)
> +			writel(qup->config_run | blk->total_rx_len,
> +			       qup->base + QUP_MX_INPUT_CNT);
> +		else
> +			writel(qup->config_run | blk->total_rx_len,
> +			       qup->base + QUP_MX_READ_CNT);
> +	} else {
> +		qup_config |= QUP_NO_INPUT;
> +	}
> +
> +	writel(qup_config, qup->base + QUP_CONFIG);
> +}
> +
> +/*
> + * Function to configure registers related with transfer mode (FIFO/Block)
> + * before starting of i2c transfer and will be done only once in QUP RESET
> + * state.
> + */
> +static void qup_i2c_conf_mode_v2(struct qup_i2c_dev *qup)
> +{
> +	struct qup_i2c_block *blk = &qup->blk;
> +	u32 io_mode = QUP_REPACK_EN;
> +
> +	if (blk->is_tx_blk_mode) {
> +		io_mode |= QUP_OUTPUT_BLK_MODE;
> +		writel(0, qup->base + QUP_MX_WRITE_CNT);
> +	} else {
> +		writel(0, qup->base + QUP_MX_OUTPUT_CNT);
> +	}
> +
> +	if (blk->is_rx_blk_mode) {
> +		io_mode |= QUP_INPUT_BLK_MODE;
> +		writel(0, qup->base + QUP_MX_READ_CNT);
> +	} else {
> +		writel(0, qup->base + QUP_MX_INPUT_CNT);
> +	}
> +
> +	writel(io_mode, qup->base + QUP_IO_MODE);
> +}
> +
> +/*
> + * Function to clear required variables before starting of any QUP v2
> + * sub transfer
> + */
> +static void qup_i2c_clear_blk_v2(struct qup_i2c_block *blk)
> +{
> +	blk->send_last_word = false;
> +	blk->tx_tags_sent = false;
> +	blk->tx_fifo_data = 0;
> +	blk->tx_fifo_data_pos = 0;
> +	blk->tx_fifo_free = 0;
> +
> +	blk->rx_tags_fetched = false;
> +	blk->rx_fifo_data = 0;
> +	blk->rx_fifo_data_pos = 0;
> +	blk->fifo_available = 0;
> +}
> +
> +/*
> + * Function to receive data from RX FIFO for read message in QUP v2
> + * i2c transfer.
> + */
> +static void qup_i2c_recv_data(struct qup_i2c_dev *qup)
> +{
> +	struct qup_i2c_block *blk = &qup->blk;
> +	int j;
> +
> +	for (j = blk->rx_fifo_data_pos;
> +	     blk->cur_blk_len && blk->fifo_available;
> +	     blk->cur_blk_len--, blk->fifo_available--) {
> +		if (j == 0)
> +			blk->rx_fifo_data = readl(qup->base + QUP_IN_FIFO_BASE);
> +
> +		*(blk->cur_data++) = blk->rx_fifo_data;
> +		blk->rx_fifo_data >>= 8;
> +
> +		if (j == 3)
> +			j = 0;
> +		else
> +			j++;
> +	}
> +
> +	blk->rx_fifo_data_pos = j;
> +}
> +
> +/* Function to receive tags for read message in QUP v2 i2c transfer. */
> +static void qup_i2c_recv_tags(struct qup_i2c_dev *qup)
> +{
> +	struct qup_i2c_block *blk = &qup->blk;
> +
> +	blk->rx_fifo_data = readl(qup->base + QUP_IN_FIFO_BASE);
> +	blk->rx_fifo_data >>= blk->rx_tag_len  * 8;
> +	blk->rx_fifo_data_pos = blk->rx_tag_len;
> +	blk->fifo_available -= blk->rx_tag_len;
> +}
> +
> +/*
> + * This function reads the data and tags from RX FIFO. Since in read case, the
> + * tags will be preceded by received data bytes need to be written so
> + * 1. Check if rx_tags_fetched is false i.e. the start of QUP block so receive
> + *    all tag bytes and discard that.
put i.e. statement into () to make the comment more readable
> + * 2. Read the data from RX FIFO. When all the data bytes have been read then
> + *    mark the QUP_BLK_EVENT_RX_DATA_DONE in current block event and return.
> + */
> +static void qup_i2c_read_rx_fifo_v2(struct qup_i2c_dev *qup)
> +{
> +	struct qup_i2c_block *blk = &qup->blk;
> +
> +	if (!blk->rx_tags_fetched) {
> +		qup_i2c_recv_tags(qup);
> +		blk->rx_tags_fetched = true;
> +	}
> +
> +	qup_i2c_recv_data(qup);
> +	if (!blk->cur_blk_len)
> +		qup->cur_blk_events |= QUP_BLK_EVENT_RX_DATA_DONE;
> +}
> +
> +/*
> + * Function to write bytes in TX FIFO for write message in QUP v2 i2c
> + * transfer. QUP TX FIFO write works on word basis (4 bytes). New byte write to
> + * TX FIFO will be appended in this data tx_fifo_data and will be written to TX
> + * FIFO when all the 4 bytes are available.
> + */
> +static void
> +qup_i2c_write_blk_data(struct qup_i2c_dev *qup, u8 **data, unsigned int *len)
> +{
> +	struct qup_i2c_block *blk = &qup->blk;
> +	unsigned int j;
> +
> +	for (j = blk->tx_fifo_data_pos; *len && blk->tx_fifo_free;
> +	     (*len)--, blk->tx_fifo_free--) {
> +		blk->tx_fifo_data |= *(*data)++ << (j * 8);
> +		if (j == 3) {
> +			writel(blk->tx_fifo_data,
> +			       qup->base + QUP_OUT_FIFO_BASE);
> +			blk->tx_fifo_data = 0x0;
> +			j = 0;
> +		} else {
> +			j++;
> +		}
> +	}
> +
> +	blk->tx_fifo_data_pos = j;
> +}
> +
> +/* Function to transfer tags for read message in QUP v2 i2c transfer. */
> +static void qup_i2c_write_rx_tags_v2(struct qup_i2c_dev *qup)
> +{
> +	struct qup_i2c_block *blk = &qup->blk;
> +
> +	qup_i2c_write_blk_data(qup, &blk->cur_tx_tags, &blk->tx_tag_len);
> +	if (blk->tx_fifo_data_pos)
> +		writel(blk->tx_fifo_data, qup->base + QUP_OUT_FIFO_BASE);
> +
> +	qup->cur_blk_events |= QUP_BLK_EVENT_TX_DATA_DONE;
> +}
> +
> +/*
> + * This function writes the data and tags in TX FIFO. Since in write case, both
> + * tags and data need to be written and QUP write tags can have maximum 256 data
> + * length, so it follows simple internal state machine to manage it.
> + * 1. Check if tx_tags_sent is false i.e. the start of QUP block so write the
> + *    tags to TX FIFO.
put i.e. the start of QUP block in () for clarity
> + * 2. Check if send_last_word is true. This will be set when last few data bytes
> + *    less than 4 bytes are reamining to be written in FIFO because of no FIFO
> + *    space. All this data bytes are available in tx_fifo_data so write this
> + *    in FIFO and mark the tx done.
> + * 3. Write the data to TX FIFO and check for cur_blk_len. If this is non zero
> + *    then more data is pending otherwise following 3 cases can be possible
> + *    a. if tx_fifo_data_pos is zero that means all the data bytes in this block
> + *       have been written in TX FIFO so mark the tx done.
> + *    b. tx_fifo_free is zero. In this case, last few bytes (less than 4
> + *       bytes) are copied to tx_fifo_data but couldn't be sent because of
> + *       FIFO full so make send_last_word true.
> + *    c. tx_fifo_free is non zero i.e tx FIFO is free so copy the remaining data
> + *       from tx_fifo_data to tx FIFO and mark the tx done. Since,
> + *       qup_i2c_write_blk_data do write in 4 bytes and FIFO space is in
> + *       multiple of 4 bytes so tx_fifo_free will be always greater than or
> + *       equal to 4 bytes.
> + */
> +static void qup_i2c_write_tx_fifo_v2(struct qup_i2c_dev *qup)
> +{
> +	struct qup_i2c_block *blk = &qup->blk;
> +
> +	if (!blk->tx_tags_sent) {
> +		qup_i2c_write_blk_data(qup, &blk->cur_tx_tags,
> +				       &blk->tx_tag_len);
> +		blk->tx_tags_sent = true;
> +	}
> +
> +	if (blk->send_last_word)
> +		goto send_last_word;
> +
> +	qup_i2c_write_blk_data(qup, &blk->cur_data, &blk->cur_blk_len);
> +	if (!blk->cur_blk_len) {
> +		if (!blk->tx_fifo_data_pos)
> +			goto tx_data_done;
> +
> +		if (blk->tx_fifo_free)
> +			goto send_last_word;
> +
> +		blk->send_last_word = true;
> +	}
> +
> +	return;
> +
> +send_last_word:
> +	writel(blk->tx_fifo_data, qup->base + QUP_OUT_FIFO_BASE);
> +tx_data_done:
> +	qup->cur_blk_events |= QUP_BLK_EVENT_TX_DATA_DONE;
> +}
> +
> +/*
> + * Main transfer function which will be used for reading or writing i2c data.
> + * The QUP v2 supports reconfiguration during run in which multiple i2c sub
> + * transfers can be scheduled.
> + */
> +static int
> +qup_i2c_conf_xfer_v2(struct qup_i2c_dev *qup, bool is_rx, bool is_first,
> +		     bool change_pause_state)
> +{
> +	struct qup_i2c_block *blk = &qup->blk;
> +	struct i2c_msg *msg = qup->msg;
> +	int ret;
> +
> +	/*
> +	 * Check if its SMBus Block read for which the top level read will be
> +	 * done into 2 QUP reads. One with message length 1 while other one is
> +	 * with actual length.
> +	 */
> +	if (qup_i2c_check_msg_len(msg)) {
> +		if (qup->is_smbus_read) {
> +			/*
> +			 * If the message length is already read in
> +			 * the first byte of the buffer, account for
> +			 * that by setting the offset
> +			 */
> +			blk->cur_data += 1;
> +			is_first = false;
> +		} else {
> +			change_pause_state = false;
> +		}
> +	}
> +
> +	qup->config_run = is_first ? 0 : QUP_I2C_MX_CONFIG_DURING_RUN;
> +
> +	qup_i2c_clear_blk_v2(blk);
> +	qup_i2c_conf_count_v2(qup);
> +
> +	/* If it is first sub transfer, then configure i2c bus clocks */
> +	if (is_first) {
> +		ret = qup_i2c_change_state(qup, QUP_RUN_STATE);
> +		if (ret)
> +			return ret;
> +
> +		writel(qup->clk_ctl, qup->base + QUP_I2C_CLK_CTL);
> +
> +		ret = qup_i2c_change_state(qup, QUP_PAUSE_STATE);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	qup_i2c_set_blk_event(qup, is_rx);
> +	reinit_completion(&qup->xfer);
> +	enable_irq(qup->irq);
> +	/*
> +	 * In FIFO mode, tx FIFO can be written directly while in block mode the
> +	 * it will be written after getting OUT_BLOCK_WRITE_REQ interrupt
> +	 */
> +	if (!blk->is_tx_blk_mode) {
> +		blk->tx_fifo_free = qup->out_fifo_sz;
> +
> +		if (is_rx)
> +			qup_i2c_write_rx_tags_v2(qup);
> +		else
> +			qup_i2c_write_tx_fifo_v2(qup);
> +	}
> +
> +	ret = qup_i2c_change_state(qup, QUP_RUN_STATE);
> +	if (ret)
> +		goto err;
> +
> +	ret = qup_i2c_wait_for_complete(qup, msg);
> +	if (ret)
> +		goto err;
> +
> +	/* Move to pause state for all the transfers, except last one */
> +	if (change_pause_state) {
> +		ret = qup_i2c_change_state(qup, QUP_PAUSE_STATE);
> +		if (ret)
> +			goto err;
> +	}
> +
> +err:
> +	disable_irq(qup->irq);
> +	return ret;
> +}
> +
> +/*
> + * Function to transfer one read/write message in i2c transfer. It splits the
> + * message into multiple of blk_xfer_limit data length blocks and schedule each
> + * QUP block individually.
> + */
> +static int qup_i2c_xfer_v2_msg(struct qup_i2c_dev *qup, int msg_id, bool is_rx)
> +{
> +	int ret = 0;
> +	unsigned int data_len, i;
> +	struct i2c_msg *msg = qup->msg;
> +	struct qup_i2c_block *blk = &qup->blk;
> +	u8 *msg_buf = msg->buf;
> +
> +	qup->blk_xfer_limit = is_rx ? RECV_MAX_DATA_LEN : QUP_READ_LIMIT;
> +	qup_i2c_set_blk_data(qup, msg);
> +
> +	for (i = 0; i < blk->count; i++) {
> +		data_len =  qup_i2c_get_data_len(qup);
> +		blk->pos = i;
> +		blk->cur_tx_tags = blk->tags;
> +		blk->cur_blk_len = data_len;
> +		blk->tx_tag_len =
> +			qup_i2c_set_tags(blk->cur_tx_tags, qup, qup->msg);
> +
> +		blk->cur_data = msg_buf;
> +
> +		if (is_rx) {
> +			blk->total_tx_len = blk->tx_tag_len;
> +			blk->rx_tag_len = 2;
> +			blk->total_rx_len = blk->rx_tag_len + data_len;
> +		} else {
> +			blk->total_tx_len = blk->tx_tag_len + data_len;
> +			blk->total_rx_len = 0;
> +		}
> +
> +		ret = qup_i2c_conf_xfer_v2(qup, is_rx, !msg_id && !i,
> +					   !qup->is_last || i < blk->count - 1);
> +		if (ret)
> +			return ret;
> +
> +		/* Handle SMBus block read length */
> +		if (qup_i2c_check_msg_len(msg) && msg->len == 1 &&
> +		    !qup->is_smbus_read) {
> +			if (msg->buf[0] > I2C_SMBUS_BLOCK_MAX)
> +				return -EPROTO;
> +
> +			msg->len = msg->buf[0];
> +			qup->is_smbus_read = true;
> +			ret = qup_i2c_xfer_v2_msg(qup, msg_id, true);
> +			qup->is_smbus_read = false;
> +			if (ret)
> +				return ret;
> +
> +			msg->len += 1;
> +		}
> +
> +		msg_buf += data_len;
> +		blk->data_len -= qup->blk_xfer_limit;
> +	}
> +
> +	return ret;
> +}
> +
> +/*
> + * QUP v2 supports 3 modes
> + * Programmed IO using FIFO mode : Less than FIFO size
> + * Programmed IO using Block mode : Greater than FIFO size
> + * DMA using BAM : Appropriate for any transactio size but the address should be
> + *		   DMA applicable
> + *
> + * This function determines the mode which will be used for this transfer. An
> + * i2c transfer contains multiple message. Following are the rules to determine
> + * the mode used.
> + * 1. Determine the tx and rx length for each message and maximum tx and rx
> + *    length for complete transfer
> + * 2. If tx or rx length is greater than DMA threshold than use the DMA mode.
> + * 3. In FIFO or block mode, TX and RX can operate in different mode so check
> + *    for maximum tx and rx length to determine mode.
> + */
> +static int
> +qup_i2c_determine_mode(struct qup_i2c_dev *qup, struct i2c_msg msgs[], int num)
> +{
> +	int idx;
> +	bool no_dma = false;
> +	unsigned int max_tx_len = 0, max_rx_len = 0;
> +	unsigned int cur_tx_len, cur_rx_len;
> +	unsigned int total_rx_len = 0, total_tx_len = 0;
> +
> +	/* All i2c_msgs should be transferred using either dma or cpu */
> +	for (idx = 0; idx < num; idx++) {
> +		if (msgs[idx].len == 0)
> +			return -EINVAL;
> +
> +		if (msgs[idx].flags & I2C_M_RD) {
> +			cur_tx_len = 0;
> +			cur_rx_len = msgs[idx].len;
> +		} else {
> +			cur_tx_len = msgs[idx].len;
> +			cur_rx_len = 0;
> +		}
> +
> +		if (is_vmalloc_addr(msgs[idx].buf))
> +			no_dma = true;
> +
> +		max_tx_len = max(max_tx_len, cur_tx_len);
> +		max_rx_len = max(max_rx_len, cur_rx_len);
> +		total_rx_len += cur_rx_len;
> +		total_tx_len += cur_tx_len;
> +	}
> +
> +	if (!no_dma && qup->is_dma &&
> +	    (total_tx_len > qup->dma_threshold ||
> +	     total_rx_len > qup->dma_threshold)) {
> +		qup->use_dma = true;
> +	} else {
> +		qup->blk.is_tx_blk_mode =
> +			max_tx_len > qup->blk_mode_threshold ? true : false;
> +		qup->blk.is_rx_blk_mode =
> +			max_rx_len > qup->blk_mode_threshold ? true : false;
> +	}
> +
> +	return 0;
> +}
> +
>   static int qup_i2c_xfer_v2(struct i2c_adapter *adap,
>   			   struct i2c_msg msgs[],
>   			   int num)
>   {
>   	struct qup_i2c_dev *qup = i2c_get_adapdata(adap);
>   	int ret, idx = 0;
> -	unsigned int total_len = 0;
>   
>   	qup->bus_err = 0;
>   	qup->qup_err = 0;
> @@ -1457,6 +1609,10 @@ static int qup_i2c_xfer_v2(struct i2c_adapter *adap,
>   	if (ret < 0)
>   		goto out;
>   
> +	ret = qup_i2c_determine_mode(qup, msgs, num);
> +	if (ret)
> +		goto out;
> +
>   	writel(1, qup->base + QUP_SW_RESET);
>   	ret = qup_i2c_poll_state(qup, QUP_RESET_STATE);
>   	if (ret)
> @@ -1466,59 +1622,35 @@ static int qup_i2c_xfer_v2(struct i2c_adapter *adap,
>   	writel(I2C_MINI_CORE | I2C_N_VAL_V2, qup->base + QUP_CONFIG);
>   	writel(QUP_V2_TAGS_EN, qup->base + QUP_I2C_MASTER_GEN);
>   
> -	if ((qup->is_dma)) {
> -		/* All i2c_msgs should be transferred using either dma or cpu */
> +	if (qup_i2c_poll_state_i2c_master(qup)) {
> +		ret = -EIO;
> +		goto out;
> +	}
> +
> +	if (qup->use_dma) {
> +		reinit_completion(&qup->xfer);
> +		ret = qup_i2c_bam_xfer(adap, &msgs[0], num);
> +		qup->use_dma = false;
> +	} else {
> +		qup_i2c_conf_mode_v2(qup);
> +
>   		for (idx = 0; idx < num; idx++) {
> -			if (msgs[idx].len == 0) {
> -				ret = -EINVAL;
> -				goto out;
> -			}
> +			qup->msg = &msgs[idx];
> +			qup->is_last = idx == (num - 1);
>   
> -			if (is_vmalloc_addr(msgs[idx].buf))
> +			ret = qup_i2c_xfer_v2_msg(qup, idx,
> +					!!(msgs[idx].flags & I2C_M_RD));
> +			if (ret)
>   				break;
> -
> -			total_len += msgs[idx].len;
>   		}
> -
> -		if (idx == num && total_len > qup->dma_threshold)
> -			qup->use_dma = true;
> +		qup->msg = NULL;
>   	}
>   
> -	idx = 0;
> -
> -	do {
> -		if (msgs[idx].len == 0) {
> -			ret = -EINVAL;
> -			goto out;
> -		}
> -
> -		if (qup_i2c_poll_state_i2c_master(qup)) {
> -			ret = -EIO;
> -			goto out;
> -		}
> -
> -		qup->is_last = (idx == (num - 1));
> -		if (idx)
> -			qup->config_run = QUP_I2C_MX_CONFIG_DURING_RUN;
> -		else
> -			qup->config_run = 0;
> -
> -		reinit_completion(&qup->xfer);
> -
> -		if (qup->use_dma) {
> -			ret = qup_i2c_bam_xfer(adap, &msgs[idx], num);
> -			qup->use_dma = false;
> -			break;
> -		} else {
> -			if (msgs[idx].flags & I2C_M_RD)
> -				ret = qup_i2c_read_one_v2(qup, &msgs[idx]);
> -			else
> -				ret = qup_i2c_write_one_v2(qup, &msgs[idx]);
> -		}
> -	} while ((idx++ < (num - 1)) && !ret);
> +	if (!ret)
> +		ret = qup_i2c_bus_active(qup, ONE_BYTE);
>   
>   	if (!ret)
> -		ret = qup_i2c_change_state(qup, QUP_RESET_STATE);
> +		qup_i2c_change_state(qup, QUP_RESET_STATE);
>   
>   	if (ret == 0)
>   		ret = num;
> @@ -1582,6 +1714,7 @@ static int qup_i2c_probe(struct platform_device *pdev)
>   	u32 src_clk_freq = DEFAULT_SRC_CLK;
>   	u32 clk_freq = DEFAULT_CLK_FREQ;
>   	int blocks;
> +	bool is_qup_v1;
>   
>   	qup = devm_kzalloc(&pdev->dev, sizeof(*qup), GFP_KERNEL);
>   	if (!qup)
> @@ -1600,12 +1733,10 @@ static int qup_i2c_probe(struct platform_device *pdev)
>   	if (of_device_is_compatible(pdev->dev.of_node, "qcom,i2c-qup-v1.1.1")) {
>   		qup->adap.algo = &qup_i2c_algo;
>   		qup->adap.quirks = &qup_i2c_quirks;
> -		qup->is_qup_v1 = true;
> -		qup->write_tx_fifo = qup_i2c_write_tx_fifo_v1;
> -		qup->read_rx_fifo = qup_i2c_read_rx_fifo_v1;
> -		qup->write_rx_tags = qup_i2c_write_rx_tags_v1;
> +		is_qup_v1 = true;
>   	} else {
>   		qup->adap.algo = &qup_i2c_algo_v2;
> +		is_qup_v1 = false;
>   		ret = qup_i2c_req_dma(qup);
>   
>   		if (ret == -EPROBE_DEFER)
> @@ -1731,14 +1862,31 @@ static int qup_i2c_probe(struct platform_device *pdev)
>   		ret = -EIO;
>   		goto fail;
>   	}
> -	qup->out_blk_sz = blk_sizes[size] / 2;
> +	qup->out_blk_sz = blk_sizes[size];
>   
>   	size = QUP_INPUT_BLOCK_SIZE(io_mode);
>   	if (size >= ARRAY_SIZE(blk_sizes)) {
>   		ret = -EIO;
>   		goto fail;
>   	}
> -	qup->in_blk_sz = blk_sizes[size] / 2;
> +	qup->in_blk_sz = blk_sizes[size];
> +
> +	if (is_qup_v1) {
> +		/*
> +		 * in QUP v1, QUP_CONFIG uses N as 15 i.e 16 bits constitutes a
> +		 * single transfer but the block size is in bytes so divide the
> +		 * in_blk_sz and out_blk_sz by 2
> +		 */
> +		qup->in_blk_sz /= 2;
> +		qup->out_blk_sz /= 2;
> +		qup->write_tx_fifo = qup_i2c_write_tx_fifo_v1;
> +		qup->read_rx_fifo = qup_i2c_read_rx_fifo_v1;
> +		qup->write_rx_tags = qup_i2c_write_rx_tags_v1;
> +	} else {
> +		qup->write_tx_fifo = qup_i2c_write_tx_fifo_v2;
> +		qup->read_rx_fifo = qup_i2c_read_rx_fifo_v2;
> +		qup->write_rx_tags = qup_i2c_write_rx_tags_v2;
> +	}
>   
>   	size = QUP_OUTPUT_FIFO_SIZE(io_mode);
>   	qup->out_fifo_sz = qup->out_blk_sz * (2 << size);
> @@ -1746,6 +1894,7 @@ static int qup_i2c_probe(struct platform_device *pdev)
>   	size = QUP_INPUT_FIFO_SIZE(io_mode);
>   	qup->in_fifo_sz = qup->in_blk_sz * (2 << size);
>   	qup->dma_threshold = min(qup->out_fifo_sz, qup->in_fifo_sz);
> +	qup->blk_mode_threshold = qup->dma_threshold - QUP_MAX_TAGS_LEN;
>   
>   	fs_div = ((src_clk_freq / clk_freq) / 2) - 3;
>   	hs_div = 3;
>

<snip>

>>   static void qup_i2c_set_blk_event(struct qup_i2c_dev *qup, bool 
>> is_rx)
>>   {
>>   	qup->cur_blk_events = 0;
>> @@ -1442,13 +1155,452 @@ static int qup_i2c_xfer(struct i2c_adapter 
>> *adap,
>>   	return ret;
>>   }
>>   +/*
>> + * Function to configure registers related with reconfiguration 
>> during run
>> + * and will be done before each I2C sub transfer.
>> + */
> Consider changing comment style to remove the word "Function" and
> state the operation in simple present tense.
> 
> "Function to configure ..." would be "Configure ..."
> 
> Same comment for all comments of this style below.
> 


  Thanks Austin.

  I have done the changes for comments styles in v2

  https://lkml.org/lkml/2018/3/12/421

  Regards,
  Abhishek