| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAMuHMdV+7cvwxGVYGUd_nV3sUZ60YWzsWr_Ec6RJToPttUfKRA@mail.gmail.com>
Date: Mon, 17 Nov 2025 14:28:12 +0100
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: "Lad, Prabhakar" <prabhakar.csengg@...il.com>
Cc: Michael Turquette <mturquette@...libre.com>, Stephen Boyd <sboyd@...nel.org>,
Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
Magnus Damm <magnus.damm@...il.com>, linux-renesas-soc@...r.kernel.org,
linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org, Biju Das <biju.das.jz@...renesas.com>,
Fabrizio Castro <fabrizio.castro.jz@...esas.com>,
Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
Subject: Re: [PATCH v2 5/5] clk: renesas: r9a09g077: Add xSPI core and module clocks
Hi Prabhakar,
On Mon, 10 Nov 2025 at 22:38, Lad, Prabhakar <prabhakar.csengg@...il.com> wrote:
> On Mon, Nov 10, 2025 at 1:48 PM Geert Uytterhoeven <geert@...ux-m68k.org> wrote:
> > On Tue, 28 Oct 2025 at 17:52, Prabhakar <prabhakar.csengg@...il.com> wrote:
> > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
> > >
> > > Add core clocks and module clock definitions required by the xSPI
> > > (Expanded SPI) IP on the R9A09G077 SoC.
> > >
> > > Define the new SCKCR fields FSELXSPI0/FSELXSPI1 and DIVSEL_XSPI0/1 and
> > > add two new core clocks XSPI_CLK0 and XSPI_CLK1. The xSPI block uses
> > > PCLKH as its bus clock (use as module clock parent) while the operation
> > > clock (XSPI_CLKn) is derived from PLL4. To support this arrangement
> > > provide mux/div selectors and divider tables for the supported
> > > XSPI operating rates.
> > >
> > > Add CLK_TYPE_RZT2H_FSELXSPI to implement a custom divider/mux clock
> > > where the determine_rate() callback enforces the hardware constraint:
> > > when the parent output is 600MHz only dividers 8 and 16 are valid,
> > > whereas for 800MHz operation the full divider set (6,8,16,32,64) may
> > > be used. The custom determine_rate() picks the best parent/divider pair
> > > to match the requested rate and programs the appropriate SCKCR fields.
> > >
> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
> > > ---
> > > v1->v2:
> > > - Added custom divider clock type for XSPI clocks to enforce hardware
> > > constraints on supported operating rates.
> > > --- a/drivers/clk/renesas/r9a09g077-cpg.c
> > > +++ b/drivers/clk/renesas/r9a09g077-cpg.c
> > > @@ -264,6 +305,116 @@ r9a09g077_cpg_mux_clk_register(struct device *dev,
> > > return clk_hw->clk;
> > > }
> > >
> > > +static int r9a09g077_cpg_fselxspi_determine_rate(struct clk_hw *hw,
> > > + struct clk_rate_request *req)
> > > +{
> > > + struct clk_divider *divider = to_clk_divider(hw);
> > > + unsigned long parent_rate, best = 0, now;
> > > + const struct clk_div_table *clkt;
> > > + unsigned long rate = req->rate;
> > > + int div = 0;
> > > +
> > > + if (!rate)
> > > + rate = 1;
> > > +
> > > + for (clkt = divider->table; clkt->div; clkt++) {
> > > + parent_rate = clk_hw_round_rate(req->best_parent_hw, rate * clkt->div);
> >
> > I had expected the use of some *_determinate_rate_*() helper, as the
> > parent can be changed to find a better clock rate?
> > Perhaps you should use a composite clock for that?
OK, so per your test results, the core clock code does try
various parents.
> >
> > > + /*
> > > + * DIVSELXSPIx supports 800MHz and 600MHz operation.
> > > + * When the parent_rate is 600MHz, only dividers of 8 and 16
> > > + * are supported otherwise dividers of 6, 8, 16, 32, 64 are supported.
> > > + * This check ensures that FSELXSPIx is set correctly.
> > > + */
> > > + if (parent_rate == DIVSELXSPI_RATE_600MHZ &&
> >
> > Does this actually work as expected? I doubt parent_rate is guaranteed
> > to be exactly 600 or 800 MHz, and expect it can differ slightly due
> > to rounding. Hence I would look at clk_fixed_factor.div instead.
> >
> With below diff, Ive got the below results for the various freqs
> requested where appropriate parent and divider clocks are picked.
>
> @@ -317,6 +317,7 @@ static int
> r9a09g077_cpg_fselxspi_determine_rate(struct clk_hw *hw,
>
> for (clkt = divider->table; clkt->div; clkt++) {
> parent_rate = clk_hw_round_rate(req->best_parent_hw,
> rate * clkt->div);
> + pr_err("parent_rate=%lu, req-rate=%lu div=%u\n",
> parent_rate, rate, clkt->div);
> /*
> * DIVSELXSPIx supports 800MHz and 600MHz operation.
> * When the parent_rate is 600MHz, only dividers of 8 and 16
> Case 2# assigned-clock-rates = <75000000>;
> [ 12.288507] parent_rate=800000000, req-rate=75000000 div=64
> [ 12.310528] parent_rate=800000000, req-rate=75000000 div=32
> [ 12.318426] parent_rate=800000000, req-rate=75000000 div=16
> [ 12.326361] parent_rate=600000000, req-rate=75000000 div=8
> [ 12.341540] parent_rate=0, req-rate=75000000 div=6
> [ 12.347546] parent_rate=800000000, req-rate=75000000 div=64
> [ 12.357593] parent_rate=800000000, req-rate=75000000 div=32
> [ 12.367148] parent_rate=800000000, req-rate=75000000 div=16
> [ 12.418871] parent_rate=600000000, req-rate=75000000 div=8
> [ 12.433560] parent_rate=0, req-rate=75000000 div=6
[...]
Thanks for checking!
> Looking at the logs I think I could optimize the code to continue when
> parent_rate == 0
Do you know why it gets called with parent_rate == 0?
> Based on the above logs, would you prefer me to represent it as a
> composite clock?
Given the core code does try the various parent clocks, there is
no need to model it as a composite clock.
However, I still think you should look at the parent's divider value
(clk_fixed_factor.div) instead of at the actual clock rate, as that
may not be 600 or 800 MHz exactly (e.g. when underclocking the SoC
on a custom board using a 24 instead of a 25 MHz crystal).
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Powered by blists - more mailing lists