[PATCH v2] iio: ccs811: Deassert WAKE GPIO on error paths

[PATCH v2] iio: ccs811: Deassert WAKE GPIO on error paths

[Nikhil Gautam]

Ensure the WAKE GPIO is deasserted when status polling fails or
the data-ready timeout expires. Early returns could leave WAKE asserted,
preventing the sensor from returning to its low-power state and
potentially disrupting subsequent I2C transactions.

Route error paths through a common cleanup label to ensure WAKE is
deasserted before returning.

Fixes: c6644f7201f3 ("iio: chemical: Add support for external Reset and Wakeup in CCS811")
Signed-off-by: Nikhil Gautam <nikhilgtr@gmail.com>
Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>
---
 drivers/iio/chemical/ccs811.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/chemical/ccs811.c b/drivers/iio/chemical/ccs811.c
index 998c9239c4c7..dad54f66ebb3 100644
--- a/drivers/iio/chemical/ccs811.c
+++ b/drivers/iio/chemical/ccs811.c
@@ -198,18 +198,21 @@ static int ccs811_get_measurement(struct ccs811_data *data)
 	while (tries-- > 0) {
 		ret = i2c_smbus_read_byte_data(data->client, CCS811_STATUS);
 		if (ret < 0)
-			return ret;
+			goto out;
 
 		if ((ret & CCS811_STATUS_DATA_READY) || tries == 0)
 			break;
 		msleep(100);
 	}
-	if (!(ret & CCS811_STATUS_DATA_READY))
-		return -EIO;
+	if (!(ret & CCS811_STATUS_DATA_READY)) {
+		ret = -EIO;
+		goto out;
+	}
 
 	ret = i2c_smbus_read_i2c_block_data(data->client,
 					    CCS811_ALG_RESULT_DATA, 8,
 					    (char *)&data->buffer);
+out:
 	ccs811_set_wakeup(data, false);
 
 	return ret;
-- 
2.39.5

Re: [PATCH v2] iio: ccs811: Deassert WAKE GPIO on error paths

[Joshua Crofts]

On Thu, 15 Oct 2026 18:00:23 +0530
Nikhil Gautam <nikhilgtr@gmail.com> wrote:

> Ensure the WAKE GPIO is deasserted when status polling fails or
> the data-ready timeout expires. Early returns could leave WAKE asserted,
> preventing the sensor from returning to its low-power state and
> potentially disrupting subsequent I2C transactions.
> 
> Route error paths through a common cleanup label to ensure WAKE is
> deasserted before returning.
> 
> Fixes: c6644f7201f3 ("iio: chemical: Add support for external Reset and Wakeup in CCS811")
> Signed-off-by: Nikhil Gautam <nikhilgtr@gmail.com>

Ideally your signed-off-by should be last in the tag block.
No need to resend though.

> Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>
> ---

You were too fast! Wait at least 24 hours before submitting a new
patch, you might get more reviews.

-- 
Kind regards,
Joshua Crofts

Re: [PATCH v2] iio: ccs811: Deassert WAKE GPIO on error paths

[Jonathan Cameron]

On Thu, 15 Oct 2026 18:00:23 +0530
Nikhil Gautam <nikhilgtr@gmail.com> wrote:

> Ensure the WAKE GPIO is deasserted when status polling fails or
> the data-ready timeout expires. Early returns could leave WAKE asserted,
> preventing the sensor from returning to its low-power state and
> potentially disrupting subsequent I2C transactions.

I need more on this point to know whether we care if an error
means we don't deassert the wake pin.

Why would failing to go to sleep break future transactions?
I'd kind of assume we'd awaken an already awake device (which sounds
idempotent but maybe not?) and carry on from there?

Jonathan

