* [PATCH V4 1/2] dt-bindings: iio: accel: mma8452: add optional power supply property @ 2018-12-11 5:13 Anson Huang 2018-12-11 5:13 ` [PATCH V4 2/2] iio: accell: mma8452: add optional vdd/vddio regulator operation support Anson Huang 2018-12-16 10:46 ` [PATCH V4 1/2] dt-bindings: iio: accel: mma8452: add optional power supply property Jonathan Cameron 0 siblings, 2 replies; 5+ messages in thread From: Anson Huang @ 2018-12-11 5:13 UTC (permalink / raw) To: jic23@kernel.org, knaack.h@gmx.de, lars@metafoo.de, pmeerw@pmeerw.net, robh+dt@kernel.org, mark.rutland@arm.com, gregkh@linuxfoundation.org, martink@posteo.de, Leonard Crestez, rtresidd@electromag.com.au, gustavo@embeddedor.com, linux-iio@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, festevam@gmail.com, preid@electromag.com.au Cc: dl-linux-imx The accelerometer's power supply could be controlled by regulator on some platforms, add optional property "vdd/vddio" power supply to let device tree to pass phandles to the regulators to driver. Signed-off-by: Anson Huang <Anson.Huang@nxp.com> --- Documentation/devicetree/bindings/iio/accel/mma8452.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt index 2100e9a..e132394 100644 --- a/Documentation/devicetree/bindings/iio/accel/mma8452.txt +++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt @@ -20,6 +20,10 @@ Optional properties: - interrupt-names: should contain "INT1" and/or "INT2", the accelerometer's interrupt line in use. + - vdd-supply: phandle to the regulator that provides vdd power to the accelerometer. + + - vddio-supply: phandle to the regulator that provides vddio power to the accelerometer. + Example: mma8453fc@1d { -- 2.7.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH V4 2/2] iio: accell: mma8452: add optional vdd/vddio regulator operation support 2018-12-11 5:13 [PATCH V4 1/2] dt-bindings: iio: accel: mma8452: add optional power supply property Anson Huang @ 2018-12-11 5:13 ` Anson Huang 2018-12-16 10:46 ` Jonathan Cameron 2018-12-16 10:46 ` [PATCH V4 1/2] dt-bindings: iio: accel: mma8452: add optional power supply property Jonathan Cameron 1 sibling, 1 reply; 5+ messages in thread From: Anson Huang @ 2018-12-11 5:13 UTC (permalink / raw) To: jic23@kernel.org, knaack.h@gmx.de, lars@metafoo.de, pmeerw@pmeerw.net, robh+dt@kernel.org, mark.rutland@arm.com, gregkh@linuxfoundation.org, martink@posteo.de, Leonard Crestez, rtresidd@electromag.com.au, gustavo@embeddedor.com, linux-iio@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, festevam@gmail.com, preid@electromag.com.au Cc: dl-linux-imx The accelerometer's power supply could be controlled by regulator on some platforms, such as i.MX6Q-SABRESD board, the mma8451's power supply is controlled by a GPIO fixed regulator, need to make sure the regulator is enabled before any communication with mma8451, this patch adds optional vdd/vddio regulator operation support. Signed-off-by: Anson Huang <Anson.Huang@nxp.com> Acked-by: Martin Kepplinger <martink@posteo.de> --- ChangeLog since V3: - add disabling vdd regulator in vddio error path; - improve regulator enable/disable sequence. --- drivers/iio/accel/mma8452.c | 186 +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 176 insertions(+), 10 deletions(-) diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c index 421a0a8..590a95d 100644 --- a/drivers/iio/accel/mma8452.c +++ b/drivers/iio/accel/mma8452.c @@ -31,6 +31,7 @@ #include <linux/of_device.h> #include <linux/of_irq.h> #include <linux/pm_runtime.h> +#include <linux/regulator/consumer.h> #define MMA8452_STATUS 0x00 #define MMA8452_STATUS_DRDY (BIT(2) | BIT(1) | BIT(0)) @@ -107,6 +108,8 @@ struct mma8452_data { u8 data_cfg; const struct mma_chip_info *chip_info; int sleep_val; + struct regulator *vdd_reg; + struct regulator *vddio_reg; }; /** @@ -1533,10 +1536,35 @@ static int mma8452_probe(struct i2c_client *client, data->client = client; mutex_init(&data->lock); data->chip_info = match->data; + 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, MMA8452_WHO_AM_I); if (ret < 0) - return ret; + goto disable_regulators; switch (ret) { case MMA8451_DEVICE_ID: @@ -1549,7 +1577,8 @@ static int mma8452_probe(struct i2c_client *client, break; /* else: fall through */ default: - return -ENODEV; + ret = -ENODEV; + goto disable_regulators; } dev_info(&client->dev, "registering %s accelerometer; ID 0x%x\n", @@ -1566,13 +1595,13 @@ static int mma8452_probe(struct i2c_client *client, ret = mma8452_reset(client); if (ret < 0) - return ret; + goto disable_regulators; data->data_cfg = MMA8452_DATA_CFG_FS_2G; ret = i2c_smbus_write_byte_data(client, MMA8452_DATA_CFG, data->data_cfg); if (ret < 0) - return ret; + goto disable_regulators; /* * By default set transient threshold to max to avoid events if @@ -1581,7 +1610,7 @@ static int mma8452_probe(struct i2c_client *client, ret = i2c_smbus_write_byte_data(client, MMA8452_TRANSIENT_THS, MMA8452_TRANSIENT_THS_MASK); if (ret < 0) - return ret; + goto disable_regulators; if (client->irq) { int irq2; @@ -1595,7 +1624,7 @@ static int mma8452_probe(struct i2c_client *client, MMA8452_CTRL_REG5, data->chip_info->all_events); if (ret < 0) - return ret; + goto disable_regulators; dev_dbg(&client->dev, "using interrupt line INT1\n"); } @@ -1604,11 +1633,11 @@ static int mma8452_probe(struct i2c_client *client, MMA8452_CTRL_REG4, data->chip_info->enabled_events); if (ret < 0) - return ret; + goto disable_regulators; ret = mma8452_trigger_setup(indio_dev); if (ret < 0) - return ret; + goto disable_regulators; } data->ctrl_reg1 = MMA8452_CTRL_ACTIVE | @@ -1661,12 +1690,22 @@ static int mma8452_probe(struct i2c_client *client, trigger_cleanup: mma8452_trigger_cleanup(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; } static int mma8452_remove(struct i2c_client *client) { struct iio_dev *indio_dev = i2c_get_clientdata(client); + struct mma8452_data *data = iio_priv(indio_dev); + int ret; iio_device_unregister(indio_dev); @@ -1678,6 +1717,21 @@ static int mma8452_remove(struct i2c_client *client) mma8452_trigger_cleanup(indio_dev); mma8452_standby(iio_priv(indio_dev)); + if (!IS_ERR(data->vddio_reg)) { + ret = regulator_disable(data->vddio_reg); + if (ret) { + dev_err(&client->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(&client->dev, "failed to disable VDD regulator\n"); + return ret; + } + } + return 0; } @@ -1696,6 +1750,21 @@ static int mma8452_runtime_suspend(struct device *dev) return -EAGAIN; } + 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; } @@ -1705,6 +1774,23 @@ static int mma8452_runtime_resume(struct device *dev) struct mma8452_data *data = iio_priv(indio_dev); int ret, sleep_val; + 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; + } + } + ret = mma8452_active(data); if (ret < 0) return ret; @@ -1723,14 +1809,94 @@ static int mma8452_runtime_resume(struct device *dev) #ifdef CONFIG_PM_SLEEP static int mma8452_suspend(struct device *dev) { - return mma8452_standby(iio_priv(i2c_get_clientdata( + struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev)); + struct mma8452_data *data = iio_priv(indio_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; + } + } + + ret = mma8452_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 mma8452_resume(struct device *dev) { - return mma8452_active(iio_priv(i2c_get_clientdata( + struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev)); + struct mma8452_data *data = iio_priv(indio_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; + } + } + + ret = mma8452_active(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; } #endif -- 2.7.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH V4 2/2] iio: accell: mma8452: add optional vdd/vddio regulator operation support 2018-12-11 5:13 ` [PATCH V4 2/2] iio: accell: mma8452: add optional vdd/vddio regulator operation support Anson Huang @ 2018-12-16 10:46 ` Jonathan Cameron 2018-12-16 14:08 ` Jonathan Cameron 0 siblings, 1 reply; 5+ messages in thread From: Jonathan Cameron @ 2018-12-16 10:46 UTC (permalink / raw) To: Anson Huang Cc: knaack.h@gmx.de, lars@metafoo.de, pmeerw@pmeerw.net, robh+dt@kernel.org, mark.rutland@arm.com, gregkh@linuxfoundation.org, martink@posteo.de, Leonard Crestez, rtresidd@electromag.com.au, gustavo@embeddedor.com, linux-iio@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, festevam@gmail.com, preid@electromag.com.au, dl-linux-imx On Tue, 11 Dec 2018 05:13:20 +0000 Anson Huang <anson.huang@nxp.com> wrote: > The accelerometer's power supply could be controlled by regulator > on some platforms, such as i.MX6Q-SABRESD board, the mma8451's > power supply is controlled by a GPIO fixed regulator, need to make > sure the regulator is enabled before any communication with mma8451, > this patch adds optional vdd/vddio regulator operation support. > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com> > Acked-by: Martin Kepplinger <martink@posteo.de> Applied to the togreg branch of iio.git and pushed out as testing for the autobuilders to play with it. Thanks, Jonathan > --- > ChangeLog since V3: > - add disabling vdd regulator in vddio error path; > - improve regulator enable/disable sequence. > --- > drivers/iio/accel/mma8452.c | 186 +++++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 176 insertions(+), 10 deletions(-) > > diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c > index 421a0a8..590a95d 100644 > --- a/drivers/iio/accel/mma8452.c > +++ b/drivers/iio/accel/mma8452.c > @@ -31,6 +31,7 @@ > #include <linux/of_device.h> > #include <linux/of_irq.h> > #include <linux/pm_runtime.h> > +#include <linux/regulator/consumer.h> > > #define MMA8452_STATUS 0x00 > #define MMA8452_STATUS_DRDY (BIT(2) | BIT(1) | BIT(0)) > @@ -107,6 +108,8 @@ struct mma8452_data { > u8 data_cfg; > const struct mma_chip_info *chip_info; > int sleep_val; > + struct regulator *vdd_reg; > + struct regulator *vddio_reg; > }; > > /** > @@ -1533,10 +1536,35 @@ static int mma8452_probe(struct i2c_client *client, > data->client = client; > mutex_init(&data->lock); > data->chip_info = match->data; > + 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, MMA8452_WHO_AM_I); > if (ret < 0) > - return ret; > + goto disable_regulators; > > switch (ret) { > case MMA8451_DEVICE_ID: > @@ -1549,7 +1577,8 @@ static int mma8452_probe(struct i2c_client *client, > break; > /* else: fall through */ > default: > - return -ENODEV; > + ret = -ENODEV; > + goto disable_regulators; > } > > dev_info(&client->dev, "registering %s accelerometer; ID 0x%x\n", > @@ -1566,13 +1595,13 @@ static int mma8452_probe(struct i2c_client *client, > > ret = mma8452_reset(client); > if (ret < 0) > - return ret; > + goto disable_regulators; > > data->data_cfg = MMA8452_DATA_CFG_FS_2G; > ret = i2c_smbus_write_byte_data(client, MMA8452_DATA_CFG, > data->data_cfg); > if (ret < 0) > - return ret; > + goto disable_regulators; > > /* > * By default set transient threshold to max to avoid events if > @@ -1581,7 +1610,7 @@ static int mma8452_probe(struct i2c_client *client, > ret = i2c_smbus_write_byte_data(client, MMA8452_TRANSIENT_THS, > MMA8452_TRANSIENT_THS_MASK); > if (ret < 0) > - return ret; > + goto disable_regulators; > > if (client->irq) { > int irq2; > @@ -1595,7 +1624,7 @@ static int mma8452_probe(struct i2c_client *client, > MMA8452_CTRL_REG5, > data->chip_info->all_events); > if (ret < 0) > - return ret; > + goto disable_regulators; > > dev_dbg(&client->dev, "using interrupt line INT1\n"); > } > @@ -1604,11 +1633,11 @@ static int mma8452_probe(struct i2c_client *client, > MMA8452_CTRL_REG4, > data->chip_info->enabled_events); > if (ret < 0) > - return ret; > + goto disable_regulators; > > ret = mma8452_trigger_setup(indio_dev); > if (ret < 0) > - return ret; > + goto disable_regulators; > } > > data->ctrl_reg1 = MMA8452_CTRL_ACTIVE | > @@ -1661,12 +1690,22 @@ static int mma8452_probe(struct i2c_client *client, > trigger_cleanup: > mma8452_trigger_cleanup(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; > } > > static int mma8452_remove(struct i2c_client *client) > { > struct iio_dev *indio_dev = i2c_get_clientdata(client); > + struct mma8452_data *data = iio_priv(indio_dev); > + int ret; > > iio_device_unregister(indio_dev); > > @@ -1678,6 +1717,21 @@ static int mma8452_remove(struct i2c_client *client) > mma8452_trigger_cleanup(indio_dev); > mma8452_standby(iio_priv(indio_dev)); > > + if (!IS_ERR(data->vddio_reg)) { > + ret = regulator_disable(data->vddio_reg); > + if (ret) { > + dev_err(&client->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(&client->dev, "failed to disable VDD regulator\n"); > + return ret; > + } > + } > + > return 0; > } > > @@ -1696,6 +1750,21 @@ static int mma8452_runtime_suspend(struct device *dev) > return -EAGAIN; > } > > + 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; > } > > @@ -1705,6 +1774,23 @@ static int mma8452_runtime_resume(struct device *dev) > struct mma8452_data *data = iio_priv(indio_dev); > int ret, sleep_val; > > + 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; > + } > + } > + > ret = mma8452_active(data); > if (ret < 0) > return ret; > @@ -1723,14 +1809,94 @@ static int mma8452_runtime_resume(struct device *dev) > #ifdef CONFIG_PM_SLEEP > static int mma8452_suspend(struct device *dev) > { > - return mma8452_standby(iio_priv(i2c_get_clientdata( > + struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev)); > + struct mma8452_data *data = iio_priv(indio_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; > + } > + } > + > + ret = mma8452_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 mma8452_resume(struct device *dev) > { > - return mma8452_active(iio_priv(i2c_get_clientdata( > + struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev)); > + struct mma8452_data *data = iio_priv(indio_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; > + } > + } > + > + ret = mma8452_active(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; > } > #endif > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH V4 2/2] iio: accell: mma8452: add optional vdd/vddio regulator operation support 2018-12-16 10:46 ` Jonathan Cameron @ 2018-12-16 14:08 ` Jonathan Cameron 0 siblings, 0 replies; 5+ messages in thread From: Jonathan Cameron @ 2018-12-16 14:08 UTC (permalink / raw) To: Anson Huang Cc: knaack.h@gmx.de, lars@metafoo.de, pmeerw@pmeerw.net, robh+dt@kernel.org, mark.rutland@arm.com, gregkh@linuxfoundation.org, martink@posteo.de, Leonard Crestez, rtresidd@electromag.com.au, gustavo@embeddedor.com, linux-iio@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, festevam@gmail.com, preid@electromag.com.au, dl-linux-imx On Sun, 16 Dec 2018 10:46:39 +0000 Jonathan Cameron <jic23@kernel.org> wrote: > On Tue, 11 Dec 2018 05:13:20 +0000 > Anson Huang <anson.huang@nxp.com> wrote: > > > The accelerometer's power supply could be controlled by regulator > > on some platforms, such as i.MX6Q-SABRESD board, the mma8451's > > power supply is controlled by a GPIO fixed regulator, need to make > > sure the regulator is enabled before any communication with mma8451, > > this patch adds optional vdd/vddio regulator operation support. > > > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com> > > Acked-by: Martin Kepplinger <martink@posteo.de> > Applied to the togreg branch of iio.git and pushed out as testing > for the autobuilders to play with it. Sorry again for this. I was half asleep. This one isn't optional either, it just may not be controllable / queryable. So please use the non optional regulator request. If there is one specified it will wait for it to be available by deferring. If not then it'll just provide a stub regulator so that the rest of the code doesn't need to care. Patches dropped for now. Thanks, Jonathan > > Thanks, > > Jonathan > > > --- > > ChangeLog since V3: > > - add disabling vdd regulator in vddio error path; > > - improve regulator enable/disable sequence. > > --- > > drivers/iio/accel/mma8452.c | 186 +++++++++++++++++++++++++++++++++++++++++--- > > 1 file changed, 176 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c > > index 421a0a8..590a95d 100644 > > --- a/drivers/iio/accel/mma8452.c > > +++ b/drivers/iio/accel/mma8452.c > > @@ -31,6 +31,7 @@ > > #include <linux/of_device.h> > > #include <linux/of_irq.h> > > #include <linux/pm_runtime.h> > > +#include <linux/regulator/consumer.h> > > > > #define MMA8452_STATUS 0x00 > > #define MMA8452_STATUS_DRDY (BIT(2) | BIT(1) | BIT(0)) > > @@ -107,6 +108,8 @@ struct mma8452_data { > > u8 data_cfg; > > const struct mma_chip_info *chip_info; > > int sleep_val; > > + struct regulator *vdd_reg; > > + struct regulator *vddio_reg; > > }; > > > > /** > > @@ -1533,10 +1536,35 @@ static int mma8452_probe(struct i2c_client *client, > > data->client = client; > > mutex_init(&data->lock); > > data->chip_info = match->data; > > + 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, MMA8452_WHO_AM_I); > > if (ret < 0) > > - return ret; > > + goto disable_regulators; > > > > switch (ret) { > > case MMA8451_DEVICE_ID: > > @@ -1549,7 +1577,8 @@ static int mma8452_probe(struct i2c_client *client, > > break; > > /* else: fall through */ > > default: > > - return -ENODEV; > > + ret = -ENODEV; > > + goto disable_regulators; > > } > > > > dev_info(&client->dev, "registering %s accelerometer; ID 0x%x\n", > > @@ -1566,13 +1595,13 @@ static int mma8452_probe(struct i2c_client *client, > > > > ret = mma8452_reset(client); > > if (ret < 0) > > - return ret; > > + goto disable_regulators; > > > > data->data_cfg = MMA8452_DATA_CFG_FS_2G; > > ret = i2c_smbus_write_byte_data(client, MMA8452_DATA_CFG, > > data->data_cfg); > > if (ret < 0) > > - return ret; > > + goto disable_regulators; > > > > /* > > * By default set transient threshold to max to avoid events if > > @@ -1581,7 +1610,7 @@ static int mma8452_probe(struct i2c_client *client, > > ret = i2c_smbus_write_byte_data(client, MMA8452_TRANSIENT_THS, > > MMA8452_TRANSIENT_THS_MASK); > > if (ret < 0) > > - return ret; > > + goto disable_regulators; > > > > if (client->irq) { > > int irq2; > > @@ -1595,7 +1624,7 @@ static int mma8452_probe(struct i2c_client *client, > > MMA8452_CTRL_REG5, > > data->chip_info->all_events); > > if (ret < 0) > > - return ret; > > + goto disable_regulators; > > > > dev_dbg(&client->dev, "using interrupt line INT1\n"); > > } > > @@ -1604,11 +1633,11 @@ static int mma8452_probe(struct i2c_client *client, > > MMA8452_CTRL_REG4, > > data->chip_info->enabled_events); > > if (ret < 0) > > - return ret; > > + goto disable_regulators; > > > > ret = mma8452_trigger_setup(indio_dev); > > if (ret < 0) > > - return ret; > > + goto disable_regulators; > > } > > > > data->ctrl_reg1 = MMA8452_CTRL_ACTIVE | > > @@ -1661,12 +1690,22 @@ static int mma8452_probe(struct i2c_client *client, > > trigger_cleanup: > > mma8452_trigger_cleanup(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; > > } > > > > static int mma8452_remove(struct i2c_client *client) > > { > > struct iio_dev *indio_dev = i2c_get_clientdata(client); > > + struct mma8452_data *data = iio_priv(indio_dev); > > + int ret; > > > > iio_device_unregister(indio_dev); > > > > @@ -1678,6 +1717,21 @@ static int mma8452_remove(struct i2c_client *client) > > mma8452_trigger_cleanup(indio_dev); > > mma8452_standby(iio_priv(indio_dev)); > > > > + if (!IS_ERR(data->vddio_reg)) { > > + ret = regulator_disable(data->vddio_reg); > > + if (ret) { > > + dev_err(&client->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(&client->dev, "failed to disable VDD regulator\n"); > > + return ret; > > + } > > + } > > + > > return 0; > > } > > > > @@ -1696,6 +1750,21 @@ static int mma8452_runtime_suspend(struct device *dev) > > return -EAGAIN; > > } > > > > + 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; > > } > > > > @@ -1705,6 +1774,23 @@ static int mma8452_runtime_resume(struct device *dev) > > struct mma8452_data *data = iio_priv(indio_dev); > > int ret, sleep_val; > > > > + 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; > > + } > > + } > > + > > ret = mma8452_active(data); > > if (ret < 0) > > return ret; > > @@ -1723,14 +1809,94 @@ static int mma8452_runtime_resume(struct device *dev) > > #ifdef CONFIG_PM_SLEEP > > static int mma8452_suspend(struct device *dev) > > { > > - return mma8452_standby(iio_priv(i2c_get_clientdata( > > + struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev)); > > + struct mma8452_data *data = iio_priv(indio_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; > > + } > > + } > > + > > + ret = mma8452_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 mma8452_resume(struct device *dev) > > { > > - return mma8452_active(iio_priv(i2c_get_clientdata( > > + struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev)); > > + struct mma8452_data *data = iio_priv(indio_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; > > + } > > + } > > + > > + ret = mma8452_active(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; > > } > > #endif > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH V4 1/2] dt-bindings: iio: accel: mma8452: add optional power supply property 2018-12-11 5:13 [PATCH V4 1/2] dt-bindings: iio: accel: mma8452: add optional power supply property Anson Huang 2018-12-11 5:13 ` [PATCH V4 2/2] iio: accell: mma8452: add optional vdd/vddio regulator operation support Anson Huang @ 2018-12-16 10:46 ` Jonathan Cameron 1 sibling, 0 replies; 5+ messages in thread From: Jonathan Cameron @ 2018-12-16 10:46 UTC (permalink / raw) To: Anson Huang Cc: knaack.h@gmx.de, lars@metafoo.de, pmeerw@pmeerw.net, robh+dt@kernel.org, mark.rutland@arm.com, gregkh@linuxfoundation.org, martink@posteo.de, Leonard Crestez, rtresidd@electromag.com.au, gustavo@embeddedor.com, linux-iio@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, festevam@gmail.com, preid@electromag.com.au, dl-linux-imx On Tue, 11 Dec 2018 05:13:14 +0000 Anson Huang <anson.huang@nxp.com> wrote: > The accelerometer's power supply could be controlled by regulator > on some platforms, add optional property "vdd/vddio" power supply > to let device tree to pass phandles to the regulators to driver. > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com> Applied to the togreg branch of iio.git and pushed out as testing for the autobuilders to play with it. I'll not be pushing this out as non rebasing for a week or so hence plenty of time to add additional reviews. Thanks, Jonathan > --- > Documentation/devicetree/bindings/iio/accel/mma8452.txt | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/Documentation/devicetree/bindings/iio/accel/mma8452.txt b/Documentation/devicetree/bindings/iio/accel/mma8452.txt > index 2100e9a..e132394 100644 > --- a/Documentation/devicetree/bindings/iio/accel/mma8452.txt > +++ b/Documentation/devicetree/bindings/iio/accel/mma8452.txt > @@ -20,6 +20,10 @@ Optional properties: > - interrupt-names: should contain "INT1" and/or "INT2", the accelerometer's > interrupt line in use. > > + - vdd-supply: phandle to the regulator that provides vdd power to the accelerometer. > + > + - vddio-supply: phandle to the regulator that provides vddio power to the accelerometer. > + > Example: > > mma8453fc@1d { ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-12-16 14:08 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-12-11 5:13 [PATCH V4 1/2] dt-bindings: iio: accel: mma8452: add optional power supply property Anson Huang 2018-12-11 5:13 ` [PATCH V4 2/2] iio: accell: mma8452: add optional vdd/vddio regulator operation support Anson Huang 2018-12-16 10:46 ` Jonathan Cameron 2018-12-16 14:08 ` Jonathan Cameron 2018-12-16 10:46 ` [PATCH V4 1/2] dt-bindings: iio: accel: mma8452: add optional power supply property Jonathan Cameron
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.