[PATCH] ASoC: sigmadsp: Disable cache mechanism for readbacks

[PATCH] ASoC: sigmadsp: Disable cache mechanism for readbacks

[Niklas Carlsson]

From: Niklas Carlsson <niklasc@axis.com>

The ALSA control readback functionality only works for non-volatile
controls, i.e. control values that does not change on their own without
driver interaction.

This doesn't work for readbacks since the DSP firmware updates the
control value. Disable the cache mechanism in the driver if the control
name matches the prefix used for readbacks to ensure that the control
value is valid.

Signed-off-by: Niklas Carlsson <niklasc@axis.com>
---
 sound/soc/codecs/sigmadsp.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/sigmadsp.c b/sound/soc/codecs/sigmadsp.c
index 76c77dc8ecf7..b992216aee55 100644
--- a/sound/soc/codecs/sigmadsp.c
+++ b/sound/soc/codecs/sigmadsp.c
@@ -24,6 +24,8 @@
 #define SIGMA_FW_CHUNK_TYPE_CONTROL 1
 #define SIGMA_FW_CHUNK_TYPE_SAMPLERATES 2
 
+#define READBACK_CTRL_NAME "ReadBack"
+
 struct sigmadsp_control {
 	struct list_head head;
 	uint32_t samplerates;
@@ -31,6 +33,7 @@ struct sigmadsp_control {
 	unsigned int num_bytes;
 	const char *name;
 	struct snd_kcontrol *kcontrol;
+	bool is_readback;
 	bool cached;
 	uint8_t cache[];
 };
@@ -141,7 +144,8 @@ static int sigmadsp_ctrl_put(struct snd_kcontrol *kcontrol,
 
 	if (ret == 0) {
 		memcpy(ctrl->cache, data, ctrl->num_bytes);
-		ctrl->cached = true;
+		if (!ctrl->is_readback)
+			ctrl->cached = true;
 	}
 
 	mutex_unlock(&sigmadsp->lock);
@@ -164,7 +168,8 @@ static int sigmadsp_ctrl_get(struct snd_kcontrol *kcontrol,
 	}
 
 	if (ret == 0) {
-		ctrl->cached = true;
+		if (!ctrl->is_readback)
+			ctrl->cached = true;
 		memcpy(ucontrol->value.bytes.data, ctrl->cache,
 			ctrl->num_bytes);
 	}
@@ -231,6 +236,15 @@ static int sigma_fw_load_control(struct sigmadsp *sigmadsp,
 	name[name_len] = '\0';
 	ctrl->name = name;
 
+	/*
+	 * Readbacks doesn't work with non-volatile controls, since the
+	 * firmware updates the control value without driver interaction. Mark
+	 * the readbacks to ensure that the values are not cached.
+	 */
+	if (ctrl->name && strncmp(ctrl->name, READBACK_CTRL_NAME,
+				  (sizeof(READBACK_CTRL_NAME) - 1)) == 0)
+		ctrl->is_readback = true;
+
 	ctrl->addr = le16_to_cpu(ctrl_chunk->addr);
 	ctrl->num_bytes = num_bytes;
 	ctrl->samplerates = le32_to_cpu(chunk->samplerates);
-- 
2.20.1

[PATCH] ASoC: sigmadsp: Disable cache mechanism for readbacks

[Niklas Carlsson]

From: Niklas Carlsson <niklasc@axis.com>

The ALSA control readback functionality only works for non-volatile
controls, i.e. control values that does not change on their own without
driver interaction.

This doesn't work for readbacks since the DSP firmware updates the
control value. Disable the cache mechanism in the driver if the control
name matches the prefix used for readbacks to ensure that the control
value is valid.

Signed-off-by: Niklas Carlsson <niklasc@axis.com>
---
 sound/soc/codecs/sigmadsp.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/sigmadsp.c b/sound/soc/codecs/sigmadsp.c
index 76c77dc8ecf7..b992216aee55 100644
--- a/sound/soc/codecs/sigmadsp.c
+++ b/sound/soc/codecs/sigmadsp.c
@@ -24,6 +24,8 @@
 #define SIGMA_FW_CHUNK_TYPE_CONTROL 1
 #define SIGMA_FW_CHUNK_TYPE_SAMPLERATES 2
 
+#define READBACK_CTRL_NAME "ReadBack"
+
 struct sigmadsp_control {
 	struct list_head head;
 	uint32_t samplerates;
@@ -31,6 +33,7 @@ struct sigmadsp_control {
 	unsigned int num_bytes;
 	const char *name;
 	struct snd_kcontrol *kcontrol;
+	bool is_readback;
 	bool cached;
 	uint8_t cache[];
 };
@@ -141,7 +144,8 @@ static int sigmadsp_ctrl_put(struct snd_kcontrol *kcontrol,
 
 	if (ret == 0) {
 		memcpy(ctrl->cache, data, ctrl->num_bytes);
-		ctrl->cached = true;
+		if (!ctrl->is_readback)
+			ctrl->cached = true;
 	}
 
 	mutex_unlock(&sigmadsp->lock);
@@ -164,7 +168,8 @@ static int sigmadsp_ctrl_get(struct snd_kcontrol *kcontrol,
 	}
 
 	if (ret == 0) {
-		ctrl->cached = true;
+		if (!ctrl->is_readback)
+			ctrl->cached = true;
 		memcpy(ucontrol->value.bytes.data, ctrl->cache,
 			ctrl->num_bytes);
 	}
@@ -231,6 +236,15 @@ static int sigma_fw_load_control(struct sigmadsp *sigmadsp,
 	name[name_len] = '\0';
 	ctrl->name = name;
 
+	/*
+	 * Readbacks doesn't work with non-volatile controls, since the
+	 * firmware updates the control value without driver interaction. Mark
+	 * the readbacks to ensure that the values are not cached.
+	 */
+	if (ctrl->name && strncmp(ctrl->name, READBACK_CTRL_NAME,
+				  (sizeof(READBACK_CTRL_NAME) - 1)) == 0)
+		ctrl->is_readback = true;
+
 	ctrl->addr = le16_to_cpu(ctrl_chunk->addr);
 	ctrl->num_bytes = num_bytes;
 	ctrl->samplerates = le32_to_cpu(chunk->samplerates);
-- 
2.20.1

Re: [PATCH] ASoC: sigmadsp: Disable cache mechanism for readbacks

[Mark Brown]

On Mon, 19 Apr 2021 16:49:01 +0200, Niklas Carlsson wrote:
> The ALSA control readback functionality only works for non-volatile
> controls, i.e. control values that does not change on their own without
> driver interaction.
> 
> This doesn't work for readbacks since the DSP firmware updates the
> control value. Disable the cache mechanism in the driver if the control
> name matches the prefix used for readbacks to ensure that the control
> value is valid.

Applied to

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

Thanks!

[1/1] ASoC: sigmadsp: Disable cache mechanism for readbacks
      commit: 11a9b987611f17e6a95e9bb34c1f5f9aa0b6ae25

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

Re: [PATCH] ASoC: sigmadsp: Disable cache mechanism for readbacks

[Mark Brown]

On Mon, 19 Apr 2021 16:49:01 +0200, Niklas Carlsson wrote:
> The ALSA control readback functionality only works for non-volatile
> controls, i.e. control values that does not change on their own without
> driver interaction.
> 
> This doesn't work for readbacks since the DSP firmware updates the
> control value. Disable the cache mechanism in the driver if the control
> name matches the prefix used for readbacks to ensure that the control
> value is valid.

Applied to

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

Thanks!

[1/1] ASoC: sigmadsp: Disable cache mechanism for readbacks
      commit: 11a9b987611f17e6a95e9bb34c1f5f9aa0b6ae25

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