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)
With that, you even may have
#define BMP380_STATUS_DRDY_PRESS_AND_TEMP_MASK ...
if (!(reg & BMP380_STATUS_DRDY_PRESS_AND_TEMP_MASK)) {
dev_err(data->dev, "Measurement cycle didn't complete.\n");
return -EBUSY;
}
which makes it all obvious.
--
With Best Regards,
Andy Shevchenko
