Re: [PATCH v4 02/15] DMA: shdma: add r8a7740 DMAC data to the device ID table

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Guennadi,

On Tuesday 23 July 2013 23:19:29 Guennadi Liakhovetski wrote:
> On Wed, 24 Jul 2013, Magnus Damm wrote:
> > On Tue, Jul 23, 2013 at 7:49 PM, Guennadi Liakhovetski 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 <g.liakhovetski+renesas@gmail.com>
> > > ---
> > > 
> > > 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.

Just for the record, I've already fixed some of those issues in my tree, and 
I'll work on the remaining ones (three instances of <mach/irqs.h> for the 
irq_pin() macro).

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

-- 
Regards,

Laurent Pinchart