This series implements minor optimizations for Intel pinctrl driver.
The numbers are as tested with gcc 7.5.0 and may vary with newer versions.
Raag Jadav (4):
pinctrl: intel: optimize set_mux hook
pinctrl: intel: optimize irq_set_type hook
pinctrl: intel: simplify exit path of set_mux hook
pinctrl: intel: simplify exit path of gpio_request_enable hook
drivers/pinctrl/intel/pinctrl-intel.c | 63 +++++++++++++++------------
1 file changed, 35 insertions(+), 28 deletions(-)
--
2.17.1
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%
Signed-off-by: Raag Jadav <[email protected]>
---
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
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 <[email protected]>
> ---
> 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
> 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?
Forgot to add. This is with default -O2.
Is it a good idea to mention it?
> 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.
On Thu, Jun 08, 2023 at 10:20:58AM +0000, Jadav, Raag wrote:
> > 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?
>
> Forgot to add. This is with default -O2.
> Is it a good idea to mention it?
Yes, I think it is.
On Thu, Jun 08, 2023 at 12:30:13PM +0530, Raag Jadav wrote:
> This series implements minor optimizations for Intel pinctrl driver.
>
> The numbers are as tested with gcc 7.5.0 and may vary with newer versions.
Please, follow what Mika commented on on both series and issue v2 of each
of them.
--
With Best Regards,
Andy Shevchenko