diff --git a/.clang-format b/.clang-format index 63ebf38..87fe5e6 100644 --- a/.clang-format +++ b/.clang-format @@ -1,13 +1,15 @@ --- BasedOnStyle: LLVM +Standard: Latest ColumnLimit: 120 IndentWidth: 4 TabWidth: 4 UseTab: ForIndentation AlignEscapedNewlines: DontAlign AllowShortFunctionsOnASingleLine: Empty -AlwaysBreakTemplateDeclarations: true +BreakTemplateDeclarations: Yes BreakBeforeBraces: Custom BraceWrapping: AfterFunction: true +InsertBraces: true ... diff --git a/CMakeLists.txt b/CMakeLists.txt index eb6fc15..fa356a6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -12,7 +12,7 @@ if(NOT LIBAVR_ROOT) set(LIBAVR_ROOT ${CMAKE_CURRENT_SOURCE_DIR}/libavr) endif() if(NOT EXISTS ${LIBAVR_ROOT}/CMakeLists.txt) - message(FATAL_ERROR "libavr not found at ${LIBAVR_ROOT} — run: git submodule update --init libavr") + message(FATAL_ERROR "libavr not found at ${LIBAVR_ROOT} - run: git submodule update --init libavr") endif() add_subdirectory(${LIBAVR_ROOT} libavr-build) diff --git a/libavr b/libavr index b719ed7..26109e1 160000 --- a/libavr +++ b/libavr @@ -1 +1 @@ -Subproject commit b719ed74d8d462ea4e6877fc55e8ba64335eb01a +Subproject commit 26109e172be4ab5cf17c9efb8c7e36d1f1caa485 diff --git a/src/board.hpp b/src/board.hpp index b7913f4..b9eb65a 100644 --- a/src/board.hpp +++ b/src/board.hpp @@ -3,7 +3,7 @@ #include // The board composition: every peripheral of the fan controller in one -// place. ATmega328P at 16 MHz — thermistor divider on ADC0 (PC0), fan on +// place. ATmega328P at 16 MHz - thermistor divider on ADC0 (PC0), fan on // OC0B (PD5) at 50 kHz, console on the hardware UART. namespace app { @@ -20,7 +20,7 @@ class uptime { static std::uint64_t millis() { - avr::irq::atomic_guard lock; + avr::irq::interrupt_guard lock; return ms; } }; @@ -35,25 +35,29 @@ class sampler { static constexpr std::uint16_t samples = 1000; public: - using input = dev::adc<{.input = avr::adc::input_pin(0), - .trigger = avr::adc::trigger::free_running, - .on_conversion = [](std::uint16_t value) { - sum = sum + value; - count = count + 1; - if (count >= samples) { - window = static_cast(sum / samples); - sum = 0; - count = 0; - ready = true; - } - }}>; + using input = dev::adc<{.trigger = avr::adc::trigger::free_running, + .on_conversion = + [](std::uint16_t value) { + sum = sum + value; + count = count + 1; + if (count >= samples) { + window = static_cast(sum / samples); + sum = 0; + count = 0; + ready = true; + } + }}, + avr::adc::input>; + // The bound input, whose start() is free-running's one kick. + using thermistor = input::in; // The finished average (raw 10-bit), once per window. static bool take(std::uint16_t &value) { - avr::irq::atomic_guard lock; - if (!ready) + avr::irq::interrupt_guard lock; + if (!ready) { return false; + } value = window; ready = false; return true; @@ -62,7 +66,10 @@ class sampler { using fan = dev::pwm; -using serial_t = dev::uart0<{.baud = 115200_Bd, .rx_buffer = 32, .max_baud_error = 2.5_pct}>; +// 115200 at 16 MHz lands +2.1 % off, past the receiver-tolerance table the +// solver holds rates to - the rate this board has always spoken, so the +// override states that it is meant. +using serial_t = dev::uart0<{.baud = 115200_Bd, .rx_buffer = 32, .allow_baud_error = true}>; inline constexpr serial_t serial{}; } // namespace app diff --git a/src/bootloader.hpp b/src/bootloader.hpp index 4a73131..6b258ae 100644 --- a/src/bootloader.hpp +++ b/src/bootloader.hpp @@ -1,7 +1,5 @@ #pragma once -#include - #include #include "board.hpp" @@ -11,7 +9,7 @@ // The legacy firmware did this with a watchdog reset: `bootloader` armed the // watchdog and hung, and the next boot noticed WDRF and jumped to the boot // section. That works for TinySafeBoot and **does not work for pureboot**, which -// deliberately hands straight back to the application on WDRF — an unattended +// deliberately hands straight back to the application on WDRF - an unattended // board that watchdog-resets in a loop must not sit in a loader instead of // running. So a reset-based route into pureboot opens no window at all, and on a // board whose only way in is the firmware that is a lockout. @@ -22,7 +20,7 @@ // The address is this board's, and it is not the legacy one: the loader lives in // the top 512 bytes at 0x7e00 (`hfuse d4` puts the boot section at 0x7c00 with // pureboot's staging slot below its own slot). The legacy firmware probed 0x7800 -// — a 2 KB boot section's base — which on this board reads erased, so its check +// - a 2 KB boot section's base - which on this board reads erased, so its check // was always false and its `bootloader` command never actually arrived anywhere. namespace app { @@ -32,14 +30,14 @@ class bootloader { using guard = dev::watchdog<{.timeout = 16_ms}>; // The top 512 bytes. An erased slot reads 0xffff, which is not an - // instruction any loader begins with — so this asks "is a loader installed" + // instruction any loader begins with - so this asks "is a loader installed" // rather than "is it the one I expect", which is the check the legacy // firmware got wrong in the other direction by testing one specific byte. static constexpr std::uint16_t base = 0x7e00; static bool present() { - return pgm_read_word(base) != 0xffff; + return avr::flash_load(reinterpret_cast(base)) != 0xffff; } // A function pointer holds a word address on AVR, so the byte address @@ -52,24 +50,25 @@ class bootloader { } public: - // Call first thing in main. reset_cause() reads *and clears* MCUSR, which + // Call first thing in main. consume_reset_cause() reads *and clears* MCUSR, which // matters on its own: a lingering WDRF forces the watchdog back on at its // shortest timeout. The diversion below is a leftover of the legacy route - // and is kept only because it is free and cannot hurt — with BOOTRST + // and is kept only because it is free and cannot hurt - with BOOTRST // programmed the loader has already run before this line, so nothing // normally reaches it. static void handle_reset() { - auto cause = avr::power::reset_cause(); + auto cause = avr::power::consume_reset_cause(); guard::disable(); - if (cause.watchdog && present()) + if (cause.watchdog && present()) { call(reinterpret_cast(base / 2)); + } } // Hand over for real: no reset, so no WDRF for the loader to refuse. [[noreturn]] static void enter() { - // Interrupts first — the receive vector and the timer live in this + // Interrupts first - the receive vector and the timer live in this // application's vector table, and once the loader is running there is no // application to vector into. avr::irq::disable(); @@ -77,7 +76,7 @@ class bootloader { // Release the USART. While TXEN0 is set the peripheral owns PD1, not the // port register, so a loader that bit-bangs the same pin receives - // perfectly and answers into nothing — mute, not deaf, and unverifiable + // perfectly and answers into nothing - mute, not deaf, and unverifiable // from the host. pureboot clears this itself; TinySafeBoot, which is what // this board still carries, does not. Four bytes make the hand-over work // for either one, which is the only reason this route can be tested diff --git a/src/controller.hpp b/src/controller.hpp index 1a0795f..db616a3 100644 --- a/src/controller.hpp +++ b/src/controller.hpp @@ -6,7 +6,7 @@ #include "curve.hpp" #include "thermistor.hpp" -// Control loop: averaged thermistor samples → temperature → fan duty +// Control loop: averaged thermistor samples -> temperature -> fan duty // through the curve table (auto) or a console-set value (manual). namespace app { @@ -20,27 +20,29 @@ class controller { public: static void init() { - fan::duty(avr::percent_t{10000}); // full blast until the first reading + fan::set_duty(avr::percent_t{10000}); // full blast until the first reading } static void poll() { std::uint16_t sample; - if (!sampler::take(sample)) + if (!sampler::take(sample)) { return; + } adc_average = sample; temp_quarters = thermistor::quarters(sample); have_data = true; - if (auto_mode) + if (auto_mode) { percent = curve::duty(static_cast((temp_quarters + 2) / 4)); - fan::duty(avr::percent_t{static_cast(percent * 100)}); + } + fan::set_duty(avr::percent_t{static_cast(percent * 100)}); } static void set_manual(std::uint8_t p) { auto_mode = false; percent = p; - fan::duty(avr::percent_t{static_cast(p * 100)}); + fan::set_duty(avr::percent_t{static_cast(p * 100)}); } static void set_automatic() diff --git a/src/curve.hpp b/src/curve.hpp index 5dce19f..53f44d9 100644 --- a/src/curve.hpp +++ b/src/curve.hpp @@ -5,9 +5,9 @@ #include -// The auto-mode fan curve, tabulated at compile time: a cubic in °C -// (0.002246·x³ − 0.09·x² + 0.91·x, zero below 20 °C) as a flash_table of -// duty percent per °C. +// The auto-mode fan curve, tabulated at compile time: a cubic in C +// (0.002246*x^3 - 0.09*x^2 + 0.91*x, zero below 20 C) as a flash_table of +// duty percent per C. namespace app::curve { namespace detail { @@ -15,33 +15,39 @@ namespace detail { consteval std::uint8_t duty_entry(int celsius) { double x = celsius; - if (x < 20) + if (x < 20) { return 0; + } double duty = 0.002246 * x * x * x - 0.09 * x * x + 0.91 * x; - if (duty < 0) + if (duty < 0) { duty = 0; - if (duty > 100) + } + if (duty > 100) { duty = 100; + } return static_cast(duty + 0.5); } inline constexpr avr::flash_table<[] { std::array out{}; - for (int t = 0; t < 100; ++t) + for (int t = 0; t < 100; ++t) { out[static_cast(t)] = duty_entry(t); + } return out; }()> table; } // namespace detail -// Duty percent for a temperature (clamped to the 0..99 °C table window). +// Duty percent for a temperature (clamped to the 0..99 C table window). inline std::uint8_t duty(std::int8_t celsius) { - if (celsius < 0) + if (celsius < 0) { celsius = 0; - if (celsius > 99) + } + if (celsius > 99) { celsius = 99; + } return detail::table[static_cast(celsius)]; } diff --git a/src/main.cpp b/src/main.cpp index bbb54a6..70ee500 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -16,14 +16,15 @@ int main() avr::init(); avr::irq::enable(); - sampler::input::start(); + sampler::thermistor::start(); controller::init(); terminal::init(); while (true) { controller::poll(); - if (controller::data_available()) + if (controller::data_available()) { statistics::record(static_cast((controller::temperature_quarters() + 2) / 4)); + } terminal::poll(); } } diff --git a/src/statistics.hpp b/src/statistics.hpp index 1ac35b1..9763d5b 100644 --- a/src/statistics.hpp +++ b/src/statistics.hpp @@ -7,9 +7,9 @@ #include "board.hpp" -// Temperature histogram: one uint32 bucket per °C 0..99, sampled once a +// Temperature histogram: one uint32 bucket per C 0..99, sampled once a // second, written back to EEPROM every 30 minutes (update() only touches -// changed bytes). Erased EEPROM reads back as 0xffffffff — treated as 0. +// changed bytes). Erased EEPROM reads back as 0xffffffff - treated as 0. namespace app { class statistics { @@ -34,9 +34,11 @@ class statistics { static void init() { histogram = stored::read(); - for (auto &bucket : histogram) - if (bucket == 0xffffffff) + for (auto &bucket : histogram) { + if (bucket == 0xffffffff) { bucket = 0; + } + } } static void record(std::int8_t celsius) @@ -65,34 +67,41 @@ class statistics { static std::uint8_t min_temperature() { - for (std::uint8_t i = 0; i < range; ++i) - if (histogram[i]) + for (std::uint8_t i = 0; i < range; ++i) { + if (histogram[i]) { return i; + } + } return range; } static std::uint8_t max_temperature() { - for (std::uint8_t i = range; i > 0; --i) - if (histogram[i - 1]) + for (std::uint8_t i = range; i > 0; --i) { + if (histogram[i - 1]) { return i - 1; + } + } return 0; } static std::uint64_t total_samples() { std::uint64_t total = 0; - for (auto bucket : histogram) + for (auto bucket : histogram) { total += bucket; + } return total; } static std::uint32_t highest_bucket() { std::uint32_t highest = 0; - for (auto bucket : histogram) - if (bucket > highest) + for (auto bucket : histogram) { + if (bucket > highest) { highest = bucket; + } + } return highest; } diff --git a/src/terminal.hpp b/src/terminal.hpp index 41c27ea..e64412b 100644 --- a/src/terminal.hpp +++ b/src/terminal.hpp @@ -20,14 +20,15 @@ namespace app { class terminal { static constexpr char ctrl_c = 0x03; - static constexpr std::uint8_t line_max = 24; - static inline char line[line_max]{}; + static constexpr char backspace = 0x08; + static constexpr char del = 0x7f; + static inline std::array line{}; static inline std::uint8_t at = 0; static inline bool overflowed = false; static inline bool monitoring = false; static inline std::uint64_t last_monitor = 0; - // Commands, in the order they are matched — which is the order the original + // Commands, in the order they are matched - which is the order the original // firmware matched them in, and that order is load-bearing. An abbreviation // resolves to the *first* entry it prefixes, so `s` is show (not statistics, // not set) exactly as it always was, and anything appended to this list @@ -55,7 +56,7 @@ class terminal { // Column the descriptions' colons line up in, counted from the start of the // name. The longest name is `bootloader` at 10, so 12 leaves it a space and - // one dot — the original's layout exactly. + // one dot - the original's layout exactly. static constexpr std::uint8_t help_column = 12; static void prompt() @@ -63,27 +64,29 @@ class terminal { serial << "> "_P; } - // `name ....: ` — the dotted label the original used everywhere it printed a + // `name ....: ` - the dotted label the original used everywhere it printed a // list of things, which is what makes a column of values readable without // counting spaces. One renderer for all three users; only the column differs. static void label(std::string_view text, std::uint8_t column) { serial << text << ' '; - for (auto i = text.size() + 1; i < column; ++i) + for (auto i = text.size() + 1; i < column; ++i) { serial << '.'; + } serial << ": "_P; } // The same, for a label that is a literal rather than a command name: it // stays in flash, and its width comes from the type, so the padding needs no - // hand-counted constant. The command names cannot use this — they are + // hand-counted constant. The command names cannot use this - they are // string_views because they are matched at run time. template static void label(Flash text, std::uint8_t column) { serial << text << ' '; - for (auto i = Flash::size + 1; i < column; ++i) + for (auto i = Flash::size + 1; i < column; ++i) { serial << '.'; + } serial << ": "_P; } @@ -92,13 +95,14 @@ class terminal { label(name, help_column); } - // Quarter-°C as a signed decimal with a two-digit fraction. The sign is taken + // Quarter- C as a signed decimal with a two-digit fraction. The sign is taken // off first: C++ gives a negative remainder for a negative dividend, so // `(q % 4) * 25` on -40.25 C yields -25 and prints "-40.-25". static void temperature(std::int16_t quarters) { - if (quarters < 0) + if (quarters < 0) { serial << '-'; + } auto magnitude = static_cast(quarters < 0 ? -quarters : quarters); serial << magnitude / 4 << '.' << avr::dec<{.width = 2, .fill = '0'}>((magnitude % 4) * 25); } @@ -142,8 +146,9 @@ class terminal { static std::uint32_t resistance() { auto adc = controller::last_adc(); - if (adc >= 1023) + if (adc >= 1023) { return 0xffffffff; // open circuit: the divider has no solution + } return static_cast(thermistor::series_resistor) * adc / (1023u - adc); } @@ -160,10 +165,11 @@ class terminal { serial << controller::last_adc() << " / 1023\r\n"_P; label("Resistance"_P, reading_column); - if (auto ohms = resistance(); ohms == open_circuit) + if (auto ohms = resistance(); ohms == open_circuit) { serial << "open circuit\r\n"_P; - else + } else { serial << ohms << " Ohm\r\n"_P; + } label("Temperature"_P, reading_column); temperature(controller::temperature_quarters()); @@ -171,10 +177,11 @@ class terminal { label("Fan speed"_P, reading_column); serial << controller::fan_percent() << "% "_P; - if (controller::automatic()) + if (controller::automatic()) { serial << "auto\r\n"_P; - else + } else { serial << "manual\r\n"_P; + } } // Every whole degree from 10 to 60 with a bar, which is the original's and is @@ -187,8 +194,9 @@ class terminal { auto duty = curve::duty(static_cast(t)); serial << avr::dec<{.width = 2, .fill = '0'}>(t) << " C = "_P << avr::dec<{.width = 3, .fill = ' '}>(duty) << "% |"_P; - for (std::uint8_t i = 0; i < duty; ++i) + for (std::uint8_t i = 0; i < duty; ++i) { serial << '#'; + } serial << "\r\n"_P; } } @@ -204,16 +212,18 @@ class terminal { { auto empty = statistics::total_samples() == 0; label("Minimum temperature"_P, stat_column); - if (empty) + if (empty) { serial << "not available\r\n"_P; - else + } else { serial << statistics::min_temperature() << " C\r\n"_P; + } label("Maximum temperature"_P, stat_column); - if (empty) + if (empty) { serial << "not available\r\n"_P; - else + } else { serial << statistics::max_temperature() << " C\r\n"_P; + } label("Total samples"_P, stat_column); serial << static_cast(statistics::total_samples()) << "\r\n"_P; @@ -232,8 +242,9 @@ class terminal { // neighbouring buckets, which on a distribution this narrow is the whole // picture. std::uint32_t factor = highest / bar_max > 1 ? highest / bar_max : 1; - while (highest / factor > bar_max) + while (highest / factor > bar_max) { ++factor; + } for (std::uint8_t t = statistics::min_temperature(); t <= statistics::max_temperature(); ++t) { auto count = statistics::bucket(t); @@ -241,8 +252,9 @@ class terminal { // instead of trailing off the ragged right-hand end of the bars. serial << avr::dec<{.width = 2, .fill = '0'}>(t) << " C : "_P << avr::dec<{.width = 10, .fill = ' '}>(count) << " |"_P; - for (std::uint32_t i = 0; i < count / factor; ++i) + for (std::uint32_t i = 0; i < count / factor; ++i) { serial << '#'; + } serial << "\r\n"_P; } } @@ -253,20 +265,22 @@ class terminal { // starts_with, not substr: substr throws std::out_of_range, and one // potentially-throwing call is enough to pull in std::terminate, which does // not exist in a freestanding AVR build. The link fails rather than the - // firmware, so this is a build-time trap rather than a runtime one — but it + // firmware, so this is a build-time trap rather than a runtime one - but it // is a trap, and the whole file avoids substr for that reason. static bool matches(std::string_view input, const command &c) { - if (input.empty()) + if (input.empty()) { return false; - if (c.exact) + } + if (c.exact) { return input == c.name; + } return c.name.starts_with(input); } static void dispatch(std::string_view input) { - // A line that overflowed the buffer is not a command — it is the tail of + // A line that overflowed the buffer is not a command - it is the tail of // one. Acting on it is how a truncated `reset` becomes a surprise. if (overflowed) { serial << "input too long, ignored\r\n"_P; @@ -275,21 +289,23 @@ class terminal { } // Split on the first space with the (pointer, length) constructor rather - // than substr, which throws — see matches(). + // than substr, which throws - see matches(). const auto space = input.find(' '); const auto word = space == std::string_view::npos ? input : std::string_view{input.data(), space}; const auto rest = space == std::string_view::npos ? std::string_view{} : std::string_view{input.data() + space + 1, input.size() - space - 1}; - if (word.empty()) + if (word.empty()) { return; + } std::uint8_t which = commands.size(); - for (std::uint8_t i = 0; i < commands.size(); ++i) + for (std::uint8_t i = 0; i < commands.size(); ++i) { if (matches(word, commands[i])) { which = i; break; } + } switch (which) { case 0: @@ -331,8 +347,9 @@ class terminal { break; } percent = static_cast(percent * 10 + (c - '0')); - if (percent > 100) + if (percent > 100) { valid = false; + } } if (valid) { controller::set_manual(static_cast(percent)); @@ -411,7 +428,7 @@ class terminal { prompt(); // a bare Enter just reprompts, no gap needed continue; } - dispatch(std::string_view{line, at}); + dispatch(std::string_view{line.data(), at}); at = 0; if (!monitoring) { // A blank line between a command's output and the next @@ -420,13 +437,13 @@ class terminal { serial << "\r\n"_P; prompt(); } - } else if (c == 0x7f || c == 0x08) { + } else if (c == del || c == backspace) { if (at) { --at; serial << "\b \b"_P; } } else if (c >= ' ') { - if (at < line_max) { + if (at < line.size()) { line[at++] = c; serial << c; // echo } else { diff --git a/src/thermistor.hpp b/src/thermistor.hpp index 3e0b469..fa3c18b 100644 --- a/src/thermistor.hpp +++ b/src/thermistor.hpp @@ -7,8 +7,8 @@ // NTC thermistor on a series divider, solved entirely at compile time: // the Beta equation (logarithm and all) runs consteval into a libavr -// flash_table — the firmware never does floating point. Raw 10-bit ADC -// counts map to quarter-°C with linear interpolation between table steps. +// flash_table - the firmware never does floating point. Raw 10-bit ADC +// counts map to quarter- C with linear interpolation between table steps. namespace app::thermistor { inline constexpr double series_resistor = 9951; @@ -27,35 +27,40 @@ consteval double temperature_of(double adc) return 1.0 / steinhart - 273.15; } -// 256 entries over the 10-bit range (steps of 4 counts), quarter-°C, +// 256 entries over the 10-bit range (steps of 4 counts), quarter- C, // clamped to a sane sensor window; entry 256 mirrors 255 so interpolation // at full scale has a right neighbour. consteval std::int16_t quarters_entry(int index) { double adc = index * 4.0; - if (adc < 4) + if (adc < 4) { adc = 4; - if (adc > 1019) + } + if (adc > 1019) { adc = 1019; + } double t = temperature_of(adc) * 4.0; - if (t < -40 * 4) + if (t < -40 * 4) { t = -40 * 4; - if (t > 125 * 4) + } + if (t > 125 * 4) { t = 125 * 4; + } return static_cast(t < 0 ? t - 0.5 : t + 0.5); } inline constexpr avr::flash_table<[] { std::array out{}; - for (int i = 0; i < 257; ++i) + for (int i = 0; i < 257; ++i) { out[static_cast(i)] = quarters_entry(i < 256 ? i : 255); + } return out; }()> table; } // namespace detail -// Temperature in quarter-°C from a raw (or averaged) 10-bit sample. +// Temperature in quarter- C from a raw (or averaged) 10-bit sample. inline std::int16_t quarters(std::uint16_t adc) { std::uint16_t index = adc >> 2; // the 257th entry backs index+1 at full scale diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index c89daf1..367b73e 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -1,6 +1,6 @@ # The board has no reset line and no programming header, so the loader-entry # route in the emitted image is the only thing standing between a firmware change -# and an unreflashable board. It has been wrong before — see the script. +# and an unreflashable board. It has been wrong before - see the script. find_package(Python3 COMPONENTS Interpreter) if(Python3_FOUND) add_test(NAME fantemp.reachability @@ -8,5 +8,5 @@ if(Python3_FOUND) --objdump ${CMAKE_OBJDUMP} --elf $ --image $/fantemp.bin) else() - message(STATUS "Python not found — the reachability check is skipped") + message(STATUS "Python not found - the reachability check is skipped") endif() diff --git a/test/check_reachability.py b/test/check_reachability.py index 96a5d02..44b57a4 100644 --- a/test/check_reachability.py +++ b/test/check_reachability.py @@ -3,7 +3,7 @@ This board has no reset line and no programming header. The single route to the bootloader is the running firmware's `bootloader` command, so a firmware that -gets that route wrong is a board that cannot be reflashed — and the failure is +gets that route wrong is a board that cannot be reflashed - and the failure is silent, because everything else still works. It has been wrong before. The firmware this one replaces probed and jumped to @@ -15,7 +15,7 @@ hardware, which is what this replaces. Three properties, all read out of the disassembly rather than the source: 1. The image ends below the boot section. `hfuse d4` puts that at 0x7c00, so an - application reaching into it would be overwritten by the loader — or worse, + application reaching into it would be overwritten by the loader - or worse, executed at reset, since BOOTRST points there. 2. The hand-over targets the loader base. A word address of 0x3f00 is byte 0x7e00; anything else is the 0x7800 bug again. @@ -60,7 +60,7 @@ def main() -> int: text = subprocess.run([args.objdump, "-d", str(args.elf)], capture_output=True, text=True, check=True).stdout - # The address the hand-over actually targets, read at its call sites — not + # The address the hand-over actually targets, read at its call sites - not # "does the image contain this byte somewhere", which proves nothing: 0x3f is # an ordinary constant that appears in the curve tables, so a check like that # passes just as happily on the 0x7800 bug it is supposed to catch. @@ -83,7 +83,7 @@ def main() -> int: want = LOADER_BASE // 2 if not sites: - failures.append("no call to bootloader::call with a loaded target — the " + failures.append("no call to bootloader::call with a loaded target - the " "hand-over could not be read out of the image") elif wrong := [a for a in sites if a != want]: failures.append(f"the hand-over targets word {[hex(a) for a in wrong]} " @@ -95,13 +95,13 @@ def main() -> int: # An icall/ijmp has to exist for that address to be jumped to indirectly. if not re.search(r"\b(icall|ijmp)\b", text): - failures.append("no icall/ijmp — the hand-over cannot reach across flash") + failures.append("no icall/ijmp - the hand-over cannot reach across flash") else: print(" ok an indirect call exists (a relative one cannot reach)") # What actually reaches WDTCSR, not what the image happens to load somewhere. # A timed disable writes WDCE|WDE (0x18) and then zero. Arming writes WDE - # *without* WDCE — including 0x08, a 16 ms timeout with every prescaler bit + # *without* WDCE - including 0x08, a 16 ms timeout with every prescaler bit # clear, which is precisely what the legacy route used and is why this cannot # be a check for "a prescaler is present". WDCE, WDE = 0x10, 0x08 @@ -114,14 +114,14 @@ def main() -> int: values.append(0 if reg == "r1" else held.get(reg)) armed = [v for v in values if v is not None and (v & WDE) and not (v & WDCE)] if armed: - failures.append(f"WDTCSR is written {[hex(v) for v in armed]} — WDE without " + failures.append(f"WDTCSR is written {[hex(v) for v in armed]} - WDE without " f"WDCE is arming the watchdog, and a reset-based hand-over " f"opens no pureboot window") elif not values: print(" ok the watchdog is never written") else: print(f" ok WDTCSR writes are {[hex(v) if v is not None else '?' for v in values]}" - f" — unlock and clear, never an arm") + f" - unlock and clear, never an arm") for line in failures: print(f" FAIL {line}")