[PATCH v4 1/4] regulator: helper routine to extract regulator_init_data

[PATCH v4 1/4] regulator: helper routine to extract regulator_init_data

[Rajendra Nayak]

The helper routine is meant to be used by the regulator drivers
to extract the regulator_init_data structure from the data
that is passed from device tree.
'consumer_supplies' which is part of regulator_init_data is not extracted
as the regulator consumer mappings are passed through DT differently,
implemented in subsequent patches.
Similarly the regulator<-->parent/supply mapping is handled in
subsequent patches.

Also add documentation for regulator bindings to be used to pass
regulator_init_data struct information from device tree.

Some of the regulator properties which are linux and board specific,
are left out since its not clear if they can
be in someway embedded into the kernel or passed in from DT.
They will be revisited later.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 .../devicetree/bindings/regulator/regulator.txt    |   33 ++++++++
 drivers/regulator/Kconfig                          |    7 ++
 drivers/regulator/Makefile                         |    1 +
 drivers/regulator/of_regulator.c                   |   81 ++++++++++++++++++++
 include/linux/regulator/of_regulator.h             |   20 +++++
 5 files changed, 142 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/regulator/regulator.txt
 create mode 100644 drivers/regulator/of_regulator.c
 create mode 100644 include/linux/regulator/of_regulator.h

diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt
new file mode 100644
index 0000000..c488bf3
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/regulator.txt
@@ -0,0 +1,33 @@
+Voltage/Current Regulators
+
+Optional properties:
+- regulator-name: A string used as a descriptive name for regulator outputs
+- regulator-min-uV: smallest voltage consumers may set
+- regulator-max-uV: largest voltage consumers may set
+- regulator-uV-offset: Offset applied to voltages to compensate for voltage drops
+- regulator-min-uA: smallest current consumers may set
+- regulator-max-uA: largest current consumers may set
+- regulator-always-on: boolean, regulator should never be disabled
+- regulator-boot-on: bootloader/firmware enabled regulator
+- <name>-supply: phandle to the parent supply/regulator node
+
+Example:
+
+	xyzreg: regulator@0 {
+		regulator-min-uV = <1000000>;
+		regulator-max-uV = <2500000>;
+		regulator-always-on;
+		vin-supply = <&vin>;
+	};
+
+The same binding used by a regulator to reference its
+supply can be used by any consumer to reference its
+regulator/supply
+
+Example of a device node referencing a regulator node,
+
+	devicenode: node@0x0 {
+		...
+		...
+		<name>-supply = <&xyzreg>;
+	};
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index c7fd2c0..8106958 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -64,6 +64,13 @@ config REGULATOR_USERSPACE_CONSUMER
 
           If unsure, say no.
 
+config OF_REGULATOR
+	bool
+	depends on OF
+	help
+	  OpenFirmware regulator framework helper routines that can
+	  used by regulator drivers to extract data from device tree.
+
 config REGULATOR_BQ24022
 	tristate "TI bq24022 Dual Input 1-Cell Li-Ion Charger IC"
 	help
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 040d5aa..e6bc009 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_REGULATOR) += core.o dummy.o
 obj-$(CONFIG_REGULATOR_FIXED_VOLTAGE) += fixed.o
 obj-$(CONFIG_REGULATOR_VIRTUAL_CONSUMER) += virtual.o
 obj-$(CONFIG_REGULATOR_USERSPACE_CONSUMER) += userspace-consumer.o
+obj-$(CONFIG_OF_REGULATOR) += of_regulator.o
 
 obj-$(CONFIG_REGULATOR_AD5398) += ad5398.o
 obj-$(CONFIG_REGULATOR_BQ24022) += bq24022.o
diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
new file mode 100644
index 0000000..798faaa
--- /dev/null
+++ b/drivers/regulator/of_regulator.c
@@ -0,0 +1,81 @@
+/*
+ * OF helpers for regulator framework
+ *
+ * Copyright (C) 2011 Texas Instruments, Inc.
+ * Rajendra Nayak <rnayak@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/regulator/machine.h>
+
+static void of_get_regulation_constraints(struct device_node *np,
+					struct regulator_init_data **init_data)
+{
+	const __be32 *min_uV, *max_uV, *uV_offset;
+	const __be32 *min_uA, *max_uA;
+	struct regulation_constraints *constraints = &(*init_data)->constraints;
+
+	constraints->name = of_get_property(np, "regulator-name", NULL);
+
+	min_uV = of_get_property(np, "regulator-min-uV", NULL);
+	if (min_uV)
+		constraints->min_uV = be32_to_cpu(*min_uV);
+	max_uV = of_get_property(np, "regulator-max-uV", NULL);
+	if (max_uV)
+		constraints->max_uV = be32_to_cpu(*max_uV);
+
+	/* Voltage change possible? */
+	if (constraints->min_uV != constraints->max_uV)
+		constraints->valid_ops_mask |= REGULATOR_CHANGE_VOLTAGE;
+
+	uV_offset = of_get_property(np, "regulator-uV-offset", NULL);
+	if (uV_offset)
+		constraints->uV_offset = be32_to_cpu(*uV_offset);
+	min_uA = of_get_property(np, "regulator-min-uA", NULL);
+	if (min_uA)
+		constraints->min_uA = be32_to_cpu(*min_uA);
+	max_uA = of_get_property(np, "regulator-max-uA", NULL);
+	if (max_uA)
+		constraints->max_uA = be32_to_cpu(*max_uA);
+
+	/* Current change possible? */
+	if (constraints->min_uA != constraints->max_uA)
+		constraints->valid_ops_mask |= REGULATOR_CHANGE_CURRENT;
+
+	if (of_find_property(np, "regulator-boot-on", NULL))
+		constraints->boot_on = true;
+
+	if (of_find_property(np, "regulator-always-on", NULL))
+		constraints->always_on = true;
+	else /* status change should be possible if not always on. */
+		constraints->valid_ops_mask |= REGULATOR_CHANGE_STATUS;
+}
+
+/**
+ * of_get_regulator_init_data - extract regulator_init_data structure info
+ * @dev: device requesting for regulator_init_data
+ *
+ * Populates regulator_init_data structure by extracting data from device
+ * tree node, returns a pointer to the populated struture or NULL if memory
+ * alloc fails.
+ */
+struct regulator_init_data *of_get_regulator_init_data(struct device *dev)
+{
+	struct regulator_init_data *init_data;
+
+	if (!dev->of_node)
+		return NULL;
+
+	init_data = devm_kzalloc(dev, sizeof(*init_data), GFP_KERNEL);
+	if (!init_data)
+		return NULL; /* Out of memory? */
+
+	of_get_regulation_constraints(dev->of_node, &init_data);
+	return init_data;
+}
diff --git a/include/linux/regulator/of_regulator.h b/include/linux/regulator/of_regulator.h
new file mode 100644
index 0000000..621c071
--- /dev/null
+++ b/include/linux/regulator/of_regulator.h
@@ -0,0 +1,20 @@
+/*
+ * OpenFirmware regulator support routines
+ *
+ */
+
+#ifndef __LINUX_OF_REG_H
+#define __LINUX_OF_REG_H
+
+#if defined(CONFIG_OF_REGULATOR)
+extern struct regulator_init_data
+	*of_get_regulator_init_data(struct device *dev);
+#else
+static inline struct regulator_init_data
+	*of_get_regulator_init_data(struct device *dev)
+{
+	return NULL;
+}
+#endif /* CONFIG_OF_REGULATOR */
+
+#endif /* __LINUX_OF_REG_H */
-- 
1.7.1

Re: [PATCH v4 1/4] regulator: helper routine to extract regulator_init_data

[Olof Johansson]

On Thu, Oct 27, 2011 at 06:54:24PM +0530, Rajendra Nayak wrote:
> The helper routine is meant to be used by the regulator drivers
> to extract the regulator_init_data structure from the data
> that is passed from device tree.
> 'consumer_supplies' which is part of regulator_init_data is not extracted
> as the regulator consumer mappings are passed through DT differently,
> implemented in subsequent patches.
> Similarly the regulator<-->parent/supply mapping is handled in
> subsequent patches.
> 
> Also add documentation for regulator bindings to be used to pass
> regulator_init_data struct information from device tree.
> 
> Some of the regulator properties which are linux and board specific,
> are left out since its not clear if they can
> be in someway embedded into the kernel or passed in from DT.
> They will be revisited later.
> 
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
>  .../devicetree/bindings/regulator/regulator.txt    |   33 ++++++++
>  drivers/regulator/Kconfig                          |    7 ++
>  drivers/regulator/Makefile                         |    1 +
>  drivers/regulator/of_regulator.c                   |   81 ++++++++++++++++++++
>  include/linux/regulator/of_regulator.h             |   20 +++++
>  5 files changed, 142 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/regulator/regulator.txt
>  create mode 100644 drivers/regulator/of_regulator.c
>  create mode 100644 include/linux/regulator/of_regulator.h
> 
> diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt
> new file mode 100644
> index 0000000..c488bf3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/regulator.txt
> @@ -0,0 +1,33 @@
> +Voltage/Current Regulators
> +

