alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/9] ASoC: tas2770: Fix calling reset in probe
@ 2020-09-18 19:05 Dan Murphy
  2020-09-18 19:05 ` [PATCH 2/9] ASoC: tas2770: Add missing bias level power states Dan Murphy
                   ` (8 more replies)
  0 siblings, 9 replies; 14+ messages in thread
From: Dan Murphy @ 2020-09-18 19:05 UTC (permalink / raw)
  To: lgirdwood, broonie, tiwai, robh+dt
  Cc: devicetree, alsa-devel, linux-kernel, Dan Murphy

tas2770_reset is called during i2c probe. The reset calls the
snd_soc_component_write which depends on the tas2770->component being
available. The component pointer is not set until codec_probe so move
the reset to the codec_probe after the pointer is set.

Fixes: 1a476abc723e6 ("tas2770: add tas2770 smart PA kernel driver")
Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 sound/soc/codecs/tas2770.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/tas2770.c b/sound/soc/codecs/tas2770.c
index c09851834395..03d7ad1885b8 100644
--- a/sound/soc/codecs/tas2770.c
+++ b/sound/soc/codecs/tas2770.c
@@ -575,6 +575,8 @@ static int tas2770_codec_probe(struct snd_soc_component *component)
 
 	tas2770->component = component;
 
+	tas2770_reset(tas2770);
+
 	return 0;
 }
 
@@ -771,8 +773,6 @@ static int tas2770_i2c_probe(struct i2c_client *client,
 	tas2770->channel_size = 0;
 	tas2770->slot_width = 0;
 
-	tas2770_reset(tas2770);
-
 	result = tas2770_register_codec(tas2770);
 	if (result)
 		dev_err(tas2770->dev, "Register codec failed.\n");
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/9] ASoC: tas2770: Add missing bias level power states
  2020-09-18 19:05 [PATCH 1/9] ASoC: tas2770: Fix calling reset in probe Dan Murphy
@ 2020-09-18 19:05 ` Dan Murphy
  2020-09-18 19:05 ` [PATCH 3/9] dt-bindings: tas2770: Fix I2C addresses for the TAS2770 Dan Murphy
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Dan Murphy @ 2020-09-18 19:05 UTC (permalink / raw)
  To: lgirdwood, broonie, tiwai, robh+dt
  Cc: devicetree, alsa-devel, linux-kernel, Dan Murphy

Add the BIAS_STANDBY and BIAS_PREPARE to the set_bias_level or else the
driver will return -EINVAL which is not correct as they are valid
states.

Fixes: 1a476abc723e6 ("tas2770: add tas2770 smart PA kernel driver")
Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 sound/soc/codecs/tas2770.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/sound/soc/codecs/tas2770.c b/sound/soc/codecs/tas2770.c
index 03d7ad1885b8..7c6f61946ab3 100644
--- a/sound/soc/codecs/tas2770.c
+++ b/sound/soc/codecs/tas2770.c
@@ -57,7 +57,12 @@ static int tas2770_set_bias_level(struct snd_soc_component *component,
 			TAS2770_PWR_CTRL_MASK,
 			TAS2770_PWR_CTRL_ACTIVE);
 		break;
-
+	case SND_SOC_BIAS_STANDBY:
+	case SND_SOC_BIAS_PREPARE:
+		snd_soc_component_update_bits(component,
+			TAS2770_PWR_CTRL,
+			TAS2770_PWR_CTRL_MASK, TAS2770_PWR_CTRL_MUTE);
+		break;
 	case SND_SOC_BIAS_OFF:
 		snd_soc_component_update_bits(component,
 			TAS2770_PWR_CTRL,
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/9] dt-bindings: tas2770: Fix I2C addresses for the TAS2770
  2020-09-18 19:05 [PATCH 1/9] ASoC: tas2770: Fix calling reset in probe Dan Murphy
  2020-09-18 19:05 ` [PATCH 2/9] ASoC: tas2770: Add missing bias level power states Dan Murphy
@ 2020-09-18 19:05 ` Dan Murphy
  2020-09-18 19:05 ` [PATCH 4/9] ASoC: tas2770: Fix required DT properties in the code Dan Murphy
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Dan Murphy @ 2020-09-18 19:05 UTC (permalink / raw)
  To: lgirdwood, broonie, tiwai, robh+dt
  Cc: devicetree, alsa-devel, linux-kernel, Dan Murphy

The I2C addresses listed in the yaml are not correct. The addresses can
range from 0x41 through 0x48 based on register configurations. Fix the
example and the description.

Fixes: 4b7151dadfd4 ("dt-bindings: ASoC: Add tas2770 smart PA dt bindings")
Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 Documentation/devicetree/bindings/sound/tas2770.yaml | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/tas2770.yaml b/Documentation/devicetree/bindings/sound/tas2770.yaml
index 33a90f829c80..bb26d081c9fa 100644
--- a/Documentation/devicetree/bindings/sound/tas2770.yaml
+++ b/Documentation/devicetree/bindings/sound/tas2770.yaml
@@ -24,7 +24,7 @@ properties:
   reg:
     maxItems: 1
     description: |
-       I2C address of the device can be one of these 0x4c, 0x4d, 0x4e or 0x4f
+       I2C address of the device can be between 0x41 to 0x48.
 
   reset-gpio:
     description: GPIO used to reset the device.
@@ -62,9 +62,9 @@ examples:
    i2c0 {
      #address-cells = <1>;
      #size-cells = <0>;
-     codec: codec@4c {
+     codec: codec@41 {
        compatible = "ti,tas2770";
-       reg = <0x4c>;
+       reg = <0x41>;
        #sound-dai-cells = <1>;
        interrupt-parent = <&gpio1>;
        interrupts = <14>;
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4/9] ASoC: tas2770: Fix required DT properties in the code
  2020-09-18 19:05 [PATCH 1/9] ASoC: tas2770: Fix calling reset in probe Dan Murphy
  2020-09-18 19:05 ` [PATCH 2/9] ASoC: tas2770: Add missing bias level power states Dan Murphy
  2020-09-18 19:05 ` [PATCH 3/9] dt-bindings: tas2770: Fix I2C addresses for the TAS2770 Dan Murphy
@ 2020-09-18 19:05 ` Dan Murphy
  2020-09-18 19:05 ` [PATCH 5/9] ASoC: tas2770: Fix unbalanced calls to pm_runtime Dan Murphy
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Dan Murphy @ 2020-09-18 19:05 UTC (permalink / raw)
  To: lgirdwood, broonie, tiwai, robh+dt
  Cc: devicetree, alsa-devel, linux-kernel, Dan Murphy

The devicetree binding indicates that the ti,asi-format, ti,imon-slot-no
and ti,vmon-slot-no are not required but the driver requires them or it
fails to probe. Honor the binding and allow these entries to be optional
and set the corresponding values to the default values for each as defined
in the data sheet.

Fixes: 1a476abc723e6 ("tas2770: add tas2770 smart PA kernel driver")
Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 sound/soc/codecs/tas2770.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/sound/soc/codecs/tas2770.c b/sound/soc/codecs/tas2770.c
index 7c6f61946ab3..bdfdad5f4f64 100644
--- a/sound/soc/codecs/tas2770.c
+++ b/sound/soc/codecs/tas2770.c
@@ -708,29 +708,28 @@ static int tas2770_parse_dt(struct device *dev, struct tas2770_priv *tas2770)
 	rc = fwnode_property_read_u32(dev->fwnode, "ti,asi-format",
 					&tas2770->asi_format);
 	if (rc) {
-		dev_err(tas2770->dev, "Looking up %s property failed %d\n",
-			"ti,asi-format", rc);
-		goto end;
+		dev_info(tas2770->dev, "Property %s is missing setting default slot\n",
+			"ti,asi-format");
+		tas2770->asi_format = 0;
 	}
 
 	rc = fwnode_property_read_u32(dev->fwnode, "ti,imon-slot-no",
 			&tas2770->i_sense_slot);
 	if (rc) {
-		dev_err(tas2770->dev, "Looking up %s property failed %d\n",
-			"ti,imon-slot-no", rc);
-		goto end;
+		dev_info(tas2770->dev, "Property %s is missing setting default slot\n",
+			"ti,imon-slot-no");
+		tas2770->i_sense_slot = 0;
 	}
 
 	rc = fwnode_property_read_u32(dev->fwnode, "ti,vmon-slot-no",
 				&tas2770->v_sense_slot);
 	if (rc) {
-		dev_err(tas2770->dev, "Looking up %s property failed %d\n",
-			"ti,vmon-slot-no", rc);
-		goto end;
+		dev_info(tas2770->dev, "Property %s is missing setting default slot\n",
+			"ti,vmon-slot-no");
+		tas2770->v_sense_slot = 2;
 	}
 
-end:
-	return rc;
+	return 0;
 }
 
 static int tas2770_i2c_probe(struct i2c_client *client,
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 5/9] ASoC: tas2770: Fix unbalanced calls to pm_runtime
  2020-09-18 19:05 [PATCH 1/9] ASoC: tas2770: Fix calling reset in probe Dan Murphy
                   ` (2 preceding siblings ...)
  2020-09-18 19:05 ` [PATCH 4/9] ASoC: tas2770: Fix required DT properties in the code Dan Murphy
@ 2020-09-18 19:05 ` Dan Murphy
  2020-09-18 19:05 ` [PATCH 6/9] ASoC: tas2770: Convert bit mask to GENMASK in header Dan Murphy
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Dan Murphy @ 2020-09-18 19:05 UTC (permalink / raw)
  To: lgirdwood, broonie, tiwai, robh+dt
  Cc: devicetree, alsa-devel, linux-kernel, Dan Murphy

Fix the unbalanced call to the pm_runtime_disable when removing the
module.  pm_runtime_enable is not called nor is the pm_runtime setup in
the code.  Remove the i2c_remove function and the pm_runtime_disable.

Fixes: 1a476abc723e6 ("tas2770: add tas2770 smart PA kernel driver")
Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 sound/soc/codecs/tas2770.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/sound/soc/codecs/tas2770.c b/sound/soc/codecs/tas2770.c
index bdfdad5f4f64..16979583cd68 100644
--- a/sound/soc/codecs/tas2770.c
+++ b/sound/soc/codecs/tas2770.c
@@ -16,7 +16,6 @@
 #include <linux/i2c.h>
 #include <linux/gpio.h>
 #include <linux/gpio/consumer.h>
-#include <linux/pm_runtime.h>
 #include <linux/regulator/consumer.h>
 #include <linux/firmware.h>
 #include <linux/regmap.h>
@@ -785,13 +784,6 @@ static int tas2770_i2c_probe(struct i2c_client *client,
 	return result;
 }
 
-static int tas2770_i2c_remove(struct i2c_client *client)
-{
-	pm_runtime_disable(&client->dev);
-	return 0;
-}
-
-
 static const struct i2c_device_id tas2770_i2c_id[] = {
 	{ "tas2770", 0},
 	{ }
@@ -812,7 +804,6 @@ static struct i2c_driver tas2770_i2c_driver = {
 		.of_match_table = of_match_ptr(tas2770_of_match),
 	},
 	.probe      = tas2770_i2c_probe,
-	.remove     = tas2770_i2c_remove,
 	.id_table   = tas2770_i2c_id,
 };
 
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 6/9] ASoC: tas2770: Convert bit mask to GENMASK in header
  2020-09-18 19:05 [PATCH 1/9] ASoC: tas2770: Fix calling reset in probe Dan Murphy
                   ` (3 preceding siblings ...)
  2020-09-18 19:05 ` [PATCH 5/9] ASoC: tas2770: Fix unbalanced calls to pm_runtime Dan Murphy
@ 2020-09-18 19:05 ` Dan Murphy
  2020-09-21 19:04   ` Mark Brown
  2020-09-18 19:05 ` [PATCH 7/9] ASoC: tas2770: Fix error handling with update_bits Dan Murphy
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 14+ messages in thread
From: Dan Murphy @ 2020-09-18 19:05 UTC (permalink / raw)
  To: lgirdwood, broonie, tiwai, robh+dt
  Cc: devicetree, alsa-devel, linux-kernel, Dan Murphy

