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

-enum tasdevice_dsp_fw_state {

• TASDEVICE_DSP_FW_NONE = 0,

+enum tasdevice_fw_state {

• /* Driver in startup mode, not load any firmware. */ TASDEVICE_DSP_FW_PENDING,
• /* DSP firmware in the system, but parsing error. */ TASDEVICE_DSP_FW_FAIL,
• /*

• * Only RCA (Reconfigurable Architecture) firmware load
  

• * successfully.
  

• */
  

• TASDEVICE_RCA_FW_OK,
• /* Both RCA and DSP firmware load successfully. */ TASDEVICE_DSP_FW_ALL_OK,

I appreciate the effort to document the states, but for the RCA cases we can have two 'success' states?

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

• /*

• * If no dsp-related kcontrol created, the dsp resource will be freed.
  

• */
  

• ret = tasdevice_dsp_create_ctrls(tas_priv);

• if (ret) {

• dev_err(tas_priv->dev, "dsp controls error\n");
  

• goto out;
  

• }

  tas_priv->fw_state = TASDEVICE_DSP_FW_ALL_OK;

from this code, it seems that the RCA case goes from RCA_FW_OK to TASDEVICE_DSP_FW_ALL_OK, so there's a difference between the two states, no?

@@ -466,14 +474,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;
  

• switch (tas_priv->fw_state) {
• case TASDEVICE_RCA_FW_OK:
• case TASDEVICE_DSP_FW_ALL_OK:

• return 0;
  

• default:

• return -EINVAL;
  

maybe keep the error logs?