Skip to contentSkip to conversation

ASoC: TI codecs: sort the reg_defaults tables

PATCH In review Peter Ujfalusi 2 participants 7 messages Aug 5 – Aug 8

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

cover ASoC: TI codecs: sort the reg_defaults tables

The reg_defaults array of a regmap_config must be sorted by ascending register address: regcache_lookup_reg() finds entries in it with bsearch(), and regcache_init() only kmemdup_array()s the array, it never sorts it. See commit fd80df352ba1 ("regcache: Add support for sorting defaults arrays").

When the array is not sorted, bsearch() descends into the wrong half and a number of entries can never be found. regcache_reg_needs_sync() then cannot compare the cached value against the default and always reports that a sync is needed, so those registers are written to the device on every regcache_sync() even if they were never touched.

I found this while chasing a SoundWire NAK on an unimplemented register and audited the reg_defaults tables in sound/ by replaying lib/bsearch.c over each of them. These are the affected TI/Burr-Brown codecs:

  pcm512x_reg_defaults    45 entries, 24 unreachable
  tas675x_reg_defaults    69 entries, 14 unreachable
  tas2552_reg_defs        21 entries,  7 unreachable
  tas2764_reg_defaults    11 entries,  1 unreachable
  tas2780_reg_defaults    10 entries,  1 unreachable

The patches only reorder the existing entries. The text of every entry is kept verbatim, no default value is changed, and each resulting table was verified to be fully reachable.

tas2783-sdw was fixed separately, it had already been posted.

The series is based on broonie/for-linus.

Peter Ujfalusi (5):
ASoC: pcm512x: sort the register default table
ASoC: tas2552: sort the register default table
ASoC: tas2764: sort the register default table
ASoC: tas2780: sort the register default table
ASoC: tas675x: sort the register default table

 sound/soc/codecs/pcm512x.c | 40 +++++++++++++++++++-------------------
 sound/soc/codecs/tas2552.c | 14 ++++++-------
 sound/soc/codecs/tas2764.c |  2 +-
 sound/soc/codecs/tas2780.c |  2 +-
 sound/soc/codecs/tas675x.c | 18 ++++++++---------
 5 files changed, 38 insertions(+), 38 deletions(-)
Signature · 1 line
2.55.0
#1 lore raw

PATCH 1/5 ASoC: pcm512x: sort the register default table

reg_defaults must be sorted by ascending register address, as regcache_lookup_reg() locates entries in it with bsearch(). See commit fd80df352ba1 ("regcache: Add support for sorting defaults arrays").

PCM512x_AUTO_MUTE (page 0, register 59) is listed before PCM512x_ERROR_DETECT (page 0, register 37) and PCM512x_VCOM_CTRL_2 (page 1, register 9) is listed before the page 0 clocking block, so the bsearch() descends into the wrong half of the table. 24 of the 45 entries are unreachable, among them every PLL coefficient and clock divider default. regcache_reg_needs_sync() then cannot compare them against their default and reports that a sync is needed, so they are written to the device on every regcache_sync() even when they were never touched.

Sort the table by register address.

Fixes 5a3af1293194 ("ASoC: pcm512x: Add PCM512x driver")
---
 sound/soc/codecs/pcm512x.c | 40 +++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 20 deletions(-)
