| Message ID | 20230422161021.1143989-1-oswald.buddenhagen@gmx.de |
|---|---|
| State | New |
| Headers | show |
| Series | ALSA: emu10k1: fix multi-channel playback device class | expand |
On Mon, Apr 24, 2023 at 12:32:58PM +0200, Oswald Buddenhagen wrote: >so i'd postulate that these enums are effectively dead, and both the >risk and the gain of this change are about zero. speaking of dead code, lots of drivers explicitly assign info_flags = 0, dev_class = SNDRV_PCM_CLASS_GENERIC, and dev_subclass = SNDRV_PCM_SUBCLASS_GENERIC_MIX, which are all technically pointless, as the pcm struct is kzalloc'd anyway. and of course it's entirely inconsistent (with just about every combination present), because it obviously works without this. how would you feel about a patch that just removes all these? regards
On Tue, Apr 25, 2023 at 10:15:36AM +0200, Takashi Iwai wrote: >On Tue, 25 Apr 2023 10:00:45 +0200, >Oswald Buddenhagen wrote: >> speaking of dead code, lots of drivers explicitly assign info_flags = >> 0, dev_class = SNDRV_PCM_CLASS_GENERIC, and dev_subclass = >> SNDRV_PCM_SUBCLASS_GENERIC_MIX, which are all technically pointless, >> as the pcm struct is kzalloc'd anyway. > >The dev_class and info_flags initializations to zero aren't entirely >pointless, IMO. It explicitly shows that the default value is used. > that can be said about many other fields as well. but defaults are there for a reason, to reduce the noise. not even the example in the docu includes it. and if we were consistent about the "be explicit" approach, we should add it to a lot of drivers which lack it - which feels just wrong. regards
diff --git a/sound/pci/emu10k1/emupcm.c b/sound/pci/emu10k1/emupcm.c index 87c3b19c6b2d..c04ef6ea188e 100644 --- a/sound/pci/emu10k1/emupcm.c +++ b/sound/pci/emu10k1/emupcm.c @@ -1377,7 +1377,8 @@ int snd_emu10k1_pcm_multi(struct snd_emu10k1 *emu, int device) snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, &snd_emu10k1_efx_playback_ops); pcm->info_flags = 0; - pcm->dev_subclass = SNDRV_PCM_SUBCLASS_GENERIC_MIX; + pcm->dev_class = SNDRV_PCM_CLASS_MULTI; + pcm->dev_subclass = SNDRV_PCM_SUBCLASS_MULTI_MIX; strcpy(pcm->name, "Multichannel Playback"); emu->pcm_multi = pcm;
It's multi, not mono/stereo. AFAICT, this doesn't do anything in the kernel. Also, I think the subclass is meaningless for devices with just one stream, but whatever. Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de> --- sound/pci/emu10k1/emupcm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)