[RESEND PATCHv3 0/2] regulator: handling of error conditions for usb drivers

[RESEND PATCHv3 0/2] regulator: handling of error conditions for usb drivers

[Axel Haslam]

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

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.

These patches extend the fixed regulator driver to handle an optional
over current gpio pin.

Changes v3 -> 3.1
* added Acked-by for DT bindings

Changes v2 -> v3
* dropped 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.9.3

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

[RESEND PATCHv3 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
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@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.9.3

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

[RESEND PATCHv3 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-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
---
 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.9.3

--
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: [RESEND PATCHv3 2/2] regulator: fixed: Handle optional overcurrent pin

[Mark Brown]

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

On Thu, Dec 15, 2016 at 12:28:56PM +0100, Axel Haslam wrote:

> +		ret = devm_request_threaded_irq(&pdev->dev,
> +				gpiod_to_irq(drvdata->oc_gpio), NULL,
> +				reg_fixed_overcurrent_irq, irqflags,
> +				"over_current", drvdata);

>  	drvdata->dev = devm_regulator_register(&pdev->dev, &drvdata->desc,
>  					       &cfg);

We are registering a managed interrupt for the interrupt handler and we
are registering it before we register the regulator.  This means that
the interrupt may fire in both probe and remove paths without the
regulator which it will then try to use in the interrupt handler,
potentially crashing the system.  It's better to register the interrupt
after the regulator (which will make the managed bit OK) to avoid this
possibility.

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

Re: [RESEND PATCHv3 2/2] regulator: fixed: Handle optional overcurrent pin

[Axel Haslam]

On Thu, Dec 15, 2016 at 1:19 PM, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Thu, Dec 15, 2016 at 12:28:56PM +0100, Axel Haslam wrote:
>
>> +             ret = devm_request_threaded_irq(&pdev->dev,
>> +                             gpiod_to_irq(drvdata->oc_gpio), NULL,
>> +                             reg_fixed_overcurrent_irq, irqflags,
>> +                             "over_current", drvdata);
>
>>       drvdata->dev = devm_regulator_register(&pdev->dev, &drvdata->desc,
>>                                              &cfg);
>
> We are registering a managed interrupt for the interrupt handler and we
> are registering it before we register the regulator.  This means that
> the interrupt may fire in both probe and remove paths without the
> regulator which it will then try to use in the interrupt handler,
> potentially crashing the system.  It's better to register the interrupt
> after the regulator (which will make the managed bit OK) to avoid this
> possibility.

will fix, and send v4.

Regards,
Axel.
--
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