ASoC: adau17x1: Unused exported functions changed to internal

PATCH In review Robert Rosengren 4 participants 6 messages Aug 13, 2018 – Aug 17, 2018

Subsystems (3)

Sound ASoC Analog devices inc ASoC codec drivers

lore

Share & tools

lore

Robert Rosengren

author

2018-08-13 07:34 UTC #0 lore raw

PATCH v2 1/2 ASoC: adau17x1: Unused exported functions changed to internal

adau17x1_setup_firmware and adau17x1_has_dsp is only used internally, so making them static instead of exported.

Change-Id I4882513ab457966b0371ac2c1024550ca6edbe0c

Signed-off-by Robert Rosengren <[email protected]>

---
 sound/soc/codecs/adau17x1.c | 9 +++++----
 sound/soc/codecs/adau17x1.h | 4 ----
 2 files changed, 5 insertions(+), 8 deletions(-)

sound/soc/codecs/adau17x1.c +5 -4

index 57169b8ff14e..2f2afb4c0c88 100644
@@ -60,6 +60,9 @@ static const struct snd_kcontrol_new adau17x1_controls[] = {
 	SOC_ENUM("Mic Bias Mode", adau17x1_mic_bias_mode_enum),
 };
 
+static int adau17x1_setup_firmware(struct snd_soc_component *component,
+	unsigned int rate);
+
 static int adau17x1_pll_event(struct snd_soc_dapm_widget *w,
 	struct snd_kcontrol *kcontrol, int event)
 {

@@ -313,7 +316,7 @@ static const struct snd_soc_dapm_route adau17x1_no_dsp_dapm_routes[] = {
 	{ "Capture", NULL, "Right Decimator" },
 };
 
-bool adau17x1_has_dsp(struct adau *adau)
+static bool adau17x1_has_dsp(struct adau *adau)
 {
 	switch (adau->type) {
 	case ADAU1761:

@@ -324,7 +327,6 @@ bool adau17x1_has_dsp(struct adau *adau)
 		return false;
 	}
 }
-EXPORT_SYMBOL_GPL(adau17x1_has_dsp);
 
 static int adau17x1_set_dai_pll(struct snd_soc_dai *dai, int pll_id,
 	int source, unsigned int freq_in, unsigned int freq_out)

@@ -836,7 +838,7 @@ bool adau17x1_volatile_register(struct device *dev, unsigned int reg)
 }
 EXPORT_SYMBOL_GPL(adau17x1_volatile_register);
 
-int adau17x1_setup_firmware(struct snd_soc_component *component,
+static int adau17x1_setup_firmware(struct snd_soc_component *component,
 	unsigned int rate)
 {
 	int ret;

@@ -880,7 +882,6 @@ int adau17x1_setup_firmware(struct snd_soc_component *component,
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(adau17x1_setup_firmware);
 
 int adau17x1_add_widgets(struct snd_soc_component *component)
 {

sound/soc/codecs/adau17x1.h +0 -4

index e6fe87beec07..98a3b6f5bc96 100644
@@ -68,10 +68,6 @@ int adau17x1_resume(struct snd_soc_component *component);
 
 extern const struct snd_soc_dai_ops adau17x1_dai_ops;
 
-int adau17x1_setup_firmware(struct snd_soc_component *component,
-	unsigned int rate);
-bool adau17x1_has_dsp(struct adau *adau);
-
 #define ADAU17X1_CLOCK_CONTROL			0x4000
 #define ADAU17X1_PLL_CONTROL			0x4002
 #define ADAU17X1_REC_POWER_MGMT			0x4009

Signature · 1 line

2.11.0

Robert Rosengren

author

2018-08-13 07:34 UTC #1 lore raw

PATCH v2 2/2 ASoC: adau17x1: Implemented safeload support

From: Danny Smith <[email protected]>

Safeload support has been implemented which is used when updating for instance filter parameters using alsa controls. Without safeload support audio can become distorted during update.

Signed-off-by Danny Smith <[email protected]>

Signed-off-by Robert Rosengren <[email protected]>

---
 sound/soc/codecs/adau17x1.c | 68 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 66 insertions(+), 2 deletions(-)

sound/soc/codecs/adau17x1.c +66 -2

index 2f2afb4c0c88..6d0b2261c959 100644
@@ -26,6 +26,10 @@
 #include "adau17x1.h"
 #include "adau-utils.h"
 
+#define ADAU17X1_SAFELOAD_TARGET_ADDRESS 0x0006
+#define ADAU17X1_SAFELOAD_TRIGGER 0x0007
+#define ADAU17X1_SAFELOAD_DATA(i) (0x0001 + (i))
+
 static const char * const adau17x1_capture_mixer_boost_text[] = {
 	"Normal operation", "Boost Level 1", "Boost Level 2", "Boost Level 3",
 };

@@ -328,6 +332,17 @@ static bool adau17x1_has_dsp(struct adau *adau)
 	}
 }
 
+static bool adau17x1_has_safeload(struct adau *adau)
+{
+	switch (adau->type) {
+	case ADAU1761:
+	case ADAU1781:
+		return true;
+	default:
+		return false;
+	}
+}
+
 static int adau17x1_set_dai_pll(struct snd_soc_dai *dai, int pll_id,
 	int source, unsigned int freq_in, unsigned int freq_out)
 {

@@ -958,6 +973,50 @@ int adau17x1_resume(struct snd_soc_component *component)
 }
 EXPORT_SYMBOL_GPL(adau17x1_resume);
 
+static int adau17x1_safeload(struct sigmadsp *sigmadsp, unsigned int addr,
+	const uint8_t bytes[], size_t len)
+{
+	uint8_t buf[4];
+	unsigned int i;
+	int ret;
+
+	/* target address is offset by 1 */
+	unsigned int addr_offset = addr - 1;
+
+	/* write safeload addresses */
+	for (i = 0; i < len / 4; i++) {
+		memcpy(buf, bytes + i * 4, 4);
+		ret = regmap_raw_write(sigmadsp->control_data,
+			ADAU17X1_SAFELOAD_DATA(i), buf, 4);
+		if (ret < 0)
+			return ret;
+	}
+
+	/* write target address */
+	buf[0] = (addr_offset >> 24) & 0xff;
+	buf[1] = (addr_offset >> 16) & 0xff;
+	buf[2] = (addr_offset >> 8) & 0xff;
+	buf[3] = (addr_offset) & 0xff;
+	ret = regmap_raw_write(sigmadsp->control_data,
+		ADAU17X1_SAFELOAD_TARGET_ADDRESS, buf, 4);
+	if (ret < 0)
+		return ret;
+
+	/* write nbr of words to trigger address */
+	buf[0] = 0x00;
+	buf[1] = 0x00;
+	buf[2] = 0x00;
+	buf[3] = i & 0xff;
+	ret = regmap_raw_write(sigmadsp->control_data,
+		ADAU17X1_SAFELOAD_TRIGGER, buf, 4);
+
+	return ret;
+}
+
+static const struct sigmadsp_ops adau17x1_sigmadsp_ops = {
+	.safeload = adau17x1_safeload,
+};
+
 int adau17x1_probe(struct device *dev, struct regmap *regmap,
 	enum adau17x1_type type, void (*switch_mode)(struct device *dev),
 	const char *firmware_name)

@@ -1003,8 +1062,13 @@ int adau17x1_probe(struct device *dev, struct regmap *regmap,
 	dev_set_drvdata(dev, adau);
 
 	if (firmware_name) {
-		adau->sigmadsp = devm_sigmadsp_init_regmap(dev, regmap, NULL,
-			firmware_name);
+		if (adau17x1_has_safeload(adau)) {
+			adau->sigmadsp = devm_sigmadsp_init_regmap(dev, regmap,
+				&adau17x1_sigmadsp_ops, firmware_name);
+		} else {
+			adau->sigmadsp = devm_sigmadsp_init_regmap(dev, regmap,
+				NULL, firmware_name);
+		}
 		if (IS_ERR(adau->sigmadsp)) {
 			dev_warn(dev, "Could not find firmware file: %ld\n",
 				PTR_ERR(adau->sigmadsp));

Signature · 1 line

2.11.0

Lars-Peter Clausen

maintainer

2018-08-13 10:44 UTC #2 lore raw

Re: [PATCH v2 2/2] ASoC: adau17x1: Implemented safeload support

Quoted 9 lines · Robert Rosengren

From: Danny Smith <[email protected]>
Safeload support has been implemented which is used when updating for instance filter parameters using alsa controls. Without safeload support audio can become distorted during update.
Signed-off-by Danny Smith <[email protected]>
Signed-off-by Robert Rosengren <[email protected]>

Hi,

Thanks for implementing this. Some comments inline.

[...]

sound/soc/codecs/adau17x1.c int adau17x1_resume(struct snd_soc_component *component) · line 988

Quoted 13 lines

sound/soc/codecs/adau17x1.c +13 -0

+static int adau17x1_safeload(struct sigmadsp *sigmadsp, unsigned int addr,
+	const uint8_t bytes[], size_t len)
+{
+	uint8_t buf[4];
+	unsigned int i;
+	int ret;
+
+	/* target address is offset by 1 */
+	unsigned int addr_offset = addr - 1;
+
+	/* write safeload addresses */
+	for (i = 0; i < len / 4; i++) {
+		memcpy(buf, bytes + i * 4, 4);

Is this memcpy really required or could we just write bytes directly in a single regmap_raw_write()? I believe the adau17x1 will auto increment the register addresses.

And I suppose we also need to handle the case when len is not a multiple of 4.

sound/soc/codecs/adau17x1.c int adau17x1_resume(struct snd_soc_component *component) · line 999

Quoted 11 lines

sound/soc/codecs/adau17x1.c +11 -0

+		ret = regmap_raw_write(sigmadsp->control_data,
+			ADAU17X1_SAFELOAD_DATA(i), buf, 4);
+		if (ret < 0)
+			return ret;
+	}
+
+	/* write target address */
+	buf[0] = (addr_offset >> 24) & 0xff;
+	buf[1] = (addr_offset >> 16) & 0xff;
+	buf[2] = (addr_offset >> 8) & 0xff;
+	buf[3] = (addr_offset) & 0xff;

This could be slightly simplified using

	put_unaligend_be32(addr_offset, buf);

sound/soc/codecs/adau17x1.c int adau17x1_resume(struct snd_soc_component *component) · line 1009

Quoted 10 lines

sound/soc/codecs/adau17x1.c +10 -0

+	ret = regmap_raw_write(sigmadsp->control_data,
+		ADAU17X1_SAFELOAD_TARGET_ADDRESS, buf, 4);
+	if (ret < 0)
+		return ret;
+
+	/* write nbr of words to trigger address */
+	buf[0] = 0x00;
+	buf[1] = 0x00;
+	buf[2] = 0x00;
+	buf[3] = i & 0xff;

Same here I guess

Quoted 5 lines

sound/soc/codecs/adau17x1.c +5 -0

+	ret = regmap_raw_write(sigmadsp->control_data,
+		ADAU17X1_SAFELOAD_TRIGGER, buf, 4);
+
+	return ret;
+}

Danny Smith

2018-08-17 08:06 UTC #3 lore raw

Re: [PATCH v2 2/2] ASoC: adau17x1: Implemented safeload support

Quoted 32 lines · Lars-Peter Clausen

Quoted 9 lines · Robert Rosengren
From: Danny Smith <[email protected]>
Safeload support has been implemented which is used when updating for instance filter parameters using alsa controls. Without safeload support audio can become distorted during update.
Signed-off-by Danny Smith <[email protected]>
Signed-off-by Robert Rosengren <[email protected]>
Hi,
Thanks for implementing this. Some comments inline.
[...]
sound/soc/codecs/adau17x1.c int adau17x1_resume(struct snd_soc_component *component) · line 988
Quoted 13 lines
sound/soc/codecs/adau17x1.c +13 -0
+static int adau17x1_safeload(struct sigmadsp *sigmadsp, unsigned int addr,
+	const uint8_t bytes[], size_t len)
+{
+	uint8_t buf[4];
+	unsigned int i;
+	int ret;
+
+	/* target address is offset by 1 */
+	unsigned int addr_offset = addr - 1;
+
+	/* write safeload addresses */
+	for (i = 0; i < len / 4; i++) {
+		memcpy(buf, bytes + i * 4, 4);
Is this memcpy really required or could we just write bytes directly in a single regmap_raw_write()? I believe the adau17x1 will auto increment the register addresses.

Quoted 1 line

And I suppose we also need to handle the case when len is not a multiple of 4.

A single regmap_raw_write should work, forgot about the addresses auto incrementing. What would be the proper way of handling the case where len is not a multiple of 4?

Quoted 33 lines

sound/soc/codecs/adau17x1.c int adau17x1_resume(struct snd_soc_component *component) · line 999
Quoted 12 lines
sound/soc/codecs/adau17x1.c +11 -0
+		ret = regmap_raw_write(sigmadsp->control_data,
+			ADAU17X1_SAFELOAD_DATA(i), buf, 4);
+		if (ret < 0)
+			return ret;
+	}
+
+	/* write target address */
+	buf[0] = (addr_offset >> 24) & 0xff;
+	buf[1] = (addr_offset >> 16) & 0xff;
+	buf[2] = (addr_offset >> 8) & 0xff;
+	buf[3] = (addr_offset) & 0xff;
This could be slightly simplified using
	put_unaligend_be32(addr_offset, buf);
sound/soc/codecs/adau17x1.c int adau17x1_resume(struct snd_soc_component *component) · line 1009
Quoted 10 lines
sound/soc/codecs/adau17x1.c +10 -0
+	ret = regmap_raw_write(sigmadsp->control_data,
+		ADAU17X1_SAFELOAD_TARGET_ADDRESS, buf, 4);
+	if (ret < 0)
+		return ret;
+
+	/* write nbr of words to trigger address */
+	buf[0] = 0x00;
+	buf[1] = 0x00;
+	buf[2] = 0x00;
+	buf[3] = i & 0xff;
Same here I guess
Quoted 5 lines
sound/soc/codecs/adau17x1.c +5 -0
+	ret = regmap_raw_write(sigmadsp->control_data,
+		ADAU17X1_SAFELOAD_TRIGGER, buf, 4);
+
+	return ret;
+}

Lars-Peter Clausen

maintainer Acked-by on v2 1/2

2018-08-13 10:45 UTC #4 lore raw

In reply to Robert Rosengren

Quoted 5 lines · Robert Rosengren

adau17x1_setup_firmware and adau17x1_has_dsp is only used internally, so making them static instead of exported.
Change-Id I4882513ab457966b0371ac2c1024550ca6edbe0c
Signed-off-by Robert Rosengren <[email protected]>

Thanks for cleaning this up.

Acked-by Lars-Peter Clausen <[email protected]>

Mark Brown

maintainer

2018-08-14 14:25 UTC #5 lore raw

Applied "ASoC: adau17x1: Unused exported functions changed to internal" to the asoc tree

In reply to Robert Rosengren

The patch

ASoC: adau17x1: Unused exported functions changed to internal

has been applied to the asoc tree at

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

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

Quoted 1 line

From 164c0f6454186d1b0e22c27dfe3d3d59fc80f979 Mon Sep 17 00:00:00 2001

From: Robert Rosengren <[email protected]>
Date: Mon, 13 Aug 2018 09:33:58 +0200
Subject: [PATCH] ASoC: adau17x1: Unused exported functions changed to internal

adau17x1_setup_firmware and adau17x1_has_dsp is only used internally, so making them static instead of exported.

Change-Id I4882513ab457966b0371ac2c1024550ca6edbe0c

Signed-off-by Robert Rosengren <[email protected]>

Acked-by Lars-Peter Clausen <[email protected]>

Signed-off-by Mark Brown <[email protected]>

---
 sound/soc/codecs/adau17x1.c | 9 +++++----
 sound/soc/codecs/adau17x1.h | 4 ----
 2 files changed, 5 insertions(+), 8 deletions(-)

sound/soc/codecs/adau17x1.c +5 -4

index 57169b8ff14e..2f2afb4c0c88 100644
@@ -60,6 +60,9 @@ static const struct snd_kcontrol_new adau17x1_controls[] = {
 	SOC_ENUM("Mic Bias Mode", adau17x1_mic_bias_mode_enum),
 };
 
+static int adau17x1_setup_firmware(struct snd_soc_component *component,
+	unsigned int rate);
+
 static int adau17x1_pll_event(struct snd_soc_dapm_widget *w,
 	struct snd_kcontrol *kcontrol, int event)
 {

@@ -313,7 +316,7 @@ static const struct snd_soc_dapm_route adau17x1_no_dsp_dapm_routes[] = {
 	{ "Capture", NULL, "Right Decimator" },
 };
 
-bool adau17x1_has_dsp(struct adau *adau)
+static bool adau17x1_has_dsp(struct adau *adau)
 {
 	switch (adau->type) {
 	case ADAU1761:

@@ -324,7 +327,6 @@ bool adau17x1_has_dsp(struct adau *adau)
 		return false;
 	}
 }
-EXPORT_SYMBOL_GPL(adau17x1_has_dsp);
 
 static int adau17x1_set_dai_pll(struct snd_soc_dai *dai, int pll_id,
 	int source, unsigned int freq_in, unsigned int freq_out)

@@ -836,7 +838,7 @@ bool adau17x1_volatile_register(struct device *dev, unsigned int reg)
 }
 EXPORT_SYMBOL_GPL(adau17x1_volatile_register);
 
-int adau17x1_setup_firmware(struct snd_soc_component *component,
+static int adau17x1_setup_firmware(struct snd_soc_component *component,
 	unsigned int rate)
 {
 	int ret;

@@ -880,7 +882,6 @@ int adau17x1_setup_firmware(struct snd_soc_component *component,
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(adau17x1_setup_firmware);
 
 int adau17x1_add_widgets(struct snd_soc_component *component)
 {

sound/soc/codecs/adau17x1.h +0 -4

index e6fe87beec07..98a3b6f5bc96 100644
@@ -68,10 +68,6 @@ int adau17x1_resume(struct snd_soc_component *component);
 
 extern const struct snd_soc_dai_ops adau17x1_dai_ops;
 
-int adau17x1_setup_firmware(struct snd_soc_component *component,
-	unsigned int rate);
-bool adau17x1_has_dsp(struct adau *adau);
-
 #define ADAU17X1_CLOCK_CONTROL			0x4000
 #define ADAU17X1_PLL_CONTROL			0x4002
 #define ADAU17X1_REC_POWER_MGMT			0x4009

Signature · 1 line

2.18.0