linux-kernel - Re: [PATCH v1 1/4] pinctrl: intel: optimize set

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230608080821.GF45886@black.fi.intel.com>
Date:   Thu, 8 Jun 2023 11:08:21 +0300
From:   Mika Westerberg <mika.westerberg@...ux.intel.com>
To:     Raag Jadav <raag.jadav@...el.com>
Cc:     linus.walleij@...aro.org, andriy.shevchenko@...ux.intel.com,
        linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org,
        mallikarjunappa.sangannavar@...el.com, pandith.n@...el.com
Subject: Re: [PATCH v1 1/4] pinctrl: intel: optimize set_mux hook

On Thu, Jun 08, 2023 at 12:30:14PM +0530, Raag Jadav wrote:
> Utilize a temporary variable for common shift operation
> inside ->set_mux() hook and save a few bytes.
> 
> add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-3 (-3)
> Function                                     old     new   delta
> intel_pinmux_set_mux                         245     242      -3
> Total: Before=10472, After=10469, chg -0.03%

Shouldn't the compiler be able to optimize this if you ask with the -Ox
options?

I don't really see much benefit for "optimizations" like this. That said
using temporary variable here improves readability so this one is
acceptable by me. As long as you change the commit message accordingly.

> Signed-off-by: Raag Jadav <raag.jadav@...el.com>
> ---
>  drivers/pinctrl/intel/pinctrl-intel.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
> index c7a71c49df0a..e8adf2580321 100644
> --- a/drivers/pinctrl/intel/pinctrl-intel.c
> +++ b/drivers/pinctrl/intel/pinctrl-intel.c
> @@ -411,18 +411,19 @@ static int intel_pinmux_set_mux(struct pinctrl_dev *pctldev,
>  	/* Now enable the mux setting for each pin in the group */
>  	for (i = 0; i < grp->grp.npins; i++) {
>  		void __iomem *padcfg0;
> -		u32 value;
> +		u32 value, pmode;
>  
>  		padcfg0 = intel_get_padcfg(pctrl, grp->grp.pins[i], PADCFG0);
> -		value = readl(padcfg0);
>  
> +		value = readl(padcfg0);
>  		value &= ~PADCFG0_PMODE_MASK;
>  
>  		if (grp->modes)
> -			value |= grp->modes[i] << PADCFG0_PMODE_SHIFT;
> +			pmode = grp->modes[i];
>  		else
> -			value |= grp->mode << PADCFG0_PMODE_SHIFT;
> +			pmode = grp->mode;
>  
> +		value |= pmode << PADCFG0_PMODE_SHIFT;
>  		writel(value, padcfg0);
>  	}
>  
> -- 
> 2.17.1