LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v2] dma: vdma: Fix compilation warnings
@ 2015-03-30 13:18 Kedareswara rao Appana
  2015-03-30 17:18 ` Vinod Koul
  2015-04-17 17:49 ` Vinod Koul
  0 siblings, 2 replies; 5+ messages in thread
From: Kedareswara rao Appana @ 2015-03-30 13:18 UTC (permalink / raw)
  To: vinod.koul, sfr; +Cc: linux-next, linux-kernel, anirudh, Kedareswara rao Appana

This patch fixes the following compilation warnings.
In file included from drivers/dma/xilinx/xilinx_vdma.c:26:0:
include/linux/dmapool.h:18:4: warning: 'struct device' declared inside parameter list
    size_t size, size_t align, size_t allocation);
    ^
include/linux/dmapool.h:18:4: warning: its scope is only this definition or declaration, which is probably not what you want
include/linux/dmapool.h:31:7: warning: 'struct device' declared inside parameter list
       size_t size, size_t align, size_t allocation);
       ^
drivers/dma/xilinx/xilinx_vdma.c: In function 'xilinx_vdma_alloc_chan_resources':
drivers/dma/xilinx/xilinx_vdma.c:501:20: warning: passing argument 2 of 'dma_pool_create' from incompatible pointer type
  chan->desc_pool = dma_pool_create("xilinx_vdma_desc_pool",
                    ^
In file included from drivers/dma/xilinx/xilinx_vdma.c:26:0:
include/linux/dmapool.h:17:18: note: expected 'struct device *' but argument is of type 'struct device *'
 struct dma_pool *dma_pool_create(const char *name, struct device *dev, .

Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
---
This patch is rebased on the slave-dma next.
Changes for v2:
- Don't include header file instead use struct device as 
  suggested by Stephen Rothwell.

 include/linux/dmapool.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/dmapool.h b/include/linux/dmapool.h
index 022e34f..52456aa 100644
--- a/include/linux/dmapool.h
+++ b/include/linux/dmapool.h
@@ -14,6 +14,8 @@
 #include <asm/io.h>
 #include <asm/scatterlist.h>
 
+struct device;
+
 struct dma_pool *dma_pool_create(const char *name, struct device *dev, 
 			size_t size, size_t align, size_t allocation);
 
-- 
2.1.2


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] dma: vdma: Fix compilation warnings
  2015-03-30 13:18 [PATCH v2] dma: vdma: Fix compilation warnings Kedareswara rao Appana
@ 2015-03-30 17:18 ` Vinod Koul
  2015-03-30 20:35   ` Stephen Rothwell
  2015-04-17 17:49 ` Vinod Koul
  1 sibling, 1 reply; 5+ messages in thread
From: Vinod Koul @ 2015-03-30 17:18 UTC (permalink / raw)
  To: Kedareswara rao Appana
  Cc: sfr, linux-next, linux-kernel, anirudh, Kedareswara rao Appana

On Mon, Mar 30, 2015 at 06:48:29PM +0530, Kedareswara rao Appana wrote:
> This patch fixes the following compilation warnings.
> In file included from drivers/dma/xilinx/xilinx_vdma.c:26:0:
> include/linux/dmapool.h:18:4: warning: 'struct device' declared inside parameter list
>     size_t size, size_t align, size_t allocation);
>     ^
> include/linux/dmapool.h:18:4: warning: its scope is only this definition or declaration, which is probably not what you want
> include/linux/dmapool.h:31:7: warning: 'struct device' declared inside parameter list
>        size_t size, size_t align, size_t allocation);
>        ^
> drivers/dma/xilinx/xilinx_vdma.c: In function 'xilinx_vdma_alloc_chan_resources':
> drivers/dma/xilinx/xilinx_vdma.c:501:20: warning: passing argument 2 of 'dma_pool_create' from incompatible pointer type
>   chan->desc_pool = dma_pool_create("xilinx_vdma_desc_pool",
>                     ^
> In file included from drivers/dma/xilinx/xilinx_vdma.c:26:0:
> include/linux/dmapool.h:17:18: note: expected 'struct device *' but argument is of type 'struct device *'
>  struct dma_pool *dma_pool_create(const char *name, struct device *dev, .
> 
Well this does fix this error but this can also be fixed by rearranging the
driver header files order.  Since I am not inclined to update a patch for
dmapool.h I would go for rearranging drivers header

--><8---------------><8--------------

diff --git a/drivers/dma/xilinx/xilinx_vdma.c
b/drivers/dma/xilinx/xilinx_vdma.c
index d8434d465885..356ca4bc0ea5 100644
--- a/drivers/dma/xilinx/xilinx_vdma.c
+++ b/drivers/dma/xilinx/xilinx_vdma.c
@@ -23,12 +23,12 @@
  */

 #include <linux/bitops.h>
-#include <linux/dmapool.h>
 #include <linux/dma/xilinx_dma.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/module.h>
+#include <linux/dmapool.h>
 #include <linux/of_address.h>
 #include <linux/of_dma.h>
 #include <linux/of_platform.h>

Any objections?

-- 
~Vinod
> Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
> ---
> This patch is rebased on the slave-dma next.
> Changes for v2:
> - Don't include header file instead use struct device as 
>   suggested by Stephen Rothwell.
> 
>  include/linux/dmapool.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/linux/dmapool.h b/include/linux/dmapool.h
> index 022e34f..52456aa 100644
> --- a/include/linux/dmapool.h
> +++ b/include/linux/dmapool.h
> @@ -14,6 +14,8 @@
>  #include <asm/io.h>
>  #include <asm/scatterlist.h>
>  
> +struct device;
> +
>  struct dma_pool *dma_pool_create(const char *name, struct device *dev, 
>  			size_t size, size_t align, size_t allocation);
>  
> -- 
> 2.1.2
> 

-- 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] dma: vdma: Fix compilation warnings
  2015-03-30 17:18 ` Vinod Koul
@ 2015-03-30 20:35   ` Stephen Rothwell
  2015-03-31  4:57     ` Vinod Koul
  0 siblings, 1 reply; 5+ messages in thread
From: Stephen Rothwell @ 2015-03-30 20:35 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Kedareswara rao Appana, linux-next, linux-kernel, anirudh,
	Kedareswara rao Appana

[-- Attachment #1: Type: text/plain, Size: 2641 bytes --]

Hi Vinod,

On Mon, 30 Mar 2015 22:48:46 +0530 Vinod Koul <vinod.koul@intel.com> wrote:
>
> On Mon, Mar 30, 2015 at 06:48:29PM +0530, Kedareswara rao Appana wrote:
> > This patch fixes the following compilation warnings.
> > In file included from drivers/dma/xilinx/xilinx_vdma.c:26:0:
> > include/linux/dmapool.h:18:4: warning: 'struct device' declared inside parameter list
> >     size_t size, size_t align, size_t allocation);
> >     ^
> > include/linux/dmapool.h:18:4: warning: its scope is only this definition or declaration, which is probably not what you want
> > include/linux/dmapool.h:31:7: warning: 'struct device' declared inside parameter list
> >        size_t size, size_t align, size_t allocation);
> >        ^
> > drivers/dma/xilinx/xilinx_vdma.c: In function 'xilinx_vdma_alloc_chan_resources':
> > drivers/dma/xilinx/xilinx_vdma.c:501:20: warning: passing argument 2 of 'dma_pool_create' from incompatible pointer type
> >   chan->desc_pool = dma_pool_create("xilinx_vdma_desc_pool",
> >                     ^
> > In file included from drivers/dma/xilinx/xilinx_vdma.c:26:0:
> > include/linux/dmapool.h:17:18: note: expected 'struct device *' but argument is of type 'struct device *'
> >  struct dma_pool *dma_pool_create(const char *name, struct device *dev, .
> > 
> Well this does fix this error but this can also be fixed by rearranging the
> driver header files order.  Since I am not inclined to update a patch for
> dmapool.h I would go for rearranging drivers header
> 
> --><8---------------><8--------------
> 
> diff --git a/drivers/dma/xilinx/xilinx_vdma.c
> b/drivers/dma/xilinx/xilinx_vdma.c
> index d8434d465885..356ca4bc0ea5 100644
> --- a/drivers/dma/xilinx/xilinx_vdma.c
> +++ b/drivers/dma/xilinx/xilinx_vdma.c
> @@ -23,12 +23,12 @@
>   */
> 
>  #include <linux/bitops.h>
> -#include <linux/dmapool.h>
>  #include <linux/dma/xilinx_dma.h>
>  #include <linux/init.h>
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
>  #include <linux/module.h>
> +#include <linux/dmapool.h>
>  #include <linux/of_address.h>
>  #include <linux/of_dma.h>
>  #include <linux/of_platform.h>
> 
> Any objections?

Yes.  The error is in dmapool.h so it should be fixed once and for
all.  The supplied patch is very unintrusive and means that the problem
won't reappear when someone does some rearrangement of includes in the
future.  The file in question really has no particular maintainer.
Even after your suggested patch, dmapool.h still depend on an implicit
include of device.h.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] dma: vdma: Fix compilation warnings
  2015-03-30 20:35   ` Stephen Rothwell
@ 2015-03-31  4:57     ` Vinod Koul
  0 siblings, 0 replies; 5+ messages in thread
From: Vinod Koul @ 2015-03-31  4:57 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: Kedareswara rao Appana, linux-next, linux-kernel, anirudh,
	Kedareswara rao Appana

[-- Attachment #1: Type: text/plain, Size: 3176 bytes --]

On Tue, Mar 31, 2015 at 07:35:43AM +1100, Stephen Rothwell wrote:
Hi Stephen,
>
> On Mon, 30 Mar 2015 22:48:46 +0530 Vinod Koul <vinod.koul@intel.com> wrote:
> >
> > On Mon, Mar 30, 2015 at 06:48:29PM +0530, Kedareswara rao Appana wrote:
> > > This patch fixes the following compilation warnings.
> > > In file included from drivers/dma/xilinx/xilinx_vdma.c:26:0:
> > > include/linux/dmapool.h:18:4: warning: 'struct device' declared inside parameter list
> > >     size_t size, size_t align, size_t allocation);
> > >     ^
> > > include/linux/dmapool.h:18:4: warning: its scope is only this definition or declaration, which is probably not what you want
> > > include/linux/dmapool.h:31:7: warning: 'struct device' declared inside parameter list
> > >        size_t size, size_t align, size_t allocation);
> > >        ^
> > > drivers/dma/xilinx/xilinx_vdma.c: In function 'xilinx_vdma_alloc_chan_resources':
> > > drivers/dma/xilinx/xilinx_vdma.c:501:20: warning: passing argument 2 of 'dma_pool_create' from incompatible pointer type
> > >   chan->desc_pool = dma_pool_create("xilinx_vdma_desc_pool",
> > >                     ^
> > > In file included from drivers/dma/xilinx/xilinx_vdma.c:26:0:
> > > include/linux/dmapool.h:17:18: note: expected 'struct device *' but argument is of type 'struct device *'
> > >  struct dma_pool *dma_pool_create(const char *name, struct device *dev, .
> > > 
> > Well this does fix this error but this can also be fixed by rearranging the
> > driver header files order.  Since I am not inclined to update a patch for
> > dmapool.h I would go for rearranging drivers header
> > 
> > --><8---------------><8--------------
> > 
> > diff --git a/drivers/dma/xilinx/xilinx_vdma.c
> > b/drivers/dma/xilinx/xilinx_vdma.c
> > index d8434d465885..356ca4bc0ea5 100644
> > --- a/drivers/dma/xilinx/xilinx_vdma.c
> > +++ b/drivers/dma/xilinx/xilinx_vdma.c
> > @@ -23,12 +23,12 @@
> >   */
> > 
> >  #include <linux/bitops.h>
> > -#include <linux/dmapool.h>
> >  #include <linux/dma/xilinx_dma.h>
> >  #include <linux/init.h>
> >  #include <linux/interrupt.h>
> >  #include <linux/io.h>
> >  #include <linux/module.h>
> > +#include <linux/dmapool.h>
> >  #include <linux/of_address.h>
> >  #include <linux/of_dma.h>
> >  #include <linux/of_platform.h>
> > 
> > Any objections?
> 
> Yes.  The error is in dmapool.h so it should be fixed once and for
> all.  The supplied patch is very unintrusive and means that the problem
> won't reappear when someone does some rearrangement of includes in the
> future.  The file in question really has no particular maintainer.
> Even after your suggested patch, dmapool.h still depend on an implicit
> include of device.h.
I agree with your points, but isnt the order of headers also a thumb rule.
Typicaly a driver file will include core includes followed by subsystem
specfic includes.

Should a header have no dependency for its include ? I do come across
multiple examples of this in kernel

Yes fixing it in dmapool is also correct, so should we move to having
headers agnostic to the order of inclusion eventually ?

Thanks
-- 
~Vinod

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] dma: vdma: Fix compilation warnings
  2015-03-30 13:18 [PATCH v2] dma: vdma: Fix compilation warnings Kedareswara rao Appana
  2015-03-30 17:18 ` Vinod Koul
@ 2015-04-17 17:49 ` Vinod Koul
  1 sibling, 0 replies; 5+ messages in thread
From: Vinod Koul @ 2015-04-17 17:49 UTC (permalink / raw)
  To: Kedareswara rao Appana
  Cc: sfr, linux-next, linux-kernel, anirudh, Kedareswara rao Appana

On Mon, Mar 30, 2015 at 06:48:29PM +0530, Kedareswara rao Appana wrote:
> This patch fixes the following compilation warnings.
> In file included from drivers/dma/xilinx/xilinx_vdma.c:26:0:
> include/linux/dmapool.h:18:4: warning: 'struct device' declared inside parameter list
>     size_t size, size_t align, size_t allocation);
>     ^
> include/linux/dmapool.h:18:4: warning: its scope is only this definition or declaration, which is probably not what you want
> include/linux/dmapool.h:31:7: warning: 'struct device' declared inside parameter list
>        size_t size, size_t align, size_t allocation);
>        ^
> drivers/dma/xilinx/xilinx_vdma.c: In function 'xilinx_vdma_alloc_chan_resources':
> drivers/dma/xilinx/xilinx_vdma.c:501:20: warning: passing argument 2 of 'dma_pool_create' from incompatible pointer type
>   chan->desc_pool = dma_pool_create("xilinx_vdma_desc_pool",
>                     ^
> In file included from drivers/dma/xilinx/xilinx_vdma.c:26:0:
> include/linux/dmapool.h:17:18: note: expected 'struct device *' but argument is of type 'struct device *'
>  struct dma_pool *dma_pool_create(const char *name, struct device *dev, .
> 
I have applied this now

-- 
~Vinod

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-04-17 17:54 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-30 13:18 [PATCH v2] dma: vdma: Fix compilation warnings Kedareswara rao Appana
2015-03-30 17:18 ` Vinod Koul
2015-03-30 20:35   ` Stephen Rothwell
2015-03-31  4:57     ` Vinod Koul
2015-04-17 17:49 ` Vinod Koul

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).