Re: [PATCH] PM / Domains: add debugfs listing of struct generic_pm_domain-s

[Greg Kroah-Hartman]

On Wed, Sep 10, 2014 at 01:28:56PM +0200, Maciej Matraszek wrote:
> Add /sys/kernel/debug/pm_genpd/pm_genpd_summary file, which
> lists power domains in the system, their statuses and attached devices,
> resembling /sys/kernel/debug/clk/clk_summary.
> 
> Currently it is impossible to inspect (from userland) whether
> a power domain is on or off. And, if it is on, which device blocks it
> from powering down. This change allows developers working on
> embedded devices power efficiency to list all necessary information
> about generic power domains in one place.
> 
> The content of pm_genpd/pm_genpd_summary file is generated by iterating
> over all generic power domain in the system, and, for each,
> over registered devices.
> 
> Example output:
> $ cat  /sys/kernel/debug/pm_genpd/pm_genpd_summary
>     domain                      status         slaves
>            /device                                      runtime status
> ----------------------------------------------------------------------
> isp-power-domain                off
>     /devices/soc/12180000.fimc-is                       suspended
>     /devices/soc/12180000.fimc-is/120a0000.fimc-lite    suspended
> lcd0-power-domain               on
>     /devices/soc/11c80000.dsi                           active
>     /devices/soc/11c00000.fimd                          active
>     /devices/soc/11e20000.sysmmu                        suspended
> g3d-power-domain                off
>     /devices/soc/13001000.gpu                           suspended
> mfc-power-domain                off
>     /devices/soc/13400000.mfc-codec                     suspended
>     /devices/soc/13620000.sysmmu                        suspended
> cam-power-domain                off
>     /devices/soc/11830000.jpeg-codec                    suspended
>     /devices/soc/11a60000.sysmmu                        suspended
>     /devices/soc/12180000.fimc-is/120c0000.csis         suspended
> 
> To enable this feature, compile the kernel with debugfs
> and CONFIG_PM_ADVANCED_DEBUG enabled.
> 
> Signed-off-by: Maciej Matraszek <m.matraszek@xxxxxxxxxxx>
> ---
>  drivers/base/power/domain.c | 153 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 153 insertions(+)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index eee55c1e5fde..d1aa731ba0db 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2189,3 +2189,156 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
>  	list_add(&genpd->gpd_list_node, &gpd_list);
>  	mutex_unlock(&gpd_list_lock);
>  }
> +
> +
> +/***        debugfs support        ***/
> +
> +#ifdef CONFIG_PM_ADVANCED_DEBUG
> +#include <linux/pm.h>
> +#include <linux/device.h>
> +#include <linux/debugfs.h>
> +#include <linux/kobject.h>
> +static struct dentry *pm_genpd_debugfs_dir;
> +
> +/*
> + * TODO: This function is a slightly modified version of rtpm_status_show
> + * from sysfs.c, but dependencies between PM_GENERIC_DOMAINS and PM_RUNTIME
> + * are too loose to generalize it.
> + */
> +#ifdef CONFIG_PM_RUNTIME
> +static void rtpm_status_str(struct seq_file *s, struct device *dev)
> +{
> +	static const char * const status_lookup[] = {
> +		[RPM_ACTIVE] = "active",
> +		[RPM_RESUMING] = "resuming",
> +		[RPM_SUSPENDED] = "suspended",
> +		[RPM_SUSPENDING] = "suspending"
> +	};
> +	const char *p = "";
> +
> +	if (dev->power.runtime_error)
> +		p = "error";
> +	else if (dev->power.disable_depth)
> +		p = "unsupported";
> +	else if (dev->power.runtime_status < ARRAY_SIZE(status_lookup))
> +		p = status_lookup[dev->power.runtime_status];
> +	else
> +		WARN_ON(1);
> +
> +	seq_puts(s, p);
> +}
> +#else
> +static void rtpm_status_str(struct seq_file *s, struct device *dev)
> +{
> +	seq_puts(s, "active");
> +}
> +#endif
> +
> +static int pm_genpd_summary_one(struct seq_file *s,
> +		struct generic_pm_domain *gpd)
> +{
> +	static const char * const status_lookup[] = {
> +		[GPD_STATE_ACTIVE] = "on",
> +		[GPD_STATE_WAIT_MASTER] = "wait-master",
> +		[GPD_STATE_BUSY] = "busy",
> +		[GPD_STATE_REPEAT] = "off-in-progress",
> +		[GPD_STATE_POWER_OFF] = "off"
> +	};
> +	struct pm_domain_data *pm_data;
> +	const char *kobj_path;
> +	struct gpd_link *link;
> +	int ret;
> +
> +	ret = mutex_lock_interruptible(&gpd->lock);
> +	if (ret)
> +		return -ERESTARTSYS;
> +
> +	if (WARN_ON(gpd->status >= ARRAY_SIZE(status_lookup)))
> +		goto exit;
> +	seq_printf(s, "%-30s  %-15s  ", gpd->name, status_lookup[gpd->status]);
> +
> +	/*
> +	 * Modifications on the list require holding locks on both
> +	 * master and slave, so we are safe.
> +	 * Also gpd->name is immutable.
> +	 */
> +	list_for_each_entry(link, &gpd->master_links, master_node) {
> +		seq_printf(s, "%s", link->slave->name);
> +		if (!list_is_last(&link->master_node, &gpd->master_links))
> +			seq_puts(s, ", ");
> +	}
> +
> +	list_for_each_entry(pm_data, &gpd->dev_list, list_node) {
> +		kobj_path = kobject_get_path(&pm_data->dev->kobj, GFP_KERNEL);
> +		if (kobj_path == NULL)
> +			continue;
> +
> +		seq_printf(s, "\n    %-50s  ", kobj_path);
> +		rtpm_status_str(s, pm_data->dev);
> +		kfree(kobj_path);
> +	}
> +
> +	seq_puts(s, "\n");
> +exit:
> +	mutex_unlock(&gpd->lock);
> +
> +	return 0;
> +}
> +
> +static int pm_genpd_summary_show(struct seq_file *s, void *data)
> +{
> +	struct generic_pm_domain *gpd;
> +	int ret = 0;
> +
> +	seq_puts(s, "    domain                      status         slaves\n");
> +	seq_puts(s, "           /device                                      runtime status\n");
> +	seq_puts(s, "----------------------------------------------------------------------\n");
> +
> +	ret = mutex_lock_interruptible(&gpd_list_lock);
> +	if (ret)
> +		return -ERESTARTSYS;
> +
> +	list_for_each_entry(gpd, &gpd_list, gpd_list_node) {
> +		ret = pm_genpd_summary_one(s, gpd);
> +		if (ret)
> +			break;
> +	}
> +	mutex_unlock(&gpd_list_lock);
> +
> +	return ret;
> +}
> +
> +static int pm_genpd_summary_open(struct inode *inode, struct file *file)
> +{
> +	return single_open(file, pm_genpd_summary_show, inode->i_private);
> +}
> +
> +static const struct file_operations pm_genpd_summary_fops = {
> +	.open = pm_genpd_summary_open,
> +	.read = seq_read,
> +	.llseek = seq_lseek,
> +	.release = single_release,
> +};
> +
> +static int __init pm_genpd_debug_init(void)
> +{
> +	struct dentry *d;
> +
> +	pm_genpd_debugfs_dir = debugfs_create_dir("pm_genpd", NULL);
> +
> +	/* No debugfs */
> +	if (IS_ERR(pm_genpd_debugfs_dir))
> +		return PTR_ERR(pm_genpd_debugfs_dir);

You shouldn't care at all, just do this check:

> +	if (!pm_genpd_debugfs_dir)
> +		return -ENOMEM;

And all will be fine, because:

> +	d = debugfs_create_file("pm_genpd_summary", S_IRUGO,
> +			pm_genpd_debugfs_dir, NULL, &pm_genpd_summary_fops);

That call will resolve away to nothing if debugfs is not enabled, and it
doesn't make much sense to do anything with an "error" of no debugfs.

> +	if (!d)
> +		return -ENOMEM;
> +
> +	return 0;
> +}
> +late_initcall(pm_genpd_debug_init);
> +#endif /* CONFIG_PM_ADVANCED_DEBUG */

No tear-down of the debugfs files on shutdown?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/