[PATCH] usb: gadget: #include device.h in gadget.h

[PATCH] usb: gadget: #include device.h in gadget.h

[Patrick Pannuto]

gadget.h uses structures defined in device.h, it must include it. In
most cases, gadget.h is preceded by linux/platform_device.h, but if
you are grouping headers sanely, device.h may not be pulled in until
*after* gadget (e.g. mach/msm_device.h), thus gadget.h should not
rely on something else #including device.h

include/linux/usb/gadget.h:488: error: field 'dev' has incomplete type
include/linux/usb/gadget.h: In function 'set_gadget_data':
include/linux/usb/gadget.h:492:
	error: implicit declaration of function 'dev_set_drvdata'
include/linux/usb/gadget.h: In function 'get_gadget_data':
include/linux/usb/gadget.h:494:
	error: implicit declaration of function 'dev_get_drvdata'
include/linux/usb/gadget.h:494:
	warning: return makes pointer from integer without a cast
include/linux/usb/gadget.h: At top level:
include/linux/usb/gadget.h:780: error: field 'driver' has incomplete type
In file included from include/linux/dmaengine.h:24,
                 from include/linux/skbuff.h:30,
                 from include/linux/if_ether.h:124,
                 from drivers/usb/gadget/u_ether.h:27,
                 from drivers/usb/gadget/android.c:37:
include/linux/device.h:494: error: conflicting types for 'dev_get_drvdata'
include/linux/usb/gadget.h:494:
	note: previous implicit declaration of 'dev_get_drvdata' was here
include/linux/device.h:495:
	warning: conflicting types for 'dev_set_drvdata'
include/linux/usb/gadget.h:492:
	note: previous implicit declaration of 'dev_set_drvdata' was here

Change-Id: Ia06309b76ac91fc305a1a50fb5e3469cb6adb623
Signed-off-by: Patrick Pannuto <ppannuto@codeaurora.org>
---
 include/linux/usb/gadget.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index bbf45d5..ddca035 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -15,6 +15,8 @@
 #ifndef __LINUX_USB_GADGET_H
 #define __LINUX_USB_GADGET_H
 
+#include <linux/device.h>
+
 struct usb_ep;
 
 /**
-- 
1.7.1

Re: [PATCH] usb: gadget: #include device.h in gadget.h

[Greg KH]

On Wed, Jul 07, 2010 at 05:47:08PM -0700, Patrick Pannuto wrote:
> gadget.h uses structures defined in device.h, it must include it. In
> most cases, gadget.h is preceded by linux/platform_device.h, but if
> you are grouping headers sanely, device.h may not be pulled in until
> *after* gadget (e.g. mach/msm_device.h), thus gadget.h should not
> rely on something else #including device.h
> 
> include/linux/usb/gadget.h:488: error: field 'dev' has incomplete type

Why not just provide an "empty" prototype for whatever is needed.
Generally, having header files include header files is frowned apon, and
a number of people have been working on slowly unwinding a lot of this
type of mess that we currently have.

> include/linux/usb/gadget.h: In function 'set_gadget_data':
> include/linux/usb/gadget.h:492:
> 	error: implicit declaration of function 'dev_set_drvdata'

Ick, but then, this gets messier.

How about just fixing up the .c file that the problem happens in, to
include device.h first?  Is this an issue in the current tree somehow?

thanks,

greg k-h

Re: [PATCH] usb: gadget: #include device.h in gadget.h

[Sergei Shtylyov]

Hello.

Greg KH wrote:

>> gadget.h uses structures defined in device.h, it must include it. In
>> most cases, gadget.h is preceded by linux/platform_device.h, but if
>> you are grouping headers sanely, device.h may not be pulled in until
>> *after* gadget (e.g. mach/msm_device.h), thus gadget.h should not
>> rely on something else #including device.h

    As well as a number of other headers. I have postaed a patch addressing 
the missing #include's already.

>> include/linux/usb/gadget.h:488: error: field 'dev' has incomplete type

> Why not just provide an "empty" prototype for whatever is needed.

    Empty prototype of what, 'struct device'? Have you looked at the code at all?

struct usb_gadget {
	/* readonly to gadget driver */
	const struct usb_gadget_ops	*ops;
	struct usb_ep			*ep0;
	struct list_head		ep_list;	/* of usb_ep */
	enum usb_device_speed		speed;
	unsigned			is_dualspeed:1;
	unsigned			is_otg:1;
	unsigned			is_a_peripheral:1;
	unsigned			b_hnp_enable:1;
	unsigned			a_hnp_support:1;
	unsigned			a_alt_hnp_support:1;
	const char			*name;
	struct device			dev;
};

    How this is going to work with "empty prototype"?.

