diff mbox series

ASoC: meson: aiu: fix duplicate debugfs directory error

Message ID 7d6c2755-77ac-86db-899f-7342ee9e69b5@gmail.com
State New
Headers show
Series ASoC: meson: aiu: fix duplicate debugfs directory error | expand

Commit Message

Heiner Kallweit March 8, 2022, 7 p.m. UTC
On a S905W-based system I get the following error:
debugfs: Directory 'c1105400.audio-controller' with parent 'P230-Q200' already present!

Turned out that multiple components having the same name triggers this
error in soc_init_component_debugfs(). With the patch the error is
gone and that's the debugfs entries.

/sys/kernel/debug/asoc/P230-Q200/aiu_acodec:c1105400.audio-controller
/sys/kernel/debug/asoc/P230-Q200/aiu_hdmi:c1105400.audio-controller
/sys/kernel/debug/asoc/P230-Q200/aiu_cpu:c1105400.audio-controller

Because debugfs is affected only, this may not be something for stable.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 sound/soc/meson/aiu.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

Mark Brown March 9, 2022, 1:27 p.m. UTC | #1
On Wed, Mar 09, 2022 at 12:42:04AM +0100, Jerome Brunet wrote:

> If this is really a problem and Mark is Ok with, what you want to do is
> revert commit 024714223323 ("ASoC: meson: aiu: simplify component addition")

I'm OK with that.
Heiner Kallweit March 9, 2022, 7:46 p.m. UTC | #2
On 09.03.2022 00:42, Jerome Brunet wrote:
> 
> On Tue 08 Mar 2022 at 20:00, Heiner Kallweit <hkallweit1@gmail.com> wrote:
> 
>> On a S905W-based system I get the following error:
>> debugfs: Directory 'c1105400.audio-controller' with parent 'P230-Q200' already present!
>>
>> Turned out that multiple components having the same name triggers this
>> error in soc_init_component_debugfs().
> 
> Because the component name is directly derived from the dev name
> 
>> With the patch the error is
>> gone and that's the debugfs entries.
>>
>> /sys/kernel/debug/asoc/P230-Q200/aiu_acodec:c1105400.audio-controller
>> /sys/kernel/debug/asoc/P230-Q200/aiu_hdmi:c1105400.audio-controller
>> /sys/kernel/debug/asoc/P230-Q200/aiu_cpu:c1105400.audio-controller
>>
>> Because debugfs is affected only, this may not be something for stable.
> 
> It is not entirely true that only debugfs is affected, though it is the
> only thing really complaining.
> 
> I had a similar tweak around debugfs entry when I first submitted the
> AIU. There was a discussion around that:
> 
> https://patchwork.kernel.org/project/linux-amlogic/patch/20200213155159.3235792-6-jbrunet@baylibre.com/
> 
> I proposed a solution which got merged but ended up breaking other cards
> because there was some assumptions around what the component name
> is: basically the dev_name since there was originally 1:1 mapping
> So it had to be reverted.
> 
> Full details here:
> https://patchwork.kernel.org/project/alsa-devel/patch/20200214134704.342501-1-jbrunet@baylibre.com/
> 
> Unfortunately I did not had the time to continue working on it since them
> 
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>>  sound/soc/meson/aiu.c | 14 ++++++++++++++
>>  1 file changed, 14 insertions(+)
>>
>> diff --git a/sound/soc/meson/aiu.c b/sound/soc/meson/aiu.c
>> index d299a70db..c1a2aea5f 100644
>> --- a/sound/soc/meson/aiu.c
>> +++ b/sound/soc/meson/aiu.c
>> @@ -68,6 +68,20 @@ int aiu_of_xlate_dai_name(struct snd_soc_component *component,
>>  
>>  	*dai_name = dai->driver->name;
> 
> While I don't really mind one way or the other about the prefix itself,
> aiu_of_xlate_dai_name() is not the place to tweak it.
> 
> This should be done before adding the component, not when parsing the DAI
> with DT.
> 
> If this is really a problem and Mark is Ok with, what you want to do is
> revert commit 024714223323 ("ASoC: meson: aiu: simplify component addition")
>>  
Thanks a lot for the links and for pointing me in the right direction.
The revert you mentioned would need a little bit of trivial tweaking
due to changes of the underlying code.
However based on what I read so far in the linked discussions I have
another idea to make the solution more generic. I'll submit the
patches and then we can decide how to go on.

>> +	switch (component_id) {
>> +	case AIU_CPU:
>> +		component->debugfs_prefix = "aiu_cpu";
>> +		break;
>> +	case AIU_HDMI:
>> +		component->debugfs_prefix = "aiu_hdmi";
>> +		break;
>> +	case AIU_ACODEC:
>> +		component->debugfs_prefix = "aiu_acodec";
>> +		break;
>> +	default:
>> +		break;
>> +	}
>> +
>>  	return 0;
>>  }
>
Geraldo Nascimento March 10, 2022, 1:50 p.m. UTC | #3
On Tue, Mar 08, 2022 at 08:00:14PM +0100, Heiner Kallweit wrote:
> On a S905W-based system I get the following error:
> debugfs: Directory 'c1105400.audio-controller' with parent 'P230-Q200' already present!
> 
> Turned out that multiple components having the same name triggers this
> error in soc_init_component_debugfs(). With the patch the error is
> gone and that's the debugfs entries.

