Re: [PATCH] iio: mlx90614: fix missing GPIO direction return value checks

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 27 Apr 2026 22:58:00 +0100
Salah Triki <salah.triki@gmail.com> wrote:

> The functions gpiod_direction_output() and gpiod_direction_input() can
> fail, but their return values were previously ignored.
> 
> If an error occurs during the GPIO configuration, the function should
> abort the wake-up sequence and return the error code. More importantly,
> failing to check these values could lead to the I2C bus remaining
> locked if an error occurs after i2c_lock_bus() is called.
> 
> Add return value checks and ensure the I2C bus is properly unlocked
> via a goto label in case of failure.
> 
> Fixes: eb4b07dae4d4 ("iio: mlx90614: Add power management")
> Signed-off-by: Salah Triki <salah.triki@gmail.com>
> ---
>  drivers/iio/temperature/mlx90614.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/temperature/mlx90614.c b/drivers/iio/temperature/mlx90614.c
> index 1ad21b73e1b4..ce12b44c84e0 100644
> --- a/drivers/iio/temperature/mlx90614.c
> +++ b/drivers/iio/temperature/mlx90614.c
> @@ -489,6 +489,7 @@ static int mlx90614_sleep(struct mlx90614_data *data)
>  static int mlx90614_wakeup(struct mlx90614_data *data)
>  {
>  	const struct mlx_chip_info *chip_info = data->chip_info;
> +	int ret;
>  
>  	if (!data->wakeup_gpio) {
>  		dev_dbg(&data->client->dev, "Wake-up disabled");
> @@ -498,9 +499,17 @@ static int mlx90614_wakeup(struct mlx90614_data *data)
>  	dev_dbg(&data->client->dev, "Requesting wake-up");
>  
>  	i2c_lock_bus(data->client->adapter, I2C_LOCK_ROOT_ADAPTER);
> -	gpiod_direction_output(data->wakeup_gpio, 0);
> +
> +	ret = gpiod_direction_output(data->wakeup_gpio, 0);
> +	if (ret)
> +		goto out_unlock;
> +
>  	msleep(chip_info->wakeup_delay_ms);
> -	gpiod_direction_input(data->wakeup_gpio);
If this did go forwards, I'd prefer the section under the lock moved
into a helper function so that we can do

	ret = helper();
	i2c_unlock_bus();
	if (ret)
		return ret;

Rather than introducing a goto for a section of code in the middle of
the function that isn't appropriate if we were to later introduce some
error path after this.


> +
> +	ret = gpiod_direction_input(data->wakeup_gpio);
> +	if (ret)
> +		goto out_unlock;
> +
>  	i2c_unlock_bus(data->client->adapter, I2C_LOCK_ROOT_ADAPTER);
>  
>  	data->ready_timestamp = jiffies +
> @@ -515,6 +524,10 @@ static int mlx90614_wakeup(struct mlx90614_data *data)
>  	i2c_smbus_read_word_data(data->client, chip_info->op_eeprom_config1);
>  
>  	return 0;
> +
> +out_unlock:
> +	i2c_unlock_bus(data->client->adapter, I2C_LOCK_ROOT_ADAPTER);
> +	return ret;
>  }
>  
>  /* Return wake-up GPIO or NULL if sleep functionality should be disabled. */