> -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?
