2018-12-17 05:05:02

by Anson Huang

[permalink] [raw]
Subject: [PATCH V5 1/2] dt-bindings: iio: magnetometer: add dt-bindings for freescale mag3110

Add Freescale MAG3110 dt-bindings and remove it from trivial-devices
dt-bingding doc.

Signed-off-by: Anson Huang <[email protected]>
---
.../bindings/iio/magnetometer/mag3110.txt | 27 ++++++++++++++++++++++
.../devicetree/bindings/trivial-devices.txt | 1 -
2 files changed, 27 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/iio/magnetometer/mag3110.txt

diff --git a/Documentation/devicetree/bindings/iio/magnetometer/mag3110.txt b/Documentation/devicetree/bindings/iio/magnetometer/mag3110.txt
new file mode 100644
index 0000000..bdd40bc
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/magnetometer/mag3110.txt
@@ -0,0 +1,27 @@
+* FREESCALE MAG3110 magnetometer sensor
+
+Required properties:
+
+ - compatible : should be "fsl,mag3110"
+ - reg : the I2C address of the magnetometer
+
+Optional properties:
+
+ - interrupts: the sole interrupt generated by the device
+
+ Refer to interrupt-controller/interrupts.txt for generic interrupt client
+ node bindings.
+
+ - vdd-supply: phandle to the regulator that provides power to the sensor.
+ - vddio-supply: phandle to the regulator that provides power to the sensor's IO.
+
+Example:
+
+magnetometer@e {
+ compatible = "fsl,mag3110";
+ reg = <0x0e>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_i2c3_mag3110_int>;
+ interrupt-parent = <&gpio3>;
+ interrupts = <16 IRQ_TYPE_EDGE_RISING>;
+};
diff --git a/Documentation/devicetree/bindings/trivial-devices.txt b/Documentation/devicetree/bindings/trivial-devices.txt
index 6ab001f..5f086ac 100644
--- a/Documentation/devicetree/bindings/trivial-devices.txt
+++ b/Documentation/devicetree/bindings/trivial-devices.txt
@@ -43,7 +43,6 @@ domintech,dmard10 DMARD10: 3-axis Accelerometer
epson,rx8010 I2C-BUS INTERFACE REAL TIME CLOCK MODULE
epson,rx8581 I2C-BUS INTERFACE REAL TIME CLOCK MODULE
emmicro,em3027 EM Microelectronic EM3027 Real-time Clock
-fsl,mag3110 MAG3110: Xtrinsic High Accuracy, 3D Magnetometer
fsl,mma7660 MMA7660FC: 3-Axis Orientation/Motion Detection Sensor
fsl,mma8450 MMA8450Q: Xtrinsic Low-power, 3-axis Xtrinsic Accelerometer
fsl,mpl3115 MPL3115: Absolute Digital Pressure Sensor
--
2.7.4



2018-12-17 05:06:03

by Anson Huang

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

The magnetometer's power supplies could be controllable on some platforms,
such as i.MX6Q-SABRESD board, the mag3110's power supplies are controlled
by a GPIO fixed regulator, need to make sure the regulators are enabled
before any communication with mag3110, this patch adds vdd/vddio regulator
operation support.

