2020-09-18 19:08:11

by Dan Murphy

[permalink] [raw]
Subject: [PATCH 1/9] ASoC: tas2770: Fix calling reset in probe

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 <[email protected]>
---
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


2020-09-18 19:08:15

by Dan Murphy

[permalink] [raw]
Subject: [PATCH 9/9] ASoC: tas2770: Refactor sample rate function

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 <[email protected]>
---
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

2020-09-18 19:09:36

by Dan Murphy

[permalink] [raw]
Subject: [PATCH 2/9] ASoC: tas2770: Add missing bias level power states

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 <[email protected]>
---
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

2020-09-18 19:09:58

by Dan Murphy

[permalink] [raw]
Subject: [PATCH 5/9] ASoC: tas2770: Fix unbalanced calls to pm_runtime

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 <[email protected]>
---
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

2020-09-22 01:00:17

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 1/9] ASoC: tas2770: Fix calling reset in probe

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