| 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: <87wm2o6u1g.wl-tiwai@suse.de>
Date: Mon, 15 Dec 2025 09:14:35 +0100
From: Takashi Iwai <tiwai@...e.de>
To: CM-Tsai Leo - 蔡紘紳
<leo.tsai@...dia.com.tw>
Cc: Takashi Iwai <tiwai@...e.de>,
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>,
"linux-sound@...r.kernel.org" <linux-sound@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: 回覆: [PATCH] ALSA: hda/cm9825:
Add new project GENE_TWL7 for AAEON
On Mon, 15 Dec 2025 04:26:25 +0100,
CM-Tsai Leo - 蔡紘紳 wrote:
>
> Dear Takashi,
>
> Sorry for the patch we sent you was unclear.
>
> AAEON is one of our customers, and GENE_TWL7 is a project of their team.
> We confirmed with other customers a few days ago that only AAEON will not require further changes, so we decided to proceed with AAEON’s patch first.
>
>
>
> > @@ -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 */
>
> We found AC_AMP_SET_OUTPUT | AC_AMP_SET_RIGHT is not 0xa0 and AC_AMP_SET_OUTPUT | AC_AMP_SET_LEFT is not 0x90, so replaced the symbols with numbers.
> Please let me know if there is a better way to express this.
If there is a correction of verbs for the existing models, better to
split to another patch just for the correction. Then it can be
applied to the existing releases as a stable fix, too.
But yet, passing numbers like 0xa0 or 0x90 looks weird, and it needs
more commends why those values are set.
>
> > +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},
> > + {}
> > +};
>
> The IC design team told us that these settings are related to the internal operation of the IC and are somewhat complex, so they only provided us with the intended purpose.
>
> /*
> * 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},
> {}
> };
>
> Above are the added comments. Please let me know if this is acceptable or not.
They look better now.
> > 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;
>
>
> In the GENE_TWL7 project, 0x36 pin widget is used as the line-out
> pin widget.
And yet it's handled as a headphone?
> We will add another pin widget specifically for line-out with a
> distinct name to avoid confusion with the headphone (HP) pin.
Well, it's very confusing. You have two line-outs, and one of them is
treated as a headphone, or what? You need to show a "big picture".
> > +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);
>
> This setting is used to enable support for the CTIA/OMTP interface,
> which was missing in the previous patch.
If so, it should be corrected by another patch instead of folding into
a patch for a new model.
thanks,
Takashi
Powered by blists - more mailing lists