[PATCH 0/3] regulator: Send events on over current condition

[PATCH 0/3] regulator: Send events on over current condition

[ahaslam]

From: Axel Haslam <ahaslam@baylibre.com>

Some usb drivers rely on external power switches/regulators to handle
the port vbus. Some of these drivers currently are still using gpios for
the enable pin and also the over current indicator.

We would like to move these drivers to use a regulator instead, because
it makes the driver generic allowing to use in the future any type of
regulator (maybe i2c based), And also it helps removing some code making
DT migration simpler and avoiding to add new bindings each time.

Vbus control is easy enough as the infrastructure to handle this
is in place using a fixed regulator for the simple gpio case.

Handling of the over current pin, however, needs some modifications
to the regulator framework, to be able to transmit the over current
pin status to the usb driver.

This is an attempt to extend the fixed regulator to handle the over
current pin, and send the status via a notification with minimal
framework changes, avoiding to use get_status which would have to 
be exported to consumers or get_mode which is not supposed to be
used for this purpose.

Axel Haslam (3):
  regulator: core: Add over current changed event
  regulator: fixed: Handle optional overcurrent pin
  regulator: fixed: dt: Allow an optional over current pin

 .../bindings/regulator/fixed-regulator.txt         |  4 ++
 drivers/regulator/fixed.c                          | 55 ++++++++++++++++++++++
 include/linux/regulator/consumer.h                 |  2 +
 include/linux/regulator/fixed.h                    |  3 ++
 4 files changed, 64 insertions(+)

-- 
1.9.1

[RFC 1/3] regulator: core: Add over current changed event

[ahaslam]

From: Axel Haslam <ahaslam@baylibre.com>

Regulator consumers may be interested to know when the
over current condition is over.

Add an over currerent "changed" event. The registered useres
for this event can then check the over current flag to know
the status of the over current condition.

Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
 include/linux/regulator/consumer.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
index 6921082..8e33f72 100644
--- a/include/linux/regulator/consumer.h
+++ b/include/linux/regulator/consumer.h
@@ -103,6 +103,7 @@
  *                      Data passed is old voltage cast to (void *).
  * PRE_DISABLE    Regulator is about to be disabled
  * ABORT_DISABLE  Regulator disable failed for some reason
+ * OVER_CURRENT_CHANGE  Regulator over current condition changed
  *
  * NOTE: These events can be OR'ed together when passed into handler.
  */
@@ -119,6 +120,7 @@
 #define REGULATOR_EVENT_ABORT_VOLTAGE_CHANGE	0x200
 #define REGULATOR_EVENT_PRE_DISABLE		0x400
 #define REGULATOR_EVENT_ABORT_DISABLE		0x800
