[PATCH 0/3] ARM: dts: qcom: msm8974-hammerhead: add support for mpu6515

[PATCH 0/3] ARM: dts: qcom: msm8974-hammerhead: add support for mpu6515

[Brian Masney]

This patch set adds support for the gyroscope / accelerometer
(mpu6515), magnetometer (ak8963), and temperature / pressure (bmp280)
sensors to the LG Nexus 5 (hammerhead) phone. Bindings for the MPU 6515
variant are added, along with regulator support to that driver in order
to correctly configure these drivers via device tree.

Brian Masney (3):
  iio: imu: mpu6050: add support for 6515 variant
  iio: imu: mpu6050: add support for regulator framework
  ARM: dts: qcom: msm8974-hammerhead: add device tree bindings for
    mpu6515

 .../bindings/iio/imu/inv_mpu6050.txt          |  2 +
 .../qcom-msm8974-lge-nexus5-hammerhead.dts    | 56 +++++++++++++++++
 arch/arm/boot/dts/qcom-msm8974.dtsi           | 11 ++++
 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c    | 63 +++++++++++++++++--
 drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c     |  7 ++-
 drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h     |  5 ++
 drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c     |  9 +++
 7 files changed, 147 insertions(+), 6 deletions(-)

-- 
2.17.1

[PATCH 1/3] iio: imu: mpu6050: add support for 6515 variant

[Brian Masney]

This patch adds support for the MPU 6515 variant. Confirmed that the
driver functions correctly on a LG Nexus 5 (hammerhead) phone.

Signed-off-by: Brian Masney <masneyb@onstation.org>
Signed-off-by: Jonathan Marek <jonathan@marek.ca>
---
This is a variation of Jonathan Marek's patch from postmarketOS
https://gitlab.com/postmarketOS/linux-postmarketos/commit/b8ad1ec1859c8bbcbce94944b3f4dd68f8f9fc37
with the following changes:

- The driver was previously throwing a 'whoami mismatch got XXX' error.
  Corrected ordering of hw_info variable to match the index of the
  inv_devices enum. The mpu6515 needed to come after mpu6500, not
  mpu6050.
- Removed regulator code. See next patch in this series.

I got confirmation from Jonathan M. that it was ok for me to submit his
patch upstream with these changes.

Jonathan C.: I assume that it is ok for the device tree binding
documentation to go through IIO? If not, I can split this out into a
separate patch if needed.

 Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt | 1 +
 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c                | 6 ++++++
 drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c                 | 5 +++++
 drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h                 | 2 ++
 4 files changed, 14 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt b/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
index 5f4777e8cc9e..b7def51c8ad9 100644
--- a/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
+++ b/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
@@ -6,6 +6,7 @@ Required properties:
  - compatible : should be one of
 		"invensense,mpu6050"
  		"invensense,mpu6500"
+		"invensense,mpu6515"
 		"invensense,mpu9150"
 		"invensense,mpu9250"
 		"invensense,mpu9255"
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index de68e83fc52d..12c1b9507007 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -103,6 +103,12 @@ static const struct inv_mpu6050_hw hw_info[] = {
 		.reg = &reg_set_6500,
 		.config = &chip_config_6050,
 	},
