* PATCH - vt1618 7.1 Audio
@ 2008-08-20 0:17 John L. Utz III
2008-08-20 8:28 ` Takashi Iwai
0 siblings, 1 reply; 9+ messages in thread
From: John L. Utz III @ 2008-08-20 0:17 UTC (permalink / raw)
To: ALSA Developers
Signed-off-by: John L. Utz III john.utz@dmx.com
diff --git a/pci/ac97/ac97_codec.c b/pci/ac97/ac97_codec.c
index d0023e9..a34f1ea 100644
--- a/pci/ac97/ac97_codec.c
+++ b/pci/ac97/ac97_codec.c
@@ -168,7 +168,7 @@ static const struct ac97_codec_id snd_ac97_codec_ids[]
= {
{ 0x54584e20, 0xffffffff, "TLC320AD9xC", NULL, NULL },
{ 0x56494161, 0xffffffff, "VIA1612A", NULL, NULL }, // modified ICE1232
with S/PDIF
{ 0x56494170, 0xffffffff, "VIA1617A", patch_vt1617a, NULL }, // modified
VT1616 with S/PDIF
-{ 0x56494182, 0xffffffff, "VIA1618", NULL, NULL },
+{ 0x56494182, 0xffffffff, "VIA1618", patch_vt1618, NULL }, // clean
sheet of crinkled paper
{ 0x57454301, 0xffffffff, "W83971D", NULL, NULL },
{ 0x574d4c00, 0xffffffff, "WM9701,WM9701A", NULL, NULL },
{ 0x574d4C03, 0xffffffff, "WM9703,WM9707,WM9708,WM9717", patch_wolfson03,
NULL},
@@ -609,7 +609,6 @@ AC97_SINGLE("PC Speaker Playback Volume",
AC97_PC_BEEP, 1, 15, 1)
static const struct snd_kcontrol_new snd_ac97_controls_mic_boost =
AC97_SINGLE("Mic Boost (+20dB)", AC97_MIC, 6, 1, 0);
-
static const char* std_rec_sel[] = {"Mic", "CD", "Video", "Aux", "Line",
"Mix", "Mix Mono", "Phone"};
static const char* std_3d_path[] = {"pre 3D", "post 3D"};
static const char* std_mix[] = {"Mix", "Mic"};
diff --git a/pci/ac97/ac97_patch.c b/pci/ac97/ac97_patch.c
index bb028f8..2042415 100644
--- a/pci/ac97/ac97_patch.c
+++ b/pci/ac97/ac97_patch.c
@@ -3465,7 +3465,7 @@ static int patch_vt1616(struct snd_ac97 * ac97)
/*
* unfortunately, the vt1617a stashes the twiddlers required for
- * nooding the i/o jacks on 2 different regs. * thameans that we cant
+ * nooding the i/o jacks on 2 different regs. that means that we cant
* use the easy way provided by AC97_ENUM_DOUBLE() we have to write
* are own funcs.
*
@@ -3576,6 +3576,413 @@ int patch_vt1617a(struct snd_ac97 * ac97)
return err;
}
+
+/* use these alot in the 1618 code but i cant find a better place to put
them */
+
+static const char* std_enable[] = {"Enabled", "Disabled"};
+static const char* std_disable[] = {"Disabled","Enabled"};
+
+/* disable enable c/lfe exchange */
+
+static int snd_ac97_vt1618_clex_info(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_info *uinfo)
+{
+ return ac97_enum_text_info(kcontrol, uinfo, std_disable, 2);
+}
+
+static int snd_ac97_vt1618_clex_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol);
+
+ ucontrol->value.enumerated.item[0] = (snd_ac97_read(pac97, 0x5a) &
+ 0x0100) >> 8;
+ return 0;
+}
+
+static int snd_ac97_vt1618_clex_put(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol);
+
+ /* notice relationship between mask and shift! */
+
+ return snd_ac97_update_bits(pac97, 0x5a, 0x0100,
+ ucontrol->value.enumerated.item[0] << 8);
+}
+
+
+/* disable enable dc offset */
+
+static int snd_ac97_vt1618_dcof_info(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_info *uinfo)
+{
+ return ac97_enum_text_info(kcontrol, uinfo, std_disable, 2);
+}
+
+static int snd_ac97_vt1618_dcof_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol);
+
+ ucontrol->value.enumerated.item[0] = (snd_ac97_read(pac97, 0x5a) &
+ 0x0400) >> 10;
+ return 0;
+}
+
+static int snd_ac97_vt1618_dcof_put(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol);
+
+ /* notice relationship between mask and shift! */
+
+ return snd_ac97_update_bits(pac97, 0x5a, 0x0400,
+ ucontrol->value.enumerated.item[0] << 10);
+}
+
+
+/* enable disable headphone amp */
+
+static int snd_ac97_vt1618_hamp_info(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_info *uinfo)
+{
+ return ac97_enum_text_info(kcontrol, uinfo, std_enable, 2);
+}
+
+static int snd_ac97_vt1618_hamp_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol);
+
+ ucontrol->value.enumerated.item[0] = (snd_ac97_read(pac97, 0x5c) &
+ 0x0020) >> 5;
+ return 0;
+}
+
+static int snd_ac97_vt1618_hamp_put(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol);
+
+ /* notice relationship between mask and shift! */
+
+ return snd_ac97_update_bits(pac97, 0x5c, 0x0020,
+ ucontrol->value.enumerated.item[0] << 5);
+}
+
+
+/* enable disable surround back channel */
+
+static int snd_ac97_vt1618_srbk_info(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_info *uinfo)
+{
+ return ac97_enum_text_info(kcontrol, uinfo, std_enable, 2);
+}
+
+static int snd_ac97_vt1618_srbk_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol);
+
+ ucontrol->value.enumerated.item[0] = (snd_ac97_read(pac97, 0x5c) &
+ 0x0008) >> 3;
+ return 0;
+}
+
+static int snd_ac97_vt1618_srbk_put(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol);
+
+ /* notice relationship between mask and shift! */
+
+ return snd_ac97_update_bits(pac97, 0x5c, 0x0008,
+ ucontrol->value.enumerated.item[0] << 3);
+}
+
+
+/* enable disable soft mute */
+
+static int snd_ac97_vt1618_smute_info(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_info *uinfo)
+{
+ return ac97_enum_text_info(kcontrol, uinfo, std_enable, 2);
+}
+
+static int snd_ac97_vt1618_smute_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol);
+
+ ucontrol->value.enumerated.item[0] = (snd_ac97_read(pac97, 0x5c) &
+ 0x0001);
+ return 0;
+}
+
+static int snd_ac97_vt1618_smute_put(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol);
+
+ /* notice relationship between mask and shift! */
+
+ return snd_ac97_update_bits(pac97, 0x5c, 0x0001,
+ ucontrol->value.enumerated.item[0]);
+}
+
+
+/* config aux in jack - not found on 3 jack motherboards or soundcards */
+
+static int snd_ac97_vt1618_auxin_info(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_info *uinfo)
+{
+ static const char* texts[] = {"Aux In", "Surround Back Out"};
+ return ac97_enum_text_info(kcontrol, uinfo, texts, 2);
+}
+
+static int snd_ac97_vt1618_auxin_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol);
+
+ ucontrol->value.enumerated.item[0] = (snd_ac97_read(pac97, 0x76) &
+ 0x0008) >> 3;
+
+ return 0;
+}
+
+static int snd_ac97_vt1618_auxin_put(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol);
+
+ /* notice relationship between mask and shift! */
+
+ return snd_ac97_update_bits(pac97, 0x76, 0x0008,
+ ucontrol->value.enumerated.item[0] << 3);
+}
+
+
+/*
+ * VIA implements 'Smart 5.1' completely differently on the 1618 than
+ * it does on the 1617a. awesome! They seem to have sourced this
+ * particular revision of the technology from somebody else, it's
+ * called Universal Audio Jack, it shows up on some other folk's chips
+ * as well.
+ *
+ * ordering in this list reflects vt1618 docs for Reg 60h and
+ * the block diagram, DACs are as follows:
+ *
+ * OUT_O -> Front,
+ * OUT_1 -> Surround,
+ * OUT_2 -> C/LFE
+ *
+ * Unlike the 1617a, each OUT has a consistent set of mappings
+ * for all bitpatterns other than 00:
+ *
+ * 01 Unmixed Output
+ * 10 Line In
+ * 11 Mic In
+ *
+ * Special Case of 00:
+ *
+ * OUT_0 Mixed Output
+ * OUT_1 Reserved
+ * OUT_2 Reserved
+ *
+ * I have no idea what the hell Reserved does, but on an MSI
+ * CN700T, i have to set it to get surround output - YMMV, bad
+ * shit may happen.
+ *
+ * If other chips use Universal Audio Jack, then this code might be
+ * applicable to them.
+ */
+
+/* copied from ac97_surround_jack_mode_info() ordering in this list
+ * reflects vt1618 docs for Vendor Defined Register 0x60 */
+
+static int snd_ac97_vt1618_UAJ0_info(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_info *uinfo)
+{
+ static const char* texts[] = { "Speaker Out", "DAC Unmixed Out", "Line
In", "Mic In"};
+ return ac97_enum_text_info(kcontrol, uinfo, texts, 4);
+}
+
+/* UAJ1 and UAJ2 are not supposed to have 00 written to them?? i
+ * dunno, because thats something that i have to do to get 5.1 out to
+ * work. */
+
+static int snd_ac97_vt1618_UAJ1_info(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_info *uinfo)
+{
+ static const char* texts[] = { "Surround Out", "DAC Unmixed Out",
"Line In", "Mic In"};
+ return ac97_enum_text_info(kcontrol, uinfo, texts, 4);
+}
+
+static int snd_ac97_vt1618_UAJ2_info(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_info *uinfo)
+{
+ static const char* texts[] = { "Center/LowFre Out", "DAC Unmixed
Out", "Line In", "Mic In"};
+ return ac97_enum_text_info(kcontrol, uinfo, texts, 4);
+}
+
+/* All of the vt1618 Universal Audio Jack twiddlers are on
+ Vendor Defined Register 0x60, page 0. The bits, and thus
+ the mask, are the only thing that changes */
+
+static int snd_ac97_vt1618_UAJ_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol, unsigned short mask)
+{
+ struct snd_ac97 *pac97;
+ ushort usDatPag, usUAJ, usShift = 0; // 0 is a possible value
+
+ if(0x000C == mask) usShift = 2;
+ if(0x0030 == mask) usShift = 4;
+
+ pac97 = snd_kcontrol_chip(kcontrol);
+
+ mutex_lock(&pac97->page_mutex);
+
+ usDatPag = snd_ac97_read(pac97, AC97_INT_PAGING) & AC97_PAGE_MASK;
+ snd_ac97_update_bits(pac97, AC97_INT_PAGING, AC97_PAGE_MASK, 0);
+
+ usUAJ = snd_ac97_read(pac97, 0x60) & mask;
+
+ snd_ac97_update_bits(pac97, AC97_INT_PAGING, AC97_PAGE_MASK, usDatPag);
+ mutex_unlock(&pac97->page_mutex); // unlock paging
+
+ ucontrol->value.enumerated.item[0] = usUAJ >> usShift;
+
+ return 0;
+}
+
+static int snd_ac97_vt1618_UAJ_put(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol,
+ unsigned short mask)
+{
+ struct snd_ac97 *pac97;
+ ushort usUAJ, usShift = 0; /* 0 is a possible value */
+
+ if(0x000C == mask) usShift = 2;
+ if(0x0030 == mask) usShift = 4;
+
+ pac97 = snd_kcontrol_chip(kcontrol);
+
+ usUAJ = ucontrol->value.enumerated.item[0] << usShift;
+
+ return ac97_update_bits_page(pac97, 0x60, mask, usUAJ, 0);
+}
+
+static int snd_ac97_vt1618_UAJ0_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ return snd_ac97_vt1618_UAJ_get(kcontrol, ucontrol, 0x0003);
+}
+
+static int snd_ac97_vt1618_UAJ1_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ return snd_ac97_vt1618_UAJ_get(kcontrol, ucontrol, 0x000C);
+}
+
+static int snd_ac97_vt1618_UAJ2_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ return snd_ac97_vt1618_UAJ_get(kcontrol, ucontrol, 0x0030);
+}
+
+static int snd_ac97_vt1618_UAJ0_put(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ return snd_ac97_vt1618_UAJ_put(kcontrol, ucontrol, 0x0003);
+}
+
+static int snd_ac97_vt1618_UAJ1_put(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ return snd_ac97_vt1618_UAJ_put(kcontrol, ucontrol, 0x000C);
+}
+
+static int snd_ac97_vt1618_UAJ2_put(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ return snd_ac97_vt1618_UAJ_put(kcontrol, ucontrol, 0x0030);
+}
+
+static const struct snd_kcontrol_new snd_ac97_controls_vt1618[] = {
+
+ {
+ .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+ .name = "Center LFE Exchange",
+ .info = snd_ac97_vt1618_clex_info,
+ .get = snd_ac97_vt1618_clex_get,
+ .put = snd_ac97_vt1618_clex_put
+ },
+ {
+ .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+ .name = "DC Offset",
+ .info = snd_ac97_vt1618_dcof_info,
+ .get = snd_ac97_vt1618_dcof_get,
+ .put = snd_ac97_vt1618_dcof_put
+ },diff --git a/pci/ac97/ac97_codec.c b/pci/ac97/ac97_codec.c
index d0023e9..a34f1ea 100644
--- a/pci/ac97/ac97_codec.c
+++ b/pci/ac97/ac97_codec.c
@@ -168,7 +168,7 @@ static const struct ac97_codec_id snd_ac97_codec_ids[]
= {
{ 0x54584e20, 0xffffffff, "TLC320AD9xC", NULL, NULL },
{ 0x56494161, 0xffffffff, "VIA1612A", NULL, NULL }, // modified ICE1232
with S/PDIF
{ 0x56494170, 0xffffffff, "VIA1617A", patch_vt1617a, NULL }, // modified
VT1616 with S/PDIF
-{ 0x56494182, 0xffffffff, "VIA1618", NULL, NULL },
+{ 0x56494182, 0xffffffff, "VIA1618", patch_vt1618, NULL }, // clean
sheet of crinkled paper
{ 0x57454301, 0xffffffff, "W83971D", NULL, NULL },
{ 0x574d4c00, 0xffffffff, "WM9701,WM9701A", NULL, NULL },
{ 0x574d4C03, 0xffffffff, "WM9703,WM9707,WM9708,WM9717", patch_wolfson03,
NULL},
@@ -609,7 +609,6 @@ AC97_SINGLE("PC Speaker Playback Volume",
AC97_PC_BEEP, 1, 15, 1)
static const struct snd_kcontrol_new snd_ac97_controls_mic_boost =
AC97_SINGLE("Mic Boost (+20dB)", AC97_MIC, 6, 1, 0);
-
static const char* std_rec_sel[] = {"Mic", "CD", "Video", "Aux", "Line",
"Mix", "Mix Mono", "Phone"};
static const char* std_3d_path[] = {"pre 3D", "post 3D"};
static const char* std_mix[] = {"Mix", "Mic"};
diff --git a/pci/ac97/ac97_patch.c b/pci/ac97/ac97_patch.c
index bb028f8..2042415 100644
--- a/pci/ac97/ac97_patch.c
+++ b/pci/ac97/ac97_patch.c
@@ -3465,7 +3465,7 @@ static int patch_vt1616(struct snd_ac97 * ac97)
/*
* unfortunately, the vt1617a stashes the twiddlers required for
- * nooding the i/o jacks on 2 different regs. * thameans that we cant
+ * nooding the i/o jacks on 2 different regs. that means that we cant
* use the easy way provided by AC97_ENUM_DOUBLE() we have to write
* are own funcs.
*
@@ -3576,6 +3576,413 @@ int patch_vt1617a(struct snd_ac97 * ac97)
return err;
}
+
+/* use these alot in the 1618 code but i cant find a better place to put
them */
+
+static const char* std_enable[] = {"Enabled", "Disabled"};
+static const char* std_disable[] = {"Disabled","Enabled"};
+
+/* disable enable c/lfe exchange */
+
+static int snd_ac97_vt1618_clex_info(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_info *uinfo)
+{
+ return ac97_enum_text_info(kcontrol, uinfo, std_disable, 2);
+}
+
+static int snd_ac97_vt1618_clex_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol);
+
+ ucontrol->value.enumerated.item[0] = (snd_ac97_read(pac97, 0x5a) &
+ 0x0100) >> 8;
+ return 0;
+}
+
+static int snd_ac97_vt1618_clex_put(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol);
+
+ /* notice relationship between mask and shift! */
+
+ return snd_ac97_update_bits(pac97, 0x5a, 0x0100,
+ ucontrol->value.enumerated.item[0] << 8);
+}
+
+
+/* disable enable dc offset */
+
+static int snd_ac97_vt1618_dcof_info(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_info *uinfo)
+{
+ return ac97_enum_text_info(kcontrol, uinfo, std_disable, 2);
+}
+
+static int snd_ac97_vt1618_dcof_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol);
+
+ ucontrol->value.enumerated.item[0] = (snd_ac97_read(pac97, 0x5a) &
+ 0x0400) >> 10;
+ return 0;
+}
+
+static int snd_ac97_vt1618_dcof_put(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol);
+
+ /* notice relationship between mask and shift! */
+
+ return snd_ac97_update_bits(pac97, 0x5a, 0x0400,
+ ucontrol->value.enumerated.item[0] << 10);
+}
+
+
+/* enable disable headphone amp */
+
+static int snd_ac97_vt1618_hamp_info(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_info *uinfo)
+{
+ return ac97_enum_text_info(kcontrol, uinfo, std_enable, 2);
+}
+
+static int snd_ac97_vt1618_hamp_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol);
+
+ ucontrol->value.enumerated.item[0] = (snd_ac97_read(pac97, 0x5c) &
+ 0x0020) >> 5;
+ return 0;
+}
+
+static int snd_ac97_vt1618_hamp_put(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol);
+
+ /* notice relationship between mask and shift! */
+
+ return snd_ac97_update_bits(pac97, 0x5c, 0x0020,
+ ucontrol->value.enumerated.item[0] << 5);
+}
+
+
+/* enable disable surround back channel */
+
+static int snd_ac97_vt1618_srbk_info(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_info *uinfo)
+{
+ return ac97_enum_text_info(kcontrol, uinfo, std_enable, 2);
+}
+
+static int snd_ac97_vt1618_srbk_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol);
+
+ ucontrol->value.enumerated.item[0] = (snd_ac97_read(pac97, 0x5c) &
+ 0x0008) >> 3;
+ return 0;
+}
+
+static int snd_ac97_vt1618_srbk_put(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol);
+
+ /* notice relationship between mask and shift! */
+
+ return snd_ac97_update_bits(pac97, 0x5c, 0x0008,
+ ucontrol->value.enumerated.item[0] << 3);
+}
+
+
+/* enable disable soft mute */
+
+static int snd_ac97_vt1618_smute_info(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_info *uinfo)
+{
+ return ac97_enum_text_info(kcontrol, uinfo, std_enable, 2);
+}
+
+static int snd_ac97_vt1618_smute_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol);
+
+ ucontrol->value.enumerated.item[0] = (snd_ac97_read(pac97, 0x5c) &
+ 0x0001);
+ return 0;
+}
+
+static int snd_ac97_vt1618_smute_put(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol);
+
+ /* notice relationship between mask and shift! */
+
+ return snd_ac97_update_bits(pac97, 0x5c, 0x0001,
+ ucontrol->value.enumerated.item[0]);
+}
+
+
+/* config aux in jack - not found on 3 jack motherboards or soundcards */
+
+static int snd_ac97_vt1618_auxin_info(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_info *uinfo)
+{
+ static const char* texts[] = {"Aux In", "Surround Back Out"};
+ return ac97_enum_text_info(kcontrol, uinfo, texts, 2);
+}
+
+static int snd_ac97_vt1618_auxin_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol);
+
+ ucontrol->value.enumerated.item[0] = (snd_ac97_read(pac97, 0x76) &
+ 0x0008) >> 3;
+
+ return 0;
+}
+
+static int snd_ac97_vt1618_auxin_put(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol);
+
+ /* notice relationship between mask and shift! */
+
+ return snd_ac97_update_bits(pac97, 0x76, 0x0008,
+ ucontrol->value.enumerated.item[0] << 3);
+}
+
+
+/*
+ * VIA implements 'Smart 5.1' completely differently on the 1618 than
+ * it does on the 1617a. awesome! They seem to have sourced this
+ * particular revision of the technology from somebody else, it's
+ * called Universal Audio Jack, it shows up on some other folk's chips
+ * as well.
+ *
+ * ordering in this list reflects vt1618 docs for Reg 60h and
+ * the block diagram, DACs are as follows:
+ *
+ * OUT_O -> Front,
+ * OUT_1 -> Surround,
+ * OUT_2 -> C/LFE
+ *
+ * Unlike the 1617a, each OUT has a consistent set of mappings
+ * for all bitpatterns other than 00:
+ *
+ * 01 Unmixed Output
+ * 10 Line In
+ * 11 Mic In
+ *
+ * Special Case of 00:
+ *
+ * OUT_0 Mixed Output
+ * OUT_1 Reserved
+ * OUT_2 Reserved
+ *
+ * I have no idea what the hell Reserved does, but on an MSI
+ * CN700T, i have to set it to get surround output - YMMV, bad
+ * shit may happen.
+ *
+ * If other chips use Universal Audio Jack, then this code might be
+ * applicable to them.
+ */
+
+/* copied from ac97_surround_jack_mode_info() ordering in this list
+ * reflects vt1618 docs for Vendor Defined Register 0x60 */
+
+static int snd_ac97_vt1618_UAJ0_info(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_info *uinfo)
+{
+ static const char* texts[] = { "Speaker Out", "DAC Unmixed Out", "Line
In", "Mic In"};
+ return ac97_enum_text_info(kcontrol, uinfo, texts, 4);
+}
+
+/* UAJ1 and UAJ2 are not supposed to have 00 written to them?? i
+ * dunno, because thats something that i have to do to get 5.1 out to
+ * work. */
+
+static int snd_ac97_vt1618_UAJ1_info(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_info *uinfo)
+{
+ static const char* texts[] = { "Surround Out", "DAC Unmixed Out",
"Line In", "Mic In"};
+ return ac97_enum_text_info(kcontrol, uinfo, texts, 4);
+}
+
+static int snd_ac97_vt1618_UAJ2_info(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_info *uinfo)
+{
+ static const char* texts[] = { "Center/LowFre Out", "DAC Unmixed
Out", "Line In", "Mic In"};
+ return ac97_enum_text_info(kcontrol, uinfo, texts, 4);
+}
+
+/* All of the vt1618 Universal Audio Jack twiddlers are on
+ Vendor Defined Register 0x60, page 0. The bits, and thus
+ the mask, are the only thing that changes */
+
+static int snd_ac97_vt1618_UAJ_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol, unsigned short mask)
+{
+ struct snd_ac97 *pac97;
+ ushort usDatPag, usUAJ, usShift = 0; // 0 is a possible value
+
+ if(0x000C == mask) usShift = 2;
+ if(0x0030 == mask) usShift = 4;
+
+ pac97 = snd_kcontrol_chip(kcontrol);
+
+ mutex_lock(&pac97->page_mutex);
+
+ usDatPag = snd_ac97_read(pac97, AC97_INT_PAGING) & AC97_PAGE_MASK;
+ snd_ac97_update_bits(pac97, AC97_INT_PAGING, AC97_PAGE_MASK, 0);
+
+ usUAJ = snd_ac97_read(pac97, 0x60) & mask;
+
+ snd_ac97_update_bits(pac97, AC97_INT_PAGING, AC97_PAGE_MASK, usDatPag);
+ mutex_unlock(&pac97->page_mutex); // unlock paging
+
+ ucontrol->value.enumerated.item[0] = usUAJ >> usShift;
+
+ return 0;
+}
+
+static int snd_ac97_vt1618_UAJ_put(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol,
+ unsigned short mask)
+{
+ struct snd_ac97 *pac97;
+ ushort usUAJ, usShift = 0; /* 0 is a possible value */
+
+ if(0x000C == mask) usShift = 2;
+ if(0x0030 == mask) usShift = 4;
+
+ pac97 = snd_kcontrol_chip(kcontrol);
+
+ usUAJ = ucontrol->value.enumerated.item[0] << usShift;
+
+ return ac97_update_bits_page(pac97, 0x60, mask, usUAJ, 0);
+}
+
+static int snd_ac97_vt1618_UAJ0_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ return snd_ac97_vt1618_UAJ_get(kcontrol, ucontrol, 0x0003);
+}
+
+static int snd_ac97_vt1618_UAJ1_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ return snd_ac97_vt1618_UAJ_get(kcontrol, ucontrol, 0x000C);
+}
+
+static int snd_ac97_vt1618_UAJ2_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ return snd_ac97_vt1618_UAJ_get(kcontrol, ucontrol, 0x0030);
+}
+
+static int snd_ac97_vt1618_UAJ0_put(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ return snd_ac97_vt1618_UAJ_put(kcontrol, ucontrol, 0x0003);
+}
+
+static int snd_ac97_vt1618_UAJ1_put(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ return snd_ac97_vt1618_UAJ_put(kcontrol, ucontrol, 0x000C);
+}
+
+static int snd_ac97_vt1618_UAJ2_put(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ return snd_ac97_vt1618_UAJ_put(kcontrol, ucontrol, 0x0030);
+}
+
+static const struct snd_kcontrol_new snd_ac97_controls_vt1618[] = {
+
+ {
+ .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+ .name = "Center LFE Exchange",
+ .info = snd_ac97_vt1618_clex_info,
+ .get = snd_ac97_vt1618_clex_get,
+ .put = snd_ac97_vt1618_clex_put
+ },
+ {
+ .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+ .name = "DC Offset",
+ .info = snd_ac97_vt1618_dcof_info,
+ .get = snd_ac97_vt1618_dcof_get,
+ .put = snd_ac97_vt1618_dcof_diff --git a/pci/ac97/ac97_codec.c
b/pci/ac97/ac97_codec.c
index d0023e9..a34f1ea 100644
--- a/pci/ac97/ac97_codec.c
+++ b/pci/ac97/ac97_codec.c
@@ -168,7 +168,7 @@ static const struct ac97_codec_id snd_ac97_codec_ids[]
= {
{ 0x54584e20, 0xffffffff, "TLC320AD9xC", NULL, NULL },
{ 0x56494161, 0xffffffff, "VIA1612A", NULL, NULL }, // modified ICE1232
with S/PDIF
{ 0x56494170, 0xffffffff, "VIA1617A", patch_vt1617a, NULL }, // modified
VT1616 with S/PDIF
-{ 0x56494182, 0xffffffff, "VIA1618", NULL, NULL },
+{ 0x56494182, 0xffffffff, "VIA1618", patch_vt1618, NULL }, // clean
sheet of crinkled paper
{ 0x57454301, 0xffffffff, "W83971D", NULL, NULL },
{ 0x574d4c00, 0xffffffff, "WM9701,WM9701A", NULL, NULL },
{ 0x574d4C03, 0xffffffff, "WM9703,WM9707,WM9708,WM9717", patch_wolfson03,
NULL},
@@ -609,7 +609,6 @@ AC97_SINGLE("PC Speaker Playback Volume",
AC97_PC_BEEP, 1, 15, 1)
static const struct snd_kcontrol_new snd_ac97_controls_mic_boost =
AC97_SINGLE("Mic Boost (+20dB)", AC97_MIC, 6, 1, 0);
-
static const char* std_rec_sel[] = {"Mic", "CD", "Video", "Aux", "Line",
"Mix", "Mix Mono", "Phone"};
static const char* std_3d_path[] = {"pre 3D", "post 3D"};
static const char* std_mix[] = {"Mix", "Mic"};
diff --git a/pci/ac97/ac97_patch.c b/pci/ac97/ac97_patch.c
index bb028f8..2042415 100644
--- a/pci/ac97/ac97_patch.c
+++ b/pci/ac97/ac97_patch.c
@@ -3465,7 +3465,7 @@ static int patch_vt1616(struct snd_ac97 * ac97)
/*
* unfortunately, the vt1617a stashes the twiddlers required for
- * nooding the i/o jacks on 2 different regs. * thameans that we cant
+ * nooding the i/o jacks on 2 different regs. that means that we cant
* use the easy way provided by AC97_ENUM_DOUBLE() we have to write
* are own funcs.
*
@@ -3576,6 +3576,413 @@ int patch_vt1617a(struct snd_ac97 * ac97)
return err;
}
+
+/* use these alot in the 1618 code but i cant find a better place to put
them */
+
+static const char* std_enable[] = {"Enabled", "Disabled"};
+static const char* std_disable[] = {"Disabled","Enabled"};
+
+/* disable enable c/lfe exchange */
+
+static int snd_ac97_vt1618_clex_info(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_info *uinfo)
+{
+ return ac97_enum_text_info(kcontrol, uinfo, std_disable, 2);
+}
+
+static int snd_ac97_vt1618_clex_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol);
+
+ ucontrol->value.enumerated.item[0] = (snd_ac97_read(pac97, 0x5a) &
+ 0x0100) >> 8;
+ return 0;
+}
+
+static int snd_ac97_vt1618_clex_put(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol);
+
+ /* notice relationship between mask and shift! */
+
+ return snd_ac97_update_bits(pac97, 0x5a, 0x0100,
+ ucontrol->value.enumerated.item[0] << 8);
+}
+
+
+/* disable enable dc offset */
+
+static int snd_ac97_vt1618_dcof_info(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_info *uinfo)
+{
+ return ac97_enum_text_info(kcontrol, uinfo, std_disable, 2);
+}
+
+static int snd_ac97_vt1618_dcof_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol);
+
+ ucontrol->value.enumerated.item[0] = (snd_ac97_read(pac97, 0x5a) &
+ 0x0400) >> 10;
+ return 0;
+}
+
+static int snd_ac97_vt1618_dcof_put(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol);
+
+ /* notice relationship between mask and shift! */
+
+ return snd_ac97_update_bits(pac97, 0x5a, 0x0400,
+ ucontrol->value.enumerated.item[0] << 10);
+}
+
+
+/* enable disable headphone amp */
+
+static int snd_ac97_vt1618_hamp_info(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_info *uinfo)
+{
+ return ac97_enum_text_info(kcontrol, uinfo, std_enable, 2);
+}
+
+static int snd_ac97_vt1618_hamp_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol);
+
+ ucontrol->value.enumerated.item[0] = (snd_ac97_read(pac97, 0x5c) &
+ 0x0020) >> 5;
+ return 0;
+}
+
+static int snd_ac97_vt1618_hamp_put(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol);
+
+ /* notice relationship between mask and shift! */
+
+ return snd_ac97_update_bits(pac97, 0x5c, 0x0020,
+ ucontrol->value.enumerated.item[0] << 5);
+}
+
+
+/* enable disable surround back channel */
+
+static int snd_ac97_vt1618_srbk_info(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_info *uinfo)
+{
+ return ac97_enum_text_info(kcontrol, uinfo, std_enable, 2);
+}
+
+static int snd_ac97_vt1618_srbk_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol);
+
+ ucontrol->value.enumerated.item[0] = (snd_ac97_read(pac97, 0x5c) &
+ 0x0008) >> 3;
+ return 0;
+}
+
+static int snd_ac97_vt1618_srbk_put(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol);
+
+ /* notice relationship between mask and shift! */
+
+ return snd_ac97_update_bits(pac97, 0x5c, 0x0008,
+ ucontrol->value.enumerated.item[0] << 3);
+}
+
+
+/* enable disable soft mute */
+
+static int snd_ac97_vt1618_smute_info(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_info *uinfo)
+{
+ return ac97_enum_text_info(kcontrol, uinfo, std_enable, 2);
+}
+
+static int snd_ac97_vt1618_smute_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol);
+
+ ucontrol->value.enumerated.item[0] = (snd_ac97_read(pac97, 0x5c) &
+ 0x0001);
+ return 0;
+}
+
+static int snd_ac97_vt1618_smute_put(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol);
+
+ /* notice relationship between mask and shift! */
+
+ return snd_ac97_update_bits(pac97, 0x5c, 0x0001,
+ ucontrol->value.enumerated.item[0]);
+}
+
+
+/* config aux in jack - not found on 3 jack motherboards or soundcards */
+
+static int snd_ac97_vt1618_auxin_info(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_info *uinfo)
+{
+ static const char* texts[] = {"Aux In", "Surround Back Out"};
+ return ac97_enum_text_info(kcontrol, uinfo, texts, 2);
+}
+
+static int snd_ac97_vt1618_auxin_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol);
+
+ ucontrol->value.enumerated.item[0] = (snd_ac97_read(pac97, 0x76) &
+ 0x0008) >> 3;
+
+ return 0;
+}
+
+static int snd_ac97_vt1618_auxin_put(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol);
+
+ /* notice relationship between mask and shift! */
+
+ return snd_ac97_update_bits(pac97, 0x76, 0x0008,
+ ucontrol->value.enumerated.item[0] << 3);
+}
+
+
+/*
+ * VIA implements 'Smart 5.1' completely differently on the 1618 than
+ * it does on the 1617a. awesome! They seem to have sourced this
+ * particular revision of the technology from somebody else, it's
+ * called Universal Audio Jack, it shows up on some other folk's chips
+ * as well.
+ *
+ * ordering in this list reflects vt1618 docs for Reg 60h and
+ * the block diagram, DACs are as follows:
+ *
+ * OUT_O -> Front,
+ * OUT_1 -> Surround,
+ * OUT_2 -> C/LFE
+ *
+ * Unlike the 1617a, each OUT has a consistent set of mappings
+ * for all bitpatterns other than 00:
+ *
+ * 01 Unmixed Output
+ * 10 Line In
+ * 11 Mic In
+ *
+ * Special Case of 00:
+ *
+ * OUT_0 Mixed Output
+ * OUT_1 Reserved
+ * OUT_2 Reserved
+ *
+ * I have no idea what the hell Reserved does, but on an MSI
+ * CN700T, i have to set it to get surround output - YMMV, bad
+ * shit may happen.
+ *
+ * If other chips use Universal Audio Jack, then this code might be
+ * applicable to them.
+ */
+
+/* copied from ac97_surround_jack_mode_info() ordering in this list
+ * reflects vt1618 docs for Vendor Defined Register 0x60 */
+
+static int snd_ac97_vt1618_UAJ0_info(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_info *uinfo)
+{
+ static const char* texts[] = { "Speaker Out", "DAC Unmixed Out", "Line
In", "Mic In"};
+ return ac97_enum_text_info(kcontrol, uinfo, texts, 4);
+}
+
+/* UAJ1 and UAJ2 are not supposed to have 00 written to them?? i
+ * dunno, because thats something that i have to do to get 5.1 out to
+ * work. */
+
+static int snd_ac97_vt1618_UAJ1_info(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_info *uinfo)
+{
+ static const char* texts[] = { "Surround Out", "DAC Unmixed Out",
"Line In", "Mic In"};
+ return ac97_enum_text_info(kcontrol, uinfo, texts, 4);
+}
+
+static int snd_ac97_vt1618_UAJ2_info(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_info *uinfo)
+{
+ static const char* texts[] = { "Center/LowFre Out", "DAC Unmixed
Out", "Line In", "Mic In"};
+ return ac97_enum_text_info(kcontrol, uinfo, texts, 4);
+}
+
+/* All of the vt1618 Universal Audio Jack twiddlers are on
+ Vendor Defined Register 0x60, page 0. The bits, and thus
+ the mask, are the only thing that changes */
+
+static int snd_ac97_vt1618_UAJ_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol, unsigned short mask)
+{
+ struct snd_ac97 *pac97;
+ ushort usDatPag, usUAJ, usShift = 0; // 0 is a possible value
+
+ if(0x000C == mask) usShift = 2;
+ if(0x0030 == mask) usShift = 4;
+
+ pac97 = snd_kcontrol_chip(kcontrol);
+
+ mutex_lock(&pac97->page_mutex);
+
+ usDatPag = snd_ac97_read(pac97, AC97_INT_PAGING) & AC97_PAGE_MASK;
+ snd_ac97_update_bits(pac97, AC97_INT_PAGING, AC97_PAGE_MASK, 0);
+
+ usUAJ = snd_ac97_read(pac97, 0x60) & mask;
+
+ snd_ac97_update_bits(pac97, AC97_INT_PAGING, AC97_PAGE_MASK, usDatPag);
+ mutex_unlock(&pac97->page_mutex); // unlock paging
+
+ ucontrol->value.enumerated.item[0] = usUAJ >> usShift;
+
+ return 0;
+}
+
+static int snd_ac97_vt1618_UAJ_put(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol,
+ unsigned short mask)
+{
+ struct snd_ac97 *pac97;
+ ushort usUAJ, usShift = 0; /* 0 is a possible value */
+
+ if(0x000C == mask) usShift = 2;
+ if(0x0030 == mask) usShift = 4;
+
+ pac97 = snd_kcontrol_chip(kcontrol);
+
+ usUAJ = ucontrol->value.enumerated.item[0] << usShift;
+
+ return ac97_update_bits_page(pac97, 0x60, mask, usUAJ, 0);
+}
+
+static int snd_ac97_vt1618_UAJ0_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ return snd_ac97_vt1618_UAJ_get(kcontrol, ucontrol, 0x0003);
+}
+
+static int snd_ac97_vt1618_UAJ1_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ return snd_ac97_vt1618_UAJ_get(kcontrol, ucontrol, 0x000C);
+}
+
+static int snd_ac97_vt1618_UAJ2_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ return snd_ac97_vt1618_UAJ_get(kcontrol, ucontrol, 0x0030);
+}
+
+static int snd_ac97_vt1618_UAJ0_put(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ return snd_ac97_vt1618_UAJ_put(kcontrol, ucontrol, 0x0003);
+}
+
+static int snd_ac97_vt1618_UAJ1_put(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ return snd_ac97_vt1618_UAJ_put(kcontrol, ucontrol, 0x000C);
+}
+
+static int snd_ac97_vt1618_UAJ2_put(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ return snd_ac97_vt1618_UAJ_put(kcontrol, ucontrol, 0x0030);
+}
+
+static const struct snd_kcontrol_new snd_ac97_controls_vt1618[] = {
+
+ {
+ .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+ .name = "Center LFE Exchange",
+ .info = snd_ac97_vt1618_clex_info,
+ .get = snd_ac97_vt1618_clex_get,
+ .put = snd_ac97_vt1618_clex_put
+ },
+ {
+ .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+ .name = "DC Offset",
+ .info = snd_ac97_vt1618_dcof_info,
+ .get = snd_ac97_vt1618_dcof_get,
+ .put = snd_ac97_vt1618_dcof_put
+ },
+ {
+ .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+ .name = "Headphone Amp",
+ .info = snd_ac97_vt1618_hamp_info,
+ .get = snd_ac97_vt1618_hamp_get,
+ .put = snd_ac97_vt1618_hamp_put
+ },
+ {
+ .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+ .name = "Surround Back",
+ .info = snd_ac97_vt1618_srbk_info,
+ .get = snd_ac97_vt1618_srbk_get,
+ .put = snd_ac97_vt1618_srbk_put
+ },
+ {
+ .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+ .name = "Soft Mute",
+ .info = snd_ac97_vt1618_smute_info,
+ .get = snd_ac97_vt1618_smute_get,
+ .put = snd_ac97_vt1618_smute_put
+ },
+ {
+ .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+ .name = "Aux In Jack",
+ .info = snd_ac97_vt1618_auxin_info,
+ .get = snd_ac97_vt1618_auxin_get,
+ .put = snd_ac97_vt1618_auxin_put
+ },
+ {
+ .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+ .name = "Speaker Jack",
+ .info = snd_ac97_vt1618_UAJ0_info,
+ .get = snd_ac97_vt1618_UAJ0_get,
+ .put = snd_ac97_vt1618_UAJ0_put
+ },
+ {
+ .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+ .name = "Line In Jack",
+ .info = snd_ac97_vt1618_UAJ1_info,
+ .get = snd_ac97_vt1618_UAJ1_get,
+ .put = snd_ac97_vt1618_UAJ1_put
+ },
+ {
+ .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+ .name = "Mic In Jack",
+ .info = snd_ac97_vt1618_UAJ2_info,
+ .get = snd_ac97_vt1618_UAJ2_get,
+ .put = snd_ac97_vt1618_UAJ2_put
+ }
+};
+
+int patch_vt1618(struct snd_ac97 *ac97)
+{
+ return patch_build_controls(ac97, snd_ac97_controls_vt1618,
+ ARRAY_SIZE(snd_ac97_controls_vt1618));
+}
+
+
/*
*/
static void it2646_update_jacks(struct snd_ac97 *ac97)
put
+ },
+ {
+ .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+ .name = "Headphone Amp",
+ .info = snd_ac97_vt1618_hamp_info,
+ .get = snd_ac97_vt1618_hamp_get,
+ .put = snd_ac97_vt1618_hamp_put
+ },
+ {
+ .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+ .name = "Surround Back",
+ .info = snd_ac97_vt1618_srbk_info,
+ .get = snd_ac97_vt1618_srbk_get,
+ .put = snd_ac97_vt1618_srbk_put
+ },
+ {
+ .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+ .name = "Soft Mute",
+ .info = snd_ac97_vt1618_smute_info,
+ .get = snd_ac97_vt1618_smute_get,
+ .put = snd_ac97_vt1618_smute_put
+ },
+ {
+ .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+ .name = "Aux In Jack",
+ .info = snd_ac97_vt1618_auxin_info,
+ .get = snd_ac97_vt1618_auxin_get,
+ .put = snd_ac97_vt1618_auxin_put
+ },
+ {
+ .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+ .name = "Speaker Jack",
+ .info = snd_ac97_vt1618_UAJ0_info,
+ .get = snd_ac97_vt1618_UAJ0_get,
+ .put = snd_ac97_vt1618_UAJ0_put
+ },
+ {
+ .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+ .name = "Line In Jack",
+ .info = snd_ac97_vt1618_UAJ1_info,
+ .get = snd_ac97_vt1618_UAJ1_get,
+ .put = snd_ac97_vt1618_UAJ1_put
+ },
+ {
+ .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+ .name = "Mic In Jack",
+ .info = snd_ac97_vt1618_UAJ2_info,
+ .get = snd_ac97_vt1618_UAJ2_get,
+ .put = snd_ac97_vt1618_UAJ2_put
+ }
+};
+
+int patch_vt1618(struct snd_ac97 *ac97)
+{
+ return patch_build_controls(ac97, snd_ac97_controls_vt1618,
+ ARRAY_SIZE(snd_ac97_controls_vt1618));
+}
+
+
/*
*/
static void it2646_update_jacks(struct snd_ac97 *ac97)
+ {
+ .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+ .name = "Headphone Amp",
+ .info = snd_ac97_vt1618_hamp_info,
+ .get = snd_ac97_vt1618_hamp_get,
+ .put = snd_ac97_vt1618_hamp_put
+ },
+ {
+ .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+ .name = "Surround Back",
+ .info = snd_ac97_vt1618_srbk_info,
+ .get = snd_ac97_vt1618_srbk_get,
+ .put = snd_ac97_vt1618_srbk_put
+ },
+ {
+ .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+ .name = "Soft Mute",
+ .info = snd_ac97_vt1618_smute_info,
+ .get = snd_ac97_vt1618_smute_get,
+ .put = snd_ac97_vt1618_smute_put
+ },
+ {
+ .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+ .name = "Aux In Jack",
+ .info = snd_ac97_vt1618_auxin_info,
+ .get = snd_ac97_vt1618_auxin_get,
+ .put = snd_ac97_vt1618_auxin_put
+ },
+ {
+ .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+ .name = "Speaker Jack",
+ .info = snd_ac97_vt1618_UAJ0_info,
+ .get = snd_ac97_vt1618_UAJ0_get,
+ .put = snd_ac97_vt1618_UAJ0_put
+ },
+ {
+ .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+ .name = "Line In Jack",
+ .info = snd_ac97_vt1618_UAJ1_info,
+ .get = snd_ac97_vt1618_UAJ1_get,
+ .put = snd_ac97_vt1618_UAJ1_put
+ },
+ {
+ .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+ .name = "Mic In Jack",
+ .info = snd_ac97_vt1618_UAJ2_info,
+ .get = snd_ac97_vt1618_UAJ2_get,
+ .put = snd_ac97_vt1618_UAJ2_put
+ }
+};
+
+int patch_vt1618(struct snd_ac97 *ac97)
+{
+ return patch_build_controls(ac97, snd_ac97_controls_vt1618,
+ ARRAY_SIZE(snd_ac97_controls_vt1618));
+}
+
+
/*
*/
static void it2646_update_jacks(struct snd_ac97 *ac97)
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: PATCH - vt1618 7.1 Audio
2008-08-20 0:17 PATCH - vt1618 7.1 Audio John L. Utz III
@ 2008-08-20 8:28 ` Takashi Iwai
2008-08-20 17:50 ` John L. Utz III
0 siblings, 1 reply; 9+ messages in thread
From: Takashi Iwai @ 2008-08-20 8:28 UTC (permalink / raw)
To: John L. Utz III; +Cc: ALSA Developers
At Tue, 19 Aug 2008 17:17:44 -0700,
John L. Utz III wrote:
>
> Signed-off-by: John L. Utz III john.utz@dmx.com
Thanks for the patch.
First off, could you give a brief description what this patch
does/fixes exactly?
Below is a quick review.
> diff --git a/pci/ac97/ac97_codec.c b/pci/ac97/ac97_codec.c
> index d0023e9..a34f1ea 100644
> --- a/pci/ac97/ac97_codec.c
> +++ b/pci/ac97/ac97_codec.c
> @@ -168,7 +168,7 @@ static const struct ac97_codec_id snd_ac97_codec_ids[]
> = {
> { 0x54584e20, 0xffffffff, "TLC320AD9xC", NULL, NULL },
> { 0x56494161, 0xffffffff, "VIA1612A", NULL, NULL }, // modified ICE1232
> with S/PDIF
> { 0x56494170, 0xffffffff, "VIA1617A", patch_vt1617a, NULL }, // modified
> VT1616 with S/PDIF
> -{ 0x56494182, 0xffffffff, "VIA1618", NULL, NULL },
> +{ 0x56494182, 0xffffffff, "VIA1618", patch_vt1618, NULL }, // clean
Please avoid C++ comments. Yeah, we see it in other places here and
there, but you don't need to introduce more in the newly added code.
> +
> +/* use these alot in the 1618 code but i cant find a better place to put
> them */
> +
> +static const char* std_enable[] = {"Enabled", "Disabled"};
> +static const char* std_disable[] = {"Disabled","Enabled"};
They must be consistent -- usually 0 = disable, 1 = enable.
In general, the controls like "XXX Disable" is bad. A switch should
be to turn on, not to turn off. If the register bit is to disable
something, implement the control element in a reverse way.
Also, you can consider to implement it as a switch, not as an enum,
depending on the role. Then it'll be often better handled in many
mixer apps.
The rest are mostly about coding styles.
> +static int snd_ac97_vt1618_UAJ0_info(struct snd_kcontrol *kcontrol,
> + struct snd_ctl_elem_info *uinfo)
> +{
> + static const char* texts[] = { "Speaker Out", "DAC Unmixed Out", "Line
> In", "Mic In"};
Avoid too long lines (also in other places).
> +{
> + struct snd_ac97 *pac97;
> + ushort usDatPag, usUAJ, usShift = 0; // 0 is a possible value
ushort isn't a standard type. Use either unsigned short or u16 if it
must be 16bit.
Also, a Windows style variable name should be avoided. People tend to
hate it.
> + if(0x000C == mask) usShift = 2;
> + if(0x0030 == mask) usShift = 4;
Fix coding style please.
> +
> + pac97 = snd_kcontrol_chip(kcontrol);
> + mutex_lock(&pac97->page_mutex);
Keep to use tabs instead of space letters (found in other places).
> +static int snd_ac97_vt1618_UAJ0_get(struct snd_kcontrol *kcontrol,
> + struct snd_ctl_elem_value *ucontrol)
> +{
> + return snd_ac97_vt1618_UAJ_get(kcontrol, ucontrol, 0x0003);
> +}
> +
> +static int snd_ac97_vt1618_UAJ1_get(struct snd_kcontrol *kcontrol,
> + struct snd_ctl_elem_value *ucontrol)
> +{
> + return snd_ac97_vt1618_UAJ_get(kcontrol, ucontrol, 0x000C);
> +}
You can use private_value field in each definition of the control,
so that you don't have to create each different function.
About the coding style, I recommend to run checkpatch.pl script once
before reposting your patch. The script is found in scripts directory
of the recent linux-kernel source tree.
Takashi
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: PATCH - vt1618 7.1 Audio
2008-08-20 8:28 ` Takashi Iwai
@ 2008-08-20 17:50 ` John L. Utz III
2008-08-20 19:05 ` Rene Herman
2008-08-20 19:21 ` Mark Brown
0 siblings, 2 replies; 9+ messages in thread
From: John L. Utz III @ 2008-08-20 17:50 UTC (permalink / raw)
To: Takashi Iwai; +Cc: ALSA Developers
Hi Takashi;
tnx for the review, a few questions inline.
On Wed, 20 Aug 2008 01:28:39 -0700, Takashi Iwai <tiwai@suse.de> wrote:
> At Tue, 19 Aug 2008 17:17:44 -0700,
> John L. Utz III wrote:
>>
>> Signed-off-by: John L. Utz III john.utz@dmx.com
>
> Thanks for the patch.
>
> First off, could you give a brief description what this patch
> does/fixes exactly?
>
> Below is a quick review.
>
>> diff --git a/pci/ac97/ac97_codec.c b/pci/ac97/ac97_codec.c
>> index d0023e9..a34f1ea 100644
>> --- a/pci/ac97/ac97_codec.c
>> +++ b/pci/ac97/ac97_codec.c
>> @@ -168,7 +168,7 @@ static const struct ac97_codec_id
>> snd_ac97_codec_ids[]
>> = {
>> { 0x54584e20, 0xffffffff, "TLC320AD9xC", NULL, NULL },
>> { 0x56494161, 0xffffffff, "VIA1612A", NULL, NULL }, // modified
>> ICE1232
>> with S/PDIF
>> { 0x56494170, 0xffffffff, "VIA1617A", patch_vt1617a, NULL }, //
>> modified
>> VT1616 with S/PDIF
>> -{ 0x56494182, 0xffffffff, "VIA1618", NULL, NULL },
>> +{ 0x56494182, 0xffffffff, "VIA1618", patch_vt1618, NULL }, // clean
>
> Please avoid C++ comments. Yeah, we see it in other places here and
> there, but you don't need to introduce more in the newly added code.
>
OK. takes more space tho.
>> +
>> +/* use these alot in the 1618 code but i cant find a better place to
>> put
>> them */
>> +
>> +static const char* std_enable[] = {"Enabled", "Disabled"};
>> +static const char* std_disable[] = {"Disabled","Enabled"};
>
> They must be consistent -- usually 0 = disable, 1 = enable.
> In general, the controls like "XXX Disable" is bad. A switch should
> be to turn on, not to turn off. If the register bit is to disable
> something, implement the control element in a reverse way.
OK.
note that it makes the code more complex in some places because the chip
is inconsistent.
> Also, you can consider to implement it as a switch, not as an enum,
> depending on the role. Then it'll be often better handled in many
> mixer apps.
>
> The rest are mostly about coding styles.
>
>> +static int snd_ac97_vt1618_UAJ0_info(struct snd_kcontrol *kcontrol,
>> + struct snd_ctl_elem_info *uinfo)
>> +{
>> + static const char* texts[] = { "Speaker Out", "DAC Unmixed Out", "Line
>> In", "Mic In"};
>
> Avoid too long lines (also in other places).
OK. 80 column?
>> +{
>> + struct snd_ac97 *pac97;
>> + ushort usDatPag, usUAJ, usShift = 0; // 0 is a possible value
>
> ushort isn't a standard type. Use either unsigned short or u16 if it
> must be 16bit.
> Also, a Windows style variable name should be avoided. People tend to
> hate it.
Why would you call this 'Windows Style?' Is that supposed to be a
perjorative comment?
The code compiled with gcc and creates a linux kernel module and I am
using Gentoo. Wouldnt that then make it reasonable to call it 'Linux
Style' or 'GPL Style' or 'Gentoo Style? or 'John Utz Style' ?
Having variable type decorations provides valuable context and (IMHO)
makes the code more maintainable.
>> + if(0x000C == mask) usShift = 2;
>> + if(0x0030 == mask) usShift = 4;
>
> Fix coding style please.
specifics please. wait. nevermind, i'll run checkpatch against it. i
forgot to do that last nite. :-)
>> +
>> + pac97 = snd_kcontrol_chip(kcontrol);
>> + mutex_lock(&pac97->page_mutex);
>
> Keep to use tabs instead of space letters (found in other places).
OK
>> +static int snd_ac97_vt1618_UAJ0_get(struct snd_kcontrol *kcontrol,
>> + struct snd_ctl_elem_value *ucontrol)
>> +{
>> + return snd_ac97_vt1618_UAJ_get(kcontrol, ucontrol, 0x0003);
>> +}
>> +
>> +static int snd_ac97_vt1618_UAJ1_get(struct snd_kcontrol *kcontrol,
>> + struct snd_ctl_elem_value *ucontrol)
>> +{
>> + return snd_ac97_vt1618_UAJ_get(kcontrol, ucontrol, 0x000C);
>> +}
>
> You can use private_value field in each definition of the control,
> so that you don't have to create each different function.
OH, that would be AWESOME. Can you elaborate? I'll go grep for private
value in the existing code and see if i can find an example whilst i wait
for your explanation.
> About the coding style, I recommend to run checkpatch.pl script once
> before reposting your patch. The script is found in scripts directory
> of the recent linux-kernel source tree.
yes. i realized as i was walking to the bus last nite that i had forgotten
to do so.
johnu
> Takashi
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: PATCH - vt1618 7.1 Audio
2008-08-20 17:50 ` John L. Utz III
@ 2008-08-20 19:05 ` Rene Herman
2008-08-20 19:16 ` John L. Utz III
2008-08-20 19:21 ` Mark Brown
1 sibling, 1 reply; 9+ messages in thread
From: Rene Herman @ 2008-08-20 19:05 UTC (permalink / raw)
To: John L. Utz III; +Cc: Takashi Iwai, ALSA Developers
On 20-08-08 19:50, John L. Utz III wrote:
> On Wed, 20 Aug 2008 01:28:39 -0700, Takashi Iwai <tiwai@suse.de> wrote:
>> Also, a Windows style variable name should be avoided. People tend to
>> hate it.
>
> Why would you call this 'Windows Style?' Is that supposed to be a
> perjorative comment?
You insult really quickly... :-)
The style is more generally known as Hungarian Notation after its
Hungarian "inventor" who was a chief architect at Microsoft. This
notational conventation saw widespread use in the Windows API and not
much use anywhere beyond that.
Rene.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: PATCH - vt1618 7.1 Audio
2008-08-20 19:05 ` Rene Herman
@ 2008-08-20 19:16 ` John L. Utz III
2008-08-20 19:54 ` Rene Herman
0 siblings, 1 reply; 9+ messages in thread
From: John L. Utz III @ 2008-08-20 19:16 UTC (permalink / raw)
To: Rene Herman; +Cc: Takashi Iwai, ALSA Developers
On Wed, 20 Aug 2008 12:05:33 -0700, Rene Herman <rene.herman@keyaccess.nl>
wrote:
> On 20-08-08 19:50, John L. Utz III wrote:
>
>> On Wed, 20 Aug 2008 01:28:39 -0700, Takashi Iwai <tiwai@suse.de> wrote:
>
>>> Also, a Windows style variable name should be avoided. People tend to
>>> hate it.
>> Why would you call this 'Windows Style?' Is that supposed to be a
>> perjorative comment?
>
> You insult really quickly... :-)
Quickly, but not deeply. :-)
>
> The style is more generally known as Hungarian Notation after its
> Hungarian "inventor" who was a chief architect at Microsoft.
Yuppers. I know. Charles Simonyi. I really, really, like it.
> This notational conventation saw widespread use in the Windows API and
> not much use anywhere beyond that.
with an example of 'beyond that' being John Utz's code.
Be it C, Java, C#, Javascript, Ada or any languages that use strong types.
I dont do it in shell script tho:
# Let any boot observers know what devices we found
echo "Type MotherBoard: ($TYPEMBD)"
echo "Type Soundcard: ($TYPESND)"
echo "Type Ethenet: ($TYPEETH)"
echo "Type Modem: ($TYPEMDM)"
anyway, we can argue about it again after i get a checkpatch compliant
patch to submit....
johnu
> Rene.
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: PATCH - vt1618 7.1 Audio
2008-08-20 17:50 ` John L. Utz III
2008-08-20 19:05 ` Rene Herman
@ 2008-08-20 19:21 ` Mark Brown
1 sibling, 0 replies; 9+ messages in thread
From: Mark Brown @ 2008-08-20 19:21 UTC (permalink / raw)
To: John L. Utz III; +Cc: Takashi Iwai, ALSA Developers
On Wed, Aug 20, 2008 at 10:50:49AM -0700, John L. Utz III wrote:
> On Wed, 20 Aug 2008 01:28:39 -0700, Takashi Iwai <tiwai@suse.de> wrote:
> > They must be consistent -- usually 0 = disable, 1 = enable.
> > In general, the controls like "XXX Disable" is bad. A switch should
> > be to turn on, not to turn off. If the register bit is to disable
> > something, implement the control element in a reverse way.
> OK.
> note that it makes the code more complex in some places because the chip
> is inconsistent.
ASoC does this by having a flag in the control specific data saying if
the control is inverted which works pretty well for dealing with this
situation.
> > Avoid too long lines (also in other places).
> OK. 80 column?
Yes. See Documentation/CodingStyle (and checkpatch).
> > Also, a Windows style variable name should be avoided. People tend to
> > hate it.
> Why would you call this 'Windows Style?' Is that supposed to be a
> perjorative comment?
This style is used throughout the Windows API documentation and is
therefore frequently encounterd in Windows code but it's rarely
encountered anywhere else. It's called Hungarian notation, though in
the form it's normally seen it's very far from what was originally
intended by that.
> Having variable type decorations provides valuable context and (IMHO)
> makes the code more maintainable.
Many people have forcefuly held views in the opposite direction - google
will turn up a bunch. I have the following in my fortune database:
prepAs nounOthers verbHave verbNoted, pronThis nounStyle verbIs
verbCalled adjHungarian nounNotation. pronI verbFind pronIt
advCompletely adjUseless. pronI verbHope pronYou verbCan verbSee
possMy nounPoint -- verbEspecially nounWhen possThe advPrefixes
conjBecome verbUtterly adjWrong. :-)
-- Chris Torek <nospam@elf.eng.bsdi.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: PATCH - vt1618 7.1 Audio
2008-08-20 19:16 ` John L. Utz III
@ 2008-08-20 19:54 ` Rene Herman
2008-08-20 20:50 ` John L. Utz III
0 siblings, 1 reply; 9+ messages in thread
From: Rene Herman @ 2008-08-20 19:54 UTC (permalink / raw)
To: John L. Utz III; +Cc: Takashi Iwai, ALSA Developers
On 20-08-08 21:16, John L. Utz III wrote:
> anyway, we can argue about it again after i get a checkpatch
> compliant patch to submit....
Let's get a headstart...
Note that Hungarian is not a private Takashi Iwai or ALSA dislike; it's
rather specifically against the Linux kernel coding style (although the
actual CodingStyle document only mentions Hungarian for function names).
Most contributors have had to adjust a few local habits and in the end,
it's generally worth it. If all is good and well, open source code is
read many more times than it's written after all and any (significant)
style inconsistency costs every reader just a little mental hickup.
Review resources are one of the more scarcely available ones in this
development process, so a consistent style definitely pays.
Sure, sure, I also try ty get away with things, but Hungarians hickup
rather loudly.
Rene.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: PATCH - vt1618 7.1 Audio
2008-08-20 19:54 ` Rene Herman
@ 2008-08-20 20:50 ` John L. Utz III
2008-08-20 21:00 ` Rene Herman
0 siblings, 1 reply; 9+ messages in thread
From: John L. Utz III @ 2008-08-20 20:50 UTC (permalink / raw)
To: Rene Herman; +Cc: Takashi Iwai, ALSA Developers
On Wed, 20 Aug 2008 12:54:23 -0700, Rene Herman <rene.herman@keyaccess.nl>
wrote:
> On 20-08-08 21:16, John L. Utz III wrote:
>
>> anyway, we can argue about it again after i get a checkpatch
>> compliant patch to submit....
>
> Let's get a headstart...
hokydokysmoky!
> Note that Hungarian is not a private Takashi Iwai or ALSA dislike; it's
> rather specifically against the Linux kernel coding style (although the
> actual CodingStyle document only mentions Hungarian for function names).
'rather specifically' is your interpretation, not a fact.
CodingStyle.C4 just asks that naming be 'Spartan'.
So, to map 'Spartan' to my code:
this is an unsigned short variable that contains a value that will be used
to shift another variable:
usigned short usShift;
ucontrol->value.enumerated.item[0] << usShift;
IMHO, this is a legitimate name in the context of CodingStyle.C4
> Most contributors have had to adjust a few local habits and in the end,
> it's generally worth it.
right.
'tis why i didnt submit something with a 2 space indent. :-)
`tis why i left my leading 'p's out of the function argument signatures.
> If all is good and well, open source code is read many more times than
> it's
> written after all and any (significant) style inconsistency costs every
> reader
> just a little mental hickup.
right. but we differ on what is an inconsistency.
> Review resources are one of the more scarcely available ones in this
> development process, so a consistent style definitely pays.
>
> Sure, sure, I also try ty get away with things, but Hungarians hickup
> rather loudly.
I think that folks might be really surprised how helpful hungarian
notation can be.
But this is a team effort and i am not gonna get all mulish about it
because it's a very silly thing to be mulish about.
If Takashi tells me that he rejects the next version of the patch solely
due to the naming of my variables then i will change them.
But if he isnt going to reject it on that basis then they will stay the
way they are.
It is open source after all. Somebody could submit a patch to change them
to something they like better.
> Rene.
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: PATCH - vt1618 7.1 Audio
2008-08-20 20:50 ` John L. Utz III
@ 2008-08-20 21:00 ` Rene Herman
0 siblings, 0 replies; 9+ messages in thread
From: Rene Herman @ 2008-08-20 21:00 UTC (permalink / raw)
To: John L. Utz III; +Cc: Takashi Iwai, ALSA Developers
On 20-08-08 22:50, John L. Utz III wrote:
> On Wed, 20 Aug 2008 12:54:23 -0700, Rene Herman
> <rene.herman@keyaccess.nl> wrote:
>> Note that Hungarian is not a private Takashi Iwai or ALSA dislike;
>> it's rather specifically against the Linux kernel coding style
>> (although the actual CodingStyle document only mentions Hungarian for
>> function names).
>
> 'rather specifically' is your interpretation, not a fact.
No it's not my interpretation. It's based on many years of reading the
linux kernel mailing list and seing patches being rejected over things
like it for example. Also ssee the rest of the tree.
As to being surprised, I wouldn't be, having used Hungarian for years in
Windows programming.
Rene.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2008-08-20 20:59 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-08-20 0:17 PATCH - vt1618 7.1 Audio John L. Utz III
2008-08-20 8:28 ` Takashi Iwai
2008-08-20 17:50 ` John L. Utz III
2008-08-20 19:05 ` Rene Herman
2008-08-20 19:16 ` John L. Utz III
2008-08-20 19:54 ` Rene Herman
2008-08-20 20:50 ` John L. Utz III
2008-08-20 21:00 ` Rene Herman
2008-08-20 19:21 ` Mark Brown
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.