On Wed, Aug 28, 2013 at 09:18:55AM +0100, Hongbo Zhang wrote: > On 08/27/2013 07:25 PM, Mark Rutland wrote: > > On Tue, Aug 27, 2013 at 11:42:01AM +0100, hongbo.zhang@xxxxxxxxxxxxx wrote: > >> From: Hongbo Zhang <hongbo.zhang@xxxxxxxxxxxxx> > >> > >> 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@xxxxxxxxxxxxx> > >> --- > >> .../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. > Hmm. Stephen questioned here too, it seems this is a default rule. > Although Scott@freescale had explained our thoughts, I'd like to edit > this item like this: > > "must include "fsl,eloplus-dma", and a "fsl,CHIP-dma" is optional, where > CHIP is the processor name" > > We don't list all the chip name because we have tens of them and we > cannot list all of them, and it is unnecessary to list them because we > even don't use "fsl,CHIP-dma" in the new driver, add "fsl,CHIP-dma" here > just make it questionable when it presents in example and old dts files. > > I remove the examples in bracket "(mpc8349, mpc8360, etc.)" because we > can see the real example below. > I don't say" if "fsl,CHIP-dma" presents, it should be the first one, and > the "fsl,eloplus-dma" should be the second" because it is common rule. > the description language should be clear and concise too I think. Actually, you've convinced me for the form as you originally converted it (must include "fsl,elo-dma"), given that the other strings aren't used to give information anywhere and "fsl,CHIP-dma" doesn't fully define a valid string. > >> +- 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. > not clear with your point "multiple definitions", we really have > multiple dma channels for one dma controller. > cell-index is used as channel index, this is an old method used by old > driver, my patch didn't touch this part. Sorry, I'd misunderstood the cell-index property. More noise from me. Given that, this looks fine to me. Acked-by: Mark Rutland <mark.rutland@xxxxxxx> > >> > >> 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 devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