There should be a mandatory compatible field here, right? I.e. a topmost
generic one, "regulator" or similar.

> +Optional properties:
> +- regulator-name: A string used as a descriptive name for regulator outputs

Ah, there it is. Ignore my previous comment then, Mark. :)

> +- regulator-min-uV: smallest voltage consumers may set
> +- regulator-max-uV: largest voltage consumers may set
> +- regulator-uV-offset: Offset applied to voltages to compensate for voltage drops
> +- regulator-min-uA: smallest current consumers may set
> +- regulator-max-uA: largest current consumers may set
> +- regulator-always-on: boolean, regulator should never be disabled
> +- regulator-boot-on: bootloader/firmware enabled regulator

Once you have a compatible field that can determine what kind of device node,
and binding, this is, you can drop the regulator- prefix and save some space in
the device tree. Properties are rarely prefixed by their subsystem. Only
exception would/could be the regulator-name property where it could make
sense to keep the prefix.

Also, lower-caps is common instead of V and A.

> +- <name>-supply: phandle to the parent supply/regulator node

Having a fixed name here instead of a free form string would probably be a good
idea?



-Olof

Re: [PATCH v4 1/4] regulator: helper routine to extract regulator_init_data

[Mark Brown]

On Fri, Nov 04, 2011 at 01:29:05PM -0700, Olof Johansson wrote:
> On Thu, Oct 27, 2011 at 06:54:24PM +0530, Rajendra Nayak wrote:

> > @@ -0,0 +1,33 @@
> > +Voltage/Current Regulators

> There should be a mandatory compatible field here, right? I.e. a topmost
> generic one, "regulator" or similar.

It's not really useful for the regulator subsystem to directly bind to
the device as something needs to actually control it, the idea is that
this binding is included by reference in the bindings for specific
devices.

> Also, lower-caps is common instead of V and A.

On the other hand the case is pretty important for SI units

> > +- <name>-supply: phandle to the parent supply/regulator node

> Having a fixed name here instead of a free form string would probably be a good
> idea?

The name will be fixed by the individual device bindings, this is
specifying the general form of a supply property.  Each device binding
will define the set of supplies that the device can use.

Re: [PATCH v4 1/4] regulator: helper routine to extract regulator_init_data

[Olof Johansson]

On Fri, Nov 04, 2011 at 09:14:48PM +0000, Mark Brown wrote:
> On Fri, Nov 04, 2011 at 01:29:05PM -0700, Olof Johansson wrote:
> > On Thu, Oct 27, 2011 at 06:54:24PM +0530, Rajendra Nayak wrote:
> 
> > > @@ -0,0 +1,33 @@
> > > +Voltage/Current Regulators
> 
> > There should be a mandatory compatible field here, right? I.e. a topmost
> > generic one, "regulator" or similar.
> 
> It's not really useful for the regulator subsystem to directly bind to
> the device as something needs to actually control it, the idea is that
> this binding is included by reference in the bindings for specific
> devices.

Right, same goes for many other devices. Some use a toplevel compatible field,
some do not. Either way, not a big deal if you don't want to include one.

> > Also, lower-caps is common instead of V and A.
> 
> On the other hand the case is pretty important for SI units

Yeah, true. The fixed regulators used microvolt instead, which could be a good
way to do it.

> > > +- <name>-supply: phandle to the parent supply/regulator node
> 
> > Having a fixed name here instead of a free form string would probably be a good
> > idea?
> 
> The name will be fixed by the individual device bindings, this is
> specifying the general form of a supply property.  Each device binding
> will define the set of supplies that the device can use.

Ah, ok. It shouldn't be a part of this binding then and instead be added
to the bindings for the consumers.


-Olof

Re: [PATCH v4 1/4] regulator: helper routine to extract regulator_init_data

[Mark Brown]

On Fri, Nov 04, 2011 at 02:22:16PM -0700, Olof Johansson wrote:
> On Fri, Nov 04, 2011 at 09:14:48PM +0000, Mark Brown wrote:

> > The name will be fixed by the individual device bindings, this is
> > specifying the general form of a supply property.  Each device binding
> > will define the set of supplies that the device can use.

> Ah, ok. It shouldn't be a part of this binding then and instead be added
> to the bindings for the consumers.

I think it's useful to define how consumers are supposed to do this
somewhere - it is actually part of the core binding how consumers are
supposed to do this.

There's also a bit of magic here for chained supplies with one regulator
supplying another (eg, using a DCDC to drop the system supply down to a
lower voltage to supply a bunch of LDOs for improved efficiency).

Re: [PATCH v4 1/4] regulator: helper routine to extract regulator_init_data

[Olof Johansson]

On Fri, Nov 4, 2011 at 2:29 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Fri, Nov 04, 2011 at 02:22:16PM -0700, Olof Johansson wrote:
>> On Fri, Nov 04, 2011 at 09:14:48PM +0000, Mark Brown wrote:
>
>> > The name will be fixed by the individual device bindings, this is
>> > specifying the general form of a supply property.  Each device binding
>> > will define the set of supplies that the device can use.
>
>> Ah, ok. It shouldn't be a part of this binding then and instead be added
>> to the bindings for the consumers.
>
> I think it's useful to define how consumers are supposed to do this
> somewhere - it is actually part of the core binding how consumers are
> supposed to do this.

Yeah, ok, but it shouldn't be part of the description of regulator
properties per se. See how gpio does it, defining how a gpio-specifier
is crafted. The equivalent should be done for regulators.

> There's also a bit of magic here for chained supplies with one regulator
> supplying another (eg, using a DCDC to drop the system supply down to a
> lower voltage to supply a bunch of LDOs for improved efficiency).

Describing that in the device tree using regulator-specifiers
shouldn't be too bad? The LDO will reference the DCDC as the parent
supply (or input or whatever language you prefer). They don't have to
be in the same topology, they will instead be under whatever
controller/bus they are on for control -- i2c, etc.


-Olof

Re: [PATCH v4 1/4] regulator: helper routine to extract regulator_init_data

[Mark Brown]

On Fri, Nov 04, 2011 at 02:34:35PM -0700, Olof Johansson wrote:
> On Fri, Nov 4, 2011 at 2:29 PM, Mark Brown

> > I think it's useful to define how consumers are supposed to do this
> > somewhere - it is actually part of the core binding how consumers are
> > supposed to do this.

> Yeah, ok, but it shouldn't be part of the description of regulator
> properties per se. See how gpio does it, defining how a gpio-specifier
> is crafted. The equivalent should be done for regulators.

That seems to be pretty much exactly what's being done here, and like
the GPIO bindings it's specified in the core document.  Though perhaps
there's some aspect of how the document is written that's missing.

If you're talking about the specifics of the binding the GPIO bindings
do suffer from the whole magic indexes into arrays problem that makes a
lot of the older device tree bindings quite hard to read.

> > There's also a bit of magic here for chained supplies with one regulator
> > supplying another (eg, using a DCDC to drop the system supply down to a
> > lower voltage to supply a bunch of LDOs for improved efficiency).

> Describing that in the device tree using regulator-specifiers
> shouldn't be too bad? The LDO will reference the DCDC as the parent
> supply (or input or whatever language you prefer). They don't have to
> be in the same topology, they will instead be under whatever
> controller/bus they are on for control -- i2c, etc.

That's not great as it means you've got a separate binding for supplies
that happen to be connected to another regulator from that used for
other supplies on the device which is particularly confusing in the
fairly common case where a regulator chip has multiple supplies.  Using
the same method for binding all supplies seems much neater.

Re: [PATCH v4 1/4] regulator: helper routine to extract regulator_init_data

[Olof Johansson]

On Fri, Nov 4, 2011 at 2:46 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Fri, Nov 04, 2011 at 02:34:35PM -0700, Olof Johansson wrote:
>> On Fri, Nov 4, 2011 at 2:29 PM, Mark Brown
>
>> > I think it's useful to define how consumers are supposed to do this
>> > somewhere - it is actually part of the core binding how consumers are
>> > supposed to do this.
>
>> Yeah, ok, but it shouldn't be part of the description of regulator
>> properties per se. See how gpio does it, defining how a gpio-specifier
>> is crafted. The equivalent should be done for regulators.
>
> That seems to be pretty much exactly what's being done here, and like
> the GPIO bindings it's specified in the core document.  Though perhaps
> there's some aspect of how the document is written that's missing.