Hi Heiner,

Thanks for the patches!

This one has been bugging me for quite some time, I'm glad you took time
to fix it.

I'm sure Martin and the BayLibre folks will soon review both of your patches,
just wanted to thank you.

> 
> /sys/kernel/debug/asoc/P230-Q200/aiu_acodec:c1105400.audio-controller
> /sys/kernel/debug/asoc/P230-Q200/aiu_hdmi:c1105400.audio-controller
> /sys/kernel/debug/asoc/P230-Q200/aiu_cpu:c1105400.audio-controller
> 
> Because debugfs is affected only, this may not be something for stable.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  sound/soc/meson/aiu.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/sound/soc/meson/aiu.c b/sound/soc/meson/aiu.c
> index d299a70db..c1a2aea5f 100644
> --- a/sound/soc/meson/aiu.c
> +++ b/sound/soc/meson/aiu.c
> @@ -68,6 +68,20 @@ int aiu_of_xlate_dai_name(struct snd_soc_component *component,
>  
>  	*dai_name = dai->driver->name;
>  
> +	switch (component_id) {
> +	case AIU_CPU:
> +		component->debugfs_prefix = "aiu_cpu";
> +		break;
> +	case AIU_HDMI:
> +		component->debugfs_prefix = "aiu_hdmi";
> +		break;
> +	case AIU_ACODEC:
> +		component->debugfs_prefix = "aiu_acodec";
> +		break;
> +	default:
> +		break;
> +	}
> +
>  	return 0;
>  }
>  
> -- 
> 2.35.1
>
diff mbox series

Patch

diff --git a/sound/soc/meson/aiu.c b/sound/soc/meson/aiu.c
index d299a70db..c1a2aea5f 100644
--- a/sound/soc/meson/aiu.c
+++ b/sound/soc/meson/aiu.c
@@ -68,6 +68,20 @@  int aiu_of_xlate_dai_name(struct snd_soc_component *component,
 
 	*dai_name = dai->driver->name;
 
+	switch (component_id) {
+	case AIU_CPU:
+		component->debugfs_prefix = "aiu_cpu";
+		break;
+	case AIU_HDMI:
+		component->debugfs_prefix = "aiu_hdmi";
+		break;
+	case AIU_ACODEC:
+		component->debugfs_prefix = "aiu_acodec";
+		break;
+	default:
+		break;
+	}
+
 	return 0;
 }