[PATCH 0/4] ASoC:: don't use snd_soc_rtdcom_lookup()

[PATCH 0/4] ASoC:: don't use snd_soc_rtdcom_lookup()

[Kuninori Morimoto]

Hi Mark
Cc related engineer

These patches are tring to not to use snd_soc_rtdcom_lookup() function
on each drivers as much as possible,  because we might have same name
component under multi component situation.
It can't find correct component in such case.

I tried to add new feature on each drivers to not to use it,
but I can't test.
Thus, these patches should get Acked-by or Tested-by from each drivers
user/maintenor. Please test these.

After these patches, Intel / SOF drivers are still using
snd_soc_rtdcom_lookup(). Because it is very complex, I couldn't try
not to use it.
If possible, each drivers should try to not use it,
and it should be removed from ASoC.

Kuninori Morimoto (4):
  ASoC: mediatek: don't use snd_soc_rtdcom_lookup()
  ASoC: intel: baytrail: don't use snd_soc_rtdcom_lookup()
  ASoC: intel: haswell: don't use snd_soc_rtdcom_lookup()
  ASoC: amd: don't use snd_soc_rtdcom_lookup()

 sound/soc/amd/raven/acp3x-pcm-dma.c         |  6 ------
 sound/soc/intel/baytrail/sst-baytrail-pcm.c | 13 +++++++------
 sound/soc/intel/haswell/sst-haswell-pcm.c   |  4 +++-
 sound/soc/mediatek/common/mtk-afe-fe-dai.c  | 12 +++++-------
 sound/soc/mediatek/common/mtk-afe-fe-dai.h  |  3 ++-
 sound/soc/mediatek/common/mtk-base-afe.h    |  6 ++++--
 sound/soc/mediatek/mt2701/mt2701-afe-pcm.c  |  6 ++++--
 sound/soc/mediatek/mt6797/mt6797-afe-pcm.c  | 11 ++++-------
 sound/soc/mediatek/mt8173/mt8173-afe-pcm.c  |  7 ++++---
 sound/soc/mediatek/mt8183/mt8183-afe-pcm.c  | 11 ++++-------
 10 files changed, 37 insertions(+), 42 deletions(-)

-- 
2.17.1

[PATCH 1/4] ASoC: mediatek: don't use snd_soc_rtdcom_lookup()

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

We shouldn't use snd_soc_rtdcom_lookup() as much as possible.
It works today, but, will not work in the future if we support multi
CPU/Codec/Platform, because 1 rtd might have multiple same driver
named component.

mediatek drivers are using snd_soc_rtdcom_lookup() at
afe->memif_fs and/or afe->irq_fs to get component.
But, caller knows it via dai->component.
We don't need to use snd_soc_rtdcom_lookup().
This patch fixup it.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/mediatek/common/mtk-afe-fe-dai.c | 12 +++++-------
 sound/soc/mediatek/common/mtk-afe-fe-dai.h |  3 ++-
 sound/soc/mediatek/common/mtk-base-afe.h   |  6 ++++--
 sound/soc/mediatek/mt2701/mt2701-afe-pcm.c |  6 ++++--
 sound/soc/mediatek/mt6797/mt6797-afe-pcm.c | 11 ++++-------
 sound/soc/mediatek/mt8173/mt8173-afe-pcm.c |  7 ++++---
 sound/soc/mediatek/mt8183/mt8183-afe-pcm.c | 11 ++++-------
 7 files changed, 27 insertions(+), 29 deletions(-)

diff --git a/sound/soc/mediatek/common/mtk-afe-fe-dai.c b/sound/soc/mediatek/common/mtk-afe-fe-dai.c
index 375e3b492922..8ee7206aa63d 100644
--- a/sound/soc/mediatek/common/mtk-afe-fe-dai.c
+++ b/sound/soc/mediatek/common/mtk-afe-fe-dai.c
@@ -162,7 +162,7 @@ int mtk_afe_fe_hw_params(struct snd_pcm_substream *substream,
 	}
 
 	/* set rate */
