Re: [RFD] voltage/current regulator consumer interface

Re: [RFD] voltage/current regulator consumer interface

[James Kosin]

Mike Rapoport wrote:
> Hi,
> 
> Recently there was a brief discussion on linux-arm-kernel list [1] about
> controlling voltage regulator state in cases when there is no consumer device
> for a particular regulator.
> 
> I have some thoughts but I'd like to know people opinion before I start
> implementation.
> 
> Problem
> -------
> The regulator framework API provides ability to control the state of
> voltage/current regulators from within the kernel. Usually the regulator
> supplies power to a device and device driver or some hooks to the platform code
> from the device driver manipulate the regulator state. However, the regulator
> framework does not have userspace ABI that allows regulator state modifications.
> Lack of this ABI prevents fine-grained control for power consumption of devices
> such as GPS trancievers and GSM modems. Moreover, in SoC based systems it is
> possible to switch on/off power to entire subsystem when it is not used.
> 


I'd also ask the question, Why?
If exposing to user space it leaves the possibility of damaging hardware
or completely frying a board.

James

Re: [RFD] voltage/current regulator consumer interface

[Mike Rapoport]

James Kosin wrote:
> Mike Rapoport wrote:
>> Hi,
>>
>> Recently there was a brief discussion on linux-arm-kernel list [1] about
>> controlling voltage regulator state in cases when there is no consumer device
>> for a particular regulator.
>>
>> I have some thoughts but I'd like to know people opinion before I start
>> implementation.
>>
>> Problem
>> -------
>> The regulator framework API provides ability to control the state of
>> voltage/current regulators from within the kernel. Usually the regulator
>> supplies power to a device and device driver or some hooks to the platform code
>> from the device driver manipulate the regulator state. However, the regulator
>> framework does not have userspace ABI that allows regulator state modifications.
>> Lack of this ABI prevents fine-grained control for power consumption of devices
>> such as GPS trancievers and GSM modems. Moreover, in SoC based systems it is
>> possible to switch on/off power to entire subsystem when it is not used.
>>
> 
> 
> I'd also ask the question, Why?
> If exposing to user space it leaves the possibility of damaging hardware
> or completely frying a board.

Suppose you have a handheld device with GPS transceiver. You would like to give
user the ability to switch the transceiver on and off.

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

-- 
Sincerely yours,
Mike.

Re: [RFD] voltage/current regulator consumer interface

[James Kosin]

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

Mike Rapoport wrote:
> 
> James Kosin wrote:
>> Mike Rapoport wrote:
>>> Hi,
>>>
>>> Recently there was a brief discussion on linux-arm-kernel list [1] about
>>> controlling voltage regulator state in cases when there is no consumer device
>>> for a particular regulator.
>>>
>>> I have some thoughts but I'd like to know people opinion before I start
>>> implementation.
>>>
>>> Problem
>>> -------
>>> The regulator framework API provides ability to control the state of
>>> voltage/current regulators from within the kernel. Usually the regulator
>>> supplies power to a device and device driver or some hooks to the platform code
>>> from the device driver manipulate the regulator state. However, the regulator
>>> framework does not have userspace ABI that allows regulator state modifications.
>>> Lack of this ABI prevents fine-grained control for power consumption of devices
>>> such as GPS trancievers and GSM modems. Moreover, in SoC based systems it is
>>> possible to switch on/off power to entire subsystem when it is not used.
>>>
>>
>> I'd also ask the question, Why?
>> If exposing to user space it leaves the possibility of damaging hardware
>> or completely frying a board.
> 
> Suppose you have a handheld device with GPS transceiver. You would like to give
> user the ability to switch the transceiver on and off.
> 

Then the GPS drivers should be made aware and let the drivers handle the
on/off interface.  If a user is allowed to turn interfaces on/off at
will with this then drivers could suffer from (shock)...  ie: you could
turn off your hard-drive in a middle of a write by the driver corrupting
data, if handled in the driver it could finish the write before turning
off the drive.  I know this is a far stretch from a GPS were the device
is only READ only.

I do agree it could be useful, but we need to be careful on how much
control the user has over the drivers and system.  To an extreme, a user
could be able to turn off CPU cores outside of the drivers control
causing serious pipeline hazards that would need to be handled at the
driver level.  This would not be an issue for GPS were the data is read
only and the missing data is really not that important to the system
operation and continued use.

James


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 258 bytes --]

Re: [RFD] voltage/current regulator consumer interface

[Mike Rapoport]

