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

On 14-07-26, 08:19, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] The macros DMA_TX and DMA_RX in the global DT binding header lack an Amlogic/SoC specific prefix, leading to namespace pollution and potential build conflicts.
> --
> 
> --- Patch [1]: [PATCH v11 1/3] dt-bindings: dma: Add Amlogic A9 SoC DMA ---
> commit 093f368eaa67418008421d43d724ce2e98651039
> Author: Xianwei Zhao <[email protected]>
> 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 introduces the YAML schema and a header
> file defining the channel type macros.
> 
> > diff --git a/Documentation/devicetree/bindings/dma/amlogic,a9-dma.yaml b/Documentation/devicetree/bindings/dma/amlogic,a9-dma.yaml
> > new file mode 100644
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/dma/amlogic,a9-dma.yaml
> [ ... ]
> > +  '#dma-cells':
> > +    const: 2
> > +    description:
> > +      The first cell is the DMA channel type(DMA_TX or DMA_RX).
> > +      The second cell is the DMA channel index.
> [ ... ]
> > diff --git a/include/dt-bindings/dma/amlogic,a9-dma.h b/include/dt-bindings/dma/amlogic,a9-dma.h
> > new file mode 100644
> > --- /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]
> Are these macro names too generic for a global dt-bindings header?
> 
> Since headers in include/dt-bindings/ are globally visible and frequently
> included by C drivers, unprefixed names like DMA_TX and DMA_RX could lead
> to namespace pollution and macro redefinition build failures if a driver
> includes this alongside other headers that use the same common names.
> 
> Would it be safer to add a vendor or SoC prefix, such as AMLOGIC_A9_DMA_TX?

I think this makes sense to add here

-- 
~Vinod

_______________________________________________
linux-amlogic mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-amlogic