[PATCH v4 0/3] iio: pressure: bmp280: GPIO error handling and cleanups

[PATCH v4 0/3] iio: pressure: bmp280: GPIO error handling and cleanups

[Salah Triki]

Hi all,

This patch series improves the GPIO error handling and addresses small
cleanups in the bmp280 driver.

Changes in v4:
  - Split patch 2/2 into two separate patches, as suggested by Markus
    Elfring.

Changes in v3:
  - Split into two separate patches, as suggested by Andy Shevchenko.
  - Improve the error message to "failed to get reset GPIO", as
    suggested by David Lechner.
  - Add Fixes and Cc tags where appropriate, as suggested by Markus
    Elfring.

Changes in v2:
  - Use IS_ERR() instead of IS_ERR_OR_NULL()
  - Drop dev_info()
  - Use gpiod_set_value_cansleep()
  - Improve commit title and message

Salah Triki (3):
  iio: pressure: bmp280: Use IS_ERR() in bmp280_common_probe()
  iio: pressure: bmp280: Remove noisy dev_info()
  iio: pressure: bmp280: Use gpiod_set_value_cansleep()

 drivers/iio/pressure/bmp280-core.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

-- 
2.43.0

[PATCH v4 1/3] iio: pressure: bmp280: Use IS_ERR() in bmp280_common_probe()

[Salah Triki]

`devm_gpiod_get_optional()` may return non-NULL error pointer on failure.
Check its return value using `IS_ERR()` and propagate the error if
necessary.

Fixes: df6e71256c84 ("iio: pressure: bmp280: Explicitly mark GPIO optional")
Cc: David Lechner <dlechner@baylibre.com>
Cc: Jonathan Cameron <jic23@kernel.org>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Nuno Sá <nuno.sa@analog.com>
Cc: Markus Elfring <Markus.Elfring@web.de>
Signed-off-by: Salah Triki <salah.triki@gmail.com>
Reviewed-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/pressure/bmp280-core.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index 74505c9ec1a0..6cdc8ed53520 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -3213,11 +3213,12 @@ int bmp280_common_probe(struct device *dev,
 
 	/* Bring chip out of reset if there is an assigned GPIO line */
 	gpiod = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(gpiod))
+		return dev_err_probe(dev, PTR_ERR(gpiod), "failed to get reset GPIO\n");
+
 	/* Deassert the signal */
-	if (gpiod) {
-		dev_info(dev, "release reset\n");
-		gpiod_set_value(gpiod, 0);
-	}
+	dev_info(dev, "release reset\n");
+	gpiod_set_value(gpiod, 0);
 
 	data->regmap = regmap;
 
-- 
2.43.0

Re: [PATCH v4 1/3] iio: pressure: bmp280: Use IS_ERR() in bmp280_common_probe()

[Jonathan Cameron]

On Mon, 18 Aug 2025 10:27:30 +0100
Salah Triki <salah.triki@gmail.com> wrote:

> `devm_gpiod_get_optional()` may return non-NULL error pointer on failure.
> Check its return value using `IS_ERR()` and propagate the error if
> necessary.
> 
> Fixes: df6e71256c84 ("iio: pressure: bmp280: Explicitly mark GPIO optional")
> Cc: David Lechner <dlechner@baylibre.com>
> Cc: Jonathan Cameron <jic23@kernel.org>
> Cc: Andy Shevchenko <andy@kernel.org>
> Cc: Nuno Sá <nuno.sa@analog.com>
> Cc: Markus Elfring <Markus.Elfring@web.de>
> Signed-off-by: Salah Triki <salah.triki@gmail.com>
> Reviewed-by: David Lechner <dlechner@baylibre.com>

Applied to the fixes-togreg branch of iio.git and marked for stable.

The rest of the series will have to wait for that to work it's way back
via upstream. 

Thanks

Jonathan

> ---
>  drivers/iio/pressure/bmp280-core.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> index 74505c9ec1a0..6cdc8ed53520 100644
> --- a/drivers/iio/pressure/bmp280-core.c
> +++ b/drivers/iio/pressure/bmp280-core.c
> @@ -3213,11 +3213,12 @@ int bmp280_common_probe(struct device *dev,
>  
>  	/* Bring chip out of reset if there is an assigned GPIO line */
>  	gpiod = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> +	if (IS_ERR(gpiod))
> +		return dev_err_probe(dev, PTR_ERR(gpiod), "failed to get reset GPIO\n");
> +
>  	/* Deassert the signal */
> -	if (gpiod) {
> -		dev_info(dev, "release reset\n");
> -		gpiod_set_value(gpiod, 0);
> -	}
> +	dev_info(dev, "release reset\n");
> +	gpiod_set_value(gpiod, 0);
>  
>  	data->regmap = regmap;
>  

Re: [PATCH v4 1/3] iio: pressure: bmp280: Use IS_ERR() in bmp280_common_probe()

[Andy Shevchenko]

On Mon, Aug 18, 2025 at 10:27:30AM +0100, Salah Triki wrote:
> `devm_gpiod_get_optional()` may return non-NULL error pointer on failure.
> Check its return value using `IS_ERR()` and propagate the error if
> necessary.

> Fixes: df6e71256c84 ("iio: pressure: bmp280: Explicitly mark GPIO optional")

> Cc: David Lechner <dlechner@baylibre.com>
> Cc: Jonathan Cameron <jic23@kernel.org>
> Cc: Andy Shevchenko <andy@kernel.org>
> Cc: Nuno Sá <nuno.sa@analog.com>
> Cc: Markus Elfring <Markus.Elfring@web.de>

Please, next time move the Cc:s to be the after '---' line...

> Signed-off-by: Salah Triki <salah.triki@gmail.com>
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> ---

...somewhere here.

It will reduce a lot the noise in the commit message. The all people will be
Cc'ed anyway (assuming you use `git send-email`).

>  drivers/iio/pressure/bmp280-core.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)

