From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.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@bp.renesas.com>
---
v1->v2:
- Added custom divider clock type for XSPI clocks to enforce hardware
constraints on supported operating rates.
---
drivers/clk/renesas/r9a09g077-cpg.c | 155 +++++++++++++++++++++++++++-
1 file changed, 154 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/renesas/r9a09g077-cpg.c b/drivers/clk/renesas/r9a09g077-cpg.c
index b46167d42084..678dc36461c0 100644
--- a/drivers/clk/renesas/r9a09g077-cpg.c
+++ b/drivers/clk/renesas/r9a09g077-cpg.c
@@ -11,6 +11,8 @@
#include <linux/device.h>
#include <linux/init.h>
#include <linux/kernel.h>
+#include <linux/math.h>
+#include <linux/types.h>
#include <dt-bindings/clock/renesas,r9a09g077-cpg-mssr.h>
#include <dt-bindings/clock/renesas,r9a09g087-cpg-mssr.h>
@@ -54,12 +56,19 @@
#define DIVSCI3ASYNC CONF_PACK(SCKCR3, 12, 2)
#define DIVSCI4ASYNC CONF_PACK(SCKCR3, 14, 2)
+#define FSELXSPI0 CONF_PACK(SCKCR, 0, 3)
+#define FSELXSPI1 CONF_PACK(SCKCR, 8, 3)
+#define DIVSEL_XSPI0 CONF_PACK(SCKCR, 6, 1)
+#define DIVSEL_XSPI1 CONF_PACK(SCKCR, 14, 1)
#define SEL_PLL CONF_PACK(SCKCR, 22, 1)
+#define DIVSELXSPI_RATE_600MHZ 600000000UL
+#define DIVSELXSPI_RATE_800MHZ 800000000UL
enum rzt2h_clk_types {
CLK_TYPE_RZT2H_DIV = CLK_TYPE_CUSTOM, /* Clock with divider */
CLK_TYPE_RZT2H_MUX, /* Clock with clock source selector */
+ CLK_TYPE_RZT2H_FSELXSPI,
};
#define DEF_DIV(_name, _id, _parent, _conf, _dtable) \
@@ -69,10 +78,13 @@ enum rzt2h_clk_types {
DEF_TYPE(_name, _id, CLK_TYPE_RZT2H_MUX, .conf = _conf, \
.parent_names = _parent_names, .num_parents = _num_parents, \
.flag = 0, .mux_flags = _mux_flags)
+#define DEF_DIV_FSELXSPI(_name, _id, _parent, _conf, _dtable) \
+ DEF_TYPE(_name, _id, CLK_TYPE_RZT2H_FSELXSPI, .conf = _conf, \
+ .parent = _parent, .dtable = _dtable, .flag = 0)
enum clk_ids {
/* Core Clock Outputs exported to DT */
- LAST_DT_CORE_CLK = R9A09G077_ETCLKE,
+ LAST_DT_CORE_CLK = R9A09G077_XSPI_CLK1,
/* External Input Clocks */
CLK_EXTAL,
@@ -88,12 +100,16 @@ enum clk_ids {
CLK_SEL_CLK_PLL2,
CLK_SEL_CLK_PLL4,
CLK_PLL4D1,
+ CLK_PLL4D1_DIV3,
+ CLK_PLL4D1_DIV4,
CLK_SCI0ASYNC,
CLK_SCI1ASYNC,
CLK_SCI2ASYNC,
CLK_SCI3ASYNC,
CLK_SCI4ASYNC,
CLK_SCI5ASYNC,
+ CLK_DIVSELXSPI0_SCKCR,
+ CLK_DIVSELXSPI1_SCKCR,
/* Module Clocks */
MOD_CLK_BASE,
@@ -105,6 +121,15 @@ static const struct clk_div_table dtable_1_2[] = {
{0, 0},
};
+static const struct clk_div_table dtable_6_8_16_32_64[] = {
+ {6, 64},
+ {5, 32},
+ {4, 16},
+ {3, 8},
+ {2, 6},
+ {0, 0},
+};
+
static const struct clk_div_table dtable_24_25_30_32[] = {
{0, 32},
{1, 30},
@@ -119,6 +144,7 @@ static const char * const sel_clk_pll0[] = { ".loco", ".pll0" };
static const char * const sel_clk_pll1[] = { ".loco", ".pll1" };
static const char * const sel_clk_pll2[] = { ".loco", ".pll2" };
static const char * const sel_clk_pll4[] = { ".loco", ".pll4" };
+static const char * const sel_clk_pll4d1_div3_div4[] = { ".pll4d1_div3", ".pll4d1_div4" };
static const struct cpg_core_clk r9a09g077_core_clks[] __initconst = {
/* External Clock Inputs */
@@ -154,6 +180,15 @@ static const struct cpg_core_clk r9a09g077_core_clks[] __initconst = {
DEF_DIV(".sci5async", CLK_SCI5ASYNC, CLK_PLL4D1, DIVSCI5ASYNC,
dtable_24_25_30_32),
+ DEF_FIXED(".pll4d1_div3", CLK_PLL4D1_DIV3, CLK_PLL4D1, 3, 1),
+ DEF_FIXED(".pll4d1_div4", CLK_PLL4D1_DIV4, CLK_PLL4D1, 4, 1),
+ DEF_MUX(".divselxspi0", CLK_DIVSELXSPI0_SCKCR, DIVSEL_XSPI0,
+ sel_clk_pll4d1_div3_div4,
+ ARRAY_SIZE(sel_clk_pll4d1_div3_div4), 0),
+ DEF_MUX(".divselxspi1", CLK_DIVSELXSPI1_SCKCR, DIVSEL_XSPI1,
+ sel_clk_pll4d1_div3_div4,
+ ARRAY_SIZE(sel_clk_pll4d1_div3_div4), 0),
+
/* Core output clk */
DEF_DIV("CA55C0", R9A09G077_CLK_CA55C0, CLK_SEL_CLK_PLL0, DIVCA55C0,
dtable_1_2),
@@ -178,9 +213,15 @@ static const struct cpg_core_clk r9a09g077_core_clks[] __initconst = {
DEF_FIXED("ETCLKC", R9A09G077_ETCLKC, CLK_SEL_CLK_PLL1, 10, 1),
DEF_FIXED("ETCLKD", R9A09G077_ETCLKD, CLK_SEL_CLK_PLL1, 20, 1),
DEF_FIXED("ETCLKE", R9A09G077_ETCLKE, CLK_SEL_CLK_PLL1, 40, 1),
+ DEF_DIV_FSELXSPI("XSPI_CLK0", R9A09G077_XSPI_CLK0, CLK_DIVSELXSPI0_SCKCR,
+ FSELXSPI0, dtable_6_8_16_32_64),
+ DEF_DIV_FSELXSPI("XSPI_CLK1", R9A09G077_XSPI_CLK1, CLK_DIVSELXSPI1_SCKCR,
+ FSELXSPI1, dtable_6_8_16_32_64),
};
static const struct mssr_mod_clk r9a09g077_mod_clks[] __initconst = {
+ DEF_MOD("xspi0", 4, R9A09G077_CLK_PCLKH),
+ DEF_MOD("xspi1", 5, R9A09G077_CLK_PCLKH),
DEF_MOD("sci0fck", 8, CLK_SCI0ASYNC),
DEF_MOD("sci1fck", 9, CLK_SCI1ASYNC),
DEF_MOD("sci2fck", 10, CLK_SCI2ASYNC),
@@ -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);
+ /*
+ * 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 &&
+ (clkt->div != 8 && clkt->div != 16))
+ continue;
+ now = DIV_ROUND_UP_ULL((u64)parent_rate, clkt->div);
+ if (abs(rate - now) < abs(rate - best)) {
+ div = clkt->div;
+ best = now;
+ req->best_parent_rate = parent_rate;
+ }
+ }
+
+ if (!div) {
+ u8 maxdiv = 0;
+
+ req->best_parent_rate = clk_hw_round_rate(req->best_parent_hw, 1);
+ /*
+ * If DIVSELXSPIx is set to 800MHz set the maximum divider
+ * or else fall back to divider of 16 which is a maximum
+ * supported divider for 600MHz operation.
+ */
+ if (req->best_parent_rate == DIVSELXSPI_RATE_800MHZ) {
+ for (clkt = divider->table; clkt->div; clkt++) {
+ if (clkt->div > maxdiv)
+ maxdiv = clkt->div;
+ }
+ div = maxdiv;
+ } else {
+ div = 16;
+ }
+ }
+
+ req->rate = DIV_ROUND_UP_ULL((u64)req->best_parent_rate, div);
+
+ return 0;
+}
+
+static struct clk * __init
+r9a09g077_cpg_fselxspi_div_clk_register(struct device *dev,
+ const struct cpg_core_clk *core,
+ void __iomem *addr,
+ struct cpg_mssr_pub *pub)
+{
+ static struct clk_ops *xspi_div_ops;
+ struct clk_init_data init = {};
+ const struct clk *parent;
+ const char *parent_name;
+ struct clk_divider *div;
+ struct clk_hw *hw;
+ int ret;
+
+ parent = pub->clks[core->parent];
+ if (IS_ERR(parent))
+ return ERR_CAST(parent);
+
+ div = devm_kzalloc(dev, sizeof(*div), GFP_KERNEL);
+ if (!div)
+ return ERR_PTR(-ENOMEM);
+
+ if (!xspi_div_ops) {
+ xspi_div_ops = devm_kzalloc(dev, sizeof(*xspi_div_ops), GFP_KERNEL);
+ if (!xspi_div_ops)
+ return ERR_PTR(-ENOMEM);
+ memcpy(xspi_div_ops, &clk_divider_ops,
+ sizeof(const struct clk_ops));
+ xspi_div_ops->determine_rate = r9a09g077_cpg_fselxspi_determine_rate;
+ }
+
+ parent_name = __clk_get_name(parent);
+ init.name = core->name;
+ init.ops = xspi_div_ops;
+ init.flags = CLK_SET_RATE_PARENT;
+ init.parent_names = &parent_name;
+ init.num_parents = 1;
+
+ div->reg = addr;
+ div->shift = GET_SHIFT(core->conf);
+ div->width = GET_WIDTH(core->conf);
+ div->flags = core->flag;
+ div->lock = &pub->rmw_lock;
+ div->hw.init = &init;
+ div->table = core->dtable;
+
+ hw = &div->hw;
+ ret = devm_clk_hw_register(dev, hw);
+ if (ret)
+ return ERR_PTR(ret);
+
+ return hw->clk;
+}
+
static struct clk * __init
r9a09g077_cpg_clk_register(struct device *dev, const struct cpg_core_clk *core,
const struct cpg_mssr_info *info,
@@ -278,6 +429,8 @@ r9a09g077_cpg_clk_register(struct device *dev, const struct cpg_core_clk *core,
return r9a09g077_cpg_div_clk_register(dev, core, addr, pub);
case CLK_TYPE_RZT2H_MUX:
return r9a09g077_cpg_mux_clk_register(dev, core, addr, pub);
+ case CLK_TYPE_RZT2H_FSELXSPI:
+ return r9a09g077_cpg_fselxspi_div_clk_register(dev, core, addr, pub);
default:
return ERR_PTR(-EINVAL);
}
--
2.43.0Hi Prabhakar,
On Tue, 28 Oct 2025 at 17:52, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.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@bp.renesas.com>
> ---
> v1->v2:
> - Added custom divider clock type for XSPI clocks to enforce hardware
> constraints on supported operating rates.
Thanks for the update!
> --- a/drivers/clk/renesas/r9a09g077-cpg.c
> +++ b/drivers/clk/renesas/r9a09g077-cpg.c
> @@ -54,12 +56,19 @@
> #define DIVSCI3ASYNC CONF_PACK(SCKCR3, 12, 2)
> #define DIVSCI4ASYNC CONF_PACK(SCKCR3, 14, 2)
>
> +#define FSELXSPI0 CONF_PACK(SCKCR, 0, 3)
> +#define FSELXSPI1 CONF_PACK(SCKCR, 8, 3)
> +#define DIVSEL_XSPI0 CONF_PACK(SCKCR, 6, 1)
> +#define DIVSEL_XSPI1 CONF_PACK(SCKCR, 14, 1)
> #define SEL_PLL CONF_PACK(SCKCR, 22, 1)
>
> +#define DIVSELXSPI_RATE_600MHZ 600000000UL
> +#define DIVSELXSPI_RATE_800MHZ 800000000UL
I find it a bit weird that the name of the define includes its value.
Perhaps just use "600 * MEGA" resp. "800 * MEGA" in the code instead?
But see below...
> @@ -154,6 +180,15 @@ static const struct cpg_core_clk r9a09g077_core_clks[] __initconst = {
> DEF_DIV(".sci5async", CLK_SCI5ASYNC, CLK_PLL4D1, DIVSCI5ASYNC,
> dtable_24_25_30_32),
>
> + DEF_FIXED(".pll4d1_div3", CLK_PLL4D1_DIV3, CLK_PLL4D1, 3, 1),
> + DEF_FIXED(".pll4d1_div4", CLK_PLL4D1_DIV4, CLK_PLL4D1, 4, 1),
Please move these two just below the existing entry for ".pll4d1".
> + DEF_MUX(".divselxspi0", CLK_DIVSELXSPI0_SCKCR, DIVSEL_XSPI0,
> + sel_clk_pll4d1_div3_div4,
> + ARRAY_SIZE(sel_clk_pll4d1_div3_div4), 0),
> + DEF_MUX(".divselxspi1", CLK_DIVSELXSPI1_SCKCR, DIVSEL_XSPI1,
> + sel_clk_pll4d1_div3_div4,
> + ARRAY_SIZE(sel_clk_pll4d1_div3_div4), 0),
> +
> /* Core output clk */
> DEF_DIV("CA55C0", R9A09G077_CLK_CA55C0, CLK_SEL_CLK_PLL0, DIVCA55C0,
> dtable_1_2),
> @@ -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;
unsigned int
> +
> + 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?
> + /*
> + * 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.
> + (clkt->div != 8 && clkt->div != 16))
> + continue;
> + now = DIV_ROUND_UP_ULL((u64)parent_rate, clkt->div);
No need to cast to u64 (DIV_ROUND_*_ULL() handle this internally).
> + if (abs(rate - now) < abs(rate - best)) {
> + div = clkt->div;
> + best = now;
> + req->best_parent_rate = parent_rate;
> + }
> + }
> +
> + if (!div) {
> + u8 maxdiv = 0;
> +
> + req->best_parent_rate = clk_hw_round_rate(req->best_parent_hw, 1);
> + /*
> + * If DIVSELXSPIx is set to 800MHz set the maximum divider
> + * or else fall back to divider of 16 which is a maximum
> + * supported divider for 600MHz operation.
> + */
> + if (req->best_parent_rate == DIVSELXSPI_RATE_800MHZ) {
> + for (clkt = divider->table; clkt->div; clkt++) {
> + if (clkt->div > maxdiv)
> + maxdiv = clkt->div;
> + }
> + div = maxdiv;
Why not hardcode the divider, like in the else branch?
> + } else {
> + div = 16;
> + }
> + }
> +
> + req->rate = DIV_ROUND_UP_ULL((u64)req->best_parent_rate, div);
No need to cast to u64.
> +
> + return 0;
> +}
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-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
Hi Geert,
Thank you for the review.
On Mon, Nov 10, 2025 at 1:48 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Tue, 28 Oct 2025 at 17:52, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.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@bp.renesas.com>
> > ---
> > v1->v2:
> > - Added custom divider clock type for XSPI clocks to enforce hardware
> > constraints on supported operating rates.
>
> Thanks for the update!
>
> > --- a/drivers/clk/renesas/r9a09g077-cpg.c
> > +++ b/drivers/clk/renesas/r9a09g077-cpg.c
>
> > @@ -54,12 +56,19 @@
> > #define DIVSCI3ASYNC CONF_PACK(SCKCR3, 12, 2)
> > #define DIVSCI4ASYNC CONF_PACK(SCKCR3, 14, 2)
> >
> > +#define FSELXSPI0 CONF_PACK(SCKCR, 0, 3)
> > +#define FSELXSPI1 CONF_PACK(SCKCR, 8, 3)
> > +#define DIVSEL_XSPI0 CONF_PACK(SCKCR, 6, 1)
> > +#define DIVSEL_XSPI1 CONF_PACK(SCKCR, 14, 1)
> > #define SEL_PLL CONF_PACK(SCKCR, 22, 1)
> >
> > +#define DIVSELXSPI_RATE_600MHZ 600000000UL
> > +#define DIVSELXSPI_RATE_800MHZ 800000000UL
>
> I find it a bit weird that the name of the define includes its value.
> Perhaps just use "600 * MEGA" resp. "800 * MEGA" in the code instead?
OK.
> But see below...
>
> > @@ -154,6 +180,15 @@ static const struct cpg_core_clk r9a09g077_core_clks[] __initconst = {
> > DEF_DIV(".sci5async", CLK_SCI5ASYNC, CLK_PLL4D1, DIVSCI5ASYNC,
> > dtable_24_25_30_32),
> >
> > + DEF_FIXED(".pll4d1_div3", CLK_PLL4D1_DIV3, CLK_PLL4D1, 3, 1),
> > + DEF_FIXED(".pll4d1_div4", CLK_PLL4D1_DIV4, CLK_PLL4D1, 4, 1),
>
> Please move these two just below the existing entry for ".pll4d1".
>
Ok, I will move it below .pll4d1
> > + DEF_MUX(".divselxspi0", CLK_DIVSELXSPI0_SCKCR, DIVSEL_XSPI0,
> > + sel_clk_pll4d1_div3_div4,
> > + ARRAY_SIZE(sel_clk_pll4d1_div3_div4), 0),
> > + DEF_MUX(".divselxspi1", CLK_DIVSELXSPI1_SCKCR, DIVSEL_XSPI1,
> > + sel_clk_pll4d1_div3_div4,
> > + ARRAY_SIZE(sel_clk_pll4d1_div3_div4), 0),
> > +
> > /* Core output clk */
> > DEF_DIV("CA55C0", R9A09G077_CLK_CA55C0, CLK_SEL_CLK_PLL0, DIVCA55C0,
> > dtable_1_2),
>
> > @@ -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;
>
> unsigned int
>
Ok.
> > +
> > + 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?
>
> > + /*
> > + * 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
Logs:
---------
Case 0# assigned-clock-rates = <133333334>;
[ 15.419300] parent_rate=800000000, req-rate=133333334 div=64
[ 15.437698] parent_rate=800000000, req-rate=133333334 div=32
[ 15.455224] parent_rate=800000000, req-rate=133333334 div=16
[ 15.501291] parent_rate=800000000, req-rate=133333334 div=8
[ 15.507801] parent_rate=800000000, req-rate=133333334 div=6
[ 15.519221] parent_rate=800000000, req-rate=133333334 div=64
[ 15.525789] parent_rate=800000000, req-rate=133333334 div=32
[ 15.549625] parent_rate=800000000, req-rate=133333334 div=16
[ 15.556120] parent_rate=800000000, req-rate=133333334 div=8
[ 15.564110] parent_rate=800000000, req-rate=133333334 div=6
root@rzt2h-evk:~# cat /sys/kernel/debug/clk/clk_summary | grep -e
"xspi0" -e "XSPI_CLK0" -e "divselxspi0"
.divselxspi0 1 1 0
800000000 0 0 50000 Y deviceless
no_connection_id
XSPI_CLK0 1 1 0
133333334 0 0 50000 Y
801c0000.spi spi
xspi0 0 1 0
250000000 0 0 50000 N deviceless
of_clk_get_from_provider
root@rzt2h-evk:~#
Case 1# assigned-clock-rates = <100000000>;
[ 15.496291] parent_rate=800000000, req-rate=100000000 div=64
[ 15.510068] parent_rate=800000000, req-rate=100000000 div=32
[ 15.517142] parent_rate=800000000, req-rate=100000000 div=16
[ 15.524047] parent_rate=800000000, req-rate=100000000 div=8
[ 15.533174] parent_rate=600000000, req-rate=100000000 div=6
[ 15.540096] parent_rate=800000000, req-rate=100000000 div=64
[ 15.548135] parent_rate=800000000, req-rate=100000000 div=32
[ 15.555119] parent_rate=800000000, req-rate=100000000 div=16
[ 15.562395] parent_rate=800000000, req-rate=100000000 div=8
[ 15.573521] parent_rate=600000000, req-rate=100000000 div=6
root@rzt2h-evk:~# cat /sys/kernel/debug/clk/clk_summary | grep -e
"xspi0" -e "XSPI_CLK0" -e "divselxspi0"
.divselxspi0 1 1 0
800000000 0 0 50000 Y deviceless
no_connection_id
XSPI_CLK0 1 1 0
100000000 0 0 50000 Y
801c0000.spi spi
xspi0 0 1 0
250000000 0 0 50000 N deviceless
of_clk_get_from_provider
root@rzt2h-evk:~#
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
root@rzt2h-evk:~# cat /sys/kernel/debug/clk/clk_summary | grep -e
"xspi0" -e "XSPI_CLK0" -e "divselxspi0"
.divselxspi0 1 1 0
600000000 0 0 50000 Y deviceless
no_connection_id
XSPI_CLK0 1 1 0
75000000 0 0 50000 Y
801c0000.spi spi
xspi0 0 1 0
250000000 0 0 50000 N deviceless
of_clk_get_from_provider
root@rzt2h-evk:~#
Case 3# assigned-clock-rates = <50000000>;
[ 15.240214] parent_rate=800000000, req-rate=50000000 div=64
[ 15.253498] parent_rate=800000000, req-rate=50000000 div=32
[ 15.261521] parent_rate=800000000, req-rate=50000000 div=16
[ 15.272941] parent_rate=0, req-rate=50000000 div=8
[ 15.280532] parent_rate=0, req-rate=50000000 div=6
[ 15.289979] parent_rate=800000000, req-rate=50000000 div=64
[ 15.298745] parent_rate=800000000, req-rate=50000000 div=32
[ 15.309879] parent_rate=800000000, req-rate=50000000 div=16
[ 15.319881] parent_rate=0, req-rate=50000000 div=8
[ 15.327977] parent_rate=0, req-rate=50000000 div=6
root@rzt2h-evk:~# cat /sys/kernel/debug/clk/clk_summary | grep -e
"xspi0" -e "XSPI_CLK0" -e "divselxspi0"
.divselxspi0 1 1 0
800000000 0 0 50000 Y deviceless
no_connection_id
XSPI_CLK0 1 1 0
50000000 0 0 50000 Y
801c0000.spi spi
xspi0 0 1 0
250000000 0 0 50000 N deviceless
of_clk_get_from_provider
root@rzt2h-evk:~#
Case 4# assigned-clock-rates = <37500000>;
[ 71.710064] parent_rate=800000000, req-rate=37500000 div=64
[ 71.718567] parent_rate=800000000, req-rate=37500000 div=32
[ 71.725137] parent_rate=600000000, req-rate=37500000 div=16
[ 71.731550] parent_rate=0, req-rate=37500000 div=8
[ 71.740622] parent_rate=0, req-rate=37500000 div=6
[ 71.746376] parent_rate=800000000, req-rate=37500000 div=64
[ 71.752887] parent_rate=800000000, req-rate=37500000 div=32
[ 71.767422] parent_rate=600000000, req-rate=37500000 div=16
[ 71.778671] parent_rate=0, req-rate=37500000 div=8
[ 71.790895] parent_rate=0, req-rate=37500000 div=6
root@rzt2h-evk:~# cat /sys/kernel/debug/clk/clk_summary | grep -e
"xspi0" -e "XSPI_CLK0" -e "divselxspi0"
.divselxspi0 1 1 0
600000000 0 0 50000 Y deviceless
no_connection_id
XSPI_CLK0 1 1 0
37500000 0 0 50000 Y
801c0000.spi spi
xspi0 0 1 0
250000000 0 0 50000 N deviceless
of_clk_get_from_provider
root@rzt2h-evk:~#
Case 5# assigned-clock-rates = <25000000>;
[ 12.411660] parent_rate=800000000, req-rate=25000000 div=64
[ 12.429285] parent_rate=800000000, req-rate=25000000 div=32
[ 12.436144] parent_rate=0, req-rate=25000000 div=16
[ 12.448110] parent_rate=0, req-rate=25000000 div=8
[ 12.458785] parent_rate=0, req-rate=25000000 div=6
[ 12.465401] parent_rate=800000000, req-rate=25000000 div=64
[ 12.482547] parent_rate=800000000, req-rate=25000000 div=32
[ 12.497126] parent_rate=0, req-rate=25000000 div=16
[ 12.509619] parent_rate=0, req-rate=25000000 div=8
[ 12.518212] parent_rate=0, req-rate=25000000 div=6
root@rzt2h-evk:~# cat /sys/kernel/debug/clk/clk_summary | grep -e
"xspi0" -e "XSPI_CLK0" -e "divselxspi0"
.divselxspi0 1 1 0
800000000 0 0 50000 Y deviceless
no_connection_id
XSPI_CLK0 1 1 0
25000000 0 0 50000 Y
801c0000.spi spi
xspi0 0 1 0
250000000 0 0 50000 N deviceless
of_clk_get_from_provider
root@rzt2h-evk:~#
Case 6# assigned-clock-rates = <12500000>;
[ 87.409877] parent_rate=800000000, req-rate=12500000 div=64
[ 87.470663] parent_rate=0, req-rate=12500000 div=32
[ 87.485940] parent_rate=0, req-rate=12500000 div=16
[ 87.492760] parent_rate=0, req-rate=12500000 div=8
[ 87.498313] parent_rate=0, req-rate=12500000 div=6
root@rzt2h-evk:~# cat /sys/kernel/debug/clk/clk_summary | grep -e
"xspi0" -e "XSPI_CLK0" -e "divselxspi0"
.divselxspi0 1 1 0
800000000 0 0 50000 Y deviceless
no_connection_id
XSPI_CLK0 1 1 0
12500000 0 0 50000 Y
801c0000.spi spi
xspi0 0 1 0
250000000 0 0 50000 N deviceless
of_clk_get_from_provider
root@rzt2h-evk:~#
Looking at the logs I think I could optimize the code to continue when
parent_rate == 0
Based on the above logs, would you prefer me to represent it as a
composite clock?
> > + (clkt->div != 8 && clkt->div != 16))
> > + continue;
> > + now = DIV_ROUND_UP_ULL((u64)parent_rate, clkt->div);
>
> No need to cast to u64 (DIV_ROUND_*_ULL() handle this internally).
>
> > + if (abs(rate - now) < abs(rate - best)) {
> > + div = clkt->div;
> > + best = now;
> > + req->best_parent_rate = parent_rate;
> > + }
> > + }
> > +
> > + if (!div) {
> > + u8 maxdiv = 0;
> > +
> > + req->best_parent_rate = clk_hw_round_rate(req->best_parent_hw, 1);
> > + /*
> > + * If DIVSELXSPIx is set to 800MHz set the maximum divider
> > + * or else fall back to divider of 16 which is a maximum
> > + * supported divider for 600MHz operation.
> > + */
> > + if (req->best_parent_rate == DIVSELXSPI_RATE_800MHZ) {
> > + for (clkt = divider->table; clkt->div; clkt++) {
> > + if (clkt->div > maxdiv)
> > + maxdiv = clkt->div;
> > + }
> > + div = maxdiv;
>
> Why not hardcode the divider, like in the else branch?
>
Agreed.
> > + } else {
> > + div = 16;
> > + }
> > + }
> > +
> > + req->rate = DIV_ROUND_UP_ULL((u64)req->best_parent_rate, div);
>
> No need to cast to u64.
>
Ok.
Cheers,
Prabhakar
Hi Geert,
On Mon, Nov 10, 2025 at 9:38 PM Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
>
> Hi Geert,
>
> Thank you for the review.
>
> On Mon, Nov 10, 2025 at 1:48 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> >
> > Hi Prabhakar,
> >
> > On Tue, 28 Oct 2025 at 17:52, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.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@bp.renesas.com>
> > > ---
> > > v1->v2:
> > > - Added custom divider clock type for XSPI clocks to enforce hardware
> > > constraints on supported operating rates.
> >
> > Thanks for the update!
> >
> > > --- a/drivers/clk/renesas/r9a09g077-cpg.c
> > > +++ b/drivers/clk/renesas/r9a09g077-cpg.c
> >
> > > @@ -54,12 +56,19 @@
> > > #define DIVSCI3ASYNC CONF_PACK(SCKCR3, 12, 2)
> > > #define DIVSCI4ASYNC CONF_PACK(SCKCR3, 14, 2)
> > >
> > > +#define FSELXSPI0 CONF_PACK(SCKCR, 0, 3)
> > > +#define FSELXSPI1 CONF_PACK(SCKCR, 8, 3)
> > > +#define DIVSEL_XSPI0 CONF_PACK(SCKCR, 6, 1)
> > > +#define DIVSEL_XSPI1 CONF_PACK(SCKCR, 14, 1)
> > > #define SEL_PLL CONF_PACK(SCKCR, 22, 1)
> > >
> > > +#define DIVSELXSPI_RATE_600MHZ 600000000UL
> > > +#define DIVSELXSPI_RATE_800MHZ 800000000UL
> >
> > I find it a bit weird that the name of the define includes its value.
> > Perhaps just use "600 * MEGA" resp. "800 * MEGA" in the code instead?
> OK.
>
> > But see below...
> >
> > > @@ -154,6 +180,15 @@ static const struct cpg_core_clk r9a09g077_core_clks[] __initconst = {
> > > DEF_DIV(".sci5async", CLK_SCI5ASYNC, CLK_PLL4D1, DIVSCI5ASYNC,
> > > dtable_24_25_30_32),
> > >
> > > + DEF_FIXED(".pll4d1_div3", CLK_PLL4D1_DIV3, CLK_PLL4D1, 3, 1),
> > > + DEF_FIXED(".pll4d1_div4", CLK_PLL4D1_DIV4, CLK_PLL4D1, 4, 1),
> >
> > Please move these two just below the existing entry for ".pll4d1".
> >
> Ok, I will move it below .pll4d1
>
> > > + DEF_MUX(".divselxspi0", CLK_DIVSELXSPI0_SCKCR, DIVSEL_XSPI0,
> > > + sel_clk_pll4d1_div3_div4,
> > > + ARRAY_SIZE(sel_clk_pll4d1_div3_div4), 0),
> > > + DEF_MUX(".divselxspi1", CLK_DIVSELXSPI1_SCKCR, DIVSEL_XSPI1,
> > > + sel_clk_pll4d1_div3_div4,
> > > + ARRAY_SIZE(sel_clk_pll4d1_div3_div4), 0),
> > > +
> > > /* Core output clk */
> > > DEF_DIV("CA55C0", R9A09G077_CLK_CA55C0, CLK_SEL_CLK_PLL0, DIVCA55C0,
> > > dtable_1_2),
> >
> > > @@ -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;
> >
> > unsigned int
> >
> Ok.
>
> > > +
> > > + 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?
> >
> > > + /*
> > > + * 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
>
> Logs:
> ---------
>
> Case 0# assigned-clock-rates = <133333334>;
> [ 15.419300] parent_rate=800000000, req-rate=133333334 div=64
> [ 15.437698] parent_rate=800000000, req-rate=133333334 div=32
> [ 15.455224] parent_rate=800000000, req-rate=133333334 div=16
> [ 15.501291] parent_rate=800000000, req-rate=133333334 div=8
> [ 15.507801] parent_rate=800000000, req-rate=133333334 div=6
> [ 15.519221] parent_rate=800000000, req-rate=133333334 div=64
> [ 15.525789] parent_rate=800000000, req-rate=133333334 div=32
> [ 15.549625] parent_rate=800000000, req-rate=133333334 div=16
> [ 15.556120] parent_rate=800000000, req-rate=133333334 div=8
> [ 15.564110] parent_rate=800000000, req-rate=133333334 div=6
>
> root@rzt2h-evk:~# cat /sys/kernel/debug/clk/clk_summary | grep -e
> "xspi0" -e "XSPI_CLK0" -e "divselxspi0"
> .divselxspi0 1 1 0
> 800000000 0 0 50000 Y deviceless
> no_connection_id
> XSPI_CLK0 1 1 0
> 133333334 0 0 50000 Y
> 801c0000.spi spi
> xspi0 0 1 0
> 250000000 0 0 50000 N deviceless
> of_clk_get_from_provider
> root@rzt2h-evk:~#
>
> Case 1# assigned-clock-rates = <100000000>;
> [ 15.496291] parent_rate=800000000, req-rate=100000000 div=64
> [ 15.510068] parent_rate=800000000, req-rate=100000000 div=32
> [ 15.517142] parent_rate=800000000, req-rate=100000000 div=16
> [ 15.524047] parent_rate=800000000, req-rate=100000000 div=8
> [ 15.533174] parent_rate=600000000, req-rate=100000000 div=6
> [ 15.540096] parent_rate=800000000, req-rate=100000000 div=64
> [ 15.548135] parent_rate=800000000, req-rate=100000000 div=32
> [ 15.555119] parent_rate=800000000, req-rate=100000000 div=16
> [ 15.562395] parent_rate=800000000, req-rate=100000000 div=8
> [ 15.573521] parent_rate=600000000, req-rate=100000000 div=6
>
> root@rzt2h-evk:~# cat /sys/kernel/debug/clk/clk_summary | grep -e
> "xspi0" -e "XSPI_CLK0" -e "divselxspi0"
> .divselxspi0 1 1 0
> 800000000 0 0 50000 Y deviceless
> no_connection_id
> XSPI_CLK0 1 1 0
> 100000000 0 0 50000 Y
> 801c0000.spi spi
> xspi0 0 1 0
> 250000000 0 0 50000 N deviceless
> of_clk_get_from_provider
> root@rzt2h-evk:~#
>
>
> 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
>
> root@rzt2h-evk:~# cat /sys/kernel/debug/clk/clk_summary | grep -e
> "xspi0" -e "XSPI_CLK0" -e "divselxspi0"
> .divselxspi0 1 1 0
> 600000000 0 0 50000 Y deviceless
> no_connection_id
> XSPI_CLK0 1 1 0
> 75000000 0 0 50000 Y
> 801c0000.spi spi
> xspi0 0 1 0
> 250000000 0 0 50000 N deviceless
> of_clk_get_from_provider
> root@rzt2h-evk:~#
>
> Case 3# assigned-clock-rates = <50000000>;
> [ 15.240214] parent_rate=800000000, req-rate=50000000 div=64
> [ 15.253498] parent_rate=800000000, req-rate=50000000 div=32
> [ 15.261521] parent_rate=800000000, req-rate=50000000 div=16
> [ 15.272941] parent_rate=0, req-rate=50000000 div=8
> [ 15.280532] parent_rate=0, req-rate=50000000 div=6
> [ 15.289979] parent_rate=800000000, req-rate=50000000 div=64
> [ 15.298745] parent_rate=800000000, req-rate=50000000 div=32
> [ 15.309879] parent_rate=800000000, req-rate=50000000 div=16
> [ 15.319881] parent_rate=0, req-rate=50000000 div=8
> [ 15.327977] parent_rate=0, req-rate=50000000 div=6
>
> root@rzt2h-evk:~# cat /sys/kernel/debug/clk/clk_summary | grep -e
> "xspi0" -e "XSPI_CLK0" -e "divselxspi0"
> .divselxspi0 1 1 0
> 800000000 0 0 50000 Y deviceless
> no_connection_id
> XSPI_CLK0 1 1 0
> 50000000 0 0 50000 Y
> 801c0000.spi spi
> xspi0 0 1 0
> 250000000 0 0 50000 N deviceless
> of_clk_get_from_provider
> root@rzt2h-evk:~#
>
>
> Case 4# assigned-clock-rates = <37500000>;
> [ 71.710064] parent_rate=800000000, req-rate=37500000 div=64
> [ 71.718567] parent_rate=800000000, req-rate=37500000 div=32
> [ 71.725137] parent_rate=600000000, req-rate=37500000 div=16
> [ 71.731550] parent_rate=0, req-rate=37500000 div=8
> [ 71.740622] parent_rate=0, req-rate=37500000 div=6
> [ 71.746376] parent_rate=800000000, req-rate=37500000 div=64
> [ 71.752887] parent_rate=800000000, req-rate=37500000 div=32
> [ 71.767422] parent_rate=600000000, req-rate=37500000 div=16
> [ 71.778671] parent_rate=0, req-rate=37500000 div=8
> [ 71.790895] parent_rate=0, req-rate=37500000 div=6
>
> root@rzt2h-evk:~# cat /sys/kernel/debug/clk/clk_summary | grep -e
> "xspi0" -e "XSPI_CLK0" -e "divselxspi0"
> .divselxspi0 1 1 0
> 600000000 0 0 50000 Y deviceless
> no_connection_id
> XSPI_CLK0 1 1 0
> 37500000 0 0 50000 Y
> 801c0000.spi spi
> xspi0 0 1 0
> 250000000 0 0 50000 N deviceless
> of_clk_get_from_provider
> root@rzt2h-evk:~#
>
>
> Case 5# assigned-clock-rates = <25000000>;
> [ 12.411660] parent_rate=800000000, req-rate=25000000 div=64
> [ 12.429285] parent_rate=800000000, req-rate=25000000 div=32
> [ 12.436144] parent_rate=0, req-rate=25000000 div=16
> [ 12.448110] parent_rate=0, req-rate=25000000 div=8
> [ 12.458785] parent_rate=0, req-rate=25000000 div=6
> [ 12.465401] parent_rate=800000000, req-rate=25000000 div=64
> [ 12.482547] parent_rate=800000000, req-rate=25000000 div=32
> [ 12.497126] parent_rate=0, req-rate=25000000 div=16
> [ 12.509619] parent_rate=0, req-rate=25000000 div=8
> [ 12.518212] parent_rate=0, req-rate=25000000 div=6
>
> root@rzt2h-evk:~# cat /sys/kernel/debug/clk/clk_summary | grep -e
> "xspi0" -e "XSPI_CLK0" -e "divselxspi0"
> .divselxspi0 1 1 0
> 800000000 0 0 50000 Y deviceless
> no_connection_id
> XSPI_CLK0 1 1 0
> 25000000 0 0 50000 Y
> 801c0000.spi spi
> xspi0 0 1 0
> 250000000 0 0 50000 N deviceless
> of_clk_get_from_provider
> root@rzt2h-evk:~#
>
> Case 6# assigned-clock-rates = <12500000>;
> [ 87.409877] parent_rate=800000000, req-rate=12500000 div=64
> [ 87.470663] parent_rate=0, req-rate=12500000 div=32
> [ 87.485940] parent_rate=0, req-rate=12500000 div=16
> [ 87.492760] parent_rate=0, req-rate=12500000 div=8
> [ 87.498313] parent_rate=0, req-rate=12500000 div=6
>
>
> root@rzt2h-evk:~# cat /sys/kernel/debug/clk/clk_summary | grep -e
> "xspi0" -e "XSPI_CLK0" -e "divselxspi0"
> .divselxspi0 1 1 0
> 800000000 0 0 50000 Y deviceless
> no_connection_id
> XSPI_CLK0 1 1 0
> 12500000 0 0 50000 Y
> 801c0000.spi spi
> xspi0 0 1 0
> 250000000 0 0 50000 N deviceless
> of_clk_get_from_provider
> root@rzt2h-evk:~#
>
> Looking at the logs I think I could optimize the code to continue when
> parent_rate == 0
>
> Based on the above logs, would you prefer me to represent it as a
> composite clock?
>
Gentle ping.
Cheers,
Prabhakar
> > > + (clkt->div != 8 && clkt->div != 16))
> > > + continue;
> > > + now = DIV_ROUND_UP_ULL((u64)parent_rate, clkt->div);
> >
> > No need to cast to u64 (DIV_ROUND_*_ULL() handle this internally).
> >
> > > + if (abs(rate - now) < abs(rate - best)) {
> > > + div = clkt->div;
> > > + best = now;
> > > + req->best_parent_rate = parent_rate;
> > > + }
> > > + }
> > > +
> > > + if (!div) {
> > > + u8 maxdiv = 0;
> > > +
> > > + req->best_parent_rate = clk_hw_round_rate(req->best_parent_hw, 1);
> > > + /*
> > > + * If DIVSELXSPIx is set to 800MHz set the maximum divider
> > > + * or else fall back to divider of 16 which is a maximum
> > > + * supported divider for 600MHz operation.
> > > + */
> > > + if (req->best_parent_rate == DIVSELXSPI_RATE_800MHZ) {
> > > + for (clkt = divider->table; clkt->div; clkt++) {
> > > + if (clkt->div > maxdiv)
> > > + maxdiv = clkt->div;
> > > + }
> > > + div = maxdiv;
> >
> > Why not hardcode the divider, like in the else branch?
> >
> Agreed.
>
> > > + } else {
> > > + div = 16;
> > > + }
> > > + }
> > > +
> > > + req->rate = DIV_ROUND_UP_ULL((u64)req->best_parent_rate, div);
> >
> > No need to cast to u64.
> >
> Ok.
>
> Cheers,
> Prabhakar
Hi Prabhakar,
On Mon, 10 Nov 2025 at 22:38, Lad, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> On Mon, Nov 10, 2025 at 1:48 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Tue, 28 Oct 2025 at 17:52, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.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@bp.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@linux-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
Hi Geert,
On Mon, Nov 17, 2025 at 1:32 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Mon, 10 Nov 2025 at 22:38, Lad, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > On Mon, Nov 10, 2025 at 1:48 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > On Tue, 28 Oct 2025 at 17:52, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.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@bp.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?
>
When it doesnt find the best parent, 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.
>
Ok.
> 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).
>
Ok, I will have to iterate over the parents to determine the divider value.
Cheers,
Prabhakar
© 2016 - 2026 Red Hat, Inc.