Re: [PATCH v8 1/3] DMA: Freescale: revise device tree bindingdocument

[Scott Wood]

On Wed, 2013-08-28 at 13:48 +0100, Mark Rutland wrote:
> 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.

More generally, bindings should always specify "must include" rather
than "must be" for compatible, since by the nature of compatible you
could have a derivative device with more features that is compatible
with the device described in the binding, and the binding shouldn't
preclude identifying the derivative device.

-Scott



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