Re: [PATCH] staging: iio: cdc: ad7746: Fix unnecessary check and assignment in ad7746_probe()
On Mon, May 17, 2021 at 11:00:06PM +0800, Tang Bin wrote: > @@ -730,11 +730,7 @@ static int ad7746_probe(struct i2c_client *client, > if (ret < 0) > return ret; > > - ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev); > - if (ret) > - return ret; > - > - return 0; > + return devm_iio_device_register(indio_dev->dev.parent, indio_dev); > } This sort of thing is done deliberately as a style choice... I probably wouldn't have written it that way myself, but there really isn't a downside to leaving it as-is. The unused "int ret = 0;" just introduces a static checker warning about unused assignments and disables the static checker warning for uninitialized variables so we want to remove that. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: iio: cdc: ad7746: Fix unnecessary check and assignment in ad7746_probe()
Hi Tang, The patch looks overall good, though I think it could be split into two pieces: one for simplifying ret declaration and another for removing the check after device register. Despite that, I guess Lucas might already be working on similar changes. https://lore.kernel.org/linux-iio/cover.1620766020.git.lucas.p.stan...@gmail.com/ As general advice, I would recommend avoiding using generic words such as fix in the subject line. It's often better to say something about the nature of what is being done. Cc: lucas.p.stan...@gmail.com Best regards, Marcelo On 05/17, Tang Bin wrote: > In the function ad7746_probe(), the return value of > devm_iio_device_register() can be zero or ret, thus it is > unnecessary to repeated check here. And delete unused > initialized value of 'ret', because it will be assigned by > the function i2c_smbus_write_byte_data(). > > Signed-off-by: Tang Bin > --- > drivers/staging/iio/cdc/ad7746.c | 8 ++-- > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/drivers/staging/iio/cdc/ad7746.c > b/drivers/staging/iio/cdc/ad7746.c > index dfd71e99e..d3b6e68df 100644 > --- a/drivers/staging/iio/cdc/ad7746.c > +++ b/drivers/staging/iio/cdc/ad7746.c > @@ -680,7 +680,7 @@ static int ad7746_probe(struct i2c_client *client, > struct ad7746_chip_info *chip; > struct iio_dev *indio_dev; > unsigned char regval = 0; > - int ret = 0; > + int ret; > > indio_dev = devm_iio_device_alloc(>dev, sizeof(*chip)); > if (!indio_dev) > @@ -730,11 +730,7 @@ static int ad7746_probe(struct i2c_client *client, > if (ret < 0) > return ret; > > - ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev); > - if (ret) > - return ret; > - > - return 0; > + return devm_iio_device_register(indio_dev->dev.parent, indio_dev); > } > > static const struct i2c_device_id ad7746_id[] = { > -- > 2.20.1.windows.1 > > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: iio: cdc: ad7746: Fix unnecessary check and assignment in ad7746_probe()
In the function ad7746_probe(), the return value of devm_iio_device_register() can be zero or ret, thus it is unnecessary to repeated check here. And delete unused initialized value of 'ret', because it will be assigned by the function i2c_smbus_write_byte_data(). Signed-off-by: Tang Bin --- drivers/staging/iio/cdc/ad7746.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c index dfd71e99e..d3b6e68df 100644 --- a/drivers/staging/iio/cdc/ad7746.c +++ b/drivers/staging/iio/cdc/ad7746.c @@ -680,7 +680,7 @@ static int ad7746_probe(struct i2c_client *client, struct ad7746_chip_info *chip; struct iio_dev *indio_dev; unsigned char regval = 0; - int ret = 0; + int ret; indio_dev = devm_iio_device_alloc(>dev, sizeof(*chip)); if (!indio_dev) @@ -730,11 +730,7 @@ static int ad7746_probe(struct i2c_client *client, if (ret < 0) return ret; - ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev); - if (ret) - return ret; - - return 0; + return devm_iio_device_register(indio_dev->dev.parent, indio_dev); } static const struct i2c_device_id ad7746_id[] = { -- 2.20.1.windows.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel