Skip to content

Add "no-reset" DT property to optionally avoid resetting the SEC #3131

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Brandon-Hurst wants to merge 2 commits into
base: adsp-6.12.0-y
Choose a base branch
from
Open

Add "no-reset" DT property to optionally avoid resetting the SEC #3131

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
56e6abd
ARM: mach-sc5xx: Add "no-reset" DT property to optionally not reset SEC
Brandon-Hurst Feb 17, 2026
7388241
dt-bindings: soc: adi: Add "no-reset" to SEC driver
Brandon-Hurst Feb 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Documentation/devicetree/bindings/soc/adi/adi,system-event-controller.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ properties:
$ref: /schemas/types.yaml#/definitions/uint32
description: Number of SHARC cores available

"no-reset":
description: Prevents resetting the SEC to preserve SHARC interrupts
Copy link
Collaborator

@pamolloy pamolloy Feb 18, 2026

Choose a reason for hiding this comment

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

Suggested change
description: Prevents resetting the SEC to preserve SHARC interrupts
description: Prevents resetting the SEC to preserve SHARC interrupts if Linux boots after the SHARC cores are booted. This is not an officially supported boot flow and other system issues may occur

Something along those lines

type: boolean

required:
- compatible
- reg
Expand Down
32 changes: 17 additions & 15 deletions drivers/soc/adi/mach-sc5xx/sec.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
// SPDX-License-Identifier: GPL-2.0-or-later
/*
* sc59x SEC
Expand Down Expand Up @@ -150,7 +150,7 @@

cleanup:
of_node_put(sec_node);
return ret;

Check warning on line 153 in drivers/soc/adi/mach-sc5xx/sec.c

View workflow job for this annotation

GitHub Actions / checks / checks 

coccicheck: 1-7  ERROR  missing put_device; call of_find_device_by_node on line 143, but without a corresponding object release within this function.
}

EXPORT_SYMBOL(get_adi_sec_from_node);
Expand Down Expand Up @@ -209,22 +209,24 @@

adi_sec->ioaddr = base;

/* Disable SYSCD_RESETb and clear RCU reset status */
adi_rcu_writel(0x00, adi_rcu, ADI_RCU_REG_CTL);
adi_rcu_writel(0x0f, adi_rcu, ADI_RCU_REG_STAT);

/* Reset SEC */
adi_sec_writel(0x02, adi_sec, ADI_SEC_REG_GCTL);
adi_sec_writel(0x02, adi_sec, ADI_SEC_REG_FCTL);

/* Initialize each core */
for (cores = 0; cores < adi_sec->cores; ++cores) {
adi_sec_writel(0x02, adi_sec,
ADI_SEC_REG_CCTL_BASE + (cores +
1) *
ADI_SEC_CCTL_SIZE);
if (!(of_property_read_bool(np, "no-reset"))) {
Copy link
Collaborator

@nunojsa nunojsa Feb 16, 2026

Choose a reason for hiding this comment

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

I wonder if there's any way to check the SHARC status so that we don't need a new property?

Copy link
Author

@Brandon-Hurst Brandon-Hurst Feb 17, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not sure if there's a way to do this that could easily cut across SC-58X, SC-59X, etc.

For the SC589, the HRM does give a bit in the RCU_MSG register that indicates the SEC has been initialized already:
image
https://www.analog.com/media/en/dsp-documentation/processor-manuals/SC58x-2158x-hrm.pdf
Pg. 234 / 6-17

Also, the SEC Core Interfaces do respectively have some enable bits in the SEC_CCTL ("SCI Control") registers for each core:
image
Pg 271 / 7-26

I'm not generally aware of how the hardware looks on the other SHARC cores, perhaps there is a portable solution that can be unified.

I see some benefit in having something in the bindings files (e.g. a DT property) in that it at least offers a bit of a general warning about shared resources...this bug took many months to identify, and one of the things that helped was looking back to older kernels and seeing they had a kernel CLI option for this. I consider a DT prop to be similarly informative. There are certainly more places where problems like this can creep in, and this fix doesn't attempt to account for all of them. But the indication in this case was helpful since the SEC is a critical system resource.

I could see the below as a solution. Let me know if this makes sense and I'll give it a build/test.

Disclosure: I had an LLM look through the PDF to find the register fields and recommend a solution. The only thing that seems not real here is the RCU0_MSG_SECINIT field, which would need to be defined.

include/linux/soc/adi/rcu.h

/* Bit values for the RCU0_MSG register */
//...
#define RCU0_MSG_C1ACTIVATE             0x00080000      /* Core 1 Activated */
#define RCU0_MSG_C2ACTIVATE             0x00100000      /* Core 2 Activated */
#define RCU0_MSG_SECINIT                0x00200000      /* SEC Initialized */

drivers/soc/adi/mach-sc5xx/sec.c

/* Check the SHARC cores & SEC for early runtime activity */
u32 rcu_msg = adi_rcu_readl(adi_rcu, ADI_RCU_REG_MSG);
bool sharc_active = (rcu_msg & (RCU0_MSG_C1ACTIVATE | RCU0_MSG_C2ACTIVATE)) != 0;
bool sec_initialized = (rcu_msg & RCU0_MSG_SECINIT) != 0;

/* Reset the SEC if the ARM core is first to boot */
if (!(sharc_active || sec_initialized)) {
    dev_info(dev, "SHARC cores active, skipping SEC reset\n");
}
else {
    /* Reset the SEC */
    //...
}

Copy link
Collaborator

@pamolloy pamolloy Feb 18, 2026

Choose a reason for hiding this comment

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

This is really a hack for an unsupported use case, but better to keep the hack in tree than e-mail a patch to a customer. This will never go upstream so the commit message should include a [ADI] prefix.

Copy link
Collaborator

@nunojsa nunojsa Feb 20, 2026

Choose a reason for hiding this comment

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

Disclosure: I had an LLM look through the PDF to find the register fields and recommend a solution. The only thing that seems not real here is the RCU0_MSG_SECINIT field, which would need to be defined.

Well, if we have a reliable way of detect that the core is up and running and we do not want/should reset it, it should be a viable upstream patch. I mean, if there's a real usecase for it...

I see some benefit in having something in the bindings files (e.g. a DT property) in that it at least offers a bit of a general warning about shared resources...

However, as a DT property might be harder to sell. DT is about HW descriptions and this looks like policy/config. However the line is thin and it could be acceptable if there's no other reliable way to detect this during early boot (and again, there are real usecases for it).

/* Disable SYSCD_RESETb and clear RCU reset status */
adi_rcu_writel(0x00, adi_rcu, ADI_RCU_REG_CTL);
adi_rcu_writel(0x0f, adi_rcu, ADI_RCU_REG_STAT);

/* Reset SEC */
adi_sec_writel(0x02, adi_sec, ADI_SEC_REG_GCTL);
adi_sec_writel(0x02, adi_sec, ADI_SEC_REG_FCTL);

/* Initialize each core */
for (cores = 0; cores < adi_sec->cores; ++cores) {
adi_sec_writel(0x02, adi_sec,
ADI_SEC_REG_CCTL_BASE + (cores +
1) *
ADI_SEC_CCTL_SIZE);
}
fsleep(100);
}
udelay(100);

/* Enable SEC fault event */
adi_sec_writel(0x01, adi_sec, ADI_SEC_REG_GCTL);
Expand Down
Loading