[PATCH v3 0/2] regulator: handling of error conditions for usb drivers

[PATCH v3 0/2] regulator: handling of error conditions for usb drivers

[Axel Haslam]

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 v2 -> v3
* droped merged patch to add new API
* rebased on top of regulator-next

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 (2):
  regulator: fixed: dt: Allow an optional over current pin
  regulator: fixed: Handle optional overcurrent pin

 .../bindings/regulator/fixed-regulator.txt         |  2 +
 drivers/regulator/fixed.c                          | 59 ++++++++++++++++++++++
 2 files changed, 61 insertions(+)

-- 
2.10.1

[PATCH v3 1/2] regulator: fixed: dt: Allow an optional over current pin

[Axel Haslam]

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 | 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

[PATCH v3 1/2] regulator: fixed: dt: Allow an optional over current pin

[Axel Haslam]

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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Axel Haslam <ahaslam-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
 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

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

Re: [PATCH v3 1/2] regulator: fixed: dt: Allow an optional over current pin

[Rob Herring]

On Fri, Nov 04, 2016 at 10:35:35PM +0100, Axel Haslam wrote:
> 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 | 2 ++
>  1 file changed, 2 insertions(+)

Acked-by: Rob Herring <robh@kernel.org>

[PATCH v3 2/2] regulator: fixed: Handle optional overcurrent pin

[Axel Haslam]

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 <ahaslam@baylibre.com>
---
 drivers/regulator/fixed.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
index a43b0e8..06ed2f6 100644
--- a/drivers/regulator/fixed.c
+++ b/drivers/regulator/fixed.c
@@ -33,10 +33,12 @@
 #include <linux/acpi.h>
 #include <linux/property.h>
 #include <linux/gpio/consumer.h>
+#include <linux/interrupt.h>
 
 struct fixed_voltage_data {
 	struct regulator_desc desc;
 	struct regulator_dev *dev;
+	struct gpio_desc *oc_gpio;
 };
 
 
@@ -135,7 +137,36 @@ acpi_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)
@@ -143,6 +174,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),
@@ -221,6 +253,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 to 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

Re: [PATCH v3 0/2] regulator: handling of error conditions for usb drivers

[Axel Haslam]

Hi Mark,


On Fri, Nov 4, 2016 at 10:35 PM, Axel Haslam <ahaslam@baylibre.com> wrote:
> 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 v2 -> v3
> * droped merged patch to add new API
> * rebased on top of regulator-next
>
> 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 (2):
>   regulator: fixed: dt: Allow an optional over current pin
>   regulator: fixed: Handle optional overcurrent pin


im wondering if you have these 2 patches in queue,
if there are no objections would it be possible to take them?

The DT binding is Acked by Rob, would you prefer
i resend them with the Ack tag?

Regards
Axel.

>
>  .../bindings/regulator/fixed-regulator.txt         |  2 +
>  drivers/regulator/fixed.c                          | 59 ++++++++++++++++++++++
>  2 files changed, 61 insertions(+)
>
> --
> 2.10.1
>

Re: [PATCH v3 0/2] regulator: handling of error conditions for usb drivers

[Mark Brown]

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

On Tue, Nov 22, 2016 at 05:35:38PM +0100, Axel Haslam wrote:

> im wondering if you have these 2 patches in queue,
> if there are no objections would it be possible to take them?


Please don't send content free pings and please allow a reasonable time
for review.  People get busy, go on holiday, attend conferences and so 
on so unless there is some reason for urgency (like critical bug fixes)
please allow at least a couple of weeks for review.  If there have been
review comments then people may be waiting for those to be addressed.

Sending content free pings adds to the mail volume (if they are seen at
all) which is often the problem and since they can't be reviewed
directly if something has gone wrong you'll have to resend the patches
anyway, though there are some other maintainers who like them - if in
doubt look at how patches for the subsystem are normally handled.

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