James Kosin wrote:
> Mike Rapoport wrote:
>> James Kosin wrote:
>>> Mike Rapoport wrote:
>>>> Hi,
>>>>
>>>> Recently there was a brief discussion on linux-arm-kernel list [1] about
>>>> controlling voltage regulator state in cases when there is no consumer device
>>>> for a particular regulator.
>>>>
>>>> I have some thoughts but I'd like to know people opinion before I start
>>>> implementation.
>>>>
>>>> Problem
>>>> -------
>>>> The regulator framework API provides ability to control the state of
>>>> voltage/current regulators from within the kernel. Usually the regulator
>>>> supplies power to a device and device driver or some hooks to the platform code
>>>> from the device driver manipulate the regulator state. However, the regulator
>>>> framework does not have userspace ABI that allows regulator state modifications.
>>>> Lack of this ABI prevents fine-grained control for power consumption of devices
>>>> such as GPS trancievers and GSM modems. Moreover, in SoC based systems it is
>>>> possible to switch on/off power to entire subsystem when it is not used.
>>>>
>>> I'd also ask the question, Why?
>>> If exposing to user space it leaves the possibility of damaging hardware
>>> or completely frying a board.
>> Suppose you have a handheld device with GPS transceiver. You would like to give
>> user the ability to switch the transceiver on and off.
>>
> 
> Then the GPS drivers should be made aware and let the drivers handle the
> on/off interface.  If a user is allowed to turn interfaces on/off at
> will with this then drivers could suffer from (shock)...  ie: you could
> turn off your hard-drive in a middle of a write by the driver corrupting
> data, if handled in the driver it could finish the write before turning
> off the drive.  I know this is a far stretch from a GPS were the device
> is only READ only.

The point is there's no GPS driver... GPS transceivers are usually connected to
a serial line and the applications access the GPS data through ttySX. The case
when there is a kernel driver for device connected to a regulator is completely
different. It is then the driver responsibility to decide when to power on/off
the device.

> I do agree it could be useful, but we need to be careful on how much
> control the user has over the drivers and system.  To an extreme, a user
> could be able to turn off CPU cores outside of the drivers control
> causing serious pipeline hazards that would need to be handled at the
> driver level.  This would not be an issue for GPS were the data is read
> only and the missing data is really not that important to the system
> operation and continued use.
> 
> James
> 

-- 
Sincerely yours,
Mike.

Re: [RFD] voltage/current regulator consumer interface

[James Kosin]

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

Mike Rapoport wrote:
> James Kosin wrote:
>> Mike Rapoport wrote:
>>> James Kosin wrote:
>>>> Mike Rapoport wrote:
>>>>> Hi,
>>>>>
>>>>> Recently there was a brief discussion on linux-arm-kernel list [1] about
>>>>> controlling voltage regulator state in cases when there is no consumer device
>>>>> for a particular regulator.
>>>>>
>>>>> I have some thoughts but I'd like to know people opinion before I start
>>>>> implementation.
>>>>>
>>>>> Problem
>>>>> -------
>>>>> The regulator framework API provides ability to control the state of
>>>>> voltage/current regulators from within the kernel. Usually the regulator
>>>>> supplies power to a device and device driver or some hooks to the platform code
>>>>> from the device driver manipulate the regulator state. However, the regulator
>>>>> framework does not have userspace ABI that allows regulator state modifications.
>>>>> Lack of this ABI prevents fine-grained control for power consumption of devices
>>>>> such as GPS trancievers and GSM modems. Moreover, in SoC based systems it is
>>>>> possible to switch on/off power to entire subsystem when it is not used.
>>>>>
>>>> I'd also ask the question, Why?
>>>> If exposing to user space it leaves the possibility of damaging hardware
>>>> or completely frying a board.
>>> Suppose you have a handheld device with GPS transceiver. You would like to give
>>> user the ability to switch the transceiver on and off.
>>>
>> Then the GPS drivers should be made aware and let the drivers handle the
>> on/off interface.  If a user is allowed to turn interfaces on/off at
>> will with this then drivers could suffer from (shock)...  ie: you could
>> turn off your hard-drive in a middle of a write by the driver corrupting
>> data, if handled in the driver it could finish the write before turning
>> off the drive.  I know this is a far stretch from a GPS were the device
>> is only READ only.
> 
> The point is there's no GPS driver... GPS transceivers are usually connected to
> a serial line and the applications access the GPS data through ttySX. The case
> when there is a kernel driver for device connected to a regulator is completely
> different. It is then the driver responsibility to decide when to power on/off
> the device.
> 
>> I do agree it could be useful, but we need to be careful on how much
>> control the user has over the drivers and system.  To an extreme, a user
>> could be able to turn off CPU cores outside of the drivers control
>> causing serious pipeline hazards that would need to be handled at the
>> driver level.  This would not be an issue for GPS were the data is read
>> only and the missing data is really not that important to the system
>> operation and continued use.
>>
>> James
>>
> 
Ok.  You have a valid point.

