All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Sricharan" <sricharan@codeaurora.org>
To: 'Wolfram Sang' <wsa@the-dreams.de>
Cc: devicetree@vger.kernel.org, architt@codeaurora.org,
	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,
	iivanov@mm-sol.com, agross@codeaurora.org,
	dmaengine@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: RE: [RESEND PATCH V7 4/6] i2c: qup: Add bam dma capabilities
Date: Mon, 21 Mar 2016 19:31:27 +0530	[thread overview]
Message-ID: <004901d1837a$2df9f620$89ede260$@codeaurora.org> (raw)
In-Reply-To: <20160224101949.GB1391@katana>

Hi wolfram,

> On Mon, Feb 22, 2016 at 05:38:15PM +0530, Sricharan R wrote:
> > QUP cores can be attached to a BAM module, which acts as a dma engine
> > for the QUP core. When DMA with BAM is enabled, the BAM consumer
> pipe
> > transmitted data is written to the output FIFO and the BAM producer
> > pipe received data is read from the input FIFO.
> >
> > With BAM capabilities, qup-i2c core can transfer more than
> > 256 bytes, without a 'stop' which is not possible otherwise.
> >
> > Signed-off-by: Sricharan R <sricharan@codeaurora.org>
> > Reviewed-by: Andy Gross <andy.gross@linaro.org>
> > Tested-by: Archit Taneja <architt@codeaurora.org>
> > Tested-by: Telkar Nagender <ntelkar@codeaurora.org>
> 
> Applied to for-next, thanks!

There is a bug with only this patch which comes out when CONFIG_DEBUG_SG
option gets
enabled.  I have patch like below to fix it, but wanted to ask you if it
would correct to have this in the
original patch or should I sent a separate fix ? Sorry for the mess on this.
Not sure if the pull request is sent
already.

diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
index 23eaabb..a1fae0c 100644
--- a/drivers/i2c/busses/i2c-qup.c
+++ b/drivers/i2c/busses/i2c-qup.c
@@ -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;
 }
 
@@ -1268,6 +1265,8 @@ static int qup_i2c_xfer_v2(struct i2c_adapter *adap,
 		}
 	}
 
+	idx = 0;
+
 	do {
 		if (msgs[idx].len == 0) {
 			ret = -EINVAL;
@@ -1407,27 +1406,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, size,
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, size,
GFP_KERNEL);
 		if (!qup->btx.tag.start) {
 			ret = -ENOMEM;
 			goto fail_dma;
@@ -1566,13 +1559,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);
 	}
--

Regards,
 Sricharan

WARNING: multiple messages have this Message-ID (diff)
From: "Sricharan" <sricharan@codeaurora.org>
To: "'Wolfram Sang'" <wsa@the-dreams.de>
Cc: <devicetree@vger.kernel.org>, <architt@codeaurora.org>,
	<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>,
	<iivanov@mm-sol.com>, <agross@codeaurora.org>,
	<dmaengine@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>
Subject: RE: [RESEND PATCH V7 4/6] i2c: qup: Add bam dma capabilities
Date: Mon, 21 Mar 2016 19:31:27 +0530	[thread overview]
Message-ID: <004901d1837a$2df9f620$89ede260$@codeaurora.org> (raw)
In-Reply-To: <20160224101949.GB1391@katana>

Hi wolfram,

> On Mon, Feb 22, 2016 at 05:38:15PM +0530, Sricharan R wrote:
> > QUP cores can be attached to a BAM module, which acts as a dma engine
> > for the QUP core. When DMA with BAM is enabled, the BAM consumer
> pipe
> > transmitted data is written to the output FIFO and the BAM producer
> > pipe received data is read from the input FIFO.
> >
> > With BAM capabilities, qup-i2c core can transfer more than
> > 256 bytes, without a 'stop' which is not possible otherwise.
> >
> > Signed-off-by: Sricharan R <sricharan@codeaurora.org>
> > Reviewed-by: Andy Gross <andy.gross@linaro.org>
> > Tested-by: Archit Taneja <architt@codeaurora.org>
> > Tested-by: Telkar Nagender <ntelkar@codeaurora.org>
> 
> Applied to for-next, thanks!

