diff mbox

linux-next: Tree for May 1 (media/usb/stk1160)

Message ID 51827DB1.7000304@redhat.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Mauro Carvalho Chehab May 2, 2013, 2:52 p.m. UTC 
  Em 01-05-2013 14:59, Randy Dunlap escreveu:
> On 05/01/13 01:37, Stephen Rothwell wrote:
>> Hi all,
>>
>> Please do not add any v3.11 destined work to your linux-next included
>> branches until after v3.10-rc1 is released.
>>
>> Changes since 20130430:
>>
>
>
> When CONFIG_SND=m and CONFIG_SND_AC97_CODEC=m and
> CONFIG_VIDEO_STK1160=y
> CONFIG_VIDEO_STK1160_AC97=y
>
> drivers/built-in.o: In function `stk1160_ac97_register':
> (.text+0x122706): undefined reference to `snd_card_create'
> drivers/built-in.o: In function `stk1160_ac97_register':
> (.text+0x1227b2): undefined reference to `snd_ac97_bus'
> drivers/built-in.o: In function `stk1160_ac97_register':
> (.text+0x1227cd): undefined reference to `snd_card_free'
> drivers/built-in.o: In function `stk1160_ac97_register':
> (.text+0x12281b): undefined reference to `snd_ac97_mixer'
> drivers/built-in.o: In function `stk1160_ac97_register':
> (.text+0x122832): undefined reference to `snd_card_register'
> drivers/built-in.o: In function `stk1160_ac97_unregister':
> (.text+0x12285e): undefined reference to `snd_card_free'
>
>
> This kconfig fragment:
> config VIDEO_STK1160_AC97
> 	bool "STK1160 AC97 codec support"
> 	depends on VIDEO_STK1160 && SND
> 	select SND_AC97_CODEC
>
> is unreliable (doesn't do what some people expect) when SND=m and SND_AC97_CODEC=m,
> since VIDEO_STK1160_AC97 is a bool.

Using select is always tricky.

I can see a few possible fixes for it:

1) split the alsa part into a separate module. IMHO, this is cleaner,
but requires a little more work.

2) Use the Kconfig syntax:

	depends on SND || (SND=n)

on a tristate symbol. That behaves like:

	if SND is 'n', it won't depend on SND;
	if SND is 'm', the symbol will be 'm'
	if SND is 'y', the symbol will be 'y'.

However, as as VIDEO_STK1160_AC97 is boolean, this will require
an additional hidden Kconfig. Something like:

config VIDEO_STK1160_COMMON
	tristate "STK1160 USB video capture support"
	depends on VIDEO_DEV && I2C

config VIDEO_STK1160_AC97
	bool "STK1160 AC97 codec support"
	depends on VIDEO_STK1160_COMMON && SND

