looks good to me and I'd say that the other small things I found today and didn't comment on yet should be addressed in a follow-up patch, so that this one will finally land in upstream coreboot soon. I'll still need to verify that the resulting amdfw binary doesn't change for the different platforms; when I've done that and the binaries are identical I'll +2 the patch.

Patch set 55:Code-Review +1

View Change

2 comments:

• File util/amdfwtool/data_parse.c:

  • Patch Set #54, Line 265: (void *)

    this cast is probably unnecessary, since both sides are amd_bios_entry type

    Done

  • Patch Set #54, Line 317:

    /* get line */
    		/* blank comment */
    		if (!regexec(&blank_or_comment_expr, oneline, 0, NULL, 0)) {
    			/* skip comment and blank */
    			continue;
    		}
    
    		if (regexec(&entries_line_expr, oneline, 3, match, 0) == 0) {
    			oneline[match[1].rm_eo] = '\0';
    			oneline[match[2].rm_eo] = '\0';
    		} else {
    			/* no match */
    			continue;
    		}
    

    this code is identical to lines 340-353 below, so it's probably a good idea to factor that out into […]

    Done

To view, visit change 42859. To unsubscribe, or for help writing mail filters, visit settings.