[PATCH v2 0/3] Tegra GCPDMA: Add dma-channel-mask support

[PATCH v2 0/3] Tegra GCPDMA: Add dma-channel-mask support

[Akhil R]

Read dma-channel-mask from device tree and register only the
specified channels. This is useful to reserve some channels for the
firmware.

Also update the channel number and interrupts to include all 32
channels. The current driver was using only 31 channels as one
channel was reserved for firmware. Now with this change, the driver
can align more to the actual hardware.

v1->v2:
  * Reversed the operands and used BIT macro in 'if' condition.
  * Fixed warning reported-by: kernel test robot <lkp@intel.com>

Akhil R (3):
  dt-bindings: dmaengine: Add dma-channel-mask to Tegra GPCDMA
  arm64: tegra: Add dma-channel-mask in GPCDMA node
  dmaengine: tegra: Add support for dma-channel-mask

 .../bindings/dma/nvidia,tegra186-gpc-dma.yaml |  7 +++-
 arch/arm64/boot/dts/nvidia/tegra186.dtsi      |  4 +-
 arch/arm64/boot/dts/nvidia/tegra194.dtsi      |  4 +-
 arch/arm64/boot/dts/nvidia/tegra234.dtsi      |  4 +-
 drivers/dma/tegra186-gpc-dma.c                | 37 +++++++++++++++----
 5 files changed, 45 insertions(+), 11 deletions(-)

-- 
2.17.1

[PATCH v2 1/3] dt-bindings: dmaengine: Add dma-channel-mask to Tegra GPCDMA

[Akhil R]

Add dma-channel-mask property in Tegra GPCDMA document.

The property would help to specify the channels to be used in
kernel and reserve few for the firmware. This was previously
achieved by limiting the channel number to 31 in the driver.
Now since we can list all 32 channels, update the interrupts
property as well to list all 32 interrupts.

Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
 .../devicetree/bindings/dma/nvidia,tegra186-gpc-dma.yaml   | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/dma/nvidia,tegra186-gpc-dma.yaml b/Documentation/devicetree/bindings/dma/nvidia,tegra186-gpc-dma.yaml
index 7e575296df0c..31724cda074e 100644
--- a/Documentation/devicetree/bindings/dma/nvidia,tegra186-gpc-dma.yaml
+++ b/Documentation/devicetree/bindings/dma/nvidia,tegra186-gpc-dma.yaml
@@ -39,7 +39,7 @@ properties:
       Should contain all of the per-channel DMA interrupts in
       ascending order with respect to the DMA channel index.
     minItems: 1
-    maxItems: 31
+    maxItems: 32
 
   resets:
     maxItems: 1
@@ -52,6 +52,9 @@ properties:
 
   dma-coherent: true
 
+  dma-channel-mask:
+    maxItems: 1
+
 required:
   - compatible
   - reg
@@ -60,6 +63,7 @@ required:
   - reset-names
   - "#dma-cells"
   - iommus
+  - dma-channel-mask
 
 additionalProperties: false
 
@@ -108,5 +112,6 @@ examples:
         #dma-cells = <1>;
         iommus = <&smmu TEGRA186_SID_GPCDMA_0>;
         dma-coherent;
+        dma-channel-mask = <0xfffffffe>;
     };
 ...
-- 
2.17.1

[PATCH v2 2/3] arm64: tegra: Add dma-channel-mask in GPCDMA node

[Akhil R]

Add dma-channel-mask property in Tegra GPCDMA device tree node.

The property would help to specify the channels to be used in
kernel and reserve few for the firmware. This was previously
achieved by limiting the channel number to 31 in the driver.
Now since we can list all 32 channels, update the interrupts
property as well to list all 32 interrupts.

Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
 arch/arm64/boot/dts/nvidia/tegra186.dtsi | 4 +++-
 arch/arm64/boot/dts/nvidia/tegra194.dtsi | 4 +++-
 arch/arm64/boot/dts/nvidia/tegra234.dtsi | 4 +++-
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
index 3580fbf99091..13a84e34e094 100644
--- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
@@ -78,7 +78,8 @@
 		reg = <0x0 0x2600000 0x0 0x210000>;
 		resets = <&bpmp TEGRA186_RESET_GPCDMA>;
 		reset-names = "gpcdma";
