Hi Geert, Thank you for the review. On Monday 04 August 2014 11:09:06 Geert Uytterhoeven wrote: > On Sat, Jul 19, 2014 at 1:50 AM, Laurent Pinchart wrote: > > Separate helpers and drivers in the Kconfig and Makefile to improve > > readability and move the CONFIG_OF dependency from the Makefile to > > Kconfig. > > > > Signed-off-by: Laurent Pinchart > > <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx> > > > > --- > > > > Changes since v1: > > > > - Replace CONFIG_OF with OF in Kconfig > > --- > > > > drivers/dma/sh/Kconfig | 24 +++++++++++++++++++----- > > drivers/dma/sh/Makefile | 14 +++++++++++--- > > 2 files changed, 30 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/dma/sh/Kconfig b/drivers/dma/sh/Kconfig > > index 0f71981..0349125 100644 > > --- a/drivers/dma/sh/Kconfig > > +++ b/drivers/dma/sh/Kconfig > > @@ -2,21 +2,39 @@ > > # DMA engine configuration for sh > > # > > > > +# > > +# DMA Engine Helpers > > +# > > + > > config SH_DMAE_BASE > > bool "Renesas SuperH DMA Engine support" > > - depends on (SUPERH && SH_DMA) || ARCH_SHMOBILE || COMPILE_TEST > > + depends on SUPERH || ARCH_SHMOBILE || COMPILE_TEST > > + depends on !SUPERH || SH_DMA > > depends on !SH_DMA_API > > default y > > select DMA_ENGINE > > help > > Enable support for the Renesas SuperH DMA controllers. > > > > +# > > +# DMA Controllers > > +# > > + > > config SH_DMAE > > tristate "Renesas SuperH DMAC support" > > depends on SH_DMAE_BASE > > help > > Enable support for the Renesas SuperH DMA controllers. > > > > +if SH_DMAE > > + > > +config SH_DMAE_R8A73A4 > > + def_bool y > > + depends on ARCH_R8A73A4 > > + depends on OF > > + > > +endif > > What's the rationale behind putting this inside an "if/endif" block, instead > of using "depends on"? In my opinion it shows more clearly that the SH_DMAE_R8A73A4 option is a sub- driver of SH_DMAE. It might just be a matter of personal taste though. > BTW, I'm still puzzled by the difference between SH_DMAE_BASE and SH_DMAE, > which have identical help texts. SH_DMAE_BASE is a helper layer that aims at handling tasks required by the DMA engine API but not specific to drivers in a central location. There's nothing really SH-specific in there, it should be turned in more generic helpers and moved to the DMA engine core. SH_DMAE is the SYS-DMAC driver itself. The Kconfig symbol is badly named given that we now have different DMA engine drivers for "sh" (in the sense SuperH + ARM). > > + > > config SUDMAC > > tristate "Renesas SUDMAC support" > > depends on SH_DMAE_BASE > > @@ -34,7 +52,3 @@ config RCAR_AUDMAC_PP > > depends on SH_DMAE_BASE > > help > > Enable support for the Renesas R-Car Audio DMAC Peripheral > > Peripheral controllers.> > > - > > -config SHDMA_R8A73A4 > > - def_bool y > > - depends on ARCH_R8A73A4 && SH_DMAE != n -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html