* [PATCH v2] iio: proximity: cleanup fixes for vl53l1x-i2c
@ 2026-06-08 12:40 Siratul Islam
2026-06-08 15:30 ` Nuno Sá
0 siblings, 1 reply; 3+ messages in thread
From: Siratul Islam @ 2026-06-08 12:40 UTC (permalink / raw)
To: jic23
Cc: dlechner, nuno.sa, andy, linux-iio, linux-kernel, Siratul Islam,
Andy Shevchenko
Extract data-ready polling into a helper, fix regmap_read_poll_timeout()
argument alignment. No functional changes.
Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Siratul Islam <email@sirat.me>
---
drivers/iio/proximity/vl53l1x-i2c.c | 36 ++++++++++++++---------------
1 file changed, 17 insertions(+), 19 deletions(-)
diff --git a/drivers/iio/proximity/vl53l1x-i2c.c b/drivers/iio/proximity/vl53l1x-i2c.c
index 4d9cb3983dba..ed83999cf735 100644
--- a/drivers/iio/proximity/vl53l1x-i2c.c
+++ b/drivers/iio/proximity/vl53l1x-i2c.c
@@ -43,6 +43,7 @@
#define VL53L1X_REG_SOFT_RESET 0x0000
#define VL53L1X_REG_VHV_CONFIG__TIMEOUT_MACROP_LOOP_BOUND 0x0008
#define VL53L1X_REG_VHV_CONFIG__INIT 0x000B
+#define VL53L1X_REG_DEFAULT_CONFIG 0x002D
#define VL53L1X_REG_GPIO_HV_MUX__CTRL 0x0030
#define VL53L1X_REG_GPIO__TIO_HV_STATUS 0x0031
#define VL53L1X_REG_SYSTEM__INTERRUPT_CONFIG_GPIO 0x0046
@@ -64,7 +65,6 @@
#define VL53L1X_REG_RESULT__OSC_CALIBRATE_VAL 0x00DE
#define VL53L1X_REG_FIRMWARE__SYSTEM_STATUS 0x00E5
#define VL53L1X_REG_IDENTIFICATION__MODEL_ID 0x010F
-#define VL53L1X_REG_DEFAULT_CONFIG 0x002D
#define VL53L1X_MODEL_ID_VAL 0xEACC
@@ -191,6 +191,17 @@ static int vl53l1x_stop_ranging(struct vl53l1x_data *data)
VL53L1X_MODE_START_STOP);
}
+static int vl53l1x_wait_data_ready(struct vl53l1x_data *data)
+{
+ unsigned int val;
+
+ /* 1ms poll, 1s timeout covers max timing budgets (per ST Ultra Lite Driver) */
+ return regmap_read_poll_timeout(data->regmap,
+ VL53L1X_REG_GPIO__TIO_HV_STATUS,
+ val, (val & 1) != data->gpio_polarity,
+ 1 * USEC_PER_MSEC, 1 * USEC_PER_SEC);
+}
+
/*
* Default configuration blob from ST's VL53L1X Ultra Lite Driver
* (STSW-IMG009).
@@ -230,10 +241,9 @@ static int vl53l1x_chip_init(struct vl53l1x_data *data)
}
ret = regmap_read_poll_timeout(data->regmap,
- VL53L1X_REG_FIRMWARE__SYSTEM_STATUS, val,
- val & BIT(0),
- 1 * USEC_PER_MSEC,
- 100 * USEC_PER_MSEC);
+ VL53L1X_REG_FIRMWARE__SYSTEM_STATUS,
+ val, val & BIT(0),
+ 1 * USEC_PER_MSEC, 100 * USEC_PER_MSEC);
if (ret)
return dev_err_probe(dev, ret, "firmware boot timeout\n");
@@ -261,12 +271,7 @@ static int vl53l1x_chip_init(struct vl53l1x_data *data)
if (ret)
return ret;
- /* 1ms poll, 1s timeout covers max timing budgets (per ST Ultra Lite Driver) */
- ret = regmap_read_poll_timeout(data->regmap,
- VL53L1X_REG_GPIO__TIO_HV_STATUS, val,
- (val & 1) != data->gpio_polarity,
- 1 * USEC_PER_MSEC,
- 1000 * USEC_PER_MSEC);
+ ret = vl53l1x_wait_data_ready(data);
if (ret)
return ret;
@@ -461,14 +466,7 @@ static int vl53l1x_read_proximity(struct vl53l1x_data *data, int *val)
if (!wait_for_completion_timeout(&data->completion, HZ))
return -ETIMEDOUT;
} else {
- unsigned int rdy;
-
- /* 1ms poll, 1s timeout covers max timing budgets (per ST Ultra Lite Driver) */
- ret = regmap_read_poll_timeout(data->regmap,
- VL53L1X_REG_GPIO__TIO_HV_STATUS, rdy,
- (rdy & 1) != data->gpio_polarity,
- 1 * USEC_PER_MSEC,
- 1000 * USEC_PER_MSEC);
+ ret = vl53l1x_wait_data_ready(data);
if (ret)
return ret;
}
--
2.54.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] iio: proximity: cleanup fixes for vl53l1x-i2c
2026-06-08 12:40 [PATCH v2] iio: proximity: cleanup fixes for vl53l1x-i2c Siratul Islam
@ 2026-06-08 15:30 ` Nuno Sá
2026-06-08 17:36 ` Jonathan Cameron
0 siblings, 1 reply; 3+ messages in thread
From: Nuno Sá @ 2026-06-08 15:30 UTC (permalink / raw)
To: Siratul Islam, jic23
Cc: dlechner, nuno.sa, andy, linux-iio, linux-kernel, Andy Shevchenko
On Mon, 2026-06-08 at 18:40 +0600, Siratul Islam wrote:
> Extract data-ready polling into a helper, fix regmap_read_poll_timeout()
> argument alignment. No functional changes.
>
> Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> Signed-off-by: Siratul Islam <email@sirat.me>
> ---
Normally don't really like these one liner helper APIs but for this cases where it's easy to mess
up, it makes sense as if __we mess up__, we only need to fix one place.
Just one comment from me. With that addressed, looks good:
Reviewed-by: Nuno Sá <nuno.sa@analog.com>
> drivers/iio/proximity/vl53l1x-i2c.c | 36 ++++++++++++++---------------
> 1 file changed, 17 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/iio/proximity/vl53l1x-i2c.c b/drivers/iio/proximity/vl53l1x-i2c.c
> index 4d9cb3983dba..ed83999cf735 100644
> --- a/drivers/iio/proximity/vl53l1x-i2c.c
> +++ b/drivers/iio/proximity/vl53l1x-i2c.c
> @@ -43,6 +43,7 @@
> #define VL53L1X_REG_SOFT_RESET 0x0000
> #define VL53L1X_REG_VHV_CONFIG__TIMEOUT_MACROP_LOOP_BOUND 0x0008
> #define VL53L1X_REG_VHV_CONFIG__INIT 0x000B
> +#define VL53L1X_REG_DEFAULT_CONFIG 0x002D
> #define VL53L1X_REG_GPIO_HV_MUX__CTRL 0x0030
> #define VL53L1X_REG_GPIO__TIO_HV_STATUS 0x0031
> #define VL53L1X_REG_SYSTEM__INTERRUPT_CONFIG_GPIO 0x0046
> @@ -64,7 +65,6 @@
> #define VL53L1X_REG_RESULT__OSC_CALIBRATE_VAL 0x00DE
> #define VL53L1X_REG_FIRMWARE__SYSTEM_STATUS 0x00E5
> #define VL53L1X_REG_IDENTIFICATION__MODEL_ID 0x010F
> -#define VL53L1X_REG_DEFAULT_CONFIG 0x002D
>
> #define VL53L1X_MODEL_ID_VAL 0xEACC
>
> @@ -191,6 +191,17 @@ static int vl53l1x_stop_ranging(struct vl53l1x_data *data)
> VL53L1X_MODE_START_STOP);
> }
>
> +static int vl53l1x_wait_data_ready(struct vl53l1x_data *data)
> +{
> + unsigned int val;
> +
> + /* 1ms poll, 1s timeout covers max timing budgets (per ST Ultra Lite Driver) */
> + return regmap_read_poll_timeout(data->regmap,
> + VL53L1X_REG_GPIO__TIO_HV_STATUS,
> + val, (val & 1) != data->gpio_polarity,
> + 1 * USEC_PER_MSEC, 1 * USEC_PER_SEC);
> +}
> +
> /*
> * Default configuration blob from ST's VL53L1X Ultra Lite Driver
> * (STSW-IMG009).
> @@ -230,10 +241,9 @@ static int vl53l1x_chip_init(struct vl53l1x_data *data)
> }
>
> ret = regmap_read_poll_timeout(data->regmap,
> - VL53L1X_REG_FIRMWARE__SYSTEM_STATUS, val,
> - val & BIT(0),
> - 1 * USEC_PER_MSEC,
> - 100 * USEC_PER_MSEC);
> + VL53L1X_REG_FIRMWARE__SYSTEM_STATUS,
> + val, val & BIT(0),
> + 1 * USEC_PER_MSEC, 100 * USEC_PER_MSEC);
This looks unrelated to what you have on the commit message. Either drop it or mention it in the
message (but I'm really not sure on the added value of the above change - I guess more meaningful
line break wrt the API arguments).
- Nuno Sá
> if (ret)
> return dev_err_probe(dev, ret, "firmware boot timeout\n");
>
> @@ -261,12 +271,7 @@ static int vl53l1x_chip_init(struct vl53l1x_data *data)
> if (ret)
> return ret;
>
> - /* 1ms poll, 1s timeout covers max timing budgets (per ST Ultra Lite Driver) */
> - ret = regmap_read_poll_timeout(data->regmap,
> - VL53L1X_REG_GPIO__TIO_HV_STATUS, val,
> - (val & 1) != data->gpio_polarity,
> - 1 * USEC_PER_MSEC,
> - 1000 * USEC_PER_MSEC);
> + ret = vl53l1x_wait_data_ready(data);
> if (ret)
> return ret;
>
> @@ -461,14 +466,7 @@ static int vl53l1x_read_proximity(struct vl53l1x_data *data, int *val)
> if (!wait_for_completion_timeout(&data->completion, HZ))
> return -ETIMEDOUT;
> } else {
> - unsigned int rdy;
> -
> - /* 1ms poll, 1s timeout covers max timing budgets (per ST Ultra Lite Driver) */
> - ret = regmap_read_poll_timeout(data->regmap,
> - VL53L1X_REG_GPIO__TIO_HV_STATUS, rdy,
> - (rdy & 1) != data->gpio_polarity,
> - 1 * USEC_PER_MSEC,
> - 1000 * USEC_PER_MSEC);
> + ret = vl53l1x_wait_data_ready(data);
> if (ret)
> return ret;
> }
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] iio: proximity: cleanup fixes for vl53l1x-i2c
2026-06-08 15:30 ` Nuno Sá
@ 2026-06-08 17:36 ` Jonathan Cameron
0 siblings, 0 replies; 3+ messages in thread
From: Jonathan Cameron @ 2026-06-08 17:36 UTC (permalink / raw)
To: Nuno Sá
Cc: Siratul Islam, dlechner, nuno.sa, andy, linux-iio, linux-kernel,
Andy Shevchenko
On Mon, 08 Jun 2026 16:30:07 +0100
Nuno Sá <noname.nuno@gmail.com> wrote:
> On Mon, 2026-06-08 at 18:40 +0600, Siratul Islam wrote:
> > Extract data-ready polling into a helper, fix regmap_read_poll_timeout()
> > argument alignment. No functional changes.
> >
> > Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> > Signed-off-by: Siratul Islam <email@sirat.me>
> > ---
>
> Normally don't really like these one liner helper APIs but for this cases where it's easy to mess
> up, it makes sense as if __we mess up__, we only need to fix one place.
>
> Just one comment from me. With that addressed, looks good:
>
> Reviewed-by: Nuno Sá <nuno.sa@analog.com>
>
The changes are fine, but there are really 3 different ones in here - so
at least 2 patches (see below for why not 3).
Jonathan
> > drivers/iio/proximity/vl53l1x-i2c.c | 36 ++++++++++++++---------------
> > 1 file changed, 17 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/iio/proximity/vl53l1x-i2c.c b/drivers/iio/proximity/vl53l1x-i2c.c
> > index 4d9cb3983dba..ed83999cf735 100644
> > --- a/drivers/iio/proximity/vl53l1x-i2c.c
> > +++ b/drivers/iio/proximity/vl53l1x-i2c.c
> > @@ -43,6 +43,7 @@
> > #define VL53L1X_REG_SOFT_RESET 0x0000
> > #define VL53L1X_REG_VHV_CONFIG__TIMEOUT_MACROP_LOOP_BOUND 0x0008
> > #define VL53L1X_REG_VHV_CONFIG__INIT 0x000B
> > +#define VL53L1X_REG_DEFAULT_CONFIG 0x002D
> > #define VL53L1X_REG_GPIO_HV_MUX__CTRL 0x0030
> > #define VL53L1X_REG_GPIO__TIO_HV_STATUS 0x0031
> > #define VL53L1X_REG_SYSTEM__INTERRUPT_CONFIG_GPIO 0x0046
> > @@ -64,7 +65,6 @@
> > #define VL53L1X_REG_RESULT__OSC_CALIBRATE_VAL 0x00DE
> > #define VL53L1X_REG_FIRMWARE__SYSTEM_STATUS 0x00E5
> > #define VL53L1X_REG_IDENTIFICATION__MODEL_ID 0x010F
> > -#define VL53L1X_REG_DEFAULT_CONFIG 0x002D
There are too many things in one patch. This move is unrelated to the rest
so needs to be on its own.
> >
> > #define VL53L1X_MODEL_ID_VAL 0xEACC
> >
> > @@ -191,6 +191,17 @@ static int vl53l1x_stop_ranging(struct vl53l1x_data *data)
> > VL53L1X_MODE_START_STOP);
> > }
> >
> > +static int vl53l1x_wait_data_ready(struct vl53l1x_data *data)
> > +{
> > + unsigned int val;
> > +
> > + /* 1ms poll, 1s timeout covers max timing budgets (per ST Ultra Lite Driver) */
> > + return regmap_read_poll_timeout(data->regmap,
> > + VL53L1X_REG_GPIO__TIO_HV_STATUS,
> > + val, (val & 1) != data->gpio_polarity,
> > + 1 * USEC_PER_MSEC, 1 * USEC_PER_SEC);
> > +}
> > +
> > /*
> > * Default configuration blob from ST's VL53L1X Ultra Lite Driver
> > * (STSW-IMG009).
> > @@ -230,10 +241,9 @@ static int vl53l1x_chip_init(struct vl53l1x_data *data)
> > }
> >
> > ret = regmap_read_poll_timeout(data->regmap,
> > - VL53L1X_REG_FIRMWARE__SYSTEM_STATUS, val,
> > - val & BIT(0),
> > - 1 * USEC_PER_MSEC,
> > - 100 * USEC_PER_MSEC);
> > + VL53L1X_REG_FIRMWARE__SYSTEM_STATUS,
> > + val, val & BIT(0),
This one is using BIT(0) and above you are using the value of that (so 1).
I slightly prefer BIT(0) as I assume it is a single bit field. Even better
would be to use proper field definitions. So something like
#define VL53L1X_FIRMWARE__SYSTEM_STATUS_BOOTED BIT(0)
> > + 1 * USEC_PER_MSEC, 100 * USEC_PER_MSEC);
>
> This looks unrelated to what you have on the commit message. Either drop it or mention it in the
> message (but I'm really not sure on the added value of the above change - I guess more meaningful
> line break wrt the API arguments).
Separate patch. I don't mind all the reformatting together (so the register definition
movement). However if you follow the suggestion to use a define to name the BIT(0)
field that can be combined with reflowing this, but not the move above.
>
> - Nuno Sá
>
> > if (ret)
> > return dev_err_probe(dev, ret, "firmware boot timeout\n");
> >
> > @@ -261,12 +271,7 @@ static int vl53l1x_chip_init(struct vl53l1x_data *data)
> > if (ret)
> > return ret;
> >
> > - /* 1ms poll, 1s timeout covers max timing budgets (per ST Ultra Lite Driver) */
> > - ret = regmap_read_poll_timeout(data->regmap,
> > - VL53L1X_REG_GPIO__TIO_HV_STATUS, val,
> > - (val & 1) != data->gpio_polarity,
> > - 1 * USEC_PER_MSEC,
> > - 1000 * USEC_PER_MSEC);
> > + ret = vl53l1x_wait_data_ready(data);
> > if (ret)
> > return ret;
> >
> > @@ -461,14 +466,7 @@ static int vl53l1x_read_proximity(struct vl53l1x_data *data, int *val)
> > if (!wait_for_completion_timeout(&data->completion, HZ))
> > return -ETIMEDOUT;
> > } else {
> > - unsigned int rdy;
> > -
> > - /* 1ms poll, 1s timeout covers max timing budgets (per ST Ultra Lite Driver) */
> > - ret = regmap_read_poll_timeout(data->regmap,
> > - VL53L1X_REG_GPIO__TIO_HV_STATUS, rdy,
> > - (rdy & 1) != data->gpio_polarity,
> > - 1 * USEC_PER_MSEC,
> > - 1000 * USEC_PER_MSEC);
> > + ret = vl53l1x_wait_data_ready(data);
> > if (ret)
> > return ret;
> > }
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-06-08 17:36 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-08 12:40 [PATCH v2] iio: proximity: cleanup fixes for vl53l1x-i2c Siratul Islam
2026-06-08 15:30 ` Nuno Sá
2026-06-08 17:36 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox