diff mbox series

[11/16] ASoC: tas2781: use 0 as default prog/conf index

Message ID 88229933b7aaf0777cbe611979712e4e144b1ca1.1701906455.git.soyer@irl.hu (mailing list archive)
State Superseded
Headers show
Series ALSA: hda/tas2781: Add tas2563 support | expand

Commit Message

Gergo Koteles Dec. 7, 2023, 12:04 a.m. UTC
Invalid indexes are not the best default values.

Signed-off-by: Gergo Koteles <soyer@irl.hu>
---
 sound/soc/codecs/tas2781-comlib.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Mark Brown Dec. 7, 2023, 6:28 p.m. UTC | #1
On Thu, Dec 07, 2023 at 01:04:27AM +0100, Gergo Koteles wrote:

> Invalid indexes are not the best default values.

I'm guessing this is just fallout from the previous (not really
explained patch)?  Is there perhaps some bootstrapping issue here with
ensuring that the program and configuration get written to the device if
the user doesn't explicitly select something in a control?
Gergo Koteles Dec. 9, 2023, 6:20 p.m. UTC | #2
On Thu, 2023-12-07 at 18:28 +0000, Mark Brown wrote:
> On Thu, Dec 07, 2023 at 01:04:27AM +0100, Gergo Koteles wrote:
> 
> > Invalid indexes are not the best default values.
> 
> I'm guessing this is just fallout from the previous (not really
> explained patch)?  Is there perhaps some bootstrapping issue here with
> ensuring that the program and configuration get written to the device if
> the user doesn't explicitly select something in a control?

I added the >0 checks because I encountered a NULL pointer dereference.

Because
tasdevice_init sets
tas_priv->cur_prog = -1
tas_priv->cur_conf = -1

tasdev_fw_ready calls tasdevice_prmg_load(tas_priv, 0) which sets
tasdevice[i]->prg_no = 0

Then the playback hook calls
tasdevice_tuning_switch(tas_hda->priv, 0)
tasdevice_select_tuningprm_cfg(tas_priv, cur_prog (-1), cur_conf (-1),
profile_cfg_id (0));

tasdevice_select_tuningprm_cfg checks
if (tas_priv->tasdevice[i].cur_prog (0) != prm_no (-1)
and tries to load the program from
program = &(tas_fmw->programs[prm_no (-1)]);
tasdevice_load_data(tas_priv, &(program->dev_data));

I think the intention was to load the first program, first
configuration, first profile. And that's the safe thing to do.
So I expressed that with this commit.

Yes, I need to write much better commit messages.
diff mbox series

Patch

diff --git a/sound/soc/codecs/tas2781-comlib.c b/sound/soc/codecs/tas2781-comlib.c
index f914123c7284..635f97db033b 100644
--- a/sound/soc/codecs/tas2781-comlib.c
+++ b/sound/soc/codecs/tas2781-comlib.c
@@ -307,8 +307,8 @@  int tasdevice_init(struct tasdevice_priv *tas_priv)
 		goto out;
 	}
 
-	tas_priv->cur_prog = -1;
-	tas_priv->cur_conf = -1;
+	tas_priv->cur_prog = 0;
+	tas_priv->cur_conf = 0;
 
 	for (i = 0; i < tas_priv->ndev; i++) {
 		tas_priv->tasdevice[i].cur_book = -1;