Add Reset and Wakeup support for CCS811
Subsystems (2)
cover Add Reset and Wakeup support for CCS811
From: Manivannan Sadhasivam <[email protected]>
Hello,
This patchset adds external reset and wakeup support for CCS811 VOC gas sensor. The nRESET and nWAKE pins available on the device are utilised to provide these functionalities.
The patchset has been validated using CCS811 connected to STM32MP1 based board over I2C.
While at it, the devicetree yaml binding and OF match table for this sensor are also added.
Thanks,
Mani
Manivannan Sadhasivam (3):
dt-bindings: iio: chemical: Add binding for CCS811 VOC sensor
iio: chemical: Add support for external Reset and Wakeup in CCS811
iio: chemical: Add OF match table for CCS811 VOC sensor
.../bindings/iio/chemical/ams,ccs811.yaml | 50 ++++++++++ drivers/iio/chemical/ccs811.c | 96 +++++++++++++++++-- 2 files changed, 138 insertions(+), 8 deletions(-) create mode 100644 Documentation/devicetree/bindings/iio/chemical/ams,ccs811.yaml
Signature · 1 line
2.17.1
PATCH 1/3 dt-bindings: iio: chemical: Add binding for CCS811 VOC sensor
From: Manivannan Sadhasivam <[email protected]>
This commit adds devicetree binding for AMS CCS811 VOC sensor.
--- .../bindings/iio/chemical/ams,ccs811.yaml | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/chemical/ams,ccs811.yaml
Documentation/devicetree/bindings/iio/chemical/ams,ccs811.yaml new file +50 -0
@@ -0,0 +1,50 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/chemical/ams,ccs811.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: AMS CCS811 VOC Sensor + +maintainers: + - Narcisa Vasile <[email protected]> + +description: | + Ultra-Low Power Digital Gas Sensor for Monitoring Indoor Air Quality. + +properties: + compatible: + enum: + - ams,ccs811 + reg: + maxItems: 1 + + reset-gpios: + description: GPIO connected to the nRESET line. This is an active low + input to CCS811. + maxItems: 1 + + wakeup-gpios: + description: GPIO connected to the nWAKE line. This is an active low + input to CCS811. + maxItems: 1 + +required: + - compatible + - reg + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + ccs811@5b { + compatible = "ams,ccs811"; + reg = <0x5b>; + reset-gpios = <&gpioa 11 GPIO_ACTIVE_LOW>; + wakeup-gpios = <&gpioa 12 GPIO_ACTIVE_LOW>; + }; + }; + +...
Signature · 1 line
2.17.1
Jonathan Cameron
maintainerRe: [PATCH 1/3] dt-bindings: iio: chemical: Add binding for CCS811 VOC sensor
Quoted 5 lines · On Mon, 13 Apr 2020 00:06:56 +0530 [email protected] wrote:
From: Manivannan Sadhasivam <[email protected]>
This commit adds devicetree binding for AMS CCS811 VOC sensor.
Signed-off-by Manivannan Sadhasivam <[email protected]>
Great to see this driver getting some more attention.
A few things inline
Thanks,
Jonathan
Documentation/devicetree/bindings/iio/chemical/ams,ccs811.yaml line 1
Quoted 12 lines
--- .../bindings/iio/chemical/ams,ccs811.yaml | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/chemical/ams,ccs811.yamlDocumentation/devicetree/bindings/iio/chemical/ams,ccs811.yaml new file +1
-0@@ -0,0 +1,50 @@ +# SPDX-License-Identifier: GPL-2.0
Preferred that all new binding docs are dual licensed with BSD as well to allow there use by things other than the kernel.
Quoted 31 lines
Documentation/devicetree/bindings/iio/chemical/ams,ccs811.yaml new file +31
-0+%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/chemical/ams,ccs811.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: AMS CCS811 VOC Sensor + +maintainers: + - Narcisa Vasile <[email protected]> + +description: | + Ultra-Low Power Digital Gas Sensor for Monitoring Indoor Air Quality. + +properties: + compatible: + enum: + - ams,ccs811 + reg: + maxItems: 1 + + reset-gpios: + description: GPIO connected to the nRESET line. This is an active low + input to CCS811. + maxItems: 1 + + wakeup-gpios: + description: GPIO connected to the nWAKE line. This is an active low + input to CCS811. + maxItems: 1 + +required:
Documentation/devicetree/bindings/iio/chemical/ams,ccs811.yaml line 42
Quoted 10 lines
Documentation/devicetree/bindings/iio/chemical/ams,ccs811.yaml new file +10
-0+ - compatible + - reg + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + ccs811@5b {
Hmm. Not sure we have a generic name in the dt spec for voc sensor. Even so we should try for something appropriate.
Could go with voc@5b?
Quoted 8 lines
Documentation/devicetree/bindings/iio/chemical/ams,ccs811.yaml new file +8
-0+ compatible = "ams,ccs811"; + reg = <0x5b>; + reset-gpios = <&gpioa 11 GPIO_ACTIVE_LOW>; + wakeup-gpios = <&gpioa 12 GPIO_ACTIVE_LOW>; + }; + }; + +...
Re: [PATCH 1/3] dt-bindings: iio: chemical: Add binding for CCS811 VOC sensor
Quoted 21 lines · Jonathan Cameron
Quoted 5 lines · On Mon, 13 Apr 2020 00:06:56 +0530 [email protected] wrote:
From: Manivannan Sadhasivam <[email protected]>
This commit adds devicetree binding for AMS CCS811 VOC sensor.
Signed-off-by Manivannan Sadhasivam <[email protected]>Great to see this driver getting some more attention.
A few things inline
Thanks,
Jonathan
Quoted 5 lines
--- .../bindings/iio/chemical/ams,ccs811.yaml | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/chemical/ams,ccs811.yaml
Quoted 11 lines
Documentation/devicetree/bindings/iio/chemical/ams,ccs811.yaml line 1
Quoted 7 lines
Documentation/devicetree/bindings/iio/chemical/ams,ccs811.yaml new file +1
-0@@ -0,0 +1,50 @@ +# SPDX-License-Identifier: GPL-2.0Preferred that all new binding docs are dual licensed with BSD as well to allow there use by things other than the kernel.
Okay will do.
Quoted 44 lines
Quoted 31 lines
Documentation/devicetree/bindings/iio/chemical/ams,ccs811.yaml new file +31
-0+%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/chemical/ams,ccs811.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: AMS CCS811 VOC Sensor + +maintainers: + - Narcisa Vasile <[email protected]> + +description: | + Ultra-Low Power Digital Gas Sensor for Monitoring Indoor Air Quality. + +properties: + compatible: + enum: + - ams,ccs811 + reg: + maxItems: 1 + + reset-gpios: + description: GPIO connected to the nRESET line. This is an active low + input to CCS811. + maxItems: 1 + + wakeup-gpios: + description: GPIO connected to the nWAKE line. This is an active low + input to CCS811. + maxItems: 1 + +required:Documentation/devicetree/bindings/iio/chemical/ams,ccs811.yaml line 42
Quoted 10 lines
Documentation/devicetree/bindings/iio/chemical/ams,ccs811.yaml new file +10
-0+ - compatible + - reg + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + ccs811@5b {Hmm. Not sure we have a generic name in the dt spec for voc sensor. Even so we should try for something appropriate.
Quoted 2 lines
Could go with voc@5b?
Sounds good to me.
Thanks,
Mani
Quoted 9 lines
Quoted 8 lines
Documentation/devicetree/bindings/iio/chemical/ams,ccs811.yaml new file +8
-0+ compatible = "ams,ccs811"; + reg = <0x5b>; + reset-gpios = <&gpioa 11 GPIO_ACTIVE_LOW>; + wakeup-gpios = <&gpioa 12 GPIO_ACTIVE_LOW>; + }; + }; + +...
Rob Herring
maintainerRe: [PATCH 1/3] dt-bindings: iio: chemical: Add binding for CCS811 VOC sensor
In reply to [email protected]Quoted 10 lines · On Mon, 13 Apr 2020 00:06:56 +0530, [email protected] wrote:
From: Manivannan Sadhasivam <[email protected]>
This commit adds devicetree binding for AMS CCS811 VOC sensor.
Signed-off-by Manivannan Sadhasivam <[email protected]>--- .../bindings/iio/chemical/ams,ccs811.yaml | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/chemical/ams,ccs811.yaml
My bot found errors running 'make dt_binding_check' on your patch:
Error: Documentation/devicetree/bindings/iio/chemical/ams,ccs811.example.dts:24.38-39 syntax error
FATAL ERROR: Unable to parse input treescripts/Makefile.lib:311: recipe for target 'Documentation/devicetree/bindings/iio/chemical/ams,ccs811.example.dt.yaml' failed make[1]: *** [Documentation/devicetree/bindings/iio/chemical/ams,ccs811.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
Makefile:1262: recipe for target 'dt_binding_check' failed
make: *** [dt_binding_check] Error 2
See https://patchwork.ozlabs.org/patch/1269473
If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure dt-schema is up to date:
pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade
Please check and re-submit.
Re: [PATCH 1/3] dt-bindings: iio: chemical: Add binding for CCS811 VOC sensor
Quoted 22 lines · Rob Herring
Quoted 10 lines · On Mon, 13 Apr 2020 00:06:56 +0530, [email protected] wrote:
From: Manivannan Sadhasivam <[email protected]>
This commit adds devicetree binding for AMS CCS811 VOC sensor.
Signed-off-by Manivannan Sadhasivam <[email protected]>--- .../bindings/iio/chemical/ams,ccs811.yaml | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/chemical/ams,ccs811.yamlMy bot found errors running 'make
dt_binding_check' on your patch:Error: Documentation/devicetree/bindings/iio/chemical/ams,ccs811.example.dts:24.38-39 syntax error
FATAL ERROR: Unable to parse input treescripts/Makefile.lib:311: recipe for target 'Documentation/devicetree/bindings/iio/chemical/ams,ccs811.example.dt.yaml' failed make[1]: *** [Documentation/devicetree/bindings/iio/chemical/ams,ccs811.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
Makefile:1262: recipe for target 'dt_binding_check' failed
make: *** [dt_binding_check] Error 2
Quoted 5 lines
See https://patchwork.ozlabs.org/patch/1269473
If you already ran 'make
dt_binding_check' and didn't see the above error(s), then make sure dt-schema is up to date:
Ah, sorry. I did run 'dtbs_check' but forgot that it won't report errors in the binding schemas.
Will run dt_binding_check hereafter.
Thanks,
Mani
Quoted 3 lines
pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade
Please check and re-submit.
PATCH 2/3 iio: chemical: Add support for external Reset and Wakeup in CCS811
In reply to [email protected]From: Manivannan Sadhasivam <[email protected]>
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.
--- drivers/iio/chemical/ccs811.c | 88 +++++++++++++++++++++++++++++++---- 1 file changed, 80 insertions(+), 8 deletions(-)
drivers/iio/chemical/ccs811.c +80 -8
@@ -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 -EINVAL ; + } + +/* 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 -EINVAL ; + } + +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);
Signature · 1 line
2.17.1
Jonathan Cameron
maintainerRe: [PATCH 2/3] iio: chemical: Add support for external Reset and Wakeup in CCS811
Quoted 8 lines · On Mon, 13 Apr 2020 00:06:57 +0530 [email protected] wrote:
From: Manivannan Sadhasivam <[email protected]>
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.
Quoted 8 lines
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 <[email protected]>
One trivial thing inline to allow things to work if we have deferred probing and the gpio provider isn't ready yet. Currently you eat the error code rather than passing it on.
Otherwise looks good to me.
Thanks,
Jonathan
Quoted 76 lines
--- drivers/iio/chemical/ccs811.c | 88 +++++++++++++++++++++++++++++++---- 1 file changed, 80 insertions(+), 8 deletions(-)drivers/iio/chemical/ccs811.c +22
-1@@ -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 ; }
drivers/iio/chemical/ccs811.c static irqreturn_t ccs811_data_rdy_trigger_poll(int irq, void *private) · line 369
Quoted 10 lines
drivers/iio/chemical/ccs811.c +10
-0+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 -EINVAL ;
return PTR_ERR as it may well be a deferred signal to say try again later after the gpio provide device has been initialized.
Quoted 76 lines
drivers/iio/chemical/ccs811.c +48
-7+ } + +/* 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 -EINVAL ; + } + +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);
Re: [PATCH 2/3] iio: chemical: Add support for external Reset and Wakeup in CCS811
In reply to [email protected]Quoted 15 lines · On Mon, Apr 13, 2020 at 8:34 AM <[email protected]> wrote:
From: Manivannan Sadhasivam <[email protected]>
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.
...
drivers/iio/chemical/ccs811.c static irqreturn_t ccs811_data_rdy_trigger_poll(int irq, void *private) · line 369
Quoted 9 lines
drivers/iio/chemical/ccs811.c +6
-0+#define CCS811_SW_RESET 0xFF + 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 -EINVAL ;
Do not shadow actual error code.
drivers/iio/chemical/ccs811.c static irqreturn_t ccs811_data_rdy_trigger_poll(int irq, void *private) · line 380
Quoted 12 lines
drivers/iio/chemical/ccs811.c +11
-0+ } + +/* 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 , + };
Is 0xFF here is CCS811_SW_RESET? If so, can you put it explicitly?
Quoted 7 lines
drivers/iio/chemical/ccs811.c +7
-0+ 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; + } + }
...
drivers/iio/chemical/ccs811.c static int ccs811_probe(struct i2c_client *client, · line 420
Quoted 6 lines
drivers/iio/chemical/ccs811.c +5
-0+ 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 -EINVAL ;
Ditto.
Quoted 1 line
+ }
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [PATCH 2/3] iio: chemical: Add support for external Reset and Wakeup in CCS811
Quoted 22 lines · Andy Shevchenko
Quoted 15 lines · On Mon, Apr 13, 2020 at 8:34 AM <[email protected]> wrote:
From: Manivannan Sadhasivam <[email protected]>
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.
...
Quoted 1 line
+#define
CCS811_SW_RESET0xFF
Quoted 9 lines
drivers/iio/chemical/ccs811.c static irqreturn_t ccs811_data_rdy_trigger_poll(int irq, void *private) · line 369
Quoted 6 lines
drivers/iio/chemical/ccs811.c +5
-0+ 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 -EINVAL ;Do not shadow actual error code.
Okay. Will change below instance as well.
Quoted 9 lines
Quoted 8 lines
drivers/iio/chemical/ccs811.c +8
-0+ } + +/* 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 {
Quoted 6 lines
drivers/iio/chemical/ccs811.c static irqreturn_t ccs811_data_rdy_trigger_poll(int irq, void *private) · line 380
Quoted 3 lines
drivers/iio/chemical/ccs811.c +3
-0+static const u8 reset_seq[] = { +0xFF ,0x11 ,0xE5 ,0x72 ,0x8A , + };Is
0xFFhere isCCS811_SW_RESET? If so, can you put it explicitly?
Nope. CCS811_SW_RESET is the register address whereas 0xFF here is the actual value return to the register. I don't know what these values in array represent. So will keep them as it is.
Thanks,
Mani
Quoted 24 lines
Quoted 7 lines
drivers/iio/chemical/ccs811.c +7
-0+ 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; + } + }...
drivers/iio/chemical/ccs811.c static int ccs811_probe(struct i2c_client *client, · line 420
Quoted 6 lines
drivers/iio/chemical/ccs811.c +5
-0+ 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 -EINVAL ;Ditto.
Quoted 1 line
+ }
Signature · 2 lines
With Best Regards, Andy Shevchenko
PATCH 3/3 iio: chemical: Add OF match table for CCS811 VOC sensor
In reply to [email protected]From: Manivannan Sadhasivam <[email protected]>
Add devicetree OF match table support for CCS811 VOC sensor.
--- drivers/iio/chemical/ccs811.c | 8 ++++++++ 1 file changed, 8 insertions(+)
drivers/iio/chemical/ccs811.c +8 -0
@@ -24,6 +24,7 @@#include <linux/iio/triggered_buffer.h> #include <linux/iio/trigger_consumer.h> #include <linux/module.h> +#include <linux/of.h> #define CCS811_STATUS 0x00#define CCS811_MEAS_MODE 0x01
@@ -538,9 +539,16 @@ static const struct i2c_device_id ccs811_id[] = { };MODULE_DEVICE_TABLE (i2c, ccs811_id); +static const struct of_device_id ccs811_dt_ids[] = { + { .compatible ="ams,ccs811" }, + { } +}; +MODULE_DEVICE_TABLE (of, ccs811_dt_ids); +static struct i2c_driver ccs811_driver = { .driver = { .name ="ccs811" , + .of_match_table =of_match_ptr (ccs811_dt_ids), }, .probe = ccs811_probe, .remove = ccs811_remove,
Signature · 1 line
2.17.1
Jonathan Cameron
maintainerRe: [PATCH 3/3] iio: chemical: Add OF match table for CCS811 VOC sensor
Quoted 5 lines · On Mon, 13 Apr 2020 00:06:58 +0530 [email protected] wrote:
From: Manivannan Sadhasivam <[email protected]>
Add devicetree OF match table support for CCS811 VOC sensor.
Signed-off-by Manivannan Sadhasivam <[email protected]>
Hi,
A few small things to clean up inline
Thanks,
Jonathan
drivers/iio/chemical/ccs811.c line 27
Quoted 13 lines
--- drivers/iio/chemical/ccs811.c | 8 ++++++++ 1 file changed, 8 insertions(+)drivers/iio/chemical/ccs811.c +1
-0@@ -24,6 +24,7 @@#include <linux/iio/triggered_buffer.h> #include <linux/iio/trigger_consumer.h> #include <linux/module.h> +#include <linux/of.h>
You are including this just to get things in mod_devicetable.h so include that directly instead.
Quoted 7 lines
drivers/iio/chemical/ccs811.c +0
-0#define CCS811_STATUS 0x00#define CCS811_MEAS_MODE 0x01@@ -538,9 +539,16 @@ static const struct i2c_device_id ccs811_id[] = { };MODULE_DEVICE_TABLE (i2c, ccs811_id);
drivers/iio/chemical/ccs811.c static const struct i2c_device_id ccs811_id[] = { · line 551
Quoted 10 lines
drivers/iio/chemical/ccs811.c +7
-0+static const struct of_device_id ccs811_dt_ids[] = { + { .compatible ="ams,ccs811" }, + { } +}; +MODULE_DEVICE_TABLE (of, ccs811_dt_ids); +static struct i2c_driver ccs811_driver = { .driver = { .name ="ccs811" , + .of_match_table =of_match_ptr (ccs811_dt_ids),
No need for the of_match_ptr macro. It has several issues.
1) Blocks PRP001 ACPI magic device types being used to instantiate this using the device tree binding but under ACPI.
2) Will give warnings about ccs811_dt_ids being unused on no device tree builds.
Quoted 3 lines
drivers/iio/chemical/ccs811.c +0
-0}, .probe = ccs811_probe, .remove = ccs811_remove,
Re: [PATCH 3/3] iio: chemical: Add OF match table for CCS811 VOC sensor
Quoted 21 lines · Jonathan Cameron
Quoted 5 lines · On Mon, 13 Apr 2020 00:06:58 +0530 [email protected] wrote:
From: Manivannan Sadhasivam <[email protected]>
Add devicetree OF match table support for CCS811 VOC sensor.
Signed-off-by Manivannan Sadhasivam <[email protected]>Hi,
A few small things to clean up inline
Thanks,
Jonathan
Quoted 4 lines
--- drivers/iio/chemical/ccs811.c | 8 ++++++++ 1 file changed, 8 insertions(+)
Quoted 13 lines
drivers/iio/chemical/ccs811.c line 27
Quoted 9 lines
drivers/iio/chemical/ccs811.c +1
-0@@ -24,6 +24,7 @@#include <linux/iio/triggered_buffer.h> #include <linux/iio/trigger_consumer.h> #include <linux/module.h> +#include <linux/of.h> You are including this just to get things in
mod_devicetable.hso include that directly instead.
I added this include for of_match_ptr. Since it is not needed anymore, I'll drop this.
Quoted 19 lines
Quoted 7 lines
drivers/iio/chemical/ccs811.c +0
-0#define CCS811_STATUS 0x00#define CCS811_MEAS_MODE 0x01@@ -538,9 +539,16 @@ static const struct i2c_device_id ccs811_id[] = { };MODULE_DEVICE_TABLE (i2c, ccs811_id);drivers/iio/chemical/ccs811.c static const struct i2c_device_id ccs811_id[] = { · line 551
Quoted 10 lines
drivers/iio/chemical/ccs811.c +7
-0+static const struct of_device_id ccs811_dt_ids[] = { + { .compatible ="ams,ccs811" }, + { } +}; +MODULE_DEVICE_TABLE (of, ccs811_dt_ids); +static struct i2c_driver ccs811_driver = { .driver = { .name ="ccs811" , + .of_match_table =of_match_ptr (ccs811_dt_ids),No need for the
of_match_ptrmacro. It has several issues.
Quoted 5 lines
1) Blocks PRP001 ACPI magic device types being used to instantiate this using the device tree binding but under ACPI.
2) Will give warnings aboutccs811_dt_idsbeing unused on no device tree builds.
Okay, will use ccs811_dt_ids directly.
Thanks,
Mani
Quoted 4 lines
Quoted 3 lines
drivers/iio/chemical/ccs811.c +0
-0}, .probe = ccs811_probe, .remove = ccs811_remove,
Re: [PATCH 3/3] iio: chemical: Add OF match table for CCS811 VOC sensor
In reply to [email protected]Quoted 4 lines · On Mon, Apr 13, 2020 at 10:23 AM <[email protected]> wrote:
From: Manivannan Sadhasivam <[email protected]>
Add devicetree OF match table support for CCS811 VOC sensor.
...
Quoted 1 line
+#include <
linux/of.h>
Why?
...
drivers/iio/chemical/ccs811.c static const struct i2c_device_id ccs811_id[] = { · line 546
Quoted 5 lines
drivers/iio/chemical/ccs811.c +5
-0+static const struct of_device_id ccs811_dt_ids[] = { + { .compatible ="ams,ccs811" }, + { } +}; +MODULE_DEVICE_TABLE (of, ccs811_dt_ids);
Since it has no ugly ifdeffery...
Quoted 4 lines
static struct i2c_driver ccs811_driver = { .driver = { .name = "ccs811",+ .of_match_table =
of_match_ptr(ccs811_dt_ids),
...use of of_match_ptr() brings a compiler warning.
Drop of_match_ptr() for good. And thus drop redundant of.h.
Quoted 1 line
},
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [PATCH 3/3] iio: chemical: Add OF match table for CCS811 VOC sensor
Quoted 4 lines · Andy Shevchenko
Quoted 3 lines · On Mon, Apr 13, 2020 at 10:23 AM <[email protected]> wrote:
From: Manivannan Sadhasivam <[email protected]>
Quoted 8 lines
Quoted 1 line
Add devicetree OF match table support for CCS811 VOC sensor.
...
Quoted 1 line
+#include <
linux/of.h>Why?
As replied to Jonathan, will drop this and of_match_ptr.
Thanks,
Mani
Quoted 24 lines
...
drivers/iio/chemical/ccs811.c static const struct i2c_device_id ccs811_id[] = { · line 546
Quoted 5 lines
drivers/iio/chemical/ccs811.c +5
-0+static const struct of_device_id ccs811_dt_ids[] = { + { .compatible ="ams,ccs811" }, + { } +}; +MODULE_DEVICE_TABLE (of, ccs811_dt_ids);Since it has no ugly ifdeffery...
Quoted 4 lines
static struct i2c_driver ccs811_driver = { .driver = { .name = "ccs811",+ .of_match_table =
of_match_ptr(ccs811_dt_ids),...use of
of_match_ptr()brings a compiler warning.Drop
of_match_ptr()for good. And thus drop redundantof.h.Quoted 1 line
},Signature · 2 lines
With Best Regards, Andy Shevchenko