From: Jonathan Cameron <jic23@kernel.org>
To: andy.shevchenko@gmail.com
Cc: "Astrid Rost" <astrid.rost@axis.com>,
"Lars-Peter Clausen" <lars@metafoo.de>,
linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org,
kernel@axis.com,
"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
"Mathieu Othacehe" <m.othacehe@gmail.com>
Subject: Re: [PATCH v5 0/7] iio: light: vcnl4000: Add features for vncl4040/4200
Date: Sun, 4 Jun 2023 12:20:15 +0100 [thread overview]
Message-ID: <20230604122015.38e7b8a5@jic23-huawei> (raw)
In-Reply-To: <ZHtGAn8ZF_fhgNQn@surfacebook>
On Sat, 3 Jun 2023 16:54:10 +0300
andy.shevchenko@gmail.com wrote:
> Tue, May 30, 2023 at 04:23:58PM +0200, Astrid Rost kirjoitti:
> > Add a more complete support for vncl4040 and vcnl4200, which allows to
> > change the distance of proximity detection and interrupt support for the
> > illuminance sensor.
> >
> > Proximity functionality:
> > - Interrupt support (new on vcnl4200).
> >
> > Proximity reduce the amount of interrupts:
> > - Adaptable integration time (new on vcnl4200) - the sampling rate
> > changes according to this value.
> > - Period - interrupt is asserted if the value is above or
> > below a certain threshold.
> >
> > Proximity change the activity distance:
> > - Oversampling ratio - Amount of LED pulses per measured raw value.
> > - Calibration bias - LED current calibration of the sensor.
> >
> > Illuminance functionality:
> > - Interrupt support.
> >
> > Illuminance reduce the amount of interrupts:
> > - Adaptable integration time - the sampling rate and scale changes
> > according to this value.
> > - Period – interrupt is asserted if the value is above or
> > below a certain threshold.
>
> It's a good work, thank you for doing it!
>
> But it has a lot of small style and inconsistent issues. They are not major
> per se, but since there is more than 3, it makes sense to address. Also check
> if you can split your patches to two or three where it makes sense.
>
FWIW nothing to add from me. Agree with Andy that we are down to style
things that would be good to tidy up before applying these.
Adding levels to switch nests always makes for ugly diffs so I fully agree
with Andy that, though trivial, it is probably better to do those in two steps
for ease of review. If there is just one in a patch then meh, we can probably
cope with the extra thinking required to review them , but where it happens
several times it's worth making reviewer's lives that little bit easier!
Jonathan
prev parent reply other threads:[~2023-06-04 11:20 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-30 14:23 [PATCH v5 0/7] iio: light: vcnl4000: Add features for vncl4040/4200 Astrid Rost
2023-05-30 14:23 ` [PATCH v5 1/7] iio: light: vcnl4000: Add proximity irq for vcnl4200 Astrid Rost
2023-06-03 12:08 ` andy.shevchenko
2023-05-30 14:24 ` [PATCH v5 2/7] iio: light: vcnl4000: Add proximity ps_it " Astrid Rost
2023-06-03 12:09 ` andy.shevchenko
2023-05-30 14:24 ` [PATCH v5 3/7] iio: light: vcnl4000: Add als_it for vcnl4040/4200 Astrid Rost
2023-06-03 12:15 ` andy.shevchenko
2023-05-30 14:24 ` [PATCH v5 4/7] iio: light: vcnl4000: add illuminance irq vcnl4040/4200 Astrid Rost
2023-06-03 13:46 ` andy.shevchenko
2023-06-04 11:16 ` Jonathan Cameron
2023-05-30 14:24 ` [PATCH v5 5/7] iio: light: vcnl4000: Add period for vcnl4040/4200 Astrid Rost
2023-06-03 13:51 ` andy.shevchenko
2023-05-30 14:24 ` [PATCH v5 6/7] iio: light: vcnl4000: Add oversampling_ratio for 4040/4200 Astrid Rost
2023-06-03 13:51 ` andy.shevchenko
2023-05-30 14:24 ` [PATCH v5 7/7] iio: light: vcnl4000: Add calibration bias " Astrid Rost
2023-06-03 13:52 ` andy.shevchenko
2023-06-03 13:54 ` [PATCH v5 0/7] iio: light: vcnl4000: Add features for vncl4040/4200 andy.shevchenko
2023-06-04 11:20 ` 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=20230604122015.38e7b8a5@jic23-huawei \
--to=jic23@kernel.org \
--cc=andy.shevchenko@gmail.com \
--cc=astrid.rost@axis.com \
--cc=kernel@axis.com \
--cc=lars@metafoo.de \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=m.othacehe@gmail.com \
--cc=u.kleine-koenig@pengutronix.de \
/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