[v2] ASoC: fsl_ssi: Do not create debugfs directory

Commit Message

Fabio Estevam April 8, 2015, 5:21 p.m. UTC

From: Fabio Estevam <[e-mail]>

Since commit 81c7cfd1b22a0 ("ASoC: Move debugfs registration to the
component level") ASoC core code deals with registering debugfs, so we
should not be creating the debugfs directory locally in order to avoid
the following warning:

fsl-ssi-dai 2028000.ssi: ASoC: Failed to create component debugfs directory

Signed-off-by: Fabio Estevam <[e-mail]>
---
Changes since v1:
- Only remove dir creation/remove as suggested by Mark Brown

 sound/soc/fsl/fsl_ssi.h     |  1 -
 sound/soc/fsl/fsl_ssi_dbg.c | 13 +++----------
 2 files changed, 3 insertions(+), 11 deletions(-)

Comments

Nicolin Chen April 9, 2015, 7:49 a.m. UTC | #1

On Wed, Apr 08, 2015 at 02:21:24PM -0300, Fabio Estevam wrote:
> From: Fabio Estevam <[e-mail]>
> 
> Since commit 81c7cfd1b22a0 ("ASoC: Move debugfs registration to the
> component level") ASoC core code deals with registering debugfs, so we
> should not be creating the debugfs directory locally in order to avoid
> the following warning:
> 
> fsl-ssi-dai 2028000.ssi: ASoC: Failed to create component debugfs directory
> 
> Signed-off-by: Fabio Estevam <[e-mail]>
> ---
> Changes since v1:
> - Only remove dir creation/remove as suggested by Mark Brown

> diff --git a/sound/soc/fsl/fsl_ssi_dbg.c b/sound/soc/fsl/fsl_ssi_dbg.c
> index 5469ffb..46cea84 100644
> --- a/sound/soc/fsl/fsl_ssi_dbg.c
> +++ b/sound/soc/fsl/fsl_ssi_dbg.c
> @@ -142,22 +142,15 @@ static const struct file_operations fsl_ssi_stats_ops = {
>  
>  int fsl_ssi_debugfs_create(struct fsl_ssi_dbg *ssi_dbg, struct device *dev)
>  {
> -	ssi_dbg->dbg_dir = debugfs_create_dir(dev_name(dev), NULL);
> -	if (!ssi_dbg->dbg_dir)
> +	ssi_dbg->dbg_stats = debugfs_create_file("stats", S_IRUGO, NULL,
> +						 ssi_dbg, &fsl_ssi_stats_ops);
> +	if (!ssi_dbg->dbg_stats)
>  		return -ENOMEM;
>  
> -	ssi_dbg->dbg_stats = debugfs_create_file("stats", S_IRUGO,
> -			ssi_dbg->dbg_dir, ssi_dbg, &fsl_ssi_stats_ops);

It seems that dropping the dbg_dir and using NULL will make the stats
become an orphan inside the debugfs. I am wondering if we can get the
dbg_dir from the component so as to put this stats under the directory
of SSIn as before.

Nicolin

Lars-Peter Clausen April 9, 2015, 7:56 a.m. UTC | #2

On 04/09/2015 09:49 AM, Nicolin Chen wrote:
> On Wed, Apr 08, 2015 at 02:21:24PM -0300, Fabio Estevam wrote:
>> From: Fabio Estevam <[e-mail]>
>>
>> Since commit 81c7cfd1b22a0 ("ASoC: Move debugfs registration to the
>> component level") ASoC core code deals with registering debugfs, so we
>> should not be creating the debugfs directory locally in order to avoid
>> the following warning:
>>
>> fsl-ssi-dai 2028000.ssi: ASoC: Failed to create component debugfs directory
>>
>> Signed-off-by: Fabio Estevam <[e-mail]>
>> ---
>> Changes since v1:
>> - Only remove dir creation/remove as suggested by Mark Brown
>
>> diff --git a/sound/soc/fsl/fsl_ssi_dbg.c b/sound/soc/fsl/fsl_ssi_dbg.c
>> index 5469ffb..46cea84 100644
>> --- a/sound/soc/fsl/fsl_ssi_dbg.c
>> +++ b/sound/soc/fsl/fsl_ssi_dbg.c
>> @@ -142,22 +142,15 @@ static const struct file_operations fsl_ssi_stats_ops = {
>>
>>   int fsl_ssi_debugfs_create(struct fsl_ssi_dbg *ssi_dbg, struct device *dev)
>>   {
>> -	ssi_dbg->dbg_dir = debugfs_create_dir(dev_name(dev), NULL);
>> -	if (!ssi_dbg->dbg_dir)
>> +	ssi_dbg->dbg_stats = debugfs_create_file("stats", S_IRUGO, NULL,
>> +						 ssi_dbg, &fsl_ssi_stats_ops);
>> +	if (!ssi_dbg->dbg_stats)
>>   		return -ENOMEM;
>>
>> -	ssi_dbg->dbg_stats = debugfs_create_file("stats", S_IRUGO,
>> -			ssi_dbg->dbg_dir, ssi_dbg, &fsl_ssi_stats_ops);
>
> It seems that dropping the dbg_dir and using NULL will make the stats
> become an orphan inside the debugfs. I am wondering if we can get the
> dbg_dir from the component so as to put this stats under the directory
> of SSIn as before.

That would be the right thing I guess. But the original code registers a 
top-level debugfs directory with the name of the device. Whereas the debugfs 
directory for the component is created in asoc/CARD_NAME/. I don't see how 
these two could collide? I think the patch is trying to fix the wrong thing, 
the issue for the collision is something else.

- Lars

Patch

diff --git a/sound/soc/fsl/fsl_ssi.h b/sound/soc/fsl/fsl_ssi.h
index 5065105..68aef90 100644
--- a/sound/soc/fsl/fsl_ssi.h
+++ b/sound/soc/fsl/fsl_ssi.h
@@ -211,7 +211,6 @@  struct device;
 #if IS_ENABLED(CONFIG_DEBUG_FS)
 
 struct fsl_ssi_dbg {
-	struct dentry *dbg_dir;
 	struct dentry *dbg_stats;
 
 	struct {
diff --git a/sound/soc/fsl/fsl_ssi_dbg.c b/sound/soc/fsl/fsl_ssi_dbg.c
index 5469ffb..46cea84 100644
--- a/sound/soc/fsl/fsl_ssi_dbg.c
+++ b/sound/soc/fsl/fsl_ssi_dbg.c
@@ -142,22 +142,15 @@  static const struct file_operations fsl_ssi_stats_ops = {
 
 int fsl_ssi_debugfs_create(struct fsl_ssi_dbg *ssi_dbg, struct device *dev)
 {
-	ssi_dbg->dbg_dir = debugfs_create_dir(dev_name(dev), NULL);
-	if (!ssi_dbg->dbg_dir)
+	ssi_dbg->dbg_stats = debugfs_create_file("stats", S_IRUGO, NULL,
+						 ssi_dbg, &fsl_ssi_stats_ops);
+	if (!ssi_dbg->dbg_stats)
 		return -ENOMEM;
 
-	ssi_dbg->dbg_stats = debugfs_create_file("stats", S_IRUGO,
-			ssi_dbg->dbg_dir, ssi_dbg, &fsl_ssi_stats_ops);
-	if (!ssi_dbg->dbg_stats) {
-		debugfs_remove(ssi_dbg->dbg_dir);
-		return -ENOMEM;
-	}
-
 	return 0;
 }
 
 void fsl_ssi_debugfs_remove(struct fsl_ssi_dbg *ssi_dbg)
 {
 	debugfs_remove(ssi_dbg->dbg_stats);
-	debugfs_remove(ssi_dbg->dbg_dir);
 }