Messages in this thread | | | Date | Thu, 15 Mar 2018 21:28:00 -0300 | From | Rodrigo Siqueira <> | Subject | Re: [PATCH 7/7] staging:iio:ade7854: Add proper error handling condition |
| |
On 03/15, Dan Carpenter wrote: > On Wed, Mar 14, 2018 at 03:12:18PM -0300, Rodrigo Siqueira wrote: > > There is some improper error handling for IRQ and device register. This > > patch adds a proper verification. The IRQ correction was extracted from > > John Syne patches. > > > > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com> > > Signed-off-by: John Syne <john3909@gmail.com> > > --- > > drivers/staging/iio/meter/ade7854.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/staging/iio/meter/ade7854.c b/drivers/staging/iio/meter/ade7854.c > > index 09fd8c067738..49cbe365e43d 100644 > > --- a/drivers/staging/iio/meter/ade7854.c > > +++ b/drivers/staging/iio/meter/ade7854.c > > @@ -436,7 +436,7 @@ static int ade7854_initial_setup(struct iio_dev *indio_dev) > > > > /* Disable IRQ */ > > ret = ade7854_set_irq(dev, false); > > - if (ret) { > > + if (ret < 0) { > > dev_err(dev, "disable irq failed"); > > goto err_ret; > > } > > Why is the original wrong? It seems fine.
Hi,
If you look at "drivers/staging/iio/meter/ade7854-(i2c|spi).c", you will find a line like this: " st->write_reg = ade7854_(i2c|spi)_write_reg;". Than, if you go through the "ade7854_i2c_write_reg" (focus only on i2c for while) you will see "i2c_master_send(st->i2c, st->tx, count)", which can returns a positive number that does not necessary means an error. If you find the same for the spi case, you will end up in the function "spi_sync_transfer" that only return 0 for success or negative for failure. So, handling the negative case better fit the i2c and spi case.
Thanks
> regards, > dan carpenter > >
| |