[dif/alert_handler] Remove alert # limit and use new configuration multireg. 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 840e5d4..b45bf11 100644 --- a/sw/device/lib/dif/dif_alert_handler.c +++ b/sw/device/lib/dif/dif_alert_handler.c
@@ -23,9 +23,8 @@ return kDifAlertHandlerBadArg; } - // TODO: We currently don't support more than 16 alerts correctly. - // See: #3826 - if (params.alert_count > 16) { + // Check we do not exceed maximum number of alerts supported by the hardware. + if (params.alert_count > ALERT_HANDLER_PARAM_N_ALERTS) { return kDifAlertHandlerBadArg; } @@ -50,21 +49,28 @@ return false; } - uint32_t enable_reg = mmio_region_read32( - handler->params.base_addr, ALERT_HANDLER_ALERT_EN_SHADOWED_0_REG_OFFSET); - uint32_t alerts_reg = - mmio_region_read32(handler->params.base_addr, - ALERT_HANDLER_ALERT_CLASS_SHADOWED_0_REG_OFFSET); - for (int i = 0; i < class->alerts_len; ++i) { if (class->alerts[i] >= handler->params.alert_count) { return false; } - // Note: the value in alerts[i] corresponds directly to the bit index within - // the register. (I.e., alert N has enable bit N). - enable_reg = bitfield_bit32_write(enable_reg, class->alerts[i], true); + // Enable the alert. + // NOTE: the value in alerts[i] corresponds directly to the multireg index. + // (I.e., alert N has enable multireg N). + ptrdiff_t enable_reg_offset = ALERT_HANDLER_ALERT_EN_SHADOWED_0_REG_OFFSET + + class->alerts[i] * sizeof(uint32_t); + uint32_t enable_reg = + mmio_region_read32(handler->params.base_addr, enable_reg_offset); + // TODO: we would like to use the generated macro for the ENABLE BIT OFFSET + // below for the alert with the given index/ID, not just assume they are + // the same across all regs in the multireg. However, making this assumption + // for now. + enable_reg = bitfield_bit32_write( + enable_reg, ALERT_HANDLER_ALERT_EN_SHADOWED_0_EN_A_0_BIT, true); + mmio_region_write32_shadowed(handler->params.base_addr, enable_reg_offset, + enable_reg); + // Determine alert classification. uint32_t classification; switch (class->alert_class) { case kDifAlertHandlerClassA: @@ -87,27 +93,26 @@ return false; } - // TODO: Currently, we assume all fields are of equal width. - // See: #3826 - uint32_t field_width = bitfield_popcount32( - ALERT_HANDLER_ALERT_CLASS_SHADOWED_0_CLASS_A_0_MASK); - uint32_t field_offset = field_width * class->alerts[i]; - - alerts_reg = bitfield_field32_write( - alerts_reg, - (bitfield_field32_t){ - .mask = ALERT_HANDLER_ALERT_CLASS_SHADOWED_0_CLASS_A_0_MASK, - .index = field_offset, - }, + // Classify the alert. + // NOTE: the value in alerts[i] corresponds directly to the multireg index. + // (I.e., alert N has enable multireg N). + ptrdiff_t class_reg_offset = + ALERT_HANDLER_ALERT_CLASS_SHADOWED_0_REG_OFFSET + + class->alerts[i] * sizeof(uint32_t); + uint32_t class_reg = + mmio_region_read32(handler->params.base_addr, class_reg_offset); + // TODO: we would like to use the generated macro for the BITFIELD + // below for the alert with the given index/ID, not just assume they are + // the same across all regs in the multireg. + class_reg = bitfield_field32_write( + class_reg, ALERT_HANDLER_ALERT_CLASS_SHADOWED_0_CLASS_A_0_FIELD, classification); + mmio_region_write32_shadowed(handler->params.base_addr, class_reg_offset, + class_reg); + + // TODO: support locking the alert class configuration. } - mmio_region_write32_shadowed(handler->params.base_addr, - ALERT_HANDLER_ALERT_EN_SHADOWED_0_REG_OFFSET, - enable_reg); - mmio_region_write32_shadowed(handler->params.base_addr, - ALERT_HANDLER_ALERT_CLASS_SHADOWED_0_REG_OFFSET, - alerts_reg); return true; } @@ -491,6 +496,11 @@ if (handler == NULL || is_locked == NULL) { return kDifAlertHandlerBadArg; } + // TODO(timothytrippel): more "locking" functionality has been added that + // can lock the ping-timer-en and ping-timer-cyc with the + // ping-timer-regwen, and likewise with the alert-en-x and alert-class-x + // registers. We need to check for this here, otherwise the alerts cannot be + // enabled. uint32_t reg = mmio_region_read32(handler->params.base_addr, @@ -665,9 +675,12 @@ return kDifAlertHandlerBadArg; } - uint32_t reg = mmio_region_read32(handler->params.base_addr, - ALERT_HANDLER_ALERT_CAUSE_0_REG_OFFSET); - *is_cause = bitfield_bit32_read(reg, alert); + ptrdiff_t cause_reg_offset = + ALERT_HANDLER_ALERT_CAUSE_0_REG_OFFSET + alert * sizeof(uint32_t); + uint32_t cause_reg = + mmio_region_read32(handler->params.base_addr, cause_reg_offset); + *is_cause = + bitfield_bit32_read(cause_reg, ALERT_HANDLER_ALERT_CAUSE_0_A_0_BIT); return kDifAlertHandlerOk; }
diff --git a/sw/device/lib/dif/dif_alert_handler_unittest.cc b/sw/device/lib/dif/dif_alert_handler_unittest.cc index a352dd6..29384ab 100644 --- a/sw/device/lib/dif/dif_alert_handler_unittest.cc +++ b/sw/device/lib/dif/dif_alert_handler_unittest.cc
@@ -58,13 +58,9 @@ EXPECT_EQ(dif_alert_handler_init(params, nullptr), kDifAlertHandlerBadArg); } -INSTANTIATE_TEST_SUITE_P( - InitTestSignalCounts, InitTest, - testing::Values(1, 2, 12, 16 - // TODO: Enable these parameters once we can support them. - // See: #3826 - // 24, 32, - )); +INSTANTIATE_TEST_SUITE_P(InitTestSignalCounts, InitTest, + testing::Values(1, 2, 12, 16, 24, 32, 64, + ALERT_HANDLER_PARAM_N_ALERTS)); class ConfigTest : public AlertTest { // We provide our own dev_ member variable in this fixture, in order to @@ -748,12 +744,12 @@ TEST_F(CauseTest, IsCause) { bool flag; - EXPECT_READ32(ALERT_HANDLER_ALERT_CAUSE_0_REG_OFFSET, {{0, true}, {5, true}}); + EXPECT_READ32(ALERT_HANDLER_ALERT_CAUSE_5_REG_OFFSET, {{0, true}}); EXPECT_EQ(dif_alert_handler_alert_is_cause(&handler_, 5, &flag), kDifAlertHandlerOk); EXPECT_TRUE(flag); - EXPECT_READ32(ALERT_HANDLER_ALERT_CAUSE_0_REG_OFFSET, {{0, true}, {5, true}}); + EXPECT_READ32(ALERT_HANDLER_ALERT_CAUSE_6_REG_OFFSET, {{0, false}}); EXPECT_EQ(dif_alert_handler_alert_is_cause(&handler_, 6, &flag), kDifAlertHandlerOk); EXPECT_FALSE(flag);