Update the hardcoded masks with the GENMASK macro. Also update some of
the hardcoded bits with the BIT macro

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 sound/soc/codecs/tas2770.h | 37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/sound/soc/codecs/tas2770.h b/sound/soc/codecs/tas2770.h
index 96683971ee9b..07e3556fc702 100644
--- a/sound/soc/codecs/tas2770.h
+++ b/sound/soc/codecs/tas2770.h
@@ -19,7 +19,7 @@
 #define TAS2770_RST  BIT(0)
     /* Power Control */
 #define TAS2770_PWR_CTRL  TAS2770_REG(0X0, 0x02)
-#define TAS2770_PWR_CTRL_MASK  0x3
+#define TAS2770_PWR_CTRL_MASK  GENMASK(1, 0)
 #define TAS2770_PWR_CTRL_ACTIVE  0x0
 #define TAS2770_PWR_CTRL_MUTE  BIT(0)
 #define TAS2770_PWR_CTRL_SHUTDOWN  0x2
@@ -37,43 +37,43 @@
 #define TAS2770_TDM_CFG_REG0_SMP_MASK  BIT(5)
 #define TAS2770_TDM_CFG_REG0_SMP_48KHZ  0x0
 #define TAS2770_TDM_CFG_REG0_SMP_44_1KHZ  BIT(5)
-#define TAS2770_TDM_CFG_REG0_31_MASK  0xe
+#define TAS2770_TDM_CFG_REG0_31_MASK  GENMASK(3, 1)
 #define TAS2770_TDM_CFG_REG0_31_44_1_48KHZ  0x6
 #define TAS2770_TDM_CFG_REG0_31_88_2_96KHZ  0x8
 #define TAS2770_TDM_CFG_REG0_31_176_4_192KHZ  0xa
     /* TDM Configuration Reg1 */
 #define TAS2770_TDM_CFG_REG1  TAS2770_REG(0X0, 0x0B)
-#define TAS2770_TDM_CFG_REG1_MASK 0x3e
+#define TAS2770_TDM_CFG_REG1_MASK	GENMASK(5, 1)
 #define TAS2770_TDM_CFG_REG1_51_SHIFT  1
 #define TAS2770_TDM_CFG_REG1_RX_MASK  BIT(0)
 #define TAS2770_TDM_CFG_REG1_RX_RSING  0x0
 #define TAS2770_TDM_CFG_REG1_RX_FALING  BIT(0)
     /* TDM Configuration Reg2 */
 #define TAS2770_TDM_CFG_REG2  TAS2770_REG(0X0, 0x0C)
-#define TAS2770_TDM_CFG_REG2_RXW_MASK  0xc
+#define TAS2770_TDM_CFG_REG2_RXW_MASK	GENMASK(3, 2)
 #define TAS2770_TDM_CFG_REG2_RXW_16BITS  0x0
 #define TAS2770_TDM_CFG_REG2_RXW_24BITS  0x8
 #define TAS2770_TDM_CFG_REG2_RXW_32BITS  0xc
-#define TAS2770_TDM_CFG_REG2_RXS_MASK    0x3
+#define TAS2770_TDM_CFG_REG2_RXS_MASK    GENMASK(1, 0)
 #define TAS2770_TDM_CFG_REG2_RXS_16BITS  0x0
 #define TAS2770_TDM_CFG_REG2_RXS_24BITS  BIT(0)
 #define TAS2770_TDM_CFG_REG2_RXS_32BITS  0x2
     /* TDM Configuration Reg3 */
 #define TAS2770_TDM_CFG_REG3  TAS2770_REG(0X0, 0x0D)
-#define TAS2770_TDM_CFG_REG3_RXS_MASK  0xf0
+#define TAS2770_TDM_CFG_REG3_RXS_MASK  GENMASK(7, 4)
 #define TAS2770_TDM_CFG_REG3_RXS_SHIFT 0x4
-#define TAS2770_TDM_CFG_REG3_30_MASK  0xf
+#define TAS2770_TDM_CFG_REG3_30_MASK  GENMASK(3, 0)
 #define TAS2770_TDM_CFG_REG3_30_SHIFT 0
     /* TDM Configuration Reg5 */
 #define TAS2770_TDM_CFG_REG5  TAS2770_REG(0X0, 0x0F)
 #define TAS2770_TDM_CFG_REG5_VSNS_MASK  BIT(6)
 #define TAS2770_TDM_CFG_REG5_VSNS_ENABLE  BIT(6)
-#define TAS2770_TDM_CFG_REG5_50_MASK  0x3f
+#define TAS2770_TDM_CFG_REG5_50_MASK	GENMASK(5, 0)
     /* TDM Configuration Reg6 */
 #define TAS2770_TDM_CFG_REG6  TAS2770_REG(0X0, 0x10)
 #define TAS2770_TDM_CFG_REG6_ISNS_MASK  BIT(6)
 #define TAS2770_TDM_CFG_REG6_ISNS_ENABLE  BIT(6)
-#define TAS2770_TDM_CFG_REG6_50_MASK  0x3f
+#define TAS2770_TDM_CFG_REG6_50_MASK  GENMASK(5, 0)
     /* Brown Out Prevention Reg0 */
 #define TAS2770_BO_PRV_REG0  TAS2770_REG(0X0, 0x1B)
     /* Interrupt MASK Reg0 */
@@ -116,15 +116,16 @@
     /* Revision and PG ID */
 #define TAS2770_REV_AND_GPID  TAS2770_REG(0X0, 0x7D)
 
-#define TAS2770_POWER_ACTIVE 0
-#define TAS2770_POWER_MUTE 1
-#define TAS2770_POWER_SHUTDOWN 2
-#define ERROR_OVER_CURRENT  0x0000001
-#define ERROR_DIE_OVERTEMP  0x0000002
-#define ERROR_OVER_VOLTAGE  0x0000004
-#define ERROR_UNDER_VOLTAGE 0x0000008
-#define ERROR_BROWNOUT      0x0000010
-#define ERROR_CLASSD_PWR    0x0000020
+#define TAS2770_POWER_ACTIVE	0
+#define TAS2770_POWER_MUTE	BIT(0)
+#define TAS2770_POWER_SHUTDOWN	BIT(1)
+
+#define ERROR_OVER_CURRENT  BIT(0)
+#define ERROR_DIE_OVERTEMP  BIT(1)
+#define ERROR_OVER_VOLTAGE  BIT(2)
+#define ERROR_UNDER_VOLTAGE BIT(3)
+#define ERROR_BROWNOUT      BIT(4)
+#define ERROR_CLASSD_PWR    BIT(5)
 
 struct tas2770_priv {
 	struct device *dev;
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 7/9] ASoC: tas2770: Fix error handling with update_bits
  2020-09-18 19:05 [PATCH 1/9] ASoC: tas2770: Fix calling reset in probe Dan Murphy
                   ` (4 preceding siblings ...)
  2020-09-18 19:05 ` [PATCH 6/9] ASoC: tas2770: Convert bit mask to GENMASK in header Dan Murphy
@ 2020-09-18 19:05 ` Dan Murphy
  2020-09-18 19:05 ` [PATCH 8/9] ASoC: tas2770: Fix the spacing and new lines Dan Murphy
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Dan Murphy @ 2020-09-18 19:05 UTC (permalink / raw)
  To: lgirdwood, broonie, tiwai, robh+dt
  Cc: devicetree, alsa-devel, linux-kernel, Dan Murphy

snd_soc_update_bits returns a 1 when the bit was successfully updated,
returns a 0 is no update was needed and a negative if the call failed.
The code is currently failing the case of a successful update by just
checking for a non-zero number. Modify these checks and return the error
code only if there is a negative.

Fixes: 1a476abc723e6 ("tas2770: add tas2770 smart PA kernel driver")
Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 sound/soc/codecs/tas2770.c | 52 ++++++++++++++++++--------------------
 1 file changed, 24 insertions(+), 28 deletions(-)

diff --git a/sound/soc/codecs/tas2770.c b/sound/soc/codecs/tas2770.c
index 16979583cd68..3226c6d4493e 100644
--- a/sound/soc/codecs/tas2770.c
+++ b/sound/soc/codecs/tas2770.c
@@ -139,23 +139,18 @@ static int tas2770_dac_event(struct snd_soc_dapm_widget *w,
 			TAS2770_PWR_CTRL,
 			TAS2770_PWR_CTRL_MASK,
 			TAS2770_PWR_CTRL_MUTE);
-		if (ret)
-			goto end;
 		break;
 	case SND_SOC_DAPM_PRE_PMD:
 		ret = snd_soc_component_update_bits(component,
 			TAS2770_PWR_CTRL,
 			TAS2770_PWR_CTRL_MASK,
 			TAS2770_PWR_CTRL_SHUTDOWN);
-		if (ret)
-			goto end;
 		break;
 	default:
 		dev_err(tas2770->dev, "Not supported evevt\n");
 		return -EINVAL;
 	}
 
-end:
 	if (ret < 0)
 		return ret;
 
@@ -247,6 +242,9 @@ static int tas2770_set_bitwidth(struct tas2770_priv *tas2770, int bitwidth)
 		return -EINVAL;
 	}
 
+	if (ret < 0)
+		return ret;
+
 	tas2770->channel_size = bitwidth;
 
 	ret = snd_soc_component_update_bits(component,
@@ -255,16 +253,15 @@ static int tas2770_set_bitwidth(struct tas2770_priv *tas2770, int bitwidth)
 		TAS2770_TDM_CFG_REG5_50_MASK,
 		TAS2770_TDM_CFG_REG5_VSNS_ENABLE |
 		tas2770->v_sense_slot);
-	if (ret)
-		goto end;
+	if (ret < 0)
+		return ret;
+
 	ret = snd_soc_component_update_bits(component,
 		TAS2770_TDM_CFG_REG6,
 		TAS2770_TDM_CFG_REG6_ISNS_MASK |
 		TAS2770_TDM_CFG_REG6_50_MASK,
 		TAS2770_TDM_CFG_REG6_ISNS_ENABLE |
 		tas2770->i_sense_slot);
-
-end:
 	if (ret < 0)
 		return ret;
 
@@ -282,36 +279,35 @@ static int tas2770_set_samplerate(struct tas2770_priv *tas2770, int samplerate)
 			TAS2770_TDM_CFG_REG0,
 			TAS2770_TDM_CFG_REG0_SMP_MASK,
 			TAS2770_TDM_CFG_REG0_SMP_48KHZ);
-		if (ret)
-			goto end;
+		if (ret < 0)
+			return ret;
+
 		ret = snd_soc_component_update_bits(component,
 			TAS2770_TDM_CFG_REG0,
 			TAS2770_TDM_CFG_REG0_31_MASK,
 			TAS2770_TDM_CFG_REG0_31_44_1_48KHZ);
-		if (ret)
-			goto end;
 		break;
 	case 44100:
 		ret = snd_soc_component_update_bits(component,
 			TAS2770_TDM_CFG_REG0,
 			TAS2770_TDM_CFG_REG0_SMP_MASK,
 			TAS2770_TDM_CFG_REG0_SMP_44_1KHZ);
-		if (ret)
-			goto end;
+		if (ret < 0)
+			return ret;
+
 		ret = snd_soc_component_update_bits(component,
 			TAS2770_TDM_CFG_REG0,
 			TAS2770_TDM_CFG_REG0_31_MASK,
 			TAS2770_TDM_CFG_REG0_31_44_1_48KHZ);
-		if (ret)
-			goto end;
 		break;
 	case 96000:
 		ret = snd_soc_component_update_bits(component,
 			TAS2770_TDM_CFG_REG0,
 			TAS2770_TDM_CFG_REG0_SMP_MASK,
 			TAS2770_TDM_CFG_REG0_SMP_48KHZ);
-		if (ret)
-			goto end;
+		if (ret < 0)
+			return ret;
+
 		ret = snd_soc_component_update_bits(component,
 			TAS2770_TDM_CFG_REG0,
 			TAS2770_TDM_CFG_REG0_31_MASK,
@@ -322,8 +318,9 @@ static int tas2770_set_samplerate(struct tas2770_priv *tas2770, int samplerate)
 			TAS2770_TDM_CFG_REG0,
 			TAS2770_TDM_CFG_REG0_SMP_MASK,
 			TAS2770_TDM_CFG_REG0_SMP_44_1KHZ);
-		if (ret)
-			goto end;
+		if (ret < 0)
+			return ret;
+
 		ret = snd_soc_component_update_bits(component,
 			TAS2770_TDM_CFG_REG0,
 			TAS2770_TDM_CFG_REG0_31_MASK,
@@ -334,22 +331,22 @@ static int tas2770_set_samplerate(struct tas2770_priv *tas2770, int samplerate)
 			TAS2770_TDM_CFG_REG0,
 			TAS2770_TDM_CFG_REG0_SMP_MASK,
 			TAS2770_TDM_CFG_REG0_SMP_48KHZ);
