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

[Andy Shevchenko <andriy.shevchenko@intel.com>]

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:

...

> > > +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.

> 		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 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