Add c warning checks in device binary programs Enable `-Wall` by default to catch coding errors in compilation time. Clean up the code base on unused variables and uninitialzed values. Change-Id: I010ad1d690631116f0de03215ffde59ed5e18ac1
diff --git a/rules/matcha.bzl b/rules/matcha.bzl index 9418dbf..8caf9aa 100644 --- a/rules/matcha.bzl +++ b/rules/matcha.bzl
@@ -53,6 +53,12 @@ "smc": "//sw/device/lib/arch:smc_asic", } +MATCHA_COPTS = [ + "-Werror", + "-Wall", + "-Wno-unused-function", +] + # This helper function generates a dictionary of per-device dependencies which is used to # generate slightly different binaries for each hardware target, including two # simulation platforms (DV and Verilator), and one FPGA platform (Nexus), @@ -142,6 +148,11 @@ Containing all targets across all devices for the above generated rules. """ deps = kwargs.pop("deps", []) + + # Place the default copts first to allow per-target override. + copts = MATCHA_COPTS + kwargs.pop("copts", []) + kwargs.update({"copts": copts}) + all_targets = [] for (device, dev_deps) in per_device_deps.items(): if device not in device_deps("secure_core"): @@ -249,6 +260,10 @@ Containing all targets across all devices for the above generated rules. """ deps = kwargs.pop("deps", []) + + # Place the default copts first to allow per-target override. + copts = MATCHA_COPTS + kwargs.pop("copts", []) + kwargs.update({"copts": copts}) all_targets = [] for (device, dev_deps) in per_device_deps.items(): if device not in device_deps("secure_core"):
diff --git a/sw/device/examples/demo_hps_live/hps_demo_smc.c b/sw/device/examples/demo_hps_live/hps_demo_smc.c index 448f901..c397097 100644 --- a/sw/device/examples/demo_hps_live/hps_demo_smc.c +++ b/sw/device/examples/demo_hps_live/hps_demo_smc.c
@@ -114,9 +114,8 @@ // Render an RGB565 buffer with the score text. memset(paint_ctx.Image, 0, LCD_WIDTH * font->Height * sizeof(uint16_t)); memset(paint_str, 0, sizeof(paint_str)); - size_t len = - base_snprintf(paint_str, sizeof(paint_str), "Score 0: %d, Score 1: %d", - model_out_val0, model_out_val1); + base_snprintf(paint_str, sizeof(paint_str), "Score 0: %d, Score 1: %d", + model_out_val0, model_out_val1); Paint_DrawString_EN(&paint_ctx, 0, 0, paint_str, font, BLACK, WHITE); LCD_ClearToBufferWindow(&spi_display, paint_ctx.Image, 0, 0, LCD_WIDTH, font->Height);
diff --git a/sw/device/examples/demo_isp/isp_live_cam_test_smc.c b/sw/device/examples/demo_isp/isp_live_cam_test_smc.c index 554f80d..48bf207 100644 --- a/sw/device/examples/demo_isp/isp_live_cam_test_smc.c +++ b/sw/device/examples/demo_isp/isp_live_cam_test_smc.c
@@ -48,9 +48,6 @@ static volatile uint32_t isp_wrapper_mi_handled; static void handle_isp_wrapper_isp_irq() { - mmio_region_t ml_dmem_base_addr = - mmio_region_from_addr(TOP_MATCHA_ML_TOP_DMEM_BASE_ADDR); - uint32_t mem_val; LOG_INFO("Start of a frame."); // Print the frame to uart. size_t bytes_remaining = 320 * 240; @@ -69,11 +66,6 @@ static void handle_isp_wrapper_isrs(const dif_rv_plic_irq_id_t interrupt_id) { // NOTE: This initialization is superfluous, since the `default` case below // is effectively noreturn, but the compiler is unable to prove this. - // dif_isp_wrapper_irq_t isp_ctrl_irq = 0; - uint32_t isp_status = mmio_region_read32(isp_wrapper.base_addr, ISP_MIS); - uint32_t mi_status = - mmio_region_read32(isp_wrapper.base_addr, MI_MIS); - switch (interrupt_id) { case kTopMatchaPlicIrqIdIspWrapperIsp: isp_wrapper_isp_handled++;
diff --git a/sw/device/examples/spi_display/spi_display_smc.c b/sw/device/examples/spi_display/spi_display_smc.c index 7a4fe35..899133a 100644 --- a/sw/device/examples/spi_display/spi_display_smc.c +++ b/sw/device/examples/spi_display/spi_display_smc.c
@@ -49,7 +49,6 @@ static volatile bool isp_frame_done_irq_seen = false; void ottf_external_isr(void) { - uint32_t tx, rx; dif_rv_plic_irq_id_t plic_irq_id; // Clear IRQ from the tlul_mailbox and claim it from the PLIC.
diff --git a/sw/device/examples/spi_passthrough/spi_passthrough.c b/sw/device/examples/spi_passthrough/spi_passthrough.c index 9c57d1f..8af4c11 100644 --- a/sw/device/examples/spi_passthrough/spi_passthrough.c +++ b/sw/device/examples/spi_passthrough/spi_passthrough.c
@@ -82,12 +82,6 @@ kSpiDeviceWriteCommandSlotBase = 11, }; -static dif_irq_type_t irq_types[] = { - kDifIrqTypeEvent, kDifIrqTypeEvent, kDifIrqTypeEvent, kDifIrqTypeEvent, - kDifIrqTypeEvent, kDifIrqTypeEvent, kDifIrqTypeEvent, kDifIrqTypeEvent, - kDifIrqTypeEvent, kDifIrqTypeEvent, kDifIrqTypeEvent, kDifIrqTypeStatus, -}; - #define FULL_PAGE_SIZE 512 static const int kHalfPageSize = 256; static const int kFullPageSize = FULL_PAGE_SIZE; @@ -391,7 +385,6 @@ memcpy(full_page + kHalfPageSize, payload, kHalfPageSize); } - uint32_t *full_page_u32 = (uint32_t *)full_page; dif_toggle_t addr4b_enabled; CHECK_DIF_OK( dif_spi_device_get_4b_address_mode(&spi_device, &addr4b_enabled));
diff --git a/sw/device/examples/spi_to_host/spi_to_host_sc.c b/sw/device/examples/spi_to_host/spi_to_host_sc.c index fdc8ae0..504f290 100644 --- a/sw/device/examples/spi_to_host/spi_to_host_sc.c +++ b/sw/device/examples/spi_to_host/spi_to_host_sc.c
@@ -44,7 +44,6 @@ #define kSwStrap0Pin (22) void ottf_external_isr(void) { - uint32_t rx; dif_rv_plic_irq_id_t plic_irq_id; CHECK_DIF_OK(dif_rv_plic_irq_claim(&plic_sec, kTopMatchaPlicTargetIbex0,
diff --git a/sw/device/examples/spi_to_host/spi_to_host_smc.c b/sw/device/examples/spi_to_host/spi_to_host_smc.c index 9b062d0..91efff6 100644 --- a/sw/device/examples/spi_to_host/spi_to_host_smc.c +++ b/sw/device/examples/spi_to_host/spi_to_host_smc.c
@@ -35,7 +35,6 @@ static dif_rv_plic_t plic_smc; void ottf_external_isr(void) { - uint32_t tx, rx; dif_rv_plic_irq_id_t plic_irq_id; CHECK_DIF_OK(dif_rv_plic_irq_claim(&plic_smc, kTopMatchaPlicTargetIbex0Smc,
diff --git a/sw/device/tests/cam_ctrl_test.c b/sw/device/tests/cam_ctrl_test.c index 45675e3..051e5ef 100644 --- a/sw/device/tests/cam_ctrl_test.c +++ b/sw/device/tests/cam_ctrl_test.c
@@ -33,7 +33,7 @@ static dif_cam_ctrl_t cam_ctrl; void _ottf_main(void) { - uint32_t en_result, regwen_result; + uint32_t en_result; // Initialize the UART to enable logging for non-DV simulation platforms. if (kDeviceType != kDeviceSimDV) {
diff --git a/sw/device/tests/dma_main.c b/sw/device/tests/dma_main.c index e9bb3f6..654e8da 100644 --- a/sw/device/tests/dma_main.c +++ b/sw/device/tests/dma_main.c
@@ -54,11 +54,10 @@ void ottf_external_isr(void) { LOG_INFO("Writer interrupt received!"); - uint32_t ram_main_data[LINE_COUNT], ram_smc_data[LINE_COUNT]; + uint32_t ram_smc_data[LINE_COUNT]; dif_rv_plic_irq_id_t plic_irq_id; dif_dma_irq_state_snapshot_t intr_state; - mmio_region_t reader_start_addr = mmio_region_from_addr(READER_START_ADDR); mmio_region_t writer_start_addr = mmio_region_from_addr(WRITER_START_ADDR); // Claim PLIC IRQ, perform DMA interrupt reg checks after
diff --git a/sw/device/tests/isp_wrapper_test.c b/sw/device/tests/isp_wrapper_test.c index af14dde..1c7ca1b 100644 --- a/sw/device/tests/isp_wrapper_test.c +++ b/sw/device/tests/isp_wrapper_test.c
@@ -33,8 +33,6 @@ static dif_isp_wrapper_t isp_wrapper; void _ottf_main(void) { - uint32_t en_result, regwen_result; - // Initialize the UART to enable logging for non-DV simulation platforms. if (kDeviceType != kDeviceSimDV) { init_uart(TOP_MATCHA_UART0_BASE_ADDR, &uart);
diff --git a/sw/device/tests/rstmgr_alert_info_test.c b/sw/device/tests/rstmgr_alert_info_test.c index daa61b4..9b5515d 100644 --- a/sw/device/tests/rstmgr_alert_info_test.c +++ b/sw/device/tests/rstmgr_alert_info_test.c
@@ -481,9 +481,6 @@ */ static void prgm_alert_handler_round4(void) { - dif_alert_handler_alert_t alerts[ALERT_HANDLER_PARAM_N_ALERTS]; - dif_alert_handler_class_t alert_classes[ALERT_HANDLER_PARAM_N_ALERTS]; - // Enable all alerts to enable ping associate with them. for (int i = 0; i < ALERT_HANDLER_PARAM_N_ALERTS; ++i) { CHECK_DIF_OK(dif_alert_handler_configure_alert(
diff --git a/sw/device/tests/sim_dv/BUILD b/sw/device/tests/sim_dv/BUILD index 8b4c63b..7e707b0 100644 --- a/sw/device/tests/sim_dv/BUILD +++ b/sw/device/tests/sim_dv/BUILD
@@ -363,6 +363,10 @@ matcha_dv_test( name = "data_integrity_escalation_reset_test", srcs = ["@lowrisc_opentitan//sw/device/tests/sim_dv:data_integrity_escalation_reset_test.c"], + copts = [ + # Mask the code error from OT source + "-Wno-unused-variable", + ], deps = [ "//sw/device/lib/dif:alert_handler", "//sw/device/lib/dif:rstmgr", @@ -447,6 +451,10 @@ matcha_dv_test( name = "alert_handler_escalation_test", srcs = ["@lowrisc_opentitan//sw/device/tests/sim_dv:alert_handler_escalation.c"], + copts = [ + # Mask the code error from OT source + "-Wno-unused-variable", + ], deps = [ "//hw/top_matcha:alert_handler_regs", "//sw/device/lib/arch:device", @@ -489,6 +497,10 @@ matcha_dv_test( name = "all_escalation_resets_test", srcs = ["@lowrisc_opentitan//sw/device/tests/sim_dv:all_escalation_resets_test.c"], + copts = [ + # Mask the code error from OT source + "-Wno-unused-variable", + ], deps = [ "//sw/device/lib/dif:alert_handler", "//sw/device/lib/dif:clkmgr",
diff --git a/sw/device/tests/sim_dv/ast_clk_rst_inputs.c b/sw/device/tests/sim_dv/ast_clk_rst_inputs.c index ccc8972..72e9a00 100644 --- a/sw/device/tests/sim_dv/ast_clk_rst_inputs.c +++ b/sw/device/tests/sim_dv/ast_clk_rst_inputs.c
@@ -61,8 +61,6 @@ static volatile const uint8_t kChannel1MinLowByte; static volatile const uint8_t kChannel1MinHighByte; -static const uint32_t kPlicTarget = kTopMatchaPlicTargetIbex0; - static dif_sensor_ctrl_t sensor_ctrl; static dif_alert_handler_t alert_handler; static dif_aon_timer_t aon_timer; @@ -71,7 +69,6 @@ static dif_pwrmgr_t pwrmgr; static dif_rstmgr_t rstmgr; static dif_entropy_src_t entropy_src; -static dif_pwrmgr_wakeup_reason_t wakeup_reason; static dif_clkmgr_t clkmgr; static dif_adc_ctrl_t adc_ctrl; @@ -79,15 +76,6 @@ static volatile bool interrupt_serviced = false; static bool first_adc_setup = true; -static plic_isr_ctx_t plic_ctx = {.rv_plic = &plic, - .hart_id = kTopMatchaPlicTargetIbex0}; - -static pwrmgr_isr_ctx_t pwrmgr_isr_ctx = { - .pwrmgr = &pwrmgr, - .plic_pwrmgr_start_irq_id = kTopMatchaPlicIrqIdPwrmgrAonWakeup, - .expected_irq = kDifPwrmgrIrqWakeup, - .is_only_irq = true}; - enum { /** * The size of the buffer used in firmware to process the entropy bits in @@ -326,7 +314,6 @@ dif_entropy_src_config_t entropy_src_config, uint32_t alert_idx) { bool deepsleep; int read_fifo_depth_val; - bool clock_check_status; uint32_t unhealthy_fifos, errors, alerts; const dif_edn_t edn0 = {.base_addr = @@ -535,11 +522,6 @@ } bool test_main() { - uint32_t event_idx; - int idx = 1; - int read_fifo_depth_val; - - bool deepsleep; dif_pwrmgr_domain_config_t pwrmgr_config; const dif_entropy_src_config_t entropy_src_config = {
diff --git a/sw/device/tests/sim_dv/i2c_device_tx_rx_test.c b/sw/device/tests/sim_dv/i2c_device_tx_rx_test.c index 19154e3..2771190 100644 --- a/sw/device/tests/sim_dv/i2c_device_tx_rx_test.c +++ b/sw/device/tests/sim_dv/i2c_device_tx_rx_test.c
@@ -127,8 +127,6 @@ * This function overrides the default OTTF external ISR. */ void ottf_external_isr(void) { - uint32_t i2c_irq_fmt_threshold_id; - plic_isr_ctx_t plic_ctx = {.rv_plic = &plic, .hart_id = kTopMatchaPlicTargetIbex0};
diff --git a/sw/device/tests/sim_dv/i2c_host_tx_rx_test.c b/sw/device/tests/sim_dv/i2c_host_tx_rx_test.c index 82c9bcb..00ec85d 100644 --- a/sw/device/tests/sim_dv/i2c_host_tx_rx_test.c +++ b/sw/device/tests/sim_dv/i2c_host_tx_rx_test.c
@@ -41,12 +41,6 @@ static dif_pinmux_t pinmux; static dif_rv_plic_t plic; -static const dif_i2c_fmt_flags_t default_flags = {.start = false, - .stop = false, - .read = false, - .read_cont = false, - .suppress_nak_irq = false}; - OTTF_DEFINE_TEST_CONFIG(); /**
diff --git a/sw/device/tests/sim_dv/rv_dm_access_after_wakeup.c b/sw/device/tests/sim_dv/rv_dm_access_after_wakeup.c index 7703621..68672b3 100644 --- a/sw/device/tests/sim_dv/rv_dm_access_after_wakeup.c +++ b/sw/device/tests/sim_dv/rv_dm_access_after_wakeup.c
@@ -69,9 +69,9 @@ static void put_to_sleep(dif_pwrmgr_t *pwrmgr, bool deep_sleep) { dif_pwrmgr_domain_config_t cfg; CHECK_DIF_OK(dif_pwrmgr_get_domain_config(pwrmgr, &cfg)); - cfg = cfg & (kDifPwrmgrDomainOptionIoClockInLowPower | - kDifPwrmgrDomainOptionUsbClockInLowPower | - kDifPwrmgrDomainOptionUsbClockInActivePower) | + cfg = (cfg & (kDifPwrmgrDomainOptionIoClockInLowPower | + kDifPwrmgrDomainOptionUsbClockInLowPower | + kDifPwrmgrDomainOptionUsbClockInActivePower)) | (!deep_sleep ? kDifPwrmgrDomainOptionMainPowerInLowPower : 0); pwrmgr_testutils_enable_low_power(pwrmgr, kDifPwrmgrWakeupRequestSourceOne,
diff --git a/sw/device/tests/sim_dv/sleep_pin_mio_dio_val_test.c b/sw/device/tests/sim_dv/sleep_pin_mio_dio_val_test.c index 7c85284..36b7dde 100644 --- a/sw/device/tests/sim_dv/sleep_pin_mio_dio_val_test.c +++ b/sw/device/tests/sim_dv/sleep_pin_mio_dio_val_test.c
@@ -119,7 +119,6 @@ static uint8_t kDioPads[NUM_DIO_PADS] = {0}; // PLIC structures -static const uint32_t kPlicTarget = kTopMatchaPlicTargetIbex0; static dif_pwrmgr_t pwrmgr; static dif_pinmux_t pinmux; static dif_rv_plic_t plic;
diff --git a/sw/device/tests/sim_dv/sleep_pin_retention_test.c b/sw/device/tests/sim_dv/sleep_pin_retention_test.c index 49dfb49..3878daf 100644 --- a/sw/device/tests/sim_dv/sleep_pin_retention_test.c +++ b/sw/device/tests/sim_dv/sleep_pin_retention_test.c
@@ -52,11 +52,6 @@ // variable via backdoor_overwrite. static volatile const uint8_t kRounds = 2; -// SW testcode randomly chooses the value for 8 GPIO pins (pins are fixed) then -// invert the value for retention. SW will sends the value to SV testbench via -// LOG_INFO(). -static const uint8_t kGpioVal = 0x00; - // To wakeup and maintain GPIO, for now test enters to normal sleep only. static const bool deepPowerdown = false; @@ -171,7 +166,6 @@ bool test_main(void) { bool result = true; - dif_pinmux_index_t detector; dif_pinmux_wakeup_config_t wakeup_cfg; // Default Deep Power Down
diff --git a/sw/device/tests/sim_dv/sleep_pin_wake_test.c b/sw/device/tests/sim_dv/sleep_pin_wake_test.c index 16e6c6e..b3fecd6 100644 --- a/sw/device/tests/sim_dv/sleep_pin_wake_test.c +++ b/sw/device/tests/sim_dv/sleep_pin_wake_test.c
@@ -34,7 +34,6 @@ OTTF_DEFINE_TEST_CONFIG(); // PLIC structures -static const uint32_t kPlicTarget = kTopMatchaPlicTargetIbex0; static dif_pwrmgr_t pwrmgr; static dif_pinmux_t pinmux; static dif_rv_plic_t plic; @@ -71,7 +70,6 @@ } bool test_main(void) { - dif_pinmux_index_t detector; dif_pinmux_wakeup_config_t wakeup_cfg; // Default Deep Power Down
diff --git a/sw/device/tests/sim_dv/spi_host_tx_rx_test.c b/sw/device/tests/sim_dv/spi_host_tx_rx_test.c index 9160a23..738a7f0 100644 --- a/sw/device/tests/sim_dv/spi_host_tx_rx_test.c +++ b/sw/device/tests/sim_dv/spi_host_tx_rx_test.c
@@ -220,8 +220,8 @@ // Setup spi host configuration LOG_INFO("Testing spi_host%0d", kSPIHostIdx); - uintptr_t base_addr; - uint64_t clkHz; + uintptr_t base_addr = 0; + uint64_t clkHz = 0; switch (kSPIHostIdx) { case 0: { base_addr = TOP_MATCHA_SPI_HOST0_BASE_ADDR; @@ -235,6 +235,7 @@ } default: LOG_FATAL("Invalid kSPIHostIdx: %d", kSPIHostIdx); + abort(); } CHECK_DIF_OK(dif_spi_host_init(mmio_region_from_addr(base_addr), &spi_host)); init_spi_host(&spi_host, (uint32_t)clkHz);
diff --git a/sw/device/tests/sim_dv/sysrst_ctrl_in_irq_test.c b/sw/device/tests/sim_dv/sysrst_ctrl_in_irq_test.c index 5b73940..0804ca1 100644 --- a/sw/device/tests/sim_dv/sysrst_ctrl_in_irq_test.c +++ b/sw/device/tests/sim_dv/sysrst_ctrl_in_irq_test.c
@@ -66,13 +66,6 @@ kTopMatchaPinmuxInselIoc7, kTopMatchaPinmuxInselIoc9, }; -static const dif_sysrst_ctrl_pin_t kSysrstCtrlInputs[] = { - kDifSysrstCtrlPinKey0In, kDifSysrstCtrlPinKey1In, - kDifSysrstCtrlPinKey2In, kDifSysrstCtrlPinPowerButtonIn, - kDifSysrstCtrlPinAcPowerPresentIn, kDifSysrstCtrlPinLidOpenIn, - kDifSysrstCtrlPinEcResetInOut, kDifSysrstCtrlPinFlashWriteProtectInOut, -}; - void test_phase_sync() { test_status_set(kTestStatusInTest); test_status_set(kTestStatusInWfi);
diff --git a/sw/device/tests/smc/smc_cam_i2c_test.c b/sw/device/tests/smc/smc_cam_i2c_test.c index 6f3c861..64d27d2 100644 --- a/sw/device/tests/smc/smc_cam_i2c_test.c +++ b/sw/device/tests/smc/smc_cam_i2c_test.c
@@ -41,7 +41,6 @@ static volatile bool isp_frame_done_irq_seen = false; void ottf_external_isr(void) { - top_matcha_plic_peripheral_smc_t peripheral; dif_rv_plic_irq_id_t interrupt_id; CHECK_DIF_OK(dif_rv_plic_irq_claim(&plic, kTopMatchaPlicTargetIbex0Smc, &interrupt_id));