Signed-off-by: Anson Huang <[email protected]>
---
ChangeLog since V4:
- using devm_regulator_get() instead of devm_regulator_get_optional() since the regulator is
there always, if dtb does NOT specify one, regulator framework will assign dummy regulator for it
---
drivers/iio/magnetometer/mag3110.c | 92 ++++++++++++++++++++++++++++++++++----
1 file changed, 84 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/magnetometer/mag3110.c b/drivers/iio/magnetometer/mag3110.c
index f063355..32660ce 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,48 @@ 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(&client->dev, "vdd");
+ if (IS_ERR(data->vdd_reg)) {
+ ret = PTR_ERR(data->vdd_reg);
+ if (ret != -EPROBE_DEFER)
+ dev_err(&client->dev, "failed to get VDD regulator!\n");
+ return ret;
+ }
+
+ ret = regulator_enable(data->vdd_reg);
+ if (ret) {
+ dev_err(&client->dev, "failed to enable VDD regulator!\n");
+ return ret;
+ }
+
+ data->vddio_reg = devm_regulator_get(&client->dev, "vddio");
+ if (IS_ERR(data->vddio_reg)) {
+ ret = PTR_ERR(data->vddio_reg);
+ if (ret != -EPROBE_DEFER)
+ dev_err(&client->dev, "failed to get VDDIO regulator!\n");
+ goto disable_regulator_vdd;
+ }
+
+ ret = regulator_enable(data->vddio_reg);
+ if (ret) {
+ dev_err(&client->dev, "failed to enable VDDIO regulator!\n");
+ 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 +533,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,16 +554,24 @@ 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:
+ regulator_disable(data->vddio_reg);
+disable_regulator_vdd:
+ regulator_disable(data->vdd_reg);
+
return ret;
}

static int mag3110_remove(struct i2c_client *client)
{
struct iio_dev *indio_dev = i2c_get_clientdata(client);
+ struct mag3110_data *data = iio_priv(indio_dev);

iio_device_unregister(indio_dev);
iio_triggered_buffer_cleanup(indio_dev);
mag3110_standby(iio_priv(indio_dev));
+ regulator_disable(data->vddio_reg);
+ regulator_disable(data->vdd_reg);

return 0;
}
@@ -537,14 +579,48 @@ 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;
+
+ ret = regulator_disable(data->vddio_reg);
+ if (ret) {
+ dev_err(dev, "failed to disable VDDIO regulator\n");
+ return ret;
+ }
+
+ 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;
+
+ ret = regulator_enable(data->vdd_reg);
+ if (ret) {
+ dev_err(dev, "failed to enable VDD regulator\n");
+ return ret;
+ }
+
+ ret = regulator_enable(data->vddio_reg);
+ if (ret) {
+ dev_err(dev, "failed to enable VDDIO regulator\n");
+ regulator_disable(data->vdd_reg);
+ return ret;
+ }

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


2018-12-17 14:57:38

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH V5 1/2] dt-bindings: iio: magnetometer: add dt-bindings for freescale mag3110

On Sun, Dec 16, 2018 at 10:47 PM Anson Huang <[email protected]> wrote:
>
> Add Freescale MAG3110 dt-bindings and remove it from trivial-devices
> dt-bingding doc.
>
> Signed-off-by: Anson Huang <[email protected]>
> ---
> .../bindings/iio/magnetometer/mag3110.txt | 27 ++++++++++++++++++++++
> .../devicetree/bindings/trivial-devices.txt | 1 -
> 2 files changed, 27 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/devicetree/bindings/iio/magnetometer/mag3110.txt

trivial-devices.txt is gone now as it's converted to json-schema. I
can take this patch thru my tree and fix it up.

Rob

2018-12-23 11:25:48

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH V5 1/2] dt-bindings: iio: magnetometer: add dt-bindings for freescale mag3110

On Mon, 17 Dec 2018 08:55:53 -0600
Rob Herring <[email protected]> wrote:

> On Sun, Dec 16, 2018 at 10:47 PM Anson Huang <[email protected]> wrote:
> >
> > Add Freescale MAG3110 dt-bindings and remove it from trivial-devices
> > dt-bingding doc.
> >
> > Signed-off-by: Anson Huang <[email protected]>
> > ---
> > .../bindings/iio/magnetometer/mag3110.txt | 27 ++++++++++++++++++++++
> > .../devicetree/bindings/trivial-devices.txt | 1 -
> > 2 files changed, 27 insertions(+), 1 deletion(-)
> > create mode 100644 Documentation/devicetree/bindings/iio/magnetometer/mag3110.txt
>
> trivial-devices.txt is gone now as it's converted to json-schema. I
> can take this patch thru my tree and fix it up.
>
> Rob