config VIDEO_STK1160
	tristate
	depends on ((SND || (SND=n) || !VIDEO_STK1160_AC97) && VIDEO_STK1160_COMMON
	default y
	select SND_AC97_CODEC if SND
	select VIDEOBUF2_VMALLOC
	select VIDEO_SAA711X
	select SND_AC97_CODEC

We do already something similar to the above for the mutual dependency
of most media drivers for I2C and V4L2 and/or DVB core.

There's just one small drawback with the above: if SND='m', even if
the user selects VIDEO_STK1160_COMMON='y', VIDEO_STK1160 will be 'm'.

A quick test here with make allyesconfig and then changing SND to m
seemed to produce the right value for CONFIG_VIDEO_STK1160:

Selecting STK1160_AC97:

$ grep -e STK1160 -e SND= .config
CONFIG_VIDEO_STK1160_COMMON=y
CONFIG_VIDEO_STK1160_AC97=y
CONFIG_VIDEO_STK1160=m
CONFIG_SND=m

Unselecting STK1160_AC97:

$ grep -e STK1160 -e SND= .config
CONFIG_VIDEO_STK1160_COMMON=y
# CONFIG_VIDEO_STK1160_AC97 is not set
CONFIG_VIDEO_STK1160=y
CONFIG_SND=m

With a little more work, it could be possible to find a way to
avoid the drawback of saying to the user that the module will be
builtin, but compiling it as a module.

Regards,
Mauro.

-

[media] stk1160: Make stk1160 module if SND is m and audio support is selected

As reported by Randy:

When CONFIG_SND=m and CONFIG_SND_AC97_CODEC=m and
CONFIG_VIDEO_STK1160=y
CONFIG_VIDEO_STK1160_AC97=y

drivers/built-in.o: In function `stk1160_ac97_register':
(.text+0x122706): undefined reference to `snd_card_create'
drivers/built-in.o: In function `stk1160_ac97_register':
(.text+0x1227b2): undefined reference to `snd_ac97_bus'
drivers/built-in.o: In function `stk1160_ac97_register':
(.text+0x1227cd): undefined reference to `snd_card_free'
drivers/built-in.o: In function `stk1160_ac97_register':
(.text+0x12281b): undefined reference to `snd_ac97_mixer'
drivers/built-in.o: In function `stk1160_ac97_register':
(.text+0x122832): undefined reference to `snd_card_register'
drivers/built-in.o: In function `stk1160_ac97_unregister':
(.text+0x12285e): undefined reference to `snd_card_free'

Reported-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>



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

Comments

Randy Dunlap May 2, 2013, 9:23 p.m. UTC | #1 
On 05/02/13 07:52, Mauro Carvalho Chehab wrote:
> [media] stk1160: Make stk1160 module if SND is m and audio support is selected
> 
> As reported by Randy:
> 
> When CONFIG_SND=m and CONFIG_SND_AC97_CODEC=m and
> CONFIG_VIDEO_STK1160=y
> CONFIG_VIDEO_STK1160_AC97=y
> 
> drivers/built-in.o: In function `stk1160_ac97_register':
> (.text+0x122706): undefined reference to `snd_card_create'
> drivers/built-in.o: In function `stk1160_ac97_register':
> (.text+0x1227b2): undefined reference to `snd_ac97_bus'
> drivers/built-in.o: In function `stk1160_ac97_register':
> (.text+0x1227cd): undefined reference to `snd_card_free'
> drivers/built-in.o: In function `stk1160_ac97_register':
> (.text+0x12281b): undefined reference to `snd_ac97_mixer'
> drivers/built-in.o: In function `stk1160_ac97_register':
> (.text+0x122832): undefined reference to `snd_card_register'
> drivers/built-in.o: In function `stk1160_ac97_unregister':
> (.text+0x12285e): undefined reference to `snd_card_free'
> 
> Reported-by: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

Acked-by: Randy Dunlap <rdunlap@infradead.org>

Thanks.

> 
> diff --git a/drivers/media/usb/stk1160/Kconfig b/drivers/media/usb/stk1160/Kconfig
> index 1c3a1ec..2bf6392 100644
> --- a/drivers/media/usb/stk1160/Kconfig
> +++ b/drivers/media/usb/stk1160/Kconfig
> @@ -1,8 +1,6 @@
> -config VIDEO_STK1160
> +config VIDEO_STK1160_COMMON
>      tristate "STK1160 USB video capture support"
>      depends on VIDEO_DEV && I2C
> -    select VIDEOBUF2_VMALLOC
> -    select VIDEO_SAA711X
>  
>      ---help---
>        This is a video4linux driver for STK1160 based video capture devices.
> @@ -12,9 +10,14 @@ config VIDEO_STK1160
>  
>  config VIDEO_STK1160_AC97
>      bool "STK1160 AC97 codec support"
> -    depends on VIDEO_STK1160 && SND
> -    select SND_AC97_CODEC
> -
> +    depends on VIDEO_STK1160_COMMON && SND
>      ---help---
>        Enables AC97 codec support for stk1160 driver.
> -.
> +
> +config VIDEO_STK1160
> +    tristate
> +    depends on (!VIDEO_STK1160_AC97 || (SND='n') || SND) && VIDEO_STK1160_COMMON
> +    default y
> +    select VIDEOBUF2_VMALLOC
> +    select VIDEO_SAA711X
> +    select SND_AC97_CODEC if SND
> 
> 
> --
Ezequiel Garcia May 4, 2013, 5:21 p.m. UTC | #2 
Hi Mauro,

On Thu, May 02, 2013 at 11:52:33AM -0300, Mauro Carvalho Chehab wrote:
> >
> > is unreliable (doesn't do what some people expect) when SND=m and SND_AC97_CODEC=m,
> > since VIDEO_STK1160_AC97 is a bool.
> 
> Using select is always tricky.
> 
> I can see a few possible fixes for it:
> 
> 1) split the alsa part into a separate module. IMHO, this is cleaner,
> but requires a little more work.
> 

I'm trying to split the ac97 support into a separate module.
So far I've managed to do this with two different approaches,
but both of them are broken in some way :-(

Couple questions:

1. Is it possible to force two symbols to be both built-in (=y) or both
modules (=m)? This would make one of my solutions work.

2. Do you think it's possible to split this as a module *without*
requesting the driver dynamically? I've tried the same extensions approach
as in em28xx and others, but found some problems with the way
snd-usb-audio driver registers.
 
Thanks,
Yann E. MORIN May 4, 2013, 7:59 p.m. UTC | #3 
Ezequiel, All,

On Sat, May 04, 2013 at 02:21:44PM -0300, Ezequiel Garcia wrote:
> I'm trying to split the ac97 support into a separate module.
> So far I've managed to do this with two different approaches,
> but both of them are broken in some way :-(
> 
> Couple questions:
> 
> 1. Is it possible to force two symbols to be both built-in (=y) or both
> modules (=m)? This would make one of my solutions work.

If they are always the same value, there is no need to have two symbols
in the first place.

However, given the original problem from this thread, if what you meant
was to have the second symbol either 'n' or the same as the first symbol,
ie. the following table:

    A:  n   m   m   y   y
    B:  n   n   m   n   y

Then the closest I came up with is:

    config MODULES
        bool "Modules"
    
    config A
        tristate "A"
    
    config B_dummy
        bool "B"
        depends on A
    
    config B
        tristate
        default m if A=m && B_dummy
        default y if A=y && B_dummy

where B_dummy is not used outside of Kconfig, and only A and B are the
symbols of interest (eg. to build the drivers).

Otherwise, I was not able to get the desired behviour with only the A
and B symbols.

Regards,
Yann E. MORIN.
Ezequiel Garcia May 6, 2013, 1:11 p.m. UTC | #4 
On Sat, May 04, 2013 at 09:59:50PM +0200, Yann E. MORIN wrote:
> Ezequiel, All,
> 
> On Sat, May 04, 2013 at 02:21:44PM -0300, Ezequiel Garcia wrote:
> > I'm trying to split the ac97 support into a separate module.
> > So far I've managed to do this with two different approaches,
> > but both of them are broken in some way :-(
> > 
> > Couple questions:
> > 
> > 1. Is it possible to force two symbols to be both built-in (=y) or both
> > modules (=m)? This would make one of my solutions work.
> 
> If they are always the same value, there is no need to have two symbols
> in the first place.
> 
> However, given the original problem from this thread, if what you meant
> was to have the second symbol either 'n' or the same as the first symbol,
> ie. the following table:
> 
>     A:  n   m   m   y   y
>     B:  n   n   m   n   y
> 
> Then the closest I came up with is:
> 
>     config MODULES
>         bool "Modules"
>     
>     config A
>         tristate "A"
>     
>     config B_dummy
>         bool "B"
>         depends on A
>     
>     config B
>         tristate
>         default m if A=m && B_dummy
>         default y if A=y && B_dummy
> 
> where B_dummy is not used outside of Kconfig, and only A and B are the
> symbols of interest (eg. to build the drivers).
> 

That worked like a charm!

Thanks a lot,
diff mbox

Patch

diff --git a/drivers/media/usb/stk1160/Kconfig b/drivers/media/usb/stk1160/Kconfig
index 1c3a1ec..2bf6392 100644
--- a/drivers/media/usb/stk1160/Kconfig
+++ b/drivers/media/usb/stk1160/Kconfig
@@ -1,8 +1,6 @@ 
-config VIDEO_STK1160
+config VIDEO_STK1160_COMMON
  	tristate "STK1160 USB video capture support"
  	depends on VIDEO_DEV && I2C
-	select VIDEOBUF2_VMALLOC
-	select VIDEO_SAA711X
  
  	---help---
  	  This is a video4linux driver for STK1160 based video capture devices.
@@ -12,9 +10,14 @@  config VIDEO_STK1160
  
  config VIDEO_STK1160_AC97
  	bool "STK1160 AC97 codec support"
-	depends on VIDEO_STK1160 && SND
-	select SND_AC97_CODEC
-
+	depends on VIDEO_STK1160_COMMON && SND
  	---help---
  	  Enables AC97 codec support for stk1160 driver.
-.
+
+config VIDEO_STK1160
+	tristate
+	depends on (!VIDEO_STK1160_AC97 || (SND='n') || SND) && VIDEO_STK1160_COMMON
+	default y
+	select VIDEOBUF2_VMALLOC
+	select VIDEO_SAA711X
+	select SND_AC97_CODEC if SND