Re: [PATCH 2/3] ASoC: lpass-platform: Fix broken pcm data usage
On Fri, Oct 28, 2016 at 04:32:19PM +0100, Srinivas Kandagatla wrote:
> This patch fixes lpass-platform driver which was broken in v4.9-rc1.
> lpass_pcm_data data structure holds information specific to stream.
> Holding a single private pointer to it in global lpass_data
> will not work, because it would be overwritten by for each pcm instance.
>
> This code was breaking playback when we have both playback and capture
> pcm streams, as playback settings are over written by capture settings.
>
> Fix this by moving channel allocation logic out of pcm_new to pcm_open
> so that we can store the stream specific information private_data of
> snd_pcm_runtime.
>
> Fixes: 6adcbdcd4b6e ("ASoC: lpass-platform: don't use
> snd_soc_pcm_set_drvdata()")
> Signed-off-by: Srinivas Kandagatla
> ---
> sound/soc/qcom/lpass-platform.c | 151
> ++--
> sound/soc/qcom/lpass.h | 1 -
> 2 files changed, 67 insertions(+), 85 deletions(-)
After you address Mark's comments:
Acked-by: Kenneth Westfield
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [PATCH 2/3] ASoC: lpass-platform: Fix broken pcm data usage
On Fri, Oct 28, 2016 at 04:32:19PM +0100, Srinivas Kandagatla wrote:
> This patch fixes lpass-platform driver which was broken in v4.9-rc1.
> lpass_pcm_data data structure holds information specific to stream.
> Holding a single private pointer to it in global lpass_data
> will not work, because it would be overwritten by for each pcm instance.
>
> This code was breaking playback when we have both playback and capture
> pcm streams, as playback settings are over written by capture settings.
>
> Fix this by moving channel allocation logic out of pcm_new to pcm_open
> so that we can store the stream specific information private_data of
> snd_pcm_runtime.
>
> Fixes: 6adcbdcd4b6e ("ASoC: lpass-platform: don't use
> snd_soc_pcm_set_drvdata()")
> Signed-off-by: Srinivas Kandagatla
> ---
> sound/soc/qcom/lpass-platform.c | 151
> ++--
> sound/soc/qcom/lpass.h | 1 -
> 2 files changed, 67 insertions(+), 85 deletions(-)
After you address Mark's comments:
Acked-by: Kenneth Westfield
--
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [PATCH 2/3] ASoC: lpass-platform: Fix broken pcm data usage
On 28/10/16 19:26, Mark Brown wrote: On Fri, Oct 28, 2016 at 04:32:19PM +0100, Srinivas Kandagatla wrote: This patch fixes lpass-platform driver which was broken in v4.9-rc1. lpass_pcm_data data structure holds information specific to stream. Holding a single private pointer to it in global lpass_data will not work, because it would be overwritten by for each pcm instance. This is a bug fix but you've made it depend on patch 1 which is just a refactoring and should therefore not be sent to Linus as a fix. Please don't do this, fix bugs first then do any cleanups and new functionality. That way we can send fixes to Linus without needing to pull in extra stuff. Sorry about that, I will resend the fix patch without any dependencies. Thanks, srini
Re: [PATCH 2/3] ASoC: lpass-platform: Fix broken pcm data usage
On 28/10/16 19:26, Mark Brown wrote: On Fri, Oct 28, 2016 at 04:32:19PM +0100, Srinivas Kandagatla wrote: This patch fixes lpass-platform driver which was broken in v4.9-rc1. lpass_pcm_data data structure holds information specific to stream. Holding a single private pointer to it in global lpass_data will not work, because it would be overwritten by for each pcm instance. This is a bug fix but you've made it depend on patch 1 which is just a refactoring and should therefore not be sent to Linus as a fix. Please don't do this, fix bugs first then do any cleanups and new functionality. That way we can send fixes to Linus without needing to pull in extra stuff. Sorry about that, I will resend the fix patch without any dependencies. Thanks, srini
Re: [PATCH 2/3] ASoC: lpass-platform: Fix broken pcm data usage
On Fri, Oct 28, 2016 at 04:32:19PM +0100, Srinivas Kandagatla wrote: > This patch fixes lpass-platform driver which was broken in v4.9-rc1. > lpass_pcm_data data structure holds information specific to stream. > Holding a single private pointer to it in global lpass_data > will not work, because it would be overwritten by for each pcm instance. This is a bug fix but you've made it depend on patch 1 which is just a refactoring and should therefore not be sent to Linus as a fix. Please don't do this, fix bugs first then do any cleanups and new functionality. That way we can send fixes to Linus without needing to pull in extra stuff. signature.asc Description: PGP signature
Re: [PATCH 2/3] ASoC: lpass-platform: Fix broken pcm data usage
On Fri, Oct 28, 2016 at 04:32:19PM +0100, Srinivas Kandagatla wrote: > This patch fixes lpass-platform driver which was broken in v4.9-rc1. > lpass_pcm_data data structure holds information specific to stream. > Holding a single private pointer to it in global lpass_data > will not work, because it would be overwritten by for each pcm instance. This is a bug fix but you've made it depend on patch 1 which is just a refactoring and should therefore not be sent to Linus as a fix. Please don't do this, fix bugs first then do any cleanups and new functionality. That way we can send fixes to Linus without needing to pull in extra stuff. signature.asc Description: PGP signature
[PATCH 2/3] ASoC: lpass-platform: Fix broken pcm data usage
This patch fixes lpass-platform driver which was broken in v4.9-rc1.
lpass_pcm_data data structure holds information specific to stream.
Holding a single private pointer to it in global lpass_data
will not work, because it would be overwritten by for each pcm instance.
This code was breaking playback when we have both playback and capture
pcm streams, as playback settings are over written by capture settings.
Fix this by moving channel allocation logic out of pcm_new to pcm_open
so that we can store the stream specific information private_data of
snd_pcm_runtime.
Fixes: 6adcbdcd4b6e ("ASoC: lpass-platform: don't use
snd_soc_pcm_set_drvdata()")
Signed-off-by: Srinivas Kandagatla
---
sound/soc/qcom/lpass-platform.c | 151 ++--
sound/soc/qcom/lpass.h | 1 -
2 files changed, 67 insertions(+), 85 deletions(-)
diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c
index 2570c96..4554cae 100644
--- a/sound/soc/qcom/lpass-platform.c
+++ b/sound/soc/qcom/lpass-platform.c
@@ -60,7 +60,36 @@ static int lpass_platform_pcmops_open(struct
snd_pcm_substream *substream)
{
struct snd_pcm_runtime *runtime = substream->runtime;
struct snd_soc_pcm_runtime *soc_runtime = substream->private_data;
- int ret;
+ struct snd_soc_dai *cpu_dai = soc_runtime->cpu_dai;
+ struct lpass_data *drvdata =
+ snd_soc_platform_get_drvdata(soc_runtime->platform);
+ struct lpass_variant *v = drvdata->variant;
+ int ret, dir = substream->stream;
+ struct lpass_pcm_data *data;
+
+ data = devm_kzalloc(soc_runtime->dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->i2s_port = cpu_dai->driver->id;
+ runtime->private_data = data;
+
+ if (v->alloc_dma_channel)
+ data->dma_ch = v->alloc_dma_channel(drvdata, dir);
+ if (data->dma_ch < 0)
+ return data->dma_ch;
+
+ drvdata->substream[data->dma_ch] = substream;
+
+ ret = regmap_write(drvdata->lpaif_map,
+ LPAIF_DMACTL_REG(v, data->dma_ch, dir), 0);
+ if (ret) {
+ dev_err(soc_runtime->dev,
+ "%s() error writing to rdmactl reg: %d\n",
+ __func__, ret);
+ return ret;
+ }
+
snd_soc_set_runtime_hwparams(substream, _platform_pcm_hardware);
@@ -79,13 +108,32 @@ static int lpass_platform_pcmops_open(struct
snd_pcm_substream *substream)
return 0;
}
+static int lpass_platform_pcmops_close(struct snd_pcm_substream *substream)
+{
+ struct snd_pcm_runtime *runtime = substream->runtime;
+ struct snd_soc_pcm_runtime *soc_runtime = substream->private_data;
+ struct lpass_data *drvdata =
+ snd_soc_platform_get_drvdata(soc_runtime->platform);
+ struct lpass_variant *v = drvdata->variant;
+ struct lpass_pcm_data *data;
+
+ data = runtime->private_data;
+ v = drvdata->variant;
+ drvdata->substream[data->dma_ch] = NULL;
+ if (v->free_dma_channel)
+ v->free_dma_channel(drvdata, data->dma_ch);
+
+ return 0;
+}
+
static int lpass_platform_pcmops_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params)
{
struct snd_soc_pcm_runtime *soc_runtime = substream->private_data;
struct lpass_data *drvdata =
snd_soc_platform_get_drvdata(soc_runtime->platform);
- struct lpass_pcm_data *pcm_data = drvdata->private_data;
+ struct snd_pcm_runtime *rt = substream->runtime;
+ struct lpass_pcm_data *pcm_data = rt->private_data;
struct lpass_variant *v = drvdata->variant;
snd_pcm_format_t format = params_format(params);
unsigned int channels = params_channels(params);
@@ -175,7 +223,8 @@ static int lpass_platform_pcmops_hw_free(struct
snd_pcm_substream *substream)
struct snd_soc_pcm_runtime *soc_runtime = substream->private_data;
struct lpass_data *drvdata =
snd_soc_platform_get_drvdata(soc_runtime->platform);
- struct lpass_pcm_data *pcm_data = drvdata->private_data;
+ struct snd_pcm_runtime *rt = substream->runtime;
+ struct lpass_pcm_data *pcm_data = rt->private_data;
struct lpass_variant *v = drvdata->variant;
unsigned int reg;
int ret;
@@ -195,7 +244,8 @@ static int lpass_platform_pcmops_prepare(struct
snd_pcm_substream *substream)
struct snd_soc_pcm_runtime *soc_runtime = substream->private_data;
struct lpass_data *drvdata =
snd_soc_platform_get_drvdata(soc_runtime->platform);
- struct lpass_pcm_data *pcm_data = drvdata->private_data;
+ struct snd_pcm_runtime *rt = substream->runtime;
+ struct lpass_pcm_data *pcm_data = rt->private_data;
struct lpass_variant *v = drvdata->variant;
[PATCH 2/3] ASoC: lpass-platform: Fix broken pcm data usage
This patch fixes lpass-platform driver which was broken in v4.9-rc1.
lpass_pcm_data data structure holds information specific to stream.
Holding a single private pointer to it in global lpass_data
will not work, because it would be overwritten by for each pcm instance.
This code was breaking playback when we have both playback and capture
pcm streams, as playback settings are over written by capture settings.
Fix this by moving channel allocation logic out of pcm_new to pcm_open
so that we can store the stream specific information private_data of
snd_pcm_runtime.
Fixes: 6adcbdcd4b6e ("ASoC: lpass-platform: don't use
snd_soc_pcm_set_drvdata()")
Signed-off-by: Srinivas Kandagatla
---
sound/soc/qcom/lpass-platform.c | 151 ++--
sound/soc/qcom/lpass.h | 1 -
2 files changed, 67 insertions(+), 85 deletions(-)
diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c
index 2570c96..4554cae 100644
--- a/sound/soc/qcom/lpass-platform.c
+++ b/sound/soc/qcom/lpass-platform.c
@@ -60,7 +60,36 @@ static int lpass_platform_pcmops_open(struct
snd_pcm_substream *substream)
{
struct snd_pcm_runtime *runtime = substream->runtime;
struct snd_soc_pcm_runtime *soc_runtime = substream->private_data;
- int ret;
+ struct snd_soc_dai *cpu_dai = soc_runtime->cpu_dai;
+ struct lpass_data *drvdata =
+ snd_soc_platform_get_drvdata(soc_runtime->platform);
+ struct lpass_variant *v = drvdata->variant;
+ int ret, dir = substream->stream;
+ struct lpass_pcm_data *data;
+
+ data = devm_kzalloc(soc_runtime->dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->i2s_port = cpu_dai->driver->id;
+ runtime->private_data = data;
+
+ if (v->alloc_dma_channel)
+ data->dma_ch = v->alloc_dma_channel(drvdata, dir);
+ if (data->dma_ch < 0)
+ return data->dma_ch;
+
+ drvdata->substream[data->dma_ch] = substream;
+
+ ret = regmap_write(drvdata->lpaif_map,
+ LPAIF_DMACTL_REG(v, data->dma_ch, dir), 0);
+ if (ret) {
+ dev_err(soc_runtime->dev,
+ "%s() error writing to rdmactl reg: %d\n",
+ __func__, ret);
+ return ret;
+ }
+
snd_soc_set_runtime_hwparams(substream, _platform_pcm_hardware);
@@ -79,13 +108,32 @@ static int lpass_platform_pcmops_open(struct
snd_pcm_substream *substream)
return 0;
}
+static int lpass_platform_pcmops_close(struct snd_pcm_substream *substream)
+{
+ struct snd_pcm_runtime *runtime = substream->runtime;
+ struct snd_soc_pcm_runtime *soc_runtime = substream->private_data;
+ struct lpass_data *drvdata =
+ snd_soc_platform_get_drvdata(soc_runtime->platform);
+ struct lpass_variant *v = drvdata->variant;
+ struct lpass_pcm_data *data;
+
+ data = runtime->private_data;
+ v = drvdata->variant;
+ drvdata->substream[data->dma_ch] = NULL;
+ if (v->free_dma_channel)
+ v->free_dma_channel(drvdata, data->dma_ch);
+
+ return 0;
+}
+
static int lpass_platform_pcmops_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params)
{
struct snd_soc_pcm_runtime *soc_runtime = substream->private_data;
struct lpass_data *drvdata =
snd_soc_platform_get_drvdata(soc_runtime->platform);
- struct lpass_pcm_data *pcm_data = drvdata->private_data;
+ struct snd_pcm_runtime *rt = substream->runtime;
+ struct lpass_pcm_data *pcm_data = rt->private_data;
struct lpass_variant *v = drvdata->variant;
snd_pcm_format_t format = params_format(params);
unsigned int channels = params_channels(params);
@@ -175,7 +223,8 @@ static int lpass_platform_pcmops_hw_free(struct
snd_pcm_substream *substream)
struct snd_soc_pcm_runtime *soc_runtime = substream->private_data;
struct lpass_data *drvdata =
snd_soc_platform_get_drvdata(soc_runtime->platform);
- struct lpass_pcm_data *pcm_data = drvdata->private_data;
+ struct snd_pcm_runtime *rt = substream->runtime;
+ struct lpass_pcm_data *pcm_data = rt->private_data;
struct lpass_variant *v = drvdata->variant;
unsigned int reg;
int ret;
@@ -195,7 +244,8 @@ static int lpass_platform_pcmops_prepare(struct
snd_pcm_substream *substream)
struct snd_soc_pcm_runtime *soc_runtime = substream->private_data;
struct lpass_data *drvdata =
snd_soc_platform_get_drvdata(soc_runtime->platform);
- struct lpass_pcm_data *pcm_data = drvdata->private_data;
+ struct snd_pcm_runtime *rt = substream->runtime;
+ struct lpass_pcm_data *pcm_data = rt->private_data;
struct lpass_variant *v = drvdata->variant;
int ret, ch, dir =
