[PATCH v2 0/7] ARM: dts: qcom: msm8974-hammerhead: add more sensors

[PATCH v2 0/7] ARM: dts: qcom: msm8974-hammerhead: add more sensors

[Brian Masney]

This patch set adds support for the gyroscope / accelerometer
(mpu6515), magnetometer (ak8963), temperature / pressure (bmp280), and
proximity / ALS (tsl2772) sensors to the LG Nexus 5 (hammerhead) phone.

Changes since v1:
- Correct mpu6050 patch based on feedback from Jonathan Cameron. See
  that patch for more details.
- Add support for proximity / ALS driver (tsl2772).

Brian Masney (7):
  iio: imu: mpu6050: add support for regulator framework
  ARM: dts: qcom: msm8974-hammerhead: add device tree bindings for
    mpu6515
  iio: tsl2772: add support for reading power settings from device tree
  dt-bindings: trivial: remove tsl2772
  iio: tsl2772: add support for regulator framework
  iio: tsl2772: add device tree binding for avago,apds9930
  ARM: dts: qcom: msm8974-hammerhead: add device tree bindings for ALS /
    proximity

 .../bindings/iio/imu/inv_mpu6050.txt          |   1 +
 .../devicetree/bindings/iio/light/tsl2772.txt |  44 +++++++
 .../devicetree/bindings/trivial-devices.txt   |  10 --
 .../qcom-msm8974-lge-nexus5-hammerhead.dts    |  83 +++++++++++++
 arch/arm/boot/dts/qcom-msm8974.dtsi           |  22 ++++
 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c    |  73 +++++++++++
 drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h     |   3 +
 drivers/iio/light/tsl2772.c                   | 116 +++++++++++++++++-
 include/dt-bindings/iio/amstaos,tsl2772.h     |  24 ++++
 9 files changed, 363 insertions(+), 13 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/light/tsl2772.txt
 create mode 100644 include/dt-bindings/iio/amstaos,tsl2772.h

-- 
2.17.1

[PATCH v2 1/7] iio: imu: mpu6050: add support for regulator framework

[Brian Masney]

This patch adds support for the regulator framework to the mpu6050
driver.

Signed-off-by: Brian Masney <masneyb@onstation.org>
Signed-off-by: Jonathan Marek <jonathan@marek.ca>
---
Changes since v1:
- Use devm_regulator_get() instead of devm_regulator_get_optional()
- Use devm_add_action() for cleaning up the regulator.
- Correct ordering of resume code.
- Add regulator_enabled flag to ensure regulator is not disabled twice,
  specifically the case where the device is suspended and then the
  driver is removed.

Original extra changelog from v1:

This is a variation of Jonathan Marek's patch from postmarketOS
https://gitlab.com/postmarketOS/linux-postmarketos/commit/b8ad1ec1859c8bbcbce94944b3f4dd68f8f9fc37
with the following changes:

- Stripped out 6515 variant code.
- Add the regulator to the mpu core instead of only the i2c variant.
- Add error handling.
- Release the regulator on suspend, device remove, etc.
- Device tree documentation.

 .../bindings/iio/imu/inv_mpu6050.txt          |  1 +
 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c    | 73 +++++++++++++++++++
 drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h     |  3 +
 3 files changed, 77 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt b/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
index b7def51c8ad9..d39907b12a46 100644
--- a/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
+++ b/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
@@ -21,6 +21,7 @@ Required properties:
   bindings.
 
 Optional properties:
+ - vddio-supply: regulator phandle for VDDIO supply
  - mount-matrix: an optional 3x3 mounting rotation matrix
  - i2c-gate node.  These devices also support an auxiliary i2c bus.  This is
    simple enough to be described using the i2c-gate binding. See
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index 12c1b9507007..e1fbab4bc0a0 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -23,6 +23,7 @@
 #include <linux/iio/iio.h>
 #include <linux/acpi.h>
 #include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
 #include "inv_mpu_iio.h"
 
 /*
@@ -926,6 +927,46 @@ static int inv_check_and_setup_chip(struct inv_mpu6050_state *st)
 	return result;
 }
 
+static int inv_mpu_core_enable_regulator(struct inv_mpu6050_state *st)
+{
+	int result;
+
+	result = regulator_enable(st->vddio_supply);
+	if (result) {
+		dev_err(regmap_get_device(st->map),
+			"Failed to enable regulator: %d\n", result);
+	} else {
+		st->regulator_enabled = true;
+
+		/* Give the device a little bit of time to start up. */
+		usleep_range(35000, 70000);
+	}
+
+	return result;
+}
+
+static int inv_mpu_core_disable_regulator(struct inv_mpu6050_state *st)
+{
+	int result;
+
+	if (!st->regulator_enabled)
+		return 0;
+
+	result = regulator_disable(st->vddio_supply);
+	if (result)
+		dev_err(regmap_get_device(st->map),
+			"Failed to disable regulator: %d\n", result);
+
+	st->regulator_enabled = false;
+
+	return result;
+}
+
+static void inv_mpu_core_disable_regulator_action(void *_data)
+{
+	inv_mpu_core_disable_regulator(_data);
+}
+
 int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
 		int (*inv_mpu_bus_setup)(struct iio_dev *), int chip_type)
 {
@@ -990,6 +1031,28 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
 		return -EINVAL;
 	}
 
+	st->vddio_supply = devm_regulator_get(dev, "vddio");
+	if (IS_ERR(st->vddio_supply)) {
+		if (PTR_ERR(st->vddio_supply) != -EPROBE_DEFER)
+			dev_err(dev, "Failed to get vddio regulator %d\n",
+				(int)PTR_ERR(st->vddio_supply));
+
+		return PTR_ERR(st->vddio_supply);
+	}
+
+	result = inv_mpu_core_enable_regulator(st);
+	if (result)
+		return result;
+
+	result = devm_add_action(dev, inv_mpu_core_disable_regulator_action,
+				 st);
+	if (result) {
+		inv_mpu_core_disable_regulator_action(st);
+		dev_err(dev, "Failed to setup regulator cleanup action %d\n",
+			result);
+		return result;
+	}
+
 	/* power is turned on inside check chip type*/
 	result = inv_check_and_setup_chip(st);
 	if (result)
@@ -1049,7 +1112,12 @@ static int inv_mpu_resume(struct device *dev)
 	int result;
 
 	mutex_lock(&st->lock);
+	result = inv_mpu_core_enable_regulator(st);
+	if (result)
+		goto out_unlock;
+
 	result = inv_mpu6050_set_power_itg(st, true);
+out_unlock:
 	mutex_unlock(&st->lock);
 
 	return result;
@@ -1062,6 +1130,11 @@ static int inv_mpu_suspend(struct device *dev)
 
 	mutex_lock(&st->lock);
 	result = inv_mpu6050_set_power_itg(st, false);