-		interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH>,
+		interrupts = <GIC_SPI 75 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH>,
 			     <GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH>,
 			     <GIC_SPI 78 IRQ_TYPE_LEVEL_HIGH>,
 			     <GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH>,
@@ -112,6 +113,7 @@
 		#dma-cells = <1>;
 		iommus = <&smmu TEGRA186_SID_GPCDMA_0>;
 		dma-coherent;
+		dma-channel-mask = <0xfffffffe>;
 		status = "okay";
 	};
 
diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
index 9176c4b27133..593fbf22b34f 100644
--- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
@@ -142,7 +142,8 @@
 			reg = <0x2600000 0x210000>;
 			resets = <&bpmp TEGRA194_RESET_GPCDMA>;
 			reset-names = "gpcdma";
-			interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH>,
+			interrupts = <GIC_SPI 75 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 78 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH>,
@@ -176,6 +177,7 @@
 			#dma-cells = <1>;
 			iommus = <&smmu TEGRA194_SID_GPCDMA_0>;
 			dma-coherent;
+			dma-channel-mask = <0xfffffffe>;
 			status = "okay";
 		};
 
diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
index 5852e765ad90..afd90b72cdea 100644
--- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
@@ -28,7 +28,8 @@
 			reg = <0x2600000 0x210000>;
 			resets = <&bpmp TEGRA234_RESET_GPCDMA>;
 			reset-names = "gpcdma";
-			interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH>,
+			interrupts = <GIC_SPI 75 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 78 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH>,
@@ -61,6 +62,7 @@
 				     <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>;
 			#dma-cells = <1>;
 			iommus = <&smmu_niso0 TEGRA234_SID_GPCDMA>;
+			dma-channel-mask = <0xfffffffe>;
 			dma-coherent;
 		};
 
-- 
2.17.1

[PATCH v2 3/3] dmaengine: tegra: Add support for dma-channel-mask

[Akhil R]

Add support for dma-channel-mask so that only the specified channels
are used. This helps to reserve some channels for the firmware.

This was initially achieved by limiting the channel number to 31 in
the driver and adjusting the register address to skip channel0 which
was reserved for a firmware. Now, with this change, the driver can
align more to the actual hardware which has 32 channels.

Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
---
 drivers/dma/tegra186-gpc-dma.c | 37 +++++++++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/tegra186-gpc-dma.c b/drivers/dma/tegra186-gpc-dma.c
index fa9bda4a2bc6..1d1180db6d4e 100644
--- a/drivers/dma/tegra186-gpc-dma.c
+++ b/drivers/dma/tegra186-gpc-dma.c
@@ -161,7 +161,10 @@
 #define TEGRA_GPCDMA_BURST_COMPLETION_TIMEOUT	5000 /* 5 msec */
 
 /* Channel base address offset from GPCDMA base address */
-#define TEGRA_GPCDMA_CHANNEL_BASE_ADD_OFFSET	0x20000
+#define TEGRA_GPCDMA_CHANNEL_BASE_ADDR_OFFSET	0x10000
+
+/* Default channel mask reserving channel0 */
+#define TEGRA_GPCDMA_DEFAULT_CHANNEL_MASK	0xfffffffe
 
 struct tegra_dma;
 struct tegra_dma_channel;
