snd-pcsp: relax dependancy on CONFIG_INPUT

PATCH In review Stas Sergeev 2 participants 7 messages May 8, 2008 2 messages not in the archive

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): s5h4p98yfof.wl%[email protected]

Stas Sergeev

author

2008-05-08 17:31 UTC #2 lore raw

PATCH 1/1 snd-pcsp: relax dependancy on CONFIG_INPUT

Hello.

The attached patch makes an input
driver part of snd-pcsp optional.

Would it be possible to apply that?

# HG changeset patch
# User Stas Sergeev <[email protected]>
# Date 1210267204 -14400
# Node ID 5c3c0b965df21ac55112f907be0786dfa90b9dc6
# Parent 25a20d46d89a57c89e911452171af7e4cdcc8464
snd-pcsp: relax dependancy on CONFIG_INPUT by adding CONFIG_SND_PCSP_INPUT

Signed-off-by Stas Sergeev <[email protected]>

drivers/Kconfig +12 -3

diff -r 25a20d46d89a -r 5c3c0b965df2 drivers/Kconfig
@@ -5,16 +5,14 @@ menu "Generic devices"
 
 
 config SND_PCSP
-	tristate "Internal PC speaker support"
+	tristate "PC-Speaker support"
 	depends on X86_PC && HIGH_RES_TIMERS
-	depends on INPUT
 	depends on SND
 	select SND_PCM
 	help
 	  If you don't have a sound card in your computer, you can include a
 	  driver for the PC speaker which allows it to act like a primitive
 	  sound card.
-	  This driver also replaces the pcspkr driver for beeps.
 
 	  You can compile this as a module which will be called snd-pcsp.

@@ -23,6 +21,17 @@ config SND_PCSP
 	  in your PC instead of the real speaker.
 
 	  It should not hurt to say Y or M here in all other cases.
+
+config SND_PCSP_INPUT
+	bool "PC-Speaker input events (beeps) support"
+	depends on INPUT
+	depends on SND_PCSP
+	default y
+	help
+	  This option enables the PC-Speaker system beeps.
+	  This is an alternative to the pcspkr input driver (INPUT_PCSPKR).
+	  It provides the ability to enable/disable the system
+	  beeps with the sound mixer.
 
 config SND_MPU401_UART
         tristate

drivers/pcsp/Makefile +2 -1

diff -r 25a20d46d89a -r 5c3c0b965df2 drivers/pcsp/Makefile
@@ -1,2 +1,3 @@ snd-pcsp-objs := pcsp.o pcsp_lib.o pcsp_
-snd-pcsp-objs := pcsp.o pcsp_lib.o pcsp_mixer.o pcsp_input.o
+snd-pcsp-y := pcsp.o pcsp_lib.o pcsp_mixer.o
+snd-pcsp-$(CONFIG_SND_PCSP_INPUT) += pcsp_input.o
 obj-$(CONFIG_SND_PCSP) += snd-pcsp.o

drivers/pcsp/pcsp.c +2 -0

