From: Richard Fitzgerald <rf@opensource.cirrus.com>
To: <broonie@kernel.org>
Cc: <alsa-devel@alsa-project.org>, <linux-kernel@vger.kernel.org>,
<patches@opensource.cirrus.com>,
Richard Fitzgerald <rf@opensource.cirrus.com>
Subject: [PATCH 1/5] ASoC: cs35l56: Avoid uninitialized variable in cs35l56_set_asp_slot_positions()
Date: Tue, 8 Aug 2023 17:46:58 +0100 [thread overview]
Message-ID: <20230808164702.21272-2-rf@opensource.cirrus.com> (raw)
In-Reply-To: <20230808164702.21272-1-rf@opensource.cirrus.com>
Re-implement setting of ASP TDM slots so that only the common loop to
build the register word is factored out.
The original cs35l56_set_asp_slot_positions() had an apparent
uninitialized variable if the passed register address was neither of the
ASP slot registers. In fact this would never happen because the calling
code passed valid registers.
While it's trivial to initialize the variable or add a default case,
actually the only common code was the loop at the end of the function,
which simply manipulates some mask values and is identical for either
register. Factoring out the regmap_write() didn't really gain anything.
So instead re-implement the code to replace the original function with
cs35l56_make_tdm_config_word() that only does the loop, and change the
calling code to call regmap_write() directly.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
---
sound/soc/codecs/cs35l56.c | 24 ++++++++----------------
1 file changed, 8 insertions(+), 16 deletions(-)
diff --git a/sound/soc/codecs/cs35l56.c b/sound/soc/codecs/cs35l56.c
index 19b6b4fbe5de..be400208205a 100644
--- a/sound/soc/codecs/cs35l56.c
+++ b/sound/soc/codecs/cs35l56.c
@@ -358,22 +358,11 @@ static int cs35l56_asp_dai_set_fmt(struct snd_soc_dai *codec_dai, unsigned int f
return 0;
}
-static void cs35l56_set_asp_slot_positions(struct cs35l56_private *cs35l56,
- unsigned int reg, unsigned long mask)
+static unsigned int cs35l56_make_tdm_config_word(unsigned int reg_val, unsigned long mask)
{
- unsigned int reg_val, channel_shift;
+ unsigned int channel_shift;
int bit_num;
- /* Init all slots to 63 */
- switch (reg) {
- case CS35L56_ASP1_FRAME_CONTROL1:
- reg_val = 0x3f3f3f3f;
- break;
- case CS35L56_ASP1_FRAME_CONTROL5:
- reg_val = 0x3f3f3f;
- break;
- }
-
/* Enable consecutive TX1..TXn for each of the slots set in mask */
channel_shift = 0;
for_each_set_bit(bit_num, &mask, 32) {
@@ -382,7 +371,7 @@ static void cs35l56_set_asp_slot_positions(struct cs35l56_private *cs35l56,
channel_shift += 8;
}
- regmap_write(cs35l56->base.regmap, reg, reg_val);
+ return reg_val;
}
static int cs35l56_asp_dai_set_tdm_slot(struct snd_soc_dai *dai, unsigned int tx_mask,
@@ -418,8 +407,11 @@ static int cs35l56_asp_dai_set_tdm_slot(struct snd_soc_dai *dai, unsigned int tx
if (rx_mask == 0)
rx_mask = 0xf; // ASPTX1..TX4 in slots 0..3
- cs35l56_set_asp_slot_positions(cs35l56, CS35L56_ASP1_FRAME_CONTROL1, rx_mask);
- cs35l56_set_asp_slot_positions(cs35l56, CS35L56_ASP1_FRAME_CONTROL5, tx_mask);
+ /* Default unused slots to 63 */
+ regmap_write(cs35l56->base.regmap, CS35L56_ASP1_FRAME_CONTROL1,
+ cs35l56_make_tdm_config_word(0x3f3f3f3f, rx_mask));
+ regmap_write(cs35l56->base.regmap, CS35L56_ASP1_FRAME_CONTROL5,
+ cs35l56_make_tdm_config_word(0x3f3f3f, tx_mask));
dev_dbg(cs35l56->base.dev, "tdm slot width: %u count: %u tx_mask: %#x rx_mask: %#x\n",
cs35l56->asp_slot_width, cs35l56->asp_slot_count, tx_mask, rx_mask);
--
2.30.2
next prev parent reply other threads:[~2023-08-08 16:49 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-08 16:46 [PATCH 0/5] ASoC: cs35l56: Bugfixes Richard Fitzgerald
2023-08-08 16:46 ` Richard Fitzgerald [this message]
2023-08-08 16:46 ` [PATCH 2/5] ASoC: cs35l56: Don't rely on GPIOD_OUT_LOW to set RESET initially low Richard Fitzgerald
2023-08-08 16:47 ` [PATCH 3/5] ASoC: cs35l56: Wait for control port ready during system-resume Richard Fitzgerald
2023-08-08 16:47 ` [PATCH 4/5] ASoC: wm_adsp: Expose the DSP power down actions as wm_adsp_power_down() Richard Fitzgerald
2023-08-08 16:47 ` [PATCH 5/5] ASoC: cs35l56: Call wm_adsp_power_down() before reloading firmware Richard Fitzgerald
2023-08-09 17:26 ` [PATCH 0/5] ASoC: cs35l56: Bugfixes Mark Brown
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230808164702.21272-2-rf@opensource.cirrus.com \
--to=rf@opensource.cirrus.com \
--cc=alsa-devel@alsa-project.org \
--cc=broonie@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=patches@opensource.cirrus.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 NNTP newsgroup(s).