[PATCH v2] iio: chemical: ccs811: Corrected firmware boot/application mode transition

2018-02-17 Thread Richard Lai
CCS811 has different I2C register maps in boot and application mode. When
CCS811 is in boot mode, register APP_START (0xF4) is used to transit the
firmware state from boot to application mode. However, APP_START is not a
valid register location when CCS811 is in application mode (refer to
"CCS811 Bootloader Register Map" and "CCS811 Application Register Map" in
CCS811 datasheet). The driver should not attempt to perform a write to
APP_START while CCS811 is in application mode, as this is not a valid or
documented register location.

When prob function is being called, the driver assumes the CCS811 sensor
is in boot mode, and attempts to perform a write to APP_START. Although
CCS811 powers-up in boot mode, it may have already been transited to
application mode by previous instances, e.g. unload and reload device
driver by the system, or explicitly by user. Depending on the system
design, CCS811 sensor may be permanently connected to system power source
rather than power controlled by GPIO, hence it is possible that the sensor
is never power reset, thus the firmware could be in either boot or
application mode at any given time when driver prob function is being
called.

This patch checks the STATUS register before attempting to send a write to
APP_START. Only if the firmware is not in application mode and has valid
firmware application loaded, then it will continue to start transiting the
firmware boot to application mode.

Signed-off-by: Richard Lai <rich...@richardman.com>
---
Changes in v2:
- Removed unnecessary macros introduced in previous patch

 drivers/iio/chemical/ccs811.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/iio/chemical/ccs811.c b/drivers/iio/chemical/ccs811.c
index 840a6cb..6ad8a42 100644
--- a/drivers/iio/chemical/ccs811.c
+++ b/drivers/iio/chemical/ccs811.c
@@ -129,6 +129,9 @@ static int ccs811_start_sensor_application(struct 
i2c_client *client)
if (ret < 0)
return ret;
 
+   if ((ret & CCS811_STATUS_FW_MODE_APPLICATION))
+   return 0;
+
if ((ret & CCS811_STATUS_APP_VALID_MASK) !=
CCS811_STATUS_APP_VALID_LOADED)
return -EIO;
-- 
2.7.4



[PATCH v2] iio: chemical: ccs811: Corrected firmware boot/application mode transition

2018-02-17 Thread Richard Lai
CCS811 has different I2C register maps in boot and application mode. When
CCS811 is in boot mode, register APP_START (0xF4) is used to transit the
firmware state from boot to application mode. However, APP_START is not a
valid register location when CCS811 is in application mode (refer to
"CCS811 Bootloader Register Map" and "CCS811 Application Register Map" in
CCS811 datasheet). The driver should not attempt to perform a write to
APP_START while CCS811 is in application mode, as this is not a valid or
documented register location.

When prob function is being called, the driver assumes the CCS811 sensor
is in boot mode, and attempts to perform a write to APP_START. Although
CCS811 powers-up in boot mode, it may have already been transited to
application mode by previous instances, e.g. unload and reload device
driver by the system, or explicitly by user. Depending on the system
design, CCS811 sensor may be permanently connected to system power source
rather than power controlled by GPIO, hence it is possible that the sensor
is never power reset, thus the firmware could be in either boot or
application mode at any given time when driver prob function is being
called.

This patch checks the STATUS register before attempting to send a write to
APP_START. Only if the firmware is not in application mode and has valid
firmware application loaded, then it will continue to start transiting the
firmware boot to application mode.

Signed-off-by: Richard Lai 
---
Changes in v2:
- Removed unnecessary macros introduced in previous patch

 drivers/iio/chemical/ccs811.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/iio/chemical/ccs811.c b/drivers/iio/chemical/ccs811.c
index 840a6cb..6ad8a42 100644
--- a/drivers/iio/chemical/ccs811.c
+++ b/drivers/iio/chemical/ccs811.c
@@ -129,6 +129,9 @@ static int ccs811_start_sensor_application(struct 
i2c_client *client)
if (ret < 0)
return ret;
 
+   if ((ret & CCS811_STATUS_FW_MODE_APPLICATION))
+   return 0;
+
if ((ret & CCS811_STATUS_APP_VALID_MASK) !=
CCS811_STATUS_APP_VALID_LOADED)
return -EIO;
-- 
2.7.4



[PATCH] iio: chemical: ccs811: Corrected firmware boot/application mode transition

2018-02-14 Thread Richard Lai
CCS811 has different I2C register maps in boot and application mode. When
CCS811 is in boot mode, register APP_START (0xF4) is used to transit the
firmware state from boot to application mode. However, APP_START is not a
valid register location when CCS811 is in application mode (refer to
"CCS811 Bootloader Register Map" and "CCS811 Application Register Map" in
CCS811 datasheet). The driver should not attempt to perform a write to
APP_START while CCS811 is in application mode, as this is not a valid or
documented register location.

When prob function is being called, the driver assumes the CCS811 sensor
is in boot mode, and attempts to perform a write to APP_START. Although
CCS811 powers-up in boot mode, it may have already been transited to
application mode by previous instances, e.g. unload and reload device
driver by the system, or explicitly by user. Depending on the system
design, CCS811 sensor may be permanently connected to system power source
rather than power controlled by GPIO, hence it is possible that the sensor
is never power reset, thus the firmware could be in either boot or
application mode at any given time when driver prob function is being
called.

This patch:

1) Checks the STATUS register before attempting to send a write to
APP_START. Only if the firmware is not in application mode and has
valid firmware application loaded, then it will continue to start
transiting the firmware boot to application mode.
2) Adds two macros for checking APP_VALID and FW_MODE bits in the STATUS
register of CCS811 sensor.

Signed-off-by: Richard Lai <rich...@richardman.com>
---
 drivers/iio/chemical/ccs811.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/chemical/ccs811.c b/drivers/iio/chemical/ccs811.c
index 840a6cb..4806aed 100644
--- a/drivers/iio/chemical/ccs811.c
+++ b/drivers/iio/chemical/ccs811.c
@@ -52,6 +52,11 @@
 #define CCS811_STATUS_FW_MODE_MASK BIT(7)
 #define CCS811_STATUS_FW_MODE_APPLICATION  BIT(7)
 
+#define IS_APP_VALID_LOADED(x) \
+   (CCS811_STATUS_APP_VALID_LOADED == (CCS811_STATUS_APP_VALID_MASK & x))
+#define IS_FW_MODE_APPLICATION(x) \
+   (CCS811_STATUS_FW_MODE_APPLICATION == (CCS811_STATUS_FW_MODE_MASK & x))
+
 /* Measurement modes */
 #define CCS811_MODE_IDLE   0x00
 #define CCS811_MODE_IAQ_1SEC   0x10
@@ -129,8 +134,10 @@ static int ccs811_start_sensor_application(struct 
i2c_client *client)
if (ret < 0)
return ret;
 
-   if ((ret & CCS811_STATUS_APP_VALID_MASK) !=
-   CCS811_STATUS_APP_VALID_LOADED)
+   if (IS_FW_MODE_APPLICATION(ret))
+   return 0;
+
+   if (!IS_APP_VALID_LOADED(ret))
return -EIO;
 
ret = i2c_smbus_write_byte(client, CCS811_APP_START);
@@ -141,8 +148,7 @@ static int ccs811_start_sensor_application(struct 
i2c_client *client)
if (ret < 0)
return ret;
 
-   if ((ret & CCS811_STATUS_FW_MODE_MASK) !=
-   CCS811_STATUS_FW_MODE_APPLICATION) {
+   if (!IS_FW_MODE_APPLICATION(ret)) {
dev_err(>dev, "Application failed to start. Sensor is 
still in boot mode.\n");
return -EIO;
}
-- 
2.7.4



[PATCH] iio: chemical: ccs811: Corrected firmware boot/application mode transition

2018-02-14 Thread Richard Lai
CCS811 has different I2C register maps in boot and application mode. When
CCS811 is in boot mode, register APP_START (0xF4) is used to transit the
firmware state from boot to application mode. However, APP_START is not a
valid register location when CCS811 is in application mode (refer to
"CCS811 Bootloader Register Map" and "CCS811 Application Register Map" in
CCS811 datasheet). The driver should not attempt to perform a write to
APP_START while CCS811 is in application mode, as this is not a valid or
documented register location.

When prob function is being called, the driver assumes the CCS811 sensor
is in boot mode, and attempts to perform a write to APP_START. Although
CCS811 powers-up in boot mode, it may have already been transited to
application mode by previous instances, e.g. unload and reload device
driver by the system, or explicitly by user. Depending on the system
design, CCS811 sensor may be permanently connected to system power source
rather than power controlled by GPIO, hence it is possible that the sensor
is never power reset, thus the firmware could be in either boot or
application mode at any given time when driver prob function is being
called.

This patch:

1) Checks the STATUS register before attempting to send a write to
APP_START. Only if the firmware is not in application mode and has
valid firmware application loaded, then it will continue to start
transiting the firmware boot to application mode.
2) Adds two macros for checking APP_VALID and FW_MODE bits in the STATUS
register of CCS811 sensor.

Signed-off-by: Richard Lai 
---
 drivers/iio/chemical/ccs811.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/chemical/ccs811.c b/drivers/iio/chemical/ccs811.c
index 840a6cb..4806aed 100644
--- a/drivers/iio/chemical/ccs811.c
+++ b/drivers/iio/chemical/ccs811.c
@@ -52,6 +52,11 @@
 #define CCS811_STATUS_FW_MODE_MASK BIT(7)
 #define CCS811_STATUS_FW_MODE_APPLICATION  BIT(7)
 
