Change in coreboot[master]: spi: Share common logic of vendor specific bpbits<>region functions
Hello Daniel Gröber,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/42124
to review the following change.
Change subject: spi: Share common logic of vendor specific bpbits<>region functions ......................................................................
spi: Share common logic of vendor specific bpbits<>region functions
Change-Id: Iadedf160575b20ee7fc671642412e6b3d437a20e Signed-off-by: Daniel Gröber dxld@darkboxed.org --- M src/drivers/spi/Makefile.inc M src/drivers/spi/macronix.c A src/drivers/spi/spi_common.c A src/drivers/spi/spi_common.h M src/drivers/spi/spi_flash_internal.h M src/drivers/spi/winbond.c M tests/drivers/Makefile.inc D tests/drivers/macronix-spi-test.c A tests/drivers/spi-test.c D tests/drivers/winbond-spi-test.c 10 files changed, 608 insertions(+), 635 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/42124/1
diff --git a/src/drivers/spi/Makefile.inc b/src/drivers/spi/Makefile.inc index 6dbc43a..e94d14f 100644 --- a/src/drivers/spi/Makefile.inc +++ b/src/drivers/spi/Makefile.inc @@ -14,7 +14,7 @@ $(1)-y += spi-generic.c $(1)-y += bitbang.c $(1)-$(CONFIG_COMMON_CBFS_SPI_WRAPPER) += cbfs_spi.c -$(1)-$(CONFIG_SPI_FLASH) += spi_flash.c +$(1)-$(CONFIG_SPI_FLASH) += spi_flash.c spi_common.c $(1)-$(CONFIG_SPI_SDCARD) += spi_sdcard.c $(1)-$(CONFIG_BOOT_DEVICE_SPI_FLASH_RW_NOMMAP$(2)) += boot_device_rw_nommap.c $(1)-$(CONFIG_CONSOLE_SPI_FLASH) += flashconsole.c diff --git a/src/drivers/spi/macronix.c b/src/drivers/spi/macronix.c index e5a7b98..b19f085 100644 --- a/src/drivers/spi/macronix.c +++ b/src/drivers/spi/macronix.c @@ -7,6 +7,7 @@ #include <lib.h>
#include "spi_flash_internal.h" +#include "spi_common.h" #include "spi_macronix.h"
/* MX25xx-specific commands */ @@ -47,6 +48,7 @@ .id[0] = 0x2015, .nr_sectors_shift = 9, .protection_granularity_shift = 16, + .bp_bits = 4, .bp_lower_th = 10, }, { @@ -54,6 +56,7 @@ .id[0] = 0x2016, .nr_sectors_shift = 10, .protection_granularity_shift = 16, + .bp_bits = 4, .bp_lower_th = 9, }, { @@ -61,6 +64,7 @@ .id[0] = 0x2017, .nr_sectors_shift = 11, .protection_granularity_shift = 17, + .bp_bits = 4, .bp_lower_th = 9, }, { @@ -167,36 +171,7 @@ const struct spi_flash_bpbits *bits, struct region *out) { - if (bits->bp == 0xf) { - out->offset = 0; - out->size = flash->size; - } else if (bits->bp == 0) { - out->offset = 0; - out->size = 0; - } else { - const u16 granularity_shift = - part->protection_granularity_shift; - - if (bits->bp >= part->bp_lower_th) { - /* Lower half is protected */ - const size_t unprotected_size = - MIN(1<<granularity_shift<<(14 - bits->bp), - flash->size); - - out->offset = 0; - out->size = flash->size - unprotected_size; - } else { - /* Upper half is protected */ - const size_t protected_size = - MIN(1<<(granularity_shift + bits->bp - 1), - flash->size); - - out->offset = flash->size - protected_size; - out->size = protected_size; - } - } - - return 0; + return spi_flash_common_bpbits_to_region(flash, part, bits, out); }
/** @@ -212,41 +187,11 @@ const enum spi_flash_status_reg_lockdown mode, struct spi_flash_bpbits *bits) { - if (region->offset == 0 && region->size >= flash->size) { - bits->bp = 0xf; - } else if (region->offset == 0 && region->size == 0) { - bits->bp = 0; - } else if (region->offset >= flash->size) { - return -1; - } else if (region->size >= flash->size) { - return -1; - } else { - const size_t gran_shift = params->protection_granularity_shift; - const u16 granularity = 1<<gran_shift; + int ret;
- if (region->offset == 0) { /* Lower half should be protected */ - const size_t unprotected_size = - flash->size - region->size; - - const int is_valid = - IS_POWER_OF_2(unprotected_size) && - unprotected_size >= granularity; - if (!is_valid) - return -1; - - bits->bp = 14 - log2(unprotected_size>>gran_shift); - } else { - const size_t protected_size = region->size; - - const int is_valid = - IS_POWER_OF_2(protected_size) && - protected_size >= granularity; - if (!is_valid) - return -1; - - bits->bp = log2(protected_size>>gran_shift) + 1; - } - } + ret = spi_flash_common_region_to_bpbits(flash, params, region, bits); + if (ret) + return ret;
switch (mode) { case SPI_WRITE_PROTECTION_PRESERVE: diff --git a/src/drivers/spi/spi_common.c b/src/drivers/spi/spi_common.c new file mode 100644 index 0000000..88cc23a --- /dev/null +++ b/src/drivers/spi/spi_common.c @@ -0,0 +1,133 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#include <assert.h> +#include <spi_flash.h> +#include <lib.h> + +#include "spi_flash_internal.h" +#include "spi_common.h" + +int spi_flash_common_region_to_bpbits( + const struct spi_flash *flash, + const struct spi_flash_part_id *part, + const struct region *region, + struct spi_flash_bpbits *bits) +{ + /* Need to touch TOP or BOTTOM */ + if (region_offset(region) != 0 && region_end(region) != flash->size) + return -1; + + if (region_offset(region) == 0 && part->has_tb) + bits->tb = 1; + else + bits->tb = 0; + + struct region wp_region = *region; + if (region_sz(region) > flash->size / 2 && part->has_cmp) { + assert(part->has_tb); + /* When we want to protect more than half the flash, but + * not the whole thing we can use the cmp bit to make the + * normally protected region specified by bp below to be + * what is unprotected instead */ + bits->cmp = 1; + wp_region.offset = bits->tb ? 0 : region_sz(region); + wp_region.size = flash->size - region_sz(region); + bits->tb = !bits->tb; + } else { + bits->cmp = 0; + } + + if (region_sz(&wp_region) == 0) { + bits->bp = 0; + return 0; + } else if (region_offset(&wp_region) == 0 && + region_sz(&wp_region) == flash->size) { + assert(part->bp_bits > 0); + bits->bp = (1<<part->bp_bits) - 1; + return 0; + } + + const size_t gran_shift = part->protection_granularity_shift; + const size_t granularity = 1 << gran_shift; + + if (region_sz(&wp_region) < granularity) + return -1; + + if (part->bp_lower_th && region_offset(region) == 0) { + const size_t complement_size = flash->size - region->size; + + const int is_valid = IS_POWER_OF_2(complement_size) && + complement_size >= granularity; + if (!is_valid) + return -1; + + bits->bp = 14 - log2(complement_size>>gran_shift); + } else { + const size_t size = region_sz(&wp_region); + + const int is_valid = IS_POWER_OF_2(size) && + size >= granularity; + if (!is_valid) + return -1; + + bits->bp = log2(size) - gran_shift + 1; + } + + return 0; +} + +int spi_flash_common_bpbits_to_region( + const struct spi_flash *flash, + const struct spi_flash_part_id *part, + const struct spi_flash_bpbits *bits, + struct region *out) +{ + int tb = 0; + if (part->has_tb) { + assert(!part->bp_lower_th); + tb = bits->tb; + } else if (part->bp_lower_th && bits->bp >= part->bp_lower_th) { + tb = 1; + } + + const size_t granularity = + 1 << part->protection_granularity_shift; + + size_t protected_size; + if (part->bp_lower_th && bits->bp >= part->bp_lower_th) { + size_t unprotected_size = + MIN(granularity<<(14 - bits->bp), flash->size); + protected_size = flash->size - unprotected_size; + } else{ + protected_size = + MIN(bits->bp ? granularity << (bits->bp - 1) : 0, + flash->size); + } + + if (part->has_cmp && bits->cmp) { + protected_size = flash->size - protected_size; + tb = !tb; + } + + if (protected_size == 0) { + *out = (struct region){ + .offset = 0, + .size = 0, + }; + } else if (protected_size == flash->size) { + *out = (struct region){ + .offset = 0, + .size = flash->size, + }; + } else { + if (tb) { /* tb=1: BOTTOM */ + out->offset = 0; + } else { /* tb=0: TOP */ + out->offset = flash->size - protected_size; + } + + out->size = protected_size; + } + + return 0; +} diff --git a/src/drivers/spi/spi_common.h b/src/drivers/spi/spi_common.h new file mode 100644 index 0000000..a6aab46 --- /dev/null +++ b/src/drivers/spi/spi_common.h @@ -0,0 +1,13 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +int spi_flash_common_region_to_bpbits( + const struct spi_flash *flash, + const struct spi_flash_part_id *part, + const struct region *region, + struct spi_flash_bpbits *bits); + +int spi_flash_common_bpbits_to_region( + const struct spi_flash *flash, + const struct spi_flash_part_id *part, + const struct spi_flash_bpbits *bits, + struct region *out); diff --git a/src/drivers/spi/spi_flash_internal.h b/src/drivers/spi/spi_flash_internal.h index 98837c2..8c8cff9 100644 --- a/src/drivers/spi/spi_flash_internal.h +++ b/src/drivers/spi/spi_flash_internal.h @@ -80,9 +80,12 @@ uint16_t _reserved_for_flags: 3; /* Block protection. */ uint16_t protection_granularity_shift : 5; - uint16_t bp_bits : 3; /* Used by Winbond */ + uint16_t bp_bits : 3; /* bp values starting here protect the lower half */ uint16_t bp_lower_th : 5; /* Used by Macronix */ + uint16_t has_cmp : 1; /* region complement available, currently the + * code assumes has_cmp implies has_tb */ + uint16_t has_tb : 1; /* top/bottom select available */ };
struct spi_flash_ops_descriptor { diff --git a/src/drivers/spi/winbond.c b/src/drivers/spi/winbond.c index 9605279..8b5b29e 100644 --- a/src/drivers/spi/winbond.c +++ b/src/drivers/spi/winbond.c @@ -8,6 +8,7 @@ #include <lib.h>
#include "spi_flash_internal.h" +#include "spi_common.h" #include "spi_winbond.h"
union status_reg1 { @@ -101,6 +102,7 @@ .id[0] = 0x4014, .nr_sectors_shift = 8, .fast_read_dual_output_support = 1, + .has_tb = 1, }, { /* W25Q16_V */ @@ -109,6 +111,7 @@ .fast_read_dual_output_support = 1, .protection_granularity_shift = 16, .bp_bits = 3, + .has_tb = 1, }, { /* W25Q16DW */ @@ -117,6 +120,8 @@ .fast_read_dual_output_support = 1, .protection_granularity_shift = 16, .bp_bits = 3, + .has_cmp = 1, + .has_tb = 1, }, { /* W25Q32_V */ @@ -125,6 +130,7 @@ .fast_read_dual_output_support = 1, .protection_granularity_shift = 16, .bp_bits = 3, + .has_tb = 1, }, { /* W25Q32DW */ @@ -133,6 +139,8 @@ .fast_read_dual_output_support = 1, .protection_granularity_shift = 16, .bp_bits = 3, + .has_cmp = 1, + .has_tb = 1, }, { /* W25Q64_V */ @@ -141,6 +149,7 @@ .fast_read_dual_output_support = 1, .protection_granularity_shift = 17, .bp_bits = 3, + .has_tb = 1, }, { /* W25Q64DW */ @@ -149,6 +158,8 @@ .fast_read_dual_output_support = 1, .protection_granularity_shift = 17, .bp_bits = 3, + .has_cmp = 1, + .has_tb = 1, }, { /* W25Q64JW */ @@ -157,6 +168,8 @@ .fast_read_dual_output_support = 1, .protection_granularity_shift = 17, .bp_bits = 3, + .has_cmp = 1, + .has_tb = 1, }, { /* W25Q128_V */ @@ -165,6 +178,8 @@ .fast_read_dual_output_support = 1, .protection_granularity_shift = 18, .bp_bits = 3, + .has_cmp = 1, + .has_tb = 1, }, { /* W25Q128FW */ @@ -173,6 +188,8 @@ .fast_read_dual_output_support = 1, .protection_granularity_shift = 18, .bp_bits = 3, + .has_cmp = 1, + .has_tb = 1, }, { /* W25Q128J */ @@ -181,6 +198,8 @@ .fast_read_dual_output_support = 1, .protection_granularity_shift = 18, .bp_bits = 3, + .has_cmp = 1, + .has_tb = 1, }, { /* W25Q128JW */ @@ -189,6 +208,8 @@ .fast_read_dual_output_support = 1, .protection_granularity_shift = 18, .bp_bits = 3, + .has_cmp = 1, + .has_tb = 1, }, { /* W25Q256_V */ @@ -197,6 +218,8 @@ .fast_read_dual_output_support = 1, .protection_granularity_shift = 16, .bp_bits = 4, + .has_cmp = 1, + .has_tb = 1, }, { /* W25Q256J */ @@ -205,6 +228,8 @@ .fast_read_dual_output_support = 1, .protection_granularity_shift = 16, .bp_bits = 4, + .has_cmp = 1, + .has_tb = 1, }, };
@@ -224,28 +249,7 @@ const struct spi_flash_bpbits *bits, struct region *out) { - const size_t granularity = 1 << params->protection_granularity_shift; - size_t protected_size = - MIN(bits->bp ? granularity << (bits->bp - 1) : 0, flash->size); - - int tb = bits->tb; - if (bits->cmp) { - protected_size = flash->size - protected_size; - tb = !tb; - } - - if (protected_size == 0) { - out->offset = 0; - out->size = 0; - } else if (protected_size == flash->size) { - out->offset = 0; - out->size = flash->size; - } else { - out->offset = tb ? 0 : flash->size - protected_size; - out->size = protected_size; - } - - return 0; + return spi_flash_common_bpbits_to_region(flash, params, bits, out); }
static int winbond_get_bpbits(const struct spi_flash *flash, @@ -446,43 +450,17 @@ const enum spi_flash_status_reg_lockdown mode, struct spi_flash_bpbits *bits) { - struct region wp_region; - - /* Need to touch TOP or BOTTOM */ - if (region_offset(region) != 0 && region_end(region) != flash->size) - return -1; + int ret;
if (params->bp_bits != 3 && params->bp_bits != 4) { /* FIXME: not implemented */ return -1; }
- wp_region = *region; - - if (region_offset(&wp_region) == 0) - bits->tb = 1; - else - bits->tb = 0; - - if (region_sz(&wp_region) > flash->size / 2) { - bits->cmp = 1; - wp_region.offset = bits->tb ? 0 : region_sz(&wp_region); - wp_region.size = flash->size - region_sz(&wp_region); - bits->tb = !bits->tb; - } else { - bits->cmp = 0; - } - - if (region_sz(&wp_region) == 0) { - bits->bp = 0; - } else if (IS_POWER_OF_2(region_sz(&wp_region)) && - (region_sz(&wp_region) >= - (1 << params->protection_granularity_shift))) { - bits->bp = log2(region_sz(&wp_region)) - - params->protection_granularity_shift + 1; - } else { + ret = spi_flash_common_region_to_bpbits(flash, params, region, bits); + if (ret) { printk(BIOS_ERR, "WINBOND: ERROR: unsupported region size\n"); - return -1; + return ret; }
switch (mode) { diff --git a/tests/drivers/Makefile.inc b/tests/drivers/Makefile.inc index d29502f..142dbaa 100644 --- a/tests/drivers/Makefile.inc +++ b/tests/drivers/Makefile.inc @@ -1,17 +1,11 @@ # SPDX-License-Identifier: GPL-2.0-only
-tests-y += winbond-spi-test +tests-y += spi-test
-winbond-spi-test-srcs += tests/drivers/winbond-spi-test.c -winbond-spi-test-srcs += src/drivers/spi/winbond.c -winbond-spi-test-srcs += src/drivers/spi/spi_flash.c -winbond-spi-test-srcs += src/commonlib/region.c -winbond-spi-test-srcs += src/commonlib/mem_pool.c - -tests-y += macronix-spi-test - -macronix-spi-test-srcs += tests/drivers/macronix-spi-test.c -macronix-spi-test-srcs += src/drivers/spi/macronix.c -macronix-spi-test-srcs += src/drivers/spi/spi_flash.c -macronix-spi-test-srcs += src/commonlib/region.c -macronix-spi-test-srcs += src/commonlib/mem_pool.c +spi-test-srcs += tests/drivers/spi-test.c +spi-test-srcs += src/drivers/spi/winbond.c +spi-test-srcs += src/drivers/spi/macronix.c +spi-test-srcs += src/drivers/spi/spi_flash.c +spi-test-srcs += src/drivers/spi/spi_common.c +spi-test-srcs += src/commonlib/region.c +spi-test-srcs += src/commonlib/mem_pool.c diff --git a/tests/drivers/macronix-spi-test.c b/tests/drivers/macronix-spi-test.c deleted file mode 100644 index bacb9d5..0000000 --- a/tests/drivers/macronix-spi-test.c +++ /dev/null @@ -1,202 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-or-later */ - -#include <stdarg.h> -#include <stddef.h> -#include <setjmp.h> -#include <cmocka.h> - -#include <string.h> - -#include <stdio.h> /* debug */ - -#include <commonlib/helpers.h> -#include <spi_flash.h> -#include <spi-generic.h> -#include "../drivers/spi/spi_flash_internal.h" -#include "../drivers/spi/spi_macronix.h" - -struct bp_table { - uint16_t id; - size_t size; - struct block_region { - /* these are in units of 64k-byte blocks */ - size_t offset; - size_t size; - } wp_blocks[1<<4]; -} bp_table[] = { - { - /* MX25L1605D */ - .id = 0x2015, - .size = 0x200000, - .wp_blocks = { - /* off size */ - [0] = { 0, 0 }, - [1] = { 31, 1 }, - [2] = { 30, 2 }, - [3] = { 28, 4 }, - [4] = { 24, 8 }, - [5] = { 16, 16 }, - [6] = { 0, 32 }, - [7] = { 0, 32 }, - [8] = { 0, 32 }, - [9] = { 0, 32 }, - - [10] = { 0, 16 }, - [11] = { 0, 24 }, - [12] = { 0, 28 }, - [13] = { 0, 30 }, - [14] = { 0, 31 }, - [15] = { 0, 32 }, - }, - }, - { - /* MX25L3205D */ - .id = 0x2016, - .size = 0x400000, - .wp_blocks = { - /* off size */ - [0] = { 0, 0 }, - [1] = { 63, 1 }, - [2] = { 62, 2 }, - [3] = { 60, 4 }, - [4] = { 56, 8 }, - [5] = { 48, 16 }, - [6] = { 32, 32 }, - [7] = { 0, 64 }, - [8] = { 0, 64 }, - - [9] = { 0, 32 }, - [10] = { 0, 48 }, - [11] = { 0, 56 }, - [12] = { 0, 60 }, - [13] = { 0, 62 }, - [14] = { 0, 63 }, - [15] = { 0, 64 }, - }, - }, - { - /* MX25L6405D */ - .id = 0x2017, - .size = 0x800000, - .wp_blocks = { - /* off size */ - [0] = { 0, 0 }, - [1] = { 126, 2 }, - [2] = { 124, 4 }, - [3] = { 120, 8 }, - [4] = { 112, 16 }, - [5] = { 96, 32 }, - [6] = { 64, 64 }, - [7] = { 0, 128 }, - [8] = { 0, 128 }, - - [9] = { 0, 64 }, - [10] = { 0, 96 }, - [11] = { 0, 112 }, - [12] = { 0, 120 }, - [13] = { 0, 124 }, - [14] = { 0, 126 }, - [15] = { 0, 128 }, - }, - }, -}; - - -static void macronix_region_bpbits_roundtrip_test(void **state) -{ - int n_cfgs_tested = 0; - - const struct spi_flash_vendor_info *vi = &spi_flash_macronix_vi; - - for (size_t i = 0; i < vi->nr_part_ids; i++) { - const struct spi_flash_part_id *part = &vi->ids[i]; - - if (!part->protection_granularity_shift) - continue; - - struct spi_slave spi; - struct spi_flash flash; - int ret = fill_spi_flash(&spi, &flash, vi, part); - assert_int_equal(ret, 0); - - const struct bp_table *table = NULL; - for (uint16_t j = 0; j < ARRAY_SIZE(bp_table); j++) { - if (bp_table[j].id == part->id[0]) { - table = &bp_table[j]; - break; - } - } - - assert_non_null(table); - - for (u8 bp = 0; bp <= 0xf; bp++) { - struct region expected, out = {-1, -1}; - struct region roundtripped = {-1, -1}; - - const struct block_region *wp_blocks = - &table->wp_blocks[bp]; - const size_t block_size = 1<<16; - - expected.offset = wp_blocks->offset * block_size; - expected.size = wp_blocks->size * block_size; - - struct spi_flash_bpbits bpbits = { - .bp = bp, - }; - - macronix_bpbits_to_region(&flash, part, &bpbits, &out); - - enum spi_flash_status_reg_lockdown mode = - SPI_WRITE_PROTECTION_PIN; - struct spi_flash_bpbits bpbits1 = {}; - int ret = macronix_region_to_bpbits(&flash, part, &out, - mode, &bpbits1); - assert_int_equal(ret, 0); - - /* we have to go through bpbits_to_region again as - * the bp<>range mapping isn't unique. For example - * bp=0b1111 always means everything is protected - * but bp values just after the largest upper-half - * protection also mean "all blocks protected" */ - macronix_bpbits_to_region(&flash, part, &bpbits1, - &roundtripped); - -#if 0 - /* debugging */ - print_message("\nfor part 0x%04x bp=%d:\n", - part->id[0], (int)bp); - print_message("should: 0x%08zx-0x%08zx\n", - region_offset(&expected), - region_end(&expected)); - print_message("is: 0x%08zx-0x%08zx\n", - region_offset(&out), - region_end(&out)); - print_message("rt'ed: 0x%08zx-0x%08zx\n", - region_offset(&roundtripped), - region_end(&roundtripped)); - print_message("bp1: %d\n", bp1); -#endif - - assert_true(expected.size == out.size); - assert_true(expected.offset == out.offset); - - assert_true(expected.size == roundtripped.size); - assert_true(expected.offset == roundtripped.offset); - - n_cfgs_tested++; - } - } - - assert_true(n_cfgs_tested == 48); -} - -#include "spi-mocks.c" - -int main(void) -{ - const struct CMUnitTest tests[] = { - cmocka_unit_test(macronix_region_bpbits_roundtrip_test), - }; - - return cmocka_run_group_tests(tests, NULL, NULL); -} diff --git a/tests/drivers/spi-test.c b/tests/drivers/spi-test.c new file mode 100644 index 0000000..97b0af1 --- /dev/null +++ b/tests/drivers/spi-test.c @@ -0,0 +1,410 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#include <stdarg.h> +#include <stddef.h> +#include <setjmp.h> +#include <cmocka.h> + +#include <string.h> + +#include <stdio.h> /* debug */ + +#include <commonlib/helpers.h> +#include <spi_flash.h> +#include <spi-generic.h> +#include "../drivers/spi/spi_flash_internal.h" +#include "../drivers/spi/spi_winbond.h" + +int fill_spi_flash(const struct spi_slave *spi, struct spi_flash *flash, + const struct spi_flash_vendor_info *vi, + const struct spi_flash_part_id *part); + +struct block_region { + /* these are in units of 64k-byte blocks */ + size_t offset; + size_t size; +}; + +struct bp_table { + const struct spi_flash_vendor_info *vi; + uint16_t id; + size_t size; + struct block_region wp_blocks[2][2][1<<4]; +} bp_table[] = { + { + /* W25Q16_V */ + .vi = &spi_flash_winbond_vi, + .id = 0x4015, + .wp_blocks = { + /* [cmp][tb][bp] */ + /* off size */ + [0][0][0] = { 0, 0 }, + + /* tb=x, bp=0b000: NONE */ + [0][0][0] = { 0, 0 }, + [0][1][0] = { 0, 0 }, + + /* tb=x, bp=0b11x: ALL */ + [0][0][6] = { 0, 32 }, + [0][1][6] = { 0, 32 }, + [0][0][7] = { 0, 32 }, + [0][1][7] = { 0, 32 }, + + [0][0][1] = { 31, 1 }, + [0][0][2] = { 30, 2 }, + [0][0][3] = { 28, 4 }, + [0][0][4] = { 24, 8 }, + [0][0][5] = { 16, 16 }, + + [0][1][1] = { 0, 1 }, + [0][1][2] = { 0, 2 }, + [0][1][3] = { 0, 4 }, + [0][1][4] = { 0, 8 }, + [0][1][5] = { 0, 16 }, + } + }, + { + /* W25Q256J */ + .vi = &spi_flash_winbond_vi, + .id = 0x7019, + .wp_blocks = { + /* [cmp][tb][bp] */ + /* off size */ + + /* cmp=0, tb=x, bp=0b0000: NONE */ + [0][0][0] = { 0, 0 }, + [0][1][0] = { 0, 0 }, + + /* cmp=1, tb=x, bp=0b0000: ALL */ + [1][0][0] = { 0, 512 }, + [1][1][0] = { 0, 512 }, + + /* cmp=0, tb=x, bp=0b110x|0b1x1x: ALL */ + [0][0 ... 1][10 ... 15] = { 0, 512 }, + + /* cmp=1, tb=x, bp=0b110x|0b1x1x: NONE */ + [1][0 ... 1][10 ... 15] = { 0, 0 }, + + [0][0][1] = { 511, 1 }, + [0][0][2] = { 510, 2 }, + [0][0][3] = { 508, 4 }, + [0][0][4] = { 504, 8 }, + [0][0][5] = { 496, 16 }, + [0][0][6] = { 480, 32 }, + [0][0][7] = { 448, 64 }, + [0][0][8] = { 384, 128 }, + [0][0][9] = { 256, 256 }, + + [0][1][1] = { 0, 1 }, + [0][1][2] = { 0, 2 }, + [0][1][3] = { 0, 4 }, + [0][1][4] = { 0, 8 }, + [0][1][5] = { 0, 16 }, + [0][1][6] = { 0, 32 }, + [0][1][7] = { 0, 64 }, + [0][1][8] = { 0, 128 }, + [0][1][9] = { 0, 256 }, + + [1][0][1] = { 0, 512-1 }, + [1][0][2] = { 0, 512-2 }, + [1][0][3] = { 0, 512-4 }, + [1][0][4] = { 0, 512-8 }, + [1][0][5] = { 0, 512-16 }, + [1][0][6] = { 0, 512-32 }, + [1][0][7] = { 0, 512-64 }, + [1][0][8] = { 0, 512-128 }, + [1][0][9] = { 0, 512-256 }, + + [1][1][1] = { 1, 512-1 }, + [1][1][2] = { 2, 512-2 }, + [1][1][3] = { 4, 512-4 }, + [1][1][4] = { 8, 512-8 }, + [1][1][5] = { 16, 512-16 }, + [1][1][6] = { 32, 512-32 }, + [1][1][7] = { 64, 512-64 }, + [1][1][8] = { 128, 512-128 }, + [1][1][9] = { 256, 512-256 }, + } + }, + + { + /* W25Q128FW */ + .vi = &spi_flash_winbond_vi, + .id = 0x6018, + .wp_blocks = { + /* [cmp][tb][bp] */ + /* off size */ + [0][0][0] = { 0, 0 }, + + /* cmp=0, tb=x, bp=0b000: NONE */ + [0][0][0] = { 0, 0 }, + [0][1][0] = { 0, 0 }, + + /* cmp=0, tb=x, bp=0b111: ALL */ + [0][0][7] = { 0, 256 }, + [0][1][7] = { 0, 256 }, + + [0][0][1] = { 252, 4 }, + [0][0][2] = { 248, 8 }, + [0][0][3] = { 240, 16 }, + [0][0][4] = { 224, 32 }, + [0][0][5] = { 192, 64 }, + [0][0][6] = { 128, 128 }, + + [0][1][1] = { 0, 4 }, + [0][1][2] = { 0, 8 }, + [0][1][3] = { 0, 16 }, + [0][1][4] = { 0, 32 }, + [0][1][5] = { 0, 64 }, + [0][1][6] = { 0, 128 }, + + /* cmp=1, tb=x, bp=0b000: ALL */ + [1][0][0] = { 0, 256 }, + [1][1][0] = { 0, 256 }, + + /* cmp=1, tb=x, bp=0b111: NONE */ + [1][0][7] = { 0, 0 }, + [1][1][7] = { 0, 0 }, + + [1][0][1] = { 0, 256-4 }, + [1][0][2] = { 0, 256-8 }, + [1][0][3] = { 0, 256-16 }, + [1][0][4] = { 0, 256-32 }, + [1][0][5] = { 0, 256-64 }, + [1][0][6] = { 0, 256-128 }, + + [1][1][1] = { 4, 256-4 }, + [1][1][2] = { 8, 256-8 }, + [1][1][3] = { 16, 256-16 }, + [1][1][4] = { 32, 256-32 }, + [1][1][5] = { 64, 256-64 }, + [1][1][6] = { 128, 256-128 }, + } + }, + { + /* MX25L1605D */ + .vi = &spi_flash_macronix_vi, + .id = 0x2015, + .wp_blocks = { + /* off size */ + [0][0][0] = { 0, 0 }, + [0][0][1] = { 31, 1 }, + [0][0][2] = { 30, 2 }, + [0][0][3] = { 28, 4 }, + [0][0][4] = { 24, 8 }, + [0][0][5] = { 16, 16 }, + [0][0][6] = { 0, 32 }, + [0][0][7] = { 0, 32 }, + [0][0][8] = { 0, 32 }, + [0][0][9] = { 0, 32 }, + + [0][0][10] = { 0, 16 }, + [0][0][11] = { 0, 24 }, + [0][0][12] = { 0, 28 }, + [0][0][13] = { 0, 30 }, + [0][0][14] = { 0, 31 }, + [0][0][15] = { 0, 32 }, + }, + }, + { + /* MX25L3205D */ + .vi = &spi_flash_macronix_vi, + .id = 0x2016, + .wp_blocks = { + /* off size */ + [0][0][0] = { 0, 0 }, + [0][0][1] = { 63, 1 }, + [0][0][2] = { 62, 2 }, + [0][0][3] = { 60, 4 }, + [0][0][4] = { 56, 8 }, + [0][0][5] = { 48, 16 }, + [0][0][6] = { 32, 32 }, + [0][0][7] = { 0, 64 }, + [0][0][8] = { 0, 64 }, + + [0][0][9] = { 0, 32 }, + [0][0][10] = { 0, 48 }, + [0][0][11] = { 0, 56 }, + [0][0][12] = { 0, 60 }, + [0][0][13] = { 0, 62 }, + [0][0][14] = { 0, 63 }, + [0][0][15] = { 0, 64 }, + }, + }, + { + /* MX25L6405D */ + .vi = &spi_flash_macronix_vi, + .id = 0x2017, + .wp_blocks = { + /* off size */ + [0][0][0] = { 0, 0 }, + [0][0][1] = { 126, 2 }, + [0][0][2] = { 124, 4 }, + [0][0][3] = { 120, 8 }, + [0][0][4] = { 112, 16 }, + [0][0][5] = { 96, 32 }, + [0][0][6] = { 64, 64 }, + [0][0][7] = { 0, 128 }, + [0][0][8] = { 0, 128 }, + + [0][0][9] = { 0, 64 }, + [0][0][10] = { 0, 96 }, + [0][0][11] = { 0, 112 }, + [0][0][12] = { 0, 120 }, + [0][0][13] = { 0, 124 }, + [0][0][14] = { 0, 126 }, + [0][0][15] = { 0, 128 }, + }, + }, + +}; + +static void test_one_config( + void **state, + const struct spi_flash *flash, + const struct spi_flash_part_id *part, + const struct bp_table *table, + const struct spi_flash_bpbits *bpbits) +{ + const size_t granularity = + 1<<part->protection_granularity_shift; + + struct region expected, out = {-1, -1}; + struct region roundtripped = {-1, -1}; + + flash->prot_ops->bpbits_to_region(flash, part, bpbits, &out); + + enum spi_flash_status_reg_lockdown mode = + SPI_WRITE_PROTECTION_PIN; + struct spi_flash_bpbits bpbits1; + flash->prot_ops->region_to_bpbits(flash, part, &out, mode, &bpbits1); + + flash->prot_ops->bpbits_to_region(flash, part, &bpbits1, &roundtripped); + + const struct block_region *wp_blocks = + &table->wp_blocks[bpbits->cmp][bpbits->tb][bpbits->bp]; + const size_t block_size = 1<<16; + + expected.offset = wp_blocks->offset * block_size; + expected.size = wp_blocks->size * block_size; + +#if 0 + /* debugging */ + print_message("\nfor part 0x%04x:\n", part->id[0]); + print_message("in: cmp=%d tb=%d bp=%d\n", + (int)bpbits->cmp, (int)bpbits->tb, (int)bpbits->bp); + print_message("should: 0x%08zx-0x%08zx\n", + region_offset(&expected), + region_end(&expected)); + print_message("is: 0x%08zx-0x%08zx\n", + region_offset(&out), + region_end(&out)); + print_message("rt: cmp=%d, tb=%d, bp=%d\n", + bpbits1.cmp, bpbits1.tb, bpbits1.bp); + print_message("rt'ed: 0x%08zx-0x%08zx\n", + region_offset(&roundtripped), + region_end(&roundtripped)); +#endif + + assert_true(expected.size == out.size); + assert_true(expected.offset == out.offset); + + assert_true(expected.size == roundtripped.size); + assert_true(expected.offset == roundtripped.offset); +} + +static void spi_flash_region_bpbits_roundtrip_test(void **state) +{ + int n_cfgs_tested = 0; + + for (size_t i = 0; i < ARRAY_SIZE(bp_table); i++) { + const struct bp_table *table = &bp_table[i]; + const struct spi_flash_vendor_info *vi = table->vi; + const struct spi_flash_part_id *part = NULL; + + for (size_t j = 0; j < vi->nr_part_ids; j++) { + if (vi->ids[j].id[0] == table->id) { + part = &vi->ids[j]; + break; + } + } + + assert_true(part->protection_granularity_shift); + + struct spi_slave spi; + struct spi_flash flash; + int ret = fill_spi_flash(&spi, &flash, vi, part); + assert_int_equal(ret, 0); + + u8 tb_max = part->has_tb + 1; + u8 cmp_max = part->has_cmp + 1; + for (u8 tb = 0; tb < tb_max; tb++) { + for (u8 cmp = 0; cmp < cmp_max; cmp++) { + u8 bp_max + = part->bp_bits + ? 1<<part->bp_bits + : 0x10; + for (u8 bp = 0; bp < bp_max; bp++) { + struct spi_flash_bpbits bpbits = { + .cmp = cmp, + .tb = tb, + .bp = bp, + }; + + test_one_config( + state, + &flash, + part, + table, + &bpbits); + + n_cfgs_tested++; + } + } + } + } + + size_t n_cfgs_tested_expected = 160; + if (n_cfgs_tested != n_cfgs_tested_expected) { + fail_msg("n_cfgs_tested is=%d, expected=%d!\n", + n_cfgs_tested, n_cfgs_tested_expected); + } +} + + /* Mocks */ +int do_printk(int msg_level, const char *fmt, ...) +{ + return -1; +} +int spi_claim_bus(const struct spi_slave *slave) +{ + return -1; +} +int spi_xfer_vector(const struct spi_slave *slave, + struct spi_op vectors[], size_t count) +{ + return -1; +} +void spi_release_bus(const struct spi_slave *slave) +{ +} +unsigned int spi_crop_chunk(const struct spi_slave *slave, unsigned int cmd_len, + unsigned int buf_len) +{ + return 0; +} +struct mono_time; +void timer_monotonic_get(struct mono_time *mt) +{ +} + +const struct spi_flash_ops_descriptor spi_flash_pp_0x20_sector_desc; + +int main(void) +{ + const struct CMUnitTest tests[] = { + cmocka_unit_test(spi_flash_region_bpbits_roundtrip_test), + }; + + return cmocka_run_group_tests(tests, NULL, NULL); +} diff --git a/tests/drivers/winbond-spi-test.c b/tests/drivers/winbond-spi-test.c deleted file mode 100644 index 2ed0e90..0000000 --- a/tests/drivers/winbond-spi-test.c +++ /dev/null @@ -1,301 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-or-later */ - -#include <stdarg.h> -#include <stddef.h> -#include <setjmp.h> -#include <cmocka.h> - -#include <string.h> - -#include <stdio.h> /* debug */ - -#include <commonlib/helpers.h> -#include <spi_flash.h> -#include <spi-generic.h> -#include "../drivers/spi/spi_flash_internal.h" -#include "../drivers/spi/spi_winbond.h" - -int fill_spi_flash(const struct spi_slave *spi, struct spi_flash *flash, - const struct spi_flash_vendor_info *vi, - const struct spi_flash_part_id *part); - -struct block_region { - /* these are in units of 64k-byte blocks */ - size_t offset; - size_t size; -}; - -struct bp_table { - uint16_t id; - size_t size; - struct block_region wp_blocks[2][2][1<<4]; -} bp_table[] = { - { - /* W25Q256J */ - .id = 0x7019, - .wp_blocks = { - /* [cmp][tb][bp] */ - /* off size */ - - /* cmp=0, tb=x, bp=0b0000: NONE */ - [0][0][0] = { 0, 0 }, - [0][1][0] = { 0, 0 }, - - /* cmp=1, tb=x, bp=0b0000: ALL */ - [1][0][0] = { 0, 512 }, - [1][1][0] = { 0, 512 }, - - /* cmp=0, tb=x, bp=0b110x|0b1x1x: ALL */ - [0][0 ... 1][10 ... 15] = { 0, 512 }, - - /* cmp=1, tb=x, bp=0b110x|0b1x1x: NONE */ - [1][0 ... 1][10 ... 15] = { 0, 0 }, - - [0][0][1] = { 511, 1 }, - [0][0][2] = { 510, 2 }, - [0][0][3] = { 508, 4 }, - [0][0][4] = { 504, 8 }, - [0][0][5] = { 496, 16 }, - [0][0][6] = { 480, 32 }, - [0][0][7] = { 448, 64 }, - [0][0][8] = { 384, 128 }, - [0][0][9] = { 256, 256 }, - - [0][1][1] = { 0, 1 }, - [0][1][2] = { 0, 2 }, - [0][1][3] = { 0, 4 }, - [0][1][4] = { 0, 8 }, - [0][1][5] = { 0, 16 }, - [0][1][6] = { 0, 32 }, - [0][1][7] = { 0, 64 }, - [0][1][8] = { 0, 128 }, - [0][1][9] = { 0, 256 }, - - [1][0][1] = { 0, 512-1 }, - [1][0][2] = { 0, 512-2 }, - [1][0][3] = { 0, 512-4 }, - [1][0][4] = { 0, 512-8 }, - [1][0][5] = { 0, 512-16 }, - [1][0][6] = { 0, 512-32 }, - [1][0][7] = { 0, 512-64 }, - [1][0][8] = { 0, 512-128 }, - [1][0][9] = { 0, 512-256 }, - - [1][1][1] = { 1, 512-1 }, - [1][1][2] = { 2, 512-2 }, - [1][1][3] = { 4, 512-4 }, - [1][1][4] = { 8, 512-8 }, - [1][1][5] = { 16, 512-16 }, - [1][1][6] = { 32, 512-32 }, - [1][1][7] = { 64, 512-64 }, - [1][1][8] = { 128, 512-128 }, - [1][1][9] = { 256, 512-256 }, - } - }, - - { - /* W25Q128FW */ - .id = 0x6018, - .wp_blocks = { - /* [cmp][tb][bp] */ - /* off size */ - [0][0][0] = { 0, 0 }, - - /* cmp=0, tb=x, bp=0b000: NONE */ - [0][0][0] = { 0, 0 }, - [0][1][0] = { 0, 0 }, - - /* cmp=0, tb=x, bp=0b111: ALL */ - [0][0][7] = { 0, 256 }, - [0][1][7] = { 0, 256 }, - - [0][0][1] = { 252, 4 }, - [0][0][2] = { 248, 8 }, - [0][0][3] = { 240, 16 }, - [0][0][4] = { 224, 32 }, - [0][0][5] = { 192, 64 }, - [0][0][6] = { 128, 128 }, - - [0][1][1] = { 0, 4 }, - [0][1][2] = { 0, 8 }, - [0][1][3] = { 0, 16 }, - [0][1][4] = { 0, 32 }, - [0][1][5] = { 0, 64 }, - [0][1][6] = { 0, 128 }, - - /* cmp=1, tb=x, bp=0b000: ALL */ - [1][0][0] = { 0, 256 }, - [1][1][0] = { 0, 256 }, - - /* cmp=1, tb=x, bp=0b111: NONE */ - [1][0][7] = { 0, 0 }, - [1][1][7] = { 0, 0 }, - - [1][0][1] = { 0, 256-4 }, - [1][0][2] = { 0, 256-8 }, - [1][0][3] = { 0, 256-16 }, - [1][0][4] = { 0, 256-32 }, - [1][0][5] = { 0, 256-64 }, - [1][0][6] = { 0, 256-128 }, - - [1][1][1] = { 4, 256-4 }, - [1][1][2] = { 8, 256-8 }, - [1][1][3] = { 16, 256-16 }, - [1][1][4] = { 32, 256-32 }, - [1][1][5] = { 64, 256-64 }, - [1][1][6] = { 128, 256-128 }, - } - } -}; - -static void test_one_config( - void **state, - struct spi_flash flash, - const struct spi_flash_part_id *part, - const struct bp_table *table, - const struct spi_flash_bpbits *bpbits) -{ - const size_t granularity = - 1<<part->protection_granularity_shift; - - struct region expected, out = {-1, -1}; - struct region roundtripped = {-1, -1}; - - winbond_bpbits_to_region(&flash, part, bpbits, &out); - - enum spi_flash_status_reg_lockdown mode = - SPI_WRITE_PROTECTION_PIN; - struct spi_flash_bpbits bpbits1; - winbond_region_to_bpbits(&flash, part, &out, mode, - &bpbits1); - - winbond_bpbits_to_region(&flash, part, &bpbits1, &roundtripped); - - const struct block_region *wp_blocks = - &table->wp_blocks[bpbits->cmp][bpbits->tb][bpbits->bp]; - const size_t block_size = 1<<16; - - expected.offset = wp_blocks->offset * block_size; - expected.size = wp_blocks->size * block_size; - -#if 0 - /* debugging */ - print_message("\nfor part 0x%04x cmp=%d tb=%d bp=%d:\n", - part->id[0], (int)bpbits->cmp, (int)bpbits->tb, - (int)bpbits->bp); - print_message("should: 0x%08zx-0x%08zx\n", - region_offset(&expected), - region_end(&expected)); - print_message("is: 0x%08zx-0x%08zx\n", - region_offset(&out), - region_end(&out)); - print_message("rt'ed: 0x%08zx-0x%08zx\n", - region_offset(&roundtripped), - region_end(&roundtripped)); - print_message("bp1: %d\n", bpbits1.bp); -#endif - - assert_true(expected.size == out.size); - assert_true(expected.offset == out.offset); - - assert_true(expected.size == roundtripped.size); - assert_true(expected.offset == roundtripped.offset); -} - -static void winbond_region_bpbits_roundtrip_test(void **state) -{ - int n_cfgs_tested = 0; - - const struct spi_flash_vendor_info *vi = &spi_flash_winbond_vi; - - for (size_t i = 0; i < vi->nr_part_ids; i++) { - const struct spi_flash_part_id *part = &vi->ids[i]; - - if (!part->protection_granularity_shift) - continue; - - /* FIXME: Add tables for the other parts supporting block - * protection */ - if (part->id[0] != 0x7019 && part->id[0] != 0x6018) - continue; - - struct spi_slave spi; - struct spi_flash flash; - int ret = fill_spi_flash(&spi, &flash, vi, part); - assert_int_equal(ret, 0); - - const struct bp_table *table = NULL; - for (uint16_t j = 0; j < ARRAY_SIZE(bp_table); j++) { - if (bp_table[j].id == part->id[0]) { - table = &bp_table[j]; - break; - } - } - - assert_non_null(table); - - u8 bp_max; - if (part->bp_bits == 3) - bp_max = 7; - else if (part->bp_bits == 4) - bp_max = 0xf; - else - fail_msg("Unrecognized bp_bits"); - - for (u8 tbcmp = 0; tbcmp < 4; tbcmp++) { - for (u8 bp = 0; bp <= bp_max; bp++) { - struct spi_flash_bpbits bpbits = { - .cmp = tbcmp & (1<<1), - .tb = tbcmp & (1<<0), - .bp = bp, - }; - - test_one_config( - state, flash, part, table, &bpbits); - - n_cfgs_tested++; - } - } - } - - /* print_message("n_cfgs_tested: %d\n", n_cfgs_tested); */ - assert_true(n_cfgs_tested == 96); -} - - /* Mocks */ -int do_printk(int msg_level, const char *fmt, ...) -{ - return -1; -} -int spi_claim_bus(const struct spi_slave *slave) -{ - return -1; -} -int spi_xfer_vector(const struct spi_slave *slave, - struct spi_op vectors[], size_t count) -{ - return -1; -} -void spi_release_bus(const struct spi_slave *slave) -{ -} -unsigned int spi_crop_chunk(const struct spi_slave *slave, unsigned int cmd_len, - unsigned int buf_len) -{ - return 0; -} -struct mono_time; -void timer_monotonic_get(struct mono_time *mt) -{ -} - -const struct spi_flash_ops_descriptor spi_flash_pp_0x20_sector_desc; - -int main(void) -{ - const struct CMUnitTest tests[] = { - cmocka_unit_test(winbond_region_bpbits_roundtrip_test), - }; - - return cmocka_run_group_tests(tests, NULL, NULL); -}
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42124 )
Change subject: spi: Share common logic of vendor specific bpbits<>region functions ......................................................................
Patch Set 3:
(3 comments)
https://review.coreboot.org/c/coreboot/+/42124/3/src/drivers/spi/spi_common.... File src/drivers/spi/spi_common.c:
https://review.coreboot.org/c/coreboot/+/42124/3/src/drivers/spi/spi_common.... PS3, Line 103: MIN(bits->bp ? granularity << (bits->bp - 1) : 0, Rather than doing the bp_lower_th stuff, can we just invert bits->bp when CMP=1 for Macronix? So what Macronix calls BP3 we would put in bits->cmp, their BP2-0 we put in bits->bp, and then here we do
int effective_bp = bits->bp if (part->cmp_inverts_bp && bits->cmp) effective_bp = (1 << part->bp_bits) - 1 - bits->bp; protected_size = MIN(bits->bp ? granularity << (bits->bp - 1) : 0, flash->size);
(part->bp_bits would have to be 3 for the Macronix parts then, of course.) I think that should work out for the parts you're using, and it would just take one extra bit to encode this quirk.
https://review.coreboot.org/c/coreboot/+/42124/3/src/drivers/spi/spi_flash_i... File src/drivers/spi/spi_flash_internal.h:
https://review.coreboot.org/c/coreboot/+/42124/3/src/drivers/spi/spi_flash_i... PS3, Line 88: uint16_t has_tb : 1; /* top/bottom select available */ This is the structure I'm fighting so hard to keep small because we have to store so many instances of it in the data segment. Due to the alignment of `id`, if you just go 1 bit over 4 bytes you effectively increase space requirements by 50%.
We still have a couple of free bits in the _reserved_for_flags above, so if you can get rid of the bp_lower_th and make this work with only has_tb and cmp_inverts_bp, it should fit.
https://review.coreboot.org/c/coreboot/+/42124/3/src/drivers/spi/winbond.c File src/drivers/spi/winbond.c:
https://review.coreboot.org/c/coreboot/+/42124/3/src/drivers/spi/winbond.c@5... PS3, Line 503: .region_to_bpbits = winbond_region_to_bpbits, Why not just set the spi_flash_common functions here directly?
Hello build bot (Jenkins), Martin L Roth, Daniel Gröber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42124
to look at the new patch set (#27).
Change subject: spi: Share common logic of vendor specific bpbits<>region functions ......................................................................
spi: Share common logic of vendor specific bpbits<>region functions
Here we move the the majority of the bpbits<>region functions to common code to allow sharing it with other vendors in a future commit.
Change-Id: Iadedf160575b20ee7fc671642412e6b3d437a20e Signed-off-by: Daniel Gröber dxld@darkboxed.org --- M src/drivers/spi/Makefile.inc A src/drivers/spi/spi_common.c A src/drivers/spi/spi_common.h M src/drivers/spi/spi_flash_internal.h M src/drivers/spi/spi_winbond.h M src/drivers/spi/winbond.c M tests/drivers/Makefile.inc M tests/drivers/spi-flash-winbond-test.c 8 files changed, 182 insertions(+), 85 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/42124/27
Attention is currently required from: Jakub Czapiga.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42124 )
Change subject: spi: Share common logic of vendor specific bpbits<>region functions ......................................................................
Patch Set 27:
(1 comment)
Commit Message:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-170780): https://review.coreboot.org/c/coreboot/+/42124/comment/46abdfb0_c4f71d84 PS27, Line 12: Change-Id: Iadedf160575b20ee7fc671642412e6b3d437a20e Possible repeated word: 'the'
Attention is currently required from: Jakub Czapiga.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42124 )
Change subject: spi: Share common logic of vendor specific bpbits<>region functions ......................................................................
Patch Set 28:
(1 comment)
Commit Message:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-171107): https://review.coreboot.org/c/coreboot/+/42124/comment/7dd5ef82_0737f3f9 PS28, Line 12: Change-Id: Iadedf160575b20ee7fc671642412e6b3d437a20e Possible repeated word: 'the'
Attention is currently required from: Jakub Czapiga.
Daniel Gröber (dxld) has removed Daniel Gröber from this change. ( https://review.coreboot.org/c/coreboot/+/42124 )
Change subject: spi: Share common logic of vendor specific bpbits<>region functions ......................................................................
Removed cc Daniel Gröber dxld@darkboxed.org.
Attention is currently required from: Jakub Czapiga.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42124 )
Change subject: spi: Share common logic of vendor specific bpbits<>region functions ......................................................................
Patch Set 29:
(1 comment)
Commit Message:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-171125): https://review.coreboot.org/c/coreboot/+/42124/comment/155c38aa_02a126a1 PS29, Line 12: Change-Id: Iadedf160575b20ee7fc671642412e6b3d437a20e Possible repeated word: 'the'
Attention is currently required from: Daniel Gröber, Jakub Czapiga.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42124 )
Change subject: spi: Share common logic of vendor specific bpbits<>region functions ......................................................................
Patch Set 30:
(1 comment)
Commit Message:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-171139): https://review.coreboot.org/c/coreboot/+/42124/comment/07cc1396_61613cee PS30, Line 12: Change-Id: Iadedf160575b20ee7fc671642412e6b3d437a20e Possible repeated word: 'the'
Attention is currently required from: Jakub Czapiga.
Daniel Gröber has removed Daniel Gröber from this change. ( https://review.coreboot.org/c/coreboot/+/42124 )
Change subject: spi: Share common logic of vendor specific bpbits<>region functions ......................................................................
Removed reviewer Daniel Gröber dxld@darkboxed.org.
-
build bot (Jenkins) (Code Review) -
Daniel Gröber (Code Review)
-
Daniel Gröber (dxld) (Code Review)
-
Julius Werner (Code Review)