[PATCH 2/4] Add dma sector to mpc8641hpcn board dts

[Zhang Wei-r63237]

> -----Original Message-----
> From: Segher Boessenkool [mailto:segher at kernel.crashing.org] 
> 
> >>> +		dma at 21000{
> >>> +			compatible = "fsl,mpc8xxx-dma";
> >>
> >> Please use a real name, not this "xxx" stuff.
> >
> > Does the "xxx" seems to be an evil name? :-)
> 
> You are stating this particular device is compatible to
> the "mpc8xxx-dma" device, which doesn't exist.  If it is
> supposed to mean it is compatible to the DMA controller
> on any 8000-series device, that is incorrect; in the future,
> there might be many such devices with an incompatible DMA
> controller.
> 
> Just put in some real device model name.

Ok, I'll use the first (or almost) real device model name. 

> 
> >>> +			reg = <21000 100>;
> >>> +			ranges = <0 21000 1000>;
> >>
> >> These overlap, that can't be right; it is just begging for
> >> trouble.
> >
> > There is no overlap. The 'reg' is used for the register of DMA
> > controller. And the ranges is used for belows channel registers.
> 
> And 21000..210ff is a subset of 21000..21fff.  So there _is_
> overlap.
> 

All right. Avoid this.

> > Such as ch0 at 100, reg=<100 80>, in fact, the ch0 register address is
> > 0x21100.
> 
> Sure, none of the "reg"s in any of the children overlap the
> parent "reg", but the parent "reg" overlaps the "ranges".
> 
> >>> +			ch0 at 100{
> >>> +				reg = <100 80>;
> >>> +				extended;
> >>
> >> I think you want a little more detailed property name
> >> than "extended" here?
> >
> > I've explained it in [PATCH 1/4] Add DMA sector to
> > Documentation/powerpc/booting-without-of.txt file.
> > As below:
> > +    - extended : Set the DMA channel to work at extended 
> chain mode.
> > +                 If not set, the DMA channel will work at basic
> > +                 chain mode.
> 
> Sure it's documented, but it would be good if the property
> name itself would say a bit more than just "extended".  Property
> names can be 31 characters, there's no need to make short names
> (terse is good, content-free isn't).
> 

Ok.

Thanks!
Wei.