sound/soc/codecs/pcm512x.c +20 -20
index fe3b5011fa16..03b1fcb1bf96 100644
@@ -78,28 +78,10 @@ static const struct reg_default pcm512x_reg_defaults[] = {
 	{ PCM512x_POWER,             0x00 },
 	{ PCM512x_MUTE,              0x00 },
 	{ PCM512x_DSP,               0x00 },
-	{ PCM512x_PLL_REF,           0x00 },
-	{ PCM512x_DAC_REF,           0x00 },
-	{ PCM512x_DAC_ROUTING,       0x11 },
-	{ PCM512x_DSP_PROGRAM,       0x01 },
-	{ PCM512x_CLKDET,            0x00 },
-	{ PCM512x_AUTO_MUTE,         0x00 },
-	{ PCM512x_ERROR_DETECT,      0x00 },
-	{ PCM512x_DIGITAL_VOLUME_1,  0x00 },
-	{ PCM512x_DIGITAL_VOLUME_2,  0x30 },
-	{ PCM512x_DIGITAL_VOLUME_3,  0x30 },
-	{ PCM512x_DIGITAL_MUTE_1,    0x22 },
-	{ PCM512x_DIGITAL_MUTE_2,    0x00 },
-	{ PCM512x_DIGITAL_MUTE_3,    0x07 },
-	{ PCM512x_OUTPUT_AMPLITUDE,  0x00 },
-	{ PCM512x_ANALOG_GAIN_CTRL,  0x00 },
-	{ PCM512x_UNDERVOLTAGE_PROT, 0x00 },
-	{ PCM512x_ANALOG_MUTE_CTRL,  0x00 },
-	{ PCM512x_ANALOG_GAIN_BOOST, 0x00 },
-	{ PCM512x_VCOM_CTRL_1,       0x00 },
-	{ PCM512x_VCOM_CTRL_2,       0x01 },
 	{ PCM512x_BCLK_LRCLK_CFG,    0x00 },
 	{ PCM512x_MASTER_MODE,       0x7c },
+	{ PCM512x_PLL_REF,           0x00 },
+	{ PCM512x_DAC_REF,           0x00 },
 	{ PCM512x_GPIO_DACIN,        0x00 },
 	{ PCM512x_GPIO_PLLIN,        0x00 },
 	{ PCM512x_SYNCHRONIZE,       0x10 },
@@ -117,8 +99,26 @@ static const struct reg_default pcm512x_reg_defaults[] = {
 	{ PCM512x_FS_SPEED_MODE,     0x00 },
 	{ PCM512x_IDAC_1,            0x01 },
 	{ PCM512x_IDAC_2,            0x00 },
+	{ PCM512x_ERROR_DETECT,      0x00 },
 	{ PCM512x_I2S_1,             0x02 },
 	{ PCM512x_I2S_2,             0x00 },
+	{ PCM512x_DAC_ROUTING,       0x11 },
+	{ PCM512x_DSP_PROGRAM,       0x01 },
+	{ PCM512x_CLKDET,            0x00 },
+	{ PCM512x_AUTO_MUTE,         0x00 },
+	{ PCM512x_DIGITAL_VOLUME_1,  0x00 },
+	{ PCM512x_DIGITAL_VOLUME_2,  0x30 },
+	{ PCM512x_DIGITAL_VOLUME_3,  0x30 },
+	{ PCM512x_DIGITAL_MUTE_1,    0x22 },
+	{ PCM512x_DIGITAL_MUTE_2,    0x00 },
+	{ PCM512x_DIGITAL_MUTE_3,    0x07 },
+	{ PCM512x_OUTPUT_AMPLITUDE,  0x00 },
+	{ PCM512x_ANALOG_GAIN_CTRL,  0x00 },
+	{ PCM512x_UNDERVOLTAGE_PROT, 0x00 },
+	{ PCM512x_ANALOG_MUTE_CTRL,  0x00 },
+	{ PCM512x_ANALOG_GAIN_BOOST, 0x00 },
+	{ PCM512x_VCOM_CTRL_1,       0x00 },
+	{ PCM512x_VCOM_CTRL_2,       0x01 },
 };
 
 static bool pcm512x_readable(struct device *dev, unsigned int reg)
Signature · 1 line
2.55.0
#2 lore raw

PATCH 2/5 ASoC: tas2552: sort the register default table

In reply to Peter Ujfalusi

reg_defaults must be sorted by ascending register address, as regcache_lookup_reg() locates entries in it with bsearch(). See commit fd80df352ba1 ("regcache: Add support for sorting defaults arrays").

TAS2552_OUTPUT_DATA (0x07), TAS2552_PDM_CFG (0x11), TAS2552_PGA_GAIN (0x12) and TAS2552_BOOST_APT_CTRL (0x14) are listed before TAS2552_RESERVED_0D (0x0d), TAS2552_LIMIT_RATE_HYS (0x0e) and TAS2552_CFG_2 (0x02), which leaves 7 of the 21 entries unreachable. regcache_reg_needs_sync() then cannot compare them against their default and reports that a sync is needed, so they are written to the device on every regcache_sync() even when they were never touched.

Sort the table by register address.

Fixes 5df7f71d5cdf ("ASoC: tas2552: Support TI TAS2552 Amplifier")
---
 sound/soc/codecs/tas2552.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
sound/soc/codecs/tas2552.c +7 -7
index 1a7650b9b2a7..ea083d1cec2b 100644
@@ -31,25 +31,25 @@
 
 static const struct reg_default tas2552_reg_defs[] = {
 	{TAS2552_CFG_1, 0x22},
+	{TAS2552_CFG_2, 0xef},
 	{TAS2552_CFG_3, 0x80},
 	{TAS2552_DOUT, 0x00},
-	{TAS2552_OUTPUT_DATA, 0xc0},
-	{TAS2552_PDM_CFG, 0x01},
-	{TAS2552_PGA_GAIN, 0x00},
-	{TAS2552_BOOST_APT_CTRL, 0x0f},
-	{TAS2552_RESERVED_0D, 0xbe},
-	{TAS2552_LIMIT_RATE_HYS, 0x08},
-	{TAS2552_CFG_2, 0xef},
 	{TAS2552_SER_CTRL_1, 0x00},
 	{TAS2552_SER_CTRL_2, 0x00},
+	{TAS2552_OUTPUT_DATA, 0xc0},
 	{TAS2552_PLL_CTRL_1, 0x10},
 	{TAS2552_PLL_CTRL_2, 0x00},
 	{TAS2552_PLL_CTRL_3, 0x00},
 	{TAS2552_BTIP, 0x8f},
 	{TAS2552_BTS_CTRL, 0x80},
+	{TAS2552_RESERVED_0D, 0xbe},
+	{TAS2552_LIMIT_RATE_HYS, 0x08},
 	{TAS2552_LIMIT_RELEASE, 0x04},
 	{TAS2552_LIMIT_INT_COUNT, 0x00},
+	{TAS2552_PDM_CFG, 0x01},
+	{TAS2552_PGA_GAIN, 0x00},
 	{TAS2552_EDGE_RATE_CTRL, 0x40},
+	{TAS2552_BOOST_APT_CTRL, 0x0f},
 	{TAS2552_VBAT_DATA, 0x00},
 };
 
Signature · 1 line
2.55.0
#3 lore raw

PATCH 3/5 ASoC: tas2764: sort the register default table

In reply to Peter Ujfalusi

reg_defaults must be sorted by ascending register address, as regcache_lookup_reg() locates entries in it with bsearch(). See commit fd80df352ba1 ("regcache: Add support for sorting defaults arrays").

TAS2764_DVC (0x1a) is listed before TAS2764_CHNL_0 (0x03), which makes it unreachable. regcache_reg_needs_sync() then cannot compare it against its default and reports that a sync is needed, so it is written to the device on every regcache_sync() even when it was never touched.

Sort the table by register address.

Fixes 827ed8a0fa50 ("ASoC: tas2764: Add the driver for the TAS2764")
---
 sound/soc/codecs/tas2764.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
sound/soc/codecs/tas2764.c +1 -1
index b11b998aa32a..f9c65c9f6d0a 100644
@@ -897,13 +897,13 @@ static const struct reg_default tas2764_reg_defaults[] = {
 	{ TAS2764_PAGE, 0x00 },
 	{ TAS2764_SW_RST, 0x00 },
 	{ TAS2764_PWR_CTRL, 0x1a },
-	{ TAS2764_DVC, 0x00 },
 	{ TAS2764_CHNL_0, 0x28 },
 	{ TAS2764_TDM_CFG0, 0x09 },
 	{ TAS2764_TDM_CFG1, 0x02 },
 	{ TAS2764_TDM_CFG2, 0x0a },
 	{ TAS2764_TDM_CFG3, 0x10 },
 	{ TAS2764_TDM_CFG5, 0x42 },
+	{ TAS2764_DVC, 0x00 },
 	{ TAS2764_INT_CLK_CFG, 0x19 },
 };
 
Signature · 1 line
2.55.0
#4 lore raw

PATCH 4/5 ASoC: tas2780: sort the register default table

In reply to Peter Ujfalusi

reg_defaults must be sorted by ascending register address, as regcache_lookup_reg() locates entries in it with bsearch(). See commit fd80df352ba1 ("regcache: Add support for sorting defaults arrays").

TAS2780_DVC (0x1a) is listed before TAS2780_CHNL_0 (0x03), which makes it unreachable. regcache_reg_needs_sync() then cannot compare it against its default and reports that a sync is needed, so it is written to the device on every regcache_sync() even when it was never touched.

Sort the table by register address.

Fixes eae9f9ce181b ("ASoC: add tas2780 driver")
---
 sound/soc/codecs/tas2780.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
sound/soc/codecs/tas2780.c +1 -1
index 1ec1c076204f..f1fce4305fa1 100644
@@ -527,13 +527,13 @@ static const struct reg_default tas2780_reg_defaults[] = {
 	{ TAS2780_PAGE, 0x00 },
 	{ TAS2780_SW_RST, 0x00 },
 	{ TAS2780_PWR_CTRL, 0x1a },
-	{ TAS2780_DVC, 0x00 },
 	{ TAS2780_CHNL_0, 0x00 },
 	{ TAS2780_TDM_CFG0, 0x09 },
 	{ TAS2780_TDM_CFG1, 0x02 },
 	{ TAS2780_TDM_CFG2, 0x0a },
 	{ TAS2780_TDM_CFG3, 0x10 },
 	{ TAS2780_TDM_CFG5, 0x42 },
+	{ TAS2780_DVC, 0x00 },
 };
 
 static const struct regmap_range_cfg tas2780_regmap_ranges[] = {
Signature · 1 line
2.55.0
#5 lore raw

PATCH 5/5 ASoC: tas675x: sort the register default table

In reply to Peter Ujfalusi

reg_defaults must be sorted by ascending register address, as regcache_lookup_reg() locates entries in it with bsearch(). See commit fd80df352ba1 ("regcache: Add support for sorting defaults arrays").

TAS675X_AC_LDG_CTRL_REG (0xb5), TAS675X_TWEETER_DETECT_CTRL_REG (0xb6), TAS675X_TWEETER_DETECT_THRESH_REG (0xb7) and TAS675X_AC_LDG_FREQ_CTRL_REG (0xb8) are listed before the 0x7c - 0xa0 block, which leaves 14 of the 69 entries unreachable. regcache_reg_needs_sync() then cannot compare them against their default and reports that a sync is needed, so they are written to the device on every regcache_sync() even when they were never touched.

Sort the table by register address.

Fixes 133c81f84471 ("ASoC: codecs: Add TAS67524 quad-channel audio amplifier driver")
---
 sound/soc/codecs/tas675x.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)
sound/soc/codecs/tas675x.c +9 -9
index 82526362de7b..404706b62156 100644
@@ -1924,15 +1924,6 @@ static const struct reg_default tas675x_reg_defaults[] = {
 	{ TAS675X_PWM_PHASE_M_CTRL_CH2_REG, 0x00 },
 	{ TAS675X_PWM_PHASE_M_CTRL_CH3_REG, 0x00 },
 	{ TAS675X_PWM_PHASE_M_CTRL_CH4_REG, 0x00 },
-	{ TAS675X_DC_LDG_CTRL_REG,         0x00 },
-	{ TAS675X_DC_LDG_LO_CTRL_REG,      0x00 },
-	{ TAS675X_DC_LDG_TIME_CTRL_REG,    0x00 },
-	{ TAS675X_DC_LDG_SL_CH1_CH2_CTRL_REG, 0x11 },
-	{ TAS675X_DC_LDG_SL_CH3_CH4_CTRL_REG, 0x11 },
-	{ TAS675X_AC_LDG_CTRL_REG,         0x10 },
-	{ TAS675X_TWEETER_DETECT_CTRL_REG, 0x08 },
-	{ TAS675X_TWEETER_DETECT_THRESH_REG, 0x00 },
-	{ TAS675X_AC_LDG_FREQ_CTRL_REG,    0xC8 },
 	{ TAS675X_REPORT_ROUTING_1_REG,    0x00 },
 	{ TAS675X_OTSD_RECOVERY_EN_REG,    0x00 },
 	{ TAS675X_REPORT_ROUTING_2_REG,    0xA2 },
@@ -1943,6 +1934,15 @@ static const struct reg_default tas675x_reg_defaults[] = {
 	{ TAS675X_GPIO1_OUTPUT_SEL_REG,    0x00 },
 	{ TAS675X_GPIO2_OUTPUT_SEL_REG,    0x00 },
 	{ TAS675X_GPIO_CTRL_REG,           TAS675X_GPIO_CTRL_RSTVAL },
+	{ TAS675X_DC_LDG_CTRL_REG,         0x00 },
+	{ TAS675X_DC_LDG_LO_CTRL_REG,      0x00 },
+	{ TAS675X_DC_LDG_TIME_CTRL_REG,    0x00 },
+	{ TAS675X_DC_LDG_SL_CH1_CH2_CTRL_REG, 0x11 },
+	{ TAS675X_DC_LDG_SL_CH3_CH4_CTRL_REG, 0x11 },
+	{ TAS675X_AC_LDG_CTRL_REG,         0x10 },
+	{ TAS675X_TWEETER_DETECT_CTRL_REG, 0x08 },
+	{ TAS675X_TWEETER_DETECT_THRESH_REG, 0x00 },
+	{ TAS675X_AC_LDG_FREQ_CTRL_REG,    0xC8 },
 	{ TAS675X_OTW_CTRL_CH1_CH2_REG,    0x11 },
 	{ TAS675X_OTW_CTRL_CH3_CH4_REG,    0x11 },
 };
Signature · 1 line
2.55.0
maintainer
#6 lore raw
In reply to Peter Ujfalusi
Quoted 9 lines · Peter Ujfalusi

ASoC: TI codecs: sort the reg_defaults tables

The reg_defaults array of a regmap_config must be sorted by ascending register address: regcache_lookup_reg() finds entries in it with bsearch(), and regcache_init() only kmemdup_array()s the array, it never sorts it. See commit fd80df352ba1 ("regcache: Add support for sorting defaults arrays").

[...]

Applied to

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

Thanks!

[1/5] ASoC: pcm512x: sort the register default table

      https://git.kernel.org/broonie/sound/c/767d9ae714e3
[2/5] ASoC: tas2552: sort the register default table
      https://git.kernel.org/broonie/sound/c/1cc0cb62d306
[3/5] ASoC: tas2764: sort the register default table
      https://git.kernel.org/broonie/sound/c/e7643c3f7eb3
[4/5] ASoC: tas2780: sort the register default table
      https://git.kernel.org/broonie/sound/c/e725093e9e53
[5/5] ASoC: tas675x: sort the register default table
      https://git.kernel.org/broonie/sound/c/3521d209ca70

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