[PATCH] regulator: add a data summary tree in debugfs

[PATCH] regulator: add a data summary tree in debugfs

[Heiko Stübner]

On modern systems the regulator hierarchy can get quite long and nested
with regulators supplying other regulators. In some cases when debugging
it might be nice to get a tree of these regulators, their consumers
and the regulation constraints in one go.

To achieve this add a regulator_summary sysfs node, similar to
clk_summary in the common clock framework, that walks the regulator
list and creates a tree out of the regulators, their consumers and
core per-regulator settings.

On a rk3288-firefly the regulator_summary would for example look
something like:

 regulator                      use,open,bypass      value        min        max
--------------------------------------------------------------------------------
 vcc_sys                          0,  12,     0     5000mV     5000mV     5000mV
    vcc_lan                       1,   1,     0     3300mV     3300mV     3300mV
       ff290000.ethernet                                          0mV        0mV
    vcca_33                       0,   0,     0     3300mV     3300mV     3300mV
    vcca_18                       0,   0,     0     1800mV     1800mV     1800mV
    vdd10_lcd                     0,   0,     0     1000mV     1000mV     1000mV
    vccio_sd                      0,   0,     0     3300mV     3300mV     3300mV
    vcc_20                        0,   3,     0     2000mV     2000mV     2000mV
       vcc18_lcd                  0,   0,     0     1800mV     1800mV     1800mV
       vcc_18                     0,   2,     0     1800mV     1800mV     1800mV
          ff100000.saradc                                         0mV        0mV
          ff0d0000.dwmmc                                       1650mV     1950mV
       vdd_10                     0,   0,     0     1000mV     1000mV     1000mV
    vdd_log                       0,   0,     0     1100mV     1100mV     1100mV
    vcc_io                        0,   3,     0     3300mV     3300mV     3300mV
       ff0f0000.dwmmc                                          3300mV     3400mV
       vcc_flash                  1,   1,     0     1800mV     1800mV     1800mV
          ff0f0000.dwmmc                                       1700mV     1950mV
       vcc_sd                     1,   1,     0     3300mV     3300mV     3300mV
          ff0c0000.dwmmc                                       3300mV     3400mV
    vcc_ddr                       0,   0,     0     1200mV     1200mV     1200mV
    vdd_gpu                       0,   0,     0     1000mV      850mV     1350mV
    vdd_cpu                       0,   1,     0      900mV      850mV     1350mV
       cpu0                                                     900mV      900mV
    vcc_5v                        0,   2,     0     5000mV     5000mV     5000mV
       vcc_otg_5v                 0,   0,     0     5000mV     5000mV     5000mV
       vcc_host_5v                0,   0,     0     5000mV     5000mV     5000mV

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/regulator/core.c | 113 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 113 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index a4a8a6d..950fe22 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3936,6 +3936,116 @@ static const struct file_operations supply_map_fops = {
 #endif
 };
 
