David Hendricks has posted comments on this change. ( https://review.coreboot.org/25706 )
Change subject: linux_mtd: Import driver from ChromiumOS ......................................................................
Patch Set 9: Verified+1
(5 comments)
Thanks!
https://review.coreboot.org/#/c/25706/8//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/25706/8//COMMIT_MSG@7 PS8, Line 7: linux_mtd: Import driver from ChromiumOS
Paul asked to make this a statement and I generally agree. […]
Sure. It doesn't matter in this case since the commit message is so short, but for future reference the limit for the summary line is 65 chars (https://www.coreboot.org/Git#Commit_messages) so don't expect perfect grammar all the time ;-)
https://review.coreboot.org/#/c/25706/8//COMMIT_MSG@17 PS8, Line 17: it for other Linux interfaces later on.
Maybe also mention the switch to stream functions.
Good call - Done
https://review.coreboot.org/#/c/25706/8/flashrom.8.tmpl File flashrom.8.tmpl:
https://review.coreboot.org/#/c/25706/8/flashrom.8.tmpl@1014 PS8, Line 1014: You may specify the MTD device to use with the
AIUI, this is optional (it would try to open /dev/mtd0 by default)?
Done. I only implied that mtd0 will be the default since ideally we should let whatever auto-probe logic we add choose among whatever MTD devices/chips are in the system.
https://review.coreboot.org/#/c/25706/8/linux_mtd.c File linux_mtd.c:
https://review.coreboot.org/#/c/25706/8/linux_mtd.c@16 PS8, Line 16: : #include <ctype.h> : #include <errno.h> : #include <fcntl.h> : #include <stdio.h> : #include <stdlib.h> : #include <mtd/mtd-user.h> : #include <string.h> : #include <sys/ioctl.h> : #include <sys/stat.h> : #include <unistd.h> :
Not sure, if we (still) need all of these.
I was able to get rid of a couple.
https://review.coreboot.org/#/c/25706/8/linux_mtd.c@343 PS8, Line 343: msg_pinfo("Opened %s successfully\n", dev_path); Added an informational print.
-
David Hendricks (Code Review)