I forgot one more thing:
In my SERIAL OUTPUT section quoted earlier, please note there were some
UART parity error ( there were 3 errors total, cumulative report )
Does anyone have any input on how to fix that?
Do we need shielded cable or is it something we should fix in software?
Best Regards
On Tue, Sep 10, 2013 at 4:51 PM, Min Xu <mxu(a)sanjole.com> wrote:
Hi All
Further runs (after the check in) shows that there is still a chance for
the req_ctx being transmitted by the atmel chip to be broken up (and
consequently, also combined with a later req_ctx). From what I have read
in the usb specification, there isn't anything to signal an end of stream
other than the device stop transmitting for a little while.
So I firmly believe we must expand the USB protocol header to add a length
field (and I would also recommend adding some additional fields for
housekeeping to ease future debug, e.g., address of the req_ctx, and offset
of the first byte of an apdu). If this is not agreeable, I will refrain
from further check-in, but I believe this change is a requirement to keep
in-sync
-----------
============================ HOST SOFTWARE ============================
[[ OMITTED ]]
[000078] USBT(D=002049E8, L=0738, P=02) H4/T4: B2 43 05 84 / B2 01 04 B0
>>>>>>>>>>>>>>>> USB IN [0738]
<<<<<<<<<<<<<<<
01 00 09 07 B2 43 05
84 D2 77 B8 0D FF FF FF FF
FF FF FF FF FF FF FF FF FF 90 00 00 A4 00 0C 02
....
FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF
FF 90 00 00 A4 00 0C 02 A4 6F 3C 90 00 00 B2 01
04 B0
>>>>>>>>>>>>>>>> USB IN [0064]
<<<<<<<<<<<<<<<
01 00 09 07 B2 00 FF
FF FF FF FF FF FF FF FF FF
FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF
FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF
FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF FF
*NOTICE the next USB in does not start with a USB header where as the
bytes IMMEDIATELY
*
*FOLLOWING the <green> highlited bytes are the req_ctx header *
>>>>>>>>>>>>>>>> USB IN [0960]
<<<<<<<<<<<<<<<
00 00 B2 05 04 14 B2
FF FF FF FF FF FF FF FF FF
FF FF FF FF FF FF FF FF FF FF FF 90 00 00 A4 08
[[ OMITTED ]]
B2 01 04 08 B2 00 00 00 00 00 00 00 00 90 00 00
A4 08 0C 04 A4 7F FF 6F C6 90 00 00 B2 05 04 08
01 00 09 07 B2 FF FF FF FF FF FF FF FF 90 00 01
A4 00 04 02 A4 6F 3C 61 24 01 C0 00 00 24 C0 62
[[ OMITTED ]]
07 81 02 00 19 88 01 68 90 00 01 B0 00 01 06 B0
unknown simtrace msg type 0x00 <<== BAD header indicator
============================ SERIAL OUTPUT ==============================
[000000]
[000001] (C) 2006-2011 by Harald Welte <hwelte(a)hmw-consulting.de>
This software is FREE SOFTWARE licensed under GNU GPL
[000002] Version 0.5.7-8d17-dirty compiled 20130910-155033 by
min@Min-Toshiba
[000003]
DEBUG Interface:
0) Set Pull-up 1) Clear Pull-up 2) Toggle LED1 3) Toggle LED2
9) Reset
[000004] RSTC_SR=0x00010000
[000005] LARGE req_ctx[00] initialized at 002055B0, Data: 00200A28 =>
00200DE8
[000006] LARGE req_ctx[01] initialized at 002055C4, Data: 00200DE8 =>
002011A8
[000007] LARGE req_ctx[02] initialized at 002055D8, Data: 002011A8 =>
00201568
[000008] LARGE req_ctx[03] initialized at 002055EC, Data: 00201568 =>
00201928
[000009] LARGE req_ctx[04] initialized at 00205600, Data: 00201928 =>
00201CE8
[00000A] LARGE req_ctx[05] initialized at 00205614, Data: 00201CE8 =>
002020A8
[00000B] LARGE req_ctx[06] initialized at 00205628, Data: 002020A8 =>
00202468
[00000C] LARGE req_ctx[07] initialized at 0020563C, Data: 00202468 =>
00202828
[00000D] LARGE req_ctx[08] initialized at 00205650, Data: 00202828 =>
00202BE8
[00000E] LARGE req_ctx[09] initialized at 00205664, Data: 00202BE8 =>
00202FA8
[00000F] LARGE req_ctx[10] initialized at 00205678, Data: 00202FA8 =>
00203368
[000010] LARGE req_ctx[11] initialized at 0020568C, Data: 00203368 =>
00203728
[000011] LARGE req_ctx[12] initialized at 002056A0, Data: 00203728 =>
00203AE8
[000012] LARGE req_ctx[13] initialized at 002056B4, Data: 00203AE8 =>
00203EA8
[000013] LARGE req_ctx[14] initialized at 002056C8, Data: 00203EA8 =>
00204268
[000014] LARGE req_ctx[15] initialized at 002056DC, Data: 00204268 =>
00204628
[000015] LARGE req_ctx[16] initialized at 002056F0, Data: 00204628 =>
002049E8
[000016] LARGE req_ctx[17] initialized at 00205704, Data: 002049E8 =>
00204DA8
[000017] LARGE req_ctx[18] initialized at 00205718, Data: 00204DA8 =>
00205168
[000018] LARGE req_ctx[19] initialized at 0020572C, Data: 00205168 =>
00205528
[000019] Inititalizing usbcmd_gen_init
[00001A] udp_open(425): entering
[00001B] USART Initializing
[00001C] pio_irq_register(109): registering handler 001078d8 for PIOA 7
[00001D] RST
[00001E] computed Fi(1) Di(1) ratio: 372
[00001F] ISO_SW Initializing
[000020] pio_irq_register(109): registering handler 00107cfc for PIOA 8
[000021] pio_irq_register(109): registering handler 00107d28 for PIOA 25
[000022] USART Entering Rx Mode
[000023] RST
[000024] computed Fi(1) Di(1) ratio: 372
[000025] MODE: SNIFFER
[000026] RST
[000027] computed Fi(1) Di(1) ratio: 372
[000028] main(76): entering main (idle) loop
[000029] Heart beat 00000000
[00002A] VCC_PHONE off
[00002B] Heart beat 00000001
[00002C] Heart beat 00000002
[00002D] Heart beat 00000003
[00002E] Heart beat 00000004
[00002F] Heart beat 00000005
[000030] Heart beat 00000006
[000031] Heart beat 00000007
[000032] VCC_PHONE on
[000033] RST
[000034] computed Fi(1) Di(1) ratio: 372
[000035] Heart beat 00000008
[000036] USBT(D=00200DE8, L=0027, P=00) H4/T4: 3B 9F 97 C0 / 22 81 00 F2
[000037] found Fi=9 Di=7
[000038] computed Fi(9) Di(7) ratio: 8
[000039] USBT(D=002011A8, L=0004, P=00) H4/T4: 00 00 00 00 / 01 0C 09 07
[00003A] USBT(D=00201568, L=0009, P=00) H4/T4: 00 A4 00 04 / A4 00 04 02
[00003B] USBT(D=00201928, L=0008, P=00) H4/T4: 60 A4 3F 00 / 60 A4 3F 00
[00003C] USBT(D=00201CE8, L=0137, P=00) H4/T4: 61 38 00 C0 / 09 62 90 00
[00003D] Heart beat 00000009
[00003E] Heart beat 0000000A
[00003F] Heart beat 0000000B
[000040] USBT(D=002020A8, L=0290, P=00) H4/T4: 00 A4 08 04 / 00 00 00 00
[000041] USBT(D=00202468, L=0281, P=00) H4/T4: 91 10 00 A4 / FF FF 91 10
[000042] USBT(D=00202828, L=0021, P=00) H4/T4: 80 C2 00 00 / 03 13 01 84
[000043] USBT(D=00202BE8, L=0028, P=00) H4/T4: 93 00 00 A4 / 08 00 00 FF
[000044] Heart beat 0000000C
[000045] USBT(D=00202FA8, L=0388, P=00) H4/T4: 61 3F 00 C0 / 03 13 01 84
[000046] USBT(D=00203368, L=0024, P=00) H4/T4: 93 00 80 14 / 81 03 01 00
[000047] USBT(D=00203728, L=0061, P=01) H4/T4: 91 0F 80 12 / FF 00 90 00
[000048] USBT(D=00203AE8, L=0021, P=00) H4/T4: 80 C2 00 00 / 03 13 01 84
[000049] USBT(D=00203EA8, L=0050, P=00) H4/T4: 93 00 00 B2 / 04 02 01 1E
[00004A] USBT(D=00204268, L=0118, P=00) H4/T4: 91 2B 80 12 / 03 13 01 84
[00004B] USBT(D=00204628, L=0054, P=00) H4/T4: 93 00 00 B2 / 39 02 05 8E
[00004C] USBT(D=002049E8, L=0235, P=00) H4/T4: 91 13 80 12 / 02 00 00 FF
[00004D] USBT(D=00204DA8, L=0472, P=00) H4/T4: 61 3F 01 C0 / 02 A4 6F B7
[00004E] USBT(D=00205168, L=0371, P=00) H4/T4: 6A 82 01 A4 / A4 00 0C 02
[00004F] USBT(D=00200A28, L=0280, P=00) H4/T4: A4 7F 10 90 / 08 00 00 FF
[000050] USBT(D=00200DE8, L=0506, P=00) H4/T4: 90 00 00 A4 / FF FF 90 00
[000051] Heart beat 0000000D
[000052] USBT(D=002011A8, L=0503, P=00) H4/T4: 00 A4 00 0C / B0 00 00 00
[000053] USBT(D=00201568, L=0960, P=00) H4/T4: B0 32 F4 51 / C0 00 00 22
[000054] USBT(D=00201928, L=0540, P=00) H4/T4: C0 62 20 82 / A4 08 04 06
[000055] USBT(D=00201CE8, L=0548, P=00) H4/T4: A4 7F FF 5F / B2 06 04 3F
[000056] USBT(D=002020A8, L=0190, P=00) H4/T4: B2 80 01 01 / 08 00 00 FF
[000057] USBT(D=00202468, L=0582, P=00) H4/T4: 90 00 01 2C / 02 A4 6F 06
[000058] USBT(D=00202828, L=0688, P=00) H4/T4: 61 24 01 C0 / 46 00 00 99
[000059] USBT(D=00202BE8, L=0642, P=00) H4/T4: 61 01 01 C0 / 02 A4 6F 2C
[00005A] USBT(D=00202FA8, L=0960, P=00) H4/T4: 61 22 01 C0 / 06 12 61 F4
[00005B] USBT(D=00203368, L=0960, P=00) H4/T4: 47 E9 61 38 / 85 00 00 40
[00005C] Heart beat 0000000E
[00005D] USBT(D=00203728, L=0960, P=01) H4/T4: 60 08 00 6A / 81 02 00 40
[00005E] USBT(D=00203AE8, L=0960, P=02) H4/T4: 60 D0 04 20 / 30 60 08 10
[00005F] USBT(D=00203EA8, L=0960, P=04) H4/T4: BA 84 30 60 / 00 03 70 81
[000060] USBT(D=00204268, L=0960, P=05) H4/T4: 0F 00 40 60 / 20 80 30 70
[000061] USBT(D=00204628, L=0960, P=06) H4/T4: 00 0F A8 88 / 36 45 FF FF
[000062] USBT(D=002049E8, L=0960, P=06) H4/T4: 36 3D FF FF / 3E 31 FF FF
[000063] USBT(D=00204DA8, L=0960, P=06) H4/T4: 44 00 69 C0 / 44 80 80 B1
[000064] USBT(D=00205168, L=0960, P=06) H4/T4: E0 02 30 2F / FF 36 6F FF
[000065] USBT(D=00200A28, L=0960, P=06) H4/T4: FF 36 6E FF / FF FF FF FF
[000066] USBT(D=00200DE8, L=0960, P=06) H4/T4: FF FF FF FF / FF FF FF FF
[000067] USBT(D=002011A8, L=0960, P=06) H4/T4: FF FF FF FF / B0 2D 00 00
[000068] Heart beat 0000000F
[000069] USBT(D=00201568, L=0960, P=06) H4/T4: B0 FF FF FF / 00 90 00 01
[00006A] USBT(D=00201928, L=0140, P=05) H4/T4: B2 13 04 42 / FF FF 90 00
[00006B] USBT(D=00201CE8, L=0270, P=06) H4/T4: 00 A4 00 04 / A4 00 0C 02
[00006C] USBT(D=002020A8, L=0246, P=05) H4/T4: A4 7F FF 90 / 02 00 00 FF
[00006D] USBT(D=00202468, L=0088, P=05) H4/T4: 91 0B 80 12 / 03 02 20 04
[00006E] USBT(D=00202828, L=0156, P=05) H4/T4: 90 00 00 A4 / FF FF 90 00
[00006F] USBT(D=00202BE8, L=0289, P=04) H4/T4: 00 B2 0B 04 / FF FF 90 00
[000070] USBT(D=00202FA8, L=0890, P=03) H4/T4: 01 A4 00 04 / 61 6F 90 00
[000071] USBT(D=00203368, L=0960, P=03) H4/T4: 00 B0 02 00 / FF FF FF FF
[000072] USBT(D=00203728, L=0960, P=03) H4/T4: FF FF FF FF / 01 08 80 01
[000073] USBT(D=00203AE8, L=0607, P=03) H4/T4: 40 A4 06 83 / 00 17 88 00
[000074] USBT(D=00203EA8, L=0545, P=03) H4/T4: 90 00 01 B0 / 7F FF 6F 42
[000075] USBT(D=00204268, L=0412, P=02) H4/T4: 90 00 00 B2 / 7F FF 6F 42
[000076] Heart beat 00000010
[000077] USBT(D=00204628, L=0450, P=02) H4/T4: 90 00 00 B2 / B2 03 04 14
[000078] USBT(D=002049E8, L=0738, P=02) H4/T4: B2 43 05 84 / B2 01 04 B0
[000079] USBT(D=00204DA8, L=0576, P=02) H4/T4: B2 00 FF FF / B2 05 04 08
[00007A] USBT(D=00205168, L=0869, P=02) H4/T4: B2 FF FF FF / A4 00 0C 02
[00007B] USBT(D=00200A28, L=0503, P=01) H4/T4: A4 6F 3C 90 / 02 A4 6F 3C
[00007C] USBT(D=00200DE8, L=0960, P=00) H4/T4: 61 24 01 C0 / FF FF FF FF
[00007D] USBT(D=002011A8, L=0502, P=00) H4/T4: FF FF FF FF / B2 06 04 B0
[00007E] USBT(D=00201568, L=0888, P=00) H4/T4: B2 00 FF FF / 5F 3A 4F 11
[00007F] USBT(D=00201928, L=0834, P=00) H4/T4: 61 25 00 C0 / FF FF 90 00
[000080] USBT(D=00201CE8, L=0960, P=00) H4/T4: 00 A4 08 0C / FF FF FF FF
[000081] USBT(D=002020A8, L=0398, P=00) H4/T4: FF FF FF FF / B2 0E 04 08
[000082] USBT(D=00202468, L=0960, P=00) H4/T4: B2 FF FF FF / FF FF FF FF
[000083] Heart beat 00000011
[000084] USBT(D=00202828, L=0421, P=00) H4/T4: FF FF FF FF / FF FF 90 00
[000085] USBT(D=00202BE8, L=0960, P=00) H4/T4: 01 A4 00 04 / FF FF FF FF
[000086] USBT(D=00202FA8, L=0441, P=00) H4/T4: FF FF FF FF / 02 A4 6F 28
[000087] USBT(D=00203368, L=0658, P=00) H4/T4: 61 25 01 C0 / 02 A4 6F 06
[000088] USBT(D=00203728, L=0495, P=00) H4/T4: 90 00 00 B2 / B2 02 04 1E
[000089] USBT(D=00203AE8, L=0485, P=00) H4/T4: B2 00 FF FF / FF FF 90 00
[00008A] USBT(D=00203EA8, L=0209, P=00) H4/T4: 00 A4 00 0C / FF FF FF FF
[00008B] USBT(D=00204268, L=0440, P=00) H4/T4: 61 0A 00 C0 / FF FF FF FF
[00008C] USBT(D=00204628, L=0435, P=00) H4/T4: 61 0A 00 C0 / FF FF FF FF
[00008D] Heart beat 00000012
[00008E] USBT(D=002049E8, L=0389, P=00) H4/T4: 61 04 00 C0 / B2 02 04 22
[00008F] USBT(D=00204DA8, L=0454, P=00) H4/T4: B2 23 4D 49 / A4 08 0C 04
[000090] USBT(D=00205168, L=0414, P=00) H4/T4: A4 7F FF 6F / A4 08 0C 04
[000091] USBT(D=00200A28, L=0458, P=00) H4/T4: A4 7F FF 6F / 00 00 FF FF
[000092] USBT(D=00200DE8, L=0385, P=00) H4/T4: 90 00 01 B2 / B0 00 90 00
[000093] USBT(D=002011A8, L=0271, P=00) H4/T4: 00 A4 00 0C / 02 A2 00 00
[000094] USBT(D=00201568, L=0271, P=00) H4/T4: 61 64 00 C0 / 02 A2 00 00
[000095] USBT(D=00201928, L=0472, P=00) H4/T4: 61 64 00 C0 / B2 2B 04 08
[000096] USBT(D=00201CE8, L=0082, P=00) H4/T4: B2 FF FF FF / 02 A2 00 00
[000097] USBT(D=002020A8, L=0270, P=00) H4/T4: 61 64 00 C0 / 02 A2 00 00
[000098] Heart beat 00000013
[000099] USBT(D=00202468, L=0436, P=00) H4/T4: 61 64 00 C0 / 00 C0 00 00
[00009A] USBT(D=00202828, L=0091, P=00) H4/T4: 25 C0 62 23 / FF FF FF FF
[00009B] USBT(D=00202BE8, L=0383, P=00) H4/T4: 61 64 00 C0 / FF FF FF FF
[00009C] USBT(D=00202FA8, L=0358, P=00) H4/T4: 61 64 00 C0 / FF FF FF FF
[00009D] USBT(D=00203368, L=0326, P=00) H4/T4: 61 64 00 C0 / FF FF FF FF
[00009E] USBT(D=00203728, L=0264, P=00) H4/T4: 61 64 00 C0 / FF FF FF FF
[00009F] USBT(D=00203AE8, L=0264, P=00) H4/T4: 61 64 00 C0 / FF FF FF FF
[0000A0] USBT(D=00203EA8, L=0239, P=00) H4/T4: 61 64 00 C0 / FF FF FF FF
[0000A1] USBT(D=00204268, L=0239, P=00) H4/T4: 61 64 00 C0 / FF FF FF FF
[0000A2] USBT(D=00204628, L=0263, P=00) H4/T4: 61 64 00 C0 / FF FF FF FF
[0000A3] USBT(D=002049E8, L=0172, P=00) H4/T4: 61 64 00 C0 / FF FF FF FF
[0000A4] Heart beat 00000014
[0000A5] USBT(D=00204DA8, L=0261, P=00) H4/T4: 61 64 00 C0 / 00 00 90 00
[0000A6] USBT(D=00205168, L=0311, P=00) H4/T4: 01 A4 00 04 / B2 14 04 05
[0000A7] USBT(D=00200A28, L=0108, P=00) H4/T4: B2 00 00 00 / B0 00 00 0A
[0000A8] USBT(D=00200DE8, L=0017, P=00) H4/T4: B0 98 41 08 / 09 62 90 00
[0000A9] USBT(D=002011A8, L=0010, P=00) H4/T4: 80 F2 00 0C / 00 0C 00 FF
[0000AA] Heart beat 00000015
[0000AB] RST
[0000AC] computed Fi(1) Di(1) ratio: 372
[0000AD] VCC_PHONE off
[0000AE] VCC_PHONE on
[0000AF] RST
[0000B0] computed Fi(1) Di(1) ratio: 372
[0000B1] USBT(D=00201568, L=0027, P=00) H4/T4: 3B 9F 97 C0 / 22 81 00 F2
[0000B2] found Fi=9 Di=7
[0000B3] computed Fi(9) Di(7) ratio: 8
[0000B4] USBT(D=00201928, L=0009, P=00) H4/T4: 00 A4 00 04 / A4 00 04 02
[0000B5] Heart beat 00000016
[0000B6] USBT(D=00201CE8, L=0008, P=00) H4/T4: 60 A4 3F 00 / 60 A4 3F 00
[0000B7] USBT(D=002020A8, L=0182, P=00) H4/T4: 61 38 00 C0 / 00 00 00 00
[0000B8] UART parity error: 1
[0000B9] UART parity error: 2
[0000BA] USBT(D=00202468, L=0006, P=00) H4/T4: F0 FF 00 C0 / 09 07 F0 FF
[0000BB] Heart beat 00000017
[0000BC] RST
[0000BD] computed Fi(1) Di(1) ratio: 372
[0000BE] VCC_PHONE off
[0000BF] VCC_PHONE on
[0000C0] RST
[0000C1] computed Fi(1) Di(1) ratio: 372
[0000C2] USBT(D=00202828, L=0027, P=00) H4/T4: 3B 9F 97 C0 / 22 81 00 F2
[0000C3] found Fi=9 Di=7
[0000C4] computed Fi(9) Di(7) ratio: 8
[0000C5] USBT(D=00202BE8, L=0009, P=00) H4/T4: 00 A4 00 04 / A4 00 04 02
[0000C6] USBT(D=00202FA8, L=0007, P=00) H4/T4: A4 3F 00 C0 / 07 A4 3F 00
[0000C7] Heart beat 00000018
[0000C8] USBT(D=00203368, L=0182, P=00) H4/T4: 61 38 00 C0 / 00 00 00 00
[0000C9] USBT(D=00203728, L=0006, P=00) H4/T4: 91 10 00 C0 / 09 07 91 10
[0000CA] Heart beat 00000019
[0000CB] Heart beat 0000001A
[0000CC] Heart beat 0000001B
[0000CD] Heart beat 0000001C
[0000CE] Heart beat 0000001D
[0000CF] Heart beat 0000001E
[0000D0] UART parity error: 3
[0000D1] USBT(D=00203AE8, L=0007, P=00) H4/T4: EF FF FD C0 / 07 EF FF FD
[0000D2] Heart beat 0000001F
[0000D3] Heart beat 00000020
[0000D4] Heart beat 00000021
[0000D5] Heart beat 00000022
[0000D6] Heart beat 00000023
[0000D7] Heart beat 00000024
[0000D8] Heart beat 00000025
[0000D9] Heart beat 00000026
[0000DA] Heart beat 00000027
[0000DB] Heart beat 00000028
[0000DC] Heart beat 00000029
[0000DD] Heart beat 0000002A
[0000DE] Heart beat 0000002B
[0000DF] Heart beat 0000002C
[0000E0] Heart beat 0000002D
[0000E1] Heart beat 0000002E
On Tue, Sep 10, 2013 at 2:19 PM, Min Xu <mxu(a)sanjole.com> wrote:
Hi All
I have been debugging some usb errors that I have been encountering and
here are my conclusions:
-- The error is a BULK IN transfer error, after adding further code to
display the internal message:
BULK IN transfer error; rc=-5 : libusb0-dll:err [_usb_reap_async] reaping
request failed, win error: A device attached to the system is not
functioning.
-- BUT when I used USBLyzer to analyze the underlying problem, it's
actually a [buffer overflow] problem.
1) According to usb specification, USB full speed device maximum data
packet payload size is 1023
http://www.usb.org/developers/docs/usb_20_070113.zip
2) According to the AT91SAM7S device specification, its USB endpoint
size is 64 bytes.
3) According to libusb api description, bulk transfer overflow is caused
by too small buffer (
http://libusb.sourceforge.net/api-1.0/packetoverflow.html ). Should use
multiple of the device payload size.
4) Since we don't have a size indicator in our USB protocol (our header
is 4 bytes, indicating ATR, wait timeout flag etc), the client software
must not see more than 1 req_ctx per bulk_usb_transfer call
Therefore, we SHOULD:
> A << Use a multiple of 64 bytes as
req_ctx size, and must be <= 1023
[per USB specification]. I chose 960.
Consequently, there is only ONE
size for req_ctx, no more large / small)
> B << Desktop/Host software MUST use
the same size in static void
run_mainloop(struct usb_dev_handle *devh).
Otherwise, multiple req_ctx
WILL be combined by the usb driver / libusb and the additional req_ctx
header in the middle will confuse the apdu_splitter
On Wed, Sep 4, 2013 at 10:37 PM, Min Xu <mxu(a)sanjole.com> wrote:
Hi Harald
Thank you for getting back to me so quickly. I am using Emacs on an
Ubuntu netbook to do the edits, which likes to reformat sections as I move
(copy/paste section around) and I haven't figured out a command to undo
those changes. It'd be nice if there is a .emacsrc or something that
automatically formats the code to the project standard? (presumably someone
has one? ..)
I started only wanted to make one change and submit (was initially the
req_ctx), but couldn't reliably verify it until the debugp was working
better without affecting the system, so the changes ballooned up to this
stage. So I wanted to submit the changes before I change the code
further. I will try limit the scope of changes in my future updates.
To your questions:
1) The numerical values of the RCTX_STATE are changed to facilitate the
choosing of the FIFO/double linked list of req_ctx to use. An array of
FIFO, using the state to index into the array to get the FIFO is simplest.
When I looked how the reqctx_find / ... functions are used, I found all
callers use the constant name rather than any specific numeric value.
2) I will look at this tomorrow. But I was under the impression on how
the lib_Atmel?? (sorry, can't access the source at the moment) was invoked
to install the irq handler, that the entry point to the interrupt would be
directly the function passed in. I looked at the sysirq_handler.c?? more
closely than the other so I wasn't aware of the Cstartup.c file
Since I increased the req_ctx buffer size to 1kb each, most req_ctx that
are transferred to pc contain many many apdus but very often, the req_ctx
do not begin at the start of an apdu. So I would like to have an offset (2
byte, since it could be > 256) to indicate the position of the first start
of an apdu. After thinking about it more, I would also like to add a
sequence number into the header (making the header 8 bytes then?) to better
allow for detecting dropped req_ctx (if there are any..). The offset would
be 8 for a req_ctx that starts with a new APDU, and would be 0 for an
req_ctx that only contains a middle or last fragment of an APDU.
I have already modified the host software to run on Windows (currently
using Windows 7 x64), so I will probably make those changes locally only to
help me detect any problem I might still be facing.
Again, thank you very much for getting back so quickly, and I appreciate
your product very much.
Best Regards
On Wed, Sep 4, 2013 at 9:31 PM, Harald Welte <laforge(a)gnumonks.org>wrote;wrote:
Dear Min Xu,
On Wed, Sep 04, 2013 at 07:47:46PM -1000, Min Xu wrote:
> I made a bunch of changes that significantly improved my test
scenario:
I _really_ appreciate your excellent technical work. However, I would
like you to go the little extra effort it takes to 'properly' interact
with community based Free Software projects, where there are rules like
* only one feature / logical change per patch, resulting in a series
of incremental patches, each taking us one step further, each
rendering a compile-able and functional build after being applied.
* no re-formatting of code (whitespace changes)
* no patch for local changes like different compiler name in Makefile
* stick to coding style of the project (tab-wide indent, ...) for
consistency
* remove old/dead code rather than comment it out (RCTX_STATE_...)
* avoid asm() statements whenever possible. If you need them, please
wrap them in an inline C function with descriptive name.
Some quesetions:
* why did the numberic values of RCTX_STATE_* have t be changed rather
than amended by your new values, keeping the old ones as-is?
* interrupt nesting _should_ have been active all the time, see
IRQ_Handler_Entry in Cstartup.S, where we first save SPSR and then
un-set the IRQ and FIQ bits _before_ branching to the interrupt
handler function:
/*- Enable Interrupt and Switch in Supervisor Mode */
msr CPSR_c, #ARM_MODE_SVC
Where ARM_MODE_SVC is set to 0x13, i.e. without 0x40 or 0x80.
So if there's something wrong with the existing code, it should be
fixed there rather than two copies of inline-asm in the USART and USB
IRQ handler routines.
So I do want to merge this very much, but I think it needs clean-up
before it can be merged. I don't expect to have much time for this in
the next couple of weeks, so I would be happy if either you or somebody
else on the list could work on this.
Regarding your comment about changing the USB protocol: This would lead
to host / firmware version incompatibilities, and I'd like to avoid that
if possible in any way. We already have the simtrace_hdr.flags
structure member, which contains things like SIMTRACE_FLAG_ATR. I'm not
sure if I understand your request fully, but why not simply add flags
like:
* First fragment of a fragmented APDU
* More fragments to follow for this APDU
This way a new APDU still has to start at the beginning of a USB
transfer, but no changes to the simtrace_hdr are required.
Thanks again,
Harald
--
- Harald Welte <laforge(a)gnumonks.org>
http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7
Ch. A6)