What about a GPS driver that can use the RS232 interface as the
transport.  Much like the IDE over Parallel Port in older kernels did.
This way the driver would still be responsible and not have to
completely expose the regulator control.

James


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 258 bytes --]

Re: [RFD] voltage/current regulator consumer interface

[Pavel Machek]

Hi!


> > Then the GPS drivers should be made aware and let the drivers handle the
> > on/off interface.  If a user is allowed to turn interfaces on/off at
> > will with this then drivers could suffer from (shock)...  ie: you could
> > turn off your hard-drive in a middle of a write by the driver corrupting
> > data, if handled in the driver it could finish the write before turning
> > off the drive.  I know this is a far stretch from a GPS were the device
> > is only READ only.
> 
> The point is there's no GPS driver... GPS transceivers are usually connected to
> a serial line and the applications access the GPS data through ttySX. The case
> when there is a kernel driver for device connected to a regulator is completely
> different. It is then the driver responsibility to decide when to power on/off
> the device.

Well, then you should teach serial driver to power the (GPS) device up on
open() and power it off on close()...

Giving it better name then ttyS2 would help, too. ttyGPS?

IOW write a trivial driver even through  you could get away w/o that...
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFD] voltage/current regulator consumer interface

[Mark Brown]

On Sat, Apr 25, 2009 at 10:19:33AM +0200, Pavel Machek wrote:

> Well, then you should teach serial driver to power the (GPS) device up on
> open() and power it off on close()...

This all started because this approach was nacked for serial drivers.
In any case, I'm not sure that'll scale - it may not be desirable or
reasonable to do something like that for all devices.  For example, the
devices may be connected via a technology that's not suitable (I've
worked on systems which used ethernet here).

Re: [RFD] voltage/current regulator consumer interface

[James Kosin]

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

Mark Brown wrote:
> On Sat, Apr 25, 2009 at 10:19:33AM +0200, Pavel Machek wrote:
> 
>> Well, then you should teach serial driver to power the (GPS) device up on
>> open() and power it off on close()...
> 
> This all started because this approach was nacked for serial drivers.
> In any case, I'm not sure that'll scale - it may not be desirable or
> reasonable to do something like that for all devices.  For example, the
> devices may be connected via a technology that's not suitable (I've
> worked on systems which used ethernet here).
> 
Ethernet can be treated as a serial interface as well down to the
simplest level.  I'm leaving out a lot is making this assumption; but,
it is true to a fault.

Scalability has more to do with the generic interface you use... If you
tie things to ttySxx and don't allow configuration parameters you are
also restricting the usage and scalability as well.

The GPS should be configurable so the interface is generic and allows
getting from Ethernet/PCI/IDE/serial/parallel/etc.  The transport layer
should not matter to the GPS driver.  The trick will be to allow the
user to ultimately choose, if that is what you want.

The same applies to disk devices, now we have IDE/SATA/etc. the disk
device itself doesn't depend on the interface physically, the kernel
treats all these as suitable interfaces for disk/CDROM/tape devices and
the world is not turned on its ear just because there is a different
physical interface.

James


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 258 bytes --]

Re: [RFD] voltage/current regulator consumer interface

[Mark Brown]

On Mon, Apr 27, 2009 at 09:47:32AM -0400, James Kosin wrote:
> Mark Brown wrote:

> > This all started because this approach was nacked for serial drivers.
> > In any case, I'm not sure that'll scale - it may not be desirable or
> > reasonable to do something like that for all devices.  For example, the
> > devices may be connected via a technology that's not suitable (I've
> > worked on systems which used ethernet here).

> Ethernet can be treated as a serial interface as well down to the
> simplest level.  I'm leaving out a lot is making this assumption; but,
> it is true to a fault.

The big difference with ethernet is that it's multi-drop - you can't
turn on the power based on the ethernet interface being in use since
there can be more than one device connected.

> The same applies to disk devices, now we have IDE/SATA/etc. the disk
> device itself doesn't depend on the interface physically, the kernel
> treats all these as suitable interfaces for disk/CDROM/tape devices and
> the world is not turned on its ear just because there is a different
> physical interface.

You're suggesting modifying the link layer to do different things based
on which device is connnected to it; in any case the issue here is that
this class of hardware has no in-kernel driver to provide an abstaction
- it's all done by a user space daemon or similar.

Re: [RFD] voltage/current regulator consumer interface

[Mark Brown]

On Tue, Apr 21, 2009 at 09:25:09AM -0400, James Kosin wrote:

