Messages in this thread | | | From | Andy Shevchenko <> | Date | Sat, 9 Mar 2024 18:27:56 +0200 | Subject | Re: [PATCH v1 1/1] clk: fractional-divider: Move mask calculations out of lock |
| |
On Sat, Mar 9, 2024 at 9:19 AM Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote: > Le 03/03/2024 à 13:14, Andy Shevchenko a écrit :
..
> > @@ -195,14 +195,14 @@ static int clk_fd_set_rate(struct clk_hw *hw, unsigned long rate, > > n--; > > } > > > > + mmask = GENMASK(fd->mwidth - 1, 0) << fd->mshift; > > + nmask = GENMASK(fd->nwidth - 1, 0) << fd->nshift;
> if this is a hot path, you could maybe even compute:
It's not. set_rate() may be called only on disabled (and unprepared?) clocks, which makes it already a too slow operation.
> mask = ~(GENMASK(fd->mwidth - 1, 0) << fd->mshift | > GENMASK(fd->nwidth - 1, 0) << fd->nshift) > > unless gcc is smart enough to do it by itself. > > > if (fd->lock) > > spin_lock_irqsave(fd->lock, flags); > > else > > __acquire(fd->lock); > > > > - mmask = GENMASK(fd->mwidth - 1, 0) << fd->mshift; > > - nmask = GENMASK(fd->nwidth - 1, 0) << fd->nshift; > > - > > val = clk_fd_readl(fd); > > val &= ~(mmask | nmask); > > val &= mask; > > > val |= (m << fd->mshift) | (n << fd->nshift); > > and pre-compute "(m << fd->mshift) | (n << fd->nshift)" outside of the > lock too.
All of these sound to me as premature optimisations. I only wanted to get back to the status quo.
-- With Best Regards, Andy Shevchenko
| |