linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V4 0/3] i2c: qup: Some misc fixes
@ 2016-06-10 18:08 Sricharan R
  2016-06-10 18:08 ` [PATCH V4 1/3] i2c: qup: Fix broken dma when CONFIG_DEBUG_SG is enabled Sricharan R
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Sricharan R @ 2016-06-10 18:08 UTC (permalink / raw)
  To: linux-arm-msm, ntelkar, galak, linux-kernel, andy.gross,
	linux-i2c, agross, linux-arm-kernel, nkaje, absahu
  Cc: sricharan

One for fixing the bug with CONFIG_DEBUG_SG enabled, another
to suspend the transfer for all errors instead of just for NACK
and one for correcting the wrong initial value for index variable.

[V4] Added little more description to patch#1 and split a
     change in to a new patch.
 
[V3] Added more commit description. Return more appropriate
     error codes for NACK and other bus errors. Corrected
     other bus errors handling procedure for dma mode as well.
     Removed the dev_err log for NACKs.

[V2] Removed the use of unnecessary variable assignment.

Kept the reviewed and Tested by tag for patch#1,
as there was no code change.

Depends on patch[1] for the error handling to be complete.

[1] https://lkml.org/lkml/2016/5/9/447

Sricharan R (3):
  i2c: qup: Fix broken dma when CONFIG_DEBUG_SG is enabled
  i2c: qup: Fix wrong value of index variable
  i2c: qup: Fix error handling

 drivers/i2c/busses/i2c-qup.c | 128 ++++++++++++++++++++-----------------------
 1 file changed, 58 insertions(+), 70 deletions(-)

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH V4 1/3] i2c: qup: Fix broken dma when CONFIG_DEBUG_SG is enabled
  2016-06-10 18:08 [PATCH V4 0/3] i2c: qup: Some misc fixes Sricharan R
@ 2016-06-10 18:08 ` Sricharan R
  2016-06-10 18:14   ` Sricharan
  2016-06-18 16:20   ` Wolfram Sang
  2016-06-10 18:08 ` [PATCH V4 2/3] i2c: qup: Fix wrong value of index variable Sricharan R
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 13+ messages in thread
From: Sricharan R @ 2016-06-10 18:08 UTC (permalink / raw)
  To: linux-arm-msm, ntelkar, galak, linux-kernel, andy.gross,
	linux-i2c, agross, linux-arm-kernel, nkaje, absahu
  Cc: sricharan

With CONFIG_DEBUG_SG is enabled and when dma mode is used, below dump is seen,

------------[ cut here ]------------
kernel BUG at include/linux/scatterlist.h:140!
Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.4.0-00459-g9f087b9-dirty #7
Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT)
task: ffffffc036868000 ti: ffffffc036870000 task.ti: ffffffc036870000
PC is at qup_sg_set_buf.isra.13+0x138/0x154
LR is at qup_sg_set_buf.isra.13+0x50/0x154
pc : [<ffffffc0005a0ed8>] lr : [<ffffffc0005a0df0>] pstate: 60000145
sp : ffffffc0368735c0
x29: ffffffc0368735c0 x28: ffffffc036873752
x27: ffffffc035233018 x26: ffffffc000c4e000
x25: 0000000000000000 x24: 0000000000000004
x23: 0000000000000000 x22: ffffffc035233668
x21: ffffff80004e3000 x20: ffffffc0352e0018
x19: 0000004000000000 x18: 0000000000000028
x17: 0000000000000004 x16: ffffffc0017a39c8
x15: 0000000000001cdf x14: ffffffc0019929d8
x13: ffffffc0352e0018 x12: 0000000000000000
x11: 0000000000000001 x10: 0000000000000001
x9 : ffffffc0012b2d70 x8 : ffffff80004e3000
x7 : 0000000000000018 x6 : 0000000030000000
x5 : ffffffc00199f018 x4 : ffffffc035233018
x3 : 0000000000000004 x2 : 00000000c0000000
x1 : 0000000000000003 x0 : 0000000000000000

Process swapper/0 (pid: 1, stack limit = 0xffffffc036870020)
Stack: (0xffffffc0368735c0 to 0xffffffc036874000)

sg_set_bug expects that the buf parameter passed in should be from
lowmem and a valid pageframe. This is not true for pages from
dma_alloc_coherent which can be carveouts, hence the check fails.
Change allocation of sg buffers from dma_coherent memory to kzalloc
to fix the issue. Note that now dma_map/unmap is used to make the
kzalloc'ed buffers coherent before passing it to the dmaengine.

Signed-off-by: Sricharan R <sricharan@codeaurora.org>
Reviewed-by: Andy Gross <andy.gross@linaro.org>
---
 drivers/i2c/busses/i2c-qup.c | 51 +++++++++++++-------------------------------
 1 file changed, 15 insertions(+), 36 deletions(-)

diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
index cc6439ab..43adc2d 100644
--- a/drivers/i2c/busses/i2c-qup.c
+++ b/drivers/i2c/busses/i2c-qup.c
@@ -585,8 +585,8 @@ static void qup_i2c_bam_cb(void *data)
 }
 
 static int qup_sg_set_buf(struct scatterlist *sg, void *buf,
-			  struct qup_i2c_tag *tg, unsigned int buflen,
-			  struct qup_i2c_dev *qup, int map, int dir)
+			  unsigned int buflen, struct qup_i2c_dev *qup,
+			  int dir)
 {
 	int ret;
 
@@ -595,9 +595,6 @@ static int qup_sg_set_buf(struct scatterlist *sg, void *buf,
 	if (!ret)
 		return -EINVAL;
 
-	if (!map)
-		sg_dma_address(sg) = tg->addr + ((u8 *)buf - tg->start);
-
 	return 0;
 }
 
@@ -670,16 +667,15 @@ static int qup_i2c_bam_do_xfer(struct qup_i2c_dev *qup, struct i2c_msg *msg,
 				/* scratch buf to read the start and len tags */
 				ret = qup_sg_set_buf(&qup->brx.sg[rx_buf++],
 						     &qup->brx.tag.start[0],
-						     &qup->brx.tag,
-						     2, qup, 0, 0);
+						     2, qup, DMA_FROM_DEVICE);
 
 				if (ret)
 					return ret;
 
 				ret = qup_sg_set_buf(&qup->brx.sg[rx_buf++],
 						     &msg->buf[limit * i],
-						     NULL, tlen, qup,
-						     1, DMA_FROM_DEVICE);
+						     tlen, qup,
+						     DMA_FROM_DEVICE);
 				if (ret)
 					return ret;
 
@@ -688,7 +684,7 @@ static int qup_i2c_bam_do_xfer(struct qup_i2c_dev *qup, struct i2c_msg *msg,
 			}
 			ret = qup_sg_set_buf(&qup->btx.sg[tx_buf++],
 					     &qup->start_tag.start[off],
-					     &qup->start_tag, len, qup, 0, 0);
+					     len, qup, DMA_TO_DEVICE);
 			if (ret)
 				return ret;
 
@@ -696,8 +692,7 @@ static int qup_i2c_bam_do_xfer(struct qup_i2c_dev *qup, struct i2c_msg *msg,
 			/* scratch buf to read the BAM EOT and FLUSH tags */
 			ret = qup_sg_set_buf(&qup->brx.sg[rx_buf++],
 					     &qup->brx.tag.start[0],
-					     &qup->brx.tag, 2,
-					     qup, 0, 0);
+					     2, qup, DMA_FROM_DEVICE);
 			if (ret)
 				return ret;
 		} else {
@@ -709,17 +704,15 @@ static int qup_i2c_bam_do_xfer(struct qup_i2c_dev *qup, struct i2c_msg *msg,
 				len = qup_i2c_set_tags(tags, qup, msg, 1);
 
 				ret = qup_sg_set_buf(&qup->btx.sg[tx_buf++],
-						     tags,
-						     &qup->start_tag, len,
-						     qup, 0, 0);
+						     tags, len,
+						     qup, DMA_TO_DEVICE);
 				if (ret)
 					return ret;
 
 				tx_len += len;
 				ret = qup_sg_set_buf(&qup->btx.sg[tx_buf++],
 						     &msg->buf[limit * i],
-						     NULL, tlen, qup, 1,
-						     DMA_TO_DEVICE);
+						     tlen, qup, DMA_TO_DEVICE);
 				if (ret)
 					return ret;
 				i++;
@@ -738,8 +731,7 @@ static int qup_i2c_bam_do_xfer(struct qup_i2c_dev *qup, struct i2c_msg *msg,
 							QUP_BAM_FLUSH_STOP;
 				ret = qup_sg_set_buf(&qup->btx.sg[tx_buf++],
 						     &qup->btx.tag.start[0],
-						     &qup->btx.tag, len,
-						     qup, 0, 0);
+						     len, qup, DMA_TO_DEVICE);
 				if (ret)
 					return ret;
 				tx_nents += 1;
@@ -1407,27 +1399,21 @@ static int qup_i2c_probe(struct platform_device *pdev)
 
 		/* 2 tag bytes for each block + 5 for start, stop tags */
 		size = blocks * 2 + 5;
-		qup->dpool = dma_pool_create("qup_i2c-dma-pool", &pdev->dev,
-					     size, 4, 0);
 
-		qup->start_tag.start = dma_pool_alloc(qup->dpool, GFP_KERNEL,
-						      &qup->start_tag.addr);
+		qup->start_tag.start = devm_kzalloc(&pdev->dev,
+						    size, GFP_KERNEL);
 		if (!qup->start_tag.start) {
 			ret = -ENOMEM;
 			goto fail_dma;
 		}
 
-		qup->brx.tag.start = dma_pool_alloc(qup->dpool,
-						    GFP_KERNEL,
-						    &qup->brx.tag.addr);
+		qup->brx.tag.start = devm_kzalloc(&pdev->dev, 2, GFP_KERNEL);
 		if (!qup->brx.tag.start) {
 			ret = -ENOMEM;
 			goto fail_dma;
 		}
 
-		qup->btx.tag.start = dma_pool_alloc(qup->dpool,
-						    GFP_KERNEL,
-						    &qup->btx.tag.addr);
+		qup->btx.tag.start = devm_kzalloc(&pdev->dev, 2, GFP_KERNEL);
 		if (!qup->btx.tag.start) {
 			ret = -ENOMEM;
 			goto fail_dma;
@@ -1566,13 +1552,6 @@ static int qup_i2c_remove(struct platform_device *pdev)
 	struct qup_i2c_dev *qup = platform_get_drvdata(pdev);
 
 	if (qup->is_dma) {
-		dma_pool_free(qup->dpool, qup->start_tag.start,
-			      qup->start_tag.addr);
-		dma_pool_free(qup->dpool, qup->brx.tag.start,
-			      qup->brx.tag.addr);
-		dma_pool_free(qup->dpool, qup->btx.tag.start,
-			      qup->btx.tag.addr);
-		dma_pool_destroy(qup->dpool);
 		dma_release_channel(qup->btx.dma);
 		dma_release_channel(qup->brx.dma);
 	}
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH V4 2/3] i2c: qup: Fix wrong value of index variable
  2016-06-10 18:08 [PATCH V4 0/3] i2c: qup: Some misc fixes Sricharan R
  2016-06-10 18:08 ` [PATCH V4 1/3] i2c: qup: Fix broken dma when CONFIG_DEBUG_SG is enabled Sricharan R