> 
> Route error paths through a common cleanup label to ensure WAKE is
> deasserted before returning.
> 
> Fixes: c6644f7201f3 ("iio: chemical: Add support for external Reset and Wakeup in CCS811")
> Signed-off-by: Nikhil Gautam <nikhilgtr@gmail.com>
> Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>
> ---
>  drivers/iio/chemical/ccs811.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iio/chemical/ccs811.c b/drivers/iio/chemical/ccs811.c
> index 998c9239c4c7..dad54f66ebb3 100644
> --- a/drivers/iio/chemical/ccs811.c
> +++ b/drivers/iio/chemical/ccs811.c
> @@ -198,18 +198,21 @@ static int ccs811_get_measurement(struct ccs811_data *data)
>  	while (tries-- > 0) {
>  		ret = i2c_smbus_read_byte_data(data->client, CCS811_STATUS);
>  		if (ret < 0)
> -			return ret;
> +			goto out;
>  
>  		if ((ret & CCS811_STATUS_DATA_READY) || tries == 0)
>  			break;
>  		msleep(100);
>  	}
> -	if (!(ret & CCS811_STATUS_DATA_READY))
> -		return -EIO;
> +	if (!(ret & CCS811_STATUS_DATA_READY)) {
> +		ret = -EIO;
> +		goto out;
> +	}
>  
>  	ret = i2c_smbus_read_i2c_block_data(data->client,
>  					    CCS811_ALG_RESULT_DATA, 8,
>  					    (char *)&data->buffer);
> +out:
>  	ccs811_set_wakeup(data, false);
>  
>  	return ret;

Re: [PATCH v2] iio: ccs811: Deassert WAKE GPIO on error paths

[Nikhil Gautam]

Hi Jonathan,

Thanks for the review.

>> Ensure the WAKE GPIO is deasserted when status polling fails or
>> the data-ready timeout expires. Early returns could leave WAKE asserted,
>> preventing the sensor from returning to its low-power state and
>> potentially disrupting subsequent I2C transactions.
> I need more on this point to know whether we care if an error
> means we don't deassert the wake pin.
>
> Why would failing to go to sleep break future transactions?
> I'd kind of assume we'd awaken an already awake device (which sounds
> idempotent but maybe not?) and carry on from there?
>
> Jonathan

The relevant information is in the CCS811 datasheet (v1-06),
under “CCS811 – Pin Assignment” (page 4) and “Detailed Description” 
(page 7 onwards).

The datasheet specifies that nWAKE is active-low and should be asserted
before an I²C transaction and held low throughout it. It also describes
the sleep behavior: when nWAKE is deasserted, the device enters sleep
mode and ignores I²C requests. The timing specifications specify a 50 µs
wake-up time and a minimum 20 µs deassertion interval.

I agree that the datasheet does not explicitly state that leaving nWAKE
asserted after an error will cause subsequent transactions to fail.
My concern is primarily that an early return leaves the device awake
instead of restoring the intended low-power state.

I'll adjust the justification accordingly, rather than claiming that 
subsequent
transactions would necessarily be disrupted.

Thanks,
Nikhil

Re: [PATCH v2] iio: ccs811: Deassert WAKE GPIO on error paths

[Jonathan Cameron]

On Sun, 18 Oct 2026 02:42:18 +0530
Nikhil Gautam <nikhilgtr@gmail.com> wrote:

> Hi Jonathan,
> 
> Thanks for the review.
> 
> >> Ensure the WAKE GPIO is deasserted when status polling fails or
> >> the data-ready timeout expires. Early returns could leave WAKE asserted,
> >> preventing the sensor from returning to its low-power state and
> >> potentially disrupting subsequent I2C transactions.  
> > I need more on this point to know whether we care if an error
> > means we don't deassert the wake pin.
> >
> > Why would failing to go to sleep break future transactions?
> > I'd kind of assume we'd awaken an already awake device (which sounds
> > idempotent but maybe not?) and carry on from there?
> >
> > Jonathan  
> 
> The relevant information is in the CCS811 datasheet (v1-06),
> under “CCS811 – Pin Assignment” (page 4) and “Detailed Description” 
> (page 7 onwards).
> 
> The datasheet specifies that nWAKE is active-low and should be asserted
> before an I²C transaction and held low throughout it. It also describes
> the sleep behavior: when nWAKE is deasserted, the device enters sleep
> mode and ignores I²C requests. The timing specifications specify a 50 µs
> wake-up time and a minimum 20 µs deassertion interval.
> 
> I agree that the datasheet does not explicitly state that leaving nWAKE
> asserted after an error will cause subsequent transactions to fail.
> My concern is primarily that an early return leaves the device awake
> instead of restoring the intended low-power state.
> 
> I'll adjust the justification accordingly, rather than claiming that 
> subsequent
> transactions would necessarily be disrupted.
Given it's an error path and just as likely the attempt to put it
to sleep may also fail, this becomes a possible improvement rather htan
a fix.  I'm not entirely convinced we care about that corner case
but feel free to send it with a description that talks about this
and we'll see what others think.

Thanks,

Jonathan

> 
> Thanks,
> Nikhil