[PATCH 0/3] ASoC: remove snd_soc_pcm_set/get_drvdata()

[PATCH 0/3] ASoC: remove snd_soc_pcm_set/get_drvdata()

[Kuninori Morimoto]

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(-)

[PATCH 1/3] ASoC: sst-haswell-pcm: don't use snd_soc_pcm_set_drvdata()

[Kuninori Morimoto]

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);
-- 
1.9.1

Re: [PATCH 1/3] ASoC: sst-haswell-pcm: don't use snd_soc_pcm_set_drvdata()

[Mark Brown]

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

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.

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

Re: [PATCH 1/3] ASoC: sst-haswell-pcm: don't use snd_soc_pcm_set_drvdata()

[Kuninori Morimoto]

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>

[PATCH 2/3] ASoC: lpass-platform: don't use snd_soc_pcm_set_drvdata()

[Kuninori Morimoto]

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 */
-- 
1.9.1

Applied "ASoC: lpass-platform: don't use snd_soc_pcm_set_drvdata()" to the asoc tree

[Mark Brown]

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 */
-- 
2.8.1

[PATCH 3/3] ASoC: remove snd_soc_pcm_set/get_drvdata()

[Kuninori Morimoto]

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);
-- 
1.9.1

Applied "ASoC: remove snd_soc_pcm_set/get_drvdata()" to the asoc tree

[Mark Brown]

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);
-- 
2.8.1