All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sricharan R <sricharan@codeaurora.org>
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, wsa@the-dreams.de, absahu@codeaurora.org
Cc: sricharan@codeaurora.org
Subject: [PATCH V3 2/2] i2c: qup: Fix error handling
Date: Wed, 25 May 2016 13:46:52 +0530	[thread overview]
Message-ID: <1464164212-11377-3-git-send-email-sricharan@codeaurora.org> (raw)
In-Reply-To: <1464164212-11377-1-git-send-email-sricharan@codeaurora.org>

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>
---
[V3] Change the return error code for NACK and other bus errors.
     Added the error handling procedure for other bus errors in
     dma mode. Removed the dev_err print in case of NACK.

 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 8620e99..edced86 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

WARNING: multiple messages have this Message-ID (diff)
From: sricharan@codeaurora.org (Sricharan R)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH V3 2/2] i2c: qup: Fix error handling
Date: Wed, 25 May 2016 13:46:52 +0530	[thread overview]
Message-ID: <1464164212-11377-3-git-send-email-sricharan@codeaurora.org> (raw)
In-Reply-To: <1464164212-11377-1-git-send-email-sricharan@codeaurora.org>

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>
---
[V3] Change the return error code for NACK and other bus errors.
     Added the error handling procedure for other bus errors in
     dma mode. Removed the dev_err print in case of NACK.

 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 8620e99..edced86 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

  parent reply	other threads:[~2016-05-25  8:16 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-25  8:16 [PATCH V3 0/2] i2c: qup: Some misc fixes Sricharan R
2016-05-25  8:16 ` Sricharan R
2016-05-25  8:16 ` [PATCH V3 1/2] i2c: qup: Fix broken dma when CONFIG_DEBUG_SG is enabled Sricharan R
2016-05-25  8:16   ` Sricharan R
2016-05-26 18:48   ` Wolfram Sang
2016-05-26 18:48     ` Wolfram Sang
2016-05-26 22:01     ` Sricharan
2016-05-26 22:01       ` Sricharan
2016-05-26 22:01       ` Sricharan
2016-06-09 20:46       ` Wolfram Sang
2016-06-09 20:46         ` Wolfram Sang
2016-05-25  8:16 ` Sricharan R [this message]
2016-05-25  8:16   ` [PATCH V3 2/2] i2c: qup: Fix error handling Sricharan R

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1464164212-11377-3-git-send-email-sricharan@codeaurora.org \
    --to=sricharan@codeaurora.org \
    --cc=absahu@codeaurora.org \
    --cc=agross@codeaurora.org \
    --cc=andy.gross@linaro.org \
    --cc=galak@codeaurora.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nkaje@codeaurora.org \
    --cc=ntelkar@codeaurora.org \
    --cc=wsa@the-dreams.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.