Received-SPF: pass (google.com: domain of linux-kernel+bounces-51122-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Sat, 3 Feb 2024 15:47:45 +0100
From: Dimitri Fedrau <dima.fedrau@gmail.com>
To: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Cc: 579lpy@gmail.com, javier.carrasco.cruz@gmail.com, jic23@kernel.org,
	lars@metafoo.de, linux-iio@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] iio: humidity: hdc3020: add threshold events support
Message-ID: <20240203144745.GB3727@debian>
References: <20240203090530.53374-1-dima.fedrau@gmail.com>
 <236dc9db-8525-413e-a77b-54df198c86f5@wanadoo.fr>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <236dc9db-8525-413e-a77b-54df198c86f5@wanadoo.fr>

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