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 <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge <[email protected]> Gerrit-Comment-Date: Tue, 26 Nov 2019 23:18:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
