usb: gadget: #include device.h in gadget.h

PATCH In review Patrick Pannuto 3 participants 6 messages Jul 8, 2010 USB

Share & tools

Patrick Pannuto

author

2010-07-08 00:47 UTC #0 lore raw

PATCH 1/1 usb: gadget: #include device.h in gadget.h

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

---
 include/linux/usb/gadget.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

include/linux/usb/gadget.h +2 -0

index bbf45d5..ddca035 100644
@@ -15,6 +15,8 @@
 #ifndef __LINUX_USB_GADGET_H
 #define __LINUX_USB_GADGET_H
 
+#include <linux/device.h>
+
 struct usb_ep;
 
 /**

Signature · 1 line

1.7.1

Greg KH

2010-07-08 03:36 UTC #1 lore raw

Quoted 7 lines · 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

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.

Quoted 3 lines

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

Sergei Shtylyov

2010-07-08 09:04 UTC #2 lore raw

Hello.

Quoted 5 lines · Greg KH

Quoted 5 lines
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.

Quoted 3 lines

Quoted 1 line
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"?.

Quoted 3 lines

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.

Quoted 5 lines

Quoted 3 lines
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?

Quoted 2 lines

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.

Quoted 2 lines

thanks,
greg k-h

WBR, Sergei

Greg KH

2010-07-08 15:15 UTC #3 lore raw

Quoted 4 lines · Sergei Shtylyov

Hello.
Greg KH wrote:

Quoted 8 lines

Quoted 5 lines
Quoted 5 lines
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.

Quoted 5 lines

Quoted 3 lines
Quoted 1 line
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 :)

Quoted 1 line

	struct device			dev;

Ok, that wouldn't work.

Quoted 4 lines

Quoted 2 lines
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

Sergei Shtylyov

2010-07-08 17:47 UTC #4 lore raw

Quoted 7 lines · Greg KH

Quoted 7 lines
Quoted 5 lines
Quoted 5 lines
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.

Quoted 4 lines

Quoted 2 lines
I have postaed a patch
addressing the missing #include's already.
Yes I know,

That was mostly for Patrick.

Quoted 1 line

and my same statment stands.

:-/

Quoted 6 lines

Quoted 4 lines
Quoted 2 lines
Quoted 1 line
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).

Quoted 3 lines

Quoted 1 line
	struct device			dev;
Ok, that wouldn't work.

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

Quoted 7 lines

Quoted 4 lines
Quoted 2 lines
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...

Quoted 2 lines

thanks,
greg k-h

WBR, Sergei

Sergei Shtylyov

2010-07-08 19:13 UTC #5 lore raw

In reply to Patrick Pannuto

Hello.

Quoted 5 lines · 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

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.

[...]

include/linux/usb/gadget.h line 19

Quoted 10 lines

include/linux/usb/gadget.h +2 -0

index bbf45d5..ddca035 100644
@@ -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.

include/linux/usb/gadget.h line 22

Quoted 3 lines

include/linux/usb/gadget.h +0 -0
 struct usb_ep;
 
 /**

WBR, Sergei