+#ifdef CONFIG_DEBUG_FS
+static void regulator_summary_show_subtree(struct seq_file *s,
+					   struct regulator_dev *rdev,
+					   int level)
+{
+	struct list_head *list = s->private;
+	struct regulator_dev *child;
+	struct regulation_constraints *c;
+	struct regulator *consumer;
+
+	if (!rdev)
+		return;
+
+	mutex_lock(&rdev->mutex);
+
+	seq_printf(s, "%*s%-*s %3d,%4d,%6d ",
+		   level * 3 + 1, "",
+		   30 - level * 3, rdev_get_name(rdev),
+		   rdev->use_count, rdev->open_count, rdev->bypass_count);
+
+	switch (rdev->desc->type) {
+	case REGULATOR_VOLTAGE:
+		seq_printf(s, "%8dmV ",
+			   _regulator_get_voltage(rdev) / 1000);
+		break;
+	case REGULATOR_CURRENT:
+		seq_printf(s, "%8dmA ",
+			   _regulator_get_current_limit(rdev) / 1000);
+		break;
+	}
+
+	c = rdev->constraints;
+	if (c) {
+		switch (rdev->desc->type) {
+		case REGULATOR_VOLTAGE:
+			seq_printf(s, "%8dmV %8dmV ",
+				   c->min_uV / 1000, c->max_uV / 1000);
+			break;
+		case REGULATOR_CURRENT:
+			seq_printf(s, "%8dmA %8dmA ",
+				   c->min_uA / 1000, c->max_uA / 1000);
+			break;
+		}
+	}
+
+	seq_puts(s, "\n");
+
+	list_for_each_entry(consumer, &rdev->consumer_list, list) {
+		if (consumer->dev->class == &regulator_class)
+			continue;
+
+		seq_printf(s, "%*s%-*s ",
+			   (level + 1) * 3 + 1, "",
+		   30 - (level + 1) * 3, dev_name(consumer->dev));
+
+		if (rdev->desc->type == REGULATOR_VOLTAGE)
+			seq_printf(s, "%35dmV %8dmV",
+				   consumer->min_uV / 1000,
+				   consumer->max_uV / 1000);
+
+		seq_puts(s, "\n");
+	}
+
+	mutex_unlock(&rdev->mutex);
+
+	list_for_each_entry(child, list, list) {
+		if (!child->supply || child->supply->rdev != rdev)
+			continue;
+
+		regulator_summary_show_subtree(s, child, level + 1);
+	}
+}
+
+static int regulator_summary_show(struct seq_file *s, void *data)
+{
+	struct list_head *list = s->private;
+	struct regulator_dev *rdev;
+
+	seq_puts(s, " regulator                      use,open,bypass      value        min        max\n");
+	seq_puts(s, "--------------------------------------------------------------------------------\n");
+
+	mutex_lock(&regulator_list_mutex);
+
+	list_for_each_entry(rdev, list, list) {
+		if (rdev->supply)
+			continue;
+
+		regulator_summary_show_subtree(s, rdev, 0);
+	}
+
+	mutex_unlock(&regulator_list_mutex);
+
+	return 0;
+}
+
+static int regulator_summary_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, regulator_summary_show, inode->i_private);
+}
+#endif
+
+static const struct file_operations regulator_summary_fops = {
+#ifdef CONFIG_DEBUG_FS
+	.open		= regulator_summary_open,
+	.read		= seq_read,
+	.llseek		= seq_lseek,
+	.release	= single_release,
+#endif
+};
+
 static int __init regulator_init(void)
 {
 	int ret;
@@ -3949,6 +4059,9 @@ static int __init regulator_init(void)
 	debugfs_create_file("supply_map", 0444, debugfs_root, NULL,
 			    &supply_map_fops);
 
+	debugfs_create_file("regulator_summary", 0444, debugfs_root,
+			    &regulator_list, &regulator_summary_fops);
+
 	regulator_dummy_init();
 
 	return ret;
-- 
2.1.4

Re: [PATCH] regulator: add a data summary tree in debugfs

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1408 bytes --]

On Mon, Apr 06, 2015 at 02:04:47AM +0200, Heiko Stübner wrote:

> +	switch (rdev->desc->type) {
> +	case REGULATOR_VOLTAGE:
> +		seq_printf(s, "%8dmV ",
> +			   _regulator_get_voltage(rdev) / 1000);
> +		break;
> +	case REGULATOR_CURRENT:
> +		seq_printf(s, "%8dmA ",
> +			   _regulator_get_current_limit(rdev) / 1000);
> +		break;
> +	}

We have current limits for voltage regulators too.

> +		if (rdev->desc->type == REGULATOR_VOLTAGE)
> +			seq_printf(s, "%35dmV %8dmV",
> +				   consumer->min_uV / 1000,
> +				   consumer->max_uV / 1000);

switch statements please.

