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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] The macros DMA_TX and DMA_RX are defined in a global header without any vendor or subsystem prefix.
- [Low] The binding specifies `#dma-cells: const: 2` but fails to document the semantics of the two cells.
--

commit 7a02c7e6145050221d77d7a4745ee4d4e651163b
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 dt-bindings header
file with property values for the DMA transfer directions.

> 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
[ ... ]
> +properties:
> +  compatible:
> +    const: amlogic,a9-dma
[ ... ]
> +  clock-names:
> +    const: sys
> +
> +  '#dma-cells':
> +    const: 2

[Severity: Low]
Should this include a description explaining what the two cells represent?

Usually bindings with multiple cells document the semantics of each cell
(e.g., first cell for the request line, second for transfer direction) so
clients know how to construct their dmas specifier.

[ ... ]
> 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]
Would it be safer to add a vendor or device prefix to these macros?

Since this header is globally accessible and can be included in both device
tree sources and driver code, generic names like DMA_TX and DMA_RX might
cause macro redefinition conflicts with other kernel components. Adding a
prefix like AMLOGIC_A9_ would avoid this.

> +#endif /* __DT_BINDINGS_DMA_AMLOGIC_DMA_H__ */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521-amlogic-dma-v8-0-86cc2ce94142@amlogic.com?part=1