[PATCH 1/2] ASoC: q6afe-dai: add dummy register read function

[PATCH 1/2] ASoC: q6afe-dai: add dummy register read function

[Srinivas Kandagatla]

Most of the DAPM widgets for DSP ASoC components reuse reg field
of the widgets for its internal calculations, however these are not
real registers. So read/writes to these numbers are not really
valid. However ASoC core will read these registers to get default
state during startup.

With recent changes to ASoC core, every register read/write
failures are reported very verbosely. Prior to this fails to reads
are totally ignored, so we never saw any error messages.

To fix this add dummy read function to return default value.

Reported-by: John Stultz <john.stultz@linaro.org>
Fixes: 24c4cbcfac09 ("ASoC: qdsp6: q6afe: Add q6afe dai driver")
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 sound/soc/qcom/qdsp6/q6afe-dai.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/sound/soc/qcom/qdsp6/q6afe-dai.c b/sound/soc/qcom/qdsp6/q6afe-dai.c
index 2a5302f1db98..492bdbaf13c4 100644
--- a/sound/soc/qcom/qdsp6/q6afe-dai.c
+++ b/sound/soc/qcom/qdsp6/q6afe-dai.c
@@ -1352,6 +1352,13 @@ static const struct snd_soc_dapm_widget q6afe_dai_widgets[] = {
 	SND_SOC_DAPM_AIF_OUT("DISPLAY_PORT_RX", "NULL", 0, 0, 0, 0),
 };
 
+static unsigned int q6afe_reg_read(struct snd_soc_component *component,
+				   unsigned int reg)
+{
+	/* default value */
+	return 0;
+}
+
 static const struct snd_soc_component_driver q6afe_dai_component = {
 	.name		= "q6afe-dai-component",
 	.dapm_widgets = q6afe_dai_widgets,
@@ -1359,7 +1366,7 @@ static const struct snd_soc_component_driver q6afe_dai_component = {
 	.dapm_routes = q6afe_dapm_routes,
 	.num_dapm_routes = ARRAY_SIZE(q6afe_dapm_routes),
 	.of_xlate_dai_name = q6afe_of_xlate_dai_name,
-
+	.read = q6afe_reg_read,
 };
 
 static void of_q6afe_parse_dai_data(struct device *dev,
-- 
2.21.0

[PATCH 2/2] ASoC: q6routing: add dummy register read function

[Srinivas Kandagatla]

Most of the DAPM widgets for DSP ASoC components reuse reg field
of the widgets for its internal calculations, however these are not
real registers. So read/writes to these numbers are not really
valid. However ASoC core will read these registers to get default
state during startup.

With recent changes to ASoC core, every register read/write
failures are reported very verbosely. Prior to this fails to reads
are totally ignored, so we never saw any error messages.

To fix this add dummy read function to return default value.

Reported-by: John Stultz <john.stultz@linaro.org>
Fixes: e3a33673e845 ("ASoC: qdsp6: q6routing: Add q6routing driver")
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 sound/soc/qcom/qdsp6/q6routing.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/sound/soc/qcom/qdsp6/q6routing.c b/sound/soc/qcom/qdsp6/q6routing.c
index eaa95b5a7b66..fb7c4b266d19 100644
--- a/sound/soc/qcom/qdsp6/q6routing.c
+++ b/sound/soc/qcom/qdsp6/q6routing.c
@@ -973,6 +973,13 @@ static int msm_routing_probe(struct snd_soc_component *c)
 	return 0;
 }
 
+static unsigned int q6routing_reg_read(struct snd_soc_component *component,
+				       unsigned int reg)
+{
+	/* default value */
+	return 0;
+}
+
 static const struct snd_soc_component_driver msm_soc_routing_component = {
 	.probe = msm_routing_probe,
 	.name = DRV_NAME,
@@ -981,6 +988,7 @@ static const struct snd_soc_component_driver msm_soc_routing_component = {
 	.num_dapm_widgets = ARRAY_SIZE(msm_qdsp6_widgets),
 	.dapm_routes = intercon,
 	.num_dapm_routes = ARRAY_SIZE(intercon),
+	.read = q6routing_reg_read,
 };
 
 static int q6pcm_routing_probe(struct platform_device *pdev)
-- 
2.21.0

Re: [PATCH 1/2] ASoC: q6afe-dai: add dummy register read function

[Stephan Gerhold]

On Tue, Aug 11, 2020 at 11:25:51AM +0100, Srinivas Kandagatla wrote:
> Most of the DAPM widgets for DSP ASoC components reuse reg field
> of the widgets for its internal calculations, however these are not
> real registers. So read/writes to these numbers are not really
> valid. However ASoC core will read these registers to get default
> state during startup.
> 

Actually q6afe-dai does not seem to make use of the register number.
The DAPM widgets all look like

  SND_SOC_DAPM_AIF_IN("HDMI_RX",  NULL,   0,    0,     0,      0),
                  /* (wname,    stname, wchan, wreg, wshift, winvert)

Wouldn't it be better to replace wreg = 0 with SND_SOC_NOPM in this case
so the read/write won't happen at all?

q6routing on the other hand does make use of wreg, so this would not
work there...

Also: If I remember correctly the ASoC core will also attempt to write
to these "registers" when starting to play some audio, so you might also
need to implement component->write().

Stephan

Re: [PATCH 1/2] ASoC: q6afe-dai: add dummy register read function

[Srinivas Kandagatla]

On 11/08/2020 12:04, Stephan Gerhold wrote:
> On Tue, Aug 11, 2020 at 11:25:51AM +0100, Srinivas Kandagatla wrote:
>> Most of the DAPM widgets for DSP ASoC components reuse reg field
>> of the widgets for its internal calculations, however these are not
>> real registers. So read/writes to these numbers are not really
>> valid. However ASoC core will read these registers to get default
>> state during startup.
>>
> 
> Actually q6afe-dai does not seem to make use of the register number.
> The DAPM widgets all look like
> 
>    SND_SOC_DAPM_AIF_IN("HDMI_RX",  NULL,   0,    0,     0,      0),
>                    /* (wname,    stname, wchan, wreg, wshift, winvert)
> 
> Wouldn't it be better to replace wreg = 0 with SND_SOC_NOPM in this case
> so the read/write won't happen at all?
That should work, let me try that!

--srini
> 
> q6routing on the other hand does make use of wreg, so this would not
> work there...
> 
> Also: If I remember correctly the ASoC core will also attempt to write
> to these "registers" when starting to play some audio, so you might also
> need to implement component->write().

> 
> Stephan
> 

Re: [PATCH 1/2] ASoC: q6afe-dai: add dummy register read function

[Mark Brown]

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

On Tue, Aug 11, 2020 at 01:04:09PM +0200, Stephan Gerhold wrote:

> Actually q6afe-dai does not seem to make use of the register number.
> The DAPM widgets all look like

>   SND_SOC_DAPM_AIF_IN("HDMI_RX",  NULL,   0,    0,     0,      0),
>                   /* (wname,    stname, wchan, wreg, wshift, winvert)

> Wouldn't it be better to replace wreg = 0 with SND_SOC_NOPM in this case
> so the read/write won't happen at all?

Yes, if they are not used at all then that's what _NOPM is for.

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