> Then the GPS drivers should be made aware and let the drivers handle the
> on/off interface.  If a user is allowed to turn interfaces on/off at
> will with this then drivers could suffer from (shock)...  ie: you could
> turn off your hard-drive in a middle of a write by the driver corrupting
> data, if handled in the driver it could finish the write before turning
> off the drive.  I know this is a far stretch from a GPS were the device
> is only READ only.

The proposed driver is a platform device.  This means that in order for
user space to have anything to access the kernel will have to explicitly
register a platform device and provide it with configuration.  Further,
since the driver is a standard regulator consumer the kernel will also
have to have provided explicit constraints which grant permission to the
driver to modify the state of the regulator.

> I do agree it could be useful, but we need to be careful on how much
> control the user has over the drivers and system.  To an extreme, a user
> could be able to turn off CPU cores outside of the drivers control
> causing serious pipeline hazards that would need to be handled at the
> driver level.  This would not be an issue for GPS were the data is read

This is why the regulator core requires that explicit configuration be
done by the machine in order to allow any changes to the regulator
state.

Re: [RFD] voltage/current regulator consumer interface

[Mark Brown]

On Mon, Apr 20, 2009 at 11:38:49AM -0400, James Kosin wrote:

> > such as GPS trancievers and GSM modems. Moreover, in SoC based systems it is
> > possible to switch on/off power to entire subsystem when it is not used.

> I'd also ask the question, Why?
> If exposing to user space it leaves the possibility of damaging hardware
> or completely frying a board.

There are some classes of device found in embedded systems which do
things like provide an essentially separate system which interacts with
the host system via something like a serial port.  These devices are
normally controlled entirely from user space which is fine with static
power configuration but if you want to provide regulator control for
these devices you need to either move the control of the device into the
kernel or expose the regulators the device uses to userspace.

Re: [RFD] voltage/current regulator consumer interface

[James Kosin]

Mark Brown wrote:
> On Wed, Apr 22, 2009 at 10:57:37AM +0300, Mike Rapoport wrote:
> 
>> What about "line-supply"?
> 
> In power management line normally refers to the core, unregulated power
> bus for the system and a line supply would normally be something like a
> wall wart that supplies this directly when it's connected.

Can I suggest:
"supply-switch" or "regulator-switch" may make more sense for the moment.

James

Re: [RFD] voltage/current regulator consumer interface

[Alan Cox]

> > In power management line normally refers to the core, unregulated power
> > bus for the system and a line supply would normally be something like a
> > wall wart that supplies this directly when it's connected.
> 
> Can I suggest:
> "supply-switch" or "regulator-switch" may make more sense for the moment.

Can I suggest they don't. The naming should reflect the naming actually
used in the field by people working on the technology otherwise arbitary
renaming just creates confusion

The supply switch is the big switch on the back

Alan

Re: [RFD] voltage/current regulator consumer interface

[James Kosin]

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

Alan Cox wrote:
>>> In power management line normally refers to the core, unregulated power
>>> bus for the system and a line supply would normally be something like a
>>> wall wart that supplies this directly when it's connected.
>> Can I suggest:
>> "supply-switch" or "regulator-switch" may make more sense for the moment.
> 
> Can I suggest they don't. The naming should reflect the naming actually
> used in the field by people working on the technology otherwise arbitary
> renaming just creates confusion
> 
> The supply switch is the big switch on the back
> 
> Alan
> 
Ok, "supply-switch" may have been a bad idea, I wasn't thinking yet.
The coffee hasn't kicked in yet and I'm feeling a bit down.

James


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 258 bytes --]

[RFD] voltage/current regulator consumer interface

[Mike Rapoport]

Hi,

Recently there was a brief discussion on linux-arm-kernel list [1] about
controlling voltage regulator state in cases when there is no consumer device
for a particular regulator.

I have some thoughts but I'd like to know people opinion before I start
implementation.

Problem
-------
The regulator framework API provides ability to control the state of
voltage/current regulators from within the kernel. Usually the regulator
supplies power to a device and device driver or some hooks to the platform code
from the device driver manipulate the regulator state. However, the regulator
framework does not have userspace ABI that allows regulator state modifications.
Lack of this ABI prevents fine-grained control for power consumption of devices
such as GPS trancievers and GSM modems. Moreover, in SoC based systems it is
possible to switch on/off power to entire subsystem when it is not used.

Possible solutions
------------------
- Add a platform_driver similar to drivers/regulator/virtual.c that will expose
writable attributes to sysfs thus allowing userspace to control regulator state.
It is the most simple and straight forward solution. However, I have several
questions and I cannot answer them myself:
  Should the driver handle single or several supplies at once?
  If the driver handles single supply isn't it a waste of memory to have a
platform_device per supply?
  If the driver handles several supplies how to define attributes per-supply?
  What attributes will be exposed? Would it be simple 'state' that can be
enabled or disabled? Or the entire set of micorvolts/microamps etc?

- Extend regulator framework so that regulator_consumer_supply entities will
gain their own representation in sysfs. Depending on machine constraints the
supply attributes can be either read-only or writable. I haven't dedicated much
thought to it, so a cannot state any pros and cons.

Any feedback is appreciated.

--
[1] http://lists.arm.linux.org.uk/lurker/message/20090413.125359.613f85ad.en.html

-- 
Sincerely yours,
Mike.

Re: [RFD] voltage/current regulator consumer interface

[Mark Brown]

On Mon, Apr 20, 2009 at 05:32:21PM +0300, Mike Rapoport wrote:

> It is the most simple and straight forward solution. However, I have several
> questions and I cannot answer them myself:

Some initial thoughts:

>   Should the driver handle single or several supplies at once?

I'd expect that it should be able to cope with that - a lot of hardware
takes multiple supplies.

>   If the driver handles several supplies how to define attributes per-supply?
>   What attributes will be exposed? Would it be simple 'state' that can be
> enabled or disabled? Or the entire set of micorvolts/microamps etc?

I'd expect that if you're getting into much more than a simple enable
for the entire device it'd be getting to the point where it's worth
considering writing an actual driver for the device.  That said, it's a
fairly natural extension to support more fine grained stuff if someone
does actually end up wanting it.

> - Extend regulator framework so that regulator_consumer_supply entities will
> gain their own representation in sysfs. Depending on machine constraints the
> supply attributes can be either read-only or writable. I haven't dedicated much
> thought to it, so a cannot state any pros and cons.

I'd expect making them writable to be very fragile if there is an active
consumer driver - I'd expect reference counts to get lost with enable
and disable and I'd expect drivers managing voltages and currents to get
confused if their own configuration gets changed under them.  The core
already needs to arbitrate between multiple users so it seems to make
sense to have userspace represented as other users are to the core.

Being able to read back the consumer status doesn't seem unreasonable,
though.

Re: [RFD] voltage/current regulator consumer interface

[Mike Rapoport]

Mark Brown wrote:
> On Mon, Apr 20, 2009 at 05:32:21PM +0300, Mike Rapoport wrote:
> 
>> It is the most simple and straight forward solution. However, I have several
>> questions and I cannot answer them myself:
> 
> Some initial thoughts:
> 
>>   Should the driver handle single or several supplies at once?
> 
> I'd expect that it should be able to cope with that - a lot of hardware
> takes multiple supplies.
> 
>>   If the driver handles several supplies how to define attributes per-supply?
>>   What attributes will be exposed? Would it be simple 'state' that can be
>> enabled or disabled? Or the entire set of micorvolts/microamps etc?
> 
> I'd expect that if you're getting into much more than a simple enable
> for the entire device it'd be getting to the point where it's worth
> considering writing an actual driver for the device.  That said, it's a
> fairly natural extension to support more fine grained stuff if someone
> does actually end up wanting it.

I've managed to create some preliminary "line-consumer" driver. I don't really
like the name, but I couldn't think of something better and "virtual" is
already taken :)
Feedback is appreciated.


