devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] dmaengine: qcom: bam_dma: fixes for remotely controlled bam
@ 2018-01-16 19:02 srinivas.kandagatla
  2018-01-16 19:02 ` [PATCH 1/4] dmaengine: qcom: bam_dma: make bam clk optional srinivas.kandagatla
                   ` (3 more replies)
  0 siblings, 4 replies; 19+ messages in thread
From: srinivas.kandagatla @ 2018-01-16 19:02 UTC (permalink / raw)
  To: Vinod Koul, Andy Gross, dmaengine
  Cc: Rob Herring, Mark Rutland, David Brown, Dan Williams, devicetree,
	linux-kernel, linux-arm-msm, linux-soc, yanhe, ramkri, sdharia,
	Srinivas Kandagatla

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Hi Andy,

I did hit few issues while trying out SLIMBus BAM on DB820c, this BAM instance
is remotely controlled and powered up after ADSP is booted using QMI commands.

Firstly some of the master registers are written even when the BAM is remotely
controlled, and secondly reading registers when bam is not ready yet.

These 4 patches address these issues, there are few more issues like doing PM
in simillar usecase, these will be addressed soon.

Thanks,
Srini

Srinivas Kandagatla (4):
  dmaengine: qcom: bam_dma: make bam clk optional
  dmaengine: qcom: bam_dma: add num-channels binding for remotely
    controlled
  dmaengine: qcom: bam_dma: do not write to global regs in remote mode
  dmaengine: qcom: bam_dma: Add num-ees dt binding for remotely
    controlled

 .../devicetree/bindings/dma/qcom_bam_dma.txt       |  4 ++
 drivers/dma/qcom/bam_dma.c                         | 56 +++++++++++++++-------
 2 files changed, 43 insertions(+), 17 deletions(-)

-- 
2.15.1

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

* [PATCH 1/4] dmaengine: qcom: bam_dma: make bam clk optional
  2018-01-16 19:02 [PATCH 0/4] dmaengine: qcom: bam_dma: fixes for remotely controlled bam srinivas.kandagatla
@ 2018-01-16 19:02 ` srinivas.kandagatla
  2018-01-16 19:38   ` Sagar Dharia
  2018-01-19  5:52   ` Vinod Koul
  2018-01-16 19:02 ` [PATCH 3/4] dmaengine: qcom: bam_dma: do not write to global regs in remote mode srinivas.kandagatla
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 19+ messages in thread
From: srinivas.kandagatla @ 2018-01-16 19:02 UTC (permalink / raw)
  To: Vinod Koul, Andy Gross, dmaengine
  Cc: Rob Herring, Mark Rutland, David Brown, Dan Williams, devicetree,
	linux-kernel, linux-arm-msm, linux-soc, yanhe, ramkri, sdharia,
	Srinivas Kandagatla

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

When BAM is remotely controlled it does not sound correct to control
its clk on Linux side. Make it optional, so that its not madatory
for remote controlled BAM instances.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/dma/qcom/bam_dma.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
index 03c4eb3fd314..78e488e8f96d 100644
--- a/drivers/dma/qcom/bam_dma.c
+++ b/drivers/dma/qcom/bam_dma.c
@@ -1180,13 +1180,14 @@ static int bam_dma_probe(struct platform_device *pdev)
 						"qcom,controlled-remotely");
 
 	bdev->bamclk = devm_clk_get(bdev->dev, "bam_clk");
