public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
* Re: linux-next: Tree for May 1 (media/usb/stk1160)
       [not found] <20130501183734.7ad1efca2d06e75432edabbd@canb.auug.org.au>
@ 2013-05-01 17:59 ` Randy Dunlap
  2013-05-01 19:28   ` Yann E. MORIN
  2013-05-02 14:52   ` Mauro Carvalho Chehab
  0 siblings, 2 replies; 13+ messages in thread
From: Randy Dunlap @ 2013-05-01 17:59 UTC (permalink / raw)
  To: Stephen Rothwell; +Cc: linux-next, linux-kernel, linux-media, linux-kbuild

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.


-- 
~Randy

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: linux-next: Tree for May 1 (media/usb/stk1160)
  2013-05-01 17:59 ` linux-next: Tree for May 1 (media/usb/stk1160) Randy Dunlap
@ 2013-05-01 19:28   ` Yann E. MORIN
  2013-05-01 19:32     ` Randy Dunlap
  2013-05-01 19:58     ` David Rientjes
  2013-05-02 14:52   ` Mauro Carvalho Chehab
  1 sibling, 2 replies; 13+ messages in thread
From: Yann E. MORIN @ 2013-05-01 19:28 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Stephen Rothwell, linux-next, linux-kernel, linux-media,
	linux-kbuild

On Wed, May 01, 2013 at 10:59:07AM -0700, Randy Dunlap wrote:
> 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.

I'm not sure to understand what you want, here.
I find it valid that a 'bool' can 'select' a 'tristate', to force it to 'y'.

Do you mean there is an issue with Kconfig, the parser?
  -> should Kconfig warn or error out in such a case?

Or do you mean the structure above is wrong, and should be ammended?
  -> change the 'select' to a 'depends on'?
  -> change the symbol to a tristate?

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: linux-next: Tree for May 1 (media/usb/stk1160)
  2013-05-01 19:28   ` Yann E. MORIN
@ 2013-05-01 19:32     ` Randy Dunlap
  2013-05-01 19:58     ` David Rientjes
  1 sibling, 0 replies; 13+ messages in thread
From: Randy Dunlap @ 2013-05-01 19:32 UTC (permalink / raw)
  To: Yann E. MORIN
  Cc: Stephen Rothwell, linux-next, linux-kernel, linux-media,
	linux-kbuild

On 05/01/13 12:28, Yann E. MORIN wrote:
> On Wed, May 01, 2013 at 10:59:07AM -0700, Randy Dunlap wrote:
>> 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.
> 
> I'm not sure to understand what you want, here.

I just want the build errors fixed.  I'm not asking for any particular fix.

> I find it valid that a 'bool' can 'select' a 'tristate', to force it to 'y'.

But a bool selecting a tristate that already =m does not force it to y AFAICT.
I guess that would be an acceptable change/fix.  Maybe.

> Do you mean there is an issue with Kconfig, the parser?

I think so.

>   -> should Kconfig warn or error out in such a case?
> 
> Or do you mean the structure above is wrong, and should be ammended?
>   -> change the 'select' to a 'depends on'?

That should be one way to fix the problem, yes.

>   -> change the symbol to a tristate?

I thought about that, but I don't think that it will work.  There is no
separate module that is built for AC97 codec support.


thanks,
-- 
~Randy

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: linux-next: Tree for May 1 (media/usb/stk1160)
  2013-05-01 19:28   ` Yann E. MORIN
  2013-05-01 19:32     ` Randy Dunlap
@ 2013-05-01 19:58     ` David Rientjes
  2013-05-01 20:23       ` Randy Dunlap
  1 sibling, 1 reply; 13+ messages in thread
From: David Rientjes @ 2013-05-01 19:58 UTC (permalink / raw)
  To: Yann E. MORIN
  Cc: Randy Dunlap, Stephen Rothwell, linux-next, linux-kernel,
	linux-media, linux-kbuild

On Wed, 1 May 2013, Yann E. MORIN wrote:

> > 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

This doesn't depend on SND, it depends on SND=y.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: linux-next: Tree for May 1 (media/usb/stk1160)
  2013-05-01 19:58     ` David Rientjes
@ 2013-05-01 20:23       ` Randy Dunlap
  2013-05-01 20:40         ` David Rientjes
  2013-05-01 20:53         ` Yann E. MORIN
  0 siblings, 2 replies; 13+ messages in thread
From: Randy Dunlap @ 2013-05-01 20:23 UTC (permalink / raw)
  To: David Rientjes
  Cc: Yann E. MORIN, Stephen Rothwell, linux-next, linux-kernel,
	linux-media, linux-kbuild

On 05/01/13 12:58, David Rientjes wrote:
> On Wed, 1 May 2013, Yann E. MORIN wrote:
> 
>>> 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
> 
> This doesn't depend on SND, it depends on SND=y.
> --


Maybe this option *should* depend on SND=y, but that's not what the
kconfig syntax says.  The kconfig language does not care if the variable is
a bool or a tristate when evaluating a depends expression AFAICT (but I am
only reading Documentation/kbuild/kconfig-language.txt, not the source code).


-- 
~Randy

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: linux-next: Tree for May 1 (media/usb/stk1160)
  2013-05-01 20:23       ` Randy Dunlap
