[GIT,PULL,for,v4.6-rc1] media updates

Commit Message

Stefan Lippers-Hollmann May 4, 2016, 7:28 p.m. UTC

Hi

On 2016-05-04, Linus Torvalds wrote:
> On Tue, May 3, 2016 at 9:39 PM, Stefan Lippers-Hollmann <[e-mail]> wrote:
> >
> > Just as a cross-check, this (incomplete, but au0828, cx231xx and em28xx
> > aren't needed/ loaded on my system) crude revert avoids the problem for
> > me on v4.6-rc6-113-g83858a7.  
> 
> Hmm.
> 
> That just open-codes __media_device_usb_init().
> 
> The main difference seems to be that __media_device_usb_init() ends up
> having that
> 
>      #ifdef CONFIG_USB
>      #endif
> 
> around it.
> 
> I think that is bogus.
> 
> What happens if you replace that #ifdef CONFIG_USB in
> __media_device_usb_init() with
> 
>     #if CONFIG_USB || (MODULE && CONFIG_USB_MODULE)
[...]

that throws

drivers/media/media-device.c: In function '__media_device_usb_init':
drivers/media/media-device.c:878:5: warning: "CONFIG_USB" is not defined [-Wundef]
 #if CONFIG_USB || (MODULE && CONFIG_USB_MODULE)
     ^

however, taking arch/arm/mach-omap1/include/mach/usb.h as example, 
changing it to 


indeed fixes the problem for me

Thanks a lot!

Regards
	Stefan Lippers-Hollmann

Comments

Linus Torvalds May 4, 2016, 8:49 p.m. UTC | #1

On Wed, May 4, 2016 at 12:28 PM, Stefan Lippers-Hollmann <[e-mail]> wrote:
>
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -875,7 +875,7 @@ void __media_device_usb_init(struct medi
>                              const char *board_name,
>                              const char *driver_name)
>  {
> -#ifdef CONFIG_USB
> +#if defined(CONFIG_USB) || defined(CONFIG_USB_MODULE)

Ok, that should be fine. Can you verify that it builds and works even
if USB isn't compiled in, but the media core code is?

IOW, can you test the

  CONFIG_USB=m
  CONFIG_MEDIA_CONTROLLER=y
  CONFIG_MEDIA_SUPPORT=y

case? Judging by your oops stack trace, I think you currently have
MEDIA_SUPPORT=m.

Also, I do wonder if we should move that #if to _outside_ the
function. Because inside the function, things will compile but
silently not work (like you found), if it is ever mis-used. Outside
that function, you'll get link-errors if you try to misuse that
function.

              Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Stefan Lippers-Hollmann May 4, 2016, 9:45 p.m. UTC | #2

Hi

On 2016-05-04, Linus Torvalds wrote:
> On Wed, May 4, 2016 at 12:28 PM, Stefan Lippers-Hollmann <[e-mail]> wrote:
> >
> > --- a/drivers/media/media-device.c
> > +++ b/drivers/media/media-device.c
> > @@ -875,7 +875,7 @@ void __media_device_usb_init(struct medi
> >                              const char *board_name,
> >                              const char *driver_name)
> >  {
> > -#ifdef CONFIG_USB
> > +#if defined(CONFIG_USB) || defined(CONFIG_USB_MODULE)  
> 
> Ok, that should be fine. Can you verify that it builds and works even
> if USB isn't compiled in, but the media core code is?
> 
> IOW, can you test the
> 
>   CONFIG_USB=m
>   CONFIG_MEDIA_CONTROLLER=y
>   CONFIG_MEDIA_SUPPORT=y

Builds (without warnings in drivers/media/media-device.*) and works fine
as well.

> case? Judging by your oops stack trace, I think you currently have
> MEDIA_SUPPORT=m.

My usual configuration (which, as mentioned in the previous mail, now 
builds and works as well) is:

CONFIG_MEDIA_SUPPORT=m
CONFIG_MEDIA_CONTROLLER=y
CONFIG_USB=m

> Also, I do wonder if we should move that #if to _outside_ the
> function. Because inside the function, things will compile but
> silently not work (like you found), if it is ever mis-used. Outside
> that function, you'll get link-errors if you try to misuse that
> function.

That would probably be the best approach.

Regards
	Stefan Lippers-Hollmann
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -875,7 +875,7 @@  void __media_device_usb_init(struct medi
 			     const char *board_name,
 			     const char *driver_name)
 {
-#ifdef CONFIG_USB
+#if defined(CONFIG_USB) || defined(CONFIG_USB_MODULE)
 	mdev->dev = &udev->dev;
 
 	if (driver_name)