-	if (IS_ERR(bdev->bamclk))
-		return PTR_ERR(bdev->bamclk);
-
-	ret = clk_prepare_enable(bdev->bamclk);
-	if (ret) {
-		dev_err(bdev->dev, "failed to prepare/enable clock\n");
-		return ret;
+	if (IS_ERR(bdev->bamclk)) {
+		bdev->bamclk = NULL;
+	} else {
+		ret = clk_prepare_enable(bdev->bamclk);
+		if (ret) {
+			dev_err(bdev->dev, "failed to prepare/enable clock\n");
+			return ret;
+		}
 	}
 
 	ret = bam_init(bdev);
-- 
2.15.1

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

* [PATCH 2/4] dmaengine: qcom: bam_dma: add num-channels binding for remotely controlled
       [not found] ` <20180116190236.14558-1-srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
@ 2018-01-16 19:02   ` srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A
  2018-01-19  5:55     ` Vinod Koul
       [not found]     ` <20180116190236.14558-3-srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
  2018-01-17 10:18   ` [PATCH 0/4] dmaengine: qcom: bam_dma: fixes for remotely controlled bam Vinod Koul
  1 sibling, 2 replies; 19+ messages in thread
From: srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A @ 2018-01-16 19:02 UTC (permalink / raw)
  To: Vinod Koul, Andy Gross, dmaengine-u79uwXL29TY76Z2rM5mHXA
  Cc: Rob Herring, Mark Rutland, David Brown, Dan Williams,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	linux-soc-u79uwXL29TY76Z2rM5mHXA, yanhe-jfJNa2p1gH1BDgjK7y7TUQ,
	ramkri-Rm6X0d1/PG5y9aJCnZT0Uw, sdharia-jfJNa2p1gH1BDgjK7y7TUQ,
	Srinivas Kandagatla

From: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

When Linux is master of BAM, it can directly read registers to know number
of supported channels, however when its remotely controlled reading these
registers would trigger a crash if the BAM is not yet intialized/powered up
on the remote side.

This patch adds num-channels binding to specify number of supported
dma channels on remotely controlled BAM.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 Documentation/devicetree/bindings/dma/qcom_bam_dma.txt |  2 ++
 drivers/dma/qcom/bam_dma.c                             | 13 +++++++++++--
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt b/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
index 9cbf5d9df8fd..aa6822cbb230 100644
--- a/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
+++ b/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
@@ -15,6 +15,8 @@ Required properties:
   the secure world.
 - qcom,controlled-remotely : optional, indicates that the bam is controlled by
   remote proccessor i.e. execution environment.
+- num-channels : optional, indicates supported number of DMA channels in a
+  remotely controlled bam.
 
 Example:
 
diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
index 78e488e8f96d..523bd178047a 100644
--- a/drivers/dma/qcom/bam_dma.c
+++ b/drivers/dma/qcom/bam_dma.c
@@ -1083,8 +1083,10 @@ static int bam_init(struct bam_device *bdev)
 	if (bdev->ee >= val)
 		return -EINVAL;
 
-	val = readl_relaxed(bam_addr(bdev, 0, BAM_NUM_PIPES));
-	bdev->num_channels = val & BAM_NUM_PIPES_MASK;
+	if (!bdev->num_channels) {
+		val = readl_relaxed(bam_addr(bdev, 0, BAM_NUM_PIPES));
+		bdev->num_channels = val & BAM_NUM_PIPES_MASK;
+	}
 
 	if (bdev->controlled_remotely)
 		return 0;
@@ -1179,6 +1181,13 @@ static int bam_dma_probe(struct platform_device *pdev)
 	bdev->controlled_remotely = of_property_read_bool(pdev->dev.of_node,
 						"qcom,controlled-remotely");
 
+	if (bdev->controlled_remotely) {
+		ret = of_property_read_u32(pdev->dev.of_node, "num-channels",
+					   &bdev->num_channels);
+		if (ret)
+			dev_err(bdev->dev, "num-channels unspecified in dt\n");
+	}
+
 	bdev->bamclk = devm_clk_get(bdev->dev, "bam_clk");
 	if (IS_ERR(bdev->bamclk)) {
 		bdev->bamclk = NULL;
-- 
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH 3/4] dmaengine: qcom: bam_dma: do not write to global regs in remote mode
  2018-01-16 19:02 [PATCH 0/4] dmaengine: qcom: bam_dma: fixes for remotely controlled bam srinivas.kandagatla
  2018-01-16 19:02 ` [PATCH 1/4] dmaengine: qcom: bam_dma: make bam clk optional srinivas.kandagatla
@ 2018-01-16 19:02 ` srinivas.kandagatla
  2018-01-16 19:02 ` [PATCH 4/4] dmaengine: qcom: bam_dma: Add num-ees dt binding for remotely controlled srinivas.kandagatla
       [not found] ` <20180116190236.14558-1-srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
  3 siblings, 0 replies; 19+ messages in thread
From: srinivas.kandagatla @ 2018-01-16 19:02 UTC (permalink / raw)
  To: Vinod Koul, Andy Gross, dmaengine
  Cc: Rob Herring, Mark Rutland, David Brown, Dan Williams, devicetree,
	linux-kernel, linux-arm-msm, linux-soc, yanhe, ramkri, sdharia,
	Srinivas Kandagatla

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

BAM_DESC_CNT_TRSHLD register is global register, which can only be written
when BAM is in master mode, So check the mode of operation before writing it.

Without this check SOC's xPU would catch such access and crash the system.
First noticed on DB820c while testing SLIMBus BAM.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/dma/qcom/bam_dma.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
index 523bd178047a..bbbb755d7549 100644
--- a/drivers/dma/qcom/bam_dma.c
+++ b/drivers/dma/qcom/bam_dma.c
@@ -905,12 +905,15 @@ static void bam_apply_new_config(struct bam_chan *bchan,
 	struct bam_device *bdev = bchan->bdev;
 	u32 maxburst;
 
-	if (dir == DMA_DEV_TO_MEM)
-		maxburst = bchan->slave.src_maxburst;
-	else
-		maxburst = bchan->slave.dst_maxburst;
-
-	writel_relaxed(maxburst, bam_addr(bdev, 0, BAM_DESC_CNT_TRSHLD));
+	if (!bdev->controlled_remotely) {
+		if (dir == DMA_DEV_TO_MEM)
+			maxburst = bchan->slave.src_maxburst;
+		else
+			maxburst = bchan->slave.dst_maxburst;
+
+		writel_relaxed(maxburst,
+			       bam_addr(bdev, 0, BAM_DESC_CNT_TRSHLD));
+	}
 
 	bchan->reconfigure = 0;
 }
-- 
2.15.1

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

* [PATCH 4/4] dmaengine: qcom: bam_dma: Add num-ees dt binding for remotely controlled
  2018-01-16 19:02 [PATCH 0/4] dmaengine: qcom: bam_dma: fixes for remotely controlled bam srinivas.kandagatla
  2018-01-16 19:02 ` [PATCH 1/4] dmaengine: qcom: bam_dma: make bam clk optional srinivas.kandagatla
  2018-01-16 19:02 ` [PATCH 3/4] dmaengine: qcom: bam_dma: do not write to global regs in remote mode srinivas.kandagatla
@ 2018-01-16 19:02 ` srinivas.kandagatla
       [not found]   ` <20180116190236.14558-5-srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
       [not found] ` <20180116190236.14558-1-srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
  3 siblings, 1 reply; 19+ messages in thread
From: srinivas.kandagatla @ 2018-01-16 19:02 UTC (permalink / raw)
  To: Vinod Koul, Andy Gross, dmaengine
  Cc: Rob Herring, Mark Rutland, David Brown, Dan Williams, devicetree,
	linux-kernel, linux-arm-msm, linux-soc, yanhe, ramkri, sdharia,
	Srinivas Kandagatla

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

When Linux is master of BAM, it can directly read registers to know number
of supported execution enviroments, however when its remotely controlled
reading these registers would trigger a crash if the BAM is not yet
intialized/powered up on the remote side.

This patch adds new binding num-ees to specify supported number of
Execution Environments when BAM is remotely controlled.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 Documentation/devicetree/bindings/dma/qcom_bam_dma.txt |  2 ++
 drivers/dma/qcom/bam_dma.c                             | 15 ++++++++++++---
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt b/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
index aa6822cbb230..f0d10c2b393e 100644
--- a/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
+++ b/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
@@ -17,6 +17,8 @@ Required properties:
   remote proccessor i.e. execution environment.
 - num-channels : optional, indicates supported number of DMA channels in a
   remotely controlled bam.
+- num-ees : optional, indicates supported number of Execution Environments in a
+  remotely controlled bam.
 
 Example:
 
diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
index bbbb755d7549..7a8727271d60 100644
--- a/drivers/dma/qcom/bam_dma.c
+++ b/drivers/dma/qcom/bam_dma.c
@@ -387,6 +387,7 @@ struct bam_device {
 	struct device_dma_parameters dma_parms;
 	struct bam_chan *channels;
 	u32 num_channels;
+	u32 num_ees;
 
 	/* execution environment ID, from DT */
 	u32 ee;
@@ -1079,11 +1080,14 @@ static int bam_init(struct bam_device *bdev)
 	u32 val;
 
 	/* read revision and configuration information */
-	val = readl_relaxed(bam_addr(bdev, 0, BAM_REVISION)) >> NUM_EES_SHIFT;
-	val &= NUM_EES_MASK;
+	if (!bdev->num_ees) {
+		val = readl_relaxed(bam_addr(bdev, 0, BAM_REVISION)) >> NUM_EES_SHIFT;
+		val &= NUM_EES_MASK;
+		bdev->num_ees = val;
+	}
 
 	/* check that configured EE is within range */
-	if (bdev->ee >= val)
+	if (bdev->ee >= bdev->num_ees)
 		return -EINVAL;
 
 	if (!bdev->num_channels) {
@@ -1189,6 +1193,11 @@ static int bam_dma_probe(struct platform_device *pdev)
 					   &bdev->num_channels);
 		if (ret)
 			dev_err(bdev->dev, "num-channels unspecified in dt\n");
+
+		ret = of_property_read_u32(pdev->dev.of_node, "num-ees",
+					   &bdev->num_ees);
+		if (ret)
+			dev_err(bdev->dev, "num-ees unspecified in dt\n");
 	}
 
 	bdev->bamclk = devm_clk_get(bdev->dev, "bam_clk");
-- 
2.15.1

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

* Re: [PATCH 1/4] dmaengine: qcom: bam_dma: make bam clk optional
  2018-01-16 19:02 ` [PATCH 1/4] dmaengine: qcom: bam_dma: make bam clk optional srinivas.kandagatla
@ 2018-01-16 19:38   ` Sagar Dharia
  2018-01-17  9:46     ` Srinivas Kandagatla
  2018-01-19  5:52   ` Vinod Koul
  1 sibling, 1 reply; 19+ messages in thread
From: Sagar Dharia @ 2018-01-16 19:38 UTC (permalink / raw)
  To: srinivas.kandagatla, Vinod Koul, Andy Gross, dmaengine
  Cc: Rob Herring, Mark Rutland, David Brown, Dan Williams, devicetree,
	linux-kernel, linux-arm-msm, linux-soc, yanhe, ramkri, sdharia


On 1/16/2018 12:02 PM, srinivas.kandagatla@linaro.org wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> When BAM is remotely controlled it does not sound correct to control
> its clk on Linux side. Make it optional, so that its not madatory
> for remote controlled BAM instances.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>   drivers/dma/qcom/bam_dma.c | 15 ++++++++-------
>   1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
> index 03c4eb3fd314..78e488e8f96d 100644
> --- a/drivers/dma/qcom/bam_dma.c
> +++ b/drivers/dma/qcom/bam_dma.c
> @@ -1180,13 +1180,14 @@ static int bam_dma_probe(struct platform_device *pdev)
>   						"qcom,controlled-remotely");
>   
>   	bdev->bamclk = devm_clk_get(bdev->dev, "bam_clk");
> -	if (IS_ERR(bdev->bamclk))
> -		return PTR_ERR(bdev->bamclk);
> -
> -	ret = clk_prepare_enable(bdev->bamclk);
> -	if (ret) {
> -		dev_err(bdev->dev, "failed to prepare/enable clock\n");
> -		return ret;
> +	if (IS_ERR(bdev->bamclk)) {
> +		bdev->bamclk = NULL;
> +	} else {
> +		ret = clk_prepare_enable(bdev->bamclk);
> +		if (ret) {
> +			dev_err(bdev->dev, "failed to prepare/enable clock\n");
> +			return ret;
> +		}
I believe you can also keep pm_runtime disabled if BAM is remotely 
controlled.

Thanks
Sagar
>   	}
>   
>   	ret = bam_init(bdev);

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

* Re: [PATCH 1/4] dmaengine: qcom: bam_dma: make bam clk optional
  2018-01-16 19:38   ` Sagar Dharia
@ 2018-01-17  9:46     ` Srinivas Kandagatla
  0 siblings, 0 replies; 19+ messages in thread
From: Srinivas Kandagatla @ 2018-01-17  9:46 UTC (permalink / raw)
  To: Sagar Dharia, Vinod Koul, Andy Gross, dmaengine
  Cc: Rob Herring, Mark Rutland, David Brown, Dan Williams, devicetree,
	linux-kernel, linux-arm-msm, linux-soc, yanhe, ramkri, sdharia



On 16/01/18 19:38, Sagar Dharia wrote:
> 
> On 1/16/2018 12:02 PM, srinivas.kandagatla@linaro.org wrote:
>> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>
>> When BAM is remotely controlled it does not sound correct to control
>> its clk on Linux side. Make it optional, so that its not madatory
>> for remote controlled BAM instances.
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> ---
>>   drivers/dma/qcom/bam_dma.c | 15 ++++++++-------
>>   1 file changed, 8 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
>> index 03c4eb3fd314..78e488e8f96d 100644
>> --- a/drivers/dma/qcom/bam_dma.c
>> +++ b/drivers/dma/qcom/bam_dma.c
>> @@ -1180,13 +1180,14 @@ static int bam_dma_probe(struct 
>> platform_device *pdev)
>>                           "qcom,controlled-remotely");
>>       bdev->bamclk = devm_clk_get(bdev->dev, "bam_clk");
>> -    if (IS_ERR(bdev->bamclk))
>> -        return PTR_ERR(bdev->bamclk);
>> -
>> -    ret = clk_prepare_enable(bdev->bamclk);
>> -    if (ret) {
>> -        dev_err(bdev->dev, "failed to prepare/enable clock\n");
>> -        return ret;
>> +    if (IS_ERR(bdev->bamclk)) {
>> +        bdev->bamclk = NULL;
>> +    } else {
>> +        ret = clk_prepare_enable(bdev->bamclk);
>> +        if (ret) {
>> +            dev_err(bdev->dev, "failed to prepare/enable clock\n");
>> +            return ret;
>> +        }
> I believe you can also keep pm_runtime disabled if BAM is remotely 
> controlled.

Yes, that's another topic which should be fixed too. I will add that 
patch in next version.

thanks,
srini
> 
> Thanks
> Sagar
>>       }
>>       ret = bam_init(bdev);
> 

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

* Re: [PATCH 0/4] dmaengine: qcom: bam_dma: fixes for remotely controlled bam
       [not found] ` <20180116190236.14558-1-srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
  2018-01-16 19:02   ` [PATCH 2/4] dmaengine: qcom: bam_dma: add num-channels " srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A
@ 2018-01-17 10:18   ` Vinod Koul
  2018-01-17 10:55     ` Srinivas Kandagatla
  1 sibling, 1 reply; 19+ messages in thread
From: Vinod Koul @ 2018-01-17 10:18 UTC (permalink / raw)
  To: srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A
  Cc: Andy Gross, dmaengine-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
	Mark Rutland, David Brown, Dan Williams,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	linux-soc-u79uwXL29TY76Z2rM5mHXA, yanhe-jfJNa2p1gH1BDgjK7y7TUQ,
	ramkri-Rm6X0d1/PG5y9aJCnZT0Uw, sdharia-jfJNa2p1gH1BDgjK7y7TUQ

On Tue, Jan 16, 2018 at 07:02:32PM +0000, srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> 
> Hi Andy,
> 
> I did hit few issues while trying out SLIMBus BAM on DB820c, this BAM instance
> is remotely controlled and powered up after ADSP is booted using QMI commands.

What do you mean by "remotely controlled" in this series?

> 
> Firstly some of the master registers are written even when the BAM is remotely
> controlled, and secondly reading registers when bam is not ready yet.
> 
> These 4 patches address these issues, there are few more issues like doing PM
> in simillar usecase, these will be addressed soon.
> 
> Thanks,
> Srini
> 
> Srinivas Kandagatla (4):
>   dmaengine: qcom: bam_dma: make bam clk optional
>   dmaengine: qcom: bam_dma: add num-channels binding for remotely
>     controlled
>   dmaengine: qcom: bam_dma: do not write to global regs in remote mode
>   dmaengine: qcom: bam_dma: Add num-ees dt binding for remotely
>     controlled
> 
>  .../devicetree/bindings/dma/qcom_bam_dma.txt       |  4 ++
>  drivers/dma/qcom/bam_dma.c                         | 56 +++++++++++++++-------
>  2 files changed, 43 insertions(+), 17 deletions(-)
> 
> -- 
> 2.15.1
> 

-- 
~Vinod
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 0/4] dmaengine: qcom: bam_dma: fixes for remotely controlled bam
  2018-01-17 10:18   ` [PATCH 0/4] dmaengine: qcom: bam_dma: fixes for remotely controlled bam Vinod Koul
@ 2018-01-17 10:55     ` Srinivas Kandagatla
  2018-01-17 15:59       ` Vinod Koul
  0 siblings, 1 reply; 19+ messages in thread
From: Srinivas Kandagatla @ 2018-01-17 10:55 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Andy Gross, dmaengine, Rob Herring, Mark Rutland, David Brown,
	Dan Williams, devicetree, linux-kernel, linux-arm-msm, linux-soc,
	yanhe, ramkri, sdharia



On 17/01/18 10:18, Vinod Koul wrote:
> On Tue, Jan 16, 2018 at 07:02:32PM +0000, srinivas.kandagatla@linaro.org wrote:
>> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>
>> Hi Andy,
>>
>> I did hit few issues while trying out SLIMBus BAM on DB820c, this BAM instance
>> is remotely controlled and powered up after ADSP is booted using QMI commands.
> 
> What do you mean by "remotely controlled" in this series?

DMA controller is controlled by the remote processor, which is a DSP in 
this case.
Linux side is in remote mode in this setup, this can setup transfer 
descriptors and start/stop a transfer. But all the 
initialization/powerup part will be done in the remote processor.

--srini

> 
>>
>> Firstly some of the master registers are written even when the BAM is remotely
>> controlled, and secondly reading registers when bam is not ready yet.
>>
>> These 4 patches address these issues, there are few more issues like doing PM
>> in simillar usecase, these will be addressed soon.
>>
>> Thanks,
>> Srini
>>
>> Srinivas Kandagatla (4):
>>    dmaengine: qcom: bam_dma: make bam clk optional
>>    dmaengine: qcom: bam_dma: add num-channels binding for remotely
>>      controlled
>>    dmaengine: qcom: bam_dma: do not write to global regs in remote mode
>>    dmaengine: qcom: bam_dma: Add num-ees dt binding for remotely
>>      controlled
>>
>>   .../devicetree/bindings/dma/qcom_bam_dma.txt       |  4 ++
>>   drivers/dma/qcom/bam_dma.c                         | 56 +++++++++++++++-------
>>   2 files changed, 43 insertions(+), 17 deletions(-)
>>
>> -- 
>> 2.15.1
>>
> 

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

* Re: [PATCH 0/4] dmaengine: qcom: bam_dma: fixes for remotely controlled bam
  2018-01-17 10:55     ` Srinivas Kandagatla
