Change in osmo-ccid-firmware[master]: make it work, async uart (mode 0x1/0x7 does not matter), requires delay

[laforge]

laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/16235 )

Change subject: make it work, async uart (mode 0x1/0x7 does not matter), requires delay
......................................................................


Patch Set 1:

(6 comments)

https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/16235/1/ccid_common/ccid_slot_fsm.c 
File ccid_common/ccid_slot_fsm.c:

https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/16235/1/ccid_common/ccid_slot_fsm.c@205 
PS1, Line 205: /	if (cs->slot_nr == 0) {
             : //		cs->icc_present = true;
             : //		devname = "/dev/ttyUSB5";
             : //	}
that's what we have revision control for, so typically we don't want to merge commits that add commented-out parts of the original code.. Also, it will likely break the operation on the host side?  In that case a more generic solution must be found.


https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/16235/1/sysmoOCTSIM/cuart_driver_asf4_usart_async.c 
File sysmoOCTSIM/cuart_driver_asf4_usart_async.c:

https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/16235/1/sysmoOCTSIM/cuart_driver_asf4_usart_async.c@167 
PS1, Line 167: 
let's please use /* comments all over the code, thanks.


https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/16235/1/sysmoOCTSIM/gcc/Makefile 
File sysmoOCTSIM/gcc/Makefile:

https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/16235/1/sysmoOCTSIM/gcc/Makefile@12 
PS1, Line 12:        -g3 -Wall -c -std=gnu99 $(CFLAGS_CPU)
maybe a separate commit with rationale?


https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/16235/1/sysmoOCTSIM/libosmo_emb.c 
File sysmoOCTSIM/libosmo_emb.c:

https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/16235/1/sysmoOCTSIM/libosmo_emb.c@163 
PS1, Line 163: 	/* timer */
might also be a separate commit?  or some kind of #define that you can enable at compile time to decide if logging shall be used or not?


https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/16235/1/sysmoOCTSIM/main.c 
File sysmoOCTSIM/main.c:

https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/16235/1/sysmoOCTSIM/main.c@109 
PS1, Line 109: 	
see other comment about not commenting out parts of the original code


https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/16235/1/sysmoOCTSIM/usb_descriptors.c 
File sysmoOCTSIM/usb_descriptors.c:

https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/16235/1/sysmoOCTSIM/usb_descriptors.c@a182 
PS1, Line 182: ,
maybe do the descriptor related changes as separate commit (or even multiple), stating the rationale for the respective change?



-- 
To view, visit https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/16235
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: Ia9d3b099181c24feabdb6a5fddb8db6c44efb72d
Gerrit-Change-Number: 16235
Gerrit-PatchSet: 1
Gerrit-Owner: Hoernchen <ewild at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: laforge <laforge at osmocom.org>
Gerrit-Comment-Date: Tue, 26 Nov 2019 23:18:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20191126/84c5eedc/attachment.htm>