Attention is currently required from: Felix Singer, Nico Huber, Paul Menzel, Angel Pons, Michael Niewöhner.
12 comments:
Commit Message:
Patch Set #4, Line 11: updates from IBV.
Can you please document the EC firmware version used, and give an example command?
Done
Patchset:
Some SVID/SDID examples: […]
After a second thought, I think not all laptops with ITE ECs may have a distinguishable SVID/SDID set. In order to make this programmer more generic, leaving the DMI checks and will add DMI bypass option with force flag (however, we need to add a function to get the flash context somehow or pass it to programmer init)
> The IT8987E and IT5570E/IT5576E are a bit special. […]
Got some more information on the ITEString and found out what the autload patching is really doing. It sets some kind of EC Signature flag and changes the EC flash size to be mirrored
File Makefile:
Patch Set #1, Line 839: override CONFIG_TUXEDO_EC = no
Probably many other also don't, like MEC1308 from what I can see in the code... […]
Done
File flashrom.8.tmpl:
Patch Set #4, Line 1376: The default is 1
`tuxedo_ec_init_ctx` is setting these defaults
Ack
File flashrom.c:
Patch Set #1, Line 2109: #if CONFIG_TUXEDO_EC == 1
The coreboot check above is a very bad rolemodel. It does things flashrom shouldn't
do (look at the contents) and in a way (doing programmer-specific things in common
code; a layering violation) that we try to get rid of.I think it would be best to call _verify_file_project() from programmer init. The UI
would then have to feed the file path (or contents) two times. If we really want such
a check, flashrom (or the library) shouldn't offer any operation if the contents look
incompatible.
Flashrom shouldn't even touch the hardware before content compatibility fails IMO. The approach with calling the function from programmer init is simple, but different programmers may need different arguments to be passed. I wonder how to address that. Maybe the programmer_entry structure should expose an optional method like `check_file` which would assess compatibility of the contents with the target hardware/flash.
File tuxedo_ec.c:
Patch Set #1, Line 205: ec_read_reg
Because reading the AC adapter state was always done with the hardcoded default register pair 0x62/0 […]
Ack
Patch Set #1, Line 477: internal_sleep(15000 * 64);
Unfortunately no way that we are aware of.
Ack
File tuxedo_ec.c:
dmi_init();
if (!dmi_match("^TUXEDO$")) {
msg_perr("Not a TUXEDO device\n");
return 1;
}
for (i = 0; i < ARRAY_SIZE(tuxedo_supported_boards); i++) {
if (dmi_match(tuxedo_supported_boards[i])) {
match = true;
break;
}
}
if (!match) {
msg_perr("TUXEDO EC programmer not yet supported on this device\n");
return 1;
}
I was thinking about the same. Thanks for the piece of code. […]
Done
Patch Set #4, Line 688: sizeof(struct tuxedo_ec_data)
sizeof(*ctx_data)
Done
Patch Set #4, Line 740: uint8_t *const contents, char *buf
nit: I'd flip the parameter order to match the natural assignment semantics (leftmost parameter is t […]
Done
if (strcmp(current_ec_project, new_ec_project)) {
msg_perr("Wrong EC project. This file can't be used on this machine\n");
return 1;
}
can we add a parameter for forcing here? One might use flashrom to switch from system76 ec to vendor […]
Should it be the programmer parameter or should I hook the global force flag?
To view, visit change 55715. To unsubscribe, or for help writing mail filters, visit settings.