@ 2018-01-17 15:59       ` Vinod Koul
  0 siblings, 0 replies; 19+ messages in thread
From: Vinod Koul @ 2018-01-17 15:59 UTC (permalink / raw)
  To: Srinivas Kandagatla
  Cc: Andy Gross, dmaengine, Rob Herring, Mark Rutland, David Brown,
	Dan Williams, devicetree, linux-kernel, linux-arm-msm, linux-soc,
	yanhe, ramkri, sdharia

On Wed, Jan 17, 2018 at 10:55:34AM +0000, Srinivas Kandagatla wrote:
> 
> 
> On 17/01/18 10:18, Vinod Koul wrote:
> >On Tue, Jan 16, 2018 at 07:02:32PM +0000, srinivas.kandagatla@linaro.org wrote:
> >>From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> >>
> >>Hi Andy,
> >>
> >>I did hit few issues while trying out SLIMBus BAM on DB820c, this BAM instance
> >>is remotely controlled and powered up after ADSP is booted using QMI commands.
> >
> >What do you mean by "remotely controlled" in this series?
> 
> DMA controller is controlled by the remote processor, which is a DSP in this
> case.
> Linux side is in remote mode in this setup, this can setup transfer
> descriptors and start/stop a transfer. But all the initialization/powerup
> part will be done in the remote processor.

Yeah that was my guess too. So in this case does linux see these
controllers/channels, i would presume no...

-- 
~Vinod

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

* Re: [PATCH 1/4] dmaengine: qcom: bam_dma: make bam clk optional
  2018-01-16 19:02 ` [PATCH 1/4] dmaengine: qcom: bam_dma: make bam clk optional srinivas.kandagatla
  2018-01-16 19:38   ` Sagar Dharia
@ 2018-01-19  5:52   ` Vinod Koul
  2018-01-22  9:55     ` Srinivas Kandagatla
  1 sibling, 1 reply; 19+ messages in thread
From: Vinod Koul @ 2018-01-19  5:52 UTC (permalink / raw)
  To: srinivas.kandagatla
  Cc: Andy Gross, dmaengine, Rob Herring, Mark Rutland, David Brown,
	Dan Williams, devicetree, linux-kernel, linux-arm-msm, linux-soc,
	yanhe, ramkri, sdharia

On Tue, Jan 16, 2018 at 07:02:33PM +0000, srinivas.kandagatla@linaro.org wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> 
> When BAM is remotely controlled it does not sound correct to control
> its clk on Linux side. Make it optional, so that its not madatory

s/madatory/mandatory

> for remote controlled BAM instances.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>  drivers/dma/qcom/bam_dma.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
> index 03c4eb3fd314..78e488e8f96d 100644
> --- a/drivers/dma/qcom/bam_dma.c
> +++ b/drivers/dma/qcom/bam_dma.c
> @@ -1180,13 +1180,14 @@ static int bam_dma_probe(struct platform_device *pdev)
>  						"qcom,controlled-remotely");
>  
>  	bdev->bamclk = devm_clk_get(bdev->dev, "bam_clk");

but you still do clk_get unconditionally?

> -	if (IS_ERR(bdev->bamclk))
> -		return PTR_ERR(bdev->bamclk);
> -
> -	ret = clk_prepare_enable(bdev->bamclk);
> -	if (ret) {
> -		dev_err(bdev->dev, "failed to prepare/enable clock\n");
> -		return ret;
> +	if (IS_ERR(bdev->bamclk)) {
> +		bdev->bamclk = NULL;
> +	} else {
> +		ret = clk_prepare_enable(bdev->bamclk);
> +		if (ret) {
> +			dev_err(bdev->dev, "failed to prepare/enable clock\n");
> +			return ret;
> +		}

wouldn't it be better to set that an instance is remote controlled and thus
not at all visible to Linux?

>  	}
>  
>  	ret = bam_init(bdev);
> -- 
> 2.15.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe dmaengine" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
~Vinod

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

* Re: [PATCH 2/4] dmaengine: qcom: bam_dma: add num-channels binding for remotely controlled
  2018-01-16 19:02   ` [PATCH 2/4] dmaengine: qcom: bam_dma: add num-channels " srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A
