linux-kernel - Re: [PATCH v3 2/3] iio: mpl3115: add threshold 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: <20251106202748.zuysikmqfy3mfacd@antoni-VivoBook-ASUSLaptop-X512FAY-K512FA>
Date: Thu, 6 Nov 2025 21:28:07 +0100
From: Antoni Pokusinski <apokusinski01@...il.com>
To: Andy Shevchenko <andriy.shevchenko@...el.com>
Cc: jic23@...nel.org, dlechner@...libre.com, nuno.sa@...log.com,
	andy@...nel.org, marcelo.schmitt1@...il.com,
	linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 2/3] iio: mpl3115: add threshold events support

On Wed, Nov 05, 2025 at 05:25:17PM +0200, Andy Shevchenko wrote:
> On Wed, Nov 05, 2025 at 10:56:14AM +0100, Antoni Pokusinski wrote:
> > static irqreturn_t mpl3115_interrupt_handler(int irq, void *private)
> 
> >  	struct iio_dev *indio_dev = private;
> >  	struct mpl3115_data *data = iio_priv(indio_dev);
> >  	int ret;
> > +	u8 val_press[3];
> > +	__be16 val_temp;
> 
> s/_temp/$SOMETHING meaningful/ ?
> 
In this case I'd leave the "val_temp". We have "val_press" and
"val_temp" here so imo it indicates quite clearly that this variable
stores a temperature measurement.

The cases with "tmp" that you pointed out can be a bit confusing indeed, so
I'm going to replace them with something more meaningful (e.g.
"tmp" -> "press_tgt" in read_thresh() )  

> >  	ret = i2c_smbus_read_byte_data(data->client, MPL3115_INT_SOURCE);
> >  	if (ret < 0)
> >  		return IRQ_HANDLED;
> >  
> > -	if (!(ret & MPL3115_INT_SRC_DRDY))
> > +	if (!(ret & (MPL3115_INT_SRC_TTH | MPL3115_INT_SRC_PTH |
> > +		     MPL3115_INT_SRC_DRDY)))
> >  		return IRQ_NONE;
> >  
> > -	iio_trigger_poll_nested(data->drdy_trig);
> > +	if (ret & MPL3115_INT_SRC_DRDY)
> > +		iio_trigger_poll_nested(data->drdy_trig);
> > +
> > +	if (ret & MPL3115_INT_SRC_PTH) {
> > +		iio_push_event(indio_dev,
> > +			       IIO_UNMOD_EVENT_CODE(IIO_PRESSURE, 0,
> > +						    IIO_EV_TYPE_THRESH,
> > +						    IIO_EV_DIR_RISING),
> > +						    iio_get_time_ns(indio_dev));
> > +
> > +		/* Reset the SRC_PTH bit in INT_SOURCE */
> > +		i2c_smbus_read_i2c_block_data(data->client,
> > +					      MPL3115_OUT_PRESS,
> > +					      sizeof(val_press), val_press);
> > +	}
> > +
> > +	if (ret & MPL3115_INT_SRC_TTH) {
> > +		iio_push_event(indio_dev,
> > +			       IIO_UNMOD_EVENT_CODE(IIO_TEMP, 0,
> > +						    IIO_EV_TYPE_THRESH,
> > +						    IIO_EV_DIR_RISING),
> > +						    iio_get_time_ns(indio_dev));
> > +
> > +		/* Reset the SRC_TTH bit in INT_SOURCE */
> > +		i2c_smbus_read_i2c_block_data(data->client,
> > +					      MPL3115_OUT_TEMP,
> > +					      2, (u8 *)&val_temp);

Kind regards,
Antoni