diff mbox series

[v3,4/5] ALSA: hda/tas2781: Add tas2781 HDA driver

Message ID 20230519080227.20224-1-13916275206@139.com (mailing list archive)
State Superseded
Headers show
Series [v3,1/5] ASoC: tas2781: Add Header file for tas2781 driver | expand

Commit Message

Shenghao Ding May 19, 2023, 8:02 a.m. UTC
Create tas2781 HDA driver.

Signed-off-by: Shenghao Ding <13916275206@139.com>

---
Changes in v3:
 - fixed issue | Reported-by: kernel test robot <lkp@intel.com>
   | Link: https://lore.kernel.org/oe-kbuild-all/202305022033.LiI7Ojm4-lkp@intel.com/
 Changes to be committed:
	modified:   sound/pci/hda/Kconfig
	modified:   sound/pci/hda/Makefile
	modified:   sound/pci/hda/patch_realtek.c
	new file:   sound/pci/hda/tas2781_hda_i2c.c
---
 sound/pci/hda/Kconfig           |  15 +
 sound/pci/hda/Makefile          |   2 +
 sound/pci/hda/patch_realtek.c   | 106 ++++-
 sound/pci/hda/tas2781_hda_i2c.c | 806 ++++++++++++++++++++++++++++++++
 4 files changed, 926 insertions(+), 3 deletions(-)
 create mode 100644 sound/pci/hda/tas2781_hda_i2c.c

Comments

Takashi Iwai May 21, 2023, 8:02 a.m. UTC | #1
On Fri, 19 May 2023 10:02:27 +0200,
Shenghao Ding wrote:
> 
> Create tas2781 HDA driver.
> 
> Signed-off-by: Shenghao Ding <13916275206@139.com>

First of all, please give more description.  It's far more changes
than written in four words.

Also, don't forget to put me on Cc.  I almost overlooked this one.

> diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
> index 172ffc2c332b..f5b912f90018 100644
> --- a/sound/pci/hda/patch_realtek.c
> +++ b/sound/pci/hda/patch_realtek.c
> +static int comp_match_tas2781_dev_name(struct device *dev, void *data)
> +{
> +	struct scodec_dev_name *p = data;
> +	const char *d = dev_name(dev);
> +	int n = strlen(p->bus);
> +	char tmp[32];
> +
> +	/* check the bus name */
> +	if (strncmp(d, p->bus, n))
> +		return 0;
> +	/* skip the bus number */
> +	if (isdigit(d[n]))
> +		n++;
> +	/* the rest must be exact matching */
> +	snprintf(tmp, sizeof(tmp), "-%s:00", p->hid);
> +
> +	return !strcmp(d + n, tmp);
> +}

You don't use the index here...

