Hello Raul Rangel,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/em100/+/34829
to review the following change.
Change subject: Allow selection of FPGA version for 1.8/3.3V ......................................................................
Allow selection of FPGA version for 1.8/3.3V
Add a -V flag to change the FPGA version to use so that we can switch between 1.8V and 3.3V. This is only supported on EM100-Pro-G2 units.
It should be possible to automatically change the voltage depending on the selected SPI-flash chip, but I have not looked at that yet.
BUG=chromium:918574 TEST=em100 -V 1.8 See that it changes to 1.8V and reports that on start-up: $ sudo ./em100 MCU version: 3.03 FPGA version: 2.10 (3.3V) Serial number: DP142535 SPI flash database: 4.3.01 EM100Pro currently stopped EM100Pro hold pin currently low
$ sudo ./em100 -V 1.8 MCU version: 3.03 FPGA version: 2.10 (3.3V) Serial number: DP142535 SPI flash database: 4.3.01 EM100Pro currently stopped EM100Pro hold pin currently low
Voltage set to 1.8 $ sudo ./em100 MCU version: 3.03 FPGA version: 2.10 (1.8V) Serial number: DP142535 SPI flash database: 4.3.01 EM100Pro currently stopped EM100Pro hold pin currently low
Change-Id: I9a8ea932027978129d021ec166dc7fbd126b0f1e Signed-off-by: Simon Glass sjg@chromium.org Reviewed-on: https://chromium-review.googlesource.com/1393563 Reviewed-by: Raul E Rangel rrangel@chromium.org --- M em100.c M em100.h M fpga.c 3 files changed, 105 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/em100 refs/changes/29/34829/1
diff --git a/em100.c b/em100.c index 64a4ab2..01fe761 100644 --- a/em100.c +++ b/em100.c @@ -21,6 +21,7 @@ #include <strings.h> #include <signal.h> #include <getopt.h> +#include <unistd.h> #include "em100.h"
/* SPI flash chips parameters definition */ @@ -144,6 +145,56 @@ return set_hold_pin_state(em100, pin_state); }
+static int set_fpga_voltage(struct em100 *em100, int voltage_code) +{ + int val; + + if (!fpga_reconfigure(em100)) { + printf("Couldn't reconfigure FPGA.\n"); + return 0; + } + + if (!fpga_set_voltage(em100, voltage_code)) { + printf("Couldn't set FPGA voltage.\n"); + return 0; + } + + /* Must wait 2s before issuing any other USB comand */ + sleep(2); + + if (!fpga_get_voltage(em100, &val)) { + printf("Couldn't get FPGA voltage.\n"); + return 0; + } + + if (val != voltage_code) { + printf("Invalid voltage response: %#x (expected %#x)\n", val, + voltage_code); + return 0; + } + + printf("Voltage set to %s\n", val == 18 ? "1.8" : "3.3"); + + return 1; +} + +static int set_fpga_voltage_from_str(struct em100 *em100, + const char *voltage_str) +{ + int voltage_code; + + if (!strcmp(voltage_str, "3.3")) + voltage_code = 33; + else if (!strcmp(voltage_str, "1.8")) + voltage_code = 18; + else { + printf("Invalid voltage, use 1.8 or 3.3.\n"); + return 0; + } + + return set_fpga_voltage(em100, voltage_code); +} + /** * get_serialno: fetch device's serial number * @param em100: initialized em100 device structure @@ -553,6 +604,7 @@ " -f|--firmware-dump FILE: export raw EM100pro firmware to file\n" " -g|--firmware-write FILE: export EM100pro firmware to DPFW file\n" " -S|--set-serialno NUM: set serial number to NUM\n" + " -V|--set-voltage [1.8|3.3] switch FPGA voltage\n" " -p|--holdpin [LOW|FLOAT|INPUT]: set the hold pin state\n" " -x|--device BUS:DEV use EM100pro on USB bus/device\n" " -x|--device DPxxxxxx use EM100pro with serial no DPxxxxxx\n" @@ -579,8 +631,9 @@ unsigned int serial_number = 0; unsigned long address_offset = 0; unsigned long spi_start_address = 0; + const char *voltage = NULL;
- while ((opt = getopt_long(argc, argv, "c:d:a:u:rsvtO:F:f:g:S:p:Dx:lhT", + while ((opt = getopt_long(argc, argv, "c:d:a:u:rsvtO:F:f:g:S:V:p:Dx:lhT", longopts, &idx)) != -1) { switch (opt) { case 'c': @@ -597,6 +650,9 @@ case 'u': read_filename = optarg; break; + case 'V': + voltage = optarg; + break; case 'p': holdpin = optarg; break; @@ -747,6 +803,13 @@ printf("Chip set to %s\n", desiredchip); }
+ if (voltage) { + if (!set_fpga_voltage_from_str(&em100, voltage)) { + printf("Failed configuring FPGA voltage.\n"); + return 1; + } + } + if (holdpin) { if (!set_hold_pin_state_from_str(&em100, holdpin)) { printf("Failed configuring hold pin state.\n"); diff --git a/em100.h b/em100.h index 10eea73..6a58e70 100644 --- a/em100.h +++ b/em100.h @@ -44,6 +44,9 @@ int check_fpga_status(struct em100 *em100); int read_fpga_register(struct em100 *em100, int reg, uint16_t *val); int write_fpga_register(struct em100 *em100, int reg, int val); +int fpga_set_voltage(struct em100 *em100, int voltage_code); +int fpga_get_voltage(struct em100 *em100, int *voltage_codep); +int fpga_reconfigure(struct em100 *em100);
/* hexdump.c */ void hexdump(const void *memory, size_t length); diff --git a/fpga.c b/fpga.c index 5624264..2fecc3b 100644 --- a/fpga.c +++ b/fpga.c @@ -117,3 +117,41 @@ } return 1; } + +int fpga_set_voltage(struct em100 *em100, int voltage_code) +{ + unsigned char cmd[16]; + + memset(cmd, '\0', 16); + cmd[0] = 0x24; /* Switch FPGA */ + if (voltage_code == 18) { + cmd[2] = 7; + cmd[3] = 0x80; + } + if (!send_cmd(em100->dev, cmd)) + return 0; + + return 1; +} + +int fpga_get_voltage(struct em100 *em100, int *voltage_codep) +{ + if (!get_version(em100)) + return 0; + + *voltage_codep = em100->fpga & 0x8000 ? 18 : 33; + + return 1; +} + +int fpga_reconfigure(struct em100 *em100) +{ + unsigned char cmd[16]; + + memset(cmd, '\0', 16); + cmd[0] = 0x20; /* Switch FPGA */ + if (!send_cmd(em100->dev, cmd)) + return 0; + + return 1; +}
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/em100/+/34829 )
Change subject: Allow selection of FPGA version for 1.8/3.3V ......................................................................
Patch Set 1: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/em100/+/34829 )
Change subject: Allow selection of FPGA version for 1.8/3.3V ......................................................................
Allow selection of FPGA version for 1.8/3.3V
Add a -V flag to change the FPGA version to use so that we can switch between 1.8V and 3.3V. This is only supported on EM100-Pro-G2 units.
It should be possible to automatically change the voltage depending on the selected SPI-flash chip, but I have not looked at that yet.
BUG=chromium:918574 TEST=em100 -V 1.8 See that it changes to 1.8V and reports that on start-up: $ sudo ./em100 MCU version: 3.03 FPGA version: 2.10 (3.3V) Serial number: DP142535 SPI flash database: 4.3.01 EM100Pro currently stopped EM100Pro hold pin currently low
$ sudo ./em100 -V 1.8 MCU version: 3.03 FPGA version: 2.10 (3.3V) Serial number: DP142535 SPI flash database: 4.3.01 EM100Pro currently stopped EM100Pro hold pin currently low
Voltage set to 1.8 $ sudo ./em100 MCU version: 3.03 FPGA version: 2.10 (1.8V) Serial number: DP142535 SPI flash database: 4.3.01 EM100Pro currently stopped EM100Pro hold pin currently low
Change-Id: I9a8ea932027978129d021ec166dc7fbd126b0f1e Signed-off-by: Simon Glass sjg@chromium.org Reviewed-on: https://chromium-review.googlesource.com/1393563 Reviewed-by: Raul E Rangel rrangel@chromium.org Reviewed-on: https://review.coreboot.org/c/em100/+/34829 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Raul Rangel rrangel@chromium.org --- M em100.c M em100.h M fpga.c 3 files changed, 105 insertions(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved
diff --git a/em100.c b/em100.c index db550c7..61cc454 100644 --- a/em100.c +++ b/em100.c @@ -21,6 +21,7 @@ #include <strings.h> #include <signal.h> #include <getopt.h> +#include <unistd.h> #include "em100.h"
/* SPI flash chips parameters definition */ @@ -144,6 +145,56 @@ return set_hold_pin_state(em100, pin_state); }
+static int set_fpga_voltage(struct em100 *em100, int voltage_code) +{ + int val; + + if (!fpga_reconfigure(em100)) { + printf("Couldn't reconfigure FPGA.\n"); + return 0; + } + + if (!fpga_set_voltage(em100, voltage_code)) { + printf("Couldn't set FPGA voltage.\n"); + return 0; + } + + /* Must wait 2s before issuing any other USB comand */ + sleep(2); + + if (!fpga_get_voltage(em100, &val)) { + printf("Couldn't get FPGA voltage.\n"); + return 0; + } + + if (val != voltage_code) { + printf("Invalid voltage response: %#x (expected %#x)\n", val, + voltage_code); + return 0; + } + + printf("Voltage set to %s\n", val == 18 ? "1.8" : "3.3"); + + return 1; +} + +static int set_fpga_voltage_from_str(struct em100 *em100, + const char *voltage_str) +{ + int voltage_code; + + if (!strcmp(voltage_str, "3.3")) + voltage_code = 33; + else if (!strcmp(voltage_str, "1.8")) + voltage_code = 18; + else { + printf("Invalid voltage, use 1.8 or 3.3.\n"); + return 0; + } + + return set_fpga_voltage(em100, voltage_code); +} + /** * get_serialno: fetch device's serial number * @param em100: initialized em100 device structure @@ -553,6 +604,7 @@ " -f|--firmware-dump FILE: export raw EM100pro firmware to file\n" " -g|--firmware-write FILE: export EM100pro firmware to DPFW file\n" " -S|--set-serialno NUM: set serial number to NUM\n" + " -V|--set-voltage [1.8|3.3] switch FPGA voltage\n" " -p|--holdpin [LOW|FLOAT|INPUT]: set the hold pin state\n" " -x|--device BUS:DEV use EM100pro on USB bus/device\n" " -x|--device DPxxxxxx use EM100pro with serial no DPxxxxxx\n" @@ -579,8 +631,9 @@ unsigned int serial_number = 0; unsigned long address_offset = 0; unsigned long spi_start_address = 0; + const char *voltage = NULL;
- while ((opt = getopt_long(argc, argv, "c:d:a:u:rsvtO:F:f:g:S:p:Dx:lhT", + while ((opt = getopt_long(argc, argv, "c:d:a:u:rsvtO:F:f:g:S:V:p:Dx:lhT", longopts, &idx)) != -1) { switch (opt) { case 'c': @@ -597,6 +650,9 @@ case 'u': read_filename = optarg; break; + case 'V': + voltage = optarg; + break; case 'p': holdpin = optarg; break; @@ -747,6 +803,13 @@ printf("Chip set to %s\n", desiredchip); }
+ if (voltage) { + if (!set_fpga_voltage_from_str(&em100, voltage)) { + printf("Failed configuring FPGA voltage.\n"); + return 1; + } + } + if (holdpin) { if (!set_hold_pin_state_from_str(&em100, holdpin)) { printf("Failed configuring hold pin state.\n"); diff --git a/em100.h b/em100.h index 10eea73..6a58e70 100644 --- a/em100.h +++ b/em100.h @@ -44,6 +44,9 @@ int check_fpga_status(struct em100 *em100); int read_fpga_register(struct em100 *em100, int reg, uint16_t *val); int write_fpga_register(struct em100 *em100, int reg, int val); +int fpga_set_voltage(struct em100 *em100, int voltage_code); +int fpga_get_voltage(struct em100 *em100, int *voltage_codep); +int fpga_reconfigure(struct em100 *em100);
/* hexdump.c */ void hexdump(const void *memory, size_t length); diff --git a/fpga.c b/fpga.c index 5624264..2fecc3b 100644 --- a/fpga.c +++ b/fpga.c @@ -117,3 +117,41 @@ } return 1; } + +int fpga_set_voltage(struct em100 *em100, int voltage_code) +{ + unsigned char cmd[16]; + + memset(cmd, '\0', 16); + cmd[0] = 0x24; /* Switch FPGA */ + if (voltage_code == 18) { + cmd[2] = 7; + cmd[3] = 0x80; + } + if (!send_cmd(em100->dev, cmd)) + return 0; + + return 1; +} + +int fpga_get_voltage(struct em100 *em100, int *voltage_codep) +{ + if (!get_version(em100)) + return 0; + + *voltage_codep = em100->fpga & 0x8000 ? 18 : 33; + + return 1; +} + +int fpga_reconfigure(struct em100 *em100) +{ + unsigned char cmd[16]; + + memset(cmd, '\0', 16); + cmd[0] = 0x20; /* Switch FPGA */ + if (!send_cmd(em100->dev, cmd)) + return 0; + + return 1; +}
-
Martin Roth (Code Review) -
Patrick Georgi (Code Review)
-
Raul Rangel (Code Review)