@ 2018-01-19  5:55     ` Vinod Koul
  2018-01-22  9:55       ` Srinivas Kandagatla
       [not found]     ` <20180116190236.14558-3-srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
  1 sibling, 1 reply; 19+ messages in thread
From: Vinod Koul @ 2018-01-19  5:55 UTC (permalink / raw)
  To: srinivas.kandagatla
  Cc: Andy Gross, dmaengine, Rob Herring, Mark Rutland, David Brown,
	Dan Williams, devicetree, linux-kernel, linux-arm-msm, linux-soc,
	yanhe, ramkri, sdharia

On Tue, Jan 16, 2018 at 07:02:34PM +0000, srinivas.kandagatla@linaro.org wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> 
> When Linux is master of BAM, it can directly read registers to know number
> of supported channels, however when its remotely controlled reading these
> registers would trigger a crash if the BAM is not yet intialized/powered up
> on the remote side.
> 
> This patch adds num-channels binding to specify number of supported
> dma channels on remotely controlled BAM.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>  Documentation/devicetree/bindings/dma/qcom_bam_dma.txt |  2 ++
>  drivers/dma/qcom/bam_dma.c                             | 13 +++++++++++--
>  2 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt b/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
> index 9cbf5d9df8fd..aa6822cbb230 100644
> --- a/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
> +++ b/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
> @@ -15,6 +15,8 @@ Required properties:
>    the secure world.
>  - qcom,controlled-remotely : optional, indicates that the bam is controlled by
>    remote proccessor i.e. execution environment.
> +- num-channels : optional, indicates supported number of DMA channels in a
> +  remotely controlled bam.
>  
>  Example:
>  
> diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
> index 78e488e8f96d..523bd178047a 100644
> --- a/drivers/dma/qcom/bam_dma.c
> +++ b/drivers/dma/qcom/bam_dma.c
> @@ -1083,8 +1083,10 @@ static int bam_init(struct bam_device *bdev)
>  	if (bdev->ee >= val)
>  		return -EINVAL;
>  
> -	val = readl_relaxed(bam_addr(bdev, 0, BAM_NUM_PIPES));
> -	bdev->num_channels = val & BAM_NUM_PIPES_MASK;
> +	if (!bdev->num_channels) {
> +		val = readl_relaxed(bam_addr(bdev, 0, BAM_NUM_PIPES));
> +		bdev->num_channels = val & BAM_NUM_PIPES_MASK;
> +	}
>  
>  	if (bdev->controlled_remotely)
>  		return 0;
> @@ -1179,6 +1181,13 @@ static int bam_dma_probe(struct platform_device *pdev)
>  	bdev->controlled_remotely = of_property_read_bool(pdev->dev.of_node,
>  						"qcom,controlled-remotely");
>  
> +	if (bdev->controlled_remotely) {

hmm so if we remove the remotely controlled instanced from DT and then Linux
won't see them and not do anything. Do we need to do configuration of these
instances too?

> +		ret = of_property_read_u32(pdev->dev.of_node, "num-channels",
> +					   &bdev->num_channels);
> +		if (ret)
> +			dev_err(bdev->dev, "num-channels unspecified in dt\n");
> +	}
> +
>  	bdev->bamclk = devm_clk_get(bdev->dev, "bam_clk");
>  	if (IS_ERR(bdev->bamclk)) {
>  		bdev->bamclk = NULL;
> -- 
> 2.15.1
> 

-- 
~Vinod

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

* Re: [PATCH 1/4] dmaengine: qcom: bam_dma: make bam clk optional
  2018-01-19  5:52   ` Vinod Koul
