At Wed, 14 May 2008 23:26:11 +0200, Andreas Mohr wrote:
+typedef struct {
- struct snd_pcm_substream *substream;
- int enabled;
- int running;
- unsigned long portbase;
+} snd_azf3328_audio_stream_t;
+typedef enum {
- AZF_PLAYBACK = 0,
- AZF_CAPTURE = 1,
+} snd_azf3328_stream_index_t;
Try to avoid unneeded typedefs.
@@ -237,9 +292,9 @@ /* register value containers for power management * Note: not always full I/O range preserved (just like Win driver!) */ u16 saved_regs_codec [AZF_IO_SIZE_CODEC_PM / 2];
- u16 saved_regs_io2 [AZF_IO_SIZE_IO2_PM / 2];
- u16 saved_regs_game [AZF_IO_SIZE_GAME_PM / 2];
Don't you want to align the bracket, too? :)
u16 saved_regs_mpu [AZF_IO_SIZE_MPU_PM / 2];
- u16 saved_regs_synth[AZF_IO_SIZE_SYNTH_PM / 2];
- u16 saved_regs_opl3[AZF_IO_SIZE_OPL3_PM / 2]; u16 saved_regs_mixer[AZF_IO_SIZE_MIXER_PM / 2];
#endif }; @@ -252,126 +307,181 @@
MODULE_DEVICE_TABLE(pci, snd_azf3328_ids);
+static int +snd_azf3328_io_reg_setb(unsigned reg, u8 mask, int do_set) +{
- u8 prev = inb(reg), new;
- new = (do_set) ? prev|mask : prev & ~mask;
Put spaces around ops.
- /* we need to always write the new value no matter whether it differs or not,
- since some register bits don't indicate their setting */
- outb(new, reg);
- if (new != prev)
- return 1;
- return 0;
+}
Use the normal indent level. (Strange that checkpatch didn't notice it.)
+static int +snd_azf3328_io_reg_setw(unsigned reg, u16 mask, int do_set) +{
- u16 prev = inw(reg), new;
- new = (do_set) ? prev|mask : prev & ~mask;
- /* we need to always write the new value no matter whether it differs or not,
- since some register bits don't indicate their setting */
- outw(new, reg);
- if (new != prev)
- return 1;
- return 0;
+}
Ditto.
+static void +snd_azf3328_codec_activity(struct snd_azf3328 *chip,
(snip)
/* small check to prevent shutting down the other party* in case it's active */if (!((!enable) && (chip->audio_stream[other].running)))
Let's reduce unneeded parentheses - or make it a simpler one like: if (enable || !chip->audio_stream[other].running)
@@ -930,20 +1096,22 @@ case SNDRV_PCM_TRIGGER_START:
(snip)
spin_lock(&chip->reg_lock);
/* stop playback */
/* stop playback */
- /* FIXME: clear interrupts or what??? */ snd_azf3328_codec_outw(chip, IDX_IO_PLAY_IRQTYPE, 0xffff); spin_unlock(&chip->reg_lock);
This part can be replaced easier with the new snd_azf3328_io_reg_setw(). Ditto for other actions.
snd_azf3328_dbgplay("Interrupt %ld!\nIDX_IO_PLAY_FLAGS %04x, IDX_IO_PLAY_IRQTYPE %04x, IDX_IO_IRQSTATUS %04x\n",
irq_count,
irq_count++,
Don't do this in a macro, especially if it's not always compiled in.
thanks,
Takashi