[PATCH 0/4] ASoC: sst-haswell-pcm: Move controls and DAPM elements to component

[PATCH 0/4] ASoC: sst-haswell-pcm: Move controls and DAPM elements to component

[Lars-Peter Clausen]

The sst-haswell-pcm driver is the only one that registers controls and DAPM
elements for a snd_soc_platform_driver. Moving them over to the component that
is registered by the same driver make it possible to remove the
steal_silbling_dai_hack in the core as well as the extra indirection that is
used to register the controls and DAPM elements.

Vinod, Subhransu, can you take a look at this and make sure it works as expected
and also keep this in mind for the new DPCM drivers?

Thanks,
- Lars

Lars-Peter Clausen (4):
  ASoC: Add snd_soc_component_{get,set}_drvdata()
  ASoC: sst-haswell-pcm: Alloc state struct in driver probe()
  ASoC: sst-haswell-pcm: Move controls and DAPM elements to component
  ASoC: Remove table based DAPM/control setup support from
    snd_soc_platform_driver

 include/sound/soc.h               | 28 ++++++++++---------
 sound/soc/intel/sst-haswell-pcm.c | 56 ++++++++++++++++++-------------------
 sound/soc/soc-core.c              | 58 ++++-----------------------------------
 3 files changed, 49 insertions(+), 93 deletions(-)

-- 
1.8.0

[PATCH 1/4] ASoC: Add snd_soc_component_{get, set}_drvdata()

[Lars-Peter Clausen]

Add Add snd_soc_component_{get,set}_drvdata() similar to
snd_soc_codec_{get,set}_drvdata() and snd_soc_platform_{get,set}_drvdata().
Also update them to use the new functions internally.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 include/sound/soc.h | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index e43fbb6..e6440d8 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -1299,26 +1299,37 @@ static inline void *snd_soc_card_get_drvdata(struct snd_soc_card *card)
 	return card->drvdata;
 }
 
