[dif/alert_handler] Remove need to pass in HW params.
This partially addresses #8409, with respect to the Alert Handler.
Templated IPs (which have the `templated` attribute in the `<toplevel>.hjson`
file) **_may_** have DIFs that require extra bits of information related to
the specific toplevel instantiation of said IP for DIF arg-checking purposes.
This toplevel instantiation specific information was most recently
encapsulated in the `dif_<ip>_params_t` struct, which was manually defined
in the Alert Handler's DIF header file, and passed in as an argument to
various DIFs. However, the information contained in this struct is
already automatically generated in the `alert_handler_regs.h` header
file. To reduce usage complexity, the `dif_alert_handler_params_t`
struct was deprecated, and the required parameter information is used
directly from the `alert_handler_regs.h` header file throughout the
various Alert Handler DIFs that require this information.
Signed-off-by: Timothy Trippel <ttrippel@google.com>
diff --git a/sw/device/lib/dif/dif_alert_handler.c b/sw/device/lib/dif/dif_alert_handler.c
index 5dd465e..e2a6f53 100644
--- a/sw/device/lib/dif/dif_alert_handler.c
+++ b/sw/device/lib/dif/dif_alert_handler.c
@@ -20,22 +20,11 @@
"Expected seven local alerts!");
dif_result_t dif_alert_handler_init(mmio_region_t base_addr,
- dif_alert_handler_params_t params,
dif_alert_handler_t *alert_handler) {
if (alert_handler == NULL) {
return kDifBadArg;
}
- // Check we do not exceed maximum number of alerts supported by the hardware.
- if (params.alert_count > ALERT_HANDLER_PARAM_N_ALERTS) {
- return kDifBadArg;
- }
-
- // For now, the hardware is hardwired to four signals.
- if (params.escalation_signal_count != ALERT_HANDLER_PARAM_N_ESC_SEV) {
- return kDifBadArg;
- }
-
alert_handler->base_addr = base_addr;
return kDifOk;
@@ -47,14 +36,13 @@
*/
OT_WARN_UNUSED_RESULT
static bool classify_alerts(const dif_alert_handler_t *alert_handler,
- dif_alert_handler_params_t params,
const dif_alert_handler_class_config_t *class) {
if (class->alerts == NULL && class->alerts_len != 0) {
return false;
}
for (int i = 0; i < class->alerts_len; ++i) {
- if (class->alerts[i] >= params.alert_count) {
+ if (class->alerts[i] >= ALERT_HANDLER_PARAM_N_ALERTS) {
return false;
}
@@ -246,7 +234,6 @@
*/
OT_WARN_UNUSED_RESULT
static bool configure_class(const dif_alert_handler_t *alert_handler,
- dif_alert_handler_params_t params,
const dif_alert_handler_class_config_t *class) {
ptrdiff_t reg_offset;
switch (class->alert_class) {
@@ -298,7 +285,7 @@
// Configure the escalation signals for each escalation phase. In particular,
// if an escalation phase is configured, it is also enabled.
for (int i = 0; i < class->phase_signals_len; ++i) {
- if (class->phase_signals[i].signal >= params.escalation_signal_count) {
+ if (class->phase_signals[i].signal >= ALERT_HANDLER_PARAM_N_ESC_SEV) {
return false;
}
@@ -445,7 +432,7 @@
}
dif_result_t dif_alert_handler_configure(
- const dif_alert_handler_t *alert_handler, dif_alert_handler_params_t params,
+ const dif_alert_handler_t *alert_handler,
dif_alert_handler_config_t config) {
if (alert_handler == NULL) {
return kDifBadArg;
@@ -470,13 +457,13 @@
}
for (int i = 0; i < config.classes_len; ++i) {
- if (!classify_alerts(alert_handler, params, &config.classes[i])) {
+ if (!classify_alerts(alert_handler, &config.classes[i])) {
return kDifError;
}
if (!classify_local_alerts(alert_handler, &config.classes[i])) {
return kDifError;
}
- if (!configure_class(alert_handler, params, &config.classes[i])) {
+ if (!configure_class(alert_handler, &config.classes[i])) {
return kDifError;
}
if (!configure_phase_durations(alert_handler, &config.classes[i])) {
@@ -530,10 +517,10 @@
}
dif_result_t dif_alert_handler_alert_is_cause(
- const dif_alert_handler_t *alert_handler, dif_alert_handler_params_t params,
- dif_alert_handler_alert_t alert, bool *is_cause) {
+ const dif_alert_handler_t *alert_handler, dif_alert_handler_alert_t alert,
+ bool *is_cause) {
if (alert_handler == NULL || is_cause == NULL ||
- alert >= params.alert_count) {
+ alert >= ALERT_HANDLER_PARAM_N_ALERTS) {
return kDifBadArg;
}
@@ -550,9 +537,8 @@
}
dif_result_t dif_alert_handler_alert_acknowledge(
- const dif_alert_handler_t *alert_handler, dif_alert_handler_params_t params,
- dif_alert_handler_alert_t alert) {
- if (alert_handler == NULL || alert >= params.alert_count) {
+ const dif_alert_handler_t *alert_handler, dif_alert_handler_alert_t alert) {
+ if (alert_handler == NULL || alert >= ALERT_HANDLER_PARAM_N_ALERTS) {
return kDifBadArg;
}
diff --git a/sw/device/lib/dif/dif_alert_handler.h b/sw/device/lib/dif/dif_alert_handler.h
index 4f4ec89..aba1cb8 100644
--- a/sw/device/lib/dif/dif_alert_handler.h
+++ b/sw/device/lib/dif/dif_alert_handler.h
@@ -24,30 +24,6 @@
#endif // __cplusplus
/**
- * Hardware instantiation parameters for the alert handler.
- *
- * This struct describes information about the underlying hardware that is
- * not determined until the hardware design is used as part of a top-level
- * design.
- */
-typedef struct dif_alert_handler_params {
- /**
- * The configured number of alerts.
- *
- * This value is fixed by the hardware, but not known to this library.
- */
- uint32_t alert_count;
- /**
- * The configured number of escalation signals.
- *
- * This value is fixed by the hardware, but not known to this library.
- */
- // It's actually fixed at 4 right now but this is likely to become
- // configurable too.
- uint32_t escalation_signal_count;
-} dif_alert_handler_params_t;
-
-/**
* An alert class.
*
* An alert class roughly specifies how to deal with an alert. The class
@@ -343,13 +319,11 @@
* This function does not actuate the hardware.
*
* @param base_addr Hardware instantiation base address.
- * @param params Hardware instantiation parameters.
* @param handler Out param for the initialized handle.
* @return The result of the operation.
*/
OT_WARN_UNUSED_RESULT
dif_result_t dif_alert_handler_init(mmio_region_t base_addr,
- dif_alert_handler_params_t params,
dif_alert_handler_t *alert_handler);
/**
@@ -365,7 +339,7 @@
*/
OT_WARN_UNUSED_RESULT
dif_result_t dif_alert_handler_configure(
- const dif_alert_handler_t *alert_handler, dif_alert_handler_params_t params,
+ const dif_alert_handler_t *alert_handler,
dif_alert_handler_config_t config);
/**
* Locks out alert handler configuration functionality.
@@ -410,8 +384,8 @@
*/
OT_WARN_UNUSED_RESULT
dif_result_t dif_alert_handler_alert_is_cause(
- const dif_alert_handler_t *alert_handler, dif_alert_handler_params_t params,
- dif_alert_handler_alert_t alert, bool *is_cause);
+ const dif_alert_handler_t *alert_handler, dif_alert_handler_alert_t alert,
+ bool *is_cause);
/**
* Clears an alert from the cause vector, similar to an IRQ acknowledgement.
@@ -422,8 +396,7 @@
*/
OT_WARN_UNUSED_RESULT
dif_result_t dif_alert_handler_alert_acknowledge(
- const dif_alert_handler_t *alert_handler, dif_alert_handler_params_t params,
- dif_alert_handler_alert_t alert);
+ const dif_alert_handler_t *alert_handler, dif_alert_handler_alert_t alert);
/**
* Checks whether a local alert is one of the causes for an alert IRQ.
diff --git a/sw/device/lib/dif/dif_alert_handler_unittest.cc b/sw/device/lib/dif/dif_alert_handler_unittest.cc
index 2a91979..4a3247c 100644
--- a/sw/device/lib/dif/dif_alert_handler_unittest.cc
+++ b/sw/device/lib/dif/dif_alert_handler_unittest.cc
@@ -28,42 +28,21 @@
class AlertHandlerTest : public testing::Test, public MmioTest {
protected:
dif_alert_handler_t alert_handler_ = {.base_addr = dev().region()};
-
- dif_alert_handler_params_t params_ = {
- .alert_count = kAlerts,
- .escalation_signal_count = 4,
- };
};
class InitTest : public AlertHandlerTest,
public testing::WithParamInterface<uint32_t> {};
-TEST_P(InitTest, Success) {
- dif_alert_handler_params_t params = {
- .alert_count = GetParam(),
- .escalation_signal_count = 4,
- };
-
+TEST_F(InitTest, Success) {
dif_alert_handler_t alert_handler;
- EXPECT_EQ(dif_alert_handler_init(dev().region(), params, &alert_handler),
- kDifOk);
+ EXPECT_EQ(dif_alert_handler_init(dev().region(), &alert_handler), kDifOk);
}
-TEST_P(InitTest, NullArgs) {
- dif_alert_handler_params_t params = {
- .alert_count = GetParam(),
- .escalation_signal_count = 4,
- };
-
- EXPECT_EQ(dif_alert_handler_init(dev().region(), params, nullptr),
- kDifBadArg);
+TEST_F(InitTest, NullArgs) {
+ EXPECT_EQ(dif_alert_handler_init(dev().region(), nullptr), kDifBadArg);
}
-INSTANTIATE_TEST_SUITE_P(InitTestSignalCounts, InitTest,
- testing::Values(1, 2, 12, 16, 24, 32, 48,
- ALERT_HANDLER_PARAM_N_ALERTS));
-
class ConfigTest : public AlertHandlerTest {
// We provide our own dev_ member variable in this fixture, in order to
// support IgnoreMmioCalls().
@@ -101,8 +80,7 @@
{{ALERT_HANDLER_PING_TIMER_EN_SHADOWED_PING_TIMER_EN_SHADOWED_BIT,
true}});
- EXPECT_EQ(dif_alert_handler_configure(&alert_handler_, params_, config),
- kDifLocked);
+ EXPECT_EQ(dif_alert_handler_configure(&alert_handler_, config), kDifLocked);
}
TEST_F(ConfigTest, NoClassInit) {
@@ -120,8 +98,7 @@
{{ALERT_HANDLER_PING_TIMEOUT_CYC_SHADOWED_PING_TIMEOUT_CYC_SHADOWED_OFFSET,
50}});
- EXPECT_EQ(dif_alert_handler_configure(&alert_handler_, params_, config),
- kDifOk);
+ EXPECT_EQ(dif_alert_handler_configure(&alert_handler_, config), kDifOk);
}
TEST_F(ConfigTest, TimeoutTooBig) {
@@ -131,8 +108,7 @@
1,
};
- EXPECT_EQ(dif_alert_handler_configure(&alert_handler_, params_, config),
- kDifBadArg);
+ EXPECT_EQ(dif_alert_handler_configure(&alert_handler_, config), kDifBadArg);
}
TEST_F(ConfigTest, BadClassPtr) {
@@ -142,8 +118,7 @@
.classes_len = 2,
};
- EXPECT_EQ(dif_alert_handler_configure(&alert_handler_, params_, config),
- kDifBadArg);
+ EXPECT_EQ(dif_alert_handler_configure(&alert_handler_, config), kDifBadArg);
}
TEST_F(ConfigTest, ClassInit) {
@@ -348,8 +323,7 @@
{{ALERT_HANDLER_PING_TIMEOUT_CYC_SHADOWED_PING_TIMEOUT_CYC_SHADOWED_OFFSET,
50}});
- EXPECT_EQ(dif_alert_handler_configure(&alert_handler_, params_, config),
- kDifOk);
+ EXPECT_EQ(dif_alert_handler_configure(&alert_handler_, config), kDifOk);
}
TEST_F(ConfigTest, BadAlert) {
@@ -392,8 +366,7 @@
.classes_len = classes.size(),
};
- EXPECT_EQ(dif_alert_handler_configure(&alert_handler_, params_, config),
- kDifError);
+ EXPECT_EQ(dif_alert_handler_configure(&alert_handler_, config), kDifError);
}
TEST_F(ConfigTest, BadSignalPhase) {
@@ -436,8 +409,7 @@
.classes_len = classes.size(),
};
- EXPECT_EQ(dif_alert_handler_configure(&alert_handler_, params_, config),
- kDifError);
+ EXPECT_EQ(dif_alert_handler_configure(&alert_handler_, config), kDifError);
}
TEST_F(ConfigTest, BadDurationPhase) {
@@ -480,8 +452,7 @@
.classes_len = classes.size(),
};
- EXPECT_EQ(dif_alert_handler_configure(&alert_handler_, params_, config),
- kDifError);
+ EXPECT_EQ(dif_alert_handler_configure(&alert_handler_, config), kDifError);
}
TEST_F(ConfigTest, BadPointers) {
@@ -525,23 +496,19 @@
};
classes[0].alerts = nullptr;
- EXPECT_EQ(dif_alert_handler_configure(&alert_handler_, params_, config),
- kDifError);
+ EXPECT_EQ(dif_alert_handler_configure(&alert_handler_, config), kDifError);
classes[0].alerts = alerts_a.data();
classes[0].local_alerts = nullptr;
- EXPECT_EQ(dif_alert_handler_configure(&alert_handler_, params_, config),
- kDifError);
+ EXPECT_EQ(dif_alert_handler_configure(&alert_handler_, config), kDifError);
classes[0].local_alerts = locals_a.data();
classes[0].phase_signals = nullptr;
- EXPECT_EQ(dif_alert_handler_configure(&alert_handler_, params_, config),
- kDifError);
+ EXPECT_EQ(dif_alert_handler_configure(&alert_handler_, config), kDifError);
classes[0].phase_signals = signals_a.data();
classes[0].phase_durations = nullptr;
- EXPECT_EQ(dif_alert_handler_configure(&alert_handler_, params_, config),
- kDifError);
+ EXPECT_EQ(dif_alert_handler_configure(&alert_handler_, config), kDifError);
}
TEST_F(ConfigTest, BadClass) {
@@ -584,12 +551,11 @@
.classes_len = classes.size(),
};
- EXPECT_EQ(dif_alert_handler_configure(&alert_handler_, params_, config),
- kDifError);
+ EXPECT_EQ(dif_alert_handler_configure(&alert_handler_, config), kDifError);
}
TEST_F(ConfigTest, NullArgs) {
- EXPECT_EQ(dif_alert_handler_configure(nullptr, params_, {}), kDifBadArg);
+ EXPECT_EQ(dif_alert_handler_configure(nullptr, {}), kDifBadArg);
}
class LockTest : public AlertHandlerTest {};
@@ -634,38 +600,32 @@
bool flag;
EXPECT_READ32(ALERT_HANDLER_ALERT_CAUSE_5_REG_OFFSET, {{0, true}});
- EXPECT_EQ(
- dif_alert_handler_alert_is_cause(&alert_handler_, params_, 5, &flag),
- kDifOk);
+ EXPECT_EQ(dif_alert_handler_alert_is_cause(&alert_handler_, 5, &flag),
+ kDifOk);
EXPECT_TRUE(flag);
EXPECT_READ32(ALERT_HANDLER_ALERT_CAUSE_6_REG_OFFSET, {{0, false}});
- EXPECT_EQ(
- dif_alert_handler_alert_is_cause(&alert_handler_, params_, 6, &flag),
- kDifOk);
+ EXPECT_EQ(dif_alert_handler_alert_is_cause(&alert_handler_, 6, &flag),
+ kDifOk);
EXPECT_FALSE(flag);
}
TEST_F(CauseTest, Ack) {
EXPECT_WRITE32(ALERT_HANDLER_ALERT_CAUSE_0_REG_OFFSET,
{{ALERT_HANDLER_ALERT_CAUSE_0_A_0_BIT, true}});
- EXPECT_EQ(dif_alert_handler_alert_acknowledge(&alert_handler_, params_, 0),
- kDifOk);
+ EXPECT_EQ(dif_alert_handler_alert_acknowledge(&alert_handler_, 0), kDifOk);
EXPECT_WRITE32(ALERT_HANDLER_ALERT_CAUSE_11_REG_OFFSET,
{{ALERT_HANDLER_ALERT_CAUSE_11_A_11_BIT, true}});
- EXPECT_EQ(dif_alert_handler_alert_acknowledge(&alert_handler_, params_, 11),
- kDifOk);
+ EXPECT_EQ(dif_alert_handler_alert_acknowledge(&alert_handler_, 11), kDifOk);
}
TEST_F(CauseTest, BadAlert) {
bool flag;
- EXPECT_EQ(dif_alert_handler_alert_is_cause(&alert_handler_, params_, kAlerts,
- &flag),
+ EXPECT_EQ(dif_alert_handler_alert_is_cause(&alert_handler_, kAlerts, &flag),
kDifBadArg);
- EXPECT_EQ(
- dif_alert_handler_alert_acknowledge(&alert_handler_, params_, kAlerts),
- kDifBadArg);
+ EXPECT_EQ(dif_alert_handler_alert_acknowledge(&alert_handler_, kAlerts),
+ kDifBadArg);
}
TEST_F(CauseTest, IsCauseLocal) {
@@ -713,13 +673,10 @@
TEST_F(CauseTest, NullArgs) {
bool flag;
- EXPECT_EQ(dif_alert_handler_alert_is_cause(nullptr, params_, 5, &flag),
+ EXPECT_EQ(dif_alert_handler_alert_is_cause(nullptr, 5, &flag), kDifBadArg);
+ EXPECT_EQ(dif_alert_handler_alert_is_cause(&alert_handler_, 5, nullptr),
kDifBadArg);
- EXPECT_EQ(
- dif_alert_handler_alert_is_cause(&alert_handler_, params_, 5, nullptr),
- kDifBadArg);
- EXPECT_EQ(dif_alert_handler_alert_acknowledge(nullptr, params_, 11),
- kDifBadArg);
+ EXPECT_EQ(dif_alert_handler_alert_acknowledge(nullptr, 11), kDifBadArg);
EXPECT_EQ(dif_alert_handler_local_alert_is_cause(
nullptr, kDifAlertHandlerLocalAlertEscalationPingFail, &flag),
kDifBadArg);
diff --git a/sw/device/tests/autogen/plic_all_irqs_test.c b/sw/device/tests/autogen/plic_all_irqs_test.c
index 1a88f01..14cbcd5 100644
--- a/sw/device/tests/autogen/plic_all_irqs_test.c
+++ b/sw/device/tests/autogen/plic_all_irqs_test.c
@@ -40,7 +40,6 @@
#include "sw/device/lib/testing/test_framework/test_main.h"
#include "sw/device/lib/testing/test_framework/test_status.h"
#include "sw/device/tests/plic_all_irqs_test_helper.h"
-#include "alert_handler_regs.h" // Generated.
#include "hw/top_earlgrey/sw/autogen/top_earlgrey.h"
// static dif_adc_ctrl_t adc_ctrl_aon;
@@ -258,15 +257,10 @@
* Initializes the handles to all peripherals.
*/
static void peripherals_init(void) {
- dif_alert_handler_params_t alert_handler_params = {
- .alert_count = ALERT_HANDLER_PARAM_N_ALERTS,
- .escalation_signal_count = ALERT_HANDLER_PARAM_N_ESC_SEV};
-
// PERIPHERAL_INIT(adc_ctrl, adc_ctrl_aon,
// TOP_EARLGREY_ADC_CTRL_AON_BASE_ADDR);
- PERIPHERAL_INIT_WITH_PARAMS(alert_handler, alert_handler_params,
- alert_handler,
- TOP_EARLGREY_ALERT_HANDLER_BASE_ADDR);
+ PERIPHERAL_INIT(alert_handler, alert_handler,
+ TOP_EARLGREY_ALERT_HANDLER_BASE_ADDR);
// PERIPHERAL_INIT(aon_timer, aon_timer_aon,
// TOP_EARLGREY_AON_TIMER_AON_BASE_ADDR);
PERIPHERAL_INIT(csrng, csrng, TOP_EARLGREY_CSRNG_BASE_ADDR);
diff --git a/sw/device/tests/plic_all_irqs_test_helper.h b/sw/device/tests/plic_all_irqs_test_helper.h
index 4055abf..46e063a 100644
--- a/sw/device/tests/plic_all_irqs_test_helper.h
+++ b/sw/device/tests/plic_all_irqs_test_helper.h
@@ -46,15 +46,6 @@
} while (0)
/**
- * Initializes the handle to a parameterized peripheral instance.
- */
-#define PERIPHERAL_INIT_WITH_PARAMS(peripheral, params, handle, base_addr) \
- do { \
- mmio_region_t addr = mmio_region_from_addr(base_addr); \
- CHECK_DIF_OK(dif_##peripheral##_init(addr, params, &handle)); \
- } while (0)
-
-/**
* Clears all previous interrupt invocations.
*/
#define PERIPHERAL_IRQS_CLEAR(handle) \
diff --git a/util/topgen/templates/plic_all_irqs_test.c.tpl b/util/topgen/templates/plic_all_irqs_test.c.tpl
index 98e8975..7d11f41 100644
--- a/util/topgen/templates/plic_all_irqs_test.c.tpl
+++ b/util/topgen/templates/plic_all_irqs_test.c.tpl
@@ -8,7 +8,6 @@
'entropy_src', 'gpio', 'hmac', 'i2c', 'keymgr', 'kmac',
'lc_ctrl', 'otbn', 'otp_ctrl', 'pinmux', 'pwrmgr',
'rstmgr', 'spi_device', 'sram_ctrl', 'uart', 'usbdev']
-parameterized_peripherals = ['alert_handler']
def comment(n):
return '' if n in enabled_peripherals else '// '
@@ -28,9 +27,6 @@
#include "sw/device/lib/testing/test_framework/test_main.h"
#include "sw/device/lib/testing/test_framework/test_status.h"
#include "sw/device/tests/plic_all_irqs_test_helper.h"
-% for p in parameterized_peripherals:
-${comment(p)}#include "${p}_regs.h" // Generated.
-% endfor
#include "hw/top_earlgrey/sw/autogen/top_earlgrey.h"
% for p in helper.irq_peripherals:
@@ -96,16 +92,8 @@
* Initializes the handles to all peripherals.
*/
static void peripherals_init(void) {
- dif_alert_handler_params_t alert_handler_params = {
- .alert_count = ALERT_HANDLER_PARAM_N_ALERTS,
- .escalation_signal_count = ALERT_HANDLER_PARAM_N_ESC_SEV};
-
% for p in helper.irq_peripherals:
- % if p.name in parameterized_peripherals:
- ${comment(p.name)}PERIPHERAL_INIT_WITH_PARAMS(${p.name}, ${p.name}_params, ${p.inst_name}, ${p.base_addr_name});
- % else:
${comment(p.name)}PERIPHERAL_INIT(${p.name}, ${p.inst_name}, ${p.base_addr_name});
- % endif
% endfor
mmio_region_t base_addr =
