2016-11-03 11:11:50

by Axel Haslam

[permalink] [raw]
Subject: [PATCH/RFC v2 0/3] regulator: handling of error conditions for usb drivers

Some usb drivers rely on external power switches/regulators
to for the port vbus. Some of these drivers are using
a plain gpio for the enable pin and also the over current
indicator pin.

To make these drivers more generic, we can use a regulator
to handle vbus, and send and over current event, but we are
missing a way to transmit the over current pin status, which
the usb layer may poll at any time.

We would like to move these drivers to use a regulator, this
would make the usb driver generic allowing to use any type
of regulator. Also, it would help removing code, making DT
migration simpler and avoiding new DT bindings for each driver.

These patches do 2 things:
* Add a new API, that consumers can use to poll the regulator
error status.
* Extends the fixed regulator driver to handle an optional
over current gpio pin.

Changes v1->v2
* add new API to get error status instead of extending events (Mark)
* use gpiod for fixed regulator: This spears us extra platform
data and bindings

Axel Haslam (3):
regulator: core: Add new API to poll for error conditions
regulator: fixed: dt: Allow an optional over current pin
regulator: fixed: Handle optional overcurrent pin

.../bindings/regulator/fixed-regulator.txt | 2 +
drivers/regulator/core.c | 33 ++++++++++++
drivers/regulator/fixed.c | 60 ++++++++++++++++++++++
include/linux/regulator/consumer.h | 26 ++++++++++
include/linux/regulator/driver.h | 4 ++
5 files changed, 125 insertions(+)

--
2.10.1


2016-11-03 11:12:14

by Axel Haslam

[permalink] [raw]
Subject: [PATCH/RFC v2 1/3] regulator: core: Add new API to poll for error conditions

Regulator consumers can receive event notifications when
errors are reported to the driver, but currently, there is
no way for a regulator consumer to know when the error is over.

To allow a regulator consumer to poll for error conditions
add a new API: regulator_get_error_flags.

Signed-off-by: Axel Haslam <[email protected]>
---
drivers/regulator/core.c | 33 +++++++++++++++++++++++++++++++++
include/linux/regulator/consumer.h | 26 ++++++++++++++++++++++++++
include/linux/regulator/driver.h | 4 ++++
3 files changed, 63 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 67426c0..08260c2 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3359,6 +3359,39 @@ unsigned int regulator_get_mode(struct regulator *regulator)
}
EXPORT_SYMBOL_GPL(regulator_get_mode);

+static int _regulator_get_error_flags(struct regulator_dev *rdev,
+ unsigned int *flags)
+{
+ int ret;
+
+ mutex_lock(&rdev->mutex);
+
+ /* sanity check */
+ if (!rdev->desc->ops->get_error_flags) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ ret = rdev->desc->ops->get_error_flags(rdev, flags);
+out:
+ mutex_unlock(&rdev->mutex);
+ return ret;
+}
+
+/**
+ * regulator_get_error_flags - get regulator error information
+ * @regulator: regulator source
+ * @flags: pointer to store error flags
+ *
+ * Get the current regulator error information.
+ */
+int regulator_get_error_flags(struct regulator *regulator,
+ unsigned int *flags)
+{
+ return _regulator_get_error_flags(regulator->rdev, flags);
+}
+EXPORT_SYMBOL_GPL(regulator_get_error_flags);
+
/**
* regulator_set_load - set regulator load
* @regulator: regulator source
diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
index 6921082..528eb1f 100644
--- a/include/linux/regulator/consumer.h
+++ b/include/linux/regulator/consumer.h
@@ -120,6 +120,25 @@ struct regmap;
#define REGULATOR_EVENT_PRE_DISABLE 0x400
#define REGULATOR_EVENT_ABORT_DISABLE 0x800

+/*
+ * Regulator errors that can be queried using regulator_get_error_flags
+ *
+ * UNDER_VOLTAGE Regulator output is under voltage.
+ * OVER_CURRENT Regulator output current is too high.
+ * REGULATION_OUT Regulator output is out of regulation.
+ * FAIL Regulator output has failed.
+ * OVER_TEMP Regulator over temp.
+ *
+ * NOTE: These errors can be OR'ed together.
+ */
+
+#define REGULATOR_ERROR_UNDER_VOLTAGE BIT(1)
+#define REGULATOR_ERROR_OVER_CURRENT BIT(2)
+#define REGULATOR_ERROR_REGULATION_OUT BIT(3)
+#define REGULATOR_ERROR_FAIL BIT(4)
+#define REGULATOR_ERROR_OVER_TEMP BIT(5)
+
+
/**
* struct pre_voltage_change_data - Data sent with PRE_VOLTAGE_CHANGE event
*
@@ -237,6 +256,8 @@ int regulator_get_current_limit(struct regulator *regulator);

int regulator_set_mode(struct regulator *regulator, unsigned int mode);
unsigned int regulator_get_mode(struct regulator *regulator);
+int regulator_get_error_flags(struct regulator *regulator,
+ unsigned int *flags);
int regulator_set_load(struct regulator *regulator, int load_uA);

int regulator_allow_bypass(struct regulator *regulator, bool allow);
@@ -477,6 +498,11 @@ static inline unsigned int regulator_get_mode(struct regulator *regulator)
return REGULATOR_MODE_NORMAL;
}

+static inline int regulator_get_error_flags(struct regulator *regulator)
+{
+ return -EINVAL;
+}
+
static inline int regulator_set_load(struct regulator *regulator, int load_uA)
{
return REGULATOR_MODE_NORMAL;
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index 37b5324..dac8e7b1 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -100,6 +100,7 @@ struct regulator_linear_range {
*
* @set_mode: Set the configured operating mode for the regulator.
* @get_mode: Get the configured operating mode for the regulator.
+ * @get_error_flags: Get the current error(s) for the regulator.
* @get_status: Return actual (not as-configured) status of regulator, as a
* REGULATOR_STATUS value (or negative errno)
* @get_optimum_mode: Get the most efficient operating mode for the regulator
@@ -169,6 +170,9 @@ struct regulator_ops {
int (*set_mode) (struct regulator_dev *, unsigned int mode);
unsigned int (*get_mode) (struct regulator_dev *);

+ /* retrieve current error flags on the regulator */
+ int (*get_error_flags)(struct regulator_dev *, unsigned int *flags);
+
/* Time taken to enable or set voltage on the regulator */
int (*enable_time) (struct regulator_dev *);
int (*set_ramp_delay) (struct regulator_dev *, int ramp_delay);
--
2.10.1