diff -r 25a20d46d89a -r 5c3c0b965df2 drivers/pcsp/pcsp.c
@@ -188,10 +188,12 @@ static int __devexit pcsp_remove(struct 
 
 static void pcsp_stop_beep(struct snd_pcsp *chip)
 {
+#ifdef CONFIG_SND_PCSP_INPUT
 	spin_lock_irq(&chip->substream_lock);
 	if (!chip->playback_substream)
 		pcspkr_stop_sound();
 	spin_unlock_irq(&chip->substream_lock);
+#endif	/* CONFIG_SND_PCSP_INPUT */
 }
 
 #ifdef CONFIG_PM

drivers/pcsp/pcsp_input.h +7 -0

diff -r 25a20d46d89a -r 5c3c0b965df2 drivers/pcsp/pcsp_input.h
@@ -7,8 +7,15 @@
 #ifndef __PCSP_INPUT_H__
 #define __PCSP_INPUT_H__
 
+#ifdef CONFIG_SND_PCSP_INPUT
 int __devinit pcspkr_input_init(struct input_dev **rdev, struct device *dev);
 int pcspkr_input_remove(struct input_dev *dev);
 void pcspkr_stop_sound(void);
+#else
+static inline int pcspkr_input_init(struct input_dev **rdev,
+	struct device *dev) { return 0; }
+static inline int pcspkr_input_remove(struct input_dev *dev) { return 0; }
+static inline void pcspkr_stop_sound(void) { }
+#endif	/* CONFIG_SND_PCSP_INPUT */
 
 #endif

drivers/pcsp/pcsp_mixer.c +4 -0

diff -r 25a20d46d89a -r 5c3c0b965df2 drivers/pcsp/pcsp_mixer.c
@@ -78,6 +78,7 @@ static int pcsp_treble_put(struct snd_kc
 	return changed;
 }
 
+#ifdef CONFIG_SND_PCSP_INPUT
 static int pcsp_pcspkr_info(struct snd_kcontrol *kcontrol,
 			    struct snd_ctl_elem_info *uinfo)
 {

@@ -108,6 +109,7 @@ static int pcsp_pcspkr_put(struct snd_kc
 	}
 	return changed;
 }
+#endif	/* CONFIG_SND_PCSP_INPUT */
 
 #define PCSP_MIXER_CONTROL(ctl_type, ctl_name) \
 { \

@@ -121,7 +123,9 @@ static struct snd_kcontrol_new __devinit
 static struct snd_kcontrol_new __devinitdata snd_pcsp_controls[] = {
 	PCSP_MIXER_CONTROL(enable, "Master Playback Switch"),
 	PCSP_MIXER_CONTROL(treble, "BaseFRQ Playback Volume"),
+#ifdef CONFIG_SND_PCSP_INPUT
 	PCSP_MIXER_CONTROL(pcspkr, "PC Speaker Playback Switch"),
+#endif	/* CONFIG_SND_PCSP_INPUT */
 };
 
 int __devinit snd_pcsp_new_mixer(struct snd_pcsp *chip)

_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Adrian Bunk

2008-05-08 17:37 UTC #3 lore raw

Quoted 7 lines · Stas Sergeev

Hello.
The attached patch makes an input
driver part of snd-pcsp optional.
Would it be possible to apply that?
...

Sorry for the silly question, but are there serious usecases where people in very space limited environments and with CONFIG_INPUT=n want PC-Speaker support?

cu
Adrian

Signature · 4 lines

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Stas Sergeev

author

2008-05-08 17:53 UTC #4 lore raw

Hello.

Quoted 3 lines · Adrian Bunk

Sorry for the silly question, but are there serious usecases where people in very space limited environments and with CONFIG_INPUT=n want PC-Speaker support?

I don't know, but there might be someone who wants the PCM support from pc-speaker, and hates the beeps at the same time. So they can disable it forever. :)
I also want to resolve the conflict with INPUT_PCSPKR somehow. Making the input part of snd-pcsp optional looks like the first step to me. It became known to me that loading multiple drivers for the same device will be possible in the future. I want to get everything ready for loading pcspkr and snd-pcsp together.
And overall, having snd-pcsp to depend on CONFIG_INPUT only because it can do a console beeps as a bonus, looks ill- behaved to me. So I just propose that patch as a small logical correction and cleanup, rather than a new feature.

Stas Sergeev

author

2008-05-08 18:25 UTC #5 lore raw

In reply to Adrian Bunk

Hello.

Quoted 3 lines · Adrian Bunk

Sorry for the silly question, but are there serious usecases where people in very space limited environments and with CONFIG_INPUT=n want PC-Speaker support?

Oh, I've just heard youre screaming
"this all is not worth a new config option!". :)

Here's another patch instead.
Without exporting a new option to the
user this time.

# HG changeset patch
# User Stas Sergeev <[email protected]>
# Date 1210267204 -14400
# Node ID 903c7be2316a6b892c8c20a0a8a6e21cd9a9ee8c
# Parent 25a20d46d89a57c89e911452171af7e4cdcc8464
snd-pcsp: relax dependancy on CONFIG_INPUT by adding CONFIG_SND_PCSP_INPUT

Signed-off-by Stas Sergeev <[email protected]>

drivers/Kconfig +6 -2

diff -r 25a20d46d89a -r 903c7be2316a drivers/Kconfig
@@ -5,9 +5,8 @@ menu "Generic devices"
 
 
 config SND_PCSP
-	tristate "Internal PC speaker support"
+	tristate "PC-Speaker support"
 	depends on X86_PC && HIGH_RES_TIMERS
-	depends on INPUT
 	depends on SND
 	select SND_PCM
 	help

@@ -23,6 +22,11 @@ config SND_PCSP
 	  in your PC instead of the real speaker.
 
 	  It should not hurt to say Y or M here in all other cases.
+
+config SND_PCSP_INPUT
+	def_bool y
+	depends on INPUT
+	depends on SND_PCSP
 
 config SND_MPU401_UART
         tristate

drivers/pcsp/Makefile +2 -1

diff -r 25a20d46d89a -r 903c7be2316a drivers/pcsp/Makefile
@@ -1,2 +1,3 @@ snd-pcsp-objs := pcsp.o pcsp_lib.o pcsp_
-snd-pcsp-objs := pcsp.o pcsp_lib.o pcsp_mixer.o pcsp_input.o
+snd-pcsp-y := pcsp.o pcsp_lib.o pcsp_mixer.o
+snd-pcsp-$(CONFIG_SND_PCSP_INPUT) += pcsp_input.o
 obj-$(CONFIG_SND_PCSP) += snd-pcsp.o

drivers/pcsp/pcsp.c +2 -0

diff -r 25a20d46d89a -r 903c7be2316a drivers/pcsp/pcsp.c
@@ -188,10 +188,12 @@ static int __devexit pcsp_remove(struct 
 
 static void pcsp_stop_beep(struct snd_pcsp *chip)
 {
+#ifdef CONFIG_SND_PCSP_INPUT
 	spin_lock_irq(&chip->substream_lock);
 	if (!chip->playback_substream)
 		pcspkr_stop_sound();
 	spin_unlock_irq(&chip->substream_lock);
+#endif	/* CONFIG_SND_PCSP_INPUT */
 }
 
 #ifdef CONFIG_PM

drivers/pcsp/pcsp_input.h +7 -0

diff -r 25a20d46d89a -r 903c7be2316a drivers/pcsp/pcsp_input.h
@@ -7,8 +7,15 @@
 #ifndef __PCSP_INPUT_H__
 #define __PCSP_INPUT_H__
 
+#ifdef CONFIG_SND_PCSP_INPUT
 int __devinit pcspkr_input_init(struct input_dev **rdev, struct device *dev);
 int pcspkr_input_remove(struct input_dev *dev);
 void pcspkr_stop_sound(void);
+#else
+static inline int pcspkr_input_init(struct input_dev **rdev,
+	struct device *dev) { return 0; }
+static inline int pcspkr_input_remove(struct input_dev *dev) { return 0; }
+static inline void pcspkr_stop_sound(void) { }
+#endif	/* CONFIG_SND_PCSP_INPUT */
 
 #endif

drivers/pcsp/pcsp_mixer.c +4 -0

diff -r 25a20d46d89a -r 903c7be2316a drivers/pcsp/pcsp_mixer.c
@@ -78,6 +78,7 @@ static int pcsp_treble_put(struct snd_kc
 	return changed;
 }
 
+#ifdef CONFIG_SND_PCSP_INPUT
 static int pcsp_pcspkr_info(struct snd_kcontrol *kcontrol,
 			    struct snd_ctl_elem_info *uinfo)
 {

@@ -108,6 +109,7 @@ static int pcsp_pcspkr_put(struct snd_kc
 	}
 	return changed;
 }
+#endif	/* CONFIG_SND_PCSP_INPUT */
 
 #define PCSP_MIXER_CONTROL(ctl_type, ctl_name) \
 { \

@@ -121,7 +123,9 @@ static struct snd_kcontrol_new __devinit
 static struct snd_kcontrol_new __devinitdata snd_pcsp_controls[] = {
 	PCSP_MIXER_CONTROL(enable, "Master Playback Switch"),
 	PCSP_MIXER_CONTROL(treble, "BaseFRQ Playback Volume"),
+#ifdef CONFIG_SND_PCSP_INPUT
 	PCSP_MIXER_CONTROL(pcspkr, "PC Speaker Playback Switch"),
+#endif	/* CONFIG_SND_PCSP_INPUT */
 };
 
 int __devinit snd_pcsp_new_mixer(struct snd_pcsp *chip)

_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Adrian Bunk

2008-05-08 22:18 UTC #6 lore raw

Quoted 13 lines · Stas Sergeev

Hello.
Quoted 3 lines · Adrian Bunk
Sorry for the silly question, but are there serious usecases where people in very space limited environments and with CONFIG_INPUT=n want PC-Speaker support?
Oh, I've just heard youre screaming
"this all is not worth a new config option!". :)
Here's another patch instead.
Without exporting a new option to the
user this time.
...

drivers/Kconfig config SND_PCSP · line 29

Quoted 5 lines

drivers/Kconfig +4 -0

+config SND_PCSP_INPUT
+	def_bool y
+	depends on INPUT
+	depends on SND_PCSP
...

The number of users with CONFIG_INPUT=n on a PC might be more or less zero (even more considering the fact that you have to set CONFIG_EMBEDDED=y for being able to disable INPUT) - your patch adds a few #ifdef's that don't have any effect in practice.

And your patches breaks the compilation with CONFIG_SND_PCSP=y, CONFIG_INPUT=m if anyone will ever try this combination.

The latter is not unfixable, and I might be very nitpicking here, but I do simply not see the point why we need this more complicated.

cu
Adrian

Signature · 4 lines

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Stas Sergeev

author

2008-05-08 22:38 UTC #7 lore raw

Hello.

Quoted 2 lines · Adrian Bunk

And your patches breaks the compilation with CONFIG_SND_PCSP=y, CONFIG_INPUT=m if anyone will ever try this combination.

OK, that's nasty indeed... Then I guess it is not a big deal to leave that dependancy as-is until the real need in changing it appear.

Btw, is there really no way at all to tell kconfig something like depends on INPUT if SND_PCSP_INPUT or similar?

Adrian Bunk

2008-05-08 22:49 UTC #8 lore raw

Quoted 10 lines · Stas Sergeev

Hello.
Quoted 2 lines · Adrian Bunk
And your patches breaks the compilation with CONFIG_SND_PCSP=y, CONFIG_INPUT=m if anyone will ever try this combination.
OK, that's nasty indeed... Then I guess it is not a big deal to leave that dependancy as-is until the real need in changing it appear.

Quoted 4 lines

Btw, is there really no way at all to tell kconfig something like depends on INPUT if SND_PCSP_INPUT or similar?

There are different ways how you could express this in kconfig depending on what exactly would be intended.

But the stuff we already have in kconfig has enough problematic cornercases, and making anything more complicated IMHO requires a justification that there's a real need for it.

cu
Adrian

Signature · 4 lines

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed