Hi all,
I gave a try to 2.6.15-rc7 and "make menuconfig" tells me:
Warning! Found recursive dependency: VIDEO_SAA7134_ALSA VIDEO_SAA7134_OSS VIDEO_SAA7134_ALSA
This seems to be the consequence of this patch:
http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=7bb9529602f8bb41a92275825b808a42ed33e5be
How do we fix that? menuconfig is indeed really confused by the current
setup. I have the following patch which makes it happier:
Signed-off-by: Jean Delvare <[email protected]>
---
drivers/media/video/saa7134/Kconfig | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- linux-2.6.15-rc7.orig/drivers/media/video/saa7134/Kconfig 2005-12-26 11:04:35.000000000 +0100
+++ linux-2.6.15-rc7/drivers/media/video/saa7134/Kconfig 2005-12-27 11:35:34.000000000 +0100
@@ -14,7 +14,7 @@
config VIDEO_SAA7134_ALSA
tristate "Philips SAA7134 DMA audio support"
- depends on VIDEO_SAA7134 && SOUND && SND && (!VIDEO_SAA7134_OSS || VIDEO_SAA7134_OSS = m)
+ depends on VIDEO_SAA7134 && SND
select SND_PCM_OSS
---help---
This is a video4linux driver for direct (DMA) audio in
@@ -25,7 +25,7 @@
config VIDEO_SAA7134_OSS
tristate "Philips SAA7134 DMA audio support (OSS, DEPRECATED)"
- depends on VIDEO_SAA7134 && SOUND_PRIME && (!VIDEO_SAA7134_ALSA || VIDEO_SAA7134_ALSA = m)
+ depends on VIDEO_SAA7134 && SOUND_PRIME && VIDEO_SAA7134_ALSA!=y && m
---help---
This is a video4linux driver for direct (DMA) audio in
Philips SAA713x based TV cards using OSS
However, I am not entirely happy with it because it introduces an
asymmetry: the OSS variant can no more be built-in, it is now only
available as a module. Not sure this is acceptable.
Another possibility would be to fix Kconfig to properly support this
case. Roman, what do you think?
Yet another possibility would be to not express the dependencies,
explain the mutual exclusion in the help texts, and hope that the user
will read the help texts. I doubt this will work. This seems to be what
was done for the eepro100/e100 case though (well, not even help texts
in this case).
Are there other similar cases in the kernel right now?
It would be nice to fix this before 2.6.15.
Thanks,
--
Jean Delvare
Jean,
Em Ter, 2005-12-27 ?s 21:53 +0100, Jean Delvare escreveu:
> Hi all,
>
> I gave a try to 2.6.15-rc7 and "make menuconfig" tells me:
> Warning! Found recursive dependency: VIDEO_SAA7134_ALSA VIDEO_SAA7134_OSS VIDEO_SAA7134_ALSA
>
> This seems to be the consequence of this patch:
> http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=7bb9529602f8bb41a92275825b808a42ed33e5be
>
> How do we fix that? menuconfig is indeed really confused by the current
> setup. I have the following patch which makes it happier:
Please test this patch. It seems that provides the same behavior with a
non-recursive logic.
Cheers,
Mauro.
Hi Mauro, Linus,
> > I gave a try to 2.6.15-rc7 and "make menuconfig" tells me:
> > Warning! Found recursive dependency: VIDEO_SAA7134_ALSA VIDEO_SAA7134_OSS VIDEO_SAA7134_ALSA
> >
> > This seems to be the consequence of this patch:
> > http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=7bb9529602f8bb41a92275825b808a42ed33e5be
> >
> > How do we fix that? menuconfig is indeed really confused by the current
> > setup. I have the following patch which makes it happier:
>
> Please test this patch. It seems that provides the same behavior with a
> non-recursive logic.
Looks good to me, except for the coding style. Also, depending on both
SND and SOUND doesn't make much sense, as SND itself already depends on
SOUND, so we can simplify the SAA7134_ALSA dependency list.
Linus, can you please apply this patch before releasing 2.6.15?
Thanks.
Fix the cyclic dependency issue between CONFIG_SAA7134_ALSA and
CONFIG_SAA7134_OSS (credits to Mauro Carvalho Chehab.)
Signed-off-by: Jean Delvare <[email protected]>
---
drivers/media/video/saa7134/Kconfig | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- linux-2.6.15-rc7.orig/drivers/media/video/saa7134/Kconfig 2005-12-28 19:13:38.000000000 +0100
+++ linux-2.6.15-rc7/drivers/media/video/saa7134/Kconfig 2005-12-28 20:50:37.000000000 +0100
@@ -14,7 +14,7 @@
config VIDEO_SAA7134_ALSA
tristate "Philips SAA7134 DMA audio support"
- depends on VIDEO_SAA7134 && SOUND && SND && (!VIDEO_SAA7134_OSS || VIDEO_SAA7134_OSS = m)
+ depends on VIDEO_SAA7134 && SND
select SND_PCM_OSS
---help---
This is a video4linux driver for direct (DMA) audio in
@@ -25,7 +25,7 @@
config VIDEO_SAA7134_OSS
tristate "Philips SAA7134 DMA audio support (OSS, DEPRECATED)"
- depends on VIDEO_SAA7134 && SOUND_PRIME && (!VIDEO_SAA7134_ALSA || VIDEO_SAA7134_ALSA = m)
+ depends on VIDEO_SAA7134 && SOUND_PRIME && (!VIDEO_SAA7134_ALSA || (VIDEO_SAA7134_ALSA=m && m))
---help---
This is a video4linux driver for direct (DMA) audio in
Philips SAA713x based TV cards using OSS
--
Jean Delvare
Em Qua, 2005-12-28 ?s 21:02 +0100, Jean Delvare escreveu:
> Hi Mauro, Linus,
> Looks good to me, except for the coding style. Also, depending on both
> SND and SOUND doesn't make much sense, as SND itself already depends on
> SOUND, so we can simplify the SAA7134_ALSA dependency list.
>
> Linus, can you please apply this patch before releasing 2.6.15?
>
> Thanks.
>
> Fix the cyclic dependency issue between CONFIG_SAA7134_ALSA and
> CONFIG_SAA7134_OSS (credits to Mauro Carvalho Chehab.)
>
> Signed-off-by: Jean Delvare <[email protected]>
Acked-by: Mauro Carvalho Chehab <[email protected]>
Hi,
On Tuesday 27 December 2005 21:53, Jean Delvare wrote:
> I gave a try to 2.6.15-rc7 and "make menuconfig" tells me:
> Warning! Found recursive dependency: VIDEO_SAA7134_ALSA VIDEO_SAA7134_OSS
> VIDEO_SAA7134_ALSA
Hmm, I really should make this more annoying...
> config VIDEO_SAA7134_OSS
> tristate "Philips SAA7134 DMA audio support (OSS, DEPRECATED)"
> - depends on VIDEO_SAA7134 && SOUND_PRIME && (!VIDEO_SAA7134_ALSA ||
VIDEO_SAA7134_ALSA = m)
> + depends on VIDEO_SAA7134 && SOUND_PRIME &&
> VIDEO_SAA7134_ALSA!=y && m
The easiest fix would to just change the last part into "!VIDEO_SAA7134_ALSA".
An alternative would be to use a choice.
bye, Roman
Hi,
On Wednesday 28 December 2005 21:02, Jean Delvare wrote:
> + depends on VIDEO_SAA7134 && SOUND_PRIME && (!VIDEO_SAA7134_ALSA ||
(VIDEO_SAA7134_ALSA=m && m))
Please do it a little less uglier, simply "!VIDEO_SAA7134_ALSA" is enough.
bye, Roman
Hi Roman,
> On Wednesday 28 December 2005 21:02, Jean Delvare wrote:
>
> > + depends on VIDEO_SAA7134 && SOUND_PRIME && (!VIDEO_SAA7134_ALSA ||
> (VIDEO_SAA7134_ALSA=m && m))
>
> Please do it a little less uglier, simply "!VIDEO_SAA7134_ALSA" is enough.
No, it wouldn't produce the desired effect anymore.
(!VIDEO_SAA7134_ALSA || (VIDEO_SAA7134_ALSA=m && m)) makes it possible
to compile both OSS and ALSA support as modules. Simplifying to
!VIDEO_SAA7134_ALSA would make it impossible.
Thanks,
--
Jean Delvare
Hi,
On Thursday 29 December 2005 21:13, Jean Delvare wrote:
> No, it wouldn't produce the desired effect anymore.
>
> (!VIDEO_SAA7134_ALSA || (VIDEO_SAA7134_ALSA=m && m)) makes it possible
> to compile both OSS and ALSA support as modules. Simplifying to
> !VIDEO_SAA7134_ALSA would make it impossible.
Did you try it?
bye, Roman
Hi Roman,
> On Thursday 29 December 2005 21:13, Jean Delvare wrote:
>
> > No, it wouldn't produce the desired effect anymore.
> >
> > (!VIDEO_SAA7134_ALSA || (VIDEO_SAA7134_ALSA=m && m)) makes it possible
> > to compile both OSS and ALSA support as modules. Simplifying to
> > !VIDEO_SAA7134_ALSA would make it impossible.
>
> Did you try it?
I didn't back then, just did now, and oh surprise, you are right and I
am wrong... I should have known; after all, *you* wrote kconfig, right?
I checked Documentation/kbuild/kconfig-language.txt and it all makes
sense to me now, although I have to admit the word "counterintuitive"
came to my mind at first. I guess it's just a bit hard to switch to
tristate logic when you use boolean logic all day long.
So, I'm sorry for not verifying it all before I answered to you, and
even more sorry for submitting a less-than-optimal patch for inclusion.
He comes a fix. Linus, feel free to apply it now, or after 2.6.15 is
released, at your option - it's no more a bugfix, only an optimization.
Thanks.
* * * * *
Simplify the VIDEO_SAA7134_OSS Kconfig dependency line a bit. Thanks
to Roman Zippel for the suggestion.
Signed-off-by: Jean Delvare <[email protected]>
---
drivers/media/video/saa7134/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- linux-2.6.15-rc7.orig/drivers/media/video/saa7134/Kconfig 2005-12-29 22:01:20.000000000 +0100
+++ linux-2.6.15-rc7/drivers/media/video/saa7134/Kconfig 2005-12-29 22:04:24.000000000 +0100
@@ -25,7 +25,7 @@
config VIDEO_SAA7134_OSS
tristate "Philips SAA7134 DMA audio support (OSS, DEPRECATED)"
- depends on VIDEO_SAA7134 && SOUND_PRIME && (!VIDEO_SAA7134_ALSA || (VIDEO_SAA7134_ALSA=m && m))
+ depends on VIDEO_SAA7134 && SOUND_PRIME && !VIDEO_SAA7134_ALSA
---help---
This is a video4linux driver for direct (DMA) audio in
Philips SAA713x based TV cards using OSS
--
Jean Delvare
Hi, Jean and Roman,
Em Qui, 2005-12-29 ?s 22:07 +0100, Jean Delvare escreveu:
> Hi Roman,
>
> > On Thursday 29 December 2005 21:13, Jean Delvare wrote:
> >
> > > No, it wouldn't produce the desired effect anymore.
> >
> > Did you try it?
Using choice with two tristate config options seems to provide the
better look and feel, while keeping the desired effect. The only
drawback is that Kconfig doesn't accept default values for the "config"
inside a choice. It would be nice if both choice and the ALSA/OSS
options be marked as "m" by default.
Anyway, IMHO, this is better and clearer than the previous patches.
Cheers,
Mauro.
Hi Mauro,
> > > On Thursday 29 December 2005 21:13, Jean Delvare wrote:
> > >
> > > > No, it wouldn't produce the desired effect anymore.
> > >
> > > Did you try it?
>
> Using choice with two tristate config options seems to provide the
> better look and feel, while keeping the desired effect. The only
> drawback is that Kconfig doesn't accept default values for the "config"
> inside a choice. It would be nice if both choice and the ALSA/OSS
> options be marked as "m" by default.
> Anyway, IMHO, this is better and clearer than the previous patches.
Can you please rebase it on Linus' latest (2.6.15-rc7-git4)? So that I
can give it a try...
Thanks,
--
Jean Delvare
Em Sex, 2005-12-30 ?s 12:06 +0100, Jean Delvare escreveu:
> Hi Mauro,
> Can you please rebase it on Linus' latest (2.6.15-rc7-git4)? So that I
> can give it a try...
Updated.
>
> Thanks,
Cheers,
Mauro.
Hi Mauro,
Sorry for the delay.
> > Can you please rebase it on Linus' latest (2.6.15-rc7-git4)? So that I
> > can give it a try...
> Updated.
I tried it, it works, but I like it less than the current approach.
Being able to select "Philips SAA7134 DMA audio support" as a module
while it doesn't correspond to real code is a bit confusing. Not being
able to unselect it completely is confusing as well. Thinks go even
worse when only ALSA or OSS has been selected in the general sound
menu, as you end up with a complex layout for no reason.
So I believe that "choice" is an interesting Kconfig feature when used
with boolean options, but with modules I am not convinced, especially
when these modules have different dependencies.
So if I were to decide, I would stick to the current code. But I am
obviously not the one to decide here.
Thanks,
--
Jean Delvare
Hi,
On Saturday 31 December 2005 19:39, Jean Delvare wrote:
> So I believe that "choice" is an interesting Kconfig feature when used
> with boolean options, but with modules I am not convinced, especially
> when these modules have different dependencies.
Well, I'm always open to suggestions (or even better patches) to improve
tristate choices. Such interdependent options have to be done via a choice
group, so they are handled correctly handled by kconfig, otherwise you have
to live with the current compromise. OTOH how they are mapped to the user
interface is easily changeable.
bye, Roman
Hi Roman,
> On Saturday 31 December 2005 19:39, Jean Delvare wrote:
>
> > So I believe that "choice" is an interesting Kconfig feature when used
> > with boolean options, but with modules I am not convinced, especially
> > when these modules have different dependencies.
>
> Well, I'm always open to suggestions (or even better patches) to improve
> tristate choices. Such interdependent options have to be done via a choice
> group, so they are correctly handled by kconfig, otherwise you have
> to live with the current compromise. OTOH how they are mapped to the user
> interface is easily changeable.
What I like with the current "compromise" in the SAA1734 case is that,
if the user has only OSS or only ALSA enabled in the sound menu, then
he/she only sees one available module for SAA7134 sound support. It
keeps the configuration interface as simple as it can be. While when
using a choice, the user will still be presented a pre-item/menu, with a
single choice in there. This is what I think is confusing.
If "choice" could fall back to a simple option when the dependencies
are such that only once choice is actually possible, I think it would
improve the situation, in the case of the SAA7134. But I lack time to
propose an actual patch implementing this, and I also did not
investigate to see what it would do for the other use cases of "choice"
in the current kernel tree. Lastly I guess that some people may find it
even more confusing if things were done the way I suggest - it's really
a matter of personal opinion at this point.
Given that there are only a few use cases of "choice" with tri-state
options, it might be just as easy to leave things as is and do with
what we have.
Thanks,
--
Jean Delvare