@ 2016-06-10 18:08 ` Sricharan R
  2016-06-10 18:15   ` Sricharan
  2016-06-18 16:21   ` Wolfram Sang
  2016-06-10 18:08 ` [PATCH V4 3/3] i2c: qup: Fix error handling Sricharan R
  2016-06-10 18:13 ` [PATCH V4 0/3] i2c: qup: Some misc fixes Sricharan
  3 siblings, 2 replies; 13+ messages in thread
From: Sricharan R @ 2016-06-10 18:08 UTC (permalink / raw)
  To: linux-arm-msm, ntelkar, galak, linux-kernel, andy.gross,
	linux-i2c, agross, linux-arm-kernel, nkaje, absahu
  Cc: sricharan

index gets incremented during check to determine if the
messages can be transferred with dma. But not reset after
that, resulting in wrong start value in subsequent loop,
causing failure. Fix it.

Signed-off-by: Sricharan R <sricharan@codeaurora.org>
---
 drivers/i2c/busses/i2c-qup.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
index 43adc2d..b8116e5 100644
--- a/drivers/i2c/busses/i2c-qup.c
+++ b/drivers/i2c/busses/i2c-qup.c
@@ -1260,6 +1260,8 @@ static int qup_i2c_xfer_v2(struct i2c_adapter *adap,
 		}
 	}
 
+	idx = 0;
+
 	do {
 		if (msgs[idx].len == 0) {
 			ret = -EINVAL;
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH V4 3/3] i2c: qup: Fix error handling
  2016-06-10 18:08 [PATCH V4 0/3] i2c: qup: Some misc fixes Sricharan R
  2016-06-10 18:08 ` [PATCH V4 1/3] i2c: qup: Fix broken dma when CONFIG_DEBUG_SG is enabled Sricharan R
  2016-06-10 18:08 ` [PATCH V4 2/3] i2c: qup: Fix wrong value of index variable Sricharan R
@ 2016-06-10 18:08 ` Sricharan R
  2016-06-10 18:15   ` Sricharan
  2016-06-18 16:25   ` Wolfram Sang
  2016-06-10 18:13 ` [PATCH V4 0/3] i2c: qup: Some misc fixes Sricharan
  3 siblings, 2 replies; 13+ messages in thread
From: Sricharan R @ 2016-06-10 18:08 UTC (permalink / raw)
  To: linux-arm-msm, ntelkar, galak, linux-kernel, andy.gross,
	linux-i2c, agross, linux-arm-kernel, nkaje, absahu
  Cc: sricharan

Among the bus errors reported from the QUP_MASTER_STATUS register
only NACK is considered and transfer gets suspended, while
other errors are ignored. Correct this and suspend the transfer
for other errors as well. This avoids unnecessary 'timeouts' which
happens when waiting for events that would never happen when there
is already an error condition on the bus. Also the error handling
procedure should be the same for both NACK and other bus errors in
case of dma mode. So correct that as well.

Signed-off-by: Sricharan R <sricharan@codeaurora.org>
---
 drivers/i2c/busses/i2c-qup.c | 76 ++++++++++++++++++++++++--------------------
 1 file changed, 41 insertions(+), 35 deletions(-)

diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
index b8116e5..44c80ae 100644
--- a/drivers/i2c/busses/i2c-qup.c
+++ b/drivers/i2c/busses/i2c-qup.c
@@ -310,6 +310,7 @@ static int qup_i2c_wait_ready(struct qup_i2c_dev *qup, int op, bool val,
 	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 */
@@ -321,18 +322,28 @@ static int qup_i2c_wait_ready(struct qup_i2c_dev *qup, int op, bool val,
 
 		if (((opflags & op) >> shift) == val) {
 			if ((op == QUP_OUT_NOT_EMPTY) && qup->is_last) {
-				if (!(status & I2C_STATUS_BUS_ACTIVE))
-					return 0;
+				if (!(status & I2C_STATUS_BUS_ACTIVE)) {
+					ret = 0;
+					goto done;
+				}
 			} else {
-				return 0;
+				ret = 0;
+				goto done;
 			}
 		}
 
-		if (time_after(jiffies, timeout))
-			return -ETIMEDOUT;
-
+		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,
@@ -793,39 +804,35 @@ static int qup_i2c_bam_do_xfer(struct qup_i2c_dev *qup, struct i2c_msg *msg,
 	}
 
 	if (ret || qup->bus_err || qup->qup_err) {
-		if (qup->bus_err & QUP_I2C_NACK_FLAG) {
-			msg--;
-			dev_err(qup->dev, "NACK from %x\n", msg->addr);
-			ret = -EIO;
+		if (qup_i2c_change_state(qup, QUP_RUN_STATE)) {
+			dev_err(qup->dev, "change to run state timed out");
+			goto desc_err;
+		}
 
-			if (qup_i2c_change_state(qup, QUP_RUN_STATE)) {
-				dev_err(qup->dev, "change to run state timed out");
-				return ret;
-			}
+		if (rx_nents)
+			writel(QUP_BAM_INPUT_EOT,
+			       qup->base + QUP_OUT_FIFO_BASE);
 
-			if (rx_nents)
-				writel(QUP_BAM_INPUT_EOT,
-				       qup->base + QUP_OUT_FIFO_BASE);
+		writel(QUP_BAM_FLUSH_STOP, qup->base + QUP_OUT_FIFO_BASE);
 
-			writel(QUP_BAM_FLUSH_STOP,
-			       qup->base + QUP_OUT_FIFO_BASE);
+		qup_i2c_flush(qup);
 
-			qup_i2c_flush(qup);
+		/* wait for remaining interrupts to occur */
+		if (!wait_for_completion_timeout(&qup->xfer, HZ))
+			dev_err(qup->dev, "flush timed out\n");
 
-			/* wait for remaining interrupts to occur */
-			if (!wait_for_completion_timeout(&qup->xfer, HZ))
-				dev_err(qup->dev, "flush timed out\n");
+		qup_i2c_rel_dma(qup);
 
-			qup_i2c_rel_dma(qup);
-		}
+		ret =  (qup->bus_err & QUP_I2C_NACK_FLAG) ? -ENXIO : -EIO;
 	}
 
+desc_err:
 	dma_unmap_sg(qup->dev, qup->btx.sg, tx_nents, DMA_TO_DEVICE);
 
 	if (rx_nents)
 		dma_unmap_sg(qup->dev, qup->brx.sg, rx_nents,
 			     DMA_FROM_DEVICE);
-desc_err:
+
 	return ret;
 }
 
@@ -841,9 +848,6 @@ static int qup_i2c_bam_xfer(struct i2c_adapter *adap, struct i2c_msg *msg,
 	if (ret)
 		goto out;
 
-	qup->bus_err = 0;
-	qup->qup_err = 0;
-
 	writel(0, qup->base + QUP_MX_INPUT_CNT);
 	writel(0, qup->base + QUP_MX_OUTPUT_CNT);
 
@@ -881,12 +885,8 @@ static int qup_i2c_wait_for_complete(struct qup_i2c_dev *qup,
 		ret = -ETIMEDOUT;
 	}
 
-	if (qup->bus_err || qup->qup_err) {
-		if (qup->bus_err & QUP_I2C_NACK_FLAG) {
-			dev_err(qup->dev, "NACK from %x\n", msg->addr);
-			ret = -EIO;
-		}
-	}
+	if (qup->bus_err || qup->qup_err)
+		ret =  (qup->bus_err & QUP_I2C_NACK_FLAG) ? -ENXIO : -EIO;
 
 	return ret;
 }
@@ -1178,6 +1178,9 @@ static int qup_i2c_xfer(struct i2c_adapter *adap,
 	if (ret < 0)
 		goto out;
 
+	qup->bus_err = 0;
+	qup->qup_err = 0;
+
 	writel(1, qup->base + QUP_SW_RESET);
 	ret = qup_i2c_poll_state(qup, QUP_RESET_STATE);
 	if (ret)
@@ -1227,6 +1230,9 @@ static int qup_i2c_xfer_v2(struct i2c_adapter *adap,
 	struct qup_i2c_dev *qup = i2c_get_adapdata(adap);
 	int ret, len, idx = 0, use_dma = 0;
 
+	qup->bus_err = 0;
+	qup->qup_err = 0;
+
 	ret = pm_runtime_get_sync(qup->dev);
 	if (ret < 0)
 		goto out;
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* RE: [PATCH V4 0/3] i2c: qup: Some misc fixes
  2016-06-10 18:08 [PATCH V4 0/3] i2c: qup: Some misc fixes Sricharan R
                   ` (2 preceding siblings ...)
  2016-06-10 18:08 ` [PATCH V4 3/3] i2c: qup: Fix error handling Sricharan R
@ 2016-06-10 18:13 ` Sricharan
  3 siblings, 0 replies; 13+ messages in thread
