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?

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.

• */
  

• 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

	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?

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.

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,