<p>laforge <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/simtrace2/+/16076">View Change</a></p><div style="white-space:pre-wrap">Approvals:
  Jenkins Builder: Verified
  laforge: Looks good to me, approved

</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">initialize VCC, RST, and VCC with actual values<br><br>previously the card RST, VCC, and CLK signal states have been initialized with<br>default values corresponding to an inactive reader.<br>this worked fine for actual inactive readers since the default values match<br>and would be updated when the signal changes (edge detection).<br>but if the reader is in another state, card activation detection could fail.<br>this is fixed since the actual signal values are now used during initialisation.<br><br>at the same time I changed the variable type from uint8_t to boolean since they<br>have only two possible states, and understanding the actual state when coding<br>is simpler (no need to check which integer corresponds to which state).<br><br>this change has been successfully tested on the 2 slots of OWHW board.<br><br>Change-Id: Ie9245d75d48ae93d16f97897d4fa5ad6cd402e73<br>---<br>M firmware/libcommon/include/card_emu.h<br>M firmware/libcommon/source/card_emu.c<br>M firmware/libcommon/source/mode_cardemu.c<br>M firmware/test/card_emu_tests.c<br>4 files changed, 27 insertions(+), 20 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/firmware/libcommon/include/card_emu.h b/firmware/libcommon/include/card_emu.h</span><br><span>index a3c1cf2..e545593 100644</span><br><span>--- a/firmware/libcommon/include/card_emu.h</span><br><span>+++ b/firmware/libcommon/include/card_emu.h</span><br><span>@@ -29,8 +29,18 @@</span><br><span>      CARD_IO_CLK,</span><br><span> };</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-struct card_handle *card_emu_init(uint8_t slot_num, uint8_t tc_chan, uint8_t uart_chan,</span><br><span style="color: hsl(0, 100%, 40%);">-                             uint8_t in_ep, uint8_t irq_ep);</span><br><span style="color: hsl(120, 100%, 40%);">+/** initialise card slot</span><br><span style="color: hsl(120, 100%, 40%);">+ *  @param[in] slot_num slot number (arbitrary number)</span><br><span style="color: hsl(120, 100%, 40%);">+ *  @param[in] tc_chan timer counter channel (to measure the ETU)</span><br><span style="color: hsl(120, 100%, 40%);">+ *  @param[in] uart_chan UART peripheral channel</span><br><span style="color: hsl(120, 100%, 40%);">+ *  @param[in] in_ep USB IN end point number</span><br><span style="color: hsl(120, 100%, 40%);">+ *  @param[in] irq_ep USB INTerrupt end point number</span><br><span style="color: hsl(120, 100%, 40%);">+ *  @param[in] vcc_active initial VCC signal state (true = on)</span><br><span style="color: hsl(120, 100%, 40%);">+ *  @param[in] in_reset initial RST signal state (true = reset asserted)</span><br><span style="color: hsl(120, 100%, 40%);">+ *  @param[in] clocked initial CLK signat state (true = active)</span><br><span style="color: hsl(120, 100%, 40%);">+ *  @return main card handle reference</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+struct card_handle *card_emu_init(uint8_t slot_num, uint8_t tc_chan, uint8_t uart_chan, uint8_t in_ep, uint8_t irq_ep, bool vcc_active, bool in_reset, bool clocked);</span><br><span> </span><br><span> /* process a single byte received from the reader */</span><br><span> void card_emu_process_rx_byte(struct card_handle *ch, uint8_t byte);</span><br><span>diff --git a/firmware/libcommon/source/card_emu.c b/firmware/libcommon/source/card_emu.c</span><br><span>index b7d0e1f..087867f 100644</span><br><span>--- a/firmware/libcommon/source/card_emu.c</span><br><span>+++ b/firmware/libcommon/source/card_emu.c</span><br><span>@@ -182,9 +182,9 @@</span><br><span>   enum iso7816_3_card_state state;</span><br><span> </span><br><span>         /* signal levels */</span><br><span style="color: hsl(0, 100%, 40%);">-     uint8_t vcc_active;     /* 1 = on, 0 = off */</span><br><span style="color: hsl(0, 100%, 40%);">-   uint8_t in_reset;       /* 1 = RST low, 0 = RST high */</span><br><span style="color: hsl(0, 100%, 40%);">- uint8_t clocked;        /* 1 = active, 0 = inactive */</span><br><span style="color: hsl(120, 100%, 40%);">+        bool vcc_active;        /*< if VCC is active (true = active/ON) */</span><br><span style="color: hsl(120, 100%, 40%);">+ bool in_reset;  /*< if card is in reset (true = RST low/asserted, false = RST high/ released) */</span><br><span style="color: hsl(120, 100%, 40%);">+   bool clocked;   /*< if clock is active ( true = active, false = inactive) */</span><br><span> </span><br><span>  /* timing parameters, from PTS */</span><br><span>    uint8_t fi;</span><br><span>@@ -1126,8 +1126,7 @@</span><br><span> </span><br><span> static struct card_handle card_handles[NUM_SLOTS];</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-struct card_handle *card_emu_init(uint8_t slot_num, uint8_t tc_chan, uint8_t uart_chan,</span><br><span style="color: hsl(0, 100%, 40%);">-                                  uint8_t in_ep, uint8_t irq_ep)</span><br><span style="color: hsl(120, 100%, 40%);">+struct card_handle *card_emu_init(uint8_t slot_num, uint8_t tc_chan, uint8_t uart_chan, uint8_t in_ep, uint8_t irq_ep, bool vcc_active, bool in_reset, bool clocked)</span><br><span> {</span><br><span>  struct card_handle *ch;</span><br><span> </span><br><span>@@ -1140,14 +1139,13 @@</span><br><span> </span><br><span>    INIT_LLIST_HEAD(&ch->uart_tx_queue);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* initialize the card_handle with reasonable defaults */</span><br><span>    ch->num = slot_num;</span><br><span>       ch->irq_ep = irq_ep;</span><br><span>      ch->in_ep = in_ep;</span><br><span>        ch->state = ISO_S_WAIT_POWER;</span><br><span style="color: hsl(0, 100%, 40%);">-        ch->vcc_active = 0;</span><br><span style="color: hsl(0, 100%, 40%);">-  ch->in_reset = 1;</span><br><span style="color: hsl(0, 100%, 40%);">-    ch->clocked = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+   ch->vcc_active = vcc_active;</span><br><span style="color: hsl(120, 100%, 40%);">+       ch->in_reset = in_reset;</span><br><span style="color: hsl(120, 100%, 40%);">+   ch->clocked = clocked;</span><br><span> </span><br><span>        ch->fi = 0;</span><br><span>       ch->di = 1;</span><br><span>diff --git a/firmware/libcommon/source/mode_cardemu.c b/firmware/libcommon/source/mode_cardemu.c</span><br><span>index 98818e1..2d1a687 100644</span><br><span>--- a/firmware/libcommon/source/mode_cardemu.c</span><br><span>+++ b/firmware/libcommon/source/mode_cardemu.c</span><br><span>@@ -44,7 +44,7 @@</span><br><span> static const Pin pin_usim1_vcc       = PIN_USIM1_VCC;</span><br><span> </span><br><span> #ifdef CARDEMU_SECOND_UART</span><br><span style="color: hsl(0, 100%, 40%);">-static const Pin pins_usim2[]    = {PINS_USIM2};</span><br><span style="color: hsl(120, 100%, 40%);">+static const Pin pins_usim2[]       = {PINS_USIM2};</span><br><span> static const Pin pin_usim2_rst       = PIN_USIM2_nRST;</span><br><span> static const Pin pin_usim2_vcc     = PIN_USIM2_VCC;</span><br><span> #endif</span><br><span>@@ -357,14 +357,14 @@</span><br><span> </span><br><span> static void usim1_rst_irqhandler(const Pin *pPin)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">-        int active = PIO_Get(&pin_usim1_rst) ? 0 : 1;</span><br><span style="color: hsl(120, 100%, 40%);">+     bool active = PIO_Get(&pin_usim1_rst) ? false : true;</span><br><span>    card_emu_io_statechg(cardem_inst[0].ch, CARD_IO_RST, active);</span><br><span> }</span><br><span> </span><br><span> #ifndef DETECT_VCC_BY_ADC</span><br><span> static void usim1_vcc_irqhandler(const Pin *pPin)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">-   int active = PIO_Get(&pin_usim1_vcc) ? 1 : 0;</span><br><span style="color: hsl(120, 100%, 40%);">+     bool active = PIO_Get(&pin_usim1_vcc) ? true : false;</span><br><span>    card_emu_io_statechg(cardem_inst[0].ch, CARD_IO_VCC, active);</span><br><span>        /* FIXME do this for real */</span><br><span>         card_emu_io_statechg(cardem_inst[0].ch, CARD_IO_CLK, active);</span><br><span>@@ -374,14 +374,14 @@</span><br><span> #ifdef CARDEMU_SECOND_UART</span><br><span> static void usim2_rst_irqhandler(const Pin *pPin)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">-   int active = PIO_Get(&pin_usim2_rst) ? 0 : 1;</span><br><span style="color: hsl(120, 100%, 40%);">+     bool active = PIO_Get(&pin_usim2_rst) ? false : true;</span><br><span>    card_emu_io_statechg(cardem_inst[1].ch, CARD_IO_RST, active);</span><br><span> }</span><br><span> </span><br><span> #ifndef DETECT_VCC_BY_ADC</span><br><span> static void usim2_vcc_irqhandler(const Pin *pPin)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">-   int active = PIO_Get(&pin_usim2_vcc) ? 1 : 0;</span><br><span style="color: hsl(120, 100%, 40%);">+     bool active = PIO_Get(&pin_usim2_vcc) ? true : false;</span><br><span>    card_emu_io_statechg(cardem_inst[1].ch, CARD_IO_VCC, active);</span><br><span>        /* FIXME do this for real */</span><br><span>         card_emu_io_statechg(cardem_inst[1].ch, CARD_IO_CLK, active);</span><br><span>@@ -420,7 +420,7 @@</span><br><span>  PIO_ConfigureIt(&pin_usim1_vcc, usim1_vcc_irqhandler);</span><br><span>   PIO_EnableIt(&pin_usim1_vcc);</span><br><span> #endif /* DETECT_VCC_BY_ADC */</span><br><span style="color: hsl(0, 100%, 40%);">-     cardem_inst[0].ch = card_emu_init(0, 2, 0, SIMTRACE_CARDEM_USB_EP_USIM1_DATAIN, SIMTRACE_CARDEM_USB_EP_USIM1_INT);</span><br><span style="color: hsl(120, 100%, 40%);">+    cardem_inst[0].ch = card_emu_init(0, 2, 0, SIMTRACE_CARDEM_USB_EP_USIM1_DATAIN, SIMTRACE_CARDEM_USB_EP_USIM1_INT, PIO_Get(&pin_usim1_vcc) ? true : false, PIO_Get(&pin_usim1_rst) ? false : true, PIO_Get(&pin_usim1_vcc) ? true : false);</span><br><span>       sim_switch_use_physical(0, 1);</span><br><span> </span><br><span> #ifdef CARDEMU_SECOND_UART</span><br><span>@@ -435,10 +435,9 @@</span><br><span>      PIO_ConfigureIt(&pin_usim2_vcc, usim2_vcc_irqhandler);</span><br><span>   PIO_EnableIt(&pin_usim2_vcc);</span><br><span> #endif /* DETECT_VCC_BY_ADC */</span><br><span style="color: hsl(0, 100%, 40%);">-     cardem_inst[1].ch = card_emu_init(1, 0, 1, SIMTRACE_CARDEM_USB_EP_USIM2_DATAIN, SIMTRACE_CARDEM_USB_EP_USIM2_INT);</span><br><span style="color: hsl(120, 100%, 40%);">+    cardem_inst[1].ch = card_emu_init(1, 0, 1, SIMTRACE_CARDEM_USB_EP_USIM2_DATAIN, SIMTRACE_CARDEM_USB_EP_USIM2_INT, PIO_Get(&pin_usim2_vcc) ? true : false, PIO_Get(&pin_usim2_rst) ? false : true, PIO_Get(&pin_usim2_vcc) ? true : false);</span><br><span>       sim_switch_use_physical(1, 1);</span><br><span> #endif /* CARDEMU_SECOND_UART */</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> }</span><br><span> </span><br><span> /* called if config is deactivated */</span><br><span>diff --git a/firmware/test/card_emu_tests.c b/firmware/test/card_emu_tests.c</span><br><span>index 09b2e0d..fe1739b 100644</span><br><span>--- a/firmware/test/card_emu_tests.c</span><br><span>+++ b/firmware/test/card_emu_tests.c</span><br><span>@@ -397,7 +397,7 @@</span><br><span>      struct card_handle *ch;</span><br><span>      unsigned int i;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-     ch = card_emu_init(0, 23, 42, PHONE_DATAIN, PHONE_INT);</span><br><span style="color: hsl(120, 100%, 40%);">+       ch = card_emu_init(0, 23, 42, PHONE_DATAIN, PHONE_INT, false, true, false);</span><br><span>  assert(ch);</span><br><span> </span><br><span>      usb_buf_init();</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/simtrace2/+/16076">change 16076</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/simtrace2/+/16076"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: simtrace2 </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Ie9245d75d48ae93d16f97897d4fa5ad6cd402e73 </div>
<div style="display:none"> Gerrit-Change-Number: 16076 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: tsaitgaist <kredon@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>