On Wed, 22 Mar 2023 11:07:56 +0200
Matti Vaittinen <mazziesaccount@xxxxxxxxx> wrote:
> ROHM BU27034 is an ambient light sensor with 3 channels and 3 photo diodes
> capable of detecting a very wide range of illuminance. Typical application
> is adjusting LCD and backlight power of TVs and mobile phones.
>
> Add initial support for the ROHM BU27034 ambient light sensor.
>
> NOTE:
> - Driver exposes 4 channels. One IIO_LIGHT channel providing the
> calculated lux values based on measured data from diodes #0 and
> #1. In addition, 3 IIO_INTENSITY channels are emitting the raw
> register data from all diodes for more intense user-space
> computations.
> - Sensor has GAIN values that can be adjusted from 1x to 4096x.
> - Sensor has adjustible measurement times of 5, 55, 100, 200 and
> 400 mS. Driver does not support 5 mS which has special
> limitations.
> - Driver exposes standard 'scale' adjustment which is
> implemented by:
> 1) Trying to adjust only the GAIN
> 2) If GAIN adjustment alone can't provide requested
> scale, adjusting both the time and the gain is
> attempted.
> - Driver exposes writable INT_TIME property that can be used
> for adjusting the measurement time. Time adjustment will also
> cause the driver to try to adjust the GAIN so that the
> overall scale is kept as close to the original as possible.
>
> Signed-off-by: Matti Vaittinen <mazziesaccount@xxxxxxxxx>
>
Hi Matti,
A few minor comments inline. I'll take a closer look at the rest of the
series when the discussions around the tests and devices to be used
for them settle down.
Thanks,
Jonathan
> +
> +static u64 bu27034_fixp_calc_t1(unsigned int coeff, unsigned int ch0,
> + unsigned int ch1, unsigned int gain0,
> + unsigned int gain1)
> +{
> + unsigned int helper, tmp;
> + u64 helper64;
> +
> + /*
> + * Here we could overflow even the 64bit value. Hence we
> + * multiply with gain0 only after the divisions - even though
> + * it may result loss of accuracy
> + */
> + helper64 = (u64)coeff * (u64)ch1 * (u64)ch1;
> + helper = coeff * ch1 * ch1;
> + tmp = helper * gain0;
> +
> + if (helper == helper64 && (tmp / gain0 == helper))
Similar to below. Don't bother with the non 64 bit version.
> + return tmp / (gain1 * gain1) / ch0;
> +
> + helper = gain1 * gain1;
> + if (helper > ch0) {
> + do_div(helper64, helper);
> +
> + return gain_mul_div_helper(helper64, gain0, ch0);
> + }
> +
> + do_div(helper64, ch0);
> +
> + return gain_mul_div_helper(helper64, gain0, helper);
> +}
> +
> +static u64 bu27034_fixp_calc_t23(unsigned int coeff, unsigned int ch,
> + unsigned int gain)
> +{
> + unsigned int helper;
> + u64 helper64;
> +
> + helper64 = (u64)coeff * (u64)ch;
> + helper = coeff * ch;
> +
> + if (helper == helper64)
> + return helper / gain;
> +
> + do_div(helper64, gain);
> +
> + return helper64;
I suspect that this is a premature bit of optimization so I'd just
do it in 64 bits always.
Also, if you did want to do this, check_mul_overflow() etc would help.
(linux/overflow.h)
> +}
> +
> +static int bu27034_calc_mlux(struct bu27034_data *data, __le16 *res, int *val)
> +{
> + unsigned int gain0, gain1, meastime;
> + unsigned int d1_d0_ratio_scaled;
> + u16 ch0, ch1;
Stray space after the u16
> + u64 helper64;
> + int ret;
> +
> + /*
> + * We return 0 luxes if calculation fails. This should be reasonably
0 lux
(I think)
> + * easy to spot from the buffers especially if raw-data channels show
> + * valid values
> + */