-		if (ret)
-			goto end;
+		if (ret < 0)
+			return ret;
+
 		ret = snd_soc_component_update_bits(component,
 			TAS2770_TDM_CFG_REG0,
 			TAS2770_TDM_CFG_REG0_31_MASK,
 			TAS2770_TDM_CFG_REG0_31_176_4_192KHZ);
-		if (ret)
-			goto end;
 		break;
 	case 17640:
 		ret = snd_soc_component_update_bits(component,
 			TAS2770_TDM_CFG_REG0,
 			TAS2770_TDM_CFG_REG0_SMP_MASK,
 			TAS2770_TDM_CFG_REG0_SMP_44_1KHZ);
-		if (ret)
-			goto end;
+		if (ret < 0)
+			return ret;
+
 		ret = snd_soc_component_update_bits(component,
 			TAS2770_TDM_CFG_REG0,
 			TAS2770_TDM_CFG_REG0_31_MASK,
@@ -359,7 +356,6 @@ static int tas2770_set_samplerate(struct tas2770_priv *tas2770, int samplerate)
 		ret = -EINVAL;
 	}
 
-end:
 	if (ret < 0)
 		return ret;
 
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 8/9] ASoC: tas2770: Fix the spacing and new lines
  2020-09-18 19:05 [PATCH 1/9] ASoC: tas2770: Fix calling reset in probe Dan Murphy
                   ` (5 preceding siblings ...)
  2020-09-18 19:05 ` [PATCH 7/9] ASoC: tas2770: Fix error handling with update_bits Dan Murphy
