* [PATCH] [PATCH v5] ALSA: hda/cm9825: Add GENE_TWL7 support for AAEON
@ 2025-12-16 8:01 Leo Tsai
2025-12-16 11:59 ` Takashi Iwai
0 siblings, 1 reply; 3+ messages in thread
From: Leo Tsai @ 2025-12-16 8:01 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 | 308 +++++++++++++++++++++++++++++++++++++-
1 file changed, 301 insertions(+), 7 deletions(-)
diff --git a/sound/hda/codecs/cm9825.c b/sound/hda/codecs/cm9825.c
index 5c474ce44348..569bbde2212d 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,7 +48,12 @@ 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_inputs_work;
+ struct delayed_work unsol_lineout_work;
struct delayed_work unsol_hp_work;
+ bool jd_cap_hp;
+ bool jd_cap_lineout;
+ bool jd_cap_inputs[AUTO_CFG_MAX_INS];
int quirk;
};
@@ -111,6 +122,140 @@ 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 bool cm9825_jd_cap(struct hda_codec *codec, hda_nid_t pin_id)
+{
+ int cap;
+
+ cap =
+ snd_hdac_read_parm(&codec->core, pin_id,
+ AC_PAR_PIN_CAP) & AC_PINCAP_PRES_DETECT;
+
+ codec_dbg(codec, "%s, cap 0x%X, line%d\n", __func__, cap, __LINE__);
+
+ return (bool)cap;
+}
+
+static void cm9825_unsol_inputs_delayed(struct work_struct *work)
+{
+ struct cmi_spec *spec =
+ container_of(to_delayed_work(work), struct cmi_spec,
+ unsol_inputs_work);
+ struct hda_jack_tbl *jack;
+ int i;
+ bool input_jack_plugin;
+
+ for (i = 0; i < spec->gen.autocfg.num_inputs; i++) {
+ if (!spec->jd_cap_inputs[i])
+ continue;
+
+ input_jack_plugin =
+ snd_hda_jack_detect(spec->codec,
+ spec->gen.autocfg.inputs[i].pin);
+ jack =
+ snd_hda_jack_tbl_get(spec->codec,
+ spec->gen.autocfg.inputs[i].pin);
+ if (jack) {
+ jack->block_report = 0;
+ snd_hda_jack_report_sync(spec->codec);
+ }
+
+ codec_dbg(spec->codec,
+ "%s, input_jack_plugin %d, linein_pin 0x%X, line%d\n",
+ __func__, (int)input_jack_plugin,
+ spec->gen.autocfg.inputs[i].pin, __LINE__);
+ }
+}
+
+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;
+ bool lineout_jack_plugin;
+
+ hda_nid_t lineout_pin = spec->gen.autocfg.line_out_pins[0];
+
+ lineout_jack_plugin = snd_hda_jack_detect(spec->codec, lineout_pin);
+
+ jack = snd_hda_jack_tbl_get(spec->codec, lineout_pin);
+
+ if (jack) {
+ jack->block_report = 0;
+ snd_hda_jack_report_sync(spec->codec);
+ }
+
+ codec_dbg(spec->codec,
+ "%s, lineout_jack_plugin %d, lineout_pin 0x%X, line%d\n",
+ __func__, (int)lineout_jack_plugin, lineout_pin, __LINE__);
+}
+
static void cm9825_unsol_hp_delayed(struct work_struct *work)
{
struct cmi_spec *spec =
@@ -159,16 +304,92 @@ static void hp_callback(struct hda_codec *codec, struct hda_jack_callback *cb)
tbl = snd_hda_jack_tbl_get(codec, cb->nid);
if (tbl)
tbl->block_report = 1;
- schedule_delayed_work(&spec->unsol_hp_work, msecs_to_jiffies(200));
+
+ if (cb->nid == spec->gen.autocfg.hp_pins[0] && spec->jd_cap_hp)
+ schedule_delayed_work(&spec->unsol_hp_work,
+ msecs_to_jiffies(200));
+ else if (cb->nid == spec->gen.autocfg.line_out_pins[0]
+ && spec->jd_cap_lineout)
+ schedule_delayed_work(&spec->unsol_lineout_work,
+ msecs_to_jiffies(200));
+
+ for (int i = 0; i < spec->gen.autocfg.num_inputs; i++) {
+ if (cb->nid == spec->gen.autocfg.inputs[i].pin
+ && spec->jd_cap_inputs[i])
+ schedule_delayed_work(&spec->unsol_inputs_work,
+ msecs_to_jiffies(200));
+ }
}
static void cm9825_setup_unsol(struct hda_codec *codec)
{
struct cmi_spec *spec = codec->spec;
+ int i;
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];
+
+ if (hp_pin != 0) {
+ spec->jd_cap_hp = cm9825_jd_cap(codec, hp_pin);
+ if (spec->jd_cap_hp)
+ snd_hda_jack_detect_enable_callback(codec, hp_pin,
+ hp_callback);
+ } else
+ spec->jd_cap_hp = false;
+
+ if (lineout_pin != 0) {
+ spec->jd_cap_lineout = cm9825_jd_cap(codec, lineout_pin);
+ if (spec->jd_cap_lineout)
+ snd_hda_jack_detect_enable_callback(codec, lineout_pin,
+ hp_callback);
+ } else
+ spec->jd_cap_lineout = false;
+
+ codec_dbg(codec,
+ "%s, hp_pin 0x%X, jd_cap_hp %d lineout_pin 0x%X, jd_cap_lineout %d, line%d\n",
+ __func__, (int)hp_pin, (int)spec->jd_cap_hp, (int)lineout_pin,
+ (int)spec->jd_cap_lineout, __LINE__);
+
+ for (i = 0; i < spec->gen.autocfg.num_inputs; i++) {
+ if (spec->gen.autocfg.inputs[i].pin != 0) {
+ spec->jd_cap_inputs[i] =
+ (bool)cm9825_jd_cap(codec,
+ spec->gen.autocfg.inputs[i].pin
+ );
+ if (spec->jd_cap_inputs[i])
+ snd_hda_jack_detect_enable_callback(codec,
+ spec->gen.autocfg.inputs[i].pin,
+ hp_callback);
+ } else
+ spec->jd_cap_inputs[i] = false;
+
+ codec_dbg(codec,
+ "%s, input pin[%d]: 0x%X, jd_cap_inputs %d, line%d\n",
+ __func__, i, spec->gen.autocfg.inputs[i].pin,
+ (int)spec->jd_cap_inputs[i], __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)
@@ -182,23 +403,48 @@ static int cm9825_init(struct hda_codec *codec)
static void cm9825_remove(struct hda_codec *codec)
{
struct cmi_spec *spec = codec->spec;
+ int i;
+
+ if (spec->jd_cap_hp)
+ cancel_delayed_work_sync(&spec->unsol_hp_work);
+
+ if (spec->jd_cap_lineout)
+ cancel_delayed_work_sync(&spec->unsol_lineout_work);
+
+ for (i = 0; i < spec->gen.autocfg.num_inputs; i++) {
+ if (spec->jd_cap_inputs[i]) {
+ cancel_delayed_work_sync(&spec->unsol_inputs_work);
+ break;
+ }
+ }
- cancel_delayed_work_sync(&spec->unsol_hp_work);
snd_hda_gen_remove(codec);
}
static int cm9825_suspend(struct hda_codec *codec)
{
struct cmi_spec *spec = codec->spec;
+ int i;
- cancel_delayed_work_sync(&spec->unsol_hp_work);
+ if (spec->jd_cap_hp)
+ cancel_delayed_work_sync(&spec->unsol_hp_work);
+
+ if (spec->jd_cap_lineout)
+ cancel_delayed_work_sync(&spec->unsol_lineout_work);
+
+ for (i = 0; i < spec->gen.autocfg.num_inputs; i++) {
+ if (spec->jd_cap_inputs[i]) {
+ cancel_delayed_work_sync(&spec->unsol_inputs_work);
+ break;
+ }
+ }
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;
@@ -215,6 +461,8 @@ static int cm9825_resume(struct hda_codec *codec)
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 +480,20 @@ 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_codec_init(codec);
+ snd_hda_sequence_write(codec, spec->chip_d0_verbs);
+ }
+
snd_hda_regmap_sync(codec);
hda_call_check_power_status(codec, 0x01);
@@ -242,13 +504,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 +522,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;
+ break;
+ case QUIRK_GENE_TWL7_SSID:
+ snd_hda_codec_set_name(codec, "CM9825 GENE_TWL7");
+ INIT_DELAYED_WORK(&spec->unsol_inputs_work,
+ cm9825_unsol_inputs_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;
+ /* Internal fixed device, Rear, Mic-in, 3.5mm */
+ 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] 3+ messages in thread
* Re: [PATCH] [PATCH v5] ALSA: hda/cm9825: Add GENE_TWL7 support for AAEON
2025-12-16 8:01 [PATCH] [PATCH v5] ALSA: hda/cm9825: Add GENE_TWL7 support for AAEON Leo Tsai
@ 2025-12-16 11:59 ` Takashi Iwai
2025-12-18 1:30 ` 回覆: " CM-Tsai Leo - 蔡紘紳
0 siblings, 1 reply; 3+ messages in thread
From: Takashi Iwai @ 2025-12-16 11:59 UTC (permalink / raw)
To: Leo Tsai; +Cc: perex, tiwai, rf, leo.tsai, linux-sound, linux-kernel
On Tue, 16 Dec 2025 09:01:48 +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.
Now it's getting butter, but you can give a bit more details, too.
e.g. mention about the workarounds and the extra code that are added
for this model, and explain why they are needed and how they work.
> +/*
> + * 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},
What do these connections do? IOW, why it wouldn't work without those
explicit setups?
> +
> +static bool cm9825_jd_cap(struct hda_codec *codec, hda_nid_t pin_id)
> +{
> + int cap;
> +
> + cap =
> + snd_hdac_read_parm(&codec->core, pin_id,
> + AC_PAR_PIN_CAP) & AC_PINCAP_PRES_DETECT;
> +
> + codec_dbg(codec, "%s, cap 0x%X, line%d\n", __func__, cap, __LINE__);
> +
> + return (bool)cap;
> +}
I guess you can use the existing is_jack_detectable() function.
It covers more tests.
And, instead of bool, you can keep the pin id in jd_cap_hp, and co.
Then, cm9825_unsol_hp_delayed() can be more simplified like:
if (cb->nid == spec->jd_cap_hp)
schedule_delayed_work(&spec->unsol_hp_work,
msecs_to_jiffies(200));
else if (cb->nid == spec->jp_cap_lineout)
schedule_delayed_work(&spec->unsol_lineout_work,
msecs_to_jiffies(200));
....
> +static void cm9825_unsol_inputs_delayed(struct work_struct *work)
> +{
> + struct cmi_spec *spec =
> + container_of(to_delayed_work(work), struct cmi_spec,
> + unsol_inputs_work);
> + struct hda_jack_tbl *jack;
> + int i;
> + bool input_jack_plugin;
> +
> + for (i = 0; i < spec->gen.autocfg.num_inputs; i++) {
> + if (!spec->jd_cap_inputs[i])
> + continue;
> +
> + input_jack_plugin =
> + snd_hda_jack_detect(spec->codec,
> + spec->gen.autocfg.inputs[i].pin);
> + jack =
> + snd_hda_jack_tbl_get(spec->codec,
> + spec->gen.autocfg.inputs[i].pin);
> + if (jack) {
> + jack->block_report = 0;
> + snd_hda_jack_report_sync(spec->codec);
> + }
> +
> + codec_dbg(spec->codec,
> + "%s, input_jack_plugin %d, linein_pin 0x%X, line%d\n",
> + __func__, (int)input_jack_plugin,
> + spec->gen.autocfg.inputs[i].pin, __LINE__);
> + }
This code pattern
jack_plugin = snd_hda_jack_detect(codec, pin);
jack = snd_hda_jack_tbl_get(codec, pin);
if (jack) {
jack->block_report = 0;
snd_hda_jack_report_sync(codec);
}
codec_dbug(codec, ....);
is used in multiple places in your patch, so it can be a common
function.
> @@ -242,13 +504,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 +522,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;
> + break;
> + case QUIRK_GENE_TWL7_SSID:
> + snd_hda_codec_set_name(codec, "CM9825 GENE_TWL7");
> + INIT_DELAYED_WORK(&spec->unsol_inputs_work,
> + cm9825_unsol_inputs_delayed);
> + INIT_DELAYED_WORK(&spec->unsol_lineout_work,
> + cm9825_unsol_lineout_delayed);
Basically all those delayed works can be initialized unconditionally.
It's safer than leaving as uninitialized.
thanks,
Takashi
^ permalink raw reply [flat|nested] 3+ messages in thread
* 回覆: [PATCH] [PATCH v5] ALSA: hda/cm9825: Add GENE_TWL7 support for AAEON
2025-12-16 11:59 ` Takashi Iwai
@ 2025-12-18 1:30 ` CM-Tsai Leo - 蔡紘紳
0 siblings, 0 replies; 3+ messages in thread
From: CM-Tsai Leo - 蔡紘紳 @ 2025-12-18 1:30 UTC (permalink / raw)
To: Takashi Iwai, Leo Tsai
Cc: perex@perex.cz, tiwai@suse.com, rf@opensource.cirrus.com,
linux-sound@vger.kernel.org, linux-kernel@vger.kernel.org
Dear Takashi,
> +/*
> + * 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},
Got it, we will add more comments.
> +
> +static bool cm9825_jd_cap(struct hda_codec *codec, hda_nid_t pin_id)
> +{
> + int cap;
> +
> + cap =
> + snd_hdac_read_parm(&codec->core, pin_id,
> + AC_PAR_PIN_CAP) & AC_PINCAP_PRES_DETECT;
> +
> + codec_dbg(codec, "%s, cap 0x%X, line%d\n", __func__, cap, __LINE__);
> +
> + return (bool)cap;
> +}
Got it, we will use is_jack_detectable() to instead of cm9825_jd_cap() and keep the pin id in jd_cap_hp.
> +static void cm9825_unsol_inputs_delayed(struct work_struct *work)
> +{
> + struct cmi_spec *spec =
> + container_of(to_delayed_work(work), struct cmi_spec,
> + unsol_inputs_work);
> + struct hda_jack_tbl *jack;
> + int i;
> + bool input_jack_plugin;
> +
> + for (i = 0; i < spec->gen.autocfg.num_inputs; i++) {
> + if (!spec->jd_cap_inputs[i])
> + continue;
> +
> + input_jack_plugin =
> + snd_hda_jack_detect(spec->codec,
> + spec->gen.autocfg.inputs[i].pin);
> + jack =
> + snd_hda_jack_tbl_get(spec->codec,
> + spec->gen.autocfg.inputs[i].pin);
> + if (jack) {
> + jack->block_report = 0;
> + snd_hda_jack_report_sync(spec->codec);
> + }
> +
> + codec_dbg(spec->codec,
> + "%s, input_jack_plugin %d, linein_pin 0x%X, line%d\n",
> + __func__, (int)input_jack_plugin,
> + spec->gen.autocfg.inputs[i].pin, __LINE__);
> + }
Got it, we will refactor this repeated code into a common function.
> @@ -242,13 +504,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 +522,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;
> + break;
> + case QUIRK_GENE_TWL7_SSID:
> + snd_hda_codec_set_name(codec, "CM9825 GENE_TWL7");
> + INIT_DELAYED_WORK(&spec->unsol_inputs_work,
> + cm9825_unsol_inputs_delayed);
> + INIT_DELAYED_WORK(&spec->unsol_lineout_work,
> + cm9825_unsol_lineout_delayed);
Got it, we will initialize all delayed works unconditionally.
Thanks,
Leo Tsai #853
________________________________________
寄件者: Takashi Iwai <tiwai@suse.de>
寄件日期: 2025年12月16日 下午 07:59
收件者: Leo Tsai <antivirus621@gmail.com>
副本: perex@perex.cz <perex@perex.cz>; tiwai@suse.com <tiwai@suse.com>; rf@opensource.cirrus.com <rf@opensource.cirrus.com>; CM-Tsai Leo - 蔡紘紳 <leo.tsai@cmedia.com.tw>; linux-sound@vger.kernel.org <linux-sound@vger.kernel.org>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>
主旨: Re: [PATCH] [PATCH v5] ALSA: hda/cm9825: Add GENE_TWL7 support for AAEON
On Tue, 16 Dec 2025 09:01:48 +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.
Now it's getting butter, but you can give a bit more details, too.
e.g. mention about the workarounds and the extra code that are added
for this model, and explain why they are needed and how they work.
> +/*
> + * 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},
What do these connections do? IOW, why it wouldn't work without those
explicit setups?
> +
> +static bool cm9825_jd_cap(struct hda_codec *codec, hda_nid_t pin_id)
> +{
> + int cap;
> +
> + cap =
> + snd_hdac_read_parm(&codec->core, pin_id,
> + AC_PAR_PIN_CAP) & AC_PINCAP_PRES_DETECT;
> +
> + codec_dbg(codec, "%s, cap 0x%X, line%d\n", __func__, cap, __LINE__);
> +
> + return (bool)cap;
> +}
I guess you can use the existing is_jack_detectable() function.
It covers more tests.
And, instead of bool, you can keep the pin id in jd_cap_hp, and co.
Then, cm9825_unsol_hp_delayed() can be more simplified like:
if (cb->nid == spec->jd_cap_hp)
schedule_delayed_work(&spec->unsol_hp_work,
msecs_to_jiffies(200));
else if (cb->nid == spec->jp_cap_lineout)
schedule_delayed_work(&spec->unsol_lineout_work,
msecs_to_jiffies(200));
....
> +static void cm9825_unsol_inputs_delayed(struct work_struct *work)
> +{
> + struct cmi_spec *spec =
> + container_of(to_delayed_work(work), struct cmi_spec,
> + unsol_inputs_work);
> + struct hda_jack_tbl *jack;
> + int i;
> + bool input_jack_plugin;
> +
> + for (i = 0; i < spec->gen.autocfg.num_inputs; i++) {
> + if (!spec->jd_cap_inputs[i])
> + continue;
> +
> + input_jack_plugin =
> + snd_hda_jack_detect(spec->codec,
> + spec->gen.autocfg.inputs[i].pin);
> + jack =
> + snd_hda_jack_tbl_get(spec->codec,
> + spec->gen.autocfg.inputs[i].pin);
> + if (jack) {
> + jack->block_report = 0;
> + snd_hda_jack_report_sync(spec->codec);
> + }
> +
> + codec_dbg(spec->codec,
> + "%s, input_jack_plugin %d, linein_pin 0x%X, line%d\n",
> + __func__, (int)input_jack_plugin,
> + spec->gen.autocfg.inputs[i].pin, __LINE__);
> + }
This code pattern
jack_plugin = snd_hda_jack_detect(codec, pin);
jack = snd_hda_jack_tbl_get(codec, pin);
if (jack) {
jack->block_report = 0;
snd_hda_jack_report_sync(codec);
}
codec_dbug(codec, ....);
is used in multiple places in your patch, so it can be a common
function.
> @@ -242,13 +504,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 +522,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;
> + break;
> + case QUIRK_GENE_TWL7_SSID:
> + snd_hda_codec_set_name(codec, "CM9825 GENE_TWL7");
> + INIT_DELAYED_WORK(&spec->unsol_inputs_work,
> + cm9825_unsol_inputs_delayed);
> + INIT_DELAYED_WORK(&spec->unsol_lineout_work,
> + cm9825_unsol_lineout_delayed);
Basically all those delayed works can be initialized unconditionally.
It's safer than leaving as uninitialized.
thanks,
Takashi
『The information contained in this email and attachment is confidential and is for the use of the intended recipient only. Any disclosure, copying or distribution of this email and attachment without the sender's consent is strictly prohibited. If you are not the intended recipient, please promptly notify the sender and delete this email and attachment entirely without using, retaining, or disclosing any of its contents. The recipient is responsible for ensuring that this email is virus free and the sender accepts no liability for any damages caused by virus transmitted by this email.』
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-12-18 6:05 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-16 8:01 [PATCH] [PATCH v5] ALSA: hda/cm9825: Add GENE_TWL7 support for AAEON Leo Tsai
2025-12-16 11:59 ` Takashi Iwai
2025-12-18 1:30 ` 回覆: " CM-Tsai Leo - 蔡紘紳
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox