* [PATCH] iio: light: gp2ap020a00f: Correct return type to int
@ 2026-02-11 0:48 Ethan Tidmore
2026-02-11 8:44 ` Andy Shevchenko
2026-02-14 19:08 ` Jonathan Cameron
0 siblings, 2 replies; 4+ messages in thread
From: Ethan Tidmore @ 2026-02-11 0:48 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: Andy Shevchenko, linux-iio, linux-kernel, Ethan Tidmore
The function gp2ap020a00f_get_thresh_reg() can return type -EINVAL in
its error path. Yet, the function has return type of u8. Added error
checking for gp2ap020a00f_get_thresh_reg() return's value.
Fixes: 5d6a25bad035 ("iio:gp2ap020a00f: Switch to new event config interface")
Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
---
drivers/iio/light/gp2ap020a00f.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/light/gp2ap020a00f.c b/drivers/iio/light/gp2ap020a00f.c
index c7df4b258e2c..492d1a029f77 100644
--- a/drivers/iio/light/gp2ap020a00f.c
+++ b/drivers/iio/light/gp2ap020a00f.c
@@ -992,7 +992,7 @@ static irqreturn_t gp2ap020a00f_trigger_handler(int irq, void *data)
return IRQ_HANDLED;
}
-static u8 gp2ap020a00f_get_thresh_reg(const struct iio_chan_spec *chan,
+static int gp2ap020a00f_get_thresh_reg(const struct iio_chan_spec *chan,
enum iio_event_direction event_dir)
{
switch (chan->type) {
@@ -1023,12 +1023,18 @@ static int gp2ap020a00f_write_event_val(struct iio_dev *indio_dev,
struct gp2ap020a00f_data *data = iio_priv(indio_dev);
bool event_en = false;
u8 thresh_val_id;
- u8 thresh_reg_l;
+ int thresh_reg_l;
int err = 0;
mutex_lock(&data->lock);
thresh_reg_l = gp2ap020a00f_get_thresh_reg(chan, dir);
+
+ if (thresh_reg_l < 0) {
+ err = thresh_reg_l;
+ goto error_unlock;
+ }
+
thresh_val_id = GP2AP020A00F_THRESH_VAL_ID(thresh_reg_l);
if (thresh_val_id > GP2AP020A00F_THRESH_PH) {
@@ -1080,13 +1086,18 @@ static int gp2ap020a00f_read_event_val(struct iio_dev *indio_dev,
int *val, int *val2)
{
struct gp2ap020a00f_data *data = iio_priv(indio_dev);
- u8 thresh_reg_l;
+ int thresh_reg_l;
int err = IIO_VAL_INT;
mutex_lock(&data->lock);
thresh_reg_l = gp2ap020a00f_get_thresh_reg(chan, dir);
+ if (thresh_reg_l < 0) {
+ err = thresh_reg_l;
+ goto error_unlock;
+ }
+
if (thresh_reg_l > GP2AP020A00F_PH_L_REG) {
err = -EINVAL;
goto error_unlock;
--
2.53.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] iio: light: gp2ap020a00f: Correct return type to int
2026-02-11 0:48 [PATCH] iio: light: gp2ap020a00f: Correct return type to int Ethan Tidmore
@ 2026-02-11 8:44 ` Andy Shevchenko
2026-02-14 19:08 ` Jonathan Cameron
1 sibling, 0 replies; 4+ messages in thread
From: Andy Shevchenko @ 2026-02-11 8:44 UTC (permalink / raw)
To: Ethan Tidmore; +Cc: Jonathan Cameron, Andy Shevchenko, linux-iio, linux-kernel
On Tue, Feb 10, 2026 at 06:48:19PM -0600, Ethan Tidmore wrote:
> The function gp2ap020a00f_get_thresh_reg() can return type -EINVAL in
> its error path. Yet, the function has return type of u8. Added error
> checking for gp2ap020a00f_get_thresh_reg() return's value.
...
> -static u8 gp2ap020a00f_get_thresh_reg(const struct iio_chan_spec *chan,
> +static int gp2ap020a00f_get_thresh_reg(const struct iio_chan_spec *chan,
> enum iio_event_direction event_dir)
While at it, correct the indentation of the second line.
...
> mutex_lock(&data->lock);
>
> thresh_reg_l = gp2ap020a00f_get_thresh_reg(chan, dir);
> +
Drop this blank line.
> + if (thresh_reg_l < 0) {
> + err = thresh_reg_l;
> + goto error_unlock;
> + }
> thresh_reg_l = gp2ap020a00f_get_thresh_reg(chan, dir);
>
Ditto, no blank line between an assignment and a conditional that checks
the value.
> + if (thresh_reg_l < 0) {
> + err = thresh_reg_l;
> + goto error_unlock;
> + }
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] iio: light: gp2ap020a00f: Correct return type to int
2026-02-11 0:48 [PATCH] iio: light: gp2ap020a00f: Correct return type to int Ethan Tidmore
2026-02-11 8:44 ` Andy Shevchenko
@ 2026-02-14 19:08 ` Jonathan Cameron
2026-02-14 19:25 ` Ethan Tidmore
1 sibling, 1 reply; 4+ messages in thread
From: Jonathan Cameron @ 2026-02-14 19:08 UTC (permalink / raw)
To: Ethan Tidmore; +Cc: Andy Shevchenko, linux-iio, linux-kernel
On Tue, 10 Feb 2026 18:48:19 -0600
Ethan Tidmore <ethantidmore06@gmail.com> wrote:
> The function gp2ap020a00f_get_thresh_reg() can return type -EINVAL in
> its error path. Yet, the function has return type of u8. Added error
> checking for gp2ap020a00f_get_thresh_reg() return's value.
Hi Ethan,
Did you find a path via which it can actually return an error? char
and dir are tightly constrained inputs.
I think the error return is probably just defensive coding against future bugs
and compiler warning supression.
If so it's still a good thing to clean up but Fixes tag and backports
probably not appropriate.
And best of all, once it's not a fix we can use some guard()
magic to make these functions and your new checks cleaner.
guard() usage would be a precursor patch with this one on top to minimize
churn.
Thanks,
Jonathan
>
> Fixes: 5d6a25bad035 ("iio:gp2ap020a00f: Switch to new event config interface")
> Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
> ---
> drivers/iio/light/gp2ap020a00f.c | 17 ++++++++++++++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/light/gp2ap020a00f.c b/drivers/iio/light/gp2ap020a00f.c
> index c7df4b258e2c..492d1a029f77 100644
> --- a/drivers/iio/light/gp2ap020a00f.c
> +++ b/drivers/iio/light/gp2ap020a00f.c
> @@ -992,7 +992,7 @@ static irqreturn_t gp2ap020a00f_trigger_handler(int irq, void *data)
> return IRQ_HANDLED;
> }
>
> -static u8 gp2ap020a00f_get_thresh_reg(const struct iio_chan_spec *chan,
> +static int gp2ap020a00f_get_thresh_reg(const struct iio_chan_spec *chan,
> enum iio_event_direction event_dir)
> {
> switch (chan->type) {
> @@ -1023,12 +1023,18 @@ static int gp2ap020a00f_write_event_val(struct iio_dev *indio_dev,
> struct gp2ap020a00f_data *data = iio_priv(indio_dev);
> bool event_en = false;
> u8 thresh_val_id;
> - u8 thresh_reg_l;
> + int thresh_reg_l;
> int err = 0;
>
> mutex_lock(&data->lock);
>
> thresh_reg_l = gp2ap020a00f_get_thresh_reg(chan, dir);
> +
> + if (thresh_reg_l < 0) {
> + err = thresh_reg_l;
> + goto error_unlock;
> + }
> +
> thresh_val_id = GP2AP020A00F_THRESH_VAL_ID(thresh_reg_l);
>
> if (thresh_val_id > GP2AP020A00F_THRESH_PH) {
> @@ -1080,13 +1086,18 @@ static int gp2ap020a00f_read_event_val(struct iio_dev *indio_dev,
> int *val, int *val2)
> {
> struct gp2ap020a00f_data *data = iio_priv(indio_dev);
> - u8 thresh_reg_l;
> + int thresh_reg_l;
> int err = IIO_VAL_INT;
>
> mutex_lock(&data->lock);
>
> thresh_reg_l = gp2ap020a00f_get_thresh_reg(chan, dir);
>
> + if (thresh_reg_l < 0) {
> + err = thresh_reg_l;
> + goto error_unlock;
> + }
> +
> if (thresh_reg_l > GP2AP020A00F_PH_L_REG) {
> err = -EINVAL;
> goto error_unlock;
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] iio: light: gp2ap020a00f: Correct return type to int
2026-02-14 19:08 ` Jonathan Cameron
@ 2026-02-14 19:25 ` Ethan Tidmore
0 siblings, 0 replies; 4+ messages in thread
From: Ethan Tidmore @ 2026-02-14 19:25 UTC (permalink / raw)
To: Jonathan Cameron, Ethan Tidmore; +Cc: Andy Shevchenko, linux-iio, linux-kernel
On Sat Feb 14, 2026 at 1:08 PM CST, Jonathan Cameron wrote:
> On Tue, 10 Feb 2026 18:48:19 -0600
> Ethan Tidmore <ethantidmore06@gmail.com> wrote:
...
> Did you find a path via which it can actually return an error? char
> and dir are tightly constrained inputs.
>
> I think the error return is probably just defensive coding against future bugs
> and compiler warning supression.
Yes you're right. I was just correcting a signness warning from smatch
and just added the extra checks.
>
> If so it's still a good thing to clean up but Fixes tag and backports
> probably not appropriate.
>
> And best of all, once it's not a fix we can use some guard()
> magic to make these functions and your new checks cleaner.
> guard() usage would be a precursor patch with this one on top to minimize
> churn.
I'll send a series adding these checks and using guard() and will drop
the Fixes: tag.
Thanks,
ET
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-02-14 19:25 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-11 0:48 [PATCH] iio: light: gp2ap020a00f: Correct return type to int Ethan Tidmore
2026-02-11 8:44 ` Andy Shevchenko
2026-02-14 19:08 ` Jonathan Cameron
2026-02-14 19:25 ` Ethan Tidmore
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox