Skip to contentSkip to conversation

iio:pressure:ms5611: fix and enhancements

PATCH In review Gregor Boirie 2 participants 3 messages Mar 17, 2016 – Mar 20, 2016 IIO

lore
Share & tools
lore

Gregor Boirie

author
#0 lore raw

v7 cover iio:pressure:ms5611: fix and enhancements

Changes since v1:
* temperature compensation fix:
* reword commit message
* remove useless parenthesis
* power regulator: reword commit message
* add oversampling rate support

Changes since v2:
* remove non printable characters from commit messages

Changes since v3:
* drop first 3 patches since applied onto testing
* fix oops at regulator probing time if regulator support not compiled-in
* complete device-tree support and documentation
* remove continuous sampling support (see Jonathan's "kthread tight loop trigger" proposal)
* checkpatch'ed

Changes since v4:
* drop first 2 patches since applied
* amend commit to include "Fixes" tag

Changes since v5:
* reword patch subject line and include "Fixes" tag in commit message

Changes since v6:
* use devm_regulator_get to allow dummy regulator usage
* ensure regulator is properly disabled

Regards,
Gregor.

Gregor Boirie (1):
iio:pressure:ms5611: fix missing regulator_disable

 drivers/iio/pressure/ms5611.h      |  3 +++
 drivers/iio/pressure/ms5611_core.c | 41 +++++++++++++++++++++++++++++---------
 2 files changed, 35 insertions(+), 9 deletions(-)
Signature · 1 line
2.1.4
author
#1 lore raw

PATCH v7 1/1 iio:pressure:ms5611: fix missing regulator_disable

Ensure optional regulator is properly disabled when present.

Fixes 3145229f9191 ("iio:pressure:ms5611: power regulator support")
Signed-off-by Gregor Boirie <[email protected]>
---
 drivers/iio/pressure/ms5611.h      |  3 +++
 drivers/iio/pressure/ms5611_core.c | 41 +++++++++++++++++++++++++++++---------
 2 files changed, 35 insertions(+), 9 deletions(-)
drivers/iio/pressure/ms5611.h +3 -0
index d725a307..ccda63c 100644
@@ -16,6 +16,8 @@
 #include <linux/iio/iio.h>
 #include <linux/mutex.h>
 
+struct regulator;
+
 #define MS5611_RESET			0x1e
 #define MS5611_READ_ADC			0x00
 #define MS5611_READ_PROM_WORD		0xA0

@@ -57,6 +59,7 @@ struct ms5611_state {
 					  s32 *temp, s32 *pressure);
 
 	struct ms5611_chip_info *chip_info;
+	struct regulator *vdd;
 };
 
 int ms5611_probe(struct iio_dev *indio_dev, struct device *dev,
drivers/iio/pressure/ms5611_core.c +32 -9

76 lines not on this page · raw message

Signature · 1 line
2.1.4
maintainer
#2 lore raw

Re: [PATCH v7 1/1] iio:pressure:ms5611: fix missing regulator_disable

Quoted 4 lines · Gregor Boirie

Ensure optional regulator is properly disabled when present.

Fixes 3145229f9191 ("iio:pressure:ms5611: power regulator support")
Signed-off-by Gregor Boirie <[email protected]>

I've applied this to the togreg branch of iio.git. Decided to do this rather than fixes as the worst that happens is a regulator gets left on. As this support is relatively new and before that it always got left on, I'm not considering this to important.

Feel free to argue otherwise!

Jonathan

Quoted 111 lines
---
 drivers/iio/pressure/ms5611.h      |  3 +++
 drivers/iio/pressure/ms5611_core.c | 41 +++++++++++++++++++++++++++++---------
 2 files changed, 35 insertions(+), 9 deletions(-)
drivers/iio/pressure/ms5611.h +3 -0
index d725a307..ccda63c 100644
@@ -16,6 +16,8 @@
 #include <linux/iio/iio.h>
 #include <linux/mutex.h>
 
+struct regulator;
+
 #define MS5611_RESET			0x1e
 #define MS5611_READ_ADC			0x00
 #define MS5611_READ_PROM_WORD		0xA0

@@ -57,6 +59,7 @@ struct ms5611_state {
 					  s32 *temp, s32 *pressure);
 
 	struct ms5611_chip_info *chip_info;
+	struct regulator *vdd;
 };
 
 int ms5611_probe(struct iio_dev *indio_dev, struct device *dev,
drivers/iio/pressure/ms5611_core.c +32 -9
index 37dbc04..c4e6586 100644
@@ -387,24 +387,45 @@ static const struct iio_info ms5611_info = {
 static int ms5611_init(struct iio_dev *indio_dev)
 {
 	int ret;
-	struct regulator *vdd = devm_regulator_get(indio_dev->dev.parent,
-	                                           "vdd");
+	struct ms5611_state *st = iio_priv(indio_dev);
 
 	/* Enable attached regulator if any. */
-	if (!IS_ERR(vdd)) {
-		ret = regulator_enable(vdd);
+	st->vdd = devm_regulator_get(indio_dev->dev.parent, "vdd");
+	if (!IS_ERR(st->vdd)) {
+		ret = regulator_enable(st->vdd);
 		if (ret) {
 			dev_err(indio_dev->dev.parent,
-			        "failed to enable Vdd supply: %d\n", ret);
+				"failed to enable Vdd supply: %d\n", ret);
 			return ret;
 		}
+	} else {
+		ret = PTR_ERR(st->vdd);
+		if (ret != -ENODEV)
+			return ret;
 	}
 
 	ret = ms5611_reset(indio_dev);
 	if (ret < 0)
-		return ret;
+		goto err_regulator_disable;
+
+	ret = ms5611_read_prom(indio_dev);
+	if (ret < 0)
+		goto err_regulator_disable;
+
+	return 0;
 
-	return ms5611_read_prom(indio_dev);
+err_regulator_disable:
+	if (!IS_ERR_OR_NULL(st->vdd))
+		regulator_disable(st->vdd);
+	return ret;
+}
+
+static void ms5611_fini(const struct iio_dev *indio_dev)
+{
+	const struct ms5611_state *st = iio_priv(indio_dev);
+
+	if (!IS_ERR_OR_NULL(st->vdd))
+		regulator_disable(st->vdd);
 }
 
 int ms5611_probe(struct iio_dev *indio_dev, struct device *dev,

@@ -436,7 +457,7 @@ int ms5611_probe(struct iio_dev *indio_dev, struct device *dev,
 					 ms5611_trigger_handler, NULL);
 	if (ret < 0) {
 		dev_err(dev, "iio triggered buffer setup failed\n");
-		return ret;
+		goto err_fini;
 	}
 
 	ret = iio_device_register(indio_dev);

@@ -449,7 +470,8 @@ int ms5611_probe(struct iio_dev *indio_dev, struct device *dev,
 
 err_buffer_cleanup:
 	iio_triggered_buffer_cleanup(indio_dev);
-
+err_fini:
+	ms5611_fini(indio_dev);
 	return ret;
 }
 EXPORT_SYMBOL(ms5611_probe);

@@ -458,6 +480,7 @@ int ms5611_remove(struct iio_dev *indio_dev)
 {
 	iio_device_unregister(indio_dev);
 	iio_triggered_buffer_cleanup(indio_dev);
+	ms5611_fini(indio_dev);
 
 	return 0;
 }