Hi Guennadi, On Wed, Jul 24, 2013 at 6:19 AM, Guennadi Liakhovetski <[email protected]> wrote: > On Wed, 24 Jul 2013, Magnus Damm wrote: > >> Hi Guennadi, >> >> Thanks for your efforts on this. >> >> On Tue, Jul 23, 2013 at 7:49 PM, Guennadi Liakhovetski >> <[email protected]> wrote: >> > This configuration data will be re-used, when DMAC DT support is added to >> > r8a7740, DMAC platform data in setup-r8a7740.c will be removed. >> > >> > Signed-off-by: Guennadi Liakhovetski <[email protected]> >> > --- >> > >> > v4: make struct sh_dmae_pdata r8a7740_dma_pdata "const" >> > >> >> [snip] >> >> > --- /dev/null >> > +++ b/drivers/dma/sh/shdma-r8a7740.c >> > @@ -0,0 +1,95 @@ >> > +#include <linux/sh_dma.h> >> > + >> > +#include <mach/dma-register.h> >> > +#include <mach/r8a7740.h> >> >> Including stuff from <mach/..> isn't really compatible with >> MULTIPLATFORM, > > Hmm, right. I modeled this arch-specific driver code after Laurent's > pinctrl driver revamp, which also includes <mach/*.h> headers. So, we'll > have to think how to fix both.
I mentioned this to Laurent when he started converting to PINCTRL, and I believe the only remaining bits are the static GPIO-to-IRQ tables. >> so please don't write new code like this. Actually we >> don't want any code under drivers/ to include stuff from the mach >> directory. > > Sure, understood. Good. >> I suggest that you arrange your code in a way so the C version of DMAC >> support has tables with slave ids as usual under >> arch/arm/mach-shmobile/, but the DT bits that operate independently of >> C stay in drivers/... Over time we will get rid of the C version, and >> until that happens the DT and C version can coexist in parallel. > > That's already how it is. Data, that I took to drivers/dma/sh/ is needed > for both DT and C. DMA stuff, needed only for C are only DMAC devices and > resources. I think, I might be able to carry those DMA specific headers > and defines over from mach/ to drivers/dma/sh. Maybe it would be easier to > do this in several steps: > > 1. add my drivers/dma/sh/shdma-<arch>.c files *with* mach/ headers > 2. switch arches over to those files > (the above two steps are already done in my patch series) > 3. move headers to drivers/dma/sh > > Ok, alternatively, I might be able to do (1) above without using mach/ > headers at all by directly copying them to drivers/dma/sh/ and then > removing the original mach/headers in step (2)? I'll look in more detail > at the code tomorrow. Thanks. My apologies for reviewing your code late in the cycle, but I've now looked through this series and the following questions popped up: 1) How will it look like in DT when a DMA Engine slave device will use DMA? 2) Isn't it possible to leave the SHDMA_SLAVE_xx bits to only be used by legacy C SoC and board code in arch/arm/mach-shmobile? I don't understand why you have to move them over to drivers/... I just assume these SHDMA_SLAVE bits won't be used by 1) above. 3) How difficult would it be to describe the information in "struct sh_dmae_slave_config" using DT? 4) It seems that some patches in this series are unrelated. Can you submit 4/15 and 9/15 from v4 independently somehow? 5) Reducing and extending (reducing is optional of course) At this point you cover r8a7740, r8a73a0 and r8a73a4. I'm not sure why your picked those 3 SoCs, but perhaps it would be a good idea to select a single SoC to begin with but extend the support to also provide DT code that makes use of DMA Engine in slave devices like for instance MMCIF (this would cover question 1) above). When we have agreed on the big picture then additional SoCs can easily be added later. 6) Remove untested bits And while doing this conversion, perhaps this is a good opportunity to only move over DMA Engine slaves that can be tested? Having tons of unused DMA configuration seems overly heavy to me. If it's not tested then it's broken. Cheers, / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
