hal/cortex-m: Unify SCB and NVIC into SCS

[agkaminski]

JIRA: RTOS-886

Description

Motivation and Context

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Chore (refactoring, style fixes, git/CI config, submodule management, no code logic changes)

How Has This Been Tested?

• Already covered by automatic testing.
• New test added: (add PR link here).
• Tested by hand on: (list targets here).

Checklist:

• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing linter checks and tests passed.
• My changes generate no new compilation warnings for any of the targets.

Special treatment

• This PR needs additional PRs to work (list the PRs, preferably in merge-order).
• I will merge this PR by myself when appropriate.

[github-actions]

Unit Test Results

7 723 tests  ±0   7 008 ✅ ±0   37m 14s ⏱️ +17s
  443 suites ±0     715 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit 41a96db. ± Comparison against base commit 83ffd64.

♻️ This comment has been updated with latest results.

[Darchiv]

LGTM. Adding @anglov

[anglov]

Not checked manually, but looks good

I would suggest to clean up hal_* functions name to match module name in future

[agkaminski]

Not checked manually, but looks good

I would suggest to clean up hal_* functions name to match module name in future

Thought about it and I'm not sure about that. nvic/scb function names seem somewhat nice, although it violates our naming convention (prefix is not a module name). @Darchiv I can change that in the scope of this PR, should I?

[Darchiv]

Not checked manually, but looks good
I would suggest to clean up hal_* functions name to match module name in future

Thought about it and I'm not sure about that. nvic/scb function names seem somewhat nice, although it violates our naming convention (prefix is not a module name). @Darchiv I can change that in the scope of this PR, should I?

Not sure. How will you rename them? _hal_scsNvicSetIRQ() is somewhat long, but _hal_scsSetIRQ() does not relate to the module it operates in.

[agkaminski]

Not checked manually, but looks good
I would suggest to clean up hal_* functions name to match module name in future

Thought about it and I'm not sure about that. nvic/scb function names seem somewhat nice, although it violates our naming convention (prefix is not a module name). @Darchiv I can change that in the scope of this PR, should I?

Not sure. How will you rename them? _hal_scsNvicSetIRQ() is somewhat long, but _hal_scsSetIRQ() does not relate to the module it operates in.

That's my problem exactly 😄 Pure SCS seems fine to me, though

[Darchiv]

Not checked manually, but looks good
I would suggest to clean up hal_* functions name to match module name in future

Thought about it and I'm not sure about that. nvic/scb function names seem somewhat nice, although it violates our naming convention (prefix is not a module name). @Darchiv I can change that in the scope of this PR, should I?

Not sure. How will you rename them? _hal_scsNvicSetIRQ() is somewhat long, but _hal_scsSetIRQ() does not relate to the module it operates in.

That's my problem exactly 😄 Pure SCS seems fine to me, though

SCS is fine, but I think functions should have consistent names, e.g.

void _hal_nvicSetIRQ(s8 irqn, u8 state);
void _hal_nvicSetPriority(s8 irqn, u32 priority);
void _hal_nvicSetPending(s8 irqn);
int _hal_nvicGetPendingIRQ(s8 irqn);
int _hal_nvicGetActive(s8 irqn);

Sometimes with IRQ, sometimes not. Maybe even move the common part to be a prefix? E.g.

void _hal_scsIRQSet(s8 irqn, u8 state);
void _hal_scsIRQSetPriority(s8 irqn, u32 priority);
void _hal_scsIRQSetPending(s8 irqn);
int _hal_scsIRQGetPending(s8 irqn);
int _hal_scsIRQGetActive(s8 irqn);

but this doesn't look as easy to read as I thought it would.

[agkaminski]

Not checked manually, but looks good
I would suggest to clean up hal_* functions name to match module name in future

Thought about it and I'm not sure about that. nvic/scb function names seem somewhat nice, although it violates our naming convention (prefix is not a module name). @Darchiv I can change that in the scope of this PR, should I?

Not sure. How will you rename them? _hal_scsNvicSetIRQ() is somewhat long, but _hal_scsSetIRQ() does not relate to the module it operates in.

That's my problem exactly 😄 Pure SCS seems fine to me, though

SCS is fine, but I think functions should have consistent names, e.g.

void _hal_nvicSetIRQ(s8 irqn, u8 state);
void _hal_nvicSetPriority(s8 irqn, u32 priority);
void _hal_nvicSetPending(s8 irqn);
int _hal_nvicGetPendingIRQ(s8 irqn);
int _hal_nvicGetActive(s8 irqn);

Sometimes with IRQ, sometimes not. Maybe even move the common part to be a prefix? E.g.

void _hal_scsIRQSet(s8 irqn, u8 state);
void _hal_scsIRQSetPriority(s8 irqn, u32 priority);
void _hal_scsIRQSetPending(s8 irqn);
int _hal_scsIRQGetPending(s8 irqn);
int _hal_scsIRQGetActive(s8 irqn);

but this doesn't look as easy to read as I thought it would.

I like your proposition, imho it is readable, sticks to our naming convention better and generally better than the current version.