On Sat, 25 Nov 2023 17:17:06 +0100
Javier Carrasco <javier.carrasco.cruz@xxxxxxxxx> wrote:
> On 25.11.23 16:11, Jonathan Cameron wrote:
> > On Sat, 25 Nov 2023 12:56:57 +0100
> > Javier Carrasco <javier.carrasco.cruz@xxxxxxxxx> wrote:
> >
> >> The Vishay VEMl6075 is a low power, 16-bit resolution UVA and UVB
> >> light sensor with I2C interface and noise compensation (visible and
> >> infrarred).
> >>
> >> Every UV channel generates an output signal measured in counts per
> >> integration period, where the integration time is configurable.
> >>
> >> This driver adds support for both UV channels and the ultraviolet
> >> index (UVI) inferred from them according to the device application note
> >> with open-air (no teflon) coefficients.
> >>
> >> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@xxxxxxxxx>
> >
> > Hi Javier,
> >
> > A few more minor things. Looks good in general.
> >
> > Jonathan
> >
> >> +struct veml6075_data {
> >> + struct i2c_client *client;
> >> + struct regmap *regmap;
> >> + struct mutex lock; /* integration time/measurement trigger lock */
> >
> > Could perhaps be clearer. Maybe something like
> > /* Prevent integration time changing during a measurement */
> >
> It prevents integration time changing as well as measurement triggers
> while a measurement is underway. I just wanted to cover both usages with
> a short comment in the same line.
Ah. Well I misunderstood it, so burn a few more lines :)
> >> +
> >> +static int veml6075_uva_comp(int raw_uva, int comp1, int comp2)
> >> +{
> >> + int comp1a_c, comp2a_c, uva_comp;
> >> +
> >> + comp1a_c = (comp1 * VEML6075_A_COEF) / 1000U;
> >> + comp2a_c = (comp2 * VEML6075_B_COEF) / 1000U;
> >> + uva_comp = raw_uva - comp1a_c - comp2a_c;
> >> +
> >> + return clamp_val(uva_comp, 0, U16_MAX);
> >> +}
> >> +
> >> +static int veml6075_uvb_comp(int raw_uvb, int comp1, int comp2)
> >> +{
> >> + int comp1b_c, comp2b_c, uvb_comp;
> >> +
> >> + comp1b_c = (comp1 * VEML6075_C_COEF) / 1000U;
> >
> > Any of units.h appropriate here? I'm not sure if the / 1000U is a units
> > thing or not.
> >
> These divisions are used to scale the coefficients down, as they are
> defined as entire numbers. These coefficients have no units and the
> resulting value is a count.
Fair enough.
>
> I have nothing to add to the rest of your comments. I will start working
> on v3.
>
> Thanks again for your thorough review.
>
> Best regards,
> Javier Carrasco