@ 2020-09-18 19:05 ` Dan Murphy
  2020-09-18 19:05 ` [PATCH 9/9] ASoC: tas2770: Refactor sample rate function Dan Murphy
  2020-09-22  0:05 ` [PATCH 1/9] ASoC: tas2770: Fix calling reset in probe Mark Brown
  8 siblings, 0 replies; 14+ messages in thread
From: Dan Murphy @ 2020-09-18 19:05 UTC (permalink / raw)
  To: lgirdwood, broonie, tiwai, robh+dt
  Cc: devicetree, alsa-devel, linux-kernel, Dan Murphy

Fix up the spacing for argument alignment and add new lines to separate
code. Eliminate unneccessary goto statements when the error code could
just be returned.

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 sound/soc/codecs/tas2770.c | 300 ++++++++++++++++---------------------
 1 file changed, 131 insertions(+), 169 deletions(-)

diff --git a/sound/soc/codecs/tas2770.c b/sound/soc/codecs/tas2770.c
index 3226c6d4493e..b17cf0a7f785 100644
--- a/sound/soc/codecs/tas2770.c
+++ b/sound/soc/codecs/tas2770.c
@@ -39,6 +39,7 @@ static void tas2770_reset(struct tas2770_priv *tas2770)
 		msleep(20);
 		gpiod_set_value_cansleep(tas2770->reset_gpio, 1);
 	}
+
 	snd_soc_component_write(tas2770->component, TAS2770_SW_RST,
 		TAS2770_RST);
 }
@@ -51,27 +52,24 @@ static int tas2770_set_bias_level(struct snd_soc_component *component,
 
 	switch (level) {
 	case SND_SOC_BIAS_ON:
-		snd_soc_component_update_bits(component,
-			TAS2770_PWR_CTRL,
-			TAS2770_PWR_CTRL_MASK,
-			TAS2770_PWR_CTRL_ACTIVE);
+		snd_soc_component_update_bits(component, TAS2770_PWR_CTRL,
+					      TAS2770_PWR_CTRL_MASK,
+					      TAS2770_PWR_CTRL_ACTIVE);
 		break;
 	case SND_SOC_BIAS_STANDBY:
 	case SND_SOC_BIAS_PREPARE:
-		snd_soc_component_update_bits(component,
-			TAS2770_PWR_CTRL,
-			TAS2770_PWR_CTRL_MASK, TAS2770_PWR_CTRL_MUTE);
+		snd_soc_component_update_bits(component, TAS2770_PWR_CTRL,
+					      TAS2770_PWR_CTRL_MASK,
+					      TAS2770_PWR_CTRL_MUTE);
 		break;
 	case SND_SOC_BIAS_OFF:
-		snd_soc_component_update_bits(component,
-			TAS2770_PWR_CTRL,
-			TAS2770_PWR_CTRL_MASK,
-			TAS2770_PWR_CTRL_SHUTDOWN);
+		snd_soc_component_update_bits(component, TAS2770_PWR_CTRL,
+					      TAS2770_PWR_CTRL_MASK,
+					      TAS2770_PWR_CTRL_SHUTDOWN);
 		break;
 
 	default:
-		dev_err(tas2770->dev,
-				"wrong power level setting %d\n", level);
+		dev_err(tas2770->dev, "wrong power level setting %d\n", level);
 		return -EINVAL;
 	}
 
@@ -83,11 +81,9 @@ static int tas2770_codec_suspend(struct snd_soc_component *component)
 {
 	int ret;
 
-	ret = snd_soc_component_update_bits(component,
-		TAS2770_PWR_CTRL,
-		TAS2770_PWR_CTRL_MASK,
-		TAS2770_PWR_CTRL_SHUTDOWN);
-
+	ret = snd_soc_component_update_bits(component, TAS2770_PWR_CTRL,
+					    TAS2770_PWR_CTRL_MASK,
+					    TAS2770_PWR_CTRL_SHUTDOWN);
 	if (ret < 0)
 		return ret;
 
@@ -98,11 +94,9 @@ static int tas2770_codec_resume(struct snd_soc_component *component)
 {
 	int ret;
 
-	ret = snd_soc_component_update_bits(component,
-		TAS2770_PWR_CTRL,
-		TAS2770_PWR_CTRL_MASK,
-		TAS2770_PWR_CTRL_ACTIVE);
-
+	ret = snd_soc_component_update_bits(component, TAS2770_PWR_CTRL,
+					    TAS2770_PWR_CTRL_MASK,
+					    TAS2770_PWR_CTRL_ACTIVE);
 	if (ret < 0)
 		return ret;
 
@@ -135,16 +129,14 @@ static int tas2770_dac_event(struct snd_soc_dapm_widget *w,
 
 	switch (event) {
 	case SND_SOC_DAPM_POST_PMU:
-		ret = snd_soc_component_update_bits(component,
-			TAS2770_PWR_CTRL,
-			TAS2770_PWR_CTRL_MASK,
-			TAS2770_PWR_CTRL_MUTE);
+		ret = snd_soc_component_update_bits(component, TAS2770_PWR_CTRL,
+						    TAS2770_PWR_CTRL_MASK,
+						    TAS2770_PWR_CTRL_MUTE);
 		break;
 	case SND_SOC_DAPM_PRE_PMD:
-		ret = snd_soc_component_update_bits(component,
-			TAS2770_PWR_CTRL,
-			TAS2770_PWR_CTRL_MASK,
-			TAS2770_PWR_CTRL_SHUTDOWN);
+		ret = snd_soc_component_update_bits(component, TAS2770_PWR_CTRL,
+						    TAS2770_PWR_CTRL_MASK,
+						    TAS2770_PWR_CTRL_SHUTDOWN);
 		break;
 	default:
 		dev_err(tas2770->dev, "Not supported evevt\n");
@@ -164,14 +156,11 @@ static const struct snd_kcontrol_new vsense_switch =
 
 static const struct snd_soc_dapm_widget tas2770_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,
-				&tas2770_asi1_mux),
-	SND_SOC_DAPM_SWITCH("ISENSE", TAS2770_PWR_CTRL, 3, 1,
-			&isense_switch),
-	SND_SOC_DAPM_SWITCH("VSENSE", TAS2770_PWR_CTRL, 2, 1,
-			&vsense_switch),
+	SND_SOC_DAPM_MUX("ASI1 Sel", SND_SOC_NOPM, 0, 0, &tas2770_asi1_mux),
+	SND_SOC_DAPM_SWITCH("ISENSE", TAS2770_PWR_CTRL, 3, 1, &isense_switch),
+	SND_SOC_DAPM_SWITCH("VSENSE", TAS2770_PWR_CTRL, 2, 1, &vsense_switch),
 	SND_SOC_DAPM_DAC_E("DAC", NULL, SND_SOC_NOPM, 0, 0, tas2770_dac_event,
-	SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD),
+			   SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD),
 	SND_SOC_DAPM_OUTPUT("OUT"),
 	SND_SOC_DAPM_SIGGEN("VMON"),
 	SND_SOC_DAPM_SIGGEN("IMON")
@@ -194,15 +183,13 @@ static int tas2770_mute(struct snd_soc_dai *dai, int mute, int direction)
 	int ret;
 
 	if (mute)
-		ret = snd_soc_component_update_bits(component,
-			TAS2770_PWR_CTRL,
-			TAS2770_PWR_CTRL_MASK,
-			TAS2770_PWR_CTRL_MUTE);
+		ret = snd_soc_component_update_bits(component, TAS2770_PWR_CTRL,
+						    TAS2770_PWR_CTRL_MASK,
+						    TAS2770_PWR_CTRL_MUTE);
 	else
-		ret = snd_soc_component_update_bits(component,
-			TAS2770_PWR_CTRL,
-			TAS2770_PWR_CTRL_MASK,
-			TAS2770_PWR_CTRL_ACTIVE);
+		ret = snd_soc_component_update_bits(component, TAS2770_PWR_CTRL,
+						    TAS2770_PWR_CTRL_MASK,
+						    TAS2770_PWR_CTRL_ACTIVE);
 
 	if (ret < 0)
 		return ret;
@@ -217,24 +204,21 @@ static int tas2770_set_bitwidth(struct tas2770_priv *tas2770, int bitwidth)
 
 	switch (bitwidth) {
 	case SNDRV_PCM_FORMAT_S16_LE:
-		ret = snd_soc_component_update_bits(component,
-			TAS2770_TDM_CFG_REG2,
-			TAS2770_TDM_CFG_REG2_RXW_MASK,
-			TAS2770_TDM_CFG_REG2_RXW_16BITS);
+		ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG2,
+						    TAS2770_TDM_CFG_REG2_RXW_MASK,
+						    TAS2770_TDM_CFG_REG2_RXW_16BITS);
 		tas2770->v_sense_slot = tas2770->i_sense_slot + 2;
 		break;
 	case SNDRV_PCM_FORMAT_S24_LE:
-		ret = snd_soc_component_update_bits(component,
-			TAS2770_TDM_CFG_REG2,
-			TAS2770_TDM_CFG_REG2_RXW_MASK,
-			TAS2770_TDM_CFG_REG2_RXW_24BITS);
+		ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG2,
+						    TAS2770_TDM_CFG_REG2_RXW_MASK,
+						    TAS2770_TDM_CFG_REG2_RXW_24BITS);
 		tas2770->v_sense_slot = tas2770->i_sense_slot + 4;
 		break;
 	case SNDRV_PCM_FORMAT_S32_LE:
-		ret = snd_soc_component_update_bits(component,
-			TAS2770_TDM_CFG_REG2,
-			TAS2770_TDM_CFG_REG2_RXW_MASK,
-			TAS2770_TDM_CFG_REG2_RXW_32BITS);
+		ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG2,
+						    TAS2770_TDM_CFG_REG2_RXW_MASK,
+						    TAS2770_TDM_CFG_REG2_RXW_32BITS);
 		tas2770->v_sense_slot = tas2770->i_sense_slot + 4;
 		break;
 
@@ -247,21 +231,19 @@ static int tas2770_set_bitwidth(struct tas2770_priv *tas2770, int bitwidth)
 
 	tas2770->channel_size = bitwidth;
 
-	ret = snd_soc_component_update_bits(component,
-		TAS2770_TDM_CFG_REG5,
-		TAS2770_TDM_CFG_REG5_VSNS_MASK |
-		TAS2770_TDM_CFG_REG5_50_MASK,
-		TAS2770_TDM_CFG_REG5_VSNS_ENABLE |
+	ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG5,
+					    TAS2770_TDM_CFG_REG5_VSNS_MASK |
+					    TAS2770_TDM_CFG_REG5_50_MASK,
+					    TAS2770_TDM_CFG_REG5_VSNS_ENABLE |
 		tas2770->v_sense_slot);
 	if (ret < 0)
 		return ret;
 
-	ret = snd_soc_component_update_bits(component,
-		TAS2770_TDM_CFG_REG6,
-		TAS2770_TDM_CFG_REG6_ISNS_MASK |
-		TAS2770_TDM_CFG_REG6_50_MASK,
-		TAS2770_TDM_CFG_REG6_ISNS_ENABLE |
-		tas2770->i_sense_slot);
+	ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG6,
+					    TAS2770_TDM_CFG_REG6_ISNS_MASK |
+					    TAS2770_TDM_CFG_REG6_50_MASK,
+					    TAS2770_TDM_CFG_REG6_ISNS_ENABLE |
+					    tas2770->i_sense_slot);
 	if (ret < 0)
 		return ret;
 
@@ -275,82 +257,70 @@ static int tas2770_set_samplerate(struct tas2770_priv *tas2770, int samplerate)
 
 	switch (samplerate) {
 	case 48000:
-		ret = snd_soc_component_update_bits(component,
-			TAS2770_TDM_CFG_REG0,
-			TAS2770_TDM_CFG_REG0_SMP_MASK,
-			TAS2770_TDM_CFG_REG0_SMP_48KHZ);
+		ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG0,
+						    TAS2770_TDM_CFG_REG0_SMP_MASK,
+						    TAS2770_TDM_CFG_REG0_SMP_48KHZ);
 		if (ret < 0)
 			return ret;
 
-		ret = snd_soc_component_update_bits(component,
-			TAS2770_TDM_CFG_REG0,
-			TAS2770_TDM_CFG_REG0_31_MASK,
-			TAS2770_TDM_CFG_REG0_31_44_1_48KHZ);
+		ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG0,
+						    TAS2770_TDM_CFG_REG0_31_MASK,
+						    TAS2770_TDM_CFG_REG0_31_44_1_48KHZ);
 		break;
 	case 44100:
-		ret = snd_soc_component_update_bits(component,
-			TAS2770_TDM_CFG_REG0,
-			TAS2770_TDM_CFG_REG0_SMP_MASK,
-			TAS2770_TDM_CFG_REG0_SMP_44_1KHZ);
+		ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG0,
+						    TAS2770_TDM_CFG_REG0_SMP_MASK,
+						    TAS2770_TDM_CFG_REG0_SMP_44_1KHZ);
 		if (ret < 0)
 			return ret;
 
-		ret = snd_soc_component_update_bits(component,
-			TAS2770_TDM_CFG_REG0,
-			TAS2770_TDM_CFG_REG0_31_MASK,
-			TAS2770_TDM_CFG_REG0_31_44_1_48KHZ);
+		ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG0,
+						    TAS2770_TDM_CFG_REG0_31_MASK,
+						    TAS2770_TDM_CFG_REG0_31_44_1_48KHZ);
 		break;
 	case 96000:
-		ret = snd_soc_component_update_bits(component,
-			TAS2770_TDM_CFG_REG0,
-			TAS2770_TDM_CFG_REG0_SMP_MASK,
-			TAS2770_TDM_CFG_REG0_SMP_48KHZ);
+		ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG0,
+						    TAS2770_TDM_CFG_REG0_SMP_MASK,
+						    TAS2770_TDM_CFG_REG0_SMP_48KHZ);
 		if (ret < 0)
 			return ret;
 
-		ret = snd_soc_component_update_bits(component,
-			TAS2770_TDM_CFG_REG0,
-			TAS2770_TDM_CFG_REG0_31_MASK,
-			TAS2770_TDM_CFG_REG0_31_88_2_96KHZ);
+		ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG0,
+						    TAS2770_TDM_CFG_REG0_31_MASK,
+						    TAS2770_TDM_CFG_REG0_31_88_2_96KHZ);
 		break;
 	case 88200:
-		ret = snd_soc_component_update_bits(component,
-			TAS2770_TDM_CFG_REG0,
-			TAS2770_TDM_CFG_REG0_SMP_MASK,
-			TAS2770_TDM_CFG_REG0_SMP_44_1KHZ);
+		ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG0,
+						    TAS2770_TDM_CFG_REG0_SMP_MASK,
+						    TAS2770_TDM_CFG_REG0_SMP_44_1KHZ);
 		if (ret < 0)
 			return ret;
 
-		ret = snd_soc_component_update_bits(component,
-			TAS2770_TDM_CFG_REG0,
-			TAS2770_TDM_CFG_REG0_31_MASK,
-			TAS2770_TDM_CFG_REG0_31_88_2_96KHZ);
+		ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG0,
+						    TAS2770_TDM_CFG_REG0_31_MASK,
+						    TAS2770_TDM_CFG_REG0_31_88_2_96KHZ);
 		break;
 	case 19200:
-		ret = snd_soc_component_update_bits(component,
-			TAS2770_TDM_CFG_REG0,
-			TAS2770_TDM_CFG_REG0_SMP_MASK,
-			TAS2770_TDM_CFG_REG0_SMP_48KHZ);
+		ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG0,
+						    TAS2770_TDM_CFG_REG0_SMP_MASK,
+						    TAS2770_TDM_CFG_REG0_SMP_48KHZ);
 		if (ret < 0)
 			return ret;
 
-		ret = snd_soc_component_update_bits(component,
-			TAS2770_TDM_CFG_REG0,
-			TAS2770_TDM_CFG_REG0_31_MASK,
-			TAS2770_TDM_CFG_REG0_31_176_4_192KHZ);
+		ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG0,
+						    TAS2770_TDM_CFG_REG0_31_MASK,
+						    TAS2770_TDM_CFG_REG0_31_176_4_192KHZ);
 		break;
 	case 17640:
-		ret = snd_soc_component_update_bits(component,
-			TAS2770_TDM_CFG_REG0,
-			TAS2770_TDM_CFG_REG0_SMP_MASK,
-			TAS2770_TDM_CFG_REG0_SMP_44_1KHZ);
+		ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG0,
+						    TAS2770_TDM_CFG_REG0_SMP_MASK,
+						    TAS2770_TDM_CFG_REG0_SMP_44_1KHZ);
 		if (ret < 0)
 			return ret;
 
-		ret = snd_soc_component_update_bits(component,
-			TAS2770_TDM_CFG_REG0,
-			TAS2770_TDM_CFG_REG0_31_MASK,
-			TAS2770_TDM_CFG_REG0_31_176_4_192KHZ);
+		ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG0,
+						    TAS2770_TDM_CFG_REG0_31_MASK,
+						    TAS2770_TDM_CFG_REG0_31_176_4_192KHZ);
 		break;
 	default:
 		ret = -EINVAL;
@@ -373,23 +343,19 @@ static int tas2770_hw_params(struct snd_pcm_substream *substream,
 	int ret;
 
 	ret = tas2770_set_bitwidth(tas2770, params_format(params));
-	if (ret < 0)
-		goto end;
-
-
-	ret = tas2770_set_samplerate(tas2770, params_rate(params));
+	if (ret)
+		return ret;
 
-end:
-	return ret;
+	return tas2770_set_samplerate(tas2770, params_rate(params));
 }
 
 static int tas2770_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
 {
-	u8 tdm_rx_start_slot = 0, asi_cfg_1 = 0;
-	int ret;
 	struct snd_soc_component *component = dai->component;
 	struct tas2770_priv *tas2770 =
 			snd_soc_component_get_drvdata(component);
+	u8 tdm_rx_start_slot = 0, asi_cfg_1 = 0;
+	int ret;
 
 	switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
 	case SND_SOC_DAIFMT_CBS_CFS:
@@ -412,8 +378,8 @@ static int tas2770_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
 	}
 
 	ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG1,
-		TAS2770_TDM_CFG_REG1_RX_MASK,
-		asi_cfg_1);
+					    TAS2770_TDM_CFG_REG1_RX_MASK,
+					    asi_cfg_1);
 	if (ret < 0)
 		return ret;
 
@@ -437,8 +403,8 @@ static int tas2770_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
 	}
 
 	ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG1,
-		TAS2770_TDM_CFG_REG1_MASK,
-	(tdm_rx_start_slot << TAS2770_TDM_CFG_REG1_51_SHIFT));
+					    TAS2770_TDM_CFG_REG1_MASK,
+					    (tdm_rx_start_slot << TAS2770_TDM_CFG_REG1_51_SHIFT));
 	if (ret < 0)
 		return ret;
 
@@ -464,6 +430,7 @@ static int tas2770_set_dai_tdm_slot(struct snd_soc_dai *dai,
 	if (slots == 1) {
 		if (tx_mask != 1)
 			return -EINVAL;
+
 		left_slot = 0;
 		right_slot = 0;
 	} else {
@@ -481,43 +448,36 @@ static int tas2770_set_dai_tdm_slot(struct snd_soc_dai *dai,
 		return -EINVAL;
 
 	ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG3,
-		TAS2770_TDM_CFG_REG3_30_MASK,
-		(left_slot << TAS2770_TDM_CFG_REG3_30_SHIFT));
+					    TAS2770_TDM_CFG_REG3_30_MASK,
+					    (left_slot << TAS2770_TDM_CFG_REG3_30_SHIFT));
 	if (ret < 0)
 		return ret;
 	ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG3,
-		TAS2770_TDM_CFG_REG3_RXS_MASK,
-	(right_slot << TAS2770_TDM_CFG_REG3_RXS_SHIFT));
+					    TAS2770_TDM_CFG_REG3_RXS_MASK,
+					    (right_slot << TAS2770_TDM_CFG_REG3_RXS_SHIFT));
 	if (ret < 0)
 		return ret;
 
 	switch (slot_width) {
 	case 16:
-		ret = snd_soc_component_update_bits(component,
-			TAS2770_TDM_CFG_REG2,
-			TAS2770_TDM_CFG_REG2_RXS_MASK,
-			TAS2770_TDM_CFG_REG2_RXS_16BITS);
+		ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG2,
+						    TAS2770_TDM_CFG_REG2_RXS_MASK,
+						    TAS2770_TDM_CFG_REG2_RXS_16BITS);
 		break;
-
 	case 24:
-		ret = snd_soc_component_update_bits(component,
-			TAS2770_TDM_CFG_REG2,
-			TAS2770_TDM_CFG_REG2_RXS_MASK,
-			TAS2770_TDM_CFG_REG2_RXS_24BITS);
+		ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG2,
+						    TAS2770_TDM_CFG_REG2_RXS_MASK,
+						    TAS2770_TDM_CFG_REG2_RXS_24BITS);
 		break;
-
 	case 32:
-		ret = snd_soc_component_update_bits(component,
-			TAS2770_TDM_CFG_REG2,
-			TAS2770_TDM_CFG_REG2_RXS_MASK,
-			TAS2770_TDM_CFG_REG2_RXS_32BITS);
+		ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG2,
+						    TAS2770_TDM_CFG_REG2_RXS_MASK,
+						    TAS2770_TDM_CFG_REG2_RXS_32BITS);
 		break;
-
 	case 0:
 		/* Do not change slot width */
 		ret = 0;
 		break;
-
 	default:
 		ret = -EINVAL;
 	}
@@ -585,11 +545,9 @@ static DECLARE_TLV_DB_SCALE(tas2770_playback_volume, -12750, 50, 0);
 
 static const struct snd_kcontrol_new tas2770_snd_controls[] = {
 	SOC_SINGLE_TLV("Speaker Playback Volume", TAS2770_PLAY_CFG_REG2,
-		0, TAS2770_PLAY_CFG_REG2_VMAX, 1,
-		tas2770_playback_volume),
-	SOC_SINGLE_TLV("Amp Gain Volume", TAS2770_PLAY_CFG_REG0,
-		0, 0x14, 0,
-		tas2770_digital_tlv),
+		       0, TAS2770_PLAY_CFG_REG2_VMAX, 1, tas2770_playback_volume),
+	SOC_SINGLE_TLV("Amp Gain Volume", TAS2770_PLAY_CFG_REG0, 0, 0x14, 0,
+		       tas2770_digital_tlv),
 };
 
 static const struct snd_soc_component_driver soc_component_driver_tas2770 = {
@@ -650,6 +608,7 @@ static bool tas2770_volatile(struct device *dev, unsigned int reg)
 	case TAS2770_TEMP_LSB:
 		return true;
 	}
+
 	return false;
 }
 
@@ -668,6 +627,7 @@ static bool tas2770_writeable(struct device *dev, unsigned int reg)
 	case TAS2770_REV_AND_GPID:
 		return false;
 	}
+
 	return true;
 }
 
@@ -701,26 +661,29 @@ static int tas2770_parse_dt(struct device *dev, struct tas2770_priv *tas2770)
 	int rc = 0;
 
 	rc = fwnode_property_read_u32(dev->fwnode, "ti,asi-format",
-					&tas2770->asi_format);
+				      &tas2770->asi_format);
 	if (rc) {
 		dev_info(tas2770->dev, "Property %s is missing setting default slot\n",
-			"ti,asi-format");
+			 "ti,asi-format");
+
 		tas2770->asi_format = 0;
 	}
 
 	rc = fwnode_property_read_u32(dev->fwnode, "ti,imon-slot-no",
-			&tas2770->i_sense_slot);
+				      &tas2770->i_sense_slot);
 	if (rc) {
 		dev_info(tas2770->dev, "Property %s is missing setting default slot\n",
-			"ti,imon-slot-no");
+			 "ti,imon-slot-no");
+
 		tas2770->i_sense_slot = 0;
 	}
 
 	rc = fwnode_property_read_u32(dev->fwnode, "ti,vmon-slot-no",
-				&tas2770->v_sense_slot);
+				      &tas2770->v_sense_slot);
 	if (rc) {
 		dev_info(tas2770->dev, "Property %s is missing setting default slot\n",
-			"ti,vmon-slot-no");
+			 "ti,vmon-slot-no");
+
 		tas2770->v_sense_slot = 2;
 	}
 
@@ -733,22 +696,23 @@ static int tas2770_i2c_probe(struct i2c_client *client,
 	struct tas2770_priv *tas2770;
 	int result;
 
-	tas2770 = devm_kzalloc(&client->dev,
-		sizeof(struct tas2770_priv), GFP_KERNEL);
+	tas2770 = devm_kzalloc(&client->dev, sizeof(struct tas2770_priv),
+			       GFP_KERNEL);
 	if (!tas2770)
 		return -ENOMEM;
-	tas2770->dev = &client->dev;
 
+	tas2770->dev = &client->dev;
 	i2c_set_clientdata(client, tas2770);
 	dev_set_drvdata(&client->dev, tas2770);
+
 	tas2770->power_state = TAS2770_POWER_SHUTDOWN;
 
 	tas2770->regmap = devm_regmap_init_i2c(client, &tas2770_i2c_regmap);
 	if (IS_ERR(tas2770->regmap)) {
 		result = PTR_ERR(tas2770->regmap);
 		dev_err(&client->dev, "Failed to allocate register map: %d\n",
-					result);
-		goto end;
+			result);
+		return result;
 	}
 
 	if (client->dev.of_node) {
@@ -756,7 +720,7 @@ static int tas2770_i2c_probe(struct i2c_client *client,
 		if (result) {
 			dev_err(tas2770->dev, "%s: Failed to parse devicetree\n",
 				__func__);
-			goto end;
+			return result;
 		}
 	}
 
@@ -776,7 +740,6 @@ static int tas2770_i2c_probe(struct i2c_client *client,
 	if (result)
 		dev_err(tas2770->dev, "Register codec failed.\n");
 
-end:
 	return result;
 }
 
@@ -802,7 +765,6 @@ static struct i2c_driver tas2770_i2c_driver = {
 	.probe      = tas2770_i2c_probe,
 	.id_table   = tas2770_i2c_id,
 };
-
 module_i2c_driver(tas2770_i2c_driver);
 
 MODULE_AUTHOR("Shi Fu <shifu0704@thundersoft.com>");
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 9/9] ASoC: tas2770: Refactor sample rate function
  2020-09-18 19:05 [PATCH 1/9] ASoC: tas2770: Fix calling reset in probe Dan Murphy
                   ` (6 preceding siblings ...)
  2020-09-18 19:05 ` [PATCH 8/9] ASoC: tas2770: Fix the spacing and new lines Dan Murphy
@ 2020-09-18 19:05 ` Dan Murphy
  2020-09-22  0:05 ` [PATCH 1/9] ASoC: tas2770: Fix calling reset in probe Mark Brown
  8 siblings, 0 replies; 14+ messages in thread
From: Dan Murphy @ 2020-09-18 19:05 UTC (permalink / raw)
  To: lgirdwood, broonie, tiwai, robh+dt
  Cc: devicetree, alsa-devel, linux-kernel, Dan Murphy

Refactor the tas2770_set_samplerate to simplify the code and access the
I2C bus only once per rate request. The ramp rate and sample rate bits
are contained in the same register so a single call to the
snd_soc_update_bits function is all that is needed

Signed-off-by: Dan Murphy <dmurphy@ti.com>
---
 sound/soc/codecs/tas2770.c | 75 ++++++++++----------------------------
 1 file changed, 19 insertions(+), 56 deletions(-)

diff --git a/sound/soc/codecs/tas2770.c b/sound/soc/codecs/tas2770.c
index b17cf0a7f785..386aaa11fa08 100644
--- a/sound/soc/codecs/tas2770.c
+++ b/sound/soc/codecs/tas2770.c
@@ -252,80 +252,43 @@ static int tas2770_set_bitwidth(struct tas2770_priv *tas2770, int bitwidth)
 
 static int tas2770_set_samplerate(struct tas2770_priv *tas2770, int samplerate)
 {
-	int ret;
 	struct snd_soc_component *component = tas2770->component;
+	int ramp_rate_val;
+	int ret;
 
 	switch (samplerate) {
 	case 48000:
-		ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG0,
-						    TAS2770_TDM_CFG_REG0_SMP_MASK,
-						    TAS2770_TDM_CFG_REG0_SMP_48KHZ);
-		if (ret < 0)
-			return ret;
-
-		ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG0,
-						    TAS2770_TDM_CFG_REG0_31_MASK,
-						    TAS2770_TDM_CFG_REG0_31_44_1_48KHZ);
+		ramp_rate_val = TAS2770_TDM_CFG_REG0_SMP_48KHZ |
+				TAS2770_TDM_CFG_REG0_31_44_1_48KHZ;
 		break;
 	case 44100:
-		ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG0,
-						    TAS2770_TDM_CFG_REG0_SMP_MASK,
-						    TAS2770_TDM_CFG_REG0_SMP_44_1KHZ);
-		if (ret < 0)
-			return ret;
-
-		ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG0,
-						    TAS2770_TDM_CFG_REG0_31_MASK,
-						    TAS2770_TDM_CFG_REG0_31_44_1_48KHZ);
+		ramp_rate_val = TAS2770_TDM_CFG_REG0_SMP_44_1KHZ |
+				TAS2770_TDM_CFG_REG0_31_44_1_48KHZ;
 		break;
 	case 96000:
-		ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG0,
-						    TAS2770_TDM_CFG_REG0_SMP_MASK,
-						    TAS2770_TDM_CFG_REG0_SMP_48KHZ);
-		if (ret < 0)
-			return ret;
-
-		ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG0,
-						    TAS2770_TDM_CFG_REG0_31_MASK,
-						    TAS2770_TDM_CFG_REG0_31_88_2_96KHZ);
+		ramp_rate_val = TAS2770_TDM_CFG_REG0_SMP_48KHZ |
+				TAS2770_TDM_CFG_REG0_31_88_2_96KHZ;
 		break;
 	case 88200:
-		ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG0,
-						    TAS2770_TDM_CFG_REG0_SMP_MASK,
-						    TAS2770_TDM_CFG_REG0_SMP_44_1KHZ);
-		if (ret < 0)
-			return ret;
-
-		ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG0,
-						    TAS2770_TDM_CFG_REG0_31_MASK,
-						    TAS2770_TDM_CFG_REG0_31_88_2_96KHZ);
+		ramp_rate_val = TAS2770_TDM_CFG_REG0_SMP_44_1KHZ |
+				TAS2770_TDM_CFG_REG0_31_88_2_96KHZ;
 		break;
 	case 19200:
-		ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG0,
-						    TAS2770_TDM_CFG_REG0_SMP_MASK,
-						    TAS2770_TDM_CFG_REG0_SMP_48KHZ);
-		if (ret < 0)
-			return ret;
-
-		ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG0,
-						    TAS2770_TDM_CFG_REG0_31_MASK,
-						    TAS2770_TDM_CFG_REG0_31_176_4_192KHZ);
+		ramp_rate_val = TAS2770_TDM_CFG_REG0_SMP_48KHZ |
+				TAS2770_TDM_CFG_REG0_31_176_4_192KHZ;
 		break;
 	case 17640:
-		ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG0,
-						    TAS2770_TDM_CFG_REG0_SMP_MASK,
-						    TAS2770_TDM_CFG_REG0_SMP_44_1KHZ);
-		if (ret < 0)
-			return ret;
-
-		ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG0,
-						    TAS2770_TDM_CFG_REG0_31_MASK,
-						    TAS2770_TDM_CFG_REG0_31_176_4_192KHZ);
+		ramp_rate_val = TAS2770_TDM_CFG_REG0_SMP_44_1KHZ |
+				TAS2770_TDM_CFG_REG0_31_176_4_192KHZ;
 		break;
 	default:
-		ret = -EINVAL;
+		return -EINVAL;
 	}
 
+	ret = snd_soc_component_update_bits(component, TAS2770_TDM_CFG_REG0,
+					    TAS2770_TDM_CFG_REG0_SMP_MASK |
+					    TAS2770_TDM_CFG_REG0_31_MASK,
+					    ramp_rate_val);
 	if (ret < 0)
 		return ret;
 
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 6/9] ASoC: tas2770: Convert bit mask to GENMASK in header
  2020-09-18 19:05 ` [PATCH 6/9] ASoC: tas2770: Convert bit mask to GENMASK in header Dan Murphy
