Re: [PATCH] staging: iio: cdc: ad7746: Fix unnecessary check andassignment in ad7746_probe()
On Tue, 18 May 2021 17:27:07 +0800 tangbin wrote: > Hi Dan: > > On 2021/5/18 15:52, Dan Carpenter wrote: > > 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. > > > Got it, I will send this patch for you. I fall a bit different on this and would consider the above a cleanup though one I'd prefer to get with more significant stuff rather than on it's own. However, there is already a patch in revision that includes the same change from Lucas Stankus. > > Thanks > > Tang Bin > > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: iio: cdc: ad7746: Fix unnecessary check andassignment in ad7746_probe()
Hi Dan: On 2021/5/18 15:52, Dan Carpenter wrote: 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. Got it, I will send this patch for you. Thanks Tang Bin ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: iio: cdc: ad7746: Fix unnecessary check andassignment in ad7746_probe()
Hi Marcelo: On 2021/5/18 6:14, Marcelo Schmitt wrote: 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/ Thanks for your reply, I really don't know someone has send the similar one. I forget this patch. 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. OK, got it! Thanks Tang Bin 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(&client->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