2018-12-10 07:19:19

by Anson Huang

[permalink] [raw]
Subject: [PATCH V2] iio: magnetometer: mag3110: add optional vdd/vddio regulator operation support

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 <[email protected]>
---
ChangeLog since V1:
- Add defer_probe check if the return value from devm_regulator_get_optional is ERR_PTR(-EPROBE_DEFER);
- Replace the "vcc" with "vdd" according to datasheet, and add optional "vddio" supply as well since
mag3110 also has a vddio power supply.
---
drivers/iio/magnetometer/mag3110.c | 77 +++++++++++++++++++++++++++++++++++---
1 file changed, 71 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/magnetometer/mag3110.c b/drivers/iio/magnetometer/mag3110.c
index f063355..6308b7f 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,40 @@ static int mag3110_probe(struct i2c_client *client,
struct iio_dev *indio_dev;
int ret;

+ 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 if (data->vdd_reg == ERR_PTR(-EPROBE_DEFER)) {
+ return -EPROBE_DEFER;
+ }
+
+ 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");
+ return ret;
+ }
+ } else if (data->vddio_reg == ERR_PTR(-EPROBE_DEFER)) {
+ return -EPROBE_DEFER;
+ }
+
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->client = client;
mutex_init(&data->lock);

@@ -537,14 +563,53 @@ 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->vdd_reg)) {
+ ret = regulator_disable(data->vdd_reg);
+ if (ret) {
+ dev_err(dev, "failed to disable VDD regulator\n");
+ 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;
+ }
+ }
+
+ 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");
+ return ret;
+ }
+ }

return i2c_smbus_write_byte_data(data->client, MAG3110_CTRL_REG1,
data->ctrl_reg1);
--
2.7.4



2018-12-10 08:26:48

by Phil Reid

[permalink] [raw]
Subject: Re: [PATCH V2] iio: magnetometer: mag3110: add optional vdd/vddio regulator operation support

G'day Anson,

On 10/12/2018 3:17 PM, Anson Huang 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.
>

[snip]

> +
> + 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 if (data->vdd_reg == ERR_PTR(-EPROBE_DEFER)) {
> + return -EPROBE_DEFER;
> + }
> +
IANAE, but normally the return pattern for devm_regulator_get_optional

if (!IS_ERR(reg)) {
regulator_enable ...
} else {
ret = PTR_ERR(reg);
if (ret != -ENODEV)
return ret;
}



--
Regards
Phil

2018-12-10 09:56:37

by Fabio Estevam

[permalink] [raw]
Subject: Re: [PATCH V2] iio: magnetometer: mag3110: add optional vdd/vddio regulator operation support

Hi Anson,

On Mon, Dec 10, 2018 at 5:17 AM Anson Huang <[email protected]> wrote:

> + 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 if (data->vdd_reg == ERR_PTR(-EPROBE_DEFER)) {
> + return -EPROBE_DEFER;
> + }
> +
> + 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");
> + return ret;

In the case of error you should disable the vdd_reg regulator.

> + }
> + } else if (data->vddio_reg == ERR_PTR(-EPROBE_DEFER)) {
> + return -EPROBE_DEFER;
> + }
> +
> ret = i2c_smbus_read_byte_data(client, MAG3110_WHO_AM_I);
> if (ret < 0)
> return ret;

and here you should disable vddio_reg.