From 7a24b876ad8cdd56457e881d384a038922b508c3 Mon Sep 17 00:00:00 2001 From: Charles Keepax Date: Tue, 18 Mar 2025 17:14:45 +0000 Subject: [PATCH 01/15] ASoC: ops-test: Add some basic kunit tests for soc-ops Add some basic kunit tests for some of the ASoC control put and get helpers. This will assist in doing some refactoring. Note that presently some tests fail, but the rest of the series will fix these up. Signed-off-by: Charles Keepax Link: https://patch.msgid.link/20250318171459.3203730-2-ckeepax@opensource.cirrus.com Signed-off-by: Mark Brown --- sound/soc/Kconfig | 7 + sound/soc/Makefile | 4 + sound/soc/soc-ops-test.c | 541 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 552 insertions(+) create mode 100644 sound/soc/soc-ops-test.c diff --git a/sound/soc/Kconfig b/sound/soc/Kconfig index 5efba76abb31..8b7d51266f81 100644 --- a/sound/soc/Kconfig +++ b/sound/soc/Kconfig @@ -81,6 +81,13 @@ config SND_SOC_UTILS_KUNIT_TEST help If you want to perform tests on ALSA SoC utils library say Y here. +config SND_SOC_OPS_KUNIT_TEST + tristate "KUnit tests for SoC ops" + depends on KUNIT + default KUNIT_ALL_TESTS + help + If you want to perform tests on ALSA SoC ops library say Y here. + config SND_SOC_ACPI tristate diff --git a/sound/soc/Makefile b/sound/soc/Makefile index 08baaa11d813..358e227c5ab6 100644 --- a/sound/soc/Makefile +++ b/sound/soc/Makefile @@ -21,6 +21,10 @@ ifneq ($(CONFIG_SND_SOC_UTILS_KUNIT_TEST),) obj-$(CONFIG_SND_SOC_UTILS_KUNIT_TEST) += soc-utils-test.o endif +ifneq ($(CONFIG_SND_SOC_OPS_KUNIT_TEST),) +obj-$(CONFIG_SND_SOC_OPS_KUNIT_TEST) += soc-ops-test.o +endif + ifneq ($(CONFIG_SND_SOC_GENERIC_DMAENGINE_PCM),) snd-soc-core-y += soc-generic-dmaengine-pcm.o endif diff --git a/sound/soc/soc-ops-test.c b/sound/soc/soc-ops-test.c new file mode 100644 index 000000000000..dc1e482bba6a --- /dev/null +++ b/sound/soc/soc-ops-test.c @@ -0,0 +1,541 @@ +// SPDX-License-Identifier: GPL-2.0-only +// Copyright (C) 2025 Cirrus Logic, Inc. and +// Cirrus Logic International Semiconductor Ltd. + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +enum soc_ops_test_control_layout { + SOC_OPS_TEST_SINGLE, + SOC_OPS_TEST_DOUBLE, + SOC_OPS_TEST_DOUBLE_R, +}; + +#define TEST_MC(clayout, xmin, xmax, xpmax, xsign, xinvert) \ + .mc = { \ + .min = xmin, .max = xmax, .platform_max = xpmax, \ + .reg = 0, .shift = 0, .sign_bit = xsign, .invert = xinvert, \ + .rreg = SOC_OPS_TEST_##clayout == SOC_OPS_TEST_DOUBLE_R ? 1 : 0, \ + .rshift = SOC_OPS_TEST_##clayout == SOC_OPS_TEST_DOUBLE ? 16 : 0, \ + } + +#define TEST_UINFO(clayout, ctype, cmin, cmax) \ + .uinfo = { \ + .type = SNDRV_CTL_ELEM_TYPE_##ctype, \ + .count = SOC_OPS_TEST_##clayout == SOC_OPS_TEST_SINGLE ? 1 : 2, \ + .value.integer.min = cmin, \ + .value.integer.max = cmax, \ + } + +#define ITEST(cname, clayout, ctype, cfunc, cmin, cmax, \ + xmin, xmax, xpmax, xsign, xinvert) \ + { \ + .name = cname, \ + .func_name = #cfunc, \ + .layout = SOC_OPS_TEST_##clayout, \ + .info = snd_soc_info_##cfunc, \ + TEST_MC(clayout, xmin, xmax, xpmax, xsign, xinvert), \ + TEST_UINFO(clayout, ctype, cmin, cmax), \ + } + +#define ATEST(clayout, cfunc, cctl, cret, cinit, \ + xmask, xreg, xmin, xmax, xpmax, xsign, xinvert) \ + { \ + .func_name = #cfunc, \ + .layout = SOC_OPS_TEST_##clayout, \ + .put = snd_soc_put_##cfunc, \ + .get = snd_soc_get_##cfunc, \ + TEST_MC(clayout, xmin, xmax, xpmax, xsign, xinvert), \ + .lctl = cctl, .rctl = cctl, \ + .lmask = SOC_OPS_TEST_##clayout == SOC_OPS_TEST_DOUBLE ? \ + (xmask) | (xmask) << 16 : (xmask), \ + .rmask = SOC_OPS_TEST_##clayout == SOC_OPS_TEST_DOUBLE_R ? (xmask) : 0, \ + .init = cinit ? 0xFFFFFFFF : 0x00000000, \ + .lreg = SOC_OPS_TEST_##clayout == SOC_OPS_TEST_DOUBLE ? \ + (xreg) | (xreg) << 16 : (xreg), \ + .rreg = SOC_OPS_TEST_##clayout == SOC_OPS_TEST_DOUBLE_R ? (xreg) : 0, \ + .ret = cret, \ + } + +struct soc_ops_test_priv { + struct kunit *test; + + struct snd_soc_component component; +}; + +struct info_test_param { + const char * const name; + const char * const func_name; + enum soc_ops_test_control_layout layout; + struct soc_mixer_control mc; + int (*info)(struct snd_kcontrol *kctl, struct snd_ctl_elem_info *info); + + struct snd_ctl_elem_info uinfo; +}; + +struct access_test_param { + const char * const func_name; + enum soc_ops_test_control_layout layout; + struct soc_mixer_control mc; + int (*put)(struct snd_kcontrol *kctl, struct snd_ctl_elem_value *value); + int (*get)(struct snd_kcontrol *kctl, struct snd_ctl_elem_value *value); + + unsigned int init; + unsigned int lmask; + unsigned int rmask; + unsigned int lreg; + unsigned int rreg; + long lctl; + long rctl; + int ret; +}; + +static const struct info_test_param all_info_test_params[] = { + // Handling of volume control name for types + ITEST("Test Control", SINGLE, BOOLEAN, volsw, 0, 1, 0, 1, 0, 0, 0), + ITEST("Test Volume", SINGLE, INTEGER, volsw, 0, 1, 0, 1, 0, 0, 0), + ITEST("Test Volume Control", SINGLE, BOOLEAN, volsw, 0, 1, 0, 1, 0, 0, 0), + ITEST("Test Control", DOUBLE_R, BOOLEAN, volsw, 0, 1, 0, 1, 0, 0, 0), + ITEST("Test Volume", DOUBLE_R, INTEGER, volsw, 0, 1, 0, 1, 0, 0, 0), + ITEST("Test Volume Control", DOUBLE_R, BOOLEAN, volsw, 0, 1, 0, 1, 0, 0, 0), + ITEST("Test Control", DOUBLE, BOOLEAN, volsw, 0, 1, 0, 1, 0, 0, 0), + ITEST("Test Volume", DOUBLE, INTEGER, volsw, 0, 1, 0, 1, 0, 0, 0), + ITEST("Test Volume Control", DOUBLE, BOOLEAN, volsw, 0, 1, 0, 1, 0, 0, 0), + ITEST("Test Control", SINGLE, BOOLEAN, volsw, 0, 1, 0, 1, 0, 0, 1), + ITEST("Test Volume", SINGLE, INTEGER, volsw, 0, 1, 0, 1, 0, 0, 1), + ITEST("Test Volume Control", SINGLE, BOOLEAN, volsw, 0, 1, 0, 1, 0, 0, 1), + ITEST("Test Control", DOUBLE, BOOLEAN, volsw, 0, 1, 0, 1, 0, 0, 1), + ITEST("Test Volume", DOUBLE, INTEGER, volsw, 0, 1, 0, 1, 0, 0, 1), + ITEST("Test Volume Control", DOUBLE, BOOLEAN, volsw, 0, 1, 0, 1, 0, 0, 1), + ITEST("Test Control", SINGLE, INTEGER, volsw, 0, 2, 0, 2, 0, 0, 0), + ITEST("Test Volume", SINGLE, INTEGER, volsw, 0, 2, 0, 2, 0, 0, 0), + ITEST("Test Volume Control", SINGLE, INTEGER, volsw, 0, 2, 0, 2, 0, 0, 0), + ITEST("Test Control", SINGLE, INTEGER, volsw, 0, 1, 0, 2, 1, 0, 0), + ITEST("Test Volume", SINGLE, INTEGER, volsw, 0, 1, 0, 2, 1, 0, 0), + ITEST("Test Volume Control", SINGLE, INTEGER, volsw, 0, 1, 0, 2, 1, 0, 0), + // Negative minimums + ITEST("Test Control", SINGLE, INTEGER, volsw, 0, 20, -10, 10, 0, 4, 0), + ITEST("Test Control", SINGLE, INTEGER, volsw, 0, 15, -10, 10, 15, 4, 0), + ITEST("Test Control", SINGLE, INTEGER, volsw, 0, 20, -10, 10, 0, 4, 1), + ITEST("Test Control", SINGLE, INTEGER, volsw, 0, 15, -10, 10, 15, 4, 1), + // SX control volume control naming + ITEST("Test Control", SINGLE, BOOLEAN, volsw_sx, 0, 1, 0xF, 1, 0, 0, 0), + ITEST("Test Volume", SINGLE, INTEGER, volsw_sx, 0, 1, 0xF, 1, 0, 0, 0), + ITEST("Test Volume Control", SINGLE, BOOLEAN, volsw_sx, 0, 1, 0xF, 1, 0, 0, 0), + ITEST("Test Control", SINGLE, INTEGER, volsw_sx, 0, 4, 0xE, 4, 0, 0, 0), + ITEST("Test Volume", SINGLE, INTEGER, volsw_sx, 0, 4, 0xE, 4, 0, 0, 0), + ITEST("Test Volume Control", SINGLE, INTEGER, volsw_sx, 0, 4, 0xE, 4, 0, 0, 0), + ITEST("Test Control", SINGLE, INTEGER, volsw_sx, 0, 3, 0xE, 4, 3, 0, 0), + ITEST("Test Volume", SINGLE, INTEGER, volsw_sx, 0, 3, 0xE, 4, 3, 0, 0), + ITEST("Test Volume Control", SINGLE, INTEGER, volsw_sx, 0, 3, 0xE, 4, 3, 0, 0), +}; + +static const struct access_test_param all_access_test_params[] = { + // Single positive value controls + ATEST(SINGLE, volsw, 10, 1, false, 0x1F, 0x0A, 0, 20, 0, 0, 0), + ATEST(SINGLE, volsw, 0, 0, false, 0x1F, 0x00, 0, 20, 0, 0, 0), + ATEST(SINGLE, volsw, 20, 1, false, 0x1F, 0x14, 0, 20, 0, 0, 0), + ATEST(SINGLE, volsw, 10, 1, false, 0x1F, 0x0A, 0, 20, 15, 0, 0), + ATEST(SINGLE, volsw, 25, -22, false, 0x1F, 0x00, 0, 20, 15, 0, 0), + ATEST(SINGLE, volsw, 15, 1, false, 0x1F, 0x0F, 0, 20, 15, 0, 0), + // Inverted single positive value controls + ATEST(SINGLE, volsw, 10, 1, false, 0x1F, 0x0A, 0, 20, 0, 0, 1), + ATEST(SINGLE, volsw, 0, 1, false, 0x1F, 0x14, 0, 20, 0, 0, 1), + ATEST(SINGLE, volsw, 20, 0, false, 0x1F, 0x00, 0, 20, 0, 0, 1), + ATEST(SINGLE, volsw, 10, 1, false, 0x1F, 0x0A, 0, 20, 15, 0, 1), + ATEST(SINGLE, volsw, 25, -22, false, 0x1F, 0x00, 0, 20, 15, 0, 1), + ATEST(SINGLE, volsw, 15, 1, false, 0x1F, 0x05, 0, 20, 15, 0, 1), + ATEST(SINGLE, volsw, 10, 1, true, 0x1F, 0x0A, 0, 20, 0, 0, 0), + ATEST(SINGLE, volsw, 0, 1, true, 0x1F, 0x00, 0, 20, 0, 0, 0), + ATEST(SINGLE, volsw, 20, 1, true, 0x1F, 0x14, 0, 20, 0, 0, 0), + ATEST(SINGLE, volsw, 10, 1, true, 0x1F, 0x0A, 0, 20, 15, 0, 0), + ATEST(SINGLE, volsw, 25, -22, true, 0x1F, 0x00, 0, 20, 15, 0, 0), + ATEST(SINGLE, volsw, 15, 1, true, 0x1F, 0x0F, 0, 20, 15, 0, 0), + // Single negative value controls + ATEST(SINGLE, volsw, 10, 0, false, 0x1F, 0x00, -10, 10, 0, 4, 0), + ATEST(SINGLE, volsw, 0, 1, false, 0x1F, 0x16, -10, 10, 0, 4, 0), + ATEST(SINGLE, volsw, 20, 1, false, 0x1F, 0x0A, -10, 10, 0, 4, 0), + ATEST(SINGLE, volsw, 10, 0, false, 0x1F, 0x00, -10, 10, 15, 4, 0), + ATEST(SINGLE, volsw, 25, -22, false, 0x1F, 0x00, -10, 10, 15, 4, 0), + ATEST(SINGLE, volsw, 15, 1, false, 0x1F, 0x05, -10, 10, 15, 4, 0), + // Single non-zero minimum positive value controls + ATEST(SINGLE, volsw, 10, 1, false, 0x1F, 0x14, 10, 30, 0, 0, 0), + ATEST(SINGLE, volsw, 0, 1, false, 0x1F, 0x0A, 10, 30, 0, 0, 0), + ATEST(SINGLE, volsw, 20, 1, false, 0x1F, 0x1E, 10, 30, 0, 0, 0), + ATEST(SINGLE, volsw, 10, 1, false, 0x1F, 0x14, 10, 30, 15, 0, 0), + ATEST(SINGLE, volsw, 25, -22, false, 0x1F, 0x00, 10, 30, 15, 0, 0), + ATEST(SINGLE, volsw, 15, 1, false, 0x1F, 0x19, 10, 30, 15, 0, 0), + // Inverted single non-zero minimum positive value controls + ATEST(SINGLE, volsw, 10, 1, false, 0x1F, 0x14, 10, 30, 0, 0, 1), + ATEST(SINGLE, volsw, 0, 1, false, 0x1F, 0x1E, 10, 30, 0, 0, 1), + ATEST(SINGLE, volsw, 20, 1, false, 0x1F, 0x0A, 10, 30, 0, 0, 1), + ATEST(SINGLE, volsw, 10, 1, false, 0x1F, 0x14, 10, 30, 15, 0, 1), + ATEST(SINGLE, volsw, 25, -22, false, 0x1F, 0x00, 10, 30, 15, 0, 1), + ATEST(SINGLE, volsw, 15, 1, false, 0x1F, 0x0F, 10, 30, 15, 0, 1), + // Double register positive value controls + ATEST(DOUBLE_R, volsw, 10, 1, false, 0x1F, 0x0A, 0, 20, 0, 0, 0), + ATEST(DOUBLE_R, volsw, 0, 0, false, 0x1F, 0x00, 0, 20, 0, 0, 0), + ATEST(DOUBLE_R, volsw, 20, 1, false, 0x1F, 0x14, 0, 20, 0, 0, 0), + ATEST(DOUBLE_R, volsw, 10, 1, false, 0x1F, 0x0A, 0, 20, 15, 0, 0), + ATEST(DOUBLE_R, volsw, 25, -22, false, 0x1F, 0x00, 0, 20, 15, 0, 0), + ATEST(DOUBLE_R, volsw, 15, 1, false, 0x1F, 0x0F, 0, 20, 15, 0, 0), + // Double register negative value controls + ATEST(DOUBLE_R, volsw, 10, 0, false, 0x1F, 0x00, -10, 10, 0, 4, 0), + ATEST(DOUBLE_R, volsw, 0, 1, false, 0x1F, 0x16, -10, 10, 0, 4, 0), + ATEST(DOUBLE_R, volsw, 20, 1, false, 0x1F, 0x0A, -10, 10, 0, 4, 0), + ATEST(DOUBLE_R, volsw, 10, 0, false, 0x1F, 0x00, -10, 10, 15, 4, 0), + ATEST(DOUBLE_R, volsw, 25, -22, false, 0x1F, 0x00, -10, 10, 15, 4, 0), + ATEST(DOUBLE_R, volsw, 15, 1, false, 0x1F, 0x05, -10, 10, 15, 4, 0), + ATEST(DOUBLE_R, volsw, 10, 1, true, 0x1F, 0x00, -10, 10, 0, 4, 0), + ATEST(DOUBLE_R, volsw, 0, 1, true, 0x1F, 0x16, -10, 10, 0, 4, 0), + ATEST(DOUBLE_R, volsw, 20, 1, true, 0x1F, 0x0A, -10, 10, 0, 4, 0), + ATEST(DOUBLE_R, volsw, 10, 1, true, 0x1F, 0x00, -10, 10, 15, 4, 0), + ATEST(DOUBLE_R, volsw, 25, -22, true, 0x1F, 0x00, -10, 10, 15, 4, 0), + ATEST(DOUBLE_R, volsw, 15, 1, true, 0x1F, 0x05, -10, 10, 15, 4, 0), + // Inverted double register negative value controls + ATEST(DOUBLE_R, volsw, 10, 1, true, 0x1F, 0x00, -10, 10, 0, 4, 1), + ATEST(DOUBLE_R, volsw, 0, 1, true, 0x1F, 0x0A, -10, 10, 0, 4, 1), + ATEST(DOUBLE_R, volsw, 20, 1, true, 0x1F, 0x16, -10, 10, 0, 4, 1), + ATEST(DOUBLE_R, volsw, 10, 1, true, 0x1F, 0x00, -10, 10, 15, 4, 1), + ATEST(DOUBLE_R, volsw, 25, -22, true, 0x1F, 0x00, -10, 10, 15, 4, 1), + ATEST(DOUBLE_R, volsw, 15, 1, true, 0x1F, 0x1B, -10, 10, 15, 4, 1), + // Double register non-zero minimum positive value controls + ATEST(DOUBLE_R, volsw, 10, 1, false, 0x1F, 0x14, 10, 30, 0, 0, 0), + ATEST(DOUBLE_R, volsw, 0, 1, false, 0x1F, 0x0A, 10, 30, 0, 0, 0), + ATEST(DOUBLE_R, volsw, 20, 1, false, 0x1F, 0x1E, 10, 30, 0, 0, 0), + ATEST(DOUBLE_R, volsw, 10, 1, false, 0x1F, 0x14, 10, 30, 15, 0, 0), + ATEST(DOUBLE_R, volsw, 25, -22, false, 0x1F, 0x00, 10, 30, 15, 0, 0), + ATEST(DOUBLE_R, volsw, 15, 1, false, 0x1F, 0x19, 10, 30, 15, 0, 0), + // Double shift positive value controls + ATEST(DOUBLE, volsw, 10, 1, false, 0x1F, 0x0A, 0, 20, 0, 0, 0), + ATEST(DOUBLE, volsw, 0, 0, false, 0x1F, 0x00, 0, 20, 0, 0, 0), + ATEST(DOUBLE, volsw, 20, 1, false, 0x1F, 0x14, 0, 20, 0, 0, 0), + ATEST(DOUBLE, volsw, 10, 1, false, 0x1F, 0x0A, 0, 20, 15, 0, 0), + ATEST(DOUBLE, volsw, 25, -22, false, 0x1F, 0x00, 0, 20, 15, 0, 0), + ATEST(DOUBLE, volsw, 15, 1, false, 0x1F, 0x0F, 0, 20, 15, 0, 0), + // Double shift negative value controls + ATEST(DOUBLE, volsw, 10, 0, false, 0x1F, 0x00, -10, 10, 0, 4, 0), + ATEST(DOUBLE, volsw, 0, 1, false, 0x1F, 0x16, -10, 10, 0, 4, 0), + ATEST(DOUBLE, volsw, 20, 1, false, 0x1F, 0x0A, -10, 10, 0, 4, 0), + ATEST(DOUBLE, volsw, 10, 0, false, 0x1F, 0x00, -10, 10, 15, 4, 0), + ATEST(DOUBLE, volsw, 25, -22, false, 0x1F, 0x00, -10, 10, 15, 4, 0), + ATEST(DOUBLE, volsw, 15, 1, false, 0x1F, 0x05, -10, 10, 15, 4, 0), + // Inverted double shift negative value controls + ATEST(DOUBLE, volsw, 10, 0, false, 0x1F, 0x00, -10, 10, 0, 4, 1), + ATEST(DOUBLE, volsw, 0, 1, false, 0x1F, 0x0A, -10, 10, 0, 4, 1), + ATEST(DOUBLE, volsw, 20, 1, false, 0x1F, 0x16, -10, 10, 0, 4, 1), + ATEST(DOUBLE, volsw, 10, 0, false, 0x1F, 0x00, -10, 10, 15, 4, 1), + ATEST(DOUBLE, volsw, 25, -22, false, 0x1F, 0x00, -10, 10, 15, 4, 1), + ATEST(DOUBLE, volsw, 15, 1, false, 0x1F, 0x1B, -10, 10, 15, 4, 1), + // Double shift non-zero minimum positive value controls + ATEST(DOUBLE, volsw, 10, 1, false, 0x1F, 0x14, 10, 30, 0, 0, 0), + ATEST(DOUBLE, volsw, 0, 1, false, 0x1F, 0x0A, 10, 30, 0, 0, 0), + ATEST(DOUBLE, volsw, 20, 1, false, 0x1F, 0x1E, 10, 30, 0, 0, 0), + ATEST(DOUBLE, volsw, 10, 1, false, 0x1F, 0x14, 10, 30, 15, 0, 0), + ATEST(DOUBLE, volsw, 25, -22, false, 0x1F, 0x00, 10, 30, 15, 0, 0), + ATEST(DOUBLE, volsw, 15, 1, false, 0x1F, 0x19, 10, 30, 15, 0, 0), + ATEST(DOUBLE, volsw, 10, 1, true, 0x1F, 0x14, 10, 30, 0, 0, 0), + ATEST(DOUBLE, volsw, 0, 1, true, 0x1F, 0x0A, 10, 30, 0, 0, 0), + ATEST(DOUBLE, volsw, 20, 1, true, 0x1F, 0x1E, 10, 30, 0, 0, 0), + ATEST(DOUBLE, volsw, 10, 1, true, 0x1F, 0x14, 10, 30, 15, 0, 0), + ATEST(DOUBLE, volsw, 25, -22, true, 0x1F, 0x00, 10, 30, 15, 0, 0), + ATEST(DOUBLE, volsw, 15, 1, true, 0x1F, 0x19, 10, 30, 15, 0, 0), + // Single SX all values + ATEST(SINGLE, volsw_sx, 0, 1, false, 0xF, 0x0F, 0x0F, 4, 0, 0, 0), + ATEST(SINGLE, volsw_sx, 1, 0, false, 0xF, 0x00, 0x0F, 4, 0, 0, 0), + ATEST(SINGLE, volsw_sx, 2, 1, false, 0xF, 0x01, 0x0F, 4, 0, 0, 0), + ATEST(SINGLE, volsw_sx, 3, 1, false, 0xF, 0x02, 0x0F, 4, 0, 0, 0), + ATEST(SINGLE, volsw_sx, 4, 1, false, 0xF, 0x03, 0x0F, 4, 0, 0, 0), + ATEST(SINGLE, volsw_sx, 5, -22, false, 0xF, 0x00, 0x0F, 4, 0, 0, 0), + ATEST(SINGLE, volsw_sx, 0, 0, true, 0xF, 0x0F, 0x0F, 4, 0, 0, 0), + ATEST(SINGLE, volsw_sx, 1, 1, true, 0xF, 0x00, 0x0F, 4, 0, 0, 0), + ATEST(SINGLE, volsw_sx, 2, 1, true, 0xF, 0x01, 0x0F, 4, 0, 0, 0), + ATEST(SINGLE, volsw_sx, 3, 1, true, 0xF, 0x02, 0x0F, 4, 0, 0, 0), + ATEST(SINGLE, volsw_sx, 4, 1, true, 0xF, 0x03, 0x0F, 4, 0, 0, 0), + ATEST(SINGLE, volsw_sx, 5, -22, true, 0xF, 0x00, 0x0F, 4, 0, 0, 0), + // Inverted single SX all values + ATEST(SINGLE, volsw_sx, 0, 1, false, 0x1F, 0x03, 0x0F, 4, 0, 0, 1), + ATEST(SINGLE, volsw_sx, 1, 1, false, 0x1F, 0x02, 0x0F, 4, 0, 0, 1), + ATEST(SINGLE, volsw_sx, 2, 1, false, 0x1F, 0x01, 0x0F, 4, 0, 0, 1), + ATEST(SINGLE, volsw_sx, 3, 0, false, 0x1F, 0x00, 0x0F, 4, 0, 0, 1), + ATEST(SINGLE, volsw_sx, 4, 1, false, 0x1F, 0x0F, 0x0F, 4, 0, 0, 1), + ATEST(SINGLE, volsw_sx, 5, -22, false, 0x1F, 0x00, 0x0F, 4, 0, 0, 1), + // Single SX select values + ATEST(SINGLE, volsw_sx, 0, 1, false, 0xFF, 0x88, 0x88, 144, 0, 0, 0), + ATEST(SINGLE, volsw_sx, 1, 1, false, 0xFF, 0x89, 0x88, 144, 0, 0, 0), + ATEST(SINGLE, volsw_sx, 119, 1, false, 0xFF, 0xFF, 0x88, 144, 0, 0, 0), + ATEST(SINGLE, volsw_sx, 120, 0, false, 0xFF, 0x00, 0x88, 144, 0, 0, 0), + ATEST(SINGLE, volsw_sx, 121, 1, false, 0xFF, 0x01, 0x88, 144, 0, 0, 0), + ATEST(SINGLE, volsw_sx, 143, 1, false, 0xFF, 0x17, 0x88, 144, 0, 0, 0), + ATEST(SINGLE, volsw_sx, 144, 1, false, 0xFF, 0x18, 0x88, 144, 0, 0, 0), + ATEST(SINGLE, volsw_sx, 145, -22, false, 0xFF, 0x00, 0x88, 144, 0, 0, 0), + ATEST(SINGLE, volsw_sx, 0, 1, true, 0xFF, 0x88, 0x88, 144, 0, 0, 0), + ATEST(SINGLE, volsw_sx, 1, 1, true, 0xFF, 0x89, 0x88, 144, 0, 0, 0), + ATEST(SINGLE, volsw_sx, 119, 0, true, 0xFF, 0xFF, 0x88, 144, 0, 0, 0), + ATEST(SINGLE, volsw_sx, 120, 1, true, 0xFF, 0x00, 0x88, 144, 0, 0, 0), + ATEST(SINGLE, volsw_sx, 121, 1, true, 0xFF, 0x01, 0x88, 144, 0, 0, 0), + ATEST(SINGLE, volsw_sx, 143, 1, true, 0xFF, 0x17, 0x88, 144, 0, 0, 0), + ATEST(SINGLE, volsw_sx, 144, 1, true, 0xFF, 0x18, 0x88, 144, 0, 0, 0), + ATEST(SINGLE, volsw_sx, 145, -22, true, 0xFF, 0x00, 0x88, 144, 0, 0, 0), + // Double shift SX select values + ATEST(DOUBLE, volsw_sx, 0, 1, true, 0xFF, 0x88, 0x88, 144, 0, 0, 0), + ATEST(DOUBLE, volsw_sx, 1, 1, true, 0xFF, 0x89, 0x88, 144, 0, 0, 0), + ATEST(DOUBLE, volsw_sx, 119, 0, true, 0xFF, 0xFF, 0x88, 144, 0, 0, 0), + ATEST(DOUBLE, volsw_sx, 120, 1, true, 0xFF, 0x00, 0x88, 144, 0, 0, 0), + ATEST(DOUBLE, volsw_sx, 121, 1, true, 0xFF, 0x01, 0x88, 144, 0, 0, 0), + ATEST(DOUBLE, volsw_sx, 143, 1, true, 0xFF, 0x17, 0x88, 144, 0, 0, 0), + ATEST(DOUBLE, volsw_sx, 144, 1, true, 0xFF, 0x18, 0x88, 144, 0, 0, 0), + ATEST(DOUBLE, volsw_sx, 145, -22, true, 0xFF, 0x00, 0x88, 144, 0, 0, 0), + // Double register SX select values + ATEST(DOUBLE_R, volsw_sx, 0, 1, true, 0xFF, 0x88, 0x88, 144, 0, 0, 0), + ATEST(DOUBLE_R, volsw_sx, 1, 1, true, 0xFF, 0x89, 0x88, 144, 0, 0, 0), + ATEST(DOUBLE_R, volsw_sx, 119, 0, true, 0xFF, 0xFF, 0x88, 144, 0, 0, 0), + ATEST(DOUBLE_R, volsw_sx, 120, 1, true, 0xFF, 0x00, 0x88, 144, 0, 0, 0), + ATEST(DOUBLE_R, volsw_sx, 121, 1, true, 0xFF, 0x01, 0x88, 144, 0, 0, 0), + ATEST(DOUBLE_R, volsw_sx, 143, 1, true, 0xFF, 0x17, 0x88, 144, 0, 0, 0), + ATEST(DOUBLE_R, volsw_sx, 144, 1, true, 0xFF, 0x18, 0x88, 144, 0, 0, 0), + ATEST(DOUBLE_R, volsw_sx, 145, -22, true, 0xFF, 0x00, 0x88, 144, 0, 0, 0), +}; + +static const char *control_type_str(const snd_ctl_elem_type_t type) +{ + switch (type) { + case SNDRV_CTL_ELEM_TYPE_BOOLEAN: + return "bool"; + case SNDRV_CTL_ELEM_TYPE_INTEGER: + return "int"; + default: + return "unknown"; + } +} + +static const char *control_layout_str(const enum soc_ops_test_control_layout layout) +{ + switch (layout) { + case SOC_OPS_TEST_SINGLE: + return "single"; + case SOC_OPS_TEST_DOUBLE: + return "double"; + case SOC_OPS_TEST_DOUBLE_R: + return "double_r"; + default: + return "unknown"; + } +}; + +static int mock_regmap_read(void *context, const void *reg_buf, + const size_t reg_size, void *val_buf, + size_t val_size) +{ + struct soc_ops_test_priv *priv = context; + + KUNIT_FAIL(priv->test, "Unexpected bus read"); + + return -EIO; +} + +static int mock_regmap_gather_write(void *context, + const void *reg_buf, size_t reg_size, + const void *val_buf, size_t val_size) +{ + struct soc_ops_test_priv *priv = context; + + KUNIT_FAIL(priv->test, "Unexpected bus gather_write"); + + return -EIO; +} + +static int mock_regmap_write(void *context, const void *val_buf, + size_t val_size) +{ + struct soc_ops_test_priv *priv = context; + + KUNIT_FAIL(priv->test, "Unexpected bus write"); + + return -EIO; +} + +static const struct regmap_bus mock_regmap_bus = { + .read = mock_regmap_read, + .write = mock_regmap_write, + .gather_write = mock_regmap_gather_write, + .reg_format_endian_default = REGMAP_ENDIAN_NATIVE, + .val_format_endian_default = REGMAP_ENDIAN_NATIVE, +}; + +static const struct regmap_config mock_regmap_config = { + .reg_bits = 32, + .val_bits = 32, + .reg_format_endian = REGMAP_ENDIAN_NATIVE, + .val_format_endian = REGMAP_ENDIAN_NATIVE, + .max_register = 0x1, + .cache_type = REGCACHE_FLAT, +}; + +static int soc_ops_test_init(struct kunit *test) +{ + struct soc_ops_test_priv *priv; + struct regmap *regmap; + struct device *dev; + + priv = kunit_kzalloc(test, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + priv->test = test; + + dev = kunit_device_register(test, "soc_ops_test_drv"); + if (IS_ERR(dev)) + return PTR_ERR(dev); + + regmap = devm_regmap_init(dev, &mock_regmap_bus, priv, &mock_regmap_config); + if (IS_ERR(regmap)) + return PTR_ERR(regmap); + + /* No actual hardware, we just use the cache */ + regcache_cache_only(regmap, true); + + priv->component.dev = dev; + priv->component.regmap = regmap; + mutex_init(&priv->component.io_mutex); + + test->priv = priv; + + return 0; +} + +static void soc_ops_test_exit(struct kunit *test) +{ + struct soc_ops_test_priv *priv = test->priv; + + kunit_device_unregister(test, priv->component.dev); +} + +static void info_test_desc(const struct info_test_param *param, char *desc) +{ + snprintf(desc, KUNIT_PARAM_DESC_SIZE, + "%s %s %s: ctl range: %ld->%ld, reg range: %d->%d(%d), sign: %d, inv: %d", + control_layout_str(param->layout), param->func_name, + control_type_str(param->uinfo.type), + param->uinfo.value.integer.min, param->uinfo.value.integer.max, + param->mc.min, param->mc.max, param->mc.platform_max, + param->mc.sign_bit, param->mc.invert); +} + +static void soc_ops_test_info(struct kunit *test) +{ + struct soc_ops_test_priv *priv = test->priv; + const struct info_test_param *param = test->param_value; + const struct snd_ctl_elem_info *target = ¶m->uinfo; + struct snd_ctl_elem_info result; + struct snd_kcontrol kctl = { + .private_data = &priv->component, + .private_value = (unsigned long)¶m->mc, + }; + int ret; + + strscpy(kctl.id.name, param->name, sizeof(kctl.id.name)); + + ret = param->info(&kctl, &result); + KUNIT_ASSERT_FALSE(test, ret); + + KUNIT_EXPECT_EQ(test, result.count, target->count); + KUNIT_EXPECT_EQ(test, result.type, target->type); + KUNIT_EXPECT_EQ(test, result.value.integer.min, target->value.integer.min); + KUNIT_EXPECT_EQ(test, result.value.integer.max, target->value.integer.max); +} + +static void access_test_desc(const struct access_test_param *param, char *desc) +{ + if (param->ret < 0) { + snprintf(desc, KUNIT_PARAM_DESC_SIZE, + "%s %s: %ld,%ld -> range: %d->%d(%d), sign: %d, inv: %d -> err: %d", + control_layout_str(param->layout), param->func_name, + param->lctl, param->rctl, + param->mc.min, param->mc.max, param->mc.platform_max, + param->mc.sign_bit, param->mc.invert, + param->ret); + } else { + snprintf(desc, KUNIT_PARAM_DESC_SIZE, + "%s %s: %ld,%ld -> range: %d->%d(%d), sign: %d, inv: %d -> %#x,%#x", + control_layout_str(param->layout), param->func_name, + param->lctl, param->rctl, + param->mc.min, param->mc.max, param->mc.platform_max, + param->mc.sign_bit, param->mc.invert, + param->lreg, param->rreg); + } +} + +static void soc_ops_test_access(struct kunit *test) +{ + struct soc_ops_test_priv *priv = test->priv; + const struct access_test_param *param = test->param_value; + struct snd_kcontrol kctl = { + .private_data = &priv->component, + .private_value = (unsigned long)¶m->mc, + }; + struct snd_ctl_elem_value result; + unsigned int val; + int ret; + + ret = regmap_write(priv->component.regmap, 0x0, param->init); + KUNIT_ASSERT_FALSE(test, ret); + ret = regmap_write(priv->component.regmap, 0x1, param->init); + KUNIT_ASSERT_FALSE(test, ret); + + result.value.integer.value[0] = param->lctl; + result.value.integer.value[1] = param->rctl; + + ret = param->put(&kctl, &result); + KUNIT_ASSERT_EQ(test, ret, param->ret); + if (ret < 0) + return; + + ret = regmap_read(priv->component.regmap, 0x0, &val); + KUNIT_ASSERT_FALSE(test, ret); + KUNIT_EXPECT_EQ(test, val, (param->init & ~param->lmask) | param->lreg); + + ret = regmap_read(priv->component.regmap, 0x1, &val); + KUNIT_ASSERT_FALSE(test, ret); + KUNIT_EXPECT_EQ(test, val, (param->init & ~param->rmask) | param->rreg); + + result.value.integer.value[0] = 0; + result.value.integer.value[1] = 0; + + ret = param->get(&kctl, &result); + KUNIT_ASSERT_GE(test, ret, 0); + + KUNIT_EXPECT_EQ(test, result.value.integer.value[0], param->lctl); + if (param->layout != SOC_OPS_TEST_SINGLE) + KUNIT_EXPECT_EQ(test, result.value.integer.value[1], param->rctl); + else + KUNIT_EXPECT_EQ(test, result.value.integer.value[1], 0); +} + +KUNIT_ARRAY_PARAM(all_info_tests, all_info_test_params, info_test_desc); +KUNIT_ARRAY_PARAM(all_access_tests, all_access_test_params, access_test_desc); + +static struct kunit_case soc_ops_test_cases[] = { + KUNIT_CASE_PARAM(soc_ops_test_info, all_info_tests_gen_params), + KUNIT_CASE_PARAM(soc_ops_test_access, all_access_tests_gen_params), + {} +}; + +static struct kunit_suite soc_ops_test_suite = { + .name = "soc-ops", + .init = soc_ops_test_init, + .exit = soc_ops_test_exit, + .test_cases = soc_ops_test_cases, +}; + +kunit_test_suites(&soc_ops_test_suite); + +MODULE_DESCRIPTION("ASoC soc-ops kunit test"); +MODULE_LICENSE("GPL"); From 534bfb330b2612199b2afaafc769ccb42bebb204 Mon Sep 17 00:00:00 2001 From: Charles Keepax Date: Tue, 18 Mar 2025 17:14:46 +0000 Subject: [PATCH 02/15] ASoC: ops: Minor formatting fixups No functional changes just tidying up some tabbing etc. Signed-off-by: Charles Keepax Link: https://patch.msgid.link/20250318171459.3203730-3-ckeepax@opensource.cirrus.com Signed-off-by: Mark Brown --- sound/soc/soc-ops.c | 106 ++++++++++++++++++++++---------------------- 1 file changed, 54 insertions(+), 52 deletions(-) diff --git a/sound/soc/soc-ops.c b/sound/soc/soc-ops.c index cd5f927bcd4e..9039bf3b6fb4 100644 --- a/sound/soc/soc-ops.c +++ b/sound/soc/soc-ops.c @@ -37,7 +37,7 @@ * Returns 0 for success. */ int snd_soc_info_enum_double(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_info *uinfo) + struct snd_ctl_elem_info *uinfo) { struct soc_enum *e = (struct soc_enum *)kcontrol->private_value; @@ -56,7 +56,7 @@ EXPORT_SYMBOL_GPL(snd_soc_info_enum_double); * Returns 0 for success. */ int snd_soc_get_enum_double(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_value *ucontrol) + struct snd_ctl_elem_value *ucontrol) { struct snd_soc_component *component = snd_kcontrol_chip(kcontrol); struct soc_enum *e = (struct soc_enum *)kcontrol->private_value; @@ -87,7 +87,7 @@ EXPORT_SYMBOL_GPL(snd_soc_get_enum_double); * Returns 0 for success. */ int snd_soc_put_enum_double(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_value *ucontrol) + struct snd_ctl_elem_value *ucontrol) { struct snd_soc_component *component = snd_kcontrol_chip(kcontrol); struct soc_enum *e = (struct soc_enum *)kcontrol->private_value; @@ -124,8 +124,9 @@ EXPORT_SYMBOL_GPL(snd_soc_put_enum_double); * the given registervalue into a signed integer if sign_bit is non-zero. */ static void snd_soc_read_signed(struct snd_soc_component *component, - unsigned int reg, unsigned int mask, unsigned int shift, - unsigned int sign_bit, int *signed_val) + unsigned int reg, unsigned int mask, + unsigned int shift, unsigned int sign_bit, + int *signed_val) { int ret; unsigned int val; @@ -168,7 +169,7 @@ static void snd_soc_read_signed(struct snd_soc_component *component, * Returns 0 for success. */ int snd_soc_info_volsw(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_info *uinfo) + struct snd_ctl_elem_info *uinfo) { struct soc_mixer_control *mc = (struct soc_mixer_control *)kcontrol->private_value; @@ -247,7 +248,7 @@ EXPORT_SYMBOL_GPL(snd_soc_info_volsw_sx); * Returns 0 for success. */ int snd_soc_get_volsw(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_value *ucontrol) + struct snd_ctl_elem_value *ucontrol) { struct snd_soc_component *component = snd_kcontrol_chip(kcontrol); struct soc_mixer_control *mc = @@ -300,7 +301,7 @@ EXPORT_SYMBOL_GPL(snd_soc_get_volsw); * Returns 0 for success. */ int snd_soc_put_volsw(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_value *ucontrol) + struct snd_ctl_elem_value *ucontrol) { struct snd_soc_component *component = snd_kcontrol_chip(kcontrol); struct soc_mixer_control *mc = @@ -362,9 +363,8 @@ int snd_soc_put_volsw(struct snd_kcontrol *kcontrol, err = snd_soc_component_update_bits(component, reg2, val_mask, val2); /* Don't discard any error code or drop change flag */ - if (ret == 0 || err < 0) { + if (ret == 0 || err < 0) ret = err; - } } return ret; @@ -382,11 +382,11 @@ EXPORT_SYMBOL_GPL(snd_soc_put_volsw); * Returns 0 for success. */ int snd_soc_get_volsw_sx(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_value *ucontrol) + struct snd_ctl_elem_value *ucontrol) { struct snd_soc_component *component = snd_kcontrol_chip(kcontrol); struct soc_mixer_control *mc = - (struct soc_mixer_control *)kcontrol->private_value; + (struct soc_mixer_control *)kcontrol->private_value; unsigned int reg = mc->reg; unsigned int reg2 = mc->rreg; unsigned int shift = mc->shift; @@ -423,18 +423,17 @@ int snd_soc_put_volsw_sx(struct snd_kcontrol *kcontrol, { struct snd_soc_component *component = snd_kcontrol_chip(kcontrol); struct soc_mixer_control *mc = - (struct soc_mixer_control *)kcontrol->private_value; - + (struct soc_mixer_control *)kcontrol->private_value; unsigned int reg = mc->reg; unsigned int reg2 = mc->rreg; unsigned int shift = mc->shift; unsigned int rshift = mc->rshift; + unsigned int val, val_mask; int max = mc->max; int min = mc->min; unsigned int mask = (1U << (fls(min + max) - 1)) - 1; int err = 0; int ret; - unsigned int val, val_mask; if (ucontrol->value.integer.value[0] < 0) return -EINVAL; @@ -465,13 +464,13 @@ int snd_soc_put_volsw_sx(struct snd_kcontrol *kcontrol, val2 = val2 << rshift; err = snd_soc_component_update_bits(component, reg2, val_mask, - val2); + val2); /* Don't discard any error code or drop change flag */ - if (ret == 0 || err < 0) { + if (ret == 0 || err < 0) ret = err; - } } + return ret; } EXPORT_SYMBOL_GPL(snd_soc_put_volsw_sx); @@ -487,7 +486,7 @@ EXPORT_SYMBOL_GPL(snd_soc_put_volsw_sx); * returns 0 for success. */ int snd_soc_info_volsw_range(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_info *uinfo) + struct snd_ctl_elem_info *uinfo) { struct soc_mixer_control *mc = (struct soc_mixer_control *)kcontrol->private_value; @@ -516,7 +515,7 @@ EXPORT_SYMBOL_GPL(snd_soc_info_volsw_range); * Returns 0 for success. */ int snd_soc_put_volsw_range(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_value *ucontrol) + struct snd_ctl_elem_value *ucontrol) { struct soc_mixer_control *mc = (struct soc_mixer_control *)kcontrol->private_value; @@ -568,11 +567,10 @@ int snd_soc_put_volsw_range(struct snd_kcontrol *kcontrol, val = val << shift; err = snd_soc_component_update_bits(component, rreg, val_mask, - val); + val); /* Don't discard any error code or drop change flag */ - if (ret == 0 || err < 0) { + if (ret == 0 || err < 0) ret = err; - } } return ret; @@ -589,7 +587,7 @@ EXPORT_SYMBOL_GPL(snd_soc_put_volsw_range); * Returns 0 for success. */ int snd_soc_get_volsw_range(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_value *ucontrol) + struct snd_ctl_elem_value *ucontrol) { struct snd_soc_component *component = snd_kcontrol_chip(kcontrol); struct soc_mixer_control *mc = @@ -663,8 +661,7 @@ static int snd_soc_clip_to_platform_max(struct snd_kcontrol *kctl) * * Return 0 for success, else error. */ -int snd_soc_limit_volume(struct snd_soc_card *card, - const char *name, int max) +int snd_soc_limit_volume(struct snd_soc_card *card, const char *name, int max) { struct snd_kcontrol *kctl; int ret = -EINVAL; @@ -675,12 +672,15 @@ int snd_soc_limit_volume(struct snd_soc_card *card, kctl = snd_soc_card_get_kcontrol(card, name); if (kctl) { - struct soc_mixer_control *mc = (struct soc_mixer_control *)kctl->private_value; + struct soc_mixer_control *mc = + (struct soc_mixer_control *)kctl->private_value; + if (max <= mc->max - mc->min) { mc->platform_max = max; ret = snd_soc_clip_to_platform_max(kctl); } } + return ret; } EXPORT_SYMBOL_GPL(snd_soc_limit_volume); @@ -740,8 +740,8 @@ int snd_soc_bytes_put(struct snd_kcontrol *kcontrol, { struct snd_soc_component *component = snd_kcontrol_chip(kcontrol); struct soc_bytes *params = (void *)kcontrol->private_value; - int ret, len; unsigned int val, mask; + int ret, len; if (!component->regmap || !params->num_regs) return -EINVAL; @@ -772,15 +772,13 @@ int snd_soc_bytes_put(struct snd_kcontrol *kcontrol, break; case 2: mask = ~params->mask; - ret = regmap_parse_val(component->regmap, - &mask, &mask); + ret = regmap_parse_val(component->regmap, &mask, &mask); if (ret != 0) return ret; ((u16 *)data)[0] &= mask; - ret = regmap_parse_val(component->regmap, - &val, &val); + ret = regmap_parse_val(component->regmap, &val, &val); if (ret != 0) return ret; @@ -788,15 +786,13 @@ int snd_soc_bytes_put(struct snd_kcontrol *kcontrol, break; case 4: mask = ~params->mask; - ret = regmap_parse_val(component->regmap, - &mask, &mask); + ret = regmap_parse_val(component->regmap, &mask, &mask); if (ret != 0) return ret; ((u32 *)data)[0] &= mask; - ret = regmap_parse_val(component->regmap, - &val, &val); + ret = regmap_parse_val(component->regmap, &val, &val); if (ret != 0) return ret; @@ -812,7 +808,7 @@ int snd_soc_bytes_put(struct snd_kcontrol *kcontrol, EXPORT_SYMBOL_GPL(snd_soc_bytes_put); int snd_soc_bytes_info_ext(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_info *ucontrol) + struct snd_ctl_elem_info *ucontrol) { struct soc_bytes_ext *params = (void *)kcontrol->private_value; @@ -824,7 +820,7 @@ int snd_soc_bytes_info_ext(struct snd_kcontrol *kcontrol, EXPORT_SYMBOL_GPL(snd_soc_bytes_info_ext); int snd_soc_bytes_tlv_callback(struct snd_kcontrol *kcontrol, int op_flag, - unsigned int size, unsigned int __user *tlv) + unsigned int size, unsigned int __user *tlv) { struct soc_bytes_ext *params = (void *)kcontrol->private_value; unsigned int count = size < params->max ? size : params->max; @@ -840,6 +836,7 @@ int snd_soc_bytes_tlv_callback(struct snd_kcontrol *kcontrol, int op_flag, ret = params->put(kcontrol, tlv, count); break; } + return ret; } EXPORT_SYMBOL_GPL(snd_soc_bytes_tlv_callback); @@ -856,10 +853,11 @@ EXPORT_SYMBOL_GPL(snd_soc_bytes_tlv_callback); * Returns 0 for success. */ int snd_soc_info_xr_sx(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_info *uinfo) + struct snd_ctl_elem_info *uinfo) { struct soc_mreg_control *mc = (struct soc_mreg_control *)kcontrol->private_value; + uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER; uinfo->count = 1; uinfo->value.integer.min = mc->min; @@ -883,7 +881,7 @@ EXPORT_SYMBOL_GPL(snd_soc_info_xr_sx); * Returns 0 for success. */ int snd_soc_get_xr_sx(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_value *ucontrol) + struct snd_ctl_elem_value *ucontrol) { struct snd_soc_component *component = snd_kcontrol_chip(kcontrol); struct soc_mreg_control *mc = @@ -891,17 +889,18 @@ int snd_soc_get_xr_sx(struct snd_kcontrol *kcontrol, unsigned int regbase = mc->regbase; unsigned int regcount = mc->regcount; unsigned int regwshift = component->val_bytes * BITS_PER_BYTE; - unsigned int regwmask = (1UL<invert; - unsigned long mask = (1UL<nbits)-1; + unsigned long mask = (1UL << mc->nbits) - 1; long min = mc->min; long max = mc->max; long val = 0; unsigned int i; for (i = 0; i < regcount; i++) { - unsigned int regval = snd_soc_component_read(component, regbase+i); - val |= (regval & regwmask) << (regwshift*(regcount-i-1)); + unsigned int regval = snd_soc_component_read(component, regbase + i); + + val |= (regval & regwmask) << (regwshift * (regcount - i - 1)); } val &= mask; if (min < 0 && val > max) @@ -928,7 +927,7 @@ EXPORT_SYMBOL_GPL(snd_soc_get_xr_sx); * Returns 0 for success. */ int snd_soc_put_xr_sx(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_value *ucontrol) + struct snd_ctl_elem_value *ucontrol) { struct snd_soc_component *component = snd_kcontrol_chip(kcontrol); struct soc_mreg_control *mc = @@ -936,9 +935,9 @@ int snd_soc_put_xr_sx(struct snd_kcontrol *kcontrol, unsigned int regbase = mc->regbase; unsigned int regcount = mc->regcount; unsigned int regwshift = component->val_bytes * BITS_PER_BYTE; - unsigned int regwmask = (1UL<invert; - unsigned long mask = (1UL<nbits)-1; + unsigned long mask = (1UL << mc->nbits) - 1; long max = mc->max; long val = ucontrol->value.integer.value[0]; int ret = 0; @@ -950,10 +949,13 @@ int snd_soc_put_xr_sx(struct snd_kcontrol *kcontrol, val = max - val; val &= mask; for (i = 0; i < regcount; i++) { - unsigned int regval = (val >> (regwshift*(regcount-i-1))) & regwmask; - unsigned int regmask = (mask >> (regwshift*(regcount-i-1))) & regwmask; - int err = snd_soc_component_update_bits(component, regbase+i, + unsigned int regval = (val >> (regwshift * (regcount - i - 1))) & + regwmask; + unsigned int regmask = (mask >> (regwshift * (regcount - i - 1))) & + regwmask; + int err = snd_soc_component_update_bits(component, regbase + i, regmask, regval); + if (err < 0) return err; if (err > 0) @@ -974,7 +976,7 @@ EXPORT_SYMBOL_GPL(snd_soc_put_xr_sx); * Returns 0 for success. */ int snd_soc_get_strobe(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_value *ucontrol) + struct snd_ctl_elem_value *ucontrol) { struct snd_soc_component *component = snd_kcontrol_chip(kcontrol); struct soc_mixer_control *mc = @@ -1007,7 +1009,7 @@ EXPORT_SYMBOL_GPL(snd_soc_get_strobe); * Returns 1 for success. */ int snd_soc_put_strobe(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_value *ucontrol) + struct snd_ctl_elem_value *ucontrol) { struct snd_soc_component *component = snd_kcontrol_chip(kcontrol); struct soc_mixer_control *mc = From 7f978180673b4f3b68fcc66fc1f1d74a1fc5a93a Mon Sep 17 00:00:00 2001 From: Charles Keepax Date: Tue, 18 Mar 2025 17:14:47 +0000 Subject: [PATCH 03/15] ASoC: ops: Update comments for xr_sx control helpers Update the comments for the xr_sx control helper functions to better clarify the difference between these and the normal sx helpers. Signed-off-by: Charles Keepax Link: https://patch.msgid.link/20250318171459.3203730-4-ckeepax@opensource.cirrus.com Signed-off-by: Mark Brown --- sound/soc/soc-ops.c | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/sound/soc/soc-ops.c b/sound/soc/soc-ops.c index 9039bf3b6fb4..dac55138210d 100644 --- a/sound/soc/soc-ops.c +++ b/sound/soc/soc-ops.c @@ -846,9 +846,10 @@ EXPORT_SYMBOL_GPL(snd_soc_bytes_tlv_callback); * @kcontrol: mreg control * @uinfo: control element information * - * Callback to provide information of a control that can - * span multiple codec registers which together - * forms a single signed value in a MSB/LSB manner. + * Callback to provide information of a control that can span multiple + * codec registers which together forms a single signed value. Note + * that unlike the non-xr variant of sx controls these may or may not + * include the sign bit, depending on nbits, and there is no shift. * * Returns 0 for success. */ @@ -872,11 +873,12 @@ EXPORT_SYMBOL_GPL(snd_soc_info_xr_sx); * @kcontrol: mreg control * @ucontrol: control element information * - * Callback to get the value of a control that can span - * multiple codec registers which together forms a single - * signed value in a MSB/LSB manner. The control supports - * specifying total no of bits used to allow for bitfields - * across the multiple codec registers. + * Callback to get the value of a control that can span multiple codec + * registers which together forms a single signed value. The control + * supports specifying total no of bits used to allow for bitfields + * across the multiple codec registers. Note that unlike the non-xr + * variant of sx controls these may or may not include the sign bit, + * depending on nbits, and there is no shift. * * Returns 0 for success. */ @@ -918,11 +920,12 @@ EXPORT_SYMBOL_GPL(snd_soc_get_xr_sx); * @kcontrol: mreg control * @ucontrol: control element information * - * Callback to set the value of a control that can span - * multiple codec registers which together forms a single - * signed value in a MSB/LSB manner. The control supports - * specifying total no of bits used to allow for bitfields - * across the multiple codec registers. + * Callback to set the value of a control that can span multiple codec + * registers which together forms a single signed value. The control + * supports specifying total no of bits used to allow for bitfields + * across the multiple codec registers. Note that unlike the non-xr + * variant of sx controls these may or may not include the sign bit, + * depending on nbits, and there is no shift. * * Returns 0 for success. */ From c6002c1177cafb4462b6c188d2a62eb67f15165f Mon Sep 17 00:00:00 2001 From: Charles Keepax Date: Tue, 18 Mar 2025 17:14:48 +0000 Subject: [PATCH 04/15] ASoC: ops: Update mask generation to use GENMASK Use GENMASK to make the masks for the various control helper functions. Also factor out a shared helper function for the volsw and volsw_range controls since the same code is appropriate for each. Note this does add support for sign_bit into the volsw_range callbacks. Signed-off-by: Charles Keepax Link: https://patch.msgid.link/20250318171459.3203730-5-ckeepax@opensource.cirrus.com Signed-off-by: Mark Brown --- sound/soc/soc-ops.c | 46 +++++++++++++++++++++++++-------------------- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/sound/soc/soc-ops.c b/sound/soc/soc-ops.c index dac55138210d..54945017e1f1 100644 --- a/sound/soc/soc-ops.c +++ b/sound/soc/soc-ops.c @@ -158,6 +158,20 @@ static void snd_soc_read_signed(struct snd_soc_component *component, *signed_val = ret; } +static int soc_mixer_mask(struct soc_mixer_control *mc) +{ + if (mc->sign_bit) + return GENMASK(mc->sign_bit, 0); + else + return GENMASK(fls(mc->max) - 1, 0); +} + +static int soc_mixer_sx_mask(struct soc_mixer_control *mc) +{ + // min + max will take us 1-bit over the size of the mask + return GENMASK(fls(mc->min + mc->max) - 2, 0); +} + /** * snd_soc_info_volsw - single mixer info callback * @kcontrol: mixer control @@ -260,13 +274,10 @@ int snd_soc_get_volsw(struct snd_kcontrol *kcontrol, int max = mc->max; int min = mc->min; int sign_bit = mc->sign_bit; - unsigned int mask = (1ULL << fls(max)) - 1; + unsigned int mask = soc_mixer_mask(mc); unsigned int invert = mc->invert; int val; - if (sign_bit) - mask = BIT(sign_bit + 1) - 1; - snd_soc_read_signed(component, reg, mask, shift, sign_bit, &val); ucontrol->value.integer.value[0] = val - min; @@ -312,17 +323,13 @@ int snd_soc_put_volsw(struct snd_kcontrol *kcontrol, unsigned int rshift = mc->rshift; int max = mc->max; int min = mc->min; - unsigned int sign_bit = mc->sign_bit; - unsigned int mask = (1 << fls(max)) - 1; + unsigned int mask = soc_mixer_mask(mc); unsigned int invert = mc->invert; int err, ret; bool type_2r = false; unsigned int val2 = 0; unsigned int val, val_mask; - if (sign_bit) - mask = BIT(sign_bit + 1) - 1; - if (ucontrol->value.integer.value[0] < 0) return -EINVAL; val = ucontrol->value.integer.value[0]; @@ -391,9 +398,8 @@ int snd_soc_get_volsw_sx(struct snd_kcontrol *kcontrol, unsigned int reg2 = mc->rreg; unsigned int shift = mc->shift; unsigned int rshift = mc->rshift; - int max = mc->max; int min = mc->min; - unsigned int mask = (1U << (fls(min + max) - 1)) - 1; + unsigned int mask = soc_mixer_sx_mask(mc); unsigned int val; val = snd_soc_component_read(component, reg); @@ -431,7 +437,7 @@ int snd_soc_put_volsw_sx(struct snd_kcontrol *kcontrol, unsigned int val, val_mask; int max = mc->max; int min = mc->min; - unsigned int mask = (1U << (fls(min + max) - 1)) - 1; + unsigned int mask = soc_mixer_sx_mask(mc); int err = 0; int ret; @@ -525,7 +531,7 @@ int snd_soc_put_volsw_range(struct snd_kcontrol *kcontrol, unsigned int shift = mc->shift; int min = mc->min; int max = mc->max; - unsigned int mask = (1 << fls(max)) - 1; + unsigned int mask = soc_mixer_mask(mc); unsigned int invert = mc->invert; unsigned int val, val_mask; int err, ret, tmp; @@ -597,7 +603,7 @@ int snd_soc_get_volsw_range(struct snd_kcontrol *kcontrol, unsigned int shift = mc->shift; int min = mc->min; int max = mc->max; - unsigned int mask = (1 << fls(max)) - 1; + unsigned int mask = soc_mixer_mask(mc); unsigned int invert = mc->invert; unsigned int val; @@ -891,9 +897,9 @@ int snd_soc_get_xr_sx(struct snd_kcontrol *kcontrol, unsigned int regbase = mc->regbase; unsigned int regcount = mc->regcount; unsigned int regwshift = component->val_bytes * BITS_PER_BYTE; - unsigned int regwmask = (1UL << regwshift) - 1; + unsigned int regwmask = GENMASK(regwshift - 1, 0); + unsigned long mask = GENMASK(mc->nbits - 1, 0); unsigned int invert = mc->invert; - unsigned long mask = (1UL << mc->nbits) - 1; long min = mc->min; long max = mc->max; long val = 0; @@ -938,9 +944,9 @@ int snd_soc_put_xr_sx(struct snd_kcontrol *kcontrol, unsigned int regbase = mc->regbase; unsigned int regcount = mc->regcount; unsigned int regwshift = component->val_bytes * BITS_PER_BYTE; - unsigned int regwmask = (1UL << regwshift) - 1; + unsigned int regwmask = GENMASK(regwshift - 1, 0); + unsigned long mask = GENMASK(mc->nbits - 1, 0); unsigned int invert = mc->invert; - unsigned long mask = (1UL << mc->nbits) - 1; long max = mc->max; long val = ucontrol->value.integer.value[0]; int ret = 0; @@ -986,7 +992,7 @@ int snd_soc_get_strobe(struct snd_kcontrol *kcontrol, (struct soc_mixer_control *)kcontrol->private_value; unsigned int reg = mc->reg; unsigned int shift = mc->shift; - unsigned int mask = 1 << shift; + unsigned int mask = BIT(shift); unsigned int invert = mc->invert != 0; unsigned int val; @@ -1019,7 +1025,7 @@ int snd_soc_put_strobe(struct snd_kcontrol *kcontrol, (struct soc_mixer_control *)kcontrol->private_value; unsigned int reg = mc->reg; unsigned int shift = mc->shift; - unsigned int mask = 1 << shift; + unsigned int mask = BIT(shift); unsigned int invert = mc->invert != 0; unsigned int strobe = ucontrol->value.enumerated.item[0] != 0; unsigned int val1 = (strobe ^ invert) ? mask : 0; From eeb76cb1fa0dcccf33091b04ba150076dfbeb6fd Mon Sep 17 00:00:00 2001 From: Charles Keepax Date: Tue, 18 Mar 2025 17:14:49 +0000 Subject: [PATCH 05/15] ASoC: ops: Factor out helper to check valid control values Most of the put handlers have identical code to verify the value passed in from user-space. Factor this out into a single helper function. Signed-off-by: Charles Keepax Link: https://patch.msgid.link/20250318171459.3203730-6-ckeepax@opensource.cirrus.com Signed-off-by: Mark Brown --- sound/soc/soc-ops.c | 82 ++++++++++++++++++++++++--------------------- 1 file changed, 43 insertions(+), 39 deletions(-) diff --git a/sound/soc/soc-ops.c b/sound/soc/soc-ops.c index 54945017e1f1..53a141426a96 100644 --- a/sound/soc/soc-ops.c +++ b/sound/soc/soc-ops.c @@ -158,6 +158,20 @@ static void snd_soc_read_signed(struct snd_soc_component *component, *signed_val = ret; } +static int soc_mixer_valid_ctl(struct soc_mixer_control *mc, long val, int max) +{ + if (val < 0) + return -EINVAL; + + if (mc->platform_max && val > mc->platform_max) + return -EINVAL; + + if (val > max) + return -EINVAL; + + return 0; +} + static int soc_mixer_mask(struct soc_mixer_control *mc) { if (mc->sign_bit) @@ -330,26 +344,24 @@ int snd_soc_put_volsw(struct snd_kcontrol *kcontrol, unsigned int val2 = 0; unsigned int val, val_mask; - if (ucontrol->value.integer.value[0] < 0) - return -EINVAL; + ret = soc_mixer_valid_ctl(mc, ucontrol->value.integer.value[0], + mc->max - mc->min); + if (ret) + return ret; + val = ucontrol->value.integer.value[0]; - if (mc->platform_max && val > mc->platform_max) - return -EINVAL; - if (val > max - min) - return -EINVAL; val = (val + min) & mask; if (invert) val = max - val; val_mask = mask << shift; val = val << shift; if (snd_soc_volsw_is_stereo(mc)) { - if (ucontrol->value.integer.value[1] < 0) - return -EINVAL; + ret = soc_mixer_valid_ctl(mc, ucontrol->value.integer.value[1], + mc->max - mc->min); + if (ret) + return ret; + val2 = ucontrol->value.integer.value[1]; - if (mc->platform_max && val2 > mc->platform_max) - return -EINVAL; - if (val2 > max - min) - return -EINVAL; val2 = (val2 + min) & mask; if (invert) val2 = max - val2; @@ -435,19 +447,16 @@ int snd_soc_put_volsw_sx(struct snd_kcontrol *kcontrol, unsigned int shift = mc->shift; unsigned int rshift = mc->rshift; unsigned int val, val_mask; - int max = mc->max; int min = mc->min; unsigned int mask = soc_mixer_sx_mask(mc); int err = 0; int ret; - if (ucontrol->value.integer.value[0] < 0) - return -EINVAL; + ret = soc_mixer_valid_ctl(mc, ucontrol->value.integer.value[0], mc->max); + if (ret) + return ret; + val = ucontrol->value.integer.value[0]; - if (mc->platform_max && val > mc->platform_max) - return -EINVAL; - if (val > max) - return -EINVAL; val_mask = mask << shift; val = (val + min) & mask; val = val << shift; @@ -458,13 +467,14 @@ int snd_soc_put_volsw_sx(struct snd_kcontrol *kcontrol, ret = err; if (snd_soc_volsw_is_stereo(mc)) { - unsigned int val2 = ucontrol->value.integer.value[1]; + unsigned int val2; - if (mc->platform_max && val2 > mc->platform_max) - return -EINVAL; - if (val2 > max) - return -EINVAL; + ret = soc_mixer_valid_ctl(mc, ucontrol->value.integer.value[1], + mc->max); + if (ret) + return ret; + val2 = ucontrol->value.integer.value[1]; val_mask = mask << rshift; val2 = (val2 + min) & mask; val2 = val2 << rshift; @@ -534,15 +544,12 @@ int snd_soc_put_volsw_range(struct snd_kcontrol *kcontrol, unsigned int mask = soc_mixer_mask(mc); unsigned int invert = mc->invert; unsigned int val, val_mask; - int err, ret, tmp; + int err, ret; - tmp = ucontrol->value.integer.value[0]; - if (tmp < 0) - return -EINVAL; - if (mc->platform_max && tmp > mc->platform_max) - return -EINVAL; - if (tmp > mc->max - mc->min) - return -EINVAL; + ret = soc_mixer_valid_ctl(mc, ucontrol->value.integer.value[0], + mc->max - mc->min); + if (ret) + return ret; if (invert) val = (max - ucontrol->value.integer.value[0]) & mask; @@ -557,13 +564,10 @@ int snd_soc_put_volsw_range(struct snd_kcontrol *kcontrol, ret = err; if (snd_soc_volsw_is_stereo(mc)) { - tmp = ucontrol->value.integer.value[1]; - if (tmp < 0) - return -EINVAL; - if (mc->platform_max && tmp > mc->platform_max) - return -EINVAL; - if (tmp > mc->max - mc->min) - return -EINVAL; + ret = soc_mixer_valid_ctl(mc, ucontrol->value.integer.value[1], + mc->max - mc->min); + if (ret) + return ret; if (invert) val = (max - ucontrol->value.integer.value[1]) & mask; From 1522aacd0114069b7f01f047b9e5b159399af295 Mon Sep 17 00:00:00 2001 From: Charles Keepax Date: Tue, 18 Mar 2025 17:14:50 +0000 Subject: [PATCH 06/15] ASoC: ops: Replace snd_soc_read_signed() with new helper The current snd_soc_read_signed() helper is only used from snd_soc_get_volsw() and can be implemented more simply with sign_extend. Remove snd_soc_read_signed() and add a new soc_mixer_reg_to_ctl() helper. This new helper does not include the reading of the register, but does include min and max handling. This allows the helper to replace more of the duplicated code and makes it easier to process the differences between single, double register and double shift style controls. It is worth noting this adds support for sign_bit into the _range and sx callbacks and the invert option to sx callbacks. Signed-off-by: Charles Keepax Link: https://patch.msgid.link/20250318171459.3203730-7-ckeepax@opensource.cirrus.com Signed-off-by: Mark Brown --- sound/soc/soc-ops.c | 134 ++++++++++++++------------------------------ 1 file changed, 41 insertions(+), 93 deletions(-) diff --git a/sound/soc/soc-ops.c b/sound/soc/soc-ops.c index 53a141426a96..af4e67817317 100644 --- a/sound/soc/soc-ops.c +++ b/sound/soc/soc-ops.c @@ -110,52 +110,20 @@ int snd_soc_put_enum_double(struct snd_kcontrol *kcontrol, } EXPORT_SYMBOL_GPL(snd_soc_put_enum_double); -/** - * snd_soc_read_signed - Read a codec register and interpret as signed value - * @component: component - * @reg: Register to read - * @mask: Mask to use after shifting the register value - * @shift: Right shift of register value - * @sign_bit: Bit that describes if a number is negative or not. - * @signed_val: Pointer to where the read value should be stored - * - * This functions reads a codec register. The register value is shifted right - * by 'shift' bits and masked with the given 'mask'. Afterwards it translates - * the given registervalue into a signed integer if sign_bit is non-zero. - */ -static void snd_soc_read_signed(struct snd_soc_component *component, - unsigned int reg, unsigned int mask, - unsigned int shift, unsigned int sign_bit, - int *signed_val) +static int soc_mixer_reg_to_ctl(struct soc_mixer_control *mc, unsigned int reg_val, + unsigned int mask, unsigned int shift, int max) { - int ret; - unsigned int val; + int val = (reg_val >> shift) & mask; - val = snd_soc_component_read(component, reg); - val = (val >> shift) & mask; + if (mc->sign_bit) + val = sign_extend32(val, mc->sign_bit); - if (!sign_bit) { - *signed_val = val; - return; - } + val -= mc->min; - /* non-negative number */ - if (!(val & BIT(sign_bit))) { - *signed_val = val; - return; - } + if (mc->invert) + val = max - val; - ret = val; - - /* - * The register most probably does not contain a full-sized int. - * Instead we have an arbitrary number of bits in a signed - * representation which has to be translated into a full-sized int. - * This is done by filling up all bits above the sign-bit. - */ - ret |= ~((int)(BIT(sign_bit) - 1)); - - *signed_val = ret; + return val & mask; } static int soc_mixer_valid_ctl(struct soc_mixer_control *mc, long val, int max) @@ -281,34 +249,25 @@ int snd_soc_get_volsw(struct snd_kcontrol *kcontrol, struct snd_soc_component *component = snd_kcontrol_chip(kcontrol); struct soc_mixer_control *mc = (struct soc_mixer_control *)kcontrol->private_value; - unsigned int reg = mc->reg; - unsigned int reg2 = mc->rreg; - unsigned int shift = mc->shift; - unsigned int rshift = mc->rshift; - int max = mc->max; - int min = mc->min; - int sign_bit = mc->sign_bit; + int max = mc->max - mc->min; unsigned int mask = soc_mixer_mask(mc); - unsigned int invert = mc->invert; + unsigned int reg_val; int val; - snd_soc_read_signed(component, reg, mask, shift, sign_bit, &val); + reg_val = snd_soc_component_read(component, mc->reg); + val = soc_mixer_reg_to_ctl(mc, reg_val, mask, mc->shift, max); - ucontrol->value.integer.value[0] = val - min; - if (invert) - ucontrol->value.integer.value[0] = - max - ucontrol->value.integer.value[0]; + ucontrol->value.integer.value[0] = val; if (snd_soc_volsw_is_stereo(mc)) { - if (reg == reg2) - snd_soc_read_signed(component, reg, mask, rshift, sign_bit, &val); - else - snd_soc_read_signed(component, reg2, mask, shift, sign_bit, &val); + if (mc->reg == mc->rreg) { + val = soc_mixer_reg_to_ctl(mc, reg_val, mask, mc->rshift, max); + } else { + reg_val = snd_soc_component_read(component, mc->rreg); + val = soc_mixer_reg_to_ctl(mc, reg_val, mask, mc->shift, max); + } - ucontrol->value.integer.value[1] = val - min; - if (invert) - ucontrol->value.integer.value[1] = - max - ucontrol->value.integer.value[1]; + ucontrol->value.integer.value[1] = val; } return 0; @@ -408,18 +367,19 @@ int snd_soc_get_volsw_sx(struct snd_kcontrol *kcontrol, (struct soc_mixer_control *)kcontrol->private_value; unsigned int reg = mc->reg; unsigned int reg2 = mc->rreg; - unsigned int shift = mc->shift; - unsigned int rshift = mc->rshift; - int min = mc->min; unsigned int mask = soc_mixer_sx_mask(mc); - unsigned int val; + unsigned int reg_val; + int val; - val = snd_soc_component_read(component, reg); - ucontrol->value.integer.value[0] = ((val >> shift) - min) & mask; + reg_val = snd_soc_component_read(component, reg); + val = soc_mixer_reg_to_ctl(mc, reg_val, mask, mc->shift, mc->max); + + ucontrol->value.integer.value[0] = val; if (snd_soc_volsw_is_stereo(mc)) { - val = snd_soc_component_read(component, reg2); - val = ((val >> rshift) - min) & mask; + reg_val = snd_soc_component_read(component, reg2); + val = soc_mixer_reg_to_ctl(mc, reg_val, mask, mc->rshift, mc->max); + ucontrol->value.integer.value[1] = val; } @@ -602,33 +562,21 @@ int snd_soc_get_volsw_range(struct snd_kcontrol *kcontrol, struct snd_soc_component *component = snd_kcontrol_chip(kcontrol); struct soc_mixer_control *mc = (struct soc_mixer_control *)kcontrol->private_value; - unsigned int reg = mc->reg; - unsigned int rreg = mc->rreg; - unsigned int shift = mc->shift; - int min = mc->min; - int max = mc->max; + int max = mc->max - mc->min; unsigned int mask = soc_mixer_mask(mc); - unsigned int invert = mc->invert; - unsigned int val; + unsigned int reg_val; + int val; - val = snd_soc_component_read(component, reg); - ucontrol->value.integer.value[0] = (val >> shift) & mask; - if (invert) - ucontrol->value.integer.value[0] = - max - ucontrol->value.integer.value[0]; - else - ucontrol->value.integer.value[0] = - ucontrol->value.integer.value[0] - min; + reg_val = snd_soc_component_read(component, mc->reg); + val = soc_mixer_reg_to_ctl(mc, reg_val, mask, mc->shift, max); + + ucontrol->value.integer.value[0] = val; if (snd_soc_volsw_is_stereo(mc)) { - val = snd_soc_component_read(component, rreg); - ucontrol->value.integer.value[1] = (val >> shift) & mask; - if (invert) - ucontrol->value.integer.value[1] = - max - ucontrol->value.integer.value[1]; - else - ucontrol->value.integer.value[1] = - ucontrol->value.integer.value[1] - min; + reg_val = snd_soc_component_read(component, mc->rreg); + val = soc_mixer_reg_to_ctl(mc, reg_val, mask, mc->shift, max); + + ucontrol->value.integer.value[1] = val; } return 0; From ed336066202c02f0b0e47d0cf08fd8f40a42351f Mon Sep 17 00:00:00 2001 From: Charles Keepax Date: Tue, 18 Mar 2025 17:14:51 +0000 Subject: [PATCH 07/15] ASoC: ops: Add control to register value helper Add a helper function to convert from control values to register values that can replace a lot of the duplicated code in the various put handlers. This also fixes a small issue in snd_soc_put_volsw where the value is converted to a control value before doing the invert, but the invert is done against the register max which will result in incorrect values for inverted controls with a non-zero minimum. Signed-off-by: Charles Keepax Link: https://patch.msgid.link/20250318171459.3203730-8-ckeepax@opensource.cirrus.com Signed-off-by: Mark Brown --- sound/soc/soc-ops.c | 97 ++++++++++++++++++++------------------------- 1 file changed, 43 insertions(+), 54 deletions(-) diff --git a/sound/soc/soc-ops.c b/sound/soc/soc-ops.c index af4e67817317..888afdd23f84 100644 --- a/sound/soc/soc-ops.c +++ b/sound/soc/soc-ops.c @@ -126,6 +126,20 @@ static int soc_mixer_reg_to_ctl(struct soc_mixer_control *mc, unsigned int reg_v return val & mask; } +static unsigned int soc_mixer_ctl_to_reg(struct soc_mixer_control *mc, int val, + unsigned int mask, unsigned int shift, + int max) +{ + unsigned int reg_val; + + if (mc->invert) + val = max - val; + + reg_val = val + mc->min; + + return (reg_val & mask) << shift; +} + static int soc_mixer_valid_ctl(struct soc_mixer_control *mc, long val, int max) { if (val < 0) @@ -292,43 +306,35 @@ int snd_soc_put_volsw(struct snd_kcontrol *kcontrol, (struct soc_mixer_control *)kcontrol->private_value; unsigned int reg = mc->reg; unsigned int reg2 = mc->rreg; - unsigned int shift = mc->shift; - unsigned int rshift = mc->rshift; - int max = mc->max; - int min = mc->min; + int max = mc->max - mc->min; unsigned int mask = soc_mixer_mask(mc); - unsigned int invert = mc->invert; int err, ret; bool type_2r = false; unsigned int val2 = 0; unsigned int val, val_mask; - ret = soc_mixer_valid_ctl(mc, ucontrol->value.integer.value[0], - mc->max - mc->min); + ret = soc_mixer_valid_ctl(mc, ucontrol->value.integer.value[0], max); if (ret) return ret; - val = ucontrol->value.integer.value[0]; - val = (val + min) & mask; - if (invert) - val = max - val; - val_mask = mask << shift; - val = val << shift; + val = soc_mixer_ctl_to_reg(mc, ucontrol->value.integer.value[0], + mask, mc->shift, max); + val_mask = mask << mc->shift; + if (snd_soc_volsw_is_stereo(mc)) { - ret = soc_mixer_valid_ctl(mc, ucontrol->value.integer.value[1], - mc->max - mc->min); + ret = soc_mixer_valid_ctl(mc, ucontrol->value.integer.value[1], max); if (ret) return ret; - val2 = ucontrol->value.integer.value[1]; - val2 = (val2 + min) & mask; - if (invert) - val2 = max - val2; if (reg == reg2) { - val_mask |= mask << rshift; - val |= val2 << rshift; + val |= soc_mixer_ctl_to_reg(mc, + ucontrol->value.integer.value[1], + mask, mc->rshift, max); + val_mask |= mask << mc->rshift; } else { - val2 = val2 << shift; + val2 = soc_mixer_ctl_to_reg(mc, + ucontrol->value.integer.value[1], + mask, mc->shift, max); type_2r = true; } } @@ -404,10 +410,7 @@ int snd_soc_put_volsw_sx(struct snd_kcontrol *kcontrol, (struct soc_mixer_control *)kcontrol->private_value; unsigned int reg = mc->reg; unsigned int reg2 = mc->rreg; - unsigned int shift = mc->shift; - unsigned int rshift = mc->rshift; unsigned int val, val_mask; - int min = mc->min; unsigned int mask = soc_mixer_sx_mask(mc); int err = 0; int ret; @@ -416,10 +419,9 @@ int snd_soc_put_volsw_sx(struct snd_kcontrol *kcontrol, if (ret) return ret; - val = ucontrol->value.integer.value[0]; - val_mask = mask << shift; - val = (val + min) & mask; - val = val << shift; + val = soc_mixer_ctl_to_reg(mc, ucontrol->value.integer.value[0], + mask, mc->shift, mc->max); + val_mask = mask << mc->shift; err = snd_soc_component_update_bits(component, reg, val_mask, val); if (err < 0) @@ -427,20 +429,17 @@ int snd_soc_put_volsw_sx(struct snd_kcontrol *kcontrol, ret = err; if (snd_soc_volsw_is_stereo(mc)) { - unsigned int val2; - ret = soc_mixer_valid_ctl(mc, ucontrol->value.integer.value[1], mc->max); if (ret) return ret; - val2 = ucontrol->value.integer.value[1]; - val_mask = mask << rshift; - val2 = (val2 + min) & mask; - val2 = val2 << rshift; + val = soc_mixer_ctl_to_reg(mc, ucontrol->value.integer.value[1], + mask, mc->rshift, mc->max); + val_mask = mask << mc->rshift; err = snd_soc_component_update_bits(component, reg2, val_mask, - val2); + val); /* Don't discard any error code or drop change flag */ if (ret == 0 || err < 0) @@ -498,12 +497,10 @@ int snd_soc_put_volsw_range(struct snd_kcontrol *kcontrol, struct snd_soc_component *component = snd_kcontrol_chip(kcontrol); unsigned int reg = mc->reg; unsigned int rreg = mc->rreg; - unsigned int shift = mc->shift; - int min = mc->min; - int max = mc->max; + int max = mc->max - mc->min; unsigned int mask = soc_mixer_mask(mc); - unsigned int invert = mc->invert; - unsigned int val, val_mask; + unsigned int val_mask = mask << mc->shift; + unsigned int val; int err, ret; ret = soc_mixer_valid_ctl(mc, ucontrol->value.integer.value[0], @@ -511,12 +508,8 @@ int snd_soc_put_volsw_range(struct snd_kcontrol *kcontrol, if (ret) return ret; - if (invert) - val = (max - ucontrol->value.integer.value[0]) & mask; - else - val = ((ucontrol->value.integer.value[0] + min) & mask); - val_mask = mask << shift; - val = val << shift; + val = soc_mixer_ctl_to_reg(mc, ucontrol->value.integer.value[0], + mask, mc->shift, max); err = snd_soc_component_update_bits(component, reg, val_mask, val); if (err < 0) @@ -525,16 +518,12 @@ int snd_soc_put_volsw_range(struct snd_kcontrol *kcontrol, if (snd_soc_volsw_is_stereo(mc)) { ret = soc_mixer_valid_ctl(mc, ucontrol->value.integer.value[1], - mc->max - mc->min); + max); if (ret) return ret; - if (invert) - val = (max - ucontrol->value.integer.value[1]) & mask; - else - val = ((ucontrol->value.integer.value[1] + min) & mask); - val_mask = mask << shift; - val = val << shift; + val = soc_mixer_ctl_to_reg(mc, ucontrol->value.integer.value[1], + mask, mc->shift, max); err = snd_soc_component_update_bits(component, rreg, val_mask, val); From 894a37c9de4b8a447ffa609d552e91ccb447c0a9 Mon Sep 17 00:00:00 2001 From: Charles Keepax Date: Tue, 18 Mar 2025 17:14:52 +0000 Subject: [PATCH 08/15] ASoC: ops: Remove snd_soc_info_volsw_range() The only difference between snd_soc_info_volsw() and snd_soc_info_volsw_range() is that the later will not force a 2 value control to be of type integer if the name ends in "Volume". The kernel currently contains no users of snd_soc_info_volsw_range() that would return a boolean control with this code, so the risk is quite low and it seems appropriate that it should contain volume control detection. So remove snd_soc_info_volsw_range() and point its users at snd_soc_info_volsw(). Signed-off-by: Charles Keepax Link: https://patch.msgid.link/20250318171459.3203730-9-ckeepax@opensource.cirrus.com Signed-off-by: Mark Brown --- include/sound/soc.h | 12 +++++------ sound/pci/hda/tas2781_hda_i2c.c | 2 +- sound/pci/hda/tas2781_hda_spi.c | 2 +- sound/soc/soc-ops.c | 36 +++------------------------------ sound/soc/soc-topology.c | 2 +- 5 files changed, 11 insertions(+), 43 deletions(-) diff --git a/include/sound/soc.h b/include/sound/soc.h index d73fe26de166..b310f2c599f8 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -65,7 +65,7 @@ struct platform_device; .private_value = SOC_SINGLE_VALUE(reg, shift, 0, max, invert, 0) } #define SOC_SINGLE_RANGE(xname, xreg, xshift, xmin, xmax, xinvert) \ { .iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = (xname),\ - .info = snd_soc_info_volsw_range, .get = snd_soc_get_volsw_range, \ + .info = snd_soc_info_volsw, .get = snd_soc_get_volsw_range, \ .put = snd_soc_put_volsw_range, \ .private_value = SOC_SINGLE_VALUE(xreg, xshift, xmin, xmax, xinvert, 0) } #define SOC_SINGLE_TLV(xname, reg, shift, max, invert, tlv_array) \ @@ -90,7 +90,7 @@ struct platform_device; .access = SNDRV_CTL_ELEM_ACCESS_TLV_READ |\ SNDRV_CTL_ELEM_ACCESS_READWRITE,\ .tlv.p = (tlv_array), \ - .info = snd_soc_info_volsw_range, \ + .info = snd_soc_info_volsw, \ .get = snd_soc_get_volsw_range, .put = snd_soc_put_volsw_range, \ .private_value = SOC_SINGLE_VALUE(xreg, xshift, xmin, xmax, xinvert, 0) } #define SOC_DOUBLE(xname, reg, shift_left, shift_right, max, invert) \ @@ -116,7 +116,7 @@ struct platform_device; #define SOC_DOUBLE_R_RANGE(xname, reg_left, reg_right, xshift, xmin, \ xmax, xinvert) \ { .iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = (xname),\ - .info = snd_soc_info_volsw_range, \ + .info = snd_soc_info_volsw, \ .get = snd_soc_get_volsw_range, .put = snd_soc_put_volsw_range, \ .private_value = SOC_DOUBLE_R_VALUE(reg_left, reg_right, \ xshift, xmin, xmax, xinvert) } @@ -164,7 +164,7 @@ struct platform_device; .access = SNDRV_CTL_ELEM_ACCESS_TLV_READ |\ SNDRV_CTL_ELEM_ACCESS_READWRITE,\ .tlv.p = (tlv_array), \ - .info = snd_soc_info_volsw_range, \ + .info = snd_soc_info_volsw, \ .get = snd_soc_get_volsw_range, .put = snd_soc_put_volsw_range, \ .private_value = SOC_DOUBLE_R_VALUE(reg_left, reg_right, \ xshift, xmin, xmax, xinvert) } @@ -266,7 +266,7 @@ struct platform_device; .access = SNDRV_CTL_ELEM_ACCESS_TLV_READ |\ SNDRV_CTL_ELEM_ACCESS_READWRITE,\ .tlv.p = (tlv_array), \ - .info = snd_soc_info_volsw_range, \ + .info = snd_soc_info_volsw, \ .get = xhandler_get, .put = xhandler_put, \ .private_value = SOC_SINGLE_VALUE(xreg, xshift, xmin, xmax, xinvert, 0) } #define SOC_DOUBLE_EXT_TLV(xname, xreg, shift_left, shift_right, xmax, xinvert,\ @@ -569,8 +569,6 @@ int snd_soc_get_volsw_sx(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol); int snd_soc_put_volsw_sx(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol); -int snd_soc_info_volsw_range(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_info *uinfo); int snd_soc_put_volsw_range(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol); int snd_soc_get_volsw_range(struct snd_kcontrol *kcontrol, diff --git a/sound/pci/hda/tas2781_hda_i2c.c b/sound/pci/hda/tas2781_hda_i2c.c index be9a90f643eb..50c5e5f26589 100644 --- a/sound/pci/hda/tas2781_hda_i2c.c +++ b/sound/pci/hda/tas2781_hda_i2c.c @@ -45,7 +45,7 @@ .access = SNDRV_CTL_ELEM_ACCESS_TLV_READ |\ SNDRV_CTL_ELEM_ACCESS_READWRITE,\ .tlv.p = (tlv_array), \ - .info = snd_soc_info_volsw_range, \ + .info = snd_soc_info_volsw, \ .get = xhandler_get, .put = xhandler_put, \ .private_value = (unsigned long)&(struct soc_mixer_control) \ {.reg = xreg, .rreg = xreg, .shift = xshift, \ diff --git a/sound/pci/hda/tas2781_hda_spi.c b/sound/pci/hda/tas2781_hda_spi.c index 00676cbb2c8e..399f2e4c3b62 100644 --- a/sound/pci/hda/tas2781_hda_spi.c +++ b/sound/pci/hda/tas2781_hda_spi.c @@ -52,7 +52,7 @@ .access = SNDRV_CTL_ELEM_ACCESS_TLV_READ | \ SNDRV_CTL_ELEM_ACCESS_READWRITE, \ .tlv.p = (tlv_array), \ - .info = snd_soc_info_volsw_range, \ + .info = snd_soc_info_volsw, \ .get = xhandler_get, .put = xhandler_put, \ .private_value = (unsigned long)&(struct soc_mixer_control) { \ .reg = xreg, .rreg = xreg, \ diff --git a/sound/soc/soc-ops.c b/sound/soc/soc-ops.c index 888afdd23f84..1b52ba12df8d 100644 --- a/sound/soc/soc-ops.c +++ b/sound/soc/soc-ops.c @@ -169,12 +169,12 @@ static int soc_mixer_sx_mask(struct soc_mixer_control *mc) } /** - * snd_soc_info_volsw - single mixer info callback + * snd_soc_info_volsw - single mixer info callback with range. * @kcontrol: mixer control * @uinfo: control element information * - * Callback to provide information about a single mixer control, or a double - * mixer control that spans 2 registers. + * Callback to provide information, with a range, about a single mixer control, + * or a double mixer control that spans 2 registers. * * Returns 0 for success. */ @@ -450,36 +450,6 @@ int snd_soc_put_volsw_sx(struct snd_kcontrol *kcontrol, } EXPORT_SYMBOL_GPL(snd_soc_put_volsw_sx); -/** - * snd_soc_info_volsw_range - single mixer info callback with range. - * @kcontrol: mixer control - * @uinfo: control element information - * - * Callback to provide information, within a range, about a single - * mixer control. - * - * returns 0 for success. - */ -int snd_soc_info_volsw_range(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_info *uinfo) -{ - struct soc_mixer_control *mc = - (struct soc_mixer_control *)kcontrol->private_value; - int max; - - max = mc->max - mc->min; - if (mc->platform_max && mc->platform_max < max) - max = mc->platform_max; - - uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER; - uinfo->count = snd_soc_volsw_is_stereo(mc) ? 2 : 1; - uinfo->value.integer.min = 0; - uinfo->value.integer.max = max; - - return 0; -} -EXPORT_SYMBOL_GPL(snd_soc_info_volsw_range); - /** * snd_soc_put_volsw_range - single mixer put value callback with range. * @kcontrol: mixer control diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index 2b86cc3311f7..9cbddfbbc556 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -132,7 +132,7 @@ static const struct snd_soc_tplg_kcontrol_ops io_ops[] = { {SND_SOC_TPLG_CTL_BYTES, snd_soc_bytes_get, snd_soc_bytes_put, snd_soc_bytes_info}, {SND_SOC_TPLG_CTL_RANGE, snd_soc_get_volsw_range, - snd_soc_put_volsw_range, snd_soc_info_volsw_range}, + snd_soc_put_volsw_range, snd_soc_info_volsw}, {SND_SOC_TPLG_CTL_VOLSW_XR_SX, snd_soc_get_xr_sx, snd_soc_put_xr_sx, snd_soc_info_xr_sx}, {SND_SOC_TPLG_CTL_STROBE, snd_soc_get_strobe, From fd7442561cfe9516b37cdb1d229dc1f811dc86cc Mon Sep 17 00:00:00 2001 From: Charles Keepax Date: Tue, 18 Mar 2025 17:14:53 +0000 Subject: [PATCH 09/15] ASoC: ops: Remove snd_soc_get_volsw_range() With the addition of the soc_mixer_reg_to_ctl() helper it is now very clear that the only difference between snd_soc_get_volsw() and snd_soc_get_volsw_range() is that the former supports double controls with both values in the same register. As such we can combine both functions. Signed-off-by: Charles Keepax Link: https://patch.msgid.link/20250318171459.3203730-10-ckeepax@opensource.cirrus.com Signed-off-by: Mark Brown --- include/sound/soc.h | 10 ++++------ sound/soc/codecs/wm5110.c | 2 +- sound/soc/soc-ops.c | 42 +++------------------------------------ sound/soc/soc-topology.c | 2 +- 4 files changed, 9 insertions(+), 47 deletions(-) diff --git a/include/sound/soc.h b/include/sound/soc.h index b310f2c599f8..b11c6cdb0201 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -65,7 +65,7 @@ struct platform_device; .private_value = SOC_SINGLE_VALUE(reg, shift, 0, max, invert, 0) } #define SOC_SINGLE_RANGE(xname, xreg, xshift, xmin, xmax, xinvert) \ { .iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = (xname),\ - .info = snd_soc_info_volsw, .get = snd_soc_get_volsw_range, \ + .info = snd_soc_info_volsw, .get = snd_soc_get_volsw, \ .put = snd_soc_put_volsw_range, \ .private_value = SOC_SINGLE_VALUE(xreg, xshift, xmin, xmax, xinvert, 0) } #define SOC_SINGLE_TLV(xname, reg, shift, max, invert, tlv_array) \ @@ -91,7 +91,7 @@ struct platform_device; SNDRV_CTL_ELEM_ACCESS_READWRITE,\ .tlv.p = (tlv_array), \ .info = snd_soc_info_volsw, \ - .get = snd_soc_get_volsw_range, .put = snd_soc_put_volsw_range, \ + .get = snd_soc_get_volsw, .put = snd_soc_put_volsw_range, \ .private_value = SOC_SINGLE_VALUE(xreg, xshift, xmin, xmax, xinvert, 0) } #define SOC_DOUBLE(xname, reg, shift_left, shift_right, max, invert) \ { .iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = (xname),\ @@ -117,7 +117,7 @@ struct platform_device; xmax, xinvert) \ { .iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = (xname),\ .info = snd_soc_info_volsw, \ - .get = snd_soc_get_volsw_range, .put = snd_soc_put_volsw_range, \ + .get = snd_soc_get_volsw, .put = snd_soc_put_volsw_range, \ .private_value = SOC_DOUBLE_R_VALUE(reg_left, reg_right, \ xshift, xmin, xmax, xinvert) } #define SOC_DOUBLE_TLV(xname, reg, shift_left, shift_right, max, invert, tlv_array) \ @@ -165,7 +165,7 @@ struct platform_device; SNDRV_CTL_ELEM_ACCESS_READWRITE,\ .tlv.p = (tlv_array), \ .info = snd_soc_info_volsw, \ - .get = snd_soc_get_volsw_range, .put = snd_soc_put_volsw_range, \ + .get = snd_soc_get_volsw, .put = snd_soc_put_volsw_range, \ .private_value = SOC_DOUBLE_R_VALUE(reg_left, reg_right, \ xshift, xmin, xmax, xinvert) } #define SOC_DOUBLE_R_SX_TLV(xname, xreg, xrreg, xshift, xmin, xmax, tlv_array) \ @@ -571,8 +571,6 @@ int snd_soc_put_volsw_sx(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol); int snd_soc_put_volsw_range(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol); -int snd_soc_get_volsw_range(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_value *ucontrol); int snd_soc_limit_volume(struct snd_soc_card *card, const char *name, int max); int snd_soc_bytes_info(struct snd_kcontrol *kcontrol, diff --git a/sound/soc/codecs/wm5110.c b/sound/soc/codecs/wm5110.c index 64eee0d2347d..c24b42c37578 100644 --- a/sound/soc/codecs/wm5110.c +++ b/sound/soc/codecs/wm5110.c @@ -477,7 +477,7 @@ static int wm5110_in_pga_get(struct snd_kcontrol *kcontrol, */ snd_soc_dapm_mutex_lock(dapm); - ret = snd_soc_get_volsw_range(kcontrol, ucontrol); + ret = snd_soc_get_volsw(kcontrol, ucontrol); snd_soc_dapm_mutex_unlock(dapm); diff --git a/sound/soc/soc-ops.c b/sound/soc/soc-ops.c index 1b52ba12df8d..fbda8e21c5a6 100644 --- a/sound/soc/soc-ops.c +++ b/sound/soc/soc-ops.c @@ -248,12 +248,12 @@ int snd_soc_info_volsw_sx(struct snd_kcontrol *kcontrol, EXPORT_SYMBOL_GPL(snd_soc_info_volsw_sx); /** - * snd_soc_get_volsw - single mixer get callback + * snd_soc_get_volsw - single mixer get callback with range * @kcontrol: mixer control * @ucontrol: control element information * - * Callback to get the value of a single mixer control, or a double mixer - * control that spans 2 registers. + * Callback to get the value, within a range, of a single mixer control, or a + * double mixer control that spans 2 registers. * * Returns 0 for success. */ @@ -506,42 +506,6 @@ int snd_soc_put_volsw_range(struct snd_kcontrol *kcontrol, } EXPORT_SYMBOL_GPL(snd_soc_put_volsw_range); -/** - * snd_soc_get_volsw_range - single mixer get callback with range - * @kcontrol: mixer control - * @ucontrol: control element information - * - * Callback to get the value, within a range, of a single mixer control. - * - * Returns 0 for success. - */ -int snd_soc_get_volsw_range(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_value *ucontrol) -{ - struct snd_soc_component *component = snd_kcontrol_chip(kcontrol); - struct soc_mixer_control *mc = - (struct soc_mixer_control *)kcontrol->private_value; - int max = mc->max - mc->min; - unsigned int mask = soc_mixer_mask(mc); - unsigned int reg_val; - int val; - - reg_val = snd_soc_component_read(component, mc->reg); - val = soc_mixer_reg_to_ctl(mc, reg_val, mask, mc->shift, max); - - ucontrol->value.integer.value[0] = val; - - if (snd_soc_volsw_is_stereo(mc)) { - reg_val = snd_soc_component_read(component, mc->rreg); - val = soc_mixer_reg_to_ctl(mc, reg_val, mask, mc->shift, max); - - ucontrol->value.integer.value[1] = val; - } - - return 0; -} -EXPORT_SYMBOL_GPL(snd_soc_get_volsw_range); - static int snd_soc_clip_to_platform_max(struct snd_kcontrol *kctl) { struct soc_mixer_control *mc = (struct soc_mixer_control *)kctl->private_value; diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index 9cbddfbbc556..3c988925c512 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -131,7 +131,7 @@ static const struct snd_soc_tplg_kcontrol_ops io_ops[] = { snd_soc_put_enum_double, NULL}, {SND_SOC_TPLG_CTL_BYTES, snd_soc_bytes_get, snd_soc_bytes_put, snd_soc_bytes_info}, - {SND_SOC_TPLG_CTL_RANGE, snd_soc_get_volsw_range, + {SND_SOC_TPLG_CTL_RANGE, snd_soc_get_volsw, snd_soc_put_volsw_range, snd_soc_info_volsw}, {SND_SOC_TPLG_CTL_VOLSW_XR_SX, snd_soc_get_xr_sx, snd_soc_put_xr_sx, snd_soc_info_xr_sx}, From 7d5df968f95cee274740d5fa42e0798ffb59bd38 Mon Sep 17 00:00:00 2001 From: Charles Keepax Date: Tue, 18 Mar 2025 17:14:54 +0000 Subject: [PATCH 10/15] ASoC: ops: Remove snd_soc_put_volsw_range() With the addition of the soc_mixer_ctl_to_reg() helper it is now very clear that the only difference between snd_soc_put_volsw() and snd_soc_put_volsw_range() is that the former supports double controls with both values in the same register. As such we can combine both functions. Signed-off-by: Charles Keepax Link: https://patch.msgid.link/20250318171459.3203730-11-ckeepax@opensource.cirrus.com Signed-off-by: Mark Brown --- include/sound/soc.h | 10 +++---- sound/soc/codecs/wm5110.c | 2 +- sound/soc/soc-ops.c | 62 ++------------------------------------- sound/soc/soc-topology.c | 2 +- 4 files changed, 9 insertions(+), 67 deletions(-) diff --git a/include/sound/soc.h b/include/sound/soc.h index b11c6cdb0201..952ed77b8c87 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -66,7 +66,7 @@ struct platform_device; #define SOC_SINGLE_RANGE(xname, xreg, xshift, xmin, xmax, xinvert) \ { .iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = (xname),\ .info = snd_soc_info_volsw, .get = snd_soc_get_volsw, \ - .put = snd_soc_put_volsw_range, \ + .put = snd_soc_put_volsw, \ .private_value = SOC_SINGLE_VALUE(xreg, xshift, xmin, xmax, xinvert, 0) } #define SOC_SINGLE_TLV(xname, reg, shift, max, invert, tlv_array) \ { .iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, \ @@ -91,7 +91,7 @@ struct platform_device; SNDRV_CTL_ELEM_ACCESS_READWRITE,\ .tlv.p = (tlv_array), \ .info = snd_soc_info_volsw, \ - .get = snd_soc_get_volsw, .put = snd_soc_put_volsw_range, \ + .get = snd_soc_get_volsw, .put = snd_soc_put_volsw, \ .private_value = SOC_SINGLE_VALUE(xreg, xshift, xmin, xmax, xinvert, 0) } #define SOC_DOUBLE(xname, reg, shift_left, shift_right, max, invert) \ { .iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = (xname),\ @@ -117,7 +117,7 @@ struct platform_device; xmax, xinvert) \ { .iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = (xname),\ .info = snd_soc_info_volsw, \ - .get = snd_soc_get_volsw, .put = snd_soc_put_volsw_range, \ + .get = snd_soc_get_volsw, .put = snd_soc_put_volsw, \ .private_value = SOC_DOUBLE_R_VALUE(reg_left, reg_right, \ xshift, xmin, xmax, xinvert) } #define SOC_DOUBLE_TLV(xname, reg, shift_left, shift_right, max, invert, tlv_array) \ @@ -165,7 +165,7 @@ struct platform_device; SNDRV_CTL_ELEM_ACCESS_READWRITE,\ .tlv.p = (tlv_array), \ .info = snd_soc_info_volsw, \ - .get = snd_soc_get_volsw, .put = snd_soc_put_volsw_range, \ + .get = snd_soc_get_volsw, .put = snd_soc_put_volsw, \ .private_value = SOC_DOUBLE_R_VALUE(reg_left, reg_right, \ xshift, xmin, xmax, xinvert) } #define SOC_DOUBLE_R_SX_TLV(xname, xreg, xrreg, xshift, xmin, xmax, tlv_array) \ @@ -569,8 +569,6 @@ int snd_soc_get_volsw_sx(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol); int snd_soc_put_volsw_sx(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol); -int snd_soc_put_volsw_range(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_value *ucontrol); int snd_soc_limit_volume(struct snd_soc_card *card, const char *name, int max); int snd_soc_bytes_info(struct snd_kcontrol *kcontrol, diff --git a/sound/soc/codecs/wm5110.c b/sound/soc/codecs/wm5110.c index c24b42c37578..212eca675f27 100644 --- a/sound/soc/codecs/wm5110.c +++ b/sound/soc/codecs/wm5110.c @@ -497,7 +497,7 @@ static int wm5110_in_pga_put(struct snd_kcontrol *kcontrol, */ snd_soc_dapm_mutex_lock(dapm); - ret = snd_soc_put_volsw_range(kcontrol, ucontrol); + ret = snd_soc_put_volsw(kcontrol, ucontrol); snd_soc_dapm_mutex_unlock(dapm); diff --git a/sound/soc/soc-ops.c b/sound/soc/soc-ops.c index fbda8e21c5a6..d26d9e050af1 100644 --- a/sound/soc/soc-ops.c +++ b/sound/soc/soc-ops.c @@ -289,12 +289,12 @@ int snd_soc_get_volsw(struct snd_kcontrol *kcontrol, EXPORT_SYMBOL_GPL(snd_soc_get_volsw); /** - * snd_soc_put_volsw - single mixer put callback + * snd_soc_put_volsw - single mixer put callback with range * @kcontrol: mixer control * @ucontrol: control element information * - * Callback to set the value of a single mixer control, or a double mixer - * control that spans 2 registers. + * Callback to set the value , within a range, of a single mixer control, or + * a double mixer control that spans 2 registers. * * Returns 0 for success. */ @@ -450,62 +450,6 @@ int snd_soc_put_volsw_sx(struct snd_kcontrol *kcontrol, } EXPORT_SYMBOL_GPL(snd_soc_put_volsw_sx); -/** - * snd_soc_put_volsw_range - single mixer put value callback with range. - * @kcontrol: mixer control - * @ucontrol: control element information - * - * Callback to set the value, within a range, for a single mixer control. - * - * Returns 0 for success. - */ -int snd_soc_put_volsw_range(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_value *ucontrol) -{ - struct soc_mixer_control *mc = - (struct soc_mixer_control *)kcontrol->private_value; - struct snd_soc_component *component = snd_kcontrol_chip(kcontrol); - unsigned int reg = mc->reg; - unsigned int rreg = mc->rreg; - int max = mc->max - mc->min; - unsigned int mask = soc_mixer_mask(mc); - unsigned int val_mask = mask << mc->shift; - unsigned int val; - int err, ret; - - ret = soc_mixer_valid_ctl(mc, ucontrol->value.integer.value[0], - mc->max - mc->min); - if (ret) - return ret; - - val = soc_mixer_ctl_to_reg(mc, ucontrol->value.integer.value[0], - mask, mc->shift, max); - - err = snd_soc_component_update_bits(component, reg, val_mask, val); - if (err < 0) - return err; - ret = err; - - if (snd_soc_volsw_is_stereo(mc)) { - ret = soc_mixer_valid_ctl(mc, ucontrol->value.integer.value[1], - max); - if (ret) - return ret; - - val = soc_mixer_ctl_to_reg(mc, ucontrol->value.integer.value[1], - mask, mc->shift, max); - - err = snd_soc_component_update_bits(component, rreg, val_mask, - val); - /* Don't discard any error code or drop change flag */ - if (ret == 0 || err < 0) - ret = err; - } - - return ret; -} -EXPORT_SYMBOL_GPL(snd_soc_put_volsw_range); - static int snd_soc_clip_to_platform_max(struct snd_kcontrol *kctl) { struct soc_mixer_control *mc = (struct soc_mixer_control *)kctl->private_value; diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index 3c988925c512..7b0b8531bb32 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -132,7 +132,7 @@ static const struct snd_soc_tplg_kcontrol_ops io_ops[] = { {SND_SOC_TPLG_CTL_BYTES, snd_soc_bytes_get, snd_soc_bytes_put, snd_soc_bytes_info}, {SND_SOC_TPLG_CTL_RANGE, snd_soc_get_volsw, - snd_soc_put_volsw_range, snd_soc_info_volsw}, + snd_soc_put_volsw, snd_soc_info_volsw}, {SND_SOC_TPLG_CTL_VOLSW_XR_SX, snd_soc_get_xr_sx, snd_soc_put_xr_sx, snd_soc_info_xr_sx}, {SND_SOC_TPLG_CTL_STROBE, snd_soc_get_strobe, From 9dfcafe2037acc14265cead8d8a937a8bc4e01d8 Mon Sep 17 00:00:00 2001 From: Charles Keepax Date: Tue, 18 Mar 2025 17:14:55 +0000 Subject: [PATCH 11/15] ASoC: ops: Factor out common code from info callbacks snd_soc_info_volsw() and snd_soc_info_volsw_sx() do very similar things, and have a lot of code in common. Already this is causing some issues as the detection of volume controls has been fixed in the normal callback but not the sx callback. Factor out a new helper containing the common code and leave the function specific bits behind in each callback. Signed-off-by: Charles Keepax Link: https://patch.msgid.link/20250318171459.3203730-12-ckeepax@opensource.cirrus.com Signed-off-by: Mark Brown --- sound/soc/soc-ops.c | 64 ++++++++++++++++++--------------------------- 1 file changed, 26 insertions(+), 38 deletions(-) diff --git a/sound/soc/soc-ops.c b/sound/soc/soc-ops.c index d26d9e050af1..29537dd3a063 100644 --- a/sound/soc/soc-ops.c +++ b/sound/soc/soc-ops.c @@ -168,6 +168,30 @@ static int soc_mixer_sx_mask(struct soc_mixer_control *mc) return GENMASK(fls(mc->min + mc->max) - 2, 0); } +static int soc_info_volsw(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_info *uinfo, + struct soc_mixer_control *mc, int max) +{ + if (mc->platform_max && mc->platform_max < max) + max = mc->platform_max; + + uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER; + + if (max == 1) { + /* Even two value controls ending in Volume should be integer */ + const char *vol_string = strstr(kcontrol->id.name, " Volume"); + + if (!vol_string || strcmp(vol_string, " Volume")) + uinfo->type = SNDRV_CTL_ELEM_TYPE_BOOLEAN; + } + + uinfo->count = snd_soc_volsw_is_stereo(mc) ? 2 : 1; + uinfo->value.integer.min = 0; + uinfo->value.integer.max = max; + + return 0; +} + /** * snd_soc_info_volsw - single mixer info callback with range. * @kcontrol: mixer control @@ -183,29 +207,8 @@ int snd_soc_info_volsw(struct snd_kcontrol *kcontrol, { struct soc_mixer_control *mc = (struct soc_mixer_control *)kcontrol->private_value; - const char *vol_string = NULL; - int max; - max = uinfo->value.integer.max = mc->max - mc->min; - if (mc->platform_max && mc->platform_max < max) - max = mc->platform_max; - - if (max == 1) { - /* Even two value controls ending in Volume should always be integer */ - vol_string = strstr(kcontrol->id.name, " Volume"); - if (vol_string && !strcmp(vol_string, " Volume")) - uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER; - else - uinfo->type = SNDRV_CTL_ELEM_TYPE_BOOLEAN; - } else { - uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER; - } - - uinfo->count = snd_soc_volsw_is_stereo(mc) ? 2 : 1; - uinfo->value.integer.min = 0; - uinfo->value.integer.max = max; - - return 0; + return soc_info_volsw(kcontrol, uinfo, mc, mc->max - mc->min); } EXPORT_SYMBOL_GPL(snd_soc_info_volsw); @@ -227,23 +230,8 @@ int snd_soc_info_volsw_sx(struct snd_kcontrol *kcontrol, { struct soc_mixer_control *mc = (struct soc_mixer_control *)kcontrol->private_value; - int max; - if (mc->platform_max) - max = mc->platform_max; - else - max = mc->max; - - if (max == 1 && !strstr(kcontrol->id.name, " Volume")) - uinfo->type = SNDRV_CTL_ELEM_TYPE_BOOLEAN; - else - uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER; - - uinfo->count = snd_soc_volsw_is_stereo(mc) ? 2 : 1; - uinfo->value.integer.min = 0; - uinfo->value.integer.max = max; - - return 0; + return soc_info_volsw(kcontrol, uinfo, mc, mc->max); } EXPORT_SYMBOL_GPL(snd_soc_info_volsw_sx); From 318e8794e05ca1879441a602e78c74f9d7e18309 Mon Sep 17 00:00:00 2001 From: Charles Keepax Date: Tue, 18 Mar 2025 17:14:56 +0000 Subject: [PATCH 12/15] ASoC: ops: Factor out common code from put callbacks There are only two differences between snd_soc_put_volsw() and snd_soc_put_volsw_sx(). The maximum field is handled differently, and snd_soc_put_volsw() supports double controls with both values in the same register. Factor out the common code into a new helper and pass in the appropriate max value such that it is handled correctly for each control. Signed-off-by: Charles Keepax Link: https://patch.msgid.link/20250318171459.3203730-13-ckeepax@opensource.cirrus.com Signed-off-by: Mark Brown --- sound/soc/soc-ops.c | 138 +++++++++++++++++--------------------------- 1 file changed, 53 insertions(+), 85 deletions(-) diff --git a/sound/soc/soc-ops.c b/sound/soc/soc-ops.c index 29537dd3a063..0b62ffb2e222 100644 --- a/sound/soc/soc-ops.c +++ b/sound/soc/soc-ops.c @@ -192,6 +192,57 @@ static int soc_info_volsw(struct snd_kcontrol *kcontrol, return 0; } +static int soc_put_volsw(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol, + struct soc_mixer_control *mc, int mask, int max) +{ + struct snd_soc_component *component = snd_kcontrol_chip(kcontrol); + unsigned int val1, val_mask; + unsigned int val2 = 0; + bool double_r = false; + int ret; + + ret = soc_mixer_valid_ctl(mc, ucontrol->value.integer.value[0], max); + if (ret) + return ret; + + val1 = soc_mixer_ctl_to_reg(mc, ucontrol->value.integer.value[0], + mask, mc->shift, max); + val_mask = mask << mc->shift; + + if (snd_soc_volsw_is_stereo(mc)) { + ret = soc_mixer_valid_ctl(mc, ucontrol->value.integer.value[1], max); + if (ret) + return ret; + + if (mc->reg == mc->rreg) { + val1 |= soc_mixer_ctl_to_reg(mc, + ucontrol->value.integer.value[1], + mask, mc->rshift, max); + val_mask |= mask << mc->rshift; + } else { + val2 = soc_mixer_ctl_to_reg(mc, + ucontrol->value.integer.value[1], + mask, mc->shift, max); + double_r = true; + } + } + + ret = snd_soc_component_update_bits(component, mc->reg, val_mask, val1); + if (ret < 0) + return ret; + + if (double_r) { + int err = snd_soc_component_update_bits(component, mc->rreg, + val_mask, val2); + /* Don't drop change flag */ + if (err) + return err; + } + + return ret; +} + /** * snd_soc_info_volsw - single mixer info callback with range. * @kcontrol: mixer control @@ -289,57 +340,11 @@ EXPORT_SYMBOL_GPL(snd_soc_get_volsw); int snd_soc_put_volsw(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { - struct snd_soc_component *component = snd_kcontrol_chip(kcontrol); struct soc_mixer_control *mc = (struct soc_mixer_control *)kcontrol->private_value; - unsigned int reg = mc->reg; - unsigned int reg2 = mc->rreg; - int max = mc->max - mc->min; unsigned int mask = soc_mixer_mask(mc); - int err, ret; - bool type_2r = false; - unsigned int val2 = 0; - unsigned int val, val_mask; - ret = soc_mixer_valid_ctl(mc, ucontrol->value.integer.value[0], max); - if (ret) - return ret; - - val = soc_mixer_ctl_to_reg(mc, ucontrol->value.integer.value[0], - mask, mc->shift, max); - val_mask = mask << mc->shift; - - if (snd_soc_volsw_is_stereo(mc)) { - ret = soc_mixer_valid_ctl(mc, ucontrol->value.integer.value[1], max); - if (ret) - return ret; - - if (reg == reg2) { - val |= soc_mixer_ctl_to_reg(mc, - ucontrol->value.integer.value[1], - mask, mc->rshift, max); - val_mask |= mask << mc->rshift; - } else { - val2 = soc_mixer_ctl_to_reg(mc, - ucontrol->value.integer.value[1], - mask, mc->shift, max); - type_2r = true; - } - } - err = snd_soc_component_update_bits(component, reg, val_mask, val); - if (err < 0) - return err; - ret = err; - - if (type_2r) { - err = snd_soc_component_update_bits(component, reg2, val_mask, - val2); - /* Don't discard any error code or drop change flag */ - if (ret == 0 || err < 0) - ret = err; - } - - return ret; + return soc_put_volsw(kcontrol, ucontrol, mc, mask, mc->max - mc->min); } EXPORT_SYMBOL_GPL(snd_soc_put_volsw); @@ -393,48 +398,11 @@ EXPORT_SYMBOL_GPL(snd_soc_get_volsw_sx); int snd_soc_put_volsw_sx(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { - struct snd_soc_component *component = snd_kcontrol_chip(kcontrol); struct soc_mixer_control *mc = (struct soc_mixer_control *)kcontrol->private_value; - unsigned int reg = mc->reg; - unsigned int reg2 = mc->rreg; - unsigned int val, val_mask; unsigned int mask = soc_mixer_sx_mask(mc); - int err = 0; - int ret; - ret = soc_mixer_valid_ctl(mc, ucontrol->value.integer.value[0], mc->max); - if (ret) - return ret; - - val = soc_mixer_ctl_to_reg(mc, ucontrol->value.integer.value[0], - mask, mc->shift, mc->max); - val_mask = mask << mc->shift; - - err = snd_soc_component_update_bits(component, reg, val_mask, val); - if (err < 0) - return err; - ret = err; - - if (snd_soc_volsw_is_stereo(mc)) { - ret = soc_mixer_valid_ctl(mc, ucontrol->value.integer.value[1], - mc->max); - if (ret) - return ret; - - val = soc_mixer_ctl_to_reg(mc, ucontrol->value.integer.value[1], - mask, mc->rshift, mc->max); - val_mask = mask << mc->rshift; - - err = snd_soc_component_update_bits(component, reg2, val_mask, - val); - - /* Don't discard any error code or drop change flag */ - if (ret == 0 || err < 0) - ret = err; - } - - return ret; + return soc_put_volsw(kcontrol, ucontrol, mc, mask, mc->max); } EXPORT_SYMBOL_GPL(snd_soc_put_volsw_sx); From 1e3cd64a29baa874b89180ac0744178ecb00f3cd Mon Sep 17 00:00:00 2001 From: Charles Keepax Date: Wed, 19 Mar 2025 17:51:21 +0000 Subject: [PATCH 13/15] ASoC: ops: Factor out common code from get callbacks There are only two differences between snd_soc_get_volsw() and snd_soc_get_volsw_sx(). The maximum field is handled differently, and snd_soc_get_volsw() supports double controls with both values in the same register. Factor out the common code into a new helper and pass in the appropriate max value such that it is handled correctly for each control. Signed-off-by: Charles Keepax Link: https://patch.msgid.link/20250319175123.3835849-2-ckeepax@opensource.cirrus.com Signed-off-by: Mark Brown --- sound/soc/soc-ops.c | 68 +++++++++++++++++++-------------------------- 1 file changed, 29 insertions(+), 39 deletions(-) diff --git a/sound/soc/soc-ops.c b/sound/soc/soc-ops.c index 0b62ffb2e222..1d7c28993a63 100644 --- a/sound/soc/soc-ops.c +++ b/sound/soc/soc-ops.c @@ -243,6 +243,33 @@ static int soc_put_volsw(struct snd_kcontrol *kcontrol, return ret; } +static int soc_get_volsw(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol, + struct soc_mixer_control *mc, int mask, int max) +{ + struct snd_soc_component *component = snd_kcontrol_chip(kcontrol); + unsigned int reg_val; + int val; + + reg_val = snd_soc_component_read(component, mc->reg); + val = soc_mixer_reg_to_ctl(mc, reg_val, mask, mc->shift, max); + + ucontrol->value.integer.value[0] = val; + + if (snd_soc_volsw_is_stereo(mc)) { + if (mc->reg == mc->rreg) { + val = soc_mixer_reg_to_ctl(mc, reg_val, mask, mc->rshift, max); + } else { + reg_val = snd_soc_component_read(component, mc->rreg); + val = soc_mixer_reg_to_ctl(mc, reg_val, mask, mc->shift, max); + } + + ucontrol->value.integer.value[1] = val; + } + + return 0; +} + /** * snd_soc_info_volsw - single mixer info callback with range. * @kcontrol: mixer control @@ -299,31 +326,11 @@ EXPORT_SYMBOL_GPL(snd_soc_info_volsw_sx); int snd_soc_get_volsw(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { - struct snd_soc_component *component = snd_kcontrol_chip(kcontrol); struct soc_mixer_control *mc = (struct soc_mixer_control *)kcontrol->private_value; - int max = mc->max - mc->min; unsigned int mask = soc_mixer_mask(mc); - unsigned int reg_val; - int val; - reg_val = snd_soc_component_read(component, mc->reg); - val = soc_mixer_reg_to_ctl(mc, reg_val, mask, mc->shift, max); - - ucontrol->value.integer.value[0] = val; - - if (snd_soc_volsw_is_stereo(mc)) { - if (mc->reg == mc->rreg) { - val = soc_mixer_reg_to_ctl(mc, reg_val, mask, mc->rshift, max); - } else { - reg_val = snd_soc_component_read(component, mc->rreg); - val = soc_mixer_reg_to_ctl(mc, reg_val, mask, mc->shift, max); - } - - ucontrol->value.integer.value[1] = val; - } - - return 0; + return soc_get_volsw(kcontrol, ucontrol, mc, mask, mc->max - mc->min); } EXPORT_SYMBOL_GPL(snd_soc_get_volsw); @@ -361,28 +368,11 @@ EXPORT_SYMBOL_GPL(snd_soc_put_volsw); int snd_soc_get_volsw_sx(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { - struct snd_soc_component *component = snd_kcontrol_chip(kcontrol); struct soc_mixer_control *mc = (struct soc_mixer_control *)kcontrol->private_value; - unsigned int reg = mc->reg; - unsigned int reg2 = mc->rreg; unsigned int mask = soc_mixer_sx_mask(mc); - unsigned int reg_val; - int val; - reg_val = snd_soc_component_read(component, reg); - val = soc_mixer_reg_to_ctl(mc, reg_val, mask, mc->shift, mc->max); - - ucontrol->value.integer.value[0] = val; - - if (snd_soc_volsw_is_stereo(mc)) { - reg_val = snd_soc_component_read(component, reg2); - val = soc_mixer_reg_to_ctl(mc, reg_val, mask, mc->rshift, mc->max); - - ucontrol->value.integer.value[1] = val; - } - - return 0; + return soc_get_volsw(kcontrol, ucontrol, mc, mask, mc->max); } EXPORT_SYMBOL_GPL(snd_soc_get_volsw_sx); From 94dfe71f0a4eb0d7df542560c22961dedf45141d Mon Sep 17 00:00:00 2001 From: Charles Keepax Date: Wed, 19 Mar 2025 17:51:22 +0000 Subject: [PATCH 14/15] ASoC: ops: Remove some unnecessary local variables Remove some local variables that aren't adding much in terms of clarity or space saving. Signed-off-by: Charles Keepax Link: https://patch.msgid.link/20250319175123.3835849-3-ckeepax@opensource.cirrus.com Signed-off-by: Mark Brown --- sound/soc/soc-ops.c | 42 +++++++++++++++++------------------------- 1 file changed, 17 insertions(+), 25 deletions(-) diff --git a/sound/soc/soc-ops.c b/sound/soc/soc-ops.c index 1d7c28993a63..3ac5b3a62c81 100644 --- a/sound/soc/soc-ops.c +++ b/sound/soc/soc-ops.c @@ -664,9 +664,6 @@ int snd_soc_get_xr_sx(struct snd_kcontrol *kcontrol, unsigned int regwshift = component->val_bytes * BITS_PER_BYTE; unsigned int regwmask = GENMASK(regwshift - 1, 0); unsigned long mask = GENMASK(mc->nbits - 1, 0); - unsigned int invert = mc->invert; - long min = mc->min; - long max = mc->max; long val = 0; unsigned int i; @@ -676,10 +673,10 @@ int snd_soc_get_xr_sx(struct snd_kcontrol *kcontrol, val |= (regval & regwmask) << (regwshift * (regcount - i - 1)); } val &= mask; - if (min < 0 && val > max) + if (mc->min < 0 && val > mc->max) val |= ~mask; - if (invert) - val = max - val; + if (mc->invert) + val = mc->max - val; ucontrol->value.integer.value[0] = val; return 0; @@ -711,16 +708,14 @@ int snd_soc_put_xr_sx(struct snd_kcontrol *kcontrol, unsigned int regwshift = component->val_bytes * BITS_PER_BYTE; unsigned int regwmask = GENMASK(regwshift - 1, 0); unsigned long mask = GENMASK(mc->nbits - 1, 0); - unsigned int invert = mc->invert; - long max = mc->max; long val = ucontrol->value.integer.value[0]; int ret = 0; unsigned int i; if (val < mc->min || val > mc->max) return -EINVAL; - if (invert) - val = max - val; + if (mc->invert) + val = mc->max - val; val &= mask; for (i = 0; i < regcount; i++) { unsigned int regval = (val >> (regwshift * (regcount - i - 1))) & @@ -755,17 +750,16 @@ int snd_soc_get_strobe(struct snd_kcontrol *kcontrol, struct snd_soc_component *component = snd_kcontrol_chip(kcontrol); struct soc_mixer_control *mc = (struct soc_mixer_control *)kcontrol->private_value; - unsigned int reg = mc->reg; - unsigned int shift = mc->shift; - unsigned int mask = BIT(shift); unsigned int invert = mc->invert != 0; + unsigned int mask = BIT(mc->shift); unsigned int val; - val = snd_soc_component_read(component, reg); + val = snd_soc_component_read(component, mc->reg); val &= mask; - if (shift != 0 && val != 0) - val = val >> shift; + if (mc->shift != 0 && val != 0) + val = val >> mc->shift; + ucontrol->value.enumerated.item[0] = val ^ invert; return 0; @@ -788,19 +782,17 @@ int snd_soc_put_strobe(struct snd_kcontrol *kcontrol, struct snd_soc_component *component = snd_kcontrol_chip(kcontrol); struct soc_mixer_control *mc = (struct soc_mixer_control *)kcontrol->private_value; - unsigned int reg = mc->reg; - unsigned int shift = mc->shift; - unsigned int mask = BIT(shift); - unsigned int invert = mc->invert != 0; unsigned int strobe = ucontrol->value.enumerated.item[0] != 0; + unsigned int invert = mc->invert != 0; + unsigned int mask = BIT(mc->shift); unsigned int val1 = (strobe ^ invert) ? mask : 0; unsigned int val2 = (strobe ^ invert) ? 0 : mask; - int err; + int ret; - err = snd_soc_component_update_bits(component, reg, mask, val1); - if (err < 0) - return err; + ret = snd_soc_component_update_bits(component, mc->reg, mask, val1); + if (ret < 0) + return ret; - return snd_soc_component_update_bits(component, reg, mask, val2); + return snd_soc_component_update_bits(component, mc->reg, mask, val2); } EXPORT_SYMBOL_GPL(snd_soc_put_strobe); From 502a668fad12b6ca10bcbb615d62e61d3b669c99 Mon Sep 17 00:00:00 2001 From: Charles Keepax Date: Wed, 19 Mar 2025 17:51:23 +0000 Subject: [PATCH 15/15] ASoC: ops: Apply platform_max after deciding control type It doesn't really make sense for the type of a control to change based on the platform_max field. platform_max allows a specific system to limit values of a control for safety but it seems reasonable the control type should remain the same between different systems, even if it is restricted down to just two values. Move the application of platform_max to after control type determination in soc_info_volsw(). Signed-off-by: Charles Keepax Link: https://patch.msgid.link/20250319175123.3835849-4-ckeepax@opensource.cirrus.com Signed-off-by: Mark Brown --- sound/soc/soc-ops.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sound/soc/soc-ops.c b/sound/soc/soc-ops.c index 3ac5b3a62c81..8d4dd11c9aef 100644 --- a/sound/soc/soc-ops.c +++ b/sound/soc/soc-ops.c @@ -172,9 +172,6 @@ static int soc_info_volsw(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_info *uinfo, struct soc_mixer_control *mc, int max) { - if (mc->platform_max && mc->platform_max < max) - max = mc->platform_max; - uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER; if (max == 1) { @@ -185,6 +182,9 @@ static int soc_info_volsw(struct snd_kcontrol *kcontrol, uinfo->type = SNDRV_CTL_ELEM_TYPE_BOOLEAN; } + if (mc->platform_max && mc->platform_max < max) + max = mc->platform_max; + uinfo->count = snd_soc_volsw_is_stereo(mc) ? 2 : 1; uinfo->value.integer.min = 0; uinfo->value.integer.max = max;