Attention is currently required from: Felix Singer, Thomas Heijligen, Alexander Goncharov.
26 comments:
Patchset:
Hi Joursoir & Felix, thans for your reviews. I tried to address your coding style remarks, let me know if I missed something.
File dirtyjtag_spi.c:
Patch Set #4, Line 34: DJTAGUNK
It seems this is unused? […]
Yes UNK stands for UNKNOWN. I won't fix this, I will remove the enum instead since this isn't used at the moment.
I'll add it back when we settle on all the protocol changes for DJTAG2.
Patch Set #4, Line 39: 0xC0CA
Please use lower case
Done
Patch Set #4, Line 47: dirytjtag_command_identifier
typo: dir*ty*jtag
Done
Patch Set #4, Line 48: CMD_STOP = 0x00,
Please use TAB instead of two spaces.
Done
Patch Set #4, Line 57: dirytjtag_signal_identifier
typo: dir*ty*jtag
Done
Patch Set #4, Line 58: SIG_TCK = 1 << 1,
TABs here too.
Done
Patch Set #4, Line 66: char* data
`char *data`
Done
Patch Set #4, Line 87: char* data
`char *data`
Done
static inline unsigned int _min(unsigned int a, unsigned int b)
{
return (a < b) ? a : b;
}
Seems to be unused?
Done
Patch Set #4, Line 125: static int dirtyjtag_djtag1_spi_send_command(struct dirtyjtag_spi_data *context, unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr)
This is also pretty long. Same as with line 174.
Done
Patch Set #4, Line 128: writecnt+readcnt
spaces
Done
Patch Set #4, Line 129: size_t num_xfer = (len+max_xfer_size-1)/max_xfer_size; // ceil(len/max_xfer_size)
spaces
Done
Patch Set #4, Line 131: char *tx_buf = malloc(max_xfer_size*num_xfer);
spaces
Done
Patch Set #4, Line 133: msg_perr("%s: Failed tx_buf allocation", __func__);
Missing \n
Done
Patch Set #4, Line 136: char *rx_buf = malloc(max_xfer_size*num_xfer);
spaces
Done
Patch Set #4, Line 138: msg_perr("%s: Failed rx_buf allocation", __func__);
Missing \n
Done
Patch Set #4, Line 146: len%max_xfer_size
add spaces before and after %
Done
Patch Set #4, Line 147: len%max_xfer_size
same here
Done
Patch Set #4, Line 149: txn_size = 30;
How about setting this value during declaration in line 145? So it would be the "default" and if app […]
Done
Patch Set #4, Line 152: tx_buf+30*i
same here
Done
Patch Set #4, Line 155: rx_buf+i*30
same here
Done
Patch Set #4, Line 174: static int dirtyjtag_spi_spi_send_command(const struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr)
That line is pretty long. Please move each argument into a separate line or group them somehow. […]
Done
Patch Set #4, Line 248: tmp, strerror(errno));
nit: I would use one line here. It's not that long.
Done
lower case
Done
make this lower case
Done
To view, visit change 67878. To unsubscribe, or for help writing mail filters, visit settings.