sound/hda/codecs/cm9825.c | 219 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 210 insertions(+), 9 deletions(-)
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.0On 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
© 2016 - 2026 Red Hat, Inc.