[alsa-devel] [PATCH v2 1/2] ASoC: adau17x1: Unused exported functions changed to internal
![](https://secure.gravatar.com/avatar/9a9021ed5c96bdbb83049156213ddfa0.jpg?s=120&d=mm&r=g)
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 robertr@axis.com --- sound/soc/codecs/adau17x1.c | 9 +++++---- sound/soc/codecs/adau17x1.h | 4 ---- 2 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/sound/soc/codecs/adau17x1.c b/sound/soc/codecs/adau17x1.c index 57169b8ff14e..2f2afb4c0c88 100644 --- a/sound/soc/codecs/adau17x1.c +++ b/sound/soc/codecs/adau17x1.c @@ -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) { diff --git a/sound/soc/codecs/adau17x1.h b/sound/soc/codecs/adau17x1.h index e6fe87beec07..98a3b6f5bc96 100644 --- a/sound/soc/codecs/adau17x1.h +++ b/sound/soc/codecs/adau17x1.h @@ -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
![](https://secure.gravatar.com/avatar/9a9021ed5c96bdbb83049156213ddfa0.jpg?s=120&d=mm&r=g)
From: Danny Smith dannys@axis.com
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 dannys@axis.com Signed-off-by: Robert Rosengren robertr@axis.com --- sound/soc/codecs/adau17x1.c | 68 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 66 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/adau17x1.c b/sound/soc/codecs/adau17x1.c index 2f2afb4c0c88..6d0b2261c959 100644 --- a/sound/soc/codecs/adau17x1.c +++ b/sound/soc/codecs/adau17x1.c @@ -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));
![](https://secure.gravatar.com/avatar/a7c9a4773543c649adc71c95805d2cbf.jpg?s=120&d=mm&r=g)
On 08/13/2018 09:33 AM, Robert Rosengren wrote:
From: Danny Smith dannys@axis.com
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 dannys@axis.com Signed-off-by: Robert Rosengren robertr@axis.com
Hi,
Thanks for implementing this. Some comments inline.
[...]
+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.
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);
- 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
- ret = regmap_raw_write(sigmadsp->control_data,
ADAU17X1_SAFELOAD_TRIGGER, buf, 4);
- return ret;
+}
![](https://secure.gravatar.com/avatar/242b6bf3eba24023d23be7b13ade3a59.jpg?s=120&d=mm&r=g)
On 08/13/2018 12:44 PM, Lars-Peter Clausen wrote:
On 08/13/2018 09:33 AM, Robert Rosengren wrote:
From: Danny Smith dannys@axis.com
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 dannys@axis.com Signed-off-by: Robert Rosengren robertr@axis.com
Hi,
Thanks for implementing this. Some comments inline.
[...]
+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.
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?
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);
- 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
- ret = regmap_raw_write(sigmadsp->control_data,
ADAU17X1_SAFELOAD_TRIGGER, buf, 4);
- return ret;
+}
![](https://secure.gravatar.com/avatar/a7c9a4773543c649adc71c95805d2cbf.jpg?s=120&d=mm&r=g)
On 08/13/2018 09:33 AM, Robert Rosengren wrote:
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 robertr@axis.com
Thanks for cleaning this up.
Acked-by: Lars-Peter Clausen lars@metafoo.de
![](https://secure.gravatar.com/avatar/d930951cb00393ecf9c3db3a56d78fa9.jpg?s=120&d=mm&r=g)
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
From 164c0f6454186d1b0e22c27dfe3d3d59fc80f979 Mon Sep 17 00:00:00 2001
From: Robert Rosengren robert.rosengren@axis.com 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 robertr@axis.com Acked-by: Lars-Peter Clausen lars@metafoo.de Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/codecs/adau17x1.c | 9 +++++---- sound/soc/codecs/adau17x1.h | 4 ---- 2 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/sound/soc/codecs/adau17x1.c b/sound/soc/codecs/adau17x1.c index 57169b8ff14e..2f2afb4c0c88 100644 --- a/sound/soc/codecs/adau17x1.c +++ b/sound/soc/codecs/adau17x1.c @@ -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) { diff --git a/sound/soc/codecs/adau17x1.h b/sound/soc/codecs/adau17x1.h index e6fe87beec07..98a3b6f5bc96 100644 --- a/sound/soc/codecs/adau17x1.h +++ b/sound/soc/codecs/adau17x1.h @@ -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
participants (4)
-
Danny Smith
-
Lars-Peter Clausen
-
Mark Brown
-
Robert Rosengren