<p style="white-space: pre-wrap; word-wrap: break-word;">Also this should _definitely_ not update the no2e1 submodule pointer (especially to a commit that's not in the public tree)</p><p>Patch set 3:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ffd4d4;">Code-Review -2</span></p><p><a href="https://gerrit.osmocom.org/c/osmo-e1-hardware/+/21775">View Change</a></p><p>4 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-e1-hardware/+/21775/3/firmware/ice40-riscv/icE1usb/e1.c">File firmware/ice40-riscv/icE1usb/e1.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-e1-hardware/+/21775/3/firmware/ice40-riscv/icE1usb/e1.c@256">Patch Set #3, Line 256:</a> <code style="font-family:monospace,monospace">    g_e1.tx.cr = E1_TX_CR_UNFL_CLR | E1_TX_CR_ENABLE |tx_cr;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">space missing.</p><p style="white-space: pre-wrap; word-wrap: break-word;">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).</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-e1-hardware/+/21775/3/firmware/ice40-riscv/icE1usb/e1.c@267">Patch Set #3, Line 267:</a> <code style="font-family:monospace,monospace">    printf("tx_cfg(0x%04x)\n", cr);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Left over debug ?</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="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:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-e1-hardware/+/21775/3/firmware/ice40-riscv/icE1usb/ice1usb_proto.h@1">Patch Set #3, Line 1:</a> <code style="font-family:monospace,monospace">#pragma once</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Again, try to stick with the file header and comment style.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="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:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-e1-hardware/+/21775/3/firmware/ice40-riscv/icE1usb/usb_e1.c@223">Patch Set #3, Line 223:</a> <code style="font-family:monospace,monospace">        e1_tx_config(E1_TX_CR_ENABLE |</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Yeah, see previous commit comment, I think tx_config shouldn't touch the enable bit for instance.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-e1-hardware/+/21775">change 21775</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/osmo-e1-hardware/+/21775"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-e1-hardware </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I9d28566ba21a2a78def5e4a0ba07ecbc4a583aa9 </div>
<div style="display:none"> Gerrit-Change-Number: 21775 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: tnt <tnt@246tNt.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Sun, 20 Dec 2020 17:56:45 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: Yes </div>
<div style="display:none"> Gerrit-MessageType: comment </div>