* [PATCH V4] iio: magnetometer: mag3110: add optional vdd/vddio regulator operation support
@ 2018-12-11 5:06 Anson Huang
2018-12-16 10:19 ` Jonathan Cameron
0 siblings, 1 reply; 2+ messages in thread
From: Anson Huang @ 2018-12-11 5:06 UTC (permalink / raw)
To: jic23, knaack.h, lars, pmeerw, rtresidd, linux-iio, linux-kernel,
festevam, preid
Cc: dl-linux-imx
The magnetometer's power supply could be controlled by regulator
on some platforms, such as i.MX6Q-SABRESD board, the mag3110's
power supply is controlled by a GPIO fixed regulator, need to make
sure the regulator is enabled before any communication with mag3110,
this patch adds optional vdd/vddio regulator operation support.
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
ChangeLog since V3:
- add disabling vdd in vddio error path;
- improve the regulator enable/disable sequence.
---
drivers/iio/magnetometer/mag3110.c | 96 ++++++++++++++++++++++++++++++++++----
1 file changed, 88 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/magnetometer/mag3110.c b/drivers/iio/magnetometer/mag3110.c
index f063355..328c0c4 100644
--- a/drivers/iio/magnetometer/mag3110.c
+++ b/drivers/iio/magnetometer/mag3110.c
@@ -20,6 +20,7 @@
#include <linux/iio/buffer.h>
#include <linux/iio/triggered_buffer.h>
#include <linux/delay.h>
+#include <linux/regulator/consumer.h>
#define MAG3110_STATUS 0x00
#define MAG3110_OUT_X 0x01 /* MSB first */
@@ -56,6 +57,8 @@ struct mag3110_data {
struct mutex lock;
u8 ctrl_reg1;
int sleep_val;
+ struct regulator *vdd_reg;
+ struct regulator *vddio_reg;
};
static int mag3110_request(struct mag3110_data *data)
@@ -469,17 +472,46 @@ static int mag3110_probe(struct i2c_client *client,
struct iio_dev *indio_dev;
int ret;
- ret = i2c_smbus_read_byte_data(client, MAG3110_WHO_AM_I);
- if (ret < 0)
- return ret;
- if (ret != MAG3110_DEVICE_ID)
- return -ENODEV;
-
indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
if (!indio_dev)
return -ENOMEM;
data = iio_priv(indio_dev);
+
+ data->vdd_reg = devm_regulator_get_optional(&client->dev, "vdd");
+ if (!IS_ERR(data->vdd_reg)) {
+ ret = regulator_enable(data->vdd_reg);
+ if (ret) {
+ dev_err(&client->dev, "failed to enable VDD regulator\n");
+ return ret;
+ }
+ } else {
+ ret = PTR_ERR(data->vdd_reg);
+ if (ret != -ENODEV)
+ return ret;
+ }
+
+ data->vddio_reg = devm_regulator_get_optional(&client->dev, "vddio");
+ if (!IS_ERR(data->vddio_reg)) {
+ ret = regulator_enable(data->vddio_reg);
+ if (ret) {
+ dev_err(&client->dev, "failed to enable VDDIO regulator\n");
+ goto disable_regulator_vdd;
+ }
+ } else {
+ ret = PTR_ERR(data->vddio_reg);
+ if (ret != -ENODEV)
+ goto disable_regulator_vdd;
+ }
+
+ ret = i2c_smbus_read_byte_data(client, MAG3110_WHO_AM_I);
+ if (ret < 0)
+ goto disable_regulators;
+ if (ret != MAG3110_DEVICE_ID) {
+ ret = -ENODEV;
+ goto disable_regulators;
+ }
+
data->client = client;
mutex_init(&data->lock);
@@ -499,7 +531,7 @@ static int mag3110_probe(struct i2c_client *client,
ret = mag3110_change_config(data, MAG3110_CTRL_REG1, data->ctrl_reg1);
if (ret < 0)
- return ret;
+ goto disable_regulators;
ret = i2c_smbus_write_byte_data(client, MAG3110_CTRL_REG2,
MAG3110_CTRL_AUTO_MRST_EN);
@@ -520,6 +552,13 @@ static int mag3110_probe(struct i2c_client *client,
iio_triggered_buffer_cleanup(indio_dev);
standby_on_error:
mag3110_standby(iio_priv(indio_dev));
+disable_regulators:
+ if (!IS_ERR(data->vddio_reg))
+ regulator_disable(data->vddio_reg);
+disable_regulator_vdd:
+ if (!IS_ERR(data->vdd_reg))
+ regulator_disable(data->vdd_reg);
+
return ret;
}
@@ -537,14 +576,55 @@ static int mag3110_remove(struct i2c_client *client)
#ifdef CONFIG_PM_SLEEP
static int mag3110_suspend(struct device *dev)
{
- return mag3110_standby(iio_priv(i2c_get_clientdata(
+ struct mag3110_data *data = iio_priv(i2c_get_clientdata(
+ to_i2c_client(dev)));
+ int ret;
+
+ ret = mag3110_standby(iio_priv(i2c_get_clientdata(
to_i2c_client(dev))));
+ if (ret)
+ return ret;
+
+ if (!IS_ERR(data->vddio_reg)) {
+ ret = regulator_disable(data->vddio_reg);
+ if (ret) {
+ dev_err(dev, "failed to disable VDDIO regulator\n");
+ return ret;
+ }
+ }
+ if (!IS_ERR(data->vdd_reg)) {
+ ret = regulator_disable(data->vdd_reg);
+ if (ret) {
+ dev_err(dev, "failed to disable VDD regulator\n");
+ return ret;
+ }
+ }
+
+ return 0;
}
static int mag3110_resume(struct device *dev)
{
struct mag3110_data *data = iio_priv(i2c_get_clientdata(
to_i2c_client(dev)));
+ int ret;
+
+ if (!IS_ERR(data->vdd_reg)) {
+ ret = regulator_enable(data->vdd_reg);
+ if (ret) {
+ dev_err(dev, "failed to enable VDD regulator\n");
+ return ret;
+ }
+ }
+ if (!IS_ERR(data->vddio_reg)) {
+ ret = regulator_enable(data->vddio_reg);
+ if (ret) {
+ dev_err(dev, "failed to enable VDDIO regulator\n");
+ if (!IS_ERR(data->vdd_reg))
+ regulator_disable(data->vdd_reg);
+ return ret;
+ }
+ }
return i2c_smbus_write_byte_data(data->client, MAG3110_CTRL_REG1,
data->ctrl_reg1);
--
2.7.4
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH V4] iio: magnetometer: mag3110: add optional vdd/vddio regulator operation support
2018-12-11 5:06 [PATCH V4] iio: magnetometer: mag3110: add optional vdd/vddio regulator operation support Anson Huang
@ 2018-12-16 10:19 ` Jonathan Cameron
0 siblings, 0 replies; 2+ messages in thread
From: Jonathan Cameron @ 2018-12-16 10:19 UTC (permalink / raw)
To: Anson Huang
Cc: knaack.h, lars, pmeerw, rtresidd, linux-iio, linux-kernel,
festevam, preid, dl-linux-imx
On Tue, 11 Dec 2018 05:06:20 +0000
Anson Huang <anson.huang@nxp.com> wrote:
> The magnetometer's power supply could be controlled by regulator
> on some platforms, such as i.MX6Q-SABRESD board, the mag3110's
> power supply is controlled by a GPIO fixed regulator, need to make
> sure the regulator is enabled before any communication with mag3110,
> this patch adds optional vdd/vddio regulator operation support.
>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
This device currently has documented bindings in trivial-devices.txt
Adding these non trivial elements means that we need to take it out
of there and add a specific binding doc for it.
So please provide a binding doc and drop the entry in trivial-devices.
Remember to cc devicetree list and maintainers for that.
Code looks good, but as we have missed the coming merge window
anyway, we have plenty of time to tidy up those docs.
Thanks,
Jonathan
> ---
> ChangeLog since V3:
> - add disabling vdd in vddio error path;
> - improve the regulator enable/disable sequence.
> ---
> drivers/iio/magnetometer/mag3110.c | 96 ++++++++++++++++++++++++++++++++++----
> 1 file changed, 88 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/iio/magnetometer/mag3110.c b/drivers/iio/magnetometer/mag3110.c
> index f063355..328c0c4 100644
> --- a/drivers/iio/magnetometer/mag3110.c
> +++ b/drivers/iio/magnetometer/mag3110.c
> @@ -20,6 +20,7 @@
> #include <linux/iio/buffer.h>
> #include <linux/iio/triggered_buffer.h>
> #include <linux/delay.h>
> +#include <linux/regulator/consumer.h>
>
> #define MAG3110_STATUS 0x00
> #define MAG3110_OUT_X 0x01 /* MSB first */
> @@ -56,6 +57,8 @@ struct mag3110_data {
> struct mutex lock;
> u8 ctrl_reg1;
> int sleep_val;
> + struct regulator *vdd_reg;
> + struct regulator *vddio_reg;
> };
>
> static int mag3110_request(struct mag3110_data *data)
> @@ -469,17 +472,46 @@ static int mag3110_probe(struct i2c_client *client,
> struct iio_dev *indio_dev;
> int ret;
>
> - ret = i2c_smbus_read_byte_data(client, MAG3110_WHO_AM_I);
> - if (ret < 0)
> - return ret;
> - if (ret != MAG3110_DEVICE_ID)
> - return -ENODEV;
> -
> indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> if (!indio_dev)
> return -ENOMEM;
>
> data = iio_priv(indio_dev);
> +
> + data->vdd_reg = devm_regulator_get_optional(&client->dev, "vdd");
> + if (!IS_ERR(data->vdd_reg)) {
> + ret = regulator_enable(data->vdd_reg);
> + if (ret) {
> + dev_err(&client->dev, "failed to enable VDD regulator\n");
> + return ret;
> + }
> + } else {
> + ret = PTR_ERR(data->vdd_reg);
> + if (ret != -ENODEV)
> + return ret;
> + }
> +
> + data->vddio_reg = devm_regulator_get_optional(&client->dev, "vddio");
> + if (!IS_ERR(data->vddio_reg)) {
> + ret = regulator_enable(data->vddio_reg);
> + if (ret) {
> + dev_err(&client->dev, "failed to enable VDDIO regulator\n");
> + goto disable_regulator_vdd;
> + }
> + } else {
> + ret = PTR_ERR(data->vddio_reg);
> + if (ret != -ENODEV)
> + goto disable_regulator_vdd;
> + }
> +
> + ret = i2c_smbus_read_byte_data(client, MAG3110_WHO_AM_I);
> + if (ret < 0)
> + goto disable_regulators;
> + if (ret != MAG3110_DEVICE_ID) {
> + ret = -ENODEV;
> + goto disable_regulators;
> + }
> +
> data->client = client;
> mutex_init(&data->lock);
>
> @@ -499,7 +531,7 @@ static int mag3110_probe(struct i2c_client *client,
>
> ret = mag3110_change_config(data, MAG3110_CTRL_REG1, data->ctrl_reg1);
> if (ret < 0)
> - return ret;
> + goto disable_regulators;
>
> ret = i2c_smbus_write_byte_data(client, MAG3110_CTRL_REG2,
> MAG3110_CTRL_AUTO_MRST_EN);
> @@ -520,6 +552,13 @@ static int mag3110_probe(struct i2c_client *client,
> iio_triggered_buffer_cleanup(indio_dev);
> standby_on_error:
> mag3110_standby(iio_priv(indio_dev));
> +disable_regulators:
> + if (!IS_ERR(data->vddio_reg))
> + regulator_disable(data->vddio_reg);
> +disable_regulator_vdd:
> + if (!IS_ERR(data->vdd_reg))
> + regulator_disable(data->vdd_reg);
> +
> return ret;
> }
>
> @@ -537,14 +576,55 @@ static int mag3110_remove(struct i2c_client *client)
> #ifdef CONFIG_PM_SLEEP
> static int mag3110_suspend(struct device *dev)
> {
> - return mag3110_standby(iio_priv(i2c_get_clientdata(
> + struct mag3110_data *data = iio_priv(i2c_get_clientdata(
> + to_i2c_client(dev)));
> + int ret;
> +
> + ret = mag3110_standby(iio_priv(i2c_get_clientdata(
> to_i2c_client(dev))));
> + if (ret)
> + return ret;
> +
> + if (!IS_ERR(data->vddio_reg)) {
> + ret = regulator_disable(data->vddio_reg);
> + if (ret) {
> + dev_err(dev, "failed to disable VDDIO regulator\n");
> + return ret;
> + }
> + }
> + if (!IS_ERR(data->vdd_reg)) {
> + ret = regulator_disable(data->vdd_reg);
> + if (ret) {
> + dev_err(dev, "failed to disable VDD regulator\n");
> + return ret;
> + }
> + }
> +
> + return 0;
> }
>
> static int mag3110_resume(struct device *dev)
> {
> struct mag3110_data *data = iio_priv(i2c_get_clientdata(
> to_i2c_client(dev)));
> + int ret;
> +
> + if (!IS_ERR(data->vdd_reg)) {
> + ret = regulator_enable(data->vdd_reg);
> + if (ret) {
> + dev_err(dev, "failed to enable VDD regulator\n");
> + return ret;
> + }
> + }
> + if (!IS_ERR(data->vddio_reg)) {
> + ret = regulator_enable(data->vddio_reg);
> + if (ret) {
> + dev_err(dev, "failed to enable VDDIO regulator\n");
> + if (!IS_ERR(data->vdd_reg))
> + regulator_disable(data->vdd_reg);
> + return ret;
> + }
> + }
>
> return i2c_smbus_write_byte_data(data->client, MAG3110_CTRL_REG1,
> data->ctrl_reg1);
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-12-16 10:19 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-11 5:06 [PATCH V4] iio: magnetometer: mag3110: add optional vdd/vddio regulator operation support Anson Huang
2018-12-16 10:19 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).