Hoernchen has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/21117 )


Change subject: asf4 uart: properly handle uart errors
......................................................................

asf4 uart: properly handle uart errors

Uart will now lead to a (temporary) deactivation of the uart as well as
the deactivatin of the card + a a proper ccid response to notify the
host if the slot was busy.

Change-Id: Ia0efef03829b68d2b4f25899bb933b14fb9e0bd1
---
M ccid_common/ccid_slot_fsm.c
M ccid_common/cuart.h
M ccid_common/iso7816_fsm.c
M sysmoOCTSIM/cuart_driver_asf4_usart_async.c
4 files changed, 97 insertions(+), 21 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-ccid-firmware 
refs/changes/17/21117/1

diff --git a/ccid_common/ccid_slot_fsm.c b/ccid_common/ccid_slot_fsm.c
index 4cf75bc..8dccfa5 100644
--- a/ccid_common/ccid_slot_fsm.c
+++ b/ccid_common/ccid_slot_fsm.c
@@ -147,12 +147,18 @@
        msgb_free(msg);
        /* continues in iso_fsm_clot_user_cb once ATR is received */
 }
+
 static void iso_fsm_clot_user_cb(struct osmo_fsm_inst *fi, int event, int 
cause, void *data)
 {
        struct iso_fsm_slot *ss = iso7816_fsm_get_user_priv(fi);
        struct ccid_slot *cs = ss->cs;

        switch (event) {
+       /* special case: not handled as a normal callback below, in case slot 
was busy the fsm will
+        * additionally emit a proper error event handled below to notify the 
host */
+       case ISO7816_E_HW_ERR_IND:
+               card_uart_ctrl(ss->cuart, CUART_CTL_NO_RXTX, true);
+               break;
        case ISO7816_E_ATR_DONE_IND:
        case ISO7816_E_ATR_ERR_IND:
        case ISO7816_E_TPDU_DONE_IND:
@@ -219,10 +225,14 @@
                break;
        case ISO7816_E_ATR_ERR_IND:
                tpdu = data;
-               LOGPCS(cs, LOGL_DEBUG, "%s(event=%d, data=%s)\n", __func__, 
event,
-                       msgb_hexdump(tpdu));
-               resp = ccid_gen_data_block(cs, ss->seq, CCID_CMD_STATUS_FAILED, 
CCID_ERR_ICC_MUTE,
-                                          msgb_data(tpdu), msgb_length(tpdu));
+               LOGPCS(cs, LOGL_DEBUG, "%s(event=%d, data=%s)\n", __func__, 
event, msgb_hexdump(tpdu));
+
+               /* perform deactivation */
+               card_uart_ctrl(ss->cuart, CUART_CTL_RST, true);
+               card_uart_ctrl(ss->cuart, CUART_CTL_POWER_5V0, false);
+               cs->icc_powered = false;
+
+               resp = ccid_gen_data_block(cs, ss->seq, CCID_CMD_STATUS_FAILED, 
CCID_ERR_ICC_MUTE, msgb_data(tpdu), msgb_length(tpdu));
                ccid_slot_send_unbusy(cs, resp);
                cs->event = 0;
                break;
@@ -237,8 +247,13 @@
                break;
        case ISO7816_E_TPDU_FAILED_IND:
                tpdu = data;
-               LOGPCS(cs, LOGL_DEBUG, "%s(event=%d, data=%s)\n", __func__, 
event,
-                       msgb_hexdump(tpdu));
+
+               /* perform deactivation */
+               card_uart_ctrl(ss->cuart, CUART_CTL_RST, true);
+               card_uart_ctrl(ss->cuart, CUART_CTL_POWER_5V0, false);
+               cs->icc_powered = false;
+
+               LOGPCS(cs, LOGL_DEBUG, "%s(event=%d, data=%s)\n", __func__, 
event, msgb_hexdump(tpdu));
                /* FIXME: other error causes than card removal?*/
                resp = ccid_gen_data_block(cs, ss->seq, CCID_CMD_STATUS_FAILED, 
CCID_ERR_ICC_MUTE, msgb_l2(tpdu), 0);
                ccid_slot_send_unbusy(cs, resp);
diff --git a/ccid_common/cuart.h b/ccid_common/cuart.h
index c0a3a52..c8573c5 100644
--- a/ccid_common/cuart.h
+++ b/ccid_common/cuart.h
@@ -36,6 +36,7 @@
        CUART_E_RX_TIMEOUT,
        /* an entire block of data was written, as instructed in prior 
card_uart_tx() call */
        CUART_E_TX_COMPLETE,
+       CUART_E_HW_ERROR, /* might be uart parity or mystery error, might be 
something else */
 };

 extern const struct value_string card_uart_event_vals[];
diff --git a/ccid_common/iso7816_fsm.c b/ccid_common/iso7816_fsm.c
index 8e113b0..7207245 100644
--- a/ccid_common/iso7816_fsm.c
+++ b/ccid_common/iso7816_fsm.c
@@ -276,6 +276,9 @@
        case CUART_E_TX_COMPLETE:
                osmo_fsm_inst_dispatch(fi, ISO7816_E_TX_COMPL, data);
                break;
+       case CUART_E_HW_ERROR:
+               osmo_fsm_inst_dispatch(fi, ISO7816_E_HW_ERR_IND, data);
+               break;
        }
 }

@@ -361,7 +364,7 @@
        struct iso7816_3_priv *ip = get_iso7816_3_priv(fi);
        OSMO_ASSERT(fi->fsm == &iso7816_3_fsm);
        card_uart_ctrl(ip->uart, CUART_CTL_RX_TIMER_HINT, 0);
-       card_uart_ctrl(ip->uart, CUART_CTL_NO_RXTX, true);
+
        /* reset the TPDU state machine */
        osmo_fsm_inst_dispatch(ip->tpdu_fi, ISO7816_E_TPDU_CLEAR_REQ, NULL);
 }
@@ -424,6 +427,9 @@

        switch (event) {
        case ISO7816_E_HW_ERR_IND:
+               /* deactivates uart, then continues with the callbacks below 
for proper error reporting */
+               ip->user_cb(fi, ISO7816_E_HW_ERR_IND, 0, 0);
+               /* no break */
        case ISO7816_E_CARD_REMOVAL:
                /* FIXME: power off? */
                if(fi->state == ISO7816_S_WAIT_ATR || fi->state == 
ISO7816_S_IN_ATR)
diff --git a/sysmoOCTSIM/cuart_driver_asf4_usart_async.c 
b/sysmoOCTSIM/cuart_driver_asf4_usart_async.c
index 64ec9f2..42c2110 100644
--- a/sysmoOCTSIM/cuart_driver_asf4_usart_async.c
+++ b/sysmoOCTSIM/cuart_driver_asf4_usart_async.c
@@ -56,11 +56,10 @@
 #include <hpl_usart_sync.h>


-static void _SIM_error_cb(const struct usart_async_descriptor *const descr){
-       volatile uint32_t status = 
hri_sercomusart_read_STATUS_reg(descr->device.hw);
-       volatile uint32_t data = 
hri_sercomusart_read_DATA_reg(descr->device.hw);
-       volatile uint32_t errrs = 
hri_sercomusart_read_RXERRCNT_reg(descr->device.hw);
-       OSMO_ASSERT(0 == 1)
+static void _SIM_error_cb(const struct usart_async_descriptor *const io_descr, 
uint8_t slot_nr) {
+       struct card_uart *cuart = cuart4slot_nr(slot_nr);
+       OSMO_ASSERT(cuart);
+       card_uart_notification(cuart, CUART_E_HW_ERROR, 0);
 }

 /* the below ugli-ness is required as the usart_async_descriptor doesn't have
@@ -139,6 +138,44 @@
        SIM4_tx_cb, SIM5_tx_cb, SIM6_tx_cb, SIM7_tx_cb,
 };

+static void SIM0_error_cb(const struct usart_async_descriptor *const io_descr)
+{
+       _SIM_error_cb(io_descr, 0);
+}
+static void SIM1_error_cb(const struct usart_async_descriptor *const io_descr)
+{
+       _SIM_error_cb(io_descr, 1);
+}
+static void SIM2_error_cb(const struct usart_async_descriptor *const io_descr)
+{
+       _SIM_error_cb(io_descr, 2);
+}
+static void SIM3_error_cb(const struct usart_async_descriptor *const io_descr)
+{
+       _SIM_error_cb(io_descr, 3);
+}
+static void SIM4_error_cb(const struct usart_async_descriptor *const io_descr)
+{
+       _SIM_error_cb(io_descr, 4);
+}
+static void SIM5_error_cb(const struct usart_async_descriptor *const io_descr)
+{
+       _SIM_error_cb(io_descr, 5);
+}
+static void SIM6_error_cb(const struct usart_async_descriptor *const io_descr)
+{
+       _SIM_error_cb(io_descr, 6);
+}
+static void SIM7_error_cb(const struct usart_async_descriptor *const io_descr)
+{
+       _SIM_error_cb(io_descr, 7);
+}
+static usart_cb_t SIM_error_cb[8] = {
+       SIM0_error_cb, SIM1_error_cb, SIM2_error_cb, SIM3_error_cb,
+       SIM4_error_cb, SIM5_error_cb, SIM6_error_cb, SIM7_error_cb,
+};
+
+
 #include <math.h>
 #include "atmel_start.h"
 #include "atmel_start_pins.h"
@@ -182,9 +219,14 @@
 static bool slot_set_baudrate(struct card_uart *cuart, uint32_t baudrate)
 {
        uint8_t slotnr = cuart->u.asf4.slot_nr;
-
+       struct usart_async_descriptor* slot = 
SIM_peripheral_descriptors[slotnr];
+       Sercom *sercom = cuart->u.asf4.usa_pd->device.hw;
        ASSERT(slotnr < ARRAY_SIZE(SIM_peripheral_descriptors));

+       if (NULL == slot) {
+               return false;
+       }
+
        // calculate the error corresponding to the clock sources
        uint16_t bauds[ARRAY_SIZE(sercom_glck_freqs)];
        double errors[ARRAY_SIZE(sercom_glck_freqs)];
@@ -218,11 +260,6 @@
                return false;
        }

-       // set clock and baud rate
-       struct usart_async_descriptor* slot = 
SIM_peripheral_descriptors[slotnr]; // get slot
-       if (NULL == slot) {
-               return false;
-       }

        // update cached values
        cuart->u.asf4.current_baudrate = baudrate;
@@ -231,7 +268,7 @@
        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]);

        /* only wait if the uart is enabled.... */
-       if (hri_sercomusart_get_CTRLA_reg(slot->device.hw, 
SERCOM_USART_CTRLA_ENABLE)) {
+       if (hri_sercomusart_get_CTRLA_reg(sercom, SERCOM_USART_CTRLA_ENABLE)) {
                while (!usart_async_is_tx_empty(slot)); // wait for 
transmission to complete (WARNING no timeout)
                usart_async_disable(slot); // disable SERCOM peripheral
        }
@@ -241,6 +278,17 @@
        // it does not seem we need to completely disable the peripheral using 
hri_mclk_clear_APBDMASK_SERCOMn_bit
        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
        usart_async_set_baud_rate(slot, bauds[best]); // set the new baud rate
+
+       /* clear pending errors that happened while
+        * - the interrupt was off (inverse ATR? -> parity error)
+        *  -- OR --
+        * - the uart was disabled due to a hw error
+        * and enable it*/
+       hri_sercomusart_clear_interrupt_ERROR_bit(sercom);
+       hri_sercomusart_clear_STATUS_reg(sercom, 0xff);
+       volatile uint8_t dummy = hri_sercomusart_read_RXERRCNT_reg(sercom);
+
+       usart_async_flush_rx_buffer(cuart->u.asf4.usa_pd);
        usart_async_enable(slot); // re-enable SERCOM peripheral

        return true;
@@ -330,7 +378,7 @@

        usart_async_register_callback(usa_pd, USART_ASYNC_RXC_CB, 
SIM_rx_cb[slot_nr]);
        usart_async_register_callback(usa_pd, USART_ASYNC_TXC_CB, 
SIM_tx_cb[slot_nr]);
-       usart_async_register_callback(usa_pd, USART_ASYNC_ERROR_CB, 
_SIM_error_cb);
+       usart_async_register_callback(usa_pd, USART_ASYNC_ERROR_CB, 
SIM_error_cb[slot_nr]);
        usart_async_enable(usa_pd);

        // set USART baud rate to match the interface (f = 2.5 MHz) and card 
default settings (Fd = 372, Dd = 1)
@@ -385,7 +433,13 @@

        switch (ctl) {
        case CUART_CTL_NO_RXTX:
-               /* no op */
+
+               /* immediately disables the uart, useful for hw errors (parity, 
colllision, ...)
+                * uart is automatically re-enabled during slot_set_isorate 
which also clears the errors
+                * when resetting the slot which happens automatically during 
error callback handling or powerup
+                * so the uart usually does not stay disabled for a long time */
+               if (arg)
+                       _usart_async_disable(&cuart->u.asf4.usa_pd->device);
                break;
        case CUART_CTL_RX:
                if (arg){

--
To view, visit https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/21117
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-ccid-firmware
Gerrit-Branch: master
Gerrit-Change-Id: Ia0efef03829b68d2b4f25899bb933b14fb9e0bd1
Gerrit-Change-Number: 21117
Gerrit-PatchSet: 1
Gerrit-Owner: Hoernchen <[email protected]>
Gerrit-MessageType: newchange

Reply via email to