+	if (result)
+		goto out_unlock;
+
+	result = inv_mpu_core_disable_regulator(st);
+out_unlock:
 	mutex_unlock(&st->lock);
 
 	return result;
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
index e69a59659dbc..01c8fe9b4fa0 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
@@ -129,6 +129,7 @@ struct inv_mpu6050_hw {
  *  @chip_period:	chip internal period estimation (~1kHz).
  *  @it_timestamp:	timestamp from previous interrupt.
  *  @data_timestamp:	timestamp for next data sample.
+ *  @vddio_supply	voltage regulator for the chip.
  */
 struct inv_mpu6050_state {
 	struct mutex lock;
@@ -149,6 +150,8 @@ struct inv_mpu6050_state {
 	s64 chip_period;
 	s64 it_timestamp;
 	s64 data_timestamp;
+	struct regulator *vddio_supply;
+	bool regulator_enabled;
 };
 
 /*register and associated bit definition*/
-- 
2.17.1

Re: [PATCH v2 1/7] iio: imu: mpu6050: add support for regulator framework

[Rob Herring]

On Tue, Jul 17, 2018 at 04:41:52AM -0400, Brian Masney wrote:
> This patch adds support for the regulator framework to the mpu6050
> driver.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
> Signed-off-by: Jonathan Marek <jonathan@marek.ca>
> ---
> Changes since v1:
> - Use devm_regulator_get() instead of devm_regulator_get_optional()
> - Use devm_add_action() for cleaning up the regulator.
> - Correct ordering of resume code.
> - Add regulator_enabled flag to ensure regulator is not disabled twice,
>   specifically the case where the device is suspended and then the
>   driver is removed.
> 
> Original extra changelog from v1:
> 
> This is a variation of Jonathan Marek's patch from postmarketOS
> https://gitlab.com/postmarketOS/linux-postmarketos/commit/b8ad1ec1859c8bbcbce94944b3f4dd68f8f9fc37
> with the following changes:
> 
> - Stripped out 6515 variant code.
> - Add the regulator to the mpu core instead of only the i2c variant.
> - Add error handling.
> - Release the regulator on suspend, device remove, etc.
> - Device tree documentation.
> 
>  .../bindings/iio/imu/inv_mpu6050.txt          |  1 +

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

>  drivers/iio/imu/inv_mpu6050/inv_mpu_core.c    | 73 +++++++++++++++++++
>  drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h     |  3 +
>  3 files changed, 77 insertions(+)

Re: [PATCH v2 1/7] iio: imu: mpu6050: add support for regulator framework

[Jonathan Cameron]

On Tue, 17 Jul 2018 04:41:52 -0400
Brian Masney <masneyb@onstation.org> wrote:

> This patch adds support for the regulator framework to the mpu6050
> driver.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
> Signed-off-by: Jonathan Marek <jonathan@marek.ca>

I'm not 100% sure what the purpose of the regulator_enabled
tracking is.  Perhaps you could point out the path where that is
needed?

Also, a missing bit of documentation needs adding.

Jonathan

> ---
> Changes since v1:
> - Use devm_regulator_get() instead of devm_regulator_get_optional()
> - Use devm_add_action() for cleaning up the regulator.
> - Correct ordering of resume code.
> - Add regulator_enabled flag to ensure regulator is not disabled twice,
>   specifically the case where the device is suspended and then the
>   driver is removed.
> 
> Original extra changelog from v1:
> 
> This is a variation of Jonathan Marek's patch from postmarketOS
> https://gitlab.com/postmarketOS/linux-postmarketos/commit/b8ad1ec1859c8bbcbce94944b3f4dd68f8f9fc37
> with the following changes:
> 
> - Stripped out 6515 variant code.
> - Add the regulator to the mpu core instead of only the i2c variant.
> - Add error handling.
> - Release the regulator on suspend, device remove, etc.
> - Device tree documentation.
> 
>  .../bindings/iio/imu/inv_mpu6050.txt          |  1 +
>  drivers/iio/imu/inv_mpu6050/inv_mpu_core.c    | 73 +++++++++++++++++++
>  drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h     |  3 +
>  3 files changed, 77 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt b/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
> index b7def51c8ad9..d39907b12a46 100644
> --- a/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
> +++ b/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
> @@ -21,6 +21,7 @@ Required properties:
>    bindings.
>  
>  Optional properties:
> + - vddio-supply: regulator phandle for VDDIO supply
>   - mount-matrix: an optional 3x3 mounting rotation matrix
>   - i2c-gate node.  These devices also support an auxiliary i2c bus.  This is
>     simple enough to be described using the i2c-gate binding. See
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index 12c1b9507007..e1fbab4bc0a0 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -23,6 +23,7 @@
>  #include <linux/iio/iio.h>
>  #include <linux/acpi.h>
>  #include <linux/platform_device.h>
> +#include <linux/regulator/consumer.h>
>  #include "inv_mpu_iio.h"
>  
>  /*
> @@ -926,6 +927,46 @@ static int inv_check_and_setup_chip(struct inv_mpu6050_state *st)
>  	return result;
>  }
>  
> +static int inv_mpu_core_enable_regulator(struct inv_mpu6050_state *st)
> +{
> +	int result;
> +
> +	result = regulator_enable(st->vddio_supply);
> +	if (result) {
> +		dev_err(regmap_get_device(st->map),
> +			"Failed to enable regulator: %d\n", result);
> +	} else {
> +		st->regulator_enabled = true;
> +
> +		/* Give the device a little bit of time to start up. */
> +		usleep_range(35000, 70000);
> +	}
> +
> +	return result;
> +}
> +
> +static int inv_mpu_core_disable_regulator(struct inv_mpu6050_state *st)
> +{
> +	int result;
> +
> +	if (!st->regulator_enabled)
> +		return 0;
> +
> +	result = regulator_disable(st->vddio_supply);
> +	if (result)
> +		dev_err(regmap_get_device(st->map),
> +			"Failed to disable regulator: %d\n", result);
> +
> +	st->regulator_enabled = false;
> +
> +	return result;
> +}
> +
> +static void inv_mpu_core_disable_regulator_action(void *_data)
> +{
> +	inv_mpu_core_disable_regulator(_data);
> +}
> +
>  int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
>  		int (*inv_mpu_bus_setup)(struct iio_dev *), int chip_type)
>  {
> @@ -990,6 +1031,28 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
>  		return -EINVAL;
>  	}
>  
> +	st->vddio_supply = devm_regulator_get(dev, "vddio");
> +	if (IS_ERR(st->vddio_supply)) {
> +		if (PTR_ERR(st->vddio_supply) != -EPROBE_DEFER)
> +			dev_err(dev, "Failed to get vddio regulator %d\n",
> +				(int)PTR_ERR(st->vddio_supply));
> +
> +		return PTR_ERR(st->vddio_supply);
> +	}
> +
> +	result = inv_mpu_core_enable_regulator(st);
> +	if (result)
> +		return result;
> +
> +	result = devm_add_action(dev, inv_mpu_core_disable_regulator_action,
> +				 st);
> +	if (result) {
> +		inv_mpu_core_disable_regulator_action(st);
> +		dev_err(dev, "Failed to setup regulator cleanup action %d\n",
> +			result);
> +		return result;
> +	}
> +
>  	/* power is turned on inside check chip type*/
>  	result = inv_check_and_setup_chip(st);
>  	if (result)
> @@ -1049,7 +1112,12 @@ static int inv_mpu_resume(struct device *dev)
>  	int result;
>  
>  	mutex_lock(&st->lock);
> +	result = inv_mpu_core_enable_regulator(st);
> +	if (result)
> +		goto out_unlock;
> +
>  	result = inv_mpu6050_set_power_itg(st, true);
> +out_unlock:
>  	mutex_unlock(&st->lock);
>  
>  	return result;
> @@ -1062,6 +1130,11 @@ static int inv_mpu_suspend(struct device *dev)
>  
>  	mutex_lock(&st->lock);
>  	result = inv_mpu6050_set_power_itg(st, false);
> +	if (result)
> +		goto out_unlock;
> +
> +	result = inv_mpu_core_disable_regulator(st);
> +out_unlock:
>  	mutex_unlock(&st->lock);
>  
>  	return result;
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> index e69a59659dbc..01c8fe9b4fa0 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> @@ -129,6 +129,7 @@ struct inv_mpu6050_hw {
>   *  @chip_period:	chip internal period estimation (~1kHz).
>   *  @it_timestamp:	timestamp from previous interrupt.
>   *  @data_timestamp:	timestamp for next data sample.
> + *  @vddio_supply	voltage regulator for the chip.
Docs missing for regulator_enabled.
>   */
>  struct inv_mpu6050_state {
>  	struct mutex lock;
> @@ -149,6 +150,8 @@ struct inv_mpu6050_state {
>  	s64 chip_period;
>  	s64 it_timestamp;
>  	s64 data_timestamp;
> +	struct regulator *vddio_supply;
> +	bool regulator_enabled;
>  };
>  
>  /*register and associated bit definition*/

Re: [PATCH v2 1/7] iio: imu: mpu6050: add support for regulator framework

[Brian Masney]

On Sat, Jul 21, 2018 at 06:31:07PM +0100, Jonathan Cameron wrote:
> On Tue, 17 Jul 2018 04:41:52 -0400
> Brian Masney <masneyb@onstation.org> wrote:
> 
> > This patch adds support for the regulator framework to the mpu6050
> > driver.
> > 
> > Signed-off-by: Brian Masney <masneyb@onstation.org>
> > Signed-off-by: Jonathan Marek <jonathan@marek.ca>
> 
> I'm not 100% sure what the purpose of the regulator_enabled
> tracking is.  Perhaps you could point out the path where that is
> needed?

I was thinking about the use case where the device is compiled into the
kernel as a module, the device is suspended, and then someone rmmods
the module. If that is not a use case that I need to worry about, then
the flag can go away and it will simplify the code.

Brian



> Also, a missing bit of documentation needs adding.
> 
> Jonathan
> 
> > ---
> > Changes since v1:
> > - Use devm_regulator_get() instead of devm_regulator_get_optional()
> > - Use devm_add_action() for cleaning up the regulator.
> > - Correct ordering of resume code.
> > - Add regulator_enabled flag to ensure regulator is not disabled twice,
> >   specifically the case where the device is suspended and then the
> >   driver is removed.
> > 
> > Original extra changelog from v1:
> > 
> > This is a variation of Jonathan Marek's patch from postmarketOS
> > https://gitlab.com/postmarketOS/linux-postmarketos/commit/b8ad1ec1859c8bbcbce94944b3f4dd68f8f9fc37
> > with the following changes:
> > 
> > - Stripped out 6515 variant code.
> > - Add the regulator to the mpu core instead of only the i2c variant.
> > - Add error handling.
> > - Release the regulator on suspend, device remove, etc.
> > - Device tree documentation.
> > 
> >  .../bindings/iio/imu/inv_mpu6050.txt          |  1 +
> >  drivers/iio/imu/inv_mpu6050/inv_mpu_core.c    | 73 +++++++++++++++++++
> >  drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h     |  3 +
> >  3 files changed, 77 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt b/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
> > index b7def51c8ad9..d39907b12a46 100644
> > --- a/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
> > +++ b/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
> > @@ -21,6 +21,7 @@ Required properties:
> >    bindings.
> >  
> >  Optional properties:
> > + - vddio-supply: regulator phandle for VDDIO supply
> >   - mount-matrix: an optional 3x3 mounting rotation matrix
> >   - i2c-gate node.  These devices also support an auxiliary i2c bus.  This is
> >     simple enough to be described using the i2c-gate binding. See
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> > index 12c1b9507007..e1fbab4bc0a0 100644
> > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> > @@ -23,6 +23,7 @@
> >  #include <linux/iio/iio.h>
> >  #include <linux/acpi.h>
> >  #include <linux/platform_device.h>
> > +#include <linux/regulator/consumer.h>
> >  #include "inv_mpu_iio.h"
> >  
> >  /*
> > @@ -926,6 +927,46 @@ static int inv_check_and_setup_chip(struct inv_mpu6050_state *st)
> >  	return result;
> >  }
> >  
> > +static int inv_mpu_core_enable_regulator(struct inv_mpu6050_state *st)
> > +{
> > +	int result;
> > +
> > +	result = regulator_enable(st->vddio_supply);
> > +	if (result) {
> > +		dev_err(regmap_get_device(st->map),
> > +			"Failed to enable regulator: %d\n", result);
> > +	} else {
> > +		st->regulator_enabled = true;
> > +
> > +		/* Give the device a little bit of time to start up. */
> > +		usleep_range(35000, 70000);
> > +	}
> > +
> > +	return result;
> > +}
> > +
> > +static int inv_mpu_core_disable_regulator(struct inv_mpu6050_state *st)
> > +{
> > +	int result;
> > +
> > +	if (!st->regulator_enabled)
> > +		return 0;
> > +
> > +	result = regulator_disable(st->vddio_supply);
> > +	if (result)
> > +		dev_err(regmap_get_device(st->map),
> > +			"Failed to disable regulator: %d\n", result);
> > +
> > +	st->regulator_enabled = false;
> > +
> > +	return result;
> > +}
> > +
> > +static void inv_mpu_core_disable_regulator_action(void *_data)
> > +{
> > +	inv_mpu_core_disable_regulator(_data);
> > +}
> > +
> >  int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
> >  		int (*inv_mpu_bus_setup)(struct iio_dev *), int chip_type)
> >  {
> > @@ -990,6 +1031,28 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
> >  		return -EINVAL;
> >  	}
> >  
> > +	st->vddio_supply = devm_regulator_get(dev, "vddio");
> > +	if (IS_ERR(st->vddio_supply)) {
> > +		if (PTR_ERR(st->vddio_supply) != -EPROBE_DEFER)
> > +			dev_err(dev, "Failed to get vddio regulator %d\n",
> > +				(int)PTR_ERR(st->vddio_supply));
> > +
> > +		return PTR_ERR(st->vddio_supply);
> > +	}
> > +
> > +	result = inv_mpu_core_enable_regulator(st);
> > +	if (result)
> > +		return result;
> > +
> > +	result = devm_add_action(dev, inv_mpu_core_disable_regulator_action,
> > +				 st);
> > +	if (result) {
> > +		inv_mpu_core_disable_regulator_action(st);
> > +		dev_err(dev, "Failed to setup regulator cleanup action %d\n",
> > +			result);
> > +		return result;
> > +	}
> > +
> >  	/* power is turned on inside check chip type*/
> >  	result = inv_check_and_setup_chip(st);
> >  	if (result)
> > @@ -1049,7 +1112,12 @@ static int inv_mpu_resume(struct device *dev)
> >  	int result;
> >  
> >  	mutex_lock(&st->lock);
> > +	result = inv_mpu_core_enable_regulator(st);
> > +	if (result)
> > +		goto out_unlock;
> > +
> >  	result = inv_mpu6050_set_power_itg(st, true);
> > +out_unlock:
> >  	mutex_unlock(&st->lock);
> >  
> >  	return result;
> > @@ -1062,6 +1130,11 @@ static int inv_mpu_suspend(struct device *dev)
> >  
> >  	mutex_lock(&st->lock);
> >  	result = inv_mpu6050_set_power_itg(st, false);
> > +	if (result)
> > +		goto out_unlock;
> > +
> > +	result = inv_mpu_core_disable_regulator(st);
> > +out_unlock:
> >  	mutex_unlock(&st->lock);
> >  
> >  	return result;
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> > index e69a59659dbc..01c8fe9b4fa0 100644
> > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> > @@ -129,6 +129,7 @@ struct inv_mpu6050_hw {
> >   *  @chip_period:	chip internal period estimation (~1kHz).
> >   *  @it_timestamp:	timestamp from previous interrupt.
> >   *  @data_timestamp:	timestamp for next data sample.
> > + *  @vddio_supply	voltage regulator for the chip.
> Docs missing for regulator_enabled.
> >   */
> >  struct inv_mpu6050_state {
> >  	struct mutex lock;
> > @@ -149,6 +150,8 @@ struct inv_mpu6050_state {
> >  	s64 chip_period;
> >  	s64 it_timestamp;
> >  	s64 data_timestamp;
> > +	struct regulator *vddio_supply;
> > +	bool regulator_enabled;
> >  };
> >  
> >  /*register and associated bit definition*/

Re: [PATCH v2 1/7] iio: imu: mpu6050: add support for regulator framework

[Jonathan Cameron]

On Sun, 22 Jul 2018 12:31:45 +0000
Brian Masney <masneyb@onstation.org> wrote:

> On Sat, Jul 21, 2018 at 06:31:07PM +0100, Jonathan Cameron wrote:
> > On Tue, 17 Jul 2018 04:41:52 -0400
> > Brian Masney <masneyb@onstation.org> wrote:
> >   
> > > This patch adds support for the regulator framework to the mpu6050
> > > driver.
> > > 
> > > Signed-off-by: Brian Masney <masneyb@onstation.org>
> > > Signed-off-by: Jonathan Marek <jonathan@marek.ca>  
> > 
> > I'm not 100% sure what the purpose of the regulator_enabled
> > tracking is.  Perhaps you could point out the path where that is
> > needed?  
> 
> I was thinking about the use case where the device is compiled into the
> kernel as a module, the device is suspended, and then someone rmmods
> the module. If that is not a use case that I need to worry about, then
> the flag can go away and it will simplify the code.
I'm not sure it's a situation that can actually happen.

Before a rmmod is acted upon I think it will always be un-suspended.
This is necessary to avoid all sorts of nasty corner cases.

It's been debate in the past on whether it is necessary to come
out of runtime suspend under these sorts of circumstances but
I don't think anyone has ever tried to make suspend and remove
play nicely.

Jonathan
> 
> Brian
> 
> 
> 
> > Also, a missing bit of documentation needs adding.
> > 
> > Jonathan
> >   
> > > ---
> > > Changes since v1:
> > > - Use devm_regulator_get() instead of devm_regulator_get_optional()
> > > - Use devm_add_action() for cleaning up the regulator.
> > > - Correct ordering of resume code.
> > > - Add regulator_enabled flag to ensure regulator is not disabled twice,
> > >   specifically the case where the device is suspended and then the
> > >   driver is removed.
> > > 
> > > Original extra changelog from v1:
> > > 
> > > This is a variation of Jonathan Marek's patch from postmarketOS
> > > https://gitlab.com/postmarketOS/linux-postmarketos/commit/b8ad1ec1859c8bbcbce94944b3f4dd68f8f9fc37
> > > with the following changes:
> > > 
> > > - Stripped out 6515 variant code.
> > > - Add the regulator to the mpu core instead of only the i2c variant.
> > > - Add error handling.
> > > - Release the regulator on suspend, device remove, etc.
> > > - Device tree documentation.
> > > 
> > >  .../bindings/iio/imu/inv_mpu6050.txt          |  1 +
> > >  drivers/iio/imu/inv_mpu6050/inv_mpu_core.c    | 73 +++++++++++++++++++
> > >  drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h     |  3 +
> > >  3 files changed, 77 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt b/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
> > > index b7def51c8ad9..d39907b12a46 100644
> > > --- a/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
> > > +++ b/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
> > > @@ -21,6 +21,7 @@ Required properties:
> > >    bindings.
> > >  
> > >  Optional properties:
> > > + - vddio-supply: regulator phandle for VDDIO supply
> > >   - mount-matrix: an optional 3x3 mounting rotation matrix
> > >   - i2c-gate node.  These devices also support an auxiliary i2c bus.  This is
> > >     simple enough to be described using the i2c-gate binding. See
> > > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> > > index 12c1b9507007..e1fbab4bc0a0 100644
> > > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> > > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> > > @@ -23,6 +23,7 @@
> > >  #include <linux/iio/iio.h>
> > >  #include <linux/acpi.h>
> > >  #include <linux/platform_device.h>
> > > +#include <linux/regulator/consumer.h>
> > >  #include "inv_mpu_iio.h"
> > >  
> > >  /*
> > > @@ -926,6 +927,46 @@ static int inv_check_and_setup_chip(struct inv_mpu6050_state *st)
> > >  	return result;
> > >  }
> > >  
> > > +static int inv_mpu_core_enable_regulator(struct inv_mpu6050_state *st)
> > > +{
> > > +	int result;
> > > +
> > > +	result = regulator_enable(st->vddio_supply);
> > > +	if (result) {
> > > +		dev_err(regmap_get_device(st->map),
> > > +			"Failed to enable regulator: %d\n", result);
> > > +	} else {
> > > +		st->regulator_enabled = true;
> > > +
> > > +		/* Give the device a little bit of time to start up. */
> > > +		usleep_range(35000, 70000);
> > > +	}
> > > +
> > > +	return result;
> > > +}
> > > +
> > > +static int inv_mpu_core_disable_regulator(struct inv_mpu6050_state *st)
> > > +{
> > > +	int result;
> > > +
> > > +	if (!st->regulator_enabled)
> > > +		return 0;
> > > +
> > > +	result = regulator_disable(st->vddio_supply);
> > > +	if (result)
> > > +		dev_err(regmap_get_device(st->map),
> > > +			"Failed to disable regulator: %d\n", result);
> > > +
> > > +	st->regulator_enabled = false;
> > > +
> > > +	return result;
> > > +}
> > > +
> > > +static void inv_mpu_core_disable_regulator_action(void *_data)
> > > +{
> > > +	inv_mpu_core_disable_regulator(_data);
> > > +}
> > > +
> > >  int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
> > >  		int (*inv_mpu_bus_setup)(struct iio_dev *), int chip_type)
> > >  {
> > > @@ -990,6 +1031,28 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
> > >  		return -EINVAL;
> > >  	}
> > >  
> > > +	st->vddio_supply = devm_regulator_get(dev, "vddio");
> > > +	if (IS_ERR(st->vddio_supply)) {
> > > +		if (PTR_ERR(st->vddio_supply) != -EPROBE_DEFER)
> > > +			dev_err(dev, "Failed to get vddio regulator %d\n",
> > > +				(int)PTR_ERR(st->vddio_supply));
> > > +
> > > +		return PTR_ERR(st->vddio_supply);
> > > +	}
> > > +
> > > +	result = inv_mpu_core_enable_regulator(st);
> > > +	if (result)
> > > +		return result;
> > > +
> > > +	result = devm_add_action(dev, inv_mpu_core_disable_regulator_action,
> > > +				 st);
> > > +	if (result) {
> > > +		inv_mpu_core_disable_regulator_action(st);
> > > +		dev_err(dev, "Failed to setup regulator cleanup action %d\n",
> > > +			result);
> > > +		return result;
> > > +	}
> > > +
> > >  	/* power is turned on inside check chip type*/
> > >  	result = inv_check_and_setup_chip(st);
> > >  	if (result)
> > > @@ -1049,7 +1112,12 @@ static int inv_mpu_resume(struct device *dev)
> > >  	int result;
> > >  
> > >  	mutex_lock(&st->lock);
> > > +	result = inv_mpu_core_enable_regulator(st);
> > > +	if (result)
> > > +		goto out_unlock;
> > > +
> > >  	result = inv_mpu6050_set_power_itg(st, true);
> > > +out_unlock:
> > >  	mutex_unlock(&st->lock);
> > >  
> > >  	return result;
> > > @@ -1062,6 +1130,11 @@ static int inv_mpu_suspend(struct device *dev)
> > >  
> > >  	mutex_lock(&st->lock);
> > >  	result = inv_mpu6050_set_power_itg(st, false);
> > > +	if (result)
> > > +		goto out_unlock;
> > > +
> > > +	result = inv_mpu_core_disable_regulator(st);
> > > +out_unlock:
> > >  	mutex_unlock(&st->lock);
> > >  
> > >  	return result;
> > > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> > > index e69a59659dbc..01c8fe9b4fa0 100644
> > > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> > > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> > > @@ -129,6 +129,7 @@ struct inv_mpu6050_hw {
> > >   *  @chip_period:	chip internal period estimation (~1kHz).
> > >   *  @it_timestamp:	timestamp from previous interrupt.
> > >   *  @data_timestamp:	timestamp for next data sample.
> > > + *  @vddio_supply	voltage regulator for the chip.  
> > Docs missing for regulator_enabled.  
> > >   */
> > >  struct inv_mpu6050_state {
> > >  	struct mutex lock;
> > > @@ -149,6 +150,8 @@ struct inv_mpu6050_state {
> > >  	s64 chip_period;
> > >  	s64 it_timestamp;
> > >  	s64 data_timestamp;
> > > +	struct regulator *vddio_supply;
> > > +	bool regulator_enabled;
> > >  };
> > >  
> > >  /*register and associated bit definition*/  

[PATCH v2 2/7] ARM: dts: qcom: msm8974-hammerhead: add device tree bindings for mpu6515

[Brian Masney]

This patch adds device tree bindings for the mpu6515 to the LG Nexus 5
(hammerhead) phone. Confirmed that the gyroscope / accelerometer
(mpu6515), magnetometer (ak8963), and temperature / pressure (bmp280)
sensors are available on the phone.

Interrupts are not working properly on the ak8963 magnetometer so they
are currently not configured.

The bmp280 retuns temperature/pressure measurement skipped errors but
will reliably work if I run:

    echo 1 > in_pressure_oversampling_ratio
    echo 1 > in_temp_oversampling_ratio

Signed-off-by: Brian Masney <masneyb@onstation.org>
Signed-off-by: Jonathan Marek <jonathan@marek.ca>
---
I'll send follow up patch(es) once I investigate why the skipped errors
are occurring with the bmp280 with the default oversampling ratios.

 .../qcom-msm8974-lge-nexus5-hammerhead.dts    | 56 +++++++++++++++++++
 arch/arm/boot/dts/qcom-msm8974.dtsi           | 11 ++++
 2 files changed, 67 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts b/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
index c2dc9d09484a..928affae1885 100644
--- a/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
+++ b/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
@@ -241,6 +241,24 @@
 				bias-pull-up;
 			};
 		};
+
+		i2c12_pins: i2c12 {
+			mux {
+				pins = "gpio87", "gpio88";
+				function = "blsp_i2c12";
+				drive-strength = <2>;
+				bias-disable;
+			};
+		};
+
+		mpu6515_pin: mpu6515 {
+			irq {
+				pins = "gpio73";
+				function = "gpio";
+				bias-disable;
+				input-enable;
+			};
+		};
 	};
 
 	sdhci@f9824900 {
@@ -277,6 +295,44 @@
 			linux,code = <KEY_VOLUMEDOWN>;
 		};
 	};
+
+	i2c@f9968000 {
+		status = "ok";
+		pinctrl-names = "default";
+		pinctrl-0 = <&i2c12_pins>;
+		clock-frequency = <100000>;
+		qcom,src-freq = <50000000>;
+
+		mpu6515@68 {
+			compatible = "invensense,mpu6515";
+			reg = <0x68>;
+			interrupts-extended = <&msmgpio 73 IRQ_TYPE_EDGE_FALLING>;
+			vddio-supply = <&pm8941_lvs1>;
+
+			pinctrl-names = "default";
+			pinctrl-0 = <&mpu6515_pin>;
+
+			i2c-gate {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				ak8963@f {
+					compatible = "asahi-kasei,ak8963";
+					reg = <0x0f>;
+					// Currently only works in polling mode.
+					// gpios = <&msmgpio 61 0>;
+					vid-supply = <&pm8941_lvs1>;
+					vdd-supply = <&pm8941_l17>;
+				};
+
+				bmp280@76 {
+					compatible = "bosch,bmp280";
+					reg = <0x76>;
+					vdda-supply = <&pm8941_lvs1>;
+					vddd-supply = <&pm8941_l17>;
+				};
+			};
+		};
+	};
 };
 
 &spmi_bus {
diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
index d9019a49b292..cebb6ae9143a 100644
--- a/arch/arm/boot/dts/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
@@ -737,6 +737,17 @@
 			dma-names = "tx", "rx";
 		};
 
+		blsp_i2c12: i2c@f9968000 {
+			status = "disabled";
+			compatible = "qcom,i2c-qup-v2.1.1";
+			reg = <0xf9968000 0x1000>;
+			interrupts = <0 106 IRQ_TYPE_NONE>;
+			clocks = <&gcc GCC_BLSP2_QUP6_I2C_APPS_CLK>, <&gcc GCC_BLSP2_AHB_CLK>;
+			clock-names = "core", "iface";
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
 		spmi_bus: spmi@fc4cf000 {
 			compatible = "qcom,spmi-pmic-arb";
 			reg-names = "core", "intr", "cnfg";
-- 
2.17.1

[PATCH v2 3/7] iio: tsl2772: add support for reading power settings from device tree

[Brian Masney]

This patch adds support for optionally reading the prox_diode and
prox_power settings from device tree. This was tested using a LG
Nexus 5 (hammerhead) which requires a different diode than the driver
default for the IR LED.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
The next patch in the series removes the tsl2772 driver from the
trivial-devices.txt file. I separated it out so that change can go
through device tree.

 .../devicetree/bindings/iio/light/tsl2772.txt | 39 +++++++++++++++++++
 drivers/iio/light/tsl2772.c                   | 16 ++++++++
 include/dt-bindings/iio/amstaos,tsl2772.h     | 24 ++++++++++++
 3 files changed, 79 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/light/tsl2772.txt
 create mode 100644 include/dt-bindings/iio/amstaos,tsl2772.h

diff --git a/Documentation/devicetree/bindings/iio/light/tsl2772.txt b/Documentation/devicetree/bindings/iio/light/tsl2772.txt
new file mode 100644
index 000000000000..ab553d52b9fc
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/light/tsl2772.txt
@@ -0,0 +1,39 @@
+* AMS/TAOS ALS and proximity sensor
+
+Required properties:
+
+  - compatible: Should be one of
+		"amstaos,tsl2571"
+		"amstaos,tsl2671"
+		"amstaos,tmd2671"
+		"amstaos,tsl2771"
+		"amstaos,tmd2771"
+		"amstaos,tsl2572"
+		"amstaos,tsl2672"
+		"amstaos,tmd2672"
+		"amstaos,tsl2772"
+		"amstaos,tmd2772"
+  - reg: the I2C address of the device
+
+Optional properties:
+
+  - amstaos,prox_diode - must be TSL2772_DIODE0, TSL2772_DIODE1, or
+                         TSL2772_DIODE_BOTH.
+  - amstaos,prox_power - must be TSL2772_100_mA, TSL2772_50_mA, TSL2772_25_mA,
+                         or TSL2772_13_mA.
+  - interrupt-parent: should be the phandle for the interrupt controller
+  - interrupts: the sole interrupt generated by the device
+
+  Refer to interrupt-controller/interrupts.txt for generic interrupt client
+  node bindings.
+
+Example:
+
+#include <dt-bindings/iio/amstaos,tsl2772.h>
+
+tsl2772@39 {
+	compatible = "amstaos,tsl2772";
+	reg = <0x39>;
+	interrupts-extended = <&msmgpio 61 IRQ_TYPE_EDGE_FALLING>;
+	amstaos,prox_diode = <TSL2772_DIODE0>;
+};
diff --git a/drivers/iio/light/tsl2772.c b/drivers/iio/light/tsl2772.c
index 34d42a2504c9..0be57f2ecb02 100644
--- a/drivers/iio/light/tsl2772.c
+++ b/drivers/iio/light/tsl2772.c
@@ -515,6 +515,18 @@ static int tsl2772_get_prox(struct iio_dev *indio_dev)
 	return ret;
 }
 
+#ifdef CONFIG_OF
+static void tsl2772_parse_dt(struct tsl2772_chip *chip)
+{
+	struct device_node *of_node = chip->client->dev.of_node;
+
+	of_property_read_u32(of_node, "amstaos,prox_diode",
+			     &chip->settings.prox_diode);
+	of_property_read_u32(of_node, "amstaos,prox_power",
+			     &chip->settings.prox_power);
+}
+#endif
+
 /**
  * tsl2772_defaults() - Populates the device nominal operating parameters
  *                      with those provided by a 'platform' data struct or
@@ -541,6 +553,10 @@ static void tsl2772_defaults(struct tsl2772_chip *chip)
 		memcpy(chip->tsl2772_device_lux,
 		       tsl2772_default_lux_table_group[chip->id],
 		       TSL2772_DEFAULT_TABLE_BYTES);
+
+#ifdef CONFIG_OF
+	tsl2772_parse_dt(chip);
+#endif
 }
 
 /**
diff --git a/include/dt-bindings/iio/amstaos,tsl2772.h b/include/dt-bindings/iio/amstaos,tsl2772.h
new file mode 100644
index 000000000000..ad6f9fbc0845
--- /dev/null
+++ b/include/dt-bindings/iio/amstaos,tsl2772.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Device driver for monitoring ambient light intensity (lux)
+ * and proximity (prox) within the TAOS TSL2772 family of devices.
+ *
+ * Copyright (c) 2012 TAOS Corporation.
+ * Copyright (c) 2017-2018 Brian Masney <masneyb@onstation.org>
+ */
+
+#ifndef _DT_BINDINGS_AMSTAOS_TSL2772_H
+#define _DT_BINDINGS_AMSTAOS_TSL2772_H
+
+/* Proximity diode to use */
+#define TSL2772_DIODE0                  0x01
+#define TSL2772_DIODE1                  0x02
+#define TSL2772_DIODE_BOTH              0x03
+
+/* LED Power */
+#define TSL2772_100_mA                  0x00
+#define TSL2772_50_mA                   0x01
+#define TSL2772_25_mA                   0x02
+#define TSL2772_13_mA                   0x03
+
+#endif /* _DT_BINDINGS_AMSTAOS_TSL2772_H */
-- 
2.17.1

Re: [PATCH v2 3/7] iio: tsl2772: add support for reading power settings from device tree

[Rob Herring]

On Tue, Jul 17, 2018 at 04:41:54AM -0400, Brian Masney wrote:
> This patch adds support for optionally reading the prox_diode and
> prox_power settings from device tree. This was tested using a LG
> Nexus 5 (hammerhead) which requires a different diode than the driver
> default for the IR LED.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
> ---
> The next patch in the series removes the tsl2772 driver from the
> trivial-devices.txt file. I separated it out so that change can go
> through device tree.
> 
>  .../devicetree/bindings/iio/light/tsl2772.txt | 39 +++++++++++++++++++

Please split DT bindings to separate patch.

>  drivers/iio/light/tsl2772.c                   | 16 ++++++++
>  include/dt-bindings/iio/amstaos,tsl2772.h     | 24 ++++++++++++
>  3 files changed, 79 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/light/tsl2772.txt
>  create mode 100644 include/dt-bindings/iio/amstaos,tsl2772.h
> 
> diff --git a/Documentation/devicetree/bindings/iio/light/tsl2772.txt b/Documentation/devicetree/bindings/iio/light/tsl2772.txt
> new file mode 100644
> index 000000000000..ab553d52b9fc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/light/tsl2772.txt
> @@ -0,0 +1,39 @@
> +* AMS/TAOS ALS and proximity sensor
> +
> +Required properties:
> +
> +  - compatible: Should be one of
> +		"amstaos,tsl2571"
> +		"amstaos,tsl2671"
> +		"amstaos,tmd2671"
> +		"amstaos,tsl2771"
> +		"amstaos,tmd2771"
> +		"amstaos,tsl2572"
> +		"amstaos,tsl2672"
> +		"amstaos,tmd2672"
> +		"amstaos,tsl2772"
> +		"amstaos,tmd2772"
> +  - reg: the I2C address of the device
> +
> +Optional properties:
> +
> +  - amstaos,prox_diode - must be TSL2772_DIODE0, TSL2772_DIODE1, or
> +                         TSL2772_DIODE_BOTH.

s/_/-/

> +  - amstaos,prox_power - must be TSL2772_100_mA, TSL2772_50_mA, TSL2772_25_mA,
> +                         or TSL2772_13_mA.

I wonder if this should be common. Perhaps we should use the existing 
'led-max-microamp' as this is setting the current for an IR LED.

And while called 'power' this setting is current.

> +  - interrupt-parent: should be the phandle for the interrupt controller

Don't need to document this. It's implied by interrupts (and could be in 
a parent node).

> +  - interrupts: the sole interrupt generated by the device
> +
> +  Refer to interrupt-controller/interrupts.txt for generic interrupt client
> +  node bindings.
> +
> +Example:
> +
> +#include <dt-bindings/iio/amstaos,tsl2772.h>
> +
> +tsl2772@39 {
> +	compatible = "amstaos,tsl2772";
> +	reg = <0x39>;
> +	interrupts-extended = <&msmgpio 61 IRQ_TYPE_EDGE_FALLING>;
> +	amstaos,prox_diode = <TSL2772_DIODE0>;
> +};

Re: [PATCH v2 3/7] iio: tsl2772: add support for reading power settings from device tree

[Jonathan Cameron]

On Fri, 20 Jul 2018 11:36:35 -0600
Rob Herring <robh@kernel.org> wrote:

> On Tue, Jul 17, 2018 at 04:41:54AM -0400, Brian Masney wrote:
> > This patch adds support for optionally reading the prox_diode and
> > prox_power settings from device tree. This was tested using a LG
> > Nexus 5 (hammerhead) which requires a different diode than the driver
> > default for the IR LED.
> > 
> > Signed-off-by: Brian Masney <masneyb@onstation.org>
> > ---
> > The next patch in the series removes the tsl2772 driver from the
> > trivial-devices.txt file. I separated it out so that change can go
> > through device tree.
> > 
> >  .../devicetree/bindings/iio/light/tsl2772.txt | 39 +++++++++++++++++++  
> 
> Please split DT bindings to separate patch.
> 
> >  drivers/iio/light/tsl2772.c                   | 16 ++++++++
> >  include/dt-bindings/iio/amstaos,tsl2772.h     | 24 ++++++++++++
> >  3 files changed, 79 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/iio/light/tsl2772.txt
> >  create mode 100644 include/dt-bindings/iio/amstaos,tsl2772.h
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/light/tsl2772.txt b/Documentation/devicetree/bindings/iio/light/tsl2772.txt
> > new file mode 100644
> > index 000000000000..ab553d52b9fc
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/light/tsl2772.txt
> > @@ -0,0 +1,39 @@
> > +* AMS/TAOS ALS and proximity sensor
> > +
> > +Required properties:
> > +
> > +  - compatible: Should be one of
> > +		"amstaos,tsl2571"
> > +		"amstaos,tsl2671"
> > +		"amstaos,tmd2671"
> > +		"amstaos,tsl2771"
> > +		"amstaos,tmd2771"
> > +		"amstaos,tsl2572"
> > +		"amstaos,tsl2672"
> > +		"amstaos,tmd2672"
> > +		"amstaos,tsl2772"
> > +		"amstaos,tmd2772"
> > +  - reg: the I2C address of the device
> > +
> > +Optional properties:
> > +
> > +  - amstaos,prox_diode - must be TSL2772_DIODE0, TSL2772_DIODE1, or
> > +                         TSL2772_DIODE_BOTH.  
> 
> s/_/-/
> 
> > +  - amstaos,prox_power - must be TSL2772_100_mA, TSL2772_50_mA, TSL2772_25_mA,
> > +                         or TSL2772_13_mA.  
> 
> I wonder if this should be common. Perhaps we should use the existing 
> 'led-max-microamp' as this is setting the current for an IR LED.

Seems reasonable, then perhaps have two controls to turn on the diodes
above.

> 
> And while called 'power' this setting is current.

Also can we have real values?  I really don't like defines if they
aren't absolutely necessary - particularly when there is a nice real
unit to be used.

> 
> > +  - interrupt-parent: should be the phandle for the interrupt controller  
> 
> Don't need to document this. It's implied by interrupts (and could be in 
> a parent node).
> 
> > +  - interrupts: the sole interrupt generated by the device
> > +
> > +  Refer to interrupt-controller/interrupts.txt for generic interrupt client
> > +  node bindings.
> > +
> > +Example:
> > +
> > +#include <dt-bindings/iio/amstaos,tsl2772.h>
> > +
> > +tsl2772@39 {
> > +	compatible = "amstaos,tsl2772";
> > +	reg = <0x39>;
> > +	interrupts-extended = <&msmgpio 61 IRQ_TYPE_EDGE_FALLING>;
> > +	amstaos,prox_diode = <TSL2772_DIODE0>;
> > +};  
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 3/7] iio: tsl2772: add support for reading power settings from device tree

