mbox series

[v5,0/7] iio: light: vcnl4000: Add features for vncl4040/4200

Message ID 20230530142405.1679146-1-astrid.rost@axis.com (mailing list archive)
Headers show
Series iio: light: vcnl4000: Add features for vncl4040/4200 | expand

Message

Astrid Rost May 30, 2023, 2:23 p.m. UTC
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.

changes v2:
- [PATCH v2 3/7] Fixed calculation of al_scale.
  Fix the value of vcnl4040 according to the data-sheet.
  Use div_u64 for the division.
scription for the branch

changes v3:
- [PATCH v3 1-3/7] Add differences between the chips as variables in
  chip-spec.
- [PATCH v3 4/7] Changed commit message.
- [PATCH v3 5/7] Use period instead of debounce time. This causes some
  calculations as the period is a time and the chip allows to set a certain
  amount of measurements above/below the threshold, before throwing an
  interrupt.
- [PATCH v3 6/7] Changed commit message.

changes v4:
- [PATCH v3 1-3/7] Fix setting correct als_it for vcnl4040.
- [PATCH v3 5/7] Use MICRO macro.
  Fix values greater than 1 s for the proximity period.

changes v5:
[PATCH v5 2/7]:
- Calculate ps_it from ps_it_times by usinh NSEC_PER_USEC.
[PATCH v5 3/7]:
- Calculate als_it from ps_it_times by using NSEC_PER_USEC.
- Store scale step factor in chip_spec.
- Fixes sampling_rate to ns + 20 %.
[PATCH v5 3/7 - 7/7]
- Changed formatting.
- Changed some variable names.

Astrid Rost (7):
  [PATCH v4 1/7] iio: light: vcnl4000: Add proximity irq for vcnl4200
  [PATCH v4 2/7] iio: light: vcnl4000: Add proximity ps_it for vcnl4200
  [PATCH v4 3/7] iio: light: vcnl4000: Add als_it for vcnl4040/4200
  [PATCH v4 4/7] iio: light: vcnl4000: add illuminance irq vcnl4040/4200
  [PATCH v4 5/7] iio: light: vcnl4000: Add period for vcnl4040/4200
  [PATCH v4 6/7] iio: light: vcnl4000: Add oversampling_ratio for 4040/4200
  [PATCH v4 7/7] iio: light: vcnl4000: Add calibration bias for 4040/4200

 drivers/iio/light/vcnl4000.c | 727 +++++++++++++++++++++++++++++++----
 1 file changed, 657 insertions(+), 70 deletions(-)

Comments

Andy Shevchenko June 3, 2023, 1:54 p.m. UTC | #1
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.
Jonathan Cameron June 4, 2023, 11:20 a.m. UTC | #2
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