@@ -246,6 +249,7 @@ struct tegra_dma {
 	const struct tegra_dma_chip_data *chip_data;
 	unsigned long sid_m2d_reserved;
 	unsigned long sid_d2m_reserved;
+	u32 chan_mask;
 	void __iomem *base_addr;
 	struct device *dev;
 	struct dma_device dma_dev;
@@ -1288,7 +1292,7 @@ static struct dma_chan *tegra_dma_of_xlate(struct of_phandle_args *dma_spec,
 }
 
 static const struct tegra_dma_chip_data tegra186_dma_chip_data = {
-	.nr_channels = 31,
+	.nr_channels = 32,
 	.channel_reg_size = SZ_64K,
 	.max_dma_count = SZ_1G,
 	.hw_support_pause = false,
@@ -1296,7 +1300,7 @@ static const struct tegra_dma_chip_data tegra186_dma_chip_data = {
 };
 
 static const struct tegra_dma_chip_data tegra194_dma_chip_data = {
-	.nr_channels = 31,
+	.nr_channels = 32,
 	.channel_reg_size = SZ_64K,
 	.max_dma_count = SZ_1G,
 	.hw_support_pause = true,
@@ -1304,7 +1308,7 @@ static const struct tegra_dma_chip_data tegra194_dma_chip_data = {
 };
 
 static const struct tegra_dma_chip_data tegra234_dma_chip_data = {
-	.nr_channels = 31,
+	.nr_channels = 32,
 	.channel_reg_size = SZ_64K,
 	.max_dma_count = SZ_1G,
 	.hw_support_pause = true,
@@ -1380,15 +1384,28 @@ static int tegra_dma_probe(struct platform_device *pdev)
 	}
 	stream_id = iommu_spec->ids[0] & 0xffff;
 
+	ret = device_property_read_u32(&pdev->dev, "dma-channel-mask",
+				       &tdma->chan_mask);
+	if (ret) {
+		dev_warn(&pdev->dev,
+			 "Missing dma-channel-mask property, using default channel mask %#x\n",
+			 TEGRA_GPCDMA_DEFAULT_CHANNEL_MASK);
+		tdma->chan_mask = TEGRA_GPCDMA_DEFAULT_CHANNEL_MASK;
+	}
+
 	INIT_LIST_HEAD(&tdma->dma_dev.channels);
 	for (i = 0; i < cdata->nr_channels; i++) {
 		struct tegra_dma_channel *tdc = &tdma->channels[i];
 
+		/* Check for channel mask */
+		if (!(tdma->chan_mask & BIT(i)))
+			continue;
+
 		tdc->irq = platform_get_irq(pdev, i);
 		if (tdc->irq < 0)
 			return tdc->irq;
 
-		tdc->chan_base_offset = TEGRA_GPCDMA_CHANNEL_BASE_ADD_OFFSET +
+		tdc->chan_base_offset = TEGRA_GPCDMA_CHANNEL_BASE_ADDR_OFFSET +
 					i * cdata->channel_reg_size;
 		snprintf(tdc->name, sizeof(tdc->name), "gpcdma.%d", i);
 		tdc->tdma = tdma;
@@ -1449,8 +1466,8 @@ static int tegra_dma_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	dev_info(&pdev->dev, "GPC DMA driver register %d channels\n",
-		 cdata->nr_channels);
+	dev_info(&pdev->dev, "GPC DMA driver register %lu channels\n",
+		 hweight_long(tdma->chan_mask));
 
 	return 0;
 }
@@ -1473,6 +1490,9 @@ static int __maybe_unused tegra_dma_pm_suspend(struct device *dev)
 	for (i = 0; i < tdma->chip_data->nr_channels; i++) {
 		struct tegra_dma_channel *tdc = &tdma->channels[i];
 
+		if (!(tdma->chan_mask & BIT(i)))
+			continue;
+
 		if (tdc->dma_desc) {
 			dev_err(tdma->dev, "channel %u busy\n", i);
 			return -EBUSY;
@@ -1492,6 +1512,9 @@ static int __maybe_unused tegra_dma_pm_resume(struct device *dev)
 	for (i = 0; i < tdma->chip_data->nr_channels; i++) {
 		struct tegra_dma_channel *tdc = &tdma->channels[i];
 
+		if (!(tdma->chan_mask & BIT(i)))
+			continue;
+
 		tegra_dma_program_sid(tdc, tdc->stream_id);
 	}
 
-- 
2.17.1

Re: [PATCH v2 1/3] dt-bindings: dmaengine: Add dma-channel-mask to Tegra GPCDMA

[Jon Hunter]

On 19/09/2022 12:25, Akhil R wrote:
> Add dma-channel-mask property in Tegra GPCDMA document.
> 
> The property would help to specify the channels to be used in
> kernel and reserve few for the firmware. This was previously
> achieved by limiting the channel number to 31 in the driver.
> Now since we can list all 32 channels, update the interrupts
> property as well to list all 32 interrupts.
> 
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
> ---
>   .../devicetree/bindings/dma/nvidia,tegra186-gpc-dma.yaml   | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/dma/nvidia,tegra186-gpc-dma.yaml b/Documentation/devicetree/bindings/dma/nvidia,tegra186-gpc-dma.yaml
> index 7e575296df0c..31724cda074e 100644
> --- a/Documentation/devicetree/bindings/dma/nvidia,tegra186-gpc-dma.yaml
> +++ b/Documentation/devicetree/bindings/dma/nvidia,tegra186-gpc-dma.yaml
> @@ -39,7 +39,7 @@ properties:
>         Should contain all of the per-channel DMA interrupts in
>         ascending order with respect to the DMA channel index.
>       minItems: 1
> -    maxItems: 31
> +    maxItems: 32
>   
>     resets:
>       maxItems: 1
> @@ -52,6 +52,9 @@ properties:
>   
>     dma-coherent: true
>   
> +  dma-channel-mask:
> +    maxItems: 1
> +
>   required:
>     - compatible
>     - reg
> @@ -60,6 +63,7 @@ required:
>     - reset-names
>     - "#dma-cells"
>     - iommus
> +  - dma-channel-mask
>   
>   additionalProperties: false
>   
> @@ -108,5 +112,6 @@ examples:
>           #dma-cells = <1>;
>           iommus = <&smmu TEGRA186_SID_GPCDMA_0>;
>           dma-coherent;
> +        dma-channel-mask = <0xfffffffe>;
>       };
>   ...


Reviewed-by: Jon Hunter <jonathanh@nvidia.com>

Thanks!
Jon

-- 
nvpublic

Re: [PATCH v2 2/3] arm64: tegra: Add dma-channel-mask in GPCDMA node

[Jon Hunter]

On 19/09/2022 12:25, Akhil R wrote:
> Add dma-channel-mask property in Tegra GPCDMA device tree node.
> 
> The property would help to specify the channels to be used in
> kernel and reserve few for the firmware. This was previously
> achieved by limiting the channel number to 31 in the driver.
> Now since we can list all 32 channels, update the interrupts
> property as well to list all 32 interrupts.
> 
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
> ---
>   arch/arm64/boot/dts/nvidia/tegra186.dtsi | 4 +++-
>   arch/arm64/boot/dts/nvidia/tegra194.dtsi | 4 +++-
>   arch/arm64/boot/dts/nvidia/tegra234.dtsi | 4 +++-
>   3 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
> index 3580fbf99091..13a84e34e094 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
> @@ -78,7 +78,8 @@
>   		reg = <0x0 0x2600000 0x0 0x210000>;
>   		resets = <&bpmp TEGRA186_RESET_GPCDMA>;
>   		reset-names = "gpcdma";
> -		interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH>,
> +		interrupts = <GIC_SPI 75 IRQ_TYPE_LEVEL_HIGH>,
> +			     <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH>,
>   			     <GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH>,
>   			     <GIC_SPI 78 IRQ_TYPE_LEVEL_HIGH>,
>   			     <GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH>,
> @@ -112,6 +113,7 @@
>   		#dma-cells = <1>;
>   		iommus = <&smmu TEGRA186_SID_GPCDMA_0>;
>   		dma-coherent;
> +		dma-channel-mask = <0xfffffffe>;
>   		status = "okay";
>   	};
>   
> diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
> index 9176c4b27133..593fbf22b34f 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
> @@ -142,7 +142,8 @@
>   			reg = <0x2600000 0x210000>;
>   			resets = <&bpmp TEGRA194_RESET_GPCDMA>;
>   			reset-names = "gpcdma";
> -			interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH>,
> +			interrupts = <GIC_SPI 75 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH>,
>   				     <GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH>,
>   				     <GIC_SPI 78 IRQ_TYPE_LEVEL_HIGH>,
>   				     <GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH>,
> @@ -176,6 +177,7 @@
>   			#dma-cells = <1>;
>   			iommus = <&smmu TEGRA194_SID_GPCDMA_0>;
>   			dma-coherent;
> +			dma-channel-mask = <0xfffffffe>;
>   			status = "okay";
>   		};
>   
> diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> index 5852e765ad90..afd90b72cdea 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> @@ -28,7 +28,8 @@
>   			reg = <0x2600000 0x210000>;
>   			resets = <&bpmp TEGRA234_RESET_GPCDMA>;
>   			reset-names = "gpcdma";
> -			interrupts = <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH>,
> +			interrupts = <GIC_SPI 75 IRQ_TYPE_LEVEL_HIGH>,
> +				     <GIC_SPI 76 IRQ_TYPE_LEVEL_HIGH>,
>   				     <GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH>,
>   				     <GIC_SPI 78 IRQ_TYPE_LEVEL_HIGH>,
>   				     <GIC_SPI 79 IRQ_TYPE_LEVEL_HIGH>,
> @@ -61,6 +62,7 @@
>   				     <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>;
>   			#dma-cells = <1>;
>   			iommus = <&smmu_niso0 TEGRA234_SID_GPCDMA>;
> +			dma-channel-mask = <0xfffffffe>;
>   			dma-coherent;
>   		};
>   


Reviewed-by: Jon Hunter <jonathanh@nvidia.com>

Thanks!
Jon

-- 
nvpublic

Re: [PATCH v2 3/3] dmaengine: tegra: Add support for dma-channel-mask

[Jon Hunter]

On 19/09/2022 12:25, Akhil R wrote:
> Add support for dma-channel-mask so that only the specified channels
> are used. This helps to reserve some channels for the firmware.
> 
> This was initially achieved by limiting the channel number to 31 in
> the driver and adjusting the register address to skip channel0 which
> was reserved for a firmware. Now, with this change, the driver can
> align more to the actual hardware which has 32 channels.
> 
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
> ---
>   drivers/dma/tegra186-gpc-dma.c | 37 +++++++++++++++++++++++++++-------
>   1 file changed, 30 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/dma/tegra186-gpc-dma.c b/drivers/dma/tegra186-gpc-dma.c
> index fa9bda4a2bc6..1d1180db6d4e 100644
> --- a/drivers/dma/tegra186-gpc-dma.c
> +++ b/drivers/dma/tegra186-gpc-dma.c
> @@ -161,7 +161,10 @@
>   #define TEGRA_GPCDMA_BURST_COMPLETION_TIMEOUT	5000 /* 5 msec */
>   
>   /* Channel base address offset from GPCDMA base address */
> -#define TEGRA_GPCDMA_CHANNEL_BASE_ADD_OFFSET	0x20000
> +#define TEGRA_GPCDMA_CHANNEL_BASE_ADDR_OFFSET	0x10000

Why did this value change? There is no mention in the commit message. If 
this was incorrect before, then this needs to be a separate patch and 
tagged with the appropriate fixes tag so that this can be picked up for 
stable.

Thanks
Jon

-- 
nvpublic

RE: [PATCH v2 3/3] dmaengine: tegra: Add support for dma-channel-mask

[Akhil R]

> On 19/09/2022 12:25, Akhil R wrote:
> > Add support for dma-channel-mask so that only the specified channels
> > are used. This helps to reserve some channels for the firmware.
> >
> > This was initially achieved by limiting the channel number to 31 in
> > the driver and adjusting the register address to skip channel0 which
> > was reserved for a firmware. Now, with this change, the driver can
> > align more to the actual hardware which has 32 channels.
> >
> > Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
> > ---
> >   drivers/dma/tegra186-gpc-dma.c | 37 +++++++++++++++++++++++++++-------
> >   1 file changed, 30 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/dma/tegra186-gpc-dma.c b/drivers/dma/tegra186-gpc-
> dma.c
> > index fa9bda4a2bc6..1d1180db6d4e 100644
> > --- a/drivers/dma/tegra186-gpc-dma.c
> > +++ b/drivers/dma/tegra186-gpc-dma.c
> > @@ -161,7 +161,10 @@
> >   #define TEGRA_GPCDMA_BURST_COMPLETION_TIMEOUT	5000 /* 5
> msec */
> >
> >   /* Channel base address offset from GPCDMA base address */
> > -#define TEGRA_GPCDMA_CHANNEL_BASE_ADD_OFFSET	0x20000
> > +#define TEGRA_GPCDMA_CHANNEL_BASE_ADDR_OFFSET	0x10000
> 
> Why did this value change? There is no mention in the commit message. If
> this was incorrect before, then this needs to be a separate patch and
> tagged with the appropriate fixes tag so that this can be picked up for
> stable.
This is mentioned in the commit message. 

"... and adjusting the register address to skip channel0 ..."

Probably it is not very clear that it directs to this change. Shall I update the
commit message to have a clearer context?

Regards,
Akhil

Re: [PATCH v2 3/3] dmaengine: tegra: Add support for dma-channel-mask

[Jon Hunter]

On 23/09/2022 11:17, Akhil R wrote:
>> On 19/09/2022 12:25, Akhil R wrote:
>>> Add support for dma-channel-mask so that only the specified channels
>>> are used. This helps to reserve some channels for the firmware.
>>>
>>> This was initially achieved by limiting the channel number to 31 in
>>> the driver and adjusting the register address to skip channel0 which
>>> was reserved for a firmware. Now, with this change, the driver can
>>> align more to the actual hardware which has 32 channels.
>>>
>>> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
>>> ---
>>>    drivers/dma/tegra186-gpc-dma.c | 37 +++++++++++++++++++++++++++-------
>>>    1 file changed, 30 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/dma/tegra186-gpc-dma.c b/drivers/dma/tegra186-gpc-
>> dma.c
>>> index fa9bda4a2bc6..1d1180db6d4e 100644
>>> --- a/drivers/dma/tegra186-gpc-dma.c
>>> +++ b/drivers/dma/tegra186-gpc-dma.c
>>> @@ -161,7 +161,10 @@
>>>    #define TEGRA_GPCDMA_BURST_COMPLETION_TIMEOUT	5000 /* 5
>> msec */
>>>
>>>    /* Channel base address offset from GPCDMA base address */
>>> -#define TEGRA_GPCDMA_CHANNEL_BASE_ADD_OFFSET	0x20000
>>> +#define TEGRA_GPCDMA_CHANNEL_BASE_ADDR_OFFSET	0x10000
>>
>> Why did this value change? There is no mention in the commit message. If
>> this was incorrect before, then this needs to be a separate patch and
>> tagged with the appropriate fixes tag so that this can be picked up for
>> stable.
> This is mentioned in the commit message.
> 
> "... and adjusting the register address to skip channel0 ..."
> 
> Probably it is not very clear that it directs to this change. Shall I update the
> commit message to have a clearer context?

Ah OK. I was wondering how this worked with 'channel_reg_size' but 
looking closer I see channel_reg_size is always SZ_64K. I wonder why we 
even bother having this parameter and don't use SZ_64K directly?

Anyway, for now this is fine.

Jon

-- 
nvpublic

Re: [PATCH v2 3/3] dmaengine: tegra: Add support for dma-channel-mask

[Jon Hunter]

On 19/09/2022 12:25, Akhil R wrote:
> Add support for dma-channel-mask so that only the specified channels
> are used. This helps to reserve some channels for the firmware.
> 
> This was initially achieved by limiting the channel number to 31 in
> the driver and adjusting the register address to skip channel0 which
> was reserved for a firmware. Now, with this change, the driver can
> align more to the actual hardware which has 32 channels.
> 
> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
> ---
>   drivers/dma/tegra186-gpc-dma.c | 37 +++++++++++++++++++++++++++-------
>   1 file changed, 30 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/dma/tegra186-gpc-dma.c b/drivers/dma/tegra186-gpc-dma.c
> index fa9bda4a2bc6..1d1180db6d4e 100644
> --- a/drivers/dma/tegra186-gpc-dma.c
> +++ b/drivers/dma/tegra186-gpc-dma.c
> @@ -161,7 +161,10 @@
>   #define TEGRA_GPCDMA_BURST_COMPLETION_TIMEOUT	5000 /* 5 msec */
>   
>   /* Channel base address offset from GPCDMA base address */
> -#define TEGRA_GPCDMA_CHANNEL_BASE_ADD_OFFSET	0x20000
> +#define TEGRA_GPCDMA_CHANNEL_BASE_ADDR_OFFSET	0x10000
> +
> +/* Default channel mask reserving channel0 */
> +#define TEGRA_GPCDMA_DEFAULT_CHANNEL_MASK	0xfffffffe
>   
>   struct tegra_dma;
>   struct tegra_dma_channel;
> @@ -246,6 +249,7 @@ struct tegra_dma {
>   	const struct tegra_dma_chip_data *chip_data;
>   	unsigned long sid_m2d_reserved;
>   	unsigned long sid_d2m_reserved;
> +	u32 chan_mask;
>   	void __iomem *base_addr;
>   	struct device *dev;
>   	struct dma_device dma_dev;
> @@ -1288,7 +1292,7 @@ static struct dma_chan *tegra_dma_of_xlate(struct of_phandle_args *dma_spec,
>   }
>   
>   static const struct tegra_dma_chip_data tegra186_dma_chip_data = {
> -	.nr_channels = 31,
> +	.nr_channels = 32,
>   	.channel_reg_size = SZ_64K,
>   	.max_dma_count = SZ_1G,
>   	.hw_support_pause = false,
> @@ -1296,7 +1300,7 @@ static const struct tegra_dma_chip_data tegra186_dma_chip_data = {
>   };
>   
>   static const struct tegra_dma_chip_data tegra194_dma_chip_data = {
> -	.nr_channels = 31,
> +	.nr_channels = 32,
>   	.channel_reg_size = SZ_64K,
>   	.max_dma_count = SZ_1G,
>   	.hw_support_pause = true,
> @@ -1304,7 +1308,7 @@ static const struct tegra_dma_chip_data tegra194_dma_chip_data = {
>   };
>   
>   static const struct tegra_dma_chip_data tegra234_dma_chip_data = {
> -	.nr_channels = 31,
> +	.nr_channels = 32,
>   	.channel_reg_size = SZ_64K,
>   	.max_dma_count = SZ_1G,
>   	.hw_support_pause = true,
> @@ -1380,15 +1384,28 @@ static int tegra_dma_probe(struct platform_device *pdev)
>   	}
>   	stream_id = iommu_spec->ids[0] & 0xffff;
>   
> +	ret = device_property_read_u32(&pdev->dev, "dma-channel-mask",
> +				       &tdma->chan_mask);
> +	if (ret) {
> +		dev_warn(&pdev->dev,
> +			 "Missing dma-channel-mask property, using default channel mask %#x\n",
> +			 TEGRA_GPCDMA_DEFAULT_CHANNEL_MASK);
> +		tdma->chan_mask = TEGRA_GPCDMA_DEFAULT_CHANNEL_MASK;
> +	}
> +
>   	INIT_LIST_HEAD(&tdma->dma_dev.channels);
>   	for (i = 0; i < cdata->nr_channels; i++) {
>   		struct tegra_dma_channel *tdc = &tdma->channels[i];
>   
> +		/* Check for channel mask */
> +		if (!(tdma->chan_mask & BIT(i)))
> +			continue;
> +
>   		tdc->irq = platform_get_irq(pdev, i);
>   		if (tdc->irq < 0)
>   			return tdc->irq;
>   
> -		tdc->chan_base_offset = TEGRA_GPCDMA_CHANNEL_BASE_ADD_OFFSET +
> +		tdc->chan_base_offset = TEGRA_GPCDMA_CHANNEL_BASE_ADDR_OFFSET +
>   					i * cdata->channel_reg_size;
>   		snprintf(tdc->name, sizeof(tdc->name), "gpcdma.%d", i);
>   		tdc->tdma = tdma;
> @@ -1449,8 +1466,8 @@ static int tegra_dma_probe(struct platform_device *pdev)
>   		return ret;
>   	}
>   
> -	dev_info(&pdev->dev, "GPC DMA driver register %d channels\n",
> -		 cdata->nr_channels);
> +	dev_info(&pdev->dev, "GPC DMA driver register %lu channels\n",
> +		 hweight_long(tdma->chan_mask));
>   
>   	return 0;
>   }
> @@ -1473,6 +1490,9 @@ static int __maybe_unused tegra_dma_pm_suspend(struct device *dev)
>   	for (i = 0; i < tdma->chip_data->nr_channels; i++) {
>   		struct tegra_dma_channel *tdc = &tdma->channels[i];
>   
> +		if (!(tdma->chan_mask & BIT(i)))
> +			continue;
> +
>   		if (tdc->dma_desc) {
>   			dev_err(tdma->dev, "channel %u busy\n", i);
>   			return -EBUSY;
> @@ -1492,6 +1512,9 @@ static int __maybe_unused tegra_dma_pm_resume(struct device *dev)
>   	for (i = 0; i < tdma->chip_data->nr_channels; i++) {
>   		struct tegra_dma_channel *tdc = &tdma->channels[i];
>   
> +		if (!(tdma->chan_mask & BIT(i)))
> +			continue;
> +
>   		tegra_dma_program_sid(tdc, tdc->stream_id);
>   	}
>   


Reviewed-by: Jon Hunter <jonathanh@nvidia.com>

Thanks!
Jon

-- 
nvpublic

RE: [PATCH v2 3/3] dmaengine: tegra: Add support for dma-channel-mask

[Akhil R]

> On 23/09/2022 11:17, Akhil R wrote:
> >> On 19/09/2022 12:25, Akhil R wrote:
> >>> Add support for dma-channel-mask so that only the specified channels
> >>> are used. This helps to reserve some channels for the firmware.
> >>>
> >>> This was initially achieved by limiting the channel number to 31 in
> >>> the driver and adjusting the register address to skip channel0 which
> >>> was reserved for a firmware. Now, with this change, the driver can
> >>> align more to the actual hardware which has 32 channels.
> >>>
> >>> Signed-off-by: Akhil R <akhilrajeev@nvidia.com>
> >>> ---
> >>>    drivers/dma/tegra186-gpc-dma.c | 37 +++++++++++++++++++++++++++---
> ----
> >>>    1 file changed, 30 insertions(+), 7 deletions(-)
> >>>
> >>> diff --git a/drivers/dma/tegra186-gpc-dma.c b/drivers/dma/tegra186-gpc-
> >> dma.c
> >>> index fa9bda4a2bc6..1d1180db6d4e 100644
> >>> --- a/drivers/dma/tegra186-gpc-dma.c
> >>> +++ b/drivers/dma/tegra186-gpc-dma.c
> >>> @@ -161,7 +161,10 @@
> >>>    #define TEGRA_GPCDMA_BURST_COMPLETION_TIMEOUT	5000 /* 5
> >> msec */
> >>>
> >>>    /* Channel base address offset from GPCDMA base address */
> >>> -#define TEGRA_GPCDMA_CHANNEL_BASE_ADD_OFFSET	0x20000
> >>> +#define TEGRA_GPCDMA_CHANNEL_BASE_ADDR_OFFSET	0x10000
> >>
> >> Why did this value change? There is no mention in the commit message. If
> >> this was incorrect before, then this needs to be a separate patch and
> >> tagged with the appropriate fixes tag so that this can be picked up for
> >> stable.
> > This is mentioned in the commit message.
> >
> > "... and adjusting the register address to skip channel0 ..."
> >
> > Probably it is not very clear that it directs to this change. Shall I update the
> > commit message to have a clearer context?
> 
> Ah OK. I was wondering how this worked with 'channel_reg_size' but
> looking closer I see channel_reg_size is always SZ_64K. I wonder why we
> even bother having this parameter and don't use SZ_64K directly?
There is an offset from the base address which the per channel registers start.
Although this offset value happens to match with the channel_reg_size, this is
not actually the per channel register size.
> 
> Anyway, for now this is fine.
> 
Thanks for the review.

Regards,
Akhil

Re: [PATCH v2 3/3] dmaengine: tegra: Add support for dma-channel-mask

[Jon Hunter]

On 23/09/2022 12:09, Akhil R wrote:

...

>> Ah OK. I was wondering how this worked with 'channel_reg_size' but
>> looking closer I see channel_reg_size is always SZ_64K. I wonder why we
>> even bother having this parameter and don't use SZ_64K directly?
> There is an offset from the base address which the per channel registers start.
> Although this offset value happens to match with the channel_reg_size, this is
> not actually the per channel register size.

Yes I see that, but I mean why do we even bother having this 
channel_reg_size parameter? Does not look like we need this (currently). 
All we need is ...

  tdc->chan_base_offset = TEGRA_GPCDMA_CHANNEL_BASE_ADDR_OFFSET +
                          (i * SZ_64K);

Jon

-- 
nvpublic

RE: [PATCH v2 3/3] dmaengine: tegra: Add support for dma-channel-mask

[Akhil R]

> >> Ah OK. I was wondering how this worked with 'channel_reg_size' but
> >> looking closer I see channel_reg_size is always SZ_64K. I wonder why we
> >> even bother having this parameter and don't use SZ_64K directly?
> > There is an offset from the base address which the per channel registers start.
> > Although this offset value happens to match with the channel_reg_size, this is
> > not actually the per channel register size.
> 
> Yes I see that, but I mean why do we even bother having this
> channel_reg_size parameter? Does not look like we need this (currently).
> All we need is ...
> 
>   tdc->chan_base_offset = TEGRA_GPCDMA_CHANNEL_BASE_ADDR_OFFSET +
>                           (i * SZ_64K);
> 
Ah. Ok. Got it now. Would add this as an improvement in another patch.

Thanks,
Akhil