Revert "[clkmgr] Allow software to control clock stepdown"
This reverts commit 4bee429d0160d74d3d2166193b1e8dbbae62d892 for causing
private CI failures.
```
Error-[MFNF] Member not found
../src/lowrisc_dv_clkmgr_env_0.1/clkmgr_scoreboard.sv, 444
"this.ral."
Could not find member 'extclk_sel_regwen' in class 'clkmgr_reg_block', at
"../src/lowrisc_dv_clkmgr_ral_pkg_0/clkmgr_ral_pkg.sv", 965.
```
Signed-off-by: Philipp Wagner <phw@lowrisc.org>
diff --git a/hw/ip/clkmgr/data/clkmgr.hjson.tpl b/hw/ip/clkmgr/data/clkmgr.hjson.tpl
index 949ff1c..0ec3605 100644
--- a/hw/ip/clkmgr/data/clkmgr.hjson.tpl
+++ b/hw/ip/clkmgr/data/clkmgr.hjson.tpl
@@ -125,8 +125,8 @@
registers: [
- { name: "EXTCLK_CTRL_REGWEN",
- desc: "External clock control write enable",
+ { name: "EXTCLK_SEL_REGWEN",
+ desc: "External clock select write enable",
swaccess: "rw0c",
hwaccess: "none",
fields: [
@@ -134,24 +134,24 @@
name: "EN",
resval: "1"
desc: '''
- When 1, the value of !!EXTCLK_CTRL can be set. When 0, writes to !!EXTCLK_CTRL have no
+ When 1, the value of !!EXTCLK_SEL can be set. When 0, writes to !!EXTCLK_SEL have no
effect.
'''
},
]
},
- { name: "EXTCLK_CTRL",
+ { name: "EXTCLK_SEL",
desc: '''
Select external clock
''',
- regwen: "EXTCLK_CTRL_REGWEN",
+ regwen: "EXTCLK_SEL_REGWEN",
swaccess: "rw",
hwaccess: "hro",
fields: [
{
bits: "3:0",
- name: "SEL",
+ name: "VAL",
desc: '''
A value of b1010 selects external clock as clock for the system.
While this register can always be programmed, it only takes effect when the system is in
@@ -160,17 +160,6 @@
All other values are invalid and keep clocks on internal sources.
'''
resval: "0x5"
- },
- {
- bits: "7:4",
- name: "STEP_DOWN",
- desc: '''
- A value of b1010 steps down the clock dividers by a factor of 2 if the !!EXTCLK_CTRL.SEL
- field is also set to b1010.
-
- All other values have no effect.
- '''
- resval: "0x5"
}
]
},
diff --git a/hw/ip/clkmgr/data/clkmgr.sv.tpl b/hw/ip/clkmgr/data/clkmgr.sv.tpl
index f4601d8..6efe895 100644
--- a/hw/ip/clkmgr/data/clkmgr.sv.tpl
+++ b/hw/ip/clkmgr/data/clkmgr.sv.tpl
@@ -168,8 +168,7 @@
.clk_i,
.rst_ni,
.en_i(lc_dft_en_i),
- .byp_req_i(lc_tx_t'(reg2hw.extclk_ctrl.sel.q)),
- .step_down_req_i(lc_tx_t'(reg2hw.extclk_ctrl.step_down.q)),
+ .byp_req(lc_tx_t'(reg2hw.extclk_sel.q)),
.ast_clk_byp_req_o,
.ast_clk_byp_ack_i,
.lc_clk_byp_req_i,
diff --git a/hw/ip/clkmgr/doc/_index.md b/hw/ip/clkmgr/doc/_index.md
index ec60bad..f056fc9 100644
--- a/hw/ip/clkmgr/doc/_index.md
+++ b/hw/ip/clkmgr/doc/_index.md
@@ -189,18 +189,18 @@
Software request for external clocks is not always valid.
Software is only able to request for external clocks when dft functions are [allowed]({{< relref "hw/ip/lc_ctrl/doc/_index.md#dft_en" >}}).
-When software requests the external clock switch, it also has the option to request whether the clock divider should be stepped down by a factor of 2.
-
When the life cycle controller requests external clock, a request signal `lc_clk_byp_req_i` is sent from `lc_ctrl` to `clkmgr`.
`clkmgr` then forwards the request to `ast` through `ast_clk_byp_req_o`, which performs the actual clock switch.
-When the clock switch is complete, the clock dividers are stepped down by a factor of 2 and the life cycle controller is acknowledged through `lc_clk_byp_ack_o`.
+When the clock switch is complete, the life cycle controller is acknowledged through `lc_clk_byp_ack_o`.
-When software requests external clock, the register bit {{< regref "EXTCLK_CTRL" >}} is written.
+When software requests external clock, the register bit {{< regref "EXTCLK_SEL" >}} is written.
If dft functions are allowed, the `clkmgr` sends a request signal `ast_clk_byp_req_o` to `ast`.
-When the divider is stepped down, a divide-by-4 clock becomes divide-by-2 clock , and a divide-by-2 becomes a divide-by-1 clock.
-This allows external connection to be either nominal frequencies or nominal divided-by-2.
+In both cases, when the clock switch is complete, the internal dividers of the `clkmgr` are stepped down by 2x.
+A divide-by-4 clock becomes divide-by-2 clock , and a divide-by-2 becomes a divide-by-1 clock.
+The step down function will be made more flexible in the future as it is highly dependent on the ratio of internal to external clock ratios.
+However, given currently known requirements, a blanket 2x step down is sufficient.
### Clock Frequency Measurements
diff --git a/hw/ip/clkmgr/rtl/clkmgr_byp.sv b/hw/ip/clkmgr/rtl/clkmgr_byp.sv
index 713bf5a..a802d07 100644
--- a/hw/ip/clkmgr/rtl/clkmgr_byp.sv
+++ b/hw/ip/clkmgr/rtl/clkmgr_byp.sv
@@ -10,8 +10,7 @@
input clk_i,
input rst_ni,
input lc_tx_t en_i,
- input lc_tx_t byp_req_i,
- input lc_tx_t step_down_req_i,
+ input lc_tx_t byp_req,
output lc_tx_t ast_clk_byp_req_o,
input lc_tx_t ast_clk_byp_ack_i,
input lc_tx_t lc_clk_byp_req_i,
@@ -27,7 +26,7 @@
// Generate qualified reg clk bypass request
for (genvar i = 0; i < $bits(lc_tx_t); i++) begin : gen_clk_byp
prim_buf u_buf (
- .in_i(on_val[i] ? byp_req_i[i] & en_i[i] : byp_req_i[i] | en_i[i]),
+ .in_i(on_val[i] ? byp_req[i] & en_i[i] : byp_req[i] | en_i[i]),
.out_o(reg_clk_byp_req[i])
);
end
@@ -49,20 +48,13 @@
.lc_en_o(ast_clk_byp_req_o)
);
- lc_tx_t ast_clk_byp_ack;
prim_lc_sync u_rcv (
.clk_i,
.rst_ni,
.lc_en_i(ast_clk_byp_ack_i),
- .lc_en_o(ast_clk_byp_ack)
+ .lc_en_o(step_down_req_o)
);
- // if switch request came from software, let software dictate whether to step down
- assign step_down_req_o =
- lc_clk_byp_req_i == lc_ctrl_pkg::On ? ast_clk_byp_ack :
- reg_clk_byp_req == lc_ctrl_pkg::On ? ast_clk_byp_ack & step_down_req_i :
- lc_ctrl_pkg::Off;
-
// only ack the lc_ctrl if it made a request.
prim_lc_sender u_send (
.clk_i,
diff --git a/hw/top_earlgrey/ip/clkmgr/data/autogen/clkmgr.hjson b/hw/top_earlgrey/ip/clkmgr/data/autogen/clkmgr.hjson
index b8bb167..e25b1b0 100644
--- a/hw/top_earlgrey/ip/clkmgr/data/autogen/clkmgr.hjson
+++ b/hw/top_earlgrey/ip/clkmgr/data/autogen/clkmgr.hjson
@@ -142,8 +142,8 @@
registers: [
- { name: "EXTCLK_CTRL_REGWEN",
- desc: "External clock control write enable",
+ { name: "EXTCLK_SEL_REGWEN",
+ desc: "External clock select write enable",
swaccess: "rw0c",
hwaccess: "none",
fields: [
@@ -151,24 +151,24 @@
name: "EN",
resval: "1"
desc: '''
- When 1, the value of !!EXTCLK_CTRL can be set. When 0, writes to !!EXTCLK_CTRL have no
+ When 1, the value of !!EXTCLK_SEL can be set. When 0, writes to !!EXTCLK_SEL have no
effect.
'''
},
]
},
- { name: "EXTCLK_CTRL",
+ { name: "EXTCLK_SEL",
desc: '''
Select external clock
''',
- regwen: "EXTCLK_CTRL_REGWEN",
+ regwen: "EXTCLK_SEL_REGWEN",
swaccess: "rw",
hwaccess: "hro",
fields: [
{
bits: "3:0",
- name: "SEL",
+ name: "VAL",
desc: '''
A value of b1010 selects external clock as clock for the system.
While this register can always be programmed, it only takes effect when the system is in
@@ -177,17 +177,6 @@
All other values are invalid and keep clocks on internal sources.
'''
resval: "0x5"
- },
- {
- bits: "7:4",
- name: "STEP_DOWN",
- desc: '''
- A value of b1010 steps down the clock dividers by a factor of 2 if the !!EXTCLK_CTRL.SEL
- field is also set to b1010.
-
- All other values have no effect.
- '''
- resval: "0x5"
}
]
},
diff --git a/hw/top_earlgrey/ip/clkmgr/rtl/autogen/clkmgr.sv b/hw/top_earlgrey/ip/clkmgr/rtl/autogen/clkmgr.sv
index ff1e875..0b6c77b 100644
--- a/hw/top_earlgrey/ip/clkmgr/rtl/autogen/clkmgr.sv
+++ b/hw/top_earlgrey/ip/clkmgr/rtl/autogen/clkmgr.sv
@@ -193,8 +193,7 @@
.clk_i,
.rst_ni,
.en_i(lc_dft_en_i),
- .byp_req_i(lc_tx_t'(reg2hw.extclk_ctrl.sel.q)),
- .step_down_req_i(lc_tx_t'(reg2hw.extclk_ctrl.step_down.q)),
+ .byp_req(lc_tx_t'(reg2hw.extclk_sel.q)),
.ast_clk_byp_req_o,
.ast_clk_byp_ack_i,
.lc_clk_byp_req_i,
diff --git a/hw/top_earlgrey/ip/clkmgr/rtl/autogen/clkmgr_reg_pkg.sv b/hw/top_earlgrey/ip/clkmgr/rtl/autogen/clkmgr_reg_pkg.sv
index b98d22e..874419e 100644
--- a/hw/top_earlgrey/ip/clkmgr/rtl/autogen/clkmgr_reg_pkg.sv
+++ b/hw/top_earlgrey/ip/clkmgr/rtl/autogen/clkmgr_reg_pkg.sv
@@ -29,13 +29,8 @@
} clkmgr_reg2hw_alert_test_reg_t;
typedef struct packed {
- struct packed {
- logic [3:0] q;
- } sel;
- struct packed {
- logic [3:0] q;
- } step_down;
- } clkmgr_reg2hw_extclk_ctrl_reg_t;
+ logic [3:0] q;
+ } clkmgr_reg2hw_extclk_sel_reg_t;
typedef struct packed {
logic q;
@@ -209,8 +204,8 @@
// Register -> HW type
typedef struct packed {
- clkmgr_reg2hw_alert_test_reg_t alert_test; // [124:121]
- clkmgr_reg2hw_extclk_ctrl_reg_t extclk_ctrl; // [120:113]
+ clkmgr_reg2hw_alert_test_reg_t alert_test; // [120:117]
+ clkmgr_reg2hw_extclk_sel_reg_t extclk_sel; // [116:113]
clkmgr_reg2hw_jitter_enable_reg_t jitter_enable; // [112:112]
clkmgr_reg2hw_clk_enables_reg_t clk_enables; // [111:108]
clkmgr_reg2hw_clk_hints_reg_t clk_hints; // [107:103]
@@ -232,8 +227,8 @@
// Register offsets
parameter logic [BlockAw-1:0] CLKMGR_ALERT_TEST_OFFSET = 6'h 0;
- parameter logic [BlockAw-1:0] CLKMGR_EXTCLK_CTRL_REGWEN_OFFSET = 6'h 4;
- parameter logic [BlockAw-1:0] CLKMGR_EXTCLK_CTRL_OFFSET = 6'h 8;
+ parameter logic [BlockAw-1:0] CLKMGR_EXTCLK_SEL_REGWEN_OFFSET = 6'h 4;
+ parameter logic [BlockAw-1:0] CLKMGR_EXTCLK_SEL_OFFSET = 6'h 8;
parameter logic [BlockAw-1:0] CLKMGR_JITTER_ENABLE_OFFSET = 6'h c;
parameter logic [BlockAw-1:0] CLKMGR_CLK_ENABLES_OFFSET = 6'h 10;
parameter logic [BlockAw-1:0] CLKMGR_CLK_HINTS_OFFSET = 6'h 14;
@@ -255,8 +250,8 @@
// Register index
typedef enum int {
CLKMGR_ALERT_TEST,
- CLKMGR_EXTCLK_CTRL_REGWEN,
- CLKMGR_EXTCLK_CTRL,
+ CLKMGR_EXTCLK_SEL_REGWEN,
+ CLKMGR_EXTCLK_SEL,
CLKMGR_JITTER_ENABLE,
CLKMGR_CLK_ENABLES,
CLKMGR_CLK_HINTS,
@@ -274,8 +269,8 @@
// Register width information to check illegal writes
parameter logic [3:0] CLKMGR_PERMIT [15] = '{
4'b 0001, // index[ 0] CLKMGR_ALERT_TEST
- 4'b 0001, // index[ 1] CLKMGR_EXTCLK_CTRL_REGWEN
- 4'b 0001, // index[ 2] CLKMGR_EXTCLK_CTRL
+ 4'b 0001, // index[ 1] CLKMGR_EXTCLK_SEL_REGWEN
+ 4'b 0001, // index[ 2] CLKMGR_EXTCLK_SEL
4'b 0001, // index[ 3] CLKMGR_JITTER_ENABLE
4'b 0001, // index[ 4] CLKMGR_CLK_ENABLES
4'b 0001, // index[ 5] CLKMGR_CLK_HINTS
diff --git a/hw/top_earlgrey/ip/clkmgr/rtl/autogen/clkmgr_reg_top.sv b/hw/top_earlgrey/ip/clkmgr/rtl/autogen/clkmgr_reg_top.sv
index 24effbe..3d4b1bd 100644
--- a/hw/top_earlgrey/ip/clkmgr/rtl/autogen/clkmgr_reg_top.sv
+++ b/hw/top_earlgrey/ip/clkmgr/rtl/autogen/clkmgr_reg_top.sv
@@ -111,14 +111,12 @@
logic alert_test_we;
logic alert_test_recov_fault_wd;
logic alert_test_fatal_fault_wd;
- logic extclk_ctrl_regwen_we;
- logic extclk_ctrl_regwen_qs;
- logic extclk_ctrl_regwen_wd;
- logic extclk_ctrl_we;
- logic [3:0] extclk_ctrl_sel_qs;
- logic [3:0] extclk_ctrl_sel_wd;
- logic [3:0] extclk_ctrl_step_down_qs;
- logic [3:0] extclk_ctrl_step_down_wd;
+ logic extclk_sel_regwen_we;
+ logic extclk_sel_regwen_qs;
+ logic extclk_sel_regwen_wd;
+ logic extclk_sel_we;
+ logic [3:0] extclk_sel_qs;
+ logic [3:0] extclk_sel_wd;
logic jitter_enable_we;
logic jitter_enable_qs;
logic jitter_enable_wd;
@@ -229,18 +227,18 @@
);
- // R[extclk_ctrl_regwen]: V(False)
+ // R[extclk_sel_regwen]: V(False)
prim_subreg #(
.DW (1),
.SwAccess(prim_subreg_pkg::SwAccessW0C),
.RESVAL (1'h1)
- ) u_extclk_ctrl_regwen (
+ ) u_extclk_sel_regwen (
.clk_i (clk_i),
.rst_ni (rst_ni),
// from register interface
- .we (extclk_ctrl_regwen_we),
- .wd (extclk_ctrl_regwen_wd),
+ .we (extclk_sel_regwen_we),
+ .wd (extclk_sel_regwen_wd),
// from internal hardware
.de (1'b0),
@@ -251,23 +249,22 @@
.q (),
// to register interface (read)
- .qs (extclk_ctrl_regwen_qs)
+ .qs (extclk_sel_regwen_qs)
);
- // R[extclk_ctrl]: V(False)
- // F[sel]: 3:0
+ // R[extclk_sel]: V(False)
prim_subreg #(
.DW (4),
.SwAccess(prim_subreg_pkg::SwAccessRW),
.RESVAL (4'h5)
- ) u_extclk_ctrl_sel (
+ ) u_extclk_sel (
.clk_i (clk_i),
.rst_ni (rst_ni),
// from register interface
- .we (extclk_ctrl_we & extclk_ctrl_regwen_qs),
- .wd (extclk_ctrl_sel_wd),
+ .we (extclk_sel_we & extclk_sel_regwen_qs),
+ .wd (extclk_sel_wd),
// from internal hardware
.de (1'b0),
@@ -275,35 +272,10 @@
// to internal hardware
.qe (),
- .q (reg2hw.extclk_ctrl.sel.q),
+ .q (reg2hw.extclk_sel.q),
// to register interface (read)
- .qs (extclk_ctrl_sel_qs)
- );
-
- // F[step_down]: 7:4
- prim_subreg #(
- .DW (4),
- .SwAccess(prim_subreg_pkg::SwAccessRW),
- .RESVAL (4'h5)
- ) u_extclk_ctrl_step_down (
- .clk_i (clk_i),
- .rst_ni (rst_ni),
-
- // from register interface
- .we (extclk_ctrl_we & extclk_ctrl_regwen_qs),
- .wd (extclk_ctrl_step_down_wd),
-
- // from internal hardware
- .de (1'b0),
- .d ('0),
-
- // to internal hardware
- .qe (),
- .q (reg2hw.extclk_ctrl.step_down.q),
-
- // to register interface (read)
- .qs (extclk_ctrl_step_down_qs)
+ .qs (extclk_sel_qs)
);
@@ -1258,8 +1230,8 @@
always_comb begin
addr_hit = '0;
addr_hit[ 0] = (reg_addr == CLKMGR_ALERT_TEST_OFFSET);
- addr_hit[ 1] = (reg_addr == CLKMGR_EXTCLK_CTRL_REGWEN_OFFSET);
- addr_hit[ 2] = (reg_addr == CLKMGR_EXTCLK_CTRL_OFFSET);
+ addr_hit[ 1] = (reg_addr == CLKMGR_EXTCLK_SEL_REGWEN_OFFSET);
+ addr_hit[ 2] = (reg_addr == CLKMGR_EXTCLK_SEL_OFFSET);
addr_hit[ 3] = (reg_addr == CLKMGR_JITTER_ENABLE_OFFSET);
addr_hit[ 4] = (reg_addr == CLKMGR_CLK_ENABLES_OFFSET);
addr_hit[ 5] = (reg_addr == CLKMGR_CLK_HINTS_OFFSET);
@@ -1300,14 +1272,12 @@
assign alert_test_recov_fault_wd = reg_wdata[0];
assign alert_test_fatal_fault_wd = reg_wdata[1];
- assign extclk_ctrl_regwen_we = addr_hit[1] & reg_we & !reg_error;
+ assign extclk_sel_regwen_we = addr_hit[1] & reg_we & !reg_error;
- assign extclk_ctrl_regwen_wd = reg_wdata[0];
- assign extclk_ctrl_we = addr_hit[2] & reg_we & !reg_error;
+ assign extclk_sel_regwen_wd = reg_wdata[0];
+ assign extclk_sel_we = addr_hit[2] & reg_we & !reg_error;
- assign extclk_ctrl_sel_wd = reg_wdata[3:0];
-
- assign extclk_ctrl_step_down_wd = reg_wdata[7:4];
+ assign extclk_sel_wd = reg_wdata[3:0];
assign jitter_enable_we = addr_hit[3] & reg_we & !reg_error;
assign jitter_enable_wd = reg_wdata[0];
@@ -1391,12 +1361,11 @@
end
addr_hit[1]: begin
- reg_rdata_next[0] = extclk_ctrl_regwen_qs;
+ reg_rdata_next[0] = extclk_sel_regwen_qs;
end
addr_hit[2]: begin
- reg_rdata_next[3:0] = extclk_ctrl_sel_qs;
- reg_rdata_next[7:4] = extclk_ctrl_step_down_qs;
+ reg_rdata_next[3:0] = extclk_sel_qs;
end
addr_hit[3]: begin
