Main source code for tas2781 driver for SPI.

Signed-off-by: Baojun Xu <baojun.xu@ti.com>

---
v2:
 - Update information in file header of tas2781-spi.h.
 - Remove useless micro define.
 - Change TASDEVICE_I2CChecksum to TASDEVICE_Checksum
 - Remove enum control_bus as current code support SPI only.
 - Remove device define as current code support TAS2781 only.
 - Remove spi_device **spi_devs in structure tasdevice_priv.
 - Remove cal_binaryname in structure tasdevice_priv.
 - Remove ndev in structure tasdevice_priv.
 - Change isacpi and  isspi, replace by control_bus(I2C or SPI).
 - Remove void tasdevice_spi_dsp_remove(void *context).
 - Add acpi_device and irq in structure tas2781_hda in tas2781_hda_spi.c.
 - Remove parameter chn in all of registers access functions.
 - Add tascodec_spi_init().
 - Add chip select gpio set for SPI device.
 - Change tasdevice_tuning_switch() to tasdevice_spi_tuning_switch().
 - Change device offset from tas_priv->ndev to tas_priv->index.
 - Change tasdevice_dsp_remove() to tasdevice_spi_dsp_remove().
 - tasdevice_prmg_load() to tasdevice_spi_prmg_load().
 - Change tasdevice_config_info_remove() to
   tasdevice_spi_config_info_remove().
 - Add one dummy byte shift when read from page 2~127, or not book 0.
 - Change tasdevice_spi_change_chn_book() to tasdevice_spi_switch_book()
 - Perform put_device(physdev) before error return.
 - Change calibration data id to 2781 from 2783 in EFI
 - Change calibration data structure name in comments.
---
 sound/pci/hda/tas2781-spi.h     |  162 ++++
 sound/pci/hda/tas2781_hda_spi.c | 1336 +++++++++++++++++++++++++++++++
 2 files changed, 1498 insertions(+)
 create mode 100644 sound/pci/hda/tas2781-spi.h
 create mode 100644 sound/pci/hda/tas2781_hda_spi.c

diff --git a/sound/pci/hda/tas2781-spi.h b/sound/pci/hda/tas2781-spi.h
new file mode 100644
index 000000000000..2540bce175d1
--- /dev/null
+++ b/sound/pci/hda/tas2781-spi.h
@@ -0,0 +1,162 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+//
+// ALSA SoC Texas Instruments TAS2781 Audio Smart Amplifier
+//
+// Copyright (C) 2024 Texas Instruments Incorporated
+// https://www.ti.com
+//
+// The TAS2781 driver implements a flexible and configurable
+// algo coefficient setting for TAS2781 chips.
+//
+// Author: Baojun Xu <baojun.xu@ti.com>
+//
+
+#ifndef __TAS2781_SPI_H__
+#define __TAS2781_SPI_H__
+
+#include <sound/tas2781-dsp.h>
+
+#define TASDEVICE_RATES			(SNDRV_PCM_RATE_44100 |\
+	SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_96000 |\
+	SNDRV_PCM_RATE_88200)
+
+#define TASDEVICE_FORMATS		(SNDRV_PCM_FMTBIT_S16_LE | \
+	SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE)
+
+/*PAGE Control Register (available in page0 of each book) */
+#define TASDEVICE_PAGE_SELECT		0x00
+#define TASDEVICE_BOOKCTL_PAGE		0x00
+#define TASDEVICE_BOOKCTL_REG		127
+#define TASDEVICE_BOOK_ID(reg)		(reg / (256 * 128))
+#define TASDEVICE_PAGE_ID(reg)		((reg % (256 * 128)) / 128)
+#define TASDEVICE_PAGE_REG(reg)		((reg % (256 * 128)) % 128)
+#define TASDEVICE_PGRG(reg)		(reg % (256 * 128))
+#define TASDEVICE_REG(book, page, reg)	(((book * 256 * 128) + \
+					(page * 128)) + reg)
+
+/*Software Reset */
+#define TAS2781_REG_SWRESET		TASDEVICE_REG(0x0, 0X0, 0x01)
+#define TAS2781_REG_SWRESET_RESET	BIT(0)
+
+/*I2C Checksum */
+#define TASDEVICE_Checksum		TASDEVICE_REG(0x0, 0x0, 0x7E)
+
+/* Volume control */
+#define TAS2781_DVC_LVL			TASDEVICE_REG(0x0, 0x0, 0x1A)
+#define TAS2781_AMP_LEVEL		TASDEVICE_REG(0x0, 0x0, 0x03)
+#define TAS2781_AMP_LEVEL_MASK		GENMASK(5, 1)
+
+#define TASDEVICE_CMD_SING_W		0x1
+#define TASDEVICE_CMD_BURST		0x2
+#define TASDEVICE_CMD_DELAY		0x3
+#define TASDEVICE_CMD_FIELD_W		0x4
+
+#define TAS2781_SPI_MAX_FREQ		4000000
+
+enum device_catlog_id {
+	HP = 0,
+	OTHERS
+};
+
+struct tasdevice {
+	struct tasdevice_fw *cali_data_fmw;
+	unsigned int dev_addr;
+	unsigned int err_code;
+	unsigned char cur_book;
+	short cur_prog;
+	short cur_conf;
+	bool is_loading;
+	bool is_loaderr;
+};
+
+struct tasdevice_irqinfo {
+	int irq_gpio;
+	int irq;
+};
+
+struct calidata {
+	unsigned char *data;
+	unsigned long total_sz;
+};
+
+struct tasdevice_priv {
+	struct tasdevice tasdevice;
+	struct tasdevice_irqinfo irq_info;
+	struct tasdevice_rca rcabin;
+	struct calidata cali_data;
+	struct tasdevice_fw *fmw;
+	struct gpio_desc *reset;
+	struct gpio_desc *chipselect;
+	struct mutex codec_lock;
+	struct regmap *regmap;
+	struct device *dev;
+	struct tm tm;
+
+	enum device_catlog_id catlog_id;
+	const char *acpi_subsystem_id;
+	unsigned char crc8_lkp_tbl[CRC8_TABLE_SIZE];
+	unsigned char coef_binaryname[64];
+	unsigned char rca_binaryname[64];
+	unsigned char dev_name[32];
+	unsigned int magic_num;
+	unsigned int chip_id;
+	unsigned int sysclk;
+
+	int cur_prog;
+	int cur_conf;
+	int fw_state;
+	int index;
+	void *client;
+	void *codec;
+	bool force_fwload_status;
+	bool playback_started;
+	unsigned int global_addr;
+
+	int (*fw_parse_variable_header)(struct tasdevice_priv *tas_priv,
+		const struct firmware *fmw, int offset);
+	int (*fw_parse_program_data)(struct tasdevice_priv *tas_priv,
+		struct tasdevice_fw *tas_fmw,
+		const struct firmware *fmw, int offset);
+	int (*fw_parse_configuration_data)(struct tasdevice_priv *tas_priv,
+		struct tasdevice_fw *tas_fmw,
+		const struct firmware *fmw, int offset);
+	int (*tasdevice_load_block)(struct tasdevice_priv *tas_priv,
+		struct tasdev_blk *block);
+
+	int (*save_calibration)(struct tasdevice_priv *tas_priv);
+	void (*apply_calibration)(struct tasdevice_priv *tas_priv);
+};
+
+int tascodec_init(struct tasdevice_priv *tas_priv, void *codec,
+	struct module *module,
+	void (*cont)(const struct firmware *fw, void *context));
+int tasdevice_save_calibration(struct tasdevice_priv *tas_priv);
+void tasdevice_apply_calibration(struct tasdevice_priv *tas_priv);
+void tasdevice_remove(struct tasdevice_priv *tas_priv);
+int tasdevice_spi_dev_read(struct tasdevice_priv *tas_priv,
+	unsigned int reg, unsigned int *value);
+int tasdevice_spi_dev_write(struct tasdevice_priv *tas_priv,
+	unsigned int reg, unsigned int value);
+int tasdevice_spi_dev_bulk_write(
+	struct tasdevice_priv *tas_priv, unsigned int reg,
+	unsigned char *p_data, unsigned int n_length);
+int tasdevice_spi_dev_bulk_read(struct tasdevice_priv *tas_priv,
+	unsigned int reg, unsigned char *p_data, unsigned int n_length);
+int tasdevice_spi_dev_update_bits(struct tasdevice_priv *tasdevice,
+	unsigned int reg, unsigned int mask, unsigned int value);
+
+void tasdevice_spi_select_cfg_blk(void *context, int conf_no,
+	unsigned char block_type);
+void tasdevice_spi_config_info_remove(void *context);
+int tasdevice_spi_dsp_parser(void *context);
+int tasdevice_spi_rca_parser(void *context, const struct firmware *fmw);
+void tasdevice_spi_dsp_remove(void *context);
+void tasdevice_spi_calbin_remove(void *context);
+int tasdevice_spi_select_tuningprm_cfg(void *context, int prm,
+	int cfg_no, int rca_conf_no);
+int tasdevice_spi_prmg_load(void *context, int prm_no);
+int tasdevice_spi_prmg_calibdata_load(void *context, int prm_no);
+void tasdevice_spi_tuning_switch(void *context, int state);
+int tas2781_spi_load_calibration(void *context, char *file_name,
+	unsigned short i);
+#endif /* __TAS2781_SPI_H__ */
diff --git a/sound/pci/hda/tas2781_hda_spi.c b/sound/pci/hda/tas2781_hda_spi.c
new file mode 100644
index 000000000000..0ac3757ac259
--- /dev/null
+++ b/sound/pci/hda/tas2781_hda_spi.c
@@ -0,0 +1,1336 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// TAS2781 HDA SPI driver
+//
+// Copyright 2024 Texas Instruments, Inc.
+//
+// Author: Baojun Xu <baojun.xu@ti.com>
+
+#include <linux/acpi.h>
+#include <linux/crc8.h>
+#include <linux/crc32.h>
+#include <linux/efi.h>
+#include <linux/firmware.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/spi/spi.h>
+#include <sound/hda_codec.h>
+#include <sound/soc.h>
+#include <sound/tlv.h>
+#include <sound/tas2781-tlv.h>
+
+#include "tas2781-spi.h"
+
+#include "hda_local.h"
+#include "hda_auto_parser.h"
+#include "hda_component.h"
+#include "hda_jack.h"
+#include "hda_generic.h"
+
+#define TASDEVICE_CRC8_POLYNOMIAL	0x4d
+
+#define TASDEVICE_SPEAKER_CALIBRATION_SIZE	20
+
+/* No standard control callbacks for SNDRV_CTL_ELEM_IFACE_CARD
+ * Define two controls, one is Volume control callbacks, the other is
+ * flag setting control callbacks.
+ */
+
+/* Volume control callbacks for tas2781 */
+#define ACARD_SINGLE_RANGE_EXT_TLV(xname, xreg, xshift, xmin, xmax, xinvert, \
+	xhandler_get, xhandler_put, tlv_array) \
+{	.iface = SNDRV_CTL_ELEM_IFACE_CARD, .name = (xname),\
+	.access = SNDRV_CTL_ELEM_ACCESS_TLV_READ |\
+		 SNDRV_CTL_ELEM_ACCESS_READWRITE,\
+	.tlv.p = (tlv_array), \
+	.info = snd_soc_info_volsw_range, \
+	.get = xhandler_get, .put = xhandler_put, \
+	.private_value = (unsigned long)&(struct soc_mixer_control) \
+		{.reg = xreg, .rreg = xreg, .shift = xshift, \
+		 .rshift = xshift, .min = xmin, .max = xmax, \
+		 .invert = xinvert} }
+
+/* Flag control callbacks for tas2781 */
+#define ACARD_SINGLE_BOOL_EXT(xname, xdata, xhandler_get, xhandler_put) \
+{	.iface = SNDRV_CTL_ELEM_IFACE_CARD, .name = xname, \
+	.info = snd_ctl_boolean_mono_info, \
+	.get = xhandler_get, .put = xhandler_put, \
+	.private_value = xdata }
+
+
+enum calib_data {
+	R0_VAL = 0,
+	INV_R0,
+	R0LOW,
+	POWER,
+	TLIM,
+	CALIB_MAX
+};
+
+struct tas2781_hda {
+	struct device *dev;
+	struct gpio_desc *cs_gpio;
+	struct gpio_desc *reset_gpio;
+	struct tasdevice_priv *priv;
+	struct acpi_device *dacpi;
+	struct snd_kcontrol *dsp_prog_ctl;
+	struct snd_kcontrol *dsp_conf_ctl;
+	struct snd_kcontrol *prof_ctl;
+	struct snd_kcontrol *snd_ctls[3];
+
+	int irq;
+};
+
+static const struct regmap_range_cfg tasdevice_ranges[] = {
+	{
+		.range_min = 0,
+		.range_max = 256 * 128,
+		.selector_reg = TASDEVICE_PAGE_SELECT,
+		.selector_mask = 0xff,
+		.selector_shift = 0,
+		.window_start = 0,
+		.window_len = 128,
+	},
+};
+
+static const struct regmap_config tasdevice_regmap = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.cache_type = REGCACHE_NONE,
+	.ranges = tasdevice_ranges,
+	.num_ranges = ARRAY_SIZE(tasdevice_ranges),
+	.max_register = 256 * 128,
+};
+
+
+static void tas2781_spi_reset(struct tasdevice_priv *tas_dev);
+static int tasdevice_spi_init(struct tasdevice_priv *tas_priv);
+static int tasdevice_spi_amp_putvol(struct tasdevice_priv *tas_priv,
+	struct snd_ctl_elem_value *ucontrol, struct soc_mixer_control *mc);
+static int tasdevice_spi_amp_getvol(struct tasdevice_priv *tas_priv,
+	struct snd_ctl_elem_value *ucontrol, struct soc_mixer_control *mc);
+static int tasdevice_spi_digital_putvol(struct tasdevice_priv *tas_priv,
+	struct snd_ctl_elem_value *ucontrol, struct soc_mixer_control *mc);
+static int tasdevice_spi_digital_getvol(struct tasdevice_priv *tas_priv,
+	struct snd_ctl_elem_value *ucontrol, struct soc_mixer_control *mc);
+
+static int tasdevice_spi_switch_book(struct tasdevice_priv *tas_priv,
+	int book)
+{
+	struct tasdevice *tasdev = &tas_priv->tasdevice;
+	struct regmap *map = tas_priv->regmap;
+	int ret = 0;
+
+	if (tasdev->cur_book != book) {
+		/* Change to page 0 before book change. */
+		ret = regmap_write(map, TASDEVICE_PAGE_SELECT, 0);
+		if (ret < 0) {
+			dev_err(tas_priv->dev, "%s, E=%d\n", __func__, ret);
+			return ret;
+		}
+		ret = regmap_write(map, TASDEVICE_BOOKCTL_REG, book);
+		if (ret < 0)
+			dev_err(tas_priv->dev, "%s, E=%d\n", __func__, ret);
+
+		tasdev->cur_book = book;
+	}
+
+	return ret;
+}
+
+int tasdevice_spi_dev_read(struct tasdevice_priv *tas_priv,
+	unsigned int reg, unsigned int *val)
+{
+	struct regmap *map = tas_priv->regmap;
+	int ret;
+
+	ret = tasdevice_spi_switch_book(tas_priv, TASDEVICE_BOOK_ID(reg));
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * In our TAS2781 SPI mode, if read from other book (not book 0),
+	 * or read from page number larger than 1 in book 0, one byte more
+	 * read is needed, and first byte is a dummy byte, need to be ignored.
+	 */
+	if ((TASDEVICE_BOOK_ID(reg) > 0) || (TASDEVICE_PAGE_ID(reg) > 1)) {
+		unsigned char data[2];
+
+		ret = regmap_bulk_read(map, TASDEVICE_PGRG(reg), data, 2);
+		*val = data[1];
+	} else {
+		ret = regmap_read(map, TASDEVICE_PGRG(reg), val);
+	}
+	if (ret < 0)
+		dev_err(tas_priv->dev, "%s, E=%d\n", __func__, ret);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(tasdevice_spi_dev_read);
+
+int tasdevice_spi_dev_write(struct tasdevice_priv *tas_priv,
+	unsigned int reg, unsigned int value)
+{
+	struct regmap *map = tas_priv->regmap;
+	int ret;
+
+	ret = tasdevice_spi_switch_book(tas_priv, TASDEVICE_BOOK_ID(reg));
+	if (ret < 0)
+		return ret;
+
+	ret = regmap_write(map, TASDEVICE_PGRG(reg), value);
+	if (ret < 0)
+		dev_err(tas_priv->dev, "%s, E=%d\n", __func__, ret);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(tasdevice_spi_dev_write);
+
+int tasdevice_spi_dev_bulk_write(struct tasdevice_priv *tas_priv,
+	unsigned int reg, unsigned char *data, unsigned int len)
+{
+	struct regmap *map = tas_priv->regmap;
+	int ret;
+
+	ret = tasdevice_spi_switch_book(tas_priv, TASDEVICE_BOOK_ID(reg));
+	if (ret < 0)
+		return ret;
+
+	ret = regmap_bulk_write(map, TASDEVICE_PGRG(reg), data, len);
+	if (ret < 0)
+		dev_err(tas_priv->dev, "%s, E=%d\n", __func__, ret);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(tasdevice_spi_dev_bulk_write);
+
+int tasdevice_spi_dev_bulk_read(struct tasdevice_priv *tas_priv,
+	unsigned int reg, unsigned char *data, unsigned int len)
+{
+	struct regmap *map = tas_priv->regmap;
+	int ret;
+
+	ret = tasdevice_spi_switch_book(tas_priv, TASDEVICE_BOOK_ID(reg));
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * In our TAS2781 SPI mode, if read from other book (not book 0),
+	 * or read from page number larger than 1 in book 0, one byte more
+	 * read is needed, and first byte is a dummy byte, need to be ignored.
+	 */
+	if ((TASDEVICE_BOOK_ID(reg) > 0) || (TASDEVICE_PAGE_ID(reg) > 1)) {
+		unsigned char buf[129];
+
+		ret = regmap_bulk_read(map, TASDEVICE_PGRG(reg),
+			buf, len + 1);
+		memcpy(data, buf + 1, len);
+	} else {
+		ret = regmap_bulk_read(map, TASDEVICE_PGRG(reg), data, len);
+	}
+	if (ret < 0)
+		dev_err(tas_priv->dev, "%s, E=%d\n", __func__, ret);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(tasdevice_spi_dev_bulk_read);
+
+int tasdevice_spi_dev_update_bits(struct tasdevice_priv *tas_priv,
+	unsigned int reg, unsigned int mask, unsigned int value)
+{
+	struct regmap *map = tas_priv->regmap;
+	int ret;
+
+	ret = tasdevice_spi_switch_book(tas_priv, TASDEVICE_BOOK_ID(reg));
+	if (ret < 0)
+		return ret;
+
+	ret = regmap_update_bits(map, TASDEVICE_PGRG(reg), mask, value);
+	if (ret < 0)
+		dev_err(tas_priv->dev, "%s, E=%d\n", __func__, ret);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(tasdevice_spi_dev_update_bits);
+
+static void tas2781_spi_reset(struct tasdevice_priv *tas_dev)
+{
+	int ret;
+
+	if (tas_dev->reset) {
+		gpiod_set_value_cansleep(tas_dev->reset, 0);
+		usleep_range(500, 1000);
+		gpiod_set_value_cansleep(tas_dev->reset, 1);
+	} else {
+		ret = tasdevice_spi_dev_write(tas_dev,
+			TAS2781_REG_SWRESET, TAS2781_REG_SWRESET_RESET);
+		if (ret < 0)
+			dev_err(tas_dev->dev, "dev sw-reset fail, %d\n", ret);
+	}
+	usleep_range(1000, 1050);
+}
+
+static int tascodec_spi_init(struct tasdevice_priv *tas_priv, void *codec,
+	struct module *module,
+	void (*cont)(const struct firmware *fw, void *context))
+{
+	int ret = 0;
+
+	/*
+	 * Codec Lock Hold to ensure that codec_probe and firmware parsing and
+	 * loading do not simultaneously execute.
+	 */
+	mutex_lock(&tas_priv->codec_lock);
+
+	scnprintf(tas_priv->rca_binaryname, 64, "%sRCA%d.bin",
+		tas_priv->dev_name, tas_priv->index);
+	crc8_populate_msb(tas_priv->crc8_lkp_tbl, TASDEVICE_CRC8_POLYNOMIAL);
+	tas_priv->codec = codec;
+	ret = request_firmware_nowait(module, FW_ACTION_UEVENT,
+		tas_priv->rca_binaryname, tas_priv->dev, GFP_KERNEL, tas_priv,
+		cont);
+	if (ret)
+		dev_err(tas_priv->dev, "request_firmware_nowait err:0x%08x\n",
+			ret);
+
+	/* Codec Lock Release*/
+	mutex_unlock(&tas_priv->codec_lock);
+	return ret;
+}
+
+static int tasdevice_spi_init(struct tasdevice_priv *tas_priv)
+{
+	tas_priv->cur_prog = -1;
+	tas_priv->cur_conf = -1;
+
+	tas_priv->tasdevice.cur_book = -1;
+	tas_priv->tasdevice.cur_prog = -1;
+	tas_priv->tasdevice.cur_conf = -1;
+
+	mutex_init(&tas_priv->codec_lock);
+
+	return 0;
+}
+
+static int tasdevice_clamp(int val, int max, unsigned int invert)
+{
+	if (val > max)
+		val = max;
+	if (invert)
+		val = max - val;
+	if (val < 0)
+		val = 0;
+	return val;
+}
+
+static int tasdevice_spi_amp_putvol(struct tasdevice_priv *tas_priv,
+	struct snd_ctl_elem_value *ucontrol, struct soc_mixer_control *mc)
+{
+	unsigned int invert = mc->invert;
+	unsigned char mask;
+	int max = mc->max;
+	int val, ret;
+
+	mask = (1 << fls(max)) - 1;
+	mask <<= mc->shift;
+	val = tasdevice_clamp(ucontrol->value.integer.value[0], max, invert);
+	ret = tasdevice_spi_dev_update_bits(tas_priv,
+		mc->reg, mask, (unsigned int)(val << mc->shift));
+	if (ret)
+		dev_err(tas_priv->dev, "set AMP vol error in dev %d\n",
+			tas_priv->index);
+
+	return ret;
+}
+
+static int tasdevice_spi_amp_getvol(struct tasdevice_priv *tas_priv,
+	struct snd_ctl_elem_value *ucontrol, struct soc_mixer_control *mc)
+{
+	unsigned int invert = mc->invert;
+	unsigned char mask = 0;
+	int max = mc->max;
+	int ret = 0;
+	int val;
+
+	/* Read the primary device */
+	ret = tasdevice_spi_dev_read(tas_priv, mc->reg, &val);
+	if (ret) {
+		dev_err(tas_priv->dev, "%s, get AMP vol error\n", __func__);
+		return ret;
+	}
+
+	mask = (1 << fls(max)) - 1;
+	mask <<= mc->shift;
+	val = (val & mask) >> mc->shift;
+	val = tasdevice_clamp(val, max, invert);
+	ucontrol->value.integer.value[0] = val;
+
+	return ret;
+
+}
+
+static int tasdevice_spi_digital_putvol(struct tasdevice_priv *tas_priv,
+	struct snd_ctl_elem_value *ucontrol, struct soc_mixer_control *mc)
+{
+	unsigned int invert = mc->invert;
+	int max = mc->max;
+	int val, ret;
+
+	val = tasdevice_clamp(ucontrol->value.integer.value[0], max, invert);
+
+	ret = tasdevice_spi_dev_write(tas_priv, mc->reg, (unsigned int)val);
+	if (ret)
+		dev_err(tas_priv->dev, "set digital vol err in dev %d\n",
+			tas_priv->index);
+
+	return ret;
+
+}
+
+static int tasdevice_spi_digital_getvol(struct tasdevice_priv *tas_priv,
+	struct snd_ctl_elem_value *ucontrol, struct soc_mixer_control *mc)
+{
+	unsigned int invert = mc->invert;
+	int max = mc->max;
+	int ret, val;
+
+	/* Read the primary device as the whole */
+	ret = tasdevice_spi_dev_read(tas_priv, mc->reg, &val);
+	if (ret) {
+		dev_err(tas_priv->dev, "%s, get digital vol err\n", __func__);
+		return ret;
+	}
+
+	val = tasdevice_clamp(val, max, invert);
+	ucontrol->value.integer.value[0] = val;
+
+	return ret;
+
+}
+
+static int tas2781_read_acpi(struct tas2781_hda *tas_hda, const char *hid,
+	int id)
+{
+	struct tasdevice_priv *p = tas_hda->priv;
+	struct gpio_desc *cs_gpiod;
+	struct acpi_device *adev;
+	struct device *physdev;
+	struct spi_device *spi;
+	u32 values[HDA_MAX_COMPONENTS];
+	const char *sub, *property;
+	size_t nval;
+	int ret, i;
+
+	adev = acpi_dev_get_first_match_dev(hid, NULL, -1);
+	if (!adev) {
+		dev_err(p->dev,
+			"Failed to find an ACPI device for %s\n", hid);
+		return -ENODEV;
+	}
+
+	strscpy(p->dev_name, hid, sizeof(p->dev_name));
+	tas_hda->dacpi = adev;
+	physdev = get_device(acpi_get_first_physical_node(adev));
+
+	/* No side-effect to the playback even if subsystem_id is NULL*/
+	sub = acpi_get_subsystem_id(ACPI_HANDLE(physdev));
+	if (IS_ERR(sub))
+		sub = NULL;
+
+	p->acpi_subsystem_id = sub;
+
+	property = "ti,dev-index";
+	ret = device_property_count_u32(physdev, property);
+	if (ret <= 0)
+		goto err;
+
+	if (ret > ARRAY_SIZE(values)) {
+		ret = -EINVAL;
+		goto err;
+	}
+	nval = ret;
+
+	ret = device_property_read_u32_array(physdev, property, values, nval);
+	if (ret)
+		goto err;
+
+	p->index = 0xFF;
+	for (i = 0; i < nval; i++) {
+		if (values[i] == id) {
+			p->index = i;
+			break;
+		}
+	}
+	/*
+	 * It's not an error for the ID to be missing: for I2C there can be
+	 * an alias address that is not a real device. So reject silently.
+	 */
+	if (p->index == 0xFF) {
+		dev_dbg(p->dev, "No index found in %s\n", property);
+		ret = -ENODEV;
+		goto err;
+	}
+
+	p->reset = devm_gpiod_get_index_optional(physdev,
+		"reset", p->index, GPIOD_OUT_LOW);
+	if (IS_ERR(p->reset)) {
+		ret = PTR_ERR(p->reset);
+
+		/*
+		 * If RESET is shared the first amp to probe will grab the
+		 * reset line and reset all the amps
+		 */
+		if (ret != -EBUSY) {
+			dev_err_probe(p->dev, ret,
+				"Failed to get reset GPIO\n");
+			goto err;
+		}
+
+		dev_info(p->dev, "Reset GPIO busy, assume shared reset\n");
+		p->reset = NULL;
+	}
+
+	/*
+	 * Manually set the Chip Select for the second amp <cs_gpio_index>
+	 * in the node.
+	 * This is only supported for systems with 2 amps, since we cannot
+	 * expand the default number of chip selects without using cs-gpios
+	 * The CS GPIO must be set high prior to communicating with the
+	 * first amp (which uses a native chip select), to ensure the second
+	 * amp does not clash with the first.
+	 */
+	if (IS_ENABLED(CONFIG_SPI)) {
+		spi = to_spi_device(p->dev);
+
+		/*
+		 * This is obtained using driver_gpios, since only one GPIO
+		 * for CS exists, this can be obtained using index 0.
+		 */
+		cs_gpiod = devm_gpiod_get_index_optional(physdev, "cs",
+			p->index, GPIOD_OUT_LOW);
+		if (IS_ERR(cs_gpiod)) {
+			dev_err(p->dev,
+				"Unable to get CS GPIO descriptor\n");
+			ret = PTR_ERR(cs_gpiod);
+			goto err;
+		}
+		if (id == 1) {
+			spi_set_csgpiod(spi, 0, cs_gpiod);
+			p->chipselect = cs_gpiod;
+		} else {
+			gpiod_set_value_cansleep(cs_gpiod, true);
+			gpiod_put(cs_gpiod);
+		}
+		spi_setup(spi);
+	}
+	put_device(physdev);
+
+	return 0;
+
+err:
+	dev_err(p->dev, "read acpi error, ret: %d\n", ret);
+	put_device(physdev);
+	acpi_dev_put(adev);
+
+	return ret;
+}
+
+static void tas2781_hda_playback_hook(struct device *dev, int action)
+{
+	struct tas2781_hda *tas_hda = dev_get_drvdata(dev);
+
+	switch (action) {
+	case HDA_GEN_PCM_ACT_OPEN:
+		pm_runtime_get_sync(dev);
+		mutex_lock(&tas_hda->priv->codec_lock);
+		tasdevice_spi_tuning_switch(tas_hda->priv, 0);
+		mutex_unlock(&tas_hda->priv->codec_lock);
+		break;
+	case HDA_GEN_PCM_ACT_CLOSE:
+		mutex_lock(&tas_hda->priv->codec_lock);
+		tasdevice_spi_tuning_switch(tas_hda->priv, 1);
+		mutex_unlock(&tas_hda->priv->codec_lock);
+
+		pm_runtime_mark_last_busy(dev);
+		pm_runtime_put_autosuspend(dev);
+		break;
+	default:
+		dev_dbg(tas_hda->dev, "Playback action not supported: %d\n",
+			action);
+		break;
+	}
+}
+
+static int tasdevice_info_profile(struct snd_kcontrol *kcontrol,
+			struct snd_ctl_elem_info *uinfo)
+{
+	struct tasdevice_priv *tas_priv = snd_kcontrol_chip(kcontrol);
+
+	uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
+	uinfo->count = 1;
+	uinfo->value.integer.min = 0;
+	uinfo->value.integer.max = tas_priv->rcabin.ncfgs - 1;
+
+	return 0;
+}
+
+static int tasdevice_get_profile_id(struct snd_kcontrol *kcontrol,
+			struct snd_ctl_elem_value *ucontrol)
+{
+	struct tasdevice_priv *tas_priv = snd_kcontrol_chip(kcontrol);
+
+	ucontrol->value.integer.value[0] = tas_priv->rcabin.profile_cfg_id;
+
+	return 0;
+}
+
+static int tasdevice_set_profile_id(struct snd_kcontrol *kcontrol,
+	struct snd_ctl_elem_value *ucontrol)
+{
+	struct tasdevice_priv *tas_priv = snd_kcontrol_chip(kcontrol);
+	int nr_profile = ucontrol->value.integer.value[0];
+	int max = tas_priv->rcabin.ncfgs - 1;
+	int val, ret = 0;
+
+	val = clamp(nr_profile, 0, max);
+
+	if (tas_priv->rcabin.profile_cfg_id != val) {
+		tas_priv->rcabin.profile_cfg_id = val;
+		ret = 1;
+	}
+
+	return ret;
+}
+
+static int tasdevice_info_programs(struct snd_kcontrol *kcontrol,
+	struct snd_ctl_elem_info *uinfo)
+{
+	struct tasdevice_priv *tas_priv = snd_kcontrol_chip(kcontrol);
+	struct tasdevice_fw *tas_fw = tas_priv->fmw;
+
+	uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
+	uinfo->count = 1;
+	uinfo->value.integer.min = 0;
+	uinfo->value.integer.max = tas_fw->nr_programs - 1;
+
+	return 0;
+}
+
+static int tasdevice_info_config(struct snd_kcontrol *kcontrol,
+	struct snd_ctl_elem_info *uinfo)
+{
+	struct tasdevice_priv *tas_priv = snd_kcontrol_chip(kcontrol);
+	struct tasdevice_fw *tas_fw = tas_priv->fmw;
+
+	uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
+	uinfo->count = 1;
+	uinfo->value.integer.min = 0;
+	uinfo->value.integer.max = tas_fw->nr_configurations - 1;
+
+	return 0;
+}
+
+static int tasdevice_program_get(struct snd_kcontrol *kcontrol,
+	struct snd_ctl_elem_value *ucontrol)
+{
+	struct tasdevice_priv *tas_priv = snd_kcontrol_chip(kcontrol);
+
+	ucontrol->value.integer.value[0] = tas_priv->cur_prog;
+
+	return 0;
+}
+
+static int tasdevice_program_put(struct snd_kcontrol *kcontrol,
+	struct snd_ctl_elem_value *ucontrol)
+{
+	struct tasdevice_priv *tas_priv = snd_kcontrol_chip(kcontrol);
+	struct tasdevice_fw *tas_fw = tas_priv->fmw;
+	int nr_program = ucontrol->value.integer.value[0];
+	int max = tas_fw->nr_programs - 1;
+	int val, ret = 0;
+
+	val = clamp(nr_program, 0, max);
+
+	if (tas_priv->cur_prog != val) {
+		tas_priv->cur_prog = val;
+		ret = 1;
+	}
+
+	return ret;
+}
+
+static int tasdevice_config_get(struct snd_kcontrol *kcontrol,
+	struct snd_ctl_elem_value *ucontrol)
+{
+	struct tasdevice_priv *tas_priv = snd_kcontrol_chip(kcontrol);
+
+	ucontrol->value.integer.value[0] = tas_priv->cur_conf;
+
+	return 0;
+}
+
+static int tasdevice_config_put(struct snd_kcontrol *kcontrol,
+	struct snd_ctl_elem_value *ucontrol)
+{
+	struct tasdevice_priv *tas_priv = snd_kcontrol_chip(kcontrol);
+	struct tasdevice_fw *tas_fw = tas_priv->fmw;
+	int nr_config = ucontrol->value.integer.value[0];
+	int max = tas_fw->nr_configurations - 1;
+	int val, ret = 0;
+
+	val = clamp(nr_config, 0, max);
+
+	if (tas_priv->cur_conf != val) {
+		tas_priv->cur_conf = val;
+		ret = 1;
+	}
+
+	return ret;
+}
+
+/*
+ * tas2781_digital_getvol - get the volum control
+ * @kcontrol: control pointer
+ * @ucontrol: User data
+ * Customer Kcontrol for tas2781 is primarily for regmap booking, paging
+ * depends on internal regmap mechanism.
+ * tas2781 contains book and page two-level register map, especially
+ * book switching will set the register BXXP00R7F, after switching to the
+ * correct book, then leverage the mechanism for paging to access the
+ * register.
+ */
+static int tas2781_digital_getvol(struct snd_kcontrol *kcontrol,
+	struct snd_ctl_elem_value *ucontrol)
+{
+	struct tasdevice_priv *tas_priv = snd_kcontrol_chip(kcontrol);
+	struct soc_mixer_control *mc =
+		(struct soc_mixer_control *)kcontrol->private_value;
+
+	return tasdevice_spi_digital_getvol(tas_priv, ucontrol, mc);
+}
+
+static int tas2781_amp_getvol(struct snd_kcontrol *kcontrol,
+	struct snd_ctl_elem_value *ucontrol)
+{
+	struct tasdevice_priv *tas_priv = snd_kcontrol_chip(kcontrol);
+	struct soc_mixer_control *mc =
+		(struct soc_mixer_control *)kcontrol->private_value;
+
+	return tasdevice_spi_amp_getvol(tas_priv, ucontrol, mc);
+}
+
+static int tas2781_digital_putvol(struct snd_kcontrol *kcontrol,
+	struct snd_ctl_elem_value *ucontrol)
+{
+	struct tasdevice_priv *tas_priv = snd_kcontrol_chip(kcontrol);
+	struct soc_mixer_control *mc =
+		(struct soc_mixer_control *)kcontrol->private_value;
+
+	/* The check of the given value is in tasdevice_digital_putvol. */
+	return tasdevice_spi_digital_putvol(tas_priv, ucontrol, mc);
+}
+
+static int tas2781_amp_putvol(struct snd_kcontrol *kcontrol,
+	struct snd_ctl_elem_value *ucontrol)
+{
+	struct tasdevice_priv *tas_priv = snd_kcontrol_chip(kcontrol);
+	struct soc_mixer_control *mc =
+		(struct soc_mixer_control *)kcontrol->private_value;
+
+	/* The check of the given value is in tasdevice_amp_putvol. */
+	return tasdevice_spi_amp_putvol(tas_priv, ucontrol, mc);
+}
+
+static int tas2781_force_fwload_get(struct snd_kcontrol *kcontrol,
+	struct snd_ctl_elem_value *ucontrol)
+{
+	struct tasdevice_priv *tas_priv = snd_kcontrol_chip(kcontrol);
+
+	ucontrol->value.integer.value[0] = (int)tas_priv->force_fwload_status;
+	dev_dbg(tas_priv->dev, "%s : Force FWload %s\n", __func__,
+			tas_priv->force_fwload_status ? "ON" : "OFF");
+
+	return 0;
+}
+
+static int tas2781_force_fwload_put(struct snd_kcontrol *kcontrol,
+	struct snd_ctl_elem_value *ucontrol)
+{
+	struct tasdevice_priv *tas_priv = snd_kcontrol_chip(kcontrol);
+	bool change, val = (bool)ucontrol->value.integer.value[0];
+
+	if (tas_priv->force_fwload_status == val)
+		change = false;
+	else {
+		change = true;
+		tas_priv->force_fwload_status = val;
+	}
+	dev_dbg(tas_priv->dev, "%s : Force FWload %s\n", __func__,
+		tas_priv->force_fwload_status ? "ON" : "OFF");
+
+	return change;
+}
+
+static const struct snd_kcontrol_new tas2781_snd_controls[] = {
+	ACARD_SINGLE_RANGE_EXT_TLV("Speaker Analog Gain", TAS2781_AMP_LEVEL,
+		1, 0, 20, 0, tas2781_amp_getvol,
+		tas2781_amp_putvol, amp_vol_tlv),
+	ACARD_SINGLE_RANGE_EXT_TLV("Speaker Digital Gain", TAS2781_DVC_LVL,
+		0, 0, 200, 1, tas2781_digital_getvol,
+		tas2781_digital_putvol, dvc_tlv),
+	ACARD_SINGLE_BOOL_EXT("Speaker Force Firmware Load", 0,
+		tas2781_force_fwload_get, tas2781_force_fwload_put),
+};
+
+static const struct snd_kcontrol_new tas2781_prof_ctrl = {
+	.name = "Speaker Profile Id",
+	.iface = SNDRV_CTL_ELEM_IFACE_CARD,
+	.info = tasdevice_info_profile,
+	.get = tasdevice_get_profile_id,
+	.put = tasdevice_set_profile_id,
+};
+
+static const struct snd_kcontrol_new tas2781_dsp_prog_ctrl = {
+	.name = "Speaker Program Id",
+	.iface = SNDRV_CTL_ELEM_IFACE_CARD,
+	.info = tasdevice_info_programs,
+	.get = tasdevice_program_get,
+	.put = tasdevice_program_put,
+};
+
+static const struct snd_kcontrol_new tas2781_dsp_conf_ctrl = {
+	.name = "Speaker Config Id",
+	.iface = SNDRV_CTL_ELEM_IFACE_CARD,
+	.info = tasdevice_info_config,
+	.get = tasdevice_config_get,
+	.put = tasdevice_config_put,
+};
+
+static void tas2781_apply_calib(struct tasdevice_priv *tas_priv)
+{
+	static const unsigned char page_array[CALIB_MAX] = {
+		0x17, 0x18, 0x18, 0x13, 0x18
+	};
+	static const unsigned char rgno_array[CALIB_MAX] = {
+		0x74, 0x0c, 0x14, 0x70, 0x7c
+	};
+	unsigned char *data;
+	int j, rc;
+
+	data = tas_priv->cali_data.data +
+		tas_priv->index * TASDEVICE_SPEAKER_CALIBRATION_SIZE;
+	for (j = 0; j < CALIB_MAX; j++) {
+		rc = tasdevice_spi_dev_bulk_write(tas_priv,
+			TASDEVICE_REG(0, page_array[j], rgno_array[j]),
+			&(data[4 * j]), 4);
+		if (rc < 0)
+			dev_err(tas_priv->dev,
+				"chn %d calib %d bulk_wr err = %d\n",
+				tas_priv->index, j, rc);
+	}
+
+}
+
+/*
+ * Update the calibration data, including speaker impedance, f0, etc,
+ * into algo. Calibrate data is done by manufacturer in the factory.
+ * These data are used by Algo for calculating the speaker temperature,
+ * speaker membrane excursion and f0 in real time during playback.
+ * Calibration data format in EFI is V2, since 2024.
+ */
+static int tas2781_save_calibration(struct tasdevice_priv *tas_priv)
+{
+	efi_guid_t efi_guid = EFI_GUID(0x02f9af02, 0x7734, 0x4233, 0xb4, 0x3d,
+		0x93, 0xfe, 0x5a, 0xa3, 0x5d, 0xb3);
+	static efi_char16_t efi_name[] = L"CALI_DATA";
+	struct tm *tm = &tas_priv->tm;
+	unsigned int attr, crc;
+	unsigned int *tmp_val;
+	efi_status_t status;
+
+	tas_priv->cali_data.total_sz = 0;
+	/* Get real size of UEFI variable */
+	status = efi.get_variable(efi_name, &efi_guid, &attr,
+		&tas_priv->cali_data.total_sz, tas_priv->cali_data.data);
+	if (status == EFI_BUFFER_TOO_SMALL) {
+		/* Allocate data buffer of data_size bytes */
+		tas_priv->cali_data.data = devm_kzalloc(tas_priv->dev,
+			tas_priv->cali_data.total_sz, GFP_KERNEL);
+		if (!tas_priv->cali_data.data)
+			return -ENOMEM;
+		/* Get variable contents into buffer */
+		status = efi.get_variable(efi_name, &efi_guid, &attr,
+			&tas_priv->cali_data.total_sz,
+			tas_priv->cali_data.data);
+	}
+	if (status != EFI_SUCCESS)
+		return -EINVAL;
+
+	tmp_val = (unsigned int *)tas_priv->cali_data.data;
+	if (tmp_val[0] == 2781) {
+		/*
+		 * New features were added in calibrated Data V2:
+		 *     1. Added some the fields to store the link_id and
+		 *     uniqie_id for multi-link solutions
+		 *     2. Support flexible number of devices instead of
+		 *     fixed one in V1.
+		 * Layout of calibrated data V2 in UEFI(total 256 bytes):
+		 *     ChipID (2783, 4 bytes)
+		 *     Device-Sum (4 bytes)
+		 *     TimeStamp of Calibration (4 bytes)
+		 *     for (i = 0; i < Device-Sum; i++) {
+		 *             Device #i index_info () {
+		 *                     SDW link id (2bytes)
+		 *                     SDW unique_id (2bytes)
+		 *             }
+		 *             Calibrated Data of Device #i (20 bytes)
+		 *     }
+		 *     CRC (4 bytes)
+		 *     Reserved (the rest)
+		 */
+		crc = crc32(~0, tas_priv->cali_data.data,
+			3 + tmp_val[0] * 6) ^ ~0;
+		dev_dbg(tas_priv->dev, "cali crc 0x%08x PK tmp_val 0x%08x\n",
+			crc, tmp_val[3 + tmp_val[0] * 6]);
+
+		if (crc == tmp_val[3+tmp_val[0]*6]) {
+			time64_to_tm(tmp_val[2], 0, tm);
+			dev_dbg(tas_priv->dev, "%4ld-%2d-%2d, %2d:%2d:%2d\n",
+				tm->tm_year, tm->tm_mon, tm->tm_mday,
+				tm->tm_hour, tm->tm_min, tm->tm_sec);
+			tasdevice_apply_calibration(tas_priv);
+		} else {
+			tas_priv->cali_data.total_sz = 0;
+		}
+	} else {
+		/*
+		 * Calibration data is in V1 format.
+		 * struct cali_data {
+		 *      char cali_data[20];
+		 * }
+		 *
+		 * struct {
+		 *      struct cali_data cali_data[4];
+		 *      int  TimeStamp of Calibration (4 bytes)
+		 *      int CRC (4 bytes)
+		 * } ueft;
+		 */
+		crc = crc32(~0, tas_priv->cali_data.data, 84) ^ ~0;
+		dev_dbg(tas_priv->dev, "cali crc 0x%08x PK tmp_val 0x%08x\n",
+			crc, tmp_val[21]);
+
+		if (crc == tmp_val[21]) {
+			time64_to_tm(tmp_val[20], 0, tm);
+			dev_dbg(tas_priv->dev, "%4ld-%2d-%2d, %2d:%2d:%2d\n",
+				tm->tm_year, tm->tm_mon, tm->tm_mday,
+				tm->tm_hour, tm->tm_min, tm->tm_sec);
+			tasdevice_apply_calibration(tas_priv);
+		} else
+			tas_priv->cali_data.total_sz = 0;
+	}
+	return 0;
+}
+
+static void tas2781_hda_remove_controls(struct tas2781_hda *tas_hda)
+{
+	struct hda_codec *codec = tas_hda->priv->codec;
+
+	if (tas_hda->dsp_prog_ctl)
+		snd_ctl_remove(codec->card, tas_hda->dsp_prog_ctl);
+
+	if (tas_hda->dsp_conf_ctl)
+		snd_ctl_remove(codec->card, tas_hda->dsp_conf_ctl);
+
+	for (int i = ARRAY_SIZE(tas_hda->snd_ctls) - 1; i >= 0; i--)
+		if (tas_hda->snd_ctls[i])
+			snd_ctl_remove(codec->card, tas_hda->snd_ctls[i]);
+
+	if (tas_hda->prof_ctl)
+		snd_ctl_remove(codec->card, tas_hda->prof_ctl);
+}
+
+static void tasdev_fw_ready(const struct firmware *fmw, void *context)
+{
+	struct tasdevice_priv *tas_priv = context;
+	struct tas2781_hda *tas_hda = dev_get_drvdata(tas_priv->dev);
+	struct hda_codec *codec = tas_priv->codec;
+	int i, ret;
+
+	pm_runtime_get_sync(tas_priv->dev);
+	mutex_lock(&tas_priv->codec_lock);
+
+	ret = tasdevice_spi_rca_parser(tas_priv, fmw);
+	if (ret)
+		goto out;
+
+	/* Add control one time only. */
+	if (tas_priv->index == 0) {
+		tas_hda->prof_ctl = snd_ctl_new1(&tas2781_prof_ctrl, tas_priv);
+		ret = snd_ctl_add(codec->card, tas_hda->prof_ctl);
+		if (ret) {
+			dev_err(tas_priv->dev,
+				"Failed to add KControl %s = %d\n",
+				tas2781_prof_ctrl.name, ret);
+			goto out;
+		}
+
+		for (i = 0; i < ARRAY_SIZE(tas2781_snd_controls); i++) {
+			tas_hda->snd_ctls[i] =
+				snd_ctl_new1(&tas2781_snd_controls[i],
+				tas_priv);
+			ret = snd_ctl_add(codec->card, tas_hda->snd_ctls[i]);
+			if (ret) {
+				dev_err(tas_priv->dev,
+					"Failed to add KControl %s = %d\n",
+					tas2781_snd_controls[i].name, ret);
+				goto out;
+			}
+		}
+	}
+
+	tasdevice_spi_dsp_remove(tas_priv);
+
+	tas_priv->fw_state = TASDEVICE_DSP_FW_PENDING;
+	scnprintf(tas_priv->coef_binaryname, 64, "TAS2XXX%08X-%01d.bin",
+		codec->core.subsystem_id, tas_priv->index);
+	ret = tasdevice_spi_dsp_parser(tas_priv);
+	if (ret) {
+		dev_err(tas_priv->dev, "dspfw load %s error\n",
+			tas_priv->coef_binaryname);
+		tas_priv->fw_state = TASDEVICE_DSP_FW_FAIL;
+		goto out;
+	}
+
+	/* Add control one time only. */
+	if (tas_priv->index == 0) {
+		tas_hda->dsp_prog_ctl = snd_ctl_new1(&tas2781_dsp_prog_ctrl,
+			tas_priv);
+		ret = snd_ctl_add(codec->card, tas_hda->dsp_prog_ctl);
+		if (ret) {
+			dev_err(tas_priv->dev,
+				"Failed to add KControl %s = %d\n",
+				tas2781_dsp_prog_ctrl.name, ret);
+			goto out;
+		}
+
+		tas_hda->dsp_conf_ctl = snd_ctl_new1(&tas2781_dsp_conf_ctrl,
+			tas_priv);
+		ret = snd_ctl_add(codec->card, tas_hda->dsp_conf_ctl);
+		if (ret) {
+			dev_err(tas_priv->dev,
+				"Failed to add KControl %s = %d\n",
+				tas2781_dsp_conf_ctrl.name, ret);
+			goto out;
+		}
+	}
+
+	tas_priv->fw_state = TASDEVICE_DSP_FW_ALL_OK;
+	tasdevice_spi_prmg_load(tas_priv, 0);
+	if (tas_priv->fmw->nr_programs > 0)
+		tas_priv->cur_prog = 0;
+	if (tas_priv->fmw->nr_configurations > 0)
+		tas_priv->cur_conf = 0;
+
+	/*
+	 * If calibrated data occurs error, dsp will still works with default
+	 * calibrated data inside algo.
+	 */
+	tasdevice_save_calibration(tas_priv);
+
+	tasdevice_spi_tuning_switch(tas_hda->priv, 0);
+
+out:
+	mutex_unlock(&tas_hda->priv->codec_lock);
+	if (fmw)
+		release_firmware(fmw);
+	pm_runtime_mark_last_busy(tas_hda->dev);
+	pm_runtime_put_autosuspend(tas_hda->dev);
+}
+
+static int tas2781_hda_bind(struct device *dev, struct device *master,
+	void *master_data)
+{
+	struct tas2781_hda *tas_hda = dev_get_drvdata(dev);
+	struct hda_component *comps = master_data;
+	struct hda_codec *codec;
+	unsigned int subid;
+	int ret;
+
+	if (!comps || tas_hda->priv->index < 0 ||
+		tas_hda->priv->index >= HDA_MAX_COMPONENTS)
+		return -EINVAL;
+
+	comps = &comps[tas_hda->priv->index];
+	if (comps->dev)
+		return -EBUSY;
+
+	codec = comps->codec;
+	subid = codec->core.subsystem_id >> 16;
+
+	switch (subid) {
+	case 0x103C:
+		tas_hda->priv->catlog_id = HP;
+		break;
+	default:
+		tas_hda->priv->catlog_id = OTHERS;
+		break;
+	}
+
+	pm_runtime_get_sync(dev);
+
+	comps->dev = dev;
+
+	strscpy(comps->name, dev_name(dev), sizeof(comps->name));
+
+	ret = tascodec_spi_init(tas_hda->priv, codec, THIS_MODULE,
+		tasdev_fw_ready);
+	if (!ret)
+		comps->playback_hook = tas2781_hda_playback_hook;
+
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
+
+	return ret;
+}
+
+static void tas2781_hda_unbind(struct device *dev,
+	struct device *master, void *master_data)
+{
+	struct tas2781_hda *tas_hda = dev_get_drvdata(dev);
+	struct hda_component *comps = master_data;
+
+	comps = &comps[tas_hda->priv->index];
+
+	if (comps->dev == dev) {
+		comps->dev = NULL;
+		memset(comps->name, 0, sizeof(comps->name));
+		comps->playback_hook = NULL;
+	}
+
+	tas2781_hda_remove_controls(tas_hda);
+
+	tasdevice_spi_config_info_remove(tas_hda->priv);
+	tasdevice_spi_dsp_remove(tas_hda->priv);
+
+	tas_hda->priv->fw_state = TASDEVICE_DSP_FW_PENDING;
+}
+
+static const struct component_ops tas2781_hda_comp_ops = {
+	.bind = tas2781_hda_bind,
+	.unbind = tas2781_hda_unbind,
+};
+
+static void tas2781_hda_remove(struct device *dev)
+{
+	struct tas2781_hda *tas_hda = dev_get_drvdata(dev);
+
+	pm_runtime_get_sync(tas_hda->dev);
+	pm_runtime_disable(tas_hda->dev);
+
+	component_del(tas_hda->dev, &tas2781_hda_comp_ops);
+
+	pm_runtime_put_noidle(tas_hda->dev);
+
+	tasdevice_remove(tas_hda->priv);
+}
+
+static int tas2781_hda_spi_probe(struct spi_device *spi)
+{
+	struct tasdevice_priv *tas_priv;
+	struct tas2781_hda *tas_hda;
+	const char *device_name;
+	int ret;
+
+	tas_hda = devm_kzalloc(&spi->dev, sizeof(*tas_hda), GFP_KERNEL);
+	if (!tas_hda)
+		return -ENOMEM;
+
+	tas_hda->dev = &spi->dev;
+
+	tas_priv = devm_kzalloc(&spi->dev, sizeof(*tas_priv), GFP_KERNEL);
+	if (!tas_priv)
+		goto err;
+	tas_priv->dev = &spi->dev;
+	tas_priv->client = (void *)spi;
+	tas_hda->priv = tas_priv;
+
+	if (strstr(dev_name(&spi->dev), "TIAS2781")) {
+		device_name = "TIAS2781";
+		tas_priv->save_calibration = tas2781_save_calibration;
+		tas_priv->apply_calibration = tas2781_apply_calib;
+	} else
+		goto err;
+
+	tas_priv->irq_info.irq = spi->irq;
+	dev_set_drvdata(&spi->dev, tas_hda);
+	ret = tas2781_read_acpi(tas_hda, device_name,
+		spi_get_chipselect(spi, 0));
+	if (ret)
+		return dev_err_probe(tas_hda->dev, ret,
+			"Platform not supported\n");
+
+	tas_priv->regmap = devm_regmap_init_spi(spi, &tasdevice_regmap);
+	if (IS_ERR(tas_priv->regmap)) {
+		ret = PTR_ERR(tas_priv->regmap);
+		dev_err(tas_priv->dev, "Failed to allocate regmap: %d\n",
+			ret);
+		goto err;
+	}
+	ret = tasdevice_spi_init(tas_hda->priv);
+	if (ret)
+		goto err;
+
+	pm_runtime_set_autosuspend_delay(tas_hda->dev, 3000);
+	pm_runtime_use_autosuspend(tas_hda->dev);
+	pm_runtime_mark_last_busy(tas_hda->dev);
+	pm_runtime_set_active(tas_hda->dev);
+	pm_runtime_get_noresume(tas_hda->dev);
+	pm_runtime_enable(tas_hda->dev);
+
+	pm_runtime_put_autosuspend(tas_hda->dev);
+
+	tas2781_spi_reset(tas_priv);
+
+	ret = component_add(tas_hda->dev, &tas2781_hda_comp_ops);
+	if (ret) {
+		dev_err(tas_hda->dev, "Register component failed: %d\n", ret);
+		pm_runtime_disable(tas_hda->dev);
+	}
+
+err:
+	if (ret)
+		tas2781_hda_remove(&spi->dev);
+	return ret;
+}
+
+static void tas2781_hda_spi_remove(struct spi_device *spi)
+{
+	tas2781_hda_remove(&spi->dev);
+}
+
+static int tas2781_runtime_suspend(struct device *dev)
+{
+	struct tas2781_hda *tas_hda = dev_get_drvdata(dev);
+
+	dev_dbg(tas_hda->dev, "Runtime Suspend\n");
+
+	mutex_lock(&tas_hda->priv->codec_lock);
+
+	if (tas_hda->priv->playback_started) {
+		tasdevice_spi_tuning_switch(tas_hda->priv, 1);
+		tas_hda->priv->playback_started = false;
+	}
+
+	tas_hda->priv->tasdevice.cur_book = -1;
+	tas_hda->priv->tasdevice.cur_prog = -1;
+	tas_hda->priv->tasdevice.cur_conf = -1;
+
+	mutex_unlock(&tas_hda->priv->codec_lock);
+
+	return 0;
+}
+
+static int tas2781_runtime_resume(struct device *dev)
+{
+	struct tas2781_hda *tas_hda = dev_get_drvdata(dev);
+
+	dev_dbg(tas_hda->dev, "Runtime Resume\n");
+
+	mutex_lock(&tas_hda->priv->codec_lock);
+
+	tasdevice_spi_prmg_load(tas_hda->priv, tas_hda->priv->cur_prog);
+
+	/*
+	 * If calibrated data occurs error, dsp will still works with default
+	 * calibrated data inside algo.
+	 */
+	tasdevice_apply_calibration(tas_hda->priv);
+
+	mutex_unlock(&tas_hda->priv->codec_lock);
+
+	return 0;
+}
+
+static int tas2781_system_suspend(struct device *dev)
+{
+	struct tas2781_hda *tas_hda = dev_get_drvdata(dev);
+	int ret;
+
+	dev_dbg(tas_hda->priv->dev, "System Suspend\n");
+
+	ret = pm_runtime_force_suspend(dev);
+	if (ret)
+		return ret;
+
+	/* Shutdown chip before system suspend */
+	tasdevice_spi_tuning_switch(tas_hda->priv, 1);
+
+	/*
+	 * Reset GPIO may be shared, so cannot reset here.
+	 * However beyond this point, amps may be powered down.
+	 */
+	return 0;
+}
+
+static int tas2781_system_resume(struct device *dev)
+{
+	struct tas2781_hda *tas_hda = dev_get_drvdata(dev);
+	int ret;
+
+	ret = pm_runtime_force_resume(dev);
+	if (ret)
+		return ret;
+
+	mutex_lock(&tas_hda->priv->codec_lock);
+
+	tas_hda->priv->tasdevice.cur_book = -1;
+	tas_hda->priv->tasdevice.cur_prog = -1;
+	tas_hda->priv->tasdevice.cur_conf = -1;
+
+	tas2781_spi_reset(tas_hda->priv);
+	tasdevice_spi_prmg_load(tas_hda->priv, tas_hda->priv->cur_prog);
+
+	/*
+	 * If calibrated data occurs error, dsp will still work with default
+	 * calibrated data inside algo.
+	 */
+	tasdevice_apply_calibration(tas_hda->priv);
+	mutex_unlock(&tas_hda->priv->codec_lock);
+
+	return 0;
+}
+
+static const struct dev_pm_ops tas2781_hda_pm_ops = {
+	RUNTIME_PM_OPS(tas2781_runtime_suspend, tas2781_runtime_resume, NULL)
+	SYSTEM_SLEEP_PM_OPS(tas2781_system_suspend, tas2781_system_resume)
+};
+
+static const struct spi_device_id tas2781_hda_spi_id[] = {
+	{ "tas2781-hda", 0 },
+	{}
+};
+
+static const struct acpi_device_id tas2781_acpi_hda_match[] = {
+	{"TIAS2781", 0 },
+	{}
+};
+MODULE_DEVICE_TABLE(acpi, tas2781_acpi_hda_match);
+
+static struct spi_driver tas2781_hda_spi_driver = {
+	.driver = {
+		.name		= "tas2781-hda",
+		.acpi_match_table = tas2781_acpi_hda_match,
+		.pm		= &tas2781_hda_pm_ops,
+	},
+	.id_table	= tas2781_hda_spi_id,
+	.probe		= tas2781_hda_spi_probe,
+	.remove		= tas2781_hda_spi_remove,
+};
+module_spi_driver(tas2781_hda_spi_driver);
+
+MODULE_DESCRIPTION("TAS2781 HDA SPI Driver");
+MODULE_AUTHOR("Baojun, Xu, <baojun.xug@ti.com>");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(SND_SOC_TAS2781_FMWLIB);
-- 
2.40.1

Hi Baojun,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tiwai-sound/for-next]
[also build test WARNING on tiwai-sound/for-linus rafael-pm/linux-next linus/master v6.9-rc3 next-20240409]
[cannot apply to rafael-pm/acpi-bus rafael-pm/devprop]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Baojun-Xu/ALSA-hda-tas2781-Modification-for-add-tas2781-driver-for-SPI/20240409-105140
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git for-next
patch link:    https://lore.kernel.org/r/20240409024816.1180-3-baojun.xu%40ti.com
patch subject: [PATCH v2 2/3] ALSA: hda/tas2781: Main code of tas2781 driver for SPI
config: arm64-allyesconfig (https://download.01.org/0day-ci/archive/20240410/202404100015.FAgrEOeO-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 8b3b4a92adee40483c27f26c478a384cd69c6f05)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240410/202404100015.FAgrEOeO-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202404100015.FAgrEOeO-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from sound/pci/hda/tas2781_hda_spi.c:9:
   In file included from include/linux/acpi.h:39:
   In file included from include/acpi/acpi_io.h:7:
   In file included from arch/arm64/include/asm/acpi.h:14:
   In file included from include/linux/memblock.h:12:
   In file included from include/linux/mm.h:2208:
   include/linux/vmstat.h:508:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     508 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     509 |                            item];
         |                            ~~~~
   include/linux/vmstat.h:515:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     515 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     516 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/vmstat.h:522:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     522 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
   include/linux/vmstat.h:527:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     527 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     528 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/vmstat.h:536:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     536 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     537 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
>> sound/pci/hda/tas2781_hda_spi.c:1159:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
    1159 |         if (strstr(dev_name(&spi->dev), "TIAS2781")) {
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   sound/pci/hda/tas2781_hda_spi.c:1203:6: note: uninitialized use occurs here
    1203 |         if (ret)
         |             ^~~
   sound/pci/hda/tas2781_hda_spi.c:1159:2: note: remove the 'if' if its condition is always true
    1159 |         if (strstr(dev_name(&spi->dev), "TIAS2781")) {
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    1160 |                 device_name = "TIAS2781";
    1161 |                 tas_priv->save_calibration = tas2781_save_calibration;
    1162 |                 tas_priv->apply_calibration = tas2781_apply_calib;
    1163 |         } else
         |           ~~~~
    1164 |                 goto err;
         |                 ~~~~~~~~
>> sound/pci/hda/tas2781_hda_spi.c:1153:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
    1153 |         if (!tas_priv)
         |             ^~~~~~~~~
   sound/pci/hda/tas2781_hda_spi.c:1203:6: note: uninitialized use occurs here
    1203 |         if (ret)
         |             ^~~
   sound/pci/hda/tas2781_hda_spi.c:1153:2: note: remove the 'if' if its condition is always false
    1153 |         if (!tas_priv)
         |         ^~~~~~~~~~~~~~
    1154 |                 goto err;
         |                 ~~~~~~~~
   sound/pci/hda/tas2781_hda_spi.c:1144:9: note: initialize the variable 'ret' to silence this warning
    1144 |         int ret;
         |                ^
         |                 = 0
   7 warnings generated.


vim +1159 sound/pci/hda/tas2781_hda_spi.c

  1138	
  1139	static int tas2781_hda_spi_probe(struct spi_device *spi)
  1140	{
  1141		struct tasdevice_priv *tas_priv;
  1142		struct tas2781_hda *tas_hda;
  1143		const char *device_name;
  1144		int ret;
  1145	
  1146		tas_hda = devm_kzalloc(&spi->dev, sizeof(*tas_hda), GFP_KERNEL);
  1147		if (!tas_hda)
  1148			return -ENOMEM;
  1149	
  1150		tas_hda->dev = &spi->dev;
  1151	
  1152		tas_priv = devm_kzalloc(&spi->dev, sizeof(*tas_priv), GFP_KERNEL);
> 1153		if (!tas_priv)
  1154			goto err;
  1155		tas_priv->dev = &spi->dev;
  1156		tas_priv->client = (void *)spi;
  1157		tas_hda->priv = tas_priv;
  1158	
> 1159		if (strstr(dev_name(&spi->dev), "TIAS2781")) {
  1160			device_name = "TIAS2781";
  1161			tas_priv->save_calibration = tas2781_save_calibration;
  1162			tas_priv->apply_calibration = tas2781_apply_calib;
  1163		} else
  1164			goto err;
  1165	
  1166		tas_priv->irq_info.irq = spi->irq;
  1167		dev_set_drvdata(&spi->dev, tas_hda);
  1168		ret = tas2781_read_acpi(tas_hda, device_name,
  1169			spi_get_chipselect(spi, 0));
  1170		if (ret)
  1171			return dev_err_probe(tas_hda->dev, ret,
  1172				"Platform not supported\n");
  1173	
  1174		tas_priv->regmap = devm_regmap_init_spi(spi, &tasdevice_regmap);
  1175		if (IS_ERR(tas_priv->regmap)) {
  1176			ret = PTR_ERR(tas_priv->regmap);
  1177			dev_err(tas_priv->dev, "Failed to allocate regmap: %d\n",
  1178				ret);
  1179			goto err;
  1180		}
  1181		ret = tasdevice_spi_init(tas_hda->priv);
  1182		if (ret)
  1183			goto err;
  1184	
  1185		pm_runtime_set_autosuspend_delay(tas_hda->dev, 3000);
  1186		pm_runtime_use_autosuspend(tas_hda->dev);
  1187		pm_runtime_mark_last_busy(tas_hda->dev);
  1188		pm_runtime_set_active(tas_hda->dev);
  1189		pm_runtime_get_noresume(tas_hda->dev);
  1190		pm_runtime_enable(tas_hda->dev);
  1191	
  1192		pm_runtime_put_autosuspend(tas_hda->dev);
  1193	
  1194		tas2781_spi_reset(tas_priv);
  1195	
  1196		ret = component_add(tas_hda->dev, &tas2781_hda_comp_ops);
  1197		if (ret) {
  1198			dev_err(tas_hda->dev, "Register component failed: %d\n", ret);
  1199			pm_runtime_disable(tas_hda->dev);
  1200		}
  1201	
  1202	err:
  1203		if (ret)
  1204			tas2781_hda_remove(&spi->dev);
  1205		return ret;
  1206	}
  1207	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

On Tue, Apr 09, 2024 at 10:48:14AM +0800, Baojun Xu wrote:
> Main source code for tas2781 driver for SPI.

...

> +#ifndef __TAS2781_SPI_H__
> +#define __TAS2781_SPI_H__

+ bits.h
+ mutex.h
+ time.h? (for struct tm)
+ types.h

struct calidata is from?..

> +#include <sound/tas2781-dsp.h>

Not sure how this is being used.

Also some forward declarations:

+ struct device;
+ struct firmware;
+ struct gpio_desc;
+ struct regmap;

(I might missed something)

...

> +#define TASDEVICE_RATES			(SNDRV_PCM_RATE_44100 |\
> +	SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_96000 |\
> +	SNDRV_PCM_RATE_88200)

For lines likes this, the formatting can be

#define TASDEVICE_RATES							      \
	(SNDRV_PCM_RATE_44100 | SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_96000 | \
	 SNDRV_PCM_RATE_88200)

which from my p.o.v. looks better.

...

> +#define TAS2781_SPI_MAX_FREQ		4000000

4 * HZZ_PER_MHZ ?

...

> +enum device_catlog_id {

Too generic name.

> +	HP = 0,
> +	OTHERS

Ditto. Please, add namespace.

> +};

...

> +#include <linux/acpi.h>
> +#include <linux/crc8.h>
> +#include <linux/crc32.h>
> +#include <linux/efi.h>
> +#include <linux/firmware.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/pm_runtime.h>

+ property.h

> +#include <linux/regmap.h>
> +#include <linux/spi/spi.h>
> +#include <sound/hda_codec.h>
> +#include <sound/soc.h>
> +#include <sound/tlv.h>
> +#include <sound/tas2781-tlv.h>

...

> +/* No standard control callbacks for SNDRV_CTL_ELEM_IFACE_CARD
> + * Define two controls, one is Volume control callbacks, the other is
> + * flag setting control callbacks.
> + */

/*
 * Follow the example of the multi-line
 * comments style as in this comment. Apply
 * it to all your comments.
 */

...

> +	.private_value = (unsigned long)&(struct soc_mixer_control) \
> +		{.reg = xreg, .rreg = xreg, .shift = xshift, \
> +		 .rshift = xshift, .min = xmin, .max = xmax, \
> +		 .invert = xinvert} }

It is unreadable.

	.private_value = (unsigned long)&(struct soc_mixer_control) {	      \
		.reg = xreg, .rreg = xreg, .shift = xshift, .rshift = xshift, \
		.min = xmin, .max = xmax, .invert = xinvert,		      \
	},								      \


See the difference?

Please, apply this to all twisted macros:
- logical split
- leaving trailing commas
- better formatting

...

> +		.range_max = 256 * 128,

Perhaps you want to define this as it's used a lot in the C and header files.

...

> +static const struct regmap_config tasdevice_regmap = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.cache_type = REGCACHE_NONE,
> +	.ranges = tasdevice_ranges,
> +	.num_ranges = ARRAY_SIZE(tasdevice_ranges),
> +	.max_register = 256 * 128,
> +};
> +
> +

One too many blank lines.

> +static void tas2781_spi_reset(struct tasdevice_priv *tas_dev);
> +static int tasdevice_spi_init(struct tasdevice_priv *tas_priv);
> +static int tasdevice_spi_amp_putvol(struct tasdevice_priv *tas_priv,
> +	struct snd_ctl_elem_value *ucontrol, struct soc_mixer_control *mc);
> +static int tasdevice_spi_amp_getvol(struct tasdevice_priv *tas_priv,
> +	struct snd_ctl_elem_value *ucontrol, struct soc_mixer_control *mc);
> +static int tasdevice_spi_digital_putvol(struct tasdevice_priv *tas_priv,
> +	struct snd_ctl_elem_value *ucontrol, struct soc_mixer_control *mc);
> +static int tasdevice_spi_digital_getvol(struct tasdevice_priv *tas_priv,
> +	struct snd_ctl_elem_value *ucontrol, struct soc_mixer_control *mc);

Why do you need forward declarations here?

...

> +static int tasdevice_spi_switch_book(struct tasdevice_priv *tas_priv,
> +	int book)

Can be on a single line.

> +{
> +	struct tasdevice *tasdev = &tas_priv->tasdevice;
> +	struct regmap *map = tas_priv->regmap;
> +	int ret = 0;
> +
> +	if (tasdev->cur_book != book) {
> +		/* Change to page 0 before book change. */
> +		ret = regmap_write(map, TASDEVICE_PAGE_SELECT, 0);
> +		if (ret < 0) {
> +			dev_err(tas_priv->dev, "%s, E=%d\n", __func__, ret);
> +			return ret;
> +		}
> +		ret = regmap_write(map, TASDEVICE_BOOKCTL_REG, book);
> +		if (ret < 0)
> +			dev_err(tas_priv->dev, "%s, E=%d\n", __func__, ret);

Non-fatal error?

> +		tasdev->cur_book = book;
> +	}
> +
> +	return ret;
> +}

...

> +int tasdevice_spi_dev_read(struct tasdevice_priv *tas_priv,
> +	unsigned int reg, unsigned int *val)
> +{
> +	struct regmap *map = tas_priv->regmap;
> +	int ret;
> +
> +	ret = tasdevice_spi_switch_book(tas_priv, TASDEVICE_BOOK_ID(reg));
> +	if (ret < 0)
> +		return ret;
> +
> +	/*
> +	 * In our TAS2781 SPI mode, if read from other book (not book 0),
> +	 * or read from page number larger than 1 in book 0, one byte more
> +	 * read is needed, and first byte is a dummy byte, need to be ignored.
> +	 */
> +	if ((TASDEVICE_BOOK_ID(reg) > 0) || (TASDEVICE_PAGE_ID(reg) > 1)) {
> +		unsigned char data[2];
> +
> +		ret = regmap_bulk_read(map, TASDEVICE_PGRG(reg), data, 2);

sizeof(data) ?

> +		*val = data[1];
> +	} else {
> +		ret = regmap_read(map, TASDEVICE_PGRG(reg), val);
> +	}
> +	if (ret < 0)
> +		dev_err(tas_priv->dev, "%s, E=%d\n", __func__, ret);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(tasdevice_spi_dev_read);

No namespace, why?

...

> +	return ret;
> +
> +}

Here and in other places, one too many blank lines.

...

> +	static const unsigned char page_array[CALIB_MAX] = {
> +		0x17, 0x18, 0x18, 0x13, 0x18

Leave trailing comma, can be helpful in the future.

> +	};
> +	static const unsigned char rgno_array[CALIB_MAX] = {
> +		0x74, 0x0c, 0x14, 0x70, 0x7c

Ditto.

> +	};


...

> +	data = tas_priv->cali_data.data +
> +		tas_priv->index * TASDEVICE_SPEAKER_CALIBRATION_SIZE;
> +	for (j = 0; j < CALIB_MAX; j++) {
> +		rc = tasdevice_spi_dev_bulk_write(tas_priv,
> +			TASDEVICE_REG(0, page_array[j], rgno_array[j]),
> +			&(data[4 * j]), 4);
> +		if (rc < 0)
> +			dev_err(tas_priv->dev,
> +				"chn %d calib %d bulk_wr err = %d\n",
> +				tas_priv->index, j, rc);

The indentation here and in some other places is a mess. Please,
take your time to split these in more readable way.

> +	}
> +

Again, too many unneeded blank lines in the code.

> +}

...

I stopped here as there are already enough for next version.

-- 
With Best Regards,
Andy Shevchenko

Hi Andy,

Thanks for your valuable suggestions.

> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Sent: 09 April 2024 21:21
> To: Xu, Baojun
> Cc: tiwai@suse.de; robh+dt@kernel.org; lgirdwood@gmail.com; perex@perex.cz; pierre-louis.bossart@linux.intel.com; Lu, Kevin; Ding, Shenghao; Navada Kanyana, Mukund; alsa-devel@alsa-project.org; linux-kernel@vger.kernel.org; liam.r.girdwood@intel.com; yung-chuan.liao@linux.intel.com; broonie@kernel.org; soyer@irl.hu
> Subject: [EXTERNAL] Re: [PATCH v2 2/3] ALSA: hda/tas2781: Main code of tas2781 driver for SPI
> 
> On Tue, Apr 09, 2024 at 10: 48: 14AM +0800, Baojun Xu wrote: > Main source code for tas2781 driver for SPI. .. . > +#ifndef __TAS2781_SPI_H__ > +#define __TAS2781_SPI_H__ + bits. h + mutex. h + time. h? (for struct tm) + types. h struct calidata
> ZjQcmQRYFpfptBannerStart
> This message was sent from outside of Texas Instruments.
> Do not click links or open attachments unless you recognize the source of this email and know the content is safe. If you wish to report this message to IT Security, please forward the message as an attachment to phishing@list.ti.com
> 
> ZjQcmQRYFpfptBannerEnd
> 
> On Tue, Apr 09, 2024 at 10:48:14AM +0800, Baojun Xu wrote:
> > Main source code for tas2781 driver for SPI.
> 
> ...
> 
> > +#ifndef __TAS2781_SPI_H__
> > +#define __TAS2781_SPI_H__
> 
> + bits.h
> + mutex.h
> + time.h? (for struct tm)
> + types.h
> 
> struct calidata is from?..
> 
> > +#include <sound/tas2781-dsp.h>
> 
> Not sure how this is being used.

Was used for firmware binary file parser, all of file format information
was defined in this header file. It can be shared between SPI & I2C. 
> 
> Also some forward declarations:
> 
> + struct device;
> + struct firmware;
> + struct gpio_desc;
> + struct regmap;
> 
> ...
> 
> > +#define TAS2781_SPI_MAX_FREQ         4000000
> 
> 4 * HZZ_PER_MHZ ?
> 
Not found it, will use HZ_PER_MHZ in next patch.
> ...
> 
> --
> With Best Regards,
> Andy Shevchenko
> 
> 
> 
> 

On Thu, Apr 18, 2024 at 05:12:03AM +0000, Xu, Baojun wrote:
> > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Sent: 09 April 2024 21:21
> > To: Xu, Baojun
> > On Tue, Apr 09, 2024 at 10: 48: 14AM +0800, Baojun Xu wrote:
> > On Tue, Apr 09, 2024 at 10:48:14AM +0800, Baojun Xu wrote:

...

> > > +#ifndef __TAS2781_SPI_H__
> > > +#define __TAS2781_SPI_H__
> > 
> > + bits.h
> > + mutex.h
> > + time.h? (for struct tm)
> > + types.h
> > 
> > struct calidata is from?..
> > 
> > > +#include <sound/tas2781-dsp.h>
> > 
> > Not sure how this is being used.
> 
> Was used for firmware binary file parser, all of file format information
> was defined in this header file. It can be shared between SPI & I2C.

_This header_ file seems not using anything from it, does it?

> > Also some forward declarations:
> > 
> > + struct device;
> > + struct firmware;
> > + struct gpio_desc;
> > + struct regmap;

-- 
With Best Regards,
Andy Shevchenko