[PATCH 0/7] ASoC: use snd_soc_dapm_to_component()

[PATCH 0/7] ASoC: use snd_soc_dapm_to_component()

[Kuninori Morimoto]

Hi Mark

Current snd_soc_dapm_to_component() is using container_of(),
thus, caution is required to use it not to get strange pointer.

But, dapm has component pointer (= dapm->component), and we can simply use
it. If we use dapm->component instead of container_of(), above caution
is no longer needed.

Let's use dapm->component in snd_soc_dapm_to_component(), and use it.

Kuninori Morimoto (7):
  ASoC: soc-component: use dapm->component instead of container_of()
  ASoC: amd: use snd_soc_dapm_to_component()
  ASoC: sma1307: use snd_soc_dapm_to_component()
  ASoC: intel: use snd_soc_dapm_to_component()
  ASoC: mediatek: use snd_soc_dapm_to_component()
  ASoC: soc-dapm: use snd_soc_dapm_to_component()
  ASoC: soc-topology: use snd_soc_dapm_to_component()

 include/sound/soc-component.h             |  6 +--
 sound/soc/amd/acp/acp-mach-common.c       |  2 +-
 sound/soc/codecs/sma1307.c                | 32 ++++++-------
 sound/soc/intel/avs/control.c             |  2 +-
 sound/soc/mediatek/mt8195/mt8195-mt6359.c |  2 +-
 sound/soc/soc-dapm.c                      | 58 +++++++++++++++--------
 sound/soc/soc-topology.c                  |  3 +-
 7 files changed, 60 insertions(+), 45 deletions(-)

-- 
2.43.0

[PATCH 1/7] ASoC: soc-component: use dapm->component instead of container_of()

[Kuninori Morimoto]

Now, snd_soc_dapm_to_component() (A) will convert dapm to component by
container_of() (a).

(A)	static inline struct snd_soc_component *snd_soc_dapm_to_component(
		struct snd_soc_dapm_context *dapm)
	{
(a)		return container_of(dapm, struct snd_soc_component, dapm);
	}

Because (a) uses container_of(), dapm of component works, but
dapm of others will be "unknown" pointer (= not NULL).

OTOH, we will call snd_soc_dapm_init() (X) to initialize dapm, and
it will be called from snd_soc_bind_card() (p) or
soc_probe_component() (q) with pointer (component/NULL).

(p)	static int snd_soc_bind_card(...)
	{
		...
(X)		snd_soc_dapm_init(..., NULL);
		...                    ^^^^
	}

(q)	static int soc_probe_component(...)
	{
		...
(X)		snd_soc_dapm_init(, component);
		...                 ^^^^^^^^^
	}

And snd_soc_dapm_init() (X) will fill dapm->component (x)

(X)	void snd_soc_dapm_init(..., component, ...)
	{
		...
(x)		dapm->component	= component;
		...
	}

We can simply use dapm->component in snd_soc_dapm_to_component() (A).
In this case, dapm of others (p) will be just NULL.

Use dapm->component instead of container_of().
The picky note can be removed by this patch.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Cc: Benjamin Bara <benjamin.bara@skidata.com>
---
 include/sound/soc-component.h | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/include/sound/soc-component.h b/include/sound/soc-component.h