+	{
+		.whoami = INV_MPU6515_WHOAMI_VALUE,
+		.name = "MPU6515",
+		.reg = &reg_set_6500,
+		.config = &chip_config_6050,
+	},
 	{
 		.whoami = INV_MPU6000_WHOAMI_VALUE,
 		.name = "MPU6000",
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
index 495409d56207..dd758e3d403d 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
@@ -174,6 +174,7 @@ static int inv_mpu_remove(struct i2c_client *client)
 static const struct i2c_device_id inv_mpu_id[] = {
 	{"mpu6050", INV_MPU6050},
 	{"mpu6500", INV_MPU6500},
+	{"mpu6515", INV_MPU6515},
 	{"mpu9150", INV_MPU9150},
 	{"mpu9250", INV_MPU9250},
 	{"mpu9255", INV_MPU9255},
@@ -192,6 +193,10 @@ static const struct of_device_id inv_of_match[] = {
 		.compatible = "invensense,mpu6500",
 		.data = (void *)INV_MPU6500
 	},
+	{
+		.compatible = "invensense,mpu6515",
+		.data = (void *)INV_MPU6515
+	},
 	{
 		.compatible = "invensense,mpu9150",
 		.data = (void *)INV_MPU9150
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
index de8391693e17..e69a59659dbc 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
@@ -71,6 +71,7 @@ struct inv_mpu6050_reg_map {
 enum inv_devices {
 	INV_MPU6050,
 	INV_MPU6500,
+	INV_MPU6515,
 	INV_MPU6000,
 	INV_MPU9150,
 	INV_MPU9250,
@@ -256,6 +257,7 @@ struct inv_mpu6050_state {
 #define INV_MPU9150_WHOAMI_VALUE		0x68
 #define INV_MPU9250_WHOAMI_VALUE		0x71
 #define INV_MPU9255_WHOAMI_VALUE		0x73
+#define INV_MPU6515_WHOAMI_VALUE		0x74
 #define INV_ICM20608_WHOAMI_VALUE		0xAF
 
 /* scan element definition */
-- 
2.17.1

Re: [PATCH 1/3] iio: imu: mpu6050: add support for 6515 variant

[Jonathan Cameron]

On Tue, 10 Jul 2018 21:09:30 -0400
Brian Masney <masneyb@onstation.org> wrote:

> This patch adds support for the MPU 6515 variant. Confirmed that the
> driver functions correctly on a LG Nexus 5 (hammerhead) phone.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
> Signed-off-by: Jonathan Marek <jonathan@marek.ca>
> ---
> This is a variation of Jonathan Marek's patch from postmarketOS
> https://gitlab.com/postmarketOS/linux-postmarketos/commit/b8ad1ec1859c8bbcbce94944b3f4dd68f8f9fc37
> with the following changes:
> 
> - The driver was previously throwing a 'whoami mismatch got XXX' error.
>   Corrected ordering of hw_info variable to match the index of the
>   inv_devices enum. The mpu6515 needed to come after mpu6500, not
>   mpu6050.
> - Removed regulator code. See next patch in this series.
> 
> I got confirmation from Jonathan M. that it was ok for me to submit his
> patch upstream with these changes.
> 
> Jonathan C.: I assume that it is ok for the device tree binding
> documentation to go through IIO? If not, I can split this out into a
> separate patch if needed.
A single ID addition is normally uncontroversial enough that we don't bother
the DT maintainers (though of course they are welcome to look!).
So this is fine.
> 
>  Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt | 1 +
>  drivers/iio/imu/inv_mpu6050/inv_mpu_core.c                | 6 ++++++
>  drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c                 | 5 +++++
>  drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h                 | 2 ++
>  4 files changed, 14 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt b/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
> index 5f4777e8cc9e..b7def51c8ad9 100644
> --- a/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
> +++ b/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
> @@ -6,6 +6,7 @@ Required properties:
>   - compatible : should be one of
>  		"invensense,mpu6050"
>   		"invensense,mpu6500"
> +		"invensense,mpu6515"
>  		"invensense,mpu9150"
>  		"invensense,mpu9250"
>  		"invensense,mpu9255"
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index de68e83fc52d..12c1b9507007 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -103,6 +103,12 @@ static const struct inv_mpu6050_hw hw_info[] = {
>  		.reg = &reg_set_6500,
>  		.config = &chip_config_6050,
>  	},
> +	{
> +		.whoami = INV_MPU6515_WHOAMI_VALUE,
> +		.name = "MPU6515",
> +		.reg = &reg_set_6500,
> +		.config = &chip_config_6050,
> +	},
>  	{
>  		.whoami = INV_MPU6000_WHOAMI_VALUE,
>  		.name = "MPU6000",
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> index 495409d56207..dd758e3d403d 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> @@ -174,6 +174,7 @@ static int inv_mpu_remove(struct i2c_client *client)
>  static const struct i2c_device_id inv_mpu_id[] = {
>  	{"mpu6050", INV_MPU6050},
>  	{"mpu6500", INV_MPU6500},
> +	{"mpu6515", INV_MPU6515},
>  	{"mpu9150", INV_MPU9150},
>  	{"mpu9250", INV_MPU9250},
>  	{"mpu9255", INV_MPU9255},
> @@ -192,6 +193,10 @@ static const struct of_device_id inv_of_match[] = {
>  		.compatible = "invensense,mpu6500",
>  		.data = (void *)INV_MPU6500
>  	},
> +	{
> +		.compatible = "invensense,mpu6515",
> +		.data = (void *)INV_MPU6515
> +	},
>  	{
>  		.compatible = "invensense,mpu9150",
>  		.data = (void *)INV_MPU9150
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> index de8391693e17..e69a59659dbc 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> @@ -71,6 +71,7 @@ struct inv_mpu6050_reg_map {
>  enum inv_devices {
>  	INV_MPU6050,
>  	INV_MPU6500,
> +	INV_MPU6515,
>  	INV_MPU6000,
>  	INV_MPU9150,
>  	INV_MPU9250,
> @@ -256,6 +257,7 @@ struct inv_mpu6050_state {
>  #define INV_MPU9150_WHOAMI_VALUE		0x68
>  #define INV_MPU9250_WHOAMI_VALUE		0x71
>  #define INV_MPU9255_WHOAMI_VALUE		0x73
> +#define INV_MPU6515_WHOAMI_VALUE		0x74
>  #define INV_ICM20608_WHOAMI_VALUE		0xAF
>  
>  /* scan element definition */

Re: [PATCH 1/3] iio: imu: mpu6050: add support for 6515 variant

[Jonathan Cameron]

On Tue, 10 Jul 2018 21:09:30 -0400
Brian Masney <masneyb@onstation.org> wrote:

> This patch adds support for the MPU 6515 variant. Confirmed that the
> driver functions correctly on a LG Nexus 5 (hammerhead) phone.
> 
> Signed-off-by: Brian Masney <masneyb@onstation.org>
> Signed-off-by: Jonathan Marek <jonathan@marek.ca>
> ---
> This is a variation of Jonathan Marek's patch from postmarketOS
> https://gitlab.com/postmarketOS/linux-postmarketos/commit/b8ad1ec1859c8bbcbce94944b3f4dd68f8f9fc37
> with the following changes:
> 
> - The driver was previously throwing a 'whoami mismatch got XXX' error.
>   Corrected ordering of hw_info variable to match the index of the
>   inv_devices enum. The mpu6515 needed to come after mpu6500, not
>   mpu6050.
> - Removed regulator code. See next patch in this series.
> 
> I got confirmation from Jonathan M. that it was ok for me to submit his
> patch upstream with these changes.
> 
> Jonathan C.: I assume that it is ok for the device tree binding
> documentation to go through IIO? If not, I can split this out into a
> separate patch if needed.
A single ID addition is normally uncontroversial enough that we don't bother
the DT maintainers (though of course they are welcome to look!).
So this is fine.
> 
>  Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt | 1 +
>  drivers/iio/imu/inv_mpu6050/inv_mpu_core.c                | 6 ++++++
>  drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c                 | 5 +++++
>  drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h                 | 2 ++
>  4 files changed, 14 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt b/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
> index 5f4777e8cc9e..b7def51c8ad9 100644
> --- a/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
> +++ b/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
> @@ -6,6 +6,7 @@ Required properties:
>   - compatible : should be one of
>  		"invensense,mpu6050"
>   		"invensense,mpu6500"
> +		"invensense,mpu6515"
>  		"invensense,mpu9150"
>  		"invensense,mpu9250"
>  		"invensense,mpu9255"
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index de68e83fc52d..12c1b9507007 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -103,6 +103,12 @@ static const struct inv_mpu6050_hw hw_info[] = {
>  		.reg = &reg_set_6500,
>  		.config = &chip_config_6050,
>  	},
> +	{
> +		.whoami = INV_MPU6515_WHOAMI_VALUE,
> +		.name = "MPU6515",
> +		.reg = &reg_set_6500,
> +		.config = &chip_config_6050,
> +	},
>  	{
>  		.whoami = INV_MPU6000_WHOAMI_VALUE,
>  		.name = "MPU6000",
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> index 495409d56207..dd758e3d403d 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> @@ -174,6 +174,7 @@ static int inv_mpu_remove(struct i2c_client *client)
>  static const struct i2c_device_id inv_mpu_id[] = {
>  	{"mpu6050", INV_MPU6050},
>  	{"mpu6500", INV_MPU6500},
> +	{"mpu6515", INV_MPU6515},
>  	{"mpu9150", INV_MPU9150},
>  	{"mpu9250", INV_MPU9250},
>  	{"mpu9255", INV_MPU9255},
> @@ -192,6 +193,10 @@ static const struct of_device_id inv_of_match[] = {
>  		.compatible = "invensense,mpu6500",
>  		.data = (void *)INV_MPU6500
>  	},
> +	{
> +		.compatible = "invensense,mpu6515",
> +		.data = (void *)INV_MPU6515
> +	},
>  	{
>  		.compatible = "invensense,mpu9150",
>  		.data = (void *)INV_MPU9150
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> index de8391693e17..e69a59659dbc 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> @@ -71,6 +71,7 @@ struct inv_mpu6050_reg_map {
>  enum inv_devices {
>  	INV_MPU6050,
>  	INV_MPU6500,
> +	INV_MPU6515,
>  	INV_MPU6000,
>  	INV_MPU9150,
>  	INV_MPU9250,
> @@ -256,6 +257,7 @@ struct inv_mpu6050_state {
>  #define INV_MPU9150_WHOAMI_VALUE		0x68
>  #define INV_MPU9250_WHOAMI_VALUE		0x71
>  #define INV_MPU9255_WHOAMI_VALUE		0x73
> +#define INV_MPU6515_WHOAMI_VALUE		0x74
>  #define INV_ICM20608_WHOAMI_VALUE		0xAF
>  
>  /* scan element definition */

Re: [PATCH 1/3] iio: imu: mpu6050: add support for 6515 variant

[Jonathan Cameron]

On Thu, 12 Jul 2018 15:47:38 +0100
Jonathan Cameron <jonathan.cameron@huawei.com> wrote:

> On Tue, 10 Jul 2018 21:09:30 -0400
> Brian Masney <masneyb@onstation.org> wrote:
> 
> > This patch adds support for the MPU 6515 variant. Confirmed that the
> > driver functions correctly on a LG Nexus 5 (hammerhead) phone.
> > 
> > Signed-off-by: Brian Masney <masneyb@onstation.org>
> > Signed-off-by: Jonathan Marek <jonathan@marek.ca>
Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.

thanks,

Jonathan

> > ---
> > This is a variation of Jonathan Marek's patch from postmarketOS
> > https://gitlab.com/postmarketOS/linux-postmarketos/commit/b8ad1ec1859c8bbcbce94944b3f4dd68f8f9fc37
> > with the following changes:
> > 
> > - The driver was previously throwing a 'whoami mismatch got XXX' error.
> >   Corrected ordering of hw_info variable to match the index of the
> >   inv_devices enum. The mpu6515 needed to come after mpu6500, not
> >   mpu6050.
> > - Removed regulator code. See next patch in this series.
> > 
> > I got confirmation from Jonathan M. that it was ok for me to submit his
> > patch upstream with these changes.
> > 
> > Jonathan C.: I assume that it is ok for the device tree binding
> > documentation to go through IIO? If not, I can split this out into a
> > separate patch if needed.  
> A single ID addition is normally uncontroversial enough that we don't bother
> the DT maintainers (though of course they are welcome to look!).
> So this is fine.
> > 
> >  Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt | 1 +
> >  drivers/iio/imu/inv_mpu6050/inv_mpu_core.c                | 6 ++++++
> >  drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c                 | 5 +++++
> >  drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h                 | 2 ++
> >  4 files changed, 14 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt b/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
> > index 5f4777e8cc9e..b7def51c8ad9 100644
> > --- a/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
> > +++ b/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
> > @@ -6,6 +6,7 @@ Required properties:
> >   - compatible : should be one of
> >  		"invensense,mpu6050"
> >   		"invensense,mpu6500"
> > +		"invensense,mpu6515"
> >  		"invensense,mpu9150"
> >  		"invensense,mpu9250"
> >  		"invensense,mpu9255"
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> > index de68e83fc52d..12c1b9507007 100644
> > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> > @@ -103,6 +103,12 @@ static const struct inv_mpu6050_hw hw_info[] = {
> >  		.reg = &reg_set_6500,
> >  		.config = &chip_config_6050,
> >  	},
> > +	{
> > +		.whoami = INV_MPU6515_WHOAMI_VALUE,
> > +		.name = "MPU6515",
> > +		.reg = &reg_set_6500,
> > +		.config = &chip_config_6050,
> > +	},
> >  	{
> >  		.whoami = INV_MPU6000_WHOAMI_VALUE,
> >  		.name = "MPU6000",
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> > index 495409d56207..dd758e3d403d 100644
> > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> > @@ -174,6 +174,7 @@ static int inv_mpu_remove(struct i2c_client *client)
> >  static const struct i2c_device_id inv_mpu_id[] = {
> >  	{"mpu6050", INV_MPU6050},
> >  	{"mpu6500", INV_MPU6500},
> > +	{"mpu6515", INV_MPU6515},
> >  	{"mpu9150", INV_MPU9150},
> >  	{"mpu9250", INV_MPU9250},
> >  	{"mpu9255", INV_MPU9255},
> > @@ -192,6 +193,10 @@ static const struct of_device_id inv_of_match[] = {
> >  		.compatible = "invensense,mpu6500",
> >  		.data = (void *)INV_MPU6500
> >  	},
> > +	{
> > +		.compatible = "invensense,mpu6515",
> > +		.data = (void *)INV_MPU6515
> > +	},
> >  	{
> >  		.compatible = "invensense,mpu9150",
> >  		.data = (void *)INV_MPU9150
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> > index de8391693e17..e69a59659dbc 100644
> > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> > @@ -71,6 +71,7 @@ struct inv_mpu6050_reg_map {
> >  enum inv_devices {
> >  	INV_MPU6050,
> >  	INV_MPU6500,
> > +	INV_MPU6515,
> >  	INV_MPU6000,
> >  	INV_MPU9150,
> >  	INV_MPU9250,
> > @@ -256,6 +257,7 @@ struct inv_mpu6050_state {
> >  #define INV_MPU9150_WHOAMI_VALUE		0x68
> >  #define INV_MPU9250_WHOAMI_VALUE		0x71
> >  #define INV_MPU9255_WHOAMI_VALUE		0x73
> > +#define INV_MPU6515_WHOAMI_VALUE		0x74
> >  #define INV_ICM20608_WHOAMI_VALUE		0xAF
> >  
> >  /* scan element definition */  
> 
> 
> --
> 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 1/3] iio: imu: mpu6050: add support for 6515 variant

[Jonathan Cameron]

On Thu, 12 Jul 2018 15:47:38 +0100
Jonathan Cameron <jonathan.cameron@huawei.com> wrote:

> On Tue, 10 Jul 2018 21:09:30 -0400
> Brian Masney <masneyb@onstation.org> wrote:
> 
> > This patch adds support for the MPU 6515 variant. Confirmed that the
> > driver functions correctly on a LG Nexus 5 (hammerhead) phone.
> > 
> > Signed-off-by: Brian Masney <masneyb@onstation.org>
> > Signed-off-by: Jonathan Marek <jonathan@marek.ca>
Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.

thanks,

Jonathan

> > ---
> > This is a variation of Jonathan Marek's patch from postmarketOS
> > https://gitlab.com/postmarketOS/linux-postmarketos/commit/b8ad1ec1859c8bbcbce94944b3f4dd68f8f9fc37
> > with the following changes:
> > 
> > - The driver was previously throwing a 'whoami mismatch got XXX' error.
> >   Corrected ordering of hw_info variable to match the index of the
> >   inv_devices enum. The mpu6515 needed to come after mpu6500, not
> >   mpu6050.
> > - Removed regulator code. See next patch in this series.
> > 
> > I got confirmation from Jonathan M. that it was ok for me to submit his
> > patch upstream with these changes.
> > 
> > Jonathan C.: I assume that it is ok for the device tree binding
> > documentation to go through IIO? If not, I can split this out into a
> > separate patch if needed.  
> A single ID addition is normally uncontroversial enough that we don't bother
> the DT maintainers (though of course they are welcome to look!).
> So this is fine.
> > 
> >  Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt | 1 +
> >  drivers/iio/imu/inv_mpu6050/inv_mpu_core.c                | 6 ++++++
> >  drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c                 | 5 +++++
> >  drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h                 | 2 ++
> >  4 files changed, 14 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt b/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
> > index 5f4777e8cc9e..b7def51c8ad9 100644
> > --- a/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
> > +++ b/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt
> > @@ -6,6 +6,7 @@ Required properties:
> >   - compatible : should be one of
> >  		"invensense,mpu6050"
> >   		"invensense,mpu6500"
> > +		"invensense,mpu6515"
> >  		"invensense,mpu9150"
> >  		"invensense,mpu9250"
> >  		"invensense,mpu9255"
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> > index de68e83fc52d..12c1b9507007 100644
> > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> > @@ -103,6 +103,12 @@ static const struct inv_mpu6050_hw hw_info[] = {
> >  		.reg = &reg_set_6500,
> >  		.config = &chip_config_6050,
> >  	},
> > +	{
> > +		.whoami = INV_MPU6515_WHOAMI_VALUE,
> > +		.name = "MPU6515",
> > +		.reg = &reg_set_6500,
> > +		.config = &chip_config_6050,
> > +	},
> >  	{
> >  		.whoami = INV_MPU6000_WHOAMI_VALUE,
> >  		.name = "MPU6000",
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> > index 495409d56207..dd758e3d403d 100644
> > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> > @@ -174,6 +174,7 @@ static int inv_mpu_remove(struct i2c_client *client)
> >  static const struct i2c_device_id inv_mpu_id[] = {
> >  	{"mpu6050", INV_MPU6050},
> >  	{"mpu6500", INV_MPU6500},
> > +	{"mpu6515", INV_MPU6515},
> >  	{"mpu9150", INV_MPU9150},
> >  	{"mpu9250", INV_MPU9250},
> >  	{"mpu9255", INV_MPU9255},
> > @@ -192,6 +193,10 @@ static const struct of_device_id inv_of_match[] = {
> >  		.compatible = "invensense,mpu6500",
> >  		.data = (void *)INV_MPU6500
> >  	},
> > +	{
> > +		.compatible = "invensense,mpu6515",
> > +		.data = (void *)INV_MPU6515
> > +	},
> >  	{
> >  		.compatible = "invensense,mpu9150",
> >  		.data = (void *)INV_MPU9150
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> > index de8391693e17..e69a59659dbc 100644
> > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> > @@ -71,6 +71,7 @@ struct inv_mpu6050_reg_map {
> >  enum inv_devices {
> >  	INV_MPU6050,
> >  	INV_MPU6500,
> > +	INV_MPU6515,
> >  	INV_MPU6000,
> >  	INV_MPU9150,
> >  	INV_MPU9250,
> > @@ -256,6 +257,7 @@ struct inv_mpu6050_state {
> >  #define INV_MPU9150_WHOAMI_VALUE		0x68
> >  #define INV_MPU9250_WHOAMI_VALUE		0x71
> >  #define INV_MPU9255_WHOAMI_VALUE		0x73
> > +#define INV_MPU6515_WHOAMI_VALUE		0x74
> >  #define INV_ICM20608_WHOAMI_VALUE		0xAF
> >  
> >  /* scan element definition */  
> 
> 
> --
> 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

[PATCH 2/3] 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>
---
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. (See my previous patch in this series)
- 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    | 57 +++++++++++++++++--
 drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c     |  2 +-
 drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h     |  3 +
 drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c     |  9 +++
 5 files changed, 66 insertions(+), 6 deletions(-)

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..ec276b7bcc69 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,19 @@ 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 == 0) {
+		/* Give the device a little bit of time to start up. */
+		usleep_range(35000, 70000);
+	}
+
+	return result;
+}
+
 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,15 +1004,28 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
 		return -EINVAL;
 	}
 
+	st->vddio_supply = devm_regulator_get_optional(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;
+
 	/* power is turned on inside check chip type*/
 	result = inv_check_and_setup_chip(st);
 	if (result)
-		return result;
+		goto out_disable_regulator;
 
 	result = inv_mpu6050_init_config(indio_dev);
 	if (result) {
 		dev_err(dev, "Could not initialize device.\n");
-		return result;
+		goto out_disable_regulator;
 	}
 
 	if (inv_mpu_bus_setup)
@@ -1023,24 +1050,34 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
 						 NULL);
 	if (result) {
 		dev_err(dev, "configure buffer fail %d\n", result);
-		return result;
+		goto out_disable_regulator;
 	}
 	result = inv_mpu6050_probe_trigger(indio_dev, irq_type);
 	if (result) {
 		dev_err(dev, "trigger probe fail %d\n", result);
-		return result;
+		goto out_disable_regulator;
 	}
 
 	result = devm_iio_device_register(dev, indio_dev);
 	if (result) {
 		dev_err(dev, "IIO register fail %d\n", result);
-		return result;
+		goto out_disable_regulator;
 	}
 
 	return 0;
+
+out_disable_regulator:
+	regulator_disable(st->vddio_supply);
+	return result;
+
 }
 EXPORT_SYMBOL_GPL(inv_mpu_core_probe);
 
+int inv_mpu_core_remove(struct inv_mpu6050_state *st)
+{
+	return regulator_disable(st->vddio_supply);
+}
+
 #ifdef CONFIG_PM_SLEEP
 
 static int inv_mpu_resume(struct device *dev)
@@ -1050,6 +1087,11 @@ static int inv_mpu_resume(struct device *dev)
 
 	mutex_lock(&st->lock);
 	result = inv_mpu6050_set_power_itg(st, true);
+	if (result)
+		goto out_unlock;
+
+	result = inv_mpu_core_enable_regulator(st);
+out_unlock:
 	mutex_unlock(&st->lock);
 
 	return result;
@@ -1062,6 +1104,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 = regulator_disable(st->vddio_supply);
+out_unlock:
 	mutex_unlock(&st->lock);
 
 	return result;
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
index dd758e3d403d..ce227038c61c 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
@@ -164,7 +164,7 @@ static int inv_mpu_remove(struct i2c_client *client)
 		i2c_mux_del_adapters(st->muxc);
 	}
 
-	return 0;
+	return inv_mpu_core_remove(st);
 }
 
 /*
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
index e69a59659dbc..0289615a6135 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,7 @@ struct inv_mpu6050_state {
 	s64 chip_period;
 	s64 it_timestamp;
 	s64 data_timestamp;
+	struct regulator *vddio_supply;
 };
 
 /*register and associated bit definition*/
@@ -321,4 +323,5 @@ int inv_mpu_acpi_create_mux_client(struct i2c_client *client);
 void inv_mpu_acpi_delete_mux_client(struct i2c_client *client);
 int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
 		int (*inv_mpu_bus_setup)(struct iio_dev *), int chip_type);
+int inv_mpu_core_remove(struct inv_mpu6050_state *st);
 extern const struct dev_pm_ops inv_mpu_pmops;
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
index 227f50afff22..fce935f8f1e4 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
@@ -70,6 +70,14 @@ static int inv_mpu_probe(struct spi_device *spi)
 				  inv_mpu_i2c_disable, chip_type);
 }
 
+static int inv_mpu_remove(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+	struct inv_mpu6050_state *st = iio_priv(indio_dev);
+
+	return inv_mpu_core_remove(st);
+}
+
 /*
  * device id table is used to identify what device can be
  * supported by this driver
@@ -94,6 +102,7 @@ MODULE_DEVICE_TABLE(acpi, inv_acpi_match);
 
 static struct spi_driver inv_mpu_driver = {
 	.probe		=	inv_mpu_probe,
+	.remove		=	inv_mpu_remove,
 	.id_table	=	inv_mpu_id,
 	.driver = {
 		.acpi_match_table = ACPI_PTR(inv_acpi_match),
-- 
2.17.1

Re: [PATCH 2/3] iio: imu: mpu6050: add support for regulator framework

[kbuild test robot]

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

Hi Brian,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on iio/togreg]
[also build test ERROR on next-20180710]
[cannot apply to v4.18-rc4]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Brian-Masney/iio-imu-mpu6050-add-support-for-6515-variant/20180711-092023
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

>> ERROR: "inv_mpu_core_remove" [drivers/iio/imu/inv_mpu6050/inv-mpu6050-spi.ko] undefined!
>> ERROR: "inv_mpu_core_remove" [drivers/iio/imu/inv_mpu6050/inv-mpu6050-i2c.ko] undefined!

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 64645 bytes --]

Re: [PATCH 2/3] iio: imu: mpu6050: add support for regulator framework

[kbuild test robot]

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

Hi Brian,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on iio/togreg]
[also build test ERROR on next-20180710]
[cannot apply to v4.18-rc4]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Brian-Masney/iio-imu-mpu6050-add-support-for-6515-variant/20180711-092023
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

>> ERROR: "inv_mpu_core_remove" [drivers/iio/imu/inv_mpu6050/inv-mpu6050-spi.ko] undefined!
>> ERROR: "inv_mpu_core_remove" [drivers/iio/imu/inv_mpu6050/inv-mpu6050-i2c.ko] undefined!

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 64645 bytes --]

Re: [PATCH 2/3] iio: imu: mpu6050: add support for regulator framework

[Brian Masney]

On Wed, Jul 11, 2018 at 04:50:22PM +0800, kbuild test robot wrote:
> Hi Brian,
> 
> Thank you for the patch! Yet something to improve:
> 
> [auto build test ERROR on iio/togreg]
> [also build test ERROR on next-20180710]
> [cannot apply to v4.18-rc4]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> 
> url:    https://github.com/0day-ci/linux/commits/Brian-Masney/iio-imu-mpu6050-add-support-for-6515-variant/20180711-092023
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
> config: x86_64-allmodconfig (attached as .config)
> compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=x86_64 
> 
> All errors (new ones prefixed by >>):
> 
> >> ERROR: "inv_mpu_core_remove" [drivers/iio/imu/inv_mpu6050/inv-mpu6050-spi.ko] undefined!
> >> ERROR: "inv_mpu_core_remove" [drivers/iio/imu/inv_mpu6050/inv-mpu6050-i2c.ko] undefined!

This patch applies cleanly against the iio/testing branch.

https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/log/?h=testing

Brian

Re: [PATCH 2/3] iio: imu: mpu6050: add support for regulator framework

[Himanshu Jha]

Hi Brain,

On Tue, Jul 10, 2018 at 09:09:31PM -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>
> ---
>  }
>  EXPORT_SYMBOL_GPL(inv_mpu_core_probe);
>  
> +int inv_mpu_core_remove(struct inv_mpu6050_state *st)
> +{
> +	return regulator_disable(st->vddio_supply);
> +}
EXPORT_SYMBOL_GPL(inv_mpu_core_remove); ?

This is causing 0-day test build failure perhaps.

-- 
Himanshu Jha
Undergraduate Student
Department of Electronics & Communication
Guru Tegh Bahadur Institute of Technology

Re: [PATCH 2/3] iio: imu: mpu6050: add support for regulator framework

[Brian Masney]

On Wed, Jul 11, 2018 at 03:34:43PM +0530, Himanshu Jha wrote:
> Hi Brain,
> 
> On Tue, Jul 10, 2018 at 09:09:31PM -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>
> > ---
> >  }
> >  EXPORT_SYMBOL_GPL(inv_mpu_core_probe);
> >  
> > +int inv_mpu_core_remove(struct inv_mpu6050_state *st)
> > +{
> > +	return regulator_disable(st->vddio_supply);
> > +}
> EXPORT_SYMBOL_GPL(inv_mpu_core_remove); ?
> 
> This is causing 0-day test build failure perhaps.

You are right. It compiles if I have CONFIG_INV_MPU6050_IIO built into
the kernel, which is what I'm using to boot the board, but fails when
compiled as a module. Good catch 0-day!

The patch won't apply to Linus's current master branch and I assumed
that iio/togreg didn't have the required changes as well. Sorry about
the noise.

I'll send a v2 before the weekend.

Brian

Re: [PATCH 2/3] iio: imu: mpu6050: add support for regulator framework

[Jean-Baptiste Maneyrol]

Hello,

I really don't like the idea to have regulator handled inside the driver. I know this was done like that before for Nexus 5, but I think now this is something that can be done using dts only.
Does anyone know if there is a way with dts to handle regulator automatically and prevent the use in the driver? That would be a good idea to search how this handled for other drivers.

Anyway, you are enforcing regulator use in your code. It doesn't seem the code will work when there is no regulator declared in the dts, which is the case for the majority of configurations. We should at least make it optional.

Thanks for the contribution.
JB


From: Brian Masney <masneyb@onstation.org>
Sent: Wednesday, July 11, 2018 03:09
To: jic23@kernel.org; robh+dt@kernel.org; mark.rutland@arm.com; andy.gross@linaro.org; david.brown@linaro.org; linux-iio@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-msm@vger.kernel.org; linux-soc@vger.kernel.org
Cc: jonathan@marek.ca; Jean-Baptiste Maneyrol; knaack.h@gmx.de; lars@metafoo.de; pmeerw@pmeerw.net; mkelly@xevo.com; fischerdouglasc@gmail.com; bshah@kde.org; ctatlor97@gmail.com; masneyb@onstation.org
Subject: [PATCH 2/3] iio: imu: mpu6050: add support for regulator framework
  

CAUTION: This email originated from outside of the organization. Please make sure the sender is who they say they are and do not click links or open attachments unless you recognize the sender and know the content is safe.


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>
---
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. (See my previous patch in this series)
- 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    | 57 +++++++++++++++++--
 drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c     |  2 +-
 drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h     |  3 +
 drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c     |  9 +++
 5 files changed, 66 insertions(+), 6 deletions(-)

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..ec276b7bcc69 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,19 @@ 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 == 0) {
+               /* Give the device a little bit of time to start up. */
+               usleep_range(35000, 70000);
+       }
+
+       return result;
+}
+
 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,15 +1004,28 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
                return -EINVAL;
        }

+       st->vddio_supply = devm_regulator_get_optional(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;
+
        /* power is turned on inside check chip type*/
        result = inv_check_and_setup_chip(st);
        if (result)
-               return result;
+               goto out_disable_regulator;

        result = inv_mpu6050_init_config(indio_dev);
        if (result) {
                dev_err(dev, "Could not initialize device.\n");
-               return result;
+               goto out_disable_regulator;
        }

        if (inv_mpu_bus_setup)
@@ -1023,24 +1050,34 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
                                                 NULL);
        if (result) {
                dev_err(dev, "configure buffer fail %d\n", result);
-               return result;
+               goto out_disable_regulator;
        }
        result = inv_mpu6050_probe_trigger(indio_dev, irq_type);
        if (result) {
                dev_err(dev, "trigger probe fail %d\n", result);
-               return result;
+               goto out_disable_regulator;
        }

        result = devm_iio_device_register(dev, indio_dev);
        if (result) {
                dev_err(dev, "IIO register fail %d\n", result);
-               return result;
+               goto out_disable_regulator;
        }

        return 0;
+
+out_disable_regulator:
+       regulator_disable(st->vddio_supply);
+       return result;
+
 }
 EXPORT_SYMBOL_GPL(inv_mpu_core_probe);

+int inv_mpu_core_remove(struct inv_mpu6050_state *st)
+{
+       return regulator_disable(st->vddio_supply);
+}
+
 #ifdef CONFIG_PM_SLEEP

 static int inv_mpu_resume(struct device *dev)
@@ -1050,6 +1087,11 @@ static int inv_mpu_resume(struct device *dev)

        mutex_lock(&st->lock);
        result = inv_mpu6050_set_power_itg(st, true);
+       if (result)
+               goto out_unlock;
+
+       result = inv_mpu_core_enable_regulator(st);
+out_unlock:
        mutex_unlock(&st->lock);

        return result;
@@ -1062,6 +1104,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 = regulator_disable(st->vddio_supply);
+out_unlock:
        mutex_unlock(&st->lock);

        return result;
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
index dd758e3d403d..ce227038c61c 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
@@ -164,7 +164,7 @@ static int inv_mpu_remove(struct i2c_client *client)
                i2c_mux_del_adapters(st->muxc);
        }

-       return 0;
+       return inv_mpu_core_remove(st);
 }

 /*
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
index e69a59659dbc..0289615a6135 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,7 @@ struct inv_mpu6050_state {
        s64 chip_period;
        s64 it_timestamp;
        s64 data_timestamp;
+       struct regulator *vddio_supply;
 };

 /*register and associated bit definition*/
@@ -321,4 +323,5 @@ int inv_mpu_acpi_create_mux_client(struct i2c_client *client);
 void inv_mpu_acpi_delete_mux_client(struct i2c_client *client);
 int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
                int (*inv_mpu_bus_setup)(struct iio_dev *), int chip_type);
+int inv_mpu_core_remove(struct inv_mpu6050_state *st);
 extern const struct dev_pm_ops inv_mpu_pmops;
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
index 227f50afff22..fce935f8f1e4 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
@@ -70,6 +70,14 @@ static int inv_mpu_probe(struct spi_device *spi)
                                  inv_mpu_i2c_disable, chip_type);
 }

+static int inv_mpu_remove(struct spi_device *spi)
+{
+       struct iio_dev *indio_dev = spi_get_drvdata(spi);
+       struct inv_mpu6050_state *st = iio_priv(indio_dev);
+
+       return inv_mpu_core_remove(st);
+}
+
 /*
  * device id table is used to identify what device can be
  * supported by this driver
@@ -94,6 +102,7 @@ MODULE_DEVICE_TABLE(acpi, inv_acpi_match);

 static struct spi_driver inv_mpu_driver = {
        .probe          =       inv_mpu_probe,
+       .remove         =       inv_mpu_remove,
        .id_table       =       inv_mpu_id,
        .driver = {
                .acpi_match_table = ACPI_PTR(inv_acpi_match),
--
2.17.1

    

Re: [PATCH 2/3] iio: imu: mpu6050: add support for regulator framework

[Jean-Baptiste Maneyrol]

Hello,

I really don't like the idea to have regulator handled inside the driver. I=
 know this was done like that before for Nexus 5, but I think now this is s=
omething that can be done using dts only.
Does anyone know if there is a way with dts to handle regulator automatical=
ly and prevent the use in the driver? That would be a good idea to search h=
ow this handled for other drivers.

Anyway, you are enforcing regulator use in your code. It doesn't seem the c=
ode will work when there is no regulator declared in the dts, which is the =
case for the majority of configurations. We should at least make it optiona=
l.

Thanks for the contribution.
JB


From: Brian Masney <masneyb@onstation.org>
Sent: Wednesday, July 11, 2018 03:09
To: jic23@kernel.org; robh+dt@kernel.org; mark.rutland@arm.com; andy.gross@=
linaro.org; david.brown@linaro.org; linux-iio@vger.kernel.org; devicetree@v=
ger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-msm@vger.kernel.org=
; linux-soc@vger.kernel.org
Cc: jonathan@marek.ca; Jean-Baptiste Maneyrol; knaack.h@gmx.de; lars@metafo=
o.de; pmeerw@pmeerw.net; mkelly@xevo.com; fischerdouglasc@gmail.com; bshah@=
kde.org; ctatlor97@gmail.com; masneyb@onstation.org
Subject: [PATCH 2/3] iio: imu: mpu6050: add support for regulator framework
=A0=20

CAUTION: This email originated from outside of the organization. Please mak=
e sure the sender is who they say they are and do not click links or open a=
ttachments unless you recognize the sender and know the content is safe.


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>
---
This is a variation of Jonathan Marek's patch from postmarketOS
https://gitlab.com/postmarketOS/linux-postmarketos/commit/b8ad1ec1859c8bbcb=
ce94944b3f4dd68f8f9fc37
with the following changes:

- Stripped out 6515 variant code. (See my previous patch in this series)
- 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.

=A0.../bindings/iio/imu/inv_mpu6050.txt=A0=A0=A0=A0=A0=A0=A0=A0=A0 |=A0 1 +
=A0drivers/iio/imu/inv_mpu6050/inv_mpu_core.c=A0=A0=A0 | 57 +++++++++++++++=
++--
=A0drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c=A0=A0=A0=A0 |=A0 2 +-
=A0drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h=A0=A0=A0=A0 |=A0 3 +
=A0drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c=A0=A0=A0=A0 |=A0 9 +++
=A05 files changed, 66 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/imu/inv_mpu6050.txt b/Do=
cumentation/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:
=A0=A0 bindings.

=A0Optional properties:
+ - vddio-supply: regulator phandle for VDDIO supply
=A0 - mount-matrix: an optional 3x3 mounting rotation matrix
=A0 - i2c-gate node.=A0 These devices also support an auxiliary i2c bus.=A0=
 This is
=A0=A0=A0 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/i=
nv_mpu6050/inv_mpu_core.c
index 12c1b9507007..ec276b7bcc69 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 @@
=A0#include <linux/iio/iio.h>
=A0#include <linux/acpi.h>
=A0#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
=A0#include "inv_mpu_iio.h"

=A0/*
@@ -926,6 +927,19 @@ static int inv_check_and_setup_chip(struct inv_mpu6050=
_state *st)
=A0=A0=A0=A0=A0=A0=A0 return result;
=A0}

+static int inv_mpu_core_enable_regulator(struct inv_mpu6050_state *st)
+{
+=A0=A0=A0=A0=A0=A0 int result;
+
+=A0=A0=A0=A0=A0=A0 result =3D regulator_enable(st->vddio_supply);
+=A0=A0=A0=A0=A0=A0 if (result =3D=3D 0) {
+=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 /* Give the device a little bit=
 of time to start up. */
+=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 usleep_range(35000, 70000);
+=A0=A0=A0=A0=A0=A0 }
+
+=A0=A0=A0=A0=A0=A0 return result;
+}
+
=A0int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 int (*inv_mpu_bus_setup)(stru=
ct iio_dev *), int chip_type)
=A0{
@@ -990,15 +1004,28 @@ int inv_mpu_core_probe(struct regmap *regmap, int ir=
q, const char *name,
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 return -EINVAL;
=A0=A0=A0=A0=A0=A0=A0 }

+=A0=A0=A0=A0=A0=A0 st->vddio_supply =3D devm_regulator_get_optional(dev, "=
vddio");
+=A0=A0=A0=A0=A0=A0 if (IS_ERR(st->vddio_supply)) {
+=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 if (PTR_ERR(st->vddio_supply) !=
=3D -EPROBE_DEFER)
+=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 dev_err=
(dev, "Failed to get vddio regulator %d\n",
+=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=
=A0=A0=A0=A0=A0=A0 (int)PTR_ERR(st->vddio_supply));
+
+=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 return PTR_ERR(st->vddio_supply=
);
+=A0=A0=A0=A0=A0=A0 }
+
+=A0=A0=A0=A0=A0=A0 result =3D inv_mpu_core_enable_regulator(st);
+=A0=A0=A0=A0=A0=A0 if (result)
+=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 return result;
+
=A0=A0=A0=A0=A0=A0=A0 /* power is turned on inside check chip type*/
=A0=A0=A0=A0=A0=A0=A0 result =3D inv_check_and_setup_chip(st);
=A0=A0=A0=A0=A0=A0=A0 if (result)
-=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 return result;
+=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 goto out_disable_regulator;

=A0=A0=A0=A0=A0=A0=A0 result =3D inv_mpu6050_init_config(indio_dev);
=A0=A0=A0=A0=A0=A0=A0 if (result) {
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 dev_err(dev, "Could not initi=
alize device.\n");
-=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 return result;
+=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 goto out_disable_regulator;
=A0=A0=A0=A0=A0=A0=A0 }

=A0=A0=A0=A0=A0=A0=A0 if (inv_mpu_bus_setup)
@@ -1023,24 +1050,34 @@ int inv_mpu_core_probe(struct regmap *regmap, int i=
rq, const char *name,
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 NULL)=
;
=A0=A0=A0=A0=A0=A0=A0 if (result) {
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 dev_err(dev, "configure buffe=
r fail %d\n", result);
-=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 return result;
+=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 goto out_disable_regulator;
=A0=A0=A0=A0=A0=A0=A0 }
=A0=A0=A0=A0=A0=A0=A0 result =3D inv_mpu6050_probe_trigger(indio_dev, irq_t=
ype);
=A0=A0=A0=A0=A0=A0=A0 if (result) {
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 dev_err(dev, "trigger probe f=
ail %d\n", result);
-=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 return result;
+=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 goto out_disable_regulator;
=A0=A0=A0=A0=A0=A0=A0 }

=A0=A0=A0=A0=A0=A0=A0 result =3D devm_iio_device_register(dev, indio_dev);
=A0=A0=A0=A0=A0=A0=A0 if (result) {
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 dev_err(dev, "IIO register fa=
il %d\n", result);
-=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 return result;
+=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 goto out_disable_regulator;
=A0=A0=A0=A0=A0=A0=A0 }

=A0=A0=A0=A0=A0=A0=A0 return 0;
+
+out_disable_regulator:
+=A0=A0=A0=A0=A0=A0 regulator_disable(st->vddio_supply);
+=A0=A0=A0=A0=A0=A0 return result;
+
=A0}
=A0EXPORT_SYMBOL_GPL(inv_mpu_core_probe);

+int inv_mpu_core_remove(struct inv_mpu6050_state *st)
+{
+=A0=A0=A0=A0=A0=A0 return regulator_disable(st->vddio_supply);
+}
+
=A0#ifdef CONFIG_PM_SLEEP

=A0static int inv_mpu_resume(struct device *dev)
@@ -1050,6 +1087,11 @@ static int inv_mpu_resume(struct device *dev)

=A0=A0=A0=A0=A0=A0=A0 mutex_lock(&st->lock);
=A0=A0=A0=A0=A0=A0=A0 result =3D inv_mpu6050_set_power_itg(st, true);
+=A0=A0=A0=A0=A0=A0 if (result)
+=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 goto out_unlock;
+
+=A0=A0=A0=A0=A0=A0 result =3D inv_mpu_core_enable_regulator(st);
+out_unlock:
=A0=A0=A0=A0=A0=A0=A0 mutex_unlock(&st->lock);

=A0=A0=A0=A0=A0=A0=A0 return result;
@@ -1062,6 +1104,11 @@ static int inv_mpu_suspend(struct device *dev)

=A0=A0=A0=A0=A0=A0=A0 mutex_lock(&st->lock);
=A0=A0=A0=A0=A0=A0=A0 result =3D inv_mpu6050_set_power_itg(st, false);
+=A0=A0=A0=A0=A0=A0 if (result)
+=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 goto out_unlock;
+
+=A0=A0=A0=A0=A0=A0 result =3D regulator_disable(st->vddio_supply);
+out_unlock:
=A0=A0=A0=A0=A0=A0=A0 mutex_unlock(&st->lock);

=A0=A0=A0=A0=A0=A0=A0 return result;
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/in=
v_mpu6050/inv_mpu_i2c.c
index dd758e3d403d..ce227038c61c 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
@@ -164,7 +164,7 @@ static int inv_mpu_remove(struct i2c_client *client)
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 i2c_mux_del_adapters(st->muxc=
);
=A0=A0=A0=A0=A0=A0=A0 }

-=A0=A0=A0=A0=A0=A0 return 0;
+=A0=A0=A0=A0=A0=A0 return inv_mpu_core_remove(st);
=A0}

=A0/*
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/in=
v_mpu6050/inv_mpu_iio.h
index e69a59659dbc..0289615a6135 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 {
=A0 *=A0 @chip_period:=A0=A0=A0=A0=A0 chip internal period estimation (~1kH=
z).
=A0 *=A0 @it_timestamp:=A0=A0=A0=A0 timestamp from previous interrupt.
=A0 *=A0 @data_timestamp:=A0=A0 timestamp for next data sample.
+ *=A0 @vddio_supply=A0=A0=A0=A0=A0 voltage regulator for the chip.
=A0 */
=A0struct inv_mpu6050_state {
=A0=A0=A0=A0=A0=A0=A0 struct mutex lock;
@@ -149,6 +150,7 @@ struct inv_mpu6050_state {
=A0=A0=A0=A0=A0=A0=A0 s64 chip_period;
=A0=A0=A0=A0=A0=A0=A0 s64 it_timestamp;
=A0=A0=A0=A0=A0=A0=A0 s64 data_timestamp;
+=A0=A0=A0=A0=A0=A0 struct regulator *vddio_supply;
=A0};

=A0/*register and associated bit definition*/
@@ -321,4 +323,5 @@ int inv_mpu_acpi_create_mux_client(struct i2c_client *c=
lient);
=A0void inv_mpu_acpi_delete_mux_client(struct i2c_client *client);
=A0int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 int (*inv_mpu_bus_setup)(stru=
ct iio_dev *), int chip_type);
+int inv_mpu_core_remove(struct inv_mpu6050_state *st);
=A0extern const struct dev_pm_ops inv_mpu_pmops;
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c b/drivers/iio/imu/in=
v_mpu6050/inv_mpu_spi.c
index 227f50afff22..fce935f8f1e4 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
@@ -70,6 +70,14 @@ static int inv_mpu_probe(struct spi_device *spi)
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=
=A0=A0=A0=A0=A0=A0=A0=A0 inv_mpu_i2c_disable, chip_type);
=A0}

+static int inv_mpu_remove(struct spi_device *spi)
+{
+=A0=A0=A0=A0=A0=A0 struct iio_dev *indio_dev =3D spi_get_drvdata(spi);
+=A0=A0=A0=A0=A0=A0 struct inv_mpu6050_state *st =3D iio_priv(indio_dev);
+
+=A0=A0=A0=A0=A0=A0 return inv_mpu_core_remove(st);
+}
+
=A0/*
=A0 * device id table is used to identify what device can be
=A0 * supported by this driver
@@ -94,6 +102,7 @@ MODULE_DEVICE_TABLE(acpi, inv_acpi_match);

=A0static struct spi_driver inv_mpu_driver =3D {
=A0=A0=A0=A0=A0=A0=A0 .probe=A0=A0=A0=A0=A0=A0=A0=A0=A0 =3D=A0=A0=A0=A0=A0=
=A0 inv_mpu_probe,
+=A0=A0=A0=A0=A0=A0 .remove=A0=A0=A0=A0=A0=A0=A0=A0 =3D=A0=A0=A0=A0=A0=A0 i=
nv_mpu_remove,
=A0=A0=A0=A0=A0=A0=A0 .id_table=A0=A0=A0=A0=A0=A0 =3D=A0=A0=A0=A0=A0=A0 inv=
_mpu_id,
=A0=A0=A0=A0=A0=A0=A0 .driver =3D {
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 .acpi_match_table =3D ACPI_PT=
R(inv_acpi_match),
--
2.17.1

    =

Re: [PATCH 2/3] iio: imu: mpu6050: add support for regulator framework

[Brian Masney]

On Wed, Jul 11, 2018 at 12:29:33PM +0000, Jean-Baptiste Maneyrol wrote:
> Hello,
> 
> I really don't like the idea to have regulator handled inside the driver. I know this was done like that before for Nexus 5, but I think now this is something that can be done using dts only.
> Does anyone know if there is a way with dts to handle regulator automatically and prevent the use in the driver? That would be a good idea to search how this handled for other drivers.
> 
> Anyway, you are enforcing regulator use in your code. It doesn't seem the code will work when there is no regulator declared in the dts, which is the case for the majority of configurations. We should at least make it optional.
> 
> Thanks for the contribution.

My understanding of devm_regulator_get_optional() is that a stub
regulator will be returned if one is not configured. See the comment
above regulator_get_optional() for where I got this information.

https://elixir.bootlin.com/linux/v4.17/source/drivers/regulator/core.c#L1750

I agree that it would be better if this could be handled exclusively in
dts if possible, although I'm not sure how. I looked at other drivers
and commits from the last year or so and this appears to be the current
recommended approach. I'm open to other approaches to doing this.

Brian


>
> 
> From: Brian Masney <masneyb@onstation.org>
> Sent: Wednesday, July 11, 2018 03:09
> To: jic23@kernel.org; robh+dt@kernel.org; mark.rutland@arm.com; andy.gross@linaro.org; david.brown@linaro.org; linux-iio@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-msm@vger.kernel.org; linux-soc@vger.kernel.org
> Cc: jonathan@marek.ca; Jean-Baptiste Maneyrol; knaack.h@gmx.de; lars@metafoo.de; pmeerw@pmeerw.net; mkelly@xevo.com; fischerdouglasc@gmail.com; bshah@kde.org; ctatlor97@gmail.com; masneyb@onstation.org
> Subject: [PATCH 2/3] iio: imu: mpu6050: add support for regulator framework
>   
> 
> CAUTION: This email originated from outside of the organization. Please make sure the sender is who they say they are and do not click links or open attachments unless you recognize the sender and know the content is safe.
> 
> 
> 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>
> ---
> 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. (See my previous patch in this series)
> - 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    | 57 +++++++++++++++++--
>  drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c     |  2 +-
>  drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h     |  3 +
>  drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c     |  9 +++
>  5 files changed, 66 insertions(+), 6 deletions(-)
> 
> 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..ec276b7bcc69 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,19 @@ 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 == 0) {
> +               /* Give the device a little bit of time to start up. */
> +               usleep_range(35000, 70000);
> +       }
> +
> +       return result;
> +}
> +
>  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,15 +1004,28 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
>                 return -EINVAL;
>         }
> 
> +       st->vddio_supply = devm_regulator_get_optional(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;
> +
>         /* power is turned on inside check chip type*/
>         result = inv_check_and_setup_chip(st);
>         if (result)
> -               return result;
> +               goto out_disable_regulator;
> 
>         result = inv_mpu6050_init_config(indio_dev);
>         if (result) {
>                 dev_err(dev, "Could not initialize device.\n");
> -               return result;
> +               goto out_disable_regulator;
>         }
> 
>         if (inv_mpu_bus_setup)
> @@ -1023,24 +1050,34 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
>                                                  NULL);
>         if (result) {
>                 dev_err(dev, "configure buffer fail %d\n", result);
> -               return result;
> +               goto out_disable_regulator;
>         }
>         result = inv_mpu6050_probe_trigger(indio_dev, irq_type);
>         if (result) {
>                 dev_err(dev, "trigger probe fail %d\n", result);
> -               return result;
> +               goto out_disable_regulator;
>         }
> 
>         result = devm_iio_device_register(dev, indio_dev);
>         if (result) {
>                 dev_err(dev, "IIO register fail %d\n", result);
> -               return result;
> +               goto out_disable_regulator;
>         }
> 
>         return 0;
> +
> +out_disable_regulator:
> +       regulator_disable(st->vddio_supply);
> +       return result;
> +
>  }
>  EXPORT_SYMBOL_GPL(inv_mpu_core_probe);
> 
> +int inv_mpu_core_remove(struct inv_mpu6050_state *st)
> +{
> +       return regulator_disable(st->vddio_supply);
> +}
> +
>  #ifdef CONFIG_PM_SLEEP
> 
>  static int inv_mpu_resume(struct device *dev)
> @@ -1050,6 +1087,11 @@ static int inv_mpu_resume(struct device *dev)
> 
>         mutex_lock(&st->lock);
>         result = inv_mpu6050_set_power_itg(st, true);
> +       if (result)
> +               goto out_unlock;
> +
> +       result = inv_mpu_core_enable_regulator(st);
> +out_unlock:
>         mutex_unlock(&st->lock);
> 
>         return result;
> @@ -1062,6 +1104,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 = regulator_disable(st->vddio_supply);
> +out_unlock:
>         mutex_unlock(&st->lock);
> 
>         return result;
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> index dd758e3d403d..ce227038c61c 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> @@ -164,7 +164,7 @@ static int inv_mpu_remove(struct i2c_client *client)
>                 i2c_mux_del_adapters(st->muxc);
>         }
> 
> -       return 0;
> +       return inv_mpu_core_remove(st);
>  }
> 
>  /*
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> index e69a59659dbc..0289615a6135 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,7 @@ struct inv_mpu6050_state {
>         s64 chip_period;
>         s64 it_timestamp;
>         s64 data_timestamp;
> +       struct regulator *vddio_supply;
>  };
> 
>  /*register and associated bit definition*/
> @@ -321,4 +323,5 @@ int inv_mpu_acpi_create_mux_client(struct i2c_client *client);
>  void inv_mpu_acpi_delete_mux_client(struct i2c_client *client);
>  int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
>                 int (*inv_mpu_bus_setup)(struct iio_dev *), int chip_type);
> +int inv_mpu_core_remove(struct inv_mpu6050_state *st);
>  extern const struct dev_pm_ops inv_mpu_pmops;
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
> index 227f50afff22..fce935f8f1e4 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
> @@ -70,6 +70,14 @@ static int inv_mpu_probe(struct spi_device *spi)
>                                   inv_mpu_i2c_disable, chip_type);
>  }
> 
> +static int inv_mpu_remove(struct spi_device *spi)
> +{
> +       struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +       struct inv_mpu6050_state *st = iio_priv(indio_dev);
> +
> +       return inv_mpu_core_remove(st);
> +}
> +
>  /*
>   * device id table is used to identify what device can be
>   * supported by this driver
> @@ -94,6 +102,7 @@ MODULE_DEVICE_TABLE(acpi, inv_acpi_match);
> 
>  static struct spi_driver inv_mpu_driver = {
>         .probe          =       inv_mpu_probe,
> +       .remove         =       inv_mpu_remove,
>         .id_table       =       inv_mpu_id,
>         .driver = {
>                 .acpi_match_table = ACPI_PTR(inv_acpi_match),
> --
> 2.17.1
> 
>     

Re: [PATCH 2/3] iio: imu: mpu6050: add support for regulator framework

[Brian Masney]

On Wed, Jul 11, 2018 at 12:29:33PM +0000, Jean-Baptiste Maneyrol wrote:
> Hello,
> 
> I really don't like the idea to have regulator handled inside the driver. I know this was done like that before for Nexus 5, but I think now this is something that can be done using dts only.
> Does anyone know if there is a way with dts to handle regulator automatically and prevent the use in the driver? That would be a good idea to search how this handled for other drivers.
> 
> Anyway, you are enforcing regulator use in your code. It doesn't seem the code will work when there is no regulator declared in the dts, which is the case for the majority of configurations. We should at least make it optional.
> 
> Thanks for the contribution.

My understanding of devm_regulator_get_optional() is that a stub
regulator will be returned if one is not configured. See the comment
above regulator_get_optional() for where I got this information.

https://elixir.bootlin.com/linux/v4.17/source/drivers/regulator/core.c#L1750

I agree that it would be better if this could be handled exclusively in
dts if possible, although I'm not sure how. I looked at other drivers
and commits from the last year or so and this appears to be the current
recommended approach. I'm open to other approaches to doing this.

Brian


>
> 
> From: Brian Masney <masneyb@onstation.org>
> Sent: Wednesday, July 11, 2018 03:09
> To: jic23@kernel.org; robh+dt@kernel.org; mark.rutland@arm.com; andy.gross@linaro.org; david.brown@linaro.org; linux-iio@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-msm@vger.kernel.org; linux-soc@vger.kernel.org
> Cc: jonathan@marek.ca; Jean-Baptiste Maneyrol; knaack.h@gmx.de; lars@metafoo.de; pmeerw@pmeerw.net; mkelly@xevo.com; fischerdouglasc@gmail.com; bshah@kde.org; ctatlor97@gmail.com; masneyb@onstation.org
> Subject: [PATCH 2/3] iio: imu: mpu6050: add support for regulator framework
>   
> 
> CAUTION: This email originated from outside of the organization. Please make sure the sender is who they say they are and do not click links or open attachments unless you recognize the sender and know the content is safe.
> 
> 
> 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>
> ---
> 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. (See my previous patch in this series)
> - 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    | 57 +++++++++++++++++--
>  drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c     |  2 +-
>  drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h     |  3 +
>  drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c     |  9 +++
>  5 files changed, 66 insertions(+), 6 deletions(-)
> 
> 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..ec276b7bcc69 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,19 @@ 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 == 0) {
> +               /* Give the device a little bit of time to start up. */
> +               usleep_range(35000, 70000);
> +       }
> +
> +       return result;
> +}
> +
>  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,15 +1004,28 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
>                 return -EINVAL;
>         }
> 
> +       st->vddio_supply = devm_regulator_get_optional(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;
> +
>         /* power is turned on inside check chip type*/
>         result = inv_check_and_setup_chip(st);
>         if (result)
> -               return result;
> +               goto out_disable_regulator;
> 
>         result = inv_mpu6050_init_config(indio_dev);
>         if (result) {
>                 dev_err(dev, "Could not initialize device.\n");
> -               return result;
> +               goto out_disable_regulator;
>         }
> 
>         if (inv_mpu_bus_setup)
> @@ -1023,24 +1050,34 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
>                                                  NULL);
>         if (result) {
>                 dev_err(dev, "configure buffer fail %d\n", result);
> -               return result;
> +               goto out_disable_regulator;
>         }
>         result = inv_mpu6050_probe_trigger(indio_dev, irq_type);
>         if (result) {
>                 dev_err(dev, "trigger probe fail %d\n", result);
> -               return result;
> +               goto out_disable_regulator;
>         }
> 
>         result = devm_iio_device_register(dev, indio_dev);
>         if (result) {
>                 dev_err(dev, "IIO register fail %d\n", result);
> -               return result;
> +               goto out_disable_regulator;
>         }
> 
>         return 0;
> +
> +out_disable_regulator:
> +       regulator_disable(st->vddio_supply);
> +       return result;
> +
>  }
>  EXPORT_SYMBOL_GPL(inv_mpu_core_probe);
> 
> +int inv_mpu_core_remove(struct inv_mpu6050_state *st)
> +{
> +       return regulator_disable(st->vddio_supply);
> +}
> +
>  #ifdef CONFIG_PM_SLEEP
> 
>  static int inv_mpu_resume(struct device *dev)
> @@ -1050,6 +1087,11 @@ static int inv_mpu_resume(struct device *dev)
> 
>         mutex_lock(&st->lock);
>         result = inv_mpu6050_set_power_itg(st, true);
> +       if (result)
> +               goto out_unlock;
> +
> +       result = inv_mpu_core_enable_regulator(st);
> +out_unlock:
>         mutex_unlock(&st->lock);
> 
>         return result;
> @@ -1062,6 +1104,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 = regulator_disable(st->vddio_supply);
> +out_unlock:
>         mutex_unlock(&st->lock);
> 
>         return result;
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> index dd758e3d403d..ce227038c61c 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> @@ -164,7 +164,7 @@ static int inv_mpu_remove(struct i2c_client *client)
>                 i2c_mux_del_adapters(st->muxc);
>         }
> 
> -       return 0;
> +       return inv_mpu_core_remove(st);
>  }
> 
>  /*
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> index e69a59659dbc..0289615a6135 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,7 @@ struct inv_mpu6050_state {
>         s64 chip_period;
>         s64 it_timestamp;
>         s64 data_timestamp;
> +       struct regulator *vddio_supply;
>  };
> 
>  /*register and associated bit definition*/
> @@ -321,4 +323,5 @@ int inv_mpu_acpi_create_mux_client(struct i2c_client *client);
>  void inv_mpu_acpi_delete_mux_client(struct i2c_client *client);
>  int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
>                 int (*inv_mpu_bus_setup)(struct iio_dev *), int chip_type);
> +int inv_mpu_core_remove(struct inv_mpu6050_state *st);
>  extern const struct dev_pm_ops inv_mpu_pmops;
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
> index 227f50afff22..fce935f8f1e4 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
> @@ -70,6 +70,14 @@ static int inv_mpu_probe(struct spi_device *spi)
>                                   inv_mpu_i2c_disable, chip_type);
>  }
> 
> +static int inv_mpu_remove(struct spi_device *spi)
> +{
> +       struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +       struct inv_mpu6050_state *st = iio_priv(indio_dev);
> +
> +       return inv_mpu_core_remove(st);
> +}
> +
>  /*
>   * device id table is used to identify what device can be
>   * supported by this driver
> @@ -94,6 +102,7 @@ MODULE_DEVICE_TABLE(acpi, inv_acpi_match);
> 
>  static struct spi_driver inv_mpu_driver = {
>         .probe          =       inv_mpu_probe,
> +       .remove         =       inv_mpu_remove,
>         .id_table       =       inv_mpu_id,
>         .driver = {
>                 .acpi_match_table = ACPI_PTR(inv_acpi_match),
> --
> 2.17.1
> 
>     

Re: [PATCH 2/3] iio: imu: mpu6050: add support for regulator framework

[Jonathan Cameron]

On Wed, 11 Jul 2018 09:00:07 -0400
Brian Masney <masneyb@onstation.org> wrote:

> On Wed, Jul 11, 2018 at 12:29:33PM +0000, Jean-Baptiste Maneyrol wrote:
> > Hello,
> >
 
> > I really don't like the idea to have regulator handled inside the
> > driver. I know this was done like that before for Nexus 5, but I
> > think now this is something that can be done using dts only. Does
> > anyone know if there is a way with dts to handle regulator
> > automatically and prevent the use in the driver? That would be a
> > good idea to search how this handled for other drivers.
> > 
> > Anyway, you are enforcing regulator use in your code. It doesn't
> > seem the code will work when there is no regulator declared in the
> > dts, which is the case for the majority of configurations. We
> > should at least make it optional.
> > 
> > Thanks for the contribution.  
> 

> My understanding of devm_regulator_get_optional() is that a stub
> regulator will be returned if one is not configured. See the comment
> above regulator_get_optional() for where I got this information.
> 
> https://elixir.bootlin.com/linux/v4.17/source/drivers/regulator/core.c#L1750

Quite the opposite (though the text is perhaps less than crystal clear)
 - reread that comment really carefully. 

 * This is intended for use by consumers for devices which can have
 * some supplies unconnected in normal use, such as some MMC devices.
 * It can allow the regulator core to provide stub supplies for other
 * supplies requested using normal regulator_get() calls without
 * disrupting the operation of drivers that can handle absent
 * supplies.

So in it allows normal path (non "optional") one to return stubs, but when
optional versions are used, it will return an error if they aren't there
- thus letting the driver decide what to do about it.


> 
> I agree that it would be better if this could be handled exclusively in
> dts if possible, although I'm not sure how. I looked at other drivers
> and commits from the last year or so and this appears to be the current
> recommended approach. I'm open to other approaches to doing this.

In the general case it is very hard to do. Many devices have fairly complex
regulator setups and need to enable them in particular orders and can
power down subsets etc.

The case here of just providing a power supply is very simplistic and
it may well be that down the line we want to do finer grained control
in the driver.  Only a driver can know what the effect of turning
a particular power supply to a device off actually is.

Jonathan
 
> 
> Brian
> 
> 
> >
> > 
> > From: Brian Masney <masneyb@onstation.org>
> > Sent: Wednesday, July 11, 2018 03:09
> > To: jic23@kernel.org; robh+dt@kernel.org; mark.rutland@arm.com; andy.gross@linaro.org; david.brown@linaro.org; linux-iio@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-msm@vger.kernel.org; linux-soc@vger.kernel.org
> > Cc: jonathan@marek.ca; Jean-Baptiste Maneyrol; knaack.h@gmx.de; lars@metafoo.de; pmeerw@pmeerw.net; mkelly@xevo.com; fischerdouglasc@gmail.com; bshah@kde.org; ctatlor97@gmail.com; masneyb@onstation.org
> > Subject: [PATCH 2/3] iio: imu: mpu6050: add support for regulator framework
> >   
> > 
> > CAUTION: This email originated from outside of the organization. Please make sure the sender is who they say they are and do not click links or open attachments unless you recognize the sender and know the content is safe.
> > 
> > 
> > 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>
> > ---
> > 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. (See my previous patch in this series)
> > - 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    | 57 +++++++++++++++++--
> >  drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c     |  2 +-
> >  drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h     |  3 +
> >  drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c     |  9 +++
> >  5 files changed, 66 insertions(+), 6 deletions(-)
> > 
> > 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..ec276b7bcc69 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,19 @@ 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 == 0) {
> > +               /* Give the device a little bit of time to start up. */
> > +               usleep_range(35000, 70000);
> > +       }
> > +
> > +       return result;
> > +}
> > +
> >  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,15 +1004,28 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
> >                 return -EINVAL;
> >         }
> > 
> > +       st->vddio_supply = devm_regulator_get_optional(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;
> > +
> >         /* power is turned on inside check chip type*/
> >         result = inv_check_and_setup_chip(st);
> >         if (result)
> > -               return result;
> > +               goto out_disable_regulator;
> > 
> >         result = inv_mpu6050_init_config(indio_dev);
> >         if (result) {
> >                 dev_err(dev, "Could not initialize device.\n");
> > -               return result;
> > +               goto out_disable_regulator;
> >         }
> > 
> >         if (inv_mpu_bus_setup)
> > @@ -1023,24 +1050,34 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
> >                                                  NULL);
> >         if (result) {
> >                 dev_err(dev, "configure buffer fail %d\n", result);
> > -               return result;
> > +               goto out_disable_regulator;
> >         }
> >         result = inv_mpu6050_probe_trigger(indio_dev, irq_type);
> >         if (result) {
> >                 dev_err(dev, "trigger probe fail %d\n", result);
> > -               return result;
> > +               goto out_disable_regulator;
> >         }
> > 
> >         result = devm_iio_device_register(dev, indio_dev);
> >         if (result) {
> >                 dev_err(dev, "IIO register fail %d\n", result);
> > -               return result;
> > +               goto out_disable_regulator;
> >         }
> > 
> >         return 0;
> > +
> > +out_disable_regulator:
> > +       regulator_disable(st->vddio_supply);
> > +       return result;
> > +
> >  }
> >  EXPORT_SYMBOL_GPL(inv_mpu_core_probe);
> > 
> > +int inv_mpu_core_remove(struct inv_mpu6050_state *st)
> > +{
> > +       return regulator_disable(st->vddio_supply);
> > +}
> > +
> >  #ifdef CONFIG_PM_SLEEP
> > 
> >  static int inv_mpu_resume(struct device *dev)
> > @@ -1050,6 +1087,11 @@ static int inv_mpu_resume(struct device *dev)
> > 
> >         mutex_lock(&st->lock);
> >         result = inv_mpu6050_set_power_itg(st, true);
> > +       if (result)
> > +               goto out_unlock;
> > +
> > +       result = inv_mpu_core_enable_regulator(st);
> > +out_unlock:
> >         mutex_unlock(&st->lock);
> > 
> >         return result;
> > @@ -1062,6 +1104,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 = regulator_disable(st->vddio_supply);
> > +out_unlock:
> >         mutex_unlock(&st->lock);
> > 
> >         return result;
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> > index dd758e3d403d..ce227038c61c 100644
> > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> > @@ -164,7 +164,7 @@ static int inv_mpu_remove(struct i2c_client *client)
> >                 i2c_mux_del_adapters(st->muxc);
> >         }
> > 
> > -       return 0;
> > +       return inv_mpu_core_remove(st);
> >  }
> > 
> >  /*
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> > index e69a59659dbc..0289615a6135 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,7 @@ struct inv_mpu6050_state {
> >         s64 chip_period;
> >         s64 it_timestamp;
> >         s64 data_timestamp;
> > +       struct regulator *vddio_supply;
> >  };
> > 
> >  /*register and associated bit definition*/
> > @@ -321,4 +323,5 @@ int inv_mpu_acpi_create_mux_client(struct i2c_client *client);
> >  void inv_mpu_acpi_delete_mux_client(struct i2c_client *client);
> >  int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
> >                 int (*inv_mpu_bus_setup)(struct iio_dev *), int chip_type);
> > +int inv_mpu_core_remove(struct inv_mpu6050_state *st);
> >  extern const struct dev_pm_ops inv_mpu_pmops;
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
> > index 227f50afff22..fce935f8f1e4 100644
> > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
> > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
> > @@ -70,6 +70,14 @@ static int inv_mpu_probe(struct spi_device *spi)
> >                                   inv_mpu_i2c_disable, chip_type);
> >  }
> > 
> > +static int inv_mpu_remove(struct spi_device *spi)
> > +{
> > +       struct iio_dev *indio_dev = spi_get_drvdata(spi);
> > +       struct inv_mpu6050_state *st = iio_priv(indio_dev);
> > +
> > +       return inv_mpu_core_remove(st);
> > +}
> > +
> >  /*
> >   * device id table is used to identify what device can be
> >   * supported by this driver
> > @@ -94,6 +102,7 @@ MODULE_DEVICE_TABLE(acpi, inv_acpi_match);
> > 
> >  static struct spi_driver inv_mpu_driver = {
> >         .probe          =       inv_mpu_probe,
> > +       .remove         =       inv_mpu_remove,
> >         .id_table       =       inv_mpu_id,
> >         .driver = {
> >                 .acpi_match_table = ACPI_PTR(inv_acpi_match),
> > --
> > 2.17.1
> > 
> >       
> --
> 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 2/3] iio: imu: mpu6050: add support for regulator framework

[Jonathan Cameron]

On Wed, 11 Jul 2018 09:00:07 -0400
Brian Masney <masneyb@onstation.org> wrote:

> On Wed, Jul 11, 2018 at 12:29:33PM +0000, Jean-Baptiste Maneyrol wrote:
> > Hello,
> >
 
> > I really don't like the idea to have regulator handled inside the
> > driver. I know this was done like that before for Nexus 5, but I
> > think now this is something that can be done using dts only. Does
> > anyone know if there is a way with dts to handle regulator
> > automatically and prevent the use in the driver? That would be a
> > good idea to search how this handled for other drivers.
> > 
> > Anyway, you are enforcing regulator use in your code. It doesn't
> > seem the code will work when there is no regulator declared in the
> > dts, which is the case for the majority of configurations. We
> > should at least make it optional.
> > 
> > Thanks for the contribution.  
> 

> My understanding of devm_regulator_get_optional() is that a stub
> regulator will be returned if one is not configured. See the comment
> above regulator_get_optional() for where I got this information.
> 
> https://elixir.bootlin.com/linux/v4.17/source/drivers/regulator/core.c#L1750

Quite the opposite (though the text is perhaps less than crystal clear)
 - reread that comment really carefully. 

 * This is intended for use by consumers for devices which can have
 * some supplies unconnected in normal use, such as some MMC devices.
 * It can allow the regulator core to provide stub supplies for other
 * supplies requested using normal regulator_get() calls without
 * disrupting the operation of drivers that can handle absent
 * supplies.

So in it allows normal path (non "optional") one to return stubs, but when
optional versions are used, it will return an error if they aren't there
- thus letting the driver decide what to do about it.


> 
> I agree that it would be better if this could be handled exclusively in
> dts if possible, although I'm not sure how. I looked at other drivers
> and commits from the last year or so and this appears to be the current
> recommended approach. I'm open to other approaches to doing this.

In the general case it is very hard to do. Many devices have fairly complex
regulator setups and need to enable them in particular orders and can
power down subsets etc.

The case here of just providing a power supply is very simplistic and
it may well be that down the line we want to do finer grained control
in the driver.  Only a driver can know what the effect of turning
a particular power supply to a device off actually is.

Jonathan
 
> 
> Brian
> 
> 
> >
> > 
> > From: Brian Masney <masneyb@onstation.org>
> > Sent: Wednesday, July 11, 2018 03:09
> > To: jic23@kernel.org; robh+dt@kernel.org; mark.rutland@arm.com; andy.gross@linaro.org; david.brown@linaro.org; linux-iio@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-msm@vger.kernel.org; linux-soc@vger.kernel.org
> > Cc: jonathan@marek.ca; Jean-Baptiste Maneyrol; knaack.h@gmx.de; lars@metafoo.de; pmeerw@pmeerw.net; mkelly@xevo.com; fischerdouglasc@gmail.com; bshah@kde.org; ctatlor97@gmail.com; masneyb@onstation.org
> > Subject: [PATCH 2/3] iio: imu: mpu6050: add support for regulator framework
> >   
> > 
> > CAUTION: This email originated from outside of the organization. Please make sure the sender is who they say they are and do not click links or open attachments unless you recognize the sender and know the content is safe.
> > 
> > 
> > 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>
> > ---
> > 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. (See my previous patch in this series)
> > - 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    | 57 +++++++++++++++++--
> >  drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c     |  2 +-
> >  drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h     |  3 +
> >  drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c     |  9 +++
> >  5 files changed, 66 insertions(+), 6 deletions(-)
> > 
> > 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..ec276b7bcc69 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,19 @@ 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 == 0) {
> > +               /* Give the device a little bit of time to start up. */
> > +               usleep_range(35000, 70000);
> > +       }
> > +
> > +       return result;
> > +}
> > +
> >  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,15 +1004,28 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
> >                 return -EINVAL;
> >         }
> > 
> > +       st->vddio_supply = devm_regulator_get_optional(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;
> > +
> >         /* power is turned on inside check chip type*/
> >         result = inv_check_and_setup_chip(st);
> >         if (result)
> > -               return result;
> > +               goto out_disable_regulator;
> > 
> >         result = inv_mpu6050_init_config(indio_dev);
> >         if (result) {
> >                 dev_err(dev, "Could not initialize device.\n");
> > -               return result;
> > +               goto out_disable_regulator;
> >         }
> > 
> >         if (inv_mpu_bus_setup)
> > @@ -1023,24 +1050,34 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
> >                                                  NULL);
> >         if (result) {
> >                 dev_err(dev, "configure buffer fail %d\n", result);
> > -               return result;
> > +               goto out_disable_regulator;
> >         }
> >         result = inv_mpu6050_probe_trigger(indio_dev, irq_type);
> >         if (result) {
> >                 dev_err(dev, "trigger probe fail %d\n", result);
> > -               return result;
> > +               goto out_disable_regulator;
> >         }
> > 
> >         result = devm_iio_device_register(dev, indio_dev);
> >         if (result) {
> >                 dev_err(dev, "IIO register fail %d\n", result);
> > -               return result;
> > +               goto out_disable_regulator;
> >         }
> > 
> >         return 0;
> > +
> > +out_disable_regulator:
> > +       regulator_disable(st->vddio_supply);
> > +       return result;
> > +
> >  }
> >  EXPORT_SYMBOL_GPL(inv_mpu_core_probe);
> > 
> > +int inv_mpu_core_remove(struct inv_mpu6050_state *st)
> > +{
> > +       return regulator_disable(st->vddio_supply);
> > +}
> > +
> >  #ifdef CONFIG_PM_SLEEP
> > 
> >  static int inv_mpu_resume(struct device *dev)
> > @@ -1050,6 +1087,11 @@ static int inv_mpu_resume(struct device *dev)
> > 
> >         mutex_lock(&st->lock);
> >         result = inv_mpu6050_set_power_itg(st, true);
> > +       if (result)
> > +               goto out_unlock;
> > +
> > +       result = inv_mpu_core_enable_regulator(st);
> > +out_unlock:
> >         mutex_unlock(&st->lock);
> > 
> >         return result;
> > @@ -1062,6 +1104,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 = regulator_disable(st->vddio_supply);
> > +out_unlock:
> >         mutex_unlock(&st->lock);
> > 
> >         return result;
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> > index dd758e3d403d..ce227038c61c 100644
> > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> > @@ -164,7 +164,7 @@ static int inv_mpu_remove(struct i2c_client *client)
> >                 i2c_mux_del_adapters(st->muxc);
> >         }
> > 
> > -       return 0;
> > +       return inv_mpu_core_remove(st);
> >  }
> > 
> >  /*
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
> > index e69a59659dbc..0289615a6135 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,7 @@ struct inv_mpu6050_state {
> >         s64 chip_period;
> >         s64 it_timestamp;
> >         s64 data_timestamp;
> > +       struct regulator *vddio_supply;
> >  };
> > 
> >  /*register and associated bit definition*/
> > @@ -321,4 +323,5 @@ int inv_mpu_acpi_create_mux_client(struct i2c_client *client);
> >  void inv_mpu_acpi_delete_mux_client(struct i2c_client *client);
> >  int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *name,
> >                 int (*inv_mpu_bus_setup)(struct iio_dev *), int chip_type);
> > +int inv_mpu_core_remove(struct inv_mpu6050_state *st);
> >  extern const struct dev_pm_ops inv_mpu_pmops;
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
> > index 227f50afff22..fce935f8f1e4 100644
> > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
> > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
> > @@ -70,6 +70,14 @@ static int inv_mpu_probe(struct spi_device *spi)
> >                                   inv_mpu_i2c_disable, chip_type);
> >  }
> > 
> > +static int inv_mpu_remove(struct spi_device *spi)
> > +{
> > +       struct iio_dev *indio_dev = spi_get_drvdata(spi);
> > +       struct inv_mpu6050_state *st = iio_priv(indio_dev);
> > +
> > +       return inv_mpu_core_remove(st);
> > +}
> > +
> >  /*
> >   * device id table is used to identify what device can be
> >   * supported by this driver
> > @@ -94,6 +102,7 @@ MODULE_DEVICE_TABLE(acpi, inv_acpi_match);
> > 
> >  static struct spi_driver inv_mpu_driver = {
> >         .probe          =       inv_mpu_probe,
> > +       .remove         =       inv_mpu_remove,
> >         .id_table       =       inv_mpu_id,
> >         .driver = {
> >                 .acpi_match_table = ACPI_PTR(inv_acpi_match),
> > --
> > 2.17.1
> > 
> >       
> --
> 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 2/3] iio: imu: mpu6050: add support for regulator framework

[Jonathan Cameron]

On Wed, 11 Jul 2018 09:00:07 -0400
Brian Masney <masneyb@onstation.org> wrote:

> On Wed, Jul 11, 2018 at 12:29:33PM +0000, Jean-Baptiste Maneyrol wrote:
> > Hello,
> >
=20
> > I really don't like the idea to have regulator handled inside the
> > driver. I know this was done like that before for Nexus 5, but I
> > think now this is something that can be done using dts only. Does
> > anyone know if there is a way with dts to handle regulator
> > automatically and prevent the use in the driver? That would be a
> > good idea to search how this handled for other drivers.
> >=20
> > Anyway, you are enforcing regulator use in your code. It doesn't
> > seem the code will work when there is no regulator declared in the
> > dts, which is the case for the majority of configurations. We
> > should at least make it optional.
> >=20
> > Thanks for the contribution. =20
>=20

> My understanding of devm_regulator_get_optional() is that a stub
> regulator will be returned if one is not configured. See the comment
> above regulator_get_optional() for where I got this information.
>=20
> https://elixir.bootlin.com/linux/v4.17/source/drivers/regulator/core.c#L1=
750

Quite the opposite (though the text is perhaps less than crystal clear)
 - reread that comment really carefully.=20

 * This is intended for use by consumers for devices which can have
 * some supplies unconnected in normal use, such as some MMC devices.
 * It can allow the regulator core to provide stub supplies for other
 * supplies requested using normal regulator_get() calls without
 * disrupting the operation of drivers that can handle absent
 * supplies.

So in it allows normal path (non "optional") one to return stubs, but when
optional versions are used, it will return an error if they aren't there
- thus letting the driver decide what to do about it.


>=20
> I agree that it would be better if this could be handled exclusively in
> dts if possible, although I'm not sure how. I looked at other drivers
> and commits from the last year or so and this appears to be the current
> recommended approach. I'm open to other approaches to doing this.

In the general case it is very hard to do. Many devices have fairly complex
regulator setups and need to enable them in particular orders and can
power down subsets etc.

The case here of just providing a power supply is very simplistic and
it may well be that down the line we want to do finer grained control
in the driver.  Only a driver can know what the effect of turning
a particular power supply to a device off actually is.

Jonathan
=20
>=20
> Brian
>=20
>=20
> >
> >=20
> > From: Brian Masney <masneyb@onstation.org>
> > Sent: Wednesday, July 11, 2018 03:09
> > To: jic23@kernel.org; robh+dt@kernel.org; mark.rutland@arm.com; andy.gr=
oss@linaro.org; david.brown@linaro.org; linux-iio@vger.kernel.org; devicetr=
ee@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-msm@vger.kernel=
.org; linux-soc@vger.kernel.org
> > Cc: jonathan@marek.ca; Jean-Baptiste Maneyrol; knaack.h@gmx.de; lars@me=
tafoo.de; pmeerw@pmeerw.net; mkelly@xevo.com; fischerdouglasc@gmail.com; bs=
hah@kde.org; ctatlor97@gmail.com; masneyb@onstation.org
> > Subject: [PATCH 2/3] iio: imu: mpu6050: add support for regulator frame=
work
> > =A0=20
> >=20
> > CAUTION: This email originated from outside of the organization. Please=
 make sure the sender is who they say they are and do not click links or op=
en attachments unless you recognize the sender and know the content is safe.
> >=20
> >=20
> > This patch adds support for the regulator framework to the mpu6050
> > driver.
> >=20
> > Signed-off-by: Brian Masney <masneyb@onstation.org>
> > Signed-off-by: Jonathan Marek <jonathan@marek.ca>
> > ---
> > This is a variation of Jonathan Marek's patch from postmarketOS
> > https://gitlab.com/postmarketOS/linux-postmarketos/commit/b8ad1ec1859c8=
bbcbce94944b3f4dd68f8f9fc37
> > with the following changes:
> >=20
> > - Stripped out 6515 variant code. (See my previous patch in this series)
> > - 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.
> >=20
> > =A0.../bindings/iio/imu/inv_mpu6050.txt=A0=A0=A0=A0=A0=A0=A0=A0=A0 |=A0=
 1 +
> > =A0drivers/iio/imu/inv_mpu6050/inv_mpu_core.c=A0=A0=A0 | 57 +++++++++++=
++++++--
> > =A0drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c=A0=A0=A0=A0 |=A0 2 +-
> > =A0drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h=A0=A0=A0=A0 |=A0 3 +
> > =A0drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c=A0=A0=A0=A0 |=A0 9 +++
> > =A05 files changed, 66 insertions(+), 6 deletions(-)
> >=20
> > 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:
> > =A0=A0 bindings.
> >=20
> > =A0Optional properties:
> > + - vddio-supply: regulator phandle for VDDIO supply
> > =A0 - mount-matrix: an optional 3x3 mounting rotation matrix
> > =A0 - i2c-gate node.=A0 These devices also support an auxiliary i2c bus=
.=A0 This is
> > =A0=A0=A0 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/i=
mu/inv_mpu6050/inv_mpu_core.c
> > index 12c1b9507007..ec276b7bcc69 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 @@
> > =A0#include <linux/iio/iio.h>
> > =A0#include <linux/acpi.h>
> > =A0#include <linux/platform_device.h>
> > +#include <linux/regulator/consumer.h>
> > =A0#include "inv_mpu_iio.h"
> >=20
> > =A0/*
> > @@ -926,6 +927,19 @@ static int inv_check_and_setup_chip(struct inv_mpu=
6050_state *st)
> > =A0=A0=A0=A0=A0=A0=A0 return result;
> > =A0}
> >=20
> > +static int inv_mpu_core_enable_regulator(struct inv_mpu6050_state *st)
> > +{
> > +=A0=A0=A0=A0=A0=A0 int result;
> > +
> > +=A0=A0=A0=A0=A0=A0 result =3D regulator_enable(st->vddio_supply);
> > +=A0=A0=A0=A0=A0=A0 if (result =3D=3D 0) {
> > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 /* Give the device a little=
 bit of time to start up. */
> > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 usleep_range(35000, 70000);
> > +=A0=A0=A0=A0=A0=A0 }
> > +
> > +=A0=A0=A0=A0=A0=A0 return result;
> > +}
> > +
> > =A0int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *n=
ame,
> > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 int (*inv_mpu_bus_setup)(=
struct iio_dev *), int chip_type)
> > =A0{
> > @@ -990,15 +1004,28 @@ int inv_mpu_core_probe(struct regmap *regmap, in=
t irq, const char *name,
> > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 return -EINVAL;
> > =A0=A0=A0=A0=A0=A0=A0 }
> >=20
> > +=A0=A0=A0=A0=A0=A0 st->vddio_supply =3D devm_regulator_get_optional(de=
v, "vddio");
> > +=A0=A0=A0=A0=A0=A0 if (IS_ERR(st->vddio_supply)) {
> > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 if (PTR_ERR(st->vddio_suppl=
y) !=3D -EPROBE_DEFER)
> > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 dev=
_err(dev, "Failed to get vddio regulator %d\n",
> > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=
=A0=A0=A0=A0=A0=A0=A0 (int)PTR_ERR(st->vddio_supply));
> > +
> > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 return PTR_ERR(st->vddio_su=
pply);
> > +=A0=A0=A0=A0=A0=A0 }
> > +
> > +=A0=A0=A0=A0=A0=A0 result =3D inv_mpu_core_enable_regulator(st);
> > +=A0=A0=A0=A0=A0=A0 if (result)
> > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 return result;
> > +
> > =A0=A0=A0=A0=A0=A0=A0 /* power is turned on inside check chip type*/
> > =A0=A0=A0=A0=A0=A0=A0 result =3D inv_check_and_setup_chip(st);
> > =A0=A0=A0=A0=A0=A0=A0 if (result)
> > -=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 return result;
> > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 goto out_disable_regulator;
> >=20
> > =A0=A0=A0=A0=A0=A0=A0 result =3D inv_mpu6050_init_config(indio_dev);
> > =A0=A0=A0=A0=A0=A0=A0 if (result) {
> > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 dev_err(dev, "Could not i=
nitialize device.\n");
> > -=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 return result;
> > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 goto out_disable_regulator;
> > =A0=A0=A0=A0=A0=A0=A0 }
> >=20
> > =A0=A0=A0=A0=A0=A0=A0 if (inv_mpu_bus_setup)
> > @@ -1023,24 +1050,34 @@ int inv_mpu_core_probe(struct regmap *regmap, i=
nt irq, const char *name,
> > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=
 NULL);
> > =A0=A0=A0=A0=A0=A0=A0 if (result) {
> > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 dev_err(dev, "configure b=
uffer fail %d\n", result);
> > -=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 return result;
> > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 goto out_disable_regulator;
> > =A0=A0=A0=A0=A0=A0=A0 }
> > =A0=A0=A0=A0=A0=A0=A0 result =3D inv_mpu6050_probe_trigger(indio_dev, i=
rq_type);
> > =A0=A0=A0=A0=A0=A0=A0 if (result) {
> > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 dev_err(dev, "trigger pro=
be fail %d\n", result);
> > -=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 return result;
> > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 goto out_disable_regulator;
> > =A0=A0=A0=A0=A0=A0=A0 }
> >=20
> > =A0=A0=A0=A0=A0=A0=A0 result =3D devm_iio_device_register(dev, indio_de=
v);
> > =A0=A0=A0=A0=A0=A0=A0 if (result) {
> > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 dev_err(dev, "IIO registe=
r fail %d\n", result);
> > -=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 return result;
> > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 goto out_disable_regulator;
> > =A0=A0=A0=A0=A0=A0=A0 }
> >=20
> > =A0=A0=A0=A0=A0=A0=A0 return 0;
> > +
> > +out_disable_regulator:
> > +=A0=A0=A0=A0=A0=A0 regulator_disable(st->vddio_supply);
> > +=A0=A0=A0=A0=A0=A0 return result;
> > +
> > =A0}
> > =A0EXPORT_SYMBOL_GPL(inv_mpu_core_probe);
> >=20
> > +int inv_mpu_core_remove(struct inv_mpu6050_state *st)
> > +{
> > +=A0=A0=A0=A0=A0=A0 return regulator_disable(st->vddio_supply);
> > +}
> > +
> > =A0#ifdef CONFIG_PM_SLEEP
> >=20
> > =A0static int inv_mpu_resume(struct device *dev)
> > @@ -1050,6 +1087,11 @@ static int inv_mpu_resume(struct device *dev)
> >=20
> > =A0=A0=A0=A0=A0=A0=A0 mutex_lock(&st->lock);
> > =A0=A0=A0=A0=A0=A0=A0 result =3D inv_mpu6050_set_power_itg(st, true);
> > +=A0=A0=A0=A0=A0=A0 if (result)
> > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 goto out_unlock;
> > +
> > +=A0=A0=A0=A0=A0=A0 result =3D inv_mpu_core_enable_regulator(st);
> > +out_unlock:
> > =A0=A0=A0=A0=A0=A0=A0 mutex_unlock(&st->lock);
> >=20
> > =A0=A0=A0=A0=A0=A0=A0 return result;
> > @@ -1062,6 +1104,11 @@ static int inv_mpu_suspend(struct device *dev)
> >=20
> > =A0=A0=A0=A0=A0=A0=A0 mutex_lock(&st->lock);
> > =A0=A0=A0=A0=A0=A0=A0 result =3D inv_mpu6050_set_power_itg(st, false);
> > +=A0=A0=A0=A0=A0=A0 if (result)
> > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 goto out_unlock;
> > +
> > +=A0=A0=A0=A0=A0=A0 result =3D regulator_disable(st->vddio_supply);
> > +out_unlock:
> > =A0=A0=A0=A0=A0=A0=A0 mutex_unlock(&st->lock);
> >=20
> > =A0=A0=A0=A0=A0=A0=A0 return result;
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c b/drivers/iio/im=
u/inv_mpu6050/inv_mpu_i2c.c
> > index dd758e3d403d..ce227038c61c 100644
> > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c
> > @@ -164,7 +164,7 @@ static int inv_mpu_remove(struct i2c_client *client)
> > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 i2c_mux_del_adapters(st->=
muxc);
> > =A0=A0=A0=A0=A0=A0=A0 }
> >=20
> > -=A0=A0=A0=A0=A0=A0 return 0;
> > +=A0=A0=A0=A0=A0=A0 return inv_mpu_core_remove(st);
> > =A0}
> >=20
> > =A0/*
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/im=
u/inv_mpu6050/inv_mpu_iio.h
> > index e69a59659dbc..0289615a6135 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 {
> > =A0 *=A0 @chip_period:=A0=A0=A0=A0=A0 chip internal period estimation (=
~1kHz).
> > =A0 *=A0 @it_timestamp:=A0=A0=A0=A0 timestamp from previous interrupt.
> > =A0 *=A0 @data_timestamp:=A0=A0 timestamp for next data sample.
> > + *=A0 @vddio_supply=A0=A0=A0=A0=A0 voltage regulator for the chip.
> > =A0 */
> > =A0struct inv_mpu6050_state {
> > =A0=A0=A0=A0=A0=A0=A0 struct mutex lock;
> > @@ -149,6 +150,7 @@ struct inv_mpu6050_state {
> > =A0=A0=A0=A0=A0=A0=A0 s64 chip_period;
> > =A0=A0=A0=A0=A0=A0=A0 s64 it_timestamp;
> > =A0=A0=A0=A0=A0=A0=A0 s64 data_timestamp;
> > +=A0=A0=A0=A0=A0=A0 struct regulator *vddio_supply;
> > =A0};
> >=20
> > =A0/*register and associated bit definition*/
> > @@ -321,4 +323,5 @@ int inv_mpu_acpi_create_mux_client(struct i2c_clien=
t *client);
> > =A0void inv_mpu_acpi_delete_mux_client(struct i2c_client *client);
> > =A0int inv_mpu_core_probe(struct regmap *regmap, int irq, const char *n=
ame,
> > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 int (*inv_mpu_bus_setup)(=
struct iio_dev *), int chip_type);
> > +int inv_mpu_core_remove(struct inv_mpu6050_state *st);
> > =A0extern const struct dev_pm_ops inv_mpu_pmops;
> > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c b/drivers/iio/im=
u/inv_mpu6050/inv_mpu_spi.c
> > index 227f50afff22..fce935f8f1e4 100644
> > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
> > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c
> > @@ -70,6 +70,14 @@ static int inv_mpu_probe(struct spi_device *spi)
> > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 inv_mpu_i2c_disable, chip_type);
> > =A0}
> >=20
> > +static int inv_mpu_remove(struct spi_device *spi)
> > +{
> > +=A0=A0=A0=A0=A0=A0 struct iio_dev *indio_dev =3D spi_get_drvdata(spi);
> > +=A0=A0=A0=A0=A0=A0 struct inv_mpu6050_state *st =3D iio_priv(indio_dev=
);
> > +
> > +=A0=A0=A0=A0=A0=A0 return inv_mpu_core_remove(st);
> > +}
> > +
> > =A0/*
> > =A0 * device id table is used to identify what device can be
> > =A0 * supported by this driver
> > @@ -94,6 +102,7 @@ MODULE_DEVICE_TABLE(acpi, inv_acpi_match);
> >=20
> > =A0static struct spi_driver inv_mpu_driver =3D {
> > =A0=A0=A0=A0=A0=A0=A0 .probe=A0=A0=A0=A0=A0=A0=A0=A0=A0 =3D=A0=A0=A0=A0=
=A0=A0 inv_mpu_probe,
> > +=A0=A0=A0=A0=A0=A0 .remove=A0=A0=A0=A0=A0=A0=A0=A0 =3D=A0=A0=A0=A0=A0=
=A0 inv_mpu_remove,
> > =A0=A0=A0=A0=A0=A0=A0 .id_table=A0=A0=A0=A0=A0=A0 =3D=A0=A0=A0=A0=A0=A0=
 inv_mpu_id,
> > =A0=A0=A0=A0=A0=A0=A0 .driver =3D {
> > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 .acpi_match_table =3D ACP=
I_PTR(inv_acpi_match),
> > --
> > 2.17.1
> >=20
> >      =20
> --
> 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

[PATCH 3/3] 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..96158044ca10 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>;
+					// Device 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