All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexandre Belloni <alexandre.belloni@bootlin.com>
To: Hugo Villeneuve <hugo@hugovil.com>
Cc: Alessandro Zummo <a.zummo@towertech.it>,
	Hugo Villeneuve <hvilleneuve@dimonoff.com>,
	linux-rtc@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] rtc: isl1208: avoid unnecessary rc variable tests
Date: Wed, 5 Jan 2022 21:01:10 +0100	[thread overview]
Message-ID: <YdX5BocOfHE/0twa@piout.net> (raw)
In-Reply-To: <20220105193440.151359-1-hugo@hugovil.com>

On 05/01/2022 14:34:39-0500, Hugo Villeneuve wrote:
> From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> 
> The rc variable doesn't need to be tested a second time when the <if> block
> evaluates to false.
> 

rc is not tested a second time, here is the relevant listing:

-	if (client->irq > 0)
+	if (client->irq > 0) {
 ffffffff81aef647:	41 8b b5 bc 01 00 00 	mov    0x1bc(%r13),%esi
 ffffffff81aef64e:	85 f6                	test   %esi,%esi
 ffffffff81aef650:	0f 8f 35 01 00 00    	jg     ffffffff81aef78b <isl1208_probe+0x314>
 		rc = isl1208_setup_irq(client, client->irq);
 	if (rc)
 		return rc;
+	}
 
-	if (evdet_irq > 0 && evdet_irq != client->irq)
+	if (evdet_irq > 0 && evdet_irq != client->irq) {
 ffffffff81aef656:	85 db                	test   %ebx,%ebx
 ffffffff81aef658:	7e 0d                	jle    ffffffff81aef667 <isl1208_probe+0x1f0>
 ffffffff81aef65a:	41 39 9d bc 01 00 00 	cmp    %ebx,0x1bc(%r13)
@@ -1663,6 +1664,7 @@ ffffffff81aef661:	0f 85 0a 01 00 00
 		rc = isl1208_setup_irq(client, evdet_irq);
 	if (rc)
 		return rc;
+	}

As you can see, no change in assembly but it is worse to read. gcc on
arm behaves the same way.

> Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> ---
>  drivers/rtc/rtc-isl1208.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c
> index 182dfa605515..c7f04df5a0b6 100644
> --- a/drivers/rtc/rtc-isl1208.c
> +++ b/drivers/rtc/rtc-isl1208.c
> @@ -880,15 +880,17 @@ isl1208_probe(struct i2c_client *client, const struct i2c_device_id *id)
>  	if (rc)
>  		return rc;
>  
> -	if (client->irq > 0)
> +	if (client->irq > 0) {
>  		rc = isl1208_setup_irq(client, client->irq);
> -	if (rc)
> -		return rc;
> +		if (rc)
> +			return rc;
> +	}
>  
> -	if (evdet_irq > 0 && evdet_irq != client->irq)
> +	if (evdet_irq > 0 && evdet_irq != client->irq) {
>  		rc = isl1208_setup_irq(client, evdet_irq);
> -	if (rc)
> -		return rc;
> +		if (rc)
> +			return rc;
> +	}
>  
>  	rc = devm_rtc_nvmem_register(isl1208->rtc, &isl1208->nvmem_config);
>  	if (rc)
> -- 
> 2.30.2
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

  reply	other threads:[~2022-01-05 20:01 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-05 19:34 [PATCH] rtc: isl1208: avoid unnecessary rc variable tests Hugo Villeneuve
2022-01-05 20:01 ` Alexandre Belloni [this message]
2022-01-05 20:34   ` Hugo Villeneuve
2022-01-05 22:25     ` Alexandre Belloni

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YdX5BocOfHE/0twa@piout.net \
    --to=alexandre.belloni@bootlin.com \
    --cc=a.zummo@towertech.it \
    --cc=hugo@hugovil.com \
    --cc=hvilleneuve@dimonoff.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rtc@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.