Rebased this one, following patches are based on it.
This adds a programmer parameter 'spispeed' to the dediprog driver to control the transfer rate on the spi bus. The following rates are available (in Hz): 375k, 750k, 1.5M, 2.18M, 3M, 8M, 12M and 24M
The original driver reinitializes the programmer after setting the speed, so the initialization calls have moved into a new function dediprog_setup() which is called twice.
Signed-off-by: Nico Huber nico.huber@secunet.com
diff --git a/dediprog.c b/dediprog.c index a81cf83..60067a8 100644 --- a/dediprog.c +++ b/dediprog.c @@ -152,7 +152,23 @@ static int dediprog_set_spi_voltage(int millivolt) return 0; }
-#if 0 +struct dediprog_spispeeds { + const char *const name; + const int speed; +}; + +static const struct dediprog_spispeeds spispeeds[] = { + { "24M", 0x0 }, + { "12M", 0x2 }, + { "8M", 0x1 }, + { "3M", 0x3 }, + { "2.18M", 0x4 }, + { "1.5M", 0x5 }, + { "750k", 0x6 }, + { "375k", 0x7 }, + { NULL, 0x0 }, +}; + /* After dediprog_set_spi_speed, the original app always calls * dediprog_set_spi_voltage(0) and then * dediprog_check_devicestring() four times in a row. @@ -160,56 +176,20 @@ static int dediprog_set_spi_voltage(int millivolt) * This looks suspiciously like the microprocessor in the SF100 has to be * restarted/reinitialized in case the speed changes. */ -static int dediprog_set_spi_speed(uint16_t speed) +static int dediprog_set_spi_speed(unsigned int spispeed_idx) { int ret; - unsigned int khz;
- /* Case 1 and 2 are in weird order. Probably an organically "grown" - * interface. - * Base frequency is 24000 kHz, divisors are (in order) - * 1, 3, 2, 8, 11, 16, 32, 64. - */ - switch (speed) { - case 0x0: - khz = 24000; - break; - case 0x1: - khz = 8000; - break; - case 0x2: - khz = 12000; - break; - case 0x3: - khz = 3000; - break; - case 0x4: - khz = 2180; - break; - case 0x5: - khz = 1500; - break; - case 0x6: - khz = 750; - break; - case 0x7: - khz = 375; - break; - default: - msg_perr("Unknown frequency selector 0x%x! Aborting.\n", speed); - return 1; - } - msg_pdbg("Setting SPI speed to %u kHz\n", khz); + msg_pdbg("SPI speed is %sHz\n", spispeeds[spispeed_idx].name);
- ret = usb_control_msg(dediprog_handle, 0x42, 0x61, speed, 0xff, NULL, - 0x0, DEFAULT_TIMEOUT); + ret = usb_control_msg(dediprog_handle, 0x42, 0x61, spispeeds[spispeed_idx].speed, 0xff, + NULL, 0x0, DEFAULT_TIMEOUT); if (ret != 0x0) { - msg_perr("Command Set SPI Speed 0x%x failed!\n", speed); + msg_perr("Command Set SPI Speed 0x%x failed!\n", spispeeds[spispeed_idx].speed); return 1; } return 0; } -#endif
/* Bulk read interface, will read multiple 512 byte chunks aligned to 512 bytes. * @start start address @@ -736,6 +716,28 @@ static int parse_voltage(char *voltage) return millivolt; }
+static int dediprog_setup(void) +{ + /* URB 6. Command A. */ + if (dediprog_command_a()) { + return 1; + } + /* URB 7. Command A. */ + if (dediprog_command_a()) { + return 1; + } + /* URB 8. Command Prepare Receive Device String. */ + /* URB 9. Command Receive Device String. */ + if (dediprog_check_devicestring()) { + return 1; + } + /* URB 10. Command C. */ + if (dediprog_command_c()) { + return 1; + } + return 0; +} + static const struct spi_programmer spi_programmer_dediprog = { .type = SPI_CONTROLLER_DEDIPROG, .max_data_read = MAX_DATA_UNSPECIFIED, @@ -777,12 +779,21 @@ static int dediprog_shutdown(void *data) int dediprog_init(void) { struct usb_device *dev; - char *voltage; - int millivolt = 3500; - int ret; + char *voltage, *spispeed; + int spispeed_idx = 2, millivolt = 3500; + int i, ret;
msg_pspew("%s\n", __func__);
+ spispeed = extract_programmer_param("spispeed"); + if (spispeed) { + for (i = 0; spispeeds[i].name; ++i) { + if (!strcasecmp(spispeeds[i].name, spispeed)) { + spispeed_idx = i; + break; + } + } + } voltage = extract_programmer_param("voltage"); if (voltage) { millivolt = parse_voltage(voltage); @@ -821,33 +832,26 @@ int dediprog_init(void) return 1; } dediprog_endpoint = 2; - + if (register_shutdown(dediprog_shutdown, NULL)) return 1;
dediprog_set_leds(PASS_ON|BUSY_ON|ERROR_ON);
- /* URB 6. Command A. */ - if (dediprog_command_a()) { + /* Perform basic setup. */ + if (dediprog_setup()) { dediprog_set_leds(PASS_OFF|BUSY_OFF|ERROR_ON); return 1; } - /* URB 7. Command A. */ - if (dediprog_command_a()) { - dediprog_set_leds(PASS_OFF|BUSY_OFF|ERROR_ON); - return 1; - } - /* URB 8. Command Prepare Receive Device String. */ - /* URB 9. Command Receive Device String. */ - if (dediprog_check_devicestring()) { - dediprog_set_leds(PASS_OFF|BUSY_OFF|ERROR_ON); - return 1; - } - /* URB 10. Command C. */ - if (dediprog_command_c()) { + + /* After setting voltage and speed, perform setup again. */ + if (dediprog_set_spi_voltage(0) || + dediprog_set_spi_speed(spispeed_idx) || + dediprog_setup()) { dediprog_set_leds(PASS_OFF|BUSY_OFF|ERROR_ON); return 1; } + /* URB 11. Command Set SPI Voltage. */ if (dediprog_set_spi_voltage(millivolt)) { dediprog_set_leds(PASS_OFF|BUSY_OFF|ERROR_ON); diff --git a/flashrom.8 b/flashrom.8 index b054c28..99e064e 100644 --- a/flashrom.8 +++ b/flashrom.8 @@ -651,6 +651,18 @@ where can be .BR 0V ", " 1.8V ", " 2.5V ", " 3.5V or the equivalent in mV. +.sp +An optional +.B spispeed +parameter specifies the frequency of the SPI bus. Syntax is +.sp +.B " flashrom -p dediprog:spispeed=frequency" +.sp +where +.B frequency +can be +.BR 375k ", " 750k ", " 1.5M ", " 2.18M ", " 3M ", " 8M ", " 12M " or " 24M +(in Hz). The default is a frequency of 12 MHz. .SS .BR "rayer_spi " programmer The default I/O base address used for the parallel port is 0x378 and you can use
Hi Nico,
I'm very sorry about the long review delay.
Am 16.11.2012 11:23 schrieb Nico Huber:
This adds a programmer parameter 'spispeed' to the dediprog driver to control the transfer rate on the spi bus. The following rates are available (in Hz): 375k, 750k, 1.5M, 2.18M, 3M, 8M, 12M and 24M
The original driver reinitializes the programmer after setting the speed, so the initialization calls have moved into a new function dediprog_setup() which is called twice.
Signed-off-by: Nico Huber nico.huber@secunet.com
diff --git a/dediprog.c b/dediprog.c index a81cf83..60067a8 100644 --- a/dediprog.c +++ b/dediprog.c @@ -777,12 +779,21 @@ static int dediprog_shutdown(void *data) int dediprog_init(void) { struct usb_device *dev;
- char *voltage;
- int millivolt = 3500;
- int ret;
char *voltage, *spispeed;
int spispeed_idx = 2, millivolt = 3500;
int i, ret;
msg_pspew("%s\n", __func__);
spispeed = extract_programmer_param("spispeed");
if (spispeed) {
for (i = 0; spispeeds[i].name; ++i) {if (!strcasecmp(spispeeds[i].name, spispeed)) {spispeed_idx = i;break;}}}
No error handling for invalid strings, memory leak of spispeed. I have fixed the issues. See below for the updated patch.
If the updated patch is OK with you, I'll ack and commit.
Regards, Carl-Daniel
Index: flashrom-dediprog_spispeed_nicohuber/dediprog.c =================================================================== --- flashrom-dediprog_spispeed_nicohuber/dediprog.c (Revision 1648) +++ flashrom-dediprog_spispeed_nicohuber/dediprog.c (Arbeitskopie) @@ -158,7 +158,23 @@ return 0; }
-#if 0 +struct dediprog_spispeeds { + const char *const name; + const int speed; +}; + +static const struct dediprog_spispeeds spispeeds[] = { + { "24M", 0x0 }, + { "12M", 0x2 }, + { "8M", 0x1 }, + { "3M", 0x3 }, + { "2.18M", 0x4 }, + { "1.5M", 0x5 }, + { "750k", 0x6 }, + { "375k", 0x7 }, + { NULL, 0x0 }, +}; + /* After dediprog_set_spi_speed, the original app always calls * dediprog_set_spi_voltage(0) and then * dediprog_check_devicestring() four times in a row. @@ -166,56 +182,20 @@ * This looks suspiciously like the microprocessor in the SF100 has to be * restarted/reinitialized in case the speed changes. */ -static int dediprog_set_spi_speed(uint16_t speed) +static int dediprog_set_spi_speed(unsigned int spispeed_idx) { int ret; - unsigned int khz;
- /* Case 1 and 2 are in weird order. Probably an organically "grown" - * interface. - * Base frequency is 24000 kHz, divisors are (in order) - * 1, 3, 2, 8, 11, 16, 32, 64. - */ - switch (speed) { - case 0x0: - khz = 24000; - break; - case 0x1: - khz = 8000; - break; - case 0x2: - khz = 12000; - break; - case 0x3: - khz = 3000; - break; - case 0x4: - khz = 2180; - break; - case 0x5: - khz = 1500; - break; - case 0x6: - khz = 750; - break; - case 0x7: - khz = 375; - break; - default: - msg_perr("Unknown frequency selector 0x%x! Aborting.\n", speed); - return 1; - } - msg_pdbg("Setting SPI speed to %u kHz\n", khz); + msg_pdbg("SPI speed is %sHz\n", spispeeds[spispeed_idx].name);
- ret = usb_control_msg(dediprog_handle, 0x42, 0x61, speed, 0xff, NULL, - 0x0, DEFAULT_TIMEOUT); + ret = usb_control_msg(dediprog_handle, 0x42, 0x61, spispeeds[spispeed_idx].speed, 0xff, + NULL, 0x0, DEFAULT_TIMEOUT); if (ret != 0x0) { - msg_perr("Command Set SPI Speed 0x%x failed!\n", speed); + msg_perr("Command Set SPI Speed 0x%x failed!\n", spispeeds[spispeed_idx].speed); return 1; } return 0; } -#endif
/* Bulk read interface, will read multiple 512 byte chunks aligned to 512 bytes. * @start start address @@ -742,6 +722,28 @@ return millivolt; }
+static int dediprog_setup(void) +{ + /* URB 6. Command A. */ + if (dediprog_command_a()) { + return 1; + } + /* URB 7. Command A. */ + if (dediprog_command_a()) { + return 1; + } + /* URB 8. Command Prepare Receive Device String. */ + /* URB 9. Command Receive Device String. */ + if (dediprog_check_devicestring()) { + return 1; + } + /* URB 10. Command C. */ + if (dediprog_command_c()) { + return 1; + } + return 0; +} + static const struct spi_programmer spi_programmer_dediprog = { .type = SPI_CONTROLLER_DEDIPROG, .max_data_read = MAX_DATA_UNSPECIFIED, @@ -783,13 +785,29 @@ int dediprog_init(void) { struct usb_device *dev; - char *voltage, *device; + char *voltage, *device, *spispeed; + int spispeed_idx = 2; int millivolt = 3500; long usedevice = 0; - int ret; + int i, ret;
msg_pspew("%s\n", __func__);
+ spispeed = extract_programmer_param("spispeed"); + if (spispeed) { + for (i = 0; spispeeds[i].name; ++i) { + if (!strcasecmp(spispeeds[i].name, spispeed)) { + spispeed_idx = i; + break; + } + } + if (!spispeeds[i].name) { + msg_perr("Error: Invalid 'spispeed' value.\n"); + free(spispeed); + return 1; + } + free(spispeed); + } voltage = extract_programmer_param("voltage"); if (voltage) { millivolt = parse_voltage(voltage); @@ -852,33 +870,24 @@ return 1; } dediprog_endpoint = 2; - + if (register_shutdown(dediprog_shutdown, NULL)) return 1;
dediprog_set_leds(PASS_ON|BUSY_ON|ERROR_ON);
- /* URB 6. Command A. */ - if (dediprog_command_a()) { + /* Perform basic setup. */ + if (dediprog_setup()) { dediprog_set_leds(PASS_OFF|BUSY_OFF|ERROR_ON); return 1; } - /* URB 7. Command A. */ - if (dediprog_command_a()) { + + /* After setting voltage and speed, perform setup again. */ + if (dediprog_set_spi_voltage(0) || dediprog_set_spi_speed(spispeed_idx) || dediprog_setup()) { dediprog_set_leds(PASS_OFF|BUSY_OFF|ERROR_ON); return 1; } - /* URB 8. Command Prepare Receive Device String. */ - /* URB 9. Command Receive Device String. */ - if (dediprog_check_devicestring()) { - dediprog_set_leds(PASS_OFF|BUSY_OFF|ERROR_ON); - return 1; - } - /* URB 10. Command C. */ - if (dediprog_command_c()) { - dediprog_set_leds(PASS_OFF|BUSY_OFF|ERROR_ON); - return 1; - } + /* URB 11. Command Set SPI Voltage. */ if (dediprog_set_spi_voltage(millivolt)) { dediprog_set_leds(PASS_OFF|BUSY_OFF|ERROR_ON); Index: flashrom-dediprog_spispeed_nicohuber/flashrom.8 =================================================================== --- flashrom-dediprog_spispeed_nicohuber/flashrom.8 (Revision 1648) +++ flashrom-dediprog_spispeed_nicohuber/flashrom.8 (Arbeitskopie) @@ -676,6 +676,18 @@ Usage example to select the second device: .sp .B " flashrom -p dediprog:device=1" +.sp +An optional +.B spispeed +parameter specifies the frequency of the SPI bus. Syntax is +.sp +.B " flashrom -p dediprog:spispeed=frequency" +.sp +where +.B frequency +can be +.BR 375k ", " 750k ", " 1.5M ", " 2.18M ", " 3M ", " 8M ", " 12M " or " 24M +(in Hz). The default is a frequency of 12 MHz. .SS .BR "rayer_spi " programmer The default I/O base address used for the parallel port is 0x378 and you can use
Hello Carl-Daniel,
Am 19.02.2013 22:56, schrieb Carl-Daniel Hailfinger:
Hi Nico,
I'm very sorry about the long review delay.
Anyway, thanks for your review.
Am 16.11.2012 11:23 schrieb Nico Huber:
This adds a programmer parameter 'spispeed' to the dediprog driver to control the transfer rate on the spi bus. The following rates are available (in Hz): 375k, 750k, 1.5M, 2.18M, 3M, 8M, 12M and 24M
The original driver reinitializes the programmer after setting the speed, so the initialization calls have moved into a new function dediprog_setup() which is called twice.
Signed-off-by: Nico Huber nico.huber@secunet.com
diff --git a/dediprog.c b/dediprog.c index a81cf83..60067a8 100644 --- a/dediprog.c +++ b/dediprog.c @@ -777,12 +779,21 @@ static int dediprog_shutdown(void *data) int dediprog_init(void) { struct usb_device *dev;
- char *voltage;
- int millivolt = 3500;
- int ret;
char *voltage, *spispeed;
int spispeed_idx = 2, millivolt = 3500;
int i, ret;
msg_pspew("%s\n", __func__);
spispeed = extract_programmer_param("spispeed");
if (spispeed) {
for (i = 0; spispeeds[i].name; ++i) {if (!strcasecmp(spispeeds[i].name, spispeed)) {spispeed_idx = i;break;}}}
No error handling for invalid strings, memory leak of spispeed. I have fixed the issues. See below for the updated patch.
Looks like I fixed the leak earlier, but didn't submit. Sorry.
If the updated patch is OK with you, I'll ack and commit.
Looks good to me.
Regards, Nico
Am 20.02.2013 10:23 schrieb Nico Huber:
Am 16.11.2012 11:23 schrieb Nico Huber:
This adds a programmer parameter 'spispeed' to the dediprog driver to control the transfer rate on the spi bus. The following rates are available (in Hz): 375k, 750k, 1.5M, 2.18M, 3M, 8M, 12M and 24M
The original driver reinitializes the programmer after setting the speed, so the initialization calls have moved into a new function dediprog_setup() which is called twice.
Signed-off-by: Nico Huber nico.huber@secunet.com
No error handling for invalid strings, memory leak of spispeed. I have fixed the issues. See below for the updated patch.
Looks like I fixed the leak earlier, but didn't submit. Sorry.
If the updated patch is OK with you, I'll ack and commit.
Looks good to me.
Acked-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net and committed in r1649.
Regards, Carl-Daniel
Avoid setting SPI speed on firmware versions < 5.0.0 and note this limitation in the man page. Use the index stored for a given speed in the translation array instead of the index of the element in that array.
Signed-off-by: Patrick Georgi patrick.georgi@secunet.com Signed-off-by: Stefan Tauner stefan.tauner@student.tuwien.ac.at --- IMHO the speed setting was broken ever since. Has anyone every checked the resulting frequency on the bus?
I am not entirely sure why the translation table was introduced in the first place although dediprog's indices could be "stored" in the array offset, but mixing the two approaches is of course not a good idea.
Nico, can you please verify that this patch is what you actually meant to do?
dediprog.c | 13 ++++++++----- flashrom.8 | 3 ++- 2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/dediprog.c b/dediprog.c index ab5388b..77ca195 100644 --- a/dediprog.c +++ b/dediprog.c @@ -184,12 +184,15 @@ static const struct dediprog_spispeeds spispeeds[] = { */ static int dediprog_set_spi_speed(unsigned int spispeed_idx) { - int ret; + if (dediprog_firmwareversion < FIRMWARE_VERSION(5, 0, 0)) { + msg_pwarn("Skipping to set SPI speed because firmware is too old.\n"); + return 0; + }
msg_pdbg("SPI speed is %sHz\n", spispeeds[spispeed_idx].name);
- ret = usb_control_msg(dediprog_handle, 0x42, 0x61, spispeeds[spispeed_idx].speed, 0xff, - NULL, 0x0, DEFAULT_TIMEOUT); + int ret = usb_control_msg(dediprog_handle, 0x42, 0x61, spispeeds[spispeed_idx].speed, 0xff, + NULL, 0x0, DEFAULT_TIMEOUT); if (ret != 0x0) { msg_perr("Command Set SPI Speed 0x%x failed!\n", spispeeds[spispeed_idx].speed); return 1; @@ -801,12 +804,12 @@ int dediprog_init(void) if (spispeed) { for (i = 0; spispeeds[i].name; ++i) { if (!strcasecmp(spispeeds[i].name, spispeed)) { - spispeed_idx = i; + spispeed_idx = spispeeds[i].speed; break; } } if (!spispeeds[i].name) { - msg_perr("Error: Invalid 'spispeed' value.\n"); + msg_perr("Error: Invalid spispeed value: '%s'.\n", spispeed); free(spispeed); return 1; } diff --git a/flashrom.8 b/flashrom.8 index c7a6c69..4e6ab55 100644 --- a/flashrom.8 +++ b/flashrom.8 @@ -683,7 +683,8 @@ Usage example to select the second device: .sp An optional .B spispeed -parameter specifies the frequency of the SPI bus. Syntax is +parameter specifies the frequency of the SPI bus. The firmware on the device needs to be 5.0.0 or newer. +Syntax is .sp .B " flashrom -p dediprog:spispeed=frequency" .sp
-
Carl-Daniel Hailfinger -
Nico Huber -
Stefan Tauner