2023-12-07 00:06:24

by Gergo Koteles

[permalink] [raw]
Subject: [PATCH 11/16] ASoC: tas2781: use 0 as default prog/conf index

Invalid indexes are not the best default values.

Signed-off-by: Gergo Koteles <[email protected]>
---
sound/soc/codecs/tas2781-comlib.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

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;
--
2.43.0


2023-12-07 18:28:44

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 11/16] ASoC: tas2781: use 0 as default prog/conf index

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?


Attachments:
(No filename) (391.00 B)
signature.asc (499.00 B)
Download all attachments

2023-12-09 18:21:09

by Gergo Koteles

[permalink] [raw]
Subject: Re: [PATCH 11/16] ASoC: tas2781: use 0 as default prog/conf index

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.