All of lore.kernel.org
 help / color / mirror / Atom feed
From: Salah Triki <salah.triki@gmail.com>
To: Andy Shevchenko <andriy.shevchenko@intel.com>
Cc: "Crt Mori" <cmo@melexis.com>,
	"Jonathan Cameron" <jic23@kernel.org>,
	"David Lechner" <dlechner@baylibre.com>,
	"Nuno Sá" <nuno.sa@analog.com>,
	"Andy Shevchenko" <andy@kernel.org>,
	linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] iio: mlx90614: fix missing GPIO direction return value checks
Date: Tue, 28 Apr 2026 09:38:47 +0100	[thread overview]
Message-ID: <afByF-0BV1szTbzE@pc> (raw)
In-Reply-To: <afBpBNLucHg25So0@ashevche-desk.local>

On Tue, Apr 28, 2026 at 11:00:04AM +0300, Andy Shevchenko wrote:
> On Mon, Apr 27, 2026 at 10:58:00PM +0100, Salah Triki 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.
> 
> ...
> 
> > -	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);
> > +
> > +	ret = gpiod_direction_input(data->wakeup_gpio);
> > +	if (ret)
> > +		goto out_unlock;
> 
> While technically it sounds correct, the potential problem here is that you may
> fail this in case CONFIG_GPIOLIB=n. Is this GPIO optional?
> What may happen if GPIO is not optional, but for some reason setting it fails?
> 
> TL;DR:
> I am not sure about this patch. At least I'm not comfortable to take it without
> testing on real HW.
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

Thank you for your feedback. Since I don't have the physical hardware to 
perform the necessary tests and ensure there are no regressions, I agree
that it is safer to drop this patch. I don't want to risk breaking the 
driver for a minor cleanup.

Best regards,
--
Salah Triki

  reply	other threads:[~2026-04-28  8:38 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-27 21:58 [PATCH] iio: mlx90614: fix missing GPIO direction return value checks Salah Triki
2026-04-28  8:00 ` Andy Shevchenko
2026-04-28  8:38   ` Salah Triki [this message]
2026-04-28 16:04     ` Jonathan Cameron
2026-04-28 16:33       ` Crt Mori
2026-05-05  7:31         ` Salah Triki
2026-05-05  7:48           ` Crt Mori
2026-04-28 16:05 ` Jonathan Cameron

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=afByF-0BV1szTbzE@pc \
    --to=salah.triki@gmail.com \
    --cc=andriy.shevchenko@intel.com \
    --cc=andy@kernel.org \
    --cc=cmo@melexis.com \
    --cc=dlechner@baylibre.com \
    --cc=jic23@kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nuno.sa@analog.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.