| Message ID | 20200414153415.957-3-mani@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add Reset and Wakeup support for CCS811 | expand |
On Tue, 14 Apr 2020, mani@kernel.org wrote: > From: Manivannan Sadhasivam <mani@kernel.org> comments below > CCS811 VOC sensor exposes nRESET and nWAKE pins which can be connected > to GPIO pins of the host controller. These pins can be used to externally > release the device from reset and also to wake it up before any I2C > transaction. The initial driver support assumed that the nRESET pin is not > connected and the nWAKE pin is tied to ground. > > This commit improves it by adding support for controlling those two pins > externally using a host controller. For the case of reset, if the hardware > reset is not available, the mechanism to do software reset is also added. > > As a side effect of doing this, the IIO device allocation needs to be > slightly moved to top of probe to make use of priv data early. > > Signed-off-by: Manivannan Sadhasivam <mani@kernel.org> > --- > drivers/iio/chemical/ccs811.c | 88 +++++++++++++++++++++++++++++++---- > 1 file changed, 80 insertions(+), 8 deletions(-) > > diff --git a/drivers/iio/chemical/ccs811.c b/drivers/iio/chemical/ccs811.c > index 2ebdfc35bcda..951358710f64 100644 > --- a/drivers/iio/chemical/ccs811.c > +++ b/drivers/iio/chemical/ccs811.c > @@ -16,6 +16,7 @@ > */ > > #include <linux/delay.h> > +#include <linux/gpio/consumer.h> > #include <linux/i2c.h> > #include <linux/iio/iio.h> > #include <linux/iio/buffer.h> > @@ -36,6 +37,7 @@ > #define CCS811_ERR 0xE0 > /* Used to transition from boot to application mode */ > #define CCS811_APP_START 0xF4 > +#define CCS811_SW_RESET 0xFF > > /* Status register flags */ > #define CCS811_STATUS_ERROR BIT(0) > @@ -74,6 +76,7 @@ struct ccs811_data { > struct mutex lock; /* Protect readings */ > struct ccs811_reading buffer; > struct iio_trigger *drdy_trig; > + struct gpio_desc *wakeup_gpio; > bool drdy_trig_on; > }; > > @@ -166,10 +169,25 @@ static int ccs811_setup(struct i2c_client *client) > CCS811_MODE_IAQ_1SEC); > } > > +static void ccs811_set_wakeup(struct ccs811_data *data, bool enable) > +{ > + if (!data->wakeup_gpio) > + return; > + > + gpiod_set_value(data->wakeup_gpio, enable); > + > + if (enable) > + usleep_range(50, 60); > + else > + usleep_range(20, 30); > +} > + > static int ccs811_get_measurement(struct ccs811_data *data) > { > int ret, tries = 11; > > + ccs811_set_wakeup(data, true); > + > /* Maximum waiting time: 1s, as measurements are made every second */ > while (tries-- > 0) { > ret = i2c_smbus_read_byte_data(data->client, CCS811_STATUS); > @@ -183,9 +201,12 @@ static int ccs811_get_measurement(struct ccs811_data *data) > if (!(ret & CCS811_STATUS_DATA_READY)) > return -EIO; > > - return i2c_smbus_read_i2c_block_data(data->client, > + ret = i2c_smbus_read_i2c_block_data(data->client, > CCS811_ALG_RESULT_DATA, 8, > (char *)&data->buffer); > + ccs811_set_wakeup(data, false); shouldn't the ccs811_set_wakeup(data, false) call be added to all potential paths leaving the function? in particular when there is a timeout or reading the status fails? > + > + return ret; > } > > static int ccs811_read_raw(struct iio_dev *indio_dev, > @@ -336,6 +357,42 @@ static irqreturn_t ccs811_data_rdy_trigger_poll(int irq, void *private) > return IRQ_HANDLED; > } > > +static int ccs811_reset(struct i2c_client *client) > +{ > + struct gpio_desc *reset_gpio; > + int ret; > + > + reset_gpio = devm_gpiod_get_optional(&client->dev, "reset", > + GPIOD_OUT_LOW); > + if (IS_ERR(reset_gpio)) { > + dev_err(&client->dev, "Failed to acquire reset gpio\n"); > + return PTR_ERR(reset_gpio); > + } > + > + /* Try to reset using nRESET pin if available else do SW reset */ > + if (reset_gpio) { > + gpiod_set_value(reset_gpio, 1); > + usleep_range(20, 30); > + gpiod_set_value(reset_gpio, 0); > + } else { > + static const u8 reset_seq[] = { > + 0xFF, 0x11, 0xE5, 0x72, 0x8A, > + }; > + > + ret = i2c_smbus_write_i2c_block_data(client, CCS811_SW_RESET, > + sizeof(reset_seq), reset_seq); > + if (ret < 0) { > + dev_err(&client->dev, "Failed to reset sensor\n"); > + return ret; > + } > + } > + > + /* tSTART delay required after reset */ > + usleep_range(1000, 2000); > + > + return 0; > +} > + > static int ccs811_probe(struct i2c_client *client, > const struct i2c_device_id *id) > { > @@ -348,6 +405,27 @@ static int ccs811_probe(struct i2c_client *client, > | I2C_FUNC_SMBUS_READ_I2C_BLOCK)) > return -EOPNOTSUPP; > > + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); > + if (!indio_dev) > + return -ENOMEM; > + > + data = iio_priv(indio_dev); > + i2c_set_clientdata(client, indio_dev); > + data->client = client; > + > + data->wakeup_gpio = devm_gpiod_get_optional(&client->dev, "wakeup", > + GPIOD_OUT_HIGH); > + if (IS_ERR(data->wakeup_gpio)) { > + dev_err(&client->dev, "Failed to acquire wakeup gpio\n"); > + return PTR_ERR(data->wakeup_gpio); > + } > + > + ccs811_set_wakeup(data, true); > + > + ret = ccs811_reset(client); > + if (ret) ccs811_set_wakeup(data, false) missing here? and in other error paths? > + return ret; > + > /* Check hardware id (should be 0x81 for this family of devices) */ > ret = i2c_smbus_read_byte_data(client, CCS811_HW_ID); > if (ret < 0) > @@ -367,17 +445,11 @@ static int ccs811_probe(struct i2c_client *client, > return -ENODEV; > } > > - indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); > - if (!indio_dev) > - return -ENOMEM; > - > ret = ccs811_setup(client); > if (ret < 0) > return ret; > > - data = iio_priv(indio_dev); > - i2c_set_clientdata(client, indio_dev); > - data->client = client; > + ccs811_set_wakeup(data, false); > > mutex_init(&data->lock); > >
On Tue, Apr 14, 2020 at 6:34 PM <mani@kernel.org> wrote: > > From: Manivannan Sadhasivam <mani@kernel.org> > > CCS811 VOC sensor exposes nRESET and nWAKE pins which can be connected > to GPIO pins of the host controller. These pins can be used to externally > release the device from reset and also to wake it up before any I2C > transaction. The initial driver support assumed that the nRESET pin is not > connected and the nWAKE pin is tied to ground. > > This commit improves it by adding support for controlling those two pins > externally using a host controller. For the case of reset, if the hardware > reset is not available, the mechanism to do software reset is also added. > > As a side effect of doing this, the IIO device allocation needs to be > slightly moved to top of probe to make use of priv data early. Thank you for an update, my comments below. ... > + reset_gpio = devm_gpiod_get_optional(&client->dev, "reset", > + GPIOD_OUT_LOW); > + if (IS_ERR(reset_gpio)) { > + dev_err(&client->dev, "Failed to acquire reset gpio\n"); If it's a deferred probe, it would spam the log. > + return PTR_ERR(reset_gpio); > + } ... > + static const u8 reset_seq[] = { > + 0xFF, 0x11, 0xE5, 0x72, 0x8A, > + }; I would suggest to comment above from where you got this and the meaning of the numbers. ... > + data->wakeup_gpio = devm_gpiod_get_optional(&client->dev, "wakeup", > + GPIOD_OUT_HIGH); > + if (IS_ERR(data->wakeup_gpio)) { > + dev_err(&client->dev, "Failed to acquire wakeup gpio\n"); Ditto. > + return PTR_ERR(data->wakeup_gpio); > + }
On Tue, Apr 14, 2020 at 06:34:54PM +0200, Peter Meerwald-Stadler wrote: > On Tue, 14 Apr 2020, mani@kernel.org wrote: > > > From: Manivannan Sadhasivam <mani@kernel.org> > > comments below > > > CCS811 VOC sensor exposes nRESET and nWAKE pins which can be connected > > to GPIO pins of the host controller. These pins can be used to externally > > release the device from reset and also to wake it up before any I2C > > transaction. The initial driver support assumed that the nRESET pin is not > > connected and the nWAKE pin is tied to ground. > > > > This commit improves it by adding support for controlling those two pins > > externally using a host controller. For the case of reset, if the hardware > > reset is not available, the mechanism to do software reset is also added. > > > > As a side effect of doing this, the IIO device allocation needs to be > > slightly moved to top of probe to make use of priv data early. > > > > Signed-off-by: Manivannan Sadhasivam <mani@kernel.org> > > --- > > drivers/iio/chemical/ccs811.c | 88 +++++++++++++++++++++++++++++++---- > > 1 file changed, 80 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/iio/chemical/ccs811.c b/drivers/iio/chemical/ccs811.c > > index 2ebdfc35bcda..951358710f64 100644 > > --- a/drivers/iio/chemical/ccs811.c > > +++ b/drivers/iio/chemical/ccs811.c > > @@ -16,6 +16,7 @@ > > */ > > > > #include <linux/delay.h> > > +#include <linux/gpio/consumer.h> > > #include <linux/i2c.h> > > #include <linux/iio/iio.h> > > #include <linux/iio/buffer.h> > > @@ -36,6 +37,7 @@ > > #define CCS811_ERR 0xE0 > > /* Used to transition from boot to application mode */ > > #define CCS811_APP_START 0xF4 > > +#define CCS811_SW_RESET 0xFF > > > > /* Status register flags */ > > #define CCS811_STATUS_ERROR BIT(0) > > @@ -74,6 +76,7 @@ struct ccs811_data { > > struct mutex lock; /* Protect readings */ > > struct ccs811_reading buffer; > > struct iio_trigger *drdy_trig; > > + struct gpio_desc *wakeup_gpio; > > bool drdy_trig_on; > > }; > > > > @@ -166,10 +169,25 @@ static int ccs811_setup(struct i2c_client *client) > > CCS811_MODE_IAQ_1SEC); > > } > > > > +static void ccs811_set_wakeup(struct ccs811_data *data, bool enable) > > +{ > > + if (!data->wakeup_gpio) > > + return; > > + > > + gpiod_set_value(data->wakeup_gpio, enable); > > + > > + if (enable) > > + usleep_range(50, 60); > > + else > > + usleep_range(20, 30); > > +} > > + > > static int ccs811_get_measurement(struct ccs811_data *data) > > { > > int ret, tries = 11; > > > > + ccs811_set_wakeup(data, true); > > + > > /* Maximum waiting time: 1s, as measurements are made every second */ > > while (tries-- > 0) { > > ret = i2c_smbus_read_byte_data(data->client, CCS811_STATUS); > > @@ -183,9 +201,12 @@ static int ccs811_get_measurement(struct ccs811_data *data) > > if (!(ret & CCS811_STATUS_DATA_READY)) > > return -EIO; > > > > - return i2c_smbus_read_i2c_block_data(data->client, > > + ret = i2c_smbus_read_i2c_block_data(data->client, > > CCS811_ALG_RESULT_DATA, 8, > > (char *)&data->buffer); > > + ccs811_set_wakeup(data, false); > > shouldn't the ccs811_set_wakeup(data, false) call be added to all > potential paths leaving the function? > in particular when there is a timeout or reading the status fails? > Makes sense. I didn't thought about it since the current driver doesn't support pm runtime. But anyway adding this will save power. Will do. Thanks, Mani > > + > > + return ret; > > } > > > > static int ccs811_read_raw(struct iio_dev *indio_dev, > > @@ -336,6 +357,42 @@ static irqreturn_t ccs811_data_rdy_trigger_poll(int irq, void *private) > > return IRQ_HANDLED; > > } > > > > +static int ccs811_reset(struct i2c_client *client) > > +{ > > + struct gpio_desc *reset_gpio; > > + int ret; > > + > > + reset_gpio = devm_gpiod_get_optional(&client->dev, "reset", > > + GPIOD_OUT_LOW); > > + if (IS_ERR(reset_gpio)) { > > + dev_err(&client->dev, "Failed to acquire reset gpio\n"); > > + return PTR_ERR(reset_gpio); > > + } > > + > > + /* Try to reset using nRESET pin if available else do SW reset */ > > + if (reset_gpio) { > > + gpiod_set_value(reset_gpio, 1); > > + usleep_range(20, 30); > > + gpiod_set_value(reset_gpio, 0); > > + } else { > > + static const u8 reset_seq[] = { > > + 0xFF, 0x11, 0xE5, 0x72, 0x8A, > > + }; > > + > > + ret = i2c_smbus_write_i2c_block_data(client, CCS811_SW_RESET, > > + sizeof(reset_seq), reset_seq); > > + if (ret < 0) { > > + dev_err(&client->dev, "Failed to reset sensor\n"); > > + return ret; > > + } > > + } > > + > > + /* tSTART delay required after reset */ > > + usleep_range(1000, 2000); > > + > > + return 0; > > +} > > + > > static int ccs811_probe(struct i2c_client *client, > > const struct i2c_device_id *id) > > { > > @@ -348,6 +405,27 @@ static int ccs811_probe(struct i2c_client *client, > > | I2C_FUNC_SMBUS_READ_I2C_BLOCK)) > > return -EOPNOTSUPP; > > > > + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); > > + if (!indio_dev) > > + return -ENOMEM; > > + > > + data = iio_priv(indio_dev); > > + i2c_set_clientdata(client, indio_dev); > > + data->client = client; > > + > > + data->wakeup_gpio = devm_gpiod_get_optional(&client->dev, "wakeup", > > + GPIOD_OUT_HIGH); > > + if (IS_ERR(data->wakeup_gpio)) { > > + dev_err(&client->dev, "Failed to acquire wakeup gpio\n"); > > + return PTR_ERR(data->wakeup_gpio); > > + } > > + > > + ccs811_set_wakeup(data, true); > > + > > + ret = ccs811_reset(client); > > + if (ret) > > ccs811_set_wakeup(data, false) missing here? and in other error paths? > > > + return ret; > > + > > /* Check hardware id (should be 0x81 for this family of devices) */ > > ret = i2c_smbus_read_byte_data(client, CCS811_HW_ID); > > if (ret < 0) > > @@ -367,17 +445,11 @@ static int ccs811_probe(struct i2c_client *client, > > return -ENODEV; > > } > > > > - indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); > > - if (!indio_dev) > > - return -ENOMEM; > > - > > ret = ccs811_setup(client); > > if (ret < 0) > > return ret; > > > > - data = iio_priv(indio_dev); > > - i2c_set_clientdata(client, indio_dev); > > - data->client = client; > > + ccs811_set_wakeup(data, false); > > > > mutex_init(&data->lock); > > > > > > -- > > Peter Meerwald-Stadler > Mobile: +43 664 24 44 418
On Tue, Apr 14, 2020 at 07:42:24PM +0300, Andy Shevchenko wrote: > On Tue, Apr 14, 2020 at 6:34 PM <mani@kernel.org> wrote: > > > > From: Manivannan Sadhasivam <mani@kernel.org> > > > > CCS811 VOC sensor exposes nRESET and nWAKE pins which can be connected > > to GPIO pins of the host controller. These pins can be used to externally > > release the device from reset and also to wake it up before any I2C > > transaction. The initial driver support assumed that the nRESET pin is not > > connected and the nWAKE pin is tied to ground. > > > > This commit improves it by adding support for controlling those two pins > > externally using a host controller. For the case of reset, if the hardware > > reset is not available, the mechanism to do software reset is also added. > > > > As a side effect of doing this, the IIO device allocation needs to be > > slightly moved to top of probe to make use of priv data early. > > Thank you for an update, my comments below. > > ... > > > + reset_gpio = devm_gpiod_get_optional(&client->dev, "reset", > > + GPIOD_OUT_LOW); > > + if (IS_ERR(reset_gpio)) { > > > + dev_err(&client->dev, "Failed to acquire reset gpio\n"); > > If it's a deferred probe, it would spam the log. > Hmm. But error is an error isn't it? Would you recommend doing a debug print or completely removing the logging? > > + return PTR_ERR(reset_gpio); > > + } > > ... > > > + static const u8 reset_seq[] = { > > + 0xFF, 0x11, 0xE5, 0x72, 0x8A, > > + }; > > I would suggest to comment above from where you got this and the > meaning of the numbers. > The datasheet doesn't specify the meaning of these values. But will add a comment. Btw, just noticed that 0xFF is not needed and only 4 values are sufficient for SW reset. Thanks, Mani > ... > > > + data->wakeup_gpio = devm_gpiod_get_optional(&client->dev, "wakeup", > > + GPIOD_OUT_HIGH); > > + if (IS_ERR(data->wakeup_gpio)) { > > > + dev_err(&client->dev, "Failed to acquire wakeup gpio\n"); > > Ditto. > > > + return PTR_ERR(data->wakeup_gpio); > > + } > > -- > With Best Regards, > Andy Shevchenko
On Tue, Apr 14, 2020 at 8:06 PM Manivannan Sadhasivam <mani@kernel.org> wrote: > On Tue, Apr 14, 2020 at 07:42:24PM +0300, Andy Shevchenko wrote: > > On Tue, Apr 14, 2020 at 6:34 PM <mani@kernel.org> wrote: ... > > > + reset_gpio = devm_gpiod_get_optional(&client->dev, "reset", > > > + GPIOD_OUT_LOW); > > > + if (IS_ERR(reset_gpio)) { > > > > > + dev_err(&client->dev, "Failed to acquire reset gpio\n"); > > > > If it's a deferred probe, it would spam the log. > > > > Hmm. But error is an error isn't it? Would you recommend doing a debug print > or completely removing the logging? I would remove completely, but better to wait for Jonathan to comment. Maybe he prefers something like if (err != EPROBE_DEFER) dev_err() > > > + return PTR_ERR(reset_gpio); > > > + } > > > > ... > > > > > + static const u8 reset_seq[] = { > > > + 0xFF, 0x11, 0xE5, 0x72, 0x8A, > > > + }; > > > > I would suggest to comment above from where you got this and the > > meaning of the numbers. > > > > The datasheet doesn't specify the meaning of these values. But will add a > comment. Thanks! > Btw, just noticed that 0xFF is not needed and only 4 values are > sufficient for SW reset. Better to do exactly what datasheet suggests (in case it's not clear or deductible what is going on).
On Tue, 14 Apr 2020 20:54:49 +0300 Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Tue, Apr 14, 2020 at 8:06 PM Manivannan Sadhasivam <mani@kernel.org> wrote: > > On Tue, Apr 14, 2020 at 07:42:24PM +0300, Andy Shevchenko wrote: > > > On Tue, Apr 14, 2020 at 6:34 PM <mani@kernel.org> wrote: > > ... > > > > > + reset_gpio = devm_gpiod_get_optional(&client->dev, "reset", > > > > + GPIOD_OUT_LOW); > > > > + if (IS_ERR(reset_gpio)) { > > > > > > > + dev_err(&client->dev, "Failed to acquire reset gpio\n"); > > > > > > If it's a deferred probe, it would spam the log. > > > > > > > Hmm. But error is an error isn't it? Would you recommend doing a debug print > > or completely removing the logging? > > I would remove completely, but better to wait for Jonathan to comment. > Maybe he prefers something like > if (err != EPROBE_DEFER) > dev_err() Not fussed. Either drop it or do the check for defer - one I leave up to the author. > > > > > + return PTR_ERR(reset_gpio); > > > > + } > > > > > > ... > > > > > > > + static const u8 reset_seq[] = { > > > > + 0xFF, 0x11, 0xE5, 0x72, 0x8A, > > > > + }; > > > > > > I would suggest to comment above from where you got this and the > > > meaning of the numbers. > > > > > > > The datasheet doesn't specify the meaning of these values. But will add a > > comment. > > Thanks! > > > Btw, just noticed that 0xFF is not needed and only 4 values are > > sufficient for SW reset. > > Better to do exactly what datasheet suggests (in case it's not clear > or deductible what is going on). >
On Tue, Apr 14, 2020 at 07:12:59PM +0100, Jonathan Cameron wrote: > On Tue, 14 Apr 2020 20:54:49 +0300 > Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > > On Tue, Apr 14, 2020 at 8:06 PM Manivannan Sadhasivam <mani@kernel.org> wrote: > > > On Tue, Apr 14, 2020 at 07:42:24PM +0300, Andy Shevchenko wrote: > > > > On Tue, Apr 14, 2020 at 6:34 PM <mani@kernel.org> wrote: > > > > ... > > > > > > > + reset_gpio = devm_gpiod_get_optional(&client->dev, "reset", > > > > > + GPIOD_OUT_LOW); > > > > > + if (IS_ERR(reset_gpio)) { > > > > > > > > > + dev_err(&client->dev, "Failed to acquire reset gpio\n"); > > > > > > > > If it's a deferred probe, it would spam the log. > > > > > > > > > > Hmm. But error is an error isn't it? Would you recommend doing a debug print > > > or completely removing the logging? > > > > I would remove completely, but better to wait for Jonathan to comment. > > Maybe he prefers something like > > if (err != EPROBE_DEFER) > > dev_err() > > Not fussed. Either drop it or do the check for defer - one I leave up to the > author. > Okay, will just drop it. Thanks, Mani > > > > > > > + return PTR_ERR(reset_gpio); > > > > > + } > > > > > > > > ... > > > > > > > > > + static const u8 reset_seq[] = { > > > > > + 0xFF, 0x11, 0xE5, 0x72, 0x8A, > > > > > + }; > > > > > > > > I would suggest to comment above from where you got this and the > > > > meaning of the numbers. > > > > > > > > > > The datasheet doesn't specify the meaning of these values. But will add a > > > comment. > > > > Thanks! > > > > > Btw, just noticed that 0xFF is not needed and only 4 values are > > > sufficient for SW reset. > > > > Better to do exactly what datasheet suggests (in case it's not clear > > or deductible what is going on). > > >
diff --git a/drivers/iio/chemical/ccs811.c b/drivers/iio/chemical/ccs811.c index 2ebdfc35bcda..951358710f64 100644 --- a/drivers/iio/chemical/ccs811.c +++ b/drivers/iio/chemical/ccs811.c @@ -16,6 +16,7 @@ */ #include <linux/delay.h> +#include <linux/gpio/consumer.h> #include <linux/i2c.h> #include <linux/iio/iio.h> #include <linux/iio/buffer.h> @@ -36,6 +37,7 @@ #define CCS811_ERR 0xE0 /* Used to transition from boot to application mode */ #define CCS811_APP_START 0xF4 +#define CCS811_SW_RESET 0xFF /* Status register flags */ #define CCS811_STATUS_ERROR BIT(0) @@ -74,6 +76,7 @@ struct ccs811_data { struct mutex lock; /* Protect readings */ struct ccs811_reading buffer; struct iio_trigger *drdy_trig; + struct gpio_desc *wakeup_gpio; bool drdy_trig_on; }; @@ -166,10 +169,25 @@ static int ccs811_setup(struct i2c_client *client) CCS811_MODE_IAQ_1SEC); } +static void ccs811_set_wakeup(struct ccs811_data *data, bool enable) +{ + if (!data->wakeup_gpio) + return; + + gpiod_set_value(data->wakeup_gpio, enable); + + if (enable) + usleep_range(50, 60); + else + usleep_range(20, 30); +} + static int ccs811_get_measurement(struct ccs811_data *data) { int ret, tries = 11; + ccs811_set_wakeup(data, true); + /* Maximum waiting time: 1s, as measurements are made every second */ while (tries-- > 0) { ret = i2c_smbus_read_byte_data(data->client, CCS811_STATUS); @@ -183,9 +201,12 @@ static int ccs811_get_measurement(struct ccs811_data *data) if (!(ret & CCS811_STATUS_DATA_READY)) return -EIO; - return i2c_smbus_read_i2c_block_data(data->client, + ret = i2c_smbus_read_i2c_block_data(data->client, CCS811_ALG_RESULT_DATA, 8, (char *)&data->buffer); + ccs811_set_wakeup(data, false); + + return ret; } static int ccs811_read_raw(struct iio_dev *indio_dev, @@ -336,6 +357,42 @@ static irqreturn_t ccs811_data_rdy_trigger_poll(int irq, void *private) return IRQ_HANDLED; } +static int ccs811_reset(struct i2c_client *client) +{ + struct gpio_desc *reset_gpio; + int ret; + + reset_gpio = devm_gpiod_get_optional(&client->dev, "reset", + GPIOD_OUT_LOW); + if (IS_ERR(reset_gpio)) { + dev_err(&client->dev, "Failed to acquire reset gpio\n"); + return PTR_ERR(reset_gpio); + } + + /* Try to reset using nRESET pin if available else do SW reset */ + if (reset_gpio) { + gpiod_set_value(reset_gpio, 1); + usleep_range(20, 30); + gpiod_set_value(reset_gpio, 0); + } else { + static const u8 reset_seq[] = { + 0xFF, 0x11, 0xE5, 0x72, 0x8A, + }; + + ret = i2c_smbus_write_i2c_block_data(client, CCS811_SW_RESET, + sizeof(reset_seq), reset_seq); + if (ret < 0) { + dev_err(&client->dev, "Failed to reset sensor\n"); + return ret; + } + } + + /* tSTART delay required after reset */ + usleep_range(1000, 2000); + + return 0; +} + static int ccs811_probe(struct i2c_client *client, const struct i2c_device_id *id) { @@ -348,6 +405,27 @@ static int ccs811_probe(struct i2c_client *client, | I2C_FUNC_SMBUS_READ_I2C_BLOCK)) return -EOPNOTSUPP; + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); + if (!indio_dev) + return -ENOMEM; + + data = iio_priv(indio_dev); + i2c_set_clientdata(client, indio_dev); + data->client = client; + + data->wakeup_gpio = devm_gpiod_get_optional(&client->dev, "wakeup", + GPIOD_OUT_HIGH); + if (IS_ERR(data->wakeup_gpio)) { + dev_err(&client->dev, "Failed to acquire wakeup gpio\n"); + return PTR_ERR(data->wakeup_gpio); + } + + ccs811_set_wakeup(data, true); + + ret = ccs811_reset(client); + if (ret) + return ret; + /* Check hardware id (should be 0x81 for this family of devices) */ ret = i2c_smbus_read_byte_data(client, CCS811_HW_ID); if (ret < 0) @@ -367,17 +445,11 @@ static int ccs811_probe(struct i2c_client *client, return -ENODEV; } - indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); - if (!indio_dev) - return -ENOMEM; - ret = ccs811_setup(client); if (ret < 0) return ret; - data = iio_priv(indio_dev); - i2c_set_clientdata(client, indio_dev); - data->client = client; + ccs811_set_wakeup(data, false); mutex_init(&data->lock);