2016-11-03 11:11:53

by Axel Haslam

[permalink] [raw]
Subject: [PATCH/RFC v2 3/3] regulator: fixed: Handle optional overcurrent pin

Fixed regulators (ex. TPS2087D) may have a over current pin that
is activated on over current. Consumers may be interested to know
about over current events to take appropriate actions.

Allow the fix regulator to take in an optional gpio pin for over
current and send the respective event to the consumer.

Signed-off-by: Axel Haslam <[email protected]>
---
drivers/regulator/fixed.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 60 insertions(+)

diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
index 988a747..9306d38 100644
--- a/drivers/regulator/fixed.c
+++ b/drivers/regulator/fixed.c
@@ -25,15 +25,18 @@
#include <linux/regulator/driver.h>
#include <linux/regulator/fixed.h>
#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
#include <linux/slab.h>
#include <linux/of.h>
#include <linux/of_gpio.h>
#include <linux/regulator/of_regulator.h>
#include <linux/regulator/machine.h>
+#include <linux/interrupt.h>

struct fixed_voltage_data {
struct regulator_desc desc;
struct regulator_dev *dev;
+ struct gpio_desc *oc_gpio;
};


@@ -94,7 +97,36 @@ of_get_fixed_voltage_config(struct device *dev,
return config;
}

+static irqreturn_t reg_fixed_overcurrent_irq(int irq, void *data)
+{
+ struct fixed_voltage_data *drvdata = data;
+
+ regulator_notifier_call_chain(drvdata->dev,
+ REGULATOR_EVENT_OVER_CURRENT, NULL);
+
+ return IRQ_HANDLED;
+}
+
+static int reg_fixed_get_error_flags(struct regulator_dev *dev,
+ unsigned int *flags)
+{
+ struct fixed_voltage_data *drvdata = rdev_get_drvdata(dev);
+ int oc_value;
+
+ *flags = 0;
+
+ if (!drvdata->oc_gpio)
+ return 0;
+
+ oc_value = gpiod_get_value_cansleep(drvdata->oc_gpio);
+ if (oc_value)
+ *flags = REGULATOR_ERROR_OVER_CURRENT;
+
+ return 0;
+}
+
static struct regulator_ops fixed_voltage_ops = {
+ .get_error_flags = reg_fixed_get_error_flags,
};

