linux-kernel - Re: [PATCH v3 2/3] pinctrl: intel: refine ->irq_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: <ZInpT0dUUVUcKdqv@smile.fi.intel.com>
Date:   Wed, 14 Jun 2023 19:22:39 +0300
From:   Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:     Raag Jadav <raag.jadav@...el.com>
Cc:     linus.walleij@...aro.org, mika.westerberg@...ux.intel.com,
        linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org,
        mallikarjunappa.sangannavar@...el.com, pandith.n@...el.com
Subject: Re: [PATCH v3 2/3] pinctrl: intel: refine ->irq_set_type() hook

On Tue, Jun 13, 2023 at 02:20:53PM +0530, Raag Jadav wrote:
> Utilize a temporary variable for common shift operation
> in ->irq_set_type() hook and improve readability.
> While at it, simplify if-else-if chain and save a few bytes.
> 
> add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-16 (-16)
> Function                                     old     new   delta
> intel_gpio_irq_type                          317     301     -16
> Total: Before=10469, After=10453, chg -0.15%

...

>  	value = readl(reg);
> -
>  	value &= ~(PADCFG0_RXEVCFG_MASK | PADCFG0_RXINV);
>  
>  	if ((type & IRQ_TYPE_EDGE_BOTH) == IRQ_TYPE_EDGE_BOTH) {
> -		value |= PADCFG0_RXEVCFG_EDGE_BOTH << PADCFG0_RXEVCFG_SHIFT;
> +		rxevcfg = PADCFG0_RXEVCFG_EDGE_BOTH;
>  	} else if (type & IRQ_TYPE_EDGE_FALLING) {
> -		value |= PADCFG0_RXEVCFG_EDGE << PADCFG0_RXEVCFG_SHIFT;
> -		value |= PADCFG0_RXINV;
> +		rxevcfg = PADCFG0_RXEVCFG_EDGE;
>  	} else if (type & IRQ_TYPE_EDGE_RISING) {
> -		value |= PADCFG0_RXEVCFG_EDGE << PADCFG0_RXEVCFG_SHIFT;
> +		rxevcfg = PADCFG0_RXEVCFG_EDGE;
>  	} else if (type & IRQ_TYPE_LEVEL_MASK) {
> -		if (type & IRQ_TYPE_LEVEL_LOW)
> -			value |= PADCFG0_RXINV;
> +		rxevcfg = PADCFG0_RXEVCFG_LEVEL;
>  	} else {
> -		value |= PADCFG0_RXEVCFG_DISABLED << PADCFG0_RXEVCFG_SHIFT;
> +		rxevcfg = PADCFG0_RXEVCFG_DISABLED;
>  	}
>  
> +	if (type == IRQ_TYPE_EDGE_FALLING || type == IRQ_TYPE_LEVEL_LOW)
> +		value |= PADCFG0_RXINV;
> +
> +	value |= rxevcfg << PADCFG0_RXEVCFG_SHIFT;
>  	writel(value, reg);

Looking at this I realized that entire temporary variable assignments can be
done outside of spin lock. You probably would need another one for keeping
rxinv value.

Will it give us any memory reduction in comparison to the current code?

-- 
With Best Regards,
Andy Shevchenko