| Message ID | 7d6c2755-77ac-86db-899f-7342ee9e69b5@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: meson: aiu: fix duplicate debugfs directory error | expand |
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") > > + 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; > }
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.
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; >> } >
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 --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; }
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(+)