Skip to content

[top,sw/dv] Clk pwr rst smoke tests#460

Open
csabakiss-semify wants to merge 4 commits intolowRISC:mainfrom
csabakiss-semify:clk_pwr_rst_smoke_test
Open

[top,sw/dv] Clk pwr rst smoke tests#460
csabakiss-semify wants to merge 4 commits intolowRISC:mainfrom
csabakiss-semify:clk_pwr_rst_smoke_test

Conversation

@csabakiss-semify
Copy link
Copy Markdown
Collaborator

  1. Added power manager related hal files
  2. Extended reset manager hal
  3. Extended clock manager hal
  4. Added smoke tests for the managers

I can see the conflicts, even if I just finished the rebase. However, I am opening this PR to start the review process. I will fix the further conflicts just before the merge.

Signed-off-by: Csaba Kiss <csaba.kiss@semify-eda.com>
Signed-off-by: Csaba Kiss <csaba.kiss@semify-eda.com>
Signed-off-by: Csaba Kiss <csaba.kiss@semify-eda.com>
…ests to top level smoke regression and top level sim config

Signed-off-by: Csaba Kiss <csaba.kiss@semify-eda.com>
@martin-velay martin-velay changed the title Clk pwr rst smoke tests [top,sw/dv] Clk pwr rst smoke tests Apr 23, 2026
Copy link
Copy Markdown
Collaborator

@marnovandermaas marnovandermaas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some initial comments from my end.


if (rstmgr_software_reset_info_get(rstmgr)) {
return true;
if (reason & RSTMGR_RESET_INFO_POR) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, this is a good change!

DEV_WRITE(rstmgr + RSTMGR_RESET_REQ_REG, RSTMGR_RESET_REQ_TRUE);
}

bool rstmgr_software_reset_info_get(rstmgr_t rstmgr)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can now remove this function. I think it was only used in the software_reset test.

Comment on lines +11 to +25
static uint32_t clkmgr_read(clkmgr_t clkmgr, uintptr_t reg)
{
return DEV_READ(clkmgr + reg);
}

static void clkmgr_write(clkmgr_t clkmgr, uintptr_t reg, uint32_t value)
{
DEV_WRITE(clkmgr + reg, value);
}

static uint32_t clkmgr_bit(size_t clock)
{
return 1u << clock;
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These don't look like clock manager specific functions. Do they already exist elsewhere in the repo?

bool toggled = !initial;

clkmgr_gateable_clock_set_enabled(clkmgr, CLKMGR_GATEABLE_CLOCK_IO_PERI, toggled);
if (clkmgr_gateable_clock_get_enabled(clkmgr, CLKMGR_GATEABLE_CLOCK_IO_PERI) != toggled) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any way that we can confirm that the peripherals are actually stopped? I'm not sure whether reading a register will cause a bus error or just hang the design.

Comment thread sw/device/lib/hal/mocha.c
pwrmgr_t mocha_system_pwrmgr(void)
{
#if defined(__riscv_zcherihybrid)
return (pwrmgr_t)create_mmio_capability(pwrmgr_base, 0x80u);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this length can be 0x44

Comment on lines +9 to +22
static uint32_t pwrmgr_read(pwrmgr_t pwrmgr, uintptr_t reg)
{
return DEV_READ(pwrmgr + reg);
}

static void pwrmgr_write(pwrmgr_t pwrmgr, uintptr_t reg, uint32_t value)
{
DEV_WRITE(pwrmgr + reg, value);
}

uint32_t pwrmgr_control_get(pwrmgr_t pwrmgr)
{
return pwrmgr_read(pwrmgr, PWRMGR_CONTROL_REG);
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replicated from clock manager.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants