Re: [PATCH v9 1/3] dt-bindings: dma: Add Amlogic A9 SoC DMA

From: Xianwei Zhao <xianwei.zhao@amlogic.com>
To: sashiko-reviews@lists.linux.dev,
	Xianwei Zhao via B4 Relay
	<devnull+xianwei.zhao.amlogic.com@kernel.org>
Cc: Frank.Li@kernel.org, robh@kernel.org, neil.armstrong@linaro.org,
	conor+dt@kernel.org, vkoul@kernel.org,
	linux-amlogic@lists.infradead.org, dmaengine@vger.kernel.org,
	devicetree@vger.kernel.org
Subject: Re: [PATCH v9 1/3] dt-bindings: dma: Add Amlogic A9 SoC DMA
Date: Tue, 30 Jun 2026 13:05:44 +0800	[thread overview]
Message-ID: <dfa55ecd-f86a-4452-851d-8138b250912f@amlogic.com> (raw)
In-Reply-To: <20260626055033.061BD1F000E9@smtp.kernel.org>


On 2026/6/26 13:50, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [Low] Global dt-binding header defines generic 'DMA_TX' and 'DMA_RX' macros, risking namespace collisions.
> - [Low] The `#dma-cells` property lacks a description explaining the layout and purpose of its 2 cells.
> - [Low] Missing space before the opening brace in the device tree example node.
> --
> 
> commit a7b4f5e4e6207e7134f0b399abc853db92908381
> Author: Xianwei Zhao<xianwei.zhao@amlogic.com>
> 
> dt-bindings: dma: Add Amlogic A9 SoC DMA
> 
> This commit adds device tree binding documentation for the Amlogic A9 SoC
> general-purpose DMA controller. It also introduces a new header file containing
> specific property values for this controller.
> 
>> diff --git a/Documentation/devicetree/bindings/dma/amlogic,a9-dma.yaml b/Documentation/devicetree/bindings/dma/amlogic,a9-dma.yaml
>> new file mode 100644
>> index 0000000000000..efd7b2602c336
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/dma/amlogic,a9-dma.yaml
> [ ... ]
> 
>> @@ -0,0 +1,65 @@
>> +properties:
>> +  compatible:
>> +    const: amlogic,a9-dma
> [ ... ]
> 
>> +  clock-names:
>> +    const: sys
>> +
>> +  '#dma-cells':
>> +    const: 2
> [Severity: Low]
> Is a description needed for this `#dma-cells` property to explain the layout
> and purpose of its 2 cells? Without it, device tree writers won't have
> documentation clarifying which cell represents the channel ID and which
> represents the direction.
> 
Will add it.
>> +
>> +  dma-channels:
>> +    maximum: 64
> [ ... ]
> 
>> +examples:
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    dma-controller@fe400000{
> [Severity: Low]
> This isn't a bug, but should there be a space before the opening brace here?
> Normally it is formatted as `dma-controller@fe400000 {`.
> 
Will add space.
>> +        compatible = "amlogic,a9-dma";
>> +        reg = <0xfe400000 0x4000>;
>> +        interrupts = <GIC_SPI 35 IRQ_TYPE_EDGE_RISING>;
>> +        clocks = <&clkc 45>;
>> +        #dma-cells = <2>;
>> +        dma-channels = <28>;
>> +    };
>> diff --git a/include/dt-bindings/dma/amlogic,a9-dma.h b/include/dt-bindings/dma/amlogic,a9-dma.h
>> new file mode 100644
>> index 0000000000000..c59c2fd4b9561
>> --- /dev/null
>> +++ b/include/dt-bindings/dma/amlogic,a9-dma.h
>> @@ -0,0 +1,8 @@
>> +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
>> +
>> +#ifndef __DT_BINDINGS_DMA_AMLOGIC_DMA_H__
>> +#define __DT_BINDINGS_DMA_AMLOGIC_DMA_H__
>> +
>> +#define DMA_TX                       0
>> +#define DMA_RX                       1
> [Severity: Low]
> Could these macro names lead to namespace collisions since they lack a vendor
> or SoC prefix? These names are already widely used across the kernel, and
> defining them generically in a global dt-bindings header might cause
> preprocessor collisions and build failures for any driver or DTS file that
> includes this header.
> 
NO, the maintainer prefers no prefix.

>> +#endif /* __DT_BINDINGS_DMA_AMLOGIC_DMA_H__ */
> --
> Sashiko AI review ·https://sashiko.dev/#/patchset/20260626-amlogic-dma-v9-0-558d672c4a95@amlogic.com?part=1

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic