* [PATCH v5 01/10] iio: light: gp2ap020a00f: simplify locking with guard()
2026-02-23 3:40 [PATCH v5 00/10] General clean up and bug fixes Ethan Tidmore
@ 2026-02-23 3:40 ` Ethan Tidmore
2026-02-23 11:54 ` Andy Shevchenko
2026-02-23 3:40 ` [PATCH v5 02/10] iio: light: gp2ap020a00f: correct return type to int Ethan Tidmore
` (9 subsequent siblings)
10 siblings, 1 reply; 19+ messages in thread
From: Ethan Tidmore @ 2026-02-23 3:40 UTC (permalink / raw)
To: Jonathan Cameron, Andy Shevchenko
Cc: David Lechner, Nuno Sá, linux-iio, linux-kernel,
Ethan Tidmore
Use the guard() cleanup handler to manage the device lock.
This simplifies the code by removing the need for manual unlocking
and goto error handling paths.
Suggested-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
---
v5:
- Integrate with Andy Shevchenko's cleanups.
v4:
- No change.
v3:
- No change.
v2:
- Make this a series and include guard() macro as suggested by Jonathan
Cameron.
drivers/iio/light/gp2ap020a00f.c | 110 ++++++++++---------------------
1 file changed, 35 insertions(+), 75 deletions(-)
diff --git a/drivers/iio/light/gp2ap020a00f.c b/drivers/iio/light/gp2ap020a00f.c
index c7df4b258e2c..7cee15db3a1a 100644
--- a/drivers/iio/light/gp2ap020a00f.c
+++ b/drivers/iio/light/gp2ap020a00f.c
@@ -31,6 +31,7 @@
* the other one.
*/
+#include <linux/cleanup.h>
#include <linux/debugfs.h>
#include <linux/delay.h>
#include <linux/i2c.h>
@@ -1024,17 +1025,13 @@ static int gp2ap020a00f_write_event_val(struct iio_dev *indio_dev,
bool event_en = false;
u8 thresh_val_id;
u8 thresh_reg_l;
- int err = 0;
- mutex_lock(&data->lock);
+ guard(mutex)(&data->lock);
thresh_reg_l = gp2ap020a00f_get_thresh_reg(chan, dir);
thresh_val_id = GP2AP020A00F_THRESH_VAL_ID(thresh_reg_l);
-
- if (thresh_val_id > GP2AP020A00F_THRESH_PH) {
- err = -EINVAL;
- goto error_unlock;
- }
+ if (thresh_val_id > GP2AP020A00F_THRESH_PH)
+ return -EINVAL;
switch (thresh_reg_l) {
case GP2AP020A00F_TH_L_REG:
@@ -1046,30 +1043,23 @@ static int gp2ap020a00f_write_event_val(struct iio_dev *indio_dev,
&data->flags);
break;
case GP2AP020A00F_PH_L_REG:
- if (val == 0) {
- err = -EINVAL;
- goto error_unlock;
- }
+ if (val == 0)
+ return -EINVAL;
+
event_en = test_bit(GP2AP020A00F_FLAG_PROX_RISING_EV,
&data->flags);
break;
case GP2AP020A00F_PL_L_REG:
- if (val == 0) {
- err = -EINVAL;
- goto error_unlock;
- }
+ if (val == 0)
+ return -EINVAL;
+
event_en = test_bit(GP2AP020A00F_FLAG_PROX_FALLING_EV,
&data->flags);
break;
}
data->thresh_val[thresh_val_id] = val;
- err = gp2ap020a00f_write_event_threshold(data, thresh_val_id,
- event_en);
-error_unlock:
- mutex_unlock(&data->lock);
-
- return err;
+ return gp2ap020a00f_write_event_threshold(data, thresh_val_id, event_en);
}
static int gp2ap020a00f_read_event_val(struct iio_dev *indio_dev,
@@ -1081,23 +1071,16 @@ static int gp2ap020a00f_read_event_val(struct iio_dev *indio_dev,
{
struct gp2ap020a00f_data *data = iio_priv(indio_dev);
u8 thresh_reg_l;
- int err = IIO_VAL_INT;
- mutex_lock(&data->lock);
+ guard(mutex)(&data->lock);
thresh_reg_l = gp2ap020a00f_get_thresh_reg(chan, dir);
-
- if (thresh_reg_l > GP2AP020A00F_PH_L_REG) {
- err = -EINVAL;
- goto error_unlock;
- }
+ if (thresh_reg_l > GP2AP020A00F_PH_L_REG)
+ return -EINVAL;
*val = data->thresh_val[GP2AP020A00F_THRESH_VAL_ID(thresh_reg_l)];
-error_unlock:
- mutex_unlock(&data->lock);
-
- return err;
+ return IIO_VAL_INT;
}
static int gp2ap020a00f_write_prox_event_config(struct iio_dev *indio_dev,
@@ -1163,32 +1146,25 @@ static int gp2ap020a00f_write_event_config(struct iio_dev *indio_dev,
{
struct gp2ap020a00f_data *data = iio_priv(indio_dev);
enum gp2ap020a00f_cmd cmd;
- int err;
- mutex_lock(&data->lock);
+ guard(mutex)(&data->lock);
switch (chan->type) {
case IIO_PROXIMITY:
- err = gp2ap020a00f_write_prox_event_config(indio_dev, state);
- break;
+ return gp2ap020a00f_write_prox_event_config(indio_dev, state);
case IIO_LIGHT:
if (dir == IIO_EV_DIR_RISING) {
cmd = state ? GP2AP020A00F_CMD_ALS_HIGH_EV_EN :
GP2AP020A00F_CMD_ALS_HIGH_EV_DIS;
- err = gp2ap020a00f_exec_cmd(data, cmd);
+ return gp2ap020a00f_exec_cmd(data, cmd);
} else {
cmd = state ? GP2AP020A00F_CMD_ALS_LOW_EV_EN :
GP2AP020A00F_CMD_ALS_LOW_EV_DIS;
- err = gp2ap020a00f_exec_cmd(data, cmd);
+ return gp2ap020a00f_exec_cmd(data, cmd);
}
- break;
default:
- err = -EINVAL;
+ return -EINVAL;
}
-
- mutex_unlock(&data->lock);
-
- return err;
}
static int gp2ap020a00f_read_event_config(struct iio_dev *indio_dev,
@@ -1197,35 +1173,23 @@ static int gp2ap020a00f_read_event_config(struct iio_dev *indio_dev,
enum iio_event_direction dir)
{
struct gp2ap020a00f_data *data = iio_priv(indio_dev);
- int event_en = 0;
- mutex_lock(&data->lock);
+ guard(mutex)(&data->lock);
switch (chan->type) {
case IIO_PROXIMITY:
if (dir == IIO_EV_DIR_RISING)
- event_en = test_bit(GP2AP020A00F_FLAG_PROX_RISING_EV,
- &data->flags);
+ return test_bit(GP2AP020A00F_FLAG_PROX_RISING_EV, &data->flags);
else
- event_en = test_bit(GP2AP020A00F_FLAG_PROX_FALLING_EV,
- &data->flags);
- break;
+ return test_bit(GP2AP020A00F_FLAG_PROX_FALLING_EV, &data->flags);
case IIO_LIGHT:
if (dir == IIO_EV_DIR_RISING)
- event_en = test_bit(GP2AP020A00F_FLAG_ALS_RISING_EV,
- &data->flags);
+ return test_bit(GP2AP020A00F_FLAG_ALS_RISING_EV, &data->flags);
else
- event_en = test_bit(GP2AP020A00F_FLAG_ALS_FALLING_EV,
- &data->flags);
- break;
+ return test_bit(GP2AP020A00F_FLAG_ALS_FALLING_EV, &data->flags);
default:
- event_en = -EINVAL;
- break;
+ return -EINVAL;
}
-
- mutex_unlock(&data->lock);
-
- return event_en;
}
static int gp2ap020a00f_read_channel(struct gp2ap020a00f_data *data,
@@ -1385,7 +1349,7 @@ static int gp2ap020a00f_buffer_postenable(struct iio_dev *indio_dev)
struct gp2ap020a00f_data *data = iio_priv(indio_dev);
int i, err = 0;
- mutex_lock(&data->lock);
+ guard(mutex)(&data->lock);
/*
* Enable triggers according to the scan_mask. Enabling either
@@ -1413,16 +1377,13 @@ static int gp2ap020a00f_buffer_postenable(struct iio_dev *indio_dev)
}
if (err < 0)
- goto error_unlock;
+ return err;
data->buffer = kmalloc(indio_dev->scan_bytes, GFP_KERNEL);
if (!data->buffer)
- err = -ENOMEM;
-
-error_unlock:
- mutex_unlock(&data->lock);
+ return -ENOMEM;
- return err;
+ return 0;
}
static int gp2ap020a00f_buffer_predisable(struct iio_dev *indio_dev)
@@ -1430,7 +1391,7 @@ static int gp2ap020a00f_buffer_predisable(struct iio_dev *indio_dev)
struct gp2ap020a00f_data *data = iio_priv(indio_dev);
int i, err = 0;
- mutex_lock(&data->lock);
+ guard(mutex)(&data->lock);
iio_for_each_active_channel(indio_dev, i) {
switch (i) {
@@ -1449,12 +1410,11 @@ static int gp2ap020a00f_buffer_predisable(struct iio_dev *indio_dev)
}
}
- if (err == 0)
- kfree(data->buffer);
-
- mutex_unlock(&data->lock);
+ if (err)
+ return err;
- return err;
+ kfree(data->buffer);
+ return 0;
}
static const struct iio_buffer_setup_ops gp2ap020a00f_buffer_setup_ops = {
--
2.53.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v5 01/10] iio: light: gp2ap020a00f: simplify locking with guard()
2026-02-23 3:40 ` [PATCH v5 01/10] iio: light: gp2ap020a00f: simplify locking with guard() Ethan Tidmore
@ 2026-02-23 11:54 ` Andy Shevchenko
0 siblings, 0 replies; 19+ messages in thread
From: Andy Shevchenko @ 2026-02-23 11:54 UTC (permalink / raw)
To: Ethan Tidmore
Cc: Jonathan Cameron, Andy Shevchenko, David Lechner, Nuno Sá,
linux-iio, linux-kernel
On Sun, Feb 22, 2026 at 09:40:11PM -0600, Ethan Tidmore wrote:
> Use the guard() cleanup handler to manage the device lock.
> This simplifies the code by removing the need for manual unlocking
> and goto error handling paths.
>
> Suggested-by: Jonathan Cameron <jic23@kernel.org>
> Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
> ---
> v5:
> - Integrate with Andy Shevchenko's cleanups.
Bingo!
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v5 02/10] iio: light: gp2ap020a00f: correct return type to int
2026-02-23 3:40 [PATCH v5 00/10] General clean up and bug fixes Ethan Tidmore
2026-02-23 3:40 ` [PATCH v5 01/10] iio: light: gp2ap020a00f: simplify locking with guard() Ethan Tidmore
@ 2026-02-23 3:40 ` Ethan Tidmore
2026-02-23 3:40 ` [PATCH v5 03/10] iio: light: gp2ap020a00f: Use correct types for 16-bit LE data Ethan Tidmore
` (8 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Ethan Tidmore @ 2026-02-23 3:40 UTC (permalink / raw)
To: Jonathan Cameron, Andy Shevchenko
Cc: David Lechner, Nuno Sá, linux-iio, linux-kernel,
Ethan Tidmore, Andy Shevchenko
The function gp2ap020a00f_get_thresh_reg() can return -EINVAL in its
error path. Yet, the function has return type of u8. Added error
checking for gp2ap020a00f_get_thresh_reg() return value.
Detected by Smatch:
drivers/iio/light/gp2ap020a00f.c:1013 gp2ap020a00f_get_thresh_reg() warn:
signedness bug returning '(-22)'
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
---
v5:
- No change.
v4:
- Added Reviewed-by: tag from Andy Shevchenko.
v3:
- Remove Fixes: tag.
- Added Smatch warning.
v2:
- Fixed gp2ap020a00f_get_thresh_reg() parameter alignment.
- Removed unneeded whitespace between assignment and check.
drivers/iio/light/gp2ap020a00f.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/light/gp2ap020a00f.c b/drivers/iio/light/gp2ap020a00f.c
index 7cee15db3a1a..6fe4486101b8 100644
--- a/drivers/iio/light/gp2ap020a00f.c
+++ b/drivers/iio/light/gp2ap020a00f.c
@@ -993,8 +993,8 @@ 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,
- enum iio_event_direction event_dir)
+static int gp2ap020a00f_get_thresh_reg(const struct iio_chan_spec *chan,
+ enum iio_event_direction event_dir)
{
switch (chan->type) {
case IIO_PROXIMITY:
@@ -1024,11 +1024,14 @@ 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;
guard(mutex)(&data->lock);
thresh_reg_l = gp2ap020a00f_get_thresh_reg(chan, dir);
+ if (thresh_reg_l < 0)
+ return thresh_reg_l;
+
thresh_val_id = GP2AP020A00F_THRESH_VAL_ID(thresh_reg_l);
if (thresh_val_id > GP2AP020A00F_THRESH_PH)
return -EINVAL;
@@ -1070,11 +1073,13 @@ 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;
guard(mutex)(&data->lock);
thresh_reg_l = gp2ap020a00f_get_thresh_reg(chan, dir);
+ if (thresh_reg_l < 0)
+ return thresh_reg_l;
if (thresh_reg_l > GP2AP020A00F_PH_L_REG)
return -EINVAL;
--
2.53.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v5 03/10] iio: light: gp2ap020a00f: Use correct types for 16-bit LE data
2026-02-23 3:40 [PATCH v5 00/10] General clean up and bug fixes Ethan Tidmore
2026-02-23 3:40 ` [PATCH v5 01/10] iio: light: gp2ap020a00f: simplify locking with guard() Ethan Tidmore
2026-02-23 3:40 ` [PATCH v5 02/10] iio: light: gp2ap020a00f: correct return type to int Ethan Tidmore
@ 2026-02-23 3:40 ` Ethan Tidmore
2026-02-23 3:40 ` [PATCH v5 04/10] iio: light: gp2ap020a00f: Return directly from the switch cases Ethan Tidmore
` (7 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Ethan Tidmore @ 2026-02-23 3:40 UTC (permalink / raw)
To: Jonathan Cameron, Andy Shevchenko
Cc: David Lechner, Nuno Sá, linux-iio, linux-kernel,
Andy Shevchenko, Ethan Tidmore
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Instead of using byte arrays and then explicit castings, change
the types of byte arrays to be __le16 and update the endianness
conversions accordingly.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
---
v5:
- No change.
v4:
- Integrate Andy Shevchenko's cleanups.
drivers/iio/light/gp2ap020a00f.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/iio/light/gp2ap020a00f.c b/drivers/iio/light/gp2ap020a00f.c
index 6fe4486101b8..8558c4c3ef7a 100644
--- a/drivers/iio/light/gp2ap020a00f.c
+++ b/drivers/iio/light/gp2ap020a00f.c
@@ -462,7 +462,7 @@ static int gp2ap020a00f_write_event_threshold(struct gp2ap020a00f_data *data,
return regmap_bulk_write(data->regmap,
GP2AP020A00F_THRESH_REG(th_val_id),
- (u8 *)&thresh_buf, 2);
+ &thresh_buf, sizeof(thresh_buf));
}
static int gp2ap020a00f_alter_opmode(struct gp2ap020a00f_data *data,
@@ -698,18 +698,18 @@ static int wait_conversion_complete_irq(struct gp2ap020a00f_data *data)
static int gp2ap020a00f_read_output(struct gp2ap020a00f_data *data,
unsigned int output_reg, int *val)
{
- u8 reg_buf[2];
+ __le16 reg_buf;
int err;
err = wait_conversion_complete_irq(data);
if (err < 0)
dev_dbg(&data->client->dev, "data ready timeout\n");
- err = regmap_bulk_read(data->regmap, output_reg, reg_buf, 2);
+ err = regmap_bulk_read(data->regmap, output_reg, ®_buf, sizeof(reg_buf));
if (err < 0)
return err;
- *val = le16_to_cpup((__le16 *)reg_buf);
+ *val = le16_to_cpu(reg_buf);
return err;
}
@@ -867,8 +867,9 @@ static irqreturn_t gp2ap020a00f_thresh_event_handler(int irq, void *data)
{
struct iio_dev *indio_dev = data;
struct gp2ap020a00f_data *priv = iio_priv(indio_dev);
- u8 op_reg_flags, d0_reg_buf[2];
unsigned int output_val, op_reg_val;
+ __le16 d0_reg_buf;
+ u8 op_reg_flags;
int thresh_val_id, ret;
/* Read interrupt flags */
@@ -896,11 +897,11 @@ static irqreturn_t gp2ap020a00f_thresh_event_handler(int irq, void *data)
* transition is required.
*/
ret = regmap_bulk_read(priv->regmap, GP2AP020A00F_D0_L_REG,
- d0_reg_buf, 2);
+ &d0_reg_buf, sizeof(d0_reg_buf));
if (ret < 0)
goto done;
- output_val = le16_to_cpup((__le16 *)d0_reg_buf);
+ output_val = le16_to_cpu(d0_reg_buf);
if (gp2ap020a00f_adjust_lux_mode(priv, output_val))
goto done;
@@ -967,17 +968,15 @@ static irqreturn_t gp2ap020a00f_trigger_handler(int irq, void *data)
int i, out_val, ret;
iio_for_each_active_channel(indio_dev, i) {
- ret = regmap_bulk_read(priv->regmap,
- GP2AP020A00F_DATA_REG(i),
- &priv->buffer[d_size], 2);
+ ret = regmap_bulk_read(priv->regmap, GP2AP020A00F_DATA_REG(i),
+ &priv->buffer[d_size], 2);
if (ret < 0)
goto done;
if (i == GP2AP020A00F_SCAN_MODE_LIGHT_CLEAR ||
i == GP2AP020A00F_SCAN_MODE_LIGHT_IR) {
- out_val = le16_to_cpup((__le16 *)&priv->buffer[d_size]);
+ out_val = get_unaligned_le16(&priv->buffer[d_size]);
gp2ap020a00f_output_to_lux(priv, &out_val);
-
put_unaligned_le32(out_val, &priv->buffer[d_size]);
d_size += 4;
} else {
--
2.53.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v5 04/10] iio: light: gp2ap020a00f: Return directly from the switch cases
2026-02-23 3:40 [PATCH v5 00/10] General clean up and bug fixes Ethan Tidmore
` (2 preceding siblings ...)
2026-02-23 3:40 ` [PATCH v5 03/10] iio: light: gp2ap020a00f: Use correct types for 16-bit LE data Ethan Tidmore
@ 2026-02-23 3:40 ` Ethan Tidmore
2026-02-23 12:00 ` Andy Shevchenko
2026-02-23 3:40 ` [PATCH v5 05/10] iio: light: gp2ap020a00f: Fix possible error swallow Ethan Tidmore
` (6 subsequent siblings)
10 siblings, 1 reply; 19+ messages in thread
From: Ethan Tidmore @ 2026-02-23 3:40 UTC (permalink / raw)
To: Jonathan Cameron, Andy Shevchenko
Cc: David Lechner, Nuno Sá, linux-iio, linux-kernel,
Andy Shevchenko, Ethan Tidmore
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Return directly from the switch cases which makes code easier to follow.
In some cases convert pieces to the standard pattern which also unifies
it with the accepted kernel practices.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
---
v5:
- Split patch up as requested by Andy Shevchenko.
v4:
- Integrate Andy Shevchenko's cleanups.
drivers/iio/light/gp2ap020a00f.c | 60 +++++++++++++-------------------
1 file changed, 24 insertions(+), 36 deletions(-)
diff --git a/drivers/iio/light/gp2ap020a00f.c b/drivers/iio/light/gp2ap020a00f.c
index 8558c4c3ef7a..6b283b52698f 100644
--- a/drivers/iio/light/gp2ap020a00f.c
+++ b/drivers/iio/light/gp2ap020a00f.c
@@ -494,27 +494,24 @@ static int gp2ap020a00f_alter_opmode(struct gp2ap020a00f_data *data,
static int gp2ap020a00f_exec_cmd(struct gp2ap020a00f_data *data,
enum gp2ap020a00f_cmd cmd)
{
- int err = 0;
+ int err;
switch (cmd) {
case GP2AP020A00F_CMD_READ_RAW_CLEAR:
if (data->cur_opmode != GP2AP020A00F_OPMODE_SHUTDOWN)
return -EBUSY;
- err = gp2ap020a00f_set_operation_mode(data,
+ return gp2ap020a00f_set_operation_mode(data,
GP2AP020A00F_OPMODE_READ_RAW_CLEAR);
- break;
case GP2AP020A00F_CMD_READ_RAW_IR:
if (data->cur_opmode != GP2AP020A00F_OPMODE_SHUTDOWN)
return -EBUSY;
- err = gp2ap020a00f_set_operation_mode(data,
+ return gp2ap020a00f_set_operation_mode(data,
GP2AP020A00F_OPMODE_READ_RAW_IR);
- break;
case GP2AP020A00F_CMD_READ_RAW_PROXIMITY:
if (data->cur_opmode != GP2AP020A00F_OPMODE_SHUTDOWN)
return -EBUSY;
- err = gp2ap020a00f_set_operation_mode(data,
+ return gp2ap020a00f_set_operation_mode(data,
GP2AP020A00F_OPMODE_READ_RAW_PROXIMITY);
- break;
case GP2AP020A00F_CMD_TRIGGER_CLEAR_EN:
if (data->cur_opmode == GP2AP020A00F_OPMODE_PROX_DETECT)
return -EBUSY;
@@ -522,16 +519,17 @@ static int gp2ap020a00f_exec_cmd(struct gp2ap020a00f_data *data,
err = gp2ap020a00f_alter_opmode(data,
GP2AP020A00F_OPMODE_ALS,
GP2AP020A00F_ADD_MODE);
+ else
+ err = 0;
set_bit(GP2AP020A00F_FLAG_ALS_CLEAR_TRIGGER, &data->flags);
- break;
+ return err;
case GP2AP020A00F_CMD_TRIGGER_CLEAR_DIS:
clear_bit(GP2AP020A00F_FLAG_ALS_CLEAR_TRIGGER, &data->flags);
if (gp2ap020a00f_als_enabled(data))
break;
- err = gp2ap020a00f_alter_opmode(data,
+ return gp2ap020a00f_alter_opmode(data,
GP2AP020A00F_OPMODE_ALS,
GP2AP020A00F_SUBTRACT_MODE);
- break;
case GP2AP020A00F_CMD_TRIGGER_IR_EN:
if (data->cur_opmode == GP2AP020A00F_OPMODE_PROX_DETECT)
return -EBUSY;
@@ -539,16 +537,17 @@ static int gp2ap020a00f_exec_cmd(struct gp2ap020a00f_data *data,
err = gp2ap020a00f_alter_opmode(data,
GP2AP020A00F_OPMODE_ALS,
GP2AP020A00F_ADD_MODE);
+ else
+ err = 0;
set_bit(GP2AP020A00F_FLAG_ALS_IR_TRIGGER, &data->flags);
- break;
+ return err;
case GP2AP020A00F_CMD_TRIGGER_IR_DIS:
clear_bit(GP2AP020A00F_FLAG_ALS_IR_TRIGGER, &data->flags);
if (gp2ap020a00f_als_enabled(data))
break;
- err = gp2ap020a00f_alter_opmode(data,
+ return gp2ap020a00f_alter_opmode(data,
GP2AP020A00F_OPMODE_ALS,
GP2AP020A00F_SUBTRACT_MODE);
- break;
case GP2AP020A00F_CMD_TRIGGER_PROX_EN:
if (data->cur_opmode == GP2AP020A00F_OPMODE_PROX_DETECT)
return -EBUSY;
@@ -556,13 +555,12 @@ static int gp2ap020a00f_exec_cmd(struct gp2ap020a00f_data *data,
GP2AP020A00F_OPMODE_PS,
GP2AP020A00F_ADD_MODE);
set_bit(GP2AP020A00F_FLAG_PROX_TRIGGER, &data->flags);
- break;
+ return err;
case GP2AP020A00F_CMD_TRIGGER_PROX_DIS:
clear_bit(GP2AP020A00F_FLAG_PROX_TRIGGER, &data->flags);
- err = gp2ap020a00f_alter_opmode(data,
+ return gp2ap020a00f_alter_opmode(data,
GP2AP020A00F_OPMODE_PS,
GP2AP020A00F_SUBTRACT_MODE);
- break;
case GP2AP020A00F_CMD_ALS_HIGH_EV_EN:
if (test_bit(GP2AP020A00F_FLAG_ALS_RISING_EV, &data->flags))
return 0;
@@ -576,9 +574,8 @@ static int gp2ap020a00f_exec_cmd(struct gp2ap020a00f_data *data,
return err;
}
set_bit(GP2AP020A00F_FLAG_ALS_RISING_EV, &data->flags);
- err = gp2ap020a00f_write_event_threshold(data,
+ return gp2ap020a00f_write_event_threshold(data,
GP2AP020A00F_THRESH_TH, true);
- break;
case GP2AP020A00F_CMD_ALS_HIGH_EV_DIS:
if (!test_bit(GP2AP020A00F_FLAG_ALS_RISING_EV, &data->flags))
return 0;
@@ -590,9 +587,8 @@ static int gp2ap020a00f_exec_cmd(struct gp2ap020a00f_data *data,
if (err < 0)
return err;
}
- err = gp2ap020a00f_write_event_threshold(data,
+ return gp2ap020a00f_write_event_threshold(data,
GP2AP020A00F_THRESH_TH, false);
- break;
case GP2AP020A00F_CMD_ALS_LOW_EV_EN:
if (test_bit(GP2AP020A00F_FLAG_ALS_FALLING_EV, &data->flags))
return 0;
@@ -606,9 +602,8 @@ static int gp2ap020a00f_exec_cmd(struct gp2ap020a00f_data *data,
return err;
}
set_bit(GP2AP020A00F_FLAG_ALS_FALLING_EV, &data->flags);
- err = gp2ap020a00f_write_event_threshold(data,
+ return gp2ap020a00f_write_event_threshold(data,
GP2AP020A00F_THRESH_TL, true);
- break;
case GP2AP020A00F_CMD_ALS_LOW_EV_DIS:
if (!test_bit(GP2AP020A00F_FLAG_ALS_FALLING_EV, &data->flags))
return 0;
@@ -620,9 +615,8 @@ static int gp2ap020a00f_exec_cmd(struct gp2ap020a00f_data *data,
if (err < 0)
return err;
}
- err = gp2ap020a00f_write_event_threshold(data,
+ return gp2ap020a00f_write_event_threshold(data,
GP2AP020A00F_THRESH_TL, false);
- break;
case GP2AP020A00F_CMD_PROX_HIGH_EV_EN:
if (test_bit(GP2AP020A00F_FLAG_PROX_RISING_EV, &data->flags))
return 0;
@@ -636,9 +630,8 @@ static int gp2ap020a00f_exec_cmd(struct gp2ap020a00f_data *data,
return err;
}
set_bit(GP2AP020A00F_FLAG_PROX_RISING_EV, &data->flags);
- err = gp2ap020a00f_write_event_threshold(data,
+ return gp2ap020a00f_write_event_threshold(data,
GP2AP020A00F_THRESH_PH, true);
- break;
case GP2AP020A00F_CMD_PROX_HIGH_EV_DIS:
if (!test_bit(GP2AP020A00F_FLAG_PROX_RISING_EV, &data->flags))
return 0;
@@ -647,9 +640,8 @@ static int gp2ap020a00f_exec_cmd(struct gp2ap020a00f_data *data,
GP2AP020A00F_OPMODE_SHUTDOWN);
if (err < 0)
return err;
- err = gp2ap020a00f_write_event_threshold(data,
+ return gp2ap020a00f_write_event_threshold(data,
GP2AP020A00F_THRESH_PH, false);
- break;
case GP2AP020A00F_CMD_PROX_LOW_EV_EN:
if (test_bit(GP2AP020A00F_FLAG_PROX_FALLING_EV, &data->flags))
return 0;
@@ -663,9 +655,8 @@ static int gp2ap020a00f_exec_cmd(struct gp2ap020a00f_data *data,
return err;
}
set_bit(GP2AP020A00F_FLAG_PROX_FALLING_EV, &data->flags);
- err = gp2ap020a00f_write_event_threshold(data,
+ return gp2ap020a00f_write_event_threshold(data,
GP2AP020A00F_THRESH_PL, true);
- break;
case GP2AP020A00F_CMD_PROX_LOW_EV_DIS:
if (!test_bit(GP2AP020A00F_FLAG_PROX_FALLING_EV, &data->flags))
return 0;
@@ -674,12 +665,11 @@ static int gp2ap020a00f_exec_cmd(struct gp2ap020a00f_data *data,
GP2AP020A00F_OPMODE_SHUTDOWN);
if (err < 0)
return err;
- err = gp2ap020a00f_write_event_threshold(data,
+ return gp2ap020a00f_write_event_threshold(data,
GP2AP020A00F_THRESH_PL, false);
- break;
}
- return err;
+ return 0;
}
static int wait_conversion_complete_irq(struct gp2ap020a00f_data *data)
@@ -1007,10 +997,8 @@ static int gp2ap020a00f_get_thresh_reg(const struct iio_chan_spec *chan,
else
return GP2AP020A00F_TL_L_REG;
default:
- break;
+ return -EINVAL;
}
-
- return -EINVAL;
}
static int gp2ap020a00f_write_event_val(struct iio_dev *indio_dev,
--
2.53.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v5 04/10] iio: light: gp2ap020a00f: Return directly from the switch cases
2026-02-23 3:40 ` [PATCH v5 04/10] iio: light: gp2ap020a00f: Return directly from the switch cases Ethan Tidmore
@ 2026-02-23 12:00 ` Andy Shevchenko
0 siblings, 0 replies; 19+ messages in thread
From: Andy Shevchenko @ 2026-02-23 12:00 UTC (permalink / raw)
To: Ethan Tidmore
Cc: Jonathan Cameron, Andy Shevchenko, David Lechner, Nuno Sá,
linux-iio, linux-kernel
On Sun, Feb 22, 2026 at 09:40:14PM -0600, Ethan Tidmore wrote:
> Return directly from the switch cases which makes code easier to follow.
> In some cases convert pieces to the standard pattern which also unifies
> it with the accepted kernel practices.
...
> static int gp2ap020a00f_exec_cmd(struct gp2ap020a00f_data *data,
> GP2AP020A00F_OPMODE_SHUTDOWN);
> if (err < 0)
> return err;
> - err = gp2ap020a00f_write_event_threshold(data,
> + return gp2ap020a00f_write_event_threshold(data,
> GP2AP020A00F_THRESH_PL, false);
> - break;
> }
> - return err;
> + return 0;
I would make it also to be a 'default' case. But the driver uses different
styles anyway and I can't easily find a common denominator, so it's fine to
have it like this.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v5 05/10] iio: light: gp2ap020a00f: Fix possible error swallow
2026-02-23 3:40 [PATCH v5 00/10] General clean up and bug fixes Ethan Tidmore
` (3 preceding siblings ...)
2026-02-23 3:40 ` [PATCH v5 04/10] iio: light: gp2ap020a00f: Return directly from the switch cases Ethan Tidmore
@ 2026-02-23 3:40 ` Ethan Tidmore
2026-02-23 12:05 ` Andy Shevchenko
2026-02-23 3:40 ` [PATCH v5 06/10] iio: light: gp2ap020a00f: Replace custom implementation of min() Ethan Tidmore
` (5 subsequent siblings)
10 siblings, 1 reply; 19+ messages in thread
From: Ethan Tidmore @ 2026-02-23 3:40 UTC (permalink / raw)
To: Jonathan Cameron, Andy Shevchenko
Cc: David Lechner, Nuno Sá, linux-iio, linux-kernel,
Ethan Tidmore, Andy Shevchenko
Move error check into for loop in gp2ap020a00f_buffer_postenable() and
gp2ap020a00f_buffer_predisable(), this fixes a possible error swallow.
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
---
v5:
- Split this patch from the patch number 4, to keep bug fix separate
from style fix.
drivers/iio/light/gp2ap020a00f.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/light/gp2ap020a00f.c b/drivers/iio/light/gp2ap020a00f.c
index 6b283b52698f..d2156e133b75 100644
--- a/drivers/iio/light/gp2ap020a00f.c
+++ b/drivers/iio/light/gp2ap020a00f.c
@@ -1339,7 +1339,7 @@ static const struct iio_info gp2ap020a00f_info = {
static int gp2ap020a00f_buffer_postenable(struct iio_dev *indio_dev)
{
struct gp2ap020a00f_data *data = iio_priv(indio_dev);
- int i, err = 0;
+ int i, err;
guard(mutex)(&data->lock);
@@ -1366,11 +1366,10 @@ static int gp2ap020a00f_buffer_postenable(struct iio_dev *indio_dev)
GP2AP020A00F_CMD_TRIGGER_PROX_EN);
break;
}
+ if (err)
+ return err;
}
- if (err < 0)
- return err;
-
data->buffer = kmalloc(indio_dev->scan_bytes, GFP_KERNEL);
if (!data->buffer)
return -ENOMEM;
@@ -1381,7 +1380,7 @@ static int gp2ap020a00f_buffer_postenable(struct iio_dev *indio_dev)
static int gp2ap020a00f_buffer_predisable(struct iio_dev *indio_dev)
{
struct gp2ap020a00f_data *data = iio_priv(indio_dev);
- int i, err = 0;
+ int i, err;
guard(mutex)(&data->lock);
@@ -1400,11 +1399,10 @@ static int gp2ap020a00f_buffer_predisable(struct iio_dev *indio_dev)
GP2AP020A00F_CMD_TRIGGER_PROX_DIS);
break;
}
+ if (err)
+ return err;
}
- if (err)
- return err;
-
kfree(data->buffer);
return 0;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v5 05/10] iio: light: gp2ap020a00f: Fix possible error swallow
2026-02-23 3:40 ` [PATCH v5 05/10] iio: light: gp2ap020a00f: Fix possible error swallow Ethan Tidmore
@ 2026-02-23 12:05 ` Andy Shevchenko
2026-02-23 21:15 ` Jonathan Cameron
0 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2026-02-23 12:05 UTC (permalink / raw)
To: Ethan Tidmore
Cc: Jonathan Cameron, Andy Shevchenko, David Lechner, Nuno Sá,
linux-iio, linux-kernel
On Sun, Feb 22, 2026 at 09:40:15PM -0600, Ethan Tidmore wrote:
> Move error check into for loop in gp2ap020a00f_buffer_postenable() and
> gp2ap020a00f_buffer_predisable(), this fixes a possible error swallow.
...
> - int i, err = 0;
> + int i, err;
I believe we can't do that without introducing a 'default' case.
> guard(mutex)(&data->lock);
>
> @@ -1366,11 +1366,10 @@ static int gp2ap020a00f_buffer_postenable(struct iio_dev *indio_dev)
> GP2AP020A00F_CMD_TRIGGER_PROX_EN);
> break;
> }
> + if (err)
> + return err;
> }
>
> - if (err < 0)
> - return err;
(Also you might want to mention dropping '< 0' for making the style consistent
taking into account that those functions won't return any positive values.)
> data->buffer = kmalloc(indio_dev->scan_bytes, GFP_KERNEL);
> if (!data->buffer)
> return -ENOMEM;
> @@ -1381,7 +1380,7 @@ static int gp2ap020a00f_buffer_postenable(struct iio_dev *indio_dev)
> static int gp2ap020a00f_buffer_predisable(struct iio_dev *indio_dev)
> {
> struct gp2ap020a00f_data *data = iio_priv(indio_dev);
> - int i, err = 0;
> + int i, err;
Ditto.
> guard(mutex)(&data->lock);
>
> @@ -1400,11 +1399,10 @@ static int gp2ap020a00f_buffer_predisable(struct iio_dev *indio_dev)
> GP2AP020A00F_CMD_TRIGGER_PROX_DIS);
> break;
> }
> + if (err)
> + return err;
> }
>
> - if (err)
> - return err;
> -
> kfree(data->buffer);
> return 0;
> }
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v5 05/10] iio: light: gp2ap020a00f: Fix possible error swallow
2026-02-23 12:05 ` Andy Shevchenko
@ 2026-02-23 21:15 ` Jonathan Cameron
0 siblings, 0 replies; 19+ messages in thread
From: Jonathan Cameron @ 2026-02-23 21:15 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Ethan Tidmore, Andy Shevchenko, David Lechner, Nuno Sá,
linux-iio, linux-kernel
On Mon, 23 Feb 2026 14:05:19 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> On Sun, Feb 22, 2026 at 09:40:15PM -0600, Ethan Tidmore wrote:
> > Move error check into for loop in gp2ap020a00f_buffer_postenable() and
> > gp2ap020a00f_buffer_predisable(), this fixes a possible error swallow.
>
> ...
>
> > - int i, err = 0;
> > + int i, err;
>
> I believe we can't do that without introducing a 'default' case.
I agree. I'm a bit confused why LLVM at least isn't moaning about this.
Anyhow, I'm not sure why it defaulted to 0 previously...
diff --git a/drivers/iio/light/gp2ap020a00f.c b/drivers/iio/light/gp2ap020a00f.c
index d2156e133b75..64be9b8ca5e3 100644
--- a/drivers/iio/light/gp2ap020a00f.c
+++ b/drivers/iio/light/gp2ap020a00f.c
@@ -1365,6 +1365,9 @@ static int gp2ap020a00f_buffer_postenable(struct iio_dev *indio_dev)
err = gp2ap020a00f_exec_cmd(data,
GP2AP020A00F_CMD_TRIGGER_PROX_EN);
break;
+ default:
+ err = -EINVAL;
+ break;
}
if (err)
return err;
@@ -1398,6 +1401,9 @@ static int gp2ap020a00f_buffer_predisable(struct iio_dev *indio_dev)
err = gp2ap020a00f_exec_cmd(data,
GP2AP020A00F_CMD_TRIGGER_PROX_DIS);
break;
+ default:
+ err = -EINVAL;
+ break;
}
if (err)
return err;
Is the tweak I applied to resolve this.
Thanks,
Jonathan
>
> > guard(mutex)(&data->lock);
> >
> > @@ -1366,11 +1366,10 @@ static int gp2ap020a00f_buffer_postenable(struct iio_dev *indio_dev)
> > GP2AP020A00F_CMD_TRIGGER_PROX_EN);
> > break;
> > }
> > + if (err)
> > + return err;
> > }
> >
> > - if (err < 0)
> > - return err;
>
> (Also you might want to mention dropping '< 0' for making the style consistent
> taking into account that those functions won't return any positive values.)
>
> > data->buffer = kmalloc(indio_dev->scan_bytes, GFP_KERNEL);
> > if (!data->buffer)
> > return -ENOMEM;
> > @@ -1381,7 +1380,7 @@ static int gp2ap020a00f_buffer_postenable(struct iio_dev *indio_dev)
> > static int gp2ap020a00f_buffer_predisable(struct iio_dev *indio_dev)
> > {
> > struct gp2ap020a00f_data *data = iio_priv(indio_dev);
> > - int i, err = 0;
> > + int i, err;
>
> Ditto.
>
> > guard(mutex)(&data->lock);
> >
> > @@ -1400,11 +1399,10 @@ static int gp2ap020a00f_buffer_predisable(struct iio_dev *indio_dev)
> > GP2AP020A00F_CMD_TRIGGER_PROX_DIS);
> > break;
> > }
> > + if (err)
> > + return err;
> > }
> >
> > - if (err)
> > - return err;
> > -
> > kfree(data->buffer);
> > return 0;
> > }
>
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v5 06/10] iio: light: gp2ap020a00f: Replace custom implementation of min()
2026-02-23 3:40 [PATCH v5 00/10] General clean up and bug fixes Ethan Tidmore
` (4 preceding siblings ...)
2026-02-23 3:40 ` [PATCH v5 05/10] iio: light: gp2ap020a00f: Fix possible error swallow Ethan Tidmore
@ 2026-02-23 3:40 ` Ethan Tidmore
2026-02-23 3:40 ` [PATCH v5 07/10] iio: light: gp2ap020a00f: Use temporary variable for struct device Ethan Tidmore
` (4 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Ethan Tidmore @ 2026-02-23 3:40 UTC (permalink / raw)
To: Jonathan Cameron, Andy Shevchenko
Cc: David Lechner, Nuno Sá, linux-iio, linux-kernel,
Andy Shevchenko, Ethan Tidmore
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Replace custom implementation of min() to save a few lines of code.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
---
v5:
- No change.
v4:
- Integrate Andy Shevchenko's cleanups.
drivers/iio/light/gp2ap020a00f.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/light/gp2ap020a00f.c b/drivers/iio/light/gp2ap020a00f.c
index d2156e133b75..b2434d00e10a 100644
--- a/drivers/iio/light/gp2ap020a00f.c
+++ b/drivers/iio/light/gp2ap020a00f.c
@@ -38,6 +38,7 @@
#include <linux/interrupt.h>
#include <linux/irq.h>
#include <linux/irq_work.h>
+#include <linux/minmax.h>
#include <linux/module.h>
#include <linux/mod_devicetable.h>
#include <linux/mutex.h>
@@ -45,6 +46,7 @@
#include <linux/regulator/consumer.h>
#include <linux/slab.h>
#include <linux/unaligned.h>
+
#include <linux/iio/buffer.h>
#include <linux/iio/events.h>
#include <linux/iio/iio.h>
@@ -454,9 +456,7 @@ static int gp2ap020a00f_write_event_threshold(struct gp2ap020a00f_data *data,
*/
thresh_reg_val = data->thresh_val[th_val_id] / 16;
else
- thresh_reg_val = data->thresh_val[th_val_id] > 16000 ?
- 16000 :
- data->thresh_val[th_val_id];
+ thresh_reg_val = min(data->thresh_val[th_val_id], 16000U);
thresh_buf = cpu_to_le16(thresh_reg_val);
--
2.53.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v5 07/10] iio: light: gp2ap020a00f: Use temporary variable for struct device
2026-02-23 3:40 [PATCH v5 00/10] General clean up and bug fixes Ethan Tidmore
` (5 preceding siblings ...)
2026-02-23 3:40 ` [PATCH v5 06/10] iio: light: gp2ap020a00f: Replace custom implementation of min() Ethan Tidmore
@ 2026-02-23 3:40 ` Ethan Tidmore
2026-02-23 3:40 ` [PATCH v5 08/10] iio: light: gp2ap020a00f: Explicitly use string literal for driver name Ethan Tidmore
` (3 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Ethan Tidmore @ 2026-02-23 3:40 UTC (permalink / raw)
To: Jonathan Cameron, Andy Shevchenko
Cc: David Lechner, Nuno Sá, linux-iio, linux-kernel,
Andy Shevchenko, Ethan Tidmore
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Use temporary variable for struct device to make code neater.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
---
v5:
- No change.
v4:
- Integrate Andy Shevchenko's cleanups.
drivers/iio/light/gp2ap020a00f.c | 34 +++++++++++++++-----------------
1 file changed, 16 insertions(+), 18 deletions(-)
diff --git a/drivers/iio/light/gp2ap020a00f.c b/drivers/iio/light/gp2ap020a00f.c
index b2434d00e10a..97630ce9ade0 100644
--- a/drivers/iio/light/gp2ap020a00f.c
+++ b/drivers/iio/light/gp2ap020a00f.c
@@ -1187,6 +1187,7 @@ static int gp2ap020a00f_read_event_config(struct iio_dev *indio_dev,
static int gp2ap020a00f_read_channel(struct gp2ap020a00f_data *data,
struct iio_chan_spec const *chan, int *val)
{
+ struct device *dev = &data->client->dev;
enum gp2ap020a00f_cmd cmd;
int err;
@@ -1206,27 +1207,23 @@ static int gp2ap020a00f_read_channel(struct gp2ap020a00f_data *data,
err = gp2ap020a00f_exec_cmd(data, cmd);
if (err < 0) {
- dev_err(&data->client->dev,
- "gp2ap020a00f_exec_cmd failed\n");
- goto error_ret;
+ dev_err(dev, "gp2ap020a00f_exec_cmd failed\n");
+ return err;
}
err = gp2ap020a00f_read_output(data, chan->address, val);
if (err < 0)
- dev_err(&data->client->dev,
- "gp2ap020a00f_read_output failed\n");
+ dev_err(dev, "gp2ap020a00f_read_output failed\n");
err = gp2ap020a00f_set_operation_mode(data,
GP2AP020A00F_OPMODE_SHUTDOWN);
if (err < 0)
- dev_err(&data->client->dev,
- "Failed to shut down the device.\n");
+ dev_err(dev, "Failed to shut down the device.\n");
if (cmd == GP2AP020A00F_CMD_READ_RAW_CLEAR ||
cmd == GP2AP020A00F_CMD_READ_RAW_IR)
gp2ap020a00f_output_to_lux(data, val);
-error_ret:
return err;
}
@@ -1415,18 +1412,19 @@ static const struct iio_buffer_setup_ops gp2ap020a00f_buffer_setup_ops = {
static int gp2ap020a00f_probe(struct i2c_client *client)
{
const struct i2c_device_id *id = i2c_client_get_device_id(client);
+ struct device *dev = &client->dev;
struct gp2ap020a00f_data *data;
struct iio_dev *indio_dev;
struct regmap *regmap;
int err;
- indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+ indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
if (!indio_dev)
return -ENOMEM;
data = iio_priv(indio_dev);
- data->vled_reg = devm_regulator_get(&client->dev, "vled");
+ data->vled_reg = devm_regulator_get(dev, "vled");
if (IS_ERR(data->vled_reg))
return PTR_ERR(data->vled_reg);
@@ -1436,7 +1434,7 @@ static int gp2ap020a00f_probe(struct i2c_client *client)
regmap = devm_regmap_init_i2c(client, &gp2ap020a00f_regmap_config);
if (IS_ERR(regmap)) {
- dev_err(&client->dev, "Regmap initialization failed.\n");
+ dev_err(dev, "Regmap initialization failed.\n");
err = PTR_ERR(regmap);
goto error_regulator_disable;
}
@@ -1447,7 +1445,7 @@ static int gp2ap020a00f_probe(struct i2c_client *client)
ARRAY_SIZE(gp2ap020a00f_reg_init_tab));
if (err < 0) {
- dev_err(&client->dev, "Device initialization failed.\n");
+ dev_err(dev, "Device initialization failed.\n");
goto error_regulator_disable;
}
@@ -1472,11 +1470,10 @@ static int gp2ap020a00f_probe(struct i2c_client *client)
goto error_regulator_disable;
/* Allocate trigger */
- data->trig = devm_iio_trigger_alloc(&client->dev, "%s-trigger",
- indio_dev->name);
+ data->trig = devm_iio_trigger_alloc(dev, "%s-trigger", indio_dev->name);
if (data->trig == NULL) {
err = -ENOMEM;
- dev_err(&indio_dev->dev, "Failed to allocate iio trigger.\n");
+ dev_err(dev, "Failed to allocate iio trigger.\n");
goto error_uninit_buffer;
}
@@ -1488,7 +1485,7 @@ static int gp2ap020a00f_probe(struct i2c_client *client)
"gp2ap020a00f_als_event",
indio_dev);
if (err < 0) {
- dev_err(&client->dev, "Irq request failed.\n");
+ dev_err(dev, "Irq request failed.\n");
goto error_uninit_buffer;
}
@@ -1496,7 +1493,7 @@ static int gp2ap020a00f_probe(struct i2c_client *client)
err = iio_trigger_register(data->trig);
if (err < 0) {
- dev_err(&client->dev, "Failed to register iio trigger.\n");
+ dev_err(dev, "Failed to register iio trigger.\n");
goto error_free_irq;
}
@@ -1522,12 +1519,13 @@ static void gp2ap020a00f_remove(struct i2c_client *client)
{
struct iio_dev *indio_dev = i2c_get_clientdata(client);
struct gp2ap020a00f_data *data = iio_priv(indio_dev);
+ struct device *dev = &client->dev;
int err;
err = gp2ap020a00f_set_operation_mode(data,
GP2AP020A00F_OPMODE_SHUTDOWN);
if (err < 0)
- dev_err(&indio_dev->dev, "Failed to power off the device.\n");
+ dev_err(dev, "Failed to power off the device.\n");
iio_device_unregister(indio_dev);
iio_trigger_unregister(data->trig);
--
2.53.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v5 08/10] iio: light: gp2ap020a00f: Explicitly use string literal for driver name
2026-02-23 3:40 [PATCH v5 00/10] General clean up and bug fixes Ethan Tidmore
` (6 preceding siblings ...)
2026-02-23 3:40 ` [PATCH v5 07/10] iio: light: gp2ap020a00f: Use temporary variable for struct device Ethan Tidmore
@ 2026-02-23 3:40 ` Ethan Tidmore
2026-02-23 3:40 ` [PATCH v5 09/10] iio: light: gp2ap020a00f: Remove trailing comma in termination entry Ethan Tidmore
` (2 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Ethan Tidmore @ 2026-02-23 3:40 UTC (permalink / raw)
To: Jonathan Cameron, Andy Shevchenko
Cc: David Lechner, Nuno Sá, linux-iio, linux-kernel,
Andy Shevchenko, Ethan Tidmore
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
The driver name should be easily greppable and clearly spelled.
Replace a level of indirection and explicitly use string literal.
While at it, remove useless blank lines before module_*() and
MODULE_*() macros.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
---
v5:
- No change.
v4:
- Integrate Andy Shevchenko's cleanups.
drivers/iio/light/gp2ap020a00f.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/iio/light/gp2ap020a00f.c b/drivers/iio/light/gp2ap020a00f.c
index 97630ce9ade0..0d7262e1c5c9 100644
--- a/drivers/iio/light/gp2ap020a00f.c
+++ b/drivers/iio/light/gp2ap020a00f.c
@@ -55,8 +55,6 @@
#include <linux/iio/trigger_consumer.h>
#include <linux/iio/triggered_buffer.h>
-#define GP2A_I2C_NAME "gp2ap020a00f"
-
/* Registers */
#define GP2AP020A00F_OP_REG 0x00 /* Basic operations */
#define GP2AP020A00F_ALS_REG 0x01 /* ALS related settings */
@@ -1535,10 +1533,9 @@ static void gp2ap020a00f_remove(struct i2c_client *client)
}
static const struct i2c_device_id gp2ap020a00f_id[] = {
- { GP2A_I2C_NAME },
+ { "gp2ap020a00f" },
{ }
};
-
MODULE_DEVICE_TABLE(i2c, gp2ap020a00f_id);
static const struct of_device_id gp2ap020a00f_of_match[] = {
@@ -1549,14 +1546,13 @@ MODULE_DEVICE_TABLE(of, gp2ap020a00f_of_match);
static struct i2c_driver gp2ap020a00f_driver = {
.driver = {
- .name = GP2A_I2C_NAME,
+ .name = "gp2ap020a00f",
.of_match_table = gp2ap020a00f_of_match,
},
.probe = gp2ap020a00f_probe,
.remove = gp2ap020a00f_remove,
.id_table = gp2ap020a00f_id,
};
-
module_i2c_driver(gp2ap020a00f_driver);
MODULE_AUTHOR("Jacek Anaszewski <j.anaszewski@samsung.com>");
--
2.53.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v5 09/10] iio: light: gp2ap020a00f: Remove trailing comma in termination entry
2026-02-23 3:40 [PATCH v5 00/10] General clean up and bug fixes Ethan Tidmore
` (7 preceding siblings ...)
2026-02-23 3:40 ` [PATCH v5 08/10] iio: light: gp2ap020a00f: Explicitly use string literal for driver name Ethan Tidmore
@ 2026-02-23 3:40 ` Ethan Tidmore
2026-02-23 3:40 ` [PATCH v5 10/10] iio: light: gp2ap020a00f: Join some lines of code to be a single line Ethan Tidmore
2026-02-23 12:16 ` [PATCH v5 00/10] General clean up and bug fixes Andy Shevchenko
10 siblings, 0 replies; 19+ messages in thread
From: Ethan Tidmore @ 2026-02-23 3:40 UTC (permalink / raw)
To: Jonathan Cameron, Andy Shevchenko
Cc: David Lechner, Nuno Sá, linux-iio, linux-kernel,
Andy Shevchenko, Ethan Tidmore
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Termination entry by definition should be the last one, hence remove
stray comma after it.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
---
v5:
- No change.
v4:
- Integrate Andy Shevchenko's cleanups.
drivers/iio/light/gp2ap020a00f.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/light/gp2ap020a00f.c b/drivers/iio/light/gp2ap020a00f.c
index 0d7262e1c5c9..0571803155ae 100644
--- a/drivers/iio/light/gp2ap020a00f.c
+++ b/drivers/iio/light/gp2ap020a00f.c
@@ -195,7 +195,7 @@ enum gp2ap020a00f_opmode {
GP2AP020A00F_OPMODE_ALS_AND_PS,
GP2AP020A00F_OPMODE_PROX_DETECT,
GP2AP020A00F_OPMODE_SHUTDOWN,
- GP2AP020A00F_NUM_OPMODES,
+ GP2AP020A00F_NUM_OPMODES
};
enum gp2ap020a00f_cmd {
--
2.53.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v5 10/10] iio: light: gp2ap020a00f: Join some lines of code to be a single line
2026-02-23 3:40 [PATCH v5 00/10] General clean up and bug fixes Ethan Tidmore
` (8 preceding siblings ...)
2026-02-23 3:40 ` [PATCH v5 09/10] iio: light: gp2ap020a00f: Remove trailing comma in termination entry Ethan Tidmore
@ 2026-02-23 3:40 ` Ethan Tidmore
2026-02-23 12:16 ` [PATCH v5 00/10] General clean up and bug fixes Andy Shevchenko
10 siblings, 0 replies; 19+ messages in thread
From: Ethan Tidmore @ 2026-02-23 3:40 UTC (permalink / raw)
To: Jonathan Cameron, Andy Shevchenko
Cc: David Lechner, Nuno Sá, linux-iio, linux-kernel,
Andy Shevchenko, Ethan Tidmore
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
In some cases the wrapped lines are harder to follow. Join them despite
being longer than 80 characters in some cases.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
---
v5:
- No change.
v4:
- Integrate Andy Shevchenko's cleanups.
drivers/iio/light/gp2ap020a00f.c | 39 +++++++++++---------------------
1 file changed, 13 insertions(+), 26 deletions(-)
diff --git a/drivers/iio/light/gp2ap020a00f.c b/drivers/iio/light/gp2ap020a00f.c
index 0571803155ae..9d4a310f92b7 100644
--- a/drivers/iio/light/gp2ap020a00f.c
+++ b/drivers/iio/light/gp2ap020a00f.c
@@ -175,10 +175,8 @@
#define GP2AP020A00F_CHAN_TIMESTAMP 3
#define GP2AP020A00F_DATA_READY_TIMEOUT msecs_to_jiffies(1000)
-#define GP2AP020A00F_DATA_REG(chan) (GP2AP020A00F_D0_L_REG + \
- (chan) * 2)
-#define GP2AP020A00F_THRESH_REG(th_val_id) (GP2AP020A00F_TL_L_REG + \
- (th_val_id) * 2)
+#define GP2AP020A00F_DATA_REG(chan) (GP2AP020A00F_D0_L_REG + (chan) * 2)
+#define GP2AP020A00F_THRESH_REG(th_val_id) (GP2AP020A00F_TL_L_REG + (th_val_id) * 2)
#define GP2AP020A00F_THRESH_VAL_ID(reg_addr) ((reg_addr - 4) / 2)
#define GP2AP020A00F_SUBTRACT_MODE 0
@@ -390,20 +388,17 @@ static int gp2ap020a00f_set_operation_mode(struct gp2ap020a00f_data *data,
}
err = regmap_update_bits(data->regmap, GP2AP020A00F_ALS_REG,
- GP2AP020A00F_PRST_MASK, opmode_regs_settings[op]
- .als_reg);
+ GP2AP020A00F_PRST_MASK, opmode_regs_settings[op].als_reg);
if (err < 0)
return err;
err = regmap_update_bits(data->regmap, GP2AP020A00F_PS_REG,
- GP2AP020A00F_INTTYPE_MASK, opmode_regs_settings[op]
- .ps_reg);
+ GP2AP020A00F_INTTYPE_MASK, opmode_regs_settings[op].ps_reg);
if (err < 0)
return err;
err = regmap_update_bits(data->regmap, GP2AP020A00F_LED_REG,
- GP2AP020A00F_PIN_MASK, opmode_regs_settings[op]
- .led_reg);
+ GP2AP020A00F_PIN_MASK, opmode_regs_settings[op].led_reg);
if (err < 0)
return err;
}
@@ -861,8 +856,7 @@ static irqreturn_t gp2ap020a00f_thresh_event_handler(int irq, void *data)
int thresh_val_id, ret;
/* Read interrupt flags */
- ret = regmap_read(priv->regmap, GP2AP020A00F_OP_REG,
- &op_reg_val);
+ ret = regmap_read(priv->regmap, GP2AP020A00F_OP_REG, &op_reg_val);
if (ret < 0)
goto done;
@@ -874,8 +868,7 @@ static irqreturn_t gp2ap020a00f_thresh_event_handler(int irq, void *data)
/* Clear interrupt flags (if not in INTTYPE_PULSE mode) */
if (priv->cur_opmode != GP2AP020A00F_OPMODE_PROX_DETECT) {
- ret = regmap_write(priv->regmap, GP2AP020A00F_OP_REG,
- op_reg_val);
+ ret = regmap_write(priv->regmap, GP2AP020A00F_OP_REG, op_reg_val);
if (ret < 0)
goto done;
}
@@ -972,8 +965,7 @@ static irqreturn_t gp2ap020a00f_trigger_handler(int irq, void *data)
}
}
- iio_push_to_buffers_with_timestamp(indio_dev, priv->buffer,
- pf->timestamp);
+ iio_push_to_buffers_with_timestamp(indio_dev, priv->buffer, pf->timestamp);
done:
iio_trigger_notify_done(indio_dev->trig);
@@ -1023,26 +1015,22 @@ static int gp2ap020a00f_write_event_val(struct iio_dev *indio_dev,
switch (thresh_reg_l) {
case GP2AP020A00F_TH_L_REG:
- event_en = test_bit(GP2AP020A00F_FLAG_ALS_RISING_EV,
- &data->flags);
+ event_en = test_bit(GP2AP020A00F_FLAG_ALS_RISING_EV, &data->flags);
break;
case GP2AP020A00F_TL_L_REG:
- event_en = test_bit(GP2AP020A00F_FLAG_ALS_FALLING_EV,
- &data->flags);
+ event_en = test_bit(GP2AP020A00F_FLAG_ALS_FALLING_EV, &data->flags);
break;
case GP2AP020A00F_PH_L_REG:
if (val == 0)
return -EINVAL;
- event_en = test_bit(GP2AP020A00F_FLAG_PROX_RISING_EV,
- &data->flags);
+ event_en = test_bit(GP2AP020A00F_FLAG_PROX_RISING_EV, &data->flags);
break;
case GP2AP020A00F_PL_L_REG:
if (val == 0)
return -EINVAL;
- event_en = test_bit(GP2AP020A00F_FLAG_PROX_FALLING_EV,
- &data->flags);
+ event_en = test_bit(GP2AP020A00F_FLAG_PROX_FALLING_EV, &data->flags);
break;
}
@@ -1520,8 +1508,7 @@ static void gp2ap020a00f_remove(struct i2c_client *client)
struct device *dev = &client->dev;
int err;
- err = gp2ap020a00f_set_operation_mode(data,
- GP2AP020A00F_OPMODE_SHUTDOWN);
+ err = gp2ap020a00f_set_operation_mode(data, GP2AP020A00F_OPMODE_SHUTDOWN);
if (err < 0)
dev_err(dev, "Failed to power off the device.\n");
--
2.53.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v5 00/10] General clean up and bug fixes
2026-02-23 3:40 [PATCH v5 00/10] General clean up and bug fixes Ethan Tidmore
` (9 preceding siblings ...)
2026-02-23 3:40 ` [PATCH v5 10/10] iio: light: gp2ap020a00f: Join some lines of code to be a single line Ethan Tidmore
@ 2026-02-23 12:16 ` Andy Shevchenko
2026-02-23 12:17 ` Andy Shevchenko
10 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2026-02-23 12:16 UTC (permalink / raw)
To: Ethan Tidmore
Cc: Jonathan Cameron, Andy Shevchenko, David Lechner, Nuno Sá,
linux-iio, linux-kernel
On Sun, Feb 22, 2026 at 09:40:10PM -0600, Ethan Tidmore wrote:
> This series performs a general cleanup of the gp2ap020a00f IIO driver.
>
> It integrates my original work switching to the guard() cleanup handler,
> fixing a signedness warning from Smatch, an error swallow bug originally
> spotted by Andy Shevchenko which I expanded to cover both buffer setup
> functions and general cleanups written by Andy Shevchenko [1].
...
> v5:
> - Patch 1: Integrate some of Andy Shevchenko's changes.
> - Patch 4: Split patch up as requested by Andy Shevchenko.
> - Patch 5: Split this patch from the patch number 4, to keep bug fix
> separate from style fix.
Jonathan, this version is good to go in my opinion except patch 6
("iio: light: gp2ap020a00f: Fix possible error swallow").
Ethan, do *not* send a new version until we decide what to do with patch 6,
and actually the rest may be applied by Jonathan if he is okay with them.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v5 00/10] General clean up and bug fixes
2026-02-23 12:16 ` [PATCH v5 00/10] General clean up and bug fixes Andy Shevchenko
@ 2026-02-23 12:17 ` Andy Shevchenko
2026-02-23 21:18 ` Jonathan Cameron
0 siblings, 1 reply; 19+ messages in thread
From: Andy Shevchenko @ 2026-02-23 12:17 UTC (permalink / raw)
To: Ethan Tidmore
Cc: Jonathan Cameron, Andy Shevchenko, David Lechner, Nuno Sá,
linux-iio, linux-kernel
On Mon, Feb 23, 2026 at 02:16:17PM +0200, Andy Shevchenko wrote:
> On Sun, Feb 22, 2026 at 09:40:10PM -0600, Ethan Tidmore wrote:
> > This series performs a general cleanup of the gp2ap020a00f IIO driver.
> >
> > It integrates my original work switching to the guard() cleanup handler,
> > fixing a signedness warning from Smatch, an error swallow bug originally
> > spotted by Andy Shevchenko which I expanded to cover both buffer setup
> > functions and general cleanups written by Andy Shevchenko [1].
...
> > v5:
> > - Patch 1: Integrate some of Andy Shevchenko's changes.
> > - Patch 4: Split patch up as requested by Andy Shevchenko.
> > - Patch 5: Split this patch from the patch number 4, to keep bug fix
> > separate from style fix.
>
> Jonathan, this version is good to go in my opinion except patch 6
> ("iio: light: gp2ap020a00f: Fix possible error swallow").
>
> Ethan, do *not* send a new version until we decide what to do with patch 6,
> and actually the rest may be applied by Jonathan if he is okay with them.
It's patch 5, sorry for the confusion.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v5 00/10] General clean up and bug fixes
2026-02-23 12:17 ` Andy Shevchenko
@ 2026-02-23 21:18 ` Jonathan Cameron
2026-02-24 9:45 ` Andy Shevchenko
0 siblings, 1 reply; 19+ messages in thread
From: Jonathan Cameron @ 2026-02-23 21:18 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Ethan Tidmore, Andy Shevchenko, David Lechner, Nuno Sá,
linux-iio, linux-kernel
On Mon, 23 Feb 2026 14:17:14 +0200
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> On Mon, Feb 23, 2026 at 02:16:17PM +0200, Andy Shevchenko wrote:
> > On Sun, Feb 22, 2026 at 09:40:10PM -0600, Ethan Tidmore wrote:
> > > This series performs a general cleanup of the gp2ap020a00f IIO driver.
> > >
> > > It integrates my original work switching to the guard() cleanup handler,
> > > fixing a signedness warning from Smatch, an error swallow bug originally
> > > spotted by Andy Shevchenko which I expanded to cover both buffer setup
> > > functions and general cleanups written by Andy Shevchenko [1].
>
> ...
>
> > > v5:
> > > - Patch 1: Integrate some of Andy Shevchenko's changes.
> > > - Patch 4: Split patch up as requested by Andy Shevchenko.
> > > - Patch 5: Split this patch from the patch number 4, to keep bug fix
> > > separate from style fix.
> >
> > Jonathan, this version is good to go in my opinion except patch 6
> > ("iio: light: gp2ap020a00f: Fix possible error swallow").
> >
> > Ethan, do *not* send a new version until we decide what to do with patch 6,
> > and actually the rest may be applied by Jonathan if he is okay with them.
>
> It's patch 5, sorry for the confusion.
I tweaked it and applied the series to the testing branch of iio.git
thanks!
Jonathan
>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v5 00/10] General clean up and bug fixes
2026-02-23 21:18 ` Jonathan Cameron
@ 2026-02-24 9:45 ` Andy Shevchenko
0 siblings, 0 replies; 19+ messages in thread
From: Andy Shevchenko @ 2026-02-24 9:45 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Ethan Tidmore, Andy Shevchenko, David Lechner, Nuno Sá,
linux-iio, linux-kernel
On Mon, Feb 23, 2026 at 09:18:10PM +0000, Jonathan Cameron wrote:
> On Mon, 23 Feb 2026 14:17:14 +0200
> Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> > On Mon, Feb 23, 2026 at 02:16:17PM +0200, Andy Shevchenko wrote:
> > > On Sun, Feb 22, 2026 at 09:40:10PM -0600, Ethan Tidmore wrote:
...
> > > > v5:
> > > > - Patch 1: Integrate some of Andy Shevchenko's changes.
> > > > - Patch 4: Split patch up as requested by Andy Shevchenko.
> > > > - Patch 5: Split this patch from the patch number 4, to keep bug fix
> > > > separate from style fix.
> > >
> > > Jonathan, this version is good to go in my opinion except patch 6
> > > ("iio: light: gp2ap020a00f: Fix possible error swallow").
> > >
> > > Ethan, do *not* send a new version until we decide what to do with patch 6,
> > > and actually the rest may be applied by Jonathan if he is okay with them.
> >
> > It's patch 5, sorry for the confusion.
>
> I tweaked it and applied the series to the testing branch of iio.git
Ah, thank you!
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 19+ messages in thread