@ 2018-01-22  9:55     ` Srinivas Kandagatla
       [not found]       ` <8dfa8ba1-6e98-a8e4-614c-592861cef571-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
  0 siblings, 1 reply; 19+ messages in thread
From: Srinivas Kandagatla @ 2018-01-22  9:55 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Andy Gross, dmaengine, Rob Herring, Mark Rutland, David Brown,
	Dan Williams, devicetree, linux-kernel, linux-arm-msm, linux-soc,
	yanhe, ramkri, sdharia



On 19/01/18 05:52, Vinod Koul wrote:
> On Tue, Jan 16, 2018 at 07:02:33PM +0000, srinivas.kandagatla@linaro.org wrote:
>> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>
>> When BAM is remotely controlled it does not sound correct to control
>> its clk on Linux side. Make it optional, so that its not madatory
> 
> s/madatory/mandatory
> 
Yep,
>> for remote controlled BAM instances.
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> ---
>>   drivers/dma/qcom/bam_dma.c | 15 ++++++++-------
>>   1 file changed, 8 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
>> index 03c4eb3fd314..78e488e8f96d 100644
>> --- a/drivers/dma/qcom/bam_dma.c
>> +++ b/drivers/dma/qcom/bam_dma.c
>> @@ -1180,13 +1180,14 @@ static int bam_dma_probe(struct platform_device *pdev)
>>   						"qcom,controlled-remotely");
>>   
>>   	bdev->bamclk = devm_clk_get(bdev->dev, "bam_clk");
> 
> but you still do clk_get unconditionally?

