Change in flashrom[master]: ft2232_spi.c: align with Chrome OS flashrom
Nikolai Artemiev has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/42796 ) Change subject: ft2232_spi.c: align with Chrome OS flashrom ...................................................................... ft2232_spi.c: align with Chrome OS flashrom Brings over various changes: - Limit servo channel selection to valid range - Use DIS_DIV_5 constant - Update some comments - Wrap long lines Signed-off-by: Nikolai Artemiev <nartemiev@google.com> Change-Id: I24c20e9b5d7e661d0180699bbd0d1447f6bf816f --- M ft2232_spi.c 1 file changed, 20 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/96/42796/1 diff --git a/ft2232_spi.c b/ft2232_spi.c index 87e6057..083e58f 100644 --- a/ft2232_spi.c +++ b/ft2232_spi.c @@ -110,7 +110,8 @@ { int i; for (i = 0; devs_ft2232spi[i].vendor_name != NULL; i++) { - if ((devs_ft2232spi[i].device_id == ft2232_type) && (devs_ft2232spi[i].vendor_id == ft2232_vid)) + if ((devs_ft2232spi[i].device_id == ft2232_type) + && (devs_ft2232spi[i].vendor_id == ft2232_vid)) return devs_ft2232spi[i].device_name; } return "unknown device"; @@ -120,7 +121,8 @@ { int i; for (i = 0; devs_ft2232spi[i].vendor_name != NULL; i++) { - if ((devs_ft2232spi[i].device_id == ft2232_type) && (devs_ft2232spi[i].vendor_id == ft2232_vid)) + if ((devs_ft2232spi[i].device_id == ft2232_type) + && (devs_ft2232spi[i].vendor_id == ft2232_vid)) return devs_ft2232spi[i].vendor_name; } return "unknown vendor"; @@ -132,7 +134,8 @@ int r; r = ftdi_write_data(ftdic, (unsigned char *) buf, size); if (r < 0) { - msg_perr("ftdi_write_data: %d, %s\n", r, ftdi_get_error_string(ftdic)); + msg_perr("ftdi_write_data: %d, %s\n", r, + ftdi_get_error_string(ftdic)); return 1; } return 0; @@ -146,7 +149,8 @@ while (size > 0) { r = ftdi_read_data(ftdic, (unsigned char *) buf, size); if (r < 0) { - msg_perr("ftdi_read_data: %d, %s\n", r, ftdi_get_error_string(ftdic)); + msg_perr("ftdi_read_data: %d, %s\n", r, + ftdi_get_error_string(ftdic)); return 1; } buf += r; @@ -183,8 +187,8 @@ enum ftdi_interface ft2232_interface = INTERFACE_A; /* * The 'H' chips can run with an internal clock of either 12 MHz or 60 MHz, - * but the non-H chips can only run at 12 MHz. We enable the divide-by-5 - * prescaler on the former to run on the same speed. + * but the non-H chips can only run at 12 MHz. We disable the divide-by-5 + * prescaler on 'H' chips so they run at 60MHz. */ uint8_t clock_5x = 1; /* In addition to the prescaler mentioned above there is also another @@ -193,7 +197,8 @@ * div = (1 + x) * 2 <-> x = div / 2 - 1 * Hence the expressible divisors are all even numbers between 2 and * 2^17 (=131072) resulting in SCK frequencies of 6 MHz down to about - * 92 Hz for 12 MHz inputs. + * 92 Hz for 12 MHz inputs and 30 MHz down to about 458 Hz for 60 MHz + * inputs. */ uint32_t divisor = DEFAULT_DIVISOR; int f; @@ -268,14 +273,17 @@ } else if (!strcasecmp(arg, "google-servo")) { ft2232_vid = GOOGLE_VID; ft2232_type = GOOGLE_SERVO_PID; + channel_count = 2; } else if (!strcasecmp(arg, "google-servo-v2")) { ft2232_vid = GOOGLE_VID; ft2232_type = GOOGLE_SERVO_V2_PID1; + channel_count = 2; /* Default divisor is too fast, and chip ID fails */ divisor = 6; } else if (!strcasecmp(arg, "google-servo-v2-legacy")) { ft2232_vid = GOOGLE_VID; ft2232_type = GOOGLE_SERVO_V2_PID0; + channel_count = 2; } else if (!strcasecmp(arg, "flyswatter")) { ft2232_type = FTDI_FT2232H_PID; channel_count = 2; @@ -381,7 +389,8 @@ free(arg); if (f < 0 && f != -5) { - msg_perr("Unable to open FTDI device: %d (%s).\n", f, ftdi_get_error_string(ftdic)); + msg_perr("Unable to open FTDI device: %d (%s)\n", f, + ftdi_get_error_string(ftdic)); return -4; } @@ -408,7 +417,7 @@ if (clock_5x) { msg_pdbg("Disable divide-by-5 front stage\n"); - buf[0] = 0x8a; /* Disable divide-by-5. DIS_DIV_5 in newer libftdi */ + buf[0] = DIS_DIV_5; if (send_buf(ftdic, buf, 1)) { ret = -5; goto ftdi_err; @@ -453,7 +462,8 @@ ftdi_err: if ((f = ftdi_usb_close(ftdic)) < 0) { - msg_perr("Unable to close FTDI device: %d (%s)\n", f, ftdi_get_error_string(ftdic)); + msg_perr("Unable to close FTDI device: %d (%s)\n", f, + ftdi_get_error_string(ftdic)); } return ret; } -- To view, visit https://review.coreboot.org/c/flashrom/+/42796 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I24c20e9b5d7e661d0180699bbd0d1447f6bf816f Gerrit-Change-Number: 42796 Gerrit-PatchSet: 1 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-MessageType: newchange
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/42796 ) Change subject: ft2232_spi.c: align with Chrome OS flashrom ...................................................................... Patch Set 1: Code-Review+1 (1 comment) https://review.coreboot.org/c/flashrom/+/42796/1/ft2232_spi.c File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/42796/1/ft2232_spi.c@420 PS1, Line 420: DIS_DIV_5; was this not using the define before because of needing to support older libftdi on other platforms perhaps? -- To view, visit https://review.coreboot.org/c/flashrom/+/42796 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I24c20e9b5d7e661d0180699bbd0d1447f6bf816f Gerrit-Change-Number: 42796 Gerrit-PatchSet: 1 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Fri, 26 Jun 2020 05:53:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/42796 ) Change subject: ft2232_spi.c: align with Chrome OS flashrom ...................................................................... Patch Set 1: (1 comment) https://review.coreboot.org/c/flashrom/+/42796/1/ft2232_spi.c File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/42796/1/ft2232_spi.c@420 PS1, Line 420: DIS_DIV_5;
was this not using the define before because of needing to support older libftdi on other platforms […] Yeah, it was switched to use DIS_DIV_5 and then switched back in `05151b6dcbba16746aa803069dd6046a82e33599` to support older libftdi. However that was ~6 years ago, and the constant was added to libftdi ~9 years ago.
I think it's ok, but we could conditionally define DIS_DIV_5, like HAVE_FT232H on lines 29-33. -- To view, visit https://review.coreboot.org/c/flashrom/+/42796 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I24c20e9b5d7e661d0180699bbd0d1447f6bf816f Gerrit-Change-Number: 42796 Gerrit-PatchSet: 1 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Fri, 26 Jun 2020 06:37:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec@chromium.org> Gerrit-MessageType: comment
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/42796 ) Change subject: ft2232_spi.c: align with Chrome OS flashrom ...................................................................... Patch Set 1: (1 comment) https://review.coreboot.org/c/flashrom/+/42796/1/ft2232_spi.c File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/42796/1/ft2232_spi.c@420 PS1, Line 420: DIS_DIV_5;
Yeah, it was switched to use DIS_DIV_5 and then switched back in `05151b6dcbba16746aa803069dd6046a82 […] Maybe bump the minimum version requirement in meson+Makefile?
-- To view, visit https://review.coreboot.org/c/flashrom/+/42796 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I24c20e9b5d7e661d0180699bbd0d1447f6bf816f Gerrit-Change-Number: 42796 Gerrit-PatchSet: 1 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Fri, 26 Jun 2020 06:39:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec@chromium.org> Comment-In-Reply-To: Nikolai Artemiev <nartemiev@google.com> Gerrit-MessageType: comment
Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/42796 ) Change subject: ft2232_spi.c: align with Chrome OS flashrom ...................................................................... Patch Set 1: (1 comment) https://review.coreboot.org/c/flashrom/+/42796/1/ft2232_spi.c File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/42796/1/ft2232_spi.c@420 PS1, Line 420: DIS_DIV_5;
Maybe bump the minimum version requirement in meson+Makefile? It looks like they already depend on libftdi1, which has DIS_DIV_5 AFAICT.
-- To view, visit https://review.coreboot.org/c/flashrom/+/42796 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I24c20e9b5d7e661d0180699bbd0d1447f6bf816f Gerrit-Change-Number: 42796 Gerrit-PatchSet: 1 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Fri, 26 Jun 2020 07:01:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec@chromium.org> Comment-In-Reply-To: Nikolai Artemiev <nartemiev@google.com> Gerrit-MessageType: comment
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/42796 ) Change subject: ft2232_spi.c: align with Chrome OS flashrom ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/42796 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I24c20e9b5d7e661d0180699bbd0d1447f6bf816f Gerrit-Change-Number: 42796 Gerrit-PatchSet: 1 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Fri, 26 Jun 2020 07:22:17 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/42796 ) Change subject: ft2232_spi.c: align with Chrome OS flashrom ...................................................................... Patch Set 1: (1 comment) https://review.coreboot.org/c/flashrom/+/42796/1/ft2232_spi.c File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/42796/1/ft2232_spi.c@420 PS1, Line 420: DIS_DIV_5;
It looks like they already depend on libftdi1, which has DIS_DIV_5 AFAICT. OK, SGTM. Please give some time for the community to review as well.
-- To view, visit https://review.coreboot.org/c/flashrom/+/42796 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I24c20e9b5d7e661d0180699bbd0d1447f6bf816f Gerrit-Change-Number: 42796 Gerrit-PatchSet: 1 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Fri, 26 Jun 2020 07:23:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec@chromium.org> Comment-In-Reply-To: Nikolai Artemiev <nartemiev@google.com> Gerrit-MessageType: comment
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/42796 ) Change subject: ft2232_spi.c: align with Chrome OS flashrom ...................................................................... Patch Set 1: Code-Review+1 (2 comments) https://review.coreboot.org/c/flashrom/+/42796/1/ft2232_spi.c File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/42796/1/ft2232_spi.c@276 PS1, Line 276: channel_count = 2; Which is the rationale for this change? https://review.coreboot.org/c/flashrom/+/42796/1/ft2232_spi.c@420 PS1, Line 420: DIS_DIV_5;
OK, SGTM. Please give some time for the community to review as well. I think we ported the few programmers using old libftdi to use the newer version, so we should be safe
-- To view, visit https://review.coreboot.org/c/flashrom/+/42796 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I24c20e9b5d7e661d0180699bbd0d1447f6bf816f Gerrit-Change-Number: 42796 Gerrit-PatchSet: 1 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Fri, 26 Jun 2020 12:17:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Edward O'Callaghan <quasisec@chromium.org> Comment-In-Reply-To: Nikolai Artemiev <nartemiev@google.com> Gerrit-MessageType: comment
Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/42796 to look at the new patch set (#2). Change subject: ft2232_spi.c: align with Chrome OS flashrom ...................................................................... ft2232_spi.c: align with Chrome OS flashrom Brings over various changes: - Use DIS_DIV_5 constant - Update some comments - Wrap long lines Signed-off-by: Nikolai Artemiev <nartemiev@google.com> Change-Id: I24c20e9b5d7e661d0180699bbd0d1447f6bf816f --- M ft2232_spi.c 1 file changed, 17 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/96/42796/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/42796 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I24c20e9b5d7e661d0180699bbd0d1447f6bf816f Gerrit-Change-Number: 42796 Gerrit-PatchSet: 2 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: newpatchset
Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/42796 ) Change subject: ft2232_spi.c: align with Chrome OS flashrom ...................................................................... Patch Set 2: (1 comment) https://review.coreboot.org/c/flashrom/+/42796/1/ft2232_spi.c File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/42796/1/ft2232_spi.c@276 PS1, Line 276: channel_count = 2;
Which is the rationale for this change? Historically cros flashrom only supported channels A/B as it didn't have commit fbc71ac494a798. I added the 'channel_count = 2' restrictions when I brought over that commit since it didn't look like anyone used channels C/D, but we probably should support them.
-- To view, visit https://review.coreboot.org/c/flashrom/+/42796 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I24c20e9b5d7e661d0180699bbd0d1447f6bf816f Gerrit-Change-Number: 42796 Gerrit-PatchSet: 2 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Tue, 14 Jul 2020 02:20:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus@gmail.com> Gerrit-MessageType: comment
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/42796 ) Change subject: ft2232_spi.c: align with Chrome OS flashrom ...................................................................... Patch Set 2: (1 comment) https://review.coreboot.org/c/flashrom/+/42796/1/ft2232_spi.c File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/42796/1/ft2232_spi.c@276 PS1, Line 276: channel_count = 2;
Historically cros flashrom only supported channels A/B as it didn't have commit fbc71ac494a798. […] Then why isn't it mentioned in the commit message?
-- To view, visit https://review.coreboot.org/c/flashrom/+/42796 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I24c20e9b5d7e661d0180699bbd0d1447f6bf816f Gerrit-Change-Number: 42796 Gerrit-PatchSet: 2 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Tue, 14 Jul 2020 08:18:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus@gmail.com> Comment-In-Reply-To: Nikolai Artemiev <nartemiev@google.com> Gerrit-MessageType: comment
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/42796 ) Change subject: ft2232_spi.c: align with Chrome OS flashrom ...................................................................... Patch Set 2: Code-Review+2 (1 comment) https://review.coreboot.org/c/flashrom/+/42796/1/ft2232_spi.c File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/42796/1/ft2232_spi.c@276 PS1, Line 276: channel_count = 2;
Then why isn't it mentioned in the commit message? The changes were dropped from the latest patchset
-- To view, visit https://review.coreboot.org/c/flashrom/+/42796 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I24c20e9b5d7e661d0180699bbd0d1447f6bf816f Gerrit-Change-Number: 42796 Gerrit-PatchSet: 2 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Tue, 14 Jul 2020 08:20:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Angel Pons <th3fanbus@gmail.com> Comment-In-Reply-To: Nikolai Artemiev <nartemiev@google.com> Gerrit-MessageType: comment
Angel Pons has submitted this change. ( https://review.coreboot.org/c/flashrom/+/42796 ) Change subject: ft2232_spi.c: align with Chrome OS flashrom ...................................................................... ft2232_spi.c: align with Chrome OS flashrom Brings over various changes: - Use DIS_DIV_5 constant - Update some comments - Wrap long lines Signed-off-by: Nikolai Artemiev <nartemiev@google.com> Change-Id: I24c20e9b5d7e661d0180699bbd0d1447f6bf816f Reviewed-on: https://review.coreboot.org/c/flashrom/+/42796 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com> --- M ft2232_spi.c 1 file changed, 17 insertions(+), 10 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved diff --git a/ft2232_spi.c b/ft2232_spi.c index 87e6057..65ff449 100644 --- a/ft2232_spi.c +++ b/ft2232_spi.c @@ -110,7 +110,8 @@ { int i; for (i = 0; devs_ft2232spi[i].vendor_name != NULL; i++) { - if ((devs_ft2232spi[i].device_id == ft2232_type) && (devs_ft2232spi[i].vendor_id == ft2232_vid)) + if ((devs_ft2232spi[i].device_id == ft2232_type) + && (devs_ft2232spi[i].vendor_id == ft2232_vid)) return devs_ft2232spi[i].device_name; } return "unknown device"; @@ -120,7 +121,8 @@ { int i; for (i = 0; devs_ft2232spi[i].vendor_name != NULL; i++) { - if ((devs_ft2232spi[i].device_id == ft2232_type) && (devs_ft2232spi[i].vendor_id == ft2232_vid)) + if ((devs_ft2232spi[i].device_id == ft2232_type) + && (devs_ft2232spi[i].vendor_id == ft2232_vid)) return devs_ft2232spi[i].vendor_name; } return "unknown vendor"; @@ -132,7 +134,8 @@ int r; r = ftdi_write_data(ftdic, (unsigned char *) buf, size); if (r < 0) { - msg_perr("ftdi_write_data: %d, %s\n", r, ftdi_get_error_string(ftdic)); + msg_perr("ftdi_write_data: %d, %s\n", r, + ftdi_get_error_string(ftdic)); return 1; } return 0; @@ -146,7 +149,8 @@ while (size > 0) { r = ftdi_read_data(ftdic, (unsigned char *) buf, size); if (r < 0) { - msg_perr("ftdi_read_data: %d, %s\n", r, ftdi_get_error_string(ftdic)); + msg_perr("ftdi_read_data: %d, %s\n", r, + ftdi_get_error_string(ftdic)); return 1; } buf += r; @@ -183,8 +187,8 @@ enum ftdi_interface ft2232_interface = INTERFACE_A; /* * The 'H' chips can run with an internal clock of either 12 MHz or 60 MHz, - * but the non-H chips can only run at 12 MHz. We enable the divide-by-5 - * prescaler on the former to run on the same speed. + * but the non-H chips can only run at 12 MHz. We disable the divide-by-5 + * prescaler on 'H' chips so they run at 60MHz. */ uint8_t clock_5x = 1; /* In addition to the prescaler mentioned above there is also another @@ -193,7 +197,8 @@ * div = (1 + x) * 2 <-> x = div / 2 - 1 * Hence the expressible divisors are all even numbers between 2 and * 2^17 (=131072) resulting in SCK frequencies of 6 MHz down to about - * 92 Hz for 12 MHz inputs. + * 92 Hz for 12 MHz inputs and 30 MHz down to about 458 Hz for 60 MHz + * inputs. */ uint32_t divisor = DEFAULT_DIVISOR; int f; @@ -381,7 +386,8 @@ free(arg); if (f < 0 && f != -5) { - msg_perr("Unable to open FTDI device: %d (%s).\n", f, ftdi_get_error_string(ftdic)); + msg_perr("Unable to open FTDI device: %d (%s)\n", f, + ftdi_get_error_string(ftdic)); return -4; } @@ -408,7 +414,7 @@ if (clock_5x) { msg_pdbg("Disable divide-by-5 front stage\n"); - buf[0] = 0x8a; /* Disable divide-by-5. DIS_DIV_5 in newer libftdi */ + buf[0] = DIS_DIV_5; if (send_buf(ftdic, buf, 1)) { ret = -5; goto ftdi_err; @@ -453,7 +459,8 @@ ftdi_err: if ((f = ftdi_usb_close(ftdic)) < 0) { - msg_perr("Unable to close FTDI device: %d (%s)\n", f, ftdi_get_error_string(ftdic)); + msg_perr("Unable to close FTDI device: %d (%s)\n", f, + ftdi_get_error_string(ftdic)); } return ret; } -- To view, visit https://review.coreboot.org/c/flashrom/+/42796 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I24c20e9b5d7e661d0180699bbd0d1447f6bf816f Gerrit-Change-Number: 42796 Gerrit-PatchSet: 3 Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: merged
participants (3)
-
Angel Pons (Code Review) -
Edward O'Callaghan (Code Review)
-
Nikolai Artemiev (Code Review)