[Brian Masney]

On Sat, Jul 21, 2018 at 06:37:16PM +0100, Jonathan Cameron wrote:
> On Fri, 20 Jul 2018 11:36:35 -0600
> Rob Herring <robh@kernel.org> wrote:
> 
> > On Tue, Jul 17, 2018 at 04:41:54AM -0400, Brian Masney wrote:
> > > This patch adds support for optionally reading the prox_diode and
> > > prox_power settings from device tree. This was tested using a LG
> > > Nexus 5 (hammerhead) which requires a different diode than the driver
> > > default for the IR LED.
> > > 
> > > Signed-off-by: Brian Masney <masneyb@onstation.org>
> > > ---
> > > The next patch in the series removes the tsl2772 driver from the
> > > trivial-devices.txt file. I separated it out so that change can go
> > > through device tree.
> > > 
> > >  .../devicetree/bindings/iio/light/tsl2772.txt | 39 +++++++++++++++++++  
> > 
> > Please split DT bindings to separate patch.
> > 
> > >  drivers/iio/light/tsl2772.c                   | 16 ++++++++
> > >  include/dt-bindings/iio/amstaos,tsl2772.h     | 24 ++++++++++++
> > >  3 files changed, 79 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/iio/light/tsl2772.txt
> > >  create mode 100644 include/dt-bindings/iio/amstaos,tsl2772.h
> > > 
> > > diff --git a/Documentation/devicetree/bindings/iio/light/tsl2772.txt b/Documentation/devicetree/bindings/iio/light/tsl2772.txt
> > > new file mode 100644
> > > index 000000000000..ab553d52b9fc
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/iio/light/tsl2772.txt
> > > @@ -0,0 +1,39 @@
> > > +* AMS/TAOS ALS and proximity sensor
> > > +
> > > +Required properties:
> > > +
> > > +  - compatible: Should be one of
> > > +		"amstaos,tsl2571"
> > > +		"amstaos,tsl2671"
> > > +		"amstaos,tmd2671"
> > > +		"amstaos,tsl2771"
> > > +		"amstaos,tmd2771"
> > > +		"amstaos,tsl2572"
> > > +		"amstaos,tsl2672"
> > > +		"amstaos,tmd2672"
> > > +		"amstaos,tsl2772"
> > > +		"amstaos,tmd2772"
> > > +  - reg: the I2C address of the device
> > > +
> > > +Optional properties:
> > > +
> > > +  - amstaos,prox_diode - must be TSL2772_DIODE0, TSL2772_DIODE1, or
> > > +                         TSL2772_DIODE_BOTH.  
> > 
> > s/_/-/
> > 
> > > +  - amstaos,prox_power - must be TSL2772_100_mA, TSL2772_50_mA, TSL2772_25_mA,
> > > +                         or TSL2772_13_mA.  
> > 
> > I wonder if this should be common. Perhaps we should use the existing 
> > 'led-max-microamp' as this is setting the current for an IR LED.
> 
> Seems reasonable, then perhaps have two controls to turn on the diodes
> above.
> 
> > 
> > And while called 'power' this setting is current.
> 
> Also can we have real values?  I really don't like defines if they
> aren't absolutely necessary - particularly when there is a nice real
> unit to be used.

How about these options then?

amstaos,proximity-diode-0-enabled;
amstaos,proximity-diode-1-enabled;
led-max-microamp = <100000>;

Brian

Re: [PATCH v2 3/7] iio: tsl2772: add support for reading power settings from device tree

[Jonathan Cameron]

On Sun, 22 Jul 2018 12:37:20 +0000
Brian Masney <masneyb@onstation.org> wrote:

> On Sat, Jul 21, 2018 at 06:37:16PM +0100, Jonathan Cameron wrote:
> > On Fri, 20 Jul 2018 11:36:35 -0600
> > Rob Herring <robh@kernel.org> wrote:
> >   
> > > On Tue, Jul 17, 2018 at 04:41:54AM -0400, Brian Masney wrote:  
> > > > This patch adds support for optionally reading the prox_diode and
> > > > prox_power settings from device tree. This was tested using a LG
> > > > Nexus 5 (hammerhead) which requires a different diode than the driver
> > > > default for the IR LED.
> > > > 
> > > > Signed-off-by: Brian Masney <masneyb@onstation.org>
> > > > ---
> > > > The next patch in the series removes the tsl2772 driver from the
> > > > trivial-devices.txt file. I separated it out so that change can go
> > > > through device tree.
> > > > 
> > > >  .../devicetree/bindings/iio/light/tsl2772.txt | 39 +++++++++++++++++++    
> > > 
> > > Please split DT bindings to separate patch.
> > >   
> > > >  drivers/iio/light/tsl2772.c                   | 16 ++++++++
> > > >  include/dt-bindings/iio/amstaos,tsl2772.h     | 24 ++++++++++++
> > > >  3 files changed, 79 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/iio/light/tsl2772.txt
> > > >  create mode 100644 include/dt-bindings/iio/amstaos,tsl2772.h
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/iio/light/tsl2772.txt b/Documentation/devicetree/bindings/iio/light/tsl2772.txt
> > > > new file mode 100644
> > > > index 000000000000..ab553d52b9fc
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/iio/light/tsl2772.txt
> > > > @@ -0,0 +1,39 @@
> > > > +* AMS/TAOS ALS and proximity sensor
> > > > +
> > > > +Required properties:
> > > > +
> > > > +  - compatible: Should be one of
> > > > +		"amstaos,tsl2571"
> > > > +		"amstaos,tsl2671"
> > > > +		"amstaos,tmd2671"
> > > > +		"amstaos,tsl2771"
> > > > +		"amstaos,tmd2771"
> > > > +		"amstaos,tsl2572"
> > > > +		"amstaos,tsl2672"
> > > > +		"amstaos,tmd2672"
> > > > +		"amstaos,tsl2772"
> > > > +		"amstaos,tmd2772"
> > > > +  - reg: the I2C address of the device
> > > > +
> > > > +Optional properties:
> > > > +
> > > > +  - amstaos,prox_diode - must be TSL2772_DIODE0, TSL2772_DIODE1, or
> > > > +                         TSL2772_DIODE_BOTH.    
> > > 
> > > s/_/-/
> > >   
> > > > +  - amstaos,prox_power - must be TSL2772_100_mA, TSL2772_50_mA, TSL2772_25_mA,
> > > > +                         or TSL2772_13_mA.    
> > > 
> > > I wonder if this should be common. Perhaps we should use the existing 
> > > 'led-max-microamp' as this is setting the current for an IR LED.  
> > 
> > Seems reasonable, then perhaps have two controls to turn on the diodes
> > above.
> >   
> > > 
> > > And while called 'power' this setting is current.  
> > 
> > Also can we have real values?  I really don't like defines if they
> > aren't absolutely necessary - particularly when there is a nice real
> > unit to be used.  
> 
> How about these options then?
> 
> amstaos,proximity-diode-0-enabled;
> amstaos,proximity-diode-1-enabled;
> led-max-microamp = <100000>;

Works for me.  Rob?

J
> 
> Brian

Re: [PATCH v2 3/7] iio: tsl2772: add support for reading power settings from device tree

[Rob Herring]

On Sun, Jul 22, 2018 at 11:17 AM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Sun, 22 Jul 2018 12:37:20 +0000
> Brian Masney <masneyb@onstation.org> wrote:
>
> > On Sat, Jul 21, 2018 at 06:37:16PM +0100, Jonathan Cameron wrote:
> > > On Fri, 20 Jul 2018 11:36:35 -0600
> > > Rob Herring <robh@kernel.org> wrote:
> > >
> > > > On Tue, Jul 17, 2018 at 04:41:54AM -0400, Brian Masney wrote:
> > > > > This patch adds support for optionally reading the prox_diode and
> > > > > prox_power settings from device tree. This was tested using a LG
> > > > > Nexus 5 (hammerhead) which requires a different diode than the driver
> > > > > default for the IR LED.
> > > > >
> > > > > Signed-off-by: Brian Masney <masneyb@onstation.org>

