laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-e1-hardware/+/26854 ) Change subject: icE1usb fw: Update E1 core start/stop procedure ...................................................................... icE1usb fw: Update E1 core start/stop procedure Previously we were just doing a hard reset, calling init again. And to stop, it was a bit harsh as well, just calling init with 0 as argument, not cleaning pending descriptors and such. Now, the HW is initialized once and there is proper startup and shutdown procedure, leaving things in the proper state. Init of e1 hardware (call to e1_init) is also delegated to usb_e1 since it's that module that handles all state changes and config so it makes sense it handles init too rather than calling it from fw_app.c Signed-off-by: Sylvain Munaut &lt;tnt(a)246tNt.com&gt; Change-Id: I639f90ce3488a1a08e87854e74e0586010264f5d --- M firmware/ice40-riscv/icE1usb/e1.c M firmware/ice40-riscv/icE1usb/e1.h M firmware/ice40-riscv/icE1usb/fw_app.c M firmware/ice40-riscv/icE1usb/usb_e1.c 4 files changed, 87 insertions(+), 14 deletions(-) Approvals: Jenkins Builder: Verified laforge: Looks good to me, approved diff --git a/firmware/ice40-riscv/icE1usb/e1.c b/firmware/ice40-riscv/icE1usb/e1.c index 964b0bc..b1695a6 100644 --- a/firmware/ice40-riscv/icE1usb/e1.c +++ b/firmware/ice40-riscv/icE1usb/e1.c @@ -226,10 +226,11 @@ // ---------- enum e1_pipe_state { - IDLE = 0, /* not yet initialized */ - BOOT = 1, /* after e1_init(), regiters are programmed */ - RUN = 2, /* normal operation */ - RECOVER = 3, /* after underflow, overflow or alignment error */ + IDLE = 0, /* not running */ + STARTING = 1, /* after e1_start(), waiting for priming */ + RUN = 2, /* normal operation */ + RECOVER = 3, /* after underflow, overflow or alignment error */ + SHUTDOWN = 4, /* after e1_stop(), waiting for shutdown */ }; struct e1_state { @@ -324,8 +325,8 @@ e1f_init(&e1->tx.fifo, (512 * port) + 256, 256); /* Flow state */ - e1->rx.state = BOOT; - e1->tx.state = BOOT; + e1->rx.state = IDLE; + e1->tx.state = IDLE; /* Set config registers */ e1->rx.cr.cfg = rx_cr & RXCR_PERMITTED; @@ -357,6 +358,48 @@ e1_regs->tx.csr = e1->tx.cr.val; } +void +e1_start(int port) +{ + volatile struct e1_core *e1_regs = _get_regs(port); + struct e1_state *e1 = _get_state(port); + + /* Checks */ + while ((e1->rx.state == SHUTDOWN) || (e1->tx.state == SHUTDOWN)) + e1_poll(port); + + if ((e1->rx.state != IDLE) || (e1->tx.state != IDLE)) + panic("Invalid E1 hardware state (port=%d, rxs=%d, txs=%d)", + port, e1->rx.state, e1->tx.state); + + /* Clear FIFOs */ + e1f_reset(&e1->rx.fifo); + e1f_reset(&e1->tx.fifo); + + /* Flow state */ + e1->rx.state = STARTING; + e1->tx.state = STARTING; + + /* Update CRs */ + _e1_update_cr_val(port); + + e1_regs->rx.csr = e1->rx.cr.val | E1_RX_CR_OVFL_CLR; + e1_regs->tx.csr = e1->tx.cr.val | E1_TX_CR_UNFL_CLR; +} + +void +e1_stop(int port) +{ + struct e1_state *e1 = _get_state(port); + + /* Flow state */ + e1->rx.state = SHUTDOWN; + e1->tx.state = SHUTDOWN; + + /* Nothing else to do, e1_poll will stop submitting data and + * transition to IDLE when everything in-flight is done */ +} + unsigned int e1_rx_need_data(int port, unsigned int usb_addr, unsigned int max_frames, unsigned int *pos) { @@ -512,7 +555,7 @@ } /* Boot procedure */ - if (e1->tx.state == BOOT) { + if (e1->tx.state == STARTING) { if (e1f_unseen_frames(&e1->tx.fifo) < (16 * 5)) return; /* HACK: LED flow status */ @@ -521,6 +564,20 @@ } /* Handle RX */ + /* Bypass if OFF */ + if (e1->rx.state == IDLE) + goto done_rx; + + /* Shutdown */ + if (e1->rx.state == SHUTDOWN) { + if (e1->rx.in_flight == 0) { + e1->rx.state = IDLE; + _e1_update_cr_val(port); + e1_regs->rx.csr = e1->rx.cr.val; + } + goto done_rx; + } + /* Misalign ? */ if (e1->rx.state == RUN) { if (!(e1_regs->rx.csr & E1_RX_SR_ALIGNED)) { @@ -562,6 +619,20 @@ done_rx: /* Handle TX */ + /* Bypass if OFF */ + if (e1->tx.state == IDLE) + return; + + /* Shutdown */ + if (e1->tx.state == SHUTDOWN) { + if (e1->tx.in_flight == 0) { + e1->tx.state = IDLE; + _e1_update_cr_val(port); + e1_regs->tx.csr = e1->tx.cr.val; + } + return; + } + /* Underflow ? */ if (e1->tx.state == RUN) { if (e1_regs->tx.csr & E1_TX_SR_UNFL) { diff --git a/firmware/ice40-riscv/icE1usb/e1.h b/firmware/ice40-riscv/icE1usb/e1.h index a44c319..0ed43c4 100644 --- a/firmware/ice40-riscv/icE1usb/e1.h +++ b/firmware/ice40-riscv/icE1usb/e1.h @@ -11,6 +11,9 @@ /* control */ void e1_init(int port, uint16_t rx_cr, uint16_t tx_cr); +void e1_start(int port); +void e1_stop(int port); + void e1_poll(int port); void e1_debug_print(int port, bool data); diff --git a/firmware/ice40-riscv/icE1usb/fw_app.c b/firmware/ice40-riscv/icE1usb/fw_app.c index f531d08..5e9812c 100644 --- a/firmware/ice40-riscv/icE1usb/fw_app.c +++ b/firmware/ice40-riscv/icE1usb/fw_app.c @@ -101,8 +101,6 @@ usb_e1_init(); /* Start */ - e1_init(0, 0, 0); - e1_init(1, 0, 0); led_state(true); usb_connect(); diff --git a/firmware/ice40-riscv/icE1usb/usb_e1.c b/firmware/ice40-riscv/icE1usb/usb_e1.c index 9dc6f1d..96ca72a 100644 --- a/firmware/ice40-riscv/icE1usb/usb_e1.c +++ b/firmware/ice40-riscv/icE1usb/usb_e1.c @@ -289,15 +289,12 @@ switch (usb_e1->running) { case false: /* Disable E1 rx/tx */ - e1_init(port, 0, 0); + e1_stop(port); break; case true: /* Reset and Re-Enable E1 */ - e1_init(port, - _rx_config_reg(&rx_cfg_default), - _tx_config_reg(&tx_cfg_default) - ); + e1_start(port); /* Reset BDI */ usb_e1->in_bdi = 0; @@ -464,11 +461,15 @@ void usb_e1_init(void) { + uint32_t rx_cr = _rx_config_reg(&rx_cfg_default); + uint32_t tx_cr = _tx_config_reg(&tx_cfg_default); + for (int i=0; i<2; i++) { struct usb_e1_state *usb_e1 = _get_state(i); memset(usb_e1, 0x00, sizeof(struct usb_e1_state)); usb_e1->tx_cfg = tx_cfg_default; usb_e1->rx_cfg = rx_cfg_default; + e1_init(i, rx_cr, tx_cr); } usb_register_function_driver(&_e1_drv); -- To view, visit https://gerrit.osmocom.org/c/osmo-e1-hardware/+/26854 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-e1-hardware Gerrit-Branch: master Gerrit-Change-Id: I639f90ce3488a1a08e87854e74e0586010264f5d Gerrit-Change-Number: 26854 Gerrit-PatchSet: 2 Gerrit-Owner: tnt &lt;tnt(a)246tNt.com&gt; Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge &lt;laforge(a)osmocom.org&gt; Gerrit-Reviewer: tnt &lt;tnt(a)246tNt.com&gt; Gerrit-MessageType: merged