linux-kernel - Re: [PATCH v5 4/7] iio: pressure: bmp280: Use sleep and forced mode for oneshot captures

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: <Zths-IcwYyUV_kYk@smile.fi.intel.com>
Date: Wed, 4 Sep 2024 17:21:44 +0300
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Vasileios Amoiridis <vassilisamir@...il.com>
Cc: jic23@...nel.org, lars@...afoo.de, robh@...nel.org, krzk+dt@...nel.org,
	conor+dt@...nel.org, ang.iglesiasg@...il.com,
	linus.walleij@...aro.org, biju.das.jz@...renesas.com,
	javier.carrasco.cruz@...il.com, semen.protsenko@...aro.org,
	579lpy@...il.com, ak@...klinger.de, linux-iio@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	christophe.jaillet@...adoo.fr
Subject: Re: [PATCH v5 4/7] iio: pressure: bmp280: Use sleep and forced mode
 for oneshot captures

On Wed, Sep 04, 2024 at 05:17:29PM +0300, Andy Shevchenko wrote:
> On Wed, Sep 04, 2024 at 12:24:27PM +0200, Vasileios Amoiridis wrote:
> > On Tue, Sep 03, 2024 at 05:26:38PM +0300, Andy Shevchenko wrote:
> > > On Mon, Sep 02, 2024 at 08:42:19PM +0200, Vasileios Amoiridis wrote:

...

> > > > +	if (!(reg & BMP380_STATUS_DRDY_PRESS_MASK) ||
> > > > +	    !(reg & BMP380_STATUS_DRDY_TEMP_MASK)) {
> > > > +		dev_err(data->dev, "Measurement cycle didn't complete.\n");
> > > > +		return -EBUSY;
> > > > +	}
> > > 
> > > Alternatively
> > > 
> > > 	if (!((reg & BMP380_STATUS_DRDY_PRESS_MASK) &&
> > > 	    !(reg & BMP380_STATUS_DRDY_TEMP_MASK)) {
> > > 		dev_err(data->dev, "Measurement cycle didn't complete.\n");
> > > 		return -EBUSY;
> > > 	}
> > 
> > Why would I use && instead of || ? I just need one of the 2 to be true
> > (one of the 2 measurements is not complete) and I can trigger the error
> > action.
> 
> Oh, I messed up the logic inversion, but wouldn't it be simpler to read
> "we return busy if neither press nor temp drdy bit set"?
> 
> 	if (!((reg & BMP380_STATUS_DRDY_PRESS_MASK) && (reg & BMP380_STATUS_DRDY_TEMP_MASK))) {
> 		dev_err(data->dev, "Measurement cycle didn't complete.\n");
> 		return -EBUSY;
> 	}
> 
> (I left long line for the better understanding of my point, you may break it to
>  two if needed)

Scratch below, it needs more thinking...

> With that, you even may have
> 
> #define BMP380_STATUS_DRDY_PRESS_AND_TEMP_MASK ...
> 
> 	if (!(reg & BMP380_STATUS_DRDY_PRESS_AND_TEMP_MASK)) {

Maybe ^, but I have no time to dive into this, you got the idea I believe.

> 		dev_err(data->dev, "Measurement cycle didn't complete.\n");
> 		return -EBUSY;
> 	}
> 
> which makes it all obvious.

-- 
With Best Regards,
Andy Shevchenko