| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID:
<TYZPR02MB77258E74B88C1C4205785708D8A8A@TYZPR02MB7725.apcprd02.prod.outlook.com>
Date: Thu, 18 Dec 2025 01:30:31 +0000
From: CM-Tsai Leo - 蔡紘紳 <leo.tsai@...dia.com.tw>
To: Takashi Iwai <tiwai@...e.de>, Leo Tsai <antivirus621@...il.com>
CC: "perex@...ex.cz" <perex@...ex.cz>, "tiwai@...e.com" <tiwai@...e.com>,
"rf@...nsource.cirrus.com" <rf@...nsource.cirrus.com>,
"linux-sound@...r.kernel.org" <linux-sound@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject:
回覆: [PATCH] [PATCH v5] ALSA: hda/cm9825: Add GENE_TWL7 support for AAEON
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@...e.de>
寄件日期: 2025年12月16日 下午 07:59
收件者: Leo Tsai <antivirus621@...il.com>
副本: perex@...ex.cz <perex@...ex.cz>; tiwai@...e.com <tiwai@...e.com>; rf@...nsource.cirrus.com <rf@...nsource.cirrus.com>; CM-Tsai Leo - 蔡紘紳 <leo.tsai@...dia.com.tw>; linux-sound@...r.kernel.org <linux-sound@...r.kernel.org>; linux-kernel@...r.kernel.org <linux-kernel@...r.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.』
Powered by blists - more mailing lists