Jonathan, would it be possible to drop them from the commit you pushed?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 1/3] iio: pressure: bmp280: Use IS_ERR() in bmp280_common_probe()

[Jonathan Cameron]

On Wed, 20 Aug 2025 17:48:56 +0300
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Mon, Aug 18, 2025 at 10:27:30AM +0100, Salah Triki wrote:
> > `devm_gpiod_get_optional()` may return non-NULL error pointer on failure.
> > Check its return value using `IS_ERR()` and propagate the error if
> > necessary.  
> 
> > Fixes: df6e71256c84 ("iio: pressure: bmp280: Explicitly mark GPIO optional")  
> 
> > Cc: David Lechner <dlechner@baylibre.com>
> > Cc: Jonathan Cameron <jic23@kernel.org>
> > Cc: Andy Shevchenko <andy@kernel.org>
> > Cc: Nuno Sá <nuno.sa@analog.com>
> > Cc: Markus Elfring <Markus.Elfring@web.de>  
> 
> Please, next time move the Cc:s to be the after '---' line...
> 
> > Signed-off-by: Salah Triki <salah.triki@gmail.com>
> > Reviewed-by: David Lechner <dlechner@baylibre.com>
> > ---  
> 
> ...somewhere here.
> 
> It will reduce a lot the noise in the commit message. The all people will be
> Cc'ed anyway (assuming you use `git send-email`).
> 
> >  drivers/iio/pressure/bmp280-core.c | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)  
> 
> Jonathan, would it be possible to drop them from the commit you pushed?
> 

I did so, just didn't mention it.

J

[PATCH v4 2/3] iio: pressure: bmp280: Remove noisy dev_info()

[Salah Triki]

Remove `dev_info()` call as it was considered noisy and is not
necessary for normal driver operation.

Signed-off-by: Salah Triki <salah.triki@gmail.com>
Reviewed-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/pressure/bmp280-core.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index 6cdc8ed53520..1f0852fc3414 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -3217,7 +3217,6 @@ int bmp280_common_probe(struct device *dev,
 		return dev_err_probe(dev, PTR_ERR(gpiod), "failed to get reset GPIO\n");
 
 	/* Deassert the signal */
-	dev_info(dev, "release reset\n");
 	gpiod_set_value(gpiod, 0);
 
 	data->regmap = regmap;
-- 
2.43.0

Re: [PATCH v4 2/3] iio: pressure: bmp280: Remove noisy dev_info()

[Andy Shevchenko]

On Mon, Aug 18, 2025 at 10:27:31AM +0100, Salah Triki wrote:
> Remove `dev_info()` call as it was considered noisy and is not
> necessary for normal driver operation.

I agree with this.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 2/3] iio: pressure: bmp280: Remove noisy dev_info()

[Jonathan Cameron]

On Wed, 20 Aug 2025 17:49:43 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Mon, Aug 18, 2025 at 10:27:31AM +0100, Salah Triki wrote:
> > Remove `dev_info()` call as it was considered noisy and is not
> > necessary for normal driver operation.  
> 
> I agree with this.
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 

Applied. Thanks

[PATCH v4 3/3] iio: pressure: bmp280: Use gpiod_set_value_cansleep()

[Salah Triki]

Switch to `gpiod_set_value_cansleep()`, which is safe to use in
sleepable contexts like the driver probe function.

Signed-off-by: Salah Triki <salah.triki@gmail.com>
Reviewed-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/pressure/bmp280-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index 1f0852fc3414..656f6189c84c 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -3217,7 +3217,7 @@ int bmp280_common_probe(struct device *dev,
 		return dev_err_probe(dev, PTR_ERR(gpiod), "failed to get reset GPIO\n");
 
 	/* Deassert the signal */
-	gpiod_set_value(gpiod, 0);
+	gpiod_set_value_cansleep(gpiod, 0);
 
 	data->regmap = regmap;
 
-- 
2.43.0

Re: [PATCH v4 3/3] iio: pressure: bmp280: Use gpiod_set_value_cansleep()

[Andy Shevchenko]

On Mon, Aug 18, 2025 at 10:27:32AM +0100, Salah Triki wrote:
> Switch to `gpiod_set_value_cansleep()`, which is safe to use in
> sleepable contexts like the driver probe function.

The commit message is unclear on why? aspect. You need to elaborate that some
GPIO chips may be located behind slow (and hence sleepable) busses, such as
I²C).

Code wise I agree with the change.
With amended commit message
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 3/3] iio: pressure: bmp280: Use gpiod_set_value_cansleep()

[Jonathan Cameron]

On Wed, 20 Aug 2025 17:51:22 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Mon, Aug 18, 2025 at 10:27:32AM +0100, Salah Triki wrote:
> > Switch to `gpiod_set_value_cansleep()`, which is safe to use in
> > sleepable contexts like the driver probe function.  
> 
> The commit message is unclear on why? aspect. You need to elaborate that some
> GPIO chips may be located behind slow (and hence sleepable) busses, such as
> I²C).
> 
> Code wise I agree with the change.
> With amended commit message
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 

I added a brief not on slow buses and applied.

Thanks,

Jonathan