Received-SPF: pass (google.com: domain of linux-kernel+bounces-91132-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
From: Vasileios Amoiridis <vassilisamir@gmail.com>
Date: Mon, 4 Mar 2024 19:50:17 +0100
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Vasileios Amoiridis <vassilisamir@gmail.com>, jic23@kernel.org,
	lars@metafoo.de, ang.iglesiasg@gmail.com, mazziesaccount@gmail.com,
	ak@it-klinger.de, petre.rodan@subdimension.ro, phil@raspberrypi.com,
	579lpy@gmail.com, linux-iio@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/4] iio: pressure: Add timestamp and scan_masks for
 BMP280 driver
Message-ID: <20240304185017.GA3955@vamoiridPC>
References: <20240303165300.468011-1-vassilisamir@gmail.com>
 <20240303165300.468011-4-vassilisamir@gmail.com>
 <ZeW048EyOAze7oZR@smile.fi.intel.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZeW048EyOAze7oZR@smile.fi.intel.com>

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