Re: [alsa-devel] [PATCH] SOUND: Fix non-ISA_DMA_API build failure
On Fri, Jun 24, 2011 at 10:26:13AM +0200, Takashi Iwai wrote:
Hrm... I still don't understand why ES18XX or others were selected at the first place. Isn't it covered by the conditional in sound/isa/Kconfig like below?
================================================================ menuconfig SND_ISA bool "ISA sound devices" depends on ISA && ISA_DMA_API ... if SND_ISA ... config SND_ES18XX tristate "Generic ESS ES18xx driver" ... endif # SND_ISA ================================================================
Isn't SND_ISA=n in your case although ISA_DMA_API=n?
The answer is hidden in this Kconfig warning:
warning: (RADIO_MIROPCM20) selects SND_ISA which has unmet direct dependencies (SOUND && !M68K && SND && ISA && ISA_DMA_API)
This is due to the following in drivers/media/radio/Kconfig:
config RADIO_MIROPCM20 tristate "miroSOUND PCM20 radio" depends on ISA && VIDEO_V4L2 && SND select SND_ISA select SND_MIRO
So SND_ISA gets forced on even though the dependency on ISA_DMA_API is not fulfilled. That's solved by adding the dependency on ISA_DMA_API to RADIO_MIROPCM20.
Also, adlib driver is really only for ISA, so I see no big reason to allow this built for non-ISA.
With the patch applied:
[...] menuconfig SND_ISA bool "ISA sound devices" depends on ISA [...]
if SND_ISA
config SND_ADLIB tristate "AdLib FM card" select SND_OPL3_LIB [...]
So the Adlib driver will still only be built with ISA enabled. The only thing that makes the Adlib driver different from all the others in the ifdef SND_ISA ... endif bracket is that it does not directly or indirectly use the ISA DMA API and that's in the end the reason why sound/isa/Kconfig needs to be changed.
I originally approach this a different way but now that I'm explaining the details I notice that it probably makes sense to split this patch into two:
o The drivers/media/radio/Kconfig part should be applied for 3.0 and maybe -stable. o The sound/isa/Kconfig part is basically only fixing the dependency for the Adlib driver allowing it to be built on non-ISA_DMA_API system and is material for the next release after 3.0.
If you agree I'm going to repost the patch with aproprite log messages.
Ralf
Em 24-06-2011 08:16, Ralf Baechle escreveu:
On Fri, Jun 24, 2011 at 10:26:13AM +0200, Takashi Iwai wrote:
Hrm... I still don't understand why ES18XX or others were selected at the first place. Isn't it covered by the conditional in sound/isa/Kconfig like below?
================================================================ menuconfig SND_ISA bool "ISA sound devices" depends on ISA && ISA_DMA_API ... if SND_ISA ... config SND_ES18XX tristate "Generic ESS ES18xx driver" ... endif # SND_ISA ================================================================
Isn't SND_ISA=n in your case although ISA_DMA_API=n?
The answer is hidden in this Kconfig warning:
warning: (RADIO_MIROPCM20) selects SND_ISA which has unmet direct dependencies (SOUND && !M68K && SND && ISA && ISA_DMA_API)
This is due to the following in drivers/media/radio/Kconfig:
config RADIO_MIROPCM20 tristate "miroSOUND PCM20 radio" depends on ISA && VIDEO_V4L2 && SND select SND_ISA select SND_MIRO
So SND_ISA gets forced on even though the dependency on ISA_DMA_API is not fulfilled. That's solved by adding the dependency on ISA_DMA_API to RADIO_MIROPCM20.
Another option would be to convert the two above selects into depends on.
Cheers, Mauro
On Fri, Jun 24, 2011 at 08:35:22AM -0300, Mauro Carvalho Chehab wrote:
Em 24-06-2011 08:16, Ralf Baechle escreveu:
tristate "miroSOUND PCM20 radio" depends on ISA && VIDEO_V4L2 && SND select SND_ISA select SND_MIROSo SND_ISA gets forced on even though the dependency on ISA_DMA_API is not fulfilled. That's solved by adding the dependency on ISA_DMA_API to RADIO_MIROPCM20.
Another option would be to convert the two above selects into depends on.
Depends has the disadvantage that users may have to enable unobvious options first before they are offered the one they are looking for and that's what a "depends SND_ISA" would cause in this case.
Ralf
At Fri, 24 Jun 2011 12:16:08 +0100, Ralf Baechle wrote:
On Fri, Jun 24, 2011 at 10:26:13AM +0200, Takashi Iwai wrote:
Hrm... I still don't understand why ES18XX or others were selected at the first place. Isn't it covered by the conditional in sound/isa/Kconfig like below?
================================================================ menuconfig SND_ISA bool "ISA sound devices" depends on ISA && ISA_DMA_API ... if SND_ISA ... config SND_ES18XX tristate "Generic ESS ES18xx driver" ... endif # SND_ISA ================================================================
Isn't SND_ISA=n in your case although ISA_DMA_API=n?
The answer is hidden in this Kconfig warning:
warning: (RADIO_MIROPCM20) selects SND_ISA which has unmet direct dependencies (SOUND && !M68K && SND && ISA && ISA_DMA_API)
This is due to the following in drivers/media/radio/Kconfig:
config RADIO_MIROPCM20 tristate "miroSOUND PCM20 radio" depends on ISA && VIDEO_V4L2 && SND select SND_ISA select SND_MIRO
So SND_ISA gets forced on even though the dependency on ISA_DMA_API is not fulfilled. That's solved by adding the dependency on ISA_DMA_API to RADIO_MIROPCM20.
Ah, yeah, I see now.
Also, adlib driver is really only for ISA, so I see no big reason to allow this built for non-ISA.
With the patch applied:
[...] menuconfig SND_ISA bool "ISA sound devices" depends on ISA [...]
if SND_ISA
config SND_ADLIB tristate "AdLib FM card" select SND_OPL3_LIB [...]
So the Adlib driver will still only be built with ISA enabled. The only thing that makes the Adlib driver different from all the others in the ifdef SND_ISA ... endif bracket is that it does not directly or indirectly use the ISA DMA API and that's in the end the reason why sound/isa/Kconfig needs to be changed.
I originally approach this a different way but now that I'm explaining the details I notice that it probably makes sense to split this patch into two:
o The drivers/media/radio/Kconfig part should be applied for 3.0 and maybe -stable.
Yes, this will be good.
o The sound/isa/Kconfig part is basically only fixing the dependency for the Adlib driver allowing it to be built on non-ISA_DMA_API system and is material for the next release after 3.0.
Any serious reason that snd-adlib must be built even with ISA=n? As the device is really present only for ISA, it doesn't make much sense to build this even though the driver itself doesn't need ISA_DMA_API.
thanks,
Takashi
On Fri, Jun 24, 2011 at 02:22:41PM +0200, Takashi Iwai wrote:
o The drivers/media/radio/Kconfig part should be applied for 3.0 and maybe -stable.
Yes, this will be good.
I just tested that segment only and it works as expected. Will repost in a minute.
o The sound/isa/Kconfig part is basically only fixing the dependency for the Adlib driver allowing it to be built on non-ISA_DMA_API system and is material for the next release after 3.0.
Any serious reason that snd-adlib must be built even with ISA=n?
Definately not.
As the device is really present only for ISA, it doesn't make much sense to build this even though the driver itself doesn't need ISA_DMA_API.
I'm not aware of any systems that could use the Adlib in a ISA=n environment. That's why my patch left the dependency on ISA untouched.
Ralf
At Fri, 24 Jun 2011 14:08:18 +0100, Ralf Baechle wrote:
On Fri, Jun 24, 2011 at 02:22:41PM +0200, Takashi Iwai wrote:
o The drivers/media/radio/Kconfig part should be applied for 3.0 and maybe -stable.
Yes, this will be good.
I just tested that segment only and it works as expected. Will repost in a minute.
Great, thanks.
o The sound/isa/Kconfig part is basically only fixing the dependency for the Adlib driver allowing it to be built on non-ISA_DMA_API system and is material for the next release after 3.0.
Any serious reason that snd-adlib must be built even with ISA=n?
Definately not.
As the device is really present only for ISA, it doesn't make much sense to build this even though the driver itself doesn't need ISA_DMA_API.
I'm not aware of any systems that could use the Adlib in a ISA=n environment. That's why my patch left the dependency on ISA untouched.
OK, but this would just make things complex, I'm afraid.
Practically the only user of snd-adlib is the old x86 with ISA support, which implies ISA_DMA_API=y, too. Thus I don't want to touch sound/isa/Kconfig just for snd-adlib being built for some funky Kconfig setup :)
thanks,
Takashi
participants (3)
-
Mauro Carvalho Chehab -
Ralf Baechle -
Takashi Iwai