[PATCH v2 1/8] dmaengine: sh: Rework Kconfig and Makefile

[PATCH v2 1/8] dmaengine: sh: Rework Kconfig and Makefile

[Laurent Pinchart]

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@ideasonboard.com>

---

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
+
 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
diff --git a/drivers/dma/sh/Makefile b/drivers/dma/sh/Makefile
index 1ce88b2..0b13b8e 100644
--- a/drivers/dma/sh/Makefile
+++ b/drivers/dma/sh/Makefile
@@ -1,10 +1,18 @@
+#
+# DMA Engine Helpers
+#
+
 obj-$(CONFIG_SH_DMAE_BASE) += shdma-base.o shdma-of.o
-obj-$(CONFIG_SH_DMAE) += shdma.o
+
+#
+# DMA Controllers
+#
+
 shdma-y := shdmac.o
-ifeq ($(CONFIG_OF),y)
 shdma-$(CONFIG_SHDMA_R8A73A4) += shdma-r8a73a4.o
-endif
 shdma-objs := $(shdma-y)
+obj-$(CONFIG_SH_DMAE) += shdma.o
+
 obj-$(CONFIG_SUDMAC) += sudmac.o
 obj-$(CONFIG_RCAR_HPB_DMAE) += rcar-hpbdma.o
 obj-$(CONFIG_RCAR_AUDMAC_PP) += rcar-audmapp.o
-- 
1.8.5.5

Re: [PATCH v2 1/8] dmaengine: sh: Rework Kconfig and Makefile

[Geert Uytterhoeven]

Hi Laurent,

On Sat, Jul 19, 2014 at 1:50 AM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> 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@ideasonboard.com>
>
> ---
>
> 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"?

BTW, I'm still puzzled by the difference between SH_DMAE_BASE and SH_DMAE,
which have identical help texts.

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

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 1/8] dmaengine: sh: Rework Kconfig and Makefile

[Laurent Pinchart]

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@ideasonboard.com>
> > 
> > ---
> > 
> > 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