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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Wed, 29 May 2024 18:07:29 +0300
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Shenghao Ding <shenghao-ding@...com>
Cc: broonie@...nel.org, lgirdwood@...il.com, perex@...ex.cz,
	pierre-louis.bossart@...ux.intel.com, 13916275206@....com,
	alsa-devel@...a-project.org, i-salazar@...com,
	linux-kernel@...r.kernel.org, j-chadha@...com,
	liam.r.girdwood@...el.com, jaden-yue@...com,
	yung-chuan.liao@...ux.intel.com, dipa@...com, kevin-lu@...com,
	yuhsuan@...gle.com, tiwai@...e.de, baojun.xu@...com, soyer@....hu,
	Baojun.Xu@....com, judyhsiao@...gle.com, navada@...com,
	cujomalainey@...gle.com, aanya@...com, nayeem.mahmud@...com,
	savyasanchi.shukla@...radyne.com, flaviopr@...rosoft.com
Subject: 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;
>  	}

-- 
With Best Regards,
Andy Shevchenko