> +static void tas2781_generic_fixup(struct hda_codec *cdc, int action,
> +	const char *bus, const char *hid, int count)
> +{
> +	struct device *dev = hda_codec_dev(cdc);
> +	struct alc_spec *spec = cdc->spec;
> +	struct scodec_dev_name *rec;
> +	int ret, i;
> +
> +	switch (action) {
> +	case HDA_FIXUP_ACT_PRE_PROBE:
> +		for (i = 0; i < count; i++) {
> +			rec = devm_kmalloc(dev, sizeof(*rec), GFP_KERNEL);
> +			if (!rec)
> +				return;
> +			rec->bus = bus;
> +			rec->hid = hid;
> +			rec->index = i;

... and assigning here.  It means that the multiple instances would
silently break.  It's better to catch here instead.

> +static void tas2781_fixup_i2c(struct hda_codec *cdc,
> +	const struct hda_fixup *fix, int action)
> +{
> +	 tas2781_generic_fixup(cdc, action, "i2c", "TIAS2781", 1);
> +}
> +
>  /* for alc295_fixup_hp_top_speakers */
>  #include "hp_x360_helper.c"
>  
> @@ -7201,6 +7260,8 @@ enum {
>  	ALC287_FIXUP_YOGA9_14IAP7_BASS_SPK_PIN,
>  	ALC295_FIXUP_DELL_INSPIRON_TOP_SPEAKERS,
>  	ALC236_FIXUP_DELL_DUAL_CODECS,
> +	ALC287_FIXUP_TAS2781_I2C_2,
> +	ALC287_FIXUP_TAS2781_I2C_4
>  };
>  
>  /* A special fixup for Lenovo C940 and Yoga Duet 7;
> @@ -9189,6 +9250,18 @@ static const struct hda_fixup alc269_fixups[] = {
>  		.chained = true,
>  		.chain_id = ALC255_FIXUP_DELL1_MIC_NO_PRESENCE,
>  	},
> +	[ALC287_FIXUP_TAS2781_I2C_2] = {
> +		.type = HDA_FIXUP_FUNC,
> +		.v.func = tas2781_fixup_i2c,
> +		.chained = true,
> +		.chain_id = ALC269_FIXUP_THINKPAD_ACPI,
> +	},
> +	[ALC287_FIXUP_TAS2781_I2C_4] = {
> +		.type = HDA_FIXUP_FUNC,
> +		.v.func = tas2781_fixup_i2c,
> +		.chained = true,
> +		.chain_id = ALC269_FIXUP_THINKPAD_ACPI,
> +	},

What's a difference between *_2 and *_4?

> --- /dev/null
> +++ b/sound/pci/hda/tas2781_hda_i2c.c
> +static int tas2781_acpi_get_i2c_resource(struct acpi_resource
> +	*ares, void *data)
> +{
> +	struct tasdevice_priv *tas_priv = (struct tasdevice_priv *)data;
> +	struct acpi_resource_i2c_serialbus *sb;
> +
> +	if (i2c_acpi_get_i2c_resource(ares, &sb)) {
> +		if (sb->slave_address != TAS2781_GLOBAL_ADDR) {
> +			tas_priv->tasdevice[tas_priv->ndev].dev_addr =
> +				(unsigned int) sb->slave_address;
> +			tas_priv->ndev++;
> +		} else
> +			tas_priv->glb_addr.dev_addr = TAS2781_GLOBAL_ADDR;
> +

Did you run checkpatch.pl?  I thought it would complain.

> +static void tas2781_hda_playback_hook(struct device *dev, int action)
> +{
> +	struct tasdevice_priv *tas_priv = dev_get_drvdata(dev);
> +	int ret = 0;
> +
> +	dev_info(tas_priv->dev, "%s: action = %d\n", __func__, action);

Don't use dev_info().  It'd be dev_dbg() at most.

> +	switch (action) {
> +	case HDA_GEN_PCM_ACT_OPEN:
> +		pm_runtime_get_sync(dev);
> +		mutex_lock(&tas_priv->codec_lock);
> +		tas_priv->cur_conf = 0;
> +		tas_priv->rcabin.profile_cfg_id = 1;
> +		tasdevice_tuning_switch(tas_priv, 0);
> +		mutex_unlock(&tas_priv->codec_lock);
> +		break;
> +	case HDA_GEN_PCM_ACT_CLOSE:
> +		mutex_lock(&tas_priv->codec_lock);
> +		tasdevice_tuning_switch(tas_priv, 1);
> +		mutex_unlock(&tas_priv->codec_lock);
> +
> +		pm_runtime_mark_last_busy(dev);
> +		pm_runtime_put_autosuspend(dev);
> +		break;
> +	default:
> +		dev_warn(tas_priv->dev, "Playback action not supported: %d\n",
> +			action);
> +		break;
> +	}
> +
> +	if (ret)
> +		dev_err(tas_priv->dev, "Regmap access fail: %d\n", ret);

The ret is never used.

> +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);
> +
> +	tas_priv->rcabin.profile_cfg_id = ucontrol->value.integer.value[0];
> +
> +	return 1;

It should return 0 if the value is unchanged.
(Ditto for other *_put functions)

> +static int tasdevice_create_control(struct tasdevice_priv *tas_priv)
> +{
> +	char prof_ctrl_name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
> +	struct hda_codec *codec = tas_priv->codec;
> +	struct snd_kcontrol_new prof_ctrl = {
> +		.name = prof_ctrl_name,
> +		.iface = SNDRV_CTL_ELEM_IFACE_CARD,
> +		.info = tasdevice_info_profile,
> +		.get = tasdevice_get_profile_id,
> +		.put = tasdevice_set_profile_id,
> +	};
> +	int ret;
> +
> +	/* Create a mixer item for selecting the active profile */
> +	scnprintf(prof_ctrl_name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN,
> +		"tasdev-profile-id");

A too bad name as a control element.  Use a more readable one.

> +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 = (int)tas_fw->nr_programs;

The cast is superfluous.

> +static int tasdevice_info_configurations(
> +	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 = (int)tas_fw->nr_configurations - 1;

Ditto.

> +/**
> + * 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.
> + */

You shouldn't use the kerneldoc marker "/**" for local static
functions.  It's not a part of API.

> +static int tasdevice_dsp_create_ctrls(struct tasdevice_priv
> +	*tas_priv)
> +{
> +	char prog_name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
> +	char conf_name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
> +	struct hda_codec *codec = tas_priv->codec;
> +	struct snd_kcontrol_new prog_ctl = {
> +		.name = prog_name,
> +		.iface = SNDRV_CTL_ELEM_IFACE_CARD,
> +		.info = tasdevice_info_programs,
> +		.get = tasdevice_program_get,
> +		.put = tasdevice_program_put,
> +	};
> +	struct snd_kcontrol_new conf_ctl = {
> +		.name = conf_name,
> +		.iface = SNDRV_CTL_ELEM_IFACE_CARD,
> +		.info = tasdevice_info_configurations,
> +		.get = tasdevice_config_get,
> +		.put = tasdevice_config_put,
> +	};
> +	int ret;
> +
> +	scnprintf(prog_name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN, "tasdev-prog-id");
> +	scnprintf(conf_name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN, "tasdev-conf-id");

Please use better names.

> +static void tas2781_apply_calib(struct tasdevice_priv *tas_priv)
> +{
> +	unsigned char page_array[CALIB_MAX] = {0x17, 0x18, 0x18, 0x0d, 0x18};
> +	unsigned char rgno_array[CALIB_MAX] = {0x74, 0x0c, 0x14, 0x3c, 0x7c};

Should be static const arrays.

> +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 hda_codec *codec = tas_priv->codec;
> +	unsigned int subid = codec->core.subsystem_id & 0xFFFF;
> +	struct tm *tm = &tas_priv->tm;
> +	unsigned int attr, crc;
> +	unsigned int *tmp_val;
> +	efi_status_t status;
> +	int ret = 0;
> +
> +	//Lenovo devices
> +	if ((subid == 0x387d) || (subid == 0x387e) || (subid == 0x3881)
> +		|| (subid == 0x3884) || (subid == 0x3886) || (subid == 0x38a7)
> +		|| (subid == 0x38a8) || (subid == 0x38ba) || (subid == 0x38bb)
> +		|| (subid == 0x38be) || (subid == 0x38bf) || (subid == 0x38c3)
> +		|| (subid == 0x38cb) || (subid == 0x38cd))
> +		efi_guid = EFI_GUID(0x1f52d2a1, 0xbb3a, 0x457d, 0xbc, 0x09,
> +			0x43, 0xa3, 0xf4, 0x31, 0x0a, 0x92);

Here can be a problem: the device ID is embedded here, and it's hard
to find out.  You'd better to make it some quirk flag that is set in a
common place and check the flag here instead of checking ID at each
place.

> +	crc = crc32(~0, tas_priv->cali_data.data, 84) ^ ~0;
> +	dev_info(tas_priv->dev, "cali crc 0x%08x PK tmp_val 0x%08x\n",
> +		crc, tmp_val[21]);

If it's a dev_info() output, make it more understandable.

> +	if (crc == tmp_val[21]) {
> +		time64_to_tm(tmp_val[20], 0, tm);
> +		dev_info(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);

Ditto.  Or, make them a debug print instead.

> +static int tas2781_runtime_suspend(struct device *dev)
> +{
> +	struct tasdevice_priv *tas_priv = dev_get_drvdata(dev);
> +	int i, ret = 0;
> +
> +	dev_info(tas_priv->dev, "Runtime Suspend\n");

It must be a debug print.  Otherwise it'll be too annoying.

Also, as a minor nitpicking, there are many functions that set ret = 0
at the beginning but never used.  The unconditional 0 initialization
is often a bad sign indicating that the author doesn't think fully of
the code flow.  Please revisit those.


thanks,

Takashi
Shenghao Ding May 23, 2023, 11:22 a.m. UTC | #2
-----Original Message-----
From: Takashi Iwai <tiwai@suse.de> 
Sent: Sunday, May 21, 2023 4:03 PM
To: Shenghao Ding <13916275206@139.com>
Cc: broonie@kernel.org; devicetree@vger.kernel.org; krzysztof.kozlowski+dt@linaro.org; robh+dt@kernel.org; lgirdwood@gmail.com; perex@perex.cz; pierre-louis.bossart@linux.intel.com; Lu, Kevin <kevin-lu@ti.com>; Ding, Shenghao <shenghao-ding@ti.com>; alsa-devel@alsa-project.org; linux-kernel@vger.kernel.org; Xu, Baojun <x1077012@ti.com>; Gupta, Peeyush <peeyush@ti.com>; Navada Kanyana, Mukund <navada@ti.com>; gentuser@gmail.com; Ryan_Chu@wistron.com; Sam_Wu@wistron.com
Subject: [EXTERNAL] Re: [PATCH v3 4/5] ALSA: hda/tas2781: Add tas2781 HDA driver

On Fri, 19 May 2023 10:02:27 +0200,
Shenghao Ding wrote:
> 
> Create tas2781 HDA driver.
> 
> Signed-off-by: Shenghao Ding <13916275206@139.com>

First of all, please give more description.  It's far more changes than written in four words.

Also, don't forget to put me on Cc.  I almost overlooked this one.

> diff --git a/sound/pci/hda/patch_realtek.c 
> b/sound/pci/hda/patch_realtek.c index 172ffc2c332b..f5b912f90018 
> 100644
> --- a/sound/pci/hda/patch_realtek.c
> +++ b/sound/pci/hda/patch_realtek.c
> +static int comp_match_tas2781_dev_name(struct device *dev, void 
> +*data) {
> +	struct scodec_dev_name *p = data;
> +	const char *d = dev_name(dev);
> +	int n = strlen(p->bus);
> +	char tmp[32];
> +
> +	/* check the bus name */
> +	if (strncmp(d, p->bus, n))
> +		return 0;
> +	/* skip the bus number */
> +	if (isdigit(d[n]))
> +		n++;
> +	/* the rest must be exact matching */
> +	snprintf(tmp, sizeof(tmp), "-%s:00", p->hid);
> +
> +	return !strcmp(d + n, tmp);
> +}

You don't use the index here...
Accepted
> +static void tas2781_generic_fixup(struct hda_codec *cdc, int action,
> +	const char *bus, const char *hid, int count) {
> +	struct device *dev = hda_codec_dev(cdc);
> +	struct alc_spec *spec = cdc->spec;
> +	struct scodec_dev_name *rec;
> +	int ret, i;
> +
> +	switch (action) {
> +	case HDA_FIXUP_ACT_PRE_PROBE:
> +		for (i = 0; i < count; i++) {
> +			rec = devm_kmalloc(dev, sizeof(*rec), GFP_KERNEL);
> +			if (!rec)
> +				return;
> +			rec->bus = bus;
> +			rec->hid = hid;
> +			rec->index = i;

... and assigning here.  It means that the multiple instances would silently break.  It's better to catch here instead.
Accepted
> +static void tas2781_fixup_i2c(struct hda_codec *cdc,
> +	const struct hda_fixup *fix, int action) {
> +	 tas2781_generic_fixup(cdc, action, "i2c", "TIAS2781", 1); }
> +
>  /* for alc295_fixup_hp_top_speakers */  #include "hp_x360_helper.c"
>  
> @@ -7201,6 +7260,8 @@ enum {
>  	ALC287_FIXUP_YOGA9_14IAP7_BASS_SPK_PIN,
>  	ALC295_FIXUP_DELL_INSPIRON_TOP_SPEAKERS,
>  	ALC236_FIXUP_DELL_DUAL_CODECS,
> +	ALC287_FIXUP_TAS2781_I2C_2,
> +	ALC287_FIXUP_TAS2781_I2C_4
>  };
>  
>  /* A special fixup for Lenovo C940 and Yoga Duet 7; @@ -9189,6 
> +9250,18 @@ static const struct hda_fixup alc269_fixups[] = {
>  		.chained = true,
>  		.chain_id = ALC255_FIXUP_DELL1_MIC_NO_PRESENCE,
>  	},
> +	[ALC287_FIXUP_TAS2781_I2C_2] = {
> +		.type = HDA_FIXUP_FUNC,
> +		.v.func = tas2781_fixup_i2c,
> +		.chained = true,
> +		.chain_id = ALC269_FIXUP_THINKPAD_ACPI,
> +	},
> +	[ALC287_FIXUP_TAS2781_I2C_4] = {
> +		.type = HDA_FIXUP_FUNC,
> +		.v.func = tas2781_fixup_i2c,
> +		.chained = true,
> +		.chain_id = ALC269_FIXUP_THINKPAD_ACPI,
> +	},

What's a difference between *_2 and *_4?
Combine them into ALC287_FIXUP_TAS2781_I2C
> --- /dev/null
> +++ b/sound/pci/hda/tas2781_hda_i2c.c
> +static int tas2781_acpi_get_i2c_resource(struct acpi_resource
> +	*ares, void *data)
> +{
> +	struct tasdevice_priv *tas_priv = (struct tasdevice_priv *)data;
> +	struct acpi_resource_i2c_serialbus *sb;
> +
> +	if (i2c_acpi_get_i2c_resource(ares, &sb)) {
> +		if (sb->slave_address != TAS2781_GLOBAL_ADDR) {
> +			tas_priv->tasdevice[tas_priv->ndev].dev_addr =
> +				(unsigned int) sb->slave_address;
> +			tas_priv->ndev++;
> +		} else
> +			tas_priv->glb_addr.dev_addr = TAS2781_GLOBAL_ADDR;
> +

Did you run checkpatch.pl?  I thought it would complain.
Accept.
> +static void tas2781_hda_playback_hook(struct device *dev, int action) 
> +{
> +	struct tasdevice_priv *tas_priv = dev_get_drvdata(dev);
> +	int ret = 0;
> +
> +	dev_info(tas_priv->dev, "%s: action = %d\n", __func__, action);

Don't use dev_info().  It'd be dev_dbg() at most.
Accept.
> +	switch (action) {
> +	case HDA_GEN_PCM_ACT_OPEN:
> +		pm_runtime_get_sync(dev);
> +		mutex_lock(&tas_priv->codec_lock);
> +		tas_priv->cur_conf = 0;
> +		tas_priv->rcabin.profile_cfg_id = 1;
> +		tasdevice_tuning_switch(tas_priv, 0);
> +		mutex_unlock(&tas_priv->codec_lock);
> +		break;
> +	case HDA_GEN_PCM_ACT_CLOSE:
> +		mutex_lock(&tas_priv->codec_lock);
> +		tasdevice_tuning_switch(tas_priv, 1);
> +		mutex_unlock(&tas_priv->codec_lock);
> +
> +		pm_runtime_mark_last_busy(dev);
> +		pm_runtime_put_autosuspend(dev);
> +		break;
> +	default:
> +		dev_warn(tas_priv->dev, "Playback action not supported: %d\n",
> +			action);
> +		break;
> +	}
> +
> +	if (ret)
> +		dev_err(tas_priv->dev, "Regmap access fail: %d\n", ret);

The ret is never used.
Accept.
> +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);
> +
> +	tas_priv->rcabin.profile_cfg_id = ucontrol->value.integer.value[0];
> +
> +	return 1;

It should return 0 if the value is unchanged.
(Ditto for other *_put functions)
Accept.
> +static int tasdevice_create_control(struct tasdevice_priv *tas_priv) 
> +{
> +	char prof_ctrl_name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
> +	struct hda_codec *codec = tas_priv->codec;
> +	struct snd_kcontrol_new prof_ctrl = {
> +		.name = prof_ctrl_name,
> +		.iface = SNDRV_CTL_ELEM_IFACE_CARD,
> +		.info = tasdevice_info_profile,
> +		.get = tasdevice_get_profile_id,
> +		.put = tasdevice_set_profile_id,
> +	};
> +	int ret;
> +
> +	/* Create a mixer item for selecting the active profile */
> +	scnprintf(prof_ctrl_name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN,
> +		"tasdev-profile-id");

A too bad name as a control element.  Use a more readable one.
Accept.
> +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 = (int)tas_fw->nr_programs;

The cast is superfluous.
Accept.
> +static int tasdevice_info_configurations(
> +	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 = (int)tas_fw->nr_configurations - 1;

Ditto.
Accept.
> +/**
> + * 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.
> + */

You shouldn't use the kerneldoc marker "/**" for local static functions.  It's not a part of API.
Accept.
> +static int tasdevice_dsp_create_ctrls(struct tasdevice_priv
> +	*tas_priv)
> +{
> +	char prog_name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
> +	char conf_name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
> +	struct hda_codec *codec = tas_priv->codec;
> +	struct snd_kcontrol_new prog_ctl = {
> +		.name = prog_name,
> +		.iface = SNDRV_CTL_ELEM_IFACE_CARD,
> +		.info = tasdevice_info_programs,
> +		.get = tasdevice_program_get,
> +		.put = tasdevice_program_put,
> +	};
> +	struct snd_kcontrol_new conf_ctl = {
> +		.name = conf_name,
> +		.iface = SNDRV_CTL_ELEM_IFACE_CARD,
> +		.info = tasdevice_info_configurations,
> +		.get = tasdevice_config_get,
> +		.put = tasdevice_config_put,
> +	};
> +	int ret;
> +
> +	scnprintf(prog_name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN, "tasdev-prog-id");
> +	scnprintf(conf_name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN, 
> +"tasdev-conf-id");

Please use better names.
Accept.
> +static void tas2781_apply_calib(struct tasdevice_priv *tas_priv) {
> +	unsigned char page_array[CALIB_MAX] = {0x17, 0x18, 0x18, 0x0d, 0x18};
> +	unsigned char rgno_array[CALIB_MAX] = {0x74, 0x0c, 0x14, 0x3c, 
> +0x7c};

Should be static const arrays.
Accept.
> +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 hda_codec *codec = tas_priv->codec;
> +	unsigned int subid = codec->core.subsystem_id & 0xFFFF;
> +	struct tm *tm = &tas_priv->tm;
> +	unsigned int attr, crc;
> +	unsigned int *tmp_val;
> +	efi_status_t status;
> +	int ret = 0;
> +
> +	//Lenovo devices
> +	if ((subid == 0x387d) || (subid == 0x387e) || (subid == 0x3881)
> +		|| (subid == 0x3884) || (subid == 0x3886) || (subid == 0x38a7)
> +		|| (subid == 0x38a8) || (subid == 0x38ba) || (subid == 0x38bb)
> +		|| (subid == 0x38be) || (subid == 0x38bf) || (subid == 0x38c3)
> +		|| (subid == 0x38cb) || (subid == 0x38cd))
> +		efi_guid = EFI_GUID(0x1f52d2a1, 0xbb3a, 0x457d, 0xbc, 0x09,
> +			0x43, 0xa3, 0xf4, 0x31, 0x0a, 0x92);

Here can be a problem: the device ID is embedded here, and it's hard to find out.  You'd better to make it some quirk flag that is set in a common place and check the flag here instead of checking ID at each place.

Do you have example of the solution? I found some quirk flag is static in the patch_realtek.c, can't be accessed outside that file.

> +	crc = crc32(~0, tas_priv->cali_data.data, 84) ^ ~0;
> +	dev_info(tas_priv->dev, "cali crc 0x%08x PK tmp_val 0x%08x\n",
> +		crc, tmp_val[21]);

If it's a dev_info() output, make it more understandable.
I'll fix it
> +	if (crc == tmp_val[21]) {
> +		time64_to_tm(tmp_val[20], 0, tm);
> +		dev_info(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);

Ditto.  Or, make them a debug print instead.
Accepted
> +static int tas2781_runtime_suspend(struct device *dev) {
> +	struct tasdevice_priv *tas_priv = dev_get_drvdata(dev);
> +	int i, ret = 0;
> +
> +	dev_info(tas_priv->dev, "Runtime Suspend\n");

It must be a debug print.  Otherwise it'll be too annoying.
Accepted
Also, as a minor nitpicking, there are many functions that set ret = 0 at the beginning but never used.  The unconditional 0 initialization is often a bad sign indicating that the author doesn't think fully of the code flow.  Please revisit those.


thanks,

Takashi
Takashi Iwai May 23, 2023, 11:42 a.m. UTC | #3
On Tue, 23 May 2023 13:22:03 +0200,
Ding, Shenghao wrote:
> 
> > +	[ALC287_FIXUP_TAS2781_I2C_2] = {
> > +		.type = HDA_FIXUP_FUNC,
> > +		.v.func = tas2781_fixup_i2c,
> > +		.chained = true,
> > +		.chain_id = ALC269_FIXUP_THINKPAD_ACPI,
> > +	},
> > +	[ALC287_FIXUP_TAS2781_I2C_4] = {
> > +		.type = HDA_FIXUP_FUNC,
> > +		.v.func = tas2781_fixup_i2c,
> > +		.chained = true,
> > +		.chain_id = ALC269_FIXUP_THINKPAD_ACPI,
> > +	},
> 
> What's a difference between *_2 and *_4?
> Combine them into ALC287_FIXUP_TAS2781_I2C

Hm, so there is no difference in stereo and quad speakers?

> > +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 hda_codec *codec = tas_priv->codec;
> > +	unsigned int subid = codec->core.subsystem_id & 0xFFFF;
> > +	struct tm *tm = &tas_priv->tm;
> > +	unsigned int attr, crc;
> > +	unsigned int *tmp_val;
> > +	efi_status_t status;
> > +	int ret = 0;
> > +
> > +	//Lenovo devices
> > +	if ((subid == 0x387d) || (subid == 0x387e) || (subid == 0x3881)
> > +		|| (subid == 0x3884) || (subid == 0x3886) || (subid == 0x38a7)
> > +		|| (subid == 0x38a8) || (subid == 0x38ba) || (subid == 0x38bb)
> > +		|| (subid == 0x38be) || (subid == 0x38bf) || (subid == 0x38c3)
> > +		|| (subid == 0x38cb) || (subid == 0x38cd))
> > +		efi_guid = EFI_GUID(0x1f52d2a1, 0xbb3a, 0x457d, 0xbc, 0x09,
> > +			0x43, 0xa3, 0xf4, 0x31, 0x0a, 0x92);
> 
> Here can be a problem: the device ID is embedded here, and it's hard to find out.  You'd better to make it some quirk flag that is set in a common place and check the flag here instead of checking ID at each place.
> 
> Do you have example of the solution? I found some quirk flag is static in the patch_realtek.c, can't be accessed outside that file.

You may store some values in struct hda_component, I suppose?

BTW, please try to fix your mailer to do citation more correctly...


thanks,

Takashi
Shenghao Ding May 25, 2023, 12:53 p.m. UTC | #4
> -----Original Message-----
> From: Takashi Iwai <tiwai@suse.de>
> Sent: Tuesday, May 23, 2023 7:43 PM
> To: Ding, Shenghao <shenghao-ding@ti.com>
> Cc: Shenghao Ding <13916275206@139.com>; broonie@kernel.org;
> devicetree@vger.kernel.org; krzysztof.kozlowski+dt@linaro.org;
> robh+dt@kernel.org; lgirdwood@gmail.com; perex@perex.cz; pierre-
> louis.bossart@linux.intel.com; Lu, Kevin <kevin-lu@ti.com>; alsa-
> devel@alsa-project.org; linux-kernel@vger.kernel.org; Xu, Baojun
> <x1077012@ti.com>; Gupta, Peeyush <peeyush@ti.com>; Navada Kanyana,
> Mukund <navada@ti.com>; gentuser@gmail.com; Ryan_Chu@wistron.com;
> Sam_Wu@wistron.com
> Subject: Re: [EXTERNAL] Re: [PATCH v3 4/5] ALSA: hda/tas2781: Add tas2781
> HDA driver
> 
> On Tue, 23 May 2023 13:22:03 +0200,
> Ding, Shenghao wrote:
> >
> > > +	[ALC287_FIXUP_TAS2781_I2C_2] = {
> > > +		.type = HDA_FIXUP_FUNC,
> > > +		.v.func = tas2781_fixup_i2c,
> > > +		.chained = true,
> > > +		.chain_id = ALC269_FIXUP_THINKPAD_ACPI,
> > > +	},
> > > +	[ALC287_FIXUP_TAS2781_I2C_4] = {
> > > +		.type = HDA_FIXUP_FUNC,
> > > +		.v.func = tas2781_fixup_i2c,
> > > +		.chained = true,
> > > +		.chain_id = ALC269_FIXUP_THINKPAD_ACPI,
> > > +	},
> >
> > What's a difference between *_2 and *_4?
> > Combine them into ALC287_FIXUP_TAS2781_I2C
> 
> Hm, so there is no difference in stereo and quad speakers?
Yes, our firmware defines the stereo or quad speaker
> 
> > > +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 hda_codec *codec = tas_priv->codec;
> > > +	unsigned int subid = codec->core.subsystem_id & 0xFFFF;
> > > +	struct tm *tm = &tas_priv->tm;
> > > +	unsigned int attr, crc;
> > > +	unsigned int *tmp_val;
> > > +	efi_status_t status;
> > > +	int ret = 0;
> > > +
> > > +	//Lenovo devices
> > > +	if ((subid == 0x387d) || (subid == 0x387e) || (subid == 0x3881)
> > > +		|| (subid == 0x3884) || (subid == 0x3886) || (subid == 0x38a7)
> > > +		|| (subid == 0x38a8) || (subid == 0x38ba) || (subid ==
> 0x38bb)
> > > +		|| (subid == 0x38be) || (subid == 0x38bf) || (subid == 0x38c3)
> > > +		|| (subid == 0x38cb) || (subid == 0x38cd))
> > > +		efi_guid = EFI_GUID(0x1f52d2a1, 0xbb3a, 0x457d, 0xbc, 0x09,
> > > +			0x43, 0xa3, 0xf4, 0x31, 0x0a, 0x92);
> >
> > Here can be a problem: the device ID is embedded here, and it's hard to
> find out.  You'd better to make it some quirk flag that is set in a common
> place and check the flag here instead of checking ID at each place.
> >
> > Do you have example of the solution? I found some quirk flag is static in
> the patch_realtek.c, can't be accessed outside that file.
> 
> You may store some values in struct hda_component, I suppose?
I will try it.
> 
> BTW, please try to fix your mailer to do citation more correctly...
accept
> 
> 
> thanks,
> 
> Takashi
diff mbox series

Patch

diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig
index 886255a03e8b..e66257277492 100644
--- a/sound/pci/hda/Kconfig
+++ b/sound/pci/hda/Kconfig
@@ -130,6 +130,21 @@  config SND_HDA_SCODEC_CS35L41_SPI
 comment "Set to Y if you want auto-loading the side codec driver"
 	depends on SND_HDA=y && SND_HDA_SCODEC_CS35L41_SPI=m
 
+config SND_HDA_SCODEC_TAS2781_I2C
+	tristate "Build TAS2781 HD-audio side codec support for I2C Bus"
+	depends on I2C
+	depends on ACPI
+	depends on SND_SOC
+	select SND_SOC_TAS2781_COMLIB
+	select SND_SOC_TAS2781_FMWLIB
+	select CRC32_SARWATE
+	help
+	  Say Y or M here to include TAS2781 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_TAS2781_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 00d306104484..1c76609690d6 100644
--- a/sound/pci/hda/Makefile
+++ b/sound/pci/hda/Makefile
@@ -32,6 +32,7 @@  snd-hda-scodec-cs35l41-objs :=		cs35l41_hda.o
 snd-hda-scodec-cs35l41-i2c-objs :=	cs35l41_hda_i2c.o
 snd-hda-scodec-cs35l41-spi-objs :=	cs35l41_hda_spi.o
 snd-hda-cs-dsp-ctls-objs :=		hda_cs_dsp_ctl.o
+snd-hda-scodec-tas2781-i2c-objs :=	tas2781_hda_i2c.o
 
 # common driver
 obj-$(CONFIG_SND_HDA) := snd-hda-codec.o
@@ -56,6 +57,7 @@  obj-$(CONFIG_SND_HDA_SCODEC_CS35L41) += snd-hda-scodec-cs35l41.o
 obj-$(CONFIG_SND_HDA_SCODEC_CS35L41_I2C) += snd-hda-scodec-cs35l41-i2c.o
 obj-$(CONFIG_SND_HDA_SCODEC_CS35L41_SPI) += snd-hda-scodec-cs35l41-spi.o
 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
 
 # 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 172ffc2c332b..f5b912f90018 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -6705,7 +6705,7 @@  static void comp_generic_playback_hook(struct hda_pcm_stream *hinfo, struct hda_
 	}
 }
 
-struct cs35l41_dev_name {
+struct scodec_dev_name {
 	const char *bus;
 	const char *hid;
 	int index;
@@ -6714,7 +6714,7 @@  struct cs35l41_dev_name {
 /* match the device name in a slightly relaxed manner */
 static int comp_match_cs35l41_dev_name(struct device *dev, void *data)
 {
-	struct cs35l41_dev_name *p = data;
+	struct scodec_dev_name *p = data;
 	const char *d = dev_name(dev);
 	int n = strlen(p->bus);
 	char tmp[32];
@@ -6730,12 +6730,31 @@  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, void *data)
+{
+	struct scodec_dev_name *p = data;
+	const char *d = dev_name(dev);
+	int n = strlen(p->bus);
+	char tmp[32];
+
+	/* check the bus name */
+	if (strncmp(d, p->bus, n))
+		return 0;
+	/* skip the bus number */
+	if (isdigit(d[n]))
+		n++;
+	/* the rest must be exact matching */
+	snprintf(tmp, sizeof(tmp), "-%s:00", p->hid);
+
+	return !strcmp(d + n, tmp);
+}
+
 static void cs35l41_generic_fixup(struct hda_codec *cdc, int action, const char *bus,
 				  const char *hid, int count)
 {
 	struct device *dev = hda_codec_dev(cdc);
 	struct alc_spec *spec = cdc->spec;
-	struct cs35l41_dev_name *rec;
+	struct scodec_dev_name *rec;
 	int ret, i;
 
 	switch (action) {
@@ -6760,6 +6779,40 @@  static void cs35l41_generic_fixup(struct hda_codec *cdc, int action, const char
 	}
 }
 
+static void tas2781_generic_fixup(struct hda_codec *cdc, int action,
+	const char *bus, const char *hid, int count)
+{
+	struct device *dev = hda_codec_dev(cdc);
+	struct alc_spec *spec = cdc->spec;
+	struct scodec_dev_name *rec;
+	int ret, i;
+
+	switch (action) {
+	case HDA_FIXUP_ACT_PRE_PROBE:
+		for (i = 0; i < count; i++) {
+			rec = devm_kmalloc(dev, sizeof(*rec), GFP_KERNEL);
+			if (!rec)
+				return;
+			rec->bus = bus;
+			rec->hid = hid;
+			rec->index = i;
+			spec->comps[i].codec = cdc;
+			component_match_add(dev, &spec->match,
+				comp_match_tas2781_dev_name, rec);
+		}
+		ret = component_master_add_with_match(dev, &comp_master_ops,
+			spec->match);
+		if (ret)
+			codec_err(cdc,
+				"Fail to register component aggregator %d\n",
+				ret);
+		else
+			spec->gen.pcm_playback_hook =
+				comp_generic_playback_hook;
+		break;
+	}
+}
+
 static void cs35l41_fixup_i2c_two(struct hda_codec *cdc, const struct hda_fixup *fix, int action)
 {
 	cs35l41_generic_fixup(cdc, action, "i2c", "CSC3551", 2);
@@ -6787,6 +6840,12 @@  static void alc287_fixup_legion_16ithg6_speakers(struct hda_codec *cdc, const st
 	cs35l41_generic_fixup(cdc, action, "i2c", "CLSA0101", 2);
 }
 
+static void tas2781_fixup_i2c(struct hda_codec *cdc,
+	const struct hda_fixup *fix, int action)
+{
+	 tas2781_generic_fixup(cdc, action, "i2c", "TIAS2781", 1);
+}
+
 /* for alc295_fixup_hp_top_speakers */
 #include "hp_x360_helper.c"
 
@@ -7201,6 +7260,8 @@  enum {
 	ALC287_FIXUP_YOGA9_14IAP7_BASS_SPK_PIN,
 	ALC295_FIXUP_DELL_INSPIRON_TOP_SPEAKERS,
 	ALC236_FIXUP_DELL_DUAL_CODECS,
+	ALC287_FIXUP_TAS2781_I2C_2,
+	ALC287_FIXUP_TAS2781_I2C_4
 };
 
 /* A special fixup for Lenovo C940 and Yoga Duet 7;
@@ -9189,6 +9250,18 @@  static const struct hda_fixup alc269_fixups[] = {
 		.chained = true,
 		.chain_id = ALC255_FIXUP_DELL1_MIC_NO_PRESENCE,
 	},
+	[ALC287_FIXUP_TAS2781_I2C_2] = {
+		.type = HDA_FIXUP_FUNC,
+		.v.func = tas2781_fixup_i2c,
+		.chained = true,
+		.chain_id = ALC269_FIXUP_THINKPAD_ACPI,
+	},
+	[ALC287_FIXUP_TAS2781_I2C_4] = {
+		.type = HDA_FIXUP_FUNC,
+		.v.func = tas2781_fixup_i2c,
+		.chained = true,
+		.chain_id = ALC269_FIXUP_THINKPAD_ACPI,
+	},
 };
 
 static const struct snd_pci_quirk alc269_fixup_tbl[] = {
@@ -9725,6 +9798,33 @@  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, 0x387d, "Yoga S780-16 pro Quad AAC",
+		ALC287_FIXUP_TAS2781_I2C_4),
+	SND_PCI_QUIRK(0x17aa, 0x387e, "Yoga S780-16 pro Quad YC",
+		ALC287_FIXUP_TAS2781_I2C_4),
+	SND_PCI_QUIRK(0x17aa, 0x3881, "YB9 dual powe mode2 YC",
+		ALC287_FIXUP_TAS2781_I2C_2),
+	SND_PCI_QUIRK(0x17aa, 0x3884, "Y780 YG DUAL",
+		ALC287_FIXUP_TAS2781_I2C_2),
+	SND_PCI_QUIRK(0x17aa, 0x3886, "Y780 VECO DUAL",
+		ALC287_FIXUP_TAS2781_I2C_2),
+	SND_PCI_QUIRK(0x17aa, 0x38a7, "Y780P AMD YG dual",
+		ALC287_FIXUP_TAS2781_I2C_2),
+	SND_PCI_QUIRK(0x17aa, 0x38a8, "Y780P AMD VECO dual",
+		ALC287_FIXUP_TAS2781_I2C_2),
+	SND_PCI_QUIRK(0x17aa, 0x38ba, "Yoga S780-14.5 Air AMD quad YC",
+		ALC287_FIXUP_TAS2781_I2C_4),
+	SND_PCI_QUIRK(0x17aa, 0x38bb, "Yoga S780-14.5 Air AMD quad AAC",
+		ALC287_FIXUP_TAS2781_I2C_4),
+	SND_PCI_QUIRK(0x17aa, 0x38be, "Yoga S980-14.5 proX YC Dual",
+		ALC287_FIXUP_TAS2781_I2C_2),
+	SND_PCI_QUIRK(0x17aa, 0x38bf, "Yoga S980-14.5 proX LX Dual",
+		ALC287_FIXUP_TAS2781_I2C_2),
+	SND_PCI_QUIRK(0x17aa, 0x38c3, "Y980 DUAL", ALC287_FIXUP_TAS2781_I2C_2),
+	SND_PCI_QUIRK(0x17aa, 0x38cb, "Y790 YG DUAL",
+		ALC287_FIXUP_TAS2781_I2C_2),
+	SND_PCI_QUIRK(0x17aa, 0x38cd, "Y790 VECO DUAL",
+		ALC287_FIXUP_TAS2781_I2C_2),
 	SND_PCI_QUIRK(0x17aa, 0x3902, "Lenovo E50-80", ALC269_FIXUP_DMIC_THINKPAD_ACPI),
 	SND_PCI_QUIRK(0x17aa, 0x3977, "IdeaPad S210", ALC283_FIXUP_INT_MIC),
 	SND_PCI_QUIRK(0x17aa, 0x3978, "Lenovo B50-70", ALC269_FIXUP_DMIC_THINKPAD_ACPI),
diff --git a/sound/pci/hda/tas2781_hda_i2c.c b/sound/pci/hda/tas2781_hda_i2c.c
new file mode 100644
index 000000000000..a742805b749b
--- /dev/null
+++ b/sound/pci/hda/tas2781_hda_i2c.c
@@ -0,0 +1,806 @@ 
+// SPDX-License-Identifier: GPL-2.0
+//
+// TAS2781 HDA I2C driver
+//
+// Copyright 2023 Texas Instruments, Inc.
+//
+// Author: Shenghao Ding <shenghao-ding@ti.com>
+
+#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/tas2781.h>
+#include <sound/tlv.h>
+#include <sound/tas2781-tlv.h>
+
+#include "hda_local.h"
+#include "hda_auto_parser.h"
+#include "hda_component.h"
+#include "hda_jack.h"
+#include "hda_generic.h"
+
+#define TASDEVICE_SPEAKER_CALIBRATION_SIZE	20
+
+#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} }
+
+enum calib_data {
+	R0_VAL = 0,
+	INV_R0,
+	R0LOW,
+	POWER,
+	TLIM,
+	CALIB_MAX
+};
+
+static int tas2781_acpi_get_i2c_resource(struct acpi_resource
+	*ares, void *data)
+{
+	struct tasdevice_priv *tas_priv = (struct tasdevice_priv *)data;
+	struct acpi_resource_i2c_serialbus *sb;
+
+	if (i2c_acpi_get_i2c_resource(ares, &sb)) {
+		if (sb->slave_address != TAS2781_GLOBAL_ADDR) {
+			tas_priv->tasdevice[tas_priv->ndev].dev_addr =
+				(unsigned int) sb->slave_address;
+			tas_priv->ndev++;
+		} else
+			tas_priv->glb_addr.dev_addr = TAS2781_GLOBAL_ADDR;
+
+	}
+
+	return 1;
+}
+
+static int tas2781_hda_read_acpi(struct tasdevice_priv *tas_priv,
+	const char *hid)
+{
+	struct acpi_device *adev;
+	struct device *physdev;
+	LIST_HEAD(resources);
+	const char *sub;
+	int ret;
+
+	adev = acpi_dev_get_first_match_dev(hid, NULL, -1);
+	if (!adev) {
+		dev_err(tas_priv->dev,
+			"Failed to find an ACPI device for %s\n", hid);
+		return -ENODEV;
+	}
+	strcpy(tas_priv->dev_name, hid);
+	physdev = get_device(acpi_get_first_physical_node(adev));
+	acpi_dev_put(adev);
+
+	sub = acpi_get_subsystem_id(ACPI_HANDLE(physdev));
+	if (IS_ERR(sub))
+		sub = NULL;
+	dev_info(tas_priv->dev, "subid = %s\n", sub);
+
+	tas_priv->acpi_subsystem_id = sub;
+
+	ret = acpi_dev_get_resources(adev, &resources,
+		tas2781_acpi_get_i2c_resource, tas_priv);
+	if (ret < 0)
+		goto err;
+	acpi_dev_free_resource_list(&resources);
+
+	put_device(physdev);
+	dev_info(tas_priv->dev, "global addr: 0x%02x devcnt = %d\n",
+		tas_priv->glb_addr.dev_addr, tas_priv->ndev);
+
+	return 0;
+
+err:
+	dev_err(tas_priv->dev, "Failed acpi ret: %d\n", ret);
+	put_device(physdev);
+
+	return ret;
+}
+
+static void tas2781_hda_playback_hook(struct device *dev, int action)
+{
+	struct tasdevice_priv *tas_priv = dev_get_drvdata(dev);
+	int ret = 0;
+
+	dev_info(tas_priv->dev, "%s: action = %d\n", __func__, action);
+	switch (action) {
+	case HDA_GEN_PCM_ACT_OPEN:
+		pm_runtime_get_sync(dev);
+		mutex_lock(&tas_priv->codec_lock);
+		tas_priv->cur_conf = 0;
+		tas_priv->rcabin.profile_cfg_id = 1;
+		tasdevice_tuning_switch(tas_priv, 0);
+		mutex_unlock(&tas_priv->codec_lock);
+		break;
+	case HDA_GEN_PCM_ACT_CLOSE:
+		mutex_lock(&tas_priv->codec_lock);
+		tasdevice_tuning_switch(tas_priv, 1);
+		mutex_unlock(&tas_priv->codec_lock);
+
+		pm_runtime_mark_last_busy(dev);
+		pm_runtime_put_autosuspend(dev);
+		break;
+	default:
+		dev_warn(tas_priv->dev, "Playback action not supported: %d\n",
+			action);
+		break;
+	}
+
+	if (ret)
+		dev_err(tas_priv->dev, "Regmap access fail: %d\n", ret);
+}
+
+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 = 1;
+	uinfo->value.integer.max = max(0, tas_priv->rcabin.ncfgs);
+
+	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);
+
+	tas_priv->rcabin.profile_cfg_id = ucontrol->value.integer.value[0];
+
+	return 1;
+}
+
+static int tasdevice_create_control(struct tasdevice_priv *tas_priv)
+{
+	char prof_ctrl_name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
+	struct hda_codec *codec = tas_priv->codec;
+	struct snd_kcontrol_new prof_ctrl = {
+		.name = prof_ctrl_name,
+		.iface = SNDRV_CTL_ELEM_IFACE_CARD,
+		.info = tasdevice_info_profile,
+		.get = tasdevice_get_profile_id,
+		.put = tasdevice_set_profile_id,
+	};
+	int ret;
+
+	/* Create a mixer item for selecting the active profile */
+	scnprintf(prof_ctrl_name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN,
+		"tasdev-profile-id");
+	ret = snd_ctl_add(codec->card, snd_ctl_new1(&prof_ctrl, tas_priv));
+	if (ret) {
+		dev_err(tas_priv->dev, "Failed to add KControl %s = %d\n",
+			prof_ctrl.name, ret);
+		goto out;
+	}
+
+	dev_dbg(tas_priv->dev, "Added Control %s\n", prof_ctrl.name);
+
+out:
+	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 = (int)tas_fw->nr_programs;
+
+	return 0;
+}
+
+static int tasdevice_info_configurations(
+	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 = (int)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);
+	unsigned int nr_program = ucontrol->value.integer.value[0];
+
+	tas_priv->cur_prog = nr_program;
+
+	return 1;
+}
+
+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);
+	unsigned int nr_configuration = ucontrol->value.integer.value[0];
+
+	tas_priv->cur_conf = nr_configuration;
+
+	return 1;
+}
+
+/**
+ * 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_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_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;
+
+	return tasdevice_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;
+
+	return tasdevice_amp_putvol(tas_priv, ucontrol, mc);
+}
+
+static const struct snd_kcontrol_new tas2781_snd_controls[] = {
+	ACARD_SINGLE_RANGE_EXT_TLV("Amp Gain Volume", TAS2781_AMP_LEVEL,
+		1, 0, 20, 0, tas2781_amp_getvol,
+		tas2781_amp_putvol, amp_vol_tlv),
+	ACARD_SINGLE_RANGE_EXT_TLV("Digital Volume", TAS2781_DVC_LVL,
+		0, 0, 200, 1, tas2781_digital_getvol,
+		tas2781_digital_putvol, dvc_tlv),
+};
+
+static int tasdevice_dsp_create_ctrls(struct tasdevice_priv
+	*tas_priv)
+{
+	char prog_name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
+	char conf_name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
+	struct hda_codec *codec = tas_priv->codec;
+	struct snd_kcontrol_new prog_ctl = {
+		.name = prog_name,
+		.iface = SNDRV_CTL_ELEM_IFACE_CARD,
+		.info = tasdevice_info_programs,
+		.get = tasdevice_program_get,
+		.put = tasdevice_program_put,
+	};
+	struct snd_kcontrol_new conf_ctl = {
+		.name = conf_name,
+		.iface = SNDRV_CTL_ELEM_IFACE_CARD,
+		.info = tasdevice_info_configurations,
+		.get = tasdevice_config_get,
+		.put = tasdevice_config_put,
+	};
+	int ret;
+
+	scnprintf(prog_name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN, "tasdev-prog-id");
+	scnprintf(conf_name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN, "tasdev-conf-id");
+
+	ret = snd_ctl_add(codec->card, snd_ctl_new1(&prog_ctl, tas_priv));
+	if (ret) {
+		dev_err(tas_priv->dev, "Failed to add KControl %s = %d\n",
+			prog_ctl.name, ret);
+		goto out;
+	}
+
+	dev_dbg(tas_priv->dev, "Added Control %s\n", prog_ctl.name);
+
+	ret = snd_ctl_add(codec->card, snd_ctl_new1(&conf_ctl, tas_priv));
+	if (ret) {
+		dev_err(tas_priv->dev, "Failed to add KControl %s = %d\n",
+			conf_ctl.name, ret);
+		goto out;
+	}
+
+	dev_dbg(tas_priv->dev, "Added Control %s\n", conf_ctl.name);
+out:
+	return ret;
+}
+
+static void tas2781_apply_calib(struct tasdevice_priv *tas_priv)
+{
+	unsigned char page_array[CALIB_MAX] = {0x17, 0x18, 0x18, 0x0d, 0x18};
+	unsigned char rgno_array[CALIB_MAX] = {0x74, 0x0c, 0x14, 0x3c, 0x7c};
+	unsigned char *data;
+	int i, j, rc;
+
+	for (i = 0; i < tas_priv->ndev; i++) {
+		data = tas_priv->cali_data.data +
+			i * TASDEVICE_SPEAKER_CALIBRATION_SIZE;
+		for (j = 0; j < CALIB_MAX; j++) {
+			rc = tasdevice_dev_bulk_write(tas_priv, i,
+				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",
+					i, j, rc);
+		}
+	}
+}
+
+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 hda_codec *codec = tas_priv->codec;
+	unsigned int subid = codec->core.subsystem_id & 0xFFFF;
+	struct tm *tm = &tas_priv->tm;
+	unsigned int attr, crc;
+	unsigned int *tmp_val;
+	efi_status_t status;
+	int ret = 0;
+
+	//Lenovo devices
+	if ((subid == 0x387d) || (subid == 0x387e) || (subid == 0x3881)
+		|| (subid == 0x3884) || (subid == 0x3886) || (subid == 0x38a7)
+		|| (subid == 0x38a8) || (subid == 0x38ba) || (subid == 0x38bb)
+		|| (subid == 0x38be) || (subid == 0x38bf) || (subid == 0x38c3)
+		|| (subid == 0x38cb) || (subid == 0x38cd))
+		efi_guid = EFI_GUID(0x1f52d2a1, 0xbb3a, 0x457d, 0xbc, 0x09,
+			0x43, 0xa3, 0xf4, 0x31, 0x0a, 0x92);
+	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) {
+		ret = -ENODEV;
+		/* 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) {
+			ret = -EINVAL;
+			goto out;
+		}
+	}
+
+	tmp_val = (unsigned int *)tas_priv->cali_data.data;
+
+	crc = crc32(~0, tas_priv->cali_data.data, 84) ^ ~0;
+	dev_info(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_info(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);
+		tas2781_apply_calib(tas_priv);
+	}
+out:
+	return ret;
+}
+
+static void tasdevice_fw_ready(const struct firmware *fmw,
+	void *context)
+{
+	struct tasdevice_priv *tas_priv = (struct tasdevice_priv *)context;
+	struct hda_codec *codec = tas_priv->codec;
+	int i, ret = 0;
+
+	pm_runtime_get_sync(tas_priv->dev);
+	mutex_lock(&tas_priv->codec_lock);
+
+	ret = tasdevice_rca_parser(tas_priv, fmw);
+	if (ret)
+		goto out;
+	tasdevice_create_control(tas_priv);
+
+	for (i = 0; i < ARRAY_SIZE(tas2781_snd_controls); i++) {
+		ret = snd_ctl_add(codec->card,
+			snd_ctl_new1(&tas2781_snd_controls[i], tas_priv));
+		if (ret) {
+			dev_err(tas_priv->dev,
+				"Failed to add KControl %s = %d\n",
+				tas2781_snd_controls[i].name, ret);
+			goto out;
+		}
+	}
+
+	tasdevice_dsp_remove(tas_priv);
+
+	tas_priv->fw_state = TASDEVICE_DSP_FW_PENDING;
+	scnprintf(tas_priv->coef_binaryname, 64, "TAS2XXX%04X.bin",
+		codec->core.subsystem_id & 0xffff);
+	ret = tasdevice_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;
+	}
+	tasdevice_dsp_create_ctrls(tas_priv);
+
+	tas_priv->fw_state = TASDEVICE_DSP_FW_ALL_OK;
+	tasdevice_prmg_load(tas_priv, 0);
+	tas_priv->cur_prog = 0;
+
+	/* If calibrated data occurs error, dsp will still works with default
+	 * calibrated data inside algo.
+	 */
+	tas2781_save_calibration(tas_priv);
+
+out:
+	if (tas_priv->fw_state == TASDEVICE_DSP_FW_FAIL) {
+		/*If DSP FW fail, kcontrol won't be created */
+		tasdevice_config_info_remove(tas_priv);
+		tasdevice_dsp_remove(tas_priv);
+	}
+	mutex_unlock(&tas_priv->codec_lock);
+	if (fmw)
+		release_firmware(fmw);
+	pm_runtime_mark_last_busy(tas_priv->dev);
+	pm_runtime_put_autosuspend(tas_priv->dev);
+}
+
+static int tas2781_hda_bind(struct device *dev, struct device *master,
+	void *master_data)
+{
+	struct tasdevice_priv *tas_priv = dev_get_drvdata(dev);
+	struct hda_component *comps = master_data;
+	int ret = 0;
+
+	if (!comps || tas_priv->index < 0 ||
+		tas_priv->index >= HDA_MAX_COMPONENTS)
+		return -EINVAL;
+
+	comps = &comps[tas_priv->index];
+	if (comps->dev)
+		return -EBUSY;
+
+	pm_runtime_get_sync(dev);
+
+	comps->dev = dev;
+
+	strscpy(comps->name, dev_name(dev), sizeof(comps->name));
+
+	ret = tascodec_init(tas_priv, comps->codec, tasdevice_fw_ready);
+
+	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 tasdevice_priv *tas_priv = dev_get_drvdata(dev);
+	struct hda_component *comps = master_data;
+
+	if (comps[tas_priv->index].dev == dev)
+		memset(&comps[tas_priv->index], 0, sizeof(*comps));
+
+	tasdevice_config_info_remove(tas_priv);
+	tasdevice_dsp_remove(tas_priv);
+
+	tas_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 tasdevice_priv *tas_priv = dev_get_drvdata(dev);
+
+	pm_runtime_get_sync(tas_priv->dev);
+	pm_runtime_disable(tas_priv->dev);
+
+
+	component_del(tas_priv->dev, &tas2781_hda_comp_ops);
+
+	pm_runtime_put_noidle(tas_priv->dev);
+
+	tasdevice_remove(tas_priv);
+}
+
+static int tas2781_hda_i2c_probe(struct i2c_client *clt)
+{
+	struct tasdevice_priv *tas_priv;
+	const char *device_name;
+	int ret;
+
+	if (strstr(dev_name(&clt->dev), "TIAS2781"))
+		device_name = "TIAS2781";
+	else
+		return -ENODEV;
+
+	tas_priv = tasdevice_kzalloc(clt);
+	if (!tas_priv)
+		return -ENOMEM;
+
+	tas_priv->irq_info.irq = clt->irq;
+	ret = tas2781_hda_read_acpi(tas_priv, device_name);
+	if (ret)
+		return dev_err_probe(tas_priv->dev, ret,
+			"Platform not supported\n");
+
+	ret = tasdevice_init(tas_priv);
+	if (ret)
+		goto err;
+
+	pm_runtime_set_autosuspend_delay(tas_priv->dev, 3000);
+	pm_runtime_use_autosuspend(tas_priv->dev);
+	pm_runtime_mark_last_busy(tas_priv->dev);
+	pm_runtime_set_active(tas_priv->dev);
+	pm_runtime_get_noresume(tas_priv->dev);
+	pm_runtime_enable(tas_priv->dev);
+
+	pm_runtime_put_autosuspend(tas_priv->dev);
+
+	ret = component_add(tas_priv->dev, &tas2781_hda_comp_ops);
+	if (ret) {
+		dev_err(tas_priv->dev, "Register component failed: %d\n", ret);
+		pm_runtime_disable(tas_priv->dev);
+	}
+
+err:
+	if (ret)
+		tas2781_hda_remove(&clt->dev);
+	return ret;
+}
+
+static void tas2781_hda_i2c_remove(struct i2c_client *clt)
+{
+	tas2781_hda_remove(&clt->dev);
+}
+
+static int tas2781_runtime_suspend(struct device *dev)
+{
+	struct tasdevice_priv *tas_priv = dev_get_drvdata(dev);
+	int i, ret = 0;
+
+	dev_info(tas_priv->dev, "Runtime Suspend\n");
+
+	mutex_lock(&tas_priv->codec_lock);
+
+	if (tas_priv->playback_started) {
+		tasdevice_tuning_switch(tas_priv, 1);
+		tas_priv->playback_started = false;
+	}
+
+	tas_priv->cur_prog = -1;
+	tas_priv->cur_conf = -1;
+
+	for (i = 0; i < tas_priv->ndev; i++) {
+		tas_priv->tasdevice[i].cur_book = -1;
+		tas_priv->tasdevice[i].cur_prog = -1;
+		tas_priv->tasdevice[i].cur_conf = -1;
+	}
+
+	regcache_cache_only(tas_priv->regmap, true);
+	regcache_mark_dirty(tas_priv->regmap);
+
+	mutex_unlock(&tas_priv->codec_lock);
+
+	return ret;
+}
+
+static int tas2781_runtime_resume(struct device *dev)
+{
+	struct tasdevice_priv *tas_priv = dev_get_drvdata(dev);
+	unsigned long calib_data_sz =
+		tas_priv->ndev * TASDEVICE_SPEAKER_CALIBRATION_SIZE;
+	int ret = 0;
+
+	dev_info(tas_priv->dev, "Runtime Resume\n");
+
+	mutex_lock(&tas_priv->codec_lock);
+
+	regcache_cache_only(tas_priv->regmap, false);
+	ret = regcache_sync(tas_priv->regmap);
+	if (ret) {
+		dev_err(tas_priv->dev,
+			"Failed to restore register cache: %d\n", ret);
+		goto out;
+	}
+
+	tasdevice_prmg_load(tas_priv, 0);
+	tas_priv->cur_prog = 0;
+
+	/* If calibrated data occurs error, dsp will still works with default
+	 * calibrated data inside algo.
+	 */
+	if (tas_priv->cali_data.total_sz > calib_data_sz)
+		tas2781_apply_calib(tas_priv);
+
+out:
+	mutex_unlock(&tas_priv->codec_lock);
+
+	return ret;
+}
+
+static int tas2781_system_suspend(struct device *dev)
+{
+	struct tasdevice_priv *tas_priv = dev_get_drvdata(dev);
+	int ret;
+
+	dev_info(tas_priv->dev, "System Suspend\n");
+
+	ret = pm_runtime_force_suspend(dev);
+	if (ret)
+		return ret;
+
+	/* Shutdown chip before system suspend */
+	regcache_cache_only(tas_priv->regmap, false);
+	tasdevice_tuning_switch(tas_priv, 1);
+	regcache_cache_only(tas_priv->regmap, true);
+	regcache_mark_dirty(tas_priv->regmap);
+	/*
+	 * 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 tasdevice_priv *tas_priv = dev_get_drvdata(dev);
+	unsigned long calib_data_sz =
+		tas_priv->ndev * TASDEVICE_SPEAKER_CALIBRATION_SIZE;
+	int i, ret;
+
+	dev_info(tas_priv->dev, "System Resume\n");
+
+	ret = pm_runtime_force_resume(dev);
+
+	mutex_lock(&tas_priv->codec_lock);
+	tas_priv->cur_prog = -1;
+	tas_priv->cur_conf = -1;
+
+	for (i = 0; i < tas_priv->ndev; i++) {
+		tas_priv->tasdevice[i].cur_book = -1;
+		tas_priv->tasdevice[i].cur_prog = -1;
+		tas_priv->tasdevice[i].cur_conf = -1;
+	}
+	tas2781_reset(tas_priv);
+	tasdevice_prmg_load(tas_priv, 0);
+	tas_priv->cur_prog = 0;
+
+	/* If calibrated data occurs error, dsp will still works with default
+	 * calibrated data inside algo.
+	 */
+	if (tas_priv->cali_data.total_sz > calib_data_sz)
+		tas2781_apply_calib(tas_priv);
+	mutex_unlock(&tas_priv->codec_lock);
+
+	return ret;
+}
+
+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 i2c_device_id tas2781_hda_i2c_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 i2c_driver tas2781_hda_i2c_driver = {
+	.driver = {
+		.name		= "tas2781-hda",
+		.acpi_match_table = tas2781_acpi_hda_match,
+		.pm		= &tas2781_hda_pm_ops,
+	},
+	.id_table	= tas2781_hda_i2c_id,
+	.probe_new	= tas2781_hda_i2c_probe,
+	.remove		= tas2781_hda_i2c_remove,
+};
+module_i2c_driver(tas2781_hda_i2c_driver);
+
+MODULE_DESCRIPTION("TAS2781 HDA Driver");
+MODULE_AUTHOR("Shenghao Ding, TI, <shenghao-ding@ti.com>");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(SND_SOC_TAS2781_FMWLIB);