[PATCH 1/2] ASoC: Set up debugfs only once per CODEC

[PATCH 1/2] ASoC: Set up debugfs only once per CODEC

[Mark Brown]

Since the debugfs directory is current per CODEC we should only init
it when the CODEC is initialised, otherwise we end up with errors
being generated when an attempt is made to add duplicate debugfs
entries.

Since most of this stuff is actually for the card we should refactor
but this can come later.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 sound/soc/soc-core.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index a004876..ac0fa22 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1323,6 +1323,9 @@ static int soc_probe_dai_link(struct snd_soc_card *card, int num)
 				return ret;
 			}
 		}
+
+		soc_init_codec_debugfs(codec);
+
 		/* mark codec as probed and add to card codec list */
 		codec->probed = 1;
 		list_add(&codec->card_list, &card->codec_dev_list);
@@ -1400,8 +1403,6 @@ static int soc_probe_dai_link(struct snd_soc_card *card, int num)
 	if (ret < 0)
 		printk(KERN_WARNING "asoc: failed to add codec sysfs files\n");
 
-	soc_init_codec_debugfs(codec);
-
 	/* create the pcm */
 	ret = soc_new_pcm(rtd, num);
 	if (ret < 0) {
-- 
1.7.1

[PATCH 2/2] ASoC: Remove redundant device name from debugfs directory

[Mark Brown]

Since the core now includes deduplication in the name of CODEC
devices there's no need to add extra for the debugfs directory name.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 sound/soc/soc-core.c |   11 +----------
 1 files changed, 1 insertions(+), 10 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index ac0fa22..cea0fe4 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -234,16 +234,7 @@ static const struct file_operations codec_reg_fops = {
 
 static void soc_init_codec_debugfs(struct snd_soc_codec *codec)
 {
-	char codec_root[128];
-
-	if (codec->dev)
-		snprintf(codec_root, sizeof(codec_root),
-			"%s.%s", codec->name, dev_name(codec->dev));
-	else
-		snprintf(codec_root, sizeof(codec_root),
-			"%s", codec->name);
-
-	codec->debugfs_codec_root = debugfs_create_dir(codec_root,
+	codec->debugfs_codec_root = debugfs_create_dir(codec->name ,
 						       debugfs_root);
 	if (!codec->debugfs_codec_root) {
 		printk(KERN_WARNING
-- 
1.7.1

Re: [PATCH 1/2] ASoC: Set up debugfs only once per CODEC

[Peter Ujfalusi]

On Thursday 12 August 2010 17:50:31 ext Mark Brown wrote:
> Since the debugfs directory is current per CODEC we should only init
> it when the CODEC is initialised, otherwise we end up with errors
> being generated when an attempt is made to add duplicate debugfs
> entries.

But if you move the init of debugfs where you have moved it, than the machine 
driver given DAPM widgets will have no debugfs entries, since the machine init 
is going to be called later (dai_link->init)...

> 
> Since most of this stuff is actually for the card we should refactor
> but this can come later.

Sure, refactoring will help, but with this change we have missing debugfs files 
for widgets, which is kind of confusing.

-- 
Péter

> 
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
>  sound/soc/soc-core.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> index a004876..ac0fa22 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -1323,6 +1323,9 @@ static int soc_probe_dai_link(struct snd_soc_card
> *card, int num) return ret;
>  			}
>  		}
> +
> +		soc_init_codec_debugfs(codec);
> +
>  		/* mark codec as probed and add to card codec list */
>  		codec->probed = 1;
>  		list_add(&codec->card_list, &card->codec_dev_list);
> @@ -1400,8 +1403,6 @@ static int soc_probe_dai_link(struct snd_soc_card
> *card, int num) if (ret < 0)
>  		printk(KERN_WARNING "asoc: failed to add codec sysfs files\n");
> 
> -	soc_init_codec_debugfs(codec);
> -
>  	/* create the pcm */
>  	ret = soc_new_pcm(rtd, num);
>  	if (ret < 0) {

Re: [PATCH 1/2] ASoC: Set up debugfs only once per CODEC

[Mark Brown]

On Fri, Aug 13, 2010 at 07:42:44AM +0300, Peter Ujfalusi wrote:
> On Thursday 12 August 2010 17:50:31 ext Mark Brown wrote:
> > Since the debugfs directory is current per CODEC we should only init
> > it when the CODEC is initialised, otherwise we end up with errors
> > being generated when an attempt is made to add duplicate debugfs
> > entries.

> But if you move the init of debugfs where you have moved it, than the machine 
> driver given DAPM widgets will have no debugfs entries, since the machine init 
> is going to be called later (dai_link->init)...

Sure, but the big problem there is that these aren't really CODEC
specific objects.

> > Since most of this stuff is actually for the card we should refactor
> > but this can come later.

> Sure, refactoring will help, but with this change we have missing debugfs files 
> for widgets, which is kind of confusing.

Meh, it's a debug facility and I hope it'll be sorted out within a week
or so anyway.  Besides, you're not really loosing much information since
there tends to be no interesting routing outside the CODEC.

Re: [PATCH 1/2] ASoC: Set up debugfs only once per CODEC

[Peter Ujfalusi]

On Friday 13 August 2010 12:02:49 ext Mark Brown wrote:
> Sure, but the big problem there is that these aren't really CODEC
> specific objects.

Sure, but those widgets are directly connected to the CODEC's widgets.
And even if we have more than one codec under a single card, the widgets from 
the machine driver are kind of attached to the codec's chain (plus the 
interconnect routes between the multiple codecs if any).

> > > Since most of this stuff is actually for the card we should refactor
> > > but this can come later.
> > 
> > Sure, refactoring will help, but with this change we have missing debugfs
> > files for widgets, which is kind of confusing.
> 
> Meh, it's a debug facility and I hope it'll be sorted out within a week
> or so anyway.

Well, than it does not matter that much (the missing debugfs widget entries), 
if you have plans for this.

> Besides, you're not really loosing much information since
> there tends to be no interesting routing outside the CODEC.

Yeah. True. I have seen machines where there were some widgets of interest 
coming from the machine driver, but it is really only causes problems 
(inconvenience) with the debugfs.

Just wanted to mention the short term effect of this patch.

-- 
Péter

Re: [PATCH 1/2] ASoC: Set up debugfs only once?per CODEC

[Mark Brown]

On Fri, Aug 13, 2010 at 01:49:46PM +0300, Peter Ujfalusi wrote:
> On Friday 13 August 2010 12:02:49 ext Mark Brown wrote:

> > Meh, it's a debug facility and I hope it'll be sorted out within a week
> > or so anyway.

> Well, than it does not matter that much (the missing debugfs widget entries), 
> if you have plans for this.

Yes, I definitely want to deal with the root cause of the issue as well.

Re: [PATCH 1/2] ASoC: Set up debugfs only once per CODEC

[Liam Girdwood]

On Thu, 2010-08-12 at 15:50 +0100, Mark Brown wrote:
> Since the debugfs directory is current per CODEC we should only init
> it when the CODEC is initialised, otherwise we end up with errors
> being generated when an attempt is made to add duplicate debugfs
> entries.
> 
> Since most of this stuff is actually for the card we should refactor
> but this can come later.
> 
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
>  sound/soc/soc-core.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> index a004876..ac0fa22 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -1323,6 +1323,9 @@ static int soc_probe_dai_link(struct snd_soc_card *card, int num)
>  				return ret;
>  			}
>  		}
> +
> +		soc_init_codec_debugfs(codec);
> +
>  		/* mark codec as probed and add to card codec list */
>  		codec->probed = 1;
>  		list_add(&codec->card_list, &card->codec_dev_list);
> @@ -1400,8 +1403,6 @@ static int soc_probe_dai_link(struct snd_soc_card *card, int num)
>  	if (ret < 0)
>  		printk(KERN_WARNING "asoc: failed to add codec sysfs files\n");
>  
> -	soc_init_codec_debugfs(codec);
> -
>  	/* create the pcm */
>  	ret = soc_new_pcm(rtd, num);
>  	if (ret < 0) {


Both

Acked-by: Liam Girdwood <lrg@slimlogic.co.uk>
-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk