[alsa-devel] [PATCH] sound ac97: replace open-coded, error-prone stuff with AC97 bit defines
ChangeLog:
Use AC97 macros (sometimes already existing, or newly added) instead of error-prone repetition of open-coded values.
Signed-off-by: Andreas Mohr andi@lisas.de --- Patch created on -rc3, tested and checkpatch.pl'd. GIT history seems clean, should thus apply easily.
Optional, suggested patch.
Thanks!
Index: linux-2.6/include/sound/ac97_codec.h =================================================================== --- linux-2.6.orig/include/sound/ac97_codec.h 2011-02-16 23:18:39.000000000 +0100 +++ linux-2.6/include/sound/ac97_codec.h 2011-02-16 23:19:04.000000000 +0100 @@ -96,6 +96,10 @@ #define AC97_FUNC_INFO 0x68 /* Function Information */ #define AC97_SENSE_INFO 0x6a /* Sense Details */
+/* volume controls */ +#define AC97_MUTE_MASK_MONO 0x8000 +#define AC97_MUTE_MASK_STEREO 0x8080 + /* slot allocation */ #define AC97_SLOT_TAG 0 #define AC97_SLOT_CMD_ADDR 1 @@ -138,6 +142,7 @@ #define AC97_BC_18BIT_ADC 0x0100 /* 18-bit ADC resolution */ #define AC97_BC_20BIT_ADC 0x0200 /* 20-bit ADC resolution */ #define AC97_BC_ADC_MASK 0x0300 +#define AC97_BC_3D_TECH_ID_MASK 0x7c00 /* Per-vendor ID of 3D enhancement */
/* general purpose */ #define AC97_GP_DRSS_MASK 0x0c00 /* double rate slot select */ Index: linux-2.6/sound/pci/ac97/ac97_codec.c =================================================================== --- linux-2.6.orig/sound/pci/ac97/ac97_codec.c 2011-02-16 23:19:03.000000000 +0100 +++ linux-2.6/sound/pci/ac97/ac97_codec.c 2011-02-16 23:19:04.000000000 +0100 @@ -597,9 +597,9 @@ snd_ac97_page_restore(ac97, page_save); #ifdef CONFIG_SND_AC97_POWER_SAVE /* check analog mixer power-down */ - if ((val_mask & 0x8000) && + if ((val_mask & AC97_PD_EAPD) && (kcontrol->private_value & (1<<30))) { - if (val & 0x8000) + if (val & AC97_PD_EAPD) ac97->power_up &= ~(1 << (reg>>1)); else ac97->power_up |= 1 << (reg>>1); @@ -1042,20 +1042,20 @@
static int snd_ac97_try_volume_mix(struct snd_ac97 * ac97, int reg) { - unsigned short val, mask = 0x8000; + unsigned short val, mask = AC97_MUTE_MASK_MONO;
if (! snd_ac97_valid_reg(ac97, reg)) return 0;
switch (reg) { case AC97_MASTER_TONE: - return ac97->caps & 0x04 ? 1 : 0; + return ac97->caps & AC97_BC_BASS_TREBLE ? 1 : 0; case AC97_HEADPHONE: - return ac97->caps & 0x10 ? 1 : 0; + return ac97->caps & AC97_BC_HEADPHONE ? 1 : 0; case AC97_REC_GAIN_MIC: - return ac97->caps & 0x01 ? 1 : 0; + return ac97->caps & AC97_BC_DEDICATED_MIC ? 1 : 0; case AC97_3D_CONTROL: - if (ac97->caps & 0x7c00) { + if (ac97->caps & AC97_BC_3D_TECH_ID_MASK) { val = snd_ac97_read(ac97, reg); /* if nonzero - fixed and we can't set it */ return val == 0; @@ -1111,7 +1111,10 @@ *lo_max = *hi_max = 0; for (i = 0 ; i < ARRAY_SIZE(cbit); i++) { unsigned short val; - snd_ac97_write(ac97, reg, 0x8080 | cbit[i] | (cbit[i] << 8)); + snd_ac97_write( + ac97, reg, + AC97_MUTE_MASK_STEREO | cbit[i] | (cbit[i] << 8) + ); /* Do the read twice due to buffers on some ac97 codecs. * e.g. The STAC9704 returns exactly what you wrote to the register * if you read it immediately. This causes the detect routine to fail. @@ -1146,14 +1149,14 @@ unsigned short val, val1;
*max = 63; - val = 0x8080 | (0x20 << shift); + val = AC97_MUTE_MASK_STEREO | (0x20 << shift); snd_ac97_write(ac97, reg, val); val1 = snd_ac97_read(ac97, reg); if (val != val1) { *max = 31; } /* reset volume to zero */ - snd_ac97_write_cache(ac97, reg, 0x8080); + snd_ac97_write_cache(ac97, reg, AC97_MUTE_MASK_STEREO); }
static inline int printable(unsigned int x) @@ -1190,16 +1193,16 @@ if (! snd_ac97_valid_reg(ac97, reg)) return 0;
- mute_mask = 0x8000; + mute_mask = AC97_MUTE_MASK_MONO; val = snd_ac97_read(ac97, reg); if (check_stereo || (ac97->flags & AC97_STEREO_MUTES)) { /* check whether both mute bits work */ - val1 = val | 0x8080; + val1 = val | AC97_MUTE_MASK_STEREO; snd_ac97_write(ac97, reg, val1); if (val1 == snd_ac97_read(ac97, reg)) - mute_mask = 0x8080; + mute_mask = AC97_MUTE_MASK_STEREO; } - if (mute_mask == 0x8080) { + if (mute_mask == AC97_MUTE_MASK_STEREO) { struct snd_kcontrol_new tmp = AC97_DOUBLE(name, reg, 15, 7, 1, 1); if (check_amix) tmp.private_value |= (1 << 30); @@ -1275,9 +1278,11 @@ err = snd_ctl_add(card, kctl); if (err < 0) return err; - snd_ac97_write_cache(ac97, reg, - (snd_ac97_read(ac97, reg) & 0x8080) | - lo_max | (hi_max << 8)); + snd_ac97_write_cache( + ac97, reg, + (snd_ac97_read(ac97, reg) & AC97_MUTE_MASK_STEREO) + | lo_max | (hi_max << 8) + ); return 0; }
@@ -1339,7 +1344,7 @@ return err; }
- ac97->regs[AC97_CENTER_LFE_MASTER] = 0x8080; + ac97->regs[AC97_CENTER_LFE_MASTER] = AC97_MUTE_MASK_STEREO;
/* build center controls */ if ((snd_ac97_try_volume_mix(ac97, AC97_CENTER_LFE_MASTER)) @@ -1417,8 +1422,12 @@ if ((err = snd_ctl_add(card, kctl = snd_ac97_cnew(&snd_ac97_controls_pc_beep[idx], ac97))) < 0) return err; set_tlv_db_scale(kctl, db_scale_4bit); - snd_ac97_write_cache(ac97, AC97_PC_BEEP, - snd_ac97_read(ac97, AC97_PC_BEEP) | 0x801e); + snd_ac97_write_cache( + ac97, + AC97_PC_BEEP, + (snd_ac97_read(ac97, AC97_PC_BEEP) + | AC97_MUTE_MASK_MONO | 0x001e) + ); } /* build Phone controls */ @@ -1552,7 +1561,7 @@ }
/* build Simulated Stereo Enhancement control */ - if (ac97->caps & 0x0008) { + if (ac97->caps & AC97_BC_SIM_STEREO) { if ((err = snd_ctl_add(card, snd_ac97_cnew(&snd_ac97_controls_general[AC97_GENERAL_STEREO_ENHANCEMENT], ac97))) < 0) return err; } @@ -1564,7 +1573,7 @@ }
/* build Loudness control */ - if (ac97->caps & 0x0020) { + if (ac97->caps & AC97_BC_LOUDNESS) { if ((err = snd_ctl_add(card, snd_ac97_cnew(&snd_ac97_controls_general[AC97_GENERAL_LOUDNESS], ac97))) < 0) return err; } @@ -2549,8 +2558,8 @@ schedule_timeout_uninterruptible(1); } while (time_after_eq(end_time, jiffies)); /* FIXME: extra delay */ - ac97->bus->ops->write(ac97, AC97_MASTER, 0x8000); - if (snd_ac97_read(ac97, AC97_MASTER) != 0x8000) + ac97->bus->ops->write(ac97, AC97_MASTER, AC97_MUTE_MASK_MONO); + if (snd_ac97_read(ac97, AC97_MASTER) != AC97_MUTE_MASK_MONO) msleep(250); } else { end_time = jiffies + msecs_to_jiffies(100); @@ -2754,12 +2763,12 @@ int rshift = (kcontrol->private_value >> 12) & 0x0f; unsigned short mask; if (shift != rshift) - mask = 0x8080; + mask = AC97_MUTE_MASK_STEREO; else - mask = 0x8000; - snd_ac97_update_bits(ac97, AC97_POWERDOWN, 0x8000, + mask = AC97_MUTE_MASK_MONO; + snd_ac97_update_bits(ac97, AC97_POWERDOWN, AC97_PD_EAPD, (ac97->regs[AC97_MASTER] & mask) == mask ? - 0x8000 : 0); + AC97_PD_EAPD : 0); } return err; } @@ -2772,7 +2781,10 @@ return -ENOENT; msw->put = master_mute_sw_put; snd_ac97_remove_ctl(ac97, "External Amplifier", NULL); - snd_ac97_update_bits(ac97, AC97_POWERDOWN, 0x8000, 0x8000); /* mute LED on */ + snd_ac97_update_bits( + ac97, AC97_POWERDOWN, + AC97_PD_EAPD, AC97_PD_EAPD /* mute LED on */ + ); ac97->scaps |= AC97_SCAP_EAPD_LED; return 0; } @@ -2787,12 +2799,12 @@ int rshift = (kcontrol->private_value >> 12) & 0x0f; unsigned short mask; if (shift != rshift) - mask = 0x8080; + mask = AC97_MUTE_MASK_STEREO; else - mask = 0x8000; - snd_ac97_update_bits(ac97, AC97_POWERDOWN, 0x8000, + mask = AC97_MUTE_MASK_MONO; + snd_ac97_update_bits(ac97, AC97_POWERDOWN, AC97_PD_EAPD, (ac97->regs[AC97_MASTER] & mask) == mask ? - 0x8000 : 0); + AC97_PD_EAPD : 0); } return err; } @@ -2808,7 +2820,10 @@ snd_ac97_remove_ctl(ac97, "External Amplifier", NULL); snd_ac97_remove_ctl(ac97, "Headphone Playback", "Switch"); snd_ac97_remove_ctl(ac97, "Headphone Playback", "Volume"); - snd_ac97_update_bits(ac97, AC97_POWERDOWN, 0x8000, 0x8000); /* mute LED on */ + snd_ac97_update_bits( + ac97, AC97_POWERDOWN, + AC97_PD_EAPD, AC97_PD_EAPD /* mute LED on */ + ); return 0; }
At Thu, 17 Feb 2011 00:17:53 +0100, Andreas Mohr wrote:
ChangeLog:
Use AC97 macros (sometimes already existing, or newly added) instead of error-prone repetition of open-coded values.
Signed-off-by: Andreas Mohr andi@lisas.de
Patch created on -rc3, tested and checkpatch.pl'd. GIT history seems clean, should thus apply easily.
Thanks, applied this one.
Takashi
Optional, suggested patch.
Thanks!
Index: linux-2.6/include/sound/ac97_codec.h
--- linux-2.6.orig/include/sound/ac97_codec.h 2011-02-16 23:18:39.000000000 +0100 +++ linux-2.6/include/sound/ac97_codec.h 2011-02-16 23:19:04.000000000 +0100 @@ -96,6 +96,10 @@ #define AC97_FUNC_INFO 0x68 /* Function Information */ #define AC97_SENSE_INFO 0x6a /* Sense Details */
+/* volume controls */ +#define AC97_MUTE_MASK_MONO 0x8000 +#define AC97_MUTE_MASK_STEREO 0x8080
/* slot allocation */ #define AC97_SLOT_TAG 0 #define AC97_SLOT_CMD_ADDR 1 @@ -138,6 +142,7 @@ #define AC97_BC_18BIT_ADC 0x0100 /* 18-bit ADC resolution */ #define AC97_BC_20BIT_ADC 0x0200 /* 20-bit ADC resolution */ #define AC97_BC_ADC_MASK 0x0300 +#define AC97_BC_3D_TECH_ID_MASK 0x7c00 /* Per-vendor ID of 3D enhancement */
/* general purpose */ #define AC97_GP_DRSS_MASK 0x0c00 /* double rate slot select */ Index: linux-2.6/sound/pci/ac97/ac97_codec.c =================================================================== --- linux-2.6.orig/sound/pci/ac97/ac97_codec.c 2011-02-16 23:19:03.000000000 +0100 +++ linux-2.6/sound/pci/ac97/ac97_codec.c 2011-02-16 23:19:04.000000000 +0100 @@ -597,9 +597,9 @@ snd_ac97_page_restore(ac97, page_save); #ifdef CONFIG_SND_AC97_POWER_SAVE /* check analog mixer power-down */
- if ((val_mask & 0x8000) &&
- if ((val_mask & AC97_PD_EAPD) && (kcontrol->private_value & (1<<30))) {
if (val & 0x8000)
else ac97->power_up |= 1 << (reg>>1);if (val & AC97_PD_EAPD) ac97->power_up &= ~(1 << (reg>>1));
@@ -1042,20 +1042,20 @@
static int snd_ac97_try_volume_mix(struct snd_ac97 * ac97, int reg) {
- unsigned short val, mask = 0x8000;
unsigned short val, mask = AC97_MUTE_MASK_MONO;
if (! snd_ac97_valid_reg(ac97, reg)) return 0;
switch (reg) { case AC97_MASTER_TONE:
return ac97->caps & 0x04 ? 1 : 0;
case AC97_HEADPHONE:return ac97->caps & AC97_BC_BASS_TREBLE ? 1 : 0;
return ac97->caps & 0x10 ? 1 : 0;
case AC97_REC_GAIN_MIC:return ac97->caps & AC97_BC_HEADPHONE ? 1 : 0;
return ac97->caps & 0x01 ? 1 : 0;
case AC97_3D_CONTROL:return ac97->caps & AC97_BC_DEDICATED_MIC ? 1 : 0;
if (ac97->caps & 0x7c00) {
if (ac97->caps & AC97_BC_3D_TECH_ID_MASK) { val = snd_ac97_read(ac97, reg); /* if nonzero - fixed and we can't set it */ return val == 0;
@@ -1111,7 +1111,10 @@ *lo_max = *hi_max = 0; for (i = 0 ; i < ARRAY_SIZE(cbit); i++) { unsigned short val;
snd_ac97_write(ac97, reg, 0x8080 | cbit[i] | (cbit[i] << 8));
snd_ac97_write(
ac97, reg,
AC97_MUTE_MASK_STEREO | cbit[i] | (cbit[i] << 8)
/* Do the read twice due to buffers on some ac97 codecs.);
- e.g. The STAC9704 returns exactly what you wrote to the register
- if you read it immediately. This causes the detect routine to fail.
@@ -1146,14 +1149,14 @@ unsigned short val, val1;
*max = 63;
- val = 0x8080 | (0x20 << shift);
- val = AC97_MUTE_MASK_STEREO | (0x20 << shift); snd_ac97_write(ac97, reg, val); val1 = snd_ac97_read(ac97, reg); if (val != val1) { *max = 31; } /* reset volume to zero */
- snd_ac97_write_cache(ac97, reg, 0x8080);
- snd_ac97_write_cache(ac97, reg, AC97_MUTE_MASK_STEREO);
}
static inline int printable(unsigned int x) @@ -1190,16 +1193,16 @@ if (! snd_ac97_valid_reg(ac97, reg)) return 0;
- mute_mask = 0x8000;
- mute_mask = AC97_MUTE_MASK_MONO; val = snd_ac97_read(ac97, reg); if (check_stereo || (ac97->flags & AC97_STEREO_MUTES)) { /* check whether both mute bits work */
val1 = val | 0x8080;
snd_ac97_write(ac97, reg, val1); if (val1 == snd_ac97_read(ac97, reg))val1 = val | AC97_MUTE_MASK_STEREO;
mute_mask = 0x8080;
}mute_mask = AC97_MUTE_MASK_STEREO;
- if (mute_mask == 0x8080) {
- if (mute_mask == AC97_MUTE_MASK_STEREO) { struct snd_kcontrol_new tmp = AC97_DOUBLE(name, reg, 15, 7, 1, 1); if (check_amix) tmp.private_value |= (1 << 30);
@@ -1275,9 +1278,11 @@ err = snd_ctl_add(card, kctl); if (err < 0) return err;
- snd_ac97_write_cache(ac97, reg,
(snd_ac97_read(ac97, reg) & 0x8080) |
lo_max | (hi_max << 8));
- snd_ac97_write_cache(
ac97, reg,
(snd_ac97_read(ac97, reg) & AC97_MUTE_MASK_STEREO)
| lo_max | (hi_max << 8)
- ); return 0;
}
@@ -1339,7 +1344,7 @@ return err; }
- ac97->regs[AC97_CENTER_LFE_MASTER] = 0x8080;
ac97->regs[AC97_CENTER_LFE_MASTER] = AC97_MUTE_MASK_STEREO;
/* build center controls */ if ((snd_ac97_try_volume_mix(ac97, AC97_CENTER_LFE_MASTER))
@@ -1417,8 +1422,12 @@ if ((err = snd_ctl_add(card, kctl = snd_ac97_cnew(&snd_ac97_controls_pc_beep[idx], ac97))) < 0) return err; set_tlv_db_scale(kctl, db_scale_4bit);
snd_ac97_write_cache(ac97, AC97_PC_BEEP,
snd_ac97_read(ac97, AC97_PC_BEEP) | 0x801e);
snd_ac97_write_cache(
ac97,
AC97_PC_BEEP,
(snd_ac97_read(ac97, AC97_PC_BEEP)
| AC97_MUTE_MASK_MONO | 0x001e)
);
}
/* build Phone controls */
@@ -1552,7 +1561,7 @@ }
/* build Simulated Stereo Enhancement control */
- if (ac97->caps & 0x0008) {
- if (ac97->caps & AC97_BC_SIM_STEREO) { if ((err = snd_ctl_add(card, snd_ac97_cnew(&snd_ac97_controls_general[AC97_GENERAL_STEREO_ENHANCEMENT], ac97))) < 0) return err; }
@@ -1564,7 +1573,7 @@ }
/* build Loudness control */
- if (ac97->caps & 0x0020) {
- if (ac97->caps & AC97_BC_LOUDNESS) { if ((err = snd_ctl_add(card, snd_ac97_cnew(&snd_ac97_controls_general[AC97_GENERAL_LOUDNESS], ac97))) < 0) return err; }
@@ -2549,8 +2558,8 @@ schedule_timeout_uninterruptible(1); } while (time_after_eq(end_time, jiffies)); /* FIXME: extra delay */
ac97->bus->ops->write(ac97, AC97_MASTER, 0x8000);
if (snd_ac97_read(ac97, AC97_MASTER) != 0x8000)
ac97->bus->ops->write(ac97, AC97_MASTER, AC97_MUTE_MASK_MONO);
} else { end_time = jiffies + msecs_to_jiffies(100);if (snd_ac97_read(ac97, AC97_MASTER) != AC97_MUTE_MASK_MONO) msleep(250);
@@ -2754,12 +2763,12 @@ int rshift = (kcontrol->private_value >> 12) & 0x0f; unsigned short mask; if (shift != rshift)
mask = 0x8080;
elsemask = AC97_MUTE_MASK_STEREO;
mask = 0x8000;
snd_ac97_update_bits(ac97, AC97_POWERDOWN, 0x8000,
mask = AC97_MUTE_MASK_MONO;
snd_ac97_update_bits(ac97, AC97_POWERDOWN, AC97_PD_EAPD, (ac97->regs[AC97_MASTER] & mask) == mask ?
0x8000 : 0);
} return err;AC97_PD_EAPD : 0);
} @@ -2772,7 +2781,10 @@ return -ENOENT; msw->put = master_mute_sw_put; snd_ac97_remove_ctl(ac97, "External Amplifier", NULL);
- snd_ac97_update_bits(ac97, AC97_POWERDOWN, 0x8000, 0x8000); /* mute LED on */
- snd_ac97_update_bits(
ac97, AC97_POWERDOWN,
AC97_PD_EAPD, AC97_PD_EAPD /* mute LED on */
- ); ac97->scaps |= AC97_SCAP_EAPD_LED; return 0;
} @@ -2787,12 +2799,12 @@ int rshift = (kcontrol->private_value >> 12) & 0x0f; unsigned short mask; if (shift != rshift)
mask = 0x8080;
elsemask = AC97_MUTE_MASK_STEREO;
mask = 0x8000;
snd_ac97_update_bits(ac97, AC97_POWERDOWN, 0x8000,
mask = AC97_MUTE_MASK_MONO;
snd_ac97_update_bits(ac97, AC97_POWERDOWN, AC97_PD_EAPD, (ac97->regs[AC97_MASTER] & mask) == mask ?
0x8000 : 0);
} return err;AC97_PD_EAPD : 0);
} @@ -2808,7 +2820,10 @@ snd_ac97_remove_ctl(ac97, "External Amplifier", NULL); snd_ac97_remove_ctl(ac97, "Headphone Playback", "Switch"); snd_ac97_remove_ctl(ac97, "Headphone Playback", "Volume");
- snd_ac97_update_bits(ac97, AC97_POWERDOWN, 0x8000, 0x8000); /* mute LED on */
- snd_ac97_update_bits(
ac97, AC97_POWERDOWN,
AC97_PD_EAPD, AC97_PD_EAPD /* mute LED on */
- ); return 0;
}
participants (2)
-
Andreas Mohr
-
Takashi Iwai