Re: [PATCH v3] ASoc: tas2781: Enable RCA-based playback without DSP firmware download

On Wed, May 29, 2024 at 06:35:41PM +0800, Shenghao Ding wrote:

In only RCA(Reconfigurable Architecture) binary case, no DSP program will

"...RCA (..."

be working inside tas2563/tas2781, that is dsp-bypass mode, do not support speaker protection, and audio acoustic algorithms in this mode.

Some minor issues below.

...

• if (tas_priv->fw_state == TASDEVICE_DSP_FW_FAIL) {

• dev_err(tas_priv->dev, "DSP bin file not loaded\n");
  

• /*

• * Only RCA-based Playback can still work with no dsp program running
  

• * inside the chip?
  

• */
  

• if (!(tas_priv->fw_state == TASDEVICE_RCA_FW_OK ||

• tas_priv->fw_state == TASDEVICE_DSP_FW_ALL_OK)) {
  

This line has broken indentation and I already pointed out a few times to such issues. It makes harder to read the code.

• dev_err(tas_priv->dev, "No firmware loaded\n");
  

  return; }

...

scnprintf(tas_priv->coef_binaryname, 64, "%s_coef.bin", tas_priv->dev_name);

Stray change?

ret = tasdevice_dsp_parser(tas_priv);

...

• if (tas_priv->fw_state == TASDEVICE_RCA_FW_OK) {

• /*If DSP FW fail, DSP kcontrol won't be created */
  

Mind spaces in the comment.

tasdevice_dsp_remove(tas_priv);

}

...

• 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)) {
  

Broken indentation.

• dev_err(tas_priv->dev, "Bin file not loaded\n");
  

• return -EINVAL;
  

  }