> +	list_for_each_entry(child, list, list) {
> +		if (!child->supply || child->supply->rdev != rdev)
> +			continue;

Shouldn't we be complaining if the supply of a child isn't the parent?

> +static int regulator_summary_show(struct seq_file *s, void *data)
> +{
> +	struct list_head *list = s->private;
> +	struct regulator_dev *rdev;
> +
> +	seq_puts(s, " regulator                      use,open,bypass      value        min        max\n");

I can't help but think that this would look better with spaces rather
than commas both here and in the table itself.  We also seem to have too
much space for the voltages - we're unlikely to see voltages over 10V
but the space reserved looks to be enough for 10kV.  I think users with
such regulators can probably tolerate a little misformatting.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[PATCH v2] regulator: add a summary tree in debugfs

[Heiko Stübner]

On modern systems the regulator hierarchy can get quite long and nested
with regulators supplying other regulators. In some cases when debugging
it might be nice to get a tree of these regulators, their consumers
and the regulation constraints in one go.

To achieve this add a regulator_summary sysfs node, similar to
clk_summary in the common clock framework, that walks the regulator
list and creates a tree out of the regulators, their consumers and
core per-regulator settings.

On a rk3288-firefly the regulator_summary would for example look
something like:

 regulator                      use open bypass   value     min     max
-----------------------------------------------------------------------
 vcc_sys                          0   12      0  5000mV  5000mV  5000mV
    vcc_lan                       1    1      0  3300mV  3300mV  3300mV
       ff290000.ethernet                                    0mV     0mV
    vcca_33                       0    0      0  3300mV  3300mV  3300mV
    vcca_18                       0    0      0  1800mV  1800mV  1800mV
    vdd10_lcd                     0    0      0  1000mV  1000mV  1000mV
    vccio_sd                      0    0      0  3300mV  3300mV  3300mV
    vcc_20                        0    3      0  2000mV  2000mV  2000mV
       vcc18_lcd                  0    0      0  1800mV  1800mV  1800mV
       vcc_18                     0    2      0  1800mV  1800mV  1800mV
          ff100000.saradc                                   0mV     0mV
          ff0d0000.dwmmc                                 1650mV  1950mV
       vdd_10                     0    0      0  1000mV  1000mV  1000mV
    vdd_log                       0    0      0  1100mV  1100mV  1100mV
    vcc_io                        0    3      0  3300mV  3300mV  3300mV
       ff0f0000.dwmmc                                    3300mV  3400mV
       vcc_flash                  1    1      0  1800mV  1800mV  1800mV
          ff0f0000.dwmmc                                 1700mV  1950mV
       vcc_sd                     1    1      0  3300mV  3300mV  3300mV
          ff0c0000.dwmmc                                 3300mV  3400mV
    vcc_ddr                       0    0      0  1200mV  1200mV  1200mV
    vdd_gpu                       0    0      0  1000mV   850mV  1350mV
    vdd_cpu                       0    1      0   900mV   850mV  1350mV
       cpu0                                               900mV   900mV
    vcc_5v                        0    2      0  5000mV  5000mV  5000mV
       vcc_otg_5v                 0    0      0  5000mV  5000mV  5000mV
       vcc_host_5v                0    0      0  5000mV  5000mV  5000mV
 regulator-dummy                  0    0      0     0mV     0mV     0mV

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
changes since v1:
- comsetics: space instead of ","; decrease width of voltage/current columns
- output current-limit for current regulators

The questioned statement
>> +             if (!child->supply || child->supply->rdev != rdev)
>> +                     continue;
>> Shouldn't we be complaining if the supply of a child isn't the parent?

is meant to single out only those regulators from the global list that
are actual children of the current rdev. Regulators with !child->supply
are the root ones and handled in regulator_summary_show while regulators
with different parents will be handled when their parent is processed.

So there shouldn't be a need to complain. Or alternatively I'm just
overlooking a different way to do the whole parent->children resolution.


 drivers/regulator/core.c | 121 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 121 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index a4a8a6d..fb2c8d5 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3936,6 +3936,124 @@ static const struct file_operations supply_map_fops = {
 #endif
 };
 
+#ifdef CONFIG_DEBUG_FS
+static void regulator_summary_show_subtree(struct seq_file *s,
+					   struct regulator_dev *rdev,
+					   int level)
+{
+	struct list_head *list = s->private;
+	struct regulator_dev *child;
+	struct regulation_constraints *c;
+	struct regulator *consumer;
+
+	if (!rdev)
+		return;
+
+	mutex_lock(&rdev->mutex);
+
+	seq_printf(s, "%*s%-*s %3d %4d %6d ",
+		   level * 3 + 1, "",
+		   30 - level * 3, rdev_get_name(rdev),
+		   rdev->use_count, rdev->open_count, rdev->bypass_count);
+
+	switch (rdev->desc->type) {
+	case REGULATOR_VOLTAGE:
+		seq_printf(s, "%5dmV ",
+			   _regulator_get_voltage(rdev) / 1000);
+		break;
+	case REGULATOR_CURRENT:
+		seq_printf(s, "%5dmA ",
+			   _regulator_get_current_limit(rdev) / 1000);
+		break;
+	}
+
+	c = rdev->constraints;
+	if (c) {
+		switch (rdev->desc->type) {
+		case REGULATOR_VOLTAGE:
+			seq_printf(s, "%5dmV %5dmV ",
+				   c->min_uV / 1000, c->max_uV / 1000);
+			break;
+		case REGULATOR_CURRENT:
+			seq_printf(s, "%5dmA %5dmA ",
+				   c->min_uA / 1000, c->max_uA / 1000);
+			break;
+		}
+	}
+
+	seq_puts(s, "\n");
+
+	list_for_each_entry(consumer, &rdev->consumer_list, list) {
+		if (consumer->dev->class == &regulator_class)
+			continue;
+
+		seq_printf(s, "%*s%-*s ",
+			   (level + 1) * 3 + 1, "",
+			   30 - (level + 1) * 3, dev_name(consumer->dev));
+
+		switch (rdev->desc->type) {
+		case REGULATOR_VOLTAGE:
+			seq_printf(s, "%29dmV %5dmV",
+				   consumer->min_uV / 1000,
+				   consumer->max_uV / 1000);
+			break;
+		case REGULATOR_CURRENT:
+			seq_printf(s, "%37dmA",
+				regulator_get_current_limit(consumer) / 1000);
+			break;
+		}
+
+		seq_puts(s, "\n");
+	}
+
+	mutex_unlock(&rdev->mutex);
+
+	list_for_each_entry(child, list, list) {
+		/* handle only non-root regulators supplied by current rdev */
+		if (!child->supply || child->supply->rdev != rdev)
+			continue;
+
+		regulator_summary_show_subtree(s, child, level + 1);
+	}
+}
+
+static int regulator_summary_show(struct seq_file *s, void *data)
+{
+	struct list_head *list = s->private;
+	struct regulator_dev *rdev;
+
+	seq_puts(s, " regulator                      use open bypass   value     min     max\n");
+	seq_puts(s, "-----------------------------------------------------------------------\n");
+
+	mutex_lock(&regulator_list_mutex);
+
+	list_for_each_entry(rdev, list, list) {
+		if (rdev->supply)
+			continue;
+
+		regulator_summary_show_subtree(s, rdev, 0);
+	}
+
+	mutex_unlock(&regulator_list_mutex);
+
+	return 0;
+}
+
+static int regulator_summary_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, regulator_summary_show, inode->i_private);
+}
+#endif
+
+static const struct file_operations regulator_summary_fops = {
+#ifdef CONFIG_DEBUG_FS
+	.open		= regulator_summary_open,
+	.read		= seq_read,
+	.llseek		= seq_lseek,
+	.release	= single_release,
+#endif
+};
+
 static int __init regulator_init(void)
 {
 	int ret;
@@ -3949,6 +4067,9 @@ static int __init regulator_init(void)
 	debugfs_create_file("supply_map", 0444, debugfs_root, NULL,
 			    &supply_map_fops);
 
+	debugfs_create_file("regulator_summary", 0444, debugfs_root,
+			    &regulator_list, &regulator_summary_fops);
+
 	regulator_dummy_init();
 
 	return ret;
-- 
2.1.4

Re: [PATCH v2] regulator: add a summary tree in debugfs

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 331 bytes --]

On Tue, Apr 07, 2015 at 04:16:39PM +0200, Heiko Stübner wrote:

> - output current-limit for current regulators

What I meant here was that we should try to output the current limit for
all regulators, not just for current regulators.  I've applied this as
is though since it's basically OK, can you send a followup please?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH v2] regulator: add a summary tree in debugfs

[Heiko Stübner]

Am Mittwoch, 8. April 2015, 18:38:25 schrieb Mark Brown:
> On Tue, Apr 07, 2015 at 04:16:39PM +0200, Heiko Stübner wrote:
> > - output current-limit for current regulators
> 
> What I meant here was that we should try to output the current limit for
> all regulators, not just for current regulators.  I've applied this as
> is though since it's basically OK, can you send a followup please?

ah I didn't know regulator_get_current_limit() was valid for non 
REGULATOR_CURRENT devices too.

Followup follows tomorrow - or today as I've just looked at the clock :-)


Heiko