Message-ID: <1377127873.5029.72.camel@snotra.buserror.net>
Date:	Wed, 21 Aug 2013 18:31:13 -0500
From:	Scott Wood <scottwood@...escale.com>
To:	Stephen Warren <swarren@...dotorg.org>
CC:	<hongbo.zhang@...escale.com>, <vinod.koul@...el.com>,
	<djbw@...com>, <leoli@...escale.com>,
	<linuxppc-dev@...ts.ozlabs.org>, <devicetree@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v7 2/3] DMA: Freescale: Add new 8-channel DMA engine
 device tree nodes

On Wed, 2013-08-21 at 17:15 -0600, Stephen Warren wrote:
> On 08/21/2013 04:57 PM, Scott Wood wrote:
> > On Wed, 2013-08-21 at 16:40 -0600, Stephen Warren wrote:
> >> On 07/29/2013 04:49 AM, hongbo.zhang@...escale.com wrote:
> 
> >>> +- ranges            : describes the mapping between the address space of the
> >>> +                      DMA channels and the address space of the DMA controller
> >>
> >> Oh, so looking at the example, this is simply about being able to write
> >> the reg value in the child nodes more easily without having to write out
> >> the full based address of the controller in each child node.
> >>
> >> I don't think the binding document should require this;
> > 
> > It doesn't.  It just requires that there be a mapping; it doesn't have
> > to be any particular mapping.
> > 
> >> all the binding document should care about is that the child nodes have a valid reg
> >> value. Whether that reg value is <0x100100 0x80> without a ranges in the
> >> top-level DMA
> > 
> > Without a ranges property there is no translation and the registers
> > would not be memory mappable.  Linux may treat the absence of ranges as
> > an identity mapping for compatibility with some broken OF trees, but
> > it's not standard.
> 
> I would argue that missing ranges meaning 1:1 translation is now a
> standard, given that it must be true to support some DTs, it therefore
> can now be assumed?

"Some broken tree does it therefore it's fine for everyone to do it" is
awful.  We have standards documents for a reason.

Plus, not all OSes need to run on hardware with that broken firmware.
For example, the Freescale Embedded Hypervisor will not accept a missing
ranges in that way.  U-Boot does accept it, but only because the code
was copied from Linux (including the comment that says it's not supposed
to work that way).

If anything, we should remove the hack from U-Boot, and fix Linux to
only apply it in situations where it's known to be needed.

> >> Why do the channel nodes even need a compatible value? Presumably the
> >> driver for the top-level DMA node will scan these dma-channel nodes to
> >> extract the information it needs and will simply assume that all these
> >> nodes are DMA channel nodes rather than something else? I suppose this
> >> doesn't hurt, it just seems unnecessary unless you foresee other child
> >> nodes types existing in the future and hence a need to differentiate
> >> different types of nodes.
> > 
> > Other than "this is how the existing binding works and we're not going
> > to break compatibility", it allows the OS more flexibility to choose
> > whether to bind to controllers or directly to the channels.  Sometimes a
> > channel will be labelled with a different compatible if it has a fixed
> > purpose such as being connected to audio hardware (e.g. mpc8610_hpcd.dts
> > where some channels are "fsl,ssi-dma-channel").
> 
> That sounds terribly like encoding policy into DT rather than it being a
> HW description.

It is hardware description.  Those DMA channels are physically wired
into the audio hardware.  Other DMA channels in the same system aren't.

The only thing that's even slightly policy is that it assumes you aren't
going to ignore the audio device altogether, and use it as an extra
generic DMA channel.  I'm not sure that it's worthwhile to care in this
case.  If you want to be 100% policy-free then we shouldn't be
specifying a lot of the addresses we currently do, since that's actually
just how U-Boot configured things.  Sometimes simplifying assumptions
get made, when what the hardware people actually came up with is too
awkward to be worth describing directly.  In any case, this is not new,
nor is it relevant to the hardware we're currently adding support for,
and we're not going to break compatibility now.

-Scott



--
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/

Powered by blists - more mailing lists