[alsa-devel] [PATCH 0/3] ASoC: remove snd_soc_pcm_set/get_drvdata()
Hi Mark
snd_soc_pcm_set_drvdata() will set driver data to rtd->dev, but driver data of rtd->dev is already used as "rtd" on soc_post_component_init().
static int soc_post_component_init(xxx) { ... dev_set_drvdata(rtd->dev, rtd); ... }
But, ASoC has snd_soc_pcm_set/get_drvdata() to set rtd->dev. It will overwrite existing data and will break system. This patch tries to solve this issue. Intel haswell and qcom is using these functions.
** Note **
I don't have these boards, thus, I did compile test only.
Kuninori Morimoto (3): ASoC: sst-haswell-pcm: don't use snd_soc_pcm_set_drvdata() ASoC: lpass-platform: don't use snd_soc_pcm_set_drvdata() ASoC: remove snd_soc_pcm_set/get_drvdata()
include/sound/soc.h | 11 ----------- sound/soc/intel/haswell/sst-haswell-pcm.c | 1 - sound/soc/qcom/lpass-platform.c | 14 +++++++------- sound/soc/qcom/lpass.h | 2 ++ 4 files changed, 9 insertions(+), 19 deletions(-)
snd_soc_pcm_set_drvdata() will set driver data to rtd->dev, but driver data of rtd->dev is already used as "rtd" on soc_post_component_init().
static int soc_post_component_init(xxx) { ... dev_set_drvdata(rtd->dev, rtd); ... }
To remove confusion, snd_soc_pcm_set/get_drvdata() should be removed soon. This patch is for it.
intel/haswell/sst-haswell-pcm.c is using snd_soc_pcm_set(), but no one use it on this driver. Let's remove it.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com ---
** Note **
I did compile test only. Please check it
sound/soc/intel/haswell/sst-haswell-pcm.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/sound/soc/intel/haswell/sst-haswell-pcm.c b/sound/soc/intel/haswell/sst-haswell-pcm.c index 994256b..3154525 100644 --- a/sound/soc/intel/haswell/sst-haswell-pcm.c +++ b/sound/soc/intel/haswell/sst-haswell-pcm.c @@ -819,7 +819,6 @@ static int hsw_pcm_open(struct snd_pcm_substream *substream) mutex_lock(&pcm_data->mutex); pm_runtime_get_sync(pdata->dev);
- snd_soc_pcm_set_drvdata(rtd, pcm_data); pcm_data->substream = substream;
snd_soc_set_runtime_hwparams(substream, &hsw_pcm_hardware);
On Mon, Aug 01, 2016 at 06:09:54AM +0000, Kuninori Morimoto wrote:
snd_soc_pcm_set_drvdata() will set driver data to rtd->dev, but driver data of rtd->dev is already used as "rtd" on soc_post_component_init().
This doesn't apply against current code, please check and resend.
Hi Mark
snd_soc_pcm_set_drvdata() will set driver data to rtd->dev, but driver data of rtd->dev is already used as "rtd" on soc_post_component_init().
This doesn't apply against current code, please check and resend.
Thanks. It seems current your branch already has same patch. Thus, This [1/3] is not needed now.
commit c999675b04c146aa57f6e853a3746de979427fad Author: Ben Zhang benzh@chromium.org Date: Thu Jul 7 18:54:56 2016 -0700
ASoC: Intel: Fix conflicting pcm dev drvdata on haswell
soc-core sets the snd_soc_pcm_runtime->dev drvdata to snd_soc_pcm_runtime in soc_post_component_init, and access it in places like codec_reg_show.
hsw_pcm_open overwrites the drvdata to point to hsw_pcm_data, confusing soc-core, and causing crashes when cat /sys/devices/pci0000:00/INT3438:00/.../System PCM/codec_reg
This patch removes the set in hsw_pcm_open since it's no longer used. commit 7ff9d6714a5c ("ASoC: Intel: Split hsw_pcm_data for playback and capture") already removed all calls to snd_soc_pcm_get_drvdata(rtd).
Signed-off-by: Ben Zhang benzh@chromium.org Signed-off-by: Mark Brown broonie@kernel.org
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
snd_soc_pcm_set_drvdata() will set driver data to rtd->dev, but driver data of rtd->dev is already used as "rtd" on soc_post_component_init().
static int soc_post_component_init(xxx) { ... dev_set_drvdata(rtd->dev, rtd); ... }
To remove confusion, snd_soc_pcm_set/get_drvdata() should be removed soon. This patch is for it.
qcom/lpass-platform.c sets struct lpass_pcm_data to rtd->dev today, but we can replace it to struct lpass_data :: private_data.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com ---
** Note **
I did compile test only. Please check it
sound/soc/qcom/lpass-platform.c | 14 +++++++------- sound/soc/qcom/lpass.h | 2 ++ 2 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c index db000c6..a144c14 100644 --- a/sound/soc/qcom/lpass-platform.c +++ b/sound/soc/qcom/lpass-platform.c @@ -84,9 +84,9 @@ 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_pcm_data *pcm_data = snd_soc_pcm_get_drvdata(soc_runtime); struct lpass_data *drvdata = snd_soc_platform_get_drvdata(soc_runtime->platform); + struct lpass_pcm_data *pcm_data = drvdata->private_data; struct lpass_variant *v = drvdata->variant; snd_pcm_format_t format = params_format(params); unsigned int channels = params_channels(params); @@ -177,9 +177,9 @@ static int lpass_platform_pcmops_hw_params(struct snd_pcm_substream *substream, static int lpass_platform_pcmops_hw_free(struct snd_pcm_substream *substream) { struct snd_soc_pcm_runtime *soc_runtime = substream->private_data; - struct lpass_pcm_data *pcm_data = snd_soc_pcm_get_drvdata(soc_runtime); struct lpass_data *drvdata = snd_soc_platform_get_drvdata(soc_runtime->platform); + struct lpass_pcm_data *pcm_data = drvdata->private_data; struct lpass_variant *v = drvdata->variant; unsigned int reg; int ret; @@ -201,9 +201,9 @@ static int lpass_platform_pcmops_prepare(struct snd_pcm_substream *substream) { struct snd_pcm_runtime *runtime = substream->runtime; struct snd_soc_pcm_runtime *soc_runtime = substream->private_data; - struct lpass_pcm_data *pcm_data = snd_soc_pcm_get_drvdata(soc_runtime); struct lpass_data *drvdata = snd_soc_platform_get_drvdata(soc_runtime->platform); + struct lpass_pcm_data *pcm_data = drvdata->private_data; struct lpass_variant *v = drvdata->variant; int ret, ch, dir = substream->stream;
@@ -255,9 +255,9 @@ static int lpass_platform_pcmops_trigger(struct snd_pcm_substream *substream, int cmd) { struct snd_soc_pcm_runtime *soc_runtime = substream->private_data; - struct lpass_pcm_data *pcm_data = snd_soc_pcm_get_drvdata(soc_runtime); struct lpass_data *drvdata = snd_soc_platform_get_drvdata(soc_runtime->platform); + struct lpass_pcm_data *pcm_data = drvdata->private_data; struct lpass_variant *v = drvdata->variant; int ret, ch, dir = substream->stream;
@@ -331,9 +331,9 @@ static snd_pcm_uframes_t lpass_platform_pcmops_pointer( struct snd_pcm_substream *substream) { struct snd_soc_pcm_runtime *soc_runtime = substream->private_data; - struct lpass_pcm_data *pcm_data = snd_soc_pcm_get_drvdata(soc_runtime); struct lpass_data *drvdata = snd_soc_platform_get_drvdata(soc_runtime->platform); + struct lpass_pcm_data *pcm_data = drvdata->private_data; struct lpass_variant *v = drvdata->variant; unsigned int base_addr, curr_addr; int ret, ch, dir = substream->stream; @@ -483,7 +483,7 @@ static int lpass_platform_pcm_new(struct snd_soc_pcm_runtime *soc_runtime) return -ENOMEM;
data->i2s_port = cpu_dai->driver->id; - snd_soc_pcm_set_drvdata(soc_runtime, data); + drvdata->private_data = data;
psubstream = pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream; if (psubstream) { @@ -570,8 +570,8 @@ static void lpass_platform_pcm_free(struct snd_pcm *pcm) substream = pcm->streams[i].substream; if (substream) { rt = substream->private_data; - data = snd_soc_pcm_get_drvdata(rt); drvdata = snd_soc_platform_get_drvdata(rt->platform); + data = drvdata->private_data;
ch = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) ? data->rdma_ch diff --git a/sound/soc/qcom/lpass.h b/sound/soc/qcom/lpass.h index 30714ad..35b3cea 100644 --- a/sound/soc/qcom/lpass.h +++ b/sound/soc/qcom/lpass.h @@ -58,6 +58,8 @@ struct lpass_data { /* 8016 specific */ struct clk *pcnoc_mport_clk; struct clk *pcnoc_sway_clk; + + void *private_data; };
/* Vairant data per each SOC */
The patch
ASoC: lpass-platform: don't use snd_soc_pcm_set_drvdata()
has been applied to the asoc tree at
git://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 6adcbdcd4b6e8301c3a4c61284e701f87de9a409 Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Date: Mon, 1 Aug 2016 06:10:54 +0000 Subject: [PATCH] ASoC: lpass-platform: don't use snd_soc_pcm_set_drvdata()
snd_soc_pcm_set_drvdata() will set driver data to rtd->dev, but driver data of rtd->dev is already used as "rtd" on soc_post_component_init().
static int soc_post_component_init(xxx) { ... dev_set_drvdata(rtd->dev, rtd); ... }
To remove confusion, snd_soc_pcm_set/get_drvdata() should be removed soon. This patch is for it.
qcom/lpass-platform.c sets struct lpass_pcm_data to rtd->dev today, but we can replace it to struct lpass_data :: private_data.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/qcom/lpass-platform.c | 14 +++++++------- sound/soc/qcom/lpass.h | 2 ++ 2 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c index db000c6987a1..a144c14c7169 100644 --- a/sound/soc/qcom/lpass-platform.c +++ b/sound/soc/qcom/lpass-platform.c @@ -84,9 +84,9 @@ 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_pcm_data *pcm_data = snd_soc_pcm_get_drvdata(soc_runtime); struct lpass_data *drvdata = snd_soc_platform_get_drvdata(soc_runtime->platform); + struct lpass_pcm_data *pcm_data = drvdata->private_data; struct lpass_variant *v = drvdata->variant; snd_pcm_format_t format = params_format(params); unsigned int channels = params_channels(params); @@ -177,9 +177,9 @@ static int lpass_platform_pcmops_hw_params(struct snd_pcm_substream *substream, static int lpass_platform_pcmops_hw_free(struct snd_pcm_substream *substream) { struct snd_soc_pcm_runtime *soc_runtime = substream->private_data; - struct lpass_pcm_data *pcm_data = snd_soc_pcm_get_drvdata(soc_runtime); struct lpass_data *drvdata = snd_soc_platform_get_drvdata(soc_runtime->platform); + struct lpass_pcm_data *pcm_data = drvdata->private_data; struct lpass_variant *v = drvdata->variant; unsigned int reg; int ret; @@ -201,9 +201,9 @@ static int lpass_platform_pcmops_prepare(struct snd_pcm_substream *substream) { struct snd_pcm_runtime *runtime = substream->runtime; struct snd_soc_pcm_runtime *soc_runtime = substream->private_data; - struct lpass_pcm_data *pcm_data = snd_soc_pcm_get_drvdata(soc_runtime); struct lpass_data *drvdata = snd_soc_platform_get_drvdata(soc_runtime->platform); + struct lpass_pcm_data *pcm_data = drvdata->private_data; struct lpass_variant *v = drvdata->variant; int ret, ch, dir = substream->stream;
@@ -255,9 +255,9 @@ static int lpass_platform_pcmops_trigger(struct snd_pcm_substream *substream, int cmd) { struct snd_soc_pcm_runtime *soc_runtime = substream->private_data; - struct lpass_pcm_data *pcm_data = snd_soc_pcm_get_drvdata(soc_runtime); struct lpass_data *drvdata = snd_soc_platform_get_drvdata(soc_runtime->platform); + struct lpass_pcm_data *pcm_data = drvdata->private_data; struct lpass_variant *v = drvdata->variant; int ret, ch, dir = substream->stream;
@@ -331,9 +331,9 @@ static snd_pcm_uframes_t lpass_platform_pcmops_pointer( struct snd_pcm_substream *substream) { struct snd_soc_pcm_runtime *soc_runtime = substream->private_data; - struct lpass_pcm_data *pcm_data = snd_soc_pcm_get_drvdata(soc_runtime); struct lpass_data *drvdata = snd_soc_platform_get_drvdata(soc_runtime->platform); + struct lpass_pcm_data *pcm_data = drvdata->private_data; struct lpass_variant *v = drvdata->variant; unsigned int base_addr, curr_addr; int ret, ch, dir = substream->stream; @@ -483,7 +483,7 @@ static int lpass_platform_pcm_new(struct snd_soc_pcm_runtime *soc_runtime) return -ENOMEM;
data->i2s_port = cpu_dai->driver->id; - snd_soc_pcm_set_drvdata(soc_runtime, data); + drvdata->private_data = data;
psubstream = pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream; if (psubstream) { @@ -570,8 +570,8 @@ static void lpass_platform_pcm_free(struct snd_pcm *pcm) substream = pcm->streams[i].substream; if (substream) { rt = substream->private_data; - data = snd_soc_pcm_get_drvdata(rt); drvdata = snd_soc_platform_get_drvdata(rt->platform); + data = drvdata->private_data;
ch = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) ? data->rdma_ch diff --git a/sound/soc/qcom/lpass.h b/sound/soc/qcom/lpass.h index 30714ad1e138..35b3cea8207d 100644 --- a/sound/soc/qcom/lpass.h +++ b/sound/soc/qcom/lpass.h @@ -58,6 +58,8 @@ struct lpass_data { /* 8016 specific */ struct clk *pcnoc_mport_clk; struct clk *pcnoc_sway_clk; + + void *private_data; };
/* Vairant data per each SOC */
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
snd_soc_pcm_set_drvdata() will set driver data to rtd->dev, but driver data of rtd->dev is already used as "rtd" on soc_post_component_init().
static int soc_post_component_init(xxx) { ... dev_set_drvdata(rtd->dev, rtd); ... }
To remove confusion, this patch removes snd_soc_pcm_set/get_drvdata().
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- include/sound/soc.h | 11 ----------- 1 file changed, 11 deletions(-)
diff --git a/include/sound/soc.h b/include/sound/soc.h index 7ff1335..8a60f44 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -1547,17 +1547,6 @@ static inline void *snd_soc_platform_get_drvdata(struct snd_soc_platform *platfo return snd_soc_component_get_drvdata(&platform->component); }
-static inline void snd_soc_pcm_set_drvdata(struct snd_soc_pcm_runtime *rtd, - void *data) -{ - dev_set_drvdata(rtd->dev, data); -} - -static inline void *snd_soc_pcm_get_drvdata(struct snd_soc_pcm_runtime *rtd) -{ - return dev_get_drvdata(rtd->dev); -} - static inline void snd_soc_initialize_card_lists(struct snd_soc_card *card) { INIT_LIST_HEAD(&card->codec_dev_list);
The patch
ASoC: remove snd_soc_pcm_set/get_drvdata()
has been applied to the asoc tree at
git://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 1e814030954015e42621191f3adc52df2241dc08 Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Date: Mon, 1 Aug 2016 06:11:36 +0000 Subject: [PATCH] ASoC: remove snd_soc_pcm_set/get_drvdata()
snd_soc_pcm_set_drvdata() will set driver data to rtd->dev, but driver data of rtd->dev is already used as "rtd" on soc_post_component_init().
static int soc_post_component_init(xxx) { ... dev_set_drvdata(rtd->dev, rtd); ... }
To remove confusion, this patch removes snd_soc_pcm_set/get_drvdata().
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown broonie@kernel.org --- include/sound/soc.h | 11 ----------- 1 file changed, 11 deletions(-)
diff --git a/include/sound/soc.h b/include/sound/soc.h index 6144882cc96a..bc953994acb9 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -1547,17 +1547,6 @@ static inline void *snd_soc_platform_get_drvdata(struct snd_soc_platform *platfo return snd_soc_component_get_drvdata(&platform->component); }
-static inline void snd_soc_pcm_set_drvdata(struct snd_soc_pcm_runtime *rtd, - void *data) -{ - dev_set_drvdata(rtd->dev, data); -} - -static inline void *snd_soc_pcm_get_drvdata(struct snd_soc_pcm_runtime *rtd) -{ - return dev_get_drvdata(rtd->dev); -} - static inline void snd_soc_initialize_card_lists(struct snd_soc_card *card) { INIT_LIST_HEAD(&card->codec_dev_list);
participants (2)
-
Kuninori Morimoto -
Mark Brown