@ 2013-05-01 20:40         ` David Rientjes
  2013-05-01 20:53         ` Yann E. MORIN
  1 sibling, 0 replies; 13+ messages in thread
From: David Rientjes @ 2013-05-01 20:40 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Yann E. MORIN, Stephen Rothwell, linux-next, linux-kernel,
	linux-media, linux-kbuild

On Wed, 1 May 2013, Randy Dunlap wrote:

> >>> 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
> > 
> > This doesn't depend on SND, it depends on SND=y.
> > --
> 
> 
> Maybe this option *should* depend on SND=y, but that's not what the
> kconfig syntax says.  The kconfig language does not care if the variable is
> a bool or a tristate when evaluating a depends expression AFAICT (but I am
> only reading Documentation/kbuild/kconfig-language.txt, not the source code).
> 

Doing "depends on SND=y" will only allow an option to be enabled if SND is 
builtin so the snd_card_* functions above will be defined (with your 
config we have a builtin function calling a module which may or may not be 
loaded).  I think you've already addressed the snd_ac97_* functions, so 
the fix here should be relatively simple.  Yann?

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: linux-next: Tree for May 1 (media/usb/stk1160)
  2013-05-01 20:23       ` Randy Dunlap
  2013-05-01 20:40         ` David Rientjes
@ 2013-05-01 20:53         ` Yann E. MORIN
  2013-05-01 20:58           ` Randy Dunlap
  1 sibling, 1 reply; 13+ messages in thread
From: Yann E. MORIN @ 2013-05-01 20:53 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: David Rientjes, Stephen Rothwell, linux-next, linux-kernel,
	linux-media, linux-kbuild

Randy, All,

On Wed, May 01, 2013 at 01:23:25PM -0700, Randy Dunlap wrote:
> On 05/01/13 12:58, David Rientjes wrote:
> > On Wed, 1 May 2013, Yann E. MORIN wrote:
> > 
> >>> 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

BTW, can you check that:
    make silentoldconfig
does not warn about unmet dependencies for those symbols?

> > This doesn't depend on SND, it depends on SND=y.
> 
> Maybe this option *should* depend on SND=y, but that's not what the
> kconfig syntax says.

