Am Mittwoch, den 07.07.2010, 14:16 +0200 schrieb Carl-Daniel Hailfinger:
- /* Non-default port requested? */
- portpos = extract_param(&programmer_param, "it87spiport", ",:");
- if (portpos) {
char *endptr = NULL;if (strlen(portpos))forced_flashport = strtoul(portpos, &endptr, 0);/* Port 0, port >65535 and garbage strings are rejected. */if (!forced_flashport || (forced_flashport > 65535) ||(endptr && (*endptr != '\0'))) {
I don't get the idea of the if in the beginning. The strtoul mangpage states: "If there were no digits at all, strtol() stores the original value of nptr in *endptr (and returns 0)." So you can just omit the "if (strlen(portpos))" test, because strtoul() returns zero in this case (and doesn't change forced_flashport). If strtoul is executed unconditionally, you can also throw away the check for endptr not being NULL, as endptr is *always* set by strtoul.[1]
Alignment verification would be nice here, too. The datasheet should state the required alignment.
- if (forced_flashport) {
flashport = (uint16_t)forced_flashport;msg_pinfo("Forcing serial flash port 0x%04x\n",flashport);sio_write(port, 0x64, (flashport >> 8));sio_write(port, 0x65, (flashport & 0xff));- }
Can't you put this in an else branch of the if() shown above, or, in my oppinion even better: Invert the condition to something like
if(parameter makes sense) set flashport & setup hardware else print error message & exit(1)
The remaining stuff looks OK, but I didn't cross-check against the IT8705 datasheet. Do you want a cross-check too?
Regards, Michael Karcher
[1]: It is superflous also right now, as !forced_flashport would have short-circuited the *endptr test if strtoul was not called.