usb: gadget: #include device.h in gadget.h
in [Kernel]
|
Prev: classmate-laptop: make needlessly global symbols static
Next: Let host know whether the guest can handle async PF in non-userspace context.
From: Greg KH on 7 Jul 2010 23:40 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Sergei Shtylyov on 8 Jul 2010 05:10 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Greg KH on 8 Jul 2010 11:20 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Sergei Shtylyov on 8 Jul 2010 13:50 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Sergei Shtylyov on 8 Jul 2010 15:20
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |