* [PATCH] ALSA: hda/cm9825: Add new project GENE_TWL7 for AAEON
@ 2025-12-12 9:31 Leo Tsai
2025-12-14 10:30 ` Takashi Iwai
0 siblings, 1 reply; 6+ messages in thread
From: Leo Tsai @ 2025-12-12 9:31 UTC (permalink / raw)
To: perex, tiwai, rf; +Cc: leo.tsai, linux-sound, linux-kernel, Leo Tsai
Update CM9825 to support GENE_TWL7 which supports Line-out, Line-in, and Mic-in.
Signed-off-by: Leo Tsai <antivirus621@gmail.com>
---
sound/hda/codecs/cm9825.c | 219 ++++++++++++++++++++++++++++++++++++--
1 file changed, 210 insertions(+), 9 deletions(-)
diff --git a/sound/hda/codecs/cm9825.c b/sound/hda/codecs/cm9825.c
index 5c474ce44348..1c08cef8050d 100644
--- a/sound/hda/codecs/cm9825.c
+++ b/sound/hda/codecs/cm9825.c
@@ -13,6 +13,11 @@
#include "hda_jack.h"
#include "generic.h"
+enum {
+ QUIRK_CM_STD = 0x0,
+ QUIRK_GENE_TWL7_SSID = 0x160dc000
+};
+
/* CM9825 Offset Definitions */
#define CM9825_VERB_SET_HPF_1 0x781
@@ -25,6 +30,7 @@
#define CM9825_VERB_SET_VNEG 0x7a8
#define CM9825_VERB_SET_D2S 0x7a9
#define CM9825_VERB_SET_DACTRL 0x7aa
+#define CM9825_VERB_SET_P3BCP 0x7ab
#define CM9825_VERB_SET_PDNEG 0x7ac
#define CM9825_VERB_SET_VDO 0x7ad
#define CM9825_VERB_SET_CDALR 0x7b0
@@ -42,6 +48,7 @@ struct cmi_spec {
const struct hda_verb *chip_hp_present_verbs;
const struct hda_verb *chip_hp_remove_verbs;
struct hda_codec *codec;
+ struct delayed_work unsol_line_work;
struct delayed_work unsol_hp_work;
int quirk;
};
@@ -80,10 +87,8 @@ static const struct hda_verb cm9825_std_d0_verbs[] = {
{0x43, CM9825_VERB_SET_OCP, 0x33}, /* OTP set */
{0x43, CM9825_VERB_SET_GAD, 0x07}, /* ADC -3db */
{0x43, CM9825_VERB_SET_TMOD, 0x26}, /* Class D clk */
- {0x3C, AC_VERB_SET_AMP_GAIN_MUTE |
- AC_AMP_SET_OUTPUT | AC_AMP_SET_RIGHT, 0x2d}, /* Gain set */
- {0x3C, AC_VERB_SET_AMP_GAIN_MUTE |
- AC_AMP_SET_OUTPUT | AC_AMP_SET_LEFT, 0x2d}, /* Gain set */
+ {0x3c, AC_VERB_SET_AMP_GAIN_MUTE | 0xa0, 0x2d}, /* Gain set */
+ {0x3c, AC_VERB_SET_AMP_GAIN_MUTE | 0x90, 0x2d}, /* Gain set */
{0x43, CM9825_VERB_SET_HPF_1, 0x40}, /* HPF set */
{0x43, CM9825_VERB_SET_HPF_2, 0x40}, /* HPF set */
{}
@@ -111,6 +116,60 @@ static const struct hda_verb cm9825_hp_remove_verbs[] = {
{}
};
+static const struct hda_verb cm9825_gene_twl7_d3_verbs[] = {
+ {0x43, CM9825_VERB_SET_D2S, 0x62},
+ {0x43, CM9825_VERB_SET_PLL, 0x01},
+ {0x43, CM9825_VERB_SET_NEG, 0xc2},
+ {0x43, CM9825_VERB_SET_ADCL, 0x00},
+ {0x43, CM9825_VERB_SET_DACL, 0x02},
+ {0x43, CM9825_VERB_SET_MBIAS, 0x00},
+ {0x43, CM9825_VERB_SET_VNEG, 0x50},
+ {0x43, CM9825_VERB_SET_PDNEG, 0x04},
+ {0x43, CM9825_VERB_SET_CDALR, 0xf6},
+ {0x43, CM9825_VERB_SET_OTP, 0xcd},
+ {}
+};
+
+static const struct hda_verb cm9825_gene_twl7_d0_verbs[] = {
+ {0x34, AC_VERB_SET_EAPD_BTLENABLE, 0x02},
+ {0x43, CM9825_VERB_SET_SNR, 0x38},
+ {0x43, CM9825_VERB_SET_PLL, 0x00},
+ {0x43, CM9825_VERB_SET_ADCL, 0xcf},
+ {0x43, CM9825_VERB_SET_DACL, 0xaa},
+ {0x43, CM9825_VERB_SET_MBIAS, 0x1c},
+ {0x43, CM9825_VERB_SET_VNEG, 0x56},
+ {0x43, CM9825_VERB_SET_D2S, 0x62},
+ {0x43, CM9825_VERB_SET_DACTRL, 0x00},
+ {0x43, CM9825_VERB_SET_PDNEG, 0x0c},
+ {0x43, CM9825_VERB_SET_CDALR, 0xf4},
+ {0x43, CM9825_VERB_SET_OTP, 0xcd},
+ {0x43, CM9825_VERB_SET_MTCBA, 0x61},
+ {0x43, CM9825_VERB_SET_OCP, 0x33},
+ {0x43, CM9825_VERB_SET_GAD, 0x07},
+ {0x43, CM9825_VERB_SET_TMOD, 0x26},
+ {0x43, CM9825_VERB_SET_HPF_1, 0x40},
+ {0x43, CM9825_VERB_SET_HPF_2, 0x40},
+ {0x40, AC_VERB_SET_CONNECT_SEL, 0x00},
+ {0x3d, AC_VERB_SET_CONNECT_SEL, 0x01},
+ {0x46, CM9825_VERB_SET_P3BCP, 0x20},
+ {}
+};
+
+static const struct hda_verb cm9825_gene_twl7_playback_start_verbs[] = {
+ {0x43, CM9825_VERB_SET_D2S, 0xf2},
+ {0x43, CM9825_VERB_SET_VDO, 0xd4},
+ {0x43, CM9825_VERB_SET_SNR, 0x30},
+ {}
+};
+
+static const struct hda_verb cm9825_gene_twl7_playback_stop_verbs[] = {
+ {0x43, CM9825_VERB_SET_VDO, 0xc0},
+ {0x43, CM9825_VERB_SET_D2S, 0x62},
+ {0x43, CM9825_VERB_SET_VDO, 0xd0},
+ {0x43, CM9825_VERB_SET_SNR, 0x38},
+ {}
+};
+
static void cm9825_unsol_hp_delayed(struct work_struct *work)
{
struct cmi_spec *spec =
@@ -120,6 +179,9 @@ static void cm9825_unsol_hp_delayed(struct work_struct *work)
bool hp_jack_plugin = false;
int err = 0;
+ if (spec->codec->core.subsystem_id == QUIRK_GENE_TWL7_SSID)
+ hp_pin = 0x36;
+
hp_jack_plugin = snd_hda_jack_detect(spec->codec, hp_pin);
codec_dbg(spec->codec, "hp_jack_plugin %d, hp_pin 0x%X\n",
@@ -132,10 +194,13 @@ static void cm9825_unsol_hp_delayed(struct work_struct *work)
if (err)
codec_dbg(spec->codec, "codec_write err %d\n", err);
- snd_hda_sequence_write(spec->codec, spec->chip_hp_remove_verbs);
+ if (spec->codec->core.subsystem_id == QUIRK_CM_STD)
+ snd_hda_sequence_write(spec->codec,
+ spec->chip_hp_remove_verbs);
} else {
- snd_hda_sequence_write(spec->codec,
- spec->chip_hp_present_verbs);
+ if (spec->codec->core.subsystem_id == QUIRK_CM_STD)
+ snd_hda_sequence_write(spec->codec,
+ spec->chip_hp_present_verbs);
}
jack = snd_hda_jack_tbl_get(spec->codec, hp_pin);
@@ -162,13 +227,104 @@ static void hp_callback(struct hda_codec *codec, struct hda_jack_callback *cb)
schedule_delayed_work(&spec->unsol_hp_work, msecs_to_jiffies(200));
}
+static void cm9825_unsol_line_delayed(struct work_struct *work)
+{
+ struct cmi_spec *spec =
+ container_of(to_delayed_work(work), struct cmi_spec,
+ unsol_line_work);
+ struct hda_jack_tbl *jack;
+ hda_nid_t lineout_pin = 0x3b;
+ bool lineout_jack_plugin = false;
+
+ lineout_jack_plugin = snd_hda_jack_detect(spec->codec, 0x3b);
+
+ codec_dbg(spec->codec,
+ "%s, lineout_jack_plugin %d, lineout_pin 0x%X, line%d\n",
+ __func__, (int)lineout_jack_plugin, lineout_pin, __LINE__);
+
+ jack = snd_hda_jack_tbl_get(spec->codec, lineout_pin);
+
+ if (jack) {
+ jack->block_report = 0;
+ snd_hda_jack_report_sync(spec->codec);
+ }
+}
+
+static void line_callback(struct hda_codec *codec, struct hda_jack_callback *cb)
+{
+ struct cmi_spec *spec = codec->spec;
+ struct hda_jack_tbl *tbl;
+
+ /* Delay enabling the line, to let the linein-detection
+ * state machine run.
+ */
+
+ codec_dbg(codec, "%s, cb->nid 0x%X, line%d\n", __func__,
+ (int)cb->nid, __LINE__);
+
+ tbl = snd_hda_jack_tbl_get(codec, cb->nid);
+ if (tbl)
+ tbl->block_report = 1;
+ schedule_delayed_work(&spec->unsol_line_work, msecs_to_jiffies(200));
+}
+
static void cm9825_setup_unsol(struct hda_codec *codec)
{
struct cmi_spec *spec = codec->spec;
hda_nid_t hp_pin = spec->gen.autocfg.hp_pins[0];
+ if (codec->core.subsystem_id == QUIRK_GENE_TWL7_SSID)
+ hp_pin = 0x36;
+
snd_hda_jack_detect_enable_callback(codec, hp_pin, hp_callback);
+
+ if (codec->core.subsystem_id == QUIRK_GENE_TWL7_SSID) {
+ hda_nid_t linein_pin = 0x3b;
+
+ snd_hda_jack_detect_enable_callback(codec, linein_pin,
+ line_callback);
+ }
+}
+
+static void cm9825_init_hook(struct hda_codec *codec)
+{
+ unsigned int val;
+
+ codec_dbg(codec, "init hook\n");
+
+ /* OMTP */
+ val = snd_hda_codec_read(codec, 0x46, 0, 0xfec, 0x0);
+ snd_hda_codec_write(codec, 0x46, 0, 0x7ef, (val >> 24) & 0x7f);
+
+ // link reset
+ if (codec->core.subsystem_id == QUIRK_GENE_TWL7_SSID)
+ snd_hda_sequence_write(codec, cm9825_gene_twl7_d0_verbs);
+}
+
+static void cm9825_playback_pcm_hook(struct hda_pcm_stream *hinfo,
+ struct hda_codec *codec,
+ struct snd_pcm_substream *substream,
+ int action)
+{
+ struct cmi_spec *spec = codec->spec;
+
+ switch (action) {
+ case HDA_GEN_PCM_ACT_PREPARE:
+ if (codec->core.subsystem_id == QUIRK_GENE_TWL7_SSID) {
+ snd_hda_sequence_write(spec->codec,
+ cm9825_gene_twl7_playback_start_verbs);
+ }
+ break;
+ case HDA_GEN_PCM_ACT_CLEANUP:
+ if (codec->core.subsystem_id == QUIRK_GENE_TWL7_SSID) {
+ snd_hda_sequence_write(spec->codec,
+ cm9825_gene_twl7_playback_stop_verbs);
+ }
+ break;
+ default:
+ return;
+ }
}
static int cm9825_init(struct hda_codec *codec)
@@ -184,6 +340,7 @@ static void cm9825_remove(struct hda_codec *codec)
struct cmi_spec *spec = codec->spec;
cancel_delayed_work_sync(&spec->unsol_hp_work);
+ cancel_delayed_work_sync(&spec->unsol_line_work);
snd_hda_gen_remove(codec);
}
@@ -195,6 +352,9 @@ static int cm9825_suspend(struct hda_codec *codec)
snd_hda_sequence_write(codec, spec->chip_d3_verbs);
+ if (codec->core.subsystem_id == QUIRK_GENE_TWL7_SSID)
+ cancel_delayed_work_sync(&spec->unsol_line_work);
+
return 0;
}
@@ -213,7 +373,7 @@ static int cm9825_resume(struct hda_codec *codec)
msleep(150); /* for depop noise */
- snd_hda_codec_init(codec);
+ snd_hda_sequence_write(codec, spec->chip_d0_verbs);
hp_pin = spec->gen.autocfg.hp_pins[0];
hp_jack_plugin = snd_hda_jack_detect(spec->codec, hp_pin);
@@ -229,7 +389,9 @@ static int cm9825_resume(struct hda_codec *codec)
if (err)
codec_dbg(codec, "codec_write err %d\n", err);
- snd_hda_sequence_write(codec, cm9825_hp_remove_verbs);
+ if (codec->core.subsystem_id == QUIRK_CM_STD)
+ snd_hda_sequence_write(codec,
+ spec->chip_hp_remove_verbs);
}
snd_hda_regmap_sync(codec);
@@ -248,9 +410,13 @@ static int cm9825_probe(struct hda_codec *codec, const struct hda_device_id *id)
if (spec == NULL)
return -ENOMEM;
+ codec_dbg(codec, "chip_name: %s, ssid: 0x%X\n",
+ codec->core.chip_name, codec->core.subsystem_id);
+
INIT_DELAYED_WORK(&spec->unsol_hp_work, cm9825_unsol_hp_delayed);
codec->spec = spec;
spec->codec = codec;
+ spec->gen.init_hook = cm9825_init_hook;
cfg = &spec->gen.autocfg;
snd_hda_gen_spec_init(&spec->gen);
spec->chip_d0_verbs = cm9825_std_d0_verbs;
@@ -258,6 +424,41 @@ static int cm9825_probe(struct hda_codec *codec, const struct hda_device_id *id)
spec->chip_hp_present_verbs = cm9825_hp_present_verbs;
spec->chip_hp_remove_verbs = cm9825_hp_remove_verbs;
+ switch (codec->core.subsystem_id) {
+ case QUIRK_CM_STD:
+ snd_hda_codec_set_name(codec, "CM9825 STD");
+ spec->chip_d0_verbs = cm9825_std_d0_verbs;
+ spec->chip_d3_verbs = cm9825_std_d3_verbs;
+ spec->chip_hp_present_verbs = cm9825_hp_present_verbs;
+ spec->chip_hp_remove_verbs = cm9825_hp_remove_verbs;
+ break;
+ case QUIRK_GENE_TWL7_SSID:
+ snd_hda_codec_set_name(codec, "CM9825 GENE_TWL7");
+ INIT_DELAYED_WORK(&spec->unsol_line_work,
+ cm9825_unsol_line_delayed);
+ spec->gen.hp_mic = 0;
+ cfg->line_outs = 1;
+ cfg->line_out_pins[0] = 0x36;
+ cfg->line_out_type = AUTO_PIN_LINE_OUT;
+ cfg->num_inputs = 2;
+ cfg->inputs[0].pin = 0x3b;
+ cfg->inputs[0].type = AUTO_PIN_LINE_IN;
+ cfg->inputs[1].pin = 0x37;
+ cfg->inputs[1].type = AUTO_PIN_MIC;
+ cfg->inputs[1].is_headphone_mic = 1;
+ spec->chip_d0_verbs = cm9825_gene_twl7_d0_verbs;
+ spec->chip_d3_verbs = cm9825_gene_twl7_d3_verbs;
+ spec->gen.pcm_playback_hook = cm9825_playback_pcm_hook;
+ snd_hda_codec_set_pincfg(codec, 0x37, 0x24A70100);
+ break;
+ default:
+ spec->chip_d0_verbs = cm9825_std_d0_verbs;
+ spec->chip_d3_verbs = cm9825_std_d3_verbs;
+ spec->chip_hp_present_verbs = cm9825_hp_present_verbs;
+ spec->chip_hp_remove_verbs = cm9825_hp_remove_verbs;
+ break;
+ }
+
snd_hda_sequence_write(codec, spec->chip_d0_verbs);
err = snd_hda_parse_pin_defcfg(codec, cfg, NULL, 0);
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] ALSA: hda/cm9825: Add new project GENE_TWL7 for AAEON
2025-12-12 9:31 Leo Tsai
@ 2025-12-14 10:30 ` Takashi Iwai
0 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2025-12-14 10:30 UTC (permalink / raw)
To: Leo Tsai; +Cc: perex, tiwai, rf, leo.tsai, linux-sound, linux-kernel
On Fri, 12 Dec 2025 10:31:57 +0100,
Leo Tsai wrote:
>
> Update CM9825 to support GENE_TWL7 which supports Line-out, Line-in, and Mic-in.
>
> Signed-off-by: Leo Tsai <antivirus621@gmail.com>
Thanks for the update. But it still doesn't look good enough.
First off, you gave still too few descriptions. What is GENE_TWL7 at
all? And, why there are tons of code changes just for add the support
line-out, line-in and mic-in? Those need more clarifications.
And, judging from the previous patch, I guess this is only a part of
the whole changes. If more changes are planned, it's often worth to
mention it.
More about the code changes:
> @@ -80,10 +87,8 @@ static const struct hda_verb cm9825_std_d0_verbs[] = {
> {0x43, CM9825_VERB_SET_OCP, 0x33}, /* OTP set */
> {0x43, CM9825_VERB_SET_GAD, 0x07}, /* ADC -3db */
> {0x43, CM9825_VERB_SET_TMOD, 0x26}, /* Class D clk */
> - {0x3C, AC_VERB_SET_AMP_GAIN_MUTE |
> - AC_AMP_SET_OUTPUT | AC_AMP_SET_RIGHT, 0x2d}, /* Gain set */
> - {0x3C, AC_VERB_SET_AMP_GAIN_MUTE |
> - AC_AMP_SET_OUTPUT | AC_AMP_SET_LEFT, 0x2d}, /* Gain set */
> + {0x3c, AC_VERB_SET_AMP_GAIN_MUTE | 0xa0, 0x2d}, /* Gain set */
> + {0x3c, AC_VERB_SET_AMP_GAIN_MUTE | 0x90, 0x2d}, /* Gain set */
Why you replaced the symbols with numbers? That worsens the code
readability.
> +static const struct hda_verb cm9825_gene_twl7_d3_verbs[] = {
> + {0x43, CM9825_VERB_SET_D2S, 0x62},
> + {0x43, CM9825_VERB_SET_PLL, 0x01},
> + {0x43, CM9825_VERB_SET_NEG, 0xc2},
> + {0x43, CM9825_VERB_SET_ADCL, 0x00},
> + {0x43, CM9825_VERB_SET_DACL, 0x02},
> + {0x43, CM9825_VERB_SET_MBIAS, 0x00},
> + {0x43, CM9825_VERB_SET_VNEG, 0x50},
> + {0x43, CM9825_VERB_SET_PDNEG, 0x04},
> + {0x43, CM9825_VERB_SET_CDALR, 0xf6},
> + {0x43, CM9825_VERB_SET_OTP, 0xcd},
> + {}
> +};
What does those verbs do?
> +static const struct hda_verb cm9825_gene_twl7_d0_verbs[] = {
> + {0x34, AC_VERB_SET_EAPD_BTLENABLE, 0x02},
> + {0x43, CM9825_VERB_SET_SNR, 0x38},
> + {0x43, CM9825_VERB_SET_PLL, 0x00},
> + {0x43, CM9825_VERB_SET_ADCL, 0xcf},
> + {0x43, CM9825_VERB_SET_DACL, 0xaa},
> + {0x43, CM9825_VERB_SET_MBIAS, 0x1c},
> + {0x43, CM9825_VERB_SET_VNEG, 0x56},
> + {0x43, CM9825_VERB_SET_D2S, 0x62},
> + {0x43, CM9825_VERB_SET_DACTRL, 0x00},
> + {0x43, CM9825_VERB_SET_PDNEG, 0x0c},
> + {0x43, CM9825_VERB_SET_CDALR, 0xf4},
> + {0x43, CM9825_VERB_SET_OTP, 0xcd},
> + {0x43, CM9825_VERB_SET_MTCBA, 0x61},
> + {0x43, CM9825_VERB_SET_OCP, 0x33},
> + {0x43, CM9825_VERB_SET_GAD, 0x07},
> + {0x43, CM9825_VERB_SET_TMOD, 0x26},
> + {0x43, CM9825_VERB_SET_HPF_1, 0x40},
> + {0x43, CM9825_VERB_SET_HPF_2, 0x40},
> + {0x40, AC_VERB_SET_CONNECT_SEL, 0x00},
> + {0x3d, AC_VERB_SET_CONNECT_SEL, 0x01},
> + {0x46, CM9825_VERB_SET_P3BCP, 0x20},
> + {}
> +};
Ditto.
> +static const struct hda_verb cm9825_gene_twl7_playback_start_verbs[] = {
> + {0x43, CM9825_VERB_SET_D2S, 0xf2},
> + {0x43, CM9825_VERB_SET_VDO, 0xd4},
> + {0x43, CM9825_VERB_SET_SNR, 0x30},
> + {}
> +};
> +
> +static const struct hda_verb cm9825_gene_twl7_playback_stop_verbs[] = {
> + {0x43, CM9825_VERB_SET_VDO, 0xc0},
> + {0x43, CM9825_VERB_SET_D2S, 0x62},
> + {0x43, CM9825_VERB_SET_VDO, 0xd0},
> + {0x43, CM9825_VERB_SET_SNR, 0x38},
> + {}
> +};
... and those, too. Please give comments what those verbs do and why
they are needed explicitly.
> static void cm9825_unsol_hp_delayed(struct work_struct *work)
> {
> struct cmi_spec *spec =
> @@ -120,6 +179,9 @@ static void cm9825_unsol_hp_delayed(struct work_struct *work)
> bool hp_jack_plugin = false;
> int err = 0;
>
> + if (spec->codec->core.subsystem_id == QUIRK_GENE_TWL7_SSID)
> + hp_pin = 0x36;
Is it 100% sure that the headphone pin is fixed only to this node?
And why this pin isn't assigned to the autocfg.hp_pins[0] at the first
place? Is BIOS broken?
> hp_jack_plugin = snd_hda_jack_detect(spec->codec, hp_pin);
>
> codec_dbg(spec->codec, "hp_jack_plugin %d, hp_pin 0x%X\n",
> @@ -132,10 +194,13 @@ static void cm9825_unsol_hp_delayed(struct work_struct *work)
> if (err)
> codec_dbg(spec->codec, "codec_write err %d\n", err);
>
> - snd_hda_sequence_write(spec->codec, spec->chip_hp_remove_verbs);
> + if (spec->codec->core.subsystem_id == QUIRK_CM_STD)
> + snd_hda_sequence_write(spec->codec,
> + spec->chip_hp_remove_verbs);
> } else {
> - snd_hda_sequence_write(spec->codec,
> - spec->chip_hp_present_verbs);
> + if (spec->codec->core.subsystem_id == QUIRK_CM_STD)
> + snd_hda_sequence_write(spec->codec,
> + spec->chip_hp_present_verbs);
The check of subsystem_id is too ugly here and there.
If this is a conditional setup, set spec->chip_hp_present_verbs to
non-NULL only for the required model, and just check NULL instead of
subsystem_id.
> +static void cm9825_unsol_line_delayed(struct work_struct *work)
> +{
> + struct cmi_spec *spec =
> + container_of(to_delayed_work(work), struct cmi_spec,
> + unsol_line_work);
> + struct hda_jack_tbl *jack;
> + hda_nid_t lineout_pin = 0x3b;
Again, a fixed node ID. That's a very bad way.
> static void cm9825_setup_unsol(struct hda_codec *codec)
> {
> struct cmi_spec *spec = codec->spec;
>
> hda_nid_t hp_pin = spec->gen.autocfg.hp_pins[0];
>
> + if (codec->core.subsystem_id == QUIRK_GENE_TWL7_SSID)
> + hp_pin = 0x36;
Here again....
> snd_hda_jack_detect_enable_callback(codec, hp_pin, hp_callback);
> +
> + if (codec->core.subsystem_id == QUIRK_GENE_TWL7_SSID) {
This could be in better way instead of checking subsystem_id at each
place. It's error-prone.
> + hda_nid_t linein_pin = 0x3b;
Here...
> +static void cm9825_init_hook(struct hda_codec *codec)
> +{
> + unsigned int val;
> +
> + codec_dbg(codec, "init hook\n");
> +
> + /* OMTP */
> + val = snd_hda_codec_read(codec, 0x46, 0, 0xfec, 0x0);
> + snd_hda_codec_write(codec, 0x46, 0, 0x7ef, (val >> 24) & 0x7f);
What does this more exactly?
And this sequence wasn't present for STD model before the patch.
Is it safe to apply unconditionally?
> + // link reset
> + if (codec->core.subsystem_id == QUIRK_GENE_TWL7_SSID)
> + snd_hda_sequence_write(codec, cm9825_gene_twl7_d0_verbs);
Need more comment. Also, you should reconsider whether subsystem_id
check is the best way here, too.
> +static void cm9825_playback_pcm_hook(struct hda_pcm_stream *hinfo,
> + struct hda_codec *codec,
> + struct snd_pcm_substream *substream,
> + int action)
> +{
> + struct cmi_spec *spec = codec->spec;
> +
> + switch (action) {
> + case HDA_GEN_PCM_ACT_PREPARE:
> + if (codec->core.subsystem_id == QUIRK_GENE_TWL7_SSID) {
> + snd_hda_sequence_write(spec->codec,
> + cm9825_gene_twl7_playback_start_verbs);
> + }
> + break;
> + case HDA_GEN_PCM_ACT_CLEANUP:
> + if (codec->core.subsystem_id == QUIRK_GENE_TWL7_SSID) {
> + snd_hda_sequence_write(spec->codec,
> + cm9825_gene_twl7_playback_stop_verbs);
> + }
> + break;
> + default:
> + return;
> + }
If it's model-specific, rather set up the hook conditionally, instead
of checking subsystem_id in the callback.
> static int cm9825_init(struct hda_codec *codec)
> @@ -184,6 +340,7 @@ static void cm9825_remove(struct hda_codec *codec)
> struct cmi_spec *spec = codec->spec;
>
> cancel_delayed_work_sync(&spec->unsol_hp_work);
> + cancel_delayed_work_sync(&spec->unsol_line_work);
You're calling here unconditionally, while...
> snd_hda_gen_remove(codec);
> }
>
> @@ -195,6 +352,9 @@ static int cm9825_suspend(struct hda_codec *codec)
>
> snd_hda_sequence_write(codec, spec->chip_d3_verbs);
>
> + if (codec->core.subsystem_id == QUIRK_GENE_TWL7_SSID)
> + cancel_delayed_work_sync(&spec->unsol_line_work);
... here conditionally. And I believe your patch breaks STD model
when the module is unloaded, because the work isn't initialized but
calling cancel_delayed_work_sync() in the above.
And, again, the conditional call with the subsystem_id check is
error-prone.
> @@ -213,7 +373,7 @@ static int cm9825_resume(struct hda_codec *codec)
>
> msleep(150); /* for depop noise */
>
> - snd_hda_codec_init(codec);
> + snd_hda_sequence_write(codec, spec->chip_d0_verbs);
Why is this change...? It'll certainly break other things.
> hp_pin = spec->gen.autocfg.hp_pins[0];
> hp_jack_plugin = snd_hda_jack_detect(spec->codec, hp_pin);
> @@ -229,7 +389,9 @@ static int cm9825_resume(struct hda_codec *codec)
> if (err)
> codec_dbg(codec, "codec_write err %d\n", err);
>
> - snd_hda_sequence_write(codec, cm9825_hp_remove_verbs);
> + if (codec->core.subsystem_id == QUIRK_CM_STD)
> + snd_hda_sequence_write(codec,
> + spec->chip_hp_remove_verbs);
Here again ugly subystem_id conditional...
> }
>
> snd_hda_regmap_sync(codec);
> @@ -248,9 +410,13 @@ static int cm9825_probe(struct hda_codec *codec, const struct hda_device_id *id)
> if (spec == NULL)
> return -ENOMEM;
>
> + codec_dbg(codec, "chip_name: %s, ssid: 0x%X\n",
> + codec->core.chip_name, codec->core.subsystem_id);
> +
> INIT_DELAYED_WORK(&spec->unsol_hp_work, cm9825_unsol_hp_delayed);
> codec->spec = spec;
> spec->codec = codec;
> + spec->gen.init_hook = cm9825_init_hook;
> cfg = &spec->gen.autocfg;
> snd_hda_gen_spec_init(&spec->gen);
> spec->chip_d0_verbs = cm9825_std_d0_verbs;
> @@ -258,6 +424,41 @@ static int cm9825_probe(struct hda_codec *codec, const struct hda_device_id *id)
> spec->chip_hp_present_verbs = cm9825_hp_present_verbs;
> spec->chip_hp_remove_verbs = cm9825_hp_remove_verbs;
>
> + switch (codec->core.subsystem_id) {
> + case QUIRK_CM_STD:
> + snd_hda_codec_set_name(codec, "CM9825 STD");
> + spec->chip_d0_verbs = cm9825_std_d0_verbs;
> + spec->chip_d3_verbs = cm9825_std_d3_verbs;
> + spec->chip_hp_present_verbs = cm9825_hp_present_verbs;
> + spec->chip_hp_remove_verbs = cm9825_hp_remove_verbs;
> + break;
> + case QUIRK_GENE_TWL7_SSID:
> + snd_hda_codec_set_name(codec, "CM9825 GENE_TWL7");
> + INIT_DELAYED_WORK(&spec->unsol_line_work,
> + cm9825_unsol_line_delayed);
> + spec->gen.hp_mic = 0;
> + cfg->line_outs = 1;
> + cfg->line_out_pins[0] = 0x36;
> + cfg->line_out_type = AUTO_PIN_LINE_OUT;
> + cfg->num_inputs = 2;
> + cfg->inputs[0].pin = 0x3b;
> + cfg->inputs[0].type = AUTO_PIN_LINE_IN;
> + cfg->inputs[1].pin = 0x37;
> + cfg->inputs[1].type = AUTO_PIN_MIC;
> + cfg->inputs[1].is_headphone_mic = 1;
> + spec->chip_d0_verbs = cm9825_gene_twl7_d0_verbs;
> + spec->chip_d3_verbs = cm9825_gene_twl7_d3_verbs;
> + spec->gen.pcm_playback_hook = cm9825_playback_pcm_hook;
> + snd_hda_codec_set_pincfg(codec, 0x37, 0x24A70100);
> + break;
> + default:
> + spec->chip_d0_verbs = cm9825_std_d0_verbs;
> + spec->chip_d3_verbs = cm9825_std_d3_verbs;
> + spec->chip_hp_present_verbs = cm9825_hp_present_verbs;
> + spec->chip_hp_remove_verbs = cm9825_hp_remove_verbs;
> + break;
The default should be QUIRK_CM_STD, or handle it as an error. There
is no reason to define differently for default.
thanks,
Takashi
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] ALSA: hda/cm9825: Add new project GENE_TWL7 for AAEON
@ 2025-12-15 8:14 Leo Tsai
2025-12-15 8:36 ` Takashi Iwai
0 siblings, 1 reply; 6+ messages in thread
From: Leo Tsai @ 2025-12-15 8:14 UTC (permalink / raw)
To: perex, tiwai, rf; +Cc: leo.tsai, linux-sound, linux-kernel, Leo Tsai
The GENE_TWL7 project is an AAEON platform with a fixed audio
configuration consisting of line-out, line-in, and mic-in.
The audio routing and pin assignments are defined according to
the board-level hardware design and are not intended to be
dynamically changed.
Signed-off-by: Leo Tsai <antivirus621@gmail.com>
---
sound/hda/codecs/cm9825.c | 315 ++++++++++++++++++++++++++++++++++++--
1 file changed, 303 insertions(+), 12 deletions(-)
diff --git a/sound/hda/codecs/cm9825.c b/sound/hda/codecs/cm9825.c
index 5c474ce44348..d92b3df582af 100644
--- a/sound/hda/codecs/cm9825.c
+++ b/sound/hda/codecs/cm9825.c
@@ -13,6 +13,11 @@
#include "hda_jack.h"
#include "generic.h"
+enum {
+ QUIRK_CM_STD = 0x0,
+ QUIRK_GENE_TWL7_SSID = 0x160dc000
+};
+
/* CM9825 Offset Definitions */
#define CM9825_VERB_SET_HPF_1 0x781
@@ -25,6 +30,7 @@
#define CM9825_VERB_SET_VNEG 0x7a8
#define CM9825_VERB_SET_D2S 0x7a9
#define CM9825_VERB_SET_DACTRL 0x7aa
+#define CM9825_VERB_SET_P3BCP 0x7ab
#define CM9825_VERB_SET_PDNEG 0x7ac
#define CM9825_VERB_SET_VDO 0x7ad
#define CM9825_VERB_SET_CDALR 0x7b0
@@ -42,6 +48,8 @@ struct cmi_spec {
const struct hda_verb *chip_hp_present_verbs;
const struct hda_verb *chip_hp_remove_verbs;
struct hda_codec *codec;
+ struct delayed_work unsol_linein_work;
+ struct delayed_work unsol_lineout_work;
struct delayed_work unsol_hp_work;
int quirk;
};
@@ -80,10 +88,8 @@ static const struct hda_verb cm9825_std_d0_verbs[] = {
{0x43, CM9825_VERB_SET_OCP, 0x33}, /* OTP set */
{0x43, CM9825_VERB_SET_GAD, 0x07}, /* ADC -3db */
{0x43, CM9825_VERB_SET_TMOD, 0x26}, /* Class D clk */
- {0x3C, AC_VERB_SET_AMP_GAIN_MUTE |
- AC_AMP_SET_OUTPUT | AC_AMP_SET_RIGHT, 0x2d}, /* Gain set */
- {0x3C, AC_VERB_SET_AMP_GAIN_MUTE |
- AC_AMP_SET_OUTPUT | AC_AMP_SET_LEFT, 0x2d}, /* Gain set */
+ {0x3c, AC_VERB_SET_AMP_GAIN_MUTE | 0xa0, 0x2d}, /* Gain set */
+ {0x3c, AC_VERB_SET_AMP_GAIN_MUTE | 0x90, 0x2d}, /* Gain set */
{0x43, CM9825_VERB_SET_HPF_1, 0x40}, /* HPF set */
{0x43, CM9825_VERB_SET_HPF_2, 0x40}, /* HPF set */
{}
@@ -111,6 +117,72 @@ static const struct hda_verb cm9825_hp_remove_verbs[] = {
{}
};
+/*
+ * To save power, AD/CLK is turned off.
+ */
+static const struct hda_verb cm9825_gene_twl7_d3_verbs[] = {
+ {0x43, CM9825_VERB_SET_D2S, 0x62},
+ {0x43, CM9825_VERB_SET_PLL, 0x01},
+ {0x43, CM9825_VERB_SET_NEG, 0xc2},
+ {0x43, CM9825_VERB_SET_ADCL, 0x00},
+ {0x43, CM9825_VERB_SET_DACL, 0x02},
+ {0x43, CM9825_VERB_SET_MBIAS, 0x00},
+ {0x43, CM9825_VERB_SET_VNEG, 0x50},
+ {0x43, CM9825_VERB_SET_PDNEG, 0x04},
+ {0x43, CM9825_VERB_SET_CDALR, 0xf6},
+ {0x43, CM9825_VERB_SET_OTP, 0xcd},
+ {}
+};
+
+/*
+ * Enable CLK/ADC to start recording.
+ */
+static const struct hda_verb cm9825_gene_twl7_d0_verbs[] = {
+ {0x34, AC_VERB_SET_EAPD_BTLENABLE, 0x02},
+ {0x43, CM9825_VERB_SET_SNR, 0x38},
+ {0x43, CM9825_VERB_SET_PLL, 0x00},
+ {0x43, CM9825_VERB_SET_ADCL, 0xcf},
+ {0x43, CM9825_VERB_SET_DACL, 0xaa},
+ {0x43, CM9825_VERB_SET_MBIAS, 0x1c},
+ {0x43, CM9825_VERB_SET_VNEG, 0x56},
+ {0x43, CM9825_VERB_SET_D2S, 0x62},
+ {0x43, CM9825_VERB_SET_DACTRL, 0x00},
+ {0x43, CM9825_VERB_SET_PDNEG, 0x0c},
+ {0x43, CM9825_VERB_SET_CDALR, 0xf4},
+ {0x43, CM9825_VERB_SET_OTP, 0xcd},
+ {0x43, CM9825_VERB_SET_MTCBA, 0x61},
+ {0x43, CM9825_VERB_SET_OCP, 0x33},
+ {0x43, CM9825_VERB_SET_GAD, 0x07},
+ {0x43, CM9825_VERB_SET_TMOD, 0x26},
+ {0x43, CM9825_VERB_SET_HPF_1, 0x40},
+ {0x43, CM9825_VERB_SET_HPF_2, 0x40},
+ {0x40, AC_VERB_SET_CONNECT_SEL, 0x00},
+ {0x3d, AC_VERB_SET_CONNECT_SEL, 0x01},
+ {0x46, CM9825_VERB_SET_P3BCP, 0x20},
+ {}
+};
+
+/*
+ * Enable DAC to start playback.
+ */
+static const struct hda_verb cm9825_gene_twl7_playback_start_verbs[] = {
+ {0x43, CM9825_VERB_SET_D2S, 0xf2},
+ {0x43, CM9825_VERB_SET_VDO, 0xd4},
+ {0x43, CM9825_VERB_SET_SNR, 0x30},
+ {}
+};
+
+/*
+ * Disable DAC and enable de-pop noise mechanism.
+ */
+static const struct hda_verb cm9825_gene_twl7_playback_stop_verbs[] = {
+ {0x43, CM9825_VERB_SET_VDO, 0xc0},
+ {0x43, CM9825_VERB_SET_D2S, 0x62},
+ {0x43, CM9825_VERB_SET_VDO, 0xd0},
+ {0x43, CM9825_VERB_SET_SNR, 0x38},
+ {}
+};
+
static void cm9825_unsol_hp_delayed(struct work_struct *work)
{
struct cmi_spec *spec =
@@ -162,6 +234,94 @@ static void hp_callback(struct hda_codec *codec, struct hda_jack_callback *cb)
schedule_delayed_work(&spec->unsol_hp_work, msecs_to_jiffies(200));
}
+static void cm9825_unsol_linein_delayed(struct work_struct *work)
+{
+ struct cmi_spec *spec =
+ container_of(to_delayed_work(work), struct cmi_spec,
+ unsol_linein_work);
+ struct hda_jack_tbl *jack;
+
+ hda_nid_t linein_pin = spec->gen.autocfg.inputs[1].pin;
+
+ bool linein_jack_plugin = false;
+
+ linein_jack_plugin = snd_hda_jack_detect(spec->codec, linein_pin);
+
+ codec_dbg(spec->codec,
+ "%s, lineout_jack_plugin %d, linein_pin 0x%X, line%d\n",
+ __func__, (int)linein_jack_plugin, linein_pin, __LINE__);
+
+ jack = snd_hda_jack_tbl_get(spec->codec, linein_pin);
+
+ if (jack) {
+ jack->block_report = 0;
+ snd_hda_jack_report_sync(spec->codec);
+ }
+}
+
+static void linein_callback(struct hda_codec *codec,
+ struct hda_jack_callback *cb)
+{
+ struct cmi_spec *spec = codec->spec;
+ struct hda_jack_tbl *tbl;
+
+ /* Delay enabling the line, to let the linein-detection
+ * state machine run.
+ */
+
+ codec_dbg(codec, "%s, cb->nid 0x%X, line%d\n", __func__,
+ (int)cb->nid, __LINE__);
+
+ tbl = snd_hda_jack_tbl_get(codec, cb->nid);
+ if (tbl)
+ tbl->block_report = 1;
+ schedule_delayed_work(&spec->unsol_linein_work, msecs_to_jiffies(200));
+}
+
+static void cm9825_unsol_lineout_delayed(struct work_struct *work)
+{
+ struct cmi_spec *spec =
+ container_of(to_delayed_work(work), struct cmi_spec,
+ unsol_lineout_work);
+ struct hda_jack_tbl *jack;
+
+ hda_nid_t lineout_pin = spec->gen.autocfg.line_out_pins[0];
+
+ bool lineout_jack_plugin = false;
+
+ lineout_jack_plugin = snd_hda_jack_detect(spec->codec, lineout_pin);
+
+ codec_dbg(spec->codec,
+ "%s, lineout_jack_plugin %d, lineout_pin 0x%X, line%d\n",
+ __func__, (int)lineout_jack_plugin, lineout_pin, __LINE__);
+
+ jack = snd_hda_jack_tbl_get(spec->codec, lineout_pin);
+
+ if (jack) {
+ jack->block_report = 0;
+ snd_hda_jack_report_sync(spec->codec);
+ }
+}
+
+static void lineout_callback(struct hda_codec *codec,
+ struct hda_jack_callback *cb)
+{
+ struct cmi_spec *spec = codec->spec;
+ struct hda_jack_tbl *tbl;
+
+ /* Delay enabling the line, to let the linein-detection
+ * state machine run.
+ */
+
+ codec_dbg(codec, "%s, cb->nid 0x%X, line%d\n", __func__,
+ (int)cb->nid, __LINE__);
+
+ tbl = snd_hda_jack_tbl_get(codec, cb->nid);
+ if (tbl)
+ tbl->block_report = 1;
+ schedule_delayed_work(&spec->unsol_lineout_work, msecs_to_jiffies(200));
+}
+
static void cm9825_setup_unsol(struct hda_codec *codec)
{
struct cmi_spec *spec = codec->spec;
@@ -171,6 +331,44 @@ static void cm9825_setup_unsol(struct hda_codec *codec)
snd_hda_jack_detect_enable_callback(codec, hp_pin, hp_callback);
}
+static void cm9825_gene_twl7_setup_unsol(struct hda_codec *codec)
+{
+ struct cmi_spec *spec = codec->spec;
+
+ hda_nid_t lineout_pin = spec->gen.autocfg.line_out_pins[0];
+
+ hda_nid_t lineint_pin = spec->gen.autocfg.inputs[0].pin;
+
+ codec_dbg(codec, "%s, lineout_pin 0x%X, lineint_pin 0x%X, line%d\n",
+ __func__, lineout_pin, lineint_pin, __LINE__);
+
+ snd_hda_jack_detect_enable_callback(codec, lineout_pin,
+ lineout_callback);
+ snd_hda_jack_detect_enable_callback(codec, lineint_pin,
+ linein_callback);
+}
+
+static void cm9825_playback_pcm_hook(struct hda_pcm_stream *hinfo,
+ struct hda_codec *codec,
+ struct snd_pcm_substream *substream,
+ int action)
+{
+ struct cmi_spec *spec = codec->spec;
+
+ switch (action) {
+ case HDA_GEN_PCM_ACT_PREPARE:
+ snd_hda_sequence_write(spec->codec,
+ cm9825_gene_twl7_playback_start_verbs);
+ break;
+ case HDA_GEN_PCM_ACT_CLEANUP:
+ snd_hda_sequence_write(spec->codec,
+ cm9825_gene_twl7_playback_stop_verbs);
+ break;
+ default:
+ return;
+ }
+}
+
static int cm9825_init(struct hda_codec *codec)
{
snd_hda_gen_init(codec);
@@ -179,26 +377,61 @@ static int cm9825_init(struct hda_codec *codec)
return 0;
}
-static void cm9825_remove(struct hda_codec *codec)
+static void cm9825_cm_std_remove(struct hda_codec *codec)
{
struct cmi_spec *spec = codec->spec;
cancel_delayed_work_sync(&spec->unsol_hp_work);
+}
+
+static void cm9825_gene_twl7_remove(struct hda_codec *codec)
+{
+ struct cmi_spec *spec = codec->spec;
+
+ cancel_delayed_work_sync(&spec->unsol_linein_work);
+ cancel_delayed_work_sync(&spec->unsol_lineout_work);
+}
+
+static void cm9825_remove(struct hda_codec *codec)
+{
+ if (codec->core.subsystem_id == QUIRK_CM_STD)
+ cm9825_cm_std_remove(codec);
+ else if (codec->core.subsystem_id == QUIRK_GENE_TWL7_SSID)
+ cm9825_gene_twl7_remove(codec);
+
snd_hda_gen_remove(codec);
}
-static int cm9825_suspend(struct hda_codec *codec)
+static void cm9825_cm_std_suspend(struct hda_codec *codec)
{
struct cmi_spec *spec = codec->spec;
cancel_delayed_work_sync(&spec->unsol_hp_work);
+}
+
+static void cm9825_gene_twl7_suspend(struct hda_codec *codec)
+{
+ struct cmi_spec *spec = codec->spec;
+
+ cancel_delayed_work_sync(&spec->unsol_linein_work);
+ cancel_delayed_work_sync(&spec->unsol_lineout_work);
+}
+
+static int cm9825_suspend(struct hda_codec *codec)
+{
+ struct cmi_spec *spec = codec->spec;
+
+ if (codec->core.subsystem_id == QUIRK_CM_STD)
+ cm9825_cm_std_suspend(codec);
+ else if (codec->core.subsystem_id == QUIRK_GENE_TWL7_SSID)
+ cm9825_gene_twl7_suspend(codec);
snd_hda_sequence_write(codec, spec->chip_d3_verbs);
return 0;
}
-static int cm9825_resume(struct hda_codec *codec)
+static int cm9825_cm_std_resume(struct hda_codec *codec)
{
struct cmi_spec *spec = codec->spec;
hda_nid_t hp_pin = 0;
@@ -213,7 +446,7 @@ static int cm9825_resume(struct hda_codec *codec)
msleep(150); /* for depop noise */
- snd_hda_codec_init(codec);
+ snd_hda_sequence_write(codec, spec->chip_d0_verbs);
hp_pin = spec->gen.autocfg.hp_pins[0];
hp_jack_plugin = snd_hda_jack_detect(spec->codec, hp_pin);
@@ -232,6 +465,18 @@ static int cm9825_resume(struct hda_codec *codec)
snd_hda_sequence_write(codec, cm9825_hp_remove_verbs);
}
+ return 0;
+}
+
+static int cm9825_resume(struct hda_codec *codec)
+{
+ struct cmi_spec *spec = codec->spec;
+
+ if (codec->core.subsystem_id == QUIRK_CM_STD)
+ cm9825_cm_std_resume(codec);
+ else if (codec->core.subsystem_id == QUIRK_GENE_TWL7_SSID)
+ snd_hda_sequence_write(codec, spec->chip_d0_verbs);
+
snd_hda_regmap_sync(codec);
hda_call_check_power_status(codec, 0x01);
@@ -242,13 +487,16 @@ static int cm9825_probe(struct hda_codec *codec, const struct hda_device_id *id)
{
struct cmi_spec *spec;
struct auto_pin_cfg *cfg;
- int err;
+ int err = 0;
+ unsigned int val = 0;
spec = kzalloc(sizeof(*spec), GFP_KERNEL);
if (spec == NULL)
return -ENOMEM;
- INIT_DELAYED_WORK(&spec->unsol_hp_work, cm9825_unsol_hp_delayed);
+ codec_dbg(codec, "chip_name: %s, ssid: 0x%X\n",
+ codec->core.chip_name, codec->core.subsystem_id);
+
codec->spec = spec;
spec->codec = codec;
cfg = &spec->gen.autocfg;
@@ -258,6 +506,51 @@ static int cm9825_probe(struct hda_codec *codec, const struct hda_device_id *id)
spec->chip_hp_present_verbs = cm9825_hp_present_verbs;
spec->chip_hp_remove_verbs = cm9825_hp_remove_verbs;
+ switch (codec->core.subsystem_id) {
+ case QUIRK_CM_STD:
+ snd_hda_codec_set_name(codec, "CM9825 STD");
+ INIT_DELAYED_WORK(&spec->unsol_hp_work,
+ cm9825_unsol_hp_delayed);
+ spec->chip_d0_verbs = cm9825_std_d0_verbs;
+ spec->chip_d3_verbs = cm9825_std_d3_verbs;
+ spec->chip_hp_present_verbs = cm9825_hp_present_verbs;
+ spec->chip_hp_remove_verbs = cm9825_hp_remove_verbs;
+ cm9825_setup_unsol(codec);
+ break;
+ case QUIRK_GENE_TWL7_SSID:
+ snd_hda_codec_set_name(codec, "CM9825 GENE_TWL7");
+ INIT_DELAYED_WORK(&spec->unsol_linein_work,
+ cm9825_unsol_linein_delayed);
+ INIT_DELAYED_WORK(&spec->unsol_lineout_work,
+ cm9825_unsol_lineout_delayed);
+ spec->gen.hp_mic = 0;
+ cfg->line_outs = 1;
+ cfg->line_out_pins[0] = 0x36;
+ cfg->line_out_type = AUTO_PIN_LINE_OUT;
+ cfg->num_inputs = 2;
+ cfg->inputs[0].pin = 0x3b;
+ cfg->inputs[0].type = AUTO_PIN_LINE_IN;
+ cfg->inputs[1].pin = 0x37;
+ cfg->inputs[1].type = AUTO_PIN_MIC;
+ cfg->inputs[1].is_headphone_mic = 1;
+ spec->chip_d0_verbs = cm9825_gene_twl7_d0_verbs;
+ spec->chip_d3_verbs = cm9825_gene_twl7_d3_verbs;
+ spec->gen.pcm_playback_hook = cm9825_playback_pcm_hook;
+ snd_hda_codec_set_pincfg(codec, 0x37, 0x24A70100);
+ cm9825_gene_twl7_setup_unsol(codec);
+ break;
+ default:
+ err = -ENXIO;
+ break;
+ }
+
+ if (err < 0)
+ goto error;
+
+ /* Support OMTP Jack */
+ val = snd_hda_codec_read(codec, 0x46, 0, 0xfec, 0x0);
+ snd_hda_codec_write(codec, 0x46, 0, 0x7ef, (val >> 24) & 0x7f);
+
snd_hda_sequence_write(codec, spec->chip_d0_verbs);
err = snd_hda_parse_pin_defcfg(codec, cfg, NULL, 0);
@@ -267,8 +560,6 @@ static int cm9825_probe(struct hda_codec *codec, const struct hda_device_id *id)
if (err < 0)
goto error;
- cm9825_setup_unsol(codec);
-
return 0;
error:
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] ALSA: hda/cm9825: Add new project GENE_TWL7 for AAEON
2025-12-15 8:14 [PATCH] ALSA: hda/cm9825: Add new project GENE_TWL7 for AAEON Leo Tsai
@ 2025-12-15 8:36 ` Takashi Iwai
0 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2025-12-15 8:36 UTC (permalink / raw)
To: Leo Tsai; +Cc: perex, tiwai, rf, leo.tsai, linux-sound, linux-kernel
On Mon, 15 Dec 2025 09:14:21 +0100,
Leo Tsai wrote:
>
> The GENE_TWL7 project is an AAEON platform with a fixed audio
> configuration consisting of line-out, line-in, and mic-in.
> The audio routing and pin assignments are defined according to
> the board-level hardware design and are not intended to be
> dynamically changed.
>
> Signed-off-by: Leo Tsai <antivirus621@gmail.com>
Please put v2 (or later number) prefix when you resubmit a newer
patch (e.g. [PATCH v2] ALSA: hda/cm9825: ....)
One thin I haven't noticed in the previous review:
> @@ -258,6 +506,51 @@ static int cm9825_probe(struct hda_codec *codec, const struct hda_device_id *id)
> spec->chip_hp_present_verbs = cm9825_hp_present_verbs;
> spec->chip_hp_remove_verbs = cm9825_hp_remove_verbs;
>
> + switch (codec->core.subsystem_id) {
> + case QUIRK_CM_STD:
> + snd_hda_codec_set_name(codec, "CM9825 STD");
> + INIT_DELAYED_WORK(&spec->unsol_hp_work,
> + cm9825_unsol_hp_delayed);
> + spec->chip_d0_verbs = cm9825_std_d0_verbs;
> + spec->chip_d3_verbs = cm9825_std_d3_verbs;
> + spec->chip_hp_present_verbs = cm9825_hp_present_verbs;
> + spec->chip_hp_remove_verbs = cm9825_hp_remove_verbs;
> + cm9825_setup_unsol(codec);
> + break;
> + case QUIRK_GENE_TWL7_SSID:
> + snd_hda_codec_set_name(codec, "CM9825 GENE_TWL7");
> + INIT_DELAYED_WORK(&spec->unsol_linein_work,
> + cm9825_unsol_linein_delayed);
> + INIT_DELAYED_WORK(&spec->unsol_lineout_work,
> + cm9825_unsol_lineout_delayed);
> + spec->gen.hp_mic = 0;
> + cfg->line_outs = 1;
> + cfg->line_out_pins[0] = 0x36;
> + cfg->line_out_type = AUTO_PIN_LINE_OUT;
> + cfg->num_inputs = 2;
> + cfg->inputs[0].pin = 0x3b;
> + cfg->inputs[0].type = AUTO_PIN_LINE_IN;
> + cfg->inputs[1].pin = 0x37;
> + cfg->inputs[1].type = AUTO_PIN_MIC;
> + cfg->inputs[1].is_headphone_mic = 1;
You're setting up the autocfg stuff here, but
snd_hda_parse_pin_defcfg() is called after this point.
Wouldn't it override the values set here?
That is, if you'd like to set up a static pin table, it's better to
provide your own default pin table via snd_hda_apply_pincfgs().
It's something like other codecs applying the pin quirks.
Takashi
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] ALSA: hda/cm9825: Add new project GENE_TWL7 for AAEON
@ 2025-12-15 11:22 Leo Tsai
2025-12-15 12:15 ` Takashi Iwai
0 siblings, 1 reply; 6+ messages in thread
From: Leo Tsai @ 2025-12-15 11:22 UTC (permalink / raw)
To: perex, tiwai, rf; +Cc: leo.tsai, linux-sound, linux-kernel, Leo Tsai
The GENE_TWL7 project is an AAEON platform with a fixed audio
configuration consisting of line-out, line-in, and mic-in.
The audio routing and pin assignments are defined according to
the board-level hardware design and are not intended to be
dynamically changed.
Signed-off-by: Leo Tsai <antivirus621@gmail.com>
---
sound/hda/codecs/cm9825.c | 292 +++++++++++++++++++++++++++++++++++++-
1 file changed, 285 insertions(+), 7 deletions(-)
diff --git a/sound/hda/codecs/cm9825.c b/sound/hda/codecs/cm9825.c
index 5c474ce44348..5532dde89ebe 100644
--- a/sound/hda/codecs/cm9825.c
+++ b/sound/hda/codecs/cm9825.c
@@ -13,6 +13,11 @@
#include "hda_jack.h"
#include "generic.h"
+enum {
+ QUIRK_CM_STD = 0x0,
+ QUIRK_GENE_TWL7_SSID = 0x160dc000
+};
+
/* CM9825 Offset Definitions */
#define CM9825_VERB_SET_HPF_1 0x781
@@ -25,6 +30,7 @@
#define CM9825_VERB_SET_VNEG 0x7a8
#define CM9825_VERB_SET_D2S 0x7a9
#define CM9825_VERB_SET_DACTRL 0x7aa
+#define CM9825_VERB_SET_P3BCP 0x7ab
#define CM9825_VERB_SET_PDNEG 0x7ac
#define CM9825_VERB_SET_VDO 0x7ad
#define CM9825_VERB_SET_CDALR 0x7b0
@@ -42,6 +48,8 @@ struct cmi_spec {
const struct hda_verb *chip_hp_present_verbs;
const struct hda_verb *chip_hp_remove_verbs;
struct hda_codec *codec;
+ struct delayed_work unsol_linein_work;
+ struct delayed_work unsol_lineout_work;
struct delayed_work unsol_hp_work;
int quirk;
};
@@ -111,6 +119,72 @@ static const struct hda_verb cm9825_hp_remove_verbs[] = {
{}
};
+/*
+ * To save power, AD/CLK is turned off.
+ */
+static const struct hda_verb cm9825_gene_twl7_d3_verbs[] = {
+ {0x43, CM9825_VERB_SET_D2S, 0x62},
+ {0x43, CM9825_VERB_SET_PLL, 0x01},
+ {0x43, CM9825_VERB_SET_NEG, 0xc2},
+ {0x43, CM9825_VERB_SET_ADCL, 0x00},
+ {0x43, CM9825_VERB_SET_DACL, 0x02},
+ {0x43, CM9825_VERB_SET_MBIAS, 0x00},
+ {0x43, CM9825_VERB_SET_VNEG, 0x50},
+ {0x43, CM9825_VERB_SET_PDNEG, 0x04},
+ {0x43, CM9825_VERB_SET_CDALR, 0xf6},
+ {0x43, CM9825_VERB_SET_OTP, 0xcd},
+ {}
+};
+
+/*
+ * Enable CLK/ADC to start recording.
+ */
+static const struct hda_verb cm9825_gene_twl7_d0_verbs[] = {
+ {0x34, AC_VERB_SET_EAPD_BTLENABLE, 0x02},
+ {0x43, CM9825_VERB_SET_SNR, 0x38},
+ {0x43, CM9825_VERB_SET_PLL, 0x00},
+ {0x43, CM9825_VERB_SET_ADCL, 0xcf},
+ {0x43, CM9825_VERB_SET_DACL, 0xaa},
+ {0x43, CM9825_VERB_SET_MBIAS, 0x1c},
+ {0x43, CM9825_VERB_SET_VNEG, 0x56},
+ {0x43, CM9825_VERB_SET_D2S, 0x62},
+ {0x43, CM9825_VERB_SET_DACTRL, 0x00},
+ {0x43, CM9825_VERB_SET_PDNEG, 0x0c},
+ {0x43, CM9825_VERB_SET_CDALR, 0xf4},
+ {0x43, CM9825_VERB_SET_OTP, 0xcd},
+ {0x43, CM9825_VERB_SET_MTCBA, 0x61},
+ {0x43, CM9825_VERB_SET_OCP, 0x33},
+ {0x43, CM9825_VERB_SET_GAD, 0x07},
+ {0x43, CM9825_VERB_SET_TMOD, 0x26},
+ {0x43, CM9825_VERB_SET_HPF_1, 0x40},
+ {0x43, CM9825_VERB_SET_HPF_2, 0x40},
+ {0x40, AC_VERB_SET_CONNECT_SEL, 0x00},
+ {0x3d, AC_VERB_SET_CONNECT_SEL, 0x01},
+ {0x46, CM9825_VERB_SET_P3BCP, 0x20},
+ {}
+};
+
+/*
+ * Enable DAC to start playback.
+ */
+static const struct hda_verb cm9825_gene_twl7_playback_start_verbs[] = {
+ {0x43, CM9825_VERB_SET_D2S, 0xf2},
+ {0x43, CM9825_VERB_SET_VDO, 0xd4},
+ {0x43, CM9825_VERB_SET_SNR, 0x30},
+ {}
+};
+
+/*
+ * Disable DAC and enable de-pop noise mechanism.
+ */
+static const struct hda_verb cm9825_gene_twl7_playback_stop_verbs[] = {
+ {0x43, CM9825_VERB_SET_VDO, 0xc0},
+ {0x43, CM9825_VERB_SET_D2S, 0x62},
+ {0x43, CM9825_VERB_SET_VDO, 0xd0},
+ {0x43, CM9825_VERB_SET_SNR, 0x38},
+ {}
+};
+
static void cm9825_unsol_hp_delayed(struct work_struct *work)
{
struct cmi_spec *spec =
@@ -162,13 +236,138 @@ static void hp_callback(struct hda_codec *codec, struct hda_jack_callback *cb)
schedule_delayed_work(&spec->unsol_hp_work, msecs_to_jiffies(200));
}
+static void cm9825_unsol_linein_delayed(struct work_struct *work)
+{
+ struct cmi_spec *spec =
+ container_of(to_delayed_work(work), struct cmi_spec,
+ unsol_linein_work);
+ struct hda_jack_tbl *jack;
+
+ hda_nid_t linein_pin = spec->gen.autocfg.inputs[1].pin;
+
+ bool linein_jack_plugin = false;
+
+ linein_jack_plugin = snd_hda_jack_detect(spec->codec, linein_pin);
+
+ codec_dbg(spec->codec,
+ "%s, lineout_jack_plugin %d, linein_pin 0x%X, line%d\n",
+ __func__, (int)linein_jack_plugin, linein_pin, __LINE__);
+
+ jack = snd_hda_jack_tbl_get(spec->codec, linein_pin);
+
+ if (jack) {
+ jack->block_report = 0;
+ snd_hda_jack_report_sync(spec->codec);
+ }
+}
+
+static void linein_callback(struct hda_codec *codec,
+ struct hda_jack_callback *cb)
+{
+ struct cmi_spec *spec = codec->spec;
+ struct hda_jack_tbl *tbl;
+
+ /* Delay enabling the line, to let the linein-detection
+ * state machine run.
+ */
+
+ codec_dbg(codec, "%s, cb->nid 0x%X, line%d\n", __func__,
+ (int)cb->nid, __LINE__);
+
+ tbl = snd_hda_jack_tbl_get(codec, cb->nid);
+ if (tbl)
+ tbl->block_report = 1;
+ schedule_delayed_work(&spec->unsol_linein_work, msecs_to_jiffies(200));
+}
+
+static void cm9825_unsol_lineout_delayed(struct work_struct *work)
+{
+ struct cmi_spec *spec =
+ container_of(to_delayed_work(work), struct cmi_spec,
+ unsol_lineout_work);
+ struct hda_jack_tbl *jack;
+
+ hda_nid_t lineout_pin = spec->gen.autocfg.line_out_pins[0];
+
+ bool lineout_jack_plugin = false;
+
+ lineout_jack_plugin = snd_hda_jack_detect(spec->codec, lineout_pin);
+
+ codec_dbg(spec->codec,
+ "%s, lineout_jack_plugin %d, lineout_pin 0x%X, line%d\n",
+ __func__, (int)lineout_jack_plugin, lineout_pin, __LINE__);
+
+ jack = snd_hda_jack_tbl_get(spec->codec, lineout_pin);
+
+ if (jack) {
+ jack->block_report = 0;
+ snd_hda_jack_report_sync(spec->codec);
+ }
+}
+
+static void lineout_callback(struct hda_codec *codec,
+ struct hda_jack_callback *cb)
+{
+ struct cmi_spec *spec = codec->spec;
+ struct hda_jack_tbl *tbl;
+
+ /* Delay enabling the line, to let the linein-detection
+ * state machine run.
+ */
+
+ codec_dbg(codec, "%s, cb->nid 0x%X, line%d\n", __func__,
+ (int)cb->nid, __LINE__);
+
+ tbl = snd_hda_jack_tbl_get(codec, cb->nid);
+ if (tbl)
+ tbl->block_report = 1;
+ schedule_delayed_work(&spec->unsol_lineout_work, msecs_to_jiffies(200));
+}
+
static void cm9825_setup_unsol(struct hda_codec *codec)
{
struct cmi_spec *spec = codec->spec;
hda_nid_t hp_pin = spec->gen.autocfg.hp_pins[0];
- snd_hda_jack_detect_enable_callback(codec, hp_pin, hp_callback);
+ hda_nid_t lineout_pin = spec->gen.autocfg.line_out_pins[0];
+
+ hda_nid_t linein_pin = spec->gen.autocfg.inputs[1].pin;
+
+ if (hp_pin != 0)
+ snd_hda_jack_detect_enable_callback(codec, hp_pin, hp_callback);
+
+ if (lineout_pin != 0)
+ snd_hda_jack_detect_enable_callback(codec, lineout_pin, lineout_callback);
+
+ if (linein_pin != 0)
+ snd_hda_jack_detect_enable_callback(codec, linein_pin, linein_callback);
+
+
+ codec_dbg(codec, "%s, hp_pin 0x%X, lineout_pin 0x%X, linein_pin 0x%X, line%d\n", __func__,
+ (int)hp_pin, lineout_pin, linein_pin, __LINE__);
+
+}
+
+static void cm9825_playback_pcm_hook(struct hda_pcm_stream *hinfo,
+ struct hda_codec *codec,
+ struct snd_pcm_substream *substream,
+ int action)
+{
+ struct cmi_spec *spec = codec->spec;
+
+ switch (action) {
+ case HDA_GEN_PCM_ACT_PREPARE:
+ snd_hda_sequence_write(spec->codec,
+ cm9825_gene_twl7_playback_start_verbs);
+ break;
+ case HDA_GEN_PCM_ACT_CLEANUP:
+ snd_hda_sequence_write(spec->codec,
+ cm9825_gene_twl7_playback_stop_verbs);
+ break;
+ default:
+ return;
+ }
}
static int cm9825_init(struct hda_codec *codec)
@@ -179,26 +378,61 @@ static int cm9825_init(struct hda_codec *codec)
return 0;
}
-static void cm9825_remove(struct hda_codec *codec)
+static void cm9825_cm_std_remove(struct hda_codec *codec)
{
struct cmi_spec *spec = codec->spec;
cancel_delayed_work_sync(&spec->unsol_hp_work);
+}
+
+static void cm9825_gene_twl7_remove(struct hda_codec *codec)
+{
+ struct cmi_spec *spec = codec->spec;
+
+ cancel_delayed_work_sync(&spec->unsol_linein_work);
+ cancel_delayed_work_sync(&spec->unsol_lineout_work);
+}
+
+static void cm9825_remove(struct hda_codec *codec)
+{
+ if (codec->core.subsystem_id == QUIRK_CM_STD)
+ cm9825_cm_std_remove(codec);
+ else if (codec->core.subsystem_id == QUIRK_GENE_TWL7_SSID)
+ cm9825_gene_twl7_remove(codec);
+
snd_hda_gen_remove(codec);
}
-static int cm9825_suspend(struct hda_codec *codec)
+static void cm9825_cm_std_suspend(struct hda_codec *codec)
{
struct cmi_spec *spec = codec->spec;
cancel_delayed_work_sync(&spec->unsol_hp_work);
+}
+
+static void cm9825_gene_twl7_suspend(struct hda_codec *codec)
+{
+ struct cmi_spec *spec = codec->spec;
+
+ cancel_delayed_work_sync(&spec->unsol_linein_work);
+ cancel_delayed_work_sync(&spec->unsol_lineout_work);
+}
+
+static int cm9825_suspend(struct hda_codec *codec)
+{
+ struct cmi_spec *spec = codec->spec;
+
+ if (codec->core.subsystem_id == QUIRK_CM_STD)
+ cm9825_cm_std_suspend(codec);
+ else if (codec->core.subsystem_id == QUIRK_GENE_TWL7_SSID)
+ cm9825_gene_twl7_suspend(codec);
snd_hda_sequence_write(codec, spec->chip_d3_verbs);
return 0;
}
-static int cm9825_resume(struct hda_codec *codec)
+static int cm9825_cm_std_resume(struct hda_codec *codec)
{
struct cmi_spec *spec = codec->spec;
hda_nid_t hp_pin = 0;
@@ -213,7 +447,7 @@ static int cm9825_resume(struct hda_codec *codec)
msleep(150); /* for depop noise */
- snd_hda_codec_init(codec);
+ snd_hda_sequence_write(codec, spec->chip_d0_verbs);
hp_pin = spec->gen.autocfg.hp_pins[0];
hp_jack_plugin = snd_hda_jack_detect(spec->codec, hp_pin);
@@ -232,6 +466,18 @@ static int cm9825_resume(struct hda_codec *codec)
snd_hda_sequence_write(codec, cm9825_hp_remove_verbs);
}
+ return 0;
+}
+
+static int cm9825_resume(struct hda_codec *codec)
+{
+ struct cmi_spec *spec = codec->spec;
+
+ if (codec->core.subsystem_id == QUIRK_CM_STD)
+ cm9825_cm_std_resume(codec);
+ else if (codec->core.subsystem_id == QUIRK_GENE_TWL7_SSID)
+ snd_hda_sequence_write(codec, spec->chip_d0_verbs);
+
snd_hda_regmap_sync(codec);
hda_call_check_power_status(codec, 0x01);
@@ -242,13 +488,15 @@ static int cm9825_probe(struct hda_codec *codec, const struct hda_device_id *id)
{
struct cmi_spec *spec;
struct auto_pin_cfg *cfg;
- int err;
+ int err = 0;
spec = kzalloc(sizeof(*spec), GFP_KERNEL);
if (spec == NULL)
return -ENOMEM;
- INIT_DELAYED_WORK(&spec->unsol_hp_work, cm9825_unsol_hp_delayed);
+ codec_dbg(codec, "chip_name: %s, ssid: 0x%X\n",
+ codec->core.chip_name, codec->core.subsystem_id);
+
codec->spec = spec;
spec->codec = codec;
cfg = &spec->gen.autocfg;
@@ -258,6 +506,36 @@ static int cm9825_probe(struct hda_codec *codec, const struct hda_device_id *id)
spec->chip_hp_present_verbs = cm9825_hp_present_verbs;
spec->chip_hp_remove_verbs = cm9825_hp_remove_verbs;
+ switch (codec->core.subsystem_id) {
+ case QUIRK_CM_STD:
+ snd_hda_codec_set_name(codec, "CM9825 STD");
+ INIT_DELAYED_WORK(&spec->unsol_hp_work,
+ cm9825_unsol_hp_delayed);
+ spec->chip_d0_verbs = cm9825_std_d0_verbs;
+ spec->chip_d3_verbs = cm9825_std_d3_verbs;
+ spec->chip_hp_present_verbs = cm9825_hp_present_verbs;
+ spec->chip_hp_remove_verbs = cm9825_hp_remove_verbs;
+ cm9825_setup_unsol(codec);
+ break;
+ case QUIRK_GENE_TWL7_SSID:
+ snd_hda_codec_set_name(codec, "CM9825 GENE_TWL7");
+ INIT_DELAYED_WORK(&spec->unsol_linein_work,
+ cm9825_unsol_linein_delayed);
+ INIT_DELAYED_WORK(&spec->unsol_lineout_work,
+ cm9825_unsol_lineout_delayed);
+ spec->chip_d0_verbs = cm9825_gene_twl7_d0_verbs;
+ spec->chip_d3_verbs = cm9825_gene_twl7_d3_verbs;
+ spec->gen.pcm_playback_hook = cm9825_playback_pcm_hook;
+ snd_hda_codec_set_pincfg(codec, 0x37, 0x24A70100);
+ break;
+ default:
+ err = -ENXIO;
+ break;
+ }
+
+ if (err < 0)
+ goto error;
+
snd_hda_sequence_write(codec, spec->chip_d0_verbs);
err = snd_hda_parse_pin_defcfg(codec, cfg, NULL, 0);
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] ALSA: hda/cm9825: Add new project GENE_TWL7 for AAEON
2025-12-15 11:22 Leo Tsai
@ 2025-12-15 12:15 ` Takashi Iwai
0 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2025-12-15 12:15 UTC (permalink / raw)
To: Leo Tsai; +Cc: perex, tiwai, rf, leo.tsai, linux-sound, linux-kernel
On Mon, 15 Dec 2025 12:22:46 +0100,
Leo Tsai wrote:
>
> The GENE_TWL7 project is an AAEON platform with a fixed audio
> configuration consisting of line-out, line-in, and mic-in.
> The audio routing and pin assignments are defined according to
> the board-level hardware design and are not intended to be
> dynamically changed.
>
> Signed-off-by: Leo Tsai <antivirus621@gmail.com>
It's good that you react quickly, and the patch looks much better
now. But don't rush; we have time.
Before resubmission, re-read your patch again and again. At each
change, you might have broken something else, too.
And, verify whether the patch really works as expected, as well as
verify it doesn't break other models.
Last but not least, please re-read what the previous reviews told.
You forgot the version prefix in the subject again, for example.
(And if there are fixes of the existing models, rather split it as
another fix patch as a preliminary.)
About the code changes:
> +/*
> + * Enable CLK/ADC to start recording.
> + */
> +static const struct hda_verb cm9825_gene_twl7_d0_verbs[] = {
> + {0x34, AC_VERB_SET_EAPD_BTLENABLE, 0x02},
In general, we set EAPD at init phase. It's OK to do it dynamically
here, though.
> + {0x43, CM9825_VERB_SET_SNR, 0x38},
> + {0x43, CM9825_VERB_SET_PLL, 0x00},
> + {0x43, CM9825_VERB_SET_ADCL, 0xcf},
> + {0x43, CM9825_VERB_SET_DACL, 0xaa},
> + {0x43, CM9825_VERB_SET_MBIAS, 0x1c},
> + {0x43, CM9825_VERB_SET_VNEG, 0x56},
> + {0x43, CM9825_VERB_SET_D2S, 0x62},
> + {0x43, CM9825_VERB_SET_DACTRL, 0x00},
> + {0x43, CM9825_VERB_SET_PDNEG, 0x0c},
> + {0x43, CM9825_VERB_SET_CDALR, 0xf4},
> + {0x43, CM9825_VERB_SET_OTP, 0xcd},
> + {0x43, CM9825_VERB_SET_MTCBA, 0x61},
> + {0x43, CM9825_VERB_SET_OCP, 0x33},
> + {0x43, CM9825_VERB_SET_GAD, 0x07},
> + {0x43, CM9825_VERB_SET_TMOD, 0x26},
> + {0x43, CM9825_VERB_SET_HPF_1, 0x40},
> + {0x43, CM9825_VERB_SET_HPF_2, 0x40},
> + {0x40, AC_VERB_SET_CONNECT_SEL, 0x00},
> + {0x3d, AC_VERB_SET_CONNECT_SEL, 0x01},
What's the reason to tweak the node connections here?
Isn't it wired properly beforehand?
> +static void cm9825_unsol_linein_delayed(struct work_struct *work)
> +{
> + struct cmi_spec *spec =
> + container_of(to_delayed_work(work), struct cmi_spec,
> + unsol_linein_work);
> + struct hda_jack_tbl *jack;
> +
> + hda_nid_t linein_pin = spec->gen.autocfg.inputs[1].pin;
This blindly assumes that this second input is the line-in.
It means that the primary input is mic-in, and mic-in doesn't need the
delayed jack detection...?
> +static void linein_callback(struct hda_codec *codec,
> + struct hda_jack_callback *cb)
> +{
> + struct cmi_spec *spec = codec->spec;
> + struct hda_jack_tbl *tbl;
> +
> + /* Delay enabling the line, to let the linein-detection
> + * state machine run.
> + */
> +
> + codec_dbg(codec, "%s, cb->nid 0x%X, line%d\n", __func__,
> + (int)cb->nid, __LINE__);
> +
> + tbl = snd_hda_jack_tbl_get(codec, cb->nid);
> + if (tbl)
> + tbl->block_report = 1;
> + schedule_delayed_work(&spec->unsol_linein_work, msecs_to_jiffies(200));
> +}
Basically for all three (hp, line-out, line-in), the code itself is
identical except for the pin ID. They can be unified somehow better.
> static void cm9825_setup_unsol(struct hda_codec *codec)
> {
> struct cmi_spec *spec = codec->spec;
>
> hda_nid_t hp_pin = spec->gen.autocfg.hp_pins[0];
>
> - snd_hda_jack_detect_enable_callback(codec, hp_pin, hp_callback);
> + hda_nid_t lineout_pin = spec->gen.autocfg.line_out_pins[0];
> +
> + hda_nid_t linein_pin = spec->gen.autocfg.inputs[1].pin;
Again, taking inputs[1] is flaky.
> static int cm9825_init(struct hda_codec *codec)
> @@ -179,26 +378,61 @@ static int cm9825_init(struct hda_codec *codec)
> return 0;
> }
>
> -static void cm9825_remove(struct hda_codec *codec)
> +static void cm9825_cm_std_remove(struct hda_codec *codec)
> {
> struct cmi_spec *spec = codec->spec;
>
> cancel_delayed_work_sync(&spec->unsol_hp_work);
> +}
> +
> +static void cm9825_gene_twl7_remove(struct hda_codec *codec)
> +{
> + struct cmi_spec *spec = codec->spec;
> +
> + cancel_delayed_work_sync(&spec->unsol_linein_work);
> + cancel_delayed_work_sync(&spec->unsol_lineout_work);
The initializations of those work rather depend on the pin
configuration, and strictly speaking, they aren't tied with the
models. e.g. user/BIOS may override the pin configurations.
That said, the cancel calls should be conditional for each work
instatiation. You can introduce some flags, for example.
> -static int cm9825_resume(struct hda_codec *codec)
> +static int cm9825_cm_std_resume(struct hda_codec *codec)
> {
> struct cmi_spec *spec = codec->spec;
> hda_nid_t hp_pin = 0;
> @@ -213,7 +447,7 @@ static int cm9825_resume(struct hda_codec *codec)
>
> msleep(150); /* for depop noise */
>
> - snd_hda_codec_init(codec);
> + snd_hda_sequence_write(codec, spec->chip_d0_verbs);
This really changes the behavior. Are you sure that you don't miss
anything? e.g. snd_hda_codec_init() calls snd_hda_gen_init(), which
will be lost by your patch.
> @@ -258,6 +506,36 @@ static int cm9825_probe(struct hda_codec *codec, const struct hda_device_id *id)
> spec->chip_hp_present_verbs = cm9825_hp_present_verbs;
> spec->chip_hp_remove_verbs = cm9825_hp_remove_verbs;
>
> + switch (codec->core.subsystem_id) {
> + case QUIRK_CM_STD:
> + snd_hda_codec_set_name(codec, "CM9825 STD");
> + INIT_DELAYED_WORK(&spec->unsol_hp_work,
> + cm9825_unsol_hp_delayed);
> + spec->chip_d0_verbs = cm9825_std_d0_verbs;
> + spec->chip_d3_verbs = cm9825_std_d3_verbs;
> + spec->chip_hp_present_verbs = cm9825_hp_present_verbs;
> + spec->chip_hp_remove_verbs = cm9825_hp_remove_verbs;
> + cm9825_setup_unsol(codec);
Why it's called here; cm9825_setup_unsol() is called later, no?
> + break;
> + case QUIRK_GENE_TWL7_SSID:
> + snd_hda_codec_set_name(codec, "CM9825 GENE_TWL7");
> + INIT_DELAYED_WORK(&spec->unsol_linein_work,
> + cm9825_unsol_linein_delayed);
> + INIT_DELAYED_WORK(&spec->unsol_lineout_work,
> + cm9825_unsol_lineout_delayed);
> + spec->chip_d0_verbs = cm9825_gene_twl7_d0_verbs;
> + spec->chip_d3_verbs = cm9825_gene_twl7_d3_verbs;
> + spec->gen.pcm_playback_hook = cm9825_playback_pcm_hook;
> + snd_hda_codec_set_pincfg(codec, 0x37, 0x24A70100);
Don't put a magic number. I guess it's a mic pin? Define it properly
with a comment.
thanks,
Takashi
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-12-15 12:15 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-15 8:14 [PATCH] ALSA: hda/cm9825: Add new project GENE_TWL7 for AAEON Leo Tsai
2025-12-15 8:36 ` Takashi Iwai
-- strict thread matches above, loose matches on Subject: below --
2025-12-15 11:22 Leo Tsai
2025-12-15 12:15 ` Takashi Iwai
2025-12-12 9:31 Leo Tsai
2025-12-14 10:30 ` Takashi Iwai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox