<p>laforge <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/21117">View Change</a></p><div style="white-space:pre-wrap">Approvals:
laforge: Looks good to me, approved
Jenkins Builder: Verified
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">asf4 uart: properly handle uart errors<br><br>Uart errors will now lead to a (temporary) deactivation of the uart as well as<br>the deactivatin of the card + a a proper ccid response to notify the<br>host if the slot was busy.<br><br>Change-Id: Ia0efef03829b68d2b4f25899bb933b14fb9e0bd1<br>---<br>M ccid_common/ccid_slot_fsm.c<br>M ccid_common/cuart.h<br>M ccid_common/iso7816_fsm.c<br>M sysmoOCTSIM/cuart_driver_asf4_usart_async.c<br>4 files changed, 97 insertions(+), 21 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/ccid_common/ccid_slot_fsm.c b/ccid_common/ccid_slot_fsm.c</span><br><span>index 4cf75bc..8dccfa5 100644</span><br><span>--- a/ccid_common/ccid_slot_fsm.c</span><br><span>+++ b/ccid_common/ccid_slot_fsm.c</span><br><span>@@ -147,12 +147,18 @@</span><br><span> msgb_free(msg);</span><br><span> /* continues in iso_fsm_clot_user_cb once ATR is received */</span><br><span> }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> static void iso_fsm_clot_user_cb(struct osmo_fsm_inst *fi, int event, int cause, void *data)</span><br><span> {</span><br><span> struct iso_fsm_slot *ss = iso7816_fsm_get_user_priv(fi);</span><br><span> struct ccid_slot *cs = ss->cs;</span><br><span> </span><br><span> switch (event) {</span><br><span style="color: hsl(120, 100%, 40%);">+ /* special case: not handled as a normal callback below, in case slot was busy the fsm will</span><br><span style="color: hsl(120, 100%, 40%);">+ * additionally emit a proper error event handled below to notify the host */</span><br><span style="color: hsl(120, 100%, 40%);">+ case ISO7816_E_HW_ERR_IND:</span><br><span style="color: hsl(120, 100%, 40%);">+ card_uart_ctrl(ss->cuart, CUART_CTL_NO_RXTX, true);</span><br><span style="color: hsl(120, 100%, 40%);">+ break;</span><br><span> case ISO7816_E_ATR_DONE_IND:</span><br><span> case ISO7816_E_ATR_ERR_IND:</span><br><span> case ISO7816_E_TPDU_DONE_IND:</span><br><span>@@ -219,10 +225,14 @@</span><br><span> break;</span><br><span> case ISO7816_E_ATR_ERR_IND:</span><br><span> tpdu = data;</span><br><span style="color: hsl(0, 100%, 40%);">- LOGPCS(cs, LOGL_DEBUG, "%s(event=%d, data=%s)\n", __func__, event,</span><br><span style="color: hsl(0, 100%, 40%);">- msgb_hexdump(tpdu));</span><br><span style="color: hsl(0, 100%, 40%);">- resp = ccid_gen_data_block(cs, ss->seq, CCID_CMD_STATUS_FAILED, CCID_ERR_ICC_MUTE,</span><br><span style="color: hsl(0, 100%, 40%);">- msgb_data(tpdu), msgb_length(tpdu));</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPCS(cs, LOGL_DEBUG, "%s(event=%d, data=%s)\n", __func__, event, msgb_hexdump(tpdu));</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* perform deactivation */</span><br><span style="color: hsl(120, 100%, 40%);">+ card_uart_ctrl(ss->cuart, CUART_CTL_RST, true);</span><br><span style="color: hsl(120, 100%, 40%);">+ card_uart_ctrl(ss->cuart, CUART_CTL_POWER_5V0, false);</span><br><span style="color: hsl(120, 100%, 40%);">+ cs->icc_powered = false;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ resp = ccid_gen_data_block(cs, ss->seq, CCID_CMD_STATUS_FAILED, CCID_ERR_ICC_MUTE, msgb_data(tpdu), msgb_length(tpdu));</span><br><span> ccid_slot_send_unbusy(cs, resp);</span><br><span> cs->event = 0;</span><br><span> break;</span><br><span>@@ -237,8 +247,13 @@</span><br><span> break;</span><br><span> case ISO7816_E_TPDU_FAILED_IND:</span><br><span> tpdu = data;</span><br><span style="color: hsl(0, 100%, 40%);">- LOGPCS(cs, LOGL_DEBUG, "%s(event=%d, data=%s)\n", __func__, event,</span><br><span style="color: hsl(0, 100%, 40%);">- msgb_hexdump(tpdu));</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* perform deactivation */</span><br><span style="color: hsl(120, 100%, 40%);">+ card_uart_ctrl(ss->cuart, CUART_CTL_RST, true);</span><br><span style="color: hsl(120, 100%, 40%);">+ card_uart_ctrl(ss->cuart, CUART_CTL_POWER_5V0, false);</span><br><span style="color: hsl(120, 100%, 40%);">+ cs->icc_powered = false;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPCS(cs, LOGL_DEBUG, "%s(event=%d, data=%s)\n", __func__, event, msgb_hexdump(tpdu));</span><br><span> /* FIXME: other error causes than card removal?*/</span><br><span> resp = ccid_gen_data_block(cs, ss->seq, CCID_CMD_STATUS_FAILED, CCID_ERR_ICC_MUTE, msgb_l2(tpdu), 0);</span><br><span> ccid_slot_send_unbusy(cs, resp);</span><br><span>diff --git a/ccid_common/cuart.h b/ccid_common/cuart.h</span><br><span>index c0a3a52..c8573c5 100644</span><br><span>--- a/ccid_common/cuart.h</span><br><span>+++ b/ccid_common/cuart.h</span><br><span>@@ -36,6 +36,7 @@</span><br><span> CUART_E_RX_TIMEOUT,</span><br><span> /* an entire block of data was written, as instructed in prior card_uart_tx() call */</span><br><span> CUART_E_TX_COMPLETE,</span><br><span style="color: hsl(120, 100%, 40%);">+ CUART_E_HW_ERROR, /* might be uart parity or mystery error, might be something else */</span><br><span> };</span><br><span> </span><br><span> extern const struct value_string card_uart_event_vals[];</span><br><span>diff --git a/ccid_common/iso7816_fsm.c b/ccid_common/iso7816_fsm.c</span><br><span>index 8e113b0..7207245 100644</span><br><span>--- a/ccid_common/iso7816_fsm.c</span><br><span>+++ b/ccid_common/iso7816_fsm.c</span><br><span>@@ -276,6 +276,9 @@</span><br><span> case CUART_E_TX_COMPLETE:</span><br><span> osmo_fsm_inst_dispatch(fi, ISO7816_E_TX_COMPL, data);</span><br><span> break;</span><br><span style="color: hsl(120, 100%, 40%);">+ case CUART_E_HW_ERROR:</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_fsm_inst_dispatch(fi, ISO7816_E_HW_ERR_IND, data);</span><br><span style="color: hsl(120, 100%, 40%);">+ break;</span><br><span> }</span><br><span> }</span><br><span> </span><br><span>@@ -361,7 +364,7 @@</span><br><span> struct iso7816_3_priv *ip = get_iso7816_3_priv(fi);</span><br><span> OSMO_ASSERT(fi->fsm == &iso7816_3_fsm);</span><br><span> card_uart_ctrl(ip->uart, CUART_CTL_RX_TIMER_HINT, 0);</span><br><span style="color: hsl(0, 100%, 40%);">- card_uart_ctrl(ip->uart, CUART_CTL_NO_RXTX, true);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /* reset the TPDU state machine */</span><br><span> osmo_fsm_inst_dispatch(ip->tpdu_fi, ISO7816_E_TPDU_CLEAR_REQ, NULL);</span><br><span> }</span><br><span>@@ -424,6 +427,9 @@</span><br><span> </span><br><span> switch (event) {</span><br><span> case ISO7816_E_HW_ERR_IND:</span><br><span style="color: hsl(120, 100%, 40%);">+ /* deactivates uart, then continues with the callbacks below for proper error reporting */</span><br><span style="color: hsl(120, 100%, 40%);">+ ip->user_cb(fi, ISO7816_E_HW_ERR_IND, 0, 0);</span><br><span style="color: hsl(120, 100%, 40%);">+ /* no break */</span><br><span> case ISO7816_E_CARD_REMOVAL:</span><br><span> /* FIXME: power off? */</span><br><span> if(fi->state == ISO7816_S_WAIT_ATR || fi->state == ISO7816_S_IN_ATR)</span><br><span>diff --git a/sysmoOCTSIM/cuart_driver_asf4_usart_async.c b/sysmoOCTSIM/cuart_driver_asf4_usart_async.c</span><br><span>index 64ec9f2..42c2110 100644</span><br><span>--- a/sysmoOCTSIM/cuart_driver_asf4_usart_async.c</span><br><span>+++ b/sysmoOCTSIM/cuart_driver_asf4_usart_async.c</span><br><span>@@ -56,11 +56,10 @@</span><br><span> #include <hpl_usart_sync.h></span><br><span> </span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-static void _SIM_error_cb(const struct usart_async_descriptor *const descr){</span><br><span style="color: hsl(0, 100%, 40%);">- volatile uint32_t status = hri_sercomusart_read_STATUS_reg(descr->device.hw);</span><br><span style="color: hsl(0, 100%, 40%);">- volatile uint32_t data = hri_sercomusart_read_DATA_reg(descr->device.hw);</span><br><span style="color: hsl(0, 100%, 40%);">- volatile uint32_t errrs = hri_sercomusart_read_RXERRCNT_reg(descr->device.hw);</span><br><span style="color: hsl(0, 100%, 40%);">- OSMO_ASSERT(0 == 1)</span><br><span style="color: hsl(120, 100%, 40%);">+static void _SIM_error_cb(const struct usart_async_descriptor *const io_descr, uint8_t slot_nr) {</span><br><span style="color: hsl(120, 100%, 40%);">+ struct card_uart *cuart = cuart4slot_nr(slot_nr);</span><br><span style="color: hsl(120, 100%, 40%);">+ OSMO_ASSERT(cuart);</span><br><span style="color: hsl(120, 100%, 40%);">+ card_uart_notification(cuart, CUART_E_HW_ERROR, 0);</span><br><span> }</span><br><span> </span><br><span> /* the below ugli-ness is required as the usart_async_descriptor doesn't have</span><br><span>@@ -139,6 +138,44 @@</span><br><span> SIM4_tx_cb, SIM5_tx_cb, SIM6_tx_cb, SIM7_tx_cb,</span><br><span> };</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+static void SIM0_error_cb(const struct usart_async_descriptor *const io_descr)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ _SIM_error_cb(io_descr, 0);</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+static void SIM1_error_cb(const struct usart_async_descriptor *const io_descr)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ _SIM_error_cb(io_descr, 1);</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+static void SIM2_error_cb(const struct usart_async_descriptor *const io_descr)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ _SIM_error_cb(io_descr, 2);</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+static void SIM3_error_cb(const struct usart_async_descriptor *const io_descr)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ _SIM_error_cb(io_descr, 3);</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+static void SIM4_error_cb(const struct usart_async_descriptor *const io_descr)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ _SIM_error_cb(io_descr, 4);</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+static void SIM5_error_cb(const struct usart_async_descriptor *const io_descr)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ _SIM_error_cb(io_descr, 5);</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+static void SIM6_error_cb(const struct usart_async_descriptor *const io_descr)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ _SIM_error_cb(io_descr, 6);</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+static void SIM7_error_cb(const struct usart_async_descriptor *const io_descr)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ _SIM_error_cb(io_descr, 7);</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+static usart_cb_t SIM_error_cb[8] = {</span><br><span style="color: hsl(120, 100%, 40%);">+ SIM0_error_cb, SIM1_error_cb, SIM2_error_cb, SIM3_error_cb,</span><br><span style="color: hsl(120, 100%, 40%);">+ SIM4_error_cb, SIM5_error_cb, SIM6_error_cb, SIM7_error_cb,</span><br><span style="color: hsl(120, 100%, 40%);">+};</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> #include <math.h></span><br><span> #include "atmel_start.h"</span><br><span> #include "atmel_start_pins.h"</span><br><span>@@ -182,9 +219,14 @@</span><br><span> static bool slot_set_baudrate(struct card_uart *cuart, uint32_t baudrate)</span><br><span> {</span><br><span> uint8_t slotnr = cuart->u.asf4.slot_nr;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(120, 100%, 40%);">+ struct usart_async_descriptor* slot = SIM_peripheral_descriptors[slotnr];</span><br><span style="color: hsl(120, 100%, 40%);">+ Sercom *sercom = cuart->u.asf4.usa_pd->device.hw;</span><br><span> ASSERT(slotnr < ARRAY_SIZE(SIM_peripheral_descriptors));</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ if (NULL == slot) {</span><br><span style="color: hsl(120, 100%, 40%);">+ return false;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> // calculate the error corresponding to the clock sources</span><br><span> uint16_t bauds[ARRAY_SIZE(sercom_glck_freqs)];</span><br><span> double errors[ARRAY_SIZE(sercom_glck_freqs)];</span><br><span>@@ -218,11 +260,6 @@</span><br><span> return false;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- // set clock and baud rate</span><br><span style="color: hsl(0, 100%, 40%);">- struct usart_async_descriptor* slot = SIM_peripheral_descriptors[slotnr]; // get slot</span><br><span style="color: hsl(0, 100%, 40%);">- if (NULL == slot) {</span><br><span style="color: hsl(0, 100%, 40%);">- return false;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span> </span><br><span> // update cached values</span><br><span> cuart->u.asf4.current_baudrate = baudrate;</span><br><span>@@ -231,7 +268,7 @@</span><br><span> printf("(%u) switching SERCOM clock to GCLK%u (freq = %lu kHz) and baud rate to %lu bps (baud = %u)\r\n", slotnr, (best + 1) * 2, (uint32_t)(round(sercom_glck_freqs[best] / 1000)), baudrate, bauds[best]);</span><br><span> </span><br><span> /* only wait if the uart is enabled.... */</span><br><span style="color: hsl(0, 100%, 40%);">- if (hri_sercomusart_get_CTRLA_reg(slot->device.hw, SERCOM_USART_CTRLA_ENABLE)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (hri_sercomusart_get_CTRLA_reg(sercom, SERCOM_USART_CTRLA_ENABLE)) {</span><br><span> while (!usart_async_is_tx_empty(slot)); // wait for transmission to complete (WARNING no timeout)</span><br><span> usart_async_disable(slot); // disable SERCOM peripheral</span><br><span> }</span><br><span>@@ -241,6 +278,17 @@</span><br><span> // it does not seem we need to completely disable the peripheral using hri_mclk_clear_APBDMASK_SERCOMn_bit</span><br><span> hri_gclk_write_PCHCTRL_reg(GCLK, SIM_peripheral_GCLK_ID[slotnr], sercom_glck_sources[best] | (1 << GCLK_PCHCTRL_CHEN_Pos)); // set peripheral core clock and re-enable it</span><br><span> usart_async_set_baud_rate(slot, bauds[best]); // set the new baud rate</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* clear pending errors that happened while</span><br><span style="color: hsl(120, 100%, 40%);">+ * - the interrupt was off (inverse ATR? -> parity error)</span><br><span style="color: hsl(120, 100%, 40%);">+ * -- OR --</span><br><span style="color: hsl(120, 100%, 40%);">+ * - the uart was disabled due to a hw error</span><br><span style="color: hsl(120, 100%, 40%);">+ * and enable it*/</span><br><span style="color: hsl(120, 100%, 40%);">+ hri_sercomusart_clear_interrupt_ERROR_bit(sercom);</span><br><span style="color: hsl(120, 100%, 40%);">+ hri_sercomusart_clear_STATUS_reg(sercom, 0xff);</span><br><span style="color: hsl(120, 100%, 40%);">+ volatile uint8_t dummy = hri_sercomusart_read_RXERRCNT_reg(sercom);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ usart_async_flush_rx_buffer(cuart->u.asf4.usa_pd);</span><br><span> usart_async_enable(slot); // re-enable SERCOM peripheral</span><br><span> </span><br><span> return true;</span><br><span>@@ -330,7 +378,7 @@</span><br><span> </span><br><span> usart_async_register_callback(usa_pd, USART_ASYNC_RXC_CB, SIM_rx_cb[slot_nr]);</span><br><span> usart_async_register_callback(usa_pd, USART_ASYNC_TXC_CB, SIM_tx_cb[slot_nr]);</span><br><span style="color: hsl(0, 100%, 40%);">- usart_async_register_callback(usa_pd, USART_ASYNC_ERROR_CB, _SIM_error_cb);</span><br><span style="color: hsl(120, 100%, 40%);">+ usart_async_register_callback(usa_pd, USART_ASYNC_ERROR_CB, SIM_error_cb[slot_nr]);</span><br><span> usart_async_enable(usa_pd);</span><br><span> </span><br><span> // set USART baud rate to match the interface (f = 2.5 MHz) and card default settings (Fd = 372, Dd = 1)</span><br><span>@@ -385,7 +433,13 @@</span><br><span> </span><br><span> switch (ctl) {</span><br><span> case CUART_CTL_NO_RXTX:</span><br><span style="color: hsl(0, 100%, 40%);">- /* no op */</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* immediately disables the uart, useful for hw errors (parity, colllision, ...)</span><br><span style="color: hsl(120, 100%, 40%);">+ * uart is automatically re-enabled during slot_set_isorate which also clears the errors</span><br><span style="color: hsl(120, 100%, 40%);">+ * when resetting the slot which happens automatically during error callback handling or powerup</span><br><span style="color: hsl(120, 100%, 40%);">+ * so the uart usually does not stay disabled for a long time */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (arg)</span><br><span style="color: hsl(120, 100%, 40%);">+ _usart_async_disable(&cuart->u.asf4.usa_pd->device);</span><br><span> break;</span><br><span> case CUART_CTL_RX:</span><br><span> if (arg){</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/21117">change 21117</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/21117"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-ccid-firmware </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Ia0efef03829b68d2b4f25899bb933b14fb9e0bd1 </div>
<div style="display:none"> Gerrit-Change-Number: 21117 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: Hoernchen <ewild@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>