From: Sricharan @ 2016-06-10 18:13 UTC (permalink / raw)
  To: linux-arm-msm, ntelkar, galak, linux-kernel, andy.gross,
	linux-i2c, agross, linux-arm-kernel, nkaje, absahu, wsa

+Wolfram. Sorry Missed.

>-----Original Message-----
>From: linux-arm-kernel [mailto:linux-arm-kernel-bounces@lists.infradead.org] On Behalf Of Sricharan R
>Sent: Friday, June 10, 2016 11:38 PM
>To: linux-arm-msm@vger.kernel.org; ntelkar@codeaurora.org; galak@codeaurora.org; linux-kernel@vger.kernel.org;
>andy.gross@linaro.org; linux-i2c@vger.kernel.org; agross@codeaurora.org; linux-arm-kernel@lists.infradead.org;
>nkaje@codeaurora.org; absahu@codeaurora.org
>Cc: sricharan@codeaurora.org
>Subject: [PATCH V4 0/3] i2c: qup: Some misc fixes
>
>One for fixing the bug with CONFIG_DEBUG_SG enabled, another
>to suspend the transfer for all errors instead of just for NACK
>and one for correcting the wrong initial value for index variable.
>
>[V4] Added little more description to patch#1 and split a
>     change in to a new patch.
>
>[V3] Added more commit description. Return more appropriate
>     error codes for NACK and other bus errors. Corrected
>     other bus errors handling procedure for dma mode as well.
>     Removed the dev_err log for NACKs.
>
>[V2] Removed the use of unnecessary variable assignment.
>
>Kept the reviewed and Tested by tag for patch#1,
>as there was no code change.
>
>Depends on patch[1] for the error handling to be complete.
>
>[1] https://lkml.org/lkml/2016/5/9/447
>
>Sricharan R (3):
>  i2c: qup: Fix broken dma when CONFIG_DEBUG_SG is enabled
>  i2c: qup: Fix wrong value of index variable
>  i2c: qup: Fix error handling
>
> drivers/i2c/busses/i2c-qup.c | 128 ++++++++++++++++++++-----------------------
> 1 file changed, 58 insertions(+), 70 deletions(-)
>
>--
>QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux
>Foundation
>
>
>_______________________________________________
>linux-arm-kernel mailing list
>linux-arm-kernel@lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH V4 1/3] i2c: qup: Fix broken dma when CONFIG_DEBUG_SG is enabled
  2016-06-10 18:08 ` [PATCH V4 1/3] i2c: qup: Fix broken dma when CONFIG_DEBUG_SG is enabled Sricharan R
@ 2016-06-10 18:14   ` Sricharan
  2016-06-18 16:20     ` Wolfram Sang
  2016-06-18 16:20   ` Wolfram Sang
  1 sibling, 1 reply; 13+ messages in thread
