Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1375613 > unrolled thread

[PATCH v3 0/6] bam dma fixes and one dt extension

Started byStanimir Varbanov <stanimir.varbanov@linaro.org>
First post2016-04-11 10:40 +0200
Last post2016-04-11 10:50 +0200
Articles 7 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v3 0/6] bam dma fixes and one dt extension Stanimir Varbanov <stanimir.varbanov@linaro.org> - 2016-04-11 10:40 +0200
    [PATCH v3 2/6] dmaengine: qcom: bam_dma: clear BAM interrupt only if it is raised Stanimir Varbanov <stanimir.varbanov@linaro.org> - 2016-04-11 10:40 +0200
    [PATCH v3 3/6] dmaengine: qcom: bam_dma: document controlled-remotely dt property Stanimir Varbanov <stanimir.varbanov@linaro.org> - 2016-04-11 10:40 +0200
    [PATCH v3 6/6] dmaengine: qcom: bam_dma: rename BAM_MAX_DATA_SIZE define Stanimir Varbanov <stanimir.varbanov@linaro.org> - 2016-04-11 10:50 +0200
      Re: [PATCH v3 6/6] dmaengine: qcom: bam_dma: rename  BAM_MAX_DATA_SIZE define Andy Gross <andy.gross@linaro.org> - 2016-04-13 00:10 +0200
    [PATCH v3 5/6] dmaengine: qcom: bam_dma: use correct pipe FIFO size Stanimir Varbanov <stanimir.varbanov@linaro.org> - 2016-04-11 10:50 +0200
    [PATCH v3 4/6] dmaengine: qcom: bam_dma: add controlled-remotely dt property Stanimir Varbanov <stanimir.varbanov@linaro.org> - 2016-04-11 10:50 +0200

#1375613 — [PATCH v3 0/6] bam dma fixes and one dt extension

FromStanimir Varbanov <stanimir.varbanov@linaro.org>
Date2016-04-11 10:40 +0200
Subject[PATCH v3 0/6] bam dma fixes and one dt extension
Message-ID<rmFy2-2Qu-3@gated-at.bofh.it>
Hi,

Here is version 3 with following changes:
 - fixed a typo in 2/6.
 - the patch which adds dt property is seperated in two, one
   which adding binding document and another one which modifing
   the driver - requested by Vinod.
 - fixed a typo in 5/6.
 - added collected Acked,Tested-by tags.

The v2 can be found at:
https://lkml.org/lkml/2016/4/5/922

regards,
Stan

Stanimir Varbanov (6):
  dmaengine: qcom: bam_dma: fix dma free memory on remove
  dmaengine: qcom: bam_dma: clear BAM interrupt only if it is raised
  dmaengine: qcom: bam_dma: document controlled-remotely dt property
  dmaengine: qcom: bam_dma: add controlled-remotely dt property
  dmaengine: qcom: bam_dma: use correct pipe FIFO size
  dmaengine: qcom: bam_dma: rename BAM_MAX_DATA_SIZE define

 .../devicetree/bindings/dma/qcom_bam_dma.txt       |    2 ++
 drivers/dma/qcom/bam_dma.c                         |   38 +++++++++++++-------
 2 files changed, 28 insertions(+), 12 deletions(-)

-- 
1.7.9.5

[toc] | [next] | [standalone]


#1375615 — [PATCH v3 2/6] dmaengine: qcom: bam_dma: clear BAM interrupt only if it is raised

FromStanimir Varbanov <stanimir.varbanov@linaro.org>
Date2016-04-11 10:40 +0200
Subject[PATCH v3 2/6] dmaengine: qcom: bam_dma: clear BAM interrupt only if it is raised
Message-ID<rmFy3-2Qu-21@gated-at.bofh.it>
In reply to#1375613
Currently we write BAM_IRQ_CLR register with zero even when no
BAM_IRQ occured. This write has some bad side effects when the
BAM instance is for the crypto engine. In case of crypto engine
some of the BAM registers are xPU protected and they cannot be
controlled by the driver.

Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
Reviewed-by: Andy Gross <andy.gross@linaro.org>
Tested-by: Pramod Gurav <gpramod@codeaurora.org>
---
 drivers/dma/qcom/bam_dma.c |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
