Patch Set 7: Code-Review-1

Uhm, looks like something isn't right...

Found Winbond flash chip "W25Q64.V" (8192 kB, SPI) mapped at physical address 0x00000000ff800000.
No data in flash context!
Error: write protect is not supported on this flash chip.

https://paste.flashrom.org/view.php?id=3361

I have not seen that before however lets deal with it in the winbond support patch as a follow up as there is a fair amount of work here to get this split up and cleaned up. Thanks for the report though I will try to EM100 locally to repro the issue

View Change

5 comments:

• File writeprotect.h:

  • Patch Set #7, Line 44: wp_wpce775x

    Do we have this programmer upstream?

    It's coming once I have it cleaned up locally, CrOS code quality sucks.

    I removed it out of this commit and made it into a much simpler commit to deal with the writeprotect functionality scope.

• File writeprotect.c:

  • Patch Set #7, Line 873: uint8_t mx25l_read_config_register(const struct flashctx *flash);//XXX

    Hmm?

    poor cros code quality, was required to make it compile upstream. Removed from commit for now.

  • Patch Set #7, Line 1132: //case ATMEL_AT25SF128A:

    Hm?

    Removed for now.

  • Patch Set #7, Line 1473: /* FIXME: this is NOT endian-free copy. */

    How about providing a union instead of a struct? Here's an example: https://github. […]

    I have moved the winbond support into a follow up commit to deal with it separately from the wp infrastructure.

  • Patch Set #7, Line 2236:

    //case SPANSION_ID:
    	//	switch (flash->chip->model_id) {
    	//	case SPANSION_S25FS128S_L:
    	//	case SPANSION_S25FS128S_S: {
    	//		*wp = &s25fs128s_wp;
    	//		(*wp)->descrs = s25fs128s_ranges;
    	//		*num_entries = ARRAY_SIZE(s25fs128s_ranges);
    	//		break;
    	//	}
    	//	case SPANSION_S25FL256S_UL:
    	//	case SPANSION_S25FL256S_US: {
    	//		*wp = &s25fl256s_wp;
    	//		(*wp)->descrs = s25fl256s_ranges;
    	//		*num_entries = ARRAY_SIZE(s25fl256s_ranges);
    	//		break;
    	//	}
    	//	default:
    	//		msg_cerr("%s():%d Spansion flash chip mismatch (0x%04x)"
    	//			", aborting\n", __func__, __LINE__,
    	//			flash->chip->model_id);
    	//		return -1;
    	//	}
    	//	break;
    

    Um?

    Missing chips upstream, removed for now.

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