Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 78 additions & 9 deletions components/kaos/radio/src/radio_ao.c
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,10 @@ typedef struct {
QTimeEvt c5_boot; /**< C5_BOOT_SIG: the C5 just let go has booted. */
bool c5_booting; /**< That one-shot is running. */
#endif
QTimeEvt updated; /**< RADIO_UPDATED_SIG: the C5 runs its update. */
bool c5_updated; /**< An update ended this boot: the C5 runs an
* image only a link can confirm, so nothing
* resets it before the restart. */
uint8_t inst; /**< inst_phase_t */
bool inst_pending; /**< Verified and owed the channel (a debt). */
uint32_t inst_size; /**< Bytes the host promised to put on the wire.*/
Expand Down Expand Up @@ -890,6 +894,11 @@ QActive * const AO_Radio = &l_radio.super;
* link: the pulse esp-hosted itself gives the chip's enable line. */
#define RADIO_EXP_PULSE_MS 10U

/** How long after an update's teardown the board restarts: the C5 restarts
* into the new image that long after answering the activate, and boots it.
* A restart before that would link to the old image on its way out. */
#define RADIO_UPDATE_RESTART_MS (KDRV_RADIO_C5_ACTIVATE_MS + KDRV_RADIO_C5_BOOT_MS)

/* The receive protocol's answers, static and immutable: they cannot fail to
* allocate, and the console has given up its own watchdog for this command,
* so one of them must always be able to arrive. */
Expand Down Expand Up @@ -8225,8 +8234,22 @@ static void c5_update_done(Radio * const me)
KLOG_N(TAG, "C5 update done: the new image is written and activated; "
"the C5 is booting it");
}
KLOG_N(TAG, "`radio_up` brings the link back, and `c5` will then name "
"what the C5 now runs");
KLOG_N(TAG, "the board restarts by itself in %u ms, and `c5` will then "
"name what the C5 now runs", RADIO_UPDATE_RESTART_MS);
}

/**
* @brief The C5 has had time to boot its update (RADIO_UPDATED_SIG): ask
* AO_Boot to restart the board.
*
* Asked in either top state, so an update always ends in a restart, whatever
* brought the link back meanwhile.
*/
static void radio_update_restart(void)
{
KLOG_N(TAG, "the C5 runs its update: the board restarts so a link "
"reaches it");
QACTIVE_POST(AO_Boot, boot_exit_evt(), AO_Radio);
}

/**
Expand Down Expand Up @@ -8344,6 +8367,8 @@ void Radio_ctor(void)
me->c5_booting = false;
me->amp_asked = false;
#endif
QTimeEvt_ctorX(&me->updated, &me->super, RADIO_UPDATED_SIG, 0U);
me->c5_updated = false;
for ( uint8_t svc = 0U; KODE_SVC_COUNT > svc; svc++ )
{
QTimeEvt_ctorX(&me->svc_tmo[svc], &me->super, KODE_SVC_TMO_SIG, 0U);
Expand Down Expand Up @@ -9631,9 +9656,21 @@ static QState Radio_initial(Radio * const me, void const * const par)

KLOG_D(TAG, "boot: wifi_want=%d ble_want=%d",
(int)me->wifi_want, (int)me->ble_want);
if ( (me->wifi_want || me->ble_want) && radio_begin_connect(me) )
{
return Q_TRAN(&Radio_starting);
/* The C5 runs an update that only a request over the link confirms:
* the link comes up even with neither service wanted. */
bool const update_waits = kdrv_radio_note_update_waits();