> > > > > +  - amstaos,prox_diode - must be TSL2772_DIODE0, TSL2772_DIODE1, or
> > > > > +                         TSL2772_DIODE_BOTH.
> > > >
> > > > s/_/-/
> > > >
> > > > > +  - amstaos,prox_power - must be TSL2772_100_mA, TSL2772_50_mA, TSL2772_25_mA,
> > > > > +                         or TSL2772_13_mA.
> > > >
> > > > I wonder if this should be common. Perhaps we should use the existing
> > > > 'led-max-microamp' as this is setting the current for an IR LED.
> > >
> > > Seems reasonable, then perhaps have two controls to turn on the diodes
> > > above.
> > >
> > > >
> > > > And while called 'power' this setting is current.
> > >
> > > Also can we have real values?  I really don't like defines if they
> > > aren't absolutely necessary - particularly when there is a nice real
> > > unit to be used.
> >
> > How about these options then?
> >
> > amstaos,proximity-diode-0-enabled;
> > amstaos,proximity-diode-1-enabled;
> > led-max-microamp = <100000>;
>
> Works for me.  Rob?

I think we're bikeshedding, but I'd prefer a single property though
perhaps as a list (0, 1, or <0 1>) or mask. A list would be similar to
the "led-sources" property format.

Rob

Re: [PATCH v2 3/7] iio: tsl2772: add support for reading power settings from device tree

[Jonathan Cameron]

On Tue, 17 Jul 2018 04:41:54 -0400
Brian Masney <masneyb@onstation.org> wrote:

> This patch adds support for optionally reading the prox_diode and
> prox_power settings from device tree. This was tested using a LG
> Nexus 5 (hammerhead) which requires a different diode than the driver
> default for the IR LED.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>

I think we can make this binding more 'generic' as right now
the amstaos bindings aren't even general enough to apply
to future amstaos devices that need a similar binding.

I'm always anti defines rather than real values in DT (where
possible) and I think there is no reason not to use real values
here.

Jonathan

> ---
> The next patch in the series removes the tsl2772 driver from the
> trivial-devices.txt file. I separated it out so that change can go
> through device tree.
> 
>  .../devicetree/bindings/iio/light/tsl2772.txt | 39 +++++++++++++++++++
>  drivers/iio/light/tsl2772.c                   | 16 ++++++++
>  include/dt-bindings/iio/amstaos,tsl2772.h     | 24 ++++++++++++
>  3 files changed, 79 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/light/tsl2772.txt
>  create mode 100644 include/dt-bindings/iio/amstaos,tsl2772.h
> 
> diff --git a/Documentation/devicetree/bindings/iio/light/tsl2772.txt b/Documentation/devicetree/bindings/iio/light/tsl2772.txt
> new file mode 100644
> index 000000000000..ab553d52b9fc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/light/tsl2772.txt
> @@ -0,0 +1,39 @@
> +* AMS/TAOS ALS and proximity sensor
> +
> +Required properties:
> +
> +  - compatible: Should be one of
> +		"amstaos,tsl2571"
> +		"amstaos,tsl2671"
> +		"amstaos,tmd2671"
> +		"amstaos,tsl2771"
> +		"amstaos,tmd2771"
> +		"amstaos,tsl2572"
> +		"amstaos,tsl2672"
> +		"amstaos,tmd2672"
> +		"amstaos,tsl2772"
> +		"amstaos,tmd2772"
> +  - reg: the I2C address of the device
> +
> +Optional properties:
> +
> +  - amstaos,prox_diode - must be TSL2772_DIODE0, TSL2772_DIODE1, or
> +                         TSL2772_DIODE_BOTH.
> +  - amstaos,prox_power - must be TSL2772_100_mA, TSL2772_50_mA, TSL2772_25_mA,
> +                         or TSL2772_13_mA.
> +  - interrupt-parent: should be the phandle for the interrupt controller
> +  - interrupts: the sole interrupt generated by the device
> +
> +  Refer to interrupt-controller/interrupts.txt for generic interrupt client
> +  node bindings.
> +
> +Example:
> +
> +#include <dt-bindings/iio/amstaos,tsl2772.h>
> +
> +tsl2772@39 {
> +	compatible = "amstaos,tsl2772";
> +	reg = <0x39>;
> +	interrupts-extended = <&msmgpio 61 IRQ_TYPE_EDGE_FALLING>;
> +	amstaos,prox_diode = <TSL2772_DIODE0>;
> +};
> diff --git a/drivers/iio/light/tsl2772.c b/drivers/iio/light/tsl2772.c
> index 34d42a2504c9..0be57f2ecb02 100644
> --- a/drivers/iio/light/tsl2772.c
> +++ b/drivers/iio/light/tsl2772.c
> @@ -515,6 +515,18 @@ static int tsl2772_get_prox(struct iio_dev *indio_dev)
>  	return ret;
>  }
>  
> +#ifdef CONFIG_OF
> +static void tsl2772_parse_dt(struct tsl2772_chip *chip)
> +{
> +	struct device_node *of_node = chip->client->dev.of_node;
> +
> +	of_property_read_u32(of_node, "amstaos,prox_diode",
> +			     &chip->settings.prox_diode);
> +	of_property_read_u32(of_node, "amstaos,prox_power",
> +			     &chip->settings.prox_power);
> +}
> +#endif
> +
>  /**
>   * tsl2772_defaults() - Populates the device nominal operating parameters
>   *                      with those provided by a 'platform' data struct or
> @@ -541,6 +553,10 @@ static void tsl2772_defaults(struct tsl2772_chip *chip)
>  		memcpy(chip->tsl2772_device_lux,
>  		       tsl2772_default_lux_table_group[chip->id],
>  		       TSL2772_DEFAULT_TABLE_BYTES);
> +
> +#ifdef CONFIG_OF
> +	tsl2772_parse_dt(chip);
> +#endif
>  }
>  
>  /**
> diff --git a/include/dt-bindings/iio/amstaos,tsl2772.h b/include/dt-bindings/iio/amstaos,tsl2772.h
> new file mode 100644
> index 000000000000..ad6f9fbc0845
> --- /dev/null
> +++ b/include/dt-bindings/iio/amstaos,tsl2772.h
> @@ -0,0 +1,24 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Device driver for monitoring ambient light intensity (lux)
> + * and proximity (prox) within the TAOS TSL2772 family of devices.
> + *
> + * Copyright (c) 2012 TAOS Corporation.
> + * Copyright (c) 2017-2018 Brian Masney <masneyb@onstation.org>
> + */
> +
> +#ifndef _DT_BINDINGS_AMSTAOS_TSL2772_H
> +#define _DT_BINDINGS_AMSTAOS_TSL2772_H
> +
> +/* Proximity diode to use */
> +#define TSL2772_DIODE0                  0x01
> +#define TSL2772_DIODE1                  0x02
> +#define TSL2772_DIODE_BOTH              0x03

Can we have two separate parameters to enable them?

> +
> +/* LED Power */
> +#define TSL2772_100_mA                  0x00
> +#define TSL2772_50_mA                   0x01
> +#define TSL2772_25_mA                   0x02
> +#define TSL2772_13_mA                   0x03

I'd like to see real numbers rather than defines.  List the valid
values in the binding then match them to register addresses in the 
driver.

A different instance of this binding for a different taos device
might have different values that are valid.


> +
> +#endif /* _DT_BINDINGS_AMSTAOS_TSL2772_H */

[PATCH v2 4/7] dt-bindings: trivial: remove tsl2772

[Brian Masney]

Remove the tsl2772 driver from trivial-devices.txt. A separate
patch added the binding information to the file
Documentation/devicetree/bindings/iio/light/tsl2772.txt.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 Documentation/devicetree/bindings/trivial-devices.txt | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/trivial-devices.txt b/Documentation/devicetree/bindings/trivial-devices.txt
index 763a2808a95c..a977ccef7230 100644
--- a/Documentation/devicetree/bindings/trivial-devices.txt
+++ b/Documentation/devicetree/bindings/trivial-devices.txt
@@ -21,16 +21,6 @@ adi,adt7490		+/-1C TDM Extended Temp Range I.C
 adi,adxl345		Three-Axis Digital Accelerometer
 adi,adxl346		Three-Axis Digital Accelerometer (backward-compatibility value "adi,adxl345" must be listed too)
 ams,iaq-core		AMS iAQ-Core VOC Sensor
-amstaos,tsl2571		AMS/TAOS ALS and proximity sensor
-amstaos,tsl2671		AMS/TAOS ALS and proximity sensor
-amstaos,tmd2671		AMS/TAOS ALS and proximity sensor
-amstaos,tsl2771		AMS/TAOS ALS and proximity sensor
-amstaos,tmd2771		AMS/TAOS ALS and proximity sensor
-amstaos,tsl2572		AMS/TAOS ALS and proximity sensor
-amstaos,tsl2672		AMS/TAOS ALS and proximity sensor
-amstaos,tmd2672		AMS/TAOS ALS and proximity sensor
-amstaos,tsl2772		AMS/TAOS ALS and proximity sensor
-amstaos,tmd2772		AMS/TAOS ALS and proximity sensor
 at,24c08		i2c serial eeprom  (24cxx)
 atmel,at97sc3204t	i2c trusted platform module (TPM)
 capella,cm32181		CM32181: Ambient Light Sensor
-- 
2.17.1

Re: [PATCH v2 4/7] dt-bindings: trivial: remove tsl2772

[Rob Herring]

On Tue, Jul 17, 2018 at 04:41:55AM -0400, Brian Masney wrote:
> Remove the tsl2772 driver from trivial-devices.txt. A separate
> patch added the binding information to the file
> Documentation/devicetree/bindings/iio/light/tsl2772.txt.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
> ---
>  Documentation/devicetree/bindings/trivial-devices.txt | 10 ----------
>  1 file changed, 10 deletions(-)

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

[PATCH v2 5/7] iio: tsl2772: add support for regulator framework

[Brian Masney]

This patch adds support for the regulator framework to the tsl2772
driver. Driver was tested using a LG Nexus 5 (hammerhead) phone with
the two regulators and on a Raspberry Pi 2 without any regulators
controlling the power to the sensor.

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 .../devicetree/bindings/iio/light/tsl2772.txt |  4 +
 drivers/iio/light/tsl2772.c                   | 82 ++++++++++++++++++-
 2 files changed, 85 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/iio/light/tsl2772.txt b/Documentation/devicetree/bindings/iio/light/tsl2772.txt
index ab553d52b9fc..bfde8b2c8790 100644
--- a/Documentation/devicetree/bindings/iio/light/tsl2772.txt
+++ b/Documentation/devicetree/bindings/iio/light/tsl2772.txt
@@ -21,6 +21,8 @@ Optional properties:
                          TSL2772_DIODE_BOTH.
   - amstaos,prox_power - must be TSL2772_100_mA, TSL2772_50_mA, TSL2772_25_mA,
                          or TSL2772_13_mA.
