Received-SPF: pass (google.com: domain of linux-kernel+bounces-126379-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Precedence: bulk
MIME-Version: 1.0
References: <20240329205904.25002-1-ddrokosov@salutedevices.com> <20240329205904.25002-7-ddrokosov@salutedevices.com>
In-Reply-To: <20240329205904.25002-7-ddrokosov@salutedevices.com>
From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Date: Sun, 31 Mar 2024 23:40:13 +0200
Message-ID: <CAFBinCCC5KK-4_w41B-+ZJ3zdZckq_EDuAW+Kak2C0Ow8uuD6Q@mail.gmail.com>
Subject: Re: [PATCH v1 6/6] clk: meson: a1: add Amlogic A1 CPU clock
 controller driver
To: Dmitry Rokosov <ddrokosov@salutedevices.com>
Cc: neil.armstrong@linaro.org, jbrunet@baylibre.com, mturquette@baylibre.com, 
	sboyd@kernel.org, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, 
	khilman@baylibre.com, kernel@salutedevices.com, rockosov@gmail.com, 
	linux-amlogic@lists.infradead.org, linux-clk@vger.kernel.org, 
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, 
	linux-arm-kernel@lists.infradead.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Dmitry,

On Fri, Mar 29, 2024 at 9:59=E2=80=AFPM Dmitry Rokosov
<ddrokosov@salutedevices.com> wrote:
[...]
> +static struct clk_regmap cpu_fclk =3D {
> +       .data =3D &(struct clk_regmap_mux_data) {
> +               .offset =3D CPUCTRL_CLK_CTRL0,
> +               .mask =3D 0x1,
> +               .shift =3D 10,
> +       },
> +       .hw.init =3D &(struct clk_init_data) {
> +               .name =3D "cpu_fclk",
> +               .ops =3D &clk_regmap_mux_ops,
> +               .parent_hws =3D (const struct clk_hw *[]) {
> +                       &cpu_fsel0.hw,
> +                       &cpu_fsel1.hw,
Have you considered the CLK_SET_RATE_GATE flag for &cpu_fsel0.hw and
&cpu_fsel1.hw and then dropping the clock notifier below?
We use that approach with the Mali GPU clock on other SoCs, see for
example commit 8daeaea99caa ("clk: meson: meson8b: make the CCF use
the glitch-free mali mux").
It may differ from what Amlogic does in their BSP, but I don't think
that there's any harm (if it works in general) because CCF (common
clock framework) will set all clocks in the "inactive" tree and then
as a last step just change the mux (&cpu_fclk.hw). So at no point in
time will we get any other rate than a) the original CPU clock rate
before the rate change b) the new desired CPU clock rate. This is
because we have two symmetric clock trees.


Best regards,
Martin