On 04/12/2007 08:22 PM, Rask Ingemann Lambertsen wrote:
Signed-off-by: Rask Ingemann Lambertsen rask@sygehus.dk
Looking good, only some minimal comments...
+++ linux-2.6.20.6-ril/Documentation/sound/alsa/ALSA-Configuration.txt 2007-04-07 17:31:38.000000000 +0200
- Module snd-jazz16
- Module for sound cards based on Media Vision Jazz16 chip (PnP only)
nitpick --> "on the [ ... ] chip" or "on [ ... ] chips" ?
+++ linux-2.6.20.6-ril/sound/isa/Kconfig 2007-04-07 17:31:38.000000000 +0200
+config SND_JAZZ16
- tristate "Media Vision Jazz16"
- depends on SND
- select SND_OPL3_LIB
- select SND_PCM
You will want to select SND_MPU401_UART here as well now that you use the modular mpu401 uart support.
- help
Say Y here to include support for Media Vision Jazz16 basedsoundcards.You probably want to enable Jazz16 Plug and Play support(CONFIG_JAZZ16PNP) also.
I'd prefer some stronger language (^$%##@!!) here. Something like:
"Unless you have a PNPBIOS supported Jazz16, you will need to enable Jazz16 Plug and Play support (CONFIG_JAZZ16PNP) in the kernel as well"
+++ linux-2.6.20.6-ril/sound/isa/sb/jazz16.c 2007-04-05 23:30:27.000000000 +0200
+static irqreturn_t jazz16_interrupt(int irq, void *card) +{
- return snd_sb8dsp_interrupt(card);
+}
"card" is always the struct snd_card * in ALSA code. Could you call the parameter "chip" instead?
- static uint dev_num = 0;
- if (enable[dev_num])
card = snd_card_new(index[dev_num], id[dev_num], THIS_MODULE, 0);- else
card = NULL;- dev_num++;
- if (card == NULL)
return enable[dev_num] ? -ENOMEM : -ENODEV;
You should also guard against dev_num overrunning SNDRV_CARDS. How about:
static uint dev_num; uint num;
if (dev_num < SNDRV_CARDS) num = dev_num++; // wrap-around "danger" if unconditional ++ else return -ENODEV;
if (!enable[num]) return -ENODEV; card = snd_card_new(index[num], id[num], THIS_MODULE, 0); if (!card) return -ENOMEM;
note -- (some versions of) gcc don't place explicitly initialized variables in BSS, even when the initialisation value is 0, so that leaving an explicit = 0 out is kernel style.
That's all...
Rene.