Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk.

View Change

10 comments:

File tests/chip_wp.c:
- Patch Set #10, Line 141:
```
	/* Use first 4 KiB for a range. */
	range.len = 0x1000;
	assert_int_equal(0, flashrom_wp_set_range(&flash, &cfg, &range));

	/* Change range to last 4 KiB. */
	range.start = 0x00fff000;
	assert_int_equal(0, flashrom_wp_set_range(&flash, &cfg, &range));
```
  This looks like 2 in 1 tests (1 for first 4KiB range, 2 for last 4KiB). […]
  One range should be enough.
- Patch Set #10, Line 159: set_wp_mode_dummyflasher_test_success
  I think the test is now good, given the comment, but maybe better name is "switch_wp_mode_.... […]
  Done
- Patch Set #10, Line 188:
```
    /*
     * Check that "permanent" mode can't be unset and it's not considered to be
     * an error at the level of library calls (verification should be done
     * higher).
     */
```
  That's a very good comment, thank you! Only need to fix indentation.
  Done
- Patch Set #10, Line 220:
```
cfg.srp_bit_count = 1;
	assert_int_equal(0, flashrom_wp_set_mode(&cfg, WP_MODE_DISABLED));
```
  I see two parts here: 1) with srp_bit_count = 2, 2) with srp_bit_count = 1 […]
  Split these four tests.
- Patch Set #10, Line 290:
```
	assert_int_equal(0x004000, range.start);
	assert_int_equal(0xffc000, range.len);
```
  just to check, do range start/len come from spi_status? the comment above param_dup only mentions re […]
  Yes, range is encoded by those bits. Datasheets contain tables of bit values and ranges that they produce (there is some logic to individual bits, but it's not very straightforward). Expanded that commend and fixed, since it diverged from the code.
- Patch Set #10, Line 310:
```
	assert_int_equal(0, flashrom_layout_include_region(layout, "head"));
	assert_int_equal(0, flashrom_layout_include_region(layout, "tail"));
```
  It is not clear in the context of test method where "head" and "tail" come from. […]
  I've added comments. These calls are here, because the test below this one includes only the "head".
- Patch Set #10, Line 313: Write protection takes effect only after changing SRP values.
  if you can add a bit more for example […]
  Done
- Patch Set #10, Line 329: Try erasing the chip again
  also you can add continue the comment say […]
  Done
- Patch Set #10, Line 364:
```
	printf("Erase chip operation started.\n");
	assert_int_equal(0, do_erase(&flash));
	printf("Erase chip operation done.\n");
```
  Oh now I compare this test scenario and previous, and looks like first 4 KiB are special? Because in […]
  Done
File tests/write_protect.c:
- Patch Set #5, Line 143: wp=no
  Here "wp" means WP pin of the flash chip. […]
  Done

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