[PATCH 0/2] ALSA: hda/tas2563: Add tas253 HDA driver

[PATCH 0/2] ALSA: hda/tas2563: Add tas253 HDA driver

[Gergo Koteles]

The ta2563 is a smart amplifier. Similar to tas2562 but with DSP. Some 
Lenovo laptops have it to drive the bass speakers. By default, it is in 
software shutdown state.

To make the DSP work it needs a firmware and some calibration data.
The latter can be read from the EFI in Lenovo laptops.

For the correct configuration it needs additional register data.
It captured after running the Windows driver.

The firmware can be extracted as TAS2563Firmware.bin from the Windows 
driver with innoextract.
https://download.lenovo.com/consumer/mobiles/h5yd037fbfyy7kd0.exe

The driver will search for it as TAS2563-17AA3870.bin with the 14ARB7.

It uses the default program/configuration, and has no controls for these yet.

The amplifier works without firmware, but I don't know how safe is it, 
that's why the firmware is required.

Gergo Koteles (2):
  ASoc: tas2563: DSP Firmware loading support
  ALSA: hda/tas2563: Add tas2563 HDA driver

 {sound/soc/codecs => include/sound}/tas2562.h |   8 +
 include/sound/tas25xx-dsp.h                   | 100 ++++
 sound/pci/hda/Kconfig                         |  14 +
 sound/pci/hda/Makefile                        |   2 +
 sound/pci/hda/patch_realtek.c                 |  22 +-
 sound/pci/hda/tas2563_hda_i2c.c               | 508 ++++++++++++++++++
 sound/soc/codecs/Kconfig                      |   7 +
 sound/soc/codecs/Makefile                     |   2 +
 sound/soc/codecs/tas2562.c                    |   2 +-
 sound/soc/codecs/tas25xx-dsp.c                | 282 ++++++++++
 10 files changed, 942 insertions(+), 5 deletions(-)
 rename {sound/soc/codecs => include/sound}/tas2562.h (90%)
 create mode 100644 include/sound/tas25xx-dsp.h
 create mode 100644 sound/pci/hda/tas2563_hda_i2c.c
 create mode 100644 sound/soc/codecs/tas25xx-dsp.c


base-commit: ffc253263a1375a65fa6c9f62a893e9767fbebfa
-- 
2.43.0

[PATCH 1/2] ASoc: tas2563: DSP Firmware loading support

[Gergo Koteles]

The TAS2563 device has a DSP that can run firmware.
The firmware can contain up to 5 programs and 10 configurations to
support alternate audio processing.

Firmware loading is done dymacally and the program and configuration
selection is done by the user.

The binary itself contains a list of instructions for either a single
mode write or a burst write.  The single mode write is list of register
writes to different books and pages within the register map.
The burst writes is a block of data that is written to a specific
location in memory.

The firmware loader must parse and load the blocks in real time as the
binary may contain different audio profiles.

If the DSP is not needed to do audio preocessing then the DSP program
can be turned off and the device will effectively turn off the DSP.

RFC patch:
https://lore.kernel.org/lkml/6d6aaed3-dac8-e1ec-436c-9b04273df2b3@ti.com/T/

Co-developed-by: Dan Murphy <dmurphy@ti.com>
Signed-off-by: Dan Murphy <dmurphy@ti.com>
Signed-off-by: Gergo Koteles <soyer@irl.hu>
---
 {sound/soc/codecs => include/sound}/tas2562.h |   3 +
 include/sound/tas25xx-dsp.h                   | 100 +++++++
 sound/soc/codecs/Kconfig                      |   7 +
 sound/soc/codecs/Makefile                     |   2 +
 sound/soc/codecs/tas2562.c                    |   2 +-
 sound/soc/codecs/tas25xx-dsp.c                | 282 ++++++++++++++++++
 6 files changed, 395 insertions(+), 1 deletion(-)
 rename {sound/soc/codecs => include/sound}/tas2562.h (97%)
 create mode 100644 include/sound/tas25xx-dsp.h
 create mode 100644 sound/soc/codecs/tas25xx-dsp.c

diff --git a/sound/soc/codecs/tas2562.h b/include/sound/tas2562.h
similarity index 97%
rename from sound/soc/codecs/tas2562.h
rename to include/sound/tas2562.h
index 55b2a1f52ca3..f000e8f5dc88 100644
--- a/sound/soc/codecs/tas2562.h
+++ b/include/sound/tas2562.h
@@ -11,6 +11,7 @@
 #define __TAS2562_H__
 
 #define TAS2562_PAGE_CTRL	0x00
+#define TAS2562_BOOK_CTRL	0x7f
 
 #define TAS2562_REG(page, reg)	((page * 128) + reg)
 
@@ -44,6 +45,8 @@
 #define TAS2562_DVC_CFG3	TAS2562_REG(2, 0x0e)
 #define TAS2562_DVC_CFG4	TAS2562_REG(2, 0x0f)
 
+#define TAS25XX_DSP_MODE	TAS2562_REG(1, 0x02)
+
 #define TAS2562_RESET	BIT(0)
 
 #define TAS2562_MODE_MASK	GENMASK(1,0)
diff --git a/include/sound/tas25xx-dsp.h b/include/sound/tas25xx-dsp.h
new file mode 100644
index 000000000000..855e62bcf816
--- /dev/null
+++ b/include/sound/tas25xx-dsp.h
@@ -0,0 +1,100 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * tas25xx_dsp_loader.h - Texas Instruments TAS25xx Mono Audio Amplifier
+ *
+ * Copyright (C) 2020 Texas Instruments Incorporated -  http://www.ti.com
+ *
+ * Author: Dan Murphy <dmurphy@ti.com>
+ */
+
+#ifndef _TAS25XX_DSP_LOADER_H
+#define _TAS25XX_DSP_LOADER_H
+
+#define TAS25XX_NAME_SIZE	64
+#define TAS25XX_PROG_SIZE	5
+#define TAS25XX_CONFIG_SIZE	10
+
+#define TAS25XX_CMD_SING_W	0x1
+#define TAS25XX_CMD_BURST	0x2
+#define TAS25XX_CMD_DELAY	0x3
+
+struct tas25xx_cmd_reg {
+	u8 book;
+	u8 page;
+	u8 offset;
+	u8 data;
+};
+
+struct tas25xx_cmd_hdr {
+	u16 cmd_type;
+	u16 length;
+};
+
+struct tas25xx_cmd {
+	struct tas25xx_cmd_hdr hdr;
+	struct tas25xx_cmd_reg reg;
+};
+
+struct tas25xx_block_data {
+	u32 block_type;
+	u16 pram_checksum;
+	u16 yram_checksum;
+	u32 block_size;
+	u32 num_subblocks;
+};
+
+struct tas25xx_program_info {
+	char name[TAS25XX_NAME_SIZE];
+	u8 app_mode;
+	u8 pdm_i2s_mode;
+	u8 isns_pd;
+	u8 vsns_pd;
+	u8 reserved_1;
+	u16 reserved_2;
+	u8 ldg_power_up;
+	struct tas25xx_block_data blk_data;
+};
+
+struct tas25xx_config_info {
+	char name[TAS25XX_NAME_SIZE];
+	u16 devices;
+	u16 program;
+	u32 samp_rate;
+	u16 clk_src;
+	u16 sbclk_fs_ratio;
+	u32 clk_freq;
+	u32 num_blocks;
+	struct tas25xx_block_data blk_data;
+};
+
+struct tas25xx_fw_hdr {
+	u32 magic_num;
+	u32 image_size;
+	u32 checksum;
+	u32 version_num;
+	u32 dsp_version;
+	u32 drv_fw_version;
+	u32 timestamp;
+	char firmware_name[TAS25XX_NAME_SIZE];
+	u16 dev_family;
+	u16 dev_subfamily;
+	u32 num_programs;
+	u32 prog_size[TAS25XX_PROG_SIZE];
+	u32 num_configs;
+	u32 config_size[TAS25XX_CONFIG_SIZE];
+};
+
+struct tas25xx_fw_data {
+	struct tas25xx_fw_hdr *hdr;
+	u8 *config_data;
+	u8 *prog_data;
+};
+
+struct tas25xx_fw_data *tas25xx_parse_fw(struct device *dev,
+	const struct firmware *fw);
+int tas25xx_write_config(struct device *dev, struct regmap *regmap,
+	struct tas25xx_fw_data *fw_data, int config_num);
+int tas25xx_write_program(struct device *dev, struct regmap *regmap,
+	struct tas25xx_fw_data *fw_data, int prog_num);
+
+#endif /* _TAS25XX_DSP_LOADER_H */
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index f1e1dbc509f6..86e66d7e4bdc 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -238,6 +238,7 @@ config SND_SOC_ALL_CODECS
 	imply SND_SOC_STI_SAS
 	imply SND_SOC_TAS2552
 	imply SND_SOC_TAS2562
+	imply SND_SOC_TAS2562_DSP
 	imply SND_SOC_TAS2764
 	imply SND_SOC_TAS2770
 	imply SND_SOC_TAS2780
@@ -1761,6 +1762,12 @@ config SND_SOC_TAS2552
 	tristate "Texas Instruments TAS2552 Mono Audio amplifier"
 	depends on I2C
 
+config SND_SOC_TAS25XX_DSP
+	depends on I2C
+	select REGMAP_I2C
+	tristate
+	default n
+
 config SND_SOC_TAS2562
 	tristate "Texas Instruments TAS2562 Mono Audio amplifier"
 	depends on I2C
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index a87e56938ce5..807fcbb43cf5 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -378,6 +378,7 @@ snd-soc-max98504-objs := max98504.o
 snd-soc-simple-amplifier-objs := simple-amplifier.o
 snd-soc-tpa6130a2-objs := tpa6130a2.o
 snd-soc-tas2552-objs := tas2552.o
+snd-soc-tas25xx-dsp-objs := tas25xx-dsp.o
 snd-soc-tas2562-objs := tas2562.o
 snd-soc-tas2764-objs := tas2764.o
 snd-soc-tas2780-objs := tas2780.o
@@ -651,6 +652,7 @@ obj-$(CONFIG_SND_SOC_STAC9766)	+= snd-soc-stac9766.o
 obj-$(CONFIG_SND_SOC_STI_SAS)	+= snd-soc-sti-sas.o
 obj-$(CONFIG_SND_SOC_TAS2552)	+= snd-soc-tas2552.o
 obj-$(CONFIG_SND_SOC_TAS2562)	+= snd-soc-tas2562.o
+obj-$(CONFIG_SND_SOC_TAS25XX_DSP)	+= snd-soc-tas25xx-dsp.o
 obj-$(CONFIG_SND_SOC_TAS2764)	+= snd-soc-tas2764.o
 obj-$(CONFIG_SND_SOC_TAS2780)	+= snd-soc-tas2780.o
 obj-$(CONFIG_SND_SOC_TAS2781_COMLIB)	+= snd-soc-tas2781-comlib.o
diff --git a/sound/soc/codecs/tas2562.c b/sound/soc/codecs/tas2562.c
index 962c2cdfa017..ca6e9d60b255 100644
--- a/sound/soc/codecs/tas2562.c
+++ b/sound/soc/codecs/tas2562.c
@@ -20,7 +20,7 @@
 #include <sound/soc-dapm.h>
 #include <sound/tlv.h>
 
-#include "tas2562.h"
+#include <sound/tas2562.h>
 
 #define TAS2562_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE |\
 			 SNDRV_PCM_FORMAT_S32_LE)
