linux-kernel - Re: [PATCH v8 1/3] DMA: Freescale: revise device tree binding document

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20130827112509.GH19893@e106331-lin.cambridge.arm.com>
Date:	Tue, 27 Aug 2013 12:25:09 +0100
From:	Mark Rutland <mark.rutland@....com>
To:	"hongbo.zhang@...escale.com" <hongbo.zhang@...escale.com>
Cc:	"rob.herring@...xeda.com" <rob.herring@...xeda.com>,
	Pawel Moll <Pawel.Moll@....com>,
	"swarren@...dotorg.org" <swarren@...dotorg.org>,
	"ian.campbell@...rix.com" <ian.campbell@...rix.com>,
	"vinod.koul@...el.com" <vinod.koul@...el.com>,
	"djbw@...com" <djbw@...com>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v8 1/3] DMA: Freescale: revise device tree binding
 document

On Tue, Aug 27, 2013 at 11:42:01AM +0100, hongbo.zhang@...escale.com wrote:
> From: Hongbo Zhang <hongbo.zhang@...escale.com>
> 
> This patch updates the discription of each type of DMA controller and its
> channels, it is preparation for adding another new DMA controller binding, it
> also fixes some defects of indent for text alignment at the same time.
> 
> Signed-off-by: Hongbo Zhang <hongbo.zhang@...escale.com>
> ---
>  .../devicetree/bindings/powerpc/fsl/dma.txt        |   62 +++++++++-----------
>  1 file changed, 27 insertions(+), 35 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/powerpc/fsl/dma.txt b/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
> index 2a4b4bc..ddf17af 100644
> --- a/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
> +++ b/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
> @@ -1,33 +1,29 @@
> -* Freescale 83xx DMA Controller
> +* Freescale DMA Controllers
>  
> -Freescale PowerPC 83xx have on chip general purpose DMA controllers.
> +** Freescale Elo DMA Controller
> +   This is a little-endian DMA controller, used in Freescale mpc83xx series
> +   chips such as mpc8315, mpc8349, mpc8379 etc.
>  
>  Required properties:
>  
> -- compatible        : compatible list, contains 2 entries, first is
> -		 "fsl,CHIP-dma", where CHIP is the processor
> -		 (mpc8349, mpc8360, etc.) and the second is
> -		 "fsl,elo-dma"
> -- reg               : <registers mapping for DMA general status reg>
> -- ranges		: Should be defined as specified in 1) to describe the
> -		  DMA controller channels.
> +- compatible        : must include "fsl,elo-dma"

We should list the other values that may be in the list also, unless
they are really of no consequence, in which case their presence in dt is
questionable.

> +- reg               : <registers specifier for DMA general status reg>
> +- ranges            : describes the mapping between the address space of the
> +                      DMA channels and the address space of the DMA controller
>  - cell-index        : controller index.  0 for controller @ 0x8100
> -- interrupts        : <interrupt mapping for DMA IRQ>
> +- interrupts        : <interrupt specifier for DMA IRQ>
>  - interrupt-parent  : optional, if needed for interrupt mapping
>  
> -
>  - DMA channel nodes:
> -        - compatible        : compatible list, contains 2 entries, first is
> -			 "fsl,CHIP-dma-channel", where CHIP is the processor
> -			 (mpc8349, mpc8350, etc.) and the second is
> -			 "fsl,elo-dma-channel". However, see note below.
> -        - reg               : <registers mapping for channel>
> +        - compatible        : must include "fsl,elo-dma-channel"
> +                              However, see note below.

Again, I think we should list the other entries that may be in the list.
Otherwise it's not clear what the binding defines. Similarly for the
other compatible list definitions below...

> +        - reg               : <registers specifier for channel>
>          - cell-index        : dma channel index starts at 0.

I realise you haven't changed it, but it's unclear what the cell-index
property is (and somewhat confusingly there seem to be multiple
defnitions). It might be worth clarifying it while performing the other
cleanup.

>  
>  Optional properties:
> -        - interrupts        : <interrupt mapping for DMA channel IRQ>
> -			  (on 83xx this is expected to be identical to
> -			   the interrupts property of the parent node)
> +        - interrupts        : <interrupt specifier for DMA channel IRQ>
> +                              (on 83xx this is expected to be identical to
> +                              the interrupts property of the parent node)
>          - interrupt-parent  : optional, if needed for interrupt mapping
>  
>  Example:
> @@ -70,30 +66,26 @@ Example:
>  		};
>  	};
>  
> -* Freescale 85xx/86xx DMA Controller
> -
> -Freescale PowerPC 85xx/86xx have on chip general purpose DMA controllers.
> +** Freescale EloPlus DMA Controller
> +   This is DMA controller with extended addresses and chaining, mainly used in
> +   Freescale mpc85xx/86xx, Pxxx and BSC series chips, such as mpc8540, mpc8641
> +   p4080, bsc9131 etc.
>  
>  Required properties:
>  
> -- compatible        : compatible list, contains 2 entries, first is
> -		 "fsl,CHIP-dma", where CHIP is the processor
> -		 (mpc8540, mpc8540, etc.) and the second is
> -		 "fsl,eloplus-dma"
> -- reg               : <registers mapping for DMA general status reg>
> +- compatible        : must include "fsl,eloplus-dma"
> +- reg               : <registers specifier for DMA general status reg>
>  - cell-index        : controller index.  0 for controller @ 0x21000,
>                                           1 for controller @ 0xc000
> -- ranges		: Should be defined as specified in 1) to describe the
> -		  DMA controller channels.
> +- ranges            : describes the mapping between the address space of the
> +                      DMA channels and the address space of the DMA controller
>  
>  - DMA channel nodes:
> -        - compatible        : compatible list, contains 2 entries, first is
> -			 "fsl,CHIP-dma-channel", where CHIP is the processor
> -			 (mpc8540, mpc8560, etc.) and the second is
> -			 "fsl,eloplus-dma-channel". However, see note below.
> +        - compatible        : must include "fsl,eloplus-dma-channel"
> +                              However, see note below.
>          - cell-index        : dma channel index starts at 0.
> -        - reg               : <registers mapping for channel>
> -        - interrupts        : <interrupt mapping for DMA channel IRQ>
> +        - reg               : <registers specifier for channel>
> +        - interrupts        : <interrupt specifier for DMA channel IRQ>
>          - interrupt-parent  : optional, if needed for interrupt mapping
>  
>  Example:
> -- 
> 1.7.9.5

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/