Hi Martin, here is my new TINT commit - https://review.coreboot.org/#/c/17507/ (had to abandon a previous one) I fixed all the problems that were noticed by you, and other reviewers. Also: [*] added check for archive before downloading it [*] added SHA-1 checksum verification of the downloaded archive for TINT payload [*] added USB keyboard support to nvramcui as well [*] made a correct logic for USB enabling: #ifndef CONFIG_LP_USB #define CONFIG_LP_USB 0 <--- earlier it was 1, now it is 0, because - if for some board CONFIG_LP_USB is not defined, USB should not be initialized then #endif
#if IS_ENABLED(CONFIG_LP_USB) usb_initialize(); #endif
[*] excluded nvramcui from kconfig_lint search (just like tint has been excluded) --> for the same purpose
The only problem now is, because our kconfig_lint are different, my commit conflicts with yours here: https://review.coreboot.org/#/c/17443/1 . Very sorry for that, I'm new to gerrit and made many mistakes before doing it right. This latest TINT commit should be correct and final, I really hope
Mike
On Wed, Nov 16, 2016 at 6:52 AM, Martin Roth gaumless@gmail.com wrote:
Hi Mike, I'll take care of it. Thanks for pointing it out.
Martin
On Tue, Nov 15, 2016 at 3:21 PM, Mike Banon mikebdp2@gmail.com wrote:
Finally I found a way to commit this (after a few more changes, also attaching a final version to this letter just in case) - https://review.coreboot.org/#/c/17439/1
Problem with this commit is a false positive error generated by lint.lint-stable-008-kconfig :
#!!!!! Error: IS_ENABLED() used on unknown value CONFIG_LP_USB at payloads/external/tint/libpayload_tint.patch:378. # 1 errors
despite that CONFIG_LP_USB is always defined by: #ifndef CONFIG_LP_USB #define CONFIG_LP_USB 1 #endif and also in libpayload.
Please tell, how to suppress this error, or maybe there is a workaround to make it disappear?
Best regards, Mike Banon
On Mon, Nov 14, 2016 at 11:09 PM, Mike Banon mikebdp2@gmail.com wrote:
Please scroll down to the end of letter to download the attachments ("Non-text attachment was scrubbed..."), don't copy paste (because Google ate my tabs and broke the files)
On Mon, Nov 14, 2016 at 10:50 PM, Mike Banon mikebdp2@gmail.com wrote:
There are (were) five problems with a TINT payload:
- It is outdated: version 0.03b vs the latest 0.04+nmu1 . Two buffer
overflows have been fixed since that, as well as some other improvements 2) When you press 'p', "Paused - Press any key to continue" message is not displayed - because there are no "refresh()" functions between "out_printf" and "while" waiting cycle 3) When you lose or quit, PC does not reboot - could only reboot by the power button, really inconvenient. Meanwhile, coreinfo and nvramcui payloads have a working reboot function "outb(0x6, 0xcf9);" which could be borrowed 4) libpayload_tint.patch is bloated - could reduce it in half for the same effect (with a few changes to Makefile) 5) Standard USB keyboard does not work, only internal PS2-like keyboard works (at least it is this way on my G505S laptop). The same USB keyboard works for me in coreinfo thanks to "#if IS_ENABLED(CONFIG_LP_USB) usb_initialize(); #endif" right in the beginning of "int main(void)" function at coreinfo.c file
I have already fixed all these problems by making the changes to Makefile and libpayload_tint.patch , but I have no idea how to make a pull request to coreboot repository and don't have any commit rights! So if you have the commit rights, please review the changes and commit these updated versions of "coreboot/payloads/external/tint/Makefile" and "coreboot/payloads/external/tint/libpayload_tint.patch" - files attached below. Whoever commits them first could take a credit for my work
Best regards, Mike Banon
-- coreboot mailing list: coreboot@coreboot.org https://www.coreboot.org/mailman/listinfo/coreboot