From: Sricharan @ 2016-06-10 18:14 UTC (permalink / raw)
  To: linux-arm-msm, ntelkar, galak, linux-kernel, andy.gross,
	linux-i2c, agross, linux-arm-kernel, nkaje, absahu, wsa

+Wolfram Sang

>-----Original Message-----
>From: linux-arm-kernel [mailto:linux-arm-kernel-bounces@lists.infradead.org] On Behalf Of Sricharan R
>Sent: Friday, June 10, 2016 11:38 PM
>To: linux-arm-msm@vger.kernel.org; ntelkar@codeaurora.org; galak@codeaurora.org; linux-kernel@vger.kernel.org;
>andy.gross@linaro.org; linux-i2c@vger.kernel.org; agross@codeaurora.org; linux-arm-kernel@lists.infradead.org;
>nkaje@codeaurora.org; absahu@codeaurora.org
>Cc: sricharan@codeaurora.org
>Subject: [PATCH V4 1/3] i2c: qup: Fix broken dma when CONFIG_DEBUG_SG is enabled
>
>With CONFIG_DEBUG_SG is enabled and when dma mode is used, below dump is seen,
>
>------------[ cut here ]------------
>kernel BUG at include/linux/scatterlist.h:140!
>Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
>Modules linked in:
>CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.4.0-00459-g9f087b9-dirty #7
>Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT)
>task: ffffffc036868000 ti: ffffffc036870000 task.ti: ffffffc036870000
>PC is at qup_sg_set_buf.isra.13+0x138/0x154
>LR is at qup_sg_set_buf.isra.13+0x50/0x154
>pc : [<ffffffc0005a0ed8>] lr : [<ffffffc0005a0df0>] pstate: 60000145
>sp : ffffffc0368735c0
>x29: ffffffc0368735c0 x28: ffffffc036873752
>x27: ffffffc035233018 x26: ffffffc000c4e000
>x25: 0000000000000000 x24: 0000000000000004
>x23: 0000000000000000 x22: ffffffc035233668
>x21: ffffff80004e3000 x20: ffffffc0352e0018
>x19: 0000004000000000 x18: 0000000000000028
>x17: 0000000000000004 x16: ffffffc0017a39c8
>x15: 0000000000001cdf x14: ffffffc0019929d8
>x13: ffffffc0352e0018 x12: 0000000000000000
>x11: 0000000000000001 x10: 0000000000000001
>x9 : ffffffc0012b2d70 x8 : ffffff80004e3000
>x7 : 0000000000000018 x6 : 0000000030000000
>x5 : ffffffc00199f018 x4 : ffffffc035233018
>x3 : 0000000000000004 x2 : 00000000c0000000
>x1 : 0000000000000003 x0 : 0000000000000000
>
>Process swapper/0 (pid: 1, stack limit = 0xffffffc036870020)
>Stack: (0xffffffc0368735c0 to 0xffffffc036874000)
>
>sg_set_bug expects that the buf parameter passed in should be from
>lowmem and a valid pageframe. This is not true for pages from
>dma_alloc_coherent which can be carveouts, hence the check fails.
>Change allocation of sg buffers from dma_coherent memory to kzalloc
>to fix the issue. Note that now dma_map/unmap is used to make the
>kzalloc'ed buffers coherent before passing it to the dmaengine.
>
>Signed-off-by: Sricharan R <sricharan@codeaurora.org>
>Reviewed-by: Andy Gross <andy.gross@linaro.org>
>---
> drivers/i2c/busses/i2c-qup.c | 51 +++++++++++++-------------------------------
> 1 file changed, 15 insertions(+), 36 deletions(-)
>
>diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
>index cc6439ab..43adc2d 100644
>--- a/drivers/i2c/busses/i2c-qup.c
>+++ b/drivers/i2c/busses/i2c-qup.c
>@@ -585,8 +585,8 @@ static void qup_i2c_bam_cb(void *data)
> }
>
> static int qup_sg_set_buf(struct scatterlist *sg, void *buf,
>-			  struct qup_i2c_tag *tg, unsigned int buflen,
>-			  struct qup_i2c_dev *qup, int map, int dir)
>+			  unsigned int buflen, struct qup_i2c_dev *qup,
>+			  int dir)
> {
> 	int ret;
>
>@@ -595,9 +595,6 @@ static int qup_sg_set_buf(struct scatterlist *sg, void *buf,
> 	if (!ret)
> 		return -EINVAL;
>
>-	if (!map)
>-		sg_dma_address(sg) = tg->addr + ((u8 *)buf - tg->start);
>-
> 	return 0;
> }
>
>@@ -670,16 +667,15 @@ static int qup_i2c_bam_do_xfer(struct qup_i2c_dev *qup, struct i2c_msg *msg,
> 				/* scratch buf to read the start and len tags */
> 				ret = qup_sg_set_buf(&qup->brx.sg[rx_buf++],
> 						     &qup->brx.tag.start[0],
>-						     &qup->brx.tag,
>-						     2, qup, 0, 0);
>+						     2, qup, DMA_FROM_DEVICE);
>
> 				if (ret)
> 					return ret;
>
> 				ret = qup_sg_set_buf(&qup->brx.sg[rx_buf++],
> 						     &msg->buf[limit * i],
>-						     NULL, tlen, qup,
>-						     1, DMA_FROM_DEVICE);
>+						     tlen, qup,
>+						     DMA_FROM_DEVICE);
> 				if (ret)
> 					return ret;
>
>@@ -688,7 +684,7 @@ static int qup_i2c_bam_do_xfer(struct qup_i2c_dev *qup, struct i2c_msg *msg,
> 			}
> 			ret = qup_sg_set_buf(&qup->btx.sg[tx_buf++],
> 					     &qup->start_tag.start[off],
>-					     &qup->start_tag, len, qup, 0, 0);
>+					     len, qup, DMA_TO_DEVICE);
> 			if (ret)
> 				return ret;
>
>@@ -696,8 +692,7 @@ static int qup_i2c_bam_do_xfer(struct qup_i2c_dev *qup, struct i2c_msg *msg,
> 			/* scratch buf to read the BAM EOT and FLUSH tags */
> 			ret = qup_sg_set_buf(&qup->brx.sg[rx_buf++],
> 					     &qup->brx.tag.start[0],
>-					     &qup->brx.tag, 2,
>-					     qup, 0, 0);
>+					     2, qup, DMA_FROM_DEVICE);
> 			if (ret)
> 				return ret;
> 		} else {
>@@ -709,17 +704,15 @@ static int qup_i2c_bam_do_xfer(struct qup_i2c_dev *qup, struct i2c_msg *msg,
> 				len = qup_i2c_set_tags(tags, qup, msg, 1);
>
> 				ret = qup_sg_set_buf(&qup->btx.sg[tx_buf++],
>-						     tags,
>-						     &qup->start_tag, len,
>-						     qup, 0, 0);
>+						     tags, len,
>+						     qup, DMA_TO_DEVICE);
> 				if (ret)
> 					return ret;
>
> 				tx_len += len;
> 				ret = qup_sg_set_buf(&qup->btx.sg[tx_buf++],
> 						     &msg->buf[limit * i],
>-						     NULL, tlen, qup, 1,
>-						     DMA_TO_DEVICE);
>+						     tlen, qup, DMA_TO_DEVICE);
> 				if (ret)
> 					return ret;
> 				i++;
>@@ -738,8 +731,7 @@ static int qup_i2c_bam_do_xfer(struct qup_i2c_dev *qup, struct i2c_msg *msg,
> 							QUP_BAM_FLUSH_STOP;
> 				ret = qup_sg_set_buf(&qup->btx.sg[tx_buf++],
> 						     &qup->btx.tag.start[0],
>-						     &qup->btx.tag, len,
>-						     qup, 0, 0);
>+						     len, qup, DMA_TO_DEVICE);
> 				if (ret)
> 					return ret;
> 				tx_nents += 1;
>@@ -1407,27 +1399,21 @@ static int qup_i2c_probe(struct platform_device *pdev)
>
> 		/* 2 tag bytes for each block + 5 for start, stop tags */
> 		size = blocks * 2 + 5;
>-		qup->dpool = dma_pool_create("qup_i2c-dma-pool", &pdev->dev,
>-					     size, 4, 0);
>
>-		qup->start_tag.start = dma_pool_alloc(qup->dpool, GFP_KERNEL,
>-						      &qup->start_tag.addr);
>+		qup->start_tag.start = devm_kzalloc(&pdev->dev,
>+						    size, GFP_KERNEL);
> 		if (!qup->start_tag.start) {
> 			ret = -ENOMEM;
> 			goto fail_dma;
> 		}
>
>-		qup->brx.tag.start = dma_pool_alloc(qup->dpool,
>-						    GFP_KERNEL,
>-						    &qup->brx.tag.addr);
>+		qup->brx.tag.start = devm_kzalloc(&pdev->dev, 2, GFP_KERNEL);
> 		if (!qup->brx.tag.start) {
> 			ret = -ENOMEM;
> 			goto fail_dma;
> 		}
>
>-		qup->btx.tag.start = dma_pool_alloc(qup->dpool,
>-						    GFP_KERNEL,
>-						    &qup->btx.tag.addr);
>+		qup->btx.tag.start = devm_kzalloc(&pdev->dev, 2, GFP_KERNEL);
> 		if (!qup->btx.tag.start) {
> 			ret = -ENOMEM;
> 			goto fail_dma;
>@@ -1566,13 +1552,6 @@ static int qup_i2c_remove(struct platform_device *pdev)
> 	struct qup_i2c_dev *qup = platform_get_drvdata(pdev);
>
> 	if (qup->is_dma) {
>-		dma_pool_free(qup->dpool, qup->start_tag.start,
>-			      qup->start_tag.addr);
>-		dma_pool_free(qup->dpool, qup->brx.tag.start,
>-			      qup->brx.tag.addr);
>-		dma_pool_free(qup->dpool, qup->btx.tag.start,
>-			      qup->btx.tag.addr);
>-		dma_pool_destroy(qup->dpool);
> 		dma_release_channel(qup->btx.dma);
> 		dma_release_channel(qup->brx.dma);
> 	}
>--
>QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux
>Foundation
>
>
>_______________________________________________
>linux-arm-kernel mailing list
>linux-arm-kernel@lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH V4 2/3] i2c: qup: Fix wrong value of index variable
  2016-06-10 18:08 ` [PATCH V4 2/3] i2c: qup: Fix wrong value of index variable Sricharan R
@ 2016-06-10 18:15   ` Sricharan
  2016-06-18 16:21   ` Wolfram Sang
  1 sibling, 0 replies; 13+ messages in thread
From: Sricharan @ 2016-06-10 18:15 UTC (permalink / raw)
  To: linux-arm-msm, ntelkar, galak, linux-kernel, andy.gross,
	linux-i2c, agross, linux-arm-kernel, nkaje, absahu, wsa

+Wolfram Sang

>-----Original Message-----
>From: linux-arm-kernel [mailto:linux-arm-kernel-bounces@lists.infradead.org] On Behalf Of Sricharan R
>Sent: Friday, June 10, 2016 11:38 PM
>To: linux-arm-msm@vger.kernel.org; ntelkar@codeaurora.org; galak@codeaurora.org; linux-kernel@vger.kernel.org;
>andy.gross@linaro.org; linux-i2c@vger.kernel.org; agross@codeaurora.org; linux-arm-kernel@lists.infradead.org;
>nkaje@codeaurora.org; absahu@codeaurora.org
>Cc: sricharan@codeaurora.org
>Subject: [PATCH V4 2/3] i2c: qup: Fix wrong value of index variable
>
>index gets incremented during check to determine if the
>messages can be transferred with dma. But not reset after
>that, resulting in wrong start value in subsequent loop,
>causing failure. Fix it.
>
>Signed-off-by: Sricharan R <sricharan@codeaurora.org>
>---
> drivers/i2c/busses/i2c-qup.c | 2 ++
> 1 file changed, 2 insertions(+)
>
>diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
>index 43adc2d..b8116e5 100644
>--- a/drivers/i2c/busses/i2c-qup.c
>+++ b/drivers/i2c/busses/i2c-qup.c
>@@ -1260,6 +1260,8 @@ static int qup_i2c_xfer_v2(struct i2c_adapter *adap,
> 		}
> 	}
>
>+	idx = 0;
>+
> 	do {
> 		if (msgs[idx].len == 0) {
> 			ret = -EINVAL;
>--
>QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux
>Foundation
>
>
>_______________________________________________
>linux-arm-kernel mailing list
>linux-arm-kernel@lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH V4 3/3] i2c: qup: Fix error handling
  2016-06-10 18:08 ` [PATCH V4 3/3] i2c: qup: Fix error handling Sricharan R
