ASoC: TI codecs: sort the reg_defaults tables
Subsystems (4)
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
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.
--- sound/soc/codecs/pcm512x.c | 40 +++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 20 deletions(-)
sound/soc/codecs/pcm512x.c +20 -20
@@ -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
PATCH 2/5 ASoC: tas2552: sort the register default table
In reply to Peter Ujfalusireg_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.
--- sound/soc/codecs/tas2552.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
sound/soc/codecs/tas2552.c +7 -7
@@ -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
PATCH 3/5 ASoC: tas2764: sort the register default table
In reply to Peter Ujfalusireg_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.
--- sound/soc/codecs/tas2764.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
sound/soc/codecs/tas2764.c +1 -1
@@ -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
PATCH 4/5 ASoC: tas2780: sort the register default table
In reply to Peter Ujfalusireg_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.
--- sound/soc/codecs/tas2780.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
sound/soc/codecs/tas2780.c +1 -1
@@ -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
PATCH 5/5 ASoC: tas675x: sort the register default table
In reply to Peter Ujfalusireg_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.
--- sound/soc/codecs/tas675x.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
sound/soc/codecs/tas675x.c +9 -9
@@ -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
Mark Brown
maintainerQuoted 9 lines · Peter Ujfalusi
ASoC: TI codecs: sort the
reg_defaultstablesThe
reg_defaultsarray of aregmap_configmust be sorted by ascending register address:regcache_lookup_reg()finds entries in it withbsearch(), andregcache_init()only kmemdup_array()s the array, it never sorts it. See commitfd80df352ba1("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