linux-kernel - Re: [PATCH 3/4] iio: pressure: Add timestamp and scan

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]

Date: Mon, 4 Mar 2024 19:50:17 +0100
From: Vasileios Amoiridis <vassilisamir@...il.com>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc: Vasileios Amoiridis <vassilisamir@...il.com>, jic23@...nel.org,
	lars@...afoo.de, ang.iglesiasg@...il.com, mazziesaccount@...il.com,
	ak@...klinger.de, petre.rodan@...dimension.ro, phil@...pberrypi.com,
	579lpy@...il.com, linux-iio@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/4] iio: pressure: Add timestamp and scan_masks for
 BMP280 driver

On Mon, Mar 04, 2024 at 01:47:47PM +0200, Andy Shevchenko wrote:
> On Sun, Mar 03, 2024 at 05:52:59PM +0100, Vasileios Amoiridis wrote:
> > The scan mask for the BME280 device which contains humidity
> > measurement needs to become different in order for the timestamp
> > to be able to work. Scan masks are added for different combinations
> > of measurements. The temperature measurement is needed for either
> > pressure or humidity measurements.
> 
> ...
> 
> > +enum bmp280_scan {
> > +	BMP280_TEMP,
> > +	BMP280_PRESS,
> > +	BME280_HUMID,
> > +};
> 
> Hmm... Why do we need to actually copy the IIO ones? Can't we use IIO ones
> directly (or in some way)?
> 

What do you mean exactly by copying the IIO ones? These values are used as
indexes to enable channels in the avail_scan_masks below. 
> ...
> 
> > +static const unsigned long bmp280_avail_scan_masks[] = {
> > +	BIT(BMP280_TEMP),
> > +	BIT(BMP280_PRESS) | BIT(BMP280_TEMP),
> > +	0,
> 
> No comma for the terminator line.
> 
> > +};
> 
> > +static const unsigned long bme280_avail_scan_masks[] = {
> > +	BIT(BMP280_TEMP),
> > +	BIT(BMP280_PRESS) | BIT(BMP280_TEMP),
> > +	BIT(BME280_HUMID) | BIT(BMP280_TEMP),
> > +	BIT(BME280_HUMID) | BIT(BMP280_PRESS) | BIT(BMP280_TEMP),
> > +	0,
> 
> Ditto.
> 
> > +};
> 
> ...
> 
> >  	const struct iio_chan_spec *channels;
> >  	int num_channels;
> > +	const unsigned long *avail_scan_masks;
> >  	unsigned int start_up_time;
> 
> Please, run `pahole` every time you are changing data structure layout.
> Here you efficiently wasted 8 bytes of memory AFAICS.
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
>