* Re: linux-next: Tree for Oct 8 (media/usb/gspca) [not found] <20141008174923.76786a03@canb.auug.org.au> @ 2014-10-08 17:13 ` Randy Dunlap 2014-10-08 18:31 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 8+ messages in thread From: Randy Dunlap @ 2014-10-08 17:13 UTC (permalink / raw) To: Stephen Rothwell, linux-next; +Cc: linux-kernel, Hans de Goede, linux-media On 10/07/14 23:49, Stephen Rothwell wrote: > Hi all, > > Please do not add any material intended for v3.19 to you linux-next > included trees until after v3.18-rc1 has been released. > > Changes since 20141007: > I saw these build errors in gspca when CONFIG_INPUT=m but the gspca sub-drivers are builtin: drivers/built-in.o: In function `gspca_dev_probe2': (.text+0x10ef43): undefined reference to `input_allocate_device' drivers/built-in.o: In function `gspca_dev_probe2': (.text+0x10efdd): undefined reference to `input_register_device' drivers/built-in.o: In function `gspca_dev_probe2': (.text+0x10f002): undefined reference to `input_free_device' drivers/built-in.o: In function `gspca_dev_probe2': (.text+0x10f0ac): undefined reference to `input_unregister_device' drivers/built-in.o: In function `gspca_disconnect': (.text+0x10f186): undefined reference to `input_unregister_device' drivers/built-in.o: In function `sd_int_pkt_scan': se401.c:(.text+0x11373d): undefined reference to `input_event' se401.c:(.text+0x11374e): undefined reference to `input_event' drivers/built-in.o: In function `sd_pkt_scan': t613.c:(.text+0x119f0e): undefined reference to `input_event' t613.c:(.text+0x119f1f): undefined reference to `input_event' drivers/built-in.o: In function `sd_stopN': t613.c:(.text+0x11a047): undefined reference to `input_event' drivers/built-in.o:t613.c:(.text+0x11a058): more undefined references to `input_event' follow These could be fixed in Kconfig by something like (for each sub-driver that tests CONFIG_INPUT): depends on INPUT || INPUT=n Do you have another preference for fixing this? thanks, -- ~Randy ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: linux-next: Tree for Oct 8 (media/usb/gspca) 2014-10-08 17:13 ` linux-next: Tree for Oct 8 (media/usb/gspca) Randy Dunlap @ 2014-10-08 18:31 ` Mauro Carvalho Chehab 2014-10-08 20:53 ` Randy Dunlap 0 siblings, 1 reply; 8+ messages in thread From: Mauro Carvalho Chehab @ 2014-10-08 18:31 UTC (permalink / raw) To: Randy Dunlap Cc: Stephen Rothwell, linux-next, linux-kernel, Hans de Goede, linux-media Em Wed, 08 Oct 2014 10:13:39 -0700 Randy Dunlap <rdunlap@infradead.org> escreveu: > On 10/07/14 23:49, Stephen Rothwell wrote: > > Hi all, > > > > Please do not add any material intended for v3.19 to you linux-next > > included trees until after v3.18-rc1 has been released. > > > > Changes since 20141007: > > > > I saw these build errors in gspca when CONFIG_INPUT=m but the gspca > sub-drivers are builtin: > > drivers/built-in.o: In function `gspca_dev_probe2': > (.text+0x10ef43): undefined reference to `input_allocate_device' > drivers/built-in.o: In function `gspca_dev_probe2': > (.text+0x10efdd): undefined reference to `input_register_device' > drivers/built-in.o: In function `gspca_dev_probe2': > (.text+0x10f002): undefined reference to `input_free_device' > drivers/built-in.o: In function `gspca_dev_probe2': > (.text+0x10f0ac): undefined reference to `input_unregister_device' > drivers/built-in.o: In function `gspca_disconnect': > (.text+0x10f186): undefined reference to `input_unregister_device' > drivers/built-in.o: In function `sd_int_pkt_scan': > se401.c:(.text+0x11373d): undefined reference to `input_event' > se401.c:(.text+0x11374e): undefined reference to `input_event' > drivers/built-in.o: In function `sd_pkt_scan': > t613.c:(.text+0x119f0e): undefined reference to `input_event' > t613.c:(.text+0x119f1f): undefined reference to `input_event' > drivers/built-in.o: In function `sd_stopN': > t613.c:(.text+0x11a047): undefined reference to `input_event' > drivers/built-in.o:t613.c:(.text+0x11a058): more undefined references to `input_event' follow > > These could be fixed in Kconfig by something like (for each sub-driver that tests > CONFIG_INPUT): > > depends on INPUT || INPUT=n > > Do you have another preference for fixing this? Hmm... The code at the gspca subdrivers looks like: #if IS_ENABLED(CONFIG_INPUT) if (data[0] & 0x20) { input_report_key(gspca_dev->input_dev, KEY_CAMERA, 1); input_sync(gspca_dev->input_dev); input_report_key(gspca_dev->input_dev, KEY_CAMERA, 0); input_sync(gspca_dev->input_dev); } #endif As we never got any report about such bug, and this is there for a long time, I suspect that maybe the IS_ENABLED() macro had some changes on its behavior. So, IMHO, we should first check if something changed there. >From gpsca's PoV, IMHO, it should be fine to disable the webcam buttons if the webcam was compiled as builtin and the input subsystem is compiled as module. The core feature expected on a camera is to capture streams. Buttons are just a plus. Also, most cams don't even have buttons. The gspca subdriver has support for buttons for the few models that have it. So, IMHO, it should be ok to have GSPCA=y and INPUT=m, provided that the buttons will be disabled. Regards, Mauro ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: linux-next: Tree for Oct 8 (media/usb/gspca) 2014-10-08 18:31 ` Mauro Carvalho Chehab @ 2014-10-08 20:53 ` Randy Dunlap 2014-10-09 1:50 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 8+ messages in thread From: Randy Dunlap @ 2014-10-08 20:53 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Stephen Rothwell, linux-next, linux-kernel, Hans de Goede, linux-media On 10/08/14 11:31, Mauro Carvalho Chehab wrote: > Em Wed, 08 Oct 2014 10:13:39 -0700 > Randy Dunlap <rdunlap@infradead.org> escreveu: > >> On 10/07/14 23:49, Stephen Rothwell wrote: >>> Hi all, >>> >>> Please do not add any material intended for v3.19 to you linux-next >>> included trees until after v3.18-rc1 has been released. >>> >>> Changes since 20141007: >>> >> >> I saw these build errors in gspca when CONFIG_INPUT=m but the gspca >> sub-drivers are builtin: >> >> drivers/built-in.o: In function `gspca_dev_probe2': >> (.text+0x10ef43): undefined reference to `input_allocate_device' >> drivers/built-in.o: In function `gspca_dev_probe2': >> (.text+0x10efdd): undefined reference to `input_register_device' >> drivers/built-in.o: In function `gspca_dev_probe2': >> (.text+0x10f002): undefined reference to `input_free_device' >> drivers/built-in.o: In function `gspca_dev_probe2': >> (.text+0x10f0ac): undefined reference to `input_unregister_device' >> drivers/built-in.o: In function `gspca_disconnect': >> (.text+0x10f186): undefined reference to `input_unregister_device' >> drivers/built-in.o: In function `sd_int_pkt_scan': >> se401.c:(.text+0x11373d): undefined reference to `input_event' >> se401.c:(.text+0x11374e): undefined reference to `input_event' >> drivers/built-in.o: In function `sd_pkt_scan': >> t613.c:(.text+0x119f0e): undefined reference to `input_event' >> t613.c:(.text+0x119f1f): undefined reference to `input_event' >> drivers/built-in.o: In function `sd_stopN': >> t613.c:(.text+0x11a047): undefined reference to `input_event' >> drivers/built-in.o:t613.c:(.text+0x11a058): more undefined references to `input_event' follow >> >> These could be fixed in Kconfig by something like (for each sub-driver that tests >> CONFIG_INPUT): >> >> depends on INPUT || INPUT=n >> >> Do you have another preference for fixing this? > > Hmm... The code at the gspca subdrivers looks like: > > #if IS_ENABLED(CONFIG_INPUT) For builtin only, that should be #if IS_BUILTIN(CONFIG_INPUT) > if (data[0] & 0x20) { > input_report_key(gspca_dev->input_dev, KEY_CAMERA, 1); > input_sync(gspca_dev->input_dev); > input_report_key(gspca_dev->input_dev, KEY_CAMERA, 0); > input_sync(gspca_dev->input_dev); > } > #endif > > As we never got any report about such bug, and this is there for a long > time, I suspect that maybe the IS_ENABLED() macro had some changes on > its behavior. So, IMHO, we should first check if something changed there. I don't see any changes in <linux/kconfig.h>. > From gpsca's PoV, IMHO, it should be fine to disable the webcam buttons if > the webcam was compiled as builtin and the input subsystem is compiled as > module. The core feature expected on a camera is to capture streams. > Buttons are just a plus. > > Also, most cams don't even have buttons. The gspca subdriver has support > for buttons for the few models that have it. > > So, IMHO, it should be ok to have GSPCA=y and INPUT=m, provided that > the buttons will be disabled. Then all of the sub-drivers that use IS_ENABLED(CONFIG_INPUT) should be changed to use IS_BUILTIN(CONFIG_INPUT). But that is too restrictive IMO. The input subsystem will work fine when CONFIG_INPUT=m and the GSPCA drivers are also loadable modules. That's simple to express in Kconfig language but probly more messy in CPP. Thanks. -- ~Randy ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: linux-next: Tree for Oct 8 (media/usb/gspca) 2014-10-08 20:53 ` Randy Dunlap @ 2014-10-09 1:50 ` Mauro Carvalho Chehab 2014-10-09 6:45 ` Paul Bolle 0 siblings, 1 reply; 8+ messages in thread From: Mauro Carvalho Chehab @ 2014-10-09 1:50 UTC (permalink / raw) To: Randy Dunlap Cc: Stephen Rothwell, linux-next, linux-kernel, Hans de Goede, linux-media Em Wed, 08 Oct 2014 13:53:33 -0700 Randy Dunlap <rdunlap@infradead.org> escreveu: > On 10/08/14 11:31, Mauro Carvalho Chehab wrote: > > Em Wed, 08 Oct 2014 10:13:39 -0700 > > Randy Dunlap <rdunlap@infradead.org> escreveu: > > > >> On 10/07/14 23:49, Stephen Rothwell wrote: > >>> Hi all, > >>> > >>> Please do not add any material intended for v3.19 to you linux-next > >>> included trees until after v3.18-rc1 has been released. > >>> > >>> Changes since 20141007: > >>> > >> > >> I saw these build errors in gspca when CONFIG_INPUT=m but the gspca > >> sub-drivers are builtin: > >> > >> drivers/built-in.o: In function `gspca_dev_probe2': > >> (.text+0x10ef43): undefined reference to `input_allocate_device' > >> drivers/built-in.o: In function `gspca_dev_probe2': > >> (.text+0x10efdd): undefined reference to `input_register_device' > >> drivers/built-in.o: In function `gspca_dev_probe2': > >> (.text+0x10f002): undefined reference to `input_free_device' > >> drivers/built-in.o: In function `gspca_dev_probe2': > >> (.text+0x10f0ac): undefined reference to `input_unregister_device' > >> drivers/built-in.o: In function `gspca_disconnect': > >> (.text+0x10f186): undefined reference to `input_unregister_device' > >> drivers/built-in.o: In function `sd_int_pkt_scan': > >> se401.c:(.text+0x11373d): undefined reference to `input_event' > >> se401.c:(.text+0x11374e): undefined reference to `input_event' > >> drivers/built-in.o: In function `sd_pkt_scan': > >> t613.c:(.text+0x119f0e): undefined reference to `input_event' > >> t613.c:(.text+0x119f1f): undefined reference to `input_event' > >> drivers/built-in.o: In function `sd_stopN': > >> t613.c:(.text+0x11a047): undefined reference to `input_event' > >> drivers/built-in.o:t613.c:(.text+0x11a058): more undefined references to `input_event' follow > >> > >> These could be fixed in Kconfig by something like (for each sub-driver that tests > >> CONFIG_INPUT): > >> > >> depends on INPUT || INPUT=n > >> > >> Do you have another preference for fixing this? > > > > Hmm... The code at the gspca subdrivers looks like: > > > > #if IS_ENABLED(CONFIG_INPUT) > > For builtin only, that should be > > #if IS_BUILTIN(CONFIG_INPUT) > > > if (data[0] & 0x20) { > > input_report_key(gspca_dev->input_dev, KEY_CAMERA, 1); > > input_sync(gspca_dev->input_dev); > > input_report_key(gspca_dev->input_dev, KEY_CAMERA, 0); > > input_sync(gspca_dev->input_dev); > > } > > #endif > > > > As we never got any report about such bug, and this is there for a long > > time, I suspect that maybe the IS_ENABLED() macro had some changes on > > its behavior. So, IMHO, we should first check if something changed there. > > I don't see any changes in <linux/kconfig.h>. Perhaps some changes at the Kbuild source code or scripts badly affected it. > > > From gpsca's PoV, IMHO, it should be fine to disable the webcam buttons if > > the webcam was compiled as builtin and the input subsystem is compiled as > > module. The core feature expected on a camera is to capture streams. > > Buttons are just a plus. > > > > Also, most cams don't even have buttons. The gspca subdriver has support > > for buttons for the few models that have it. > > > > So, IMHO, it should be ok to have GSPCA=y and INPUT=m, provided that > > the buttons will be disabled. > > Then all of the sub-drivers that use IS_ENABLED(CONFIG_INPUT) should be > changed to use IS_BUILTIN(CONFIG_INPUT). > > But that is too restrictive IMO. The input subsystem will work fine when > CONFIG_INPUT=m and the GSPCA drivers are also loadable modules. Agreed. Maybe the solution would be something more complex like (for drivers/media/usb/gspca/zc3xx.c): #if (IS_BUILTIN(CONFIG_INPUT)) || (IS_ENABLED(CONFIG_INPUT) && !IS_BUILTIN(CONFIG_USB_GSPCA_ZC3XX)) Probably the best would be to write another macro that would evaluate like the above. > That's simple to express in Kconfig language but probly more messy in CPP. > > > Thanks. > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: linux-next: Tree for Oct 8 (media/usb/gspca) 2014-10-09 1:50 ` Mauro Carvalho Chehab @ 2014-10-09 6:45 ` Paul Bolle 2014-10-09 10:30 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 8+ messages in thread From: Paul Bolle @ 2014-10-09 6:45 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Randy Dunlap, Stephen Rothwell, linux-next, linux-kernel, Hans de Goede, linux-media On Wed, 2014-10-08 at 22:50 -0300, Mauro Carvalho Chehab wrote: > Em Wed, 08 Oct 2014 13:53:33 -0700 > Randy Dunlap <rdunlap@infradead.org> escreveu: > > On 10/08/14 11:31, Mauro Carvalho Chehab wrote: > > > From gpsca's PoV, IMHO, it should be fine to disable the webcam buttons if > > > the webcam was compiled as builtin and the input subsystem is compiled as > > > module. The core feature expected on a camera is to capture streams. > > > Buttons are just a plus. > > > > > > Also, most cams don't even have buttons. The gspca subdriver has support > > > for buttons for the few models that have it. > > > > > > So, IMHO, it should be ok to have GSPCA=y and INPUT=m, provided that > > > the buttons will be disabled. > > > > Then all of the sub-drivers that use IS_ENABLED(CONFIG_INPUT) should be > > changed to use IS_BUILTIN(CONFIG_INPUT). > > > > But that is too restrictive IMO. The input subsystem will work fine when > > CONFIG_INPUT=m and the GSPCA drivers are also loadable modules. > > Agreed. > > Maybe the solution would be something more complex like > (for drivers/media/usb/gspca/zc3xx.c): > > #if (IS_BUILTIN(CONFIG_INPUT)) || (IS_ENABLED(CONFIG_INPUT) && !IS_BUILTIN(CONFIG_USB_GSPCA_ZC3XX)) The above discussion meanders a bit, and I just stumbled onto it, but would #if IS_BUILTIN(CONFIG_INPUT) || (IS_MODULE(CONFIG_INPUT) && defined(MODULE)) cover your requirements when using macros? > Probably the best would be to write another macro that would evaluate > like the above. Paul Bolle ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: linux-next: Tree for Oct 8 (media/usb/gspca) 2014-10-09 6:45 ` Paul Bolle @ 2014-10-09 10:30 ` Mauro Carvalho Chehab 2014-10-09 11:26 ` Paul Bolle 0 siblings, 1 reply; 8+ messages in thread From: Mauro Carvalho Chehab @ 2014-10-09 10:30 UTC (permalink / raw) To: Paul Bolle Cc: Randy Dunlap, Stephen Rothwell, linux-next, linux-kernel, Hans de Goede, linux-media Em Thu, 09 Oct 2014 08:45:28 +0200 Paul Bolle <pebolle@tiscali.nl> escreveu: > On Wed, 2014-10-08 at 22:50 -0300, Mauro Carvalho Chehab wrote: > > Em Wed, 08 Oct 2014 13:53:33 -0700 > > Randy Dunlap <rdunlap@infradead.org> escreveu: > > > On 10/08/14 11:31, Mauro Carvalho Chehab wrote: > > > > From gpsca's PoV, IMHO, it should be fine to disable the webcam buttons if > > > > the webcam was compiled as builtin and the input subsystem is compiled as > > > > module. The core feature expected on a camera is to capture streams. > > > > Buttons are just a plus. > > > > > > > > Also, most cams don't even have buttons. The gspca subdriver has support > > > > for buttons for the few models that have it. > > > > > > > > So, IMHO, it should be ok to have GSPCA=y and INPUT=m, provided that > > > > the buttons will be disabled. > > > > > > Then all of the sub-drivers that use IS_ENABLED(CONFIG_INPUT) should be > > > changed to use IS_BUILTIN(CONFIG_INPUT). > > > > > > But that is too restrictive IMO. The input subsystem will work fine when > > > CONFIG_INPUT=m and the GSPCA drivers are also loadable modules. > > > > Agreed. > > > > Maybe the solution would be something more complex like > > (for drivers/media/usb/gspca/zc3xx.c): > > > > #if (IS_BUILTIN(CONFIG_INPUT)) || (IS_ENABLED(CONFIG_INPUT) && !IS_BUILTIN(CONFIG_USB_GSPCA_ZC3XX)) > > The above discussion meanders a bit, and I just stumbled onto it, but > would > #if IS_BUILTIN(CONFIG_INPUT) || (IS_MODULE(CONFIG_INPUT) && defined(MODULE)) > > cover your requirements when using macros? No. What we need to do, for all gspca sub-drivers that have optional support for buttons is to only enable the buttons support if: CONFIG_INPUT=y or CONFIG_INPUT=m and CONFIG_USB_GSPCA_submodule=m If we use a reverse logic, we need to disable the code if: # CONFIG_INPUT is not set or CONFIG_INPUT=m and CONFIG_USB_GSPCA_submodule=y The rationale for disabling the code on the last expression is that a builtin code cannot call a function inside a module. Also, as the submodule is already being compiled, we know that CONFIG_USB_GSPCA_submodule is either module or builtin. So, either one of those expressions should work: #if (IS_BUILTIN(CONFIG_INPUT)) || (IS_ENABLED(CONFIG_INPUT) && !IS_BUILTIN(CONFIG_USB_GSPCA_submodule)) or #if (IS_BUILTIN(CONFIG_INPUT)) || (IS_MODULE(CONFIG_INPUT) && IS_MODULE(CONFIG_USB_GSPCA_submodule) && defined(MODULE)) or #if (IS_BUILTIN(CONFIG_INPUT)) || (IS_ENABLED(CONFIG_INPUT) && IS_MODULE(CONFIG_USB_GSPCA_submodule)) > > > Probably the best would be to write another macro that would evaluate > > like the above. > > > Paul Bolle > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: linux-next: Tree for Oct 8 (media/usb/gspca) 2014-10-09 10:30 ` Mauro Carvalho Chehab @ 2014-10-09 11:26 ` Paul Bolle 2014-10-09 11:52 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 8+ messages in thread From: Paul Bolle @ 2014-10-09 11:26 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Randy Dunlap, Stephen Rothwell, linux-next, linux-kernel, Hans de Goede, linux-media On Thu, 2014-10-09 at 07:30 -0300, Mauro Carvalho Chehab wrote: > Em Thu, 09 Oct 2014 08:45:28 +0200 > Paul Bolle <pebolle@tiscali.nl> escreveu: > > The above discussion meanders a bit, and I just stumbled onto it, but > > would > > #if IS_BUILTIN(CONFIG_INPUT) || (IS_MODULE(CONFIG_INPUT) && defined(MODULE)) > > > > cover your requirements when using macros? > > No. What we need to do, for all gspca sub-drivers that have optional > support for buttons is to only enable the buttons support if: > > CONFIG_INPUT=y > or > CONFIG_INPUT=m and CONFIG_USB_GSPCA_submodule=m > > If we use a reverse logic, we need to disable the code if: > # CONFIG_INPUT is not set > or > CONFIG_INPUT=m and CONFIG_USB_GSPCA_submodule=y > > The rationale for disabling the code on the last expression is that a > builtin code cannot call a function inside a module. > > Also, as the submodule is already being compiled, we know that > CONFIG_USB_GSPCA_submodule is either module or builtin. > > So, either one of those expressions should work: > #if (IS_BUILTIN(CONFIG_INPUT)) || (IS_ENABLED(CONFIG_INPUT) && !IS_BUILTIN(CONFIG_USB_GSPCA_submodule)) > or > #if (IS_BUILTIN(CONFIG_INPUT)) || (IS_MODULE(CONFIG_INPUT) && IS_MODULE(CONFIG_USB_GSPCA_submodule) && defined(MODULE)) I thought MODULE was only defined for code that will be part of a module. So "IS_MODULE(CONFIG_USB_GSPCA_submodule)" and "defined(MODULE)" should be equal when used _inside_ [...]/usb/gspca/that_submodule.c, shouldn't they? That would make this option basically identical to my suggestion. Or are you thinking about using these tests outside of these submodules themselves? > or > #if (IS_BUILTIN(CONFIG_INPUT)) || (IS_ENABLED(CONFIG_INPUT) && IS_MODULE(CONFIG_USB_GSPCA_submodule)) I think it's clearer to use IS_BUILTIN(CONFIG_FOO) || (IS_MODULE(CONFIG_FOO) && [...]) Ditto above. Perhaps just a matter of taste. (Depending on INPUT is apparently not possible for these submodules. So obviously any solution needs to check whether input is available, say like if (IS_MODULE(CONFIG_INPUT)) if (!is_input_loaded()) goto no_input; Doesn't it?) Thanks, Paul Bolle ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: linux-next: Tree for Oct 8 (media/usb/gspca) 2014-10-09 11:26 ` Paul Bolle @ 2014-10-09 11:52 ` Mauro Carvalho Chehab 0 siblings, 0 replies; 8+ messages in thread From: Mauro Carvalho Chehab @ 2014-10-09 11:52 UTC (permalink / raw) To: Paul Bolle Cc: Randy Dunlap, Stephen Rothwell, linux-next, linux-kernel, Hans de Goede, linux-media Em Thu, 09 Oct 2014 13:26:10 +0200 Paul Bolle <pebolle@tiscali.nl> escreveu: > On Thu, 2014-10-09 at 07:30 -0300, Mauro Carvalho Chehab wrote: > > Em Thu, 09 Oct 2014 08:45:28 +0200 > > Paul Bolle <pebolle@tiscali.nl> escreveu: > > > The above discussion meanders a bit, and I just stumbled onto it, but > > > would > > > #if IS_BUILTIN(CONFIG_INPUT) || (IS_MODULE(CONFIG_INPUT) && defined(MODULE)) > > > > > > cover your requirements when using macros? > > > > No. What we need to do, for all gspca sub-drivers that have optional > > support for buttons is to only enable the buttons support if: > > > > CONFIG_INPUT=y > > or > > CONFIG_INPUT=m and CONFIG_USB_GSPCA_submodule=m > > > > If we use a reverse logic, we need to disable the code if: > > # CONFIG_INPUT is not set > > or > > CONFIG_INPUT=m and CONFIG_USB_GSPCA_submodule=y > > > > The rationale for disabling the code on the last expression is that a > > builtin code cannot call a function inside a module. > > > > Also, as the submodule is already being compiled, we know that > > CONFIG_USB_GSPCA_submodule is either module or builtin. > > > > So, either one of those expressions should work: > > #if (IS_BUILTIN(CONFIG_INPUT)) || (IS_ENABLED(CONFIG_INPUT) && !IS_BUILTIN(CONFIG_USB_GSPCA_submodule)) > > or > > #if (IS_BUILTIN(CONFIG_INPUT)) || (IS_MODULE(CONFIG_INPUT) && IS_MODULE(CONFIG_USB_GSPCA_submodule) && defined(MODULE)) > > I thought MODULE was only defined for code that will be part of a > module. So "IS_MODULE(CONFIG_USB_GSPCA_submodule)" and "defined(MODULE)" > should be equal when used _inside_ [...]/usb/gspca/that_submodule.c, Ah, I didn't know that. In such case, your suggestion is indeed better, as it is easier to write the patch. > shouldn't they? That would make this option basically identical to my > suggestion. Or are you thinking about using these tests outside of these > submodules themselves? > > > or > > #if (IS_BUILTIN(CONFIG_INPUT)) || (IS_ENABLED(CONFIG_INPUT) && IS_MODULE(CONFIG_USB_GSPCA_submodule)) > > I think it's clearer to use > IS_BUILTIN(CONFIG_FOO) || (IS_MODULE(CONFIG_FOO) && [...]) > > Ditto above. Perhaps just a matter of taste. > > (Depending on INPUT is apparently not possible for these submodules. No, because the main functionality (webcam support) doesn't depend on INPUT. > So > obviously any solution needs to check whether input is available, say > like > if (IS_MODULE(CONFIG_INPUT)) > if (!is_input_loaded()) > goto no_input; > > Doesn't it?) Yeah, but the thing is that it breaks at compilation time, because the builtin module would try to use some symbols that are defined only at runtime. So, the above won't solve the issue. One possibility for future Kernels would be to add some logic that would automatically load the module if a call to one of those symbols defined inside a module happens inside a builtin module. The DVB subsystem actually does that for I2C frontend drivers, using those macros: #define dvb_attach(FUNCTION, ARGS...) ({ \ void *__r = NULL; \ typeof(&FUNCTION) __a = symbol_request(FUNCTION); \ if (__a) { \ __r = (void *) __a(ARGS); \ if (__r == NULL) \ symbol_put(FUNCTION); \ } else { \ printk(KERN_ERR "DVB: Unable to find symbol "#FUNCTION"()\n"); \ } \ __r; \ }) #define dvb_detach(FUNC) symbol_put_addr(FUNC) The above works reasonably fine when there's just one symbol needed from the module driver. There are, however, some issues with such approach: 1) as symbol_request increments refcount, this works very badly when there's more than one symbol needed, as symbol_put() would need to be called as many times as symbol_request() is called; 2) lsmod doesn't list what module is actually requesting such symbol (if the caller is also a module). It just increments refcounting. There were some patches meant to fix that, send a long time ago, but were never merged. Can't remember why: http://linuxtv.org/pipermail/linux-dvb/2006-August/012322.html Due to the above issues, and because we want to better use the I2C binding model, we're currently trying to get rid of such approach for the DVB subsystem, but there are too many changes to get rid of it. So, the migration is slow. Anyway, IMHO, having such sort of logic would help to solve the issues with some weird configs like INPUT=m. Regards, Mauro ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-10-09 11:52 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20141008174923.76786a03@canb.auug.org.au>
2014-10-08 17:13 ` linux-next: Tree for Oct 8 (media/usb/gspca) Randy Dunlap
2014-10-08 18:31 ` Mauro Carvalho Chehab
2014-10-08 20:53 ` Randy Dunlap
2014-10-09 1:50 ` Mauro Carvalho Chehab
2014-10-09 6:45 ` Paul Bolle
2014-10-09 10:30 ` Mauro Carvalho Chehab
2014-10-09 11:26 ` Paul Bolle
2014-10-09 11:52 ` Mauro Carvalho Chehab
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).