index a486bc0f82e0..789d5f836bf7 100644
--- a/drivers/dma/qcom/bam_dma.c
+++ b/drivers/dma/qcom/bam_dma.c
@@ -801,13 +801,17 @@ static irqreturn_t bam_dma_irq(int irq, void *data)
 	if (srcs & P_IRQ)
 		tasklet_schedule(&bdev->task);
 
-	if (srcs & BAM_IRQ)
+	if (srcs & BAM_IRQ) {
 		clr_mask = readl_relaxed(bam_addr(bdev, 0, BAM_IRQ_STTS));
 
-	/* don't allow reorder of the various accesses to the BAM registers */
-	mb();
+		/*
+		 * don't allow reorder of the various accesses to the BAM
+		 * registers
+		 */
+		mb();
 
-	writel_relaxed(clr_mask, bam_addr(bdev, 0, BAM_IRQ_CLR));
+		writel_relaxed(clr_mask, bam_addr(bdev, 0, BAM_IRQ_CLR));
+	}
 
 	return IRQ_HANDLED;
 }
-- 
1.7.9.5

[toc] | [prev] | [next] | [standalone]


#1375618 — [PATCH v3 3/6] dmaengine: qcom: bam_dma: document controlled-remotely dt property

FromStanimir Varbanov <stanimir.varbanov@linaro.org>
Date2016-04-11 10:40 +0200
Subject[PATCH v3 3/6] dmaengine: qcom: bam_dma: document controlled-remotely dt property
Message-ID<rmFy3-2Qu-31@gated-at.bofh.it>
In reply to#1375613
Extend BAM dt bindings with controlled-remotely property. The
property will be needed to handle cases where we need to skip
register writes to initialise BAM hardware block.

Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
Reviewed-by: Andy Gross <andy.gross@linaro.org>
Acked-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/dma/qcom_bam_dma.txt       |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt b/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
index 1c9d48ea4914..9cbf5d9df8fd 100644
--- a/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
+++ b/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
@@ -13,6 +13,8 @@ Required properties:
 - clock-names: must contain "bam_clk" entry
 - qcom,ee : indicates the active Execution Environment identifier (0-7) used in
   the secure world.
+- qcom,controlled-remotely : optional, indicates that the bam is controlled by
+  remote proccessor i.e. execution environment.
 
 Example:
 
-- 
1.7.9.5

[toc] | [prev] | [next] | [standalone]


#1375621 — [PATCH v3 6/6] dmaengine: qcom: bam_dma: rename BAM_MAX_DATA_SIZE define

FromStanimir Varbanov <stanimir.varbanov@linaro.org>
Date2016-04-11 10:50 +0200
Subject[PATCH v3 6/6] dmaengine: qcom: bam_dma: rename BAM_MAX_DATA_SIZE define
Message-ID<rmFHI-2Ui-3@gated-at.bofh.it>
In reply to#1375613
It seems that the define has not been with acurate name and
makes confusion while reading the code. The more acurate
name should be BAM_FIFO_SIZE.

Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
---
 drivers/dma/qcom/bam_dma.c |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
index 7e5ad1c25e21..969b48176745 100644
--- a/drivers/dma/qcom/bam_dma.c
+++ b/drivers/dma/qcom/bam_dma.c
@@ -342,7 +342,7 @@ static const struct reg_offset_data bam_v1_7_reg_info[] = {
 
 #define BAM_DESC_FIFO_SIZE	SZ_32K
 #define MAX_DESCRIPTORS (BAM_DESC_FIFO_SIZE / sizeof(struct bam_desc_hw) - 1)
-#define BAM_MAX_DATA_SIZE	(SZ_32K - 8)
+#define BAM_FIFO_SIZE	(SZ_32K - 8)
 
 struct bam_chan {
 	struct virt_dma_chan vc;
@@ -459,7 +459,7 @@ static void bam_chan_init_hw(struct bam_chan *bchan,
 	 */
 	writel_relaxed(ALIGN(bchan->fifo_phys, sizeof(struct bam_desc_hw)),
 			bam_addr(bdev, bchan->id, BAM_P_DESC_FIFO_ADDR));
-	writel_relaxed(BAM_MAX_DATA_SIZE,
+	writel_relaxed(BAM_FIFO_SIZE,
 			bam_addr(bdev, bchan->id, BAM_P_FIFO_SIZES));
 
 	/* enable the per pipe interrupts, enable EOT, ERR, and INT irqs */
@@ -605,7 +605,7 @@ static struct dma_async_tx_descriptor *bam_prep_slave_sg(struct dma_chan *chan,
 
 	/* calculate number of required entries */
 	for_each_sg(sgl, sg, sg_len, i)
-		num_alloc += DIV_ROUND_UP(sg_dma_len(sg), BAM_MAX_DATA_SIZE);
+		num_alloc += DIV_ROUND_UP(sg_dma_len(sg), BAM_FIFO_SIZE);
 
 	/* allocate enough room to accomodate the number of entries */
 	async_desc = kzalloc(sizeof(*async_desc) +
@@ -636,10 +636,10 @@ static struct dma_async_tx_descriptor *bam_prep_slave_sg(struct dma_chan *chan,
 			desc->addr = cpu_to_le32(sg_dma_address(sg) +
 						 curr_offset);
 
-			if (remainder > BAM_MAX_DATA_SIZE) {
-				desc->size = cpu_to_le16(BAM_MAX_DATA_SIZE);
-				remainder -= BAM_MAX_DATA_SIZE;
-				curr_offset += BAM_MAX_DATA_SIZE;
+			if (remainder > BAM_FIFO_SIZE) {
+				desc->size = cpu_to_le16(BAM_FIFO_SIZE);
+				remainder -= BAM_FIFO_SIZE;
+				curr_offset += BAM_FIFO_SIZE;
 			} else {
 				desc->size = cpu_to_le16(remainder);
 				remainder = 0;
@@ -1174,7 +1174,7 @@ static int bam_dma_probe(struct platform_device *pdev)
 	/* set max dma segment size */
 	bdev->common.dev = bdev->dev;
 	bdev->common.dev->dma_parms = &bdev->dma_parms;
-	ret = dma_set_max_seg_size(bdev->common.dev, BAM_MAX_DATA_SIZE);
+	ret = dma_set_max_seg_size(bdev->common.dev, BAM_FIFO_SIZE);
 	if (ret) {
 		dev_err(bdev->dev, "cannot set maximum segment size\n");
 		goto err_bam_channel_exit;
-- 
1.7.9.5

[toc] | [prev] | [next] | [standalone]


#1377350 — Re: [PATCH v3 6/6] dmaengine: qcom: bam_dma: rename BAM_MAX_DATA_SIZE define

FromAndy Gross <andy.gross@linaro.org>
Date2016-04-13 00:10 +0200
SubjectRe: [PATCH v3 6/6] dmaengine: qcom: bam_dma: rename BAM_MAX_DATA_SIZE define
Message-ID<rneFs-6vF-15@gated-at.bofh.it>
In reply to#1375621
On Mon, Apr 11, 2016 at 11:38:43AM +0300, Stanimir Varbanov wrote:
> It seems that the define has not been with acurate name and
> makes confusion while reading the code. The more acurate
> name should be BAM_FIFO_SIZE.
> 
> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>

Reviewed-by: Andy Gross <andy.gross@linaro.org>

[toc] | [prev] | [next] | [standalone]


#1375626 — [PATCH v3 5/6] dmaengine: qcom: bam_dma: use correct pipe FIFO size

FromStanimir Varbanov <stanimir.varbanov@linaro.org>
Date2016-04-11 10:50 +0200
Subject[PATCH v3 5/6] dmaengine: qcom: bam_dma: use correct pipe FIFO size
Message-ID<rmFHJ-2Ui-25@gated-at.bofh.it>
In reply to#1375613
The pipe fifo size register must instruct the bam hw
how many hw descriptors can be pushed to fifo. Currently
we instruct the hw with 32KBytes but wrap the tail in
bam_start_dma in BAM_P_EVNT_REG on 4095 i.e. 32760. This
leads to stalled transactions when the tail wraps.

Fix this by use the correct fifo size in BAM_P_FIFO_SIZES
register i.e. 32K - 8.

Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
---
 drivers/dma/qcom/bam_dma.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
index d0f878a78fae..7e5ad1c25e21 100644
--- a/drivers/dma/qcom/bam_dma.c
+++ b/drivers/dma/qcom/bam_dma.c
@@ -459,7 +459,7 @@ static void bam_chan_init_hw(struct bam_chan *bchan,
 	 */
 	writel_relaxed(ALIGN(bchan->fifo_phys, sizeof(struct bam_desc_hw)),
 			bam_addr(bdev, bchan->id, BAM_P_DESC_FIFO_ADDR));
-	writel_relaxed(BAM_DESC_FIFO_SIZE,
+	writel_relaxed(BAM_MAX_DATA_SIZE,
 			bam_addr(bdev, bchan->id, BAM_P_FIFO_SIZES));
 
 	/* enable the per pipe interrupts, enable EOT, ERR, and INT irqs */
-- 
1.7.9.5

[toc] | [prev] | [next] | [standalone]


#1375630 — [PATCH v3 4/6] dmaengine: qcom: bam_dma: add controlled-remotely dt property

FromStanimir Varbanov <stanimir.varbanov@linaro.org>
Date2016-04-11 10:50 +0200
Subject[PATCH v3 4/6] dmaengine: qcom: bam_dma: add controlled-remotely dt property
Message-ID<rmFHJ-2Ui-37@gated-at.bofh.it>
In reply to#1375613
Some of the peripherals has bam which is controlled by remote
processor, thus the bam dma driver must avoid register writes
which initialise bam hw block. Those registers are protected
from xPU block and any writes to them will lead to secure
violation and system reboot.

Adding the contolled_remotely flag in bam driver to avoid
not permitted register writes in bam_init function.

Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
Reviewed-by: Andy Gross <andy.gross@linaro.org>
Tested-by: Pramod Gurav <gpramod@codeaurora.org>
---
 drivers/dma/qcom/bam_dma.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
index 789d5f836bf7..d0f878a78fae 100644
--- a/drivers/dma/qcom/bam_dma.c
+++ b/drivers/dma/qcom/bam_dma.c
@@ -387,6 +387,7 @@ struct bam_device {
 
 	/* execution environment ID, from DT */
 	u32 ee;
+	bool controlled_remotely;
 
 	const struct reg_offset_data *layout;
 
@@ -1042,6 +1043,9 @@ static int bam_init(struct bam_device *bdev)
 	val = readl_relaxed(bam_addr(bdev, 0, BAM_NUM_PIPES));
 	bdev->num_channels = val & BAM_NUM_PIPES_MASK;
 
+	if (bdev->controlled_remotely)
+		return 0;
+
 	/* s/w reset bam */
 	/* after reset all pipes are disabled and idle */
 	val = readl_relaxed(bam_addr(bdev, 0, BAM_CTRL));
@@ -1129,6 +1133,9 @@ static int bam_dma_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	bdev->controlled_remotely = of_property_read_bool(pdev->dev.of_node,
+						"qcom,controlled-remotely");
+
 	bdev->bamclk = devm_clk_get(bdev->dev, "bam_clk");
 	if (IS_ERR(bdev->bamclk))
 		return PTR_ERR(bdev->bamclk);
-- 
1.7.9.5

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web