[PATCH 0/3] regulator: add support for GPIO power load switches

[PATCH 0/3] regulator: add support for GPIO power load switches

[Bartosz Golaszewski]

We would like to add support for GPIO-controlled power load switches
(for example: the tps229* series from Texas Instruments). We use this
chip to power-cycle devices whose power consumption is measured using
baylibre-acme probes thus we need a way to control them from user
space.

I initially submitted a series adding this support via the iio
framework, but it was decided that iio is not the right interface for
that.

This series extends the regulator core to support regulators controlled
from user space and reuses the fixed regulator driver to support gpio
power switches.

Bartosz Golaszewski (3):
  regulator: add support for user space controlled regulators
  doc: DT: add new compatible to fixed regulator's binding
  regulator: fixed: add support for gpio power switches

 .../bindings/regulator/fixed-regulator.txt         |  4 ++-
 drivers/regulator/core.c                           | 38 +++++++++++++++++++++-
 drivers/regulator/fixed.c                          | 33 ++++++++++++++-----
 include/linux/regulator/driver.h                   |  5 +++
 4 files changed, 70 insertions(+), 10 deletions(-)

-- 
2.9.3

[PATCH 1/3] regulator: add support for user space controlled regulators

[Bartosz Golaszewski]

Add a new flag to struct regulator_desc indicating whether a regulator
can be controlled from user space and implement a routine in regulator
core allowing to toggle the regulator state via the sysfs 'state'
attribute.

This is useful for gpio power switches.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/regulator/core.c         | 38 +++++++++++++++++++++++++++++++++++++-
 include/linux/regulator/driver.h |  5 +++++
 2 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 5c1519b..f77de8f 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -99,6 +99,7 @@ struct regulator_supply_alias {
 };
 
 static int _regulator_is_enabled(struct regulator_dev *rdev);
+static int _regulator_enable(struct regulator_dev *rdev);
 static int _regulator_disable(struct regulator_dev *rdev);
 static int _regulator_get_voltage(struct regulator_dev *rdev);
 static int _regulator_get_current_limit(struct regulator_dev *rdev);
@@ -401,7 +402,42 @@ static ssize_t regulator_state_show(struct device *dev,
 
 	return ret;
 }
-static DEVICE_ATTR(state, 0444, regulator_state_show, NULL);
+static ssize_t regulator_state_set(struct device *dev,
+				   struct device_attribute *attr,
+				   const char *buf, size_t len)
+{
+	struct regulator_dev *rdev = dev_get_drvdata(dev);
+	bool enable;
+	ssize_t ret;
+
+	if (!rdev->desc->userspace_control)
+		return -EPERM;
+
+	if (sysfs_streq(buf, "enabled\n") || sysfs_streq(buf, "1"))
+		enable = true;
+	else if (sysfs_streq(buf, "disabled\n") || sysfs_streq(buf, "0"))
+		enable = false;
+	else
+		return -EINVAL;
+
+	mutex_lock(&rdev->mutex);
+
+	if ((enable && _regulator_is_enabled(rdev)) ||
+	    (!enable && !_regulator_is_enabled(rdev))) {
+		mutex_unlock(&rdev->mutex);
+		return -EBUSY;
+	}
+
+	ret = enable ? _regulator_enable(rdev) : _regulator_disable(rdev);
+
+	mutex_unlock(&rdev->mutex);
+
+	if (ret)
+		return ret;
+
+	return len;
+}
+static DEVICE_ATTR(state, 0644, regulator_state_show, regulator_state_set);
 
 static ssize_t regulator_status_show(struct device *dev,
 				   struct device_attribute *attr, char *buf)
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index 37b5324..0e7ad95 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -293,6 +293,9 @@ enum regulator_type {
  * @off_on_delay: guard time (in uS), before re-enabling a regulator
  *
  * @of_map_mode: Maps a hardware mode defined in a DeviceTree to a standard mode
+ *
+ * @userspace_control: A flag to indicate whether this regulator can be
+ *                     controlled from user-space.
  */
 struct regulator_desc {
 	const char *name;
@@ -347,6 +350,8 @@ struct regulator_desc {
 	unsigned int off_on_delay;
 
 	unsigned int (*of_map_mode)(unsigned int mode);
+
+	unsigned int userspace_control;
 };
 
 /**
-- 
2.9.3

Re: [PATCH 1/3] regulator: add support for user space controlled regulators

[Mark Brown]

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

On Fri, Dec 16, 2016 at 04:52:28PM +0100, Bartosz Golaszewski wrote:

> Add a new flag to struct regulator_desc indicating whether a regulator
> can be controlled from user space and implement a routine in regulator
> core allowing to toggle the regulator state via the sysfs 'state'
> attribute.

No, we've been through this repeatedly before - search the archives.
Write a driver for your hardware which exposes a control for bouncing
the power if that's something that makes sense in your application.
Doing this at the regulator level is just far too likely to result in
poorly integrated systems.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH 2/3] doc: DT: add new compatible to fixed regulator's binding

[Bartosz Golaszewski]

Extend the fixed regulator's device tree bindings with a new
compatible describing GPIO-driven power load switches.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 Documentation/devicetree/bindings/regulator/fixed-regulator.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/regulator/fixed-regulator.txt b/Documentation/devicetree/bindings/regulator/fixed-regulator.txt
index 4fae41d..306931c 100644
--- a/Documentation/devicetree/bindings/regulator/fixed-regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/fixed-regulator.txt
@@ -1,7 +1,9 @@
 Fixed Voltage regulators
 
 Required properties:
-- compatible: Must be "regulator-fixed";
+- compatible:
+	"regulator-fixed" for regular fixed-regulators
+	"gpio-power-switch" for gpio-driven power load switches
 
 Optional properties:
 - gpio: gpio to use for enable control
-- 
2.9.3

Re: [PATCH 2/3] doc: DT: add new compatible to fixed regulator's binding

[Rob Herring]

On Fri, Dec 16, 2016 at 04:52:29PM +0100, Bartosz Golaszewski wrote:
> Extend the fixed regulator's device tree bindings with a new
> compatible describing GPIO-driven power load switches.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  Documentation/devicetree/bindings/regulator/fixed-regulator.txt | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/regulator/fixed-regulator.txt b/Documentation/devicetree/bindings/regulator/fixed-regulator.txt
> index 4fae41d..306931c 100644
> --- a/Documentation/devicetree/bindings/regulator/fixed-regulator.txt
> +++ b/Documentation/devicetree/bindings/regulator/fixed-regulator.txt
> @@ -1,7 +1,9 @@
>  Fixed Voltage regulators
>  
>  Required properties:
> -- compatible: Must be "regulator-fixed";
> +- compatible:
> +	"regulator-fixed" for regular fixed-regulators
> +	"gpio-power-switch" for gpio-driven power load switches

Probably add "... which do no regulation" along the lines of Mark's 
comment.

>  
>  Optional properties:
>  - gpio: gpio to use for enable control
> -- 
> 2.9.3
> 

[PATCH 3/3] regulator: fixed: add support for gpio power switches

[Bartosz Golaszewski]

The difference between a regular fixed regulator and a GPIO power load
switch is the fact that the latter may be controlled from user space.

Reuse the fixed regulator driver to support power switches.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/regulator/fixed.c | 33 +++++++++++++++++++++++++--------
 1 file changed, 25 insertions(+), 8 deletions(-)

diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
index 988a747..f125a3e 100644
--- a/drivers/regulator/fixed.c
+++ b/drivers/regulator/fixed.c
@@ -31,6 +31,8 @@
 #include <linux/regulator/of_regulator.h>
 #include <linux/regulator/machine.h>
 
+#define REG_FIXED_POWER_SWITCH		BIT(0)
+
 struct fixed_voltage_data {
 	struct regulator_desc desc;
 	struct regulator_dev *dev;
@@ -97,11 +99,27 @@ of_get_fixed_voltage_config(struct device *dev,
 static struct regulator_ops fixed_voltage_ops = {
 };
 
+#if defined(CONFIG_OF)
+static const struct of_device_id fixed_of_match[] = {
+	{
+		.compatible = "regulator-fixed",
+	},
+	{
+		.compatible = "gpio-power-switch",
+		.data = (void *)REG_FIXED_POWER_SWITCH,
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, fixed_of_match);
+#endif
+
 static int reg_fixed_voltage_probe(struct platform_device *pdev)
 {
 	struct fixed_voltage_config *config;
 	struct fixed_voltage_data *drvdata;
 	struct regulator_config cfg = { };
+	const struct of_device_id *of_id;
+	long flags = 0;
 	int ret;
 
 	drvdata = devm_kzalloc(&pdev->dev, sizeof(struct fixed_voltage_data),
@@ -175,6 +193,13 @@ static int reg_fixed_voltage_probe(struct platform_device *pdev)
 	cfg.driver_data = drvdata;
 	cfg.of_node = pdev->dev.of_node;
 
+	of_id = of_match_node(fixed_of_match, pdev->dev.of_node);
+	if (of_id)
+		flags = (long)of_id->data;
+
+	if (flags & REG_FIXED_POWER_SWITCH)
+		drvdata->desc.userspace_control = 1;
+
 	drvdata->dev = devm_regulator_register(&pdev->dev, &drvdata->desc,
 					       &cfg);
 	if (IS_ERR(drvdata->dev)) {
@@ -191,14 +216,6 @@ static int reg_fixed_voltage_probe(struct platform_device *pdev)
 	return 0;
 }
 
-#if defined(CONFIG_OF)
-static const struct of_device_id fixed_of_match[] = {
-	{ .compatible = "regulator-fixed", },
-	{},
-};
-MODULE_DEVICE_TABLE(of, fixed_of_match);
-#endif
-
 static struct platform_driver regulator_fixed_voltage_driver = {
 	.probe		= reg_fixed_voltage_probe,
 	.driver		= {
-- 
2.9.3

Re: [PATCH 3/3] regulator: fixed: add support for gpio power switches

[Mark Brown]

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

On Fri, Dec 16, 2016 at 04:52:30PM +0100, Bartosz Golaszewski wrote:

> The difference between a regular fixed regulator and a GPIO power load
> switch is the fact that the latter may be controlled from user space.

This is not something I would understand or expect from the naming here.
I would expect the difference between a regulator and a switch to be
that the switch does no regulation, such things are fairly common in
system design for example as part of the logic that switches between the
different input power supplies available to the system to provide the
root system power rail.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]