There is a bug with only this patch which comes out when CONFIG_DEBUG_SG
option gets
enabled.  I have patch like below to fix it, but wanted to ask you if it
would correct to have this in the
original patch or should I sent a separate fix ? Sorry for the mess on this.
Not sure if the pull request is sent
already.

diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
index 23eaabb..a1fae0c 100644
--- a/drivers/i2c/busses/i2c-qup.c
+++ b/drivers/i2c/busses/i2c-qup.c
@@ -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;
 }
 
@@ -1268,6 +1265,8 @@ static int qup_i2c_xfer_v2(struct i2c_adapter *adap,
 		}
 	}
 
+	idx = 0;
+
 	do {
 		if (msgs[idx].len == 0) {
 			ret = -EINVAL;
@@ -1407,27 +1406,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, size,
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, size,
GFP_KERNEL);
 		if (!qup->btx.tag.start) {
 			ret = -ENOMEM;
 			goto fail_dma;
@@ -1566,13 +1559,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);
 	}
--

Regards,
 Sricharan

WARNING: multiple messages have this Message-ID (diff)
From: sricharan@codeaurora.org (Sricharan)
To: linux-arm-kernel@lists.infradead.org
Subject: [RESEND PATCH V7 4/6] i2c: qup: Add bam dma capabilities
Date: Mon, 21 Mar 2016 19:31:27 +0530	[thread overview]
Message-ID: <004901d1837a$2df9f620$89ede260$@codeaurora.org> (raw)
In-Reply-To: <20160224101949.GB1391@katana>

Hi wolfram,

> On Mon, Feb 22, 2016 at 05:38:15PM +0530, Sricharan R wrote:
> > QUP cores can be attached to a BAM module, which acts as a dma engine
> > for the QUP core. When DMA with BAM is enabled, the BAM consumer
> pipe
> > transmitted data is written to the output FIFO and the BAM producer
> > pipe received data is read from the input FIFO.
> >
> > With BAM capabilities, qup-i2c core can transfer more than
> > 256 bytes, without a 'stop' which is not possible otherwise.
> >
> > Signed-off-by: Sricharan R <sricharan@codeaurora.org>
> > Reviewed-by: Andy Gross <andy.gross@linaro.org>
> > Tested-by: Archit Taneja <architt@codeaurora.org>
> > Tested-by: Telkar Nagender <ntelkar@codeaurora.org>
> 
> Applied to for-next, thanks!

There is a bug with only this patch which comes out when CONFIG_DEBUG_SG
option gets
enabled.  I have patch like below to fix it, but wanted to ask you if it
would correct to have this in the
original patch or should I sent a separate fix ? Sorry for the mess on this.
Not sure if the pull request is sent
already.

diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
index 23eaabb..a1fae0c 100644
--- a/drivers/i2c/busses/i2c-qup.c
+++ b/drivers/i2c/busses/i2c-qup.c
@@ -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;
 }
 
@@ -1268,6 +1265,8 @@ static int qup_i2c_xfer_v2(struct i2c_adapter *adap,
 		}
 	}
 
+	idx = 0;
+
 	do {
 		if (msgs[idx].len == 0) {
 			ret = -EINVAL;
@@ -1407,27 +1406,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, size,
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, size,
GFP_KERNEL);
 		if (!qup->btx.tag.start) {
 			ret = -ENOMEM;
 			goto fail_dma;
@@ -1566,13 +1559,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);
 	}
--

Regards,
 Sricharan

  reply	other threads:[~2016-03-21 14:01 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-22 12:08 [RESEND PATCH V7 4/6] i2c: qup: Add bam dma capabilities Sricharan R
2016-02-22 12:08 ` Sricharan R
2016-02-24 10:19 ` Wolfram Sang
2016-02-24 10:19   ` Wolfram Sang
2016-03-21 14:01   ` Sricharan [this message]
2016-03-21 14:01     ` Sricharan
2016-03-21 14:01     ` Sricharan
2016-03-21 16:44     ` Wolfram Sang
2016-03-21 16:44       ` Wolfram Sang

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='004901d1837a$2df9f620$89ede260$@codeaurora.org' \
    --to=sricharan@codeaurora.org \
    --cc=agross@codeaurora.org \
    --cc=andy.gross@linaro.org \
    --cc=architt@codeaurora.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dmaengine@vger.kernel.org \
    --cc=galak@codeaurora.org \
    --cc=iivanov@mm-sol.com \
    --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=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.