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

[Andy Shevchenko]

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;
>  	}

-- 
With Best Regards,
Andy Shevchenko