[PATCH v1] ASoc: tas2781: Add name_prefix as the prefix name of DSP firmwares and calibrated data files

[PATCH v1] ASoc: tas2781: Add name_prefix as the prefix name of DSP firmwares and calibrated data files

[Shenghao Ding]

Add name_prefix as the prefix name of DSP firmwares
and calibrated data files which stored speaker
calibrated impedance.

Signed-off-by: Shenghao Ding <shenghao-ding@ti.com>
---
 sound/soc/codecs/tas2781-i2c.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/sound/soc/codecs/tas2781-i2c.c b/sound/soc/codecs/tas2781-i2c.c
index 4d1a0d836e77..cc765d45c6b5 100644
--- a/sound/soc/codecs/tas2781-i2c.c
+++ b/sound/soc/codecs/tas2781-i2c.c
@@ -394,8 +394,12 @@ static void tasdevice_fw_ready(const struct firmware *fmw,
 	 * failing to load DSP firmware is NOT an error.
 	 */
 	tas_priv->fw_state = TASDEVICE_RCA_FW_OK;
-	scnprintf(tas_priv->coef_binaryname, 64, "%s_coef.bin",
-		tas_priv->dev_name);
+	if (tas_priv->name_prefix)
+		scnprintf(tas_priv->rca_binaryname, 64, "%s-%s_coef.bin",
+			tas_priv->name_prefix, tas_priv->dev_name);
+	else
+		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",
@@ -418,8 +422,15 @@ static void tasdevice_fw_ready(const struct firmware *fmw,
 	 * calibrated data inside algo.
 	 */
 	for (i = 0; i < tas_priv->ndev; i++) {
-		scnprintf(tas_priv->cal_binaryname[i], 64, "%s_cal_0x%02x.bin",
-			tas_priv->dev_name, tas_priv->tasdevice[i].dev_addr);
+		if (tas_priv->name_prefix)
+			scnprintf(tas_priv->cal_binaryname[i], 64,
+				"%s-%s_cal_0x%02x.bin", tas_priv->name_prefix,
+				tas_priv->dev_name,
+				tas_priv->tasdevice[i].dev_addr);
+		else
+			scnprintf(tas_priv->cal_binaryname[i], 64,
+				"%s_cal_0x%02x.bin", tas_priv->dev_name,
+				tas_priv->tasdevice[i].dev_addr);
 		ret = tas2781_load_calibration(tas_priv,
 			tas_priv->cal_binaryname[i], i);
 		if (ret != 0)
-- 
2.34.1

Re: [PATCH v1] ASoc: tas2781: Add name_prefix as the prefix name of DSP firmwares and calibrated data files

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 572 bytes --]

On Sat, Jun 29, 2024 at 06:11:10PM +0800, Shenghao Ding wrote:

>  	tas_priv->fw_state = TASDEVICE_RCA_FW_OK;
> -	scnprintf(tas_priv->coef_binaryname, 64, "%s_coef.bin",
> -		tas_priv->dev_name);
> +	if (tas_priv->name_prefix)
> +		scnprintf(tas_priv->rca_binaryname, 64, "%s-%s_coef.bin",
> +			tas_priv->name_prefix, tas_priv->dev_name);
> +	else
> +		scnprintf(tas_priv->coef_binaryname, 64, "%s_coef.bin",
> +			tas_priv->dev_name);

I'll apply this but I do wonder if it's worth falling back to trying to
load the unprefixed name if we fail to load the prefixed one.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

RE: [EXTERNAL] Re: [PATCH v1] ASoc: tas2781: Add name_prefix as the prefix name of DSP firmwares and calibrated data files

[Ding, Shenghao]

Hi Brown
Thanks for your comment.

> -----Original Message-----
> From: Mark Brown <broonie@kernel.org>
> Sent: Monday, July 1, 2024 8:23 PM
> To: Ding, Shenghao <shenghao-ding@ti.com>
> Cc: andriy.shevchenko@linux.intel.com; lgirdwood@gmail.com;
> perex@perex.cz; pierre-louis.bossart@linux.intel.com;
> 13916275206@139.com; zhourui@huaqin.com; alsa-devel@alsa-project.org;
> Salazar, Ivan <i-salazar@ti.com>; linux-kernel@vger.kernel.org; Chadha,
> Jasjot Singh <j-chadha@ti.com>; liam.r.girdwood@intel.com; Yue, Jaden
> <jaden-yue@ti.com>; yung-chuan.liao@linux.intel.com; Rao, Dipa
> <dipa@ti.com>; yuhsuan@google.com; Lo, Henry <henry.lo@ti.com>;
> tiwai@suse.de; Xu, Baojun <baojun.xu@ti.com>; soyer@irl.hu;
> Baojun.Xu@fpt.com; judyhsiao@google.com; Navada Kanyana, Mukund
> <navada@ti.com>; cujomalainey@google.com; Kutty, Aanya
> <aanya@ti.com>; Mahmud, Nayeem <nayeem.mahmud@ti.com>;
> savyasanchi.shukla@netradyne.com; flaviopr@microsoft.com; Ji, Jesse
> <jesse-ji@ti.com>; darren.ye@mediatek.com
> Subject: [EXTERNAL] Re: [PATCH v1] ASoc: tas2781: Add name_prefix as the
> prefix name of DSP firmwares and calibrated data files
> 
> On Sat, Jun 29, 2024 at 06:11:10PM +0800, Shenghao Ding wrote:
> 
> >  	tas_priv->fw_state = TASDEVICE_RCA_FW_OK;
> > -	scnprintf(tas_priv->coef_binaryname, 64, "%s_coef.bin",
> > -		tas_priv->dev_name);
> > +	if (tas_priv->name_prefix)
> > +		scnprintf(tas_priv->rca_binaryname, 64, "%s-%s_coef.bin",
> > +			tas_priv->name_prefix, tas_priv->dev_name);
> > +	else
> > +		scnprintf(tas_priv->coef_binaryname, 64, "%s_coef.bin",
> > +			tas_priv->dev_name);
> 
> I'll apply this but I do wonder if it's worth falling back to trying to load the
> unprefixed name if we fail to load the prefixed one.
If fail to load dsp firmware, the driver won't load unprefixed name firmware, 
but switch tas2563/tas2781 to bypass-dsp mode automatically.
In this mode, smartamp become simple amp.
These day, I met a case from one of my customers, they put 2 pieces of tas2563, 
and 2 pieces of tas2781 in the same i2c bus. In order to identify tas2563 and 
tas2781, I think name_prefix is a good solution for this case.
Looking forward to your comment. Thanks.

Re: [EXTERNAL] Re: [PATCH v1] ASoc: tas2781: Add name_prefix as the prefix name of DSP firmwares and calibrated data files

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 943 bytes --]

On Mon, Jul 01, 2024 at 02:00:13PM +0000, Ding, Shenghao wrote:

> > I'll apply this but I do wonder if it's worth falling back to trying to load the
> > unprefixed name if we fail to load the prefixed one.

> If fail to load dsp firmware, the driver won't load unprefixed name firmware, 
> but switch tas2563/tas2781 to bypass-dsp mode automatically.
> In this mode, smartamp become simple amp.
> These day, I met a case from one of my customers, they put 2 pieces of tas2563, 
> and 2 pieces of tas2781 in the same i2c bus. In order to identify tas2563 and 
> tas2781, I think name_prefix is a good solution for this case.
> Looking forward to your comment. Thanks.

Yes, the name_prefix is a good idea and probably people want things
specifically tuned for the DSP - I was thinking about error handling or
upgrade cases where wrong calibration might work better.  Bypass mode
means the device will still function at least.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

RE: [EXTERNAL] Re: [PATCH v1] ASoc: tas2781: Add name_prefix as the prefix name of DSP firmwares and calibrated data files

[Ding, Shenghao]

Hi Brown
Thanks for your comment. Feedback is inline.
> -----Original Message-----
> From: Mark Brown <broonie@kernel.org>
> Sent: Tuesday, July 2, 2024 1:06 AM
> To: Ding, Shenghao <shenghao-ding@ti.com>
> Cc: andriy.shevchenko@linux.intel.com; lgirdwood@gmail.com;
> perex@perex.cz; pierre-louis.bossart@linux.intel.com;
> 13916275206@139.com; zhourui@huaqin.com; alsa-devel@alsa-project.org;
> Salazar, Ivan <i-salazar@ti.com>; linux-kernel@vger.kernel.org; Chadha,
> Jasjot Singh <j-chadha@ti.com>; liam.r.girdwood@intel.com; Yue, Jaden
> <jaden-yue@ti.com>; yung-chuan.liao@linux.intel.com; Rao, Dipa
> <dipa@ti.com>; yuhsuan@google.com; Lo, Henry <henry.lo@ti.com>;
> tiwai@suse.de; Xu, Baojun <baojun.xu@ti.com>; soyer@irl.hu;
> Baojun.Xu@fpt.com; judyhsiao@google.com; Navada Kanyana, Mukund
> <navada@ti.com>; cujomalainey@google.com; Kutty, Aanya
> <aanya@ti.com>; Mahmud, Nayeem <nayeem.mahmud@ti.com>;
> savyasanchi.shukla@netradyne.com; flaviopr@microsoft.com; Ji, Jesse
> <jesse-ji@ti.com>; darren.ye@mediatek.com
> Subject: Re: [EXTERNAL] Re: [PATCH v1] ASoc: tas2781: Add name_prefix as
> the prefix name of DSP firmwares and calibrated data files
> 
> On Mon, Jul 01, 2024 at 02:00:13PM +0000, Ding, Shenghao wrote:
> 
> > > I'll apply this but I do wonder if it's worth falling back to trying
> > > to load the unprefixed name if we fail to load the prefixed one.
> 
> > If fail to load dsp firmware, the driver won't load unprefixed name
> > firmware, but switch tas2563/tas2781 to bypass-dsp mode automatically.
> > In this mode, smartamp become simple amp.
> > These day, I met a case from one of my customers, they put 2 pieces of
> > tas2563, and 2 pieces of tas2781 in the same i2c bus. In order to
> > identify tas2563 and tas2781, I think name_prefix is a good solution for this
> case.
> > Looking forward to your comment. Thanks.
> 
> Yes, the name_prefix is a good idea and probably people want things
> specifically tuned for the DSP - I was thinking about error handling or
> upgrade cases where wrong calibration might work better.  Bypass mode
> means the device will still function at least.
In bypass mode, tas2563/tas2781 can still work without speaker protection or audio acoustic function.
In case of only dsp firnmware loading without calibrated data, tas2563/tas2781 still can work with
default calibrated data and default audio acoustic parameter.

Re: [PATCH v1] ASoc: tas2781: Add name_prefix as the prefix name of DSP firmwares and calibrated data files

[Mark Brown]

On Sat, 29 Jun 2024 18:11:10 +0800, Shenghao Ding wrote:
> Add name_prefix as the prefix name of DSP firmwares
> and calibrated data files which stored speaker
> calibrated impedance.
> 
> 

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoc: tas2781: Add name_prefix as the prefix name of DSP firmwares and calibrated data files
      commit: 55f0a1fa6ea4e339c797e9a3292ca0caa4ab3885

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark