On Sat, May 4, 2019 at 1:26 AM Melissa Wen <melissa.srw@xxxxxxxxx> wrote: > > Since i2c_smbus_write_byte_data returns no-positive value, this commit > making the treatment of its return value less verbose. > > Signed-off-by: Melissa Wen <melissa.srw@xxxxxxxxx> > --- > drivers/staging/iio/cdc/ad7150.c | 10 +++------- > 1 file changed, 3 insertions(+), 7 deletions(-) > > diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c > index 4ba46fb6ac02..3a4572a9e5ec 100644 > --- a/drivers/staging/iio/cdc/ad7150.c > +++ b/drivers/staging/iio/cdc/ad7150.c > @@ -201,16 +201,12 @@ static int ad7150_write_event_params(struct iio_dev *indio_dev, > ret = i2c_smbus_write_byte_data(chip->client, > ad7150_addresses[chan][4], > sens); > - if (ret < 0) > + if (ret) For i2c_smbus_write_byte_data(), checking "ret < 0" or non-zero, is the same. Changing this doesn't have any added value. > return ret; > - > - ret = i2c_smbus_write_byte_data(chip->client, > + else > + return i2c_smbus_write_byte_data(chip->client, > ad7150_addresses[chan][5], > timeout); The introduction of the "else" branch is a bit noisy. The code was a bit neater (and readable) before the else branch, and functionally identical. Well, when I say neater before, you have to understand, that I (and I assume that some other people who write drivers) have a slight fixation for this pattern: example1: ret = fn1(); if (ret < 0) // could also be just "if (ret)" return ret; ret = fn2(); if (ret < 0) // could also be just "if (ret)" return ret; example1a: +ret = fn3(); +if (ret < 0) // could also be just "if (ret)" + return ret; Various higher-level programming languages, will discourage this pattern in favor of neater patterns. I personally, have a few arguments in favor of this pattern: 1) it is closer to how the machine code ; so, closer to how a low-level instruction looks like 2) if (ever) this needs to be patched, the patch could be neat (see example1a) ; the examle assumes that it's been added via a patch at a later point in time 3) it keeps indentation level to a minimum ; this also aligns with kernel-coding guidelines (https://www.kernel.org/doc/html/v4.10/process/coding-style.html ) (indentation seems a bit OCD-like when someone points it out at a review, but it has it's value over time) > - if (ret < 0) > - return ret; > - > - return 0; > } > > static int ad7150_write_event_config(struct iio_dev *indio_dev, > -- > 2.20.1 > _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
