linux-kernel - Re: [PATCH v2 7/9] iio: adc: ad4062: Add IIO Events support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <rrhan2ae4ak47sxp4d5auxa547f6a5wehcjy5dwgedzof3jtea@n6en5lky5kow>
Date: Thu, 4 Dec 2025 22:37:51 +0100
From: Jorge Marques <gastmaier@...il.com>
To: Andy Shevchenko <andriy.shevchenko@...el.com>
Cc: Jorge Marques <jorge.marques@...log.com>, 
	Lars-Peter Clausen <lars@...afoo.de>, Michael Hennerich <Michael.Hennerich@...log.com>, 
	Jonathan Cameron <jic23@...nel.org>, David Lechner <dlechner@...libre.com>, 
	Nuno Sá <nuno.sa@...log.com>, Andy Shevchenko <andy@...nel.org>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Jonathan Corbet <corbet@....net>, Linus Walleij <linus.walleij@...aro.org>, 
	Bartosz Golaszewski <brgl@...ev.pl>, linux-iio@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org, linux-gpio@...r.kernel.org
Subject: Re: [PATCH v2 7/9] iio: adc: ad4062: Add IIO Events support

On Thu, Nov 27, 2025 at 11:13:28AM +0200, Andy Shevchenko wrote:
> On Wed, Nov 26, 2025 at 04:00:36PM +0100, Jorge Marques wrote:
> > On Mon, Nov 24, 2025 at 12:33:12PM +0200, Andy Shevchenko wrote:
> > > On Mon, Nov 24, 2025 at 10:18:06AM +0100, Jorge Marques wrote:
> 
> ...
> 
Hi Andy,
> > > > +static int __ad4062_write_event_info_value(struct ad4062_state *st,
> > > > +					   enum iio_event_direction dir, int val)
> > > > +{
> > > > +	u8 reg;
> > > > +
> > > > +	if (val > 2047 || val < -2048)
> > > > +		return -EINVAL;
> > > 
> > > There was already magic '11', perhaps define it and use there and here?
> > > 
> > > #define x11	11 // needs a good name
> > > 
> > > 	if (val > BIT(x11) || val < -BIT(x11))
> > > 	
> > Not magic number, but max and min signed 12-bit, maybe
> > 
> > 	if (val != sign_extend32(val, 11))
> 
> If you go this way, the 11 still needs a definition.
> 
Sure, I will go with AD4062_LIMIT_BITS, then usage 
	*val = sign_extend32(get_unaligned_be16(st->buf.bytes),
			     AD4062_LIMIT_BITS-1);
> > 		return -EINVAL;
> > to not look like magic numbers, or 
> >   	if (val < (-BIT(11)) || val > BIT(11) - 1)
> >   		return -EINVAL;
> > For Hysteresis I will change from
> > 
> > 	if (val >= BIT(7))
> > to 
> > 	if (val & ~GENMASK(6,0))
> 
> Not sure about this. If it's a HW-based limit, the
> 
> 	val > (BIT(x) - 1)
> 
> says that this is limited by x-bit size of the register (field).
> 
> So, I leave it to Jonathan (my personal preference here is BIT(x) - 1 approach).
> 
I don't have a preferance, will use BIT(7) - 1
> > I believe iio only passes positive to the hysteresis, but is a little clearer.
> > 
> > > > +	if (dir == IIO_EV_DIR_RISING)
> > > > +		reg = AD4062_REG_MAX_LIMIT;
> > > > +	else
> > > > +		reg = AD4062_REG_MIN_LIMIT;
> > > > +	put_unaligned_be16(val, st->buf.bytes);
> > > > +
> > > > +	return regmap_bulk_write(st->regmap, reg, &st->buf.be16,
> > > > +				 sizeof(st->buf.be16));
> > > > +}
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 
Best Regards,
Jorge