diff mbox series

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

Message ID 20250205-mchp-dma-v1-2-124b639d5afe@microchip.com (mailing list archive)
State New
Headers show
Series dmaengine: at_xdmac: retrieve DMA channels from device tree | expand

Commit Message

Dharma Balasubiramani Feb. 5, 2025, 5:47 a.m. UTC
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(-)

Comments

Krzysztof Kozlowski Feb. 5, 2025, 11:40 a.m. UTC | #1
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
Krzysztof Kozlowski Feb. 5, 2025, 2:16 p.m. UTC | #2
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
diff mbox series

Patch

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