[PATCH v2] component: add debugfs support

[PATCH v2] component: add debugfs support

[Maciej Purski]

Add 'component' directory to debugfs. Create a new file for each master,
when a master is added. Remove it on a master deletion.

Show a list of devices matched with master and indicate if
master's components were successfully added and if the whole master is
bound.

Signed-off-by: Maciej Purski <m.purski@samsung.com>
---
Changes in v2:
- use seq_printf() instead of seq_puts() when printing headers
- move whole debugfs code to the file beginning in order to avoid
  forward declarations or using multiple ifdefs
---
 drivers/base/component.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 83 insertions(+)

diff --git a/drivers/base/component.c b/drivers/base/component.c
index 89b032f..8745ad9 100644
--- a/drivers/base/component.c
+++ b/drivers/base/component.c
@@ -17,6 +17,7 @@
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/slab.h>
+#include <linux/debugfs.h>
 
 struct component;
 
@@ -41,6 +42,7 @@ struct master {
 	const struct component_master_ops *ops;
 	struct device *dev;
 	struct component_match *match;
+	struct dentry *dentry;
 };
 
 struct component {
@@ -56,6 +58,85 @@ static DEFINE_MUTEX(component_mutex);
 static LIST_HEAD(component_list);
 static LIST_HEAD(masters);
 
+#ifdef CONFIG_DEBUG_FS
+
+static struct dentry *component_debugfs_dir;
+
+static int component_devices_show(struct seq_file *s, void *data)
+{
+	struct master *m = s->private;
+	struct component_match *match = m->match;
+	size_t i;
+
+	mutex_lock(&component_mutex);
+	seq_printf(s, "%-40s %20s\n", "master name", "status");
+	seq_puts(s, "-------------------------------------------------------------\n");
+	seq_printf(s, "%-40s %20s\n\n",
+		   dev_name(m->dev), m->bound ? "bound" : "not bound");
+
+	seq_printf(s, "%-40s %20s\n", "device name", "status");
+	seq_puts(s, "-------------------------------------------------------------\n");
+	for (i = 0; i < match->num; i++) {
+		struct device *d = (struct device *)match->compare[i].data;
+
+		seq_printf(s, "%-40s %20s\n", dev_name(d),
+			   match->compare[i].component ?
+			   "registered" : "not registered");
+	}
+	mutex_unlock(&component_mutex);
+
+	return 0;
+}
+
+static int component_devices_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, component_devices_show, inode->i_private);
+}
+
+static const struct file_operations component_devices_fops = {
+	.open = component_devices_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release,
+};
+
+static int __init component_debug_init(void)
+{
+	component_debugfs_dir = debugfs_create_dir("component", NULL);
+
+	if (!component_debugfs_dir)
+		return -ENOMEM;
+
+	return 0;
+}
+
+core_initcall(component_debug_init);
+
+static void component_master_debugfs_add(struct master *m)
+{
+	m->dentry = debugfs_create_file(dev_name(m->dev), 0444,
+					component_debugfs_dir,
+					m, &component_devices_fops);
+}
+
+static void component_master_debugfs_del(struct master *m)
+{
+	debugfs_remove(m->dentry);
+	m->dentry = NULL;
+}
+
+#else
+
+static void component_master_debugfs_add(struct master *m)
+{
+}
+
+static void component_master_debugfs_del(struct master *m)
+{
+}
+
+#endif
+
 static struct master *__master_find(struct device *dev,
 	const struct component_master_ops *ops)
 {
@@ -290,6 +371,7 @@ static void free_master(struct master *master)
 	struct component_match *match = master->match;
 	int i;
 
+	component_master_debugfs_del(master);
 	list_del(&master->node);
 
 	if (match) {
@@ -323,6 +405,7 @@ int component_master_add_with_match(struct device *dev,
 	master->ops = ops;
 	master->match = match;
 
+	component_master_debugfs_add(master);
 	/* Add to the list of available masters. */
 	mutex_lock(&component_mutex);
 	list_add(&master->node, &masters);
-- 
2.7.4

Re: [PATCH v2] component: add debugfs support

[Greg Kroah-Hartman]

On Wed, Nov 15, 2017 at 01:05:26PM +0100, Maciej Purski wrote:
> Add 'component' directory to debugfs. Create a new file for each master,
> when a master is added. Remove it on a master deletion.
> 
> Show a list of devices matched with master and indicate if
> master's components were successfully added and if the whole master is
> bound.
> 
> Signed-off-by: Maciej Purski <m.purski@samsung.com>
> ---
> Changes in v2:
> - use seq_printf() instead of seq_puts() when printing headers
> - move whole debugfs code to the file beginning in order to avoid
>   forward declarations or using multiple ifdefs
> ---
>  drivers/base/component.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 83 insertions(+)
> 
> diff --git a/drivers/base/component.c b/drivers/base/component.c
> index 89b032f..8745ad9 100644
> --- a/drivers/base/component.c
> +++ b/drivers/base/component.c
> @@ -17,6 +17,7 @@
>  #include <linux/module.h>
>  #include <linux/mutex.h>
>  #include <linux/slab.h>
> +#include <linux/debugfs.h>
>  
>  struct component;
>  
> @@ -41,6 +42,7 @@ struct master {
>  	const struct component_master_ops *ops;
>  	struct device *dev;
>  	struct component_match *match;
> +	struct dentry *dentry;
>  };
>  
>  struct component {
> @@ -56,6 +58,85 @@ static DEFINE_MUTEX(component_mutex);
>  static LIST_HEAD(component_list);
>  static LIST_HEAD(masters);
>  
> +#ifdef CONFIG_DEBUG_FS
> +
> +static struct dentry *component_debugfs_dir;
> +
> +static int component_devices_show(struct seq_file *s, void *data)
> +{
> +	struct master *m = s->private;
> +	struct component_match *match = m->match;
> +	size_t i;
> +
> +	mutex_lock(&component_mutex);
> +	seq_printf(s, "%-40s %20s\n", "master name", "status");
> +	seq_puts(s, "-------------------------------------------------------------\n");
> +	seq_printf(s, "%-40s %20s\n\n",
> +		   dev_name(m->dev), m->bound ? "bound" : "not bound");
> +
> +	seq_printf(s, "%-40s %20s\n", "device name", "status");
> +	seq_puts(s, "-------------------------------------------------------------\n");
> +	for (i = 0; i < match->num; i++) {
> +		struct device *d = (struct device *)match->compare[i].data;
> +
> +		seq_printf(s, "%-40s %20s\n", dev_name(d),
> +			   match->compare[i].component ?
> +			   "registered" : "not registered");
> +	}
> +	mutex_unlock(&component_mutex);
> +
> +	return 0;
> +}
> +
> +static int component_devices_open(struct inode *inode, struct file *file)
> +{
> +	return single_open(file, component_devices_show, inode->i_private);
> +}
> +
> +static const struct file_operations component_devices_fops = {
> +	.open = component_devices_open,
> +	.read = seq_read,
> +	.llseek = seq_lseek,
> +	.release = single_release,
> +};
> +
> +static int __init component_debug_init(void)
> +{
> +	component_debugfs_dir = debugfs_create_dir("component", NULL);
> +
> +	if (!component_debugfs_dir)
> +		return -ENOMEM;

No need to test this at all, you should never fail anything if debugfs
is not working properly, just continue and move on.  The result of any
debugfs call can be fed back into any other debugfs call without any
problems.

> +
> +	return 0;
> +}
> +
> +core_initcall(component_debug_init);
> +
> +static void component_master_debugfs_add(struct master *m)
> +{
> +	m->dentry = debugfs_create_file(dev_name(m->dev), 0444,
> +					component_debugfs_dir,
> +					m, &component_devices_fops);

See, you do it well here, do the same thing when you create the initial
debugfs directory.

Also, "component" is very vague, can you think of a better term for
this?  "device_component"?  "dev_component"?  Something else?  But I
don't care, if you really like "component", that's fine.

thanks,

greg k-h

Re: [PATCH v2] component: add debugfs support

[Maciej Purski]

On 11/15/2017 03:01 PM, Greg Kroah-Hartman wrote:
> On Wed, Nov 15, 2017 at 01:05:26PM +0100, Maciej Purski wrote:
>> Add 'component' directory to debugfs. Create a new file for each master,
>> when a master is added. Remove it on a master deletion.
>>
>> Show a list of devices matched with master and indicate if
>> master's components were successfully added and if the whole master is
>> bound.
>>
>> Signed-off-by: Maciej Purski <m.purski@samsung.com>
>> ---
>> Changes in v2:
>> - use seq_printf() instead of seq_puts() when printing headers
>> - move whole debugfs code to the file beginning in order to avoid
>>    forward declarations or using multiple ifdefs
>> ---
>>   drivers/base/component.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 83 insertions(+)
>>
>> diff --git a/drivers/base/component.c b/drivers/base/component.c
>> index 89b032f..8745ad9 100644
>> --- a/drivers/base/component.c
>> +++ b/drivers/base/component.c
>> @@ -17,6 +17,7 @@
>>   #include <linux/module.h>
>>   #include <linux/mutex.h>
>>   #include <linux/slab.h>
>> +#include <linux/debugfs.h>
>>   
>>   struct component;
>>   
>> @@ -41,6 +42,7 @@ struct master {
>>   	const struct component_master_ops *ops;
>>   	struct device *dev;
>>   	struct component_match *match;
>> +	struct dentry *dentry;
>>   };
>>   
>>   struct component {
>> @@ -56,6 +58,85 @@ static DEFINE_MUTEX(component_mutex);
>>   static LIST_HEAD(component_list);
>>   static LIST_HEAD(masters);
>>   
>> +#ifdef CONFIG_DEBUG_FS
>> +
>> +static struct dentry *component_debugfs_dir;
>> +
>> +static int component_devices_show(struct seq_file *s, void *data)
>> +{
>> +	struct master *m = s->private;
>> +	struct component_match *match = m->match;
>> +	size_t i;
>> +
>> +	mutex_lock(&component_mutex);
>> +	seq_printf(s, "%-40s %20s\n", "master name", "status");
>> +	seq_puts(s, "-------------------------------------------------------------\n");
>> +	seq_printf(s, "%-40s %20s\n\n",
>> +		   dev_name(m->dev), m->bound ? "bound" : "not bound");
>> +
>> +	seq_printf(s, "%-40s %20s\n", "device name", "status");
>> +	seq_puts(s, "-------------------------------------------------------------\n");
>> +	for (i = 0; i < match->num; i++) {
>> +		struct device *d = (struct device *)match->compare[i].data;
>> +
>> +		seq_printf(s, "%-40s %20s\n", dev_name(d),
>> +			   match->compare[i].component ?
>> +			   "registered" : "not registered");
>> +	}
>> +	mutex_unlock(&component_mutex);
>> +
>> +	return 0;
>> +}
>> +
>> +static int component_devices_open(struct inode *inode, struct file *file)
>> +{
>> +	return single_open(file, component_devices_show, inode->i_private);
>> +}
>> +
>> +static const struct file_operations component_devices_fops = {
>> +	.open = component_devices_open,
>> +	.read = seq_read,
>> +	.llseek = seq_lseek,
>> +	.release = single_release,
>> +};
>> +
>> +static int __init component_debug_init(void)
>> +{
>> +	component_debugfs_dir = debugfs_create_dir("component", NULL);
>> +
>> +	if (!component_debugfs_dir)
>> +		return -ENOMEM;
> 
> No need to test this at all, you should never fail anything if debugfs
> is not working properly, just continue and move on.  The result of any
> debugfs call can be fed back into any other debugfs call without any
> problems.
> 
>> +
>> +	return 0;
>> +}
>> +
>> +core_initcall(component_debug_init);
>> +
>> +static void component_master_debugfs_add(struct master *m)
>> +{
>> +	m->dentry = debugfs_create_file(dev_name(m->dev), 0444,
>> +					component_debugfs_dir,
>> +					m, &component_devices_fops);
> 
> See, you do it well here, do the same thing when you create the initial
> debugfs directory.

I'll to this the way you suggest. Thanks.

> 
> Also, "component" is very vague, can you think of a better term for
> this?  "device_component"?  "dev_component"?  Something else?  But I
> don't care, if you really like "component", that's fine.
>

I'm not really attached to that name, so I can change it to one that you suggested.
Maybe Russell King has some preference on that?

Best regards,

Maciej Purski

> thanks,
> 
> greg k-h
> 
> 
>