Change in osmo-e1-hardware[master]: icE1usb fw: USB control request handling

[tnt]

tnt has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-e1-hardware/+/21775 )

Change subject: icE1usb fw: USB control request handling
......................................................................


Patch Set 3: Code-Review-2

(4 comments)

Also this should _definitely_ not update the no2e1 submodule pointer (especially to a commit that's not in the public tree)

https://gerrit.osmocom.org/c/osmo-e1-hardware/+/21775/3/firmware/ice40-riscv/icE1usb/e1.c 
File firmware/ice40-riscv/icE1usb/e1.c:

https://gerrit.osmocom.org/c/osmo-e1-hardware/+/21775/3/firmware/ice40-riscv/icE1usb/e1.c@256 
PS3, Line 256: 	g_e1.tx.cr = E1_TX_CR_UNFL_CLR | E1_TX_CR_ENABLE |tx_cr;
space missing.

Also while you're at it, you can fix the E1_*_CLR ... those shouldn't have been in the 'cr' variable to begin with but only set when loading the csr. (i.e. write them once at init but don't store them since we don't want to clear it every time we rewrite cr).


https://gerrit.osmocom.org/c/osmo-e1-hardware/+/21775/3/firmware/ice40-riscv/icE1usb/e1.c@267 
PS3, Line 267: 	printf("tx_cfg(0x%04x)\n", cr);
Left over debug ?


https://gerrit.osmocom.org/c/osmo-e1-hardware/+/21775/3/firmware/ice40-riscv/icE1usb/ice1usb_proto.h 
File firmware/ice40-riscv/icE1usb/ice1usb_proto.h:

https://gerrit.osmocom.org/c/osmo-e1-hardware/+/21775/3/firmware/ice40-riscv/icE1usb/ice1usb_proto.h@1 
PS3, Line 1: #pragma once
Again, try to stick with the file header and comment style.


https://gerrit.osmocom.org/c/osmo-e1-hardware/+/21775/3/firmware/ice40-riscv/icE1usb/usb_e1.c 
File firmware/ice40-riscv/icE1usb/usb_e1.c:

https://gerrit.osmocom.org/c/osmo-e1-hardware/+/21775/3/firmware/ice40-riscv/icE1usb/usb_e1.c@223 
PS3, Line 223: 	e1_tx_config(E1_TX_CR_ENABLE |
Yeah, see previous commit comment, I think tx_config shouldn't touch the enable bit for instance.



-- 
To view, visit https://gerrit.osmocom.org/c/osmo-e1-hardware/+/21775
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-e1-hardware
Gerrit-Branch: master
Gerrit-Change-Id: I9d28566ba21a2a78def5e4a0ba07ecbc4a583aa9
Gerrit-Change-Number: 21775
Gerrit-PatchSet: 3
Gerrit-Owner: laforge <laforge at osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: tnt <tnt at 246tNt.com>
Gerrit-Comment-Date: Sun, 20 Dec 2020 17:56:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20201220/513be067/attachment.htm>