Attention is currently required from: Felix Singer, Nico Huber, Michał Żygowski, Paul Menzel, Michael Niewöhner.

View Change

4 comments:

• File flashrom.8.tmpl:

  • Patch Set #4, Line 1376: The default is 1

    I don't see the code for this?

• File tuxedo_ec.c:

  • Patch Set #4, Line 661:

    	dmi_init();
    
    	if (!dmi_match("^TUXEDO$")) {
    		msg_perr("Not a TUXEDO device\n");
    		return 1;
    	}
    
    	for (i = 0; i < ARRAY_SIZE(tuxedo_supported_boards); i++) {
    		if (dmi_match(tuxedo_supported_boards[i])) {
    			match = true;
    			break;
    		}
    	}
    
    	if (!match) {
    		msg_perr("TUXEDO EC programmer not yet supported on this device\n");
    		return 1;
    	}
    

    I'd move this into a separate function, and refactor to allow adding support for non-TUXEDO machines easily:

     static const char *const tuxedo_boards[] = {
     	"^InfinityBook S 14 Gen6$",
     	"^InfinityBook S 15 Gen6$",
     };
     
     struct dmi_vendor_entry {
     	const char *vendor;
     	const char **boards;
     	size_t num_boards;
     };
     
     static const struct dmi_vendor_entry ite_ecfw_supported_boards[] = {
     	{"^TUXEDO$", tuxedo_boards, ARRAY_SIZE(tuxedo_boards)},
     };
     
     static bool is_board_supported(void)
     {
     	size_t v, b;
     
     	dmi_init();
     
     	for (v = 0; v < ARRAY_SIZE(ite_ecfw_supported_boards); v++) {
     		const struct dmi_vendor_entry *entry = &ite_ecfw_supported_boards[v];
     		if (!dmi_match(entry->vendor))
     			continue;
     
     		for (b = 0; b < entry->num_boards; b++) {
     			if (dmi_match(entry->boards[b]))
     				return true;
     		}
     
     		msg_perr("Found DMI match for vendor %s, but device is not supported\n",
     			 entry->vendor);
     	}
     
     	msg_perr("ITE ECFW programmer not (yet) supported on this device\n");
     	return false;
     }

  • Patch Set #4, Line 688: sizeof(struct tuxedo_ec_data)

    sizeof(*ctx_data)

  • Patch Set #4, Line 740: uint8_t *const contents, char *buf

    nit: I'd flip the parameter order to match the natural assignment semantics (leftmost parameter is the destination:

     char *buf, uint8_t *const contents

    This order matches what the code does:

     buf[i] = contents[i];

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