* [PATCH 0/2] ALSA: hda: cs35l56: Handle speaker id GPIOs
@ 2023-09-18 9:51 Richard Fitzgerald
2023-09-18 9:51 ` [PATCH 1/2] ALSA: hda: cs35l56: Add support for speaker id Richard Fitzgerald
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Richard Fitzgerald @ 2023-09-18 9:51 UTC (permalink / raw)
To: tiwai; +Cc: alsa-devel, patches, linux-kernel, Richard Fitzgerald
Some manufacturers use multiple sources of speakers. Motherboard
GPIOs are set to indicate which type of speaker is fitted so that
the correct tunings can be loaded. Patch #1 adds support for this
and patch #2 adds a KUnit test for the new code.
Richard Fitzgerald (2):
ALSA: hda: cs35l56: Add support for speaker id
ALSA: hda: cirrus_scodec: Add KUnit test
MAINTAINERS | 1 +
sound/pci/hda/Kconfig | 17 ++
sound/pci/hda/Makefile | 4 +
sound/pci/hda/cirrus_scodec.c | 73 ++++++
sound/pci/hda/cirrus_scodec.h | 13 +
sound/pci/hda/cirrus_scodec_test.c | 370 +++++++++++++++++++++++++++++
sound/pci/hda/cs35l56_hda.c | 24 +-
7 files changed, 501 insertions(+), 1 deletion(-)
create mode 100644 sound/pci/hda/cirrus_scodec.c
create mode 100644 sound/pci/hda/cirrus_scodec.h
create mode 100644 sound/pci/hda/cirrus_scodec_test.c
--
2.30.2
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/2] ALSA: hda: cs35l56: Add support for speaker id 2023-09-18 9:51 [PATCH 0/2] ALSA: hda: cs35l56: Handle speaker id GPIOs Richard Fitzgerald @ 2023-09-18 9:51 ` Richard Fitzgerald 2023-09-18 9:51 ` [PATCH 2/2] ALSA: hda: cirrus_scodec: Add KUnit test Richard Fitzgerald 2023-09-18 15:50 ` [PATCH 0/2] ALSA: hda: cs35l56: Handle speaker id GPIOs Takashi Iwai 2 siblings, 0 replies; 9+ messages in thread From: Richard Fitzgerald @ 2023-09-18 9:51 UTC (permalink / raw) To: tiwai; +Cc: alsa-devel, patches, linux-kernel, Richard Fitzgerald Add handling of the "spk-id-gpios" _DSD property. If present, the value indicated by the GPIOs is appended to the subsystem-id part of the firmware name to load the appropriate tunings for that speaker. Some manufacturers use multiple sources of speakers, which need different tunings for best performance. On these models the type of speaker fitted is indicated by the values of one or more GPIOs. The number formed by the GPIOs identifies the tuning required. The speaker ID is only used in combination with a _SUB identifier because the value is only meaningful if the exact model is known. The code to get the speaker ID value has been implemented as a new library so that the cs35l41_hda driver can be switched in future to share common code. This library can be extended for other common functionality shared by Cirrus Logic amp drivers. Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com> --- MAINTAINERS | 1 + sound/pci/hda/Kconfig | 5 +++ sound/pci/hda/Makefile | 2 + sound/pci/hda/cirrus_scodec.c | 73 +++++++++++++++++++++++++++++++++++ sound/pci/hda/cirrus_scodec.h | 13 +++++++ sound/pci/hda/cs35l56_hda.c | 14 ++++++- 6 files changed, 107 insertions(+), 1 deletion(-) create mode 100644 sound/pci/hda/cirrus_scodec.c create mode 100644 sound/pci/hda/cirrus_scodec.h diff --git a/MAINTAINERS b/MAINTAINERS index 90f13281d297..23e73d19f347 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -4912,6 +4912,7 @@ F: drivers/spi/spi-cs42l43* F: include/dt-bindings/sound/cs* F: include/linux/mfd/cs42l43* F: include/sound/cs* +F: sound/pci/hda/cirrus* F: sound/pci/hda/cs* F: sound/pci/hda/hda_cs_dsp_ctl.* F: sound/soc/codecs/cs* diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig index 0d7502d6e060..2980bfef0a4c 100644 --- a/sound/pci/hda/Kconfig +++ b/sound/pci/hda/Kconfig @@ -91,6 +91,9 @@ config SND_HDA_PATCH_LOADER start up. The "patch" file can be specified via patch module option, such as patch=hda-init. +config SND_HDA_CIRRUS_SCODEC + tristate + config SND_HDA_SCODEC_CS35L41 tristate select SND_HDA_GENERIC @@ -144,6 +147,7 @@ config SND_HDA_SCODEC_CS35L56_I2C select SND_HDA_GENERIC select SND_SOC_CS35L56_SHARED select SND_HDA_SCODEC_CS35L56 + select SND_HDA_CIRRUS_SCODEC select SND_HDA_CS_DSP_CONTROLS help Say Y or M here to include CS35L56 amplifier support with @@ -158,6 +162,7 @@ config SND_HDA_SCODEC_CS35L56_SPI select SND_HDA_GENERIC select SND_SOC_CS35L56_SHARED select SND_HDA_SCODEC_CS35L56 + select SND_HDA_CIRRUS_SCODEC select SND_HDA_CS_DSP_CONTROLS help Say Y or M here to include CS35L56 amplifier support with diff --git a/sound/pci/hda/Makefile b/sound/pci/hda/Makefile index f00fc9ed6096..aa445af0cf9a 100644 --- a/sound/pci/hda/Makefile +++ b/sound/pci/hda/Makefile @@ -28,6 +28,7 @@ snd-hda-codec-via-objs := patch_via.o snd-hda-codec-hdmi-objs := patch_hdmi.o hda_eld.o # side codecs +snd-hda-cirrus-scodec-objs := cirrus_scodec.o snd-hda-scodec-cs35l41-objs := cs35l41_hda.o cs35l41_hda_property.o snd-hda-scodec-cs35l41-i2c-objs := cs35l41_hda_i2c.o snd-hda-scodec-cs35l41-spi-objs := cs35l41_hda_spi.o @@ -56,6 +57,7 @@ obj-$(CONFIG_SND_HDA_CODEC_VIA) += snd-hda-codec-via.o obj-$(CONFIG_SND_HDA_CODEC_HDMI) += snd-hda-codec-hdmi.o # side codecs +obj-$(CONFIG_SND_HDA_CIRRUS_SCODEC) += snd-hda-cirrus-scodec.o obj-$(CONFIG_SND_HDA_SCODEC_CS35L41) += snd-hda-scodec-cs35l41.o obj-$(CONFIG_SND_HDA_SCODEC_CS35L41_I2C) += snd-hda-scodec-cs35l41-i2c.o obj-$(CONFIG_SND_HDA_SCODEC_CS35L41_SPI) += snd-hda-scodec-cs35l41-spi.o diff --git a/sound/pci/hda/cirrus_scodec.c b/sound/pci/hda/cirrus_scodec.c new file mode 100644 index 000000000000..8de3bc7448fa --- /dev/null +++ b/sound/pci/hda/cirrus_scodec.c @@ -0,0 +1,73 @@ +// SPDX-License-Identifier: GPL-2.0-only +// +// Common code for Cirrus side-codecs. +// +// Copyright (C) 2021, 2023 Cirrus Logic, Inc. and +// Cirrus Logic International Semiconductor Ltd. + +#include <linux/dev_printk.h> +#include <linux/gpio/consumer.h> +#include <linux/module.h> + +#include "cirrus_scodec.h" + +int cirrus_scodec_get_speaker_id(struct device *dev, int amp_index, + int num_amps, int fixed_gpio_id) +{ + struct gpio_desc *speaker_id_desc; + int speaker_id = -ENOENT; + + if (fixed_gpio_id >= 0) { + dev_dbg(dev, "Found Fixed Speaker ID GPIO (index = %d)\n", fixed_gpio_id); + speaker_id_desc = gpiod_get_index(dev, NULL, fixed_gpio_id, GPIOD_IN); + if (IS_ERR(speaker_id_desc)) { + speaker_id = PTR_ERR(speaker_id_desc); + return speaker_id; + } + speaker_id = gpiod_get_value_cansleep(speaker_id_desc); + gpiod_put(speaker_id_desc); + } else { + int base_index; + int gpios_per_amp; + int count; + int tmp; + int i; + + count = gpiod_count(dev, "spk-id"); + if (count > 0) { + speaker_id = 0; + gpios_per_amp = count / num_amps; + base_index = gpios_per_amp * amp_index; + + if (count % num_amps) + return -EINVAL; + + dev_dbg(dev, "Found %d Speaker ID GPIOs per Amp\n", gpios_per_amp); + + for (i = 0; i < gpios_per_amp; i++) { + speaker_id_desc = gpiod_get_index(dev, "spk-id", i + base_index, + GPIOD_IN); + if (IS_ERR(speaker_id_desc)) { + speaker_id = PTR_ERR(speaker_id_desc); + break; + } + tmp = gpiod_get_value_cansleep(speaker_id_desc); + gpiod_put(speaker_id_desc); + if (tmp < 0) { + speaker_id = tmp; + break; + } + speaker_id |= tmp << i; + } + } + } + + dev_dbg(dev, "Speaker ID = %d\n", speaker_id); + + return speaker_id; +} +EXPORT_SYMBOL_NS_GPL(cirrus_scodec_get_speaker_id, SND_HDA_CIRRUS_SCODEC); + +MODULE_DESCRIPTION("HDA Cirrus side-codec library"); +MODULE_AUTHOR("Richard Fitzgerald <rf@opensource.cirrus.com>"); +MODULE_LICENSE("GPL"); diff --git a/sound/pci/hda/cirrus_scodec.h b/sound/pci/hda/cirrus_scodec.h new file mode 100644 index 000000000000..ba2041d8ef24 --- /dev/null +++ b/sound/pci/hda/cirrus_scodec.h @@ -0,0 +1,13 @@ +/* SPDX-License-Identifier: GPL-2.0 + * + * Copyright (C) 2023 Cirrus Logic, Inc. and + * Cirrus Logic International Semiconductor Ltd. + */ + +#ifndef CIRRUS_SCODEC_H +#define CIRRUS_SCODEC_H + +int cirrus_scodec_get_speaker_id(struct device *dev, int amp_index, + int num_amps, int fixed_gpio_id); + +#endif /* CIRRUS_SCODEC_H */ diff --git a/sound/pci/hda/cs35l56_hda.c b/sound/pci/hda/cs35l56_hda.c index 87ffe8fbff99..44f5ca0e73e3 100644 --- a/sound/pci/hda/cs35l56_hda.c +++ b/sound/pci/hda/cs35l56_hda.c @@ -16,6 +16,7 @@ #include <sound/core.h> #include <sound/hda_codec.h> #include <sound/tlv.h> +#include "cirrus_scodec.h" #include "cs35l56_hda.h" #include "hda_component.h" #include "hda_cs_dsp_ctl.h" @@ -869,7 +870,17 @@ static int cs35l56_hda_read_acpi(struct cs35l56_hda *cs35l56, int id) "Read ACPI _SUB failed(%ld): fallback to generic firmware\n", PTR_ERR(sub)); } else { - cs35l56->system_name = sub; + ret = cirrus_scodec_get_speaker_id(cs35l56->base.dev, cs35l56->index, nval, -1); + if (ret == -ENOENT) { + cs35l56->system_name = sub; + } else if (ret >= 0) { + cs35l56->system_name = kasprintf(GFP_KERNEL, "%s-spkid%d", sub, ret); + kfree(sub); + if (!cs35l56->system_name) + return -ENOMEM; + } else { + return ret; + } } cs35l56->base.reset_gpio = devm_gpiod_get_index_optional(cs35l56->base.dev, @@ -1025,6 +1036,7 @@ const struct dev_pm_ops cs35l56_hda_pm_ops = { EXPORT_SYMBOL_NS_GPL(cs35l56_hda_pm_ops, SND_HDA_SCODEC_CS35L56); MODULE_DESCRIPTION("CS35L56 HDA Driver"); +MODULE_IMPORT_NS(SND_HDA_CIRRUS_SCODEC); MODULE_IMPORT_NS(SND_HDA_CS_DSP_CONTROLS); MODULE_IMPORT_NS(SND_SOC_CS35L56_SHARED); MODULE_AUTHOR("Richard Fitzgerald <rf@opensource.cirrus.com>"); -- 2.30.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] ALSA: hda: cirrus_scodec: Add KUnit test 2023-09-18 9:51 [PATCH 0/2] ALSA: hda: cs35l56: Handle speaker id GPIOs Richard Fitzgerald 2023-09-18 9:51 ` [PATCH 1/2] ALSA: hda: cs35l56: Add support for speaker id Richard Fitzgerald @ 2023-09-18 9:51 ` Richard Fitzgerald 2023-09-19 20:44 ` Nick Desaulniers 2023-09-18 15:50 ` [PATCH 0/2] ALSA: hda: cs35l56: Handle speaker id GPIOs Takashi Iwai 2 siblings, 1 reply; 9+ messages in thread From: Richard Fitzgerald @ 2023-09-18 9:51 UTC (permalink / raw) To: tiwai; +Cc: alsa-devel, patches, linux-kernel, Richard Fitzgerald Add a KUnit test for cirrus_scodec_get_speaker_id(). It is impractical to have enough hardware with every possible permutation of speaker id. So use a test harness to test all theoretically supported options. The test harness consists of: - a mock GPIO controller. - a mock struct device to represent the scodec driver - software nodes to provide the fwnode info that would normally come from ACPI. Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com> --- sound/pci/hda/Kconfig | 12 + sound/pci/hda/Makefile | 2 + sound/pci/hda/cirrus_scodec_test.c | 370 +++++++++++++++++++++++++++++ sound/pci/hda/cs35l56_hda.c | 10 + 4 files changed, 394 insertions(+) create mode 100644 sound/pci/hda/cirrus_scodec_test.c diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig index 2980bfef0a4c..706cdc589e6f 100644 --- a/sound/pci/hda/Kconfig +++ b/sound/pci/hda/Kconfig @@ -94,6 +94,18 @@ config SND_HDA_PATCH_LOADER config SND_HDA_CIRRUS_SCODEC tristate +config SND_HDA_CIRRUS_SCODEC_KUNIT_TEST + tristate "KUnit test for Cirrus side-codec library" if !KUNIT_ALL_TESTS + select SND_HDA_CIRRUS_SCODEC + depends on KUNIT + default KUNIT_ALL_TESTS + help + This builds KUnit tests for the cirrus side-codec library. + For more information on KUnit and unit tests in general, + please refer to the KUnit documentation in + Documentation/dev-tools/kunit/. + If in doubt, say "N". + config SND_HDA_SCODEC_CS35L41 tristate select SND_HDA_GENERIC diff --git a/sound/pci/hda/Makefile b/sound/pci/hda/Makefile index aa445af0cf9a..793e296c3f64 100644 --- a/sound/pci/hda/Makefile +++ b/sound/pci/hda/Makefile @@ -29,6 +29,7 @@ snd-hda-codec-hdmi-objs := patch_hdmi.o hda_eld.o # side codecs snd-hda-cirrus-scodec-objs := cirrus_scodec.o +snd-hda-cirrus-scodec-test-objs := cirrus_scodec_test.o snd-hda-scodec-cs35l41-objs := cs35l41_hda.o cs35l41_hda_property.o snd-hda-scodec-cs35l41-i2c-objs := cs35l41_hda_i2c.o snd-hda-scodec-cs35l41-spi-objs := cs35l41_hda_spi.o @@ -58,6 +59,7 @@ obj-$(CONFIG_SND_HDA_CODEC_HDMI) += snd-hda-codec-hdmi.o # side codecs obj-$(CONFIG_SND_HDA_CIRRUS_SCODEC) += snd-hda-cirrus-scodec.o +obj-$(CONFIG_SND_HDA_CIRRUS_SCODEC_KUNIT_TEST) += snd-hda-cirrus-scodec-test.o obj-$(CONFIG_SND_HDA_SCODEC_CS35L41) += snd-hda-scodec-cs35l41.o obj-$(CONFIG_SND_HDA_SCODEC_CS35L41_I2C) += snd-hda-scodec-cs35l41-i2c.o obj-$(CONFIG_SND_HDA_SCODEC_CS35L41_SPI) += snd-hda-scodec-cs35l41-spi.o diff --git a/sound/pci/hda/cirrus_scodec_test.c b/sound/pci/hda/cirrus_scodec_test.c new file mode 100644 index 000000000000..5eb590cd4fe2 --- /dev/null +++ b/sound/pci/hda/cirrus_scodec_test.c @@ -0,0 +1,370 @@ +// SPDX-License-Identifier: GPL-2.0-only +// +// KUnit test for the Cirrus side-codec library. +// +// Copyright (C) 2023 Cirrus Logic, Inc. and +// Cirrus Logic International Semiconductor Ltd. + +#include <kunit/test.h> +#include <linux/gpio/driver.h> +#include <linux/module.h> +#include <linux/platform_device.h> + +#include "cirrus_scodec.h" + +struct cirrus_scodec_test_gpio { + unsigned int pin_state; + struct gpio_chip chip; +}; + +struct cirrus_scodec_test_priv { + struct platform_device amp_pdev; + struct platform_device *gpio_pdev; + struct cirrus_scodec_test_gpio *gpio_priv; +}; + +static int cirrus_scodec_test_gpio_get_direction(struct gpio_chip *chip, + unsigned int offset) +{ + return GPIO_LINE_DIRECTION_IN; +} + +static int cirrus_scodec_test_gpio_direction_in(struct gpio_chip *chip, + unsigned int offset) +{ + return 0; +} + +static int cirrus_scodec_test_gpio_get(struct gpio_chip *chip, unsigned int offset) +{ + struct cirrus_scodec_test_gpio *gpio_priv = gpiochip_get_data(chip); + + return !!(gpio_priv->pin_state & BIT(offset)); +} + +static int cirrus_scodec_test_gpio_direction_out(struct gpio_chip *chip, + unsigned int offset, int value) +{ + return -EOPNOTSUPP; +} + +static void cirrus_scodec_test_gpio_set(struct gpio_chip *chip, unsigned int offset, + int value) +{ +} + +static int cirrus_scodec_test_gpio_set_config(struct gpio_chip *gc, + unsigned int offset, + unsigned long config) +{ + switch (pinconf_to_config_param(config)) { + case PIN_CONFIG_OUTPUT: + case PIN_CONFIG_OUTPUT_ENABLE: + return -EOPNOTSUPP; + default: + return 0; + } +} + +static const struct gpio_chip cirrus_scodec_test_gpio_chip = { + .label = "cirrus_scodec_test_gpio", + .owner = THIS_MODULE, + .request = gpiochip_generic_request, + .free = gpiochip_generic_free, + .get_direction = cirrus_scodec_test_gpio_get_direction, + .direction_input = cirrus_scodec_test_gpio_direction_in, + .get = cirrus_scodec_test_gpio_get, + .direction_output = cirrus_scodec_test_gpio_direction_out, + .set = cirrus_scodec_test_gpio_set, + .set_config = cirrus_scodec_test_gpio_set_config, + .base = -1, + .ngpio = 32, +}; + +static int cirrus_scodec_test_gpio_probe(struct platform_device *pdev) +{ + struct cirrus_scodec_test_gpio *gpio_priv; + int ret; + + gpio_priv = devm_kzalloc(&pdev->dev, sizeof(*gpio_priv), GFP_KERNEL); + if (!gpio_priv) + return -ENOMEM; + + /* GPIO core modifies our struct gpio_chip so use a copy */ + gpio_priv->chip = cirrus_scodec_test_gpio_chip; + ret = devm_gpiochip_add_data(&pdev->dev, &gpio_priv->chip, gpio_priv); + if (ret) + return dev_err_probe(&pdev->dev, ret, "Failed to add gpiochip\n"); + + dev_set_drvdata(&pdev->dev, gpio_priv); + + return 0; +} + +static struct platform_driver cirrus_scodec_test_gpio_driver = { + .driver.name = "cirrus_scodec_test_gpio_drv", + .probe = cirrus_scodec_test_gpio_probe, +}; + +/* software_node referencing the gpio driver */ +static const struct software_node cirrus_scodec_test_gpio_swnode = { + .name = "cirrus_scodec_test_gpio", +}; + +static int cirrus_scodec_test_create_gpio(struct kunit *test) +{ + struct cirrus_scodec_test_priv *priv = test->priv; + int ret; + + priv->gpio_pdev = platform_device_alloc(cirrus_scodec_test_gpio_driver.driver.name, -1); + if (!priv->gpio_pdev) + return -ENOMEM; + + ret = device_add_software_node(&priv->gpio_pdev->dev, &cirrus_scodec_test_gpio_swnode); + if (ret) { + platform_device_put(priv->gpio_pdev); + KUNIT_FAIL(test, "Failed to add swnode to gpio: %d\n", ret); + return ret; + } + + ret = platform_device_add(priv->gpio_pdev); + if (ret) { + platform_device_put(priv->gpio_pdev); + KUNIT_FAIL(test, "Failed to add gpio platform device: %d\n", ret); + return ret; + } + + priv->gpio_priv = dev_get_drvdata(&priv->gpio_pdev->dev); + if (!priv->gpio_priv) { + platform_device_put(priv->gpio_pdev); + KUNIT_FAIL(test, "Failed to get gpio private data: %d\n", ret); + return ret; + } + + return 0; +} + +static void cirrus_scodec_test_set_gpio_ref_arg(struct software_node_ref_args *arg, + int gpio_num) +{ + struct software_node_ref_args template = + SOFTWARE_NODE_REFERENCE(&cirrus_scodec_test_gpio_swnode, gpio_num, 0); + + *arg = template; +} + +static int cirrus_scodec_test_set_spkid_swnode(struct kunit *test, + struct device *dev, + struct software_node_ref_args *args, + int num_args) +{ + const struct property_entry props_template[] = { + PROPERTY_ENTRY_REF_ARRAY_LEN("spk-id-gpios", args, num_args), + { } + }; + struct property_entry *props; + struct software_node *node; + + node = kunit_kzalloc(test, sizeof(*node), GFP_KERNEL); + if (!node) + return -ENOMEM; + + props = kunit_kzalloc(test, sizeof(props_template), GFP_KERNEL); + if (!props) + return -ENOMEM; + + memcpy(props, props_template, sizeof(props_template)); + node->properties = props; + + return device_add_software_node(dev, node); +} + +struct cirrus_scodec_test_spkid_param { + int num_amps; + int gpios_per_amp; + int num_amps_sharing; +}; + +static void cirrus_scodec_test_spkid_parse(struct kunit *test) +{ + struct cirrus_scodec_test_priv *priv = test->priv; + const struct cirrus_scodec_test_spkid_param *param = test->param_value; + int num_spk_id_refs = param->num_amps * param->gpios_per_amp; + struct software_node_ref_args *refs; + struct device *dev = &priv->amp_pdev.dev; + unsigned int v; + int i, ret; + + refs = kunit_kcalloc(test, num_spk_id_refs, sizeof(*refs), GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, refs); + + for (i = 0, v = 0; i < num_spk_id_refs; ) { + cirrus_scodec_test_set_gpio_ref_arg(&refs[i++], v++); + + /* + * If amps are sharing GPIOs repeat the last set of + * GPIOs until we've done that number of amps. + * We have done all GPIOs for an amp when i is a multiple + * of gpios_per_amp. + * We have done all amps sharing the same GPIOs when i is + * a multiple of (gpios_per_amp * num_amps_sharing). + */ + if (!(i % param->gpios_per_amp) && + (i % (param->gpios_per_amp * param->num_amps_sharing))) + v -= param->gpios_per_amp; + } + + ret = cirrus_scodec_test_set_spkid_swnode(test, dev, refs, num_spk_id_refs); + KUNIT_EXPECT_EQ_MSG(test, ret, 0, "Failed to add swnode\n"); + + for (i = 0; i < param->num_amps; ++i) { + for (v = 0; v < (1 << param->gpios_per_amp); ++v) { + /* Set only the GPIO bits used by this amp */ + priv->gpio_priv->pin_state = + v << (param->gpios_per_amp * (i / param->num_amps_sharing)); + + ret = cirrus_scodec_get_speaker_id(dev, i, param->num_amps, -1); + KUNIT_EXPECT_EQ_MSG(test, ret, v, + "get_speaker_id failed amp:%d pin_state:%#x\n", + i, priv->gpio_priv->pin_state); + } + } +} + +static void cirrus_scodec_test_no_spkid(struct kunit *test) +{ + struct cirrus_scodec_test_priv *priv = test->priv; + struct device *dev = &priv->amp_pdev.dev; + int ret; + + ret = cirrus_scodec_get_speaker_id(dev, 0, 4, -1); + KUNIT_EXPECT_EQ(test, ret, -ENOENT); +} + +static void cirrus_scodec_test_dev_release(struct device *dev) +{ +} + +static int cirrus_scodec_test_case_init(struct kunit *test) +{ + struct cirrus_scodec_test_priv *priv; + int ret; + + priv = kunit_kzalloc(test, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + test->priv = priv; + + /* Create dummy GPIO */ + ret = cirrus_scodec_test_create_gpio(test); + if (ret < 0) + return ret; + + /* Create dummy amp driver dev */ + priv->amp_pdev.name = "cirrus_scodec_test_amp_drv"; + priv->amp_pdev.id = -1; + priv->amp_pdev.dev.release = cirrus_scodec_test_dev_release; + ret = platform_device_register(&priv->amp_pdev); + KUNIT_ASSERT_GE_MSG(test, ret, 0, "Failed to register amp platform device\n"); + + return 0; +} + +static void cirrus_scodec_test_case_exit(struct kunit *test) +{ + struct cirrus_scodec_test_priv *priv = test->priv; + + if (priv->amp_pdev.name) + platform_device_unregister(&priv->amp_pdev); + + if (priv->gpio_pdev) { + device_remove_software_node(&priv->gpio_pdev->dev); + platform_device_unregister(priv->gpio_pdev); + } +} + +static int cirrus_scodec_test_suite_init(struct kunit_suite *suite) +{ + int ret; + + /* Register mock GPIO driver */ + ret = platform_driver_register(&cirrus_scodec_test_gpio_driver); + if (ret < 0) { + kunit_err(suite, "Failed to register gpio platform driver, %d\n", ret); + return ret; + } + + return 0; +} + +static void cirrus_scodec_test_suite_exit(struct kunit_suite *suite) +{ + platform_driver_unregister(&cirrus_scodec_test_gpio_driver); +} + +static const struct cirrus_scodec_test_spkid_param cirrus_scodec_test_spkid_param_cases[] = { + { .num_amps = 2, .gpios_per_amp = 1, .num_amps_sharing = 1 }, + { .num_amps = 2, .gpios_per_amp = 2, .num_amps_sharing = 1 }, + { .num_amps = 2, .gpios_per_amp = 3, .num_amps_sharing = 1 }, + { .num_amps = 2, .gpios_per_amp = 4, .num_amps_sharing = 1 }, + { .num_amps = 3, .gpios_per_amp = 1, .num_amps_sharing = 1 }, + { .num_amps = 3, .gpios_per_amp = 2, .num_amps_sharing = 1 }, + { .num_amps = 3, .gpios_per_amp = 3, .num_amps_sharing = 1 }, + { .num_amps = 3, .gpios_per_amp = 4, .num_amps_sharing = 1 }, + { .num_amps = 4, .gpios_per_amp = 1, .num_amps_sharing = 1 }, + { .num_amps = 4, .gpios_per_amp = 2, .num_amps_sharing = 1 }, + { .num_amps = 4, .gpios_per_amp = 3, .num_amps_sharing = 1 }, + { .num_amps = 4, .gpios_per_amp = 4, .num_amps_sharing = 1 }, + + /* Same GPIO shared by all amps */ + { .num_amps = 2, .gpios_per_amp = 1, .num_amps_sharing = 2 }, + { .num_amps = 2, .gpios_per_amp = 2, .num_amps_sharing = 2 }, + { .num_amps = 2, .gpios_per_amp = 3, .num_amps_sharing = 2 }, + { .num_amps = 2, .gpios_per_amp = 4, .num_amps_sharing = 2 }, + { .num_amps = 3, .gpios_per_amp = 1, .num_amps_sharing = 3 }, + { .num_amps = 3, .gpios_per_amp = 2, .num_amps_sharing = 3 }, + { .num_amps = 3, .gpios_per_amp = 3, .num_amps_sharing = 3 }, + { .num_amps = 3, .gpios_per_amp = 4, .num_amps_sharing = 3 }, + { .num_amps = 4, .gpios_per_amp = 1, .num_amps_sharing = 4 }, + { .num_amps = 4, .gpios_per_amp = 2, .num_amps_sharing = 4 }, + { .num_amps = 4, .gpios_per_amp = 3, .num_amps_sharing = 4 }, + { .num_amps = 4, .gpios_per_amp = 4, .num_amps_sharing = 4 }, + + /* Two sets of shared GPIOs */ + { .num_amps = 4, .gpios_per_amp = 1, .num_amps_sharing = 2 }, + { .num_amps = 4, .gpios_per_amp = 2, .num_amps_sharing = 2 }, + { .num_amps = 4, .gpios_per_amp = 3, .num_amps_sharing = 2 }, + { .num_amps = 4, .gpios_per_amp = 4, .num_amps_sharing = 2 }, +}; + +static void cirrus_scodec_test_spkid_param_desc(const struct cirrus_scodec_test_spkid_param *param, + char *desc) +{ + snprintf(desc, KUNIT_PARAM_DESC_SIZE, "amps:%d gpios_per_amp:%d num_amps_sharing:%d", + param->num_amps, param->gpios_per_amp, param->num_amps_sharing); +} + +KUNIT_ARRAY_PARAM(cirrus_scodec_test_spkid, cirrus_scodec_test_spkid_param_cases, + cirrus_scodec_test_spkid_param_desc); + +static struct kunit_case cirrus_scodec_test_cases[] = { + KUNIT_CASE_PARAM(cirrus_scodec_test_spkid_parse, cirrus_scodec_test_spkid_gen_params), + KUNIT_CASE(cirrus_scodec_test_no_spkid), + { } /* terminator */ +}; + +static struct kunit_suite cirrus_scodec_test_suite = { + .name = "snd-hda-scodec-cs35l56-test", + .suite_init = cirrus_scodec_test_suite_init, + .suite_exit = cirrus_scodec_test_suite_exit, + .init = cirrus_scodec_test_case_init, + .exit = cirrus_scodec_test_case_exit, + .test_cases = cirrus_scodec_test_cases, +}; + +kunit_test_suite(cirrus_scodec_test_suite); + +MODULE_IMPORT_NS(SND_HDA_CIRRUS_SCODEC); +MODULE_AUTHOR("Richard Fitzgerald <rf@opensource.cirrus.com>"); +MODULE_LICENSE("GPL"); diff --git a/sound/pci/hda/cs35l56_hda.c b/sound/pci/hda/cs35l56_hda.c index 44f5ca0e73e3..d3cfdad7dd76 100644 --- a/sound/pci/hda/cs35l56_hda.c +++ b/sound/pci/hda/cs35l56_hda.c @@ -1035,6 +1035,16 @@ const struct dev_pm_ops cs35l56_hda_pm_ops = { }; EXPORT_SYMBOL_NS_GPL(cs35l56_hda_pm_ops, SND_HDA_SCODEC_CS35L56); +#if IS_ENABLED(CONFIG_SND_HDA_SCODEC_CS35L56_KUNIT_TEST) +/* Hooks to export static function to KUnit test */ + +int cs35l56_hda_test_hook_get_speaker_id(struct device *dev, int amp_index, int num_amps) +{ + return cs35l56_hda_get_speaker_id(dev, amp_index, num_amps); +} +EXPORT_SYMBOL_NS_GPL(cs35l56_hda_test_hook_get_speaker_id, SND_HDA_SCODEC_CS35L56); +#endif + MODULE_DESCRIPTION("CS35L56 HDA Driver"); MODULE_IMPORT_NS(SND_HDA_CIRRUS_SCODEC); MODULE_IMPORT_NS(SND_HDA_CS_DSP_CONTROLS); -- 2.30.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] ALSA: hda: cirrus_scodec: Add KUnit test 2023-09-18 9:51 ` [PATCH 2/2] ALSA: hda: cirrus_scodec: Add KUnit test Richard Fitzgerald @ 2023-09-19 20:44 ` Nick Desaulniers 2023-09-20 6:51 ` Takashi Iwai 0 siblings, 1 reply; 9+ messages in thread From: Nick Desaulniers @ 2023-09-19 20:44 UTC (permalink / raw) To: Richard Fitzgerald; +Cc: tiwai, alsa-devel, patches, linux-kernel, llvm On Mon, Sep 18, 2023 at 10:51:29AM +0100, Richard Fitzgerald wrote: > Add a KUnit test for cirrus_scodec_get_speaker_id(). It is impractical > to have enough hardware with every possible permutation of speaker id. > So use a test harness to test all theoretically supported options. > > The test harness consists of: > - a mock GPIO controller. > - a mock struct device to represent the scodec driver > - software nodes to provide the fwnode info that would normally come > from ACPI. > > Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com> > --- > sound/pci/hda/Kconfig | 12 + > sound/pci/hda/Makefile | 2 + > sound/pci/hda/cirrus_scodec_test.c | 370 +++++++++++++++++++++++++++++ > sound/pci/hda/cs35l56_hda.c | 10 + > 4 files changed, 394 insertions(+) > create mode 100644 sound/pci/hda/cirrus_scodec_test.c > > diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig > index 2980bfef0a4c..706cdc589e6f 100644 > --- a/sound/pci/hda/Kconfig > +++ b/sound/pci/hda/Kconfig > @@ -94,6 +94,18 @@ config SND_HDA_PATCH_LOADER > config SND_HDA_CIRRUS_SCODEC > tristate > > +config SND_HDA_CIRRUS_SCODEC_KUNIT_TEST > + tristate "KUnit test for Cirrus side-codec library" if !KUNIT_ALL_TESTS > + select SND_HDA_CIRRUS_SCODEC > + depends on KUNIT > + default KUNIT_ALL_TESTS > + help > + This builds KUnit tests for the cirrus side-codec library. > + For more information on KUnit and unit tests in general, > + please refer to the KUnit documentation in > + Documentation/dev-tools/kunit/. > + If in doubt, say "N". > + > config SND_HDA_SCODEC_CS35L41 > tristate > select SND_HDA_GENERIC > diff --git a/sound/pci/hda/Makefile b/sound/pci/hda/Makefile > index aa445af0cf9a..793e296c3f64 100644 > --- a/sound/pci/hda/Makefile > +++ b/sound/pci/hda/Makefile > @@ -29,6 +29,7 @@ snd-hda-codec-hdmi-objs := patch_hdmi.o hda_eld.o > > # side codecs > snd-hda-cirrus-scodec-objs := cirrus_scodec.o > +snd-hda-cirrus-scodec-test-objs := cirrus_scodec_test.o > snd-hda-scodec-cs35l41-objs := cs35l41_hda.o cs35l41_hda_property.o > snd-hda-scodec-cs35l41-i2c-objs := cs35l41_hda_i2c.o > snd-hda-scodec-cs35l41-spi-objs := cs35l41_hda_spi.o > @@ -58,6 +59,7 @@ obj-$(CONFIG_SND_HDA_CODEC_HDMI) += snd-hda-codec-hdmi.o > > # side codecs > obj-$(CONFIG_SND_HDA_CIRRUS_SCODEC) += snd-hda-cirrus-scodec.o > +obj-$(CONFIG_SND_HDA_CIRRUS_SCODEC_KUNIT_TEST) += snd-hda-cirrus-scodec-test.o > obj-$(CONFIG_SND_HDA_SCODEC_CS35L41) += snd-hda-scodec-cs35l41.o > obj-$(CONFIG_SND_HDA_SCODEC_CS35L41_I2C) += snd-hda-scodec-cs35l41-i2c.o > obj-$(CONFIG_SND_HDA_SCODEC_CS35L41_SPI) += snd-hda-scodec-cs35l41-spi.o > diff --git a/sound/pci/hda/cirrus_scodec_test.c b/sound/pci/hda/cirrus_scodec_test.c > new file mode 100644 > index 000000000000..5eb590cd4fe2 > --- /dev/null > +++ b/sound/pci/hda/cirrus_scodec_test.c > @@ -0,0 +1,370 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +// > +// KUnit test for the Cirrus side-codec library. > +// > +// Copyright (C) 2023 Cirrus Logic, Inc. and > +// Cirrus Logic International Semiconductor Ltd. > + > +#include <kunit/test.h> > +#include <linux/gpio/driver.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> > + > +#include "cirrus_scodec.h" > + > +struct cirrus_scodec_test_gpio { > + unsigned int pin_state; > + struct gpio_chip chip; > +}; > + > +struct cirrus_scodec_test_priv { > + struct platform_device amp_pdev; > + struct platform_device *gpio_pdev; > + struct cirrus_scodec_test_gpio *gpio_priv; > +}; > + > +static int cirrus_scodec_test_gpio_get_direction(struct gpio_chip *chip, > + unsigned int offset) > +{ > + return GPIO_LINE_DIRECTION_IN; > +} > + > +static int cirrus_scodec_test_gpio_direction_in(struct gpio_chip *chip, > + unsigned int offset) > +{ > + return 0; > +} > + > +static int cirrus_scodec_test_gpio_get(struct gpio_chip *chip, unsigned int offset) > +{ > + struct cirrus_scodec_test_gpio *gpio_priv = gpiochip_get_data(chip); > + > + return !!(gpio_priv->pin_state & BIT(offset)); > +} > + > +static int cirrus_scodec_test_gpio_direction_out(struct gpio_chip *chip, > + unsigned int offset, int value) > +{ > + return -EOPNOTSUPP; > +} > + > +static void cirrus_scodec_test_gpio_set(struct gpio_chip *chip, unsigned int offset, > + int value) > +{ > +} > + > +static int cirrus_scodec_test_gpio_set_config(struct gpio_chip *gc, > + unsigned int offset, > + unsigned long config) > +{ > + switch (pinconf_to_config_param(config)) { > + case PIN_CONFIG_OUTPUT: > + case PIN_CONFIG_OUTPUT_ENABLE: > + return -EOPNOTSUPP; > + default: > + return 0; > + } > +} > + > +static const struct gpio_chip cirrus_scodec_test_gpio_chip = { > + .label = "cirrus_scodec_test_gpio", > + .owner = THIS_MODULE, > + .request = gpiochip_generic_request, > + .free = gpiochip_generic_free, > + .get_direction = cirrus_scodec_test_gpio_get_direction, > + .direction_input = cirrus_scodec_test_gpio_direction_in, > + .get = cirrus_scodec_test_gpio_get, > + .direction_output = cirrus_scodec_test_gpio_direction_out, > + .set = cirrus_scodec_test_gpio_set, > + .set_config = cirrus_scodec_test_gpio_set_config, > + .base = -1, > + .ngpio = 32, > +}; > + > +static int cirrus_scodec_test_gpio_probe(struct platform_device *pdev) > +{ > + struct cirrus_scodec_test_gpio *gpio_priv; > + int ret; > + > + gpio_priv = devm_kzalloc(&pdev->dev, sizeof(*gpio_priv), GFP_KERNEL); > + if (!gpio_priv) > + return -ENOMEM; > + > + /* GPIO core modifies our struct gpio_chip so use a copy */ > + gpio_priv->chip = cirrus_scodec_test_gpio_chip; > + ret = devm_gpiochip_add_data(&pdev->dev, &gpio_priv->chip, gpio_priv); > + if (ret) > + return dev_err_probe(&pdev->dev, ret, "Failed to add gpiochip\n"); > + > + dev_set_drvdata(&pdev->dev, gpio_priv); > + > + return 0; > +} > + > +static struct platform_driver cirrus_scodec_test_gpio_driver = { > + .driver.name = "cirrus_scodec_test_gpio_drv", > + .probe = cirrus_scodec_test_gpio_probe, > +}; > + > +/* software_node referencing the gpio driver */ > +static const struct software_node cirrus_scodec_test_gpio_swnode = { > + .name = "cirrus_scodec_test_gpio", > +}; > + > +static int cirrus_scodec_test_create_gpio(struct kunit *test) > +{ > + struct cirrus_scodec_test_priv *priv = test->priv; > + int ret; > + > + priv->gpio_pdev = platform_device_alloc(cirrus_scodec_test_gpio_driver.driver.name, -1); > + if (!priv->gpio_pdev) > + return -ENOMEM; > + > + ret = device_add_software_node(&priv->gpio_pdev->dev, &cirrus_scodec_test_gpio_swnode); > + if (ret) { > + platform_device_put(priv->gpio_pdev); > + KUNIT_FAIL(test, "Failed to add swnode to gpio: %d\n", ret); > + return ret; > + } > + > + ret = platform_device_add(priv->gpio_pdev); > + if (ret) { > + platform_device_put(priv->gpio_pdev); > + KUNIT_FAIL(test, "Failed to add gpio platform device: %d\n", ret); > + return ret; > + } > + > + priv->gpio_priv = dev_get_drvdata(&priv->gpio_pdev->dev); > + if (!priv->gpio_priv) { > + platform_device_put(priv->gpio_pdev); > + KUNIT_FAIL(test, "Failed to get gpio private data: %d\n", ret); > + return ret; > + } > + > + return 0; > +} > + > +static void cirrus_scodec_test_set_gpio_ref_arg(struct software_node_ref_args *arg, > + int gpio_num) > +{ > + struct software_node_ref_args template = > + SOFTWARE_NODE_REFERENCE(&cirrus_scodec_test_gpio_swnode, gpio_num, 0); I'm observing the following error when building with: $ make LLVM=1 -j128 allmodconfig sound/pci/hda/cirrus_scodec_test.o sound/pci/hda/cirrus_scodec_test.c:151:60: error: initializer element is not a compile-time constant 151 | SOFTWARE_NODE_REFERENCE(&cirrus_scodec_test_gpio_swnode, gpio_num, 0); | ^~~~~~~~ /builds/linux/include/linux/property.h:291:37: note: expanded from macro 'SOFTWARE_NODE_REFERENCE' 291 | .nargs = ARRAY_SIZE(((u64[]){ 0, ##__VA_ARGS__ })) - 1, \ | ^~~~~~~~~~~ /builds/linux/include/linux/kernel.h:57:75: note: expanded from macro 'ARRAY_SIZE' 57 | #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr)) | ^~~ /builds/linux/include/linux/compiler.h:228:59: note: expanded from macro '__must_be_array' 228 | #define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0])) | ^ /builds/linux/include/linux/compiler_types.h:366:63: note: expanded from macro '__same_type' 366 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b)) | ^ /builds/linux/include/linux/build_bug.h:16:62: note: expanded from macro 'BUILD_BUG_ON_ZERO' 16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); }))) | ^ This needs to be fixed before being sent to mainline else it will break our builds; our CI is already red in -next over this. > + > + *arg = template; > +} > + > +static int cirrus_scodec_test_set_spkid_swnode(struct kunit *test, > + struct device *dev, > + struct software_node_ref_args *args, > + int num_args) > +{ > + const struct property_entry props_template[] = { > + PROPERTY_ENTRY_REF_ARRAY_LEN("spk-id-gpios", args, num_args), > + { } > + }; > + struct property_entry *props; > + struct software_node *node; > + > + node = kunit_kzalloc(test, sizeof(*node), GFP_KERNEL); > + if (!node) > + return -ENOMEM; > + > + props = kunit_kzalloc(test, sizeof(props_template), GFP_KERNEL); > + if (!props) > + return -ENOMEM; > + > + memcpy(props, props_template, sizeof(props_template)); > + node->properties = props; > + > + return device_add_software_node(dev, node); > +} > + > +struct cirrus_scodec_test_spkid_param { > + int num_amps; > + int gpios_per_amp; > + int num_amps_sharing; > +}; > + > +static void cirrus_scodec_test_spkid_parse(struct kunit *test) > +{ > + struct cirrus_scodec_test_priv *priv = test->priv; > + const struct cirrus_scodec_test_spkid_param *param = test->param_value; > + int num_spk_id_refs = param->num_amps * param->gpios_per_amp; > + struct software_node_ref_args *refs; > + struct device *dev = &priv->amp_pdev.dev; > + unsigned int v; > + int i, ret; > + > + refs = kunit_kcalloc(test, num_spk_id_refs, sizeof(*refs), GFP_KERNEL); > + KUNIT_ASSERT_NOT_NULL(test, refs); > + > + for (i = 0, v = 0; i < num_spk_id_refs; ) { > + cirrus_scodec_test_set_gpio_ref_arg(&refs[i++], v++); > + > + /* > + * If amps are sharing GPIOs repeat the last set of > + * GPIOs until we've done that number of amps. > + * We have done all GPIOs for an amp when i is a multiple > + * of gpios_per_amp. > + * We have done all amps sharing the same GPIOs when i is > + * a multiple of (gpios_per_amp * num_amps_sharing). > + */ > + if (!(i % param->gpios_per_amp) && > + (i % (param->gpios_per_amp * param->num_amps_sharing))) > + v -= param->gpios_per_amp; > + } > + > + ret = cirrus_scodec_test_set_spkid_swnode(test, dev, refs, num_spk_id_refs); > + KUNIT_EXPECT_EQ_MSG(test, ret, 0, "Failed to add swnode\n"); > + > + for (i = 0; i < param->num_amps; ++i) { > + for (v = 0; v < (1 << param->gpios_per_amp); ++v) { > + /* Set only the GPIO bits used by this amp */ > + priv->gpio_priv->pin_state = > + v << (param->gpios_per_amp * (i / param->num_amps_sharing)); > + > + ret = cirrus_scodec_get_speaker_id(dev, i, param->num_amps, -1); > + KUNIT_EXPECT_EQ_MSG(test, ret, v, > + "get_speaker_id failed amp:%d pin_state:%#x\n", > + i, priv->gpio_priv->pin_state); > + } > + } > +} > + > +static void cirrus_scodec_test_no_spkid(struct kunit *test) > +{ > + struct cirrus_scodec_test_priv *priv = test->priv; > + struct device *dev = &priv->amp_pdev.dev; > + int ret; > + > + ret = cirrus_scodec_get_speaker_id(dev, 0, 4, -1); > + KUNIT_EXPECT_EQ(test, ret, -ENOENT); > +} > + > +static void cirrus_scodec_test_dev_release(struct device *dev) > +{ > +} > + > +static int cirrus_scodec_test_case_init(struct kunit *test) > +{ > + struct cirrus_scodec_test_priv *priv; > + int ret; > + > + priv = kunit_kzalloc(test, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + test->priv = priv; > + > + /* Create dummy GPIO */ > + ret = cirrus_scodec_test_create_gpio(test); > + if (ret < 0) > + return ret; > + > + /* Create dummy amp driver dev */ > + priv->amp_pdev.name = "cirrus_scodec_test_amp_drv"; > + priv->amp_pdev.id = -1; > + priv->amp_pdev.dev.release = cirrus_scodec_test_dev_release; > + ret = platform_device_register(&priv->amp_pdev); > + KUNIT_ASSERT_GE_MSG(test, ret, 0, "Failed to register amp platform device\n"); > + > + return 0; > +} > + > +static void cirrus_scodec_test_case_exit(struct kunit *test) > +{ > + struct cirrus_scodec_test_priv *priv = test->priv; > + > + if (priv->amp_pdev.name) > + platform_device_unregister(&priv->amp_pdev); > + > + if (priv->gpio_pdev) { > + device_remove_software_node(&priv->gpio_pdev->dev); > + platform_device_unregister(priv->gpio_pdev); > + } > +} > + > +static int cirrus_scodec_test_suite_init(struct kunit_suite *suite) > +{ > + int ret; > + > + /* Register mock GPIO driver */ > + ret = platform_driver_register(&cirrus_scodec_test_gpio_driver); > + if (ret < 0) { > + kunit_err(suite, "Failed to register gpio platform driver, %d\n", ret); > + return ret; > + } > + > + return 0; > +} > + > +static void cirrus_scodec_test_suite_exit(struct kunit_suite *suite) > +{ > + platform_driver_unregister(&cirrus_scodec_test_gpio_driver); > +} > + > +static const struct cirrus_scodec_test_spkid_param cirrus_scodec_test_spkid_param_cases[] = { > + { .num_amps = 2, .gpios_per_amp = 1, .num_amps_sharing = 1 }, > + { .num_amps = 2, .gpios_per_amp = 2, .num_amps_sharing = 1 }, > + { .num_amps = 2, .gpios_per_amp = 3, .num_amps_sharing = 1 }, > + { .num_amps = 2, .gpios_per_amp = 4, .num_amps_sharing = 1 }, > + { .num_amps = 3, .gpios_per_amp = 1, .num_amps_sharing = 1 }, > + { .num_amps = 3, .gpios_per_amp = 2, .num_amps_sharing = 1 }, > + { .num_amps = 3, .gpios_per_amp = 3, .num_amps_sharing = 1 }, > + { .num_amps = 3, .gpios_per_amp = 4, .num_amps_sharing = 1 }, > + { .num_amps = 4, .gpios_per_amp = 1, .num_amps_sharing = 1 }, > + { .num_amps = 4, .gpios_per_amp = 2, .num_amps_sharing = 1 }, > + { .num_amps = 4, .gpios_per_amp = 3, .num_amps_sharing = 1 }, > + { .num_amps = 4, .gpios_per_amp = 4, .num_amps_sharing = 1 }, > + > + /* Same GPIO shared by all amps */ > + { .num_amps = 2, .gpios_per_amp = 1, .num_amps_sharing = 2 }, > + { .num_amps = 2, .gpios_per_amp = 2, .num_amps_sharing = 2 }, > + { .num_amps = 2, .gpios_per_amp = 3, .num_amps_sharing = 2 }, > + { .num_amps = 2, .gpios_per_amp = 4, .num_amps_sharing = 2 }, > + { .num_amps = 3, .gpios_per_amp = 1, .num_amps_sharing = 3 }, > + { .num_amps = 3, .gpios_per_amp = 2, .num_amps_sharing = 3 }, > + { .num_amps = 3, .gpios_per_amp = 3, .num_amps_sharing = 3 }, > + { .num_amps = 3, .gpios_per_amp = 4, .num_amps_sharing = 3 }, > + { .num_amps = 4, .gpios_per_amp = 1, .num_amps_sharing = 4 }, > + { .num_amps = 4, .gpios_per_amp = 2, .num_amps_sharing = 4 }, > + { .num_amps = 4, .gpios_per_amp = 3, .num_amps_sharing = 4 }, > + { .num_amps = 4, .gpios_per_amp = 4, .num_amps_sharing = 4 }, > + > + /* Two sets of shared GPIOs */ > + { .num_amps = 4, .gpios_per_amp = 1, .num_amps_sharing = 2 }, > + { .num_amps = 4, .gpios_per_amp = 2, .num_amps_sharing = 2 }, > + { .num_amps = 4, .gpios_per_amp = 3, .num_amps_sharing = 2 }, > + { .num_amps = 4, .gpios_per_amp = 4, .num_amps_sharing = 2 }, > +}; > + > +static void cirrus_scodec_test_spkid_param_desc(const struct cirrus_scodec_test_spkid_param *param, > + char *desc) > +{ > + snprintf(desc, KUNIT_PARAM_DESC_SIZE, "amps:%d gpios_per_amp:%d num_amps_sharing:%d", > + param->num_amps, param->gpios_per_amp, param->num_amps_sharing); > +} > + > +KUNIT_ARRAY_PARAM(cirrus_scodec_test_spkid, cirrus_scodec_test_spkid_param_cases, > + cirrus_scodec_test_spkid_param_desc); > + > +static struct kunit_case cirrus_scodec_test_cases[] = { > + KUNIT_CASE_PARAM(cirrus_scodec_test_spkid_parse, cirrus_scodec_test_spkid_gen_params), > + KUNIT_CASE(cirrus_scodec_test_no_spkid), > + { } /* terminator */ > +}; > + > +static struct kunit_suite cirrus_scodec_test_suite = { > + .name = "snd-hda-scodec-cs35l56-test", > + .suite_init = cirrus_scodec_test_suite_init, > + .suite_exit = cirrus_scodec_test_suite_exit, > + .init = cirrus_scodec_test_case_init, > + .exit = cirrus_scodec_test_case_exit, > + .test_cases = cirrus_scodec_test_cases, > +}; > + > +kunit_test_suite(cirrus_scodec_test_suite); > + > +MODULE_IMPORT_NS(SND_HDA_CIRRUS_SCODEC); > +MODULE_AUTHOR("Richard Fitzgerald <rf@opensource.cirrus.com>"); > +MODULE_LICENSE("GPL"); > diff --git a/sound/pci/hda/cs35l56_hda.c b/sound/pci/hda/cs35l56_hda.c > index 44f5ca0e73e3..d3cfdad7dd76 100644 > --- a/sound/pci/hda/cs35l56_hda.c > +++ b/sound/pci/hda/cs35l56_hda.c > @@ -1035,6 +1035,16 @@ const struct dev_pm_ops cs35l56_hda_pm_ops = { > }; > EXPORT_SYMBOL_NS_GPL(cs35l56_hda_pm_ops, SND_HDA_SCODEC_CS35L56); > > +#if IS_ENABLED(CONFIG_SND_HDA_SCODEC_CS35L56_KUNIT_TEST) > +/* Hooks to export static function to KUnit test */ > + > +int cs35l56_hda_test_hook_get_speaker_id(struct device *dev, int amp_index, int num_amps) > +{ > + return cs35l56_hda_get_speaker_id(dev, amp_index, num_amps); > +} > +EXPORT_SYMBOL_NS_GPL(cs35l56_hda_test_hook_get_speaker_id, SND_HDA_SCODEC_CS35L56); > +#endif > + > MODULE_DESCRIPTION("CS35L56 HDA Driver"); > MODULE_IMPORT_NS(SND_HDA_CIRRUS_SCODEC); > MODULE_IMPORT_NS(SND_HDA_CS_DSP_CONTROLS); > -- > 2.30.2 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] ALSA: hda: cirrus_scodec: Add KUnit test 2023-09-19 20:44 ` Nick Desaulniers @ 2023-09-20 6:51 ` Takashi Iwai 2023-09-20 8:27 ` Richard Fitzgerald 2023-09-20 15:19 ` Andy Shevchenko 0 siblings, 2 replies; 9+ messages in thread From: Takashi Iwai @ 2023-09-20 6:51 UTC (permalink / raw) To: Nick Desaulniers Cc: Richard Fitzgerald, tiwai, alsa-devel, patches, linux-kernel, llvm, Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus, linux-acpi On Tue, 19 Sep 2023 22:44:28 +0200, Nick Desaulniers wrote: > > On Mon, Sep 18, 2023 at 10:51:29AM +0100, Richard Fitzgerald wrote: (snip) > > +static void cirrus_scodec_test_set_gpio_ref_arg(struct software_node_ref_args *arg, > > + int gpio_num) > > +{ > > + struct software_node_ref_args template = > > + SOFTWARE_NODE_REFERENCE(&cirrus_scodec_test_gpio_swnode, gpio_num, 0); > > I'm observing the following error when building with: > > $ make LLVM=1 -j128 allmodconfig sound/pci/hda/cirrus_scodec_test.o > > sound/pci/hda/cirrus_scodec_test.c:151:60: error: initializer element is not a compile-time constant > 151 | SOFTWARE_NODE_REFERENCE(&cirrus_scodec_test_gpio_swnode, gpio_num, 0); > | ^~~~~~~~ > /builds/linux/include/linux/property.h:291:37: note: expanded from macro 'SOFTWARE_NODE_REFERENCE' > 291 | .nargs = ARRAY_SIZE(((u64[]){ 0, ##__VA_ARGS__ })) - 1, \ > | ^~~~~~~~~~~ > /builds/linux/include/linux/kernel.h:57:75: note: expanded from macro 'ARRAY_SIZE' > 57 | #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr)) > | ^~~ > /builds/linux/include/linux/compiler.h:228:59: note: expanded from macro '__must_be_array' > 228 | #define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0])) > | ^ > /builds/linux/include/linux/compiler_types.h:366:63: note: expanded from macro '__same_type' > 366 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b)) > | ^ > /builds/linux/include/linux/build_bug.h:16:62: note: expanded from macro 'BUILD_BUG_ON_ZERO' > 16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); }))) > | ^ Hm, this looks like some inconsistent handling of the temporary array passed to ARRAY_SIZE() in the SOFTWARE_NODE_REFERENCE macro. LLVM can't treat it if it contains a variable in the given array, while GCC doesn't care. A hackish workaround would be the patch like below, but it's really ugly. Ideally speaking, it should be fixed in linux/properties.h, but I have no idea how to fix there for LLVM. Adding more relevant people to Cc. thanks, Takashi --- a/sound/pci/hda/cirrus_scodec_test.c +++ b/sound/pci/hda/cirrus_scodec_test.c @@ -148,8 +148,9 @@ static void cirrus_scodec_test_set_gpio_ref_arg(struct software_node_ref_args *a int gpio_num) { struct software_node_ref_args template = - SOFTWARE_NODE_REFERENCE(&cirrus_scodec_test_gpio_swnode, gpio_num, 0); + SOFTWARE_NODE_REFERENCE(&cirrus_scodec_test_gpio_swnode, 0, 0); + template.args[0] = gpio_num; *arg = template; } ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] ALSA: hda: cirrus_scodec: Add KUnit test 2023-09-20 6:51 ` Takashi Iwai @ 2023-09-20 8:27 ` Richard Fitzgerald 2023-09-20 8:42 ` Takashi Iwai 2023-09-20 15:19 ` Andy Shevchenko 1 sibling, 1 reply; 9+ messages in thread From: Richard Fitzgerald @ 2023-09-20 8:27 UTC (permalink / raw) To: Takashi Iwai, Nick Desaulniers Cc: tiwai, alsa-devel, patches, linux-kernel, llvm, Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus, linux-acpi On 20/9/23 07:51, Takashi Iwai wrote: > On Tue, 19 Sep 2023 22:44:28 +0200, > Nick Desaulniers wrote: >> >> On Mon, Sep 18, 2023 at 10:51:29AM +0100, Richard Fitzgerald wrote: > (snip) >>> +static void cirrus_scodec_test_set_gpio_ref_arg(struct software_node_ref_args *arg, >>> + int gpio_num) >>> +{ >>> + struct software_node_ref_args template = >>> + SOFTWARE_NODE_REFERENCE(&cirrus_scodec_test_gpio_swnode, gpio_num, 0); >> >> I'm observing the following error when building with: >> >> $ make LLVM=1 -j128 allmodconfig sound/pci/hda/cirrus_scodec_test.o >> >> sound/pci/hda/cirrus_scodec_test.c:151:60: error: initializer element is not a compile-time constant >> 151 | SOFTWARE_NODE_REFERENCE(&cirrus_scodec_test_gpio_swnode, gpio_num, 0); >> | ^~~~~~~~ >> /builds/linux/include/linux/property.h:291:37: note: expanded from macro 'SOFTWARE_NODE_REFERENCE' >> 291 | .nargs = ARRAY_SIZE(((u64[]){ 0, ##__VA_ARGS__ })) - 1, \ >> | ^~~~~~~~~~~ >> /builds/linux/include/linux/kernel.h:57:75: note: expanded from macro 'ARRAY_SIZE' >> 57 | #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr)) >> | ^~~ >> /builds/linux/include/linux/compiler.h:228:59: note: expanded from macro '__must_be_array' >> 228 | #define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0])) >> | ^ >> /builds/linux/include/linux/compiler_types.h:366:63: note: expanded from macro '__same_type' >> 366 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b)) >> | ^ >> /builds/linux/include/linux/build_bug.h:16:62: note: expanded from macro 'BUILD_BUG_ON_ZERO' >> 16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); }))) >> | ^ > > Hm, this looks like some inconsistent handling of the temporary array > passed to ARRAY_SIZE() in the SOFTWARE_NODE_REFERENCE macro. LLVM > can't treat it if it contains a variable in the given array, while GCC > doesn't care. > > A hackish workaround would be the patch like below, but it's really > ugly. Ideally speaking, it should be fixed in linux/properties.h, but > I have no idea how to fix there for LLVM. > > Adding more relevant people to Cc. > > > thanks, > > Takashi > > --- a/sound/pci/hda/cirrus_scodec_test.c > +++ b/sound/pci/hda/cirrus_scodec_test.c > @@ -148,8 +148,9 @@ static void cirrus_scodec_test_set_gpio_ref_arg(struct software_node_ref_args *a > int gpio_num) > { > struct software_node_ref_args template = > - SOFTWARE_NODE_REFERENCE(&cirrus_scodec_test_gpio_swnode, gpio_num, 0); > + SOFTWARE_NODE_REFERENCE(&cirrus_scodec_test_gpio_swnode, 0, 0); > > + template.args[0] = gpio_num; > *arg = template; > } > The tests must generate sw nodes dynamically, not a fixed const struct. I wanted to avoid directly accessing the internals of the SW node structures. Use only the macros. I can rewrite this code to open-code the construction of the software_node_ref_args. Or I can wait a while in case anyone has a suggested fix for the macros. Its seems reasonable that you should be able to create software nodes dynamically. A "real" use of these might need to construct the data from some other data that is not known at runtime (for example, where the ACPI provides some information but not the necessary info). ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] ALSA: hda: cirrus_scodec: Add KUnit test 2023-09-20 8:27 ` Richard Fitzgerald @ 2023-09-20 8:42 ` Takashi Iwai 0 siblings, 0 replies; 9+ messages in thread From: Takashi Iwai @ 2023-09-20 8:42 UTC (permalink / raw) To: Richard Fitzgerald Cc: Nick Desaulniers, tiwai, alsa-devel, patches, linux-kernel, llvm, Andy Shevchenko, Daniel Scally, Heikki Krogerus, Sakari Ailus, linux-acpi On Wed, 20 Sep 2023 10:27:58 +0200, Richard Fitzgerald wrote: > > On 20/9/23 07:51, Takashi Iwai wrote: > > On Tue, 19 Sep 2023 22:44:28 +0200, > > Nick Desaulniers wrote: > >> > >> On Mon, Sep 18, 2023 at 10:51:29AM +0100, Richard Fitzgerald wrote: > > (snip) > >>> +static void cirrus_scodec_test_set_gpio_ref_arg(struct software_node_ref_args *arg, > >>> + int gpio_num) > >>> +{ > >>> + struct software_node_ref_args template = > >>> + SOFTWARE_NODE_REFERENCE(&cirrus_scodec_test_gpio_swnode, gpio_num, 0); > >> > >> I'm observing the following error when building with: > >> > >> $ make LLVM=1 -j128 allmodconfig sound/pci/hda/cirrus_scodec_test.o > >> > >> sound/pci/hda/cirrus_scodec_test.c:151:60: error: initializer element is not a compile-time constant > >> 151 | SOFTWARE_NODE_REFERENCE(&cirrus_scodec_test_gpio_swnode, gpio_num, 0); > >> | ^~~~~~~~ > >> /builds/linux/include/linux/property.h:291:37: note: expanded from macro 'SOFTWARE_NODE_REFERENCE' > >> 291 | .nargs = ARRAY_SIZE(((u64[]){ 0, ##__VA_ARGS__ })) - 1, \ > >> | ^~~~~~~~~~~ > >> /builds/linux/include/linux/kernel.h:57:75: note: expanded from macro 'ARRAY_SIZE' > >> 57 | #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr)) > >> | ^~~ > >> /builds/linux/include/linux/compiler.h:228:59: note: expanded from macro '__must_be_array' > >> 228 | #define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0])) > >> | ^ > >> /builds/linux/include/linux/compiler_types.h:366:63: note: expanded from macro '__same_type' > >> 366 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b)) > >> | ^ > >> /builds/linux/include/linux/build_bug.h:16:62: note: expanded from macro 'BUILD_BUG_ON_ZERO' > >> 16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); }))) > >> | ^ > > > > Hm, this looks like some inconsistent handling of the temporary array > > passed to ARRAY_SIZE() in the SOFTWARE_NODE_REFERENCE macro. LLVM > > can't treat it if it contains a variable in the given array, while GCC > > doesn't care. > > > > A hackish workaround would be the patch like below, but it's really > > ugly. Ideally speaking, it should be fixed in linux/properties.h, but > > I have no idea how to fix there for LLVM. > > > > Adding more relevant people to Cc. > > > > > > thanks, > > > > Takashi > > > > --- a/sound/pci/hda/cirrus_scodec_test.c > > +++ b/sound/pci/hda/cirrus_scodec_test.c > > @@ -148,8 +148,9 @@ static void cirrus_scodec_test_set_gpio_ref_arg(struct software_node_ref_args *a > > int gpio_num) > > { > > struct software_node_ref_args template = > > - SOFTWARE_NODE_REFERENCE(&cirrus_scodec_test_gpio_swnode, gpio_num, 0); > > + SOFTWARE_NODE_REFERENCE(&cirrus_scodec_test_gpio_swnode, 0, 0); > > + template.args[0] = gpio_num; > > *arg = template; > > } > > > > The tests must generate sw nodes dynamically, not a fixed const struct. > I wanted to avoid directly accessing the internals of the SW node > structures. Use only the macros. > > I can rewrite this code to open-code the construction of the > software_node_ref_args. Or I can wait a while in case anyone has a > suggested fix for the macros. > > Its seems reasonable that you should be able to create software nodes > dynamically. A "real" use of these might need to construct the data from > some other data that is not known at runtime (for example, where the > ACPI provides some information but not the necessary info). Yeah, fixing the macro would be ideal. An easy and compromise solution would be to factor out the ARRAY_SIZE() call and allow giving nargs explicitly. e.g. --- a/include/linux/property.h +++ b/include/linux/property.h @@ -285,13 +285,18 @@ struct software_node_ref_args { u64 args[NR_FWNODE_REFERENCE_ARGS]; }; -#define SOFTWARE_NODE_REFERENCE(_ref_, ...) \ +#define __SOFTWARE_NODE_REFERENCE(_ref_, _nargs_, ...) \ (const struct software_node_ref_args) { \ .node = _ref_, \ - .nargs = ARRAY_SIZE(((u64[]){ 0, ##__VA_ARGS__ })) - 1, \ + .nargs = _nargs_, \ .args = { __VA_ARGS__ }, \ } +#define SOFTWARE_NODE_REFERENCE(_ref_, ...) \ + __SOFTWARE_NODE_REFERENCE(_ref_,\ + ARRAY_SIZE(((u64[]){ 0, ##__VA_ARGS__ })) - 1,\ + ##__VA_ARGS__) + /** * struct property_entry - "Built-in" device property representation. * @name: Name of the property. --- a/sound/pci/hda/cirrus_scodec_test.c +++ b/sound/pci/hda/cirrus_scodec_test.c @@ -148,7 +148,7 @@ static void cirrus_scodec_test_set_gpio_ref_arg(struct software_node_ref_args *a int gpio_num) { struct software_node_ref_args template = - SOFTWARE_NODE_REFERENCE(&cirrus_scodec_test_gpio_swnode, gpio_num, 0); + __SOFTWARE_NODE_REFERENCE(&cirrus_scodec_test_gpio_swnode, 2, gpio_num, 0); *arg = template; } ... though I'm not convinced by this change, either. Takashi ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] ALSA: hda: cirrus_scodec: Add KUnit test 2023-09-20 6:51 ` Takashi Iwai 2023-09-20 8:27 ` Richard Fitzgerald @ 2023-09-20 15:19 ` Andy Shevchenko 1 sibling, 0 replies; 9+ messages in thread From: Andy Shevchenko @ 2023-09-20 15:19 UTC (permalink / raw) To: Takashi Iwai Cc: Nick Desaulniers, Richard Fitzgerald, tiwai, alsa-devel, patches, linux-kernel, llvm, Daniel Scally, Heikki Krogerus, Sakari Ailus, linux-acpi On Wed, Sep 20, 2023 at 08:51:57AM +0200, Takashi Iwai wrote: > On Tue, 19 Sep 2023 22:44:28 +0200, > Nick Desaulniers wrote: > > > > On Mon, Sep 18, 2023 at 10:51:29AM +0100, Richard Fitzgerald wrote: > (snip) > > > +static void cirrus_scodec_test_set_gpio_ref_arg(struct software_node_ref_args *arg, > > > + int gpio_num) > > > +{ > > > + struct software_node_ref_args template = > > > + SOFTWARE_NODE_REFERENCE(&cirrus_scodec_test_gpio_swnode, gpio_num, 0); > > > > I'm observing the following error when building with: > > > > $ make LLVM=1 -j128 allmodconfig sound/pci/hda/cirrus_scodec_test.o > > > > sound/pci/hda/cirrus_scodec_test.c:151:60: error: initializer element is not a compile-time constant > > 151 | SOFTWARE_NODE_REFERENCE(&cirrus_scodec_test_gpio_swnode, gpio_num, 0); > > | ^~~~~~~~ > > /builds/linux/include/linux/property.h:291:37: note: expanded from macro 'SOFTWARE_NODE_REFERENCE' > > 291 | .nargs = ARRAY_SIZE(((u64[]){ 0, ##__VA_ARGS__ })) - 1, \ > > | ^~~~~~~~~~~ > > /builds/linux/include/linux/kernel.h:57:75: note: expanded from macro 'ARRAY_SIZE' > > 57 | #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr)) > > | ^~~ > > /builds/linux/include/linux/compiler.h:228:59: note: expanded from macro '__must_be_array' > > 228 | #define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0])) > > | ^ > > /builds/linux/include/linux/compiler_types.h:366:63: note: expanded from macro '__same_type' > > 366 | #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b)) > > | ^ > > /builds/linux/include/linux/build_bug.h:16:62: note: expanded from macro 'BUILD_BUG_ON_ZERO' > > 16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); }))) > > | ^ > > Hm, this looks like some inconsistent handling of the temporary array > passed to ARRAY_SIZE() in the SOFTWARE_NODE_REFERENCE macro. LLVM > can't treat it if it contains a variable in the given array, while GCC > doesn't care. > > A hackish workaround would be the patch like below, but it's really > ugly. Ideally speaking, it should be fixed in linux/properties.h, but > I have no idea how to fix there for LLVM. > > Adding more relevant people to Cc. Thank you, I think it's quite easy to fix. Lemme cook the patch... -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] ALSA: hda: cs35l56: Handle speaker id GPIOs 2023-09-18 9:51 [PATCH 0/2] ALSA: hda: cs35l56: Handle speaker id GPIOs Richard Fitzgerald 2023-09-18 9:51 ` [PATCH 1/2] ALSA: hda: cs35l56: Add support for speaker id Richard Fitzgerald 2023-09-18 9:51 ` [PATCH 2/2] ALSA: hda: cirrus_scodec: Add KUnit test Richard Fitzgerald @ 2023-09-18 15:50 ` Takashi Iwai 2 siblings, 0 replies; 9+ messages in thread From: Takashi Iwai @ 2023-09-18 15:50 UTC (permalink / raw) To: Richard Fitzgerald; +Cc: tiwai, alsa-devel, patches, linux-kernel On Mon, 18 Sep 2023 11:51:27 +0200, Richard Fitzgerald wrote: > > Some manufacturers use multiple sources of speakers. Motherboard > GPIOs are set to indicate which type of speaker is fitted so that > the correct tunings can be loaded. Patch #1 adds support for this > and patch #2 adds a KUnit test for the new code. > > Richard Fitzgerald (2): > ALSA: hda: cs35l56: Add support for speaker id > ALSA: hda: cirrus_scodec: Add KUnit test Applied both patches to for-next branch now. thanks, Takashi ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-09-20 15:21 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-09-18 9:51 [PATCH 0/2] ALSA: hda: cs35l56: Handle speaker id GPIOs Richard Fitzgerald 2023-09-18 9:51 ` [PATCH 1/2] ALSA: hda: cs35l56: Add support for speaker id Richard Fitzgerald 2023-09-18 9:51 ` [PATCH 2/2] ALSA: hda: cirrus_scodec: Add KUnit test Richard Fitzgerald 2023-09-19 20:44 ` Nick Desaulniers 2023-09-20 6:51 ` Takashi Iwai 2023-09-20 8:27 ` Richard Fitzgerald 2023-09-20 8:42 ` Takashi Iwai 2023-09-20 15:19 ` Andy Shevchenko 2023-09-18 15:50 ` [PATCH 0/2] ALSA: hda: cs35l56: Handle speaker id GPIOs Takashi Iwai
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).