17 comments:
Patch Set #1, Line 646: # Enable J-Link for now.
please update the comment
nice documentation
Patch Set #1, Line 1133: 4: GND
On the first look, I thought the listing has the same order as the
physical pins. Let's see how it'd look like if it did:
+-------+
| 1 2 | 1: VTref 2:
| 3 4 | 3: TRST 4: GND
| 5 6 | 5: TDI 6: GND
+-+ 7 8 | 7: 8: GND
| 9 10 | 9: TCK 10: GND
| 11 12 | 11: 12: GND
+-+ 13 14 | 13: TDO 14:
| 15 16 | 15: RESET 16:
| 17 18 | 17: 18:
| 19 20 | 19: PWR_5V 20:
+-------+
IMO, more convenient, your call.
Patch Set #1, Line 1157: syntax where \fBfrequency\fP is the SPI clock frequency in Hz.
For all other programmers that take arbitrary numbers for `spispeed`
it's in kHz. Please use that here, too. (There is always talk about
unifying the code to parse `spispeed` throughout flashrom, patches
are welcome. :))
112 columns line limit, please remove unnecessary line breaks.
good coding overall :)
Patch Set #1, Line 8: * the Free Software Foundation; version 2 of the License.
My usual question here: Is the limitation to v2 intended (or
just copy pasted from one of the problematic files)?
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
Please remove.
Please start with /* on an otherwise empty line (like in your
other comments).
Patch Set #1, Line 21: * See https://www.segger.com/ for more info.
Usually, URLs are discouraged in the code, but I guess this one
is somewhat stable?
Patch Set #1, Line 39: UINT16_MAX / 8
This is limited by the *software* interface, right? Please mention
that in the commant, if so.
Patch Set #1, Line 71: jaylink_strerror(ret));
We exercise a 112 column limit for flashrom, no need to break the line.
Patch Set #1, Line 78: jaylink_jtag_clear_trst
Not sure if the distinction which function was actually called
would be helpful for debugging (you'd know which one it was by
the state of `reset_cs`). OTOH, it would save some lines if
you'd move the return check out of the outer if block (e.g. with
"Asserting CS failed: %s.\n").
Patch Set #1, Line 95: clear
set
Patch Set #1, Line 103: clear
set
Patch Set #1, Line 130: free(buffer);
Do not free it here or set `buffer = NULL;`. Otherwise we'd
double free on shutdown, I guess.
Patch Set #1, Line 134: buffer = tmp;
`buffer_size = length;` might be a good idea here oO
Patch Set #1, Line 378: JAYLINK_DEV_EXT_CAPS_SIZE
`sizeof(caps)` preferred
To view, visit change 28087. To unsubscribe, or for help writing mail filters, visit settings.