Received-SPF: pass (google.com: domain of linux-kernel+bounces-97951-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
MIME-Version: 1.0
References: <20240303121410.240761-1-andy.shevchenko@gmail.com> <63c898e4-fedf-44eb-8887-ffda4a81e1e6@wanadoo.fr>
In-Reply-To: <63c898e4-fedf-44eb-8887-ffda4a81e1e6@wanadoo.fr>
From: Andy Shevchenko <andy.shevchenko@gmail.com>
Date: Sat, 9 Mar 2024 18:27:56 +0200
Message-ID: <CAHp75VfDNrDcLbBwHpn52qFEH=wFdnG1sx2jrU25oYJQ6OHGHA@mail.gmail.com>
Subject: Re: [PATCH v1 1/1] clk: fractional-divider: Move mask calculations
 out of lock
To: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Cc: linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org, 
	Michael Turquette <mturquette@baylibre.com>, Stephen Boyd <sboyd@kernel.org>, 
	Heiko Stuebner <heiko@sntech.de>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Sat, Mar 9, 2024 at 9:19=E2=80=AFAM Christophe JAILLET
<christophe.jaillet@wanadoo.fr> wrote:
> Le 03/03/2024 =C3=A0 13:14, Andy Shevchenko a =C3=A9crit :

..

> > @@ -195,14 +195,14 @@ static int clk_fd_set_rate(struct clk_hw *hw, uns=
igned long rate,
> >               n--;
> >       }
> >
> > +     mmask =3D GENMASK(fd->mwidth - 1, 0) << fd->mshift;
> > +     nmask =3D 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 =3D ~(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 =3D GENMASK(fd->mwidth - 1, 0) << fd->mshift;
> > -     nmask =3D GENMASK(fd->nwidth - 1, 0) << fd->nshift;
> > -
> >       val =3D clk_fd_readl(fd);
> >       val &=3D ~(mmask | nmask);
>
>     val &=3D mask;
>
> >       val |=3D (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.

--=20
With Best Regards,
Andy Shevchenko