October 2021 - flashrom-gerrit

Change in flashrom[master]: [RFC] writeprotect: implement wp_get_ranges()
by Nikolai Artemiev (Code Review) 29 Oct '21

29 Oct '21

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/58481 to look at the new patch set (#6). Change subject: [RFC] writeprotect: implement wp_get_ranges() ...................................................................... [RFC] writeprotect: implement wp_get_ranges() Generate list of available ranges by enumerating all possible values that range bits (BPx, TB, ...) can take and using the chip's range decoding function to get the range associated with each one. BUG=b:195381327,b:153800563 TEST=flashrom --wp-list BRANCH=none Change-Id: Id51f038f03305c8536d80313e52f77d27835f34d Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M writeprotect.c M writeprotect.h 2 files changed, 119 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/81/58481/6 -- To view, visit https://review.coreboot.org/c/flashrom/+/58481 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id51f038f03305c8536d80313e52f77d27835f34d Gerrit-Change-Number: 58481 Gerrit-PatchSet: 6 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: [RFC] writeprotect: implement wp_{get,set}_range()
by Nikolai Artemiev (Code Review) 29 Oct '21

29 Oct '21

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/58482 to look at the new patch set (#6). Change subject: [RFC] writeprotect: implement wp_{get,set}_range() ...................................................................... [RFC] writeprotect: implement wp_{get,set}_range() BUG=b:195381327,b:153800563 TEST=flashrom --wp-{status,range} BRANCH=none Change-Id: I7d26f43fb05c5828b9839bb57a28fa1088dcd9a0 Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M cli_classic.c M writeprotect.c M writeprotect.h 3 files changed, 61 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/82/58482/6 -- To view, visit https://review.coreboot.org/c/flashrom/+/58482 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7d26f43fb05c5828b9839bb57a28fa1088dcd9a0 Gerrit-Change-Number: 58482 Gerrit-PatchSet: 6 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: [RFC] writeprotect: implement wp_get_mode() and wp_set_mode()
by Nikolai Artemiev (Code Review) 29 Oct '21

29 Oct '21

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/58483 to look at the new patch set (#6). Change subject: [RFC] writeprotect: implement wp_get_mode() and wp_set_mode() ...................................................................... [RFC] writeprotect: implement wp_get_mode() and wp_set_mode() BUG=b:195381327,b:153800563 TEST=flashrom --wp-{enable,disable,status} BRANCH=none Change-Id: I7b68e940f0e1359281806c98e1da119b4caf8405 Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M cli_classic.c M writeprotect.c M writeprotect.h 3 files changed, 129 insertions(+), 13 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/83/58483/6 -- To view, visit https://review.coreboot.org/c/flashrom/+/58483 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7b68e940f0e1359281806c98e1da119b4caf8405 Gerrit-Change-Number: 58483 Gerrit-PatchSet: 6 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: [RFC] writeprotect: implement wp_print_ranges()
by Nikolai Artemiev (Code Review) 29 Oct '21

29 Oct '21

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/58484 to look at the new patch set (#6). Change subject: [RFC] writeprotect: implement wp_print_ranges() ...................................................................... [RFC] writeprotect: implement wp_print_ranges() BUG=b:195381327,b:153800563 TEST=flashrom --wp-list BRANCH=none Change-Id: I82441c60c239a5b947b5a2a87b61e7bdf60bee89 Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M cli_classic.c M writeprotect.c M writeprotect.h 3 files changed, 52 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/84/58484/6 -- To view, visit https://review.coreboot.org/c/flashrom/+/58484 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I82441c60c239a5b947b5a2a87b61e7bdf60bee89 Gerrit-Change-Number: 58484 Gerrit-PatchSet: 6 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: ichspi: Split very long init function into two
by Anastasia Klimchuk (Code Review) 29 Oct '21

29 Oct '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58735 ) Change subject: ichspi: Split very long init function into two ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: I have an end goal of removing global state from ichspi, and also I want to write a test for it. I think it will take a while to accomplish. I plan to move in small steps and test changes on devices that are available. -- To view, visit https://review.coreboot.org/c/flashrom/+/58735 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6789bc456a4878e6555831ae0b80ecbdbf62938b Gerrit-Change-Number: 58735 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Fri, 29 Oct 2021 04:55:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi: Extract handling programmer param into a function
by Anastasia Klimchuk (Code Review) 29 Oct '21

29 Oct '21

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/58736 ) Change subject: ichspi: Extract handling programmer param into a function ...................................................................... ichspi: Extract handling programmer param into a function Extract processing of ich_spi_mode into a separate function which is called from init_ich_default. This makes init_ich_default more readable and avoids one local variable. TEST ME ON HW BUG=b:204488958 Change-Id: I20e2379a6fd58c9346f0a2d6daf2b8decf1f6976 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M ichspi.c 1 file changed, 38 insertions(+), 27 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/36/58736/1 diff --git a/ichspi.c b/ichspi.c index 9db241c..7a9c5cd 100644 --- a/ichspi.c +++ b/ichspi.c @@ -1749,20 +1749,49 @@ return 0; } +enum ich_spi_mode { + ich_auto, + ich_hwseq, + ich_swseq +}; + +static int get_ich_spi_mode_param(enum ich_spi_mode *ich_spi_mode) +{ + char *arg = extract_programmer_param("ich_spi_mode"); + if (arg && !strcmp(arg, "hwseq")) { + *ich_spi_mode = ich_hwseq; + msg_pspew("user selected hwseq\n"); + } else if (arg && !strcmp(arg, "swseq")) { + *ich_spi_mode = ich_swseq; + msg_pspew("user selected swseq\n"); + } else if (arg && !strcmp(arg, "auto")) { + msg_pspew("user selected auto\n"); + *ich_spi_mode = ich_auto; + } else if (arg && !strlen(arg)) { + msg_perr("Missing argument for ich_spi_mode.\n"); + free(arg); + return ERROR_FATAL; + } else if (arg) { + msg_perr("Unknown argument for ich_spi_mode: %s\n", + arg); + free(arg); + return ERROR_FATAL; + } + free(arg); + + return 0; +} + + static int init_ich_default(void *spibar, enum ich_chipset ich_gen) { unsigned int i; uint16_t tmp2; uint32_t tmp; - char *arg; int ich_spi_rw_restricted = 0; int desc_valid = 0; struct ich_descriptors desc = { 0 }; - enum ich_spi_mode { - ich_auto, - ich_hwseq, - ich_swseq - } ich_spi_mode = ich_auto; + enum ich_spi_mode ich_spi_mode = ich_auto; size_t num_freg, num_pr, reg_pr0; @@ -1817,27 +1846,9 @@ break; } - arg = extract_programmer_param("ich_spi_mode"); - if (arg && !strcmp(arg, "hwseq")) { - ich_spi_mode = ich_hwseq; - msg_pspew("user selected hwseq\n"); - } else if (arg && !strcmp(arg, "swseq")) { - ich_spi_mode = ich_swseq; - msg_pspew("user selected swseq\n"); - } else if (arg && !strcmp(arg, "auto")) { - msg_pspew("user selected auto\n"); - ich_spi_mode = ich_auto; - } else if (arg && !strlen(arg)) { - msg_perr("Missing argument for ich_spi_mode.\n"); - free(arg); - return ERROR_FATAL; - } else if (arg) { - msg_perr("Unknown argument for ich_spi_mode: %s\n", - arg); - free(arg); - return ERROR_FATAL; - } - free(arg); + int ret = get_ich_spi_mode_param(&ich_spi_mode); + if (ret) + return ret; tmp2 = mmio_readw(spibar + ICH9_REG_HSFS); msg_pdbg("0x04: 0x%04x (HSFS)\n", tmp2); -- To view, visit https://review.coreboot.org/c/flashrom/+/58736 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I20e2379a6fd58c9346f0a2d6daf2b8decf1f6976 Gerrit-Change-Number: 58736 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: ichspi: Extract initialisation of swseq and hwseq into a function
by Anastasia Klimchuk (Code Review) 29 Oct '21

29 Oct '21

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/58737 ) Change subject: ichspi: Extract initialisation of swseq and hwseq into a function ...................................................................... ichspi: Extract initialisation of swseq and hwseq into a function Initialisation of swseq_data and hwseq_data gets its own function, which is called from init_ich_default. This makes init_ich_default more readable. This patch also gives a name to (previously anonymous) struct swseq_data. Its sibling struct hwseq_data already has a name. Structs need names to be able to daclare function parameters. TEST ME ON HW BUG=b:204488958 Change-Id: I7d62b1b380e497b82dcae1284d752204cc541bd3 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M ichspi.c 1 file changed, 54 insertions(+), 48 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/37/58737/1 diff --git a/ichspi.c b/ichspi.c index 7a9c5cd..0966ec6 100644 --- a/ichspi.c +++ b/ichspi.c @@ -504,7 +504,7 @@ pprint_reg(DLOCK, SSEQ_LOCKDN, reg_val, "\n"); } -static struct { +static struct swseq_data { size_t reg_ssfsc; size_t reg_preop; size_t reg_optype; @@ -1782,6 +1782,58 @@ return 0; } +static void init_swseq_hwseq(struct swseq_data *swseq, struct hwseq_data *hwseq, + size_t *num_freg, size_t *num_pr, size_t *reg_pr0, + enum ich_chipset ich_gen) +{ + /* Moving registers / bits */ + switch (ich_gen) { + case CHIPSET_100_SERIES_SUNRISE_POINT: + case CHIPSET_C620_SERIES_LEWISBURG: + case CHIPSET_300_SERIES_CANNON_POINT: + case CHIPSET_400_SERIES_COMET_POINT: + case CHIPSET_APOLLO_LAKE: + case CHIPSET_GEMINI_LAKE: + *num_pr = 6; /* Includes GPR0 */ + *reg_pr0 = PCH100_REG_FPR0; + swseq->reg_ssfsc = PCH100_REG_SSFSC; + swseq->reg_preop = PCH100_REG_PREOP; + swseq->reg_optype = PCH100_REG_OPTYPE; + swseq->reg_opmenu = PCH100_REG_OPMENU; + hwseq->addr_mask = PCH100_FADDR_FLA; + hwseq->only_4k = true; + hwseq->hsfc_fcycle = PCH100_HSFC_FCYCLE; + break; + default: + *num_pr = 5; + *reg_pr0 = ICH9_REG_PR0; + swseq->reg_ssfsc = ICH9_REG_SSFS; + swseq->reg_preop = ICH9_REG_PREOP; + swseq->reg_optype = ICH9_REG_OPTYPE; + swseq->reg_opmenu = ICH9_REG_OPMENU; + hwseq->addr_mask = ICH9_FADDR_FLA; + hwseq->only_4k = false; + hwseq->hsfc_fcycle = HSFC_FCYCLE; + break; + } + switch (ich_gen) { + case CHIPSET_100_SERIES_SUNRISE_POINT: + *num_freg = 10; + break; + case CHIPSET_C620_SERIES_LEWISBURG: + *num_freg = 12; /* 12 MMIO regs, but 16 regions in FD spec */ + break; + case CHIPSET_300_SERIES_CANNON_POINT: + case CHIPSET_400_SERIES_COMET_POINT: + case CHIPSET_APOLLO_LAKE: + case CHIPSET_GEMINI_LAKE: + *num_freg = 16; + break; + default: + *num_freg = 5; + break; + } +} static int init_ich_default(void *spibar, enum ich_chipset ich_gen) { @@ -1798,53 +1850,7 @@ ich_generation = ich_gen; ich_spibar = spibar; - /* Moving registers / bits */ - switch (ich_gen) { - case CHIPSET_100_SERIES_SUNRISE_POINT: - case CHIPSET_C620_SERIES_LEWISBURG: - case CHIPSET_300_SERIES_CANNON_POINT: - case CHIPSET_400_SERIES_COMET_POINT: - case CHIPSET_APOLLO_LAKE: - case CHIPSET_GEMINI_LAKE: - num_pr = 6; /* Includes GPR0 */ - reg_pr0 = PCH100_REG_FPR0; - swseq_data.reg_ssfsc = PCH100_REG_SSFSC; - swseq_data.reg_preop = PCH100_REG_PREOP; - swseq_data.reg_optype = PCH100_REG_OPTYPE; - swseq_data.reg_opmenu = PCH100_REG_OPMENU; - hwseq_data.addr_mask = PCH100_FADDR_FLA; - hwseq_data.only_4k = true; - hwseq_data.hsfc_fcycle = PCH100_HSFC_FCYCLE; - break; - default: - num_pr = 5; - reg_pr0 = ICH9_REG_PR0; - swseq_data.reg_ssfsc = ICH9_REG_SSFS; - swseq_data.reg_preop = ICH9_REG_PREOP; - swseq_data.reg_optype = ICH9_REG_OPTYPE; - swseq_data.reg_opmenu = ICH9_REG_OPMENU; - hwseq_data.addr_mask = ICH9_FADDR_FLA; - hwseq_data.only_4k = false; - hwseq_data.hsfc_fcycle = HSFC_FCYCLE; - break; - } - switch (ich_gen) { - case CHIPSET_100_SERIES_SUNRISE_POINT: - num_freg = 10; - break; - case CHIPSET_C620_SERIES_LEWISBURG: - num_freg = 12; /* 12 MMIO regs, but 16 regions in FD spec */ - break; - case CHIPSET_300_SERIES_CANNON_POINT: - case CHIPSET_400_SERIES_COMET_POINT: - case CHIPSET_APOLLO_LAKE: - case CHIPSET_GEMINI_LAKE: - num_freg = 16; - break; - default: - num_freg = 5; - break; - } + init_swseq_hwseq(&swseq_data, &hwseq_data, &num_freg, &num_pr, &reg_pr0, ich_gen); int ret = get_ich_spi_mode_param(&ich_spi_mode); if (ret) -- To view, visit https://review.coreboot.org/c/flashrom/+/58737 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7d62b1b380e497b82dcae1284d752204cc541bd3 Gerrit-Change-Number: 58737 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: ichspi: Split very long init function into two
by Anastasia Klimchuk (Code Review) 29 Oct '21

29 Oct '21

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/58735 ) Change subject: ichspi: Split very long init function into two ...................................................................... ichspi: Split very long init function into two ich_init_spi is very long, but logically it can be split. Init function detects the chipset and then the rest of operations depends on the chipset. Init function is more readable now, it consists of only a switch. TEST ME ON DEVICE BUG=b:204488958 Change-Id: I6789bc456a4878e6555831ae0b80ecbdbf62938b Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M ichspi.c 1 file changed, 268 insertions(+), 252 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/35/58735/1 diff --git a/ichspi.c b/ichspi.c index d345af2..9db241c 100644 --- a/ichspi.c +++ b/ichspi.c @@ -1710,7 +1710,46 @@ .erase = ich_hwseq_block_erase, }; -int ich_init_spi(void *spibar, enum ich_chipset ich_gen) +static int init_ich7_spi(void *spibar, enum ich_chipset ich_gen) +{ + unsigned int i; + + msg_pdbg("0x00: 0x%04x (SPIS)\n", + mmio_readw(spibar + 0)); + msg_pdbg("0x02: 0x%04x (SPIC)\n", + mmio_readw(spibar + 2)); + msg_pdbg("0x04: 0x%08x (SPIA)\n", + mmio_readl(spibar + 4)); + ichspi_bbar = mmio_readl(spibar + 0x50); + msg_pdbg("0x50: 0x%08x (BBAR)\n", + ichspi_bbar); + msg_pdbg("0x54: 0x%04x (PREOP)\n", + mmio_readw(spibar + 0x54)); + msg_pdbg("0x56: 0x%04x (OPTYPE)\n", + mmio_readw(spibar + 0x56)); + msg_pdbg("0x58: 0x%08x (OPMENU)\n", + mmio_readl(spibar + 0x58)); + msg_pdbg("0x5c: 0x%08x (OPMENU+4)\n", + mmio_readl(spibar + 0x5c)); + + for (i = 0; i < 3; i++) { + int offs; + offs = 0x60 + (i * 4); + msg_pdbg("0x%02x: 0x%08x (PBR%u)\n", offs, + mmio_readl(spibar + offs), i); + } + if (mmio_readw(spibar) & (1 << 15)) { + msg_pwarn("WARNING: SPI Configuration Lockdown activated.\n"); + ichspi_lock = 1; + } + ich_init_opcodes(ich_gen); + ich_set_bbar(0, ich_gen); + register_spi_master(&spi_master_ich7, NULL); + + return 0; +} + +static int init_ich_default(void *spibar, enum ich_chipset ich_gen) { unsigned int i; uint16_t tmp2; @@ -1724,6 +1763,7 @@ ich_hwseq, ich_swseq } ich_spi_mode = ich_auto; + size_t num_freg, num_pr, reg_pr0; ich_generation = ich_gen; @@ -1777,276 +1817,252 @@ break; } - switch (ich_gen) { - case CHIPSET_ICH7: - case CHIPSET_TUNNEL_CREEK: - case CHIPSET_CENTERTON: - msg_pdbg("0x00: 0x%04x (SPIS)\n", - mmio_readw(spibar + 0)); - msg_pdbg("0x02: 0x%04x (SPIC)\n", - mmio_readw(spibar + 2)); - msg_pdbg("0x04: 0x%08x (SPIA)\n", - mmio_readl(spibar + 4)); - ichspi_bbar = mmio_readl(spibar + 0x50); - msg_pdbg("0x50: 0x%08x (BBAR)\n", - ichspi_bbar); - msg_pdbg("0x54: 0x%04x (PREOP)\n", - mmio_readw(spibar + 0x54)); - msg_pdbg("0x56: 0x%04x (OPTYPE)\n", - mmio_readw(spibar + 0x56)); - msg_pdbg("0x58: 0x%08x (OPMENU)\n", - mmio_readl(spibar + 0x58)); - msg_pdbg("0x5c: 0x%08x (OPMENU+4)\n", - mmio_readl(spibar + 0x5c)); - for (i = 0; i < 3; i++) { - int offs; - offs = 0x60 + (i * 4); - msg_pdbg("0x%02x: 0x%08x (PBR%u)\n", offs, - mmio_readl(spibar + offs), i); - } - if (mmio_readw(spibar) & (1 << 15)) { - msg_pwarn("WARNING: SPI Configuration Lockdown activated.\n"); - ichspi_lock = 1; - } - ich_init_opcodes(ich_gen); - ich_set_bbar(0, ich_gen); - register_spi_master(&spi_master_ich7, NULL); - break; - case CHIPSET_ICH8: - default: /* Future version might behave the same */ - arg = extract_programmer_param("ich_spi_mode"); - if (arg && !strcmp(arg, "hwseq")) { - ich_spi_mode = ich_hwseq; - msg_pspew("user selected hwseq\n"); - } else if (arg && !strcmp(arg, "swseq")) { - ich_spi_mode = ich_swseq; - msg_pspew("user selected swseq\n"); - } else if (arg && !strcmp(arg, "auto")) { - msg_pspew("user selected auto\n"); - ich_spi_mode = ich_auto; - } else if (arg && !strlen(arg)) { - msg_perr("Missing argument for ich_spi_mode.\n"); - free(arg); - return ERROR_FATAL; - } else if (arg) { - msg_perr("Unknown argument for ich_spi_mode: %s\n", - arg); - free(arg); - return ERROR_FATAL; - } + arg = extract_programmer_param("ich_spi_mode"); + if (arg && !strcmp(arg, "hwseq")) { + ich_spi_mode = ich_hwseq; + msg_pspew("user selected hwseq\n"); + } else if (arg && !strcmp(arg, "swseq")) { + ich_spi_mode = ich_swseq; + msg_pspew("user selected swseq\n"); + } else if (arg && !strcmp(arg, "auto")) { + msg_pspew("user selected auto\n"); + ich_spi_mode = ich_auto; + } else if (arg && !strlen(arg)) { + msg_perr("Missing argument for ich_spi_mode.\n"); free(arg); + return ERROR_FATAL; + } else if (arg) { + msg_perr("Unknown argument for ich_spi_mode: %s\n", + arg); + free(arg); + return ERROR_FATAL; + } + free(arg); - tmp2 = mmio_readw(spibar + ICH9_REG_HSFS); - msg_pdbg("0x04: 0x%04x (HSFS)\n", tmp2); - prettyprint_ich9_reg_hsfs(tmp2, ich_gen); - if (tmp2 & HSFS_FLOCKDN) { - msg_pinfo("SPI Configuration is locked down.\n"); - ichspi_lock = 1; - } - if (tmp2 & HSFS_FDV) - desc_valid = 1; - if (!(tmp2 & HSFS_FDOPSS) && desc_valid) - msg_pinfo("The Flash Descriptor Override Strap-Pin is set. Restrictions implied by\n" - "the Master Section of the flash descriptor are NOT in effect. Please note\n" - "that Protected Range (PR) restrictions still apply.\n"); - ich_init_opcodes(ich_gen); + tmp2 = mmio_readw(spibar + ICH9_REG_HSFS); + msg_pdbg("0x04: 0x%04x (HSFS)\n", tmp2); + prettyprint_ich9_reg_hsfs(tmp2, ich_gen); + if (tmp2 & HSFS_FLOCKDN) { + msg_pinfo("SPI Configuration is locked down.\n"); + ichspi_lock = 1; + } + if (tmp2 & HSFS_FDV) + desc_valid = 1; + if (!(tmp2 & HSFS_FDOPSS) && desc_valid) + msg_pinfo("The Flash Descriptor Override Strap-Pin is set. Restrictions implied by\n" + "the Master Section of the flash descriptor are NOT in effect. Please note\n" + "that Protected Range (PR) restrictions still apply.\n"); + ich_init_opcodes(ich_gen); - if (desc_valid) { - tmp2 = mmio_readw(spibar + ICH9_REG_HSFC); - msg_pdbg("0x06: 0x%04x (HSFC)\n", tmp2); - prettyprint_ich9_reg_hsfc(tmp2, ich_gen); - } + if (desc_valid) { + tmp2 = mmio_readw(spibar + ICH9_REG_HSFC); + msg_pdbg("0x06: 0x%04x (HSFC)\n", tmp2); + prettyprint_ich9_reg_hsfc(tmp2, ich_gen); + } - tmp = mmio_readl(spibar + ICH9_REG_FADDR); - msg_pdbg2("0x08: 0x%08x (FADDR)\n", tmp); + tmp = mmio_readl(spibar + ICH9_REG_FADDR); + msg_pdbg2("0x08: 0x%08x (FADDR)\n", tmp); + switch (ich_gen) { + case CHIPSET_100_SERIES_SUNRISE_POINT: + case CHIPSET_C620_SERIES_LEWISBURG: + case CHIPSET_300_SERIES_CANNON_POINT: + case CHIPSET_400_SERIES_COMET_POINT: + case CHIPSET_APOLLO_LAKE: + case CHIPSET_GEMINI_LAKE: + tmp = mmio_readl(spibar + PCH100_REG_DLOCK); + msg_pdbg("0x0c: 0x%08x (DLOCK)\n", tmp); + prettyprint_pch100_reg_dlock(tmp); + break; + default: + break; + } + + if (desc_valid) { + tmp = mmio_readl(spibar + ICH9_REG_FRAP); + msg_pdbg("0x50: 0x%08x (FRAP)\n", tmp); + msg_pdbg("BMWAG 0x%02x, ", ICH_BMWAG(tmp)); + msg_pdbg("BMRAG 0x%02x, ", ICH_BMRAG(tmp)); + msg_pdbg("BRWA 0x%02x, ", ICH_BRWA(tmp)); + msg_pdbg("BRRA 0x%02x\n", ICH_BRRA(tmp)); + + /* Handle FREGx and FRAP registers */ + for (i = 0; i < num_freg; i++) + ich_spi_rw_restricted |= ich9_handle_frap(tmp, i); + if (ich_spi_rw_restricted) + msg_pinfo("Not all flash regions are freely accessible by flashrom. This is " + "most likely\ndue to an active ME. Please see " + "https://flashrom.org/ME for details.\n"); + } + + /* Handle PR registers */ + for (i = 0; i < num_pr; i++) { + /* if not locked down try to disable PR locks first */ + if (!ichspi_lock) + ich9_set_pr(reg_pr0, i, 0, 0); + ich_spi_rw_restricted |= ich9_handle_pr(reg_pr0, i); + } + + switch (ich_spi_rw_restricted) { + case WRITE_PROT: + msg_pwarn("At least some flash regions are write protected. For write operations,\n" + "you should use a flash layout and include only writable regions. See\n" + "manpage for more details.\n"); + break; + case READ_PROT: + case LOCKED: + msg_pwarn("At least some flash regions are read protected. You have to use a flash\n" + "layout and include only accessible regions. For write operations, you'll\n" + "additionally need the --noverify-all switch. See manpage for more details.\n" + ); + break; + } + + tmp = mmio_readl(spibar + swseq_data.reg_ssfsc); + msg_pdbg("0x%zx: 0x%02x (SSFS)\n", swseq_data.reg_ssfsc, tmp & 0xff); + prettyprint_ich9_reg_ssfs(tmp); + if (tmp & SSFS_FCERR) { + msg_pdbg("Clearing SSFS.FCERR\n"); + mmio_writeb(SSFS_FCERR, spibar + swseq_data.reg_ssfsc); + } + msg_pdbg("0x%zx: 0x%06x (SSFC)\n", swseq_data.reg_ssfsc + 1, tmp >> 8); + prettyprint_ich9_reg_ssfc(tmp); + + msg_pdbg("0x%zx: 0x%04x (PREOP)\n", + swseq_data.reg_preop, mmio_readw(spibar + swseq_data.reg_preop)); + msg_pdbg("0x%zx: 0x%04x (OPTYPE)\n", + swseq_data.reg_optype, mmio_readw(spibar + swseq_data.reg_optype)); + msg_pdbg("0x%zx: 0x%08x (OPMENU)\n", + swseq_data.reg_opmenu, mmio_readl(spibar + swseq_data.reg_opmenu)); + msg_pdbg("0x%zx: 0x%08x (OPMENU+4)\n", + swseq_data.reg_opmenu + 4, mmio_readl(spibar + swseq_data.reg_opmenu + 4)); + + if (desc_valid) { switch (ich_gen) { + case CHIPSET_ICH8: case CHIPSET_100_SERIES_SUNRISE_POINT: case CHIPSET_C620_SERIES_LEWISBURG: case CHIPSET_300_SERIES_CANNON_POINT: case CHIPSET_400_SERIES_COMET_POINT: case CHIPSET_APOLLO_LAKE: case CHIPSET_GEMINI_LAKE: - tmp = mmio_readl(spibar + PCH100_REG_DLOCK); - msg_pdbg("0x0c: 0x%08x (DLOCK)\n", tmp); - prettyprint_pch100_reg_dlock(tmp); + case CHIPSET_BAYTRAIL: break; default: + ichspi_bbar = mmio_readl(spibar + ICH9_REG_BBAR); + msg_pdbg("0x%x: 0x%08x (BBAR)\n", ICH9_REG_BBAR, ichspi_bbar); + ich_set_bbar(0, ich_gen); break; } - if (desc_valid) { - tmp = mmio_readl(spibar + ICH9_REG_FRAP); - msg_pdbg("0x50: 0x%08x (FRAP)\n", tmp); - msg_pdbg("BMWAG 0x%02x, ", ICH_BMWAG(tmp)); - msg_pdbg("BMRAG 0x%02x, ", ICH_BMRAG(tmp)); - msg_pdbg("BRWA 0x%02x, ", ICH_BRWA(tmp)); - msg_pdbg("BRRA 0x%02x\n", ICH_BRRA(tmp)); - - /* Handle FREGx and FRAP registers */ - for (i = 0; i < num_freg; i++) - ich_spi_rw_restricted |= ich9_handle_frap(tmp, i); - if (ich_spi_rw_restricted) - msg_pinfo("Not all flash regions are freely accessible by flashrom. This is " - "most likely\ndue to an active ME. Please see " - "https://flashrom.org/ME for details.\n"); - } - - /* Handle PR registers */ - for (i = 0; i < num_pr; i++) { - /* if not locked down try to disable PR locks first */ - if (!ichspi_lock) - ich9_set_pr(reg_pr0, i, 0, 0); - ich_spi_rw_restricted |= ich9_handle_pr(reg_pr0, i); - } - - switch (ich_spi_rw_restricted) { - case WRITE_PROT: - msg_pwarn("At least some flash regions are write protected. For write operations,\n" - "you should use a flash layout and include only writable regions. See\n" - "manpage for more details.\n"); - break; - case READ_PROT: - case LOCKED: - msg_pwarn("At least some flash regions are read protected. You have to use a flash\n" - "layout and include only accessible regions. For write operations, you'll\n" - "additionally need the --noverify-all switch. See manpage for more details.\n" - ); - break; - } - - tmp = mmio_readl(spibar + swseq_data.reg_ssfsc); - msg_pdbg("0x%zx: 0x%02x (SSFS)\n", swseq_data.reg_ssfsc, tmp & 0xff); - prettyprint_ich9_reg_ssfs(tmp); - if (tmp & SSFS_FCERR) { - msg_pdbg("Clearing SSFS.FCERR\n"); - mmio_writeb(SSFS_FCERR, spibar + swseq_data.reg_ssfsc); - } - msg_pdbg("0x%zx: 0x%06x (SSFC)\n", swseq_data.reg_ssfsc + 1, tmp >> 8); - prettyprint_ich9_reg_ssfc(tmp); - - msg_pdbg("0x%zx: 0x%04x (PREOP)\n", - swseq_data.reg_preop, mmio_readw(spibar + swseq_data.reg_preop)); - msg_pdbg("0x%zx: 0x%04x (OPTYPE)\n", - swseq_data.reg_optype, mmio_readw(spibar + swseq_data.reg_optype)); - msg_pdbg("0x%zx: 0x%08x (OPMENU)\n", - swseq_data.reg_opmenu, mmio_readl(spibar + swseq_data.reg_opmenu)); - msg_pdbg("0x%zx: 0x%08x (OPMENU+4)\n", - swseq_data.reg_opmenu + 4, mmio_readl(spibar + swseq_data.reg_opmenu + 4)); - - if (desc_valid) { - switch (ich_gen) { - case CHIPSET_ICH8: - case CHIPSET_100_SERIES_SUNRISE_POINT: - case CHIPSET_C620_SERIES_LEWISBURG: - case CHIPSET_300_SERIES_CANNON_POINT: - case CHIPSET_400_SERIES_COMET_POINT: - case CHIPSET_APOLLO_LAKE: - case CHIPSET_GEMINI_LAKE: - case CHIPSET_BAYTRAIL: - break; - default: - ichspi_bbar = mmio_readl(spibar + ICH9_REG_BBAR); - msg_pdbg("0x%x: 0x%08x (BBAR)\n", ICH9_REG_BBAR, ichspi_bbar); - ich_set_bbar(0, ich_gen); - break; - } - - if (ich_gen == CHIPSET_ICH8) { - tmp = mmio_readl(spibar + ICH8_REG_VSCC); - msg_pdbg("0x%x: 0x%08x (VSCC)\n", ICH8_REG_VSCC, tmp); - msg_pdbg("VSCC: "); - prettyprint_ich_reg_vscc(tmp, FLASHROM_MSG_DEBUG, true); - } else { - tmp = mmio_readl(spibar + ICH9_REG_LVSCC); - msg_pdbg("0x%x: 0x%08x (LVSCC)\n", ICH9_REG_LVSCC, tmp); - msg_pdbg("LVSCC: "); - prettyprint_ich_reg_vscc(tmp, FLASHROM_MSG_DEBUG, true); - - tmp = mmio_readl(spibar + ICH9_REG_UVSCC); - msg_pdbg("0x%x: 0x%08x (UVSCC)\n", ICH9_REG_UVSCC, tmp); - msg_pdbg("UVSCC: "); - prettyprint_ich_reg_vscc(tmp, FLASHROM_MSG_DEBUG, false); - } - - switch (ich_gen) { - case CHIPSET_ICH8: - case CHIPSET_100_SERIES_SUNRISE_POINT: - case CHIPSET_C620_SERIES_LEWISBURG: - case CHIPSET_300_SERIES_CANNON_POINT: - case CHIPSET_400_SERIES_COMET_POINT: - case CHIPSET_APOLLO_LAKE: - case CHIPSET_GEMINI_LAKE: - break; - default: - tmp = mmio_readl(spibar + ICH9_REG_FPB); - msg_pdbg("0x%x: 0x%08x (FPB)\n", ICH9_REG_FPB, tmp); - break; - } - - if (read_ich_descriptors_via_fdo(ich_gen, spibar, &desc) == ICH_RET_OK) - prettyprint_ich_descriptors(ich_gen, &desc); - - /* If the descriptor is valid and indicates multiple - * flash devices we need to use hwseq to be able to - * access the second flash device. - */ - if (ich_spi_mode == ich_auto && desc.content.NC != 0) { - msg_pinfo("Enabling hardware sequencing due to " - "multiple flash chips detected.\n"); - ich_spi_mode = ich_hwseq; - } - } - - if (ich_spi_mode == ich_auto && ichspi_lock && - ich_missing_opcodes()) { - msg_pinfo("Enabling hardware sequencing because " - "some important opcode is locked.\n"); - ich_spi_mode = ich_hwseq; - } - - if (ich_spi_mode == ich_auto && - (ich_gen == CHIPSET_100_SERIES_SUNRISE_POINT || - ich_gen == CHIPSET_300_SERIES_CANNON_POINT || - ich_gen == CHIPSET_400_SERIES_COMET_POINT)) { - msg_pdbg("Enabling hardware sequencing by default for 100+ series PCH.\n"); - ich_spi_mode = ich_hwseq; - } - - if (ich_spi_mode == ich_auto && - (ich_gen == CHIPSET_APOLLO_LAKE || - ich_gen == CHIPSET_GEMINI_LAKE)) { - msg_pdbg("Enabling hardware sequencing by default for Apollo/Gemini Lake.\n"); - ich_spi_mode = ich_hwseq; - } - - if (ich_spi_mode == ich_hwseq) { - if (!desc_valid) { - msg_perr("Hardware sequencing was requested " - "but the flash descriptor is not " - "valid. Aborting.\n"); - return ERROR_FATAL; - } - - int tmpi = getFCBA_component_density(ich_gen, &desc, 0); - if (tmpi < 0) { - msg_perr("Could not determine density of flash component %d.\n", 0); - return ERROR_FATAL; - } - hwseq_data.size_comp0 = tmpi; - - tmpi = getFCBA_component_density(ich_gen, &desc, 1); - if (tmpi < 0) { - msg_perr("Could not determine density of flash component %d.\n", 1); - return ERROR_FATAL; - } - hwseq_data.size_comp1 = tmpi; - - register_opaque_master(&opaque_master_ich_hwseq, NULL); + if (ich_gen == CHIPSET_ICH8) { + tmp = mmio_readl(spibar + ICH8_REG_VSCC); + msg_pdbg("0x%x: 0x%08x (VSCC)\n", ICH8_REG_VSCC, tmp); + msg_pdbg("VSCC: "); + prettyprint_ich_reg_vscc(tmp, FLASHROM_MSG_DEBUG, true); } else { - register_spi_master(&spi_master_ich9, NULL); + tmp = mmio_readl(spibar + ICH9_REG_LVSCC); + msg_pdbg("0x%x: 0x%08x (LVSCC)\n", ICH9_REG_LVSCC, tmp); + msg_pdbg("LVSCC: "); + prettyprint_ich_reg_vscc(tmp, FLASHROM_MSG_DEBUG, true); + + tmp = mmio_readl(spibar + ICH9_REG_UVSCC); + msg_pdbg("0x%x: 0x%08x (UVSCC)\n", ICH9_REG_UVSCC, tmp); + msg_pdbg("UVSCC: "); + prettyprint_ich_reg_vscc(tmp, FLASHROM_MSG_DEBUG, false); } - break; + + switch (ich_gen) { + case CHIPSET_ICH8: + case CHIPSET_100_SERIES_SUNRISE_POINT: + case CHIPSET_C620_SERIES_LEWISBURG: + case CHIPSET_300_SERIES_CANNON_POINT: + case CHIPSET_400_SERIES_COMET_POINT: + case CHIPSET_APOLLO_LAKE: + case CHIPSET_GEMINI_LAKE: + break; + default: + tmp = mmio_readl(spibar + ICH9_REG_FPB); + msg_pdbg("0x%x: 0x%08x (FPB)\n", ICH9_REG_FPB, tmp); + break; + } + + if (read_ich_descriptors_via_fdo(ich_gen, spibar, &desc) == ICH_RET_OK) + prettyprint_ich_descriptors(ich_gen, &desc); + + /* If the descriptor is valid and indicates multiple + * flash devices we need to use hwseq to be able to + * access the second flash device. + */ + if (ich_spi_mode == ich_auto && desc.content.NC != 0) { + msg_pinfo("Enabling hardware sequencing due to " + "multiple flash chips detected.\n"); + ich_spi_mode = ich_hwseq; + } + } + + if (ich_spi_mode == ich_auto && ichspi_lock && + ich_missing_opcodes()) { + msg_pinfo("Enabling hardware sequencing because " + "some important opcode is locked.\n"); + ich_spi_mode = ich_hwseq; + } + + if (ich_spi_mode == ich_auto && + (ich_gen == CHIPSET_100_SERIES_SUNRISE_POINT || + ich_gen == CHIPSET_300_SERIES_CANNON_POINT || + ich_gen == CHIPSET_400_SERIES_COMET_POINT)) { + msg_pdbg("Enabling hardware sequencing by default for 100+ series PCH.\n"); + ich_spi_mode = ich_hwseq; + } + + if (ich_spi_mode == ich_auto && + (ich_gen == CHIPSET_APOLLO_LAKE || + ich_gen == CHIPSET_GEMINI_LAKE)) { + msg_pdbg("Enabling hardware sequencing by default for Apollo/Gemini Lake.\n"); + ich_spi_mode = ich_hwseq; + } + + if (ich_spi_mode == ich_hwseq) { + if (!desc_valid) { + msg_perr("Hardware sequencing was requested " + "but the flash descriptor is not " + "valid. Aborting.\n"); + return ERROR_FATAL; + } + + int tmpi = getFCBA_component_density(ich_gen, &desc, 0); + if (tmpi < 0) { + msg_perr("Could not determine density of flash component %d.\n", 0); + return ERROR_FATAL; + } + hwseq_data.size_comp0 = tmpi; + + tmpi = getFCBA_component_density(ich_gen, &desc, 1); + if (tmpi < 0) { + msg_perr("Could not determine density of flash component %d.\n", 1); + return ERROR_FATAL; + } + hwseq_data.size_comp1 = tmpi; + + register_opaque_master(&opaque_master_ich_hwseq, NULL); + } else { + register_spi_master(&spi_master_ich9, NULL); + } + + return 0; +} + +int ich_init_spi(void *spibar, enum ich_chipset ich_gen) +{ + switch (ich_gen) { + case CHIPSET_ICH7: + case CHIPSET_TUNNEL_CREEK: + case CHIPSET_CENTERTON: + return init_ich7_spi(spibar, ich_gen); + case CHIPSET_ICH8: + default: /* Future version might behave the same */ + return init_ich_default(spibar, ich_gen); } return 0; -- To view, visit https://review.coreboot.org/c/flashrom/+/58735 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6789bc456a4878e6555831ae0b80ecbdbf62938b Gerrit-Change-Number: 58735 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: flashchips.c: big erase blocksize first
by Simon Buhrow (Code Review) 28 Oct '21

28 Oct '21

Attention is currently required from: Nico Huber, Angel Pons. Simon Buhrow has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58467 ) Change subject: flashchips.c: big erase blocksize first ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > Have you tried with random data? You can create a file with random data like this: […] You are right. With your setup writing random_data_2.bin takes much longer with this commit than with origin. So the solution must be much smarter. Furthermore I saw that the erase function is set in walk_by_layout while the per block function is called somewhat deeper. This looks like a good solution will need reorder of steps. I´ll keep trying/thinking of how the steps you mentioned could be implemented. Any suggestions are welcome. @Angel: Thanks a lot for your detailed setup description. That helped me a lot! -- To view, visit https://review.coreboot.org/c/flashrom/+/58467 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I862ce0b5f8912565e43c340578d8126aa2e6aa3b Gerrit-Change-Number: 58467 Gerrit-PatchSet: 1 Gerrit-Owner: Simon Buhrow Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Thu, 28 Oct 2021 11:43:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Simon Buhrow Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom_tester: Use elogtool to list firmware eventlog
by Angel Pons (Code Review) 27 Oct '21

27 Oct '21

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58527 ) Change subject: flashrom_tester: Use elogtool to list firmware eventlog ...................................................................... Patch Set 3: (1 comment) Patchset: PS2: > Can we land it? Sure thing, thanks for the reminder -- To view, visit https://review.coreboot.org/c/flashrom/+/58527 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8c4be82fed28b6a19746e6b93fafce23bd8ede5d Gerrit-Change-Number: 58527 Gerrit-PatchSet: 3 Gerrit-Owner: Jack Rosenthal <jrosenth(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Ricardo Quesada <ricardoq(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Wed, 27 Oct 2021 09:13:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Jack Rosenthal <jrosenth(a)chromium.org> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit October 2021