[v8,3/3] update to support either TAS2764 or TAS2780

Commit Message

Raphael-Xu April 11, 2022, 7:56 a.m. UTC

fix no sound issue on some platforms

Signed-off-by: Raphael-Xu <[e-mail]>
---
 sound/soc/codecs/tas27xx.c | 441 ++++++++++++++++++++++++++-----------
 sound/soc/codecs/tas27xx.h |   2 +-
 2 files changed, 313 insertions(+), 130 deletions(-)

Comments

Mark Brown April 12, 2022, 2:28 p.m. UTC | #1

On Mon, Apr 11, 2022 at 03:56:52PM +0800, Raphael-Xu wrote:

>  static void tas27xx_reset(struct tas27xx_priv *tas27xx)
>  {
>  	if (tas27xx->reset_gpio) {
>  		gpiod_set_value_cansleep(tas27xx->reset_gpio, 0);
> -		msleep(20);
> +		usleep_range(2000, 2050);
>  		gpiod_set_value_cansleep(tas27xx->reset_gpio, 1);
> +		usleep_range(5000, 5050);
>  	}

This looks like an unrelated but good fix?  It should be a separate
patch.

> +			TAS27XX_PWR_CTRL,
> +			TAS27XX_PWR_CTRL_MASK,
> +			TAS27XX_PWR_CTRL_SHUTDOWN);
> +		if (ret >= 0) {
> +			tas27xx->mb_power_up = false;
> +			ret = 0;

mb_power_up seems to never be read - what purpose does it serve?

> -	return 0;
> +	if (ret < 0)
> +		pr_err("%s:%u:errCode:0x%0x:set BIAS error\n",
> +			__func__, __LINE__, ret);

Please use something like normal kernel logging styles - use dev_err()
like the rest of the function, no __func__ or __line__ and log the error
code as an integer.  In general please try to follow the kernel coding
style.

> +	mutex_unlock(&tas27xx->codec_lock);

It's not clear what this lock is protecting - it seems to be serialising
things that the core will already ensure don't run concurrently.  It at
least needs some documentation.  If it's not needed at all then a lot of
the diff could be dropped which would help a lot since as far as I can
see the bulk of the changes here are for adding this lock so it's hard
to see the device specific changes.  I'd also suggest pulling this out
into a separate patch.

> -	return 0;
> +EXIT:
> +	mutex_unlock(&tas27xx->codec_lock);

Normal coding style for labels is lower case.

>  {
> -	struct tas27xx_priv *tas27xx =
> +	struct tas27xx_priv *tas27xx =

This looks like an unneeded whitespace change?  There's a lot of these
where I can't spot what the actual change is...

>  }
> -#else
> -#define tas27xx_codec_suspend NULL
> -#define tas27xx_codec_resume NULL
>  #endif

This (and the related change below adding ifdefs for the use) are an
unrelated stylistic change and should be in a separate patch if they
make sense though I can't see any reason for them?  It's generally
considered better style not to need the ifdefs.

>  static int tas27xx_mute(struct snd_soc_dai *dai, int mute, int direction)
>  {
>  	struct snd_soc_component *component = dai->component;
> -	int ret;
> +	struct tas27xx_priv *tas27xx =
> +		snd_soc_component_get_drvdata(component);
> +	int ret = 0;
> +
> +	mutex_lock(&tas27xx->codec_lock);
>  
> +	if (!mute) {
> +		ret = snd_soc_component_read(component,
> +			TAS27XX_CLK_CFG);
> +		if (ret < 0) {
> +			dev_err(tas27xx->dev,
> +				"%s:%u:errCode:0x%x read "
> +				"TAS27XX_CLK_CFG error\n",
> +				__func__, __LINE__, ret);
> +			goto EXIT;
> +		}
> +		if ((ret & TAS27XX_CLK_CFG_MASK) != TAS27XX_CLK_CFG_ENABLE) {
> +			ret = snd_soc_component_update_bits(component,
> +				TAS27XX_CLK_CFG,
> +				TAS27XX_CLK_CFG_MASK,
> +				TAS27XX_CLK_CFG_ENABLE);
> +			if (ret < 0) {
> +				dev_err(tas27xx->dev,
> +					"%s:%u: Failed to CLK_CFG_ENABLE\n",
> +					__func__, __LINE__);
> +				goto EXIT;
> +			}
> +			usleep_range(3000, 3050);
> +		}

This clock configuration on mute is suprising - what's going on here?
It's an unusal thing to do.

>  		ret = snd_soc_component_update_bits(component,
> -					TAS27XX_TDM_CFG2,
> -					TAS27XX_TDM_CFG2_RXW_MASK,
> -					TAS27XX_TDM_CFG2_RXW_16BITS);
> +			TAS27XX_TDM_CFG2,
> +			TAS27XX_TDM_CFG2_RXW_MASK,
> +			TAS27XX_TDM_CFG2_RXW_16BITS);

Unrelated indentation change.

> @@ -522,26 +648,54 @@ static int tas27xx_codec_probe(struct snd_soc_component *component)
>  		gpiod_set_value_cansleep(tas27xx->sdz_gpio, 1);
>  
>  	tas27xx_reset(tas27xx);
> +	usleep_range(5000, 5050);

There's already a sleep in the reset function, why does this caller need
an extra one?

> -	ret = snd_soc_component_update_bits(tas27xx->component,
> -						TAS27XX_TDM_CFG5,
> +	ret = snd_soc_component_update_bits(component,

The changes to use a local component variable could probably usefully be
a separate patch, it obscures everything else that's going on.

> +static bool tas27xx_volatile(struct device *dev,
> +	unsigned int reg)

This should be a separate change probably, it looks like a bug fix.

> +{
> +	switch (reg) {
> +	case TAS27XX_SW_RST:
> +	case TAS27XX_PWR_CTRL:
> +	case TAS27XX_PAGE:

It's suprising that the power control and paging registers would be
volatile?  Same for some of the other registers...

> +	case TAS27XX_DVC:
> +	case TAS27XX_CHNL_0:
> +	case TAS27XX_TDM_CFG0:
> +	case TAS27XX_TDM_CFG1:
> +	case TAS27XX_TDM_CFG2:
> +	case TAS27XX_TDM_CFG3:
> +	case TAS27XX_TDM_CFG5:
> +	case TAS27XX_TDM_CFG6:

...like the TDM configuration.

>  static const struct i2c_device_id tas27xx_i2c_id[] = {
>  	{ "tas2764", TAS2764},
> +	{ "tas2780", TAS2780},
>  	{ }

I don't see any runtime differences between the two variants - nothing
is keyed off the ID?

>  static const struct of_device_id tas27xx_of_match[] = {
>  	{ .compatible = "ti,tas2764" },
> +	{ .compatible = "ti,tas2780" },
>  	{},
>  };

If it were we'd need to also have something here.

Patch

diff --git a/sound/soc/codecs/tas27xx.c b/sound/soc/codecs/tas27xx.c
index e02e99bd46be..ddd3e7f98757 100644
--- a/sound/soc/codecs/tas27xx.c
+++ b/sound/soc/codecs/tas27xx.c
@@ -1,5 +1,5 @@ 
 // SPDX-License-Identifier: GPL-2.0
-// Driver for the Texas Instruments TAS2764 Mono
+// Driver for the Texas Instruments TAS2764/TAS2780 Mono
 //		Audio amplifier
 // Copyright (C) 2020-2022 Texas Instruments Inc.
 
@@ -12,11 +12,9 @@ 
 #include <linux/i2c.h>
 #include <linux/gpio.h>
 #include <linux/gpio/consumer.h>
-#include <linux/regulator/consumer.h>
 #include <linux/regmap.h>
 #include <linux/of.h>
 #include <linux/of_gpio.h>
-#include <linux/slab.h>
 #include <sound/soc.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
@@ -31,21 +29,25 @@  struct tas27xx_priv {
 	struct gpio_desc *sdz_gpio;
 	struct regmap *regmap;
 	struct device *dev;
-	
+	struct mutex codec_lock;
 	int v_sense_slot;
 	int i_sense_slot;
+	int device_id;
+	bool mb_power_up;
 };
 
 enum tas27xx {
 	TAS2764 = 0,
+	TAS2780 = 1,
 };
 
 static void tas27xx_reset(struct tas27xx_priv *tas27xx)
 {
 	if (tas27xx->reset_gpio) {
 		gpiod_set_value_cansleep(tas27xx->reset_gpio, 0);
-		msleep(20);
+		usleep_range(2000, 2050);
 		gpiod_set_value_cansleep(tas27xx->reset_gpio, 1);
+		usleep_range(5000, 5050);
 	}
 
 	snd_soc_component_write(tas27xx->component, TAS27XX_SW_RST,
@@ -55,65 +57,92 @@  static void tas27xx_reset(struct tas27xx_priv *tas27xx)
 static int tas27xx_set_bias_level(struct snd_soc_component *component,
 				 enum snd_soc_bias_level level)
 {
-	struct tas27xx_priv *tas27xx =
+	struct tas27xx_priv *tas27xx =
 		snd_soc_component_get_drvdata(component);
-
+	int ret = 0;
+
+	mutex_lock(&tas27xx->codec_lock);
 	switch (level) {
 	case SND_SOC_BIAS_ON:
-		snd_soc_component_update_bits(component, TAS27XX_PWR_CTRL,
-					      TAS27XX_PWR_CTRL_MASK,
-					      TAS27XX_PWR_CTRL_ACTIVE);
+		ret = snd_soc_component_update_bits(component,
+					TAS27XX_PWR_CTRL,
+					TAS27XX_PWR_CTRL_MASK,
+					TAS27XX_PWR_CTRL_ACTIVE);
+		if (ret >= 0) {
+			tas27xx->mb_power_up = true;
+			ret = 0;
+		}
 		break;
 	case SND_SOC_BIAS_STANDBY:
 	case SND_SOC_BIAS_PREPARE:
-		snd_soc_component_update_bits(component, TAS27XX_PWR_CTRL,
-					      TAS27XX_PWR_CTRL_MASK,
-					      TAS27XX_PWR_CTRL_MUTE);
+		ret = snd_soc_component_update_bits(component,
+			TAS27XX_PWR_CTRL,
+			TAS27XX_PWR_CTRL_MASK,
+			TAS27XX_PWR_CTRL_MUTE);
+		if (ret >= 0) {
+			tas27xx->mb_power_up = true;
+			ret = 0;
+		}
 		break;
 	case SND_SOC_BIAS_OFF:
-		snd_soc_component_update_bits(component, TAS27XX_PWR_CTRL,
-					      TAS27XX_PWR_CTRL_MASK,
-					      TAS27XX_PWR_CTRL_SHUTDOWN);
+		ret = snd_soc_component_update_bits(component,
+			TAS27XX_PWR_CTRL,
+			TAS27XX_PWR_CTRL_MASK,
+			TAS27XX_PWR_CTRL_SHUTDOWN);
+		if (ret >= 0) {
+			tas27xx->mb_power_up = false;
+			ret = 0;
+		}
 		break;
 
 	default:
 		dev_err(tas27xx->dev,
-				"wrong power level setting %d\n", level);
-		return -EINVAL;
+			"wrong power level setting %d\n", level);
+		ret = -EINVAL;
 	}
-
-	return 0;
+	if (ret < 0)
+		pr_err("%s:%u:errCode:0x%0x:set BIAS error\n",
+			__func__, __LINE__, ret);
+	mutex_unlock(&tas27xx->codec_lock);
+	return ret;
 }
 
 #ifdef CONFIG_PM
 static int tas27xx_codec_suspend(struct snd_soc_component *component)
 {
-	struct tas27xx_priv *tas27xx =
+	struct tas27xx_priv *tas27xx =
 		snd_soc_component_get_drvdata(component);
-	int ret;
+	int ret = 0;
 
+	mutex_lock(&tas27xx->codec_lock);
 	ret = snd_soc_component_update_bits(component, TAS27XX_PWR_CTRL,
 					    TAS27XX_PWR_CTRL_MASK,
 					    TAS27XX_PWR_CTRL_SHUTDOWN);
 
-	if (ret < 0)
-		return ret;
-
+	if (ret < 0) {
+		pr_err("%s:%u:errCode:0x%0x:power down error\n",
+			__func__, __LINE__, ret);
+		goto EXIT;
+	}
+	ret = 0;
+	tas27xx->mb_power_up = false;
 	if (tas27xx->sdz_gpio)
 		gpiod_set_value_cansleep(tas27xx->sdz_gpio, 0);
 
 	regcache_cache_only(tas27xx->regmap, true);
 	regcache_mark_dirty(tas27xx->regmap);
-
-	return 0;
+EXIT:
+	mutex_unlock(&tas27xx->codec_lock);
+	return ret;
 }
 
 static int tas27xx_codec_resume(struct snd_soc_component *component)
 {
-	struct tas27xx_priv *tas27xx =
+	struct tas27xx_priv *tas27xx =
 		snd_soc_component_get_drvdata(component);
-	int ret;
+	int ret = 0;
 
+	mutex_lock(&tas27xx->codec_lock);
 	if (tas27xx->sdz_gpio)
 		gpiod_set_value_cansleep(tas27xx->sdz_gpio, 1);
 
@@ -121,16 +150,19 @@  static int tas27xx_codec_resume(struct snd_soc_component *component)
 					    TAS27XX_PWR_CTRL_MASK,
 					    TAS27XX_PWR_CTRL_ACTIVE);
 
-	if (ret < 0)
-		return ret;
-
+	if (ret < 0) {
+		pr_err("%s:%u:errCode:0x%0x:power down error\n",
+			__func__, __LINE__, ret);
+		goto EXIT;
+	}
+	ret = 0;
+	tas27xx->mb_power_up = false;
 	regcache_cache_only(tas27xx->regmap, false);
-
-	return regcache_sync(tas27xx->regmap);
+	ret = regcache_sync(tas27xx->regmap);
+EXIT:
+	mutex_unlock(&tas27xx->codec_lock);
+	return ret;
 }
-#else
-#define tas27xx_codec_suspend NULL
-#define tas27xx_codec_resume NULL
 #endif
 
 static const char * const tas27xx_ASI1_src[] = {
@@ -146,49 +178,53 @@  static const struct snd_kcontrol_new tas27xx_asi1_mux =
 static int tas27xx_dac_event(struct snd_soc_dapm_widget *w,
 			     struct snd_kcontrol *kcontrol, int event)
 {
-	struct snd_soc_component *component =
+	struct snd_soc_component *component =
 		snd_soc_dapm_to_component(w->dapm);
-	struct tas27xx_priv *tas27xx =
+	struct tas27xx_priv *tas27xx =
 		snd_soc_component_get_drvdata(component);
-	int ret;
+	int ret = 0;
 
+	mutex_lock(&tas27xx->codec_lock);
 	switch (event) {
 	case SND_SOC_DAPM_POST_PMU:
-		ret = snd_soc_component_update_bits(component,
+		ret = snd_soc_component_update_bits(component,
 						TAS27XX_PWR_CTRL,
 						TAS27XX_PWR_CTRL_MASK,
 						TAS27XX_PWR_CTRL_MUTE);
 		break;
 	case SND_SOC_DAPM_PRE_PMD:
-		ret = snd_soc_component_update_bits(component,
+		ret = snd_soc_component_update_bits(component,
 						TAS27XX_PWR_CTRL,
 						TAS27XX_PWR_CTRL_MASK,
 						TAS27XX_PWR_CTRL_SHUTDOWN);
 		break;
 	default:
 		dev_err(tas27xx->dev, "Unsupported event\n");
-		return -EINVAL;
+		ret = -EINVAL;
 	}
-
-	if (ret < 0)
-		return ret;
-
-	return 0;
+	if (ret < 0) {
+		pr_err("%s:%u:errCode:0x%0x:PWR_CTRL error\n",
+			__func__, __LINE__, ret);
+	} else {
+		ret = 0;
+	}
+	mutex_unlock(&tas27xx->codec_lock);
+	return ret;
 }
 
 static const struct snd_kcontrol_new isense_switch =
-	SOC_DAPM_SINGLE("Switch", TAS27XX_PWR_CTRL,
+	SOC_DAPM_SINGLE("Switch", TAS27XX_PWR_CTRL,
 			TAS27XX_ISENSE_POWER_EN, 1, 1);
 static const struct snd_kcontrol_new vsense_switch =
-	SOC_DAPM_SINGLE("Switch", TAS27XX_PWR_CTRL,
+	SOC_DAPM_SINGLE("Switch", TAS27XX_PWR_CTRL,
 			TAS27XX_VSENSE_POWER_EN, 1, 1);
 
 static const struct snd_soc_dapm_widget tas27xx_dapm_widgets[] = {
 	SND_SOC_DAPM_AIF_IN("ASI1", "ASI1 Playback", 0, SND_SOC_NOPM, 0, 0),
 	SND_SOC_DAPM_MUX("ASI1 Sel", SND_SOC_NOPM, 0, 0, &tas27xx_asi1_mux),
-	SND_SOC_DAPM_SWITCH("ISENSE", TAS27XX_PWR_CTRL,
+	SND_SOC_DAPM_SWITCH("ISENSE", TAS27XX_PWR_CTRL,
 		TAS27XX_ISENSE_POWER_EN, 1, &isense_switch),
-	SND_SOC_DAPM_SWITCH("VSENSE", TAS27XX_PWR_CTRL,
+	SND_SOC_DAPM_SWITCH("VSENSE", TAS27XX_PWR_CTRL,
 		TAS27XX_VSENSE_POWER_EN, 1, &vsense_switch),
 	SND_SOC_DAPM_DAC_E("DAC", NULL, SND_SOC_NOPM, 0, 0, tas27xx_dac_event,
 			   SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD),
@@ -211,78 +247,136 @@  static const struct snd_soc_dapm_route tas27xx_audio_map[] = {
 static int tas27xx_mute(struct snd_soc_dai *dai, int mute, int direction)
 {
 	struct snd_soc_component *component = dai->component;
-	int ret;
+	struct tas27xx_priv *tas27xx =
+		snd_soc_component_get_drvdata(component);
+	int ret = 0;
+
+	mutex_lock(&tas27xx->codec_lock);
 
+	if (!mute) {
+		ret = snd_soc_component_read(component,
+			TAS27XX_CLK_CFG);
+		if (ret < 0) {
+			dev_err(tas27xx->dev,
+				"%s:%u:errCode:0x%x read "
+				"TAS27XX_CLK_CFG error\n",
+				__func__, __LINE__, ret);
+			goto EXIT;
+		}
+		if ((ret & TAS27XX_CLK_CFG_MASK) != TAS27XX_CLK_CFG_ENABLE) {
+			ret = snd_soc_component_update_bits(component,
+				TAS27XX_CLK_CFG,
+				TAS27XX_CLK_CFG_MASK,
+				TAS27XX_CLK_CFG_ENABLE);
+			if (ret < 0) {
+				dev_err(tas27xx->dev,
+					"%s:%u: Failed to CLK_CFG_ENABLE\n",
+					__func__, __LINE__);
+				goto EXIT;
+			}
+			usleep_range(3000, 3050);
+		}
+	}
 	ret = snd_soc_component_update_bits(component, TAS27XX_PWR_CTRL,
-					    TAS27XX_PWR_CTRL_MASK,
-					    mute ? TAS27XX_PWR_CTRL_MUTE : 0);
-
-	if (ret < 0)
-		return ret;
+		TAS27XX_PWR_CTRL_MASK,
+		mute ? TAS27XX_PWR_CTRL_MUTE : 0);
+	if (ret >= 0) {
+		tas27xx->mb_power_up = mute?false:true;
+		ret = 0;
+	} else {
+		pr_err("%s:%u: Failed to set powercontrol\n",
+			__func__, __LINE__);
+	}
 
-	return 0;
+EXIT:
+	mutex_unlock(&tas27xx->codec_lock);
+	return ret;
 }
 
 static int tas27xx_set_bitwidth(struct tas27xx_priv *tas27xx,
 	snd_pcm_format_t bitwidth)
 {
 	struct snd_soc_component *component = tas27xx->component;
-	int sense_en;
-	int val;
-	int ret;
+	int sense_en, val, ret, slot_size;
 
+	mutex_lock(&tas27xx->codec_lock);
 	switch (bitwidth) {
 	case SNDRV_PCM_FORMAT_S16_LE:
 		ret = snd_soc_component_update_bits(component,
-					TAS27XX_TDM_CFG2,
-					TAS27XX_TDM_CFG2_RXW_MASK,
-					TAS27XX_TDM_CFG2_RXW_16BITS);
+			TAS27XX_TDM_CFG2,
+			TAS27XX_TDM_CFG2_RXW_MASK,
+			TAS27XX_TDM_CFG2_RXW_16BITS);
+		slot_size = TAS27XX_TDM_CFG2_RXS_16BITS;
 		break;
 	case SNDRV_PCM_FORMAT_S24_LE:
 		ret = snd_soc_component_update_bits(component,
 					TAS27XX_TDM_CFG2,
 					TAS27XX_TDM_CFG2_RXW_MASK,
 					TAS27XX_TDM_CFG2_RXW_24BITS);
+		slot_size = TAS27XX_TDM_CFG2_RXS_24BITS;
 		break;
 	case SNDRV_PCM_FORMAT_S32_LE:
 		ret = snd_soc_component_update_bits(component,
 					TAS27XX_TDM_CFG2,
 					TAS27XX_TDM_CFG2_RXW_MASK,
 					TAS27XX_TDM_CFG2_RXW_32BITS);
+		slot_size = TAS27XX_TDM_CFG2_RXS_32BITS;
 		break;
 
 	default:
-		return -EINVAL;
+		ret = -EINVAL;
 	}
 
-	if (ret < 0)
-		return ret;
+	if (ret < 0) {
+		pr_err("%s:%u:errCode:0x%x set bitwidth error\n",
+			__func__, __LINE__, ret);
+		goto EXIT;
+	}
+
+	ret = snd_soc_component_update_bits(component, TAS27XX_TDM_CFG2,
+		TAS27XX_TDM_CFG2_RXS_MASK, slot_size);
+	if (ret < 0) {
+		pr_err("%s:%u:errCode:0x%x set RX slot size error\n",
+			__func__, __LINE__, ret);
+		goto EXIT;
+	}
 
 	val = snd_soc_component_read(tas27xx->component, TAS27XX_PWR_CTRL);
-	if (val < 0)
-		return val;
+	if (val < 0) {
+		pr_err("%s:%u:errCode:0x%x read PWR_CTRL error\n",
+			__func__, __LINE__, val);
+		ret = val;
+		goto EXIT;
+	}
 
 	if (val & (1 << TAS27XX_VSENSE_POWER_EN))
 		sense_en = 0;
 	else
 		sense_en = TAS27XX_TDM_CFG5_VSNS_ENABLE;
 
-	ret = snd_soc_component_update_bits(tas27xx->component,
+	ret = snd_soc_component_update_bits(tas27xx->component,
 		TAS27XX_TDM_CFG5, TAS27XX_TDM_CFG5_VSNS_ENABLE, sense_en);
-	if (ret < 0)
-		return ret;
+	if (ret < 0) {
+		pr_err("%s:%u:errCode:0x%x enable vSNS error\n",
+			__func__, __LINE__, ret);
+		goto EXIT;
+	}
 
 	if (val & (1 << TAS27XX_ISENSE_POWER_EN))
 		sense_en = 0;
 	else
 		sense_en = TAS27XX_TDM_CFG6_ISNS_ENABLE;
 
-	ret = snd_soc_component_update_bits(tas27xx->component,
+	ret = snd_soc_component_update_bits(tas27xx->component,
 		TAS27XX_TDM_CFG6, TAS27XX_TDM_CFG6_ISNS_ENABLE, sense_en);
-	if (ret < 0)
-		return ret;
-
-	return 0;
+	if (ret < 0) {
+		pr_err("%s:%u:errCode:0x%x enable iSNS error\n",
+			__func__, __LINE__, ret);
+	}
+	ret = 0;
+EXIT:
+	mutex_unlock(&tas27xx->codec_lock);
+	return ret;
 }
 
 static int tas27xx_set_samplerate(
@@ -312,15 +406,20 @@  static int tas27xx_set_samplerate(
 	default:
 		return -EINVAL;
 	}
-
+	mutex_lock(&tas27xx->codec_lock);
 	ret = snd_soc_component_update_bits(component, TAS27XX_TDM_CFG0,
 					    TAS27XX_TDM_CFG0_SMP_MASK |
 					    TAS27XX_TDM_CFG0_MASK,
 					    ramp_rate_val);
-	if (ret < 0)
-		return ret;
-
-	return 0;
+	if (ret < 0) {
+		pr_err("%s:%u:errCode:0x%x Failed to set ramp_rate_val\n",
+			__func__, __LINE__, ret);
+		goto EXIT;
+	}
+	ret = 0;
+EXIT:
+	mutex_unlock(&tas27xx->codec_lock);
+	return ret;
 }
 
 static int tas27xx_hw_params(struct snd_pcm_substream *substream,
@@ -328,7 +427,7 @@  static int tas27xx_hw_params(struct snd_pcm_substream *substream,
 			     struct snd_soc_dai *dai)
 {
 	struct snd_soc_component *component = dai->component;
-	struct tas27xx_priv *tas27xx =
+	struct tas27xx_priv *tas27xx =
 		snd_soc_component_get_drvdata(component);
 	int ret;
 
@@ -342,11 +441,11 @@  static int tas27xx_hw_params(struct snd_pcm_substream *substream,
 static int tas27xx_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
 {
 	struct snd_soc_component *component = dai->component;
-	struct tas27xx_priv *tas27xx =
+	struct tas27xx_priv *tas27xx =
 		snd_soc_component_get_drvdata(component);
 	u8 tdm_rx_start_slot = 0, asi_cfg_1 = 0;
 	int iface;
-	int ret;
+	int ret = 0;
 
 	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
 	case SND_SOC_DAIFMT_NB_NF:
@@ -359,12 +458,15 @@  static int tas27xx_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
 		dev_err(tas27xx->dev, "ASI format Inverse is not found\n");
 		return -EINVAL;
 	}
-
+	mutex_lock(&tas27xx->codec_lock);
 	ret = snd_soc_component_update_bits(component, TAS27XX_TDM_CFG1,
 					    TAS27XX_TDM_CFG1_RX_MASK,
 					    asi_cfg_1);
-	if (ret < 0)
-		return ret;
+	if (ret < 0) {
+		pr_err("%s:%u:errCode:0x%x Failed to set asi_cfg_1\n",
+			__func__, __LINE__, ret);
+		goto EXIT;
+	}
 
 	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
 	case SND_SOC_DAIFMT_I2S:
@@ -378,23 +480,32 @@  static int tas27xx_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
 		tdm_rx_start_slot = 0;
 		break;
 	default:
-		dev_err(tas27xx->dev,
-			"DAI Format is not found, fmt=0x%x\n", fmt);
-		return -EINVAL;
+		pr_err("%s:%u:DAI Format is not found, fmt=0x%x\n",
+			__func__, __LINE__, fmt);
+		ret = -EINVAL;
+		goto EXIT;
 	}
 
 	ret = snd_soc_component_update_bits(component, TAS27XX_TDM_CFG1,
 		TAS27XX_TDM_CFG1_MASK,
 		(tdm_rx_start_slot << TAS27XX_TDM_CFG1_51_SHIFT));
-	if (ret < 0)
-		return ret;
+	if (ret < 0) {
+		pr_err("%s:%u:errCode:0x%x Failed to set tdm_rx_start_slot\n",
+			__func__, __LINE__, ret);
+		goto EXIT;
+	}
 
 	ret = snd_soc_component_update_bits(component, TAS27XX_TDM_CFG2,
 		TAS27XX_TDM_CFG2_SCFG_MASK, iface);
-	if (ret < 0)
-		return ret;
-
-	return 0;
+	if (ret < 0) {
+		pr_err("%s:%u:errCode:0x%x Failed to set iface\n",
+			__func__, __LINE__, ret);
+		goto EXIT;
+	}
+	ret = 0;
+EXIT:
+	mutex_unlock(&tas27xx->codec_lock);
+	return ret;
 }
 
 static int tas27xx_set_dai_tdm_slot(struct snd_soc_dai *dai,
@@ -403,12 +514,12 @@  static int tas27xx_set_dai_tdm_slot(struct snd_soc_dai *dai,
 				int slots, int slot_width)
 {
 	struct snd_soc_component *component = dai->component;
-	struct tas27xx_priv *tas27xx =
+	struct tas27xx_priv *tas27xx =
 		snd_soc_component_get_drvdata(component);
 	int left_slot, right_slot;
 	int slots_cfg;
 	int slot_size;
-	int ret;
+	int ret = 0;
 
 	if (tx_mask == 0 || rx_mask != 0)
 		return -EINVAL;
@@ -433,10 +544,13 @@  static int tas27xx_set_dai_tdm_slot(struct snd_soc_dai *dai,
 		return -EINVAL;
 
 	slots_cfg = (right_slot << TAS27XX_TDM_CFG3_RXS_SHIFT) | left_slot;
-
+	mutex_lock(&tas27xx->codec_lock);
 	ret = snd_soc_component_write(component, TAS27XX_TDM_CFG3, slots_cfg);
-	if (ret)
-		return ret;
+	if (ret) {
+		pr_err("%s:%u:errCode:0x%x Failed to set slots_cfg\n",
+			__func__, __LINE__, ret);
+		goto EXIT;
+	}
 
 	switch (slot_width) {
 	case 16:
@@ -449,28 +563,40 @@  static int tas27xx_set_dai_tdm_slot(struct snd_soc_dai *dai,
 		slot_size = TAS27XX_TDM_CFG2_RXS_32BITS;
 		break;
 	default:
-		return -EINVAL;
+		ret = -EINVAL;
+		goto EXIT;
 	}
 
 	ret = snd_soc_component_update_bits(component, TAS27XX_TDM_CFG2,
 					    TAS27XX_TDM_CFG2_RXS_MASK,
 					    slot_size);
-	if (ret < 0)
-		return ret;
+	if (ret < 0) {
+		pr_err("%s:%u:errCode:0x%x Failed to set slot_size\n",
+			__func__, __LINE__, ret);
+		goto EXIT;
+	}
 
 	ret = snd_soc_component_update_bits(component, TAS27XX_TDM_CFG5,
 					    TAS27XX_TDM_CFG5_50_MASK,
 					    tas27xx->v_sense_slot);
-	if (ret < 0)
-		return ret;
+	if (ret < 0) {
+		pr_err("%s:%u:errCode:0x%x Failed to set v_sense_slot\n",
+			__func__, __LINE__, ret);
+		goto EXIT;
+	}
 
 	ret = snd_soc_component_update_bits(component, TAS27XX_TDM_CFG6,
 					    TAS27XX_TDM_CFG6_50_MASK,
 					    tas27xx->i_sense_slot);
-	if (ret < 0)
-		return ret;
-
-	return 0;
+	if (ret < 0) {
+		pr_err("%s:%u:errCode:0x%x Failed to set i_sense_slot\n",
+			__func__, __LINE__, ret);
+		goto EXIT;
+	}
+	ret = 0;
+EXIT:
+	mutex_unlock(&tas27xx->codec_lock);
+	return ret;
 }
 
 static const struct snd_soc_dai_ops tas27xx_dai_ops = {
@@ -500,7 +626,7 @@  static struct snd_soc_dai_driver tas27xx_dai_driver[] = {
 		},
 		.capture = {
 			.stream_name    = "ASI1 Capture",
-			.channels_min   = 0,
+			.channels_min   = 1,
 			.channels_max   = 2,
 			.rates = TAS27XX_RATES,
 			.formats = TAS27XX_FORMATS,
@@ -512,9 +638,9 @@  static struct snd_soc_dai_driver tas27xx_dai_driver[] = {
 
 static int tas27xx_codec_probe(struct snd_soc_component *component)
 {
-	struct tas27xx_priv *tas27xx =
+	struct tas27xx_priv *tas27xx =
 		snd_soc_component_get_drvdata(component);
-	int ret;
+	int ret = 0;
 
 	tas27xx->component = component;
 
@@ -522,26 +648,54 @@  static int tas27xx_codec_probe(struct snd_soc_component *component)
 		gpiod_set_value_cansleep(tas27xx->sdz_gpio, 1);
 
 	tas27xx_reset(tas27xx);
+	usleep_range(5000, 5050);
 
-	ret = snd_soc_component_update_bits(tas27xx->component,
-						TAS27XX_TDM_CFG5,
+	ret = snd_soc_component_update_bits(component,
+		TAS27XX_CLK_CFG,
+		TAS27XX_CLK_CFG_MASK,
+		TAS27XX_CLK_CFG_ENABLE);
+	if (ret < 0) {
+		dev_err(tas27xx->dev,
+			"%s:%u: Failed to CLK_CFG_ENABLE\n",
+			__func__, __LINE__);
+		goto EXIT;
+	}
+
+	ret = snd_soc_component_update_bits(component, TAS27XX_TDM_CFG5,
 					    TAS27XX_TDM_CFG5_VSNS_ENABLE, 0);
-	if (ret < 0)
-		return ret;
+	if (ret < 0) {
+		dev_err(tas27xx->dev, "%s:%u: Failed to enable vSNS\n",
+			__func__, __LINE__);
+		goto EXIT;
+	}
 
-	ret = snd_soc_component_update_bits(tas27xx->component,
-						TAS27XX_TDM_CFG6,
+	ret = snd_soc_component_update_bits(component, TAS27XX_TDM_CFG6,
 					    TAS27XX_TDM_CFG6_ISNS_ENABLE, 0);
-	if (ret < 0)
-		return ret;
+	if (ret < 0) {
+		dev_err(tas27xx->dev, "%s:%u: Failed to enable iSNS\n",
+			__func__, __LINE__);
+		goto EXIT;
+	}
 
 	ret = snd_soc_component_update_bits(component, TAS27XX_PWR_CTRL,
 					    TAS27XX_PWR_CTRL_MASK,
 					    TAS27XX_PWR_CTRL_MUTE);
-	if (ret < 0)
-		return ret;
+	if (ret < 0) {
+		dev_err(tas27xx->dev, "%s:%u: Failed to PWR_CTRL_MUTE\n",
+			__func__, __LINE__);
+		goto EXIT;
+	}
 
-	return 0;
+	ret = snd_soc_component_write(component, TAS27XX_PWR_CTRL, 0x02);
+
+	if (ret < 0) {
+		dev_err(tas27xx->dev, "%s:%u: Failed to initial active\n",
+			__func__, __LINE__);
+		goto EXIT;
+	}
+	ret = 0;
+EXIT:
+	return ret;
 }
 
 static DECLARE_TLV_DB_SCALE(tas27xx_digital_tlv, 1100, 50, 0);
@@ -556,8 +710,10 @@  static const struct snd_kcontrol_new tas27xx_snd_controls[] = {
 
 static const struct snd_soc_component_driver soc_component_driver_tas27xx = {
 	.probe			= tas27xx_codec_probe,
+#ifdef CONFIG_PM
 	.suspend		= tas27xx_codec_suspend,
 	.resume			= tas27xx_codec_resume,
+#endif
 	.set_bias_level		= tas27xx_set_bias_level,
 	.controls		= tas27xx_snd_controls,
 	.num_controls		= ARRAY_SIZE(tas27xx_snd_controls),
@@ -595,6 +751,27 @@  static const struct regmap_range_cfg tas27xx_regmap_ranges[] = {
 	},
 };
 
+static bool tas27xx_volatile(struct device *dev,
+	unsigned int reg)
+{
+	switch (reg) {
+	case TAS27XX_SW_RST:
+	case TAS27XX_PWR_CTRL:
+	case TAS27XX_PAGE:
+	case TAS27XX_DVC:
+	case TAS27XX_CHNL_0:
+	case TAS27XX_TDM_CFG0:
+	case TAS27XX_TDM_CFG1:
+	case TAS27XX_TDM_CFG2:
+	case TAS27XX_TDM_CFG3:
+	case TAS27XX_TDM_CFG5:
+	case TAS27XX_TDM_CFG6:
+		return true;
+	default:
+		return false;
+	}
+}
+
 static const struct regmap_config tas27xx_i2c_regmap = {
 	.reg_bits = 8,
 	.val_bits = 8,
@@ -604,6 +781,7 @@  static const struct regmap_config tas27xx_i2c_regmap = {
 	.ranges = tas27xx_regmap_ranges,
 	.num_ranges = ARRAY_SIZE(tas27xx_regmap_ranges),
 	.max_register = 1 * 128,
+	.volatile_reg = tas27xx_volatile,
 };
 
 static int tas27xx_parse_dt(struct device *dev, struct tas27xx_priv *tas27xx)
@@ -619,7 +797,7 @@  static int tas27xx_parse_dt(struct device *dev, struct tas27xx_priv *tas27xx)
 		}
 	}
 
-	tas27xx->sdz_gpio = devm_gpiod_get_optional(dev, "shutdown",
+	tas27xx->sdz_gpio = devm_gpiod_get_optional(dev, "shutdown",
 		GPIOD_OUT_HIGH);
 	if (IS_ERR(tas27xx->sdz_gpio)) {
 		if (PTR_ERR(tas27xx->sdz_gpio) == -EPROBE_DEFER)
@@ -651,7 +829,7 @@  static int tas27xx_i2c_probe(struct i2c_client *client,
 			       GFP_KERNEL);
 	if (!tas27xx)
 		return -ENOMEM;
-
+	mutex_init(&tas27xx->codec_lock);
 	tas27xx->dev = &client->dev;
 	i2c_set_clientdata(client, tas27xx);
 	dev_set_drvdata(&client->dev, tas27xx);
@@ -667,12 +845,15 @@  static int tas27xx_i2c_probe(struct i2c_client *client,
 	if (client->dev.of_node) {
 		result = tas27xx_parse_dt(&client->dev, tas27xx);
 		if (result) {
-			dev_err(tas27xx->dev,
+			dev_err(tas27xx->dev,
 				"%s: Failed to parse devicetree\n", __func__);
 			return result;
 		}
 	}
 
+	tas27xx->device_id = id->driver_data;
+	dev_info(tas27xx->dev, "chip_id: %u\n", tas27xx->device_id);
+
 	return devm_snd_soc_register_component(tas27xx->dev,
 		&soc_component_driver_tas27xx, tas27xx_dai_driver,
 		ARRAY_SIZE(tas27xx_dai_driver));
@@ -680,6 +861,7 @@  static int tas27xx_i2c_probe(struct i2c_client *client,
 
 static const struct i2c_device_id tas27xx_i2c_id[] = {
 	{ "tas2764", TAS2764},
+	{ "tas2780", TAS2780},
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, tas27xx_i2c_id);
@@ -687,6 +869,7 @@  MODULE_DEVICE_TABLE(i2c, tas27xx_i2c_id);
 #if defined(CONFIG_OF)
 static const struct of_device_id tas27xx_of_match[] = {
 	{ .compatible = "ti,tas2764" },
+	{ .compatible = "ti,tas2780" },
 	{},
 };
 MODULE_DEVICE_TABLE(of, tas27xx_of_match);
diff --git a/sound/soc/codecs/tas27xx.h b/sound/soc/codecs/tas27xx.h
index 6f76645f5cd6..95923e437a38 100644
--- a/sound/soc/codecs/tas27xx.h
+++ b/sound/soc/codecs/tas27xx.h
@@ -1,6 +1,6 @@ 
 /* SPDX-License-Identifier: GPL-2.0-only */
 /*
- * tas27xx.h - ALSA SoC Texas Instruments TAS2764 Mono Audio Amplifier
+ * tas27xx.h - ALSA SoC Texas Instruments TAS2764/TAS2780 Mono Audio Amplifier
  *
  * Copyright (C) 2020-2022 Texas Instruments Incorporated - https://www.ti.com
  *