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

[Mark Brown <broonie@kernel.org>]

[-- 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 --]