+static inline void snd_soc_component_set_drvdata(struct snd_soc_component *c,
+		void *data)
+{
+	dev_set_drvdata(c->dev, data);
+}
+
+static inline void *snd_soc_component_get_drvdata(struct snd_soc_component *c)
+{
+	return dev_get_drvdata(c->dev);
+}
+
 static inline void snd_soc_codec_set_drvdata(struct snd_soc_codec *codec,
 		void *data)
 {
-	dev_set_drvdata(codec->dev, data);
+	snd_soc_component_set_drvdata(&codec->component, data);
 }
 
 static inline void *snd_soc_codec_get_drvdata(struct snd_soc_codec *codec)
 {
-	return dev_get_drvdata(codec->dev);
+	return snd_soc_component_get_drvdata(&codec->component);
 }
 
 static inline void snd_soc_platform_set_drvdata(struct snd_soc_platform *platform,
 		void *data)
 {
-	dev_set_drvdata(platform->dev, data);
+	snd_soc_component_set_drvdata(&platform->component, data);
 }
 
 static inline void *snd_soc_platform_get_drvdata(struct snd_soc_platform *platform)
 {
-	return dev_get_drvdata(platform->dev);
+	return snd_soc_component_get_drvdata(&platform->component);
 }
 
 static inline void snd_soc_pcm_set_drvdata(struct snd_soc_pcm_runtime *rtd,
-- 
1.8.0

Re: [PATCH 1/4] ASoC: Add snd_soc_component_{get, set}_drvdata()

[Vinod Koul]

On Wed, Aug 20, 2014 at 01:08:46PM +0200, Lars-Peter Clausen wrote:
> Add Add snd_soc_component_{get,set}_drvdata() similar to
> snd_soc_codec_{get,set}_drvdata() and snd_soc_platform_{get,set}_drvdata().
> Also update them to use the new functions internally.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Reviewed-by: Vinod Koul <vinod.koul@intel.com>

-- 
~Vinod

> ---
>  include/sound/soc.h | 19 +++++++++++++++----
>  1 file changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/include/sound/soc.h b/include/sound/soc.h
> index e43fbb6..e6440d8 100644
> --- a/include/sound/soc.h
> +++ b/include/sound/soc.h
> @@ -1299,26 +1299,37 @@ static inline void *snd_soc_card_get_drvdata(struct snd_soc_card *card)
>  	return card->drvdata;
>  }
>  
> +static inline void snd_soc_component_set_drvdata(struct snd_soc_component *c,
> +		void *data)
> +{
> +	dev_set_drvdata(c->dev, data);
> +}
> +
> +static inline void *snd_soc_component_get_drvdata(struct snd_soc_component *c)
> +{
> +	return dev_get_drvdata(c->dev);
> +}
> +
>  static inline void snd_soc_codec_set_drvdata(struct snd_soc_codec *codec,
>  		void *data)
>  {
> -	dev_set_drvdata(codec->dev, data);
> +	snd_soc_component_set_drvdata(&codec->component, data);
>  }
>  
>  static inline void *snd_soc_codec_get_drvdata(struct snd_soc_codec *codec)
>  {
> -	return dev_get_drvdata(codec->dev);
> +	return snd_soc_component_get_drvdata(&codec->component);
>  }
>  
>  static inline void snd_soc_platform_set_drvdata(struct snd_soc_platform *platform,
>  		void *data)
>  {
> -	dev_set_drvdata(platform->dev, data);
> +	snd_soc_component_set_drvdata(&platform->component, data);
>  }
>  
>  static inline void *snd_soc_platform_get_drvdata(struct snd_soc_platform *platform)
>  {
> -	return dev_get_drvdata(platform->dev);
> +	return snd_soc_component_get_drvdata(&platform->component);
>  }
>  
>  static inline void snd_soc_pcm_set_drvdata(struct snd_soc_pcm_runtime *rtd,
> -- 
> 1.8.0
> 

-- 

Re: [PATCH 1/4] ASoC: Add snd_soc_component_{get, set}_drvdata()

[Lars-Peter Clausen]

On 09/03/2014 02:22 PM, Vinod Koul wrote:
> On Wed, Aug 20, 2014 at 01:08:46PM +0200, Lars-Peter Clausen wrote:
>> Add Add snd_soc_component_{get,set}_drvdata() similar to
>> snd_soc_codec_{get,set}_drvdata() and snd_soc_platform_{get,set}_drvdata().
>> Also update them to use the new functions internally.
>>
>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> Reviewed-by: Vinod Koul <vinod.koul@intel.com>

Is this for the first patch or for the whole series?

Thanks,
- Lars

Re: [PATCH 1/4] ASoC: Add snd_soc_component_{get, set}_drvdata()

[Vinod Koul]

On Wed, Sep 03, 2014 at 09:06:49PM +0200, Lars-Peter Clausen wrote:
> On 09/03/2014 02:22 PM, Vinod Koul wrote:
> >On Wed, Aug 20, 2014 at 01:08:46PM +0200, Lars-Peter Clausen wrote:
> >>Add Add snd_soc_component_{get,set}_drvdata() similar to
> >>snd_soc_codec_{get,set}_drvdata() and snd_soc_platform_{get,set}_drvdata().
> >>Also update them to use the new functions internally.
> >>
> >>Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> >Reviewed-by: Vinod Koul <vinod.koul@intel.com>
> 
> Is this for the first patch or for the whole series?
Patch only. The HSW driver changes, I think Liam should ack

-- 
~Vinod

[PATCH 2/4] ASoC: sst-haswell-pcm: Alloc state struct in driver probe()

[Lars-Peter Clausen]

Resource allocations should happen in driver probe callback rather than in
snd_soc_platform probe functions. Especially if the resource is device
managed. The snd_soc_* probe/remove functions are mainly intended to be used
for things that require the component to be already bound to a card.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 sound/soc/intel/sst-haswell-pcm.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/sound/soc/intel/sst-haswell-pcm.c b/sound/soc/intel/sst-haswell-pcm.c
index 61bf6da..1de0958 100644
--- a/sound/soc/intel/sst-haswell-pcm.c
+++ b/sound/soc/intel/sst-haswell-pcm.c
@@ -778,20 +778,11 @@ static const struct snd_soc_dapm_route graph[] = {
 
 static int hsw_pcm_probe(struct snd_soc_platform *platform)
 {
+	struct hsw_priv_data *priv_data = snd_soc_platform_get_drvdata(platform);
 	struct sst_pdata *pdata = dev_get_platdata(platform->dev);
-	struct hsw_priv_data *priv_data;
-	struct device *dma_dev;
+	struct device *dma_dev = pdata->dma_dev;
 	int i, ret = 0;
 
-	if (!pdata)
-		return -ENODEV;
-
-	dma_dev = pdata->dma_dev;
-
-	priv_data = devm_kzalloc(platform->dev, sizeof(*priv_data), GFP_KERNEL);
-	priv_data->hsw = pdata->dsp;
-	snd_soc_platform_set_drvdata(platform, priv_data);
-
 	/* allocate DSP buffer page tables */
 	for (i = 0; i < ARRAY_SIZE(hsw_dais); i++) {
 
@@ -863,12 +854,23 @@ static const struct snd_soc_component_driver hsw_dai_component = {
 static int hsw_pcm_dev_probe(struct platform_device *pdev)
 {
 	struct sst_pdata *sst_pdata = dev_get_platdata(&pdev->dev);
+	struct hsw_priv_data *priv_data;
 	int ret;
 
+	if (!sst_pdata)
+		return -EINVAL;
+
+	priv_data = devm_kzalloc(&pdev->dev, sizeof(*priv_data), GFP_KERNEL);
+	if (!priv_data)
+		return -ENOMEM;
+
 	ret = sst_hsw_dsp_init(&pdev->dev, sst_pdata);
 	if (ret < 0)
 		return -ENODEV;
 
+	priv_data->hsw = sst_pdata->dsp;
+	platform_set_drvdata(pdev, priv_data);
+
 	ret = snd_soc_register_platform(&pdev->dev, &hsw_soc_platform);
 	if (ret < 0)
 		goto err_plat;
-- 
1.8.0

[PATCH 3/4] ASoC: sst-haswell-pcm: Move controls and DAPM elements to component

[Lars-Peter Clausen]

The sst-haswell-pcm driver registers both a snd_soc_component and a
snd_soc_platform and expects that the DAPM widgets for the DAIs registered by
component are added to the DAPM context of the platform. This requires us to
have a hack in the ASoC core which does so. Moving the DAPM elements over to
the component allows us to remove this hack.

While we are at it also move the controls over to the component. The controls
don't need the platform for anything other than snd_soc_platform_get_drvdata(),
this can easily be replaced by snd_soc_component_get_drvdata(). As the long
term goal is to register only a single component this is a step in the right
direction.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 sound/soc/intel/sst-haswell-pcm.c | 32 +++++++++++++++-----------------
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/sound/soc/intel/sst-haswell-pcm.c b/sound/soc/intel/sst-haswell-pcm.c
index 1de0958..33fc5c3 100644
--- a/sound/soc/intel/sst-haswell-pcm.c
+++ b/sound/soc/intel/sst-haswell-pcm.c
@@ -138,11 +138,10 @@ static inline unsigned int hsw_ipc_to_mixer(u32 value)
 static int hsw_stream_volume_put(struct snd_kcontrol *kcontrol,
 				struct snd_ctl_elem_value *ucontrol)
 {
-	struct snd_soc_platform *platform = snd_soc_kcontrol_platform(kcontrol);
+	struct snd_soc_component *cmpnt = snd_soc_kcontrol_component(kcontrol);
+	struct hsw_priv_data *pdata = snd_soc_component_get_drvdata(cmpnt);
 	struct soc_mixer_control *mc =
 		(struct soc_mixer_control *)kcontrol->private_value;
-	struct hsw_priv_data *pdata =
-		snd_soc_platform_get_drvdata(platform);
 	struct hsw_pcm_data *pcm_data = &pdata->pcm[mc->reg];
 	struct sst_hsw *hsw = pdata->hsw;
 	u32 volume;
@@ -176,11 +175,10 @@ static int hsw_stream_volume_put(struct snd_kcontrol *kcontrol,
 static int hsw_stream_volume_get(struct snd_kcontrol *kcontrol,
 				struct snd_ctl_elem_value *ucontrol)
 {
-	struct snd_soc_platform *platform = snd_soc_kcontrol_platform(kcontrol);
+	struct snd_soc_component *cmpnt = snd_soc_kcontrol_component(kcontrol);
+	struct hsw_priv_data *pdata = snd_soc_component_get_drvdata(cmpnt);
 	struct soc_mixer_control *mc =
 		(struct soc_mixer_control *)kcontrol->private_value;
-	struct hsw_priv_data *pdata =
-		snd_soc_platform_get_drvdata(platform);
 	struct hsw_pcm_data *pcm_data = &pdata->pcm[mc->reg];
 	struct sst_hsw *hsw = pdata->hsw;
 	u32 volume;
@@ -208,8 +206,8 @@ static int hsw_stream_volume_get(struct snd_kcontrol *kcontrol,
 static int hsw_volume_put(struct snd_kcontrol *kcontrol,
 				struct snd_ctl_elem_value *ucontrol)
 {
-	struct snd_soc_platform *platform = snd_soc_kcontrol_platform(kcontrol);
-	struct hsw_priv_data *pdata = snd_soc_platform_get_drvdata(platform);
+	struct snd_soc_component *cmpnt = snd_soc_kcontrol_component(kcontrol);
+	struct hsw_priv_data *pdata = snd_soc_component_get_drvdata(cmpnt);
 	struct sst_hsw *hsw = pdata->hsw;
 	u32 volume;
 
@@ -233,8 +231,8 @@ static int hsw_volume_put(struct snd_kcontrol *kcontrol,
 static int hsw_volume_get(struct snd_kcontrol *kcontrol,
 				struct snd_ctl_elem_value *ucontrol)
 {
-	struct snd_soc_platform *platform = snd_soc_kcontrol_platform(kcontrol);
-	struct hsw_priv_data *pdata = snd_soc_platform_get_drvdata(platform);
+	struct snd_soc_component *cmpnt = snd_soc_kcontrol_component(kcontrol);
+	struct hsw_priv_data *pdata = snd_soc_component_get_drvdata(cmpnt);
 	struct sst_hsw *hsw = pdata->hsw;
 	unsigned int volume = 0;
 
@@ -839,16 +837,16 @@ static struct snd_soc_platform_driver hsw_soc_platform = {
 	.ops		= &hsw_pcm_ops,
 	.pcm_new	= hsw_pcm_new,
 	.pcm_free	= hsw_pcm_free,
-	.controls	= hsw_volume_controls,
-	.num_controls	= ARRAY_SIZE(hsw_volume_controls),
-	.dapm_widgets	= widgets,
-	.num_dapm_widgets	= ARRAY_SIZE(widgets),
-	.dapm_routes	= graph,
-	.num_dapm_routes	= ARRAY_SIZE(graph),
 };
 
 static const struct snd_soc_component_driver hsw_dai_component = {
-	.name		= "haswell-dai",
+	.name = "haswell-dai",
+	.controls = hsw_volume_controls,
+	.num_controls = ARRAY_SIZE(hsw_volume_controls),
+	.dapm_widgets = widgets,
+	.num_dapm_widgets = ARRAY_SIZE(widgets),
+	.dapm_routes = graph,
+	.num_dapm_routes = ARRAY_SIZE(graph),
 };
 
 static int hsw_pcm_dev_probe(struct platform_device *pdev)
-- 
1.8.0

[PATCH 4/4] ASoC: Remove table based DAPM/control setup support from snd_soc_platform_driver

[Lars-Peter Clausen]

There are no users left and new users should rather use the component_driver
struct embedded in the snd_soc_platform_driver struct to do this. E.g.:

static const struct snd_soc_platform_driver foobar_driver = {
	.component_driver = {
		.dapm_widgets = ...,
		.num_dapm_widgets = ...,
		...,
	},
	...
};

instead of

static const struct snd_soc_platform_driver foobar_driver = {
	.dapm_widgets = ...,
	.num_dapm_widgets = ...,
	...
};

This also allows us to remove the steal_sibling_dai_widgets hack.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 include/sound/soc.h  |  9 --------
 sound/soc/soc-core.c | 58 ++++++----------------------------------------------
 2 files changed, 6 insertions(+), 61 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index e6440d8..3f07a1e 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -764,7 +764,6 @@ struct snd_soc_component {
 	unsigned int num_dapm_widgets;
 	const struct snd_soc_dapm_route *dapm_routes;
 	unsigned int num_dapm_routes;
-	bool steal_sibling_dai_widgets;
 	struct snd_soc_codec *codec;
 
 	int (*probe)(struct snd_soc_component *);
@@ -869,14 +868,6 @@ struct snd_soc_platform_driver {
 	int (*pcm_new)(struct snd_soc_pcm_runtime *);
 	void (*pcm_free)(struct snd_pcm *);
 
-	/* Default control and setup, added after probe() is run */
-	const struct snd_kcontrol_new *controls;
-	int num_controls;
-	const struct snd_soc_dapm_widget *dapm_widgets;
-	int num_dapm_widgets;
-	const struct snd_soc_dapm_route *dapm_routes;
-	int num_dapm_routes;
-
 	/*
 	 * For platform caused delay reporting.
 	 * Optional.
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index e982cb0..c3a63a1 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1089,7 +1089,6 @@ static int soc_probe_component(struct snd_soc_card *card,
 	struct snd_soc_component *component)
 {
 	struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
-	struct snd_soc_component *dai_component, *component2;
 	struct snd_soc_dai *dai;
 	int ret;
 
@@ -1116,44 +1115,12 @@ static int soc_probe_component(struct snd_soc_card *card,
 		}
 	}
 
-	/*
-	 * This is rather ugly, but certain platforms expect that the DAPM
-	 * widgets for the DAIs for components with the same parent device are
-	 * created in the platforms DAPM context. Until that is fixed we need to
-	 * keep this.
-	 */
-	if (component->steal_sibling_dai_widgets) {
-		dai_component = NULL;
-		list_for_each_entry(component2, &component_list, list) {
-			if (component == component2)
-				continue;
-
-			if (component2->dev == component->dev &&
-			    !list_empty(&component2->dai_list)) {
-				dai_component = component2;
-				break;
-			}
-		}
-	} else {
-		dai_component = component;
-		list_for_each_entry(component2, &component_list, list) {
-			if (component2->dev == component->dev &&
-			    component2->steal_sibling_dai_widgets) {
-				dai_component = NULL;
-				break;
-			}
-		}
-	}
-
-	if (dai_component) {
-		list_for_each_entry(dai, &dai_component->dai_list, list) {
-			snd_soc_dapm_new_dai_widgets(dapm, dai);
-			if (ret != 0) {
-				dev_err(component->dev,
-					"Failed to create DAI widgets %d\n",
-					ret);
-				goto err_probe;
-			}
+	list_for_each_entry(dai, &component->dai_list, list) {
+		ret = snd_soc_dapm_new_dai_widgets(dapm, dai);
+		if (ret != 0) {
+			dev_err(component->dev,
+				"Failed to create DAI widgets %d\n", ret);
+			goto err_probe;
 		}
 	}
 
@@ -4165,19 +4132,6 @@ int snd_soc_add_platform(struct device *dev, struct snd_soc_platform *platform,
 
 	platform->dev = dev;
 	platform->driver = platform_drv;
-	if (platform_drv->controls) {
-		platform->component.controls = platform_drv->controls;
-		platform->component.num_controls = platform_drv->num_controls;
-	}
-	if (platform_drv->dapm_widgets) {
-		platform->component.dapm_widgets = platform_drv->dapm_widgets;
-		platform->component.num_dapm_widgets = platform_drv->num_dapm_widgets;
-		platform->component.steal_sibling_dai_widgets = true;
-	}
-	if (platform_drv->dapm_routes) {
-		platform->component.dapm_routes = platform_drv->dapm_routes;
-		platform->component.num_dapm_routes = platform_drv->num_dapm_routes;
-	}
 
 	if (platform_drv->probe)
 		platform->component.probe = snd_soc_platform_drv_probe;
-- 
1.8.0

Re: [PATCH 0/4] ASoC: sst-haswell-pcm: Move controls and DAPM elements to component

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 445 bytes --]

On Wed, Aug 20, 2014 at 01:08:45PM +0200, Lars-Peter Clausen wrote:
> The sst-haswell-pcm driver is the only one that registers controls and DAPM
> elements for a snd_soc_platform_driver. Moving them over to the component that
> is registered by the same driver make it possible to remove the
> steal_silbling_dai_hack in the core as well as the extra indirection that is
> used to register the controls and DAPM elements.

Applied all, thanks.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]