@ 2020-09-21 19:04   ` Mark Brown
  2020-09-21 19:18     ` Dan Murphy
  0 siblings, 1 reply; 14+ messages in thread
From: Mark Brown @ 2020-09-21 19:04 UTC (permalink / raw)
  To: Dan Murphy
  Cc: devicetree, alsa-devel, tiwai, lgirdwood, linux-kernel, robh+dt

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

On Fri, Sep 18, 2020 at 02:05:45PM -0500, Dan Murphy wrote:
> Update the hardcoded masks with the GENMASK macro. Also update some of
> the hardcoded bits with the BIT macro

Cleanups like this should come after any fixes in the series, that way
fixes can be sent as fixes if needed which isn't appropriate for random
cleanups.

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 6/9] ASoC: tas2770: Convert bit mask to GENMASK in header
  2020-09-21 19:04   ` Mark Brown
@ 2020-09-21 19:18     ` Dan Murphy
  2020-09-21 19:19       ` Dan Murphy
  0 siblings, 1 reply; 14+ messages in thread
From: Dan Murphy @ 2020-09-21 19:18 UTC (permalink / raw)
  To: Mark Brown
  Cc: devicetree, alsa-devel, tiwai, lgirdwood, linux-kernel, robh+dt

Mark

On 9/21/20 2:04 PM, Mark Brown wrote:
> On Fri, Sep 18, 2020 at 02:05:45PM -0500, Dan Murphy wrote:
>> Update the hardcoded masks with the GENMASK macro. Also update some of
>> the hardcoded bits with the BIT macro
> Cleanups like this should come after any fixes in the series, that way
> fixes can be sent as fixes if needed which isn't appropriate for random
> cleanups.

OK I can re-order so the clean up comes at the end.  During the clean up 
I found patch 7-9.

Dan


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 6/9] ASoC: tas2770: Convert bit mask to GENMASK in header
  2020-09-21 19:18     ` Dan Murphy
