On 07/14/2013 10:50 AM, Arnd Bergmann wrote: > On Saturday 13 July 2013, Gerhard Sittig wrote: >> [ MPC8308 knowledge required, see below ] >> >> On Sat, Jul 13, 2013 at 09:17 +0200, Arnd Bergmann wrote: >>> >>> On Friday 12 July 2013, Gerhard Sittig wrote: >>>> +++ b/include/dt-bindings/dma/mpc512x-dma.h >>>> @@ -0,0 +1,21 @@ >>>> +/* >>>> + * This header file provides symbolic specifiers for DMA channels >>>> + * within the MPC512x SoC's DMA controller. Since requester lines >>>> + * directly map to channel numbers and no additional flexibility >>>> + * is involved, DMA channels can be considered directly associated >>>> + * with individual peripherals. >>>> + * >>>> + * This header file gets shared among DT bindings which provide >>>> + * hardware specs, and driver code which implements supporting logic. >>>> + */ >>>> + >>>> +#ifndef _DT_BINDINGS_DMA_MPC512x_DMA_H >>>> +#define _DT_BINDINGS_DMA_MPC512x_DMA_H >>>> + >>>> +#define MPC512x_DMACHAN_SCLPC 26 >>>> +#define MPC512x_DMACHAN_SDHC 30 >>>> +#define MPC512x_DMACHAN_MDDRC 32 >>>> + >>>> +#define MPC512x_DMACHAN_MAX 64 >>>> + >>> >>> I think these should not be in the header and should not bve part of the >>> binding either. They are specific to an SoC that happens to be using this >>> DMA controller but would be completely different for any other SoC with >>> the same DMA engine. These belong into the dma descriptors of the slave >>> drivers and don't need symbolic names. >> >> Thank you for the feedback. >> >> OK, so not adding the dt-bindings header leads to no change in >> the DTS nodes, which in turn collapses 5/8 into something local >> to the .c driver source (introduce an enum and replace a few >> magic numbers with names), and obsoletes 4/8 as a prerequisite. >> This will further reduce the patch set's size. > > Actually I think you will need extra changes: The dma-engine driver > should not require knowledge of any channel-specific settings. > I did not notice you had them until you mentioned the above, but > from what I can tell, you need a few flags in the dma-specifier > to replace code like > > /* only start explicitly on MDDRC channel */ > - if (cid == 32) > + if (cid == MPC512x_DMACHAN_MDDRC) > mdesc->tcd->start = 1; > > with > > mdesc->tcd->start = dmaspec->explicit_start; > > or something along these lines, where dmaspec is a data structure > derived from the fields in the DT dma specifier of the child > node. >
I think the MDDRC channel is used for mem-to-mem transfers. So there is no peripheral that is going to request it by handle. If the channel that is used for mem-to-mem transfers differs between different instances of the controller (e.g. on different SoCs) it should probably be a property of the dma controllers DT node. - Lars _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev