From: Jonathan Cameron <jic23@kernel.org>
To: "Javier Carrasco" <javier.carrasco.cruz@gmail.com>
Cc: "Shardul Deshpande" <iamsharduld@gmail.com>,
"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 v2] iio: light: veml6075: fix UV index reported at half value
Date: Wed, 1 Jul 2026 20:33:52 +0100 [thread overview]
Message-ID: <20260701203352.161f3fa3@jic23-huawei> (raw)
In-Reply-To: <DJJPU0OYEXMQ.EUU9J2PW7BKA@gmail.com>
On Sat, 27 Jun 2026 11:14:39 +0200
"Javier Carrasco" <javier.carrasco.cruz@gmail.com> wrote:
> On Sat Jun 27, 2026 at 10:53 AM CEST, Shardul Deshpande wrote:
> > veml6075_get_uvi_micro() normalises the UV index for the configured
> > integration time. The raw UVA/UVB counts scale linearly with the
> > integration time, so the responsivity-weighted sum must be divided by the
> > integration-time scale factor relative to the 50 ms base case (which is
> > returned undivided).
> >
> > Per the VEML6075 datasheet the UV_IT field selects the integration time
> > as 50, 100, 200, 400 and 800 ms for field values 0..4, i.e.
> > (50 << int_index) ms: each step doubles the integration time and hence
> > the accumulated counts. The correct scale factor relative to the 50 ms
> > base is therefore 2^int_index == (1 << int_index). (See also the Vishay
> > application note "Designing the VEML6075 Into an Application" for the
> > UV-index responsivity calculation that these constants implement.)
> >
> > The code instead divided by (2 << int_index) == 2^(int_index + 1), i.e.
> > twice the correct value, so the reported UV index was half of the true
> > value for every integration time except 50 ms. As the driver powers up
> > with VEML6075_IT_100_MS, the UV index was reported at half value out of
> > the box.
> >
> > Divide by (1 << int_index) instead. int_index is already bounded to 0..4
> > by veml6075_read_int_time_index(), and (1 << 0) == 1 reproduces the
> > undivided 50 ms case, so the per-integration-time switch collapses to a
> > single expression.
> >
> > Fixes: 3b82f43238ae ("iio: light: add VEML6075 UVA and UVB light sensor driver")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Shardul Deshpande <iamsharduld@gmail.com>
> > ---
> > Changes in v2:
> > - Collapse the per-integration-time switch into a single divide now that
> > all cases share the same expression -- int_index is bounded to 0..4 by
> > veml6075_read_int_time_index() and (1 << 0) reproduces the 50 ms case.
> > (Javier Carrasco)
> > - Add VEML6075 datasheet (UV_IT integration-time table) and application-
> > note references for the integration-time scaling in the changelog.
> >
> > Link to v1:
> > https://lore.kernel.org/linux-iio/20260626110400.68885-1-iamsharduld@gmail.com/
> >
> > drivers/iio/light/veml6075.c | 18 +++++++-----------
> > 1 file changed, 7 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/iio/light/veml6075.c b/drivers/iio/light/veml6075.c
> > index 59187244a..d0ec06eeb 100644
> > --- a/drivers/iio/light/veml6075.c
> > +++ b/drivers/iio/light/veml6075.c
> > @@ -237,17 +237,13 @@ static int veml6075_get_uvi_micro(struct veml6075_data *data, int uva_comp,
> > if (int_index < 0)
> > return int_index;
> >
> > - switch (int_index) {
> > - case VEML6075_IT_50_MS:
> > - return uvia_micro + uvib_micro;
> > - case VEML6075_IT_100_MS:
> > - case VEML6075_IT_200_MS:
> > - case VEML6075_IT_400_MS:
> > - case VEML6075_IT_800_MS:
> > - return (uvia_micro + uvib_micro) / (2 << int_index);
> > - default:
> > - return -EINVAL;
> > - }
> > + /*
> > + * The raw counts scale linearly with the integration time, which
> > + * doubles at each step (50, 100, 200, 400, 800 ms == 50 << int_index
> > + * ms; int_index is bounded to 0..4 above). Normalise to the 50 ms
> > + * base by dividing by 2^int_index; (1 << 0) == 1 leaves 50 ms as-is.
> > + */
> > + return (uvia_micro + uvib_micro) / (1 << int_index);
> > }
> >
> > static int veml6075_read_uvi(struct veml6075_data *data, int *val, int *val2)
>
> Hello Shardul, thank your for your patch.
>
> Please give potential reviewers more time to take a look at your
> patches. It's been less than 24 hours between v1 and v2, and that
> reduces the amount of people who could review and potentially improve
> your patches.
>
> In my opinion, the comment you added is way too verbose, and it includes
> information that is well known within the driver like the possible
> values for the integration time. Once it is fixed, it is clear that it
> is normalised to 50 ms because it is index 0. To be honest, I would even
> drop the comment completely.
>
The actual change in behaviour looks fine to me, so just the amendments Javier
suggests for v3.
Thanks,
Jonathan
> Best regards,
> Javier
prev parent reply other threads:[~2026-07-01 19:33 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-26 11:04 [PATCH] iio: light: veml6075: fix UV index reported at half value Shardul Deshpande
2026-06-26 12:21 ` Andy Shevchenko
2026-06-26 13:39 ` Javier Carrasco
2026-06-27 8:53 ` [PATCH v2] " Shardul Deshpande
2026-06-27 9:14 ` Javier Carrasco
2026-07-01 19:33 ` Jonathan Cameron [this message]
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=20260701203352.161f3fa3@jic23-huawei \
--to=jic23@kernel.org \
--cc=andy@kernel.org \
--cc=dlechner@baylibre.com \
--cc=iamsharduld@gmail.com \
--cc=javier.carrasco.cruz@gmail.com \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox