Hello Raul Rangel,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/em100/+/34830
to review the following change.
Change subject: Automatically select the right FPGA firmware for 1.8/3.3V ......................................................................
Automatically select the right FPGA firmware for 1.8/3.3V
With the -G2 variant we can automatically select the right voltage. Do this instead of just giving up. For the original em100 this will produce an error, with a 2s delay.
Success (new unit):
will emulate 'W25Q128FW' 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
Stopped EM100Pro Sending flash chip configuration Voltage set to 1.8 Chip set to W25Q128FW Hold pin state set to low Sent 2097152 bytes of 16777216 ...
Failure (old unit):
will emulate 'GD25Q32B' MCU version: 2.27 FPGA version: 0.85 (1.8V) Serial number: DP142073 SPI flash database: 4.3.01 EM100Pro currently running EM100Pro hold pin currently low
Stopped EM100Pro Sending flash chip configuration Invalid voltage response: 0x12 (expected 0x21) Error: The current FPGA firmware (1.8V) does not support GigaDevice GD25Q32B (3.3V) Failed configuring chip type.
Change-Id: I55596b9857c1f340d532acf5d261800a654068c6 Signed-off-by: Simon Glass sjg@chromium.org Reviewed-on: https://chromium-review.googlesource.com/1395949 Reviewed-by: Raul E Rangel rrangel@chromium.org --- M em100.c 1 file changed, 12 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/em100 refs/changes/30/34830/1
diff --git a/em100.c b/em100.c index 01fe761..fcd7bcd 100644 --- a/em100.c +++ b/em100.c @@ -510,7 +510,8 @@ */ int result = 0; int i; - int fpga_voltage, chip_voltage = 0, wrong_voltage = 0; + int fpga_voltage, chip_voltage = 0; + int req_voltage = 0;
printf("Sending flash chip configuration\n");
@@ -528,22 +529,25 @@ case 1601: /* 1.65V-2V */ case 1800: if (fpga_voltage == 3300) - wrong_voltage = 1; + req_voltage = 18; break; case 2500: /* supported by both 1.8V and 3.3V FPGA */ break; case 3300: if (fpga_voltage == 1800) - wrong_voltage = 1; + req_voltage = 33; } break; }
- if (wrong_voltage) { - printf("Error: The current FPGA firmware (%.1fV) does not " - "support %s %s (%.1fV)\n", (float)fpga_voltage/1000, - desc->vendor, desc->name, (float)chip_voltage/1000); - return 0; + if (req_voltage) { + if (!set_fpga_voltage(em100, req_voltage)) { + printf("Error: The current FPGA firmware (%.1fV) does " + "not support %s %s (%.1fV)\n", + (float)fpga_voltage / 1000, desc->vendor, + desc->name, (float)chip_voltage / 1000); + return 0; + } }
for (i = 0; i < desc->init_len; i++) {
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/em100/+/34830 )
Change subject: Automatically select the right FPGA firmware for 1.8/3.3V ......................................................................
Patch Set 1: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/em100/+/34830 )
Change subject: Automatically select the right FPGA firmware for 1.8/3.3V ......................................................................
Automatically select the right FPGA firmware for 1.8/3.3V
With the -G2 variant we can automatically select the right voltage. Do this instead of just giving up. For the original em100 this will produce an error, with a 2s delay.
Success (new unit):
will emulate 'W25Q128FW' 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
Stopped EM100Pro Sending flash chip configuration Voltage set to 1.8 Chip set to W25Q128FW Hold pin state set to low Sent 2097152 bytes of 16777216 ...
Failure (old unit):
will emulate 'GD25Q32B' MCU version: 2.27 FPGA version: 0.85 (1.8V) Serial number: DP142073 SPI flash database: 4.3.01 EM100Pro currently running EM100Pro hold pin currently low
Stopped EM100Pro Sending flash chip configuration Invalid voltage response: 0x12 (expected 0x21) Error: The current FPGA firmware (1.8V) does not support GigaDevice GD25Q32B (3.3V) Failed configuring chip type.
Change-Id: I55596b9857c1f340d532acf5d261800a654068c6 Signed-off-by: Simon Glass sjg@chromium.org Reviewed-on: https://chromium-review.googlesource.com/1395949 Reviewed-by: Raul E Rangel rrangel@chromium.org Reviewed-on: https://review.coreboot.org/c/em100/+/34830 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Raul Rangel rrangel@chromium.org --- M em100.c 1 file changed, 12 insertions(+), 8 deletions(-)
Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved
diff --git a/em100.c b/em100.c index 61cc454..1548450 100644 --- a/em100.c +++ b/em100.c @@ -510,7 +510,8 @@ */ int result = 0; int i; - int fpga_voltage, chip_voltage = 0, wrong_voltage = 0; + int fpga_voltage, chip_voltage = 0; + int req_voltage = 0;
printf("Sending flash chip configuration\n");
@@ -528,22 +529,25 @@ case 1601: /* 1.65V-2V */ case 1800: if (fpga_voltage == 3300) - wrong_voltage = 1; + req_voltage = 18; break; case 2500: /* supported by both 1.8V and 3.3V FPGA */ break; case 3300: if (fpga_voltage == 1800) - wrong_voltage = 1; + req_voltage = 33; } break; }
- if (wrong_voltage) { - printf("Error: The current FPGA firmware (%.1fV) does not " - "support %s %s (%.1fV)\n", (float)fpga_voltage/1000, - desc->vendor, desc->name, (float)chip_voltage/1000); - return 0; + if (req_voltage) { + if (!set_fpga_voltage(em100, req_voltage)) { + printf("Error: The current FPGA firmware (%.1fV) does " + "not support %s %s (%.1fV)\n", + (float)fpga_voltage / 1000, desc->vendor, + desc->name, (float)chip_voltage / 1000); + return 0; + } }
for (i = 0; i < desc->init_len; i++) {
-
Martin Roth (Code Review) -
Patrick Georgi (Code Review)
-
Raul Rangel (Code Review)