+#define IS_APP_VALID_LOADED(x) \
+   (CCS811_STATUS_APP_VALID_LOADED == (CCS811_STATUS_APP_VALID_MASK & x))
+#define IS_FW_MODE_APPLICATION(x) \
+   (CCS811_STATUS_FW_MODE_APPLICATION == (CCS811_STATUS_FW_MODE_MASK & x))
+
 /* Measurement modes */
 #define CCS811_MODE_IDLE   0x00
 #define CCS811_MODE_IAQ_1SEC   0x10
@@ -129,8 +134,10 @@ static int ccs811_start_sensor_application(struct 
i2c_client *client)
if (ret < 0)
return ret;
 
-   if ((ret & CCS811_STATUS_APP_VALID_MASK) !=
-   CCS811_STATUS_APP_VALID_LOADED)
+   if (IS_FW_MODE_APPLICATION(ret))
+   return 0;
+
+   if (!IS_APP_VALID_LOADED(ret))
return -EIO;
 
ret = i2c_smbus_write_byte(client, CCS811_APP_START);
@@ -141,8 +148,7 @@ static int ccs811_start_sensor_application(struct 
i2c_client *client)
if (ret < 0)
return ret;
 
-   if ((ret & CCS811_STATUS_FW_MODE_MASK) !=
-   CCS811_STATUS_FW_MODE_APPLICATION) {
+   if (!IS_FW_MODE_APPLICATION(ret)) {
dev_err(>dev, "Application failed to start. Sensor is 
still in boot mode.\n");
return -EIO;
}
-- 
2.7.4



[PATCH] iio: chemical: ccs811: Renamed resistance member in ccs811_reading struct

2018-02-13 Thread Richard Lai
The resistance member in ccs811_reading struct is an unsigned 16-bit
integer variable used to store RAW_DATA register bytes read from CCS811.
It is kind of misleading to name this struct member as resistance.

About the RAW_DATA register bytes, the CCS811 datasheet states that:
-
Two byte read only register which contains the latest readings from the
sense resistor.

The most significant 6 bits of the Byte 0 contain the value of the current
through the sensor (0μA to 63μA).

The lower 10 bits contain (as computed from the ADC) the readings of the
voltage across the sensor with the selected current (1023 = 1.65V)"
-

Hence, the RAW_DATA register byte contains information about electric
current and voltage of the CCS811 sensor. Calling this struct member
'resistance' is kind of misleading, although both electric current and
voltage are needed to calculate the electrical resistance of the sensor
using Ohm's law, V = I x R, in which a new channel type of IIO_RESISTANCE
may be added to the driver in the future.

Signed-off-by: Richard Lai <rich...@richardman.com>
---
 drivers/iio/chemical/ccs811.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/chemical/ccs811.c b/drivers/iio/chemical/ccs811.c
index 8e8beb7..e6f6bc4 100644
--- a/drivers/iio/chemical/ccs811.c
+++ b/drivers/iio/chemical/ccs811.c
@@ -66,7 +66,7 @@ struct ccs811_reading {
__be16 voc;
u8 status;
u8 error;
-   __be16 resistance;
+   __be16 raw_data;
 } __attribute__((__packed__));
 
 struct ccs811_data {
@@ -202,12 +202,12 @@ static int ccs811_read_raw(struct iio_dev *indio_dev,
 
switch (chan->type) {
case IIO_VOLTAGE:
-   *val = be16_to_cpu(data->buffer.resistance) &
+   *val = be16_to_cpu(data->buffer.raw_data) &
   CCS811_VOLTAGE_MASK;
ret = IIO_VAL_INT;
break;
case IIO_CURRENT:
-   *val = be16_to_cpu(data->buffer.resistance) >> 10;
+   *val = be16_to_cpu(data->buffer.raw_data) >> 10;
ret = IIO_VAL_INT;
break;
case IIO_CONCENTRATION:
-- 
2.7.4



[PATCH] iio: chemical: ccs811: Renamed resistance member in ccs811_reading struct

2018-02-13 Thread Richard Lai
The resistance member in ccs811_reading struct is an unsigned 16-bit
integer variable used to store RAW_DATA register bytes read from CCS811.
It is kind of misleading to name this struct member as resistance.

About the RAW_DATA register bytes, the CCS811 datasheet states that:
-
Two byte read only register which contains the latest readings from the
sense resistor.

The most significant 6 bits of the Byte 0 contain the value of the current
through the sensor (0μA to 63μA).

The lower 10 bits contain (as computed from the ADC) the readings of the
voltage across the sensor with the selected current (1023 = 1.65V)"
-

Hence, the RAW_DATA register byte contains information about electric
current and voltage of the CCS811 sensor. Calling this struct member
'resistance' is kind of misleading, although both electric current and
voltage are needed to calculate the electrical resistance of the sensor
using Ohm's law, V = I x R, in which a new channel type of IIO_RESISTANCE
may be added to the driver in the future.

Signed-off-by: Richard Lai 
---
 drivers/iio/chemical/ccs811.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/chemical/ccs811.c b/drivers/iio/chemical/ccs811.c
index 8e8beb7..e6f6bc4 100644
--- a/drivers/iio/chemical/ccs811.c
+++ b/drivers/iio/chemical/ccs811.c
@@ -66,7 +66,7 @@ struct ccs811_reading {
__be16 voc;
u8 status;
u8 error;
-   __be16 resistance;
+   __be16 raw_data;
 } __attribute__((__packed__));
 
 struct ccs811_data {
@@ -202,12 +202,12 @@ static int ccs811_read_raw(struct iio_dev *indio_dev,
 
switch (chan->type) {
case IIO_VOLTAGE:
-   *val = be16_to_cpu(data->buffer.resistance) &
+   *val = be16_to_cpu(data->buffer.raw_data) &
   CCS811_VOLTAGE_MASK;
ret = IIO_VAL_INT;
break;
case IIO_CURRENT:
-   *val = be16_to_cpu(data->buffer.resistance) >> 10;
+   *val = be16_to_cpu(data->buffer.raw_data) >> 10;
ret = IIO_VAL_INT;
break;
case IIO_CONCENTRATION:
-- 
2.7.4



[PATCH] iio: chemical: ccs811: Typo correction in HW_ID_VALUE constant define naming

2018-02-13 Thread Richard Lai
This particular constant was named with prefix "CCS881", which should be
"CCS811" instead, just like the rest of constant names in the file, as this
driver implementation is for AMS CCS811 sensor. "CCS881" could literally be
referring to another sensor product unrelated to AMS CCS811 sensor.

Signed-off-by: Richard Lai <rich...@richardman.com>
---
 drivers/iio/chemical/ccs811.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/chemical/ccs811.c b/drivers/iio/chemical/ccs811.c
index 840a6cb..8e8beb7 100644
--- a/drivers/iio/chemical/ccs811.c
+++ b/drivers/iio/chemical/ccs811.c
@@ -31,7 +31,7 @@
 #define CCS811_ALG_RESULT_DATA 0x02
 #define CCS811_RAW_DATA0x03
 #define CCS811_HW_ID   0x20
-#define CCS881_HW_ID_VALUE 0x81
+#define CCS811_HW_ID_VALUE 0x81
 #define CCS811_HW_VERSION  0x21
 #define CCS811_HW_VERSION_VALUE0x10
 #define CCS811_HW_VERSION_MASK 0xF0
@@ -315,7 +315,7 @@ static int ccs811_probe(struct i2c_client *client,
if (ret < 0)
return ret;
 
-   if (ret != CCS881_HW_ID_VALUE) {
+   if (ret != CCS811_HW_ID_VALUE) {
dev_err(>dev, "hardware id doesn't match CCS81x\n");
return -ENODEV;
}
-- 
2.7.4



[PATCH] iio: chemical: ccs811: Typo correction in HW_ID_VALUE constant define naming

2018-02-13 Thread Richard Lai
This particular constant was named with prefix "CCS881", which should be
"CCS811" instead, just like the rest of constant names in the file, as this
driver implementation is for AMS CCS811 sensor. "CCS881" could literally be
referring to another sensor product unrelated to AMS CCS811 sensor.

Signed-off-by: Richard Lai 
---
 drivers/iio/chemical/ccs811.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/chemical/ccs811.c b/drivers/iio/chemical/ccs811.c
index 840a6cb..8e8beb7 100644
--- a/drivers/iio/chemical/ccs811.c
+++ b/drivers/iio/chemical/ccs811.c
@@ -31,7 +31,7 @@
 #define CCS811_ALG_RESULT_DATA 0x02
 #define CCS811_RAW_DATA0x03
 #define CCS811_HW_ID   0x20
-#define CCS881_HW_ID_VALUE 0x81
+#define CCS811_HW_ID_VALUE 0x81
 #define CCS811_HW_VERSION  0x21
 #define CCS811_HW_VERSION_VALUE0x10
 #define CCS811_HW_VERSION_MASK 0xF0
@@ -315,7 +315,7 @@ static int ccs811_probe(struct i2c_client *client,
if (ret < 0)
return ret;
 
-   if (ret != CCS881_HW_ID_VALUE) {
+   if (ret != CCS811_HW_ID_VALUE) {
dev_err(>dev, "hardware id doesn't match CCS81x\n");
return -ENODEV;
}
-- 
2.7.4