* [PATCH v5 1/3] ASoC: tas2781: Fix wrong loading calibrated data sequence
@ 2024-05-12 2:50 Shenghao Ding
2024-05-12 2:50 ` [PATCH v5 2/3] " Shenghao Ding
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Shenghao Ding @ 2024-05-12 2:50 UTC (permalink / raw
To: broonie
Cc: andriy.shevchenko, lgirdwood, perex, pierre-louis.bossart,
13916275206, alsa-devel, linux-kernel, liam.r.girdwood, bard.liao,
yung-chuan.liao, kevin-lu, cameron.berkenpas, tiwai, baojun.xu,
soyer, Baojun.Xu, Shenghao Ding
Remove declaration of unused API which load calibrated data in wrong
sequence, changed the copyright year and correct file name in license
header.
Fixes: ef3bcde75d06 ("ASoc: tas2781: Add tas2781 driver")
Signed-off-by: Shenghao Ding <shenghao-ding@ti.com>
---
include/sound/tas2781-dsp.h | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/include/sound/tas2781-dsp.h b/include/sound/tas2781-dsp.h
index ea9af2726a53..7fba7ea26a4b 100644
--- a/include/sound/tas2781-dsp.h
+++ b/include/sound/tas2781-dsp.h
@@ -2,7 +2,7 @@
//
// ALSA SoC Texas Instruments TAS2781 Audio Smart Amplifier
//
-// Copyright (C) 2022 - 2023 Texas Instruments Incorporated
+// Copyright (C) 2022 - 2024 Texas Instruments Incorporated
// https://www.ti.com
//
// The TAS2781 driver implements a flexible and configurable
@@ -13,8 +13,8 @@
// Author: Kevin Lu <kevin-lu@ti.com>
//
-#ifndef __TASDEVICE_DSP_H__
-#define __TASDEVICE_DSP_H__
+#ifndef __TAS2781_DSP_H__
+#define __TAS2781_DSP_H__
#define MAIN_ALL_DEVICES 0x0d
#define MAIN_DEVICE_A 0x01
@@ -180,7 +180,6 @@ void tasdevice_calbin_remove(void *context);
int tasdevice_select_tuningprm_cfg(void *context, int prm,
int cfg_no, int rca_conf_no);
int tasdevice_prmg_load(void *context, int prm_no);
-int tasdevice_prmg_calibdata_load(void *context, int prm_no);
void tasdevice_tuning_switch(void *context, int state);
int tas2781_load_calibration(void *context, char *file_name,
unsigned short i);
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v5 2/3] ASoC: tas2781: Fix wrong loading calibrated data sequence
2024-05-12 2:50 [PATCH v5 1/3] ASoC: tas2781: Fix wrong loading calibrated data sequence Shenghao Ding
@ 2024-05-12 2:50 ` Shenghao Ding
2024-05-13 9:59 ` Andy Shevchenko
2024-05-12 2:50 ` [PATCH v5 3/3] " Shenghao Ding
2024-05-13 9:58 ` [PATCH v5 1/3] " Andy Shevchenko
2 siblings, 1 reply; 6+ messages in thread
From: Shenghao Ding @ 2024-05-12 2:50 UTC (permalink / raw
To: broonie
Cc: andriy.shevchenko, lgirdwood, perex, pierre-louis.bossart,
13916275206, alsa-devel, linux-kernel, liam.r.girdwood, bard.liao,
yung-chuan.liao, kevin-lu, cameron.berkenpas, tiwai, baojun.xu,
soyer, Baojun.Xu, Shenghao Ding
Calibrated data will be set to default after loading DSP config params,
which will cause speaker protection work abnormally. Reload calibrated
data after loading DSP config params.
Fixes: ef3bcde75d06 ("ASoc: tas2781: Add tas2781 driver")
Signed-off-by: Shenghao Ding <shenghao-ding@ti.com>
---
v5:
- No change.
v4:
- Use the the culprit of the bug itself as the fixes tag.
- Better variant for tasdev_load_calibrated_data in order to much easier
to read and understand and maintain, as it makes harder to squeeze the
code.
- Fix the indentation and move operator to the previous line.
v3:
- Remove redundant return in tasdev_load_calibrated_data
- Put the second function parameter into the previous line for
tasdev_load_calibrated_data
- | Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202405021200.YHInjV43-lkp@intel.com/
v2:
- In the Subject, fixed --> Fix
- In tas2781-fmwlib.c, tasdevice-fmw.c ---> tas2781-fmwlib.c
- dsp --> DSP
- Remove unneeded parentheses for & (dereference) operator
- Add Fixes tag
v1:
- Download calibrated data after loading the new DSP config params
- call tasdevice_prmg_load instead of tasdevice_prmg_calibdata_load, it
is unnecessary to load calibrated data after loading DSP program. Load
it after loading DSP config params each time.
- Remove tasdevice_prmg_calibdata_load, because it is unnecessary to load
calibrated data after loading DSP program.
---
sound/soc/codecs/tas2781-fmwlib.c | 103 ++++++++----------------------
1 file changed, 27 insertions(+), 76 deletions(-)
diff --git a/sound/soc/codecs/tas2781-fmwlib.c b/sound/soc/codecs/tas2781-fmwlib.c
index a6be81adcb83..265a8ca25cbb 100644
--- a/sound/soc/codecs/tas2781-fmwlib.c
+++ b/sound/soc/codecs/tas2781-fmwlib.c
@@ -2151,6 +2151,24 @@ static int tasdevice_load_data(struct tasdevice_priv *tas_priv,
return ret;
}
+static void tasdev_load_calibrated_data(struct tasdevice_priv *priv, int i)
+{
+ struct tasdevice_calibration *cal;
+ struct tasdevice_fw *cal_fmw;
+
+ cal_fmw = priv->tasdevice[i].cali_data_fmw;
+
+ /* No calibrated data for current devices, playback will go ahead. */
+ if (!cal_fmw)
+ return;
+
+ cal = cal_fmw->calibrations;
+ if (cal)
+ return;
+
+ load_calib_data(priv, &cal->dev_data);
+}
+
int tasdevice_select_tuningprm_cfg(void *context, int prm_no,
int cfg_no, int rca_conf_no)
{
@@ -2210,21 +2228,9 @@ int tasdevice_select_tuningprm_cfg(void *context, int prm_no,
for (i = 0; i < tas_priv->ndev; i++) {
if (tas_priv->tasdevice[i].is_loaderr == true)
continue;
- else if (tas_priv->tasdevice[i].is_loaderr == false
- && tas_priv->tasdevice[i].is_loading == true) {
- struct tasdevice_fw *cal_fmw =
- tas_priv->tasdevice[i].cali_data_fmw;
-
- if (cal_fmw) {
- struct tasdevice_calibration
- *cal = cal_fmw->calibrations;
-
- if (cal)
- load_calib_data(tas_priv,
- &(cal->dev_data));
- }
+ if (tas_priv->tasdevice[i].is_loaderr == false &&
+ tas_priv->tasdevice[i].is_loading == true)
tas_priv->tasdevice[i].cur_prog = prm_no;
- }
}
}
@@ -2245,11 +2251,15 @@ int tasdevice_select_tuningprm_cfg(void *context, int prm_no,
tasdevice_load_data(tas_priv, &(conf->dev_data));
for (i = 0; i < tas_priv->ndev; i++) {
if (tas_priv->tasdevice[i].is_loaderr == true) {
- status |= 1 << (i + 4);
+ status |= BIT(i + 4);
continue;
- } else if (tas_priv->tasdevice[i].is_loaderr == false
- && tas_priv->tasdevice[i].is_loading == true)
+ }
+
+ if (tas_priv->tasdevice[i].is_loaderr == false &&
+ tas_priv->tasdevice[i].is_loading == true) {
+ tasdev_load_calibrated_data(tas_priv, i);
tas_priv->tasdevice[i].cur_conf = cfg_no;
+ }
}
} else
dev_dbg(tas_priv->dev, "%s: Unneeded loading dsp conf %d\n",
@@ -2308,65 +2318,6 @@ int tasdevice_prmg_load(void *context, int prm_no)
}
EXPORT_SYMBOL_NS_GPL(tasdevice_prmg_load, SND_SOC_TAS2781_FMWLIB);
-int tasdevice_prmg_calibdata_load(void *context, int prm_no)
-{
- struct tasdevice_priv *tas_priv = (struct tasdevice_priv *) context;
- struct tasdevice_fw *tas_fmw = tas_priv->fmw;
- struct tasdevice_prog *program;
- int prog_status = 0;
- int i;
-
- if (!tas_fmw) {
- dev_err(tas_priv->dev, "%s: Firmware is NULL\n", __func__);
- goto out;
- }
-
- if (prm_no >= tas_fmw->nr_programs) {
- dev_err(tas_priv->dev,
- "%s: prm(%d) is not in range of Programs %u\n",
- __func__, prm_no, tas_fmw->nr_programs);
- goto out;
- }
-
- for (i = 0, prog_status = 0; i < tas_priv->ndev; i++) {
- if (prm_no >= 0 && tas_priv->tasdevice[i].cur_prog != prm_no) {
- tas_priv->tasdevice[i].cur_conf = -1;
- tas_priv->tasdevice[i].is_loading = true;
- prog_status++;
- }
- tas_priv->tasdevice[i].is_loaderr = false;
- }
-
- if (prog_status) {
- program = &(tas_fmw->programs[prm_no]);
- tasdevice_load_data(tas_priv, &(program->dev_data));
- for (i = 0; i < tas_priv->ndev; i++) {
- if (tas_priv->tasdevice[i].is_loaderr == true)
- continue;
- else if (tas_priv->tasdevice[i].is_loaderr == false
- && tas_priv->tasdevice[i].is_loading == true) {
- struct tasdevice_fw *cal_fmw =
- tas_priv->tasdevice[i].cali_data_fmw;
-
- if (cal_fmw) {
- struct tasdevice_calibration *cal =
- cal_fmw->calibrations;
-
- if (cal)
- load_calib_data(tas_priv,
- &(cal->dev_data));
- }
- tas_priv->tasdevice[i].cur_prog = prm_no;
- }
- }
- }
-
-out:
- return prog_status;
-}
-EXPORT_SYMBOL_NS_GPL(tasdevice_prmg_calibdata_load,
- SND_SOC_TAS2781_FMWLIB);
-
void tasdevice_tuning_switch(void *context, int state)
{
struct tasdevice_priv *tas_priv = (struct tasdevice_priv *) context;
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v5 3/3] ASoC: tas2781: Fix wrong loading calibrated data sequence
2024-05-12 2:50 [PATCH v5 1/3] ASoC: tas2781: Fix wrong loading calibrated data sequence Shenghao Ding
2024-05-12 2:50 ` [PATCH v5 2/3] " Shenghao Ding
@ 2024-05-12 2:50 ` Shenghao Ding
2024-05-13 9:58 ` [PATCH v5 1/3] " Andy Shevchenko
2 siblings, 0 replies; 6+ messages in thread
From: Shenghao Ding @ 2024-05-12 2:50 UTC (permalink / raw
To: broonie
Cc: andriy.shevchenko, lgirdwood, perex, pierre-louis.bossart,
13916275206, alsa-devel, linux-kernel, liam.r.girdwood, bard.liao,
yung-chuan.liao, kevin-lu, cameron.berkenpas, tiwai, baojun.xu,
soyer, Baojun.Xu, Shenghao Ding
Use the API with correct sequence instead of the wrong one.
Fixes: ef3bcde75d06 ("ASoc: tas2781: Add tas2781 driver")
Signed-off-by: Shenghao Ding <shenghao-ding@ti.com>
---
v5:
- correct changelog has no much relationship with the patch
v4:
- Use the the culprit of the bug itself as the fixes tag
v3:
- No changes.
v2:
- In the Subject, fixed --> Fix
- dsp --> DSP
- Add Fixes tag
- Changed the copyright year to 2024 in the related files
v1:
- Download calibrated data after loading the new DSP config params
- Call tasdevice_prmg_load instead of tasdevice_prmg_calibdata_load, it
is unnecessary to load calibrated data after loading DSP program. Load
it after loading DSP config params each time.
---
sound/soc/codecs/tas2781-i2c.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/tas2781-i2c.c b/sound/soc/codecs/tas2781-i2c.c
index b5abff230e43..9350972dfefe 100644
--- a/sound/soc/codecs/tas2781-i2c.c
+++ b/sound/soc/codecs/tas2781-i2c.c
@@ -2,7 +2,7 @@
//
// ALSA SoC Texas Instruments TAS2563/TAS2781 Audio Smart Amplifier
//
-// Copyright (C) 2022 - 2023 Texas Instruments Incorporated
+// Copyright (C) 2022 - 2024 Texas Instruments Incorporated
// https://www.ti.com
//
// The TAS2563/TAS2781 driver implements a flexible and configurable
@@ -414,7 +414,7 @@ static void tasdevice_fw_ready(const struct firmware *fmw,
__func__, tas_priv->cal_binaryname[i]);
}
- tasdevice_prmg_calibdata_load(tas_priv, 0);
+ tasdevice_prmg_load(tas_priv, 0);
tas_priv->cur_prog = 0;
out:
if (tas_priv->fw_state == TASDEVICE_DSP_FW_FAIL) {
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v5 1/3] ASoC: tas2781: Fix wrong loading calibrated data sequence
2024-05-12 2:50 [PATCH v5 1/3] ASoC: tas2781: Fix wrong loading calibrated data sequence Shenghao Ding
2024-05-12 2:50 ` [PATCH v5 2/3] " Shenghao Ding
2024-05-12 2:50 ` [PATCH v5 3/3] " Shenghao Ding
@ 2024-05-13 9:58 ` Andy Shevchenko
2 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2024-05-13 9:58 UTC (permalink / raw
To: Shenghao Ding
Cc: broonie, lgirdwood, perex, pierre-louis.bossart, 13916275206,
alsa-devel, linux-kernel, liam.r.girdwood, bard.liao,
yung-chuan.liao, kevin-lu, cameron.berkenpas, tiwai, baojun.xu,
soyer, Baojun.Xu
On Sun, May 12, 2024 at 10:50:37AM +0800, Shenghao Ding wrote:
> Remove declaration of unused API which load calibrated data in wrong
> sequence, changed the copyright year and correct file name in license
> header.
> -int tasdevice_prmg_calibdata_load(void *context, int prm_no);
This is still being used in two files. How won't this break the build?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v5 2/3] ASoC: tas2781: Fix wrong loading calibrated data sequence
2024-05-12 2:50 ` [PATCH v5 2/3] " Shenghao Ding
@ 2024-05-13 9:59 ` Andy Shevchenko
2024-05-14 6:31 ` [EXTERNAL] " Ding, Shenghao
0 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2024-05-13 9:59 UTC (permalink / raw
To: Shenghao Ding
Cc: broonie, lgirdwood, perex, pierre-louis.bossart, 13916275206,
alsa-devel, linux-kernel, liam.r.girdwood, bard.liao,
yung-chuan.liao, kevin-lu, cameron.berkenpas, tiwai, baojun.xu,
soyer, Baojun.Xu
On Sun, May 12, 2024 at 10:50:38AM +0800, Shenghao Ding wrote:
> Calibrated data will be set to default after loading DSP config params,
> which will cause speaker protection work abnormally. Reload calibrated
> data after loading DSP config params.
...
> -int tasdevice_prmg_calibdata_load(void *context, int prm_no)
> -EXPORT_SYMBOL_NS_GPL(tasdevice_prmg_calibdata_load,
> - SND_SOC_TAS2781_FMWLIB);
AFAICS the i2c glue driver still uses this, how can't this break the build?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [EXTERNAL] Re: [PATCH v5 2/3] ASoC: tas2781: Fix wrong loading calibrated data sequence
2024-05-13 9:59 ` Andy Shevchenko
@ 2024-05-14 6:31 ` Ding, Shenghao
0 siblings, 0 replies; 6+ messages in thread
From: Ding, Shenghao @ 2024-05-14 6:31 UTC (permalink / raw
To: Andy Shevchenko
Cc: broonie@kernel.org, lgirdwood@gmail.com, perex@perex.cz,
pierre-louis.bossart@linux.intel.com, 13916275206@139.com,
alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org,
liam.r.girdwood@intel.com, bard.liao@intel.com,
yung-chuan.liao@linux.intel.com, Lu, Kevin,
cameron.berkenpas@gmail.com, tiwai@suse.de, Xu, Baojun,
soyer@irl.hu, Baojun.Xu@fpt.com
Hi Andy
Thanks for your comment.
> -----Original Message-----
> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Sent: Monday, May 13, 2024 6:00 PM
> To: Ding, Shenghao <shenghao-ding@ti.com>
> Cc: broonie@kernel.org; lgirdwood@gmail.com; perex@perex.cz; pierre-
> louis.bossart@linux.intel.com; 13916275206@139.com; alsa-devel@alsa-
> project.org; linux-kernel@vger.kernel.org; liam.r.girdwood@intel.com;
> bard.liao@intel.com; yung-chuan.liao@linux.intel.com; Lu, Kevin <kevin-
> lu@ti.com>; cameron.berkenpas@gmail.com; tiwai@suse.de; Xu, Baojun
> <baojun.xu@ti.com>; soyer@irl.hu; Baojun.Xu@fpt.com
> Subject: [EXTERNAL] Re: [PATCH v5 2/3] ASoC: tas2781: Fix wrong loading
> calibrated data sequence
>
> On Sun, May 12, 2024 at 10:50:38AM +0800, Shenghao Ding wrote:
> > Calibrated data will be set to default after loading DSP config
> > params, which will cause speaker protection work abnormally. Reload
> > calibrated data after loading DSP config params.
>
> ...
>
> > -int tasdevice_prmg_calibdata_load(void *context, int prm_no)
>
> > -EXPORT_SYMBOL_NS_GPL(tasdevice_prmg_calibdata_load,
> > - SND_SOC_TAS2781_FMWLIB);
>
> AFAICS the i2c glue driver still uses this, how can't this break the build?
[PATCH v5 3/3] has removed tasdevice_prmg_calibdata_load, and
tasdevice_prmg_load(tas_priv, 0) has been called instead of
tasdevice_prmg_calibdata_load
>
> --
> With Best Regards,
> Andy Shevchenko
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-05-14 6:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-12 2:50 [PATCH v5 1/3] ASoC: tas2781: Fix wrong loading calibrated data sequence Shenghao Ding
2024-05-12 2:50 ` [PATCH v5 2/3] " Shenghao Ding
2024-05-13 9:59 ` Andy Shevchenko
2024-05-14 6:31 ` [EXTERNAL] " Ding, Shenghao
2024-05-12 2:50 ` [PATCH v5 3/3] " Shenghao Ding
2024-05-13 9:58 ` [PATCH v5 1/3] " Andy Shevchenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).