static int reg_fixed_voltage_probe(struct platform_device *pdev)
@@ -102,6 +134,7 @@ static int reg_fixed_voltage_probe(struct platform_device *pdev)
struct fixed_voltage_config *config;
struct fixed_voltage_data *drvdata;
struct regulator_config cfg = { };
+ unsigned long irqflags = IRQF_ONESHOT;
int ret;

drvdata = devm_kzalloc(&pdev->dev, sizeof(struct fixed_voltage_data),
@@ -175,6 +208,33 @@ static int reg_fixed_voltage_probe(struct platform_device *pdev)
cfg.driver_data = drvdata;
cfg.of_node = pdev->dev.of_node;

+
+ drvdata->oc_gpio = devm_gpiod_get_optional(&pdev->dev, "over-current",
+ GPIOF_DIR_IN);
+ if (IS_ERR(drvdata->oc_gpio)) {
+ ret = PTR_ERR(drvdata->oc_gpio);
+ dev_err(&pdev->dev,
+ "Failed to get over current gpio: %d\n", ret);
+ return ret;
+ }
+
+ if (drvdata->oc_gpio) {
+ if (gpiod_is_active_low(drvdata->oc_gpio))
+ irqflags |= IRQF_TRIGGER_FALLING;
+ else
+ irqflags |= IRQF_TRIGGER_RISING;
+
+ ret = devm_request_threaded_irq(&pdev->dev,
+ gpiod_to_irq(drvdata->oc_gpio), NULL,
+ reg_fixed_overcurrent_irq, irqflags,
+ "over_current", drvdata);
+ if (ret) {
+ dev_err(&pdev->dev,
+ "Failed tp request irq: %d\n", ret);
+ return ret;
+ }
+ }
+
drvdata->dev = devm_regulator_register(&pdev->dev, &drvdata->desc,
&cfg);
if (IS_ERR(drvdata->dev)) {
--
2.10.1

2016-11-03 11:12:13

by Axel Haslam

[permalink] [raw]
Subject: [PATCH/RFC v2 2/3] regulator: fixed: dt: Allow an optional over current pin

Add support for an optional over current input pin which
can be used to send an over current event to the regulator
consumer.

Cc: [email protected]
Signed-off-by: Axel Haslam <[email protected]>
---
Documentation/devicetree/bindings/regulator/fixed-regulator.txt | 2 ++
1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/fixed-regulator.txt b/Documentation/devicetree/bindings/regulator/fixed-regulator.txt
index 4fae41d..b145abb 100644
--- a/Documentation/devicetree/bindings/regulator/fixed-regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/fixed-regulator.txt
@@ -11,6 +11,7 @@ If this property is missing, the default assumed is Active low.
- gpio-open-drain: GPIO is open drain type.
If this property is missing then default assumption is false.
-vin-supply: Input supply name.
+- over-current-gpios: Input gpio that signal an over current condition.

Any property defined as part of the core regulator
binding, defined in regulator.txt, can also be used.
@@ -26,6 +27,7 @@ Example:
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <1800000>;
gpio = <&gpio1 16 0>;
+ over-current-gpios = <&gpio1 18 0>;
startup-delay-us = <70000>;
enable-active-high;
regulator-boot-on;
--
2.10.1

2016-11-04 18:16:47

by Mark Brown

[permalink] [raw]
Subject: Applied "regulator: core: Add new API to poll for error conditions" to the regulator tree

The patch

regulator: core: Add new API to poll for error conditions

has been applied to the regulator tree at

git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 1b5b42216469b05ef4b5916cb40b127dfab1da88 Mon Sep 17 00:00:00 2001
From: Axel Haslam <[email protected]>
Date: Thu, 3 Nov 2016 12:11:42 +0100
Subject: [PATCH] regulator: core: Add new API to poll for error conditions

Regulator consumers can receive event notifications when
errors are reported to the driver, but currently, there is
no way for a regulator consumer to know when the error is over.

To allow a regulator consumer to poll for error conditions
add a new API: regulator_get_error_flags.

Signed-off-by: Axel Haslam <[email protected]>
Signed-off-by: Mark Brown <[email protected]>
---
drivers/regulator/core.c | 33 +++++++++++++++++++++++++++++++++
include/linux/regulator/consumer.h | 26 ++++++++++++++++++++++++++
include/linux/regulator/driver.h | 4 ++++
3 files changed, 63 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 67426c0477d3..08260c215895 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3359,6 +3359,39 @@ unsigned int regulator_get_mode(struct regulator *regulator)
}
EXPORT_SYMBOL_GPL(regulator_get_mode);

+static int _regulator_get_error_flags(struct regulator_dev *rdev,
+ unsigned int *flags)
+{
+ int ret;
+
+ mutex_lock(&rdev->mutex);
+
+ /* sanity check */
+ if (!rdev->desc->ops->get_error_flags) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ ret = rdev->desc->ops->get_error_flags(rdev, flags);
+out:
+ mutex_unlock(&rdev->mutex);
+ return ret;
+}
+
+/**
+ * regulator_get_error_flags - get regulator error information
+ * @regulator: regulator source
+ * @flags: pointer to store error flags
+ *
+ * Get the current regulator error information.
+ */
+int regulator_get_error_flags(struct regulator *regulator,
+ unsigned int *flags)
+{
+ return _regulator_get_error_flags(regulator->rdev, flags);
+}
+EXPORT_SYMBOL_GPL(regulator_get_error_flags);
+
/**
* regulator_set_load - set regulator load
* @regulator: regulator source
diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
index 692108222271..528eb1f5273e 100644
--- a/include/linux/regulator/consumer.h
+++ b/include/linux/regulator/consumer.h
@@ -120,6 +120,25 @@ struct regmap;
#define REGULATOR_EVENT_PRE_DISABLE 0x400
#define REGULATOR_EVENT_ABORT_DISABLE 0x800

+/*
+ * Regulator errors that can be queried using regulator_get_error_flags
+ *
+ * UNDER_VOLTAGE Regulator output is under voltage.
+ * OVER_CURRENT Regulator output current is too high.
+ * REGULATION_OUT Regulator output is out of regulation.
+ * FAIL Regulator output has failed.
+ * OVER_TEMP Regulator over temp.
+ *
+ * NOTE: These errors can be OR'ed together.
+ */
+
+#define REGULATOR_ERROR_UNDER_VOLTAGE BIT(1)
+#define REGULATOR_ERROR_OVER_CURRENT BIT(2)
+#define REGULATOR_ERROR_REGULATION_OUT BIT(3)
+#define REGULATOR_ERROR_FAIL BIT(4)
+#define REGULATOR_ERROR_OVER_TEMP BIT(5)
+
+
/**
* struct pre_voltage_change_data - Data sent with PRE_VOLTAGE_CHANGE event
*
@@ -237,6 +256,8 @@ int regulator_get_current_limit(struct regulator *regulator);

int regulator_set_mode(struct regulator *regulator, unsigned int mode);
unsigned int regulator_get_mode(struct regulator *regulator);
+int regulator_get_error_flags(struct regulator *regulator,
+ unsigned int *flags);
int regulator_set_load(struct regulator *regulator, int load_uA);

int regulator_allow_bypass(struct regulator *regulator, bool allow);
@@ -477,6 +498,11 @@ static inline unsigned int regulator_get_mode(struct regulator *regulator)
return REGULATOR_MODE_NORMAL;
}

+static inline int regulator_get_error_flags(struct regulator *regulator)
+{
+ return -EINVAL;
+}
+
static inline int regulator_set_load(struct regulator *regulator, int load_uA)
{
return REGULATOR_MODE_NORMAL;
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index 37b532410528..dac8e7b16bc6 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -100,6 +100,7 @@ struct regulator_linear_range {
*
* @set_mode: Set the configured operating mode for the regulator.
* @get_mode: Get the configured operating mode for the regulator.
+ * @get_error_flags: Get the current error(s) for the regulator.
* @get_status: Return actual (not as-configured) status of regulator, as a
* REGULATOR_STATUS value (or negative errno)
* @get_optimum_mode: Get the most efficient operating mode for the regulator
@@ -169,6 +170,9 @@ struct regulator_ops {
int (*set_mode) (struct regulator_dev *, unsigned int mode);
unsigned int (*get_mode) (struct regulator_dev *);

+ /* retrieve current error flags on the regulator */
+ int (*get_error_flags)(struct regulator_dev *, unsigned int *flags);
+
/* Time taken to enable or set voltage on the regulator */
int (*enable_time) (struct regulator_dev *);
int (*set_ramp_delay) (struct regulator_dev *, int ramp_delay);
--
2.10.1

2016-11-04 18:20:16

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH/RFC v2 3/3] regulator: fixed: Handle optional overcurrent pin

On Thu, Nov 03, 2016 at 12:11:44PM +0100, Axel Haslam wrote:
> Fixed regulators (ex. TPS2087D) may have a over current pin that
> is activated on over current. Consumers may be interested to know
> about over current events to take appropriate actions.

This doesn't apply against current code, please check and resend.


Attachments:
(No filename) (320.00 B)
signature.asc (455.00 B)
Download all attachments