[PATCH v2] dma: vdma: Fix compilation warnings

[PATCH v2] dma: vdma: Fix compilation warnings

[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

Re: [PATCH v2] dma: vdma: Fix compilation warnings

[Vinod Koul]

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
> 

-- 

Re: [PATCH v2] dma: vdma: Fix compilation warnings

[Stephen Rothwell]

[-- 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 --]

Re: [PATCH v2] dma: vdma: Fix compilation warnings

[Vinod Koul]

[-- 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 --]

Re: [PATCH v2] dma: vdma: Fix compilation warnings

[Vinod Koul]

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