diff --git a/sound/soc/codecs/tas25xx-dsp.c b/sound/soc/codecs/tas25xx-dsp.c
new file mode 100644
index 000000000000..d5081fa01441
--- /dev/null
+++ b/sound/soc/codecs/tas25xx-dsp.c
@@ -0,0 +1,282 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// Firmware loader for the Texas Instruments TAS25XX DSP
+// Copyright (C) 2020 Texas Instruments Inc.
+
+#include <linux/module.h>
+#include <linux/errno.h>
+#include <linux/device.h>
+#include <linux/firmware.h>
+#include <linux/i2c.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+
+#include <sound/tas2562.h>
+#include <sound/tas25xx-dsp.h>
+
+
+static void tas25xx_process_fw_delay(struct tas25xx_cmd *cmd)
+{
+	mdelay(cpu_to_be16(cmd->hdr.length));
+}
+
+static int tas25xx_process_fw_single(struct regmap *regmap,
+	struct tas25xx_cmd *cmd)
+{
+	int ret;
+	int num_writes = cpu_to_be16(cmd->hdr.length);
+	struct tas25xx_cmd_reg *reg = &cmd->reg;
+
+	for (int i = 0; i < num_writes; i++, reg++) {
+		/* Reset Page to 0 to access BOOK_CTRL */
+		ret = regmap_write(regmap, TAS2562_PAGE_CTRL, 0);
+		if (ret)
+			return ret;
+
+		ret = regmap_write(regmap, TAS2562_BOOK_CTRL, reg->book);
+		if (ret)
+			return ret;
+
+		ret = regmap_write(regmap, TAS2562_REG(reg->page, reg->offset),
+			reg->data);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int tas25xx_process_fw_burst(struct regmap *regmap,
+	struct tas25xx_cmd *cmd)
+{
+	int ret;
+
+	/* Reset Page to 0 to access BOOK_CTRL */
+	ret = regmap_write(regmap, TAS2562_PAGE_CTRL, 0);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(regmap, TAS2562_BOOK_CTRL, cmd->reg.book);
+	if (ret)
+		return ret;
+
+	ret = regmap_bulk_write(regmap, TAS2562_REG(cmd->reg.page, cmd->reg.offset), &cmd[1],
+				cpu_to_be16(cmd->hdr.length));
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int tas25xx_process_block(struct device *dev, struct regmap *regmap,
+	struct tas25xx_cmd *cmd, int max_block_size)
+{
+	int ret;
+	int block_read;
+
+	const int hdr_size = sizeof(struct tas25xx_cmd_hdr);
+	const int reg_size = sizeof(struct tas25xx_cmd_reg);
+	const int cmd_size = sizeof(struct tas25xx_cmd);
+
+	switch (cpu_to_be16(cmd->hdr.cmd_type)) {
+	case TAS25XX_CMD_SING_W:
+		block_read = cpu_to_be16(cmd->hdr.length) * reg_size + hdr_size;
+		break;
+	case TAS25XX_CMD_BURST:
+		block_read = cpu_to_be16(cmd->hdr.length) + cmd_size;
+		break;
+	case TAS25XX_CMD_DELAY:
+		block_read = 4;
+		break;
+	default:
+		block_read = 0;
+	}
+
+	if (block_read > max_block_size) {
+		dev_err(dev,
+			"Corrupt firmware: block_read > max_block_size %d %d\n",
+			block_read, max_block_size);
+		return -EINVAL;
+	}
+
+	switch (cpu_to_be16(cmd->hdr.cmd_type)) {
+	case TAS25XX_CMD_SING_W:
+		ret = tas25xx_process_fw_single(regmap, cmd);
+		if (ret) {
+			dev_err(dev, "Failed to process single write %d\n",
+				ret);
+			return ret;
+		}
+		break;
+	case TAS25XX_CMD_BURST:
+		ret = tas25xx_process_fw_burst(regmap, cmd);
+		if (ret) {
+			dev_err(dev, "Failed to process burst write %d\n", ret);
+			return ret;
+		}
+		break;
+	case TAS25XX_CMD_DELAY:
+		tas25xx_process_fw_delay(cmd);
+		break;
+	default:
+		dev_warn(dev, "Unknown cmd type %d\n",
+			cpu_to_be16(cmd->hdr.cmd_type));
+		break;
+	};
+
+	return block_read;
+}
+
+
+int tas25xx_write_program(struct device *dev, struct regmap *regmap,
+	struct tas25xx_fw_data *fw_data, int prog_num)
+{
+	int offset = 0;
+	int length = 0;
+	struct tas25xx_program_info *prog_info;
+	struct tas25xx_fw_hdr *hdr = fw_data->hdr;
+	struct tas25xx_cmd *cmd;
+
+	if (prog_num > cpu_to_be32(hdr->num_programs))
+		return -EINVAL;
+
+	for (int i = 0; i < prog_num; i++)
+		offset += cpu_to_be32(hdr->prog_size[i]);
+
+	prog_info = (struct tas25xx_program_info *)&fw_data->prog_data[offset];
+	dev_info(dev, "Write program %d: %s\n", prog_num, prog_info->name);
+
+	int max_offset = offset + cpu_to_be32(hdr->prog_size[prog_num]);
+	int num_subblocks = cpu_to_be32(prog_info->blk_data.num_subblocks);
+
+	offset += sizeof(struct tas25xx_program_info);
+
+	for (int i = 0; i < num_subblocks; i++) {
+		cmd = (struct tas25xx_cmd *)&fw_data->prog_data[offset];
+		length = tas25xx_process_block(dev, regmap, cmd,
+			max_offset - offset);
+		if (length < 0)
+			return length;
+
+		offset += length;
+	}
+
+	/* Reset Book to 0 */
+	regmap_write(regmap, TAS2562_PAGE_CTRL, 0);
+	regmap_write(regmap, TAS2562_BOOK_CTRL, 0);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(tas25xx_write_program);
+
+int tas25xx_write_config(struct device *dev, struct regmap *regmap,
+	struct tas25xx_fw_data *fw_data, int config_num)
+{
+	int offset = 0;
+	int length = 0;
+	struct tas25xx_config_info *cfg_info;
+	struct tas25xx_fw_hdr *hdr = fw_data->hdr;
+	struct tas25xx_cmd *cmd;
+
+	if (config_num > cpu_to_be32(hdr->num_configs))
+		return -EINVAL;
+
+	for (int i = 0; i < config_num; i++)
+		offset += cpu_to_be32(hdr->config_size[i]);
+
+	cfg_info = (struct tas25xx_config_info *)&fw_data->config_data[offset];
+	dev_info(dev, "Write config %d: %s\n", config_num, cfg_info->name);
+
+	int max_offset = offset + cpu_to_be32(hdr->config_size[config_num]);
+	int num_subblocks = cpu_to_be32(cfg_info->blk_data.num_subblocks);
+
+	offset += sizeof(struct tas25xx_config_info);
+
+	for (int i = 0; i < num_subblocks; i++) {
+		cmd = (struct tas25xx_cmd *)&fw_data->config_data[offset];
+		length = tas25xx_process_block(dev, regmap, cmd,
+			max_offset - offset);
+		if (length < 0)
+			return length;
+
+		offset += length;
+	}
+
+	/* Reset Book to 0 */
+	regmap_write(regmap, TAS2562_PAGE_CTRL, 0);
+	regmap_write(regmap, TAS2562_BOOK_CTRL, 0);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(tas25xx_write_config);
+
+
+struct tas25xx_fw_data *tas25xx_parse_fw(struct device *dev,
+	const struct firmware *fw)
+{
+	u32 total_prog_sz = 0;
+	u32 total_config_sz = 0;
+	u32 prog_num = 0;
+	u32 config_num = 0;
+	int hdr_size = sizeof(struct tas25xx_fw_hdr);
+	struct tas25xx_fw_data *fw_data = NULL;
+
+	fw_data = devm_kzalloc(dev, sizeof(struct tas25xx_fw_data), GFP_KERNEL);
+	if (!fw_data)
+		goto err_fw;
+
+	if (fw->size < hdr_size)
+		goto err_data;
+
+	fw_data->hdr = devm_kzalloc(dev, hdr_size, GFP_KERNEL);
+	if (!fw_data->hdr)
+		goto err_data;
+
+	memcpy(fw_data->hdr, &fw->data[0], hdr_size);
+
+	for (int i = 0; i < cpu_to_be32(fw_data->hdr->num_programs); i++)
+		total_prog_sz += cpu_to_be32(fw_data->hdr->prog_size[i]);
+
+	for (int i = 0; i < cpu_to_be32(fw_data->hdr->num_configs); i++)
+		total_config_sz += cpu_to_be32(fw_data->hdr->config_size[i]);
+
+	if (fw->size < hdr_size + total_prog_sz + total_config_sz)
+		goto err_hdr;
+
+	fw_data->prog_data = devm_kzalloc(dev, total_prog_sz, GFP_KERNEL);
+	if (!fw_data->prog_data)
+		goto err_hdr;
+
+	memcpy(fw_data->prog_data, &fw->data[hdr_size], total_prog_sz);
+
+	fw_data->config_data = devm_kzalloc(dev, total_config_sz, GFP_KERNEL);
+	if (!fw_data->config_data)
+		goto err_prog;
+
+	memcpy(fw_data->config_data, &fw->data[hdr_size + total_prog_sz],
+		total_config_sz);
+
+	prog_num = cpu_to_be32(fw_data->hdr->num_programs);
+	config_num = cpu_to_be32(fw_data->hdr->num_configs);
+	dev_info(dev, "Firmware loaded: programs %d, configs %d\n",
+		prog_num, config_num);
+
+	return fw_data;
+
+err_prog:
+	devm_kfree(dev, fw_data->prog_data);
+err_hdr:
+	devm_kfree(dev, fw_data->hdr);
+err_data:
+	devm_kfree(dev, fw_data);
+err_fw:
+	release_firmware(fw);
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(tas25xx_parse_fw);
+
+MODULE_DESCRIPTION("TAS25xx DSP library");
+MODULE_AUTHOR("Dan Murphy <dmurphy@ti.com>");
+MODULE_LICENSE("GPL");
-- 
2.43.0

[PATCH 2/2] ALSA: hda/tas2563: Add tas2563 HDA driver

[Gergo Koteles]

Create tas2563 side codec HDA driver for Lenovo Yoga 7 14ARB7 laptops.
It has two smart amplifiers that can load firmware (tuning file) and
calibration data. The calibration data can be read from EFI variables.
All of the tas2563s in the laptop will be aggregated as one audio
speaker. The code supports realtek codec as the primary codec.

Signed-off-by: Gergo Koteles <soyer@irl.hu>
---
 include/sound/tas2562.h         |   5 +
 sound/pci/hda/Kconfig           |  14 +
 sound/pci/hda/Makefile          |   2 +
 sound/pci/hda/patch_realtek.c   |  22 +-
 sound/pci/hda/tas2563_hda_i2c.c | 508 ++++++++++++++++++++++++++++++++
 5 files changed, 547 insertions(+), 4 deletions(-)
 create mode 100644 sound/pci/hda/tas2563_hda_i2c.c

diff --git a/include/sound/tas2562.h b/include/sound/tas2562.h
index f000e8f5dc88..6d246a5db433 100644
--- a/include/sound/tas2562.h
+++ b/include/sound/tas2562.h
@@ -33,6 +33,11 @@
 #define TAS2562_TDM_CFG9	TAS2562_REG(0, 0x0f)
 #define TAS2562_TDM_CFG10	TAS2562_REG(0, 0x10)
 #define TAS2562_TDM_DET		TAS2562_REG(0, 0x11)
+#define TAS2562_BOOST_CFG1	TAS2562_REG(0, 0x33)
+#define TAS2562_BOOST_CFG2	TAS2562_REG(0, 0x34)
+#define TAS2562_BOOST_CFG3	TAS2562_REG(0, 0x35)
+#define TAS2562_BOOST_CFG4	TAS2562_REG(0, 0x40)
+#define TAS2562_ASI_CONFIG3	TAS2562_REG(0, 0x46)
 #define TAS2562_REV_ID		TAS2562_REG(0, 0x7d)
 
 #define TAS2562_RX_OFF_MASK	GENMASK(5, 1)
diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig
index 0d7502d6e060..364d1331f597 100644
--- a/sound/pci/hda/Kconfig
+++ b/sound/pci/hda/Kconfig
@@ -179,6 +179,20 @@ config SND_HDA_SCODEC_TAS2781_I2C
 comment "Set to Y if you want auto-loading the side codec driver"
 	depends on SND_HDA=y && SND_HDA_SCODEC_TAS2781_I2C=m
 
+config SND_HDA_SCODEC_TAS2563_I2C
+	tristate "Build TAS2563 HD-audio side codec support for I2C Bus"
+	depends on I2C
+	depends on ACPI
+	depends on EFI
+	depends on SND_SOC
+	select SND_SOC_TAS25XX_DSP
+	help
+	  Say Y or M here to include TAS2563 I2C HD-audio side codec support
+	  in snd-hda-intel driver, such as ALC287.
+
+comment "Set to Y if you want auto-loading the side codec driver"
+	depends on SND_HDA=y && SND_HDA_SCODEC_TAS2562_I2C=m
+
 config SND_HDA_CODEC_REALTEK
 	tristate "Build Realtek HD-audio codec support"
 	select SND_HDA_GENERIC
diff --git a/sound/pci/hda/Makefile b/sound/pci/hda/Makefile
index f00fc9ed6096..d746fab82d89 100644
--- a/sound/pci/hda/Makefile
+++ b/sound/pci/hda/Makefile
@@ -36,6 +36,7 @@ snd-hda-scodec-cs35l56-i2c-objs :=	cs35l56_hda_i2c.o
 snd-hda-scodec-cs35l56-spi-objs :=	cs35l56_hda_spi.o
 snd-hda-cs-dsp-ctls-objs :=		hda_cs_dsp_ctl.o
 snd-hda-scodec-tas2781-i2c-objs :=	tas2781_hda_i2c.o
+snd-hda-scodec-tas2563-i2c-objs :=	tas2563_hda_i2c.o
 
 # common driver
 obj-$(CONFIG_SND_HDA) := snd-hda-codec.o
@@ -64,6 +65,7 @@ obj-$(CONFIG_SND_HDA_SCODEC_CS35L56_I2C) += snd-hda-scodec-cs35l56-i2c.o
 obj-$(CONFIG_SND_HDA_SCODEC_CS35L56_SPI) += snd-hda-scodec-cs35l56-spi.o
 obj-$(CONFIG_SND_HDA_CS_DSP_CONTROLS) += snd-hda-cs-dsp-ctls.o
 obj-$(CONFIG_SND_HDA_SCODEC_TAS2781_I2C) += snd-hda-scodec-tas2781-i2c.o
+obj-$(CONFIG_SND_HDA_SCODEC_TAS2563_I2C) += snd-hda-scodec-tas2563-i2c.o
 
 # this must be the last entry after codec drivers;
 # otherwise the codec patches won't be hooked before the PCI probe
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index 9677c09cf7a9..1d3e9f77c9d4 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -6770,7 +6770,7 @@ static int comp_match_cs35l41_dev_name(struct device *dev, void *data)
 	return !strcmp(d + n, tmp);
 }
 
-static int comp_match_tas2781_dev_name(struct device *dev,
+static int comp_match_tas2xxx_dev_name(struct device *dev,
 	void *data)
 {
 	struct scodec_dev_name *p = data;
@@ -6823,7 +6823,7 @@ static void cs35l41_generic_fixup(struct hda_codec *cdc, int action, const char
 	}
 }
 
-static void tas2781_generic_fixup(struct hda_codec *cdc, int action,
+static void tas2xxx_generic_fixup(struct hda_codec *cdc, int action,
 	const char *bus, const char *hid)
 {
 	struct device *dev = hda_codec_dev(cdc);
@@ -6841,7 +6841,7 @@ static void tas2781_generic_fixup(struct hda_codec *cdc, int action,
 		rec->index = 0;
 		spec->comps[0].codec = cdc;
 		component_match_add(dev, &spec->match,
-			comp_match_tas2781_dev_name, rec);
+			comp_match_tas2xxx_dev_name, rec);
 		ret = component_master_add_with_match(dev, &comp_master_ops,
 			spec->match);
 		if (ret)
@@ -6888,7 +6888,13 @@ static void alc287_fixup_legion_16ithg6_speakers(struct hda_codec *cdc, const st
 static void tas2781_fixup_i2c(struct hda_codec *cdc,
 	const struct hda_fixup *fix, int action)
 {
-	 tas2781_generic_fixup(cdc, action, "i2c", "TIAS2781");
+	 tas2xxx_generic_fixup(cdc, action, "i2c", "TIAS2781");
+}
+
+static void tas2563_fixup_i2c(struct hda_codec *cdc,
+	const struct hda_fixup *fix, int action)
+{
+	 tas2xxx_generic_fixup(cdc, action, "i2c", "INT8866");
 }
 
 /* for alc295_fixup_hp_top_speakers */
@@ -7358,6 +7364,7 @@ enum {
 	ALC236_FIXUP_DELL_DUAL_CODECS,
 	ALC287_FIXUP_CS35L41_I2C_2_THINKPAD_ACPI,
 	ALC287_FIXUP_TAS2781_I2C,
+	ALC287_FIXUP_TAS2563_I2C,
 	ALC245_FIXUP_HP_MUTE_LED_COEFBIT,
 	ALC245_FIXUP_HP_X360_MUTE_LEDS,
 	ALC287_FIXUP_THINKPAD_I2S_SPK,
@@ -9447,6 +9454,12 @@ static const struct hda_fixup alc269_fixups[] = {
 		.chained = true,
 		.chain_id = ALC269_FIXUP_THINKPAD_ACPI,
 	},
+	[ALC287_FIXUP_TAS2563_I2C] = {
+		.type = HDA_FIXUP_FUNC,
+		.v.func = tas2563_fixup_i2c,
+		.chained = true,
+		.chain_id = ALC285_FIXUP_THINKPAD_HEADSET_JACK,
+	},
 	[ALC245_FIXUP_HP_MUTE_LED_COEFBIT] = {
 		.type = HDA_FIXUP_FUNC,
 		.v.func = alc245_fixup_hp_mute_led_coefbit,
@@ -10056,6 +10069,7 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = {
 	SND_PCI_QUIRK(0x17aa, 0x3853, "Lenovo Yoga 7 15ITL5", ALC287_FIXUP_YOGA7_14ITL_SPEAKERS),
 	SND_PCI_QUIRK(0x17aa, 0x3855, "Legion 7 16ITHG6", ALC287_FIXUP_LEGION_16ITHG6),
 	SND_PCI_QUIRK(0x17aa, 0x3869, "Lenovo Yoga7 14IAL7", ALC287_FIXUP_YOGA9_14IAP7_BASS_SPK_PIN),
+	SND_PCI_QUIRK(0x17aa, 0x3870, "Lenovo Yoga 7 14ARB7", ALC287_FIXUP_TAS2563_I2C),
 	SND_PCI_QUIRK(0x17aa, 0x387d, "Yoga S780-16 pro Quad AAC", ALC287_FIXUP_TAS2781_I2C),
 	SND_PCI_QUIRK(0x17aa, 0x387e, "Yoga S780-16 pro Quad YC", ALC287_FIXUP_TAS2781_I2C),
 	SND_PCI_QUIRK(0x17aa, 0x3881, "YB9 dual power mode2 YC", ALC287_FIXUP_TAS2781_I2C),
diff --git a/sound/pci/hda/tas2563_hda_i2c.c b/sound/pci/hda/tas2563_hda_i2c.c
new file mode 100644
index 000000000000..14f7c1ab6cbc
--- /dev/null
+++ b/sound/pci/hda/tas2563_hda_i2c.c
@@ -0,0 +1,508 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+//
+// TAS2563 HDA I2C driver
+//
+// Copyright (C) 2023 Gergo Koteles <soyer@irl.hu>
+
+#include <linux/acpi.h>
+#include <linux/crc8.h>
+#include <linux/crc32.h>
+#include <linux/efi.h>
+#include <linux/firmware.h>
+#include <linux/i2c.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <sound/hda_codec.h>
+#include <sound/soc.h>
+#include <sound/tlv.h>
+
+#include <sound/tas2562.h>
+#include <sound/tas25xx-dsp.h>
+
+#include "hda_local.h"
+#include "hda_auto_parser.h"
+#include "hda_component.h"
+#include "hda_jack.h"
+#include "hda_generic.h"
+
+#define TAS2563_GLOBAL_ADDR		0x48
+#define TAS2563_MAX_CHANNELS	2
+
+#define TAS2562_SW_RESET_RESET	BIT(0)
+#define TAS2562_PWR_ACTIVE	0
+#define TAS2562_PWR_SUSPEND	BIT(1)
+
+
+#define TAS2563_CAL_POWER	TAS2562_REG(0x0d, 0x3c)
+#define TAS2563_CAL_R0		TAS2562_REG(0x0f, 0x34)
+#define TAS2563_CAL_INVR0	TAS2562_REG(0x0f, 0x40)
+#define TAS2563_CAL_R0_LOW	TAS2562_REG(0x0f, 0x48)
+#define TAS2563_CAL_TLIM	TAS2562_REG(0x10, 0x14)
+#define TAS2563_CAL_N		5
+#define TAS2563_CAL_DATA_SIZE	4
+
+static unsigned int cal_regs[TAS2563_CAL_N] = {
+	TAS2563_CAL_POWER, TAS2563_CAL_R0, TAS2563_CAL_INVR0,
+	TAS2563_CAL_R0_LOW, TAS2563_CAL_TLIM,
+};
+
+static efi_guid_t efi_guid = EFI_GUID(0x1f52d2a1, 0xbb3a, 0x457d, 0xbc, 0x09,
+		0x43, 0xa3, 0xf4, 0x31, 0x0a, 0x92);
+
+static efi_char16_t *efi_var_names[TAS2563_MAX_CHANNELS][TAS2563_CAL_N] = {
+	{ L"Power_1", L"R0_1", L"InvR0_1", L"R0_Low_1", L"TLim_1" },
+	{ L"Power_2", L"R0_2", L"InvR0_2", L"R0_Low_2", L"TLim_2" },
+};
+
+struct tas2563_dev {
+	unsigned char dev_id;
+	unsigned int i2c_addr;
+	struct i2c_client *client;
+	struct regmap *regmap;
+	uint32_t cal_data[TAS2563_CAL_N];
+};
+
+struct tas2563_data {
+	struct device *dev;
+	struct i2c_client *client;
+	struct tas2563_dev tasdevs[TAS2563_MAX_CHANNELS];
+	unsigned char ndev;
+	char firmware_name[32];
+	struct tas25xx_fw_data *fw_data;
+};
+
+static const struct regmap_range_cfg tas2563_ranges[] = {
+	{
+		.range_min = 0,
+		.range_max = 255 * 128,
+		.selector_reg = TAS2562_PAGE_CTRL,
+		.selector_mask = 0xff,
+		.selector_shift = 0,
+		.window_start = 0,
+		.window_len = 128,
+	},
+};
+
+static const struct regmap_config tas2563_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+
+	.max_register = 255 * 128,
+	.cache_type = REGCACHE_NONE,
+	.ranges = tas2563_ranges,
+	.num_ranges = ARRAY_SIZE(tas2563_ranges),
+};
+
+#define TAS2563_REG_INIT_N 12
+static const struct reg_default tas2563_reg_init[TAS2563_MAX_CHANNELS]
+	[TAS2563_REG_INIT_N] = {
+	{
+		{ TAS2562_TDM_CFG2, 0x5a },
+		{ TAS2562_TDM_CFG4, 0xf3 },
+		{ TAS2562_TDM_CFG5, 0x42 },
+		{ TAS2562_TDM_CFG6, 0x40 },
+		{ TAS2562_BOOST_CFG1, 0xd4 },
+		{ TAS2562_BOOST_CFG3, 0xa4 },
+		{ TAS2562_REG(0x00, 0x36), 0x0b },
+		{ TAS2562_REG(0x00, 0x38), 0x21 },
+		{ TAS2562_REG(0x00, 0x3c), 0x58 },
+		{ TAS2562_BOOST_CFG4, 0xb6 },
+		{ TAS2562_ASI_CONFIG3, 0x04},
+		{ TAS2562_REG(0x00, 0x47), 0xb1 },
+	},
+	{
+		{ TAS2562_TDM_CFG2, 0x6a },
+		{ TAS2562_TDM_CFG4, 0x93 },
+		{ TAS2562_TDM_CFG5, 0x46 },
+		{ TAS2562_TDM_CFG6, 0x44 },
+		{ TAS2562_BOOST_CFG1, 0xd4 },
+		{ TAS2562_BOOST_CFG3, 0xa4 },
+		{ TAS2562_REG(0x00, 0x36), 0x0c },
+		{ TAS2562_REG(0x00, 0x38), 0x21 },
+		{ TAS2562_REG(0x00, 0x3c), 0x58 },
+		{ TAS2562_BOOST_CFG4, 0xb6 },
+		{ TAS2562_ASI_CONFIG3, 0x05},
+		{ TAS2562_REG(0x00, 0x47), 0xb0 },
+	},
+};
+
+static void tas2563_set_power(struct tas2563_data *tas2563, char power)
+{
+	int ret;
+
+	if (!tas2563->fw_data)
+		return;
+
+	for (int i = 0; i < tas2563->ndev; ++i) {
+		struct regmap *regmap = tas2563->tasdevs[i].regmap;
+
+		ret = regmap_write(regmap, TAS2562_PWR_CTRL, power);
+		if (ret)
+			dev_err(tas2563->dev, "Error setting power\n");
+	}
+}
+
+static void tas2563_tasdev_setup(struct tas2563_data *tas2563,
+	struct tas2563_dev *tasdev)
+{
+	int ret;
+	struct regmap *regmap = tasdev->regmap;
+
+	if (!tas2563->fw_data)
+		return;
+
+	ret = regmap_write(regmap, TAS2562_SW_RESET, TAS2562_SW_RESET_RESET);
+	if (ret)
+		dev_err(tas2563->dev, "Error resetting device\n");
+
+	ret = tas25xx_write_program(tas2563->dev, regmap, tas2563->fw_data, 0);
+	if (ret)
+		dev_err(tas2563->dev, "Error writing program\n");
+
+	ret = tas25xx_write_config(tas2563->dev, regmap, tas2563->fw_data, 0);
+	if (ret)
+		dev_err(tas2563->dev, "Error writing config\n");
+
+	for (int i = 0; i < TAS2563_REG_INIT_N; ++i) {
+		struct reg_default reg = tas2563_reg_init[tasdev->dev_id][i];
+
+		ret = regmap_write(regmap, reg.reg, reg.def);
+		if (ret)
+			dev_err(tas2563->dev, "Error writing init regs\n");
+	}
+
+	ret = regmap_write(regmap, TAS25XX_DSP_MODE, 1);
+	if (ret)
+		dev_err(tas2563->dev, "Error enabling DSP\n");
+
+	for (int i = 0; i < TAS2563_CAL_N; ++i) {
+		ret = regmap_bulk_write(regmap, cal_regs[i],
+			&tasdev->cal_data[i], TAS2563_CAL_DATA_SIZE);
+		if (ret)
+			dev_err(tas2563->dev, "Error writing calib regs\n");
+	}
+
+	ret = regmap_write(regmap, TAS2562_PWR_CTRL, 0);
+	if (ret)
+		dev_err(tas2563->dev, "Error setting power on\n");
+}
+
+static void tas2563_fw_loaded(const struct firmware *fw, void *context)
+{
+	struct tas2563_data *tas2563 = context;
+
+	if (!fw)
+		return;
+
+	tas2563->fw_data = tas25xx_parse_fw(tas2563->dev, fw);
+	if (!tas2563->fw_data) {
+		dev_err(tas2563->dev, "Failed to parse firmware\n");
+		return;
+	}
+
+	for (int i = 0; i < tas2563->ndev; ++i)
+		tas2563_tasdev_setup(tas2563, &tas2563->tasdevs[i]);
+}
+
+static void tas2563_hda_playback_hook(struct device *dev, int action)
+{
+	struct tas2563_data *tas2563 = dev_get_drvdata(dev);
+
+	dev_dbg(tas2563->dev, "%s: action = %d\n", __func__, action);
+	switch (action) {
+	case HDA_GEN_PCM_ACT_OPEN:
+		pm_runtime_get_sync(dev);
+		tas2563_set_power(tas2563, TAS2562_PWR_ACTIVE);
+		break;
+	case HDA_GEN_PCM_ACT_CLOSE:
+		tas2563_set_power(tas2563, TAS2562_PWR_SUSPEND);
+		pm_runtime_mark_last_busy(dev);
+		pm_runtime_put_autosuspend(dev);
+		break;
+	default:
+		dev_dbg(tas2563->dev, "Playback action not supported: %d\n",
+			action);
+		break;
+	}
+}
+
+static int tas2563_hda_bind(struct device *dev, struct device *master,
+	void *master_data)
+{
+	struct tas2563_data *tas2563 = dev_get_drvdata(dev);
+	struct hda_component *comps = master_data;
+	struct hda_codec *codec;
+	int ret;
+
+	if (!comps)
+		return -EINVAL;
+
+	if (comps->dev)
+		return -EBUSY;
+	comps->dev = dev;
+	codec = comps->codec;
+
+	pm_runtime_get_sync(dev);
+
+	strscpy(comps->name, dev_name(dev), sizeof(comps->name));
+	scnprintf(tas2563->firmware_name, 32, "TAS2563-%08X.bin",
+		codec->core.subsystem_id);
+
+	ret = request_firmware_nowait(THIS_MODULE, FW_ACTION_UEVENT,
+				tas2563->firmware_name, tas2563->dev,
+				GFP_KERNEL, tas2563,
+				tas2563_fw_loaded);
+	if (ret)
+		dev_err(tas2563->dev, "request_firmware_nowait err: %d\n",
+			ret);
+
+	comps->playback_hook = tas2563_hda_playback_hook;
+
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
+
+	return ret;
+}
+
+static void tas2563_hda_unbind(struct device *dev,
+	struct device *master, void *master_data)
+{
+	struct hda_component *comps = master_data;
+
+	if (comps->dev != dev)
+		return;
+
+	comps->dev = NULL;
+	comps->playback_hook = NULL;
+}
+
+static const struct component_ops tas2563_hda_comp_ops = {
+	.bind = tas2563_hda_bind,
+	.unbind = tas2563_hda_unbind,
+};
+
+static int tas2563_tasdev_init_regmap(struct tas2563_data *tas2563,
+	struct tas2563_dev *tasdev)
+{
+	tasdev->regmap = devm_regmap_init_i2c(tasdev->client,
+		&tas2563_regmap_config);
+	if (IS_ERR(tasdev->regmap))
+		return PTR_ERR(tasdev->regmap);
+	return 0;
+}
+
+static int tas2563_tasdev_init_client(struct tas2563_data *tas2563,
+	struct tas2563_dev *tasdev)
+{
+	tasdev->client = tas2563->client->addr == tasdev->i2c_addr
+		? tas2563->client : devm_i2c_new_dummy_device(tas2563->dev,
+				tas2563->client->adapter, tasdev->i2c_addr);
+	if (IS_ERR(tasdev->client))
+		return PTR_ERR(tasdev->client);
+	return 0;
+}
+
+/* Update the calibrate data, including speaker impedance, f0, etc, into algo.
+ * Calibrate data is done by manufacturer in the factory. These data are used
+ * by Algo for calucating the speaker temperature, speaker membrance excursion
+ * and f0 in real time during playback.
+ */
+static int tas2563_tasdev_read_efi(struct tas2563_data *tas2563,
+	struct tas2563_dev *tasdev)
+{
+	efi_status_t status;
+	unsigned int attr;
+	unsigned long max_size = TAS2563_CAL_DATA_SIZE;
+
+	for (int i = 0; i < TAS2563_CAL_N; ++i) {
+		status = efi.get_variable(efi_var_names[tasdev->dev_id][i],
+			&efi_guid, &attr, &max_size,
+			&tasdev->cal_data[i]);
+		if (status != EFI_SUCCESS)
+			return -EINVAL;
+		tasdev->cal_data[i] = cpu_to_be32(tasdev->cal_data[i]);
+	}
+
+	dev_info(tas2563->dev,
+		"Calibration data %d: power:%08x r0:%08x inv_r0:%08x r0_low:%08x tlim:%08x\n",
+		tasdev->dev_id, tasdev->cal_data[0], tasdev->cal_data[1],
+		tasdev->cal_data[2], tasdev->cal_data[3], tasdev->cal_data[4]);
+
+	return 0;
+}
+
+static int tas2563_get_i2c_res(struct acpi_resource *ares, void *data)
+{
+	struct tas2563_data *tas2563 = data;
+	struct acpi_resource_i2c_serialbus *sb;
+	struct tas2563_dev *tasdev;
+
+	if (i2c_acpi_get_i2c_resource(ares, &sb)) {
+		if (tas2563->ndev < 2 &&
+			sb->slave_address != TAS2563_GLOBAL_ADDR) {
+			tasdev = &tas2563->tasdevs[tas2563->ndev];
+			tasdev->dev_id = tas2563->ndev;
+			tasdev->i2c_addr =
+				(unsigned int)sb->slave_address;
+			tas2563->ndev++;
+		}
+	}
+	return 1;
+}
+
+static int tas2563_read_acpi(struct tas2563_data *tas2563)
+{
+	struct acpi_device *adev;
+	LIST_HEAD(resources);
+	int ret;
+
+	adev = ACPI_COMPANION(tas2563->dev);
+	if (!adev) {
+		dev_err(tas2563->dev, "Error could not get ACPI device\n");
+		return -ENODEV;
+	}
+
+	ret = acpi_dev_get_resources(adev, &resources, tas2563_get_i2c_res,
+		tas2563);
+	if (ret < 0) {
+		dev_err(tas2563->dev, "Read acpi error, ret: %d\n", ret);
+		return ret;
+	}
+
+	acpi_dev_free_resource_list(&resources);
+
+	return 0;
+}
+
+static int tas2563_hda_i2c_probe(struct i2c_client *client)
+{
+	struct tas2563_data *tas2563;
+	int ret;
+
+	tas2563 = devm_kzalloc(&client->dev, sizeof(struct tas2563_data),
+		GFP_KERNEL);
+	if (!tas2563)
+		return -ENOMEM;
+	tas2563->dev = &client->dev;
+	tas2563->client = client;
+
+	dev_set_drvdata(tas2563->dev, tas2563);
+
+	ret = tas2563_read_acpi(tas2563);
+	if (ret)
+		return dev_err_probe(tas2563->dev, ret,
+			"Platform not supported\n");
+
+	for (int i = 0; i < tas2563->ndev; ++i) {
+		struct tas2563_dev *tasdev = &tas2563->tasdevs[i];
+
+		ret = tas2563_tasdev_read_efi(tas2563, tasdev);
+		if (ret)
+			return dev_err_probe(tas2563->dev, ret,
+				"Calibration data cannot be read from EFI\n");
+
+		ret = tas2563_tasdev_init_client(tas2563, tasdev);
+		if (ret)
+			return dev_err_probe(tas2563->dev, ret,
+				"Failed to init i2c client\n");
+
+		ret = tas2563_tasdev_init_regmap(tas2563, tasdev);
+		if (ret)
+			return dev_err_probe(tas2563->dev, ret,
+				"Failed to allocate register map\n");
+	}
+
+	ret = component_add(tas2563->dev, &tas2563_hda_comp_ops);
+	if (ret) {
+		return dev_err_probe(tas2563->dev, ret,
+			"Register component failed\n");
+	}
+
+	pm_runtime_set_autosuspend_delay(tas2563->dev, 3000);
+	pm_runtime_use_autosuspend(tas2563->dev);
+	pm_runtime_mark_last_busy(tas2563->dev);
+	pm_runtime_set_active(tas2563->dev);
+	pm_runtime_get_noresume(tas2563->dev);
+	pm_runtime_enable(tas2563->dev);
+
+	pm_runtime_put_autosuspend(tas2563->dev);
+
+	return 0;
+}
+
+static void tas2563_hda_i2c_remove(struct i2c_client *client)
+{
+	struct tas2563_data *tas2563 = dev_get_drvdata(&client->dev);
+
+	pm_runtime_get_sync(tas2563->dev);
+	pm_runtime_disable(tas2563->dev);
+
+	component_del(tas2563->dev, &tas2563_hda_comp_ops);
+
+	pm_runtime_put_noidle(tas2563->dev);
+}
+
+static int tas2563_system_suspend(struct device *dev)
+{
+	struct tas2563_data *tas2563 = dev_get_drvdata(dev);
+	int ret;
+
+	dev_dbg(tas2563->dev, "System Suspend\n");
+
+	ret = pm_runtime_force_suspend(dev);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int tas2563_system_resume(struct device *dev)
+{
+	int ret;
+	struct tas2563_data *tas2563 = dev_get_drvdata(dev);
+
+	dev_dbg(tas2563->dev, "System Resume\n");
+
+	ret = pm_runtime_force_resume(dev);
+	if (ret)
+		return ret;
+
+	for (int i = 0; i < tas2563->ndev; ++i)
+		tas2563_tasdev_setup(tas2563, &tas2563->tasdevs[i]);
+
+	return 0;
+}
+
+static const struct dev_pm_ops tas2563_hda_pm_ops = {
+	SYSTEM_SLEEP_PM_OPS(tas2563_system_suspend, tas2563_system_resume)
+};
+
+static const struct i2c_device_id tas2563_hda_i2c_id[] = {
+	{ "tas2563-hda", 0 },
+	{}
+};
+
+static const struct acpi_device_id tas2563_acpi_hda_match[] = {
+	{"INT8866", 0 },
+	{}
+};
+MODULE_DEVICE_TABLE(acpi, tas2563_acpi_hda_match);
+
+static struct i2c_driver tas2563_hda_i2c_driver = {
+	.driver = {
+		.name		= "tas2563-hda",
+		.acpi_match_table = tas2563_acpi_hda_match,
+		.pm		= &tas2563_hda_pm_ops,
+	},
+	.id_table	= tas2563_hda_i2c_id,
+	.probe		= tas2563_hda_i2c_probe,
+	.remove		= tas2563_hda_i2c_remove,
+};
+module_i2c_driver(tas2563_hda_i2c_driver);
+
+MODULE_DESCRIPTION("TAS2563 HDA Driver");
+MODULE_AUTHOR("Gergo Koteles <soyer@irl.hu>");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(SND_SOC_TAS25XX_DSP);
-- 
2.43.0

Re: [PATCH 1/2] ASoc: tas2563: DSP Firmware loading support

[Pierre-Louis Bossart]

> Firmware loading is done dymacally and the program and configuration

dynamically

> selection is done by the user.
> 
> The binary itself contains a list of instructions for either a single
> mode write or a burst write.  The single mode write is list of register
> writes to different books and pages within the register map.
> The burst writes is a block of data that is written to a specific
> location in memory.
> 
> The firmware loader must parse and load the blocks in real time as the
> binary may contain different audio profiles.
> 
> If the DSP is not needed to do audio preocessing then the DSP program

preprocessing

> can be turned off and the device will effectively turn off the DSP.

> ---
>  {sound/soc/codecs => include/sound}/tas2562.h |   3 +
>  include/sound/tas25xx-dsp.h                   | 100 +++++++
>  sound/soc/codecs/Kconfig                      |   7 +
>  sound/soc/codecs/Makefile                     |   2 +
>  sound/soc/codecs/tas2562.c                    |   2 +-

are tas2562 and tas2563 (from commit subject) the same?

>  sound/soc/codecs/tas25xx-dsp.c                | 282 ++++++++++++++++++
>  6 files changed, 395 insertions(+), 1 deletion(-)
>  rename {sound/soc/codecs => include/sound}/tas2562.h (97%)
>  create mode 100644 include/sound/tas25xx-dsp.h
>  create mode 100644 sound/soc/codecs/tas25xx-dsp.c
> 
> diff --git a/sound/soc/codecs/tas2562.h b/include/sound/tas2562.h
> similarity index 97%
> rename from sound/soc/codecs/tas2562.h
> rename to include/sound/tas2562.h

> diff --git a/sound/soc/codecs/tas25xx-dsp.c b/sound/soc/codecs/tas25xx-dsp.c
> new file mode 100644
> index 000000000000..d5081fa01441
> --- /dev/null
> +++ b/sound/soc/codecs/tas25xx-dsp.c
> @@ -0,0 +1,282 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// Firmware loader for the Texas Instruments TAS25XX DSP
> +// Copyright (C) 2020 Texas Instruments Inc.
> +
> +#include <linux/module.h>
> +#include <linux/errno.h>
> +#include <linux/device.h>
> +#include <linux/firmware.h>
> +#include <linux/i2c.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +#include <linux/delay.h>

nit-pick: alphabetical order?

> +
> +#include <sound/tas2562.h>
> +#include <sound/tas25xx-dsp.h>
> +
> +
> +static void tas25xx_process_fw_delay(struct tas25xx_cmd *cmd)
> +{
> +	mdelay(cpu_to_be16(cmd->hdr.length));

is this the length of the header, or the duration of the delay?

Someone will get it wrong with this naming...

> +}
> +
> +static int tas25xx_process_fw_single(struct regmap *regmap,
> +	struct tas25xx_cmd *cmd)
> +{
> +	int ret;
> +	int num_writes = cpu_to_be16(cmd->hdr.length);
> +	struct tas25xx_cmd_reg *reg = &cmd->reg;

reverse x-mas style recommended, e.g. move 'int ret;' last

> +
> +	for (int i = 0; i < num_writes; i++, reg++) {
> +		/* Reset Page to 0 to access BOOK_CTRL */
> +		ret = regmap_write(regmap, TAS2562_PAGE_CTRL, 0);
> +		if (ret)
> +			return ret;
> +
> +		ret = regmap_write(regmap, TAS2562_BOOK_CTRL, reg->book);
> +		if (ret)
> +			return ret;
> +
> +		ret = regmap_write(regmap, TAS2562_REG(reg->page, reg->offset),
> +			reg->data);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int tas25xx_process_fw_burst(struct regmap *regmap,
> +	struct tas25xx_cmd *cmd)
> +{
> +	int ret;
> +
> +	/* Reset Page to 0 to access BOOK_CTRL */
> +	ret = regmap_write(regmap, TAS2562_PAGE_CTRL, 0);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(regmap, TAS2562_BOOK_CTRL, cmd->reg.book);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_bulk_write(regmap, TAS2562_REG(cmd->reg.page, cmd->reg.offset), &cmd[1],
> +				cpu_to_be16(cmd->hdr.length));
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int tas25xx_process_block(struct device *dev, struct regmap *regmap,
> +	struct tas25xx_cmd *cmd, int max_block_size)
> +{
> +	int ret;
> +	int block_read;
> +
> +	const int hdr_size = sizeof(struct tas25xx_cmd_hdr);
> +	const int reg_size = sizeof(struct tas25xx_cmd_reg);
> +	const int cmd_size = sizeof(struct tas25xx_cmd);
> +
> +	switch (cpu_to_be16(cmd->hdr.cmd_type)) {
> +	case TAS25XX_CMD_SING_W:
> +		block_read = cpu_to_be16(cmd->hdr.length) * reg_size + hdr_size;
> +		break;
> +	case TAS25XX_CMD_BURST:
> +		block_read = cpu_to_be16(cmd->hdr.length) + cmd_size;
> +		break;
> +	case TAS25XX_CMD_DELAY:
> +		block_read = 4;
> +		break;
> +	default:
> +		block_read = 0;
> +	}
> +
> +	if (block_read > max_block_size) {
> +		dev_err(dev,
> +			"Corrupt firmware: block_read > max_block_size %d %d\n",
> +			block_read, max_block_size);
> +		return -EINVAL;
> +	}
> +
> +	switch (cpu_to_be16(cmd->hdr.cmd_type)) {
> +	case TAS25XX_CMD_SING_W:
> +		ret = tas25xx_process_fw_single(regmap, cmd);
> +		if (ret) {
> +			dev_err(dev, "Failed to process single write %d\n",
> +				ret);
> +			return ret;
> +		}
> +		break;
> +	case TAS25XX_CMD_BURST:
> +		ret = tas25xx_process_fw_burst(regmap, cmd);
> +		if (ret) {
> +			dev_err(dev, "Failed to process burst write %d\n", ret);
> +			return ret;
> +		}
> +		break;
> +	case TAS25XX_CMD_DELAY:
> +		tas25xx_process_fw_delay(cmd);
> +		break;
> +	default:
> +		dev_warn(dev, "Unknown cmd type %d\n",
> +			cpu_to_be16(cmd->hdr.cmd_type));
> +		break;
> +	};
> +
> +	return block_read;
> +}
> +
> +
> +int tas25xx_write_program(struct device *dev, struct regmap *regmap,
> +	struct tas25xx_fw_data *fw_data, int prog_num)
> +{
> +	int offset = 0;
> +	int length = 0;

useless init

> +	struct tas25xx_program_info *prog_info;
> +	struct tas25xx_fw_hdr *hdr = fw_data->hdr;
> +	struct tas25xx_cmd *cmd;

again reverse x-mas style would look much better

> +
> +	if (prog_num > cpu_to_be32(hdr->num_programs))
> +		return -EINVAL;
> +
> +	for (int i = 0; i < prog_num; i++)
> +		offset += cpu_to_be32(hdr->prog_size[i]);
> +
> +	prog_info = (struct tas25xx_program_info *)&fw_data->prog_data[offset];
> +	dev_info(dev, "Write program %d: %s\n", prog_num, prog_info->name);
> +
> +	int max_offset = offset + cpu_to_be32(hdr->prog_size[prog_num]);
> +	int num_subblocks = cpu_to_be32(prog_info->blk_data.num_subblocks);

It's not illegal but not consistent to declare variables in the middle
of code for no good reason.

> +	offset += sizeof(struct tas25xx_program_info);
> +
> +	for (int i = 0; i < num_subblocks; i++) {
> +		cmd = (struct tas25xx_cmd *)&fw_data->prog_data[offset];
> +		length = tas25xx_process_block(dev, regmap, cmd,
> +			max_offset - offset);
> +		if (length < 0)
> +			return length;
> +
> +		offset += length;
> +	}
> +
> +	/* Reset Book to 0 */
> +	regmap_write(regmap, TAS2562_PAGE_CTRL, 0);
> +	regmap_write(regmap, TAS2562_BOOK_CTRL, 0);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(tas25xx_write_program);
> +
> +int tas25xx_write_config(struct device *dev, struct regmap *regmap,
> +	struct tas25xx_fw_data *fw_data, int config_num)
> +{
> +	int offset = 0;
> +	int length = 0;

useless init again

> +	struct tas25xx_config_info *cfg_info;
> +	struct tas25xx_fw_hdr *hdr = fw_data->hdr;
> +	struct tas25xx_cmd *cmd;
> +
> +	if (config_num > cpu_to_be32(hdr->num_configs))
> +		return -EINVAL;
> +
> +	for (int i = 0; i < config_num; i++)
> +		offset += cpu_to_be32(hdr->config_size[i]);
> +
> +	cfg_info = (struct tas25xx_config_info *)&fw_data->config_data[offset];
> +	dev_info(dev, "Write config %d: %s\n", config_num, cfg_info->name);
> +
> +	int max_offset = offset + cpu_to_be32(hdr->config_size[config_num]);
> +	int num_subblocks = cpu_to_be32(cfg_info->blk_data.num_subblocks);
> +
> +	offset += sizeof(struct tas25xx_config_info);
> +
> +	for (int i = 0; i < num_subblocks; i++) {
> +		cmd = (struct tas25xx_cmd *)&fw_data->config_data[offset];
> +		length = tas25xx_process_block(dev, regmap, cmd,
> +			max_offset - offset);
> +		if (length < 0)
> +			return length;
> +
> +		offset += length;
> +	}
> +
> +	/* Reset Book to 0 */
> +	regmap_write(regmap, TAS2562_PAGE_CTRL, 0);
> +	regmap_write(regmap, TAS2562_BOOK_CTRL, 0);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(tas25xx_write_config);
> +
> +
> +struct tas25xx_fw_data *tas25xx_parse_fw(struct device *dev,
> +	const struct firmware *fw)
> +{
> +	u32 total_prog_sz = 0;
> +	u32 total_config_sz = 0;
> +	u32 prog_num = 0;
> +	u32 config_num = 0;

last two inits are useless

> +	int hdr_size = sizeof(struct tas25xx_fw_hdr);
> +	struct tas25xx_fw_data *fw_data = NULL;
> +
> +	fw_data = devm_kzalloc(dev, sizeof(struct tas25xx_fw_data), GFP_KERNEL);
> +	if (!fw_data)
> +		goto err_fw;
> +
> +	if (fw->size < hdr_size)
> +		goto err_data;
> +
> +	fw_data->hdr = devm_kzalloc(dev, hdr_size, GFP_KERNEL);
> +	if (!fw_data->hdr)
> +		goto err_data;
> +
> +	memcpy(fw_data->hdr, &fw->data[0], hdr_size);
> +
> +	for (int i = 0; i < cpu_to_be32(fw_data->hdr->num_programs); i++)
> +		total_prog_sz += cpu_to_be32(fw_data->hdr->prog_size[i]);
> +
> +	for (int i = 0; i < cpu_to_be32(fw_data->hdr->num_configs); i++)
> +		total_config_sz += cpu_to_be32(fw_data->hdr->config_size[i]);
> +
> +	if (fw->size < hdr_size + total_prog_sz + total_config_sz)
> +		goto err_hdr;
> +
> +	fw_data->prog_data = devm_kzalloc(dev, total_prog_sz, GFP_KERNEL);
> +	if (!fw_data->prog_data)
> +		goto err_hdr;
> +
> +	memcpy(fw_data->prog_data, &fw->data[hdr_size], total_prog_sz);
> +
> +	fw_data->config_data = devm_kzalloc(dev, total_config_sz, GFP_KERNEL);
> +	if (!fw_data->config_data)
> +		goto err_prog;
> +
> +	memcpy(fw_data->config_data, &fw->data[hdr_size + total_prog_sz],
> +		total_config_sz);
> +
> +	prog_num = cpu_to_be32(fw_data->hdr->num_programs);
> +	config_num = cpu_to_be32(fw_data->hdr->num_configs);
> +	dev_info(dev, "Firmware loaded: programs %d, configs %d\n",
> +		prog_num, config_num);
> +
> +	return fw_data;
> +
> +err_prog:
> +	devm_kfree(dev, fw_data->prog_data);
> +err_hdr:
> +	devm_kfree(dev, fw_data->hdr);
> +err_data:
> +	devm_kfree(dev, fw_data);
> +err_fw:
> +	release_firmware(fw);
> +
> +	return NULL;
> +}
> +EXPORT_SYMBOL_GPL(tas25xx_parse_fw);
> +
> +MODULE_DESCRIPTION("TAS25xx DSP library");
> +MODULE_AUTHOR("Dan Murphy <dmurphy@ti.com>");
> +MODULE_LICENSE("GPL");

Re: [PATCH 2/2] ALSA: hda/tas2563: Add tas2563 HDA driver

[Pierre-Louis Bossart]

> diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
> index 9677c09cf7a9..1d3e9f77c9d4 100644
> --- a/sound/pci/hda/patch_realtek.c
> +++ b/sound/pci/hda/patch_realtek.c
> @@ -6770,7 +6770,7 @@ static int comp_match_cs35l41_dev_name(struct device *dev, void *data)
>  	return !strcmp(d + n, tmp);
>  }
>  
> -static int comp_match_tas2781_dev_name(struct device *dev,
> +static int comp_match_tas2xxx_dev_name(struct device *dev,
>  	void *data)
>  {
>  	struct scodec_dev_name *p = data;
> @@ -6823,7 +6823,7 @@ static void cs35l41_generic_fixup(struct hda_codec *cdc, int action, const char
>  	}
>  }
>  
> -static void tas2781_generic_fixup(struct hda_codec *cdc, int action,
> +static void tas2xxx_generic_fixup(struct hda_codec *cdc, int action,
>  	const char *bus, const char *hid)
>  {
>  	struct device *dev = hda_codec_dev(cdc);
> @@ -6841,7 +6841,7 @@ static void tas2781_generic_fixup(struct hda_codec *cdc, int action,
>  		rec->index = 0;
>  		spec->comps[0].codec = cdc;
>  		component_match_add(dev, &spec->match,
> -			comp_match_tas2781_dev_name, rec);
> +			comp_match_tas2xxx_dev_name, rec);
>  		ret = component_master_add_with_match(dev, &comp_master_ops,
>  			spec->match);
>  		if (ret)
> @@ -6888,7 +6888,13 @@ static void alc287_fixup_legion_16ithg6_speakers(struct hda_codec *cdc, const st
>  static void tas2781_fixup_i2c(struct hda_codec *cdc,
>  	const struct hda_fixup *fix, int action)
>  {
> -	 tas2781_generic_fixup(cdc, action, "i2c", "TIAS2781");
> +	 tas2xxx_generic_fixup(cdc, action, "i2c", "TIAS2781");
> +}

this sort of rename should be part of a separate patch IMHO, it'd be
easier to review.

> +
> +static void tas2563_fixup_i2c(struct hda_codec *cdc,
> +	const struct hda_fixup *fix, int action)
> +{
> +	 tas2xxx_generic_fixup(cdc, action, "i2c", "INT8866");

Any specific reason to use an Intel ACPI identifier? Why not use
"TIAS2563" ?

> +#define TAS2563_REG_INIT_N 12

newline

> +static const struct reg_default tas2563_reg_init[TAS2563_MAX_CHANNELS]
> +	[TAS2563_REG_INIT_N] = {
> +	{
> +		{ TAS2562_TDM_CFG2, 0x5a },
> +		{ TAS2562_TDM_CFG4, 0xf3 },
> +		{ TAS2562_TDM_CFG5, 0x42 },
> +		{ TAS2562_TDM_CFG6, 0x40 },
> +		{ TAS2562_BOOST_CFG1, 0xd4 },
> +		{ TAS2562_BOOST_CFG3, 0xa4 },
> +		{ TAS2562_REG(0x00, 0x36), 0x0b },
> +		{ TAS2562_REG(0x00, 0x38), 0x21 },
> +		{ TAS2562_REG(0x00, 0x3c), 0x58 },
> +		{ TAS2562_BOOST_CFG4, 0xb6 },
> +		{ TAS2562_ASI_CONFIG3, 0x04},
> +		{ TAS2562_REG(0x00, 0x47), 0xb1 },

> +/* Update the calibrate data, including speaker impedance, f0, etc, into algo.

update the calibration data,

> + * Calibrate data is done by manufacturer in the factory. These data are used

The manufacturer calibrates the data in the factory.

> + * by Algo for calucating the speaker temperature, speaker membrance excursion

calculating

membrane


> +static int tas2563_hda_i2c_probe(struct i2c_client *client)
> +{
> +	struct tas2563_data *tas2563;
> +	int ret;
> +
> +	tas2563 = devm_kzalloc(&client->dev, sizeof(struct tas2563_data),
> +		GFP_KERNEL);
> +	if (!tas2563)
> +		return -ENOMEM;
> +	tas2563->dev = &client->dev;
> +	tas2563->client = client;
> +
> +	dev_set_drvdata(tas2563->dev, tas2563);
> +
> +	ret = tas2563_read_acpi(tas2563);
> +	if (ret)
> +		return dev_err_probe(tas2563->dev, ret,
> +			"Platform not supported\n");
> +
> +	for (int i = 0; i < tas2563->ndev; ++i) {
> +		struct tas2563_dev *tasdev = &tas2563->tasdevs[i];
> +
> +		ret = tas2563_tasdev_read_efi(tas2563, tasdev);
> +		if (ret)
> +			return dev_err_probe(tas2563->dev, ret,
> +				"Calibration data cannot be read from EFI\n");
> +
> +		ret = tas2563_tasdev_init_client(tas2563, tasdev);
> +		if (ret)
> +			return dev_err_probe(tas2563->dev, ret,
> +				"Failed to init i2c client\n");
> +
> +		ret = tas2563_tasdev_init_regmap(tas2563, tasdev);
> +		if (ret)
> +			return dev_err_probe(tas2563->dev, ret,
> +				"Failed to allocate register map\n");
> +	}
> +
> +	ret = component_add(tas2563->dev, &tas2563_hda_comp_ops);
> +	if (ret) {
> +		return dev_err_probe(tas2563->dev, ret,
> +			"Register component failed\n");
> +	}

I wonder how many of those tests actually depend on deferred probe, and
if this isn't a case of copy-paste "just in case"?

> +
> +	pm_runtime_set_autosuspend_delay(tas2563->dev, 3000);
> +	pm_runtime_use_autosuspend(tas2563->dev);
> +	pm_runtime_mark_last_busy(tas2563->dev);
> +	pm_runtime_set_active(tas2563->dev);
> +	pm_runtime_get_noresume(tas2563->dev);
> +	pm_runtime_enable(tas2563->dev);
> +
> +	pm_runtime_put_autosuspend(tas2563->dev);

the sequence get_noresume/enable/put_autosuspend makes no sense to me.
doing a get_noresume *before* enable should do exactly nothing, and
releasing the resource would already be handled with autosuspend based
on the last_busy mark.

> +
> +	return 0;
> +}
> +
> +static void tas2563_hda_i2c_remove(struct i2c_client *client)
> +{
> +	struct tas2563_data *tas2563 = dev_get_drvdata(&client->dev);
> +
> +	pm_runtime_get_sync(tas2563->dev);
> +	pm_runtime_disable(tas2563->dev);
> +
> +	component_del(tas2563->dev, &tas2563_hda_comp_ops);
> +
> +	pm_runtime_put_noidle(tas2563->dev);

that pm_runtime sequence also makes no sense to me, if you disable
pm_runtime the last command is useless/no-op.

> +}
> +
> +static int tas2563_system_suspend(struct device *dev)
> +{
> +	struct tas2563_data *tas2563 = dev_get_drvdata(dev);
> +	int ret;
> +
> +	dev_dbg(tas2563->dev, "System Suspend\n");
> +
> +	ret = pm_runtime_force_suspend(dev);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int tas2563_system_resume(struct device *dev)
> +{
> +	int ret;
> +	struct tas2563_data *tas2563 = dev_get_drvdata(dev);
> +
> +	dev_dbg(tas2563->dev, "System Resume\n");
> +
> +	ret = pm_runtime_force_resume(dev);
> +	if (ret)
> +		return ret;
> +
> +	for (int i = 0; i < tas2563->ndev; ++i)
> +		tas2563_tasdev_setup(tas2563, &tas2563->tasdevs[i]);
> +
> +	return 0;
> +}
> +
> +static const struct dev_pm_ops tas2563_hda_pm_ops = {
> +	SYSTEM_SLEEP_PM_OPS(tas2563_system_suspend, tas2563_system_resume)

where's the pm_runtime stuff?

> +};

Re: [PATCH 1/2] ASoc: tas2563: DSP Firmware loading support

[Gergo Koteles]

Hi Pierre-Louis,

Thank you for your review.

On Mon, 2023-12-04 at 18:05 -0600, Pierre-Louis Bossart wrote:

> > 
> >  sound/soc/codecs/tas2562.c                    |   2 +-
> 
> are tas2562 and tas2563 (from commit subject) the same?
> 
No, tas2563 is register compatible with tas2562.

> > 
> > +#include <linux/slab.h>
> > +#include <linux/delay.h>
> 
> nit-pick: alphabetical order?
> 
Good idea, I'll fix it and the others in the next version.

> > +
> > +#include <sound/tas2562.h>
> > +#include <sound/tas25xx-dsp.h>
> > +
> > +
> > +static void tas25xx_process_fw_delay(struct tas25xx_cmd *cmd)
> > +{
> > +	mdelay(cpu_to_be16(cmd->hdr.length));
> 
> is this the length of the header, or the duration of the delay?
> 
> Someone will get it wrong with this naming...
> 
I think this is because of the format.
If cmd_type is TAS25XX_CMD_DELAY, then hdr.length is the length of the
delay.
At least I think so, based on
https://lore.kernel.org/lkml/6d6aaed3-dac8-e1ec-436c-9b04273df2b3@ti.com/T/
https://github.com/torvalds/linux/blob/bee0e7762ad2c6025b9f5245c040fcc36ef2bde8/sound/soc/codecs/tas2781-fmwlib.c#L769

But I don't see any TAS25XX_CMD_DELAY command in the firmware I'm
using.

> > 
> > +	prog_num = cpu_to_be32(fw_data->hdr->num_programs);
> > +	config_num = cpu_to_be32(fw_data->hdr->num_configs);
> > +	dev_info(dev, "Firmware loaded: programs %d, configs %d\n",
> > +		prog_num, config_num);
> > +

For me:
release_firmware(fw) is missing from here

> > +	return fw_data;
> > +
> > +err_prog:
> > +	devm_kfree(dev, fw_data->prog_data);
> > +err_hdr:
> > +	devm_kfree(dev, fw_data->hdr);
> > +err_data:
> > +	devm_kfree(dev, fw_data);
> > +err_fw:
> > +	release_firmware(fw);
> > +
> > +	return NULL;
> > +}
> > 

Regards,

Gergo

Re: [PATCH 2/2] ALSA: hda/tas2563: Add tas2563 HDA driver

[Gergo Koteles]

Hi Pierre-Louis,

On Mon, 2023-12-04 at 18:22 -0600, Pierre-Louis Bossart wrote:
> 
> 
> > 
> > -	 tas2781_generic_fixup(cdc, action, "i2c", "TIAS2781");
> > +	 tas2xxx_generic_fixup(cdc, action, "i2c", "TIAS2781");
> > +}
> 
> this sort of rename should be part of a separate patch IMHO, it'd be
> easier to review.
> 
Okey.

> > +
> > +static void tas2563_fixup_i2c(struct hda_codec *cdc,
> > +	const struct hda_fixup *fix, int action)
> > +{
> > +	 tas2xxx_generic_fixup(cdc, action, "i2c", "INT8866");
> 
> Any specific reason to use an Intel ACPI identifier? Why not use
> "TIAS2563" ?
> 
INT8866 is in the ACPI.
I don't know why Lenovo uses this name.
I think it's more internal than intel.

   Scope (_SB.I2CD)
    {
        Device (TAS)
        {
            Name (_HID, "INT8866")  // _HID: Hardware ID
            Name (_UID, Zero)  // _UID: Unique ID
            Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource
Settings
            {
                Name (RBUF, ResourceTemplate ()
                {
                    I2cSerialBusV2 (0x004C, ControllerInitiated,
0x00061A80,
                        AddressingMode7Bit, "\\_SB.I2CD",
                        0x00, ResourceConsumer, , Exclusive,
                        )
                    I2cSerialBusV2 (0x004D, ControllerInitiated,
0x00061A80,
                        AddressingMode7Bit, "\\_SB.I2CD",
                        0x00, ResourceConsumer, , Exclusive,
                        )
                    GpioInt (Edge, ActiveLow, SharedAndWake, PullNone,
0x0000,
                        "\\_SB.GPIO", 0x00, ResourceConsumer, ,
                        )
                        {   // Pin list
                            0x0020
                        }
                })
                Return (RBUF) /* \_SB_.I2CD.TAS_._CRS.RBUF */
            }

            Method (_STA, 0, NotSerialized)  // _STA: Status
            {
                Return (0x0F)
            }
        }
    }


> > +#define TAS2563_REG_INIT_N 12
> 
> newline
> 
> > +static const struct reg_default tas2563_reg_init[TAS2563_MAX_CHANNELS]
> > +	[TAS2563_REG_INIT_N] = {
> > +	{
> > +		{ TAS2562_TDM_CFG2, 0x5a },
> > +		{ TAS2562_TDM_CFG4, 0xf3 },
> > +		{ TAS2562_TDM_CFG5, 0x42 },
> > +		{ TAS2562_TDM_CFG6, 0x40 },
> > +		{ TAS2562_BOOST_CFG1, 0xd4 },
> > +		{ TAS2562_BOOST_CFG3, 0xa4 },
> > +		{ TAS2562_REG(0x00, 0x36), 0x0b },
> > +		{ TAS2562_REG(0x00, 0x38), 0x21 },
> > +		{ TAS2562_REG(0x00, 0x3c), 0x58 },
> > +		{ TAS2562_BOOST_CFG4, 0xb6 },
> > +		{ TAS2562_ASI_CONFIG3, 0x04},
> > +		{ TAS2562_REG(0x00, 0x47), 0xb1 },
> 
> > +/* Update the calibrate data, including speaker impedance, f0, etc, into algo.
> 
> update the calibration data,
> 
> > + * Calibrate data is done by manufacturer in the factory. These data are used
> 
> The manufacturer calibrates the data in the factory.
> 
> > + * by Algo for calucating the speaker temperature, speaker membrance excursion
> 
> calculating
> 
> membrane
> 
> 
> > +static int tas2563_hda_i2c_probe(struct i2c_client *client)
> > +{
> > +	struct tas2563_data *tas2563;
> > +	int ret;
> > +
> > +	tas2563 = devm_kzalloc(&client->dev, sizeof(struct tas2563_data),
> > +		GFP_KERNEL);
> > +	if (!tas2563)
> > +		return -ENOMEM;
> > +	tas2563->dev = &client->dev;
> > +	tas2563->client = client;
> > +
> > +	dev_set_drvdata(tas2563->dev, tas2563);
> > +
> > +	ret = tas2563_read_acpi(tas2563);
> > +	if (ret)
> > +		return dev_err_probe(tas2563->dev, ret,
> > +			"Platform not supported\n");
> > +
> > +	for (int i = 0; i < tas2563->ndev; ++i) {
> > +		struct tas2563_dev *tasdev = &tas2563->tasdevs[i];
> > +
> > +		ret = tas2563_tasdev_read_efi(tas2563, tasdev);
> > +		if (ret)
> > +			return dev_err_probe(tas2563->dev, ret,
> > +				"Calibration data cannot be read from EFI\n");
> > +
> > +		ret = tas2563_tasdev_init_client(tas2563, tasdev);
> > +		if (ret)
> > +			return dev_err_probe(tas2563->dev, ret,
> > +				"Failed to init i2c client\n");
> > +
> > +		ret = tas2563_tasdev_init_regmap(tas2563, tasdev);
> > +		if (ret)
> > +			return dev_err_probe(tas2563->dev, ret,
> > +				"Failed to allocate register map\n");
> > +	}
> > +
> > +	ret = component_add(tas2563->dev, &tas2563_hda_comp_ops);
> > +	if (ret) {
> > +		return dev_err_probe(tas2563->dev, ret,
> > +			"Register component failed\n");
> > +	}
> 
> I wonder how many of those tests actually depend on deferred probe, and
> if this isn't a case of copy-paste "just in case"?
> 
> > +
> > +	pm_runtime_set_autosuspend_delay(tas2563->dev, 3000);
> > +	pm_runtime_use_autosuspend(tas2563->dev);
> > +	pm_runtime_mark_last_busy(tas2563->dev);
> > +	pm_runtime_set_active(tas2563->dev);
> > +	pm_runtime_get_noresume(tas2563->dev);
> > +	pm_runtime_enable(tas2563->dev);
> > +
> > +	pm_runtime_put_autosuspend(tas2563->dev);
> 
> the sequence get_noresume/enable/put_autosuspend makes no sense to me.
> doing a get_noresume *before* enable should do exactly nothing, and
> releasing the resource would already be handled with autosuspend based
> on the last_busy mark.
> 
I copied this from the tas2781-hda driver, I'll dig into this more.

> > +
> > +	return 0;
> > +}
> > +
> > +static void tas2563_hda_i2c_remove(struct i2c_client *client)
> > +{
> > +	struct tas2563_data *tas2563 = dev_get_drvdata(&client->dev);
> > +
> > +	pm_runtime_get_sync(tas2563->dev);
> > +	pm_runtime_disable(tas2563->dev);
> > +
> > +	component_del(tas2563->dev, &tas2563_hda_comp_ops);
> > +
> > +	pm_runtime_put_noidle(tas2563->dev);
> 
> that pm_runtime sequence also makes no sense to me, if you disable
> pm_runtime the last command is useless/no-op.
> 
> > +}
> > +
> > +static int tas2563_system_suspend(struct device *dev)
> > +{
> > +	struct tas2563_data *tas2563 = dev_get_drvdata(dev);
> > +	int ret;
> > +
> > +	dev_dbg(tas2563->dev, "System Suspend\n");
> > +
> > +	ret = pm_runtime_force_suspend(dev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return 0;
> > +}
> > +
> > +static int tas2563_system_resume(struct device *dev)
> > +{
> > +	int ret;
> > +	struct tas2563_data *tas2563 = dev_get_drvdata(dev);
> > +
> > +	dev_dbg(tas2563->dev, "System Resume\n");
> > +
> > +	ret = pm_runtime_force_resume(dev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	for (int i = 0; i < tas2563->ndev; ++i)
> > +		tas2563_tasdev_setup(tas2563, &tas2563->tasdevs[i]);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct dev_pm_ops tas2563_hda_pm_ops = {
> > +	SYSTEM_SLEEP_PM_OPS(tas2563_system_suspend, tas2563_system_resume)
> 
> where's the pm_runtime stuff?
> 

The amp stores its state in software shutdown mode.
The tas2563_hda_playback_hook wakes/shutdowns the amp, not the
pm_runtime.

> > +};
> 

Regards,

Gergo

Re: [PATCH 2/2] ALSA: hda/tas2563: Add tas2563 HDA driver

[Pierre-Louis Bossart]

>>> +
>>> +static void tas2563_fixup_i2c(struct hda_codec *cdc,
>>> +	const struct hda_fixup *fix, int action)
>>> +{
>>> +	 tas2xxx_generic_fixup(cdc, action, "i2c", "INT8866");
>>
>> Any specific reason to use an Intel ACPI identifier? Why not use
>> "TIAS2563" ?
>>
> INT8866 is in the ACPI.
> I don't know why Lenovo uses this name.
> I think it's more internal than intel.
> 
>    Scope (_SB.I2CD)
>     {
>         Device (TAS)
>         {
>             Name (_HID, "INT8866")  // _HID: Hardware ID

Ouch, I hope they checked with Intel that this isn't an HID already in
use...

>             Name (_UID, Zero)  // _UID: Unique ID
>             Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource
> Settings
>             {
>                 Name (RBUF, ResourceTemplate ()
>                 {
>                     I2cSerialBusV2 (0x004C, ControllerInitiated,
> 0x00061A80,
>                         AddressingMode7Bit, "\\_SB.I2CD",
>                         0x00, ResourceConsumer, , Exclusive,
>                         )
>                     I2cSerialBusV2 (0x004D, ControllerInitiated,
> 0x00061A80,
>                         AddressingMode7Bit, "\\_SB.I2CD",
>                         0x00, ResourceConsumer, , Exclusive,
>                         )
>                     GpioInt (Edge, ActiveLow, SharedAndWake, PullNone,
> 0x0000,
>                         "\\_SB.GPIO", 0x00, ResourceConsumer, ,
>                         )
>                         {   // Pin list
>                             0x0020
>                         }
>                 })
>                 Return (RBUF) /* \_SB_.I2CD.TAS_._CRS.RBUF */
>             }
> 
>             Method (_STA, 0, NotSerialized)  // _STA: Status
>             {
>                 Return (0x0F)
>             }
>         }
>     }

>>> +static int tas2563_system_resume(struct device *dev)
>>> +{
>>> +	int ret;
>>> +	struct tas2563_data *tas2563 = dev_get_drvdata(dev);
>>> +
>>> +	dev_dbg(tas2563->dev, "System Resume\n");
>>> +
>>> +	ret = pm_runtime_force_resume(dev);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	for (int i = 0; i < tas2563->ndev; ++i)
>>> +		tas2563_tasdev_setup(tas2563, &tas2563->tasdevs[i]);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static const struct dev_pm_ops tas2563_hda_pm_ops = {
>>> +	SYSTEM_SLEEP_PM_OPS(tas2563_system_suspend, tas2563_system_resume)
>>
>> where's the pm_runtime stuff?
>>
> 
> The amp stores its state in software shutdown mode.
> The tas2563_hda_playback_hook wakes/shutdowns the amp, not the
> pm_runtime.

My point was that you have all these pm_runtime_ calls in the code, but
nothing that provides pm_runtime suspend-resume functions so not sure
what exactly the result is?

Re: [PATCH 2/2] ALSA: hda/tas2563: Add tas2563 HDA driver

[Pierre-Louis Bossart]

>>>> +static const struct dev_pm_ops tas2563_hda_pm_ops = {
>>>> +	SYSTEM_SLEEP_PM_OPS(tas2563_system_suspend, tas2563_system_resume)
>>>
>>> where's the pm_runtime stuff?
>>>
>>
>> The amp stores its state in software shutdown mode.
>> The tas2563_hda_playback_hook wakes/shutdowns the amp, not the
>> pm_runtime.
> 
> My point was that you have all these pm_runtime_ calls in the code, but
> nothing that provides pm_runtime suspend-resume functions so not sure
> what exactly the result is?

if the inspiration was the tas2781, then see below it does have a
RUNTIME_PM_OPS line as well as runtime_suspend/resume routines.

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)
};

Re: [PATCH 2/2] ALSA: hda/tas2563: Add tas2563 HDA driver

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 607 bytes --]

On Tue, Dec 05, 2023 at 10:01:08AM -0600, Pierre-Louis Bossart wrote:

> >>> +static const struct dev_pm_ops tas2563_hda_pm_ops = {
> >>> +	SYSTEM_SLEEP_PM_OPS(tas2563_system_suspend, tas2563_system_resume)

> >> where's the pm_runtime stuff?

> > The amp stores its state in software shutdown mode.
> > The tas2563_hda_playback_hook wakes/shutdowns the amp, not the
> > pm_runtime.

> My point was that you have all these pm_runtime_ calls in the code, but
> nothing that provides pm_runtime suspend-resume functions so not sure
> what exactly the result is?

It *could* be ACPI doing something I guess...

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/2] ALSA: hda/tas2563: Add tas2563 HDA driver

[Gergo Koteles]

On Tue, 2023-12-05 at 10:01 -0600, Pierre-Louis Bossart wrote:
> > > > +
> > > > +static void tas2563_fixup_i2c(struct hda_codec *cdc,
> > > > +	const struct hda_fixup *fix, int action)
> > > > +{
> > > > +	 tas2xxx_generic_fixup(cdc, action, "i2c", "INT8866");
> > > 
> > > Any specific reason to use an Intel ACPI identifier? Why not use
> > > "TIAS2563" ?
> > > 
> > INT8866 is in the ACPI.
> > I don't know why Lenovo uses this name.
> > I think it's more internal than intel.
> > 
> >    Scope (_SB.I2CD)
> >     {
> >         Device (TAS)
> >         {
> >             Name (_HID, "INT8866")  // _HID: Hardware ID
> 
> Ouch, I hope they checked with Intel that this isn't an HID already in
> use...
> 
It looks the INT prefix is not reserved. (yet)
https://uefi.org/ACPI_ID_List?acpi_search=INT

> > 
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +static const struct dev_pm_ops tas2563_hda_pm_ops = {
> > > > +	SYSTEM_SLEEP_PM_OPS(tas2563_system_suspend, tas2563_system_resume)
> > > 
> > > where's the pm_runtime stuff?
> > > 
> > 
> > The amp stores its state in software shutdown mode.
> > The tas2563_hda_playback_hook wakes/shutdowns the amp, not the
> > pm_runtime.
> 
> My point was that you have all these pm_runtime_ calls in the code, but
> nothing that provides pm_runtime suspend-resume functions so not sure
> what exactly the result is?
> 
> 
I think nothing. I haven't experienced anything unusual recently.

Re: [PATCH 2/2] ALSA: hda/tas2563: Add tas2563 HDA driver

[Pierre-Louis Bossart]

>>>>> +static void tas2563_fixup_i2c(struct hda_codec *cdc,
>>>>> +	const struct hda_fixup *fix, int action)
>>>>> +{
>>>>> +	 tas2xxx_generic_fixup(cdc, action, "i2c", "INT8866");
>>>>
>>>> Any specific reason to use an Intel ACPI identifier? Why not use
>>>> "TIAS2563" ?
>>>>
>>> INT8866 is in the ACPI.
>>> I don't know why Lenovo uses this name.
>>> I think it's more internal than intel.
>>>
>>>    Scope (_SB.I2CD)
>>>     {
>>>         Device (TAS)
>>>         {
>>>             Name (_HID, "INT8866")  // _HID: Hardware ID
>>
>> Ouch, I hope they checked with Intel that this isn't an HID already in
>> use...
>>
> It looks the INT prefix is not reserved. (yet)
> https://uefi.org/ACPI_ID_List?acpi_search=INT

It's been de-facto reclaimed by Intel over the years, apparently using
INTC or INTL was too hard for some of my colleagues...

There are lots of INT devices in the kernel today, here's a small list
for sound/soc/codecs only

rt274.c:        { "INT34C2", 0 },
rt286.c:        { "INT343A", 0 },
rt298.c:        { "INT343A", 0 },
ssm4567.c:      { "INT343B", 0 },

Those INT values were added by Intel teams though, it's really odd to
see Lenovo use an INT-based HID. Should really use 104C2563 or something.


>>>>> +	return 0;
>>>>> +}
>>>>> +
>>>>> +static const struct dev_pm_ops tas2563_hda_pm_ops = {
>>>>> +	SYSTEM_SLEEP_PM_OPS(tas2563_system_suspend, tas2563_system_resume)
>>>>
>>>> where's the pm_runtime stuff?
>>>>
>>>
>>> The amp stores its state in software shutdown mode.
>>> The tas2563_hda_playback_hook wakes/shutdowns the amp, not the
>>> pm_runtime.
>>
>> My point was that you have all these pm_runtime_ calls in the code, but
>> nothing that provides pm_runtime suspend-resume functions so not sure
>> what exactly the result is?
>>
>>
> I think nothing. I haven't experienced anything unusual recently.

you can probably see from the /sys directory what the pm_runtime power
state is, most likely the status is 'unknown'.

Re: [PATCH 2/2] ALSA: hda/tas2563: Add tas2563 HDA driver

[Amadeusz Sławiński]

On 12/5/2023 6:22 PM, Pierre-Louis Bossart wrote:
> 
> 
>>>>>> +static void tas2563_fixup_i2c(struct hda_codec *cdc,
>>>>>> +	const struct hda_fixup *fix, int action)
>>>>>> +{
>>>>>> +	 tas2xxx_generic_fixup(cdc, action, "i2c", "INT8866");
>>>>>
>>>>> Any specific reason to use an Intel ACPI identifier? Why not use
>>>>> "TIAS2563" ?
>>>>>

Will just note that prefix should probably be TXNW (not TIAS) as 
discussed recently on list.

>>>> INT8866 is in the ACPI.
>>>> I don't know why Lenovo uses this name.
>>>> I think it's more internal than intel.
>>>>
>>>>     Scope (_SB.I2CD)
>>>>      {
>>>>          Device (TAS)
>>>>          {
>>>>              Name (_HID, "INT8866")  // _HID: Hardware ID
>>>
>>> Ouch, I hope they checked with Intel that this isn't an HID already in
>>> use...
>>>
>> It looks the INT prefix is not reserved. (yet)
>> https://uefi.org/ACPI_ID_List?acpi_search=INT
> 
> It's been de-facto reclaimed by Intel over the years, apparently using
> INTC or INTL was too hard for some of my colleagues...
> 

Perhaps it should be reserved then, so it is present on above list?

> There are lots of INT devices in the kernel today, here's a small list
> for sound/soc/codecs only
> 
> rt274.c:        { "INT34C2", 0 },
> rt286.c:        { "INT343A", 0 },
> rt298.c:        { "INT343A", 0 },
> ssm4567.c:      { "INT343B", 0 },
> 
> Those INT values were added by Intel teams though, it's really odd to
> see Lenovo use an INT-based HID. Should really use 104C2563 or something.

I will just note that those RT ones are used on quite old RVPs, and yes 
I would have also preferred if they had used "real" IDs, but it is 
unlikely that anyone fixes it after all this time ;).

Adding Andy to CC, as he commented recently about problematic 
assignments of ACPI IDs on this list, maybe he can shed some light on 
the "INT" prefix.

Re: [PATCH 0/2] ALSA: hda/tas2563: Add tas253 HDA driver

[Gergo Koteles]

Please disregard this patch.
The tas2781-hda driver will handle this.

https://lore.kernel.org/all/cover.1701906455.git.soyer@irl.hu/

On Tue, 2023-12-05 at 00:45 +0100, Gergo Koteles wrote:
> The ta2563 is a smart amplifier. Similar to tas2562 but with DSP. Some 
> Lenovo laptops have it to drive the bass speakers. By default, it is in 
> software shutdown state.
> 
> To make the DSP work it needs a firmware and some calibration data.
> The latter can be read from the EFI in Lenovo laptops.
> 
> For the correct configuration it needs additional register data.
> It captured after running the Windows driver.
> 
> The firmware can be extracted as TAS2563Firmware.bin from the Windows 
> driver with innoextract.
> https://download.lenovo.com/consumer/mobiles/h5yd037fbfyy7kd0.exe
> 
> The driver will search for it as TAS2563-17AA3870.bin with the 14ARB7.
> 
> It uses the default program/configuration, and has no controls for these yet.
> 
> The amplifier works without firmware, but I don't know how safe is it, 
> that's why the firmware is required.
> 
> Gergo Koteles (2):
>   ASoc: tas2563: DSP Firmware loading support
>   ALSA: hda/tas2563: Add tas2563 HDA driver
> 
>  {sound/soc/codecs => include/sound}/tas2562.h |   8 +
>  include/sound/tas25xx-dsp.h                   | 100 ++++
>  sound/pci/hda/Kconfig                         |  14 +
>  sound/pci/hda/Makefile                        |   2 +
>  sound/pci/hda/patch_realtek.c                 |  22 +-
>  sound/pci/hda/tas2563_hda_i2c.c               | 508 ++++++++++++++++++
>  sound/soc/codecs/Kconfig                      |   7 +
>  sound/soc/codecs/Makefile                     |   2 +
>  sound/soc/codecs/tas2562.c                    |   2 +-
>  sound/soc/codecs/tas25xx-dsp.c                | 282 ++++++++++
>  10 files changed, 942 insertions(+), 5 deletions(-)
>  rename {sound/soc/codecs => include/sound}/tas2562.h (90%)
>  create mode 100644 include/sound/tas25xx-dsp.h
>  create mode 100644 sound/pci/hda/tas2563_hda_i2c.c
>  create mode 100644 sound/soc/codecs/tas25xx-dsp.c
> 
> 
> base-commit: ffc253263a1375a65fa6c9f62a893e9767fbebfa

Re: [PATCH 2/2] ALSA: hda/tas2563: Add tas2563 HDA driver

[Andy Shevchenko]

On Wed, Dec 06, 2023 at 05:07:16PM +0100, Amadeusz Sławiński wrote:
> On 12/5/2023 6:22 PM, Pierre-Louis Bossart wrote:

...

> > > > > > > +	 tas2xxx_generic_fixup(cdc, action, "i2c", "INT8866");
> > > > > > 
> > > > > > Any specific reason to use an Intel ACPI identifier? Why not use
> > > > > > "TIAS2563" ?
> 
> Will just note that prefix should probably be TXNW (not TIAS) as discussed
> recently on list.

...which should come directly from TI as it's their responsibility to allocate
an ACPI ID.

...

> > > > > INT8866 is in the ACPI.
> > > > > I don't know why Lenovo uses this name.
> > > > > I think it's more internal than intel.

This is wrong (PNP) ID.

...

> > > > >              Name (_HID, "INT8866")  // _HID: Hardware ID
> > > > 
> > > > Ouch, I hope they checked with Intel that this isn't an HID already in
> > > > use...
> > > > 
> > > It looks the INT prefix is not reserved. (yet)
> > > https://uefi.org/ACPI_ID_List?acpi_search=INT

You are looking into wrong registry, and yeah, Intel used wrong PNP ID
for years...

> > It's been de-facto reclaimed by Intel over the years, apparently using
> > INTC or INTL was too hard for some of my colleagues...
> 
> Perhaps it should be reserved then, so it is present on above list?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] ALSA: hda/tas2563: Add tas2563 HDA driver

[kernel test robot]

Hi Gergo,

kernel test robot noticed the following build warnings:

[auto build test WARNING on ffc253263a1375a65fa6c9f62a893e9767fbebfa]

url:    https://github.com/intel-lab-lkp/linux/commits/Gergo-Koteles/ASoc-tas2563-DSP-Firmware-loading-support/20231205-074901
base:   ffc253263a1375a65fa6c9f62a893e9767fbebfa
patch link:    https://lore.kernel.org/r/4a2f31d4eb8479789ceb1daf2e93ec0e25c23171.1701733441.git.soyer%40irl.hu
patch subject: [PATCH 2/2] ALSA: hda/tas2563: Add tas2563 HDA driver
config: x86_64-randconfig-r121-20231210 (https://download.01.org/0day-ci/archive/20231210/202312100942.CvTnDpkg-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231210/202312100942.CvTnDpkg-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/202312100942.CvTnDpkg-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> sound/soc/codecs/tas25xx-dsp.c:21:9: sparse: sparse: restricted __be16 degrades to integer
>> sound/soc/codecs/tas25xx-dsp.c:21:9: sparse: sparse: incorrect type in initializer (different base types) @@     expected unsigned long __ms @@     got restricted __be16 [usertype] @@
   sound/soc/codecs/tas25xx-dsp.c:21:9: sparse:     expected unsigned long __ms
   sound/soc/codecs/tas25xx-dsp.c:21:9: sparse:     got restricted __be16 [usertype]
>> sound/soc/codecs/tas25xx-dsp.c:21:9: sparse: sparse: restricted __be16 degrades to integer
>> sound/soc/codecs/tas25xx-dsp.c:21:9: sparse: sparse: restricted __be16 degrades to integer
>> sound/soc/codecs/tas25xx-dsp.c:21:9: sparse: sparse: restricted __be16 degrades to integer
>> sound/soc/codecs/tas25xx-dsp.c:21:9: sparse: sparse: restricted __be16 degrades to integer
>> sound/soc/codecs/tas25xx-dsp.c:28:26: sparse: sparse: incorrect type in initializer (different base types) @@     expected int num_writes @@     got restricted __be16 [usertype] @@
   sound/soc/codecs/tas25xx-dsp.c:28:26: sparse:     expected int num_writes
   sound/soc/codecs/tas25xx-dsp.c:28:26: sparse:     got restricted __be16 [usertype]
>> sound/soc/codecs/tas25xx-dsp.c:65:33: sparse: sparse: incorrect type in argument 4 (different base types) @@     expected unsigned long [usertype] val_count @@     got restricted __be16 [usertype] @@
   sound/soc/codecs/tas25xx-dsp.c:65:33: sparse:     expected unsigned long [usertype] val_count
   sound/soc/codecs/tas25xx-dsp.c:65:33: sparse:     got restricted __be16 [usertype]
   sound/soc/codecs/tas25xx-dsp.c:84:30: sparse: sparse: restricted __be16 degrades to integer
   sound/soc/codecs/tas25xx-dsp.c:87:30: sparse: sparse: restricted __be16 degrades to integer
   sound/soc/codecs/tas25xx-dsp.c:82:17: sparse: sparse: restricted __be16 degrades to integer
   sound/soc/codecs/tas25xx-dsp.c:82:17: sparse: sparse: restricted __be16 degrades to integer
   sound/soc/codecs/tas25xx-dsp.c:82:17: sparse: sparse: restricted __be16 degrades to integer
   sound/soc/codecs/tas25xx-dsp.c:103:17: sparse: sparse: restricted __be16 degrades to integer
   sound/soc/codecs/tas25xx-dsp.c:103:17: sparse: sparse: restricted __be16 degrades to integer
   sound/soc/codecs/tas25xx-dsp.c:103:17: sparse: sparse: restricted __be16 degrades to integer
>> sound/soc/codecs/tas25xx-dsp.c:141:24: sparse: sparse: restricted __be32 degrades to integer
>> sound/soc/codecs/tas25xx-dsp.c:145:24: sparse: sparse: invalid assignment: +=
>> sound/soc/codecs/tas25xx-dsp.c:145:24: sparse:    left side has type int
>> sound/soc/codecs/tas25xx-dsp.c:145:24: sparse:    right side has type restricted __be32
   sound/soc/codecs/tas25xx-dsp.c:150:35: sparse: sparse: restricted __be32 degrades to integer
>> sound/soc/codecs/tas25xx-dsp.c:151:29: sparse: sparse: incorrect type in initializer (different base types) @@     expected int num_subblocks @@     got restricted __be32 [usertype] @@
   sound/soc/codecs/tas25xx-dsp.c:151:29: sparse:     expected int num_subblocks
   sound/soc/codecs/tas25xx-dsp.c:151:29: sparse:     got restricted __be32 [usertype]
   sound/soc/codecs/tas25xx-dsp.c:182:26: sparse: sparse: restricted __be32 degrades to integer
   sound/soc/codecs/tas25xx-dsp.c:186:24: sparse: sparse: invalid assignment: +=
   sound/soc/codecs/tas25xx-dsp.c:186:24: sparse:    left side has type int
   sound/soc/codecs/tas25xx-dsp.c:186:24: sparse:    right side has type restricted __be32
   sound/soc/codecs/tas25xx-dsp.c:191:35: sparse: sparse: restricted __be32 degrades to integer
   sound/soc/codecs/tas25xx-dsp.c:192:29: sparse: sparse: incorrect type in initializer (different base types) @@     expected int num_subblocks @@     got restricted __be32 [usertype] @@
   sound/soc/codecs/tas25xx-dsp.c:192:29: sparse:     expected int num_subblocks
   sound/soc/codecs/tas25xx-dsp.c:192:29: sparse:     got restricted __be32 [usertype]
   sound/soc/codecs/tas25xx-dsp.c:238:29: sparse: sparse: restricted __be32 degrades to integer
   sound/soc/codecs/tas25xx-dsp.c:239:31: sparse: sparse: invalid assignment: +=
>> sound/soc/codecs/tas25xx-dsp.c:239:31: sparse:    left side has type unsigned int
   sound/soc/codecs/tas25xx-dsp.c:239:31: sparse:    right side has type restricted __be32
   sound/soc/codecs/tas25xx-dsp.c:241:29: sparse: sparse: restricted __be32 degrades to integer
   sound/soc/codecs/tas25xx-dsp.c:242:33: sparse: sparse: invalid assignment: +=
   sound/soc/codecs/tas25xx-dsp.c:242:33: sparse:    left side has type unsigned int
   sound/soc/codecs/tas25xx-dsp.c:242:33: sparse:    right side has type restricted __be32
>> sound/soc/codecs/tas25xx-dsp.c:260:18: sparse: sparse: incorrect type in assignment (different base types) @@     expected unsigned int [usertype] prog_num @@     got restricted __be32 [usertype] @@
   sound/soc/codecs/tas25xx-dsp.c:260:18: sparse:     expected unsigned int [usertype] prog_num
   sound/soc/codecs/tas25xx-dsp.c:260:18: sparse:     got restricted __be32 [usertype]
>> sound/soc/codecs/tas25xx-dsp.c:261:20: sparse: sparse: incorrect type in assignment (different base types) @@     expected unsigned int [usertype] config_num @@     got restricted __be32 [usertype] @@
   sound/soc/codecs/tas25xx-dsp.c:261:20: sparse:     expected unsigned int [usertype] config_num
   sound/soc/codecs/tas25xx-dsp.c:261:20: sparse:     got restricted __be32 [usertype]
--
>> sound/pci/hda/tas2563_hda_i2c.c:325:37: sparse: sparse: incorrect type in assignment (different base types) @@     expected unsigned int @@     got restricted __be32 [usertype] @@
   sound/pci/hda/tas2563_hda_i2c.c:325:37: sparse:     expected unsigned int
   sound/pci/hda/tas2563_hda_i2c.c:325:37: sparse:     got restricted __be32 [usertype]

vim +325 sound/pci/hda/tas2563_hda_i2c.c

   306	
   307	/* Update the calibrate data, including speaker impedance, f0, etc, into algo.
   308	 * Calibrate data is done by manufacturer in the factory. These data are used
   309	 * by Algo for calucating the speaker temperature, speaker membrance excursion
   310	 * and f0 in real time during playback.
   311	 */
   312	static int tas2563_tasdev_read_efi(struct tas2563_data *tas2563,
   313		struct tas2563_dev *tasdev)
   314	{
   315		efi_status_t status;
   316		unsigned int attr;
   317		unsigned long max_size = TAS2563_CAL_DATA_SIZE;
   318	
   319		for (int i = 0; i < TAS2563_CAL_N; ++i) {
   320			status = efi.get_variable(efi_var_names[tasdev->dev_id][i],
   321				&efi_guid, &attr, &max_size,
   322				&tasdev->cal_data[i]);
   323			if (status != EFI_SUCCESS)
   324				return -EINVAL;
 > 325			tasdev->cal_data[i] = cpu_to_be32(tasdev->cal_data[i]);
   326		}
   327	
   328		dev_info(tas2563->dev,
   329			"Calibration data %d: power:%08x r0:%08x inv_r0:%08x r0_low:%08x tlim:%08x\n",
   330			tasdev->dev_id, tasdev->cal_data[0], tasdev->cal_data[1],
   331			tasdev->cal_data[2], tasdev->cal_data[3], tasdev->cal_data[4]);
   332	
   333		return 0;
   334	}
   335	

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