diff --git a/drivers/regulator/line-consumer.c b/drivers/regulator/line-consumer.c
new file mode 100644
index 0000000..e7cba4b
--- /dev/null
+++ b/drivers/regulator/line-consumer.c
@@ -0,0 +1,211 @@
+/*
+ * line-consumer.c
+ *
+ * Copyright 2009 CompuLab, Ltd..
+ *
+ * Author: Mike Rapoport <mike@compulab.co.il>
+ *
+ * Based of virtual consumer driver:
+ *   Copyright 2008 Wolfson Microelectronics PLC.
+ *   Author: Mark Brown <broonie@opensource.wolfsonmicro.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/err.h>
+#include <linux/mutex.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
+#include <linux/regulator/line-consumer.h>
+
+struct line_consumer_data {
+	const char *name;
+
+	struct mutex lock;
+	bool enabled;
+
+	int num_supplies;
+	struct regulator_bulk_data *supplies;
+};
+
+static ssize_t show_name(struct device *dev,
+			  struct device_attribute *attr, char *buf)
+{
+	struct line_consumer_data *data = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%s\n", data->name);
+}
+
+static ssize_t show_state(struct device *dev,
+			  struct device_attribute *attr, char *buf)
+{
+	struct line_consumer_data *data = dev_get_drvdata(dev);
+
+	if (data->enabled)
+		return sprintf(buf, "enabled\n");
+
+	return sprintf(buf, "disabled\n");
+}
+
+static ssize_t set_state(struct device *dev, struct device_attribute *attr,
+			 const char *buf, size_t count)
+{
+	struct line_consumer_data *data = dev_get_drvdata(dev);
+	bool enabled;
+	int ret;
+
+	/*
+	 * sysfs_streq() doesn't need the \n's, but we add them so the strings
+	 * will be shared with show_state(), above.
+	 */
+	if (sysfs_streq(buf, "enabled\n") == 0)
+		enabled = true;
+	else if (sysfs_streq(buf, "disabled\n") == 0)
+		enabled = false;
+	else {
+		dev_err(dev, "Configuring invalid mode\n");
+		return count;
+	}
+
+	mutex_lock(&data->lock);
+	if (enabled != data->enabled) {
+		if (enabled)
+			ret = regulator_bulk_enable(data->num_supplies,
+						    data->supplies);
+		else
+			ret = regulator_bulk_disable(data->num_supplies,
+						     data->supplies);
+
+		if (ret == 0)
+			data->enabled = enabled;
+		else
+			dev_err(dev, "Failed to configure state: %d\n", ret);
+	}
+	mutex_unlock(&data->lock);
+
+	return count;
+}
+
+static DEVICE_ATTR(name, 0444, show_name, NULL);
+static DEVICE_ATTR(state, 0666, show_state, set_state);
+
+static struct device_attribute *attributes[] = {
+	&dev_attr_name,
+	&dev_attr_state,
+};
+
+static int regulator_line_consumer_probe(struct platform_device *pdev)
+{
+	struct regulator_line_consumer_data *pdata;
+	struct line_consumer_data *drvdata;
+	int ret, i;
+
+	pdata = pdev->dev.platform_data;
+	if (!pdata)
+		return -EINVAL;
+
+	drvdata = kzalloc(sizeof(struct line_consumer_data), GFP_KERNEL);
+	if (drvdata == NULL)
+		return -ENOMEM;
+
+	drvdata->name = pdata->name;
+	drvdata->num_supplies = pdata->num_supplies;
+
+	drvdata->supplies = kzalloc(sizeof(struct regulator_bulk_data) *
+				    drvdata->num_supplies,
+				    GFP_KERNEL);
+	if (drvdata->supplies == NULL) {
+		ret = -ENOMEM;
+		goto err_alloc_supplies;
+	}
+
+	mutex_init(&drvdata->lock);
+
+	for (i = 0; i < drvdata->num_supplies; i++)
+		drvdata->supplies[i].supply = pdata->supplies[i].supply;
+
+	ret = regulator_bulk_get(&pdev->dev, drvdata->num_supplies,
+				 drvdata->supplies);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to get supplies: %d\n", ret);
+		goto err_alloc_supplies;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(attributes); i++) {
+		ret = device_create_file(&pdev->dev, attributes[i]);
+		if (ret != 0)
+			goto err_create_attrs;
+	}
+
+	if (pdata->init_on)
+		ret = regulator_bulk_enable(drvdata->num_supplies,
+					    drvdata->supplies);
+	else
+		ret = regulator_bulk_disable(drvdata->num_supplies,
+					     drvdata->supplies);
+
+	drvdata->enabled = pdata->init_on;
+
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to set initial state: %d\n", ret);
+		goto err_create_attrs;
+	}
+
+	platform_set_drvdata(pdev, drvdata);
+
+	return 0;
+
+err_create_attrs:
+	for (i = 0; i < ARRAY_SIZE(attributes); i++)
+		device_remove_file(&pdev->dev, attributes[i]);
+
+	regulator_bulk_free(drvdata->num_supplies, drvdata->supplies);
+
+err_alloc_supplies:
+	kfree(drvdata);
+	return ret;
+}
+
+static int regulator_line_consumer_remove(struct platform_device *pdev)
+{
+	struct line_consumer_data *drvdata = platform_get_drvdata(pdev);
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(attributes); i++)
+		device_remove_file(&pdev->dev, attributes[i]);
+
+	regulator_bulk_free(drvdata->num_supplies, drvdata->supplies);
+	kfree(drvdata->supplies);
+	kfree(drvdata);
+
+	return 0;
+}
+
+static struct platform_driver regulator_line_consumer_driver = {
+	.probe		= regulator_line_consumer_probe,
+	.remove		= regulator_line_consumer_remove,
+	.driver		= {
+		.name		= "reg-line-consumer",
+	},
+};
+
+
+static int __init regulator_line_consumer_init(void)
+{
+	return platform_driver_register(&regulator_line_consumer_driver);
+}
+module_init(regulator_line_consumer_init);
+
+static void __exit regulator_line_consumer_exit(void)
+{
+	platform_driver_unregister(&regulator_line_consumer_driver);
+}
+module_exit(regulator_line_consumer_exit);
+
+MODULE_AUTHOR("Mike Rapoport <mike@compulab.co.il>");
+MODULE_DESCRIPTION("Line consumer for voltage and current regulators");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/regulator/line-consumer.h b/include/linux/regulator/line-consumer.h
new file mode 100644
index 0000000..df978dd
--- /dev/null
+++ b/include/linux/regulator/line-consumer.h
@@ -0,0 +1,25 @@
+#ifndef __REGULATOR_PLATFORM_CONSUMER_H_
+#define __REGULATOR_PLATFORM_CONSUMER_H_
+
+struct regulator_consumer_supply;
+
+/**
+ * struct regulator_line_consumer_data - line consumer
+ * initialisation data.
+ *
+ * @name: Name for the consumer line
+ * @num_supplies: Number of supplies feeding the line
+ * @supplies: Supplies configuration.
+ * @init_on: Set if the regulators supplying the line should be
+ *           enabled during initialisation
+ */
+struct regulator_line_consumer_data {
+	const char *name;
+
+	int num_supplies;
+	struct regulator_consumer_supply *supplies;
+
+	bool init_on;
+};
+
+#endif /* __REGULATOR_PLATFORM_CONSUMER_H_ */