Yep, sounds good. A slightly more elaborate description of the binding
inspired by the gpio.txt document would be the way to do that.

> If you're talking about the specifics of the binding the GPIO bindings
> do suffer from the whole magic indexes into arrays problem that makes a
> lot of the older device tree bindings quite hard to read.

I wasn't talking about that in this case, but that is unfortunate and
I do agree that the ordered array of gpios is messy and hard to
follow.

I've been trying to catch those as they are introduced -- i.e. we
moved away from the magic array-of-three-gpios to discrete properties
on the sdhci binding for tegra, for example. We'll keep an eye on the
same for regulators.

>> > There's also a bit of magic here for chained supplies with one regulator
>> > supplying another (eg, using a DCDC to drop the system supply down to a
>> > lower voltage to supply a bunch of LDOs for improved efficiency).
>
>> Describing that in the device tree using regulator-specifiers
>> shouldn't be too bad? The LDO will reference the DCDC as the parent
>> supply (or input or whatever language you prefer). They don't have to
>> be in the same topology, they will instead be under whatever
>> controller/bus they are on for control -- i2c, etc.
>
> That's not great as it means you've got a separate binding for supplies
> that happen to be connected to another regulator from that used for
> other supplies on the device which is particularly confusing in the
> fairly common case where a regulator chip has multiple supplies.  Using
> the same method for binding all supplies seems much neater.

I'm not following the above 100%, but I think you are saying that you
would prefer to describe the regulator / power hierarchy in the
functional topology instead of how the various regulators and supplies
are organized on i2c busses and other controllers?  And the obvious
one that would be less than trivial to find a home for would be the
top-level or freestanding fixed regulators that don't sit on a
controlling bus.

If that is the case then this is yet another case of where we have a
couple of different structures that are overlaid on each other in the
device tree. Another is/will be clocking.

That should be OK, since you can have a binding for your PMIC that
splits up the various rails in separate subnodes, where each can
specify a reference to the upstream supply for that rail, etc. Would
that work?

Finding a best-practices location for where to put the fixed
regulators that don't sit on a controlling bus would be the trickiest
part, and I suppose we will just have to make up a location for those
(/power-supplies or something).  But various regulators will need to
be spread around the device tree a bit, in particular the i2c vs gpio
controlled ones will need to sit on their controlling busses.


-Olof

Re: [PATCH v4 1/4] regulator: helper routine to extract regulator_init_data

[Mark Brown]

On Fri, Nov 04, 2011 at 03:16:12PM -0700, Olof Johansson wrote:
> On Fri, Nov 4, 2011 at 2:46 PM, Mark Brown

> >> Describing that in the device tree using regulator-specifiers
> >> shouldn't be too bad? The LDO will reference the DCDC as the parent
> >> supply (or input or whatever language you prefer). They don't have to
> >> be in the same topology, they will instead be under whatever
> >> controller/bus they are on for control -- i2c, etc.

> > That's not great as it means you've got a separate binding for supplies
> > that happen to be connected to another regulator from that used for
> > other supplies on the device which is particularly confusing in the
> > fairly common case where a regulator chip has multiple supplies.  Using
> > the same method for binding all supplies seems much neater.

> I'm not following the above 100%, but I think you are saying that you
> would prefer to describe the regulator / power hierarchy in the
> functional topology instead of how the various regulators and supplies
> are organized on i2c busses and other controllers?  And the obvious
> one that would be less than trivial to find a home for would be the
> top-level or freestanding fixed regulators that don't sit on a
> controlling bus.

No, that's not the issue at all.  The issue is that we want a single way
of describing the supplies a device has regardless of their function
(which is what the existing stuff does).

Consider the case of a simple regulator with register control.  It is
going to have a supply used for the regulator itself and almost
certainly also a separate digital buffer supply used to reference the
digital I/O.  It seems bad to specify the first supply in a different
manner to the second, and there are more complex examples where a supply
can be both a regulator input and also a more general purpose supply.

Re: [PATCH v4 1/4] regulator: helper routine to extract regulator_init_data

[Olof Johansson]