@ 2020-09-21 19:19       ` Dan Murphy
  2020-09-22  9:47         ` Mark Brown
  0 siblings, 1 reply; 14+ messages in thread
From: Dan Murphy @ 2020-09-21 19:19 UTC (permalink / raw)
  To: Mark Brown
  Cc: devicetree, alsa-devel, tiwai, lgirdwood, linux-kernel, robh+dt

Mark

On 9/21/20 2:18 PM, Dan Murphy wrote:
> Mark
>
> On 9/21/20 2:04 PM, Mark Brown wrote:
>> On Fri, Sep 18, 2020 at 02:05:45PM -0500, Dan Murphy wrote:
>>> Update the hardcoded masks with the GENMASK macro. Also update some of
>>> the hardcoded bits with the BIT macro
>> Cleanups like this should come after any fixes in the series, that way
>> fixes can be sent as fixes if needed which isn't appropriate for random
>> cleanups.
>
> OK I can re-order so the clean up comes at the end.  During the clean 
> up I found patch 7-9.
>
Forgot to ask are you going to take 1-5? If so I can rebase on top of 
for-5.10 and re-submit.

Dan


> Dan
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/9] ASoC: tas2770: Fix calling reset in probe
  2020-09-18 19:05 [PATCH 1/9] ASoC: tas2770: Fix calling reset in probe Dan Murphy
                   ` (7 preceding siblings ...)
  2020-09-18 19:05 ` [PATCH 9/9] ASoC: tas2770: Refactor sample rate function Dan Murphy
