Re: [PATCH] iio: humidity: hdc3020: add threshold events support

[Dimitri Fedrau]

Am Sat, Feb 03, 2024 at 10:58:24AM +0100 schrieb Christophe JAILLET:
> Le 03/02/2024 à 10:05, Dimitri Fedrau a écrit :
>[...]
> > diff --git a/drivers/iio/humidity/hdc3020.c b/drivers/iio/humidity/hdc3020.c
> > index ed70415512f6..1cdff7af4ca8 100644
> > --- a/drivers/iio/humidity/hdc3020.c
> > +++ b/drivers/iio/humidity/hdc3020.c
> > @@ -16,16 +16,27 @@
> >   #include <linux/init.h>
> >   #include <linux/module.h>
> >   #include <linux/mutex.h>
> > +#include <linux/interrupt.h>
> 
> Nit: alphabetical order could be kept
>
Will fix it.
> >   #include <asm/unaligned.h>
> >   #include <linux/iio/iio.h>
> > +#include <linux/iio/events.h>
> 
> Nit: same
> 
> ...
> 
Will fix it.
> > +static const u8 HDC3020_R_T_RH_THRESH_LOW[2] = { 0xE1, 0x02 };
> > +static const u8 HDC3020_R_R_RH_THRESH_LOW_CLR[2] = { 0xE1, 0x09 };
> 
> I don't know what the R and T are for, but shoukld this be
> HDC3020_R_T_RH_THRESH_LOW_CLR to match other adjacent line?
>
You are right, should match the others. Will fix it.
> > +static const u8 HDC3020_R_T_RH_THRESH_HIGH_CLR[2] = { 0xE1, 0x14 };
> > +static const u8 HDC3020_R_T_RH_THRESH_HIGH[2] = { 0xE1, 0x1F };
> 
> ...
>
>[...]
> Maybe something like:
>   #define MIN_TEMP -45 (or -40)
>   #define MAX_TEMP 120
> in order to avoid hard coded constant?
> 
Will add the constants, thanks.
>[...]

Best regards,
Dimitri