+#define REGULATOR_EVENT_OVER_CURRENT_CHANGE     0x1000
 
 /**
  * struct pre_voltage_change_data - Data sent with PRE_VOLTAGE_CHANGE event
-- 
1.9.1

[RFC 2/3] regulator: fixed: Handle optional overcurrent pin

[ahaslam]

From: Axel Haslam <ahaslam@baylibre.com>

Fixed regulators (ex. TPS2087D) may have a over current pin that
is activated when an over current condition is detected. Consumers
may be interested to know about the status changes of this pin to
disable the regulator and notify framework layers or userspace.

Allow the fix regulator to take in an optional gpio pin to
send the event changes to the consumer.

Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
 drivers/regulator/fixed.c       | 47 +++++++++++++++++++++++++++++++++++++++++
 include/linux/regulator/fixed.h |  3 +++
 2 files changed, 50 insertions(+)

diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
index 988a747..9937139 100644
--- a/drivers/regulator/fixed.c
+++ b/drivers/regulator/fixed.c
@@ -30,10 +30,14 @@
 #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;
+	int oc_gpio;
+	unsigned has_oc_gpio:1;
+	unsigned oc_high:1;
 };
 
 
@@ -94,6 +98,22 @@ struct fixed_voltage_data {
 	return config;
 }
 
+static irqreturn_t reg_fixed_overcurrent_irq(int irq, void *data)
+{
+	struct fixed_voltage_data *drvdata = data;
+	unsigned long event = REGULATOR_EVENT_OVER_CURRENT_CHANGE;
+	int oc_value;
+
+	oc_value = gpio_get_value_cansleep(drvdata->oc_gpio);
+	if (oc_value == drvdata->oc_high)
+		event |= REGULATOR_EVENT_OVER_CURRENT;
+
+	regulator_notifier_call_chain(drvdata->dev, event, NULL);
+
+	return IRQ_HANDLED;
+}
+
+
 static struct regulator_ops fixed_voltage_ops = {
 };
 
@@ -175,6 +195,33 @@ static int reg_fixed_voltage_probe(struct platform_device *pdev)
 	cfg.driver_data = drvdata;
 	cfg.of_node = pdev->dev.of_node;
 
+	if (config->has_oc_gpio && gpio_is_valid(config->oc_gpio)) {
+
+		ret = devm_gpio_request_one(&pdev->dev,
+					config->oc_gpio,
+					GPIOF_DIR_IN, "oc_gpio");
+		if (ret) {
+			dev_err(&pdev->dev,
+				"Failed to request gpio: %d\n", ret);
+			return ret;
+		}
+
+		ret = devm_request_threaded_irq(&pdev->dev,
+				gpio_to_irq(config->oc_gpio), NULL,
+				reg_fixed_overcurrent_irq,
+				IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING |
+				IRQF_ONESHOT,
+				"over_current", drvdata);
+		if (ret) {
+			dev_err(&pdev->dev,
+				"Failed tp request irq: %d\n", ret);
+			return ret;
+		}
+		drvdata->oc_gpio = config->oc_gpio;
+		drvdata->oc_high = config->oc_high;
+		drvdata->has_oc_gpio = config->has_oc_gpio;
+	}
+
 	drvdata->dev = devm_regulator_register(&pdev->dev, &drvdata->desc,
 					       &cfg);
 	if (IS_ERR(drvdata->dev)) {
diff --git a/include/linux/regulator/fixed.h b/include/linux/regulator/fixed.h
index 48918be..79357be 100644
--- a/include/linux/regulator/fixed.h
+++ b/include/linux/regulator/fixed.h
@@ -50,10 +50,13 @@ struct fixed_voltage_config {
 	const char *input_supply;
 	int microvolts;
 	int gpio;
+	int oc_gpio;
 	unsigned startup_delay;
 	unsigned gpio_is_open_drain:1;
 	unsigned enable_high:1;
 	unsigned enabled_at_boot:1;
+	unsigned has_oc_gpio:1;
+	unsigned oc_high:1;
 	struct regulator_init_data *init_data;
 };
 
-- 
1.9.1

[RFC 3/3] regulator: fixed: dt: Allow an optional over current pin

[ahaslam]

From: Axel Haslam <ahaslam@baylibre.com>

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

Cc: devicetree@vger.kernel.org
Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
---
 Documentation/devicetree/bindings/regulator/fixed-regulator.txt | 4 ++++
 drivers/regulator/fixed.c                                       | 8 ++++++++
 2 files changed, 12 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/fixed-regulator.txt b/Documentation/devicetree/bindings/regulator/fixed-regulator.txt
index 4fae41d..0c140cb 100644
--- a/Documentation/devicetree/bindings/regulator/fixed-regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/fixed-regulator.txt
@@ -11,6 +11,8 @@ 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.
+- oc-gpio: Input gpio that signals an over current condition.
+- oc-active-high: The polarity of the over current pin is high.
 
 Any property defined as part of the core regulator
 binding, defined in regulator.txt, can also be used.
@@ -26,9 +28,11 @@ Example:
 		regulator-min-microvolt = <1800000>;
 		regulator-max-microvolt = <1800000>;
 		gpio = <&gpio1 16 0>;
+		oc-gpio = <&gpio1 18 0>;
 		startup-delay-us = <70000>;
 		enable-active-high;
 		regulator-boot-on;
 		gpio-open-drain;
+		oc-active-high;
 		vin-supply = <&parent_reg>;
 	};
diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
index 9937139..f02d24c 100644
--- a/drivers/regulator/fixed.c
+++ b/drivers/regulator/fixed.c
@@ -86,6 +86,14 @@ struct fixed_voltage_data {
 	if ((config->gpio < 0) && (config->gpio != -ENOENT))
 		return ERR_PTR(config->gpio);
 
+	config->oc_gpio = of_get_named_gpio(np, "oc-gpio", 0);
+	if (gpio_is_valid(config->oc_gpio))
+		config->has_oc_gpio = true;
+	else if (config->oc_gpio != -ENOENT)
+		return ERR_PTR(config->oc_gpio);
+
+	config->oc_high = of_property_read_bool(np, "oc-active-high");
+
 	of_property_read_u32(np, "startup-delay-us", &config->startup_delay);
 
 	config->enable_high = of_property_read_bool(np, "enable-active-high");
-- 
1.9.1

Re: [RFC 1/3] regulator: core: Add over current changed event

[Mark Brown]

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

On Wed, Oct 26, 2016 at 09:00:52PM +0200, ahaslam@baylibre.com wrote:
> From: Axel Haslam <ahaslam@baylibre.com>
> 
> Regulator consumers may be interested to know when the
> over current condition is over.
> 
> Add an over currerent "changed" event. The registered useres
> for this event can then check the over current flag to know
> the status of the over current condition.

Would a more general event for error conditions work as well?  Thinking
about this I'm unclear how interested consumers are going to be in the
specific error condition as opposed to the fact that the regulator ran
into trouble, and I can imagine that some regulators will report the
same root cause differently - another regulator might detect an
excessive current draw by seeing the output voltage collapse and the
regulator go out of regulation for example.

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

Re: [RFC 1/3] regulator: core: Add over current changed event

[Axel Haslam]

Hi Mark,

On Fri, Oct 28, 2016 at 8:22 PM, Mark Brown <broonie@kernel.org> wrote:
> On Wed, Oct 26, 2016 at 09:00:52PM +0200, ahaslam@baylibre.com wrote:
>> From: Axel Haslam <ahaslam@baylibre.com>
>>
>> Regulator consumers may be interested to know when the
>> over current condition is over.
>>
>> Add an over currerent "changed" event. The registered useres
>> for this event can then check the over current flag to know
>> the status of the over current condition.
>
> Would a more general event for error conditions work as well?  Thinking
> about this I'm unclear how interested consumers are going to be in the
> specific error condition as opposed to the fact that the regulator ran
> into trouble, and I can imagine that some regulators will report the
> same root cause differently - another regulator might detect an
> excessive current draw by seeing the output voltage collapse and the
> regulator go out of regulation for example.

Sorry if i misunderstood, but if we make the name generic,
i think it might change a bit the definition of the flags,
The flags will not represent events, but states.

i think today each time an event occurs a notification is sent with the
corresponding flag(s) set.

if we use a generic name, It means that each time the regulator driver
sends an event, it should check which "other" error conditons tied to the
generic flag are present and set the corresponding bits too.

illustrative example:
today over current and over temp are two different events
we send one notification for each with only the bits tied to the
event that is happening set.

if we add a generic error flag, it would mean that if over current happens
and we set the generic error flag, we would also have to check
if over temp is present to set or not that flag. similarly, when the over
temp event happens the regulator driver would have to check if over
current is present too.


Regards
Axel.

Regards
Axel.

Re: [RFC 1/3] regulator: core: Add over current changed event

[Axel Haslam]

Hi Mark,

On Fri, Oct 28, 2016 at 9:41 PM, Axel Haslam <ahaslam@baylibre.com> wrote:
> Hi Mark,
>
> On Fri, Oct 28, 2016 at 8:22 PM, Mark Brown <broonie@kernel.org> wrote:
>> On Wed, Oct 26, 2016 at 09:00:52PM +0200, ahaslam@baylibre.com wrote:
>>> From: Axel Haslam <ahaslam@baylibre.com>
>>>
>>> Regulator consumers may be interested to know when the
>>> over current condition is over.
>>>
>>> Add an over currerent "changed" event. The registered useres
>>> for this event can then check the over current flag to know
>>> the status of the over current condition.
>>
>> Would a more general event for error conditions work as well?  Thinking
>> about this I'm unclear how interested consumers are going to be in the
>> specific error condition as opposed to the fact that the regulator ran
>> into trouble, and I can imagine that some regulators will report the
>> same root cause differently - another regulator might detect an
>> excessive current draw by seeing the output voltage collapse and the
>> regulator go out of regulation for example.
>

After some more thought,

I can change the logic a bit, and send an event named something like:

REGULATOR_EVENT_ERRORS_CLEARED

would that make more sense?

-Axel.


> Sorry if i misunderstood, but if we make the name generic,
> i think it might change a bit the definition of the flags,
> The flags will not represent events, but states.
>
> i think today each time an event occurs a notification is sent with the
> corresponding flag(s) set.
>
> if we use a generic name, It means that each time the regulator driver
> sends an event, it should check which "other" error conditons tied to the
> generic flag are present and set the corresponding bits too.
>
> illustrative example:
> today over current and over temp are two different events
> we send one notification for each with only the bits tied to the
> event that is happening set.
>
> if we add a generic error flag, it would mean that if over current happens
> and we set the generic error flag, we would also have to check
> if over temp is present to set or not that flag. similarly, when the over
> temp event happens the regulator driver would have to check if over
> current is present too.
>

Re: [RFC 1/3] regulator: core: Add over current changed event

[Mark Brown]

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

On Fri, Oct 28, 2016 at 09:41:44PM +0200, Axel Haslam wrote:

> i think today each time an event occurs a notification is sent with the
> corresponding flag(s) set.

Right, so I think the problem here is actually that you called this 
REGULATOR_EVENT_OVER_CURRENT_CHANGE with the _CHANGE on the end which
means it's just saying that the user has to go poll to see if the device
is or is not over current separately at which point you may as well pull
in all the other error things into what you're polling for.  If you'd
dropped the _CHANGE it'd be consistent with the other events we have for
errors and fine.

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

Re: [RFC 1/3] regulator: core: Add over current changed event

[Axel Haslam]

Hi Mark,

On Sat, Oct 29, 2016 at 8:40 PM, Mark Brown <broonie@kernel.org> wrote:
> On Fri, Oct 28, 2016 at 09:41:44PM +0200, Axel Haslam wrote:
>
>> i think today each time an event occurs a notification is sent with the
>> corresponding flag(s) set.
>
> Right, so I think the problem here is actually that you called this
> REGULATOR_EVENT_OVER_CURRENT_CHANGE with the _CHANGE on the end which
> means it's just saying that the user has to go poll to see if the device
> is or is not over current separately at which point you may as well pull
> in all the other error things into what you're polling for.  If you'd
> dropped the _CHANGE it'd be consistent with the other events we have for
> errors and fine.

The event  REGULATOR_EVENT_OVER_CURRENT allready exists.
what is missing and what i would need form the usb driver, is a way for
the consumer to know that the over current condition is over.
since i cannot do this with get mode, and get status is not exported,

We can do this adding a more generic event flag:
REGULATOR_EVENT_ERRORS_CLEARED

that would be sent by the supply when all errors are over, and the
regulator is back to  normal operation.

Regards
Axel.

Re: [RFC 1/3] regulator: core: Add over current changed event

[Mark Brown]

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

On Sun, Oct 30, 2016 at 01:02:21PM +0100, Axel Haslam wrote:

> The event  REGULATOR_EVENT_OVER_CURRENT allready exists.
> what is missing and what i would need form the usb driver, is a way for
> the consumer to know that the over current condition is over.
> since i cannot do this with get mode, and get status is not exported,

> We can do this adding a more generic event flag:
> REGULATOR_EVENT_ERRORS_CLEARED

> that would be sent by the supply when all errors are over, and the
> regulator is back to  normal operation.

That's a different thing and definitely not what you were saying in the
changelog.  I don't think this is something that it makes sense to do
with events as it's not something that devices will tend to generate
interrupts for, anything that is going to rely on events for that is
going to be broken.  Hardware is mostly designed with the idea that
errors are catastrophic.

If you really care about things clearing then you need to add a sensible
interface for exposting all the possible error conditions that users can
poll.  The reason get_mode() got rejected was that error statuses and
modes are completely different things, get_status() is not going to work
for you since it is very common for multiple errors to happen at the
same time.

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

Re: [RFC 1/3] regulator: core: Add over current changed event

[Axel Haslam]

Hi Mark,

On Mon, Oct 31, 2016 at 5:22 PM, Mark Brown <broonie@kernel.org> wrote:
> On Sun, Oct 30, 2016 at 01:02:21PM +0100, Axel Haslam wrote:
>
>> The event  REGULATOR_EVENT_OVER_CURRENT allready exists.
>> what is missing and what i would need form the usb driver, is a way for
>> the consumer to know that the over current condition is over.
>> since i cannot do this with get mode, and get status is not exported,
>
>> We can do this adding a more generic event flag:
>> REGULATOR_EVENT_ERRORS_CLEARED
>
>> that would be sent by the supply when all errors are over, and the
>> regulator is back to  normal operation.
>
> That's a different thing and definitely not what you were saying in the
> changelog.  I don't think this is something that it makes sense to do
> with events as it's not something that devices will tend to generate
> interrupts for, anything that is going to rely on events for that is
> going to be broken.  Hardware is mostly designed with the idea that
> errors are catastrophic.
>
> If you really care about things clearing then you need to add a sensible
> interface for exposting all the possible error conditions that users can
> poll.  The reason get_mode() got rejected was that error statuses and
> modes are completely different things, get_status() is not going to work
> for you since it is very common for multiple errors to happen at the
> same time.

Ok, sorry if i was unclear in the change log.
ill add a new interface and lets see if it makes more sense in v2.

Regards
Axel.