On Fri, Nov 4, 2011 at 3:35 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Fri, Nov 04, 2011 at 03:16:12PM -0700, Olof Johansson wrote:
>> On Fri, Nov 4, 2011 at 2:46 PM, Mark Brown
>
>> >> Describing that in the device tree using regulator-specifiers
>> >> shouldn't be too bad? The LDO will reference the DCDC as the parent
>> >> supply (or input or whatever language you prefer). They don't have to
>> >> be in the same topology, they will instead be under whatever
>> >> controller/bus they are on for control -- i2c, etc.
>
>> > That's not great as it means you've got a separate binding for supplies
>> > that happen to be connected to another regulator from that used for
>> > other supplies on the device which is particularly confusing in the
>> > fairly common case where a regulator chip has multiple supplies.  Using
>> > the same method for binding all supplies seems much neater.
>
>> I'm not following the above 100%, but I think you are saying that you
>> would prefer to describe the regulator / power hierarchy in the
>> functional topology instead of how the various regulators and supplies
>> are organized on i2c busses and other controllers?  And the obvious
>> one that would be less than trivial to find a home for would be the
>> top-level or freestanding fixed regulators that don't sit on a
>> controlling bus.
>
> No, that's not the issue at all.  The issue is that we want a single way
> of describing the supplies a device has regardless of their function
> (which is what the existing stuff does).
>
> Consider the case of a simple regulator with register control.  It is
> going to have a supply used for the regulator itself and almost
> certainly also a separate digital buffer supply used to reference the
> digital I/O.  It seems bad to specify the first supply in a different
> manner to the second, and there are more complex examples where a supply
> can be both a regulator input and also a more general purpose supply.

Ah, we're misunderstanding each other again (as just discussed on irc
as well), and we're in agreement here as far as I can tell.

Named properties using regulator-specifiers to reference upstream
supplies should work well enough for any use today, and if it needs to
be reconsidered in the future we can revisit it then.


-Olof
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 1/4] regulator: helper routine to extract regulator_init_data

[Rajendra Nayak]

Hi Olof,

On Saturday 05 November 2011 01:59 AM, Olof Johansson wrote:
>> +- regulator-min-uV: smallest voltage consumers may set
>> >  +- regulator-max-uV: largest voltage consumers may set
>> >  +- regulator-uV-offset: Offset applied to voltages to compensate for voltage drops
>> >  +- regulator-min-uA: smallest current consumers may set
>> >  +- regulator-max-uA: largest current consumers may set
>> >  +- regulator-always-on: boolean, regulator should never be disabled
>> >  +- regulator-boot-on: bootloader/firmware enabled regulator
 >
> Once you have a compatible field that can determine what kind of device node,
> and binding, this is, you can drop the regulator- prefix and save some space in
> the device tree. Properties are rarely prefixed by their subsystem. Only
> exception would/could be the regulator-name property where it could make
> sense to keep the prefix.

given that we decided not to have a compatible field like "regulator"
for the top-level bindings and instead have a device specific one,
and based on the discussions with Grant here[1], I hope its OK to keep
these the way they are.

regards,
Rajendra

[1] 
http://lists.ozlabs.org/pipermail/devicetree-discuss/2011-September/008196.html

Re: [PATCH v4 1/4] regulator: helper routine to extract regulator_init_data

[Rajendra Nayak]

On Saturday 05 November 2011 02:52 AM, Olof Johansson wrote:
>>> >  >  Also, lower-caps is common instead of V and A.
>> >
>> >  On the other hand the case is pretty important for SI units
> Yeah, true. The fixed regulators used microvolt instead, which could be a good
> way to do it.
>

Ok, will use microvolt and microamp across all bindings.

Re: [PATCH v4 1/4] regulator: helper routine to extract regulator_init_data

[Rajendra Nayak]

On Saturday 05 November 2011 03:46 AM, Olof Johansson wrote:
>>> Yeah, ok, but it shouldn't be part of the description of regulator
>>> >>  properties per se. See how gpio does it, defining how a gpio-specifier
>>> >>  is crafted. The equivalent should be done for regulators.
>> >
>> >  That seems to be pretty much exactly what's being done here, and like
>> >  the GPIO bindings it's specified in the core document.  Though perhaps
>> >  there's some aspect of how the document is written that's missing.
 >
> Yep, sounds good. A slightly more elaborate description of the binding
> inspired by the gpio.txt document would be the way to do that.
>
Ok, will add more elaborate documentation for this in the core
bindings.