+  - vdd-supply: phandle to the regulator that provides power to the sensor.
+  - vddio-supply: phandle to the regulator that provides power to the bus.
   - interrupt-parent: should be the phandle for the interrupt controller
   - interrupts: the sole interrupt generated by the device
 
@@ -35,5 +37,7 @@ tsl2772@39 {
 	compatible = "amstaos,tsl2772";
 	reg = <0x39>;
 	interrupts-extended = <&msmgpio 61 IRQ_TYPE_EDGE_FALLING>;
+	vdd-supply = <&pm8941_l17>;
+	vddio-supply = <&pm8941_lvs1>;
 	amstaos,prox_diode = <TSL2772_DIODE0>;
 };
diff --git a/drivers/iio/light/tsl2772.c b/drivers/iio/light/tsl2772.c
index 0be57f2ecb02..f6a27f8b3ca7 100644
--- a/drivers/iio/light/tsl2772.c
+++ b/drivers/iio/light/tsl2772.c
@@ -20,6 +20,7 @@
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 #include <linux/platform_data/tsl2772.h>
+#include <linux/regulator/consumer.h>
 
 /* Cal defs */
 #define PROX_STAT_CAL			0
@@ -146,6 +147,9 @@ struct tsl2772_chip {
 	struct mutex prox_mutex;
 	struct mutex als_mutex;
 	struct i2c_client *client;
+	struct regulator *vdd_supply;
+	struct regulator *vddio_supply;
+	bool regulators_enabled;
 	u16 prox_data;
 	struct tsl2772_als_info als_cur_info;
 	struct tsl2772_settings settings;
@@ -609,6 +613,46 @@ static int tsl2772_als_calibrate(struct iio_dev *indio_dev)
 	return ret;
 }
 