> Generally, having header files include header files is frowned apon, and
> a number of people have been working on slowly unwinding a lot of this
> type of mess that we currently have.

    IMO they could put their time to better use.

>> include/linux/usb/gadget.h: In function 'set_gadget_data':
>> include/linux/usb/gadget.h:492:
>> 	error: implicit declaration of function 'dev_set_drvdata'

> Ick, but then, this gets messier.

    What gets messier?

> How about just fixing up the .c file that the problem happens in, to
> include device.h first?  Is this an issue in the current tree somehow?

    In my opinion, this is just insane approach.

> thanks,
> greg k-h

WBR, Sergei

Re: [PATCH] usb: gadget: #include device.h in gadget.h

[Greg KH]

On Thu, Jul 08, 2010 at 01:03:27PM +0400, Sergei Shtylyov wrote:
> Hello.
> 
> Greg KH wrote:
> 
> >>gadget.h uses structures defined in device.h, it must include it. In
> >>most cases, gadget.h is preceded by linux/platform_device.h, but if
> >>you are grouping headers sanely, device.h may not be pulled in until
> >>*after* gadget (e.g. mach/msm_device.h), thus gadget.h should not
> >>rely on something else #including device.h
> 
>    As well as a number of other headers. I have postaed a patch
> addressing the missing #include's already.

Yes I know, and my same statment stands.

> >>include/linux/usb/gadget.h:488: error: field 'dev' has incomplete type
> 
> >Why not just provide an "empty" prototype for whatever is needed.
> 
>    Empty prototype of what, 'struct device'? Have you looked at the code at all?

Nope, I try not to :)

> 	struct device			dev;

Ok, that wouldn't work.

> >How about just fixing up the .c file that the problem happens in, to
> >include device.h first?  Is this an issue in the current tree somehow?
> 
>    In my opinion, this is just insane approach.

Sorry, but that seems to go against what the rest of the kernel is
doing.

thanks,

greg k-h

Re: [PATCH] usb: gadget: #include device.h in gadget.h

[Sergei Shtylyov]

Greg KH wrote:

>>>> gadget.h uses structures defined in device.h, it must include it. In
>>>> most cases, gadget.h is preceded by linux/platform_device.h, but if
>>>> you are grouping headers sanely, device.h may not be pulled in until
>>>> *after* gadget (e.g. mach/msm_device.h), thus gadget.h should not
>>>> rely on something else #including device.h

>>    As well as a number of other headers.

    Totally six, to be precise.

>> I have postaed a patch
>> addressing the missing #include's already.

> Yes I know,

    That was mostly for Patrick.

> and my same statment stands.

    :-/

>>>> include/linux/usb/gadget.h:488: error: field 'dev' has incomplete type
>>> Why not just provide an "empty" prototype for whatever is needed.

>>    Empty prototype of what, 'struct device'? Have you looked at the code at all?

> Nope, I try not to :)

    Right, that file has been "stained" by one #include already (which seems 
to be useless though).

>> 	struct device			dev;

> Ok, that wouldn't work.

    Then let's just leave it as it is. :-)

>>> How about just fixing up the .c file that the problem happens in, to
>>> include device.h first?  Is this an issue in the current tree somehow?

>>    In my opinion, this is just insane approach.

> Sorry, but that seems to go against what the rest of the kernel is
> doing.

    Thus far, I've seen headers satisfying their own dependencies, and people 
accepting patches to add missing #include's to headers. This list was the 
first place where I've learned that the problems should be addressed not where 
they exist but left to be dealt with at every place where a defective header 
is used (and the time wasted on that). I haven't heard any convincing 
arguments for this cause so far...

> thanks,
> greg k-h

WBR, Sergei

Re: [PATCH] usb: gadget: #include device.h in gadget.h

[Sergei Shtylyov]

Hello.

Patrick Pannuto wrote:

> gadget.h uses structures defined in device.h, it must include it. In
> most cases, gadget.h is preceded by linux/platform_device.h, but if
> you are grouping headers sanely, device.h may not be pulled in until
> *after* gadget (e.g. mach/msm_device.h), thus gadget.h should not
> rely on something else #including device.h

    Sigh, I've already submitted a more complete patch, adding 6 #include's 
but it seems that prevailing opinion in this list is to leave things as they 
are in the header, and deal with the fallout wherever it's used.

[...]

> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
> index bbf45d5..ddca035 100644
> --- a/include/linux/usb/gadget.h
> +++ b/include/linux/usb/gadget.h
> @@ -15,6 +15,8 @@
>  #ifndef __LINUX_USB_GADGET_H
>  #define __LINUX_USB_GADGET_H
>  
> +#include <linux/device.h>
> +

    Besides, this is not against the recent kernel -- there should be #include 
<linux/slab.h> here.

>  struct usb_ep;
>  
>  /**

WBR, Sergei