I'd say  Documentation/kbuild/kconfig-language.txt  is not complete wrt
the current syntax, grammar and semantics of the language. :-(

> The kconfig language does not care if the variable is
> a bool or a tristate when evaluating a depends expression AFAICT (but I am
> only reading Documentation/kbuild/kconfig-language.txt, not the source code).

Yes, it does, I've just tried with the following snippet:

    config MODULES
        bool "modules"
    
    config A
        tristate "A"
    
    config B
        tristate "B"
        depends on A
    
    config C
        tristate "C"
    
    config D
        bool "D"
        depends on C
        select B
    
    config E
        bool "E"
        depends on C=y
        select B

As you can test, E will not be visible if C is not =y, while D will be
visible if C is =m or =y.

Also, if A=m (and C=n), then B can only be =n or =m.

But with the test-case above, if C=y and ( D=y or E=y ), then B will be
forced to =y, even though A might be unset, which means silentoldconfig
would warn abount unmet dependencies:
    warning: (D && E) selects B which has unmet direct dependencies (A)

Worse! With: A=m, C=y, D=y  -> B is forced to =y, which is wrong because
it can only be =n or =m (see above), but silentoldconfig will not warn
about this situation.

Sigh... :-(

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: linux-next: Tree for May 1 (media/usb/stk1160)
  2013-05-01 20:53         ` Yann E. MORIN
@ 2013-05-01 20:58           ` Randy Dunlap
  0 siblings, 0 replies; 13+ messages in thread
From: Randy Dunlap @ 2013-05-01 20:58 UTC (permalink / raw)
  To: Yann E. MORIN
  Cc: David Rientjes, Stephen Rothwell, linux-next, linux-kernel,
	linux-media, linux-kbuild

On 05/01/13 13:53, Yann E. MORIN wrote:
> Randy, All,
> 
> On Wed, May 01, 2013 at 01:23:25PM -0700, Randy Dunlap wrote:
>> On 05/01/13 12:58, David Rientjes wrote:
>>> On Wed, 1 May 2013, Yann E. MORIN wrote:
>>>
>>>>> 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
> 
> BTW, can you check that:
>     make silentoldconfig
> does not warn about unmet dependencies for those symbols?

'make silentoldconfig' on the config file that I am using only gives me this:

warning: (VIDEO_EM28XX) selects VIDEO_MT9V011 which has unmet direct dependencies (MEDIA_SUPPORT && I2C && VIDEO_V4L2 && MEDIA_CAMERA_SUPPORT)


>>> This doesn't depend on SND, it depends on SND=y.
>>
>> Maybe this option *should* depend on SND=y, but that's not what the
>> kconfig syntax says.
> 
> I'd say  Documentation/kbuild/kconfig-language.txt  is not complete wrt
> the current syntax, grammar and semantics of the language. :-(

OK, that's not surprising.

thanks,
-- 
~Randy

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: linux-next: Tree for May 1 (media/usb/stk1160)
  2013-05-01 17:59 ` linux-next: Tree for May 1 (media/usb/stk1160) Randy Dunlap
  2013-05-01 19:28   ` Yann E. MORIN
@ 2013-05-02 14:52   ` Mauro Carvalho Chehab
  2013-05-02 21:23     ` Randy Dunlap
  2013-05-04 17:21     ` Splitting stk1160-ac97 as a module (Re: linux-next: Tree for May 1 (media/usb/stk1160)) Ezequiel Garcia
  1 sibling, 2 replies; 13+ messages in thread
From: Mauro Carvalho Chehab @ 2013-05-02 14:52 UTC (permalink / raw)
  To: Randy Dunlap, Yann E. MORIN, Ezequiel García
  Cc: Stephen Rothwell, linux-next, linux-kernel, linux-media,
	linux-kbuild

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>

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



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: linux-next: Tree for May 1 (media/usb/stk1160)
  2013-05-02 14:52   ` Mauro Carvalho Chehab
@ 2013-05-02 21:23     ` Randy Dunlap
  2013-05-04 17:21     ` Splitting stk1160-ac97 as a module (Re: linux-next: Tree for May 1 (media/usb/stk1160)) Ezequiel Garcia
  1 sibling, 0 replies; 13+ messages in thread
From: Randy Dunlap @ 2013-05-02 21:23 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Yann E. MORIN, Ezequiel García, Stephen Rothwell, linux-next,
	linux-kernel, linux-media, linux-kbuild

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
> 
> 
> -- 


-- 
~Randy

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Splitting stk1160-ac97 as a module (Re: linux-next: Tree for May 1 (media/usb/stk1160))
  2013-05-02 14:52   ` Mauro Carvalho Chehab
  2013-05-02 21:23     ` Randy Dunlap
@ 2013-05-04 17:21     ` Ezequiel Garcia
  2013-05-04 19:59       ` Yann E. MORIN
  1 sibling, 1 reply; 13+ messages in thread
From: Ezequiel Garcia @ 2013-05-04 17:21 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Randy Dunlap, Yann E. MORIN, Ezequiel García,
	Stephen Rothwell, linux-next, linux-kernel, linux-media,
	linux-kbuild

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,
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Splitting stk1160-ac97 as a module (Re: linux-next: Tree for May 1 (media/usb/stk1160))
  2013-05-04 17:21     ` Splitting stk1160-ac97 as a module (Re: linux-next: Tree for May 1 (media/usb/stk1160)) Ezequiel Garcia
@ 2013-05-04 19:59       ` Yann E. MORIN
  2013-05-06 13:11         ` Ezequiel Garcia
  0 siblings, 1 reply; 13+ messages in thread
From: Yann E. MORIN @ 2013-05-04 19:59 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: Mauro Carvalho Chehab, Randy Dunlap, Ezequiel García,
	Stephen Rothwell, linux-next, linux-kernel, linux-media,
	linux-kbuild

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.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: Splitting stk1160-ac97 as a module (Re: linux-next: Tree for May 1 (media/usb/stk1160))
  2013-05-04 19:59       ` Yann E. MORIN
@ 2013-05-06 13:11         ` Ezequiel Garcia
  0 siblings, 0 replies; 13+ messages in thread
From: Ezequiel Garcia @ 2013-05-06 13:11 UTC (permalink / raw)
  To: Yann E. MORIN
  Cc: Mauro Carvalho Chehab, Randy Dunlap, Ezequiel García,
	Stephen Rothwell, linux-next, linux-kernel, linux-media,
	linux-kbuild

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,
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2013-05-06 13:11 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20130501183734.7ad1efca2d06e75432edabbd@canb.auug.org.au>
2013-05-01 17:59 ` linux-next: Tree for May 1 (media/usb/stk1160) Randy Dunlap
2013-05-01 19:28   ` Yann E. MORIN
2013-05-01 19:32     ` Randy Dunlap
2013-05-01 19:58     ` David Rientjes
2013-05-01 20:23       ` Randy Dunlap
2013-05-01 20:40         ` David Rientjes
2013-05-01 20:53         ` Yann E. MORIN
2013-05-01 20:58           ` Randy Dunlap
2013-05-02 14:52   ` Mauro Carvalho Chehab
2013-05-02 21:23     ` Randy Dunlap
2013-05-04 17:21     ` Splitting stk1160-ac97 as a module (Re: linux-next: Tree for May 1 (media/usb/stk1160)) Ezequiel Garcia
2013-05-04 19:59       ` Yann E. MORIN
2013-05-06 13:11         ` Ezequiel Garcia

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox