Hi Geert,
On Mon, Mar 31, 2025 at 7:25 PM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
>
> Hi Biju,
>
> On Mon, 31 Mar 2025 at 17:33, Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > > From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> > > On Mon, 31 Mar 2025 at 16:34, Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > > > > From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> On Mon, 31 Mar 2025
> > > > > at 15:54, Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > > > > > > From: Biju Das <biju.das.jz@xxxxxxxxxxxxxx> Document support for
> > > > > > > the Expanded Serial Peripheral Interface (xSPI) Controller in
> > > > > > > the Renesas RZ/G3E
> > > > > > > (R9A09G047) SoC.
> > > > > > >
> > > > > > > Reviewed-by: Rob Herring (Arm) <robh@xxxxxxxxxx>
> > > > > > > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > > > >
> > > > > > > --- /dev/null
> > > > > > > +++ b/Documentation/devicetree/bindings/memory-controllers/renes
> > > > > > > +++ as,r
> > > > > > > +++ zg3e
> > > > > > > +++ -xspi.yaml
> > > > >
> > > > > > > + spi@11030000 {
> > > > > > > + compatible = "renesas,r9a09g047-xspi";
> > > > > > > + reg = <0x11030000 0x10000>, <0x20000000 0x10000000>;
> > > > > > > + reg-names = "regs", "dirmap";
> > > > > > > + interrupts = <GIC_SPI 228 IRQ_TYPE_EDGE_RISING>,
> > > > > > > + <GIC_SPI 229 IRQ_TYPE_EDGE_RISING>;
> > > > > > > + interrupt-names = "pulse", "err_pulse";
> > > > > > > + clocks = <&cpg CPG_MOD 0x9f>, <&cpg CPG_MOD 0xa0>,
> > > > > > > + <&cpg CPG_MOD 0xa1>, <&cpg CPG_MOD 0xa1>;
> > > > > >
> > > > > > On the next version I am going to update spix2 clk as <&cpg
> > > > > > CPG_CORE R9A09G047_SPI_CLK_SPIX2>
>
> According to the RZ/G3E clock system diagram, (the parent of) clk_spi
> is derived from (the parent of) clk_spix2, not the other way around?
> So you can model clk_spi as a fixed divider clock with parent clk_spix2
> and factor two. I.e. provide a new core clock R9A09G047_SPI_CLK_SPI
> instead of your proposed R9A09G047_SPI_CLK_SPIX2?
>
With this approach when R9A09G047_SPI_CLK_SPI is used as a core clock
and XSPI node is disabled the clk_summary reports the core clock is ON
(while it's actually OFF).
root@rzv2h-evk:~# cat /sys/kernel/debug/clk/clk_summary | grep spi
.smux2_xspi_clk1 0 0 0
320000000 0 0 50000 Y deviceless
no_connection_id
.pllcm33_xspi 0 0 0
40000000 0 0 50000 Y deviceless
no_connection_id
spi_clk_spix2 0 0 0
40000000 0 0 50000 N deviceless
no_connection_id
spi_clk_spi 0 0 0
20000000 0 0 50000 Y deviceless
no_connection_id
spi_aclk 0 0 0
200000000 0 0 50000 N deviceless
no_connection_id
spi_hclk 0 0 0
200000000 0 0 50000 N deviceless
no_connection_id
.smux2_xspi_clk0 0 0 0
533333333 0 0 50000 Y deviceless
no_connection_id
Can we maybe use a unused ON index and ON bit for example 25, 0 (ie
0x190) and represent this is a module clock for example for the
spi_clk_spix2 clock and use this in the DT and let the CPG core code
handle such turning ON/OF the module clocks based on the enable count
which will be handled internally in the driver?
I have some POC code for the paired clocks which will handle the
enable count of the paired module clocks, below is the diff. Please
share your thoughts.
diff --git a/arch/arm64/boot/dts/renesas/r9a09g057.dtsi
b/arch/arm64/boot/dts/renesas/r9a09g057.dtsi
index 3c5984ee27ca..8a8f59ffdb4c 100644
--- a/arch/arm64/boot/dts/renesas/r9a09g057.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a09g057.dtsi
@@ -290,10 +290,10 @@ xspi: spi@11030000 {
interrupt-names = "pulse", "err_pulse";
clocks = <&cpg CPG_MOD 0x9f>,
<&cpg CPG_MOD 0xa0>,
- <&cpg CPG_CORE R9A09G057_SPI_CLK_SPI>,
- <&cpg CPG_MOD 0xa1>;
+ <&cpg CPG_MOD 0xa1>,
+ <&cpg CPG_MOD 0x190>;
clock-names = "ahb", "axi", "spi", "spix2";
- assigned-clocks = <&cpg CPG_CORE R9A09G057_SPI_CLK_SPI>;
+ assigned-clocks = <&cpg CPG_MOD 0xa1>;
assigned-clock-rates = <133333334>;
resets = <&cpg 0xa3>, <&cpg 0xa4>;
reset-names = "hresetn", "aresetn";
diff --git a/drivers/clk/renesas/r9a09g057-cpg.c
b/drivers/clk/renesas/r9a09g057-cpg.c
index 9952474bcf48..d5eef17ad5fc 100644
--- a/drivers/clk/renesas/r9a09g057-cpg.c
+++ b/drivers/clk/renesas/r9a09g057-cpg.c
@@ -43,6 +43,7 @@ enum clk_ids {
CLK_SMUX2_XSPI_CLK0,
CLK_SMUX2_XSPI_CLK1,
CLK_PLLCM33_XSPI,
+ CLK_PLLCM33_XSPIX2,
CLK_PLLCLN_DIV2,
CLK_PLLCLN_DIV8,
CLK_PLLCLN_DIV16,
@@ -176,6 +177,7 @@ static const struct cpg_core_clk
r9a09g057_core_clks[] __initconst = {
DEF_SMUX(".smux2_xspi_clk1", CLK_SMUX2_XSPI_CLK1,
SSEL1_SELCTL3, smux2_xspi_clk1),
DEF_CSDIV(".pllcm33_xspi", CLK_PLLCM33_XSPI,
CLK_SMUX2_XSPI_CLK1, CSDIV0_DIVCTL3,
dtable_2_16),
+ DEF_FIXED(".pllcm33_xspix2", CLK_PLLCM33_XSPIX2,
CLK_PLLCM33_XSPI, 2, 1),
DEF_FIXED(".pllcln_div2", CLK_PLLCLN_DIV2, CLK_PLLCLN, 1, 2),
DEF_FIXED(".pllcln_div8", CLK_PLLCLN_DIV8, CLK_PLLCLN, 1, 8),
@@ -231,7 +233,6 @@ static const struct cpg_core_clk
r9a09g057_core_clks[] __initconst = {
CLK_PLLETH_DIV_125_FIX, 1, 1),
DEF_FIXED("gbeth_1_clk_ptp_ref_i", R9A09G057_GBETH_1_CLK_PTP_REF_I,
CLK_PLLETH_DIV_125_FIX, 1, 1),
- DEF_FIXED("spi_clk_spi", R9A09G057_SPI_CLK_SPI, CLK_PLLCM33_XSPI, 1, 2),
};
static const struct rzv2h_mod_clk r9a09g057_mod_clks[] __initconst = {
@@ -311,8 +312,10 @@ static const struct rzv2h_mod_clk
r9a09g057_mod_clks[] __initconst = {
BUS_MSTOP(4, BIT(5))),
DEF_MOD("spi_aclk", CLK_PLLCM33_GEAR, 10, 0, 5, 0,
BUS_MSTOP(4, BIT(5))),
- DEF_MOD_NO_PM("spi_clk_spix2", CLK_PLLCM33_XSPI, 10, 1, 5, 2,
+ DEF_MOD_PAIRED("spi_clk_spi", CLK_PLLCM33_XSPI, 10, 1, 5, 1,
BUS_MSTOP(4, BIT(5))),
+ DEF_MOD_PAIRED_ALIAS("spi_clk_spix2", CLK_PLLCM33_XSPIX2, 25, 0, 5, 2,
+ BUS_MSTOP(4, BIT(5)), 10, 1),
DEF_MOD("sdhi_0_imclk", CLK_PLLCLN_DIV8, 10, 3, 5, 3,
BUS_MSTOP(8, BIT(2))),
DEF_MOD("sdhi_0_imclk2", CLK_PLLCLN_DIV8, 10, 4, 5, 4,
@@ -508,7 +511,7 @@ const struct rzv2h_cpg_info r9a09g057_cpg_info
__initconst = {
/* Module Clocks */
.mod_clks = r9a09g057_mod_clks,
.num_mod_clks = ARRAY_SIZE(r9a09g057_mod_clks),
- .num_hw_mod_clks = 25 * 16,
+ .num_hw_mod_clks = 26 * 16,
/* Resets */
.resets = r9a09g057_resets,
diff --git a/drivers/clk/renesas/rzv2h-cpg.c b/drivers/clk/renesas/rzv2h-cpg.c
index 97bcd252fcbf..847ea71df865 100644
--- a/drivers/clk/renesas/rzv2h-cpg.c
+++ b/drivers/clk/renesas/rzv2h-cpg.c
@@ -71,6 +71,12 @@
#define CPG_CLKSTATUS0 (0x700)
+struct rzv2h_paired_clock {
+ u8 on_index;
+ u8 on_bit;
+ unsigned int enabled_count;
+};
+
/**
* struct rzv2h_cpg_priv - Clock Pulse Generator Private Data
*
@@ -87,6 +93,8 @@
* @rcdev: Reset controller entity
* @dsi_limits: PLL DSI parameters limits
* @plldsi_div_parameters: PLL DSI and divider parameters configuration
+ * @pair_clks: Array of paired clocks
+ * @num_pair_clks: Number of paired clocks in pair_clks[]
*/
struct rzv2h_cpg_priv {
struct device *dev;
@@ -102,6 +110,9 @@ struct rzv2h_cpg_priv {
atomic_t *mstop_count;
+ struct rzv2h_paired_clock *pair_clks;
+ unsigned int num_pair_clks;
+
struct reset_controller_dev rcdev;
const struct rzv2h_pll_div_limits *dsi_limits;
@@ -131,6 +142,11 @@ struct pll_clk {
* @mon_index: monitor register offset
* @mon_bit: monitor bit
* @ext_clk_mux_index: mux index for external clock source, or -1 if internal
+ * @paired: indicates if this clock is paired with another clock ie it shares
+ * the same ON bit and has a separate MON bit.
+ * @alias: indicates if this clock is paired and has been given dummy ON bit
+ * @pair_clk: pointer to paired clock structure if this clock is paired,
+ * otherwise NULL.
*/
struct mod_clock {
struct rzv2h_cpg_priv *priv;
@@ -142,6 +158,9 @@ struct mod_clock {
s8 mon_index;
u8 mon_bit;
s8 ext_clk_mux_index;
+ bool paired;
+ bool alias;
+ struct rzv2h_paired_clock *pair_clk;
};
#define to_mod_clock(_hw) container_of(_hw, struct mod_clock, hw)
@@ -853,8 +872,13 @@ static int rzv2h_mod_clock_is_enabled(struct clk_hw *hw)
return 0;
}
- offset = GET_CLK_ON_OFFSET(clock->on_index);
- bitmask = BIT(clock->on_bit);
+ if (clock->alias) {
+ offset = GET_CLK_ON_OFFSET(clock->pair_clk->on_index);
+ bitmask = BIT(clock->pair_clk->on_bit);
+ } else {
+ offset = GET_CLK_ON_OFFSET(clock->on_index);
+ bitmask = BIT(clock->on_bit);
+ }
return readl(priv->base + offset) & bitmask;
}
@@ -870,24 +894,57 @@ static int rzv2h_mod_clock_endisable(struct
clk_hw *hw, bool enable)
u32 value;
int error;
- dev_dbg(dev, "CLK_ON 0x%x/%pC %s\n", reg, hw->clk,
- str_on_off(enable));
+ if (clock->alias) {
+ reg = GET_CLK_ON_OFFSET(clock->pair_clk->on_index);
+ bitmask = BIT(clock->pair_clk->on_bit);
+ }
- if (enabled == enable)
+ if (clock->paired)
+ dev_err(dev, "CLK_ON 0x%x/%pC %s\n", reg, hw->clk,
+ str_on_off(enable));
+
+ if (enabled == enable) {
+ if (clock->paired && enable && clock->pair_clk->enabled_count) {
+ clock->pair_clk->enabled_count++;
+ dev_err(dev, "1: CLK_ON 0x%x/%pC %s cnt:%u\n",
+ reg, hw->clk, str_on_off(enable),
+ clock->pair_clk->enabled_count);
+ }
return 0;
+ }
value = bitmask << 16;
if (enable) {
+ if (clock->paired && clock->pair_clk->enabled_count) {
+ clock->pair_clk->enabled_count++;
+ dev_err(dev, "2: CLK_ON 0x%x/%pC %s cnt:%u\n",
+ reg, hw->clk, str_on_off(enable),
+ clock->pair_clk->enabled_count);
+ goto check_mon;
+ }
value |= bitmask;
writel(value, priv->base + reg);
if (clock->mstop_data != BUS_MSTOP_NONE)
rzv2h_mod_clock_mstop_enable(priv, clock->mstop_data);
+ if (clock->paired)
+ clock->pair_clk->enabled_count++;
} else {
+ if (clock->paired) {
+ if (clock->pair_clk->enabled_count)
+ clock->pair_clk->enabled_count--;
+ dev_err(dev, "3: CLK_ON 0x%x/%pC %s cnt:%u\n",
+ reg, hw->clk, str_on_off(enable),
+ clock->pair_clk->enabled_count);
+ if (clock->pair_clk->enabled_count > 0)
+ goto check_mon;
+ }
+
if (clock->mstop_data != BUS_MSTOP_NONE)
rzv2h_mod_clock_mstop_disable(priv, clock->mstop_data);
writel(value, priv->base + reg);
}
+check_mon:
if (!enable || clock->mon_index < 0)
return 0;
@@ -918,6 +975,55 @@ static const struct clk_ops rzv2h_mod_clock_ops = {
.is_enabled = rzv2h_mod_clock_is_enabled,
};
+static int rzv2h_mod_clk_get_paired_cfg(struct rzv2h_cpg_priv *priv,
+ struct mod_clock *clock,
+ const struct rzv2h_mod_clk *mod)
+{
+ unsigned int i;
+ u8 on_index;
+ u8 on_bit;
+
+
+ if (!clock->paired)
+ return 0;
+
+ /* Get the paired clock configuration */
+ if (clock->alias) {
+ on_index = mod->alias_on_index;
+ on_bit = mod->alias_on_bit;
+ } else {
+ on_index = clock->on_index;
+ on_bit = clock->on_bit;
+ }
+
+ for (i = 0; i < priv->num_pair_clks; i++) {
+ if (priv->pair_clks[i].on_index == on_index &&
+ priv->pair_clks[i].on_bit == on_bit) {
+ clock->pair_clk = &priv->pair_clks[i];
+ pr_err("cpg: 2: register paired clock %s
on_index %u on_bit %u num:%u\n",
+ clock->hw.init->name, on_index, on_bit,
+ priv->num_pair_clks);
+ return 0;
+ }
+ }
+
+ priv->num_pair_clks++;
+ priv->pair_clks = devm_krealloc(priv->dev, priv->pair_clks,
+ sizeof(*priv->pair_clks) *
priv->num_pair_clks, GFP_KERNEL);
+ if (!priv->pair_clks)
+ return -ENOMEM;
+
+ pr_err("cpg: register paired clock %s on_index %u on_bit %u num:%u\n",
+ clock->hw.init->name, on_index, on_bit,
+ priv->num_pair_clks);
+ priv->pair_clks[priv->num_pair_clks - 1].on_index = on_index;
+ priv->pair_clks[priv->num_pair_clks - 1].on_bit = on_bit;
+ priv->pair_clks[priv->num_pair_clks - 1].enabled_count = 0;
+ clock->pair_clk = &priv->pair_clks[priv->num_pair_clks - 1];
+
+ return 0;
+}
+
static void __init
rzv2h_cpg_register_mod_clk(const struct rzv2h_mod_clk *mod,
struct rzv2h_cpg_priv *priv)
@@ -966,7 +1072,13 @@ rzv2h_cpg_register_mod_clk(const struct
rzv2h_mod_clk *mod,
clock->priv = priv;
clock->hw.init = &init;
clock->mstop_data = mod->mstop_data;
-
+ clock->paired = mod->paired;
+ clock->alias = mod->alias;
+ ret = rzv2h_mod_clk_get_paired_cfg(priv, clock, mod);
+ if (ret) {
+ clk = ERR_PTR(ret);
+ goto fail;
+ }
ret = devm_clk_hw_register(dev, &clock->hw);
if (ret) {
clk = ERR_PTR(ret);
diff --git a/drivers/clk/renesas/rzv2h-cpg.h b/drivers/clk/renesas/rzv2h-cpg.h
index bce131bec80b..87b62fc908ba 100644
--- a/drivers/clk/renesas/rzv2h-cpg.h
+++ b/drivers/clk/renesas/rzv2h-cpg.h
@@ -223,6 +223,11 @@ enum clk_types {
* @mon_index: monitor register index
* @mon_bit: monitor bit
* @ext_clk_mux_index: mux index for external clock source, or -1 if internal
+ * @paired: indicates if this clock is paired with another clock,
i.e., it shares
+ * the same ON bit and has a separate MON bit.
+ * @alias: indicates if this clock is paired and has been given dummy ON bit
+ * @alias_on_index: For paired clocks, the index of the alias ON bit
+ * @alias_on_bit: For paired clocks, the index of the alias ON bit
*/
struct rzv2h_mod_clk {
const char *name;
@@ -235,10 +240,15 @@ struct rzv2h_mod_clk {
s8 mon_index;
u8 mon_bit;
s8 ext_clk_mux_index;
+ bool paired;
+ bool alias;
+ u8 alias_on_index;
+ u8 alias_on_bit;
};
#define DEF_MOD_BASE(_name, _mstop, _parent, _critical, _no_pm, _onindex, \
- _onbit, _monindex, _monbit, _ext_clk_mux_index) \
+ _onbit, _monindex, _monbit, _ext_clk_mux_index, _paired, \
+ _alias, _alias_on_index, _alias_on_bit) \
{ \
.name = (_name), \
.mstop_data = (_mstop), \
@@ -250,23 +260,40 @@ struct rzv2h_mod_clk {
.mon_index = (_monindex), \
.mon_bit = (_monbit), \
.ext_clk_mux_index = (_ext_clk_mux_index), \
+ .paired = (_paired), \
+ .alias = (_alias), \
+ .alias_on_index = (_alias_on_index), \
+ .alias_on_bit = (_alias_on_bit), \
}
#define DEF_MOD(_name, _parent, _onindex, _onbit, _monindex, _monbit, _mstop) \
- DEF_MOD_BASE(_name, _mstop, _parent, false, false, _onindex,
_onbit, _monindex, _monbit, -1)
+ DEF_MOD_BASE(_name, _mstop, _parent, false, false, _onindex, _onbit, \
+ _monindex, _monbit, -1, false, false, 0, 0)
#define DEF_MOD_CRITICAL(_name, _parent, _onindex, _onbit, _monindex,
_monbit, _mstop) \
- DEF_MOD_BASE(_name, _mstop, _parent, true, false, _onindex,
_onbit, _monindex, _monbit, -1)
+ DEF_MOD_BASE(_name, _mstop, _parent, true, false, _onindex,
_onbit, _monindex, \
+ _monbit, -1, false, false, 0, 0)
#define DEF_MOD_NO_PM(_name, _parent, _onindex, _onbit, _monindex,
_monbit, _mstop) \
- DEF_MOD_BASE(_name, _mstop, _parent, false, true, _onindex,
_onbit, _monindex, _monbit, -1)
+ DEF_MOD_BASE(_name, _mstop, _parent, false, true, _onindex,
_onbit, _monindex, \
+ _monbit, -1, false, false, 0, 0)
#define DEF_MOD_MUX_EXTERNAL(_name, _parent, _onindex, _onbit,
_monindex, _monbit, _mstop, \
_ext_clk_mux_index) \
DEF_MOD_BASE(_name, _mstop, _parent, false, false, _onindex,
_onbit, _monindex, _monbit, \
- _ext_clk_mux_index)
+ _ext_clk_mux_index, false, false, 0, 0)
-/**
+#define DEF_MOD_PAIRED(_name, _parent, _onindex, _onbit, _monindex,
_monbit, _mstop) \
+ DEF_MOD_BASE(_name, _mstop, _parent, false, false, _onindex,
_onbit, _monindex, \
+ _monbit, -1, true, false, 0, 0)
+
+#define DEF_MOD_PAIRED_ALIAS(_name, _parent, _onindex, _onbit,
_monindex, _monbit, _mstop, \
+ _alias_on_index, _alias_on_bit) \
+ DEF_MOD_BASE(_name, _mstop, _parent, false, false, _onindex,
_onbit, _monindex, _monbit, -1, \
+ true, true, _alias_on_index, _alias_on_bit)
+
+
+ /**
* struct rzv2h_reset - Reset definitions
*
* @reset_index: reset register index
# Output when module is loaded
root@rzv2h-evk:~# cat /sys/kernel/debug/clk/clk_summary | grep spi
spi_aclk 0 1 0
200000000 0 0 50000 N deviceless
of_clk_get_from_provider
spi_hclk 0 1 0
200000000 0 0 50000 N deviceless
of_clk_get_from_provider
.smux2_xspi_clk0 1 1 0
533333333 0 0 50000 Y deviceless
no_connection_id
.smux2_xspi_clk1 1 1 0
533333333 0 0 50000 Y deviceless
no_connection_id
.pllcm33_xspi 2 2 0
133333334 0 0 50000 Y deviceless
no_connection_id
spi_clk_spi 1 2 0
133333334 0 0 50000 Y
11030000.spi spi
.pllcm33_xspix2 1 1 0
266666668 0 0 50000 Y
deviceless no_connection_id
spi_clk_spix2 1 2 0
266666668 0 0 50000 Y
11030000.spi spix2
# Output when module is unloaded
root@rzv2h-evk:~# cat /sys/kernel/debug/clk/clk_summary | grep spi
spi_aclk 0 0 0
200000000 0 0 50000 N deviceless
no_connection_id
spi_hclk 0 0 0
200000000 0 0 50000 N deviceless
no_connection_id
.smux2_xspi_clk0 0 0 0
533333333 0 0 50000 Y deviceless
no_connection_id
.smux2_xspi_clk1 0 0 0
533333333 0 0 50000 Y deviceless
no_connection_id
.pllcm33_xspi 0 0 0
133333334 0 0 50000 Y deviceless
no_connection_id
spi_clk_spi 0 0 0
133333334 0 0 50000 N
deviceless no_connection_id
.pllcm33_xspix2 0 0 0
266666668 0 0 50000 Y
deviceless no_connection_id
spi_clk_spix2 0 0 0
266666668 0 0 50000 N
deviceless no_connection_id
root@rzv2h-evk:~#
root@rzv2h-evk:~#
Cheers,
Prabhakar
