Re: [PATCH v3 2/3] pinctrl: intel: refine ->irq_set_type() hook

[Andy Shevchenko]

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