Received-SPF: pass (google.com: domain of linux-kernel+bounces-95232-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
From: Mukesh Kumar Savaliya <quic_msavaliy@quicinc.com>
To: konrad.dybcio@linaro.org, bjorn.andersson@linaro.org, vkoul@kernel.org,
        andi.shyti@kernel.org, wsa@kernel.org, linux-arm-msm@vger.kernel.org,
        dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-i2c@vger.kernel.org
Cc: quic_vdadhani@quicinc.com,
        Mukesh Kumar Savaliya <quic_msavaliy@quicinc.com>
Subject: [PATCH v2] i2c: i2c-qcom-geni: Parse Error correctly in i2c GSI mode
Date: Thu,  7 Mar 2024 15:06:05 +0530
Message-Id: <20240307093605.4142639-1-quic_msavaliy@quicinc.com>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

We are seeing transfer failure instead of NACK error for simple
device scan test.Ideally it should report exact error like NACK
if device is not present.

We may also expect errors like BUS_PROTO or ARB_LOST, hence we are
adding such error support in GSI mode and reporting it accordingly
by adding respective error logs.

During geni_i2c_gpi_xfer(), we should expect callback param as a
transfer result. For that we have added a new structure named
gpi_i2c_result, which will store xfer result.

Upon receiving an interrupt, gpi_process_xfer_compl_event() will
store transfer result into status variable and then call the
dmaengine_desc_callback_invoke(). Hence i2c_gpi_cb_result() can
parse the respective errors.

while parsing error from the status param, use FIELD_GET with the
mask instead of multiple shifting operations for each error.

Fixes: d8703554f4de ("i2c: qcom-geni: Add support for GPI DMA")
Co-developed-by: Viken Dadhaniya <quic_vdadhani@quicinc.com>
Signed-off-by: Viken Dadhaniya <quic_vdadhani@quicinc.com>
Signed-off-by: Mukesh Kumar Savaliya <quic_msavaliy@quicinc.com>
---
---
- Commit log changed we->We.
- Explained the problem that we are not detecing NACK error.
- Removed Heap based memory allocation and hence memory leakage issue.
- Used FIELD_GET and removed shiting and masking every time as suggested by Bjorn.
- Changed commit log to reflect the code changes done.
- Removed adding anything into struct gpi_i2c_config and created new structure
  for error status as suggested by Bjorn.
---
 drivers/dma/qcom/gpi.c             | 12 +++++++++++-
 drivers/i2c/busses/i2c-qcom-geni.c | 19 +++++++++++++++----
 include/linux/dma/qcom-gpi-dma.h   | 10 ++++++++++
 3 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
index 1c93864e0e4d..e3508d51fdc9 100644
--- a/drivers/dma/qcom/gpi.c
+++ b/drivers/dma/qcom/gpi.c
@@ -1076,7 +1076,17 @@ static void gpi_process_xfer_compl_event(struct gchan *gchan,
 	dev_dbg(gpii->gpi_dev->dev, "Residue %d\n", result.residue);
 
 	dma_cookie_complete(&vd->tx);
-	dmaengine_desc_get_callback_invoke(&vd->tx, &result);
+	if (gchan->protocol == QCOM_GPI_I2C) {
+		struct dmaengine_desc_callback cb;
+		struct gpi_i2c_result *i2c;
+
+		dmaengine_desc_get_callback(&vd->tx, &cb);
+		i2c = cb.callback_param;
+		i2c->status = compl_event->status;
+		dmaengine_desc_callback_invoke(&cb, &result);
+	} else {
+		dmaengine_desc_get_callback_invoke(&vd->tx, &result);
+	}
 
 gpi_free_desc:
 	spin_lock_irqsave(&gchan->vc.lock, flags);
diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
index da94df466e83..36a7c0c0ff54 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -66,6 +66,7 @@ enum geni_i2c_err_code {
 	GENI_TIMEOUT,
 };
 
+#define I2C_DMA_TX_IRQ_MASK	GENMASK(12, 5)
 #define DM_I2C_CB_ERR		((BIT(NACK) | BIT(BUS_PROTO) | BIT(ARB_LOST)) \
 									<< 5)
 
@@ -99,6 +100,7 @@ struct geni_i2c_dev {
 	struct dma_chan *rx_c;
 	bool gpi_mode;
 	bool abort_done;
+	struct gpi_i2c_result i2c_result;
 };
 
 struct geni_i2c_desc {
@@ -484,9 +486,18 @@ static int geni_i2c_tx_one_msg(struct geni_i2c_dev *gi2c, struct i2c_msg *msg,
 
 static void i2c_gpi_cb_result(void *cb, const struct dmaengine_result *result)
 {
-	struct geni_i2c_dev *gi2c = cb;
-
-	if (result->result != DMA_TRANS_NOERROR) {
+	struct gpi_i2c_result *i2c_res = cb;
+	struct geni_i2c_dev *gi2c = container_of(i2c_res, struct geni_i2c_dev, i2c_result);
+	u32 status;
+
+	status = FIELD_GET(I2C_DMA_TX_IRQ_MASK, i2c_res->status);
+	if (status == BIT(NACK)) {
+		geni_i2c_err(gi2c, NACK);
+	} else if (status == BIT(BUS_PROTO)) {
+		geni_i2c_err(gi2c, BUS_PROTO);
+	} else if (status == BIT(ARB_LOST)) {
+		geni_i2c_err(gi2c, ARB_LOST);
+	} else if (result->result != DMA_TRANS_NOERROR) {
 		dev_err(gi2c->se.dev, "DMA txn failed:%d\n", result->result);
 		gi2c->err = -EIO;
 	} else if (result->residue) {
@@ -568,7 +579,7 @@ static int geni_i2c_gpi(struct geni_i2c_dev *gi2c, struct i2c_msg *msg,
 	}
 
 	desc->callback_result = i2c_gpi_cb_result;
-	desc->callback_param = gi2c;
+	desc->callback_param = &gi2c->i2c_result;
 
 	dmaengine_submit(desc);
 	*buf = dma_buf;
diff --git a/include/linux/dma/qcom-gpi-dma.h b/include/linux/dma/qcom-gpi-dma.h
index 6680dd1a43c6..f585c6a35e51 100644
--- a/include/linux/dma/qcom-gpi-dma.h
+++ b/include/linux/dma/qcom-gpi-dma.h
@@ -80,4 +80,14 @@ struct gpi_i2c_config {
 	bool multi_msg;
 };
 
+/**
+ * struct gpi_i2c_result - i2c transfer status result in GSI mode
+ *
+ * @status: store txfer status value as part of callback
+ *
+ */
+struct gpi_i2c_result {
+	u32 status;
+};
+
 #endif /* QCOM_GPI_DMA_H */
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project