* [PATCH] dma: sh: inherit debug options from the subsystem for sh
@ 2014-06-29 9:10 Wolfram Sang
2014-06-29 9:32 ` Geert Uytterhoeven
` (11 more replies)
0 siblings, 12 replies; 13+ messages in thread
From: Wolfram Sang @ 2014-06-29 9:10 UTC (permalink / raw)
To: linux-sh
From: Wolfram Sang <wsa+renesas@sang-engineering.com>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
drivers/dma/sh/Makefile | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/dma/sh/Makefile b/drivers/dma/sh/Makefile
index 1ce88b28cfc6..275297a89bd5 100644
--- a/drivers/dma/sh/Makefile
+++ b/drivers/dma/sh/Makefile
@@ -1,3 +1,6 @@
+ccflags-$(CONFIG_DMADEVICES_DEBUG) := -DDEBUG
+ccflags-$(CONFIG_DMADEVICES_VDEBUG) += -DVERBOSE_DEBUG
+
obj-$(CONFIG_SH_DMAE_BASE) += shdma-base.o shdma-of.o
obj-$(CONFIG_SH_DMAE) += shdma.o
shdma-y := shdmac.o
--
2.0.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] dma: sh: inherit debug options from the subsystem for sh
2014-06-29 9:10 [PATCH] dma: sh: inherit debug options from the subsystem for sh Wolfram Sang
@ 2014-06-29 9:32 ` Geert Uytterhoeven
2014-06-29 19:56 ` Laurent Pinchart
` (10 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Geert Uytterhoeven @ 2014-06-29 9:32 UTC (permalink / raw)
To: linux-sh
Hi Wolfram,
On Sun, Jun 29, 2014 at 11:10 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
> --- a/drivers/dma/sh/Makefile
> +++ b/drivers/dma/sh/Makefile
> @@ -1,3 +1,6 @@
> +ccflags-$(CONFIG_DMADEVICES_DEBUG) := -DDEBUG
> +ccflags-$(CONFIG_DMADEVICES_VDEBUG) += -DVERBOSE_DEBUG
> +
> obj-$(CONFIG_SH_DMAE_BASE) += shdma-base.o shdma-of.o
> obj-$(CONFIG_SH_DMAE) += shdma.o
> shdma-y := shdmac.o
Thanks!
There are a few more subdirectories that could benefit from a similar change.
I'm wondering whether this can be fixed at the drivers/dma/Makefile level,
as these config options were clearly intended to apply to all DMA drivers?
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] dma: sh: inherit debug options from the subsystem for sh
2014-06-29 9:10 [PATCH] dma: sh: inherit debug options from the subsystem for sh Wolfram Sang
2014-06-29 9:32 ` Geert Uytterhoeven
@ 2014-06-29 19:56 ` Laurent Pinchart
2014-06-30 13:51 ` Vinod Koul
` (9 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Laurent Pinchart @ 2014-06-29 19:56 UTC (permalink / raw)
To: linux-sh
Hi Wolfram,
Thank you for the patch.
On Sunday 29 June 2014 11:10:05 Wolfram Sang wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
> drivers/dma/sh/Makefile | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/dma/sh/Makefile b/drivers/dma/sh/Makefile
> index 1ce88b28cfc6..275297a89bd5 100644
> --- a/drivers/dma/sh/Makefile
> +++ b/drivers/dma/sh/Makefile
> @@ -1,3 +1,6 @@
> +ccflags-$(CONFIG_DMADEVICES_DEBUG) := -DDEBUG
Isn't this discouraged in favour of using dynamic printk ? I've recently
submitted a similar patch for the OMAP4 ISS driver and it got nacked.
> +ccflags-$(CONFIG_DMADEVICES_VDEBUG) += -DVERBOSE_DEBUG
> +
> obj-$(CONFIG_SH_DMAE_BASE) += shdma-base.o shdma-of.o
> obj-$(CONFIG_SH_DMAE) += shdma.o
> shdma-y := shdmac.o
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] dma: sh: inherit debug options from the subsystem for sh
2014-06-29 9:10 [PATCH] dma: sh: inherit debug options from the subsystem for sh Wolfram Sang
2014-06-29 9:32 ` Geert Uytterhoeven
2014-06-29 19:56 ` Laurent Pinchart
@ 2014-06-30 13:51 ` Vinod Koul
2014-07-03 8:48 ` Wolfram Sang
` (8 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Vinod Koul @ 2014-06-30 13:51 UTC (permalink / raw)
To: linux-sh
On Sun, Jun 29, 2014 at 09:56:43PM +0200, Laurent Pinchart wrote:
> Hi Wolfram,
>
> Thank you for the patch.
>
> On Sunday 29 June 2014 11:10:05 Wolfram Sang wrote:
> > From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> >
> > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > ---
> > drivers/dma/sh/Makefile | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/dma/sh/Makefile b/drivers/dma/sh/Makefile
> > index 1ce88b28cfc6..275297a89bd5 100644
> > --- a/drivers/dma/sh/Makefile
> > +++ b/drivers/dma/sh/Makefile
> > @@ -1,3 +1,6 @@
> > +ccflags-$(CONFIG_DMADEVICES_DEBUG) := -DDEBUG
>
> Isn't this discouraged in favour of using dynamic printk ? I've recently
> submitted a similar patch for the OMAP4 ISS driver and it got nacked.
With dynamic debug, these flags dont make much sense unless you want something
to be printed *always* during boot.
unforuntely, this patch doesnt provide reason why we should do this here!
--
~Vinod
>
> > +ccflags-$(CONFIG_DMADEVICES_VDEBUG) += -DVERBOSE_DEBUG
> > +
> > obj-$(CONFIG_SH_DMAE_BASE) += shdma-base.o shdma-of.o
> > obj-$(CONFIG_SH_DMAE) += shdma.o
> > shdma-y := shdmac.o
>
> --
> Regards,
>
> Laurent Pinchart
>
--
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] dma: sh: inherit debug options from the subsystem for sh
2014-06-29 9:10 [PATCH] dma: sh: inherit debug options from the subsystem for sh Wolfram Sang
` (2 preceding siblings ...)
2014-06-30 13:51 ` Vinod Koul
@ 2014-07-03 8:48 ` Wolfram Sang
2014-07-03 9:14 ` Vinod Koul
` (7 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Wolfram Sang @ 2014-07-03 8:48 UTC (permalink / raw)
To: linux-sh
[-- Attachment #1: Type: text/plain, Size: 349 bytes --]
Hi,
> With dynamic debug, these flags dont make much sense unless you want something
> to be printed *always* during boot.
Yes, this was exactly what I wanted. So, for consistency reasons I
copied the existing mechanism over to sh. If it turns out that the
desired default is something different, I'll send patches for that.
Thanks,
Wolfram
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] dma: sh: inherit debug options from the subsystem for sh
2014-06-29 9:10 [PATCH] dma: sh: inherit debug options from the subsystem for sh Wolfram Sang
` (3 preceding siblings ...)
2014-07-03 8:48 ` Wolfram Sang
@ 2014-07-03 9:14 ` Vinod Koul
2014-07-10 11:56 ` Wolfram Sang
` (6 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Vinod Koul @ 2014-07-03 9:14 UTC (permalink / raw)
To: linux-sh
[-- Attachment #1: Type: text/plain, Size: 556 bytes --]
On Thu, Jul 03, 2014 at 10:48:34AM +0200, Wolfram Sang wrote:
> Hi,
>
> > With dynamic debug, these flags dont make much sense unless you want something
> > to be printed *always* during boot.
>
> Yes, this was exactly what I wanted. So, for consistency reasons I
> copied the existing mechanism over to sh. If it turns out that the
> desired default is something different, I'll send patches for that.
Ah good, and while at it, Please *do* mention this in changelog!
also fix the subsystem name to dmaengine in patch title
--
~Vinod
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] dma: sh: inherit debug options from the subsystem for sh
2014-06-29 9:10 [PATCH] dma: sh: inherit debug options from the subsystem for sh Wolfram Sang
` (4 preceding siblings ...)
2014-07-03 9:14 ` Vinod Koul
@ 2014-07-10 11:56 ` Wolfram Sang
2014-07-10 12:25 ` Laurent Pinchart
` (5 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Wolfram Sang @ 2014-07-10 11:56 UTC (permalink / raw)
To: linux-sh
[-- Attachment #1: Type: text/plain, Size: 790 bytes --]
On Thu, Jul 03, 2014 at 02:32:25PM +0530, Vinod Koul wrote:
> On Thu, Jul 03, 2014 at 10:48:34AM +0200, Wolfram Sang wrote:
> > Hi,
> >
> > > With dynamic debug, these flags dont make much sense unless you want something
> > > to be printed *always* during boot.
> >
> > Yes, this was exactly what I wanted. So, for consistency reasons I
> > copied the existing mechanism over to sh. If it turns out that the
> > desired default is something different, I'll send patches for that.
>
> Ah good, and while at it, Please *do* mention this in changelog!
>
> also fix the subsystem name to dmaengine in patch title
So, I assume you are okay with the change now that I gave the reason?
But should I fix the other subdirs, too? Or keep the change minimal
(= only for sh)?
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] dma: sh: inherit debug options from the subsystem for sh
2014-06-29 9:10 [PATCH] dma: sh: inherit debug options from the subsystem for sh Wolfram Sang
` (5 preceding siblings ...)
2014-07-10 11:56 ` Wolfram Sang
@ 2014-07-10 12:25 ` Laurent Pinchart
2014-07-10 12:48 ` Wolfram Sang
` (4 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Laurent Pinchart @ 2014-07-10 12:25 UTC (permalink / raw)
To: linux-sh
[-- Attachment #1: Type: text/plain, Size: 693 bytes --]
Hi Wolfram,
On Thursday 03 July 2014 10:48:34 Wolfram Sang wrote:
> Hi,
>
> > With dynamic debug, these flags dont make much sense unless you want
> > something to be printed *always* during boot.
>
> Yes, this was exactly what I wanted.
Why is that ? My understanding is that the purpose of the
CONFIG_DMADEVICES_DEBUG option is to enable debug messages for debugging. As
it requires recompiling the kernel, wouldn't dynamic printk be a better
alternative ?
> So, for consistency reasons I copied the existing mechanism over to sh. If
> it turns out that the desired default is something different, I'll send
> patches for that.
--
Regards,
Laurent Pinchart
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] dma: sh: inherit debug options from the subsystem for sh
2014-06-29 9:10 [PATCH] dma: sh: inherit debug options from the subsystem for sh Wolfram Sang
` (6 preceding siblings ...)
2014-07-10 12:25 ` Laurent Pinchart
@ 2014-07-10 12:48 ` Wolfram Sang
2014-07-10 15:21 ` Vinod Koul
` (3 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Wolfram Sang @ 2014-07-10 12:48 UTC (permalink / raw)
To: linux-sh
[-- Attachment #1: Type: text/plain, Size: 616 bytes --]
> > > With dynamic debug, these flags dont make much sense unless you want
> > > something to be printed *always* during boot.
> >
> > Yes, this was exactly what I wanted.
>
> Why is that ? My understanding is that the purpose of the
> CONFIG_DMADEVICES_DEBUG option is to enable debug messages for debugging. As
> it requires recompiling the kernel, wouldn't dynamic printk be a better
> alternative ?
I didn't mind recompiling once for the prize of a smaller kernel image
(only the debug strings I wanted are included).
But let's just drop this patch, it is not worth all this discussion.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] dma: sh: inherit debug options from the subsystem for sh
2014-06-29 9:10 [PATCH] dma: sh: inherit debug options from the subsystem for sh Wolfram Sang
` (7 preceding siblings ...)
2014-07-10 12:48 ` Wolfram Sang
@ 2014-07-10 15:21 ` Vinod Koul
2014-07-10 15:22 ` Vinod Koul
` (2 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Vinod Koul @ 2014-07-10 15:21 UTC (permalink / raw)
To: linux-sh
[-- Attachment #1: Type: text/plain, Size: 738 bytes --]
On Thu, Jul 10, 2014 at 02:25:58PM +0200, Laurent Pinchart wrote:
> Hi Wolfram,
>
> On Thursday 03 July 2014 10:48:34 Wolfram Sang wrote:
> > Hi,
> >
> > > With dynamic debug, these flags dont make much sense unless you want
> > > something to be printed *always* during boot.
> >
> > Yes, this was exactly what I wanted.
>
> Why is that ? My understanding is that the purpose of the
> CONFIG_DMADEVICES_DEBUG option is to enable debug messages for debugging. As
> it requires recompiling the kernel, wouldn't dynamic printk be a better
> alternative ?
No it wont. Pls see the discussion.
Lots of dmanegine device gets used at boot, so for testing those scenarios it
will help to enable debug!
--
~Vinod
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] dma: sh: inherit debug options from the subsystem for sh
2014-06-29 9:10 [PATCH] dma: sh: inherit debug options from the subsystem for sh Wolfram Sang
` (8 preceding siblings ...)
2014-07-10 15:21 ` Vinod Koul
@ 2014-07-10 15:22 ` Vinod Koul
2014-07-10 15:42 ` Laurent Pinchart
2014-07-11 8:32 ` Simon Horman
11 siblings, 0 replies; 13+ messages in thread
From: Vinod Koul @ 2014-07-10 15:22 UTC (permalink / raw)
To: linux-sh
[-- Attachment #1: Type: text/plain, Size: 985 bytes --]
On Thu, Jul 10, 2014 at 01:56:48PM +0200, Wolfram Sang wrote:
> On Thu, Jul 03, 2014 at 02:32:25PM +0530, Vinod Koul wrote:
> > On Thu, Jul 03, 2014 at 10:48:34AM +0200, Wolfram Sang wrote:
> > > Hi,
> > >
> > > > With dynamic debug, these flags dont make much sense unless you want something
> > > > to be printed *always* during boot.
> > >
> > > Yes, this was exactly what I wanted. So, for consistency reasons I
> > > copied the existing mechanism over to sh. If it turns out that the
> > > desired default is something different, I'll send patches for that.
> >
> > Ah good, and while at it, Please *do* mention this in changelog!
> >
> > also fix the subsystem name to dmaengine in patch title
>
> So, I assume you are okay with the change now that I gave the reason?
> But should I fix the other subdirs, too? Or keep the change minimal
> (= only for sh)?
Yes with a right reason :)
And yes makes sense to do so for whole subsystem.
--
~Vinod
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] dma: sh: inherit debug options from the subsystem for sh
2014-06-29 9:10 [PATCH] dma: sh: inherit debug options from the subsystem for sh Wolfram Sang
` (9 preceding siblings ...)
2014-07-10 15:22 ` Vinod Koul
@ 2014-07-10 15:42 ` Laurent Pinchart
2014-07-11 8:32 ` Simon Horman
11 siblings, 0 replies; 13+ messages in thread
From: Laurent Pinchart @ 2014-07-10 15:42 UTC (permalink / raw)
To: linux-sh
[-- Attachment #1: Type: text/plain, Size: 906 bytes --]
Hi Vinod,
On Thursday 10 July 2014 20:39:29 Vinod Koul wrote:
> On Thu, Jul 10, 2014 at 02:25:58PM +0200, Laurent Pinchart wrote:
> > On Thursday 03 July 2014 10:48:34 Wolfram Sang wrote:
> > > Hi,
> > >
> > > > With dynamic debug, these flags dont make much sense unless you want
> > > > something to be printed *always* during boot.
> > >
> > > Yes, this was exactly what I wanted.
> >
> > Why is that ? My understanding is that the purpose of the
> > CONFIG_DMADEVICES_DEBUG option is to enable debug messages for debugging.
> > As it requires recompiling the kernel, wouldn't dynamic printk be a
> > better alternative ?
>
> No it wont. Pls see the discussion.
>
> Lots of dmanegine device gets used at boot, so for testing those scenarios
> it will help to enable debug!
OK, that's the point I've missed. I agree with the patch then.
--
Regards,
Laurent Pinchart
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] dma: sh: inherit debug options from the subsystem for sh
2014-06-29 9:10 [PATCH] dma: sh: inherit debug options from the subsystem for sh Wolfram Sang
` (10 preceding siblings ...)
2014-07-10 15:42 ` Laurent Pinchart
@ 2014-07-11 8:32 ` Simon Horman
11 siblings, 0 replies; 13+ messages in thread
From: Simon Horman @ 2014-07-11 8:32 UTC (permalink / raw)
To: linux-sh
On Thu, Jul 10, 2014 at 05:42:13PM +0200, Laurent Pinchart wrote:
> Hi Vinod,
>
> On Thursday 10 July 2014 20:39:29 Vinod Koul wrote:
> > On Thu, Jul 10, 2014 at 02:25:58PM +0200, Laurent Pinchart wrote:
> > > On Thursday 03 July 2014 10:48:34 Wolfram Sang wrote:
> > > > Hi,
> > > >
> > > > > With dynamic debug, these flags dont make much sense unless you want
> > > > > something to be printed *always* during boot.
> > > >
> > > > Yes, this was exactly what I wanted.
> > >
> > > Why is that ? My understanding is that the purpose of the
> > > CONFIG_DMADEVICES_DEBUG option is to enable debug messages for debugging.
> > > As it requires recompiling the kernel, wouldn't dynamic printk be a
> > > better alternative ?
> >
> > No it wont. Pls see the discussion.
> >
> > Lots of dmanegine device gets used at boot, so for testing those scenarios
> > it will help to enable debug!
>
> OK, that's the point I've missed. I agree with the patch then.
Hi,
in order to try and create a smoother path for changes in this area to land
I have pushed them to the shdma-for-v3.17 branch of my renesas tree
on kernel.org. It is merged into the devel and next branches of that tree
and should appear in linux-next in the near future.
My intention is to send a pull-request for this branch once it has sat
in next for a short time. I hope that this approach is useful to all parties.
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2014-07-11 8:32 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-29 9:10 [PATCH] dma: sh: inherit debug options from the subsystem for sh Wolfram Sang
2014-06-29 9:32 ` Geert Uytterhoeven
2014-06-29 19:56 ` Laurent Pinchart
2014-06-30 13:51 ` Vinod Koul
2014-07-03 8:48 ` Wolfram Sang
2014-07-03 9:14 ` Vinod Koul
2014-07-10 11:56 ` Wolfram Sang
2014-07-10 12:25 ` Laurent Pinchart
2014-07-10 12:48 ` Wolfram Sang
2014-07-10 15:21 ` Vinod Koul
2014-07-10 15:22 ` Vinod Koul
2014-07-10 15:42 ` Laurent Pinchart
2014-07-11 8:32 ` Simon Horman
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.