@ 2016-06-10 18:15   ` Sricharan
  2016-06-18 16:25   ` Wolfram Sang
  1 sibling, 0 replies; 13+ messages in thread
From: Sricharan @ 2016-06-10 18:15 UTC (permalink / raw)
  To: linux-arm-msm, ntelkar, galak, linux-kernel, andy.gross,
	linux-i2c, agross, linux-arm-kernel, nkaje, absahu, wsa

+Wolfram Sang

>-----Original Message-----
>From: linux-arm-kernel [mailto:linux-arm-kernel-bounces@lists.infradead.org] On Behalf Of Sricharan R
>Sent: Friday, June 10, 2016 11:38 PM
>To: linux-arm-msm@vger.kernel.org; ntelkar@codeaurora.org; galak@codeaurora.org; linux-kernel@vger.kernel.org;
>andy.gross@linaro.org; linux-i2c@vger.kernel.org; agross@codeaurora.org; linux-arm-kernel@lists.infradead.org;
>nkaje@codeaurora.org; absahu@codeaurora.org
>Cc: sricharan@codeaurora.org
>Subject: [PATCH V4 3/3] i2c: qup: Fix error handling
>
>Among the bus errors reported from the QUP_MASTER_STATUS register
>only NACK is considered and transfer gets suspended, while
>other errors are ignored. Correct this and suspend the transfer
>for other errors as well. This avoids unnecessary 'timeouts' which
>happens when waiting for events that would never happen when there
>is already an error condition on the bus. Also the error handling
>procedure should be the same for both NACK and other bus errors in
>case of dma mode. So correct that as well.
>
>Signed-off-by: Sricharan R <sricharan@codeaurora.org>
>---
> drivers/i2c/busses/i2c-qup.c | 76 ++++++++++++++++++++++++--------------------
> 1 file changed, 41 insertions(+), 35 deletions(-)
>
>diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
>index b8116e5..44c80ae 100644
>--- a/drivers/i2c/busses/i2c-qup.c
>+++ b/drivers/i2c/busses/i2c-qup.c
>@@ -310,6 +310,7 @@ static int qup_i2c_wait_ready(struct qup_i2c_dev *qup, int op, bool val,
> 	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 */
>@@ -321,18 +322,28 @@ static int qup_i2c_wait_ready(struct qup_i2c_dev *qup, int op, bool val,
>
> 		if (((opflags & op) >> shift) == val) {
> 			if ((op == QUP_OUT_NOT_EMPTY) && qup->is_last) {
>-				if (!(status & I2C_STATUS_BUS_ACTIVE))
>-					return 0;
>+				if (!(status & I2C_STATUS_BUS_ACTIVE)) {
>+					ret = 0;
>+					goto done;
>+				}
> 			} else {
>-				return 0;
>+				ret = 0;
>+				goto done;
> 			}
> 		}
>
>-		if (time_after(jiffies, timeout))
>-			return -ETIMEDOUT;
>-
>+		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,
>@@ -793,39 +804,35 @@ static int qup_i2c_bam_do_xfer(struct qup_i2c_dev *qup, struct i2c_msg *msg,
> 	}
>
> 	if (ret || qup->bus_err || qup->qup_err) {
>-		if (qup->bus_err & QUP_I2C_NACK_FLAG) {
>-			msg--;
>-			dev_err(qup->dev, "NACK from %x\n", msg->addr);
>-			ret = -EIO;
>+		if (qup_i2c_change_state(qup, QUP_RUN_STATE)) {
>+			dev_err(qup->dev, "change to run state timed out");
>+			goto desc_err;
>+		}
>
>-			if (qup_i2c_change_state(qup, QUP_RUN_STATE)) {
>-				dev_err(qup->dev, "change to run state timed out");
>-				return ret;
>-			}
>+		if (rx_nents)
>+			writel(QUP_BAM_INPUT_EOT,
>+			       qup->base + QUP_OUT_FIFO_BASE);
>
>-			if (rx_nents)
>-				writel(QUP_BAM_INPUT_EOT,
>-				       qup->base + QUP_OUT_FIFO_BASE);
>+		writel(QUP_BAM_FLUSH_STOP, qup->base + QUP_OUT_FIFO_BASE);
>
>-			writel(QUP_BAM_FLUSH_STOP,
>-			       qup->base + QUP_OUT_FIFO_BASE);
>+		qup_i2c_flush(qup);
>
>-			qup_i2c_flush(qup);
>+		/* wait for remaining interrupts to occur */
>+		if (!wait_for_completion_timeout(&qup->xfer, HZ))
>+			dev_err(qup->dev, "flush timed out\n");
>
>-			/* wait for remaining interrupts to occur */
>-			if (!wait_for_completion_timeout(&qup->xfer, HZ))
>-				dev_err(qup->dev, "flush timed out\n");
>+		qup_i2c_rel_dma(qup);
>
>-			qup_i2c_rel_dma(qup);
>-		}
>+		ret =  (qup->bus_err & QUP_I2C_NACK_FLAG) ? -ENXIO : -EIO;
> 	}
>
>+desc_err:
> 	dma_unmap_sg(qup->dev, qup->btx.sg, tx_nents, DMA_TO_DEVICE);
>
> 	if (rx_nents)
> 		dma_unmap_sg(qup->dev, qup->brx.sg, rx_nents,
> 			     DMA_FROM_DEVICE);
>-desc_err:
>+
> 	return ret;
> }
>
>@@ -841,9 +848,6 @@ static int qup_i2c_bam_xfer(struct i2c_adapter *adap, struct i2c_msg *msg,
> 	if (ret)
> 		goto out;
>
>-	qup->bus_err = 0;
>-	qup->qup_err = 0;
>-
> 	writel(0, qup->base + QUP_MX_INPUT_CNT);
> 	writel(0, qup->base + QUP_MX_OUTPUT_CNT);
>
>@@ -881,12 +885,8 @@ static int qup_i2c_wait_for_complete(struct qup_i2c_dev *qup,
> 		ret = -ETIMEDOUT;
> 	}
>
>-	if (qup->bus_err || qup->qup_err) {
>-		if (qup->bus_err & QUP_I2C_NACK_FLAG) {
>-			dev_err(qup->dev, "NACK from %x\n", msg->addr);
>-			ret = -EIO;
>-		}
>-	}
>+	if (qup->bus_err || qup->qup_err)
>+		ret =  (qup->bus_err & QUP_I2C_NACK_FLAG) ? -ENXIO : -EIO;
>
> 	return ret;
> }
>@@ -1178,6 +1178,9 @@ static int qup_i2c_xfer(struct i2c_adapter *adap,
> 	if (ret < 0)
> 		goto out;
>
>+	qup->bus_err = 0;
>+	qup->qup_err = 0;
>+
> 	writel(1, qup->base + QUP_SW_RESET);
> 	ret = qup_i2c_poll_state(qup, QUP_RESET_STATE);
> 	if (ret)
>@@ -1227,6 +1230,9 @@ static int qup_i2c_xfer_v2(struct i2c_adapter *adap,
> 	struct qup_i2c_dev *qup = i2c_get_adapdata(adap);
> 	int ret, len, idx = 0, use_dma = 0;
>
>+	qup->bus_err = 0;
>+	qup->qup_err = 0;
>+
> 	ret = pm_runtime_get_sync(qup->dev);
> 	if (ret < 0)
> 		goto out;
>--
>QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux
>Foundation
>
>
>_______________________________________________
>linux-arm-kernel mailing list
>linux-arm-kernel@lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH V4 1/3] i2c: qup: Fix broken dma when CONFIG_DEBUG_SG is enabled
  2016-06-10 18:08 ` [PATCH V4 1/3] i2c: qup: Fix broken dma when CONFIG_DEBUG_SG is enabled Sricharan R
  2016-06-10 18:14   ` Sricharan
@ 2016-06-18 16:20   ` Wolfram Sang
  1 sibling, 0 replies; 13+ messages in thread
From: Wolfram Sang @ 2016-06-18 16:20 UTC (permalink / raw)
  To: Sricharan R
  Cc: linux-arm-msm, ntelkar, galak, linux-kernel, andy.gross,
	linux-i2c, agross, linux-arm-kernel, nkaje, absahu

[-- Attachment #1: Type: text/plain, Size: 182 bytes --]

On Fri, Jun 10, 2016 at 11:38:19PM +0530, Sricharan R wrote:
> With CONFIG_DEBUG_SG is enabled and when dma mode is used, below dump is seen,
> 

Applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH V4 1/3] i2c: qup: Fix broken dma when CONFIG_DEBUG_SG is enabled
  2016-06-10 18:14   ` Sricharan
@ 2016-06-18 16:20     ` Wolfram Sang
  0 siblings, 0 replies; 13+ messages in thread
From: Wolfram Sang @ 2016-06-18 16:20 UTC (permalink / raw)
  To: Sricharan
  Cc: linux-arm-msm, ntelkar, galak, linux-kernel, andy.gross,
	linux-i2c, agross, linux-arm-kernel, nkaje, absahu

[-- Attachment #1: Type: text/plain, Size: 140 bytes --]

On Fri, Jun 10, 2016 at 11:44:34PM +0530, Sricharan wrote:
> +Wolfram Sang

No need to. Patchwork will catch the patches for the i2c list.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH V4 2/3] i2c: qup: Fix wrong value of index variable
  2016-06-10 18:08 ` [PATCH V4 2/3] i2c: qup: Fix wrong value of index variable Sricharan R
  2016-06-10 18:15   ` Sricharan
@ 2016-06-18 16:21   ` Wolfram Sang
  2016-06-18 16:26     ` Wolfram Sang
  1 sibling, 1 reply; 13+ messages in thread
From: Wolfram Sang @ 2016-06-18 16:21 UTC (permalink / raw)
  To: Sricharan R
  Cc: linux-arm-msm, ntelkar, galak, linux-kernel, andy.gross,
	linux-i2c, agross, linux-arm-kernel, nkaje, absahu

[-- Attachment #1: Type: text/plain, Size: 365 bytes --]

On Fri, Jun 10, 2016 at 11:38:20PM +0530, Sricharan R wrote:
> index gets incremented during check to determine if the
> messages can be transferred with dma. But not reset after
> that, resulting in wrong start value in subsequent loop,
> causing failure. Fix it.
> 
> Signed-off-by: Sricharan R <sricharan@codeaurora.org>

Applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH V4 3/3] i2c: qup: Fix error handling
  2016-06-10 18:08 ` [PATCH V4 3/3] i2c: qup: Fix error handling Sricharan R
  2016-06-10 18:15   ` Sricharan
@ 2016-06-18 16:25   ` Wolfram Sang
  1 sibling, 0 replies; 13+ messages in thread
From: Wolfram Sang @ 2016-06-18 16:25 UTC (permalink / raw)
  To: Sricharan R
  Cc: linux-arm-msm, ntelkar, galak, linux-kernel, andy.gross,
	linux-i2c, agross, linux-arm-kernel, nkaje, absahu

[-- Attachment #1: Type: text/plain, Size: 680 bytes --]

On Fri, Jun 10, 2016 at 11:38:21PM +0530, Sricharan R wrote:
> Among the bus errors reported from the QUP_MASTER_STATUS register
> only NACK is considered and transfer gets suspended, while
> other errors are ignored. Correct this and suspend the transfer
> for other errors as well. This avoids unnecessary 'timeouts' which
> happens when waiting for events that would never happen when there
> is already an error condition on the bus. Also the error handling
> procedure should be the same for both NACK and other bus errors in
> case of dma mode. So correct that as well.
> 
> Signed-off-by: Sricharan R <sricharan@codeaurora.org>

Applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH V4 2/3] i2c: qup: Fix wrong value of index variable
  2016-06-18 16:21   ` Wolfram Sang
@ 2016-06-18 16:26     ` Wolfram Sang
  0 siblings, 0 replies; 13+ messages in thread
From: Wolfram Sang @ 2016-06-18 16:26 UTC (permalink / raw)
  To: Sricharan R
  Cc: linux-arm-msm, ntelkar, galak, linux-kernel, andy.gross,
	linux-i2c, agross, linux-arm-kernel, nkaje, absahu

[-- Attachment #1: Type: text/plain, Size: 492 bytes --]

On Sat, Jun 18, 2016 at 06:21:39PM +0200, Wolfram Sang wrote:
> On Fri, Jun 10, 2016 at 11:38:20PM +0530, Sricharan R wrote:
> > index gets incremented during check to determine if the
> > messages can be transferred with dma. But not reset after
> > that, resulting in wrong start value in subsequent loop,
> > causing failure. Fix it.
> > 
> > Signed-off-by: Sricharan R <sricharan@codeaurora.org>
> 
> Applied to for-next, thanks!

Reconsidered and applied to for-current.



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2016-06-18 16:26 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-10 18:08 [PATCH V4 0/3] i2c: qup: Some misc fixes Sricharan R
2016-06-10 18:08 ` [PATCH V4 1/3] i2c: qup: Fix broken dma when CONFIG_DEBUG_SG is enabled Sricharan R
2016-06-10 18:14   ` Sricharan
2016-06-18 16:20     ` Wolfram Sang
2016-06-18 16:20   ` Wolfram Sang
2016-06-10 18:08 ` [PATCH V4 2/3] i2c: qup: Fix wrong value of index variable Sricharan R
2016-06-10 18:15   ` Sricharan
2016-06-18 16:21   ` Wolfram Sang
2016-06-18 16:26     ` Wolfram Sang
2016-06-10 18:08 ` [PATCH V4 3/3] i2c: qup: Fix error handling Sricharan R
2016-06-10 18:15   ` Sricharan
2016-06-18 16:25   ` Wolfram Sang
2016-06-10 18:13 ` [PATCH V4 0/3] i2c: qup: Some misc fixes Sricharan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).