Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41612 )
Change subject: util: Add spd_tools to generate SPDs for TGL boards ......................................................................
Patch Set 8:
(5 comments)
Patch Set 7:
(6 comments)
one more round and done.
We ought to consider putting some test files into here but this is nice stuff.
Thanks for the helpful comments, Ron! I will look into adding test files as follow-up.
https://review.coreboot.org/c/coreboot/+/41612/7/util/spd_tools/intel/lp4x/g... File util/spd_tools/intel/lp4x/gen_part_id.go:
https://review.coreboot.org/c/coreboot/+/41612/7/util/spd_tools/intel/lp4x/g... PS7, Line 47: return fmt.Errorf("Incorrect number of arguments\n")
The go convention is no puncutation (!) or newline characters, so probably best to remove the \n and […]
Done
https://review.coreboot.org/c/coreboot/+/41612/7/util/spd_tools/intel/lp4x/g... PS7, Line 63: func readParts() ([]string, error) {
for testing and other purposes, might be nicer if you could pass the file name into readParts: […]
Done
https://review.coreboot.org/c/coreboot/+/41612/7/util/spd_tools/intel/lp4x/g... PS7, Line 82: func readSpdManifest() (map[string]string, map[string]int, error) {
same here, pass name in as a parameter. […]
Done
https://review.coreboot.org/c/coreboot/+/41612/7/util/spd_tools/intel/lp4x/g... PS7, Line 135: spdToIndexMap map[string]int) ([]partIds, error) {
up to you, but my personal preference is not to split lines this way. […]
Done
https://review.coreboot.org/c/coreboot/+/41612/7/util/spd_tools/intel/lp4x/g... PS7, Line 188: return ioutil.WriteFile(filepath.Join(os.Args[MAKEFILE_DIR_ARG], makefileName), []byte(s), 0644)
very nice, hope you like this too. […]
Done