| 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: <a9c424859b8544fa8ae56b7a30150317@ti.com>
Date: Tue, 28 May 2024 03:10:21 +0000
From: "Ding, Shenghao" <shenghao-ding@...com>
To: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>,
"broonie@...nel.org" <broonie@...nel.org>
CC: "andriy.shevchenko@...ux.intel.com" <andriy.shevchenko@...ux.intel.com>,
"lgirdwood@...il.com" <lgirdwood@...il.com>,
"perex@...ex.cz"
<perex@...ex.cz>,
"13916275206@....com" <13916275206@....com>,
"alsa-devel@...a-project.org" <alsa-devel@...a-project.org>,
"Salazar, Ivan"
<i-salazar@...com>,
"linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>,
"Chadha, Jasjot Singh" <j-chadha@...com>,
"liam.r.girdwood@...el.com" <liam.r.girdwood@...el.com>,
"Yue, Jaden"
<jaden-yue@...com>,
"yung-chuan.liao@...ux.intel.com"
<yung-chuan.liao@...ux.intel.com>,
"Rao, Dipa" <dipa@...com>, "Lu, Kevin"
<kevin-lu@...com>,
"yuhsuan@...gle.com" <yuhsuan@...gle.com>,
"tiwai@...e.de"
<tiwai@...e.de>, "Xu, Baojun" <baojun.xu@...com>,
"soyer@....hu"
<soyer@....hu>,
"Baojun.Xu@....com" <Baojun.Xu@....com>,
"judyhsiao@...gle.com" <judyhsiao@...gle.com>,
"cujomalainey@...gle.com"
<cujomalainey@...gle.com>,
"Kutty, Aanya" <aanya@...com>,
"Mahmud, Nayeem"
<nayeem.mahmud@...com>,
"Navada Kanyana, Mukund" <navada@...com>
Subject: RE: [EXTERNAL] Re: [PATCH v2] ASoc: tas2781: Playback can work when
only RCA binary loading without dsp firmware loading
Thanks for your comments.
> -----Original Message-----
> From: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
> Sent: Monday, May 27, 2024 9:44 PM
> To: Ding, Shenghao <shenghao-ding@...com>; broonie@...nel.org
> Cc: andriy.shevchenko@...ux.intel.com; lgirdwood@...il.com;
> perex@...ex.cz; 13916275206@....com; alsa-devel@...a-project.org;
> Salazar, Ivan <i-salazar@...com>; linux-kernel@...r.kernel.org; Chadha,
> Jasjot Singh <j-chadha@...com>; liam.r.girdwood@...el.com; Yue, Jaden
> <jaden-yue@...com>; yung-chuan.liao@...ux.intel.com; Rao, Dipa
> <dipa@...com>; Lu, Kevin <kevin-lu@...com>; yuhsuan@...gle.com;
> tiwai@...e.de; Xu, Baojun <baojun.xu@...com>; soyer@....hu;
> Baojun.Xu@....com; judyhsiao@...gle.com; Navada Kanyana, Mukund
> <navada@...com>; cujomalainey@...gle.com; Kutty, Aanya
> <aanya@...com>; Mahmud, Nayeem <nayeem.mahmud@...com>
> Subject: [EXTERNAL] Re: [PATCH v2] ASoc: tas2781: Playback can work when
> only RCA binary loading without dsp firmware loading
>
>
>
> On 5/24/24 20:47, Shenghao Ding wrote:
> > In only RCA binary loading case, only default dsp program inside the
> > chip will be work.
>
> What does 'RCA' stand for?
Reconfigurable architecture binary file
>
> Also clarify the commit title without double negatives, e.g.
>
> "Enable RCA-based playback without DSP firmware download"
> > - if (tas_priv->fw_state == TASDEVICE_DSP_FW_FAIL) {
> > - dev_err(tas_priv->dev, "DSP bin file not loaded\n");
> > + /*
> > + * Only RCA file loaded can still work with default dsp program inside
> > + * the chip?
>
> reword the commit and remove question mark.
accept
>
> > + */
> > + if (!(tas_priv->fw_state == TASDEVICE_RCA_FW_OK ||
> > + tas_priv->fw_state == TASDEVICE_DSP_FW_ALL_OK)) {
> > + dev_err(tas_priv->dev, "No firmware loaded\n");
> > return;
> > }
> >
> > if (state == 0) {
> > - if (tas_priv->cur_prog < tas_fmw->nr_programs) {
> > + if (tas_fmw && tas_priv->cur_prog < tas_fmw->nr_programs)
> {
> > /*dsp mode or tuning mode*/
>
> spaces in comments
accept
>
> > profile_cfg_id = tas_priv->rcabin.profile_cfg_id;
> > tasdevice_select_tuningprm_cfg(tas_priv,
> > @@ -2340,9 +2345,10 @@ void tasdevice_tuning_switch(void *context, int
> > state)
> >
> > tasdevice_select_cfg_blk(tas_priv, profile_cfg_id,
> > TASDEVICE_BIN_BLK_PRE_POWER_UP);
> > - } else
> > + } else {
> > tasdevice_select_cfg_blk(tas_priv, profile_cfg_id,
> > TASDEVICE_BIN_BLK_PRE_SHUTDOWN);
> > + }
> > }
> > EXPORT_SYMBOL_NS_GPL(tasdevice_tuning_switch,
> > SND_SOC_TAS2781_FMWLIB);
> > diff --git a/sound/soc/codecs/tas2781-i2c.c
> > b/sound/soc/codecs/tas2781-i2c.c index 9350972dfefe..ccb9313ada9b
> > 100644
> > --- a/sound/soc/codecs/tas2781-i2c.c
> > +++ b/sound/soc/codecs/tas2781-i2c.c
> > @@ -380,23 +380,30 @@ static void tasdevice_fw_ready(const struct
> firmware *fmw,
> > mutex_lock(&tas_priv->codec_lock);
> >
> > ret = tasdevice_rca_parser(tas_priv, fmw);
> > - if (ret)
> > + if (ret) {
> > + tasdevice_config_info_remove(tas_priv);
> > goto out;
> > + }
> > tasdevice_create_control(tas_priv);
> >
> > tasdevice_dsp_remove(tas_priv);
> > tasdevice_calbin_remove(tas_priv);
> > - tas_priv->fw_state = TASDEVICE_DSP_FW_PENDING;
> > + tas_priv->fw_state = TASDEVICE_RCA_FW_OK;
> > scnprintf(tas_priv->coef_binaryname, 64, "%s_coef.bin",
> > tas_priv->dev_name);
> > +
> > ret = tasdevice_dsp_parser(tas_priv);
> > if (ret) {
> > dev_err(tas_priv->dev, "dspfw load %s error\n",
> > tas_priv->coef_binaryname);
> > - tas_priv->fw_state = TASDEVICE_DSP_FW_FAIL;
> > goto out;
> > }
> > - tasdevice_dsp_create_ctrls(tas_priv);
> > +
> > + ret = tasdevice_dsp_create_ctrls(tas_priv);
> > + if (ret) {
> > + dev_err(tas_priv->dev, "dsp controls error\n");
> > + goto out;
> > + }
>
> this seems unrelated to the boot process? Move to a different patch?
It is a part of boot process, if no dsp-related kcontrol created, the dsp resource will be freed.
>
> >
> > tas_priv->fw_state = TASDEVICE_DSP_FW_ALL_OK;
> >
> > @@ -417,9 +424,8 @@ static void tasdevice_fw_ready(const struct
> firmware *fmw,
> > tasdevice_prmg_load(tas_priv, 0);
> > tas_priv->cur_prog = 0;
> > out:
> > - if (tas_priv->fw_state == TASDEVICE_DSP_FW_FAIL) {
> > - /*If DSP FW fail, kcontrol won't be created */
> > - tasdevice_config_info_remove(tas_priv);
> > + if (tas_priv->fw_state == TASDEVICE_RCA_FW_OK) {
> > + /*If DSP FW fail, DSP kcontrol won't be created */
>
> It looks like you're no longer using PENDING and FAIL states?
> The state machine is becoming really hard to follow.
Remove unused states.
>
> > tasdevice_dsp_remove(tas_priv);
> > }
> > mutex_unlock(&tas_priv->codec_lock);
> > @@ -466,14 +472,14 @@ static int tasdevice_startup(struct
> > snd_pcm_substream *substream, {
> > struct snd_soc_component *codec = dai->component;
> > struct tasdevice_priv *tas_priv =
> snd_soc_component_get_drvdata(codec);
> > - int ret = 0;
> >
> > - if (tas_priv->fw_state != TASDEVICE_DSP_FW_ALL_OK) {
> > - dev_err(tas_priv->dev, "DSP bin file not loaded\n");
> > - ret = -EINVAL;
> > + if (!(tas_priv->fw_state == TASDEVICE_DSP_FW_ALL_OK ||
> > + tas_priv->fw_state == TASDEVICE_RCA_FW_OK)) {
> > + dev_err(tas_priv->dev, "Bin file not loaded\n");
> > + return -EINVAL;
> > }
> >
> > - return ret;
> > + return 0;
> > }
> >
> > static int tasdevice_hw_params(struct snd_pcm_substream *substream,
Powered by blists - more mailing lists