+static int tsl2772_enable_regulators(struct tsl2772_chip *chip)
+{
+	int ret;
+
+	ret = regulator_enable(chip->vddio_supply);
+	if (ret < 0) {
+		dev_err(&chip->client->dev, "Failed to enable regulator: %d\n",
+			ret);
+		return ret;
+	}
+
+	ret = regulator_enable(chip->vdd_supply);
+	if (ret < 0) {
+		regulator_disable(chip->vddio_supply);
+		dev_err(&chip->client->dev, "Failed to enable regulator: %d\n",
+			ret);
+		return ret;
+	}
+
+	chip->regulators_enabled = true;
+
+	return 0;
+}
+
+static void tsl2772_disable_regulators(struct tsl2772_chip *chip)
+{
+	if (!chip->regulators_enabled)
+		return;
+
+	regulator_disable(chip->vdd_supply);
+	regulator_disable(chip->vddio_supply);
+
+	chip->regulators_enabled = false;
+}
+
+static void tsl2772_disable_regulators_action(void *_data)
+{
+	tsl2772_disable_regulators(_data);
+}
+
 static int tsl2772_chip_on(struct iio_dev *indio_dev)
 {
 	struct tsl2772_chip *chip = iio_priv(indio_dev);
@@ -666,6 +710,10 @@ static int tsl2772_chip_on(struct iio_dev *indio_dev)
 	chip->als_gain_time_scale = als_time_us *
 		tsl2772_als_gain[chip->settings.als_gain];
 
+	ret = tsl2772_enable_regulators(chip);
+	if (ret < 0)
+		return ret;
+
 	/*
 	 * TSL2772 Specific power-on / adc enable sequence
 	 * Power on the device 1st.
@@ -724,10 +772,13 @@ static int tsl2772_chip_on(struct iio_dev *indio_dev)
 static int tsl2772_chip_off(struct iio_dev *indio_dev)
 {
 	struct tsl2772_chip *chip = iio_priv(indio_dev);
+	int ret;
 
 	/* turn device off */
 	chip->tsl2772_chip_status = TSL2772_CHIP_SUSPENDED;
-	return tsl2772_write_control_reg(chip, 0x00);
+	ret = tsl2772_write_control_reg(chip, 0x00);
+	tsl2772_disable_regulators(chip);
+	return ret;
 }
 
 /**
@@ -1666,6 +1717,35 @@ static int tsl2772_probe(struct i2c_client *clientp,
 	chip->client = clientp;
 	i2c_set_clientdata(clientp, indio_dev);
 
+	chip->vddio_supply = devm_regulator_get(&clientp->dev, "vddio");
+	if (IS_ERR(chip->vddio_supply)) {
+		if (PTR_ERR(chip->vddio_supply) != -EPROBE_DEFER)
+			dev_err(&clientp->dev,
+				"Failed to get vddio regulator %d\n",
+				(int)PTR_ERR(chip->vddio_supply));
+
+		return PTR_ERR(chip->vddio_supply);
+	}
+
+	chip->vdd_supply = devm_regulator_get(&clientp->dev, "vdd");
+	if (IS_ERR(chip->vdd_supply)) {
+		if (PTR_ERR(chip->vdd_supply) != -EPROBE_DEFER)
+			dev_err(&clientp->dev,
+				"Failed to get vdd regulator %d\n",
+				(int)PTR_ERR(chip->vdd_supply));
+
+		return PTR_ERR(chip->vdd_supply);
+	}
+
+	ret = devm_add_action(&clientp->dev, tsl2772_disable_regulators_action,
+			      chip);
+	if (ret < 0) {
+		tsl2772_disable_regulators_action(chip);
+		dev_err(&clientp->dev, "Failed to setup regulator cleanup action %d\n",
+			ret);
+		return ret;
+	}
+
 	ret = i2c_smbus_read_byte_data(chip->client,
 				       TSL2772_CMD_REG | TSL2772_CHIPID);
 	if (ret < 0)
-- 
2.17.1

Re: [PATCH v2 5/7] iio: tsl2772: add support for regulator framework

[Rob Herring]

On Tue, Jul 17, 2018 at 04:41:56AM -0400, Brian Masney wrote:
> This patch adds support for the regulator framework to the tsl2772
> driver. Driver was tested using a LG Nexus 5 (hammerhead) phone with
> the two regulators and on a Raspberry Pi 2 without any regulators
> controlling the power to the sensor.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
> ---
>  .../devicetree/bindings/iio/light/tsl2772.txt |  4 +

This belongs with the binding patch. Bindings should be complete, not 
extended as a driver changes. 

>  drivers/iio/light/tsl2772.c                   | 82 ++++++++++++++++++-
>  2 files changed, 85 insertions(+), 1 deletion(-)

Re: [PATCH v2 5/7] iio: tsl2772: add support for regulator framework

[Jonathan Cameron]

On Tue, 17 Jul 2018 04:41:56 -0400
Brian Masney <masneyb@onstation.org> wrote:

> This patch adds support for the regulator framework to the tsl2772
> driver. Driver was tested using a LG Nexus 5 (hammerhead) phone with
> the two regulators and on a Raspberry Pi 2 without any regulators
> controlling the power to the sensor.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
There is an ordering issue in probe and hence the tear down.  Fixing
it is unfortunately going to make the rest of the handling more complex...

Jonathan

> ---
>  .../devicetree/bindings/iio/light/tsl2772.txt |  4 +
>  drivers/iio/light/tsl2772.c                   | 82 ++++++++++++++++++-
>  2 files changed, 85 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/light/tsl2772.txt b/Documentation/devicetree/bindings/iio/light/tsl2772.txt
> index ab553d52b9fc..bfde8b2c8790 100644
> --- a/Documentation/devicetree/bindings/iio/light/tsl2772.txt
> +++ b/Documentation/devicetree/bindings/iio/light/tsl2772.txt
> @@ -21,6 +21,8 @@ Optional properties:
>                           TSL2772_DIODE_BOTH.
>    - amstaos,prox_power - must be TSL2772_100_mA, TSL2772_50_mA, TSL2772_25_mA,
>                           or TSL2772_13_mA.
> +  - vdd-supply: phandle to the regulator that provides power to the sensor.
> +  - vddio-supply: phandle to the regulator that provides power to the bus.
>    - interrupt-parent: should be the phandle for the interrupt controller
>    - interrupts: the sole interrupt generated by the device
>  
> @@ -35,5 +37,7 @@ tsl2772@39 {
>  	compatible = "amstaos,tsl2772";
>  	reg = <0x39>;
>  	interrupts-extended = <&msmgpio 61 IRQ_TYPE_EDGE_FALLING>;
> +	vdd-supply = <&pm8941_l17>;
> +	vddio-supply = <&pm8941_lvs1>;
>  	amstaos,prox_diode = <TSL2772_DIODE0>;
>  };
> diff --git a/drivers/iio/light/tsl2772.c b/drivers/iio/light/tsl2772.c
> index 0be57f2ecb02..f6a27f8b3ca7 100644
> --- a/drivers/iio/light/tsl2772.c
> +++ b/drivers/iio/light/tsl2772.c
> @@ -20,6 +20,7 @@
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
>  #include <linux/platform_data/tsl2772.h>
> +#include <linux/regulator/consumer.h>
>  
>  /* Cal defs */
>  #define PROX_STAT_CAL			0
> @@ -146,6 +147,9 @@ struct tsl2772_chip {
>  	struct mutex prox_mutex;
>  	struct mutex als_mutex;
>  	struct i2c_client *client;
> +	struct regulator *vdd_supply;
> +	struct regulator *vddio_supply;
> +	bool regulators_enabled;
>  	u16 prox_data;
>  	struct tsl2772_als_info als_cur_info;
>  	struct tsl2772_settings settings;
> @@ -609,6 +613,46 @@ static int tsl2772_als_calibrate(struct iio_dev *indio_dev)
>  	return ret;
>  }
>  
> +static int tsl2772_enable_regulators(struct tsl2772_chip *chip)
> +{
> +	int ret;
> +
> +	ret = regulator_enable(chip->vddio_supply);
> +	if (ret < 0) {
> +		dev_err(&chip->client->dev, "Failed to enable regulator: %d\n",
> +			ret);
> +		return ret;
> +	}
> +
> +	ret = regulator_enable(chip->vdd_supply);
> +	if (ret < 0) {
> +		regulator_disable(chip->vddio_supply);
> +		dev_err(&chip->client->dev, "Failed to enable regulator: %d\n",
> +			ret);
> +		return ret;
> +	}
> +
> +	chip->regulators_enabled = true;
> +
> +	return 0;
> +}
> +
> +static void tsl2772_disable_regulators(struct tsl2772_chip *chip)
> +{
> +	if (!chip->regulators_enabled)
> +		return;
> +
> +	regulator_disable(chip->vdd_supply);
> +	regulator_disable(chip->vddio_supply);
> +
> +	chip->regulators_enabled = false;

hmm. this gets fiddly so I can see why you are using
this flag to avoid missbalanced enables / disables.

> +}
> +
> +static void tsl2772_disable_regulators_action(void *_data)
> +{
> +	tsl2772_disable_regulators(_data);
> +}
> +
>  static int tsl2772_chip_on(struct iio_dev *indio_dev)
>  {
>  	struct tsl2772_chip *chip = iio_priv(indio_dev);
> @@ -666,6 +710,10 @@ static int tsl2772_chip_on(struct iio_dev *indio_dev)
>  	chip->als_gain_time_scale = als_time_us *
>  		tsl2772_als_gain[chip->settings.als_gain];
>  
> +	ret = tsl2772_enable_regulators(chip);
> +	if (ret < 0)
> +		return ret;
> +
>  	/*
>  	 * TSL2772 Specific power-on / adc enable sequence
>  	 * Power on the device 1st.
> @@ -724,10 +772,13 @@ static int tsl2772_chip_on(struct iio_dev *indio_dev)
>  static int tsl2772_chip_off(struct iio_dev *indio_dev)
>  {
>  	struct tsl2772_chip *chip = iio_priv(indio_dev);
> +	int ret;
>  
>  	/* turn device off */
>  	chip->tsl2772_chip_status = TSL2772_CHIP_SUSPENDED;
> -	return tsl2772_write_control_reg(chip, 0x00);
> +	ret = tsl2772_write_control_reg(chip, 0x00);
> +	tsl2772_disable_regulators(chip);

Blank line here would be nice.

> +	return ret;
>  }
>  
>  /**
> @@ -1666,6 +1717,35 @@ static int tsl2772_probe(struct i2c_client *clientp,
>  	chip->client = clientp;
>  	i2c_set_clientdata(clientp, indio_dev);
>  
> +	chip->vddio_supply = devm_regulator_get(&clientp->dev, "vddio");
> +	if (IS_ERR(chip->vddio_supply)) {
> +		if (PTR_ERR(chip->vddio_supply) != -EPROBE_DEFER)
> +			dev_err(&clientp->dev,
> +				"Failed to get vddio regulator %d\n",
> +				(int)PTR_ERR(chip->vddio_supply));
> +
> +		return PTR_ERR(chip->vddio_supply);
> +	}
> +
> +	chip->vdd_supply = devm_regulator_get(&clientp->dev, "vdd");
> +	if (IS_ERR(chip->vdd_supply)) {
> +		if (PTR_ERR(chip->vdd_supply) != -EPROBE_DEFER)
> +			dev_err(&clientp->dev,
> +				"Failed to get vdd regulator %d\n",
> +				(int)PTR_ERR(chip->vdd_supply));
> +
> +		return PTR_ERR(chip->vdd_supply);
> +	}
> +
> +	ret = devm_add_action(&clientp->dev, tsl2772_disable_regulators_action,
> +			      chip);

Can't do them together. you need to think about where you might
get a failure.  What if it is after the first enable but before the
second?  In that case you've just left a regulator on as we haven't done this
setup yet.

> +	if (ret < 0) {
> +		tsl2772_disable_regulators_action(chip);
> +		dev_err(&clientp->dev, "Failed to setup regulator cleanup action %d\n",
> +			ret);
> +		return ret;
> +	}
> +
>  	ret = i2c_smbus_read_byte_data(chip->client,
>  				       TSL2772_CMD_REG | TSL2772_CHIPID);
>  	if (ret < 0)

[PATCH v2 6/7] iio: tsl2772: add device tree binding for avago,apds9930

[Brian Masney]

The Avago APDS9930 has the same register set as the TAOS/AMS TSL2772 so
this patch adds the correct device tree bindings and the appropriate
LUX table values based on the values in the datasheet. Driver was tested
on a LG Nexus 5 (hammerhead) phone.

avago,apds9930 datasheet:
https://www.mouser.com/datasheet/2/678/avago_AV02-3190EN_DS_APDS-9930_2014-03-25[1]-1217273.pdf

tsl2772 datasheet:
https://ams.com/eng/content/download/291503/1066377/file/TSL2772_DS000181_2-00.pdf

Signed-off-by: Brian Masney <masneyb@onstation.org>
---
 .../devicetree/bindings/iio/light/tsl2772.txt  |  1 +
 drivers/iio/light/tsl2772.c                    | 18 ++++++++++++++++--
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/light/tsl2772.txt b/Documentation/devicetree/bindings/iio/light/tsl2772.txt
index bfde8b2c8790..e842db749ab2 100644
--- a/Documentation/devicetree/bindings/iio/light/tsl2772.txt
+++ b/Documentation/devicetree/bindings/iio/light/tsl2772.txt
@@ -13,6 +13,7 @@ Required properties:
 		"amstaos,tmd2672"
 		"amstaos,tsl2772"
 		"amstaos,tmd2772"
+		"avago,apds9930"
   - reg: the I2C address of the device
 
 Optional properties:
diff --git a/drivers/iio/light/tsl2772.c b/drivers/iio/light/tsl2772.c
index f6a27f8b3ca7..e5b449aec20c 100644
--- a/drivers/iio/light/tsl2772.c
+++ b/drivers/iio/light/tsl2772.c
@@ -119,7 +119,8 @@ enum {
 	tsl2672,
 	tmd2672,
 	tsl2772,
-	tmd2772
+	tmd2772,
+	apds9930,
 };
 
 enum {
@@ -201,6 +202,12 @@ static const struct tsl2772_lux tmd2x72_lux_table[TSL2772_DEF_LUX_TABLE_SZ] = {
 	{     0,      0 },
 };
 
+static const struct tsl2772_lux apds9930_lux_table[TSL2772_DEF_LUX_TABLE_SZ] = {
+	{ 52000,  96824 },
+	{ 38792,  67132 },
+	{     0,      0 },
+};
+
 static const struct tsl2772_lux *tsl2772_default_lux_table_group[] = {
 	[tsl2571] = tsl2x71_lux_table,
 	[tsl2671] = tsl2x71_lux_table,
@@ -212,6 +219,7 @@ static const struct tsl2772_lux *tsl2772_default_lux_table_group[] = {
 	[tmd2672] = tmd2x72_lux_table,
 	[tsl2772] = tsl2x72_lux_table,
 	[tmd2772] = tmd2x72_lux_table,
+	[apds9930] = apds9930_lux_table,
 };
 
 static const struct tsl2772_settings tsl2772_default_settings = {
@@ -262,6 +270,7 @@ static const int tsl2772_int_time_avail[][6] = {
 	[tmd2672] = { 0, 2730, 0, 2730, 0, 699000 },
 	[tsl2772] = { 0, 2730, 0, 2730, 0, 699000 },
 	[tmd2772] = { 0, 2730, 0, 2730, 0, 699000 },
+	[apds9930] = { 0, 2730, 0, 2730, 0, 699000 },
 };
 
 static int tsl2772_int_calibscale_avail[] = { 1, 8, 16, 120 };
@@ -287,7 +296,8 @@ static const u8 device_channel_config[] = {
 	[tsl2672] = PRX2,
 	[tmd2672] = PRX2,
 	[tsl2772] = ALSPRX2,
-	[tmd2772] = ALSPRX2
+	[tmd2772] = ALSPRX2,
+	[apds9930] = ALSPRX2,
 };
 
 static int tsl2772_read_status(struct tsl2772_chip *chip)
@@ -501,6 +511,7 @@ static int tsl2772_get_prox(struct iio_dev *indio_dev)
 	case tmd2672:
 	case tsl2772:
 	case tmd2772:
+	case apds9930:
 		if (!(ret & TSL2772_STA_PRX_VALID)) {
 			ret = -EINVAL;
 			goto prox_poll_err;
@@ -1325,6 +1336,7 @@ static int tsl2772_device_id_verif(int id, int target)
 	case tmd2672:
 	case tsl2772:
 	case tmd2772:
+	case apds9930:
 		return (id & 0xf0) == SWORDFISH_ID;
 	}
 
@@ -1852,6 +1864,7 @@ static const struct i2c_device_id tsl2772_idtable[] = {
 	{ "tmd2672", tmd2672 },
 	{ "tsl2772", tsl2772 },
 	{ "tmd2772", tmd2772 },
+	{ "apds9930", apds9930},
 	{}
 };
 
@@ -1868,6 +1881,7 @@ static const struct of_device_id tsl2772_of_match[] = {
 	{ .compatible = "amstaos,tmd2672" },
 	{ .compatible = "amstaos,tsl2772" },
 	{ .compatible = "amstaos,tmd2772" },
+	{ .compatible = "avago,apds9930" },
 	{}
 };
 MODULE_DEVICE_TABLE(of, tsl2772_of_match);
-- 
2.17.1

[PATCH v2 7/7] ARM: dts: qcom: msm8974-hammerhead: add device tree bindings for ALS / proximity

[Brian Masney]

This patch adds device tree bindings for the tsl2772 ALS / proximity
sensor for the LG Nexus 5 (hammerhead) phone.

Signed-off-by: Brian Masney <masneyb@onstation.org>
Signed-off-by: Jonathan Marek <jonathan@marek.ca>
---
 .../qcom-msm8974-lge-nexus5-hammerhead.dts    | 27 +++++++++++++++++++
 arch/arm/boot/dts/qcom-msm8974.dtsi           | 11 ++++++++
 2 files changed, 38 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts b/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
index 928affae1885..63c09e2a2eb2 100644
--- a/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
+++ b/arch/arm/boot/dts/qcom-msm8974-lge-nexus5-hammerhead.dts
@@ -3,6 +3,7 @@
 #include "qcom-pm8841.dtsi"
 #include "qcom-pm8941.dtsi"
 #include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/iio/amstaos,tsl2772.h>
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
 
@@ -242,6 +243,15 @@
 			};
 		};
 
+		i2c3_pins: i2c3 {
+			mux {
+				pins = "gpio10", "gpio11";
+				function = "blsp_i2c3";
+				drive-strength = <2>;
+				bias-disable;
+			};
+		};
+
 		i2c12_pins: i2c12 {
 			mux {
 				pins = "gpio87", "gpio88";
@@ -333,6 +343,23 @@
 			};
 		};
 	};
+
+	i2c@f9925000 {
+		status = "ok";
+		pinctrl-names = "default";
+		pinctrl-0 = <&i2c3_pins>;
+		clock-frequency = <100000>;
+		qcom,src-freq = <50000000>;
+
+		avago_apds993@39 {
+			compatible = "avago,apds9930";
+			reg = <0x39>;
+			interrupts-extended = <&msmgpio 61 IRQ_TYPE_EDGE_FALLING>;
+			vdd-supply = <&pm8941_l17>;
+			vddio-supply = <&pm8941_lvs1>;
+			amstaos,prox_diode = <TSL2772_DIODE0>;
+		};
+	};
 };
 
 &spmi_bus {
diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
index cebb6ae9143a..6dcf2bee66fb 100644
--- a/arch/arm/boot/dts/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
@@ -713,6 +713,17 @@
 			#size-cells = <0>;
 		};
 
+		blsp_i2c3: i2c@f9925000 {
+			status = "disabled";
+			compatible = "qcom,i2c-qup-v2.1.1";
+			reg = <0xf9925000 0x1000>;
+			interrupts = <0 97 IRQ_TYPE_NONE>;
+			clocks = <&gcc GCC_BLSP1_QUP3_I2C_APPS_CLK>, <&gcc GCC_BLSP1_AHB_CLK>;
+			clock-names = "core", "iface";
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
 		blsp_i2c8: i2c@f9964000 {
 			status = "disabled";
 			compatible = "qcom,i2c-qup-v2.1.1";
-- 
2.17.1