[PATCH 0/2] dmaengine: at_xdmac: retrieve DMA channels from device tree

[PATCH 0/2] dmaengine: at_xdmac: retrieve DMA channels from device tree

[Dharma Balasubiramani]

This patch series adds support to get the number of DMA channels available in
XDMAC from dts. This property is required when the channel count cannot be read
from the XDMAC_GTYPE register (which occurs when accessing from non-secure
world on certain devices)

Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
---
Dharma Balasubiramani (1):
      dt-bindings: dma: at_xdmac: document dma-channels property

Tony Han (1):
      dmaengine: at_xdmac: get the number of DMA channels from device tree

 .../devicetree/bindings/dma/atmel,sama5d4-dma.yaml | 26 ++++++++++++++--------
 drivers/dma/at_xdmac.c                             | 26 +++++++++++++++++++---
 2 files changed, 40 insertions(+), 12 deletions(-)
---
base-commit: 40b8e93e17bff4a4e0cc129e04f9fdf5daa5397e
change-id: 20250205-mchp-dma-355c838f26f4

Best regards,
-- 
Dharma Balasubiramani <dharma.b@microchip.com>

[PATCH 1/2] dmaengine: at_xdmac: get the number of DMA channels from device tree

[Dharma Balasubiramani]

From: Tony Han <tony.han@microchip.com>

In case of kernel runs in non-secure mode, the number of DMA channels can be
got from device tree since the value read from GTYPE register is "0" as it's
always secured. As the number of channels can never be negative, update them
to the type "unsigned".

Signed-off-by: Tony Han <tony.han@microchip.com>
Reviewed-by: Cristian Birsan <cristian.birsan@microchip.com>
---
 drivers/dma/at_xdmac.c | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c
index ba25c23164e7..f777b0665c63 100644
--- a/drivers/dma/at_xdmac.c
+++ b/drivers/dma/at_xdmac.c
@@ -2259,12 +2259,29 @@ static int __maybe_unused atmel_xdmac_runtime_resume(struct device *dev)
 	return clk_enable(atxdmac->clk);
 }
 
+static inline int at_xdmac_get_channel_number(struct platform_device *pdev,
+					      u32 reg, u32 *pchannels)
+{
+	int ret;
+
+	if (reg) {
+		*pchannels = AT_XDMAC_NB_CH(reg);
+		return 0;
+	}
+
+	ret = of_property_read_u32(pdev->dev.of_node, "dma-channels", pchannels);
+	if (ret)
+		dev_err(&pdev->dev, "can't get number of channels\n");
+
+	return ret;
+}
+
 static int at_xdmac_probe(struct platform_device *pdev)
 {
 	struct at_xdmac	*atxdmac;
-	int		irq, nr_channels, i, ret;
+	int		irq, ret;
 	void __iomem	*base;
-	u32		reg;
+	u32		nr_channels, i, reg;
 
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0)
@@ -2280,7 +2297,10 @@ static int at_xdmac_probe(struct platform_device *pdev)
 	 * of channels to do the allocation.
 	 */
 	reg = readl_relaxed(base + AT_XDMAC_GTYPE);
-	nr_channels = AT_XDMAC_NB_CH(reg);
+	ret = at_xdmac_get_channel_number(pdev, reg, &nr_channels);
+	if (ret)
+		return ret;
+
 	if (nr_channels > AT_XDMAC_MAX_CHAN) {
 		dev_err(&pdev->dev, "invalid number of channels (%u)\n",
 			nr_channels);

-- 
2.43.0

[PATCH 2/2] dt-bindings: dma: at_xdmac: document dma-channels property

[Dharma Balasubiramani]

Add document for the property "dma-channels" for XDMA controller.

Also reorder properties to group related items together.

Signed-off-by: Tony Han <tony.han@microchip.com>
Reviewed-by: Cristian Birsan <cristian.birsan@microchip.com>
Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
---
 .../devicetree/bindings/dma/atmel,sama5d4-dma.yaml | 26 ++++++++++++++--------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/dma/atmel,sama5d4-dma.yaml b/Documentation/devicetree/bindings/dma/atmel,sama5d4-dma.yaml
index 9ca1c5d1f00f..b9fda35d2138 100644
--- a/Documentation/devicetree/bindings/dma/atmel,sama5d4-dma.yaml
+++ b/Documentation/devicetree/bindings/dma/atmel,sama5d4-dma.yaml
@@ -33,15 +33,6 @@ properties:
               - microchip,sam9x7-dma
           - const: atmel,sama5d4-dma
 
-  "#dma-cells":
-    description: |
-      Represents the number of integer cells in the `dmas` property of client
-      devices. The single cell specifies the channel configuration register:
-        - bit 13: SIF (Source Interface Identifier) for memory interface.
-        - bit 14: DIF (Destination Interface Identifier) for peripheral interface.
-        - bit 30-24: PERID (Peripheral Identifier).
-    const: 1
-
   reg:
     maxItems: 1
 
@@ -54,6 +45,23 @@ properties:
   clock-names:
     const: dma_clk
 
+  "#dma-cells":
+    description: |
+      Represents the number of integer cells in the `dmas` property of client
+      devices. The single cell specifies the channel configuration register:
+        - bit 13: SIF (Source Interface Identifier) for memory interface.
+        - bit 14: DIF (Destination Interface Identifier) for peripheral interface.
+        - bit 30-24: PERID (Peripheral Identifier).
+    const: 1
+
+  dma-channels:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Represents the number of DMA channels available in XDMA controller. This
+      property is required when the channel count cannot be read from the
+      XDMAC_GTYPE register (which occurs when accessing from non-secure world
+      on certain devices).
+
 required:
   - compatible
   - reg

-- 
2.43.0

Re: [PATCH 2/2] dt-bindings: dma: at_xdmac: document dma-channels property

[Krzysztof Kozlowski]

On Wed, Feb 05, 2025 at 11:17:03AM +0530, Dharma Balasubiramani wrote:
> Add document for the property "dma-channels" for XDMA controller.

I don't understand why. You are duplicating dma schema.

The same as with other patch - your commit msg is redundant. You say
what we see the diff but you never explain why you are doing these
changes. And in both cases this is really non-obvious.

Apply this feedback to all future contributions - say why you are doing
changes instead of repeating what subject and diff are already saying.

Best regards,
Krzysztof

Re: [PATCH 2/2] dt-bindings: dma: at_xdmac: document dma-channels property

[Dharma.B]

Hi Krzysztof,

On 05/02/25 5:10 pm, Krzysztof Kozlowski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Wed, Feb 05, 2025 at 11:17:03AM +0530, Dharma Balasubiramani wrote:
>> Add document for the property "dma-channels" for XDMA controller.
> 
> I don't understand why. You are duplicating dma schema.
> 
> The same as with other patch - your commit msg is redundant. You say
> what we see the diff but you never explain why you are doing these
> changes. And in both cases this is really non-obvious.
> 
> Apply this feedback to all future contributions - say why you are doing
> changes instead of repeating what subject and diff are already saying.

Thanks for your feedback, do you want me to include the description of 
dma-channels from the diff in the commit message?

The reason for this change is:
"This property is required when the channel count cannot be read from 
the XDMAC_GTYPE register (which occurs when accessing from the 
non-secure world on certain devices)."

Isn't this already clear from the description in the diff?

> 
> Best regards,
> Krzysztof
> 


-- 
With Best Regards,
Dharma B.

Re: [PATCH 2/2] dt-bindings: dma: at_xdmac: document dma-channels property

[Krzysztof Kozlowski]

On 05/02/2025 14:58, Dharma.B@microchip.com wrote:
> Hi Krzysztof,
> 
> On 05/02/25 5:10 pm, Krzysztof Kozlowski wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> On Wed, Feb 05, 2025 at 11:17:03AM +0530, Dharma Balasubiramani wrote:
>>> Add document for the property "dma-channels" for XDMA controller.
>>
>> I don't understand why. You are duplicating dma schema.

Here-----------------------^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

>>
>> The same as with other patch - your commit msg is redundant. You say
>> what we see the diff but you never explain why you are doing these
>> changes. And in both cases this is really non-obvious.
>>
>> Apply this feedback to all future contributions - say why you are doing
>> changes instead of repeating what subject and diff are already saying.
> 
> Thanks for your feedback, do you want me to include the description of 
> dma-channels from the diff in the commit message?
> 
> The reason for this change is:
> "This property is required when the channel count cannot be read from 
> the XDMAC_GTYPE register (which occurs when accessing from the 
> non-secure world on certain devices)."
> 
> Isn't this already clear from the description in the diff?


OK, so this describes why you need this. It does not solve why do you
need this patch. Do you see any warnings?

Best regards,
Krzysztof

Re: [PATCH 2/2] dt-bindings: dma: at_xdmac: document dma-channels property

[Dharma.B]

Hi Krzysztof,

On 05/02/25 7:46 pm, Krzysztof Kozlowski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 05/02/2025 14:58, Dharma.B@microchip.com wrote:
>> Hi Krzysztof,
>>
>> On 05/02/25 5:10 pm, Krzysztof Kozlowski wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> On Wed, Feb 05, 2025 at 11:17:03AM +0530, Dharma Balasubiramani wrote:
>>>> Add document for the property "dma-channels" for XDMA controller.
>>>
>>> I don't understand why. You are duplicating dma schema.
> 
> Here-----------------------^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Thanks for pointing it out.

> 
>>>
>>> The same as with other patch - your commit msg is redundant. You say
>>> what we see the diff but you never explain why you are doing these
>>> changes. And in both cases this is really non-obvious.
>>>
>>> Apply this feedback to all future contributions - say why you are doing
>>> changes instead of repeating what subject and diff are already saying.
>>
>> Thanks for your feedback, do you want me to include the description of
>> dma-channels from the diff in the commit message?
>>
>> The reason for this change is:
>> "This property is required when the channel count cannot be read from
>> the XDMAC_GTYPE register (which occurs when accessing from the
>> non-secure world on certain devices)."
>>
>> Isn't this already clear from the description in the diff?
> 
> 
> OK, so this describes why you need this. It does not solve why do you
> need this patch. Do you see any warnings?

No, I don’t see any warnings because this property is already defined in 
`dma-common.yaml`, which this binding references. You’re right—I’m 
unnecessarily duplicating the DMA schema. Please drop this patch. 
Apologies for the inconvenience.

> 
> Best regards,
> Krzysztof
> 


-- 
With Best Regards,
Dharma B.