Only reason to do this way is to not break existing users in the mainline.

remotely controlled BAM is already supported in upstream driver, there 
are users of this who pass clk from device tree, If I make this 
conditional then subsequent reads to the BAM registers for those 
instances might crash the system.

This sounds wrong to control clk from linux for the dma controller which 
is remotely controlled. These users should be transitioned to new 
bindings once the new bindings endup in the mainline.

> 
>> -	if (IS_ERR(bdev->bamclk))
>> -		return PTR_ERR(bdev->bamclk);
>> -
>> -	ret = clk_prepare_enable(bdev->bamclk);
>> -	if (ret) {
>> -		dev_err(bdev->dev, "failed to prepare/enable clock\n");
>> -		return ret;
>> +	if (IS_ERR(bdev->bamclk)) {
>> +		bdev->bamclk = NULL;
>> +	} else {
>> +		ret = clk_prepare_enable(bdev->bamclk);
>> +		if (ret) {
>> +			dev_err(bdev->dev, "failed to prepare/enable clock\n");
>> +			return ret;
>> +		}
> 
> wouldn't it be better to set that an instance is remote controlled and thus
> not at all visible to Linux?

We already have a flag "controlled_remotely" for that in the driver.

thanks,
srini
> 
>>   	}
>>   
>>   	ret = bam_init(bdev);
>> -- 
>> 2.15.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe dmaengine" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

* Re: [PATCH 2/4] dmaengine: qcom: bam_dma: add num-channels binding for remotely controlled
  2018-01-19  5:55     ` Vinod Koul
@ 2018-01-22  9:55       ` Srinivas Kandagatla
  0 siblings, 0 replies; 19+ messages in thread
From: Srinivas Kandagatla @ 2018-01-22  9:55 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Andy Gross, dmaengine, Rob Herring, Mark Rutland, David Brown,
	Dan Williams, devicetree, linux-kernel, linux-arm-msm, linux-soc,
	yanhe, ramkri, sdharia



On 19/01/18 05:55, Vinod Koul wrote:
> On Tue, Jan 16, 2018 at 07:02:34PM +0000, srinivas.kandagatla@linaro.org wrote:
>> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>>
>> When Linux is master of BAM, it can directly read registers to know number
>> of supported channels, however when its remotely controlled reading these
>> registers would trigger a crash if the BAM is not yet intialized/powered up
>> on the remote side.
>>
>> This patch adds num-channels binding to specify number of supported
>> dma channels on remotely controlled BAM.
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> ---
>>   Documentation/devicetree/bindings/dma/qcom_bam_dma.txt |  2 ++
>>   drivers/dma/qcom/bam_dma.c                             | 13 +++++++++++--
>>   2 files changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt b/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
>> index 9cbf5d9df8fd..aa6822cbb230 100644
>> --- a/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
>> +++ b/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
>> @@ -15,6 +15,8 @@ Required properties:
>>     the secure world.
>>   - qcom,controlled-remotely : optional, indicates that the bam is controlled by
>>     remote proccessor i.e. execution environment.
>> +- num-channels : optional, indicates supported number of DMA channels in a
>> +  remotely controlled bam.
>>   
>>   Example:
>>   
>> diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
>> index 78e488e8f96d..523bd178047a 100644
>> --- a/drivers/dma/qcom/bam_dma.c
>> +++ b/drivers/dma/qcom/bam_dma.c
>> @@ -1083,8 +1083,10 @@ static int bam_init(struct bam_device *bdev)
>>   	if (bdev->ee >= val)
>>   		return -EINVAL;
>>   
>> -	val = readl_relaxed(bam_addr(bdev, 0, BAM_NUM_PIPES));
>> -	bdev->num_channels = val & BAM_NUM_PIPES_MASK;
>> +	if (!bdev->num_channels) {
>> +		val = readl_relaxed(bam_addr(bdev, 0, BAM_NUM_PIPES));
>> +		bdev->num_channels = val & BAM_NUM_PIPES_MASK;
>> +	}
>>   
>>   	if (bdev->controlled_remotely)
>>   		return 0;
>> @@ -1179,6 +1181,13 @@ static int bam_dma_probe(struct platform_device *pdev)
>>   	bdev->controlled_remotely = of_property_read_bool(pdev->dev.of_node,
>>   						"qcom,controlled-remotely");
>>   
>> +	if (bdev->controlled_remotely) {
> 
> hmm so if we remove the remotely controlled instanced from DT and then Linux
> won't see them and not do anything. Do we need to do configuration of these
> instances too?
No, bindings "num-channels" and "num-ees" are applicable to remote 
controlled instances only, which is documented in the DT bindings.

Normally these values come from register reads, which are not possible 
in case of remotely controlled instances as BAM driver would not know if 
the remote is powered up or not.

thanks,
srini

> 
>> +		ret = of_property_read_u32(pdev->dev.of_node, "num-channels",
>> +					   &bdev->num_channels);
>> +		if (ret)
>> +			dev_err(bdev->dev, "num-channels unspecified in dt\n");
>> +	}
>> +
>>   	bdev->bamclk = devm_clk_get(bdev->dev, "bam_clk");
>>   	if (IS_ERR(bdev->bamclk)) {
>>   		bdev->bamclk = NULL;
>> -- 
>> 2.15.1
>>
> 

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

* Re: [PATCH 1/4] dmaengine: qcom: bam_dma: make bam clk optional
       [not found]       ` <8dfa8ba1-6e98-a8e4-614c-592861cef571-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
@ 2018-01-23  9:19         ` Vinod Koul
  2018-01-23  9:20           ` Srinivas Kandagatla
  0 siblings, 1 reply; 19+ messages in thread
From: Vinod Koul @ 2018-01-23  9:19 UTC (permalink / raw)
  To: Srinivas Kandagatla
  Cc: Andy Gross, dmaengine-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
	Mark Rutland, David Brown, Dan Williams,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	linux-soc-u79uwXL29TY76Z2rM5mHXA, yanhe-jfJNa2p1gH1BDgjK7y7TUQ,
	ramkri-Rm6X0d1/PG5y9aJCnZT0Uw, sdharia-jfJNa2p1gH1BDgjK7y7TUQ

On Mon, Jan 22, 2018 at 09:55:01AM +0000, Srinivas Kandagatla wrote:

> >>@@ -1180,13 +1180,14 @@ static int bam_dma_probe(struct platform_device *pdev)
> >>  						"qcom,controlled-remotely");
> >>  	bdev->bamclk = devm_clk_get(bdev->dev, "bam_clk");
> >
> >but you still do clk_get unconditionally?
> 
> Only reason to do this way is to not break existing users in the mainline.
> 
> remotely controlled BAM is already supported in upstream driver, there are
> users of this who pass clk from device tree, If I make this conditional then
> subsequent reads to the BAM registers for those instances might crash the
> system.

But these instances are remote controlled, so if we stop representing them
in Linux, why would we read them?

-- 
~Vinod
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 1/4] dmaengine: qcom: bam_dma: make bam clk optional
  2018-01-23  9:19         ` Vinod Koul
@ 2018-01-23  9:20           ` Srinivas Kandagatla
  0 siblings, 0 replies; 19+ messages in thread
From: Srinivas Kandagatla @ 2018-01-23  9:20 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Andy Gross, dmaengine-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
	Mark Rutland, David Brown, Dan Williams,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	linux-soc-u79uwXL29TY76Z2rM5mHXA, yanhe-jfJNa2p1gH1BDgjK7y7TUQ,
	ramkri-Rm6X0d1/PG5y9aJCnZT0Uw, sdharia-jfJNa2p1gH1BDgjK7y7TUQ



On 23/01/18 09:19, Vinod Koul wrote:
> On Mon, Jan 22, 2018 at 09:55:01AM +0000, Srinivas Kandagatla wrote:
> 
>>>> @@ -1180,13 +1180,14 @@ static int bam_dma_probe(struct platform_device *pdev)
>>>>   						"qcom,controlled-remotely");
>>>>   	bdev->bamclk = devm_clk_get(bdev->dev, "bam_clk");
>>>
>>> but you still do clk_get unconditionally?
>>
>> Only reason to do this way is to not break existing users in the mainline.
>>
>> remotely controlled BAM is already supported in upstream driver, there are
>> users of this who pass clk from device tree, If I make this conditional then
>> subsequent reads to the BAM registers for those instances might crash the
>> system.
> 
> But these instances are remote controlled, so if we stop representing them
> in Linux, why would we read them?

Plan is that we would transition those users once we get these 
bindings/changes in. Currently I don't have access to any of those 
devices so I made the changes safe, such that it does not break devices 
on mainline.

--srini

> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 2/4] dmaengine: qcom: bam_dma: add num-channels binding for remotely controlled
       [not found]     ` <20180116190236.14558-3-srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
@ 2018-01-29 16:19       ` Rob Herring
  0 siblings, 0 replies; 19+ messages in thread
From: Rob Herring @ 2018-01-29 16:19 UTC (permalink / raw)
  To: srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A
  Cc: Vinod Koul, Andy Gross, dmaengine-u79uwXL29TY76Z2rM5mHXA,
	Mark Rutland, David Brown, Dan Williams,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	linux-soc-u79uwXL29TY76Z2rM5mHXA, yanhe-jfJNa2p1gH1BDgjK7y7TUQ,
	ramkri-Rm6X0d1/PG5y9aJCnZT0Uw, sdharia-jfJNa2p1gH1BDgjK7y7TUQ

On Tue, Jan 16, 2018 at 07:02:34PM +0000, srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> 
> When Linux is master of BAM, it can directly read registers to know number
> of supported channels, however when its remotely controlled reading these
> registers would trigger a crash if the BAM is not yet intialized/powered up
> on the remote side.
> 
> This patch adds num-channels binding to specify number of supported
> dma channels on remotely controlled BAM.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/dma/qcom_bam_dma.txt |  2 ++

Reviewed-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

>  drivers/dma/qcom/bam_dma.c                             | 13 +++++++++++--
>  2 files changed, 13 insertions(+), 2 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 4/4] dmaengine: qcom: bam_dma: Add num-ees dt binding for remotely controlled
       [not found]   ` <20180116190236.14558-5-srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
@ 2018-01-29 16:21     ` Rob Herring
  2018-01-30  9:18       ` Srinivas Kandagatla
  0 siblings, 1 reply; 19+ messages in thread
From: Rob Herring @ 2018-01-29 16:21 UTC (permalink / raw)
  To: srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A
  Cc: Vinod Koul, Andy Gross, dmaengine-u79uwXL29TY76Z2rM5mHXA,
	Mark Rutland, David Brown, Dan Williams,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	linux-soc-u79uwXL29TY76Z2rM5mHXA, yanhe-jfJNa2p1gH1BDgjK7y7TUQ,
	ramkri-Rm6X0d1/PG5y9aJCnZT0Uw, sdharia-jfJNa2p1gH1BDgjK7y7TUQ

On Tue, Jan 16, 2018 at 07:02:36PM +0000, srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> 
> When Linux is master of BAM, it can directly read registers to know number
> of supported execution enviroments, however when its remotely controlled
> reading these registers would trigger a crash if the BAM is not yet
> intialized/powered up on the remote side.
> 
> This patch adds new binding num-ees to specify supported number of
> Execution Environments when BAM is remotely controlled.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/dma/qcom_bam_dma.txt |  2 ++
>  drivers/dma/qcom/bam_dma.c                             | 15 ++++++++++++---
>  2 files changed, 14 insertions(+), 3 deletions(-)

The correct split is the binding changes in 1 patch. Driver changes 
separate.

> 
> diff --git a/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt b/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
> index aa6822cbb230..f0d10c2b393e 100644
> --- a/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
> +++ b/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
> @@ -17,6 +17,8 @@ Required properties:
>    remote proccessor i.e. execution environment.
>  - num-channels : optional, indicates supported number of DMA channels in a
>    remotely controlled bam.
> +- num-ees : optional, indicates supported number of Execution Environments in a
> +  remotely controlled bam.

This one needs a vendor prefix as it is not a common property.

>  
>  Example:
>  
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 4/4] dmaengine: qcom: bam_dma: Add num-ees dt binding for remotely controlled
  2018-01-29 16:21     ` Rob Herring
@ 2018-01-30  9:18       ` Srinivas Kandagatla
  0 siblings, 0 replies; 19+ messages in thread
From: Srinivas Kandagatla @ 2018-01-30  9:18 UTC (permalink / raw)
  To: Rob Herring
  Cc: Vinod Koul, Andy Gross, dmaengine-u79uwXL29TY76Z2rM5mHXA,
	Mark Rutland, David Brown, Dan Williams,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	linux-soc-u79uwXL29TY76Z2rM5mHXA, yanhe-jfJNa2p1gH1BDgjK7y7TUQ,
	ramkri-Rm6X0d1/PG5y9aJCnZT0Uw, sdharia-jfJNa2p1gH1BDgjK7y7TUQ

Thanks for the review,

On 29/01/18 16:21, Rob Herring wrote:
> On Tue, Jan 16, 2018 at 07:02:36PM +0000, srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org wrote:
>> From: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>
>> When Linux is master of BAM, it can directly read registers to know number
>> of supported execution enviroments, however when its remotely controlled
>> reading these registers would trigger a crash if the BAM is not yet
>> intialized/powered up on the remote side.
>>
>> This patch adds new binding num-ees to specify supported number of
>> Execution Environments when BAM is remotely controlled.
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>>   Documentation/devicetree/bindings/dma/qcom_bam_dma.txt |  2 ++
>>   drivers/dma/qcom/bam_dma.c                             | 15 ++++++++++++---
>>   2 files changed, 14 insertions(+), 3 deletions(-)
> 
> The correct split is the binding changes in 1 patch. Driver changes
> separate.
> 
Sure, Will Split it in next version.
>>
>> diff --git a/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt b/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
>> index aa6822cbb230..f0d10c2b393e 100644
>> --- a/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
>> +++ b/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
>> @@ -17,6 +17,8 @@ Required properties:
>>     remote proccessor i.e. execution environment.
>>   - num-channels : optional, indicates supported number of DMA channels in a
>>     remotely controlled bam.
>> +- num-ees : optional, indicates supported number of Execution Environments in a
>> +  remotely controlled bam.
> 
> This one needs a vendor prefix as it is not a common property.
> 
Make sense, I will change it in next version.

Thanks,
Srini
>>   
>>   Example:
>>   
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

end of thread, other threads:[~2018-01-30  9:18 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-16 19:02 [PATCH 0/4] dmaengine: qcom: bam_dma: fixes for remotely controlled bam srinivas.kandagatla
2018-01-16 19:02 ` [PATCH 1/4] dmaengine: qcom: bam_dma: make bam clk optional srinivas.kandagatla
2018-01-16 19:38   ` Sagar Dharia
2018-01-17  9:46     ` Srinivas Kandagatla
2018-01-19  5:52   ` Vinod Koul
2018-01-22  9:55     ` Srinivas Kandagatla
     [not found]       ` <8dfa8ba1-6e98-a8e4-614c-592861cef571-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2018-01-23  9:19         ` Vinod Koul
2018-01-23  9:20           ` Srinivas Kandagatla
2018-01-16 19:02 ` [PATCH 3/4] dmaengine: qcom: bam_dma: do not write to global regs in remote mode srinivas.kandagatla
2018-01-16 19:02 ` [PATCH 4/4] dmaengine: qcom: bam_dma: Add num-ees dt binding for remotely controlled srinivas.kandagatla
     [not found]   ` <20180116190236.14558-5-srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2018-01-29 16:21     ` Rob Herring
2018-01-30  9:18       ` Srinivas Kandagatla
     [not found] ` <20180116190236.14558-1-srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2018-01-16 19:02   ` [PATCH 2/4] dmaengine: qcom: bam_dma: add num-channels " srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A
2018-01-19  5:55     ` Vinod Koul
2018-01-22  9:55       ` Srinivas Kandagatla
     [not found]     ` <20180116190236.14558-3-srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2018-01-29 16:19       ` Rob Herring
2018-01-17 10:18   ` [PATCH 0/4] dmaengine: qcom: bam_dma: fixes for remotely controlled bam Vinod Koul
2018-01-17 10:55     ` Srinivas Kandagatla
2018-01-17 15:59       ` Vinod Koul

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).