index 61534ac0edd1..63928773df32 100644
--- a/include/sound/soc-component.h
+++ b/include/sound/soc-component.h
@@ -265,15 +265,11 @@ struct snd_soc_component {
  * snd_soc_dapm_to_component() - Casts a DAPM context to the component it is
  *  embedded in
  * @dapm: The DAPM context to cast to the component
- *
- * This function must only be used on DAPM contexts that are known to be part of
- * a component (e.g. in a component driver). Otherwise the behavior is
- * undefined.
  */
 static inline struct snd_soc_component *snd_soc_dapm_to_component(
 	struct snd_soc_dapm_context *dapm)
 {
-	return container_of(dapm, struct snd_soc_component, dapm);
+	return dapm->component;
 }
 
 /**
-- 
2.43.0

[PATCH 2/7] ASoC: amd: use snd_soc_dapm_to_component()

[Kuninori Morimoto]

We can use snd_soc_dapm_to_component(). Let's use it

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/amd/acp/acp-mach-common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/amd/acp/acp-mach-common.c b/sound/soc/amd/acp/acp-mach-common.c
index a0dab85088ec..b68ee80896ec 100644
--- a/sound/soc/amd/acp/acp-mach-common.c
+++ b/sound/soc/amd/acp/acp-mach-common.c
@@ -1340,7 +1340,7 @@ static int acp_rtk_set_bias_level(struct snd_soc_card *card,
 				  struct snd_soc_dapm_context *dapm,
 				  enum snd_soc_bias_level level)
 {
-	struct snd_soc_component *component = dapm->component;
+	struct snd_soc_component *component = snd_soc_dapm_to_component(dapm);
 	struct acp_card_drvdata *drvdata = card->drvdata;
 	int ret = 0;
 
-- 
2.43.0

[PATCH 3/7] ASoC: sma1307: use snd_soc_dapm_to_component()

[Kuninori Morimoto]

We can use snd_soc_dapm_to_component(). Let's use it

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/codecs/sma1307.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/sound/soc/codecs/sma1307.c b/sound/soc/codecs/sma1307.c
index 498189ab691c..043bdba47e4d 100644
--- a/sound/soc/codecs/sma1307.c
+++ b/sound/soc/codecs/sma1307.c
@@ -805,8 +805,8 @@ static int sma1307_dapm_aif_in_get(struct snd_kcontrol *kcontrol,
 {
 	struct snd_soc_dapm_context *dapm =
 	    snd_soc_dapm_kcontrol_dapm(kcontrol);
-	struct sma1307_priv *sma1307 =
-	    snd_soc_component_get_drvdata(dapm->component);
+	struct snd_soc_component *component = snd_soc_dapm_to_component(dapm);
+	struct sma1307_priv *sma1307 = snd_soc_component_get_drvdata(component);
 
 	ucontrol->value.enumerated.item[0] = (unsigned int)sma1307->dapm_aif_in;
 	snd_soc_dapm_put_enum_double(kcontrol, ucontrol);
@@ -819,8 +819,8 @@ static int sma1307_dapm_aif_in_put(struct snd_kcontrol *kcontrol,
 {
 	struct snd_soc_dapm_context *dapm =
 	    snd_soc_dapm_kcontrol_dapm(kcontrol);
-	struct sma1307_priv *sma1307 =
-	    snd_soc_component_get_drvdata(dapm->component);
+	struct snd_soc_component *component = snd_soc_dapm_to_component(dapm);
+	struct sma1307_priv *sma1307 = snd_soc_component_get_drvdata(component);
 	int val = (int)ucontrol->value.enumerated.item[0];
 	bool change;
 
@@ -845,8 +845,8 @@ static int sma1307_dapm_sdo_setting_get(struct snd_kcontrol *kcontrol,
 {
 	struct snd_soc_dapm_context *dapm =
 	    snd_soc_dapm_kcontrol_dapm(kcontrol);
-	struct sma1307_priv *sma1307 =
-	    snd_soc_component_get_drvdata(dapm->component);
+	struct snd_soc_component *component = snd_soc_dapm_to_component(dapm);
+	struct sma1307_priv *sma1307 = snd_soc_component_get_drvdata(component);
 
 	ucontrol->value.enumerated.item[0] =
 	    (unsigned int)sma1307->dapm_sdo_setting;
@@ -860,8 +860,8 @@ static int sma1307_dapm_sdo_setting_put(struct snd_kcontrol *kcontrol,
 {
 	struct snd_soc_dapm_context *dapm =
 	    snd_soc_dapm_kcontrol_dapm(kcontrol);
-	struct sma1307_priv *sma1307 =
-	    snd_soc_component_get_drvdata(dapm->component);
+	struct snd_soc_component *component = snd_soc_dapm_to_component(dapm);
+	struct sma1307_priv *sma1307 = snd_soc_component_get_drvdata(component);
 	int val = (int)ucontrol->value.enumerated.item[0];
 	bool change;
 
@@ -886,8 +886,8 @@ static int sma1307_dapm_aif_out_get(struct snd_kcontrol *kcontrol,
 {
 	struct snd_soc_dapm_context *dapm =
 	    snd_soc_dapm_kcontrol_dapm(kcontrol);
-	struct sma1307_priv *sma1307 =
-	    snd_soc_component_get_drvdata(dapm->component);
+	struct snd_soc_component *component = snd_soc_dapm_to_component(dapm);
+	struct sma1307_priv *sma1307 = snd_soc_component_get_drvdata(component);
 	unsigned int val = 0;
 
 	if (!strcmp(kcontrol->id.name, SMA1307_AIF_OUT0_NAME)) {
@@ -910,8 +910,8 @@ static int sma1307_dapm_aif_out_put(struct snd_kcontrol *kcontrol,
 {
 	struct snd_soc_dapm_context *dapm =
 	    snd_soc_dapm_kcontrol_dapm(kcontrol);
-	struct sma1307_priv *sma1307 =
-	    snd_soc_component_get_drvdata(dapm->component);
+	struct snd_soc_component *component = snd_soc_dapm_to_component(dapm);
+	struct sma1307_priv *sma1307 = snd_soc_component_get_drvdata(component);
 	int val = (int)ucontrol->value.enumerated.item[0];
 	bool change;
 
@@ -948,8 +948,8 @@ static int sma1307_dapm_sdo_enable_get(struct snd_kcontrol *kcontrol,
 {
 	struct snd_soc_dapm_context *dapm =
 	    snd_soc_dapm_kcontrol_dapm(kcontrol);
-	struct sma1307_priv *sma1307 =
-	    snd_soc_component_get_drvdata(dapm->component);
+	struct snd_soc_component *component = snd_soc_dapm_to_component(dapm);
+	struct sma1307_priv *sma1307 = snd_soc_component_get_drvdata(component);
 
 	ucontrol->value.integer.value[0] = (long)sma1307->dapm_sdo_en;
 	snd_soc_dapm_put_volsw(kcontrol, ucontrol);
@@ -962,8 +962,8 @@ static int sma1307_dapm_sdo_enable_put(struct snd_kcontrol *kcontrol,
 {
 	struct snd_soc_dapm_context *dapm =
 	    snd_soc_dapm_kcontrol_dapm(kcontrol);
-	struct sma1307_priv *sma1307 =
-	    snd_soc_component_get_drvdata(dapm->component);
+	struct snd_soc_component *component = snd_soc_dapm_to_component(dapm);
+	struct sma1307_priv *sma1307 = snd_soc_component_get_drvdata(component);
 	int val = (int)ucontrol->value.integer.value[0];
 	bool change;
 
-- 
2.43.0

[PATCH 4/7] ASoC: intel: use snd_soc_dapm_to_component()

[Kuninori Morimoto]

We can use snd_soc_dapm_to_component(). Let's use it

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/intel/avs/control.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/intel/avs/control.c b/sound/soc/intel/avs/control.c
index 2e01dc75a15a..1af270ef0c8b 100644
--- a/sound/soc/intel/avs/control.c
+++ b/sound/soc/intel/avs/control.c
@@ -19,7 +19,7 @@ static struct avs_dev *avs_get_kcontrol_adev(struct snd_kcontrol *kcontrol)
 
 	w = snd_soc_dapm_kcontrol_widget(kcontrol);
 
-	return to_avs_dev(w->dapm->component->dev);
+	return to_avs_dev(snd_soc_dapm_to_component(w->dapm)->dev);
 }
 
 static struct avs_path_module *avs_get_volume_module(struct avs_dev *adev, u32 id)
-- 
2.43.0

[PATCH 5/7] ASoC: mediatek: use snd_soc_dapm_to_component()

[Kuninori Morimoto]

We can use snd_soc_dapm_to_component(). Let's use it

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/mediatek/mt8195/mt8195-mt6359.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/mediatek/mt8195/mt8195-mt6359.c b/sound/soc/mediatek/mt8195/mt8195-mt6359.c
index df29a9fa5aee..d42b182f4994 100644
--- a/sound/soc/mediatek/mt8195/mt8195-mt6359.c
+++ b/sound/soc/mediatek/mt8195/mt8195-mt6359.c
@@ -646,7 +646,7 @@ static int mt8195_etdm_hw_params_fixup(struct snd_soc_pcm_runtime *rtd,
 static int mt8195_set_bias_level_post(struct snd_soc_card *card,
 	struct snd_soc_dapm_context *dapm, enum snd_soc_bias_level level)
 {
-	struct snd_soc_component *component = dapm->component;
+	struct snd_soc_component *component = snd_soc_dapm_to_component(dapm);
 	struct mtk_soc_card_data *soc_card_data = snd_soc_card_get_drvdata(card);
 	struct mt8195_mt6359_priv *priv = soc_card_data->mach_priv;
 	int ret;
-- 
2.43.0

[PATCH 6/7] ASoC: soc-dapm: use snd_soc_dapm_to_component()

[Kuninori Morimoto]

We can use snd_soc_dapm_to_component(). Let's use it

We have been used snd_soc_dapm_to_component() before commit 3fe9f5882cf7
("ASoC: dapm: avoid container_of() to get component"). But was updated
to directly use dapm->component, because Tegra got issue from it because
their dapm was from card. Now, we can use snd_soc_dapm_to_component().

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Cc: Benjamin Bara <benjamin.bara@skidata.com>
---
 sound/soc/soc-dapm.c | 58 +++++++++++++++++++++++++++++---------------
 1 file changed, 38 insertions(+), 20 deletions(-)

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index b7818388984e..7c84cc86f1e4 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -621,39 +621,52 @@ static void dapm_reset(struct snd_soc_card *card)
 
 static const char *soc_dapm_prefix(struct snd_soc_dapm_context *dapm)
 {
-	if (!dapm->component)
+	struct snd_soc_component *component = snd_soc_dapm_to_component(dapm);
+
+	if (!component)
 		return NULL;
-	return dapm->component->name_prefix;
+
+	return component->name_prefix;
 }
 
 static unsigned int soc_dapm_read(struct snd_soc_dapm_context *dapm, int reg)
 {
-	if (!dapm->component)
+	struct snd_soc_component *component = snd_soc_dapm_to_component(dapm);
+
+	if (!component)
 		return -EIO;
-	return  snd_soc_component_read(dapm->component, reg);
+
+	return  snd_soc_component_read(component, reg);
 }
 
 static int soc_dapm_update_bits(struct snd_soc_dapm_context *dapm,
 	int reg, unsigned int mask, unsigned int value)
 {
-	if (!dapm->component)
+	struct snd_soc_component *component = snd_soc_dapm_to_component(dapm);
+
+	if (!component)
 		return -EIO;
-	return snd_soc_component_update_bits(dapm->component, reg,
-					     mask, value);
+
+	return snd_soc_component_update_bits(component, reg, mask, value);
 }
 
 static int soc_dapm_test_bits(struct snd_soc_dapm_context *dapm,
 	int reg, unsigned int mask, unsigned int value)
 {
-	if (!dapm->component)
+	struct snd_soc_component *component = snd_soc_dapm_to_component(dapm);
+
+	if (!component)
 		return -EIO;
-	return snd_soc_component_test_bits(dapm->component, reg, mask, value);
+
+	return snd_soc_component_test_bits(component, reg, mask, value);
 }
 
 static void soc_dapm_async_complete(struct snd_soc_dapm_context *dapm)
 {
-	if (dapm->component)
-		snd_soc_component_async_complete(dapm->component);
+	struct snd_soc_component *component = snd_soc_dapm_to_component(dapm);
+
+	if (component)
+		snd_soc_component_async_complete(component);
 }
 
 static struct snd_soc_dapm_widget *
@@ -696,10 +709,11 @@ dapm_wcache_lookup(struct snd_soc_dapm_widget *w, const char *name)
 int snd_soc_dapm_force_bias_level(struct snd_soc_dapm_context *dapm,
 	enum snd_soc_bias_level level)
 {
+	struct snd_soc_component *component = snd_soc_dapm_to_component(dapm);
 	int ret = 0;
 
-	if (dapm->component)
-		ret = snd_soc_component_set_bias_level(dapm->component, level);
+	if (component)
+		ret = snd_soc_component_set_bias_level(component, level);
 
 	if (ret == 0)
 		dapm->bias_level = level;
@@ -1653,6 +1667,7 @@ static void dapm_seq_run(struct snd_soc_card *card,
 	int cur_subseq = -1;
 	int cur_reg = SND_SOC_NOPM;
 	struct snd_soc_dapm_context *cur_dapm = NULL;
+	struct snd_soc_component *cur_component = NULL;
 	int i;
 	int *sort;
 
@@ -1670,11 +1685,11 @@ static void dapm_seq_run(struct snd_soc_card *card,
 			if (!list_empty(&pending))
 				dapm_seq_run_coalesced(card, &pending);
 
-			if (cur_dapm && cur_dapm->component) {
+			if (cur_dapm && cur_component) {
 				for (i = 0; i < ARRAY_SIZE(dapm_up_seq); i++)
 					if (sort[i] == cur_sort)
 						snd_soc_component_seq_notifier(
-							cur_dapm->component,
+							cur_component,
 							i, cur_subseq);
 			}
 
@@ -1686,6 +1701,7 @@ static void dapm_seq_run(struct snd_soc_card *card,
 			cur_subseq = INT_MIN;
 			cur_reg = SND_SOC_NOPM;
 			cur_dapm = NULL;
+			cur_component = NULL;
 		}
 
 		switch (w->id) {
@@ -1719,6 +1735,7 @@ static void dapm_seq_run(struct snd_soc_card *card,
 			cur_subseq = w->subseq;
 			cur_reg = w->reg;
 			cur_dapm = w->dapm;
+			cur_component = snd_soc_dapm_to_component(cur_dapm);
 			list_move(&w->power_list, &pending);
 			break;
 		}
@@ -1731,11 +1748,11 @@ static void dapm_seq_run(struct snd_soc_card *card,
 	if (!list_empty(&pending))
 		dapm_seq_run_coalesced(card, &pending);
 
-	if (cur_dapm && cur_dapm->component) {
+	if (cur_dapm && cur_component) {
 		for (i = 0; i < ARRAY_SIZE(dapm_up_seq); i++)
 			if (sort[i] == cur_sort)
 				snd_soc_component_seq_notifier(
-					cur_dapm->component,
+					cur_component,
 					i, cur_subseq);
 	}
 
@@ -2193,8 +2210,9 @@ static ssize_t dapm_widget_power_read_file(struct file *file,
 			if (!p->connect)
 				continue;
 
-			c_name = p->node[rdir]->dapm->component ?
-				p->node[rdir]->dapm->component->name : NULL;
+			struct snd_soc_component *component = snd_soc_dapm_to_component(p->node[rdir]->dapm);
+
+			c_name = component ? component->name : NULL;
 			ret += scnprintf(buf + ret, PAGE_SIZE - ret,
 					" %s  \"%s\" \"%s\" \"%s\"\n",
 					(rdir == SND_SOC_DAPM_DIR_IN) ? "in" : "out",
@@ -2784,7 +2802,7 @@ int snd_soc_dapm_update_dai(struct snd_pcm_substream *substream,
 
 int snd_soc_dapm_widget_name_cmp(struct snd_soc_dapm_widget *widget, const char *s)
 {
-	struct snd_soc_component *component = widget->dapm->component;
+	struct snd_soc_component *component = snd_soc_dapm_to_component(widget->dapm);
 	const char *wname = widget->name;
 
 	if (component && component->name_prefix)
-- 
2.43.0

[PATCH 7/7] ASoC: soc-topology: use snd_soc_dapm_to_component()

[Kuninori Morimoto]

We can use snd_soc_dapm_to_component(). Let's use it

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/soc-topology.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index 7b0b8531bb32..df9673995f62 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -1242,7 +1242,8 @@ static int soc_tplg_dapm_widget_create(struct soc_tplg *tplg,
 	return 0;
 
 ready_err:
-	soc_tplg_remove_widget(widget->dapm->component, &widget->dobj, SOC_TPLG_PASS_WIDGET);
+	soc_tplg_remove_widget(snd_soc_dapm_to_component(widget->dapm),
+			       &widget->dobj, SOC_TPLG_PASS_WIDGET);
 	snd_soc_dapm_free_widget(widget);
 hdr_err:
 	kfree(template.sname);
-- 
2.43.0

Re: [PATCH 1/7] ASoC: soc-component: use dapm->component instead of container_of()

[Cezary Rojewski]

On 2025-04-09 4:57 AM, Kuninori Morimoto wrote:
> Now, snd_soc_dapm_to_component() (A) will convert dapm to component by
> container_of() (a).
> 
> (A)	static inline struct snd_soc_component *snd_soc_dapm_to_component(
> 		struct snd_soc_dapm_context *dapm)
> 	{
> (a)		return container_of(dapm, struct snd_soc_component, dapm);
> 	}
> 
> Because (a) uses container_of(), dapm of component works, but
> dapm of others will be "unknown" pointer (= not NULL).
> 
> OTOH, we will call snd_soc_dapm_init() (X) to initialize dapm, and
> it will be called from snd_soc_bind_card() (p) or
> soc_probe_component() (q) with pointer (component/NULL).
> 
> (p)	static int snd_soc_bind_card(...)
> 	{
> 		...
> (X)		snd_soc_dapm_init(..., NULL);
> 		...                    ^^^^
> 	}
> 
> (q)	static int soc_probe_component(...)
> 	{
> 		...
> (X)		snd_soc_dapm_init(, component);
> 		...                 ^^^^^^^^^
> 	}
> 
> And snd_soc_dapm_init() (X) will fill dapm->component (x)
> 
> (X)	void snd_soc_dapm_init(..., component, ...)
> 	{
> 		...
> (x)		dapm->component	= component;
> 		...
> 	}
> 
> We can simply use dapm->component in snd_soc_dapm_to_component() (A).
> In this case, dapm of others (p) will be just NULL.
> 
> Use dapm->component instead of container_of().
> The picky note can be removed by this patch.
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> Cc: Benjamin Bara <benjamin.bara@skidata.com>
> ---
>   include/sound/soc-component.h | 6 +-----
>   1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/include/sound/soc-component.h b/include/sound/soc-component.h
> index 61534ac0edd1..63928773df32 100644
> --- a/include/sound/soc-component.h
> +++ b/include/sound/soc-component.h
> @@ -265,15 +265,11 @@ struct snd_soc_component {
>    * snd_soc_dapm_to_component() - Casts a DAPM context to the component it is
>    *  embedded in
>    * @dapm: The DAPM context to cast to the component
> - *
> - * This function must only be used on DAPM contexts that are known to be part of
> - * a component (e.g. in a component driver). Otherwise the behavior is
> - * undefined.
>    */
>   static inline struct snd_soc_component *snd_soc_dapm_to_component(
>   	struct snd_soc_dapm_context *dapm)
>   {
> -	return container_of(dapm, struct snd_soc_component, dapm);
> +	return dapm->component;
>   }
>   
>   /**

Why not drop the wrapper all along? Personally, I find assignment 'comp 
= dapm->component' more readable than the wrapper above. We have very 
long prefixes in sound/soc/ in general, and the devs end up reaching 
char line-limits.

I'd rather focus on 'struct snd_soc_dapm_context' exclusively. A valid 
pointer to this type is required for many functions found in 
include/sound/ yet the struct is more of framework-internal entity. 
Strict separation between dapm-context "for the framework" from 
dapm-context "for the user" would grant better readability benefits.

Kind regards,
Czarek

Re: [PATCH 4/7] ASoC: intel: use snd_soc_dapm_to_component()

[Cezary Rojewski]

On 2025-04-09 4:57 AM, Kuninori Morimoto wrote:
> We can use snd_soc_dapm_to_component(). Let's use it
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

I'd appreciate "ASoC: Intel: avs:" prefix in the title. Keeps it 
coherent with other avs-driver changes.

> ---
>   sound/soc/intel/avs/control.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/soc/intel/avs/control.c b/sound/soc/intel/avs/control.c
> index 2e01dc75a15a..1af270ef0c8b 100644
> --- a/sound/soc/intel/avs/control.c
> +++ b/sound/soc/intel/avs/control.c
> @@ -19,7 +19,7 @@ static struct avs_dev *avs_get_kcontrol_adev(struct snd_kcontrol *kcontrol)
>   
>   	w = snd_soc_dapm_kcontrol_widget(kcontrol);
>   
> -	return to_avs_dev(w->dapm->component->dev);
> +	return to_avs_dev(snd_soc_dapm_to_component(w->dapm)->dev);
>   }
>   
>   static struct avs_path_module *avs_get_volume_module(struct avs_dev *adev, u32 id)

Re: [PATCH 1/7] ASoC: soc-component: use dapm->component instead of container_of()

[Kuninori Morimoto]

Hi Cezary

Thank you for your review.

> >   static inline struct snd_soc_component *snd_soc_dapm_to_component(
> >   	struct snd_soc_dapm_context *dapm)
> >   {
> > -	return container_of(dapm, struct snd_soc_component, dapm);
> > +	return dapm->component;
> >   }
(snip)
> Why not drop the wrapper all along? Personally, I find assignment 'comp 
> = dapm->component' more readable than the wrapper above. We have very 
> long prefixes in sound/soc/ in general, and the devs end up reaching 
> char line-limits.

Ah, yes indeed. I didn't notice this patern.

> I'd rather focus on 'struct snd_soc_dapm_context' exclusively. A valid 
> pointer to this type is required for many functions found in 
> include/sound/ yet the struct is more of framework-internal entity. 
> Strict separation between dapm-context "for the framework" from 
> dapm-context "for the user" would grant better readability benefits.

I confused about this.
If we drop snd_soc_dapm_to_component() function, and directly
use dapm->component, isn't it going to be the opposite of separation?

If my understand was correct of "strict separation", "user" can use
dapm-context pointer, but can't access to its member variable,
like struct clk ?

Thank you for your help !!

Best regards
---
Kuninori Morimoto

Re: [PATCH 4/7] ASoC: intel: use snd_soc_dapm_to_component()

[Kuninori Morimoto]

Hi Cezary

> > We can use snd_soc_dapm_to_component(). Let's use it
> > 
> > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> I'd appreciate "ASoC: Intel: avs:" prefix in the title. Keeps it 
> coherent with other avs-driver changes.

OK, thanks.
Will do in v2

Thank you for your help !!

Best regards
---
Kuninori Morimoto

Re: [PATCH 1/7] ASoC: soc-component: use dapm->component instead of container_of()

[Kuninori Morimoto]

Hi Cezary, again

> > I'd rather focus on 'struct snd_soc_dapm_context' exclusively. A valid 
> > pointer to this type is required for many functions found in 
> > include/sound/ yet the struct is more of framework-internal entity. 
> > Strict separation between dapm-context "for the framework" from 
> > dapm-context "for the user" would grant better readability benefits.

I have checked each drivers which is using struct snd_soc_dapm_context.
It seems not only framework but many user drivers need to use/access to it.

So separation seems impossible, and dropping wrapper is maybe not good idea.
But I can understand that it will be very longer code :)

	x = dapm->component;
	x = snd_soc_dapm_to_component(dapm);

For me personally, wrapper function/macro is not so bad idea.
It is much safer to use function/macro rather than touching them directly.

Anyway, I will try to keep current patch-set style.
But any comment is very welcome.
Thank you for your help !!

Best regards
---
Kuninori Morimoto

Re: [PATCH 1/7] ASoC: soc-component: use dapm->component instead of container_of()

[Cezary Rojewski]

On 2025-04-11 4:04 AM, Kuninori Morimoto wrote:
> 
> Hi Cezary, again
> 
>>> I'd rather focus on 'struct snd_soc_dapm_context' exclusively. A valid
>>> pointer to this type is required for many functions found in
>>> include/sound/ yet the struct is more of framework-internal entity.
>>> Strict separation between dapm-context "for the framework" from
>>> dapm-context "for the user" would grant better readability benefits.
> 
> I have checked each drivers which is using struct snd_soc_dapm_context.
> It seems not only framework but many user drivers need to use/access to it.
> 
> So separation seems impossible, and dropping wrapper is maybe not good idea.
> But I can understand that it will be very longer code :)
> 
> 	x = dapm->component;
> 	x = snd_soc_dapm_to_component(dapm);
> 
> For me personally, wrapper function/macro is not so bad idea.
> It is much safer to use function/macro rather than touching them directly.
> 
> Anyway, I will try to keep current patch-set style.
> But any comment is very welcome.
> Thank you for your help !!


I see two replies, hope it's fine if I just reply to this one. I love 
the enthusiasm though :D

What I meant is that even the name of the type - 'dapm_context' - speaks 
of framework-internal stuff. The fields that define that struct also are 
not for the users (drivers) really. Perhaps there is a way to get rid of 
the dapm_context pointer entirely in the argument lists of public 
functions found in include/sound/. The pointer could be obtained e.g.: 
via container_of() by the framework's internal handler when needed.

Otherwise, I do not see real benefit of using 'x = 
snd_soc_dapm_to_component()' over 'x = dapm->component'. In my opinion 
there's no need to provide wrapper for every field we have. No hard 
blocks though.


Kind regards,
Czarek

Re: [PATCH 1/7] ASoC: soc-component: use dapm->component instead of container_of()

[Kuninori Morimoto]

Hi Cezary

Thank you for your feedback

> What I meant is that even the name of the type - 'dapm_context' - speaks 
> of framework-internal stuff. The fields that define that struct also are 
> not for the users (drivers) really. Perhaps there is a way to get rid of 
> the dapm_context pointer entirely in the argument lists of public 
> functions found in include/sound/. The pointer could be obtained e.g.: 
> via container_of() by the framework's internal handler when needed.

Hmm...

In my quick check, the reason why user driver directly use dapm->xxx is
to get/access

	dapm->card
	dapm->component
	dapm->dev
	dapm->bias_level
	dapm->idle_bias_off

OTOH, many drivers are already using dapm related functions, like

	snd_soc_dapm_new_controls()
	snd_soc_dapm_add_routes()
	snd_soc_dapm_disable_pin_unlocked()
	snd_soc_dapm_sync_unlocked()
	snd_soc_dapm_mutex_lock()
	...

So, I think it is easy to hide dapm member from user driver, but
dapm_context pointer itself is still needed in user driver.

I think we can do is below

To hide dapm_context details from user driver
	- define struct dapm_context name only in include/sound.
	- define struct dapm_context details in soc_dapm.c

To use dapm_context related feature in user driver
	- use snd_soc_dapm_xxx() function in all user driver

I think it can be 1st step. We can reconsider about 2nd step ?
But what do you think ?

Thank you for your help !!

Best regards
---
Kuninori Morimoto

Re: [PATCH 1/7] ASoC: soc-component: use dapm->component instead of container_of()

[Kuninori Morimoto]

Hi Cezary, Mark

> 	dapm->card
> 	dapm->component
> 	dapm->dev
> 	dapm->bias_level
> 	dapm->idle_bias_off
(snip)
> 	snd_soc_dapm_new_controls()
> 	snd_soc_dapm_add_routes()
> 	snd_soc_dapm_disable_pin_unlocked()
> 	snd_soc_dapm_sync_unlocked()
> 	snd_soc_dapm_mutex_lock()
> 	...
(snip)
> To hide dapm_context details from user driver
> 	- define struct dapm_context name only in include/sound.
> 	- define struct dapm_context details in soc_dapm.c
> 
> To use dapm_context related feature in user driver
> 	- use snd_soc_dapm_xxx() function in all user driver

We have 2 way to get dapm_context (card vs component).
So if soc-dapm has function for these, we can remove dapm_context from
header (Not 100% sure for now, but maybe).
It will be big patch-set. I'm OK to create it. But what do you think ?

--- soc-dapm.c -------

static __snd_soc_dapm_xxx(dapm_context, ...)
{
	...
}

int snd_soc_dapm_xxx_for_card(card, xxx) {
	__snd_soc_dapm_xxx(card->dapm, ...);
}

int snd_soc_dapm_xxx_for_component(component, xxx) {
	__snd_soc_dapm_xxx(component->dapm, ...);
}

--- soc-dapm.h -------

#define snd_soc_dapm_xxx(x) _Generic((x, ...),			\
	struct snd_soc_card * :		snd_soc_dapm_xxx_for_card, \
	struct snd_soc_component * :	snd_soc_dapm_xxx_for_component)(x, ...)

Thank you for your help !!

Best regards
---
Kuninori Morimoto

Re: [PATCH 1/7] ASoC: soc-component: use dapm->component instead of container_of()

[Cezary Rojewski]

On 2025-04-14 4:01 AM, Kuninori Morimoto wrote:
> 
> Hi Cezary, Mark
> 
>> 	dapm->card
>> 	dapm->component
>> 	dapm->dev
>> 	dapm->bias_level
>> 	dapm->idle_bias_off
> (snip)
>> 	snd_soc_dapm_new_controls()
>> 	snd_soc_dapm_add_routes()
>> 	snd_soc_dapm_disable_pin_unlocked()
>> 	snd_soc_dapm_sync_unlocked()
>> 	snd_soc_dapm_mutex_lock()
>> 	...
> (snip)
>> To hide dapm_context details from user driver
>> 	- define struct dapm_context name only in include/sound.
>> 	- define struct dapm_context details in soc_dapm.c
>>
>> To use dapm_context related feature in user driver
>> 	- use snd_soc_dapm_xxx() function in all user driver
> 
> We have 2 way to get dapm_context (card vs component).
> So if soc-dapm has function for these, we can remove dapm_context from
> header (Not 100% sure for now, but maybe).
> It will be big patch-set. I'm OK to create it. But what do you think ?
> 
> --- soc-dapm.c -------
> 
> static __snd_soc_dapm_xxx(dapm_context, ...)
> {
> 	...
> }
> 
> int snd_soc_dapm_xxx_for_card(card, xxx) {
> 	__snd_soc_dapm_xxx(card->dapm, ...);
> }
> 
> int snd_soc_dapm_xxx_for_component(component, xxx) {
> 	__snd_soc_dapm_xxx(component->dapm, ...);
> }
> 
> --- soc-dapm.h -------
> 
> #define snd_soc_dapm_xxx(x) _Generic((x, ...),			\
> 	struct snd_soc_card * :		snd_soc_dapm_xxx_for_card, \
> 	struct snd_soc_component * :	snd_soc_dapm_xxx_for_component)(x, ...)
> 

Sorry for the delay in response, Kuninori.

Now, personally I'm up even for big patchsets i.e.: it's 'just' a 
_proposal_, nothing is merged by default and every good engineering 
discussion I see as absolute win.

Reducing the need for users (sound drivers) to manipulate dapm_context 
is a good thing. I'll tell more once I see the actual proposal. The 
pseudo-code shown above, provided is used by soc-core.c alone, looks good.


Kind regards,
Czarek

Re: [PATCH 1/7] ASoC: soc-component: use dapm->component instead of container_of()

[Kuninori Morimoto]

Hi Cezary

Thank you for your feedback

> >> 	dapm->card
> >> 	dapm->component
> >> 	dapm->dev
> >> 	dapm->bias_level
> >> 	dapm->idle_bias_off
> > (snip)
> >> 	snd_soc_dapm_new_controls()
> >> 	snd_soc_dapm_add_routes()
> >> 	snd_soc_dapm_disable_pin_unlocked()
> >> 	snd_soc_dapm_sync_unlocked()
> >> 	snd_soc_dapm_mutex_lock()
> >> 	...
> > (snip)
> >> To hide dapm_context details from user driver
> >> 	- define struct dapm_context name only in include/sound.
> >> 	- define struct dapm_context details in soc_dapm.c
> >>
> >> To use dapm_context related feature in user driver
> >> 	- use snd_soc_dapm_xxx() function in all user driver
> > 
> > We have 2 way to get dapm_context (card vs component).
> > So if soc-dapm has function for these, we can remove dapm_context from
> > header (Not 100% sure for now, but maybe).
> > It will be big patch-set. I'm OK to create it. But what do you think ?
> > 
> > --- soc-dapm.c -------
> > 
> > static __snd_soc_dapm_xxx(dapm_context, ...)
> > {
> > 	...
> > }
> > 
> > int snd_soc_dapm_xxx_for_card(card, xxx) {
> > 	__snd_soc_dapm_xxx(card->dapm, ...);
> > }
> > 
> > int snd_soc_dapm_xxx_for_component(component, xxx) {
> > 	__snd_soc_dapm_xxx(component->dapm, ...);
> > }
> > 
> > --- soc-dapm.h -------
> > 
> > #define snd_soc_dapm_xxx(x) _Generic((x, ...),			\
> > 	struct snd_soc_card * :		snd_soc_dapm_xxx_for_card, \
> > 	struct snd_soc_component * :	snd_soc_dapm_xxx_for_component)(x, ...)
> > 
> 
> Sorry for the delay in response, Kuninori.
> 
> Now, personally I'm up even for big patchsets i.e.: it's 'just' a 
> _proposal_, nothing is merged by default and every good engineering 
> discussion I see as absolute win.
> 
> Reducing the need for users (sound drivers) to manipulate dapm_context 
> is a good thing. I'll tell more once I see the actual proposal. The 
> pseudo-code shown above, provided is used by soc-core.c alone, looks good.

Thanks.
I'm now trying to cleanup dpcm code. Because it can be big patch-set,
I will post...

	- 1st step
	  Add new functions. It hide dapm-context from user driver.
	  It uses card or component. soc-dapm.c update only,
	  no user drivers are updated so far.

	- 2nd step
	  If all necessary new functions are added, switch to use
	  it, and remove old functions and dapm-context.
	  A lot of user drivers will be updated.

Before posting above, I would like to post my local remaining patches
as step zero ;)

Thank you for your help !!

Best regards
---
Kuninori Morimoto