-- 
Sincerely yours,
Mike.

Re: [RFD] voltage/current regulator consumer interface

[Mark Brown]

On Tue, Apr 21, 2009 at 03:00:41PM +0300, Mike Rapoport wrote:

> I've managed to create some preliminary "line-consumer" driver. I don't really
> like the name, but I couldn't think of something better and "virtual" is
> already taken :)

"userspace" or "user"?  The goal here is to let userspace applications
be consumers.

> +static ssize_t set_state(struct device *dev, struct device_attribute *attr,
> +			 const char *buf, size_t count)
> +{
> +	struct line_consumer_data *data = dev_get_drvdata(dev);
> +	bool enabled;
> +	int ret;
> +
> +	/*
> +	 * sysfs_streq() doesn't need the \n's, but we add them so the strings
> +	 * will be shared with show_state(), above.
> +	 */
> +	if (sysfs_streq(buf, "enabled\n") == 0)
> +		enabled = true;
> +	else if (sysfs_streq(buf, "disabled\n") == 0)
> +		enabled = false;

I'd be inclined to also accept 1 and 0 here.

> +static DEVICE_ATTR(name, 0444, show_name, NULL);
> +static DEVICE_ATTR(state, 0666, show_state, set_state);

Permissions for set_state() should probably be tighter?

> +	for (i = 0; i < ARRAY_SIZE(attributes); i++) {
> +		ret = device_create_file(&pdev->dev, attributes[i]);
> +		if (ret != 0)
> +			goto err_create_attrs;
> +	}

device_add_attributes()?

> +	if (pdata->init_on)
> +		ret = regulator_bulk_enable(drvdata->num_supplies,
> +					    drvdata->supplies);
> +	else
> +		ret = regulator_bulk_disable(drvdata->num_supplies,
> +					     drvdata->supplies);

The disable case will lead to unbalanced enables and disables, loosing
a reference to the supply.  Just don't do anything if the supplies
should not be enabled.

> +static int regulator_line_consumer_remove(struct platform_device *pdev)
> +{
> +	struct line_consumer_data *drvdata = platform_get_drvdata(pdev);
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(attributes); i++)
> +		device_remove_file(&pdev->dev, attributes[i]);
> +
> +	regulator_bulk_free(drvdata->num_supplies, drvdata->supplies);
> +	kfree(drvdata->supplies);
> +	kfree(drvdata);
> +
> +	return 0;
> +}

Hrm.  You probably want to disable the supplies if you weren't using
them here.

Re: [RFD] voltage/current regulator consumer interface

[Mike Rapoport]

Mark Brown wrote:
> On Tue, Apr 21, 2009 at 03:00:41PM +0300, Mike Rapoport wrote:
> 
>> I've managed to create some preliminary "line-consumer" driver. I don't really
>> like the name, but I couldn't think of something better and "virtual" is
>> already taken :)
> 
> "userspace" or "user"?  The goal here is to let userspace applications
> be consumers.

I don't quite understand your intention here. Do you mean that it would bebetter
to use a character device rather than sysfs?

>> +static ssize_t set_state(struct device *dev, struct device_attribute *attr,
>> +			 const char *buf, size_t count)
>> +{
>> +	struct line_consumer_data *data = dev_get_drvdata(dev);
>> +	bool enabled;
>> +	int ret;
>> +
>> +	/*
>> +	 * sysfs_streq() doesn't need the \n's, but we add them so the strings
>> +	 * will be shared with show_state(), above.
>> +	 */
>> +	if (sysfs_streq(buf, "enabled\n") == 0)
>> +		enabled = true;
>> +	else if (sysfs_streq(buf, "disabled\n") == 0)
>> +		enabled = false;
> 
> I'd be inclined to also accept 1 and 0 here.
> 
>> +static DEVICE_ATTR(name, 0444, show_name, NULL);
>> +static DEVICE_ATTR(state, 0666, show_state, set_state);
> 
> Permissions for set_state() should probably be tighter?
> 
>> +	for (i = 0; i < ARRAY_SIZE(attributes); i++) {
>> +		ret = device_create_file(&pdev->dev, attributes[i]);
>> +		if (ret != 0)
>> +			goto err_create_attrs;
>> +	}
> 
> device_add_attributes()?
> 
>> +	if (pdata->init_on)
>> +		ret = regulator_bulk_enable(drvdata->num_supplies,
>> +					    drvdata->supplies);
>> +	else
>> +		ret = regulator_bulk_disable(drvdata->num_supplies,
>> +					     drvdata->supplies);
> 
> The disable case will lead to unbalanced enables and disables, loosing
> a reference to the supply.  Just don't do anything if the supplies
> should not be enabled.
> 
>> +static int regulator_line_consumer_remove(struct platform_device *pdev)
>> +{
>> +	struct line_consumer_data *drvdata = platform_get_drvdata(pdev);
>> +	int i;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(attributes); i++)
>> +		device_remove_file(&pdev->dev, attributes[i]);
>> +
>> +	regulator_bulk_free(drvdata->num_supplies, drvdata->supplies);
>> +	kfree(drvdata->supplies);
>> +	kfree(drvdata);
>> +
>> +	return 0;
>> +}
> 
> Hrm.  You probably want to disable the supplies if you weren't using
> them here.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

-- 
Sincerely yours,
Mike.

Re: [RFD] voltage/current regulator consumer interface

[Mark Brown]

On Tue, Apr 21, 2009 at 04:54:51PM +0300, Mike Rapoport wrote:
> Mark Brown wrote:
> > On Tue, Apr 21, 2009 at 03:00:41PM +0300, Mike Rapoport wrote:

> >> I've managed to create some preliminary "line-consumer" driver. I don't really
> >> like the name, but I couldn't think of something better and "virtual" is
> >> already taken :)

> > "userspace" or "user"?  The goal here is to let userspace applications
> > be consumers.

> I don't quite understand your intention here. Do you mean that it would bebetter
> to use a character device rather than sysfs?

I'm talking about the name.

Re: [RFD] voltage/current regulator consumer interface

[Mike Rapoport]

Mark Brown wrote:
> On Tue, Apr 21, 2009 at 04:54:51PM +0300, Mike Rapoport wrote:
>> Mark Brown wrote:
>>> On Tue, Apr 21, 2009 at 03:00:41PM +0300, Mike Rapoport wrote:
> 
>>>> I've managed to create some preliminary "line-consumer" driver. I don't really
>>>> like the name, but I couldn't think of something better and "virtual" is
>>>> already taken :)
> 
>>> "userspace" or "user"?  The goal here is to let userspace applications
>>> be consumers.
> 
>> I don't quite understand your intention here. Do you mean that it would bebetter
>> to use a character device rather than sysfs?
> 
> I'm talking about the name.

As I said, I could not invent something better. Any idea?

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

-- 
Sincerely yours,
Mike.

Re: [RFD] voltage/current regulator consumer interface

[Mark Brown]

On Tue, Apr 21, 2009 at 05:01:45PM +0300, Mike Rapoport wrote:
> Mark Brown wrote:

> >>> "userspace" or "user"?  The goal here is to let userspace applications
> >>> be consumers.

> > I'm talking about the name.

> As I said, I could not invent something better. Any idea?

Those are my ideas :)

Re: [RFD] voltage/current regulator consumer interface

[Mike Rapoport]

Mark Brown wrote:
> On Tue, Apr 21, 2009 at 05:01:45PM +0300, Mike Rapoport wrote:
>> Mark Brown wrote:
> 
>>>>> "userspace" or "user"?  The goal here is to let userspace applications
>>>>> be consumers.
> 
>>> I'm talking about the name.

What about "line-supply"?

>> As I said, I could not invent something better. Any idea?
> 
> Those are my ideas :)
> 

-- 
Sincerely yours,
Mike.

Re: [RFD] voltage/current regulator consumer interface

[Mark Brown]

On Wed, Apr 22, 2009 at 10:57:37AM +0300, Mike Rapoport wrote:

> What about "line-supply"?

In power management line normally refers to the core, unregulated power
bus for the system and a line supply would normally be something like a
wall wart that supplies this directly when it's connected.