-	ret = mtk_memif_set_rate_substream(substream, id, rate);
+	ret = mtk_memif_set_rate_substream(dai->component, substream, id, rate);
 	if (ret) {
 		dev_err(afe->dev, "%s(), error, id %d, set rate %d, ret %d\n",
 			__func__, id, rate, ret);
@@ -225,7 +225,7 @@ int mtk_afe_fe_trigger(struct snd_pcm_substream *substream, int cmd,
 				       irq_data->irq_cnt_shift);
 
 		/* set irq fs */
-		fs = afe->irq_fs(substream, runtime->rate);
+		fs = afe->irq_fs(dai->component, substream, runtime->rate);
 
 		if (fs < 0)
 			return -EINVAL;
@@ -502,12 +502,10 @@ int mtk_memif_set_rate(struct mtk_base_afe *afe,
 }
 EXPORT_SYMBOL_GPL(mtk_memif_set_rate);
 
-int mtk_memif_set_rate_substream(struct snd_pcm_substream *substream,
+int mtk_memif_set_rate_substream(struct snd_soc_component *component,
+				 struct snd_pcm_substream *substream,
 				 int id, unsigned int rate)
 {
-	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct snd_soc_component *component =
-		snd_soc_rtdcom_lookup(rtd, AFE_PCM_NAME);
 	struct mtk_base_afe *afe = snd_soc_component_get_drvdata(component);
 
 	int fs = 0;
@@ -518,7 +516,7 @@ int mtk_memif_set_rate_substream(struct snd_pcm_substream *substream,
 		return -EINVAL;
 	}
 
-	fs = afe->memif_fs(substream, rate);
+	fs = afe->memif_fs(component, substream, rate);
 
 	if (fs < 0)
 		return -EINVAL;
diff --git a/sound/soc/mediatek/common/mtk-afe-fe-dai.h b/sound/soc/mediatek/common/mtk-afe-fe-dai.h
index 8cec90671827..d8fab806dbd0 100644
--- a/sound/soc/mediatek/common/mtk-afe-fe-dai.h
+++ b/sound/soc/mediatek/common/mtk-afe-fe-dai.h
@@ -44,7 +44,8 @@ int mtk_memif_set_channel(struct mtk_base_afe *afe,
 			  int id, unsigned int channel);
 int mtk_memif_set_rate(struct mtk_base_afe *afe,
 		       int id, unsigned int rate);
-int mtk_memif_set_rate_substream(struct snd_pcm_substream *substream,
+int mtk_memif_set_rate_substream(struct snd_soc_component *component,
+				 struct snd_pcm_substream *substream,
 				 int id, unsigned int rate);
 int mtk_memif_set_format(struct mtk_base_afe *afe,
 			 int id, snd_pcm_format_t format);
diff --git a/sound/soc/mediatek/common/mtk-base-afe.h b/sound/soc/mediatek/common/mtk-base-afe.h
index a8cf44d98244..c5e3c680332d 100644
--- a/sound/soc/mediatek/common/mtk-base-afe.h
+++ b/sound/soc/mediatek/common/mtk-base-afe.h
@@ -97,9 +97,11 @@ struct mtk_base_afe {
 	unsigned int num_dai_drivers;
 
 	const struct snd_pcm_hardware *mtk_afe_hardware;
-	int (*memif_fs)(struct snd_pcm_substream *substream,
+	int (*memif_fs)(struct snd_soc_component *component,
+			struct snd_pcm_substream *substream,
 			unsigned int rate);
-	int (*irq_fs)(struct snd_pcm_substream *substream,
+	int (*irq_fs)(struct snd_soc_component *component,
+		      struct snd_pcm_substream *substream,
 		      unsigned int rate);
 	int (*get_dai_fs)(struct mtk_base_afe *afe,
 			  int dai_id, unsigned int rate);
diff --git a/sound/soc/mediatek/mt2701/mt2701-afe-pcm.c b/sound/soc/mediatek/mt2701/mt2701-afe-pcm.c
index f0250b0dd734..456f3a0b98f9 100644
--- a/sound/soc/mediatek/mt2701/mt2701-afe-pcm.c
+++ b/sound/soc/mediatek/mt2701/mt2701-afe-pcm.c
@@ -491,7 +491,8 @@ static int mt2701_dlm_fe_trigger(struct snd_pcm_substream *substream,
 	}
 }
 
-static int mt2701_memif_fs(struct snd_pcm_substream *substream,
+static int mt2701_memif_fs(struct snd_soc_component *component,
+			   struct snd_pcm_substream *substream,
 			   unsigned int rate)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
@@ -505,7 +506,8 @@ static int mt2701_memif_fs(struct snd_pcm_substream *substream,
 	return fs;
 }
 
-static int mt2701_irq_fs(struct snd_pcm_substream *substream, unsigned int rate)
+static int mt2701_irq_fs(struct snd_soc_component *component,
+			 struct snd_pcm_substream *substream, unsigned int rate)
 {
 	return mt2701_afe_i2s_fs(rate);
 }
diff --git a/sound/soc/mediatek/mt6797/mt6797-afe-pcm.c b/sound/soc/mediatek/mt6797/mt6797-afe-pcm.c
index 7f930556d961..7a0bbaaad677 100644
--- a/sound/soc/mediatek/mt6797/mt6797-afe-pcm.c
+++ b/sound/soc/mediatek/mt6797/mt6797-afe-pcm.c
@@ -136,23 +136,20 @@ static const struct snd_pcm_hardware mt6797_afe_hardware = {
 	.fifo_size = 0,
 };
 
-static int mt6797_memif_fs(struct snd_pcm_substream *substream,
+static int mt6797_memif_fs(struct snd_soc_component *component,
+			   struct snd_pcm_substream *substream,
 			   unsigned int rate)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct snd_soc_component *component =
-		snd_soc_rtdcom_lookup(rtd, AFE_PCM_NAME);
 	struct mtk_base_afe *afe = snd_soc_component_get_drvdata(component);
 	int id = asoc_rtd_to_cpu(rtd, 0)->id;
 
 	return mt6797_rate_transform(afe->dev, rate, id);
 }
 
-static int mt6797_irq_fs(struct snd_pcm_substream *substream, unsigned int rate)
+static int mt6797_irq_fs(struct snd_soc_component *component,
+			 struct snd_pcm_substream *substream, unsigned int rate)
 {
-	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct snd_soc_component *component =
-		snd_soc_rtdcom_lookup(rtd, AFE_PCM_NAME);
 	struct mtk_base_afe *afe = snd_soc_component_get_drvdata(component);
 
 	return mt6797_general_rate_transform(afe->dev, rate);
diff --git a/sound/soc/mediatek/mt8173/mt8173-afe-pcm.c b/sound/soc/mediatek/mt8173/mt8173-afe-pcm.c
index 1e3f2d786066..6262e2c69107 100644
--- a/sound/soc/mediatek/mt8173/mt8173-afe-pcm.c
+++ b/sound/soc/mediatek/mt8173/mt8173-afe-pcm.c
@@ -479,11 +479,11 @@ static int mt8173_afe_hdmi_trigger(struct snd_pcm_substream *substream, int cmd,
 	}
 }
 
-static int mt8173_memif_fs(struct snd_pcm_substream *substream,
+static int mt8173_memif_fs(struct snd_soc_component *component,
+			   struct snd_pcm_substream *substream,
 			   unsigned int rate)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct snd_soc_component *component = snd_soc_rtdcom_lookup(rtd, AFE_PCM_NAME);
 	struct mtk_base_afe *afe = snd_soc_component_get_drvdata(component);
 	struct mtk_base_afe_memif *memif = &afe->memif[asoc_rtd_to_cpu(rtd, 0)->id];
 	int fs;
@@ -509,7 +509,8 @@ static int mt8173_memif_fs(struct snd_pcm_substream *substream,
 	return fs;
 }
 
-static int mt8173_irq_fs(struct snd_pcm_substream *substream, unsigned int rate)
+static int mt8173_irq_fs(struct snd_soc_component *component,
+			 struct snd_pcm_substream *substream, unsigned int rate)
 {
 	return mt8173_afe_i2s_fs(rate);
 }
diff --git a/sound/soc/mediatek/mt8183/mt8183-afe-pcm.c b/sound/soc/mediatek/mt8183/mt8183-afe-pcm.c
index c8ded53bde1d..5071f38fba29 100644
--- a/sound/soc/mediatek/mt8183/mt8183-afe-pcm.c
+++ b/sound/soc/mediatek/mt8183/mt8183-afe-pcm.c
@@ -139,23 +139,20 @@ static const struct snd_pcm_hardware mt8183_afe_hardware = {
 	.fifo_size = 0,
 };
 
-static int mt8183_memif_fs(struct snd_pcm_substream *substream,
+static int mt8183_memif_fs(struct snd_soc_component *component,
+			   struct snd_pcm_substream *substream,
 			   unsigned int rate)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct snd_soc_component *component =
-		snd_soc_rtdcom_lookup(rtd, AFE_PCM_NAME);
 	struct mtk_base_afe *afe = snd_soc_component_get_drvdata(component);
 	int id = asoc_rtd_to_cpu(rtd, 0)->id;
 
 	return mt8183_rate_transform(afe->dev, rate, id);
 }
 
-static int mt8183_irq_fs(struct snd_pcm_substream *substream, unsigned int rate)
+static int mt8183_irq_fs(struct snd_soc_component *component,
+			 struct snd_pcm_substream *substream, unsigned int rate)
 {
-	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct snd_soc_component *component =
-		snd_soc_rtdcom_lookup(rtd, AFE_PCM_NAME);
 	struct mtk_base_afe *afe = snd_soc_component_get_drvdata(component);
 
 	return mt8183_general_rate_transform(afe->dev, rate);
-- 
2.17.1

[PATCH 2/4] ASoC: intel: baytrail: don't use snd_soc_rtdcom_lookup()

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

We shouldn't use snd_soc_rtdcom_lookup() as much as possible.
It works today, but, will not work in the future if we support multi
CPU/Codec/Platform, because 1 rtd might have multiple same driver
named component.

intel baytrail driver is using it, but we can avoid it easily
by having component pointer at sst_byt_pcm_data.
This patch removes snd_soc_rtdcom_lookup() from this driver.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/intel/baytrail/sst-baytrail-pcm.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/sound/soc/intel/baytrail/sst-baytrail-pcm.c b/sound/soc/intel/baytrail/sst-baytrail-pcm.c
index 53383055c8dc..dbd830375bc4 100644
--- a/sound/soc/intel/baytrail/sst-baytrail-pcm.c
+++ b/sound/soc/intel/baytrail/sst-baytrail-pcm.c
@@ -36,6 +36,7 @@ static const struct snd_pcm_hardware sst_byt_pcm_hardware = {
 /* private data for each PCM DSP stream */
 struct sst_byt_pcm_data {
 	struct sst_byt_stream *stream;
+	struct snd_soc_component *component;
 	struct snd_pcm_substream *substream;
 	struct mutex mutex;
 
@@ -119,12 +120,11 @@ static int sst_byt_pcm_hw_params(struct snd_soc_component *component,
 	return 0;
 }
 
-static int sst_byt_pcm_restore_stream_context(struct snd_pcm_substream *substream)
+static int sst_byt_pcm_restore_stream_context(struct sst_byt_pcm_data *pcm_data)
 {
-	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct snd_soc_component *component = snd_soc_rtdcom_lookup(rtd, DRV_NAME);
+	struct snd_soc_pcm_runtime *rtd = pcm_data->substream->private_data;
+	struct snd_soc_component *component = pcm_data->component;
 	struct sst_byt_priv_data *pdata = snd_soc_component_get_drvdata(component);
-	struct sst_byt_pcm_data *pcm_data = &pdata->pcm[substream->stream];
 	struct sst_byt *byt = pdata->byt;
 	int ret;
 
@@ -149,7 +149,7 @@ static void sst_byt_pcm_work(struct work_struct *work)
 		container_of(work, struct sst_byt_pcm_data, work);
 
 	if (snd_pcm_running(pcm_data->substream))
-		sst_byt_pcm_restore_stream_context(pcm_data->substream);
+		sst_byt_pcm_restore_stream_context(pcm_data);
 }
 
 static int sst_byt_pcm_trigger(struct snd_soc_component *component,
@@ -198,7 +198,7 @@ static u32 byt_notify_pointer(struct sst_byt_stream *stream, void *data)
 	struct snd_pcm_substream *substream = pcm_data->substream;
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct snd_soc_component *component = snd_soc_rtdcom_lookup(rtd, DRV_NAME);
+	struct snd_soc_component *component = pcm_data->component;
 	struct sst_byt_priv_data *pdata = snd_soc_component_get_drvdata(component);
 	struct sst_byt *byt = pdata->byt;
 	u32 pos, hw_pos;
@@ -242,6 +242,7 @@ static int sst_byt_pcm_open(struct snd_soc_component *component,
 	mutex_lock(&pcm_data->mutex);
 
 	pcm_data->substream = substream;
+	pcm_data->component = component;
 
 	snd_soc_set_runtime_hwparams(substream, &sst_byt_pcm_hardware);
 
-- 
2.17.1

[PATCH 3/4] ASoC: intel: haswell: don't use snd_soc_rtdcom_lookup()

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

We shouldn't use snd_soc_rtdcom_lookup() as much as possible.
It works today, but, will not work in the future if we support multi
CPU/Codec/Platform, because 1 rtd might have multiple same driver
named component.

intel haswell driver is using it, but we can avoid it easily
by having component pointer at hsw_pcm_data.
This patch removes snd_soc_rtdcom_lookup() from this driver.

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

diff --git a/sound/soc/intel/haswell/sst-haswell-pcm.c b/sound/soc/intel/haswell/sst-haswell-pcm.c
index c183f8e94ee4..15e94dae45de 100644
--- a/sound/soc/intel/haswell/sst-haswell-pcm.c
+++ b/sound/soc/intel/haswell/sst-haswell-pcm.c
@@ -108,6 +108,7 @@ struct hsw_pcm_data {
 	struct snd_pcm *hsw_pcm;
 	u32 volume[2];
 	struct snd_pcm_substream *substream;
+	struct snd_soc_component *component;
 	struct snd_compr_stream *cstream;
 	unsigned int wpos;
 	struct mutex mutex;
@@ -696,7 +697,7 @@ static u32 hsw_notify_pointer(struct sst_hsw_stream *stream, void *data)
 	struct snd_pcm_substream *substream = pcm_data->substream;
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct snd_soc_component *component = snd_soc_rtdcom_lookup(rtd, DRV_NAME);
+	struct snd_soc_component *component = pcm_data->component;
 	struct hsw_priv_data *pdata = snd_soc_component_get_drvdata(component);
 	struct sst_hsw *hsw = pdata->hsw;
 	u32 pos;
@@ -798,6 +799,7 @@ static int hsw_pcm_open(struct snd_soc_component *component,
 	pm_runtime_get_sync(pdata->dev);
 
 	pcm_data->substream = substream;
+	pcm_data->component = component;
 
 	snd_soc_set_runtime_hwparams(substream, &hsw_pcm_hardware);
 
-- 
2.17.1

[PATCH 4/4] ASoC: amd: don't use snd_soc_rtdcom_lookup()

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

We shouldn't use snd_soc_rtdcom_lookup() as much as possible.
It works today, but, will not work in the future if we support multi
CPU/Codec/Platform, because 1 rtd might have multiple same driver
named component.

acp3x-pcm-dma driver is using snd_soc_rtdcom_lookup() at
open/close() to get component by using DRV_NAME.

But, lookuped "component" and function parameter "component"
are same. We don't need to use snd_soc_rtdcom_lookup().
This patch fixup it.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/amd/raven/acp3x-pcm-dma.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/sound/soc/amd/raven/acp3x-pcm-dma.c b/sound/soc/amd/raven/acp3x-pcm-dma.c
index e362f0bc9e46..8a892ade1f40 100644
--- a/sound/soc/amd/raven/acp3x-pcm-dma.c
+++ b/sound/soc/amd/raven/acp3x-pcm-dma.c
@@ -211,14 +211,11 @@ static int acp3x_dma_open(struct snd_soc_component *component,
 			  struct snd_pcm_substream *substream)
 {
 	struct snd_pcm_runtime *runtime;
-	struct snd_soc_pcm_runtime *prtd;
 	struct i2s_dev_data *adata;
 	struct i2s_stream_instance *i2s_data;
 	int ret;
 
 	runtime = substream->runtime;
-	prtd = substream->private_data;
-	component = snd_soc_rtdcom_lookup(prtd, DRV_NAME);
 	adata = dev_get_drvdata(component->dev);
 	i2s_data = kzalloc(sizeof(*i2s_data), GFP_KERNEL);
 	if (!i2s_data)
@@ -337,11 +334,8 @@ static int acp3x_dma_mmap(struct snd_soc_component *component,
 static int acp3x_dma_close(struct snd_soc_component *component,
 			   struct snd_pcm_substream *substream)
 {
-	struct snd_soc_pcm_runtime *prtd;
 	struct i2s_dev_data *adata;
 
-	prtd = substream->private_data;
-	component = snd_soc_rtdcom_lookup(prtd, DRV_NAME);
 	adata = dev_get_drvdata(component->dev);
 
 
-- 
2.17.1

Re: [PATCH 0/4] ASoC:: don't use snd_soc_rtdcom_lookup()

[Ranjani Sridharan]

On Wed, 2020-04-22 at 13:46 +0900, Kuninori Morimoto wrote:
> Hi Mark
> Cc related engineer
> 
> These patches are tring to not to use snd_soc_rtdcom_lookup()
> function
> on each drivers as much as possible,  because we might have same name
> component under multi component situation.
> It can't find correct component in such case.
> 
> I tried to add new feature on each drivers to not to use it,
> but I can't test.
> Thus, these patches should get Acked-by or Tested-by from each
> drivers
> user/maintenor. Please test these.
> 
> After these patches, Intel / SOF drivers are still using
> snd_soc_rtdcom_lookup(). Because it is very complex, I couldn't try
> not to use it.
> If possible, each drivers should try to not use it,
> and it should be removed from ASoC.
Morimoti-san,

For my education, I understand the concept of multi-cpu/codec
components, but when or who would need multiple platform components?
This would help me able to remove the snd_soc_rtdcom_lookup() call in
SOF.

Thanks,
Ranjani

Re: [PATCH 0/4] ASoC:: don't use snd_soc_rtdcom_lookup()

[Kuninori Morimoto]

Hi

Hi Ranjani

> > These patches are tring to not to use snd_soc_rtdcom_lookup()
> > function
> > on each drivers as much as possible,  because we might have same name
> > component under multi component situation.
> > It can't find correct component in such case.
> > 
> > I tried to add new feature on each drivers to not to use it,
> > but I can't test.
> > Thus, these patches should get Acked-by or Tested-by from each
> > drivers
> > user/maintenor. Please test these.
> > 
> > After these patches, Intel / SOF drivers are still using
> > snd_soc_rtdcom_lookup(). Because it is very complex, I couldn't try
> > not to use it.
> > If possible, each drivers should try to not use it,
> > and it should be removed from ASoC.
> Morimoti-san,
> 
> For my education, I understand the concept of multi-cpu/codec
> components, but when or who would need multiple platform components?
> This would help me able to remove the snd_soc_rtdcom_lookup() call in
> SOF.

I don't know concrete system.
But it is "possible" today.
And, we don't know the future system,
having flexibility is good idea, I think.

I'm thinking removing lookup function is nice idea,
but don't feel pressure to it.
"Now you know it" is very enough for me.

Thank you for your help !!

Best regards
---
Kuninori Morimoto

Re: [PATCH 0/4] ASoC:: don't use snd_soc_rtdcom_lookup()

[Ranjani Sridharan]

On Thu, 2020-04-23 at 07:12 +0900, Kuninori Morimoto wrote:
> Hi
> 
> Hi Ranjani
> 
> > > These patches are tring to not to use snd_soc_rtdcom_lookup()
> > > function
> > > on each drivers as much as possible,  because we might have same
> > > name
> > > component under multi component situation.
> > > It can't find correct component in such case.
> > > 
> > > I tried to add new feature on each drivers to not to use it,
> > > but I can't test.
> > > Thus, these patches should get Acked-by or Tested-by from each
> > > drivers
> > > user/maintenor. Please test these.
> > > 
> > > After these patches, Intel / SOF drivers are still using
> > > snd_soc_rtdcom_lookup(). Because it is very complex, I couldn't
> > > try
> > > not to use it.
> > > If possible, each drivers should try to not use it,
> > > and it should be removed from ASoC.
> > 
> > Morimoti-san,
> > 
> > For my education, I understand the concept of multi-cpu/codec
> > components, but when or who would need multiple platform
> > components?
> > This would help me able to remove the snd_soc_rtdcom_lookup() call
> > in
> > SOF.
> 
> I don't know concrete system.
> But it is "possible" today.
> And, we don't know the future system,
> having flexibility is good idea, I think.
> 
> I'm thinking removing lookup function is nice idea,
> but don't feel pressure to it.
> "Now you know it" is very enough for me.
I am having a hard time visualizing a scenario where we would have more
than one platform component. And even if we did, I'd think that the
driver registering these components would make sure to not duplicate
the driver names. Of course, we dont really check if thats really the
case. 
Do you think it makes sense to add that check when registering a
component? If we do that, then keeping snd_soc_rtdcom_lookup() might
not be such a bad idea. 

Thanks,
Ranjani

Re: [PATCH 0/4] ASoC:: don't use snd_soc_rtdcom_lookup()

[Kuninori Morimoto]

Hi Ranjani

> I am having a hard time visualizing a scenario where we would have more
> than one platform component. And even if we did, I'd think that the
> driver registering these components would make sure to not duplicate
> the driver names. Of course, we dont really check if thats really the
> case. 
> Do you think it makes sense to add that check when registering a
> component? If we do that, then keeping snd_soc_rtdcom_lookup() might
> not be such a bad idea. 

If noone uses lookup function, having same name component itself
is not a problem.

If component driver handles different component naming, and user-driver
can knows it somehow, using lookup function is not a problem either.
But, I think "how to specific the component name" from user-driver
is difficult.
Thus, this patch-set try to avoid it by adding small feature
even though under such situation.

So, keeping lookup function itself is possible.

Thank you for your help !!

Best regards
---
Kuninori Morimoto

Re: [PATCH 0/4] ASoC:: don't use snd_soc_rtdcom_lookup()

[Mark Brown]

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

On Wed, Apr 22, 2020 at 03:39:58PM -0700, Ranjani Sridharan wrote:
> On Thu, 2020-04-23 at 07:12 +0900, Kuninori Morimoto wrote:

> > I'm thinking removing lookup function is nice idea,
> > but don't feel pressure to it.
> > "Now you know it" is very enough for me.

> I am having a hard time visualizing a scenario where we would have more
> than one platform component. And even if we did, I'd think that the
> driver registering these components would make sure to not duplicate
> the driver names. Of course, we dont really check if thats really the
> case. 

The only use case I can think of is a link where there's a CPU on both
ends for some reason.

> Do you think it makes sense to add that check when registering a
> component? If we do that, then keeping snd_soc_rtdcom_lookup() might
> not be such a bad idea. 

Yeah.

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

Re: [PATCH 0/4] ASoC:: don't use snd_soc_rtdcom_lookup()

[Ranjani Sridharan]

On Thu, 2020-04-23 at 11:53 +0100, Mark Brown wrote:
> On Wed, Apr 22, 2020 at 03:39:58PM -0700, Ranjani Sridharan wrote:
> > On Thu, 2020-04-23 at 07:12 +0900, Kuninori Morimoto wrote:
> > > I'm thinking removing lookup function is nice idea,
> > > but don't feel pressure to it.
> > > "Now you know it" is very enough for me.
> > I am having a hard time visualizing a scenario where we would have
> > more
> > than one platform component. And even if we did, I'd think that the
> > driver registering these components would make sure to not
> > duplicate
> > the driver names. Of course, we dont really check if thats really
> > the
> > case. 
> 
> The only use case I can think of is a link where there's a CPU on
> both
> ends for some reason.
> 
> > Do you think it makes sense to add that check when registering a
> > component? If we do that, then keeping snd_soc_rtdcom_lookup()
> > might
> > not be such a bad idea. 
> 
> Yeah.
Thanks, Mark. Let me send a patch to handle this check in the core.

Thanks,
Ranjani