@ 2020-09-22  0:05 ` Mark Brown
  8 siblings, 0 replies; 14+ messages in thread
From: Mark Brown @ 2020-09-22  0:05 UTC (permalink / raw)
  To: robh+dt, tiwai, lgirdwood, Dan Murphy
  Cc: devicetree, alsa-devel, linux-kernel

On Fri, 18 Sep 2020 14:05:40 -0500, Dan Murphy wrote:
> tas2770_reset is called during i2c probe. The reset calls the
> snd_soc_component_write which depends on the tas2770->component being
> available. The component pointer is not set until codec_probe so move
> the reset to the codec_probe after the pointer is set.

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/5] dt-bindings: tas2770: Fix I2C addresses for the TAS2770
      commit: b23d9eb897a1209e4d741fd69e5490f1b5b9e7cf
[2/5] ASoC: tas2770: Fix unbalanced calls to pm_runtime
      commit: d3d71c99b541040da198f43da3bbd85d8e9598cb
[3/5] ASoC: tas2770: Convert bit mask to GENMASK in header
      commit: ec9377dca2ca77eaf4fbdb09ac803f379b10d731
[4/5] ASoC: tas2770: Fix the spacing and new lines
      commit: d3964aff7331cd9695d0c18655e053b08837ff78
[5/5] ASoC: tas2770: Refactor sample rate function
      commit: be05ab41c61858cce557a1fe863ed00f38e31e97

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 6/9] ASoC: tas2770: Convert bit mask to GENMASK in header
  2020-09-21 19:19       ` Dan Murphy
@ 2020-09-22  9:47         ` Mark Brown
  0 siblings, 0 replies; 14+ messages in thread
From: Mark Brown @ 2020-09-22  9:47 UTC (permalink / raw)
  To: Dan Murphy
  Cc: devicetree, alsa-devel, tiwai, lgirdwood, linux-kernel, robh+dt

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

On Mon, Sep 21, 2020 at 02:19:36PM -0500, Dan Murphy wrote:

> Forgot to ask are you going to take 1-5? If so I can rebase on top of
> for-5.10 and re-submit.

I think managed to apply everything with manual picking things, there
weren't any actual dependencies.

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2020-09-22  9:49 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-18 19:05 [PATCH 1/9] ASoC: tas2770: Fix calling reset in probe Dan Murphy
2020-09-18 19:05 ` [PATCH 2/9] ASoC: tas2770: Add missing bias level power states Dan Murphy
2020-09-18 19:05 ` [PATCH 3/9] dt-bindings: tas2770: Fix I2C addresses for the TAS2770 Dan Murphy
2020-09-18 19:05 ` [PATCH 4/9] ASoC: tas2770: Fix required DT properties in the code Dan Murphy
2020-09-18 19:05 ` [PATCH 5/9] ASoC: tas2770: Fix unbalanced calls to pm_runtime Dan Murphy
2020-09-18 19:05 ` [PATCH 6/9] ASoC: tas2770: Convert bit mask to GENMASK in header Dan Murphy
2020-09-21 19:04   ` Mark Brown
2020-09-21 19:18     ` Dan Murphy
2020-09-21 19:19       ` Dan Murphy
2020-09-22  9:47         ` Mark Brown
2020-09-18 19:05 ` [PATCH 7/9] ASoC: tas2770: Fix error handling with update_bits Dan Murphy
2020-09-18 19:05 ` [PATCH 8/9] ASoC: tas2770: Fix the spacing and new lines Dan Murphy
2020-09-18 19:05 ` [PATCH 9/9] ASoC: tas2770: Refactor sample rate function Dan Murphy
2020-09-22  0:05 ` [PATCH 1/9] ASoC: tas2770: Fix calling reset in probe Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).