That would be great. Thanks. I'll pick up patch 2.

Acked-by: Jonathan Cameron <[email protected]>

2018-12-23 11:36:42

by Jonathan Cameron

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

On Mon, 17 Dec 2018 04:47:49 +0000
Anson Huang <[email protected]> wrote:

> The magnetometer's power supplies could be controllable on some platforms,
> such as i.MX6Q-SABRESD board, the mag3110's power supplies are controlled
> by a GPIO fixed regulator, need to make sure the regulators are enabled
> before any communication with mag3110, this patch adds vdd/vddio regulator
> operation support.
>
> Signed-off-by: Anson Huang <[email protected]>
> ---
> ChangeLog since V4:
> - using devm_regulator_get() instead of devm_regulator_get_optional() since the regulator is
> there always, if dtb does NOT specify one, regulator framework will assign dummy regulator for it
> ---
> drivers/iio/magnetometer/mag3110.c | 92 ++++++++++++++++++++++++++++++++++----
> 1 file changed, 84 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/iio/magnetometer/mag3110.c b/drivers/iio/magnetometer/mag3110.c
> index f063355..32660ce 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,48 @@ 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(&client->dev, "vdd");
> + if (IS_ERR(data->vdd_reg)) {
> + ret = PTR_ERR(data->vdd_reg);
> + if (ret != -EPROBE_DEFER)
> + dev_err(&client->dev, "failed to get VDD regulator!\n");
> + return ret;
> + }
> +
> + ret = regulator_enable(data->vdd_reg);
> + if (ret) {
> + dev_err(&client->dev, "failed to enable VDD regulator!\n");
> + return ret;
> + }

Please don't mix managed unwinding (devm) with the manual version.
It makes it harder to reason about any races. Here there aren't any
but in general please don't do it.

The easy solution here is to reorder so both regulators are acquired
before either is turned on.

Jonathan

> +
> + data->vddio_reg = devm_regulator_get(&client->dev, "vddio");
> + if (IS_ERR(data->vddio_reg)) {
> + ret = PTR_ERR(data->vddio_reg);
> + if (ret != -EPROBE_DEFER)
> + dev_err(&client->dev, "failed to get VDDIO regulator!\n");
> + goto disable_regulator_vdd;
> + }
> +
> + ret = regulator_enable(data->vddio_reg);
> + if (ret) {
> + dev_err(&client->dev, "failed to enable VDDIO regulator!\n");
> + 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 +533,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,16 +554,24 @@ 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:
> + regulator_disable(data->vddio_reg);
> +disable_regulator_vdd:
> + regulator_disable(data->vdd_reg);
> +
> return ret;
> }
>
> static int mag3110_remove(struct i2c_client *client)
> {
> struct iio_dev *indio_dev = i2c_get_clientdata(client);
> + struct mag3110_data *data = iio_priv(indio_dev);
>
> iio_device_unregister(indio_dev);
> iio_triggered_buffer_cleanup(indio_dev);
> mag3110_standby(iio_priv(indio_dev));
> + regulator_disable(data->vddio_reg);
> + regulator_disable(data->vdd_reg);
>
> return 0;
> }
> @@ -537,14 +579,48 @@ 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;
> +
> + ret = regulator_disable(data->vddio_reg);
> + if (ret) {
> + dev_err(dev, "failed to disable VDDIO regulator\n");
> + return ret;
> + }
> +
> + 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;
> +
> + ret = regulator_enable(data->vdd_reg);
> + if (ret) {
> + dev_err(dev, "failed to enable VDD regulator\n");
> + return ret;
> + }
> +
> + ret = regulator_enable(data->vddio_reg);
> + if (ret) {
> + dev_err(dev, "failed to enable VDDIO regulator\n");
> + regulator_disable(data->vdd_reg);
> + return ret;
> + }
>
> return i2c_smbus_write_byte_data(data->client, MAG3110_CTRL_REG1,
> data->ctrl_reg1);