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

Jonathan Cameron
2026-04-28T16:05:43Z

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. */