if ( update_waits && !me->wifi_want && !me->ble_want )
{
KLOG_N(TAG, "the C5 runs an update to confirm: the link comes up "
"for it");
}
if ( (me->wifi_want || me->ble_want || update_waits)
&& radio_begin_connect(me) )
{
return Q_TRAN(&Radio_starting);
}
}
return Q_TRAN(&Radio_off);
}
Expand Down Expand Up @@ -9720,13 +9757,29 @@ static QState Radio_down(Radio * const me, QEvt const * const e)
}
#endif
case RADIO_PARK_REQ_SIG: {
/* No link, so no station of ours to keep: the jump goes on and
* the next firmware resets the C5. */
KLOG_I(TAG, "no link to the C5: nothing to park");
if ( me->c5_updated )
{
/* Whoever runs next must not reset the C5 before its link
* has reached the new image. */
kdrv_radio_note_leave(KDRV_RADIO_NOTE_UPDATE);
KLOG_N(TAG, "C5 left running its update: the next boot's "
"link confirms it");
}
else
{
/* No link, so no station of ours to keep: the jump goes on
* and the next firmware resets the C5. */
KLOG_I(TAG, "no link to the C5: nothing to park");
}
boot_radio_parked();
status = Q_HANDLED();
break;
}
case RADIO_UPDATED_SIG: {
radio_update_restart();
status = Q_HANDLED();
break;
}
case ID_SIGN_REQ_SIG:
case ID_REQ_SIG: {
/* The identity lives on the C5, and the link is down. */
Expand Down Expand Up @@ -10168,8 +10221,10 @@ static QState Radio_off(Radio * const me, QEvt const * const e)
* draws nothing and its amplifier is shut down: the first thing a
* boot with no radio does (C5_EN is pulled up, so the C5 started
* with the board). The bring-up and C5_RUN_REQ_SIG let it go;
* refused, harmlessly, while esp-hosted has the reset line. */
if ( !me->wifi_want && !me->ble_want && !me->amp_asked )
* refused, harmlessly, while esp-hosted has the reset line. Never
* after an update: the hold would roll the new image back. */
if ( !me->wifi_want && !me->ble_want && !me->amp_asked
&& !me->c5_updated )
{
(void)radio_hold_c5(me);
}
Expand Down Expand Up @@ -11186,6 +11241,11 @@ static QState Radio_up(Radio * const me, QEvt const * const e)
QState status;
switch ( e->sig )
{
case RADIO_UPDATED_SIG: {
radio_update_restart();
status = Q_HANDLED();
break;
}
case Q_ENTRY_SIG: {
if ( (uint8_t)INST_LINKING == me->inst )
{
Expand Down Expand Up @@ -13051,6 +13111,15 @@ static QState Radio_updating(Radio * const me, QEvt const * const e)
/* The C5 restarted into the new image: what IO5
* holds is that image's, not known here. */
me->exp_known = false;
/* A closed C5 rolls the image back at its next reset
* unless a request has reached it first, and this boot
* has no link left to send one. The board restarts
* and the next boot's link sends it; until then
* nothing resets the C5. */
me->c5_updated = true;
QTimeEvt_armX(&me->updated,
KQP_TICKS_PER_MS(RADIO_UPDATE_RESTART_MS),
0U);
status = Q_TRAN(&Radio_off);
}
break;
Expand Down
43 changes: 33 additions & 10 deletions components/kdrv/radio/include/kdrv_radio.h
Original file line number Diff line number Diff line change
Expand Up @@ -277,35 +277,58 @@ bool kdrv_radio_c5_held(void);
/** How long the C5 is given to boot after its reset line is released:
* esp-hosted's own wait (CONFIG_ESP_HOSTED_SDIO_RESET_DELAY_MS), asserted
* against it in kdrv_radio.c. */
/* cppcheck-suppress misra-c2012-2.5 ; read by kdrv_radio.c alone, no twin */
#define KDRV_RADIO_C5_BOOT_MS 1500U

/** How long the C5 waits after answering an activate before it restarts into
* the new image: esp-hosted's slave arms a one-shot of this length
* (RESTART_TIMEOUT, slave_control.c) and then calls esp_restart(). */
#define KDRV_RADIO_C5_ACTIVATE_MS 2000U

/**
* @brief The role of the firmware a deliberate restart goes to.
* @brief What a deliberate restart tells the next firmware about the C5.
*
* A note names it, and a firmware keeps a parked C5 only when the note names
* its own role: the only firmware that can read a note meant for another is
* one built before the note existed, which ignores it.
* A jump names the role of the firmware it goes to, and a firmware keeps a
* parked C5 only when the note names its own role: the only firmware that can
* read a note meant for another is one built before the note existed, which
* ignores it. An update names no role.
*/
typedef enum {
KDRV_RADIO_NOTE_OS = 1, /**< A jump home: kodeOS. */
KDRV_RADIO_NOTE_APP = 2, /**< A jump to an app. */
KDRV_RADIO_NOTE_OS = 1, /**< A jump home: kodeOS. */
KDRV_RADIO_NOTE_APP = 2, /**< A jump to an app. */
KDRV_RADIO_NOTE_UPDATE = 3, /**< The C5 runs an update it has not
* confirmed: a closed C5 confirms it at the
* first KODE request, and its bootloader
* goes back to the old image if a reset
* comes first. */
} kdrv_radio_note_t;

/**
* @brief Leave the handover note for the next firmware.
*
* Call it only after the C5 has answered the park with OK, right before the
* restart. The note is one word in a register of the chip's
* Call it right before the restart: for a jump, only after the C5 has
* answered the park with OK; for an update, once the C5 has restarted into
* the new image. The note is one word in a register of the chip's
* low-power domain: it survives esp_restart() and every reset but a power-on,
* whichever firmware runs next. The next firmware's first link reads it and erases it,
* and keeps the C5 running only when every condition of the rule holds;
* otherwise it resets the C5 as it always did.
*
* @param target The role of the firmware the restart goes to.
* @param target What the restart leaves the C5 doing.
*/
void kdrv_radio_note_leave(kdrv_radio_note_t target);

/**
* @brief Whether this boot found an update note its first link has not read
* yet.
*
* Reads the note without erasing it, so a boot that wants neither Wi-Fi nor
* BLE can still bring the link up once, and the new image is confirmed
* instead of held in reset and given up.
*
* @return true for a KDRV_RADIO_NOTE_UPDATE note after a software reset.
*/
bool kdrv_radio_note_update_waits(void);

/**
* @brief Make the lock that gives synchronous RPCs to the C5 their turns
* (kdrv_radio_rpc.c). Once is enough, more is harmless; called by the
Expand Down
16 changes: 16 additions & 0 deletions components/kdrv/radio/src/kdrv_radio.c
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,13 @@ void kdrv_radio_note_leave(kdrv_radio_note_t target)
kdrv_radio_note_reg_write(kradio_note_pack((uint32_t)target));
}

bool kdrv_radio_note_update_waits(void)
{
kradio_note_t const note = kradio_note_unpack(kdrv_radio_note_reg_read());

return kradio_note_is_update(&note, esp_reset_reason() == ESP_RST_SW);
}

bool kdrv_radio_adopted(void)
{
return s_radio.adopted;
Expand Down Expand Up @@ -525,6 +532,10 @@ static kode_status_t radio_platform_init(void)
* every later one resets the C5. Runs before esp-hosted takes the line, which
* is the only time kdrv_radio_c5_hold() is allowed.
*
* A C5 kept for an update is not parked: it booted the new image by itself
* and nobody has spoken to it yet, so it is kept only so that the first
* request reaches that image, and there is nothing of it to adopt.
*
* @param wants_wifi This boot wants Wi-Fi.
* @return true when the C5 was left running, parked by the last firmware.
*/
Expand All @@ -544,6 +555,11 @@ static bool radio_c5_decide(bool wants_wifi)

kdrv_radio_note_reg_write(0U);

if ( !held && kradio_note_is_update(&note, reset_was_sw) )
{
KLOG_N(TAG, "C5 kept: it runs an update its first request confirms");
return false;
}
if ( !held &&
kradio_note_keeps_c5(&note, reset_was_sw, role_is_os, wants_wifi) )
{
Expand Down
17 changes: 17 additions & 0 deletions components/kdrv/radio/src/kdrv_radio_note.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,17 @@ kradio_note_t kradio_note_unpack(uint32_t word)
return note;
}

/* cppcheck-suppress misra-c2012-8.7 ; a file of its own so the host suite can compile the rule; kdrv_radio.c calls it */
bool kradio_note_is_update(const kradio_note_t *note, bool reset_was_sw)
{
if ( (NULL == note) || !reset_was_sw )
{
return false;
}
return (KRADIO_NOTE_MAGIC == note->magic)
&& ((uint32_t)KDRV_RADIO_NOTE_UPDATE == note->target);
}

/* cppcheck-suppress misra-c2012-8.7 ; a file of its own so the host suite can compile the rule; kdrv_radio.c calls it */
bool kradio_note_keeps_c5(const kradio_note_t *note, bool reset_was_sw,
bool role_is_os, bool wants_wifi)
Expand All @@ -41,6 +52,12 @@ bool kradio_note_keeps_c5(const kradio_note_t *note, bool reset_was_sw,
{
return false;
}
/* The new image is confirmed by the first request that reaches it, and a
* reset before that is what gives it up: no wish outranks that. */
if ( kradio_note_is_update(note, reset_was_sw) )
{
return true;
}
if ( !reset_was_sw || !wants_wifi )
{
return false;
Expand Down
20 changes: 17 additions & 3 deletions components/kdrv/radio/src/kdrv_radio_note.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,26 @@ uint32_t kradio_note_pack(uint32_t target);
*/
kradio_note_t kradio_note_unpack(uint32_t word);

/**
* @brief Whether the note says the C5 runs an update it has not confirmed.
*
* True when the note's magic is there, its target is KDRV_RADIO_NOTE_UPDATE
* and the reset was a software one; after any other reset the register's word
* says nothing.
*
* @param note The note as read at boot.
* @param reset_was_sw The reset reason is a software reset.
* @return true for an update's note.
*/
bool kradio_note_is_update(const kradio_note_t *note, bool reset_was_sw);

/**
* @brief Whether the C5 stays running at this boot's first link.
*
* True only when every condition holds: the note's magic is there, its target
* is a known role and this firmware's own, the reset was a software one, and
* this boot wants Wi-Fi. Any other case resets the C5.
* True for an update's note (kradio_note_is_update()), whatever the role and
* the wish. Otherwise true only when every condition holds: the note's magic
* is there, its target is a known role and this firmware's own, the reset was
* a software one, and this boot wants Wi-Fi. Any other case resets the C5.
*
* @param note The note as read at boot (before it was erased).
* @param reset_was_sw The reset reason is a software reset.
Expand Down
6 changes: 6 additions & 0 deletions components/kqp/include/kqp_signals.h
Original file line number Diff line number Diff line change
Expand Up @@ -1439,6 +1439,12 @@ enum KodeSignals {
C5_BOOT_SIG, /**< AO_Radio private one-shot QTimeEvt: the
* C5 it just let go has had its boot
* time. */
RADIO_UPDATED_SIG, /**< AO_Radio private one-shot QTimeEvt: the
* C5 has restarted into the update it was
* given and booted it; AO_Boot is asked to
* reset the board, so the next link
* reaches the new image before any reset
* of the C5. */
EXP_PULSE_SIG, /**< AO_Radio private one-shot QTimeEvt: the
* C5 reset that switches the expansion
* header's supply on has lasted long
Expand Down
46 changes: 42 additions & 4 deletions tests/host/cases/test_kdrv_radio_note.c
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,51 @@ TEST_CASE("a boot that does not want Wi-Fi resets the C5", "[radio_note]")

TEST_CASE("an unknown target resets the C5", "[radio_note]")
{
kradio_note_t zero = note_for(0U);
kradio_note_t three = note_for(3U);
kradio_note_t zero = note_for(0U);
kradio_note_t four = note_for(4U);

TEST_ASSERT_FALSE(kradio_note_keeps_c5(&zero, true, true, true));
TEST_ASSERT_FALSE(kradio_note_keeps_c5(&zero, true, false, true));
TEST_ASSERT_FALSE(kradio_note_keeps_c5(&three, true, true, true));
TEST_ASSERT_FALSE(kradio_note_keeps_c5(&three, true, false, true));
TEST_ASSERT_FALSE(kradio_note_keeps_c5(&four, true, true, true));
TEST_ASSERT_FALSE(kradio_note_keeps_c5(&four, true, false, true));
}

TEST_CASE("an update's note after a software reset keeps the C5 whatever the "
"role and the wish", "[radio_note]")
{
kradio_note_t upd = note_for((uint32_t)KDRV_RADIO_NOTE_UPDATE);

TEST_ASSERT_TRUE(kradio_note_keeps_c5(&upd, true, true, true));
TEST_ASSERT_TRUE(kradio_note_keeps_c5(&upd, true, false, true));
TEST_ASSERT_TRUE_MESSAGE(kradio_note_keeps_c5(&upd, true, true, false),
"a boot with no Wi-Fi wanted still owes the new image its first request");
TEST_ASSERT_TRUE(kradio_note_keeps_c5(&upd, true, false, false));
}

TEST_CASE("an update's note after any other reset resets the C5",
"[radio_note]")
{
kradio_note_t upd = note_for((uint32_t)KDRV_RADIO_NOTE_UPDATE);

TEST_ASSERT_FALSE(kradio_note_keeps_c5(&upd, false, true, true));
TEST_ASSERT_FALSE(kradio_note_keeps_c5(&upd, false, false, false));
TEST_ASSERT_FALSE(kradio_note_is_update(&upd, false));
}

TEST_CASE("only an update's note with its magic is an update", "[radio_note]")
{
kradio_note_t upd = note_for((uint32_t)KDRV_RADIO_NOTE_UPDATE);
kradio_note_t os = note_for((uint32_t)KDRV_RADIO_NOTE_OS);
kradio_note_t app = note_for((uint32_t)KDRV_RADIO_NOTE_APP);
kradio_note_t wrong = kradio_note_unpack(0x4B510003UL);

TEST_ASSERT_TRUE(kradio_note_is_update(&upd, true));
TEST_ASSERT_FALSE(kradio_note_is_update(&os, true));
TEST_ASSERT_FALSE(kradio_note_is_update(&app, true));
TEST_ASSERT_FALSE(kradio_note_is_update(&wrong, true));
TEST_ASSERT_FALSE(kradio_note_is_update(NULL, true));
TEST_ASSERT_EQUAL_HEX32(0x4B500003UL,
kradio_note_pack((uint32_t)KDRV_RADIO_NOTE_UPDATE));
}

TEST_CASE("a note is one word: the magic above, the target below",
Expand Down
Loading
Loading