Re: [PATCH 2/2] staging: iio: imu: Add CEVA BNO08x driver

[Dan Carpenter <dan.carpenter@oracle.com>]

On Fri, Jun 17, 2022 at 06:49:41PM +0100, Jonathan Cameron wrote:
> > +static irqreturn_t bno08x_trigger_handler(int irq, void *p)
> > +{
> > +	struct iio_poll_func *pf = p;
> > +	struct iio_dev *iio_dev = pf->indio_dev;
> > +	struct bno08x_dev *bno08x = iio_priv(iio_dev);
> > +	u8 cargo[BNO08x_CARGO_BUFFER_SIZE];
> > +	int ret;
> > +
> > +	ret = bno08x_wait_for_cargo_timeout(bno08x, cargo, BNO08x_CARGO_BUFFER_SIZE);
> > +	if (ret < 0)
> > +		goto done;
> > +
> I'm lazy - if it's less than 0 it's definitely less than 24 so no need for separate
> paths unless you add some dev_dbg/err or similar which might make sense here.
> 

Also 24 is a magic number.  But you have to be a bit careful with
combining conditions.  The two common bugs are forgot to set the error
code and type promotion to unsigned long:

	if (ret < sizeof(buf))
		goto done;

Negative ret values are > sizeof(buf) and for 0 to sizeof - 1 then the
error code isn't set so this made up example would be totally broken.
None of that applies here, of course, but I like breaking the conditions
apart just as a habit.

> > +	if (ret < 24)
> > +		goto done;
> > +

regards,
dan carpenter