<p><a href="https://review.coreboot.org/28606">View Change</a></p><p>39 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://review.coreboot.org/#/c/28606/4/payloads/external/LinuxBoot/Makefile">File payloads/external/LinuxBoot/Makefile:</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://review.coreboot.org/#/c/28606/4/payloads/external/LinuxBoot/Makefile@78">Patch Set #4, Line 78:</a> <code style="font-family:monospace,monospace"> echo "BEFOR DEF"</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">'BEFOR' may be misspelled - perhaps 'BEFORE'?</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://review.coreboot.org/#/c/28606/4/payloads/libpayload/arch/riscv/coreboot.c">File payloads/libpayload/arch/riscv/coreboot.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://review.coreboot.org/#/c/28606/4/payloads/libpayload/arch/riscv/coreboot.c@42">Patch Set #4, Line 42:</a> <code style="font-family:monospace,monospace"> switch(rec->tag) {</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">space required before the open parenthesis '('</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://review.coreboot.org/#/c/28606/4/payloads/libpayload/include/riscv/arch/asm.h">File payloads/libpayload/include/riscv/arch/asm.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://review.coreboot.org/#/c/28606/4/payloads/libpayload/include/riscv/arch/asm.h@14">Patch Set #4, Line 14:</a> <code style="font-family:monospace,monospace">#define ALIGN .align 2</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Macros with complex values should be enclosed in parentheses</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28606/4/payloads/libpayload/include/riscv/arch/asm.h@16">Patch Set #4, Line 16:</a> <code style="font-family:monospace,monospace">#define ENDPROC(name) \</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Macros with multiple statements should be enclosed in a do - while loop</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28606/4/payloads/libpayload/include/riscv/arch/asm.h@17">Patch Set #4, Line 17:</a> <code style="font-family:monospace,monospace"> .type name, %function; \</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">need consistent spacing around '%' (ctx:WxV)</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28606/4/payloads/libpayload/include/riscv/arch/asm.h@20">Patch Set #4, Line 20:</a> <code style="font-family:monospace,monospace">#define ENTRY(name) \</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Macros with multiple statements should be enclosed in a do - while loop</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28606/4/payloads/libpayload/include/riscv/arch/asm.h@21">Patch Set #4, Line 21:</a> <code style="font-family:monospace,monospace"> .section .text.name, "ax", %progbits; \</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">need consistent spacing around '%' (ctx:WxV)</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28606/4/payloads/libpayload/include/riscv/arch/asm.h@24">Patch Set #4, Line 24:</a> <code style="font-family:monospace,monospace"> name:</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">labels should not be indented</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28606/4/payloads/libpayload/include/riscv/arch/asm.h@26">Patch Set #4, Line 26:</a> <code style="font-family:monospace,monospace">#define END(name) \</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Macros with complex values should be enclosed in parentheses</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28606/4/payloads/libpayload/include/riscv/arch/asm.h@27">Patch Set #4, Line 27:</a> <code style="font-family:monospace,monospace"> .size name, .-name</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">space required before that '-' (ctx:VxV)</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://review.coreboot.org/#/c/28606/4/payloads/libpayload/include/riscv/arch/barrier.h">File payloads/libpayload/include/riscv/arch/barrier.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://review.coreboot.org/#/c/28606/4/payloads/libpayload/include/riscv/arch/barrier.h@57">Patch Set #4, Line 57:</a> <code style="font-family:monospace,monospace">#define mb()</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">memory barrier without comment</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28606/4/payloads/libpayload/include/riscv/arch/barrier.h@58">Patch Set #4, Line 58:</a> <code style="font-family:monospace,monospace">#define rmb()</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">memory barrier without comment</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28606/4/payloads/libpayload/include/riscv/arch/barrier.h@59">Patch Set #4, Line 59:</a> <code style="font-family:monospace,monospace">#define wmb()</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">memory barrier without comment</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://review.coreboot.org/#/c/28606/4/payloads/libpayload/include/riscv/arch/cache.h">File payloads/libpayload/include/riscv/arch/cache.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://review.coreboot.org/#/c/28606/4/payloads/libpayload/include/riscv/arch/cache.h@81">Patch Set #4, Line 81:</a> <code style="font-family:monospace,monospace">}</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">void function return statements are not generally useful</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://review.coreboot.org/#/c/28606/4/payloads/linuxcheck/linuxcheck.c">File payloads/linuxcheck/linuxcheck.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://review.coreboot.org/#/c/28606/4/payloads/linuxcheck/linuxcheck.c@29">Patch Set #4, Line 29:</a> <code style="font-family:monospace,monospace">// buts("Greetings from linuxcheck, via hard-coded calls to serial functions.\n");</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">line over 80 characters</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://review.coreboot.org/#/c/28606/4/payloads/linuxcheck/riscv-fu540.c">File payloads/linuxcheck/riscv-fu540.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://review.coreboot.org/#/c/28606/4/payloads/linuxcheck/riscv-fu540.c@25">Patch Set #4, Line 25:</a> <code style="font-family:monospace,monospace"> uint32_t txdata; /* Transmit data register */</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">code indent should use tabs where possible</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28606/4/payloads/linuxcheck/riscv-fu540.c@25">Patch Set #4, Line 25:</a> <code style="font-family:monospace,monospace"> uint32_t txdata; /* Transmit data register */</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">please, no spaces at the start of a line</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28606/4/payloads/linuxcheck/riscv-fu540.c@26">Patch Set #4, Line 26:</a> <code style="font-family:monospace,monospace"> uint32_t rxdata; /* Receive data register */</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">code indent should use tabs where possible</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28606/4/payloads/linuxcheck/riscv-fu540.c@26">Patch Set #4, Line 26:</a> <code style="font-family:monospace,monospace"> uint32_t rxdata; /* Receive data register */</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">please, no spaces at the start of a line</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28606/4/payloads/linuxcheck/riscv-fu540.c@27">Patch Set #4, Line 27:</a> <code style="font-family:monospace,monospace"> uint32_t txctrl; /* Transmit control register */</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">code indent should use tabs where possible</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28606/4/payloads/linuxcheck/riscv-fu540.c@27">Patch Set #4, Line 27:</a> <code style="font-family:monospace,monospace"> uint32_t txctrl; /* Transmit control register */</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">please, no spaces at the start of a line</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28606/4/payloads/linuxcheck/riscv-fu540.c@28">Patch Set #4, Line 28:</a> <code style="font-family:monospace,monospace"> uint32_t rxctrl; /* Receive control register */</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">code indent should use tabs where possible</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28606/4/payloads/linuxcheck/riscv-fu540.c@28">Patch Set #4, Line 28:</a> <code style="font-family:monospace,monospace"> uint32_t rxctrl; /* Receive control register */</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">please, no spaces at the start of a line</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28606/4/payloads/linuxcheck/riscv-fu540.c@29">Patch Set #4, Line 29:</a> <code style="font-family:monospace,monospace"> uint32_t ie; /* UART interrupt enable */</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">code indent should use tabs where possible</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28606/4/payloads/linuxcheck/riscv-fu540.c@29">Patch Set #4, Line 29:</a> <code style="font-family:monospace,monospace"> uint32_t ie; /* UART interrupt enable */</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">please, no spaces at the start of a line</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28606/4/payloads/linuxcheck/riscv-fu540.c@30">Patch Set #4, Line 30:</a> <code style="font-family:monospace,monospace"> uint32_t ip; /* UART interrupt pending */</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">code indent should use tabs where possible</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28606/4/payloads/linuxcheck/riscv-fu540.c@30">Patch Set #4, Line 30:</a> <code style="font-family:monospace,monospace"> uint32_t ip; /* UART interrupt pending */</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">please, no spaces at the start of a line</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28606/4/payloads/linuxcheck/riscv-fu540.c@31">Patch Set #4, Line 31:</a> <code style="font-family:monospace,monospace"> uint32_t div; /* Baud rate divisor */</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">code indent should use tabs where possible</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28606/4/payloads/linuxcheck/riscv-fu540.c@31">Patch Set #4, Line 31:</a> <code style="font-family:monospace,monospace"> uint32_t div; /* Baud rate divisor */</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">please, no spaces at the start of a line</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28606/4/payloads/linuxcheck/riscv-fu540.c@36">Patch Set #4, Line 36:</a> <code style="font-family:monospace,monospace"> return !(read32(®s->txdata) & TXDATA_FULL);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">code indent should use tabs where possible</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28606/4/payloads/linuxcheck/riscv-fu540.c@36">Patch Set #4, Line 36:</a> <code style="font-family:monospace,monospace"> return !(read32(®s->txdata) & TXDATA_FULL);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">please, no spaces at the start of a line</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28606/4/payloads/linuxcheck/riscv-fu540.c@41">Patch Set #4, Line 41:</a> <code style="font-family:monospace,monospace"> struct sifive_uart_registers *regs = (void *)(FU540_UART(idx));</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">code indent should use tabs where possible</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28606/4/payloads/linuxcheck/riscv-fu540.c@41">Patch Set #4, Line 41:</a> <code style="font-family:monospace,monospace"> struct sifive_uart_registers *regs = (void *)(FU540_UART(idx));</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">please, no spaces at the start of a line</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28606/4/payloads/linuxcheck/riscv-fu540.c@43">Patch Set #4, Line 43:</a> <code style="font-family:monospace,monospace"> while (!uart_can_tx(regs))</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">code indent should use tabs where possible</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28606/4/payloads/linuxcheck/riscv-fu540.c@43">Patch Set #4, Line 43:</a> <code style="font-family:monospace,monospace"> while (!uart_can_tx(regs))</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">please, no spaces at the start of a line</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28606/4/payloads/linuxcheck/riscv-fu540.c@44">Patch Set #4, Line 44:</a> <code style="font-family:monospace,monospace"> ; /* TODO: implement a timeout */</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">code indent should use tabs where possible</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28606/4/payloads/linuxcheck/riscv-fu540.c@44">Patch Set #4, Line 44:</a> <code style="font-family:monospace,monospace"> ; /* TODO: implement a timeout */</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">please, no spaces at the start of a line</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28606/4/payloads/linuxcheck/riscv-fu540.c@46">Patch Set #4, Line 46:</a> <code style="font-family:monospace,monospace"> write32(®s->txdata, data);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">code indent should use tabs where possible</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28606/4/payloads/linuxcheck/riscv-fu540.c@46">Patch Set #4, Line 46:</a> <code style="font-family:monospace,monospace"> write32(®s->txdata, data);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">please, no spaces at the start of a line</p></li></ul></li></ul><p>To view, visit <a href="https://review.coreboot.org/28606">change 28606</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://review.coreboot.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://review.coreboot.org/28606"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: coreboot </div>
<div style="display:none"> Gerrit-Branch: rampayload </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: I91df02069a0f8fd8771f73de0e866e9cea05cded </div>
<div style="display:none"> Gerrit-Change-Number: 28606 </div>
<div style="display:none"> Gerrit-PatchSet: 4 </div>
<div style="display:none"> Gerrit-Owner: Philipp Hug <philipp@hug.cx> </div>
<div style="display:none"> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> </div>
<div style="display:none"> Gerrit-Comment-Date: Sat, 15 Sep 2018 09:01:07 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-HasLabels: No </div>