* [PATCH] staging: iio: cdc: ad7746: Fix unnecessary check and assignment in ad7746_probe()
@ 2021-05-17 15:00 Tang Bin
2021-05-17 22:14 ` Marcelo Schmitt
2021-05-18 7:52 ` [PATCH] staging: iio: cdc: ad7746: Fix unnecessary check and assignment " Dan Carpenter
0 siblings, 2 replies; 6+ messages in thread
From: Tang Bin @ 2021-05-17 15:00 UTC (permalink / raw)
To: lars, Michael.Hennerich, jic23, knaack.h, pmeerw, gregkh
Cc: linux-iio, devel, linux-kernel, Tang Bin
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 <tangbin@cmss.chinamobile.com>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: iio: cdc: ad7746: Fix unnecessary check and assignment in ad7746_probe()
2021-05-17 15:00 [PATCH] staging: iio: cdc: ad7746: Fix unnecessary check and assignment in ad7746_probe() Tang Bin
@ 2021-05-17 22:14 ` Marcelo Schmitt
2021-05-18 3:19 ` [PATCH] staging: iio: cdc: ad7746: Fix unnecessary check andassignment " tangbin
2021-05-18 7:52 ` [PATCH] staging: iio: cdc: ad7746: Fix unnecessary check and assignment " Dan Carpenter
1 sibling, 1 reply; 6+ messages in thread
From: Marcelo Schmitt @ 2021-05-17 22:14 UTC (permalink / raw)
To: Tang Bin
Cc: lars, Michael.Hennerich, jic23, knaack.h, pmeerw, gregkh,
linux-iio, devel, linux-kernel, lucas.p.stankus
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.stankus@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.stankus@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 <tangbin@cmss.chinamobile.com>
> ---
> 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
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: iio: cdc: ad7746: Fix unnecessary check andassignment in ad7746_probe()
2021-05-17 22:14 ` Marcelo Schmitt
@ 2021-05-18 3:19 ` tangbin
0 siblings, 0 replies; 6+ messages in thread
From: tangbin @ 2021-05-18 3:19 UTC (permalink / raw)
To: Marcelo Schmitt
Cc: lars, Michael.Hennerich, jic23, knaack.h, pmeerw, gregkh,
linux-iio, devel, linux-kernel, lucas.p.stankus
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.stankus@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.stankus@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 <tangbin@cmss.chinamobile.com>
>> ---
>> 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
>>
>>
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: iio: cdc: ad7746: Fix unnecessary check and assignment in ad7746_probe()
2021-05-17 15:00 [PATCH] staging: iio: cdc: ad7746: Fix unnecessary check and assignment in ad7746_probe() Tang Bin
2021-05-17 22:14 ` Marcelo Schmitt
@ 2021-05-18 7:52 ` Dan Carpenter
2021-05-18 9:27 ` [PATCH] staging: iio: cdc: ad7746: Fix unnecessary check andassignment " tangbin
1 sibling, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2021-05-18 7:52 UTC (permalink / raw)
To: Tang Bin
Cc: lars, Michael.Hennerich, jic23, knaack.h, pmeerw, gregkh,
linux-iio, devel, linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: iio: cdc: ad7746: Fix unnecessary check andassignment in ad7746_probe()
2021-05-18 7:52 ` [PATCH] staging: iio: cdc: ad7746: Fix unnecessary check and assignment " Dan Carpenter
@ 2021-05-18 9:27 ` tangbin
2021-05-18 17:59 ` Jonathan Cameron
0 siblings, 1 reply; 6+ messages in thread
From: tangbin @ 2021-05-18 9:27 UTC (permalink / raw)
To: Dan Carpenter
Cc: lars, Michael.Hennerich, jic23, knaack.h, pmeerw, gregkh,
linux-iio, devel, linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: iio: cdc: ad7746: Fix unnecessary check andassignment in ad7746_probe()
2021-05-18 9:27 ` [PATCH] staging: iio: cdc: ad7746: Fix unnecessary check andassignment " tangbin
@ 2021-05-18 17:59 ` Jonathan Cameron
0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2021-05-18 17:59 UTC (permalink / raw)
To: tangbin
Cc: Dan Carpenter, lars, Michael.Hennerich, knaack.h, pmeerw, gregkh,
linux-iio, devel, linux-kernel
On Tue, 18 May 2021 17:27:07 +0800
tangbin <tangbin@cmss.chinamobile.com> 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
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-05-18 17:58 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-17 15:00 [PATCH] staging: iio: cdc: ad7746: Fix unnecessary check and assignment in ad7746_probe() Tang Bin
2021-05-17 22:14 ` Marcelo Schmitt
2021-05-18 3:19 ` [PATCH] staging: iio: cdc: ad7746: Fix unnecessary check andassignment " tangbin
2021-05-18 7:52 ` [PATCH] staging: iio: cdc: ad7746: Fix unnecessary check and assignment " Dan Carpenter
2021-05-18 9:27 ` [PATCH] staging: iio: cdc: ad7746: Fix unnecessary check andassignment " tangbin
2021-05-18 17:59 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).