Attention is currently required from: Thomas Heijligen, Edward O'Callaghan.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/74065 )
Change subject: util/flashchips_db_jsoniser: Initial version ......................................................................
Patch Set 1: Code-Review+1
(7 comments)
Patchset:
PS1: Interesting idea! It looks nice, although coding style is rather inconsistent.
File util/flashchips_db_jsoniser/main.c:
https://review.coreboot.org/c/flashrom/+/74065/comment/dc0d6e5f_2e879cb6 PS1, Line 65: ret = realloc(ret, strlen(ret) + 1); Is this needed?
https://review.coreboot.org/c/flashrom/+/74065/comment/328a4e54_f21e07f9 PS1, Line 69: static char *decode_test_state(enum test_state ts) : { : switch (ts) { : case OK: return "OK"; : case NT: return "NT"; : case BAD: return "BAD"; : case DEP: return "DEP"; : case NA: return "NA"; : } : return NULL; : }; : : static char *decode_cmdset(int cmdset) : { : switch (cmdset) { : case SPI25: return "SPI25"; : case SPI_EDI: return "SPI_EDI"; : } : return NULL; : } Indentation: please use tabs instead of spaces
https://review.coreboot.org/c/flashrom/+/74065/comment/12c8accb_a615dfe8 PS1, Line 113: return NULL; Indentation
https://review.coreboot.org/c/flashrom/+/74065/comment/e0b00009_80bc6d44 PS1, Line 162: printf("\t\t\t\t"block_erasers": [\n"); : : for (unsigned i = 0; i < NUM_ERASEFUNCTIONS; i++) { : const struct block_eraser *be = &bes[i]; : : if (i) printf(",\n"); : printf("\t\t\t\t\t{\n"); : printf("\t\t\t\t\t\t"%d": [\n", i); : for (unsigned j = 0; j < NUM_ERASEREGIONS; j++) { : struct eraseblock block = be->eraseblocks[j]; : if (j) printf(",\n"); : printf("\t\t\t\t\t\t\t{\n"); : printf("\t\t\t\t\t\t\t\t"size": %d,\n", block.size); : printf("\t\t\t\t\t\t\t\t"count": %d\n", block.size); : printf("\t\t\t\t\t\t\t}"); : } : printf("\t\t\t\t\t\t],\n"); : : enum block_erase_func block_erase = be->block_erase; : const char *block_erase_type = decode_block_eraser_type(block_erase); : printf("\t\t\t\t\t\t"block_erase": "%s"\n", block_erase_type); : : printf("\t\t\t\t\t}\n"); : } : : printf("\t\t\t\t],\n"); Indentation
https://review.coreboot.org/c/flashrom/+/74065/comment/62486a27_ffbb40e2 PS1, Line 194: case WRITE_JEDEC1: return "WRITE_JEDEC_1"; Indentation (this one uses 8 spaces)
https://review.coreboot.org/c/flashrom/+/74065/comment/5d76195a_5c834fea PS1, Line 228: printf("{\n\t\t"flashchips": [\n"); : : for (unsigned int i = 0; i < flashchips_size; i++) { : if (i) printf(",\n"); : printf("\t\t\t{\n"); : : printf("\t\t\t\t"vendor": "%s",\n", flashchips[i].vendor); : printf("\t\t\t\t"name": "%s",\n", flashchips[i].name); : : // bustype : char *bustype = flashbuses_to_text_copy(flashchips[i].bustype); : printf("\t\t\t\t"bustype": "%s",\n", bustype); : free(bustype); : : printf("\t\t\t\t"manufacture_id": %d,\n", flashchips[i].manufacture_id); : printf("\t\t\t\t"model_id": %d,\n", flashchips[i].model_id); : printf("\t\t\t\t"total_size": %d,\n", flashchips[i].total_size); : printf("\t\t\t\t"page_size": %d,\n", flashchips[i].page_size); : printf("\t\t\t\t"feature_bits": %d,\n", flashchips[i].feature_bits); : : // tested : char *pts = decode_test_state(flashchips[i].tested.probe); : char *rts = decode_test_state(flashchips[i].tested.read); : char *ets = decode_test_state(flashchips[i].tested.erase); : char *wts = decode_test_state(flashchips[i].tested.write); : char *wpts = decode_test_state(flashchips[i].tested.wp); : printf("\t\t\t\t"test_state": [\n"); : printf("\t\t\t\t\t{ "probe": "%s"},\n", pts); : printf("\t\t\t\t\t{ "read": "%s"},\n", rts); : printf("\t\t\t\t\t{ "erase": "%s"},\n", ets); : printf("\t\t\t\t\t{ "write": "%s"},\n", wts); : printf("\t\t\t\t\t{ "wp": "%s"}\n", wpts); : printf("\t\t\t\t],\n"); : : // spi_cmd_set : char *cmdset = decode_cmdset(flashchips[i].spi_cmd_set); : printf("\t\t\t\t"spi_cmd_set": "%s",\n", cmdset); : : // probe_func probe; : char *probefn = decode_probefn(flashchips[i].probe); : printf("\t\t\t\t"probe": "%s",\n", probefn); : : printf("\t\t\t\t"probe_timing": %d,\n", flashchips[i].probe_timing); : : // block_erasers[NUM_ERASEFUNCTIONS]; : decode_block_erasers(flashchips[i].block_erasers); : : // enum write_func write; : const char *writefn = decode_writefn(flashchips[i].write); : printf("\t\t\t\t"write": "%s",\n", writefn); : : // enum read_func read; : const char *readfn = decode_readfn(flashchips[i].read); : printf("\t\t\t\t"read": "%s",\n", readfn); : : // struct voltage { : // uint16_t min; : // uint16_t max; : // } voltage; : printf("\t\t\t\t"voltage": [\n"); : printf("\t\t\t\t\t{ "min": %d },\n", flashchips[i].voltage.min); : printf("\t\t\t\t\t{ "max": %d }\n", flashchips[i].voltage.max); : printf("\t\t\t\t]\n"); : : // enum write_granularity gran; : : printf("\n\t\t\t}\n"); : } : : printf("\t]\n}"); : return 0; Indentation