Attention is currently required from: Nico Huber, Angel Pons. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61275 )
Change subject: hwaccess_x86_io.c: refactor rget_io_perms() ......................................................................
Patch Set 4:
(11 comments)
File hwaccess_x86_io.c:
https://review.coreboot.org/c/flashrom/+/61275/comment/42d7a88c_f3ed1806 PS3, Line 23: static int platform_get_io_perms(void)
No need to mention the whole function signature (you too actually didn't […]
Done
https://review.coreboot.org/c/flashrom/+/61275/comment/44a01fcf_6b70ad15 PS3, Line 23: those
No platforms were named to refer to, AFAICT. So I would rather say […]
Done
https://review.coreboot.org/c/flashrom/+/61275/comment/fb77b276_cb22e715 PS3, Line 34:
please separate multiple platforms with a comma
Done
https://review.coreboot.org/c/flashrom/+/61275/comment/a6e20c7c_1594e3ab PS3, Line 49: return sysi86(SI86V86, V86SC_IOPL, PS_IOPL);
This needs <sys/sysi86.h> (our header file already includes it […]
Done
https://review.coreboot.org/c/flashrom/+/61275/comment/d136d9c7_19b685b6 PS3, Line 65: 1
Other functions, e.g. iopl(), ioperm(), return -1 on error.
Done
https://review.coreboot.org/c/flashrom/+/61275/comment/0df70ecd_89940b07 PS3, Line 65: return (io_fd = open("/dev/io", O_RDWR) >= 0 ? 0 : 1);
Please do the assignment on a separate line :)
Done
https://review.coreboot.org/c/flashrom/+/61275/comment/200ff52f_0c7dd52f PS3, Line 92: return iopl(3);
<sys/io. […]
*BSD and Darwin use other includes. The includes are currently in the header and come here with the next commit.
https://review.coreboot.org/c/flashrom/+/61275/comment/45ffbc57_8d85d3a8 PS3, Line 114: if (ret == 0)
Please add braces on all paths if one path needs them. Or `return 0` […]
Done
https://review.coreboot.org/c/flashrom/+/61275/comment/1f4f1e27_dd207b51 PS3, Line 115: NULL
In theory, this could be a return parameter of platform_get_io_perms(). […]
This can be done in a following ccommit.
https://review.coreboot.org/c/flashrom/+/61275/comment/025a6d4b_3d5dc118 PS3, Line 121: msg_perr("On NetBSD please reboot into single user mode or make sure\n"
Can we make the output changes a separate patch?
Done. See Relation Chain
https://review.coreboot.org/c/flashrom/+/61275/comment/d12f7d70_237c404e PS3, Line 124: return ret;
Better use an explicit `return 0`/`return 1` as before. Currently this […]
Done