linux-kernel - RE: [EXTERNAL] Re: [RESEND PATCH v4] 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]
Message-ID: <c56ec5c5bd7349a788251230deead24a@ti.com>
Date: Wed, 12 Jun 2024 11:51:55 +0000
From: "Ding, Shenghao" <shenghao-ding@...com>
To: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>,
        "broonie@...nel.org" <broonie@...nel.org>
CC: "andriy.shevchenko@...ux.intel.com" <andriy.shevchenko@...ux.intel.com>,
        "lgirdwood@...il.com" <lgirdwood@...il.com>,
        "perex@...ex.cz"
	<perex@...ex.cz>,
        "13916275206@....com" <13916275206@....com>,
        "zhourui@...qin.com" <zhourui@...qin.com>,
        "alsa-devel@...a-project.org"
	<alsa-devel@...a-project.org>,
        "Salazar, Ivan" <i-salazar@...com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Chadha,
 Jasjot Singh" <j-chadha@...com>,
        "liam.r.girdwood@...el.com"
	<liam.r.girdwood@...el.com>,
        "Yue, Jaden" <jaden-yue@...com>,
        "yung-chuan.liao@...ux.intel.com" <yung-chuan.liao@...ux.intel.com>,
        "Rao,
 Dipa" <dipa@...com>, "yuhsuan@...gle.com" <yuhsuan@...gle.com>,
        "Lo, Henry"
	<henry.lo@...com>, "tiwai@...e.de" <tiwai@...e.de>,
        "Xu, Baojun"
	<baojun.xu@...com>, "soyer@....hu" <soyer@....hu>,
        "Baojun.Xu@....com"
	<Baojun.Xu@....com>,
        "judyhsiao@...gle.com" <judyhsiao@...gle.com>,
        "Navada
 Kanyana, Mukund" <navada@...com>,
        "cujomalainey@...gle.com"
	<cujomalainey@...gle.com>,
        "Kutty, Aanya" <aanya@...com>,
        "Mahmud, Nayeem"
	<nayeem.mahmud@...com>,
        "savyasanchi.shukla@...radyne.com"
	<savyasanchi.shukla@...radyne.com>,
        "flaviopr@...rosoft.com"
	<flaviopr@...rosoft.com>,
        "Ji, Jesse" <jesse-ji@...com>,
        "darren.ye@...iatek.com" <darren.ye@...iatek.com>
Subject: RE: [EXTERNAL] Re: [RESEND PATCH v4] ASoc: tas2781: Enable RCA-based
 playback without DSP firmware download

Hi Pierre
Sorry for missing your valuable review comments, thanks for Broonie's remind.

> -----Original Message-----
> From: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
> Sent: Thursday, June 6, 2024 9:09 PM
> To: Ding, Shenghao <shenghao-ding@...com>; broonie@...nel.org
> Cc: andriy.shevchenko@...ux.intel.com; lgirdwood@...il.com;
> perex@...ex.cz; 13916275206@....com; zhourui@...qin.com; alsa-
> devel@...a-project.org; Salazar, Ivan <i-salazar@...com>; linux-
> kernel@...r.kernel.org; Chadha, Jasjot Singh <j-chadha@...com>;
> liam.r.girdwood@...el.com; Yue, Jaden <jaden-yue@...com>; yung-
> chuan.liao@...ux.intel.com; Rao, Dipa <dipa@...com>; yuhsuan@...gle.com;
> Lo, Henry <henry.lo@...com>; tiwai@...e.de; Xu, Baojun <baojun.xu@...com>;
> soyer@....hu; Baojun.Xu@....com; judyhsiao@...gle.com; Navada Kanyana,
> Mukund <navada@...com>; cujomalainey@...gle.com; Kutty, Aanya
> <aanya@...com>; Mahmud, Nayeem <nayeem.mahmud@...com>;
> savyasanchi.shukla@...radyne.com; flaviopr@...rosoft.com; Ji, Jesse <jesse-
> ji@...com>; darren.ye@...iatek.com
> Subject: [EXTERNAL] Re: [RESEND PATCH v4] ASoc: tas2781: Enable RCA-based
> playback without DSP firmware download
> 
> I am afraid there are still circular logic issues, or the code/comments don't
> capture what you are trying to do. . . . > diff --git a/include/sound/tas2781-
> dsp. h b/include/sound/tas2781-dsp. h > index 7fba7ea26a4b. . 3cda9da14f6d
> 100644 > ZjQcmQRYFpfptBannerStart This message was sent from outside of
> Texas Instruments.
> Do not click links or open attachments unless you recognize the source of this
> email and know the content is safe. If you wish to report this message to IT
> Security, please forward the message as an attachment to phishing@...t.ti.com
> 
> ZjQcmQRYFpfptBannerEnd
> I am afraid there are still circular logic issues, or the code/comments don't
> capture what you are trying to do....
> 
> 
..................................
> ... and here this TASDEVICE_RCA_FW_OK really means a fail.
> 
> so how can [1] consider TASDEVICE_RCA_FW_OK as a success case?
> 
> Or this this saying that the baseline is the RCA case, and then the code
> attempts to load firmware but in case of failures just keep going, i.e.
> failing to load firmware is NOT an error?
Correct.
> 
> That would be somewhat different to the commit title that says 'without DSP
> firmware download'.
> 
> Would you mind clarifying the steps please?
There's two bin files for tas2781, one is register settings(RCA bin file), the other is the dsp firmware and filter coeff.
If no RCA bin file is load, the tas2781 can't work, it will be TASDEVICE_DSP_FW_FAIL.
If only RCA bin file load, the tas2781 will work in bypass mode, which dsp do not work, neither spk protection nor acoustic 
algorithm is enabled 
(TASDEVICE_RCA_FW_OK).
If both RCA bin and dsp firmware are loaded, that is TASDEVICE_DSP_FW_ALL_OK, tas2781 work in dsp mode, both spk protection
and acoustic algorithm are enabled
> 
> 
> >  		tasdevice_dsp_remove(tas_priv);
> >  	}
> >  	mutex_unlock(&tas_priv->codec_lock);
> > @@ -466,14 +474,14 @@ static int tasdevice_startup(struct
> > snd_pcm_substream *substream,  {
> >  	struct snd_soc_component *codec = dai->component;
> >  	struct tasdevice_priv TASDEVICE_RCA_FW_OK*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;
> >  	}
> > -
> > -	return ret;
> >  }
> >
> >  static int tasdevice_hw_params(struct snd_pcm_substream *substream,