March 2023 - flashrom-gerrit - mail.coreboot.org

[M] Change in flashrom[master]: tree/: Remove internal_buses_support from global scope
by Edward O'Callaghan (Code Review) 06 Mar '23

06 Mar '23

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/73464 ) Change subject: tree/: Remove internal_buses_support from global scope ...................................................................... tree/: Remove internal_buses_support from global scope Finally the buses_supported by the internal programmer can be packed up into the board_cfg substructure and out of the global symbol scope. TEST=`$ sudo ./flashrom -p internal --flash-name`. Change-Id: I987f2726acbf78e0294f8977f5a1734d2350405e Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M board_enable.c M chipset_enable.c M ichspi.c M include/programmer.h M internal.c M internal_par.c M it87spi.c 7 files changed, 38 insertions(+), 24 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/64/73464/1 diff --git a/board_enable.c b/board_enable.c index 1ed1c5d..efb4131 100644 --- a/board_enable.c +++ b/board_enable.c @@ -606,7 +606,7 @@ uint8_t tmp; int ret = 0; - if (!(internal_buses_supported & BUS_PARALLEL)) + if (!(bcfg->internal_buses_supported & BUS_PARALLEL)) return 1; enter_conf_mode_ite(port); @@ -614,7 +614,7 @@ /* Check if at least one flash segment is enabled. */ if (tmp & 0xf0) { /* The IT8705F will respond to LPC cycles and translate them. */ - internal_buses_supported &= BUS_PARALLEL; + bcfg->internal_buses_supported &= BUS_PARALLEL; /* Flash ROM I/F Writes Enable */ tmp |= 0x04; msg_pdbg("Enabling IT8705F flash ROM interface write.\n"); diff --git a/chipset_enable.c b/chipset_enable.c index 0747d57..84206b2 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -72,13 +72,13 @@ tmp = pci_read_byte(dev, 0x40) & 0x3; switch (tmp) { case 3: - internal_buses_supported &= BUS_FWH; + cfg->bcfg->internal_buses_supported &= BUS_FWH; break; case 2: - internal_buses_supported &= BUS_LPC; + cfg->bcfg->internal_buses_supported &= BUS_LPC; break; default: - internal_buses_supported &= BUS_PARALLEL; + cfg->bcfg->internal_buses_supported &= BUS_PARALLEL; break; } @@ -228,7 +228,7 @@ uint16_t old, new; uint16_t xbcs = 0x4e; /* X-Bus Chip Select register. */ - internal_buses_supported &= BUS_PARALLEL; + cfg->bcfg->internal_buses_supported &= BUS_PARALLEL; old = pci_read_word(dev, xbcs); @@ -563,7 +563,7 @@ if ((err = enable_flash_ich_fwh_decode(cfg, dev, ich_generation)) != 0) return err; - internal_buses_supported &= BUS_FWH; + cfg->bcfg->internal_buses_supported &= BUS_FWH; return enable_flash_ich_bios_cntl_config_space(dev, ich_generation, bios_cntl); } @@ -1064,7 +1064,7 @@ if (ret_fwh == ERROR_FLASHROM_FATAL) return ret_fwh; - internal_buses_supported &= BUS_FWH; + cfg->bcfg->internal_buses_supported &= BUS_FWH; /* Get physical address of SPI Base Address and map it */ uint32_t sbase = pci_read_long(dev, 0x54) & 0xfffffe00; @@ -1218,7 +1218,7 @@ #define CS5530_ENABLE_SA2320 (1 << 2) #define CS5530_ENABLE_SA20 (1 << 6) - internal_buses_supported &= BUS_PARALLEL; + cfg->bcfg->internal_buses_supported &= BUS_PARALLEL; /* Decode 0x000E0000-0x000FFFFF (128 kB), not just 64 kB, and * decode 0xFF000000-0xFFFFFFFF (16 MB), not just 256 kB. * FIXME: Should we really touch the low mapping below 1 MB? Flashrom @@ -1406,7 +1406,7 @@ msg_pdbg("done.\n"); } - internal_buses_supported &= BUS_LPC | BUS_FWH; + cfg->bcfg->internal_buses_supported &= BUS_LPC | BUS_FWH; ret = sb600_probe_spi(cfg, dev); @@ -1554,7 +1554,7 @@ { uint8_t tmp; - internal_buses_supported &= BUS_PARALLEL; + cfg->bcfg->internal_buses_supported &= BUS_PARALLEL; tmp = INB(0xc06); tmp |= 0x1; @@ -1643,7 +1643,7 @@ switch ((val >> 5) & 0x3) { case 0x0: ret = enable_flash_mcp55(cfg, dev, name); - internal_buses_supported &= BUS_LPC; + cfg->bcfg->internal_buses_supported &= BUS_LPC; msg_pdbg("Flash bus type is LPC\n"); break; case 0x2: @@ -1651,12 +1651,12 @@ /* SPI is added in mcp6x_spi_init if it works. * Do we really want to disable LPC in this case? */ - internal_buses_supported = BUS_NONE; + cfg->bcfg->internal_buses_supported = BUS_NONE; msg_pdbg("Flash bus type is SPI\n"); break; default: /* Should not happen. */ - internal_buses_supported = BUS_NONE; + cfg->bcfg->internal_buses_supported = BUS_NONE; msg_pwarn("Flash bus type is unknown (none)\n"); msg_pinfo("Please send the log files created by \"flashrom -p internal -o logfile\" to\n" "flashrom(a)flashrom.org with \"your board name: flashrom -V\" as the subject to\n" @@ -2229,7 +2229,7 @@ "flashrom(a)flashrom.org including a verbose " "(-V) log.\nThank you!\n"); } - if (!(chipset_enables[i].buses & (internal_buses_supported | BUS_SPI))) { + if (!(chipset_enables[i].buses & (cfg->bcfg->internal_buses_supported | BUS_SPI))) { msg_pdbg("Skipping chipset enable: No supported buses enabled.\n"); continue; } diff --git a/ichspi.c b/ichspi.c index 0d3dd0e..d8f0196 100644 --- a/ichspi.c +++ b/ichspi.c @@ -2394,7 +2394,7 @@ /* Do we really need no write enable? Like the LPC one at D17F0 0x40 */ /* Not sure if it speaks all these bus protocols. */ - internal_buses_supported &= BUS_LPC | BUS_FWH; + cfg->bcfg->internal_buses_supported &= BUS_LPC | BUS_FWH; ich_generation = CHIPSET_ICH7; register_spi_master(&spi_master_via, NULL); diff --git a/include/programmer.h b/include/programmer.h index 97c5e81..2477c80 100644 --- a/include/programmer.h +++ b/include/programmer.h @@ -167,6 +167,7 @@ struct board_cfg { int is_laptop; bool laptop_ok; + enum chipbustype internal_buses_supported; }; struct board_match { @@ -275,7 +276,6 @@ extern bool force_boardmismatch; void probe_superio(void); int register_superio(struct superio s); -extern enum chipbustype internal_buses_supported; #endif /* bitbang_spi.c */ diff --git a/internal.c b/internal.c index 071ea51..bba7043 100644 --- a/internal.c +++ b/internal.c @@ -29,8 +29,6 @@ bool force_boardmismatch = false; -enum chipbustype internal_buses_supported = BUS_NONE; - static int get_params(const struct programmer_cfg *cfg, bool *boardenable, bool *boardmismatch, @@ -164,7 +162,7 @@ * is found, the host controller init routine sets the * internal_buses_supported bitfield. */ - internal_buses_supported = BUS_NONSPI; + bcfg.internal_buses_supported = BUS_NONSPI; if (try_mtd(cfg) == 0) { ret = 0; @@ -229,7 +227,7 @@ * non-legacy buses (SPI and opaque atm) are probed anyway. */ if (bcfg.is_laptop && !(bcfg.laptop_ok || force_laptop || (not_a_laptop && bcfg.is_laptop == 2))) - internal_buses_supported = BUS_NONE; + bcfg.internal_buses_supported = BUS_NONE; /* try to enable it. Failure IS an option, since not all motherboards * really need this to be done, etc., etc. @@ -257,7 +255,7 @@ } #endif - internal_par_init(internal_buses_supported); + internal_par_init(bcfg.internal_buses_supported); /* Report if a non-whitelisted laptop is detected that likely uses a legacy bus. */ report_nonwl_laptop_detected(&bcfg); diff --git a/internal_par.c b/internal_par.c index e8e387c..031123e 100644 --- a/internal_par.c +++ b/internal_par.c @@ -75,5 +75,5 @@ void internal_par_init(enum chipbustype buses) { if (buses & BUS_NONSPI) - register_par_master(&par_master_internal, internal_buses_supported, NULL); + register_par_master(&par_master_internal, buses, NULL); } diff --git a/it87spi.c b/it87spi.c index dd9ab71..9e231f5 100644 --- a/it87spi.c +++ b/it87spi.c @@ -435,7 +435,7 @@ data->flashport = flashport; data->fast_spi = true; - if (internal_buses_supported & BUS_SPI) + if (cfg->bcfg->internal_buses_supported & BUS_SPI) msg_pdbg("Overriding chipset SPI with IT87 SPI.\n"); /* FIXME: Add the SPI bus or replace the other buses with it? */ return register_spi_master(&spi_master_it87xx, data); -- To view, visit https://review.coreboot.org/c/flashrom/+/73464 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I987f2726acbf78e0294f8977f5a1734d2350405e Gerrit-Change-Number: 73464 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: it87spi.c: Plumb board_cfg to it8705f_write_enable() entry
by Edward O'Callaghan (Code Review) 06 Mar '23

06 Mar '23

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/73463 ) Change subject: it87spi.c: Plumb board_cfg to it8705f_write_enable() entry ...................................................................... it87spi.c: Plumb board_cfg to it8705f_write_enable() entry Change-Id: Ia48d4d4acd02550c72ee46bfe4042b3c75941da0 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M board_enable.c M include/programmer.h M it87spi.c 3 files changed, 13 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/63/73463/1 diff --git a/board_enable.c b/board_enable.c index d9af44e..1ed1c5d 100644 --- a/board_enable.c +++ b/board_enable.c @@ -601,7 +601,7 @@ * Suited for all boards with ITE IT8705F. * The SIS950 Super I/O probably requires a similar flash write enable. */ -int it8705f_write_enable(uint8_t port) +int it8705f_write_enable(struct board_cfg *bcfg, uint8_t port) { uint8_t tmp; int ret = 0; diff --git a/include/programmer.h b/include/programmer.h index e81d3bd..97c5e81 100644 --- a/include/programmer.h +++ b/include/programmer.h @@ -231,7 +231,7 @@ void w836xx_ext_enter(uint16_t port); void w836xx_ext_leave(uint16_t port); void probe_superio_winbond(void); -int it8705f_write_enable(uint8_t port); +int it8705f_write_enable(struct board_cfg *bcfg, uint8_t port); uint8_t sio_read(uint16_t port, uint8_t reg); void sio_write(uint16_t port, uint8_t reg, uint8_t data); void sio_mask(uint16_t port, uint8_t reg, uint8_t data, uint8_t mask); diff --git a/it87spi.c b/it87spi.c index 9c64659..dd9ab71 100644 --- a/it87spi.c +++ b/it87spi.c @@ -452,7 +452,7 @@ switch (superios[i].model) { case 0x8705: - ret |= it8705f_write_enable(superios[i].port); + ret |= it8705f_write_enable(cfg->bcfg, superios[i].port); break; case 0x8686: case 0x8716: -- To view, visit https://review.coreboot.org/c/flashrom/+/73463 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia48d4d4acd02550c72ee46bfe4042b3c75941da0 Gerrit-Change-Number: 73463 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: ichspi: Plumb programmer_cfg to via_init_spi() entry
by Edward O'Callaghan (Code Review) 06 Mar '23

06 Mar '23

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/73462 ) Change subject: ichspi: Plumb programmer_cfg to via_init_spi() entry ...................................................................... ichspi: Plumb programmer_cfg to via_init_spi() entry Change-Id: I965645432a2d077fbdb3c4cdc36d008a5846ff8d Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M chipset_enable.c M ichspi.c M include/programmer.h 3 files changed, 14 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/62/73462/1 diff --git a/chipset_enable.c b/chipset_enable.c index 16ef0f4..0747d57 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -1193,12 +1193,12 @@ return ERROR_FLASHROM_FATAL; } - return via_init_spi(spi0_mm_base); + return via_init_spi(cfg, spi0_mm_base); } static int enable_flash_vt8237s_spi(const struct programmer_cfg *cfg, struct pci_dev *dev, const char *name) { - return via_init_spi(pci_read_long(dev, 0xbc) << 8); + return via_init_spi(cfg, pci_read_long(dev, 0xbc) << 8); } static int enable_flash_cs5530(const struct programmer_cfg *cfg, struct pci_dev *dev, const char *name) diff --git a/ichspi.c b/ichspi.c index 1522d2b..0d3dd0e 100644 --- a/ichspi.c +++ b/ichspi.c @@ -2384,7 +2384,7 @@ .probe_opcode = ich_spi_probe_opcode, }; -int via_init_spi(uint32_t mmio_base) +int via_init_spi(const struct programmer_cfg *cfg, uint32_t mmio_base) { int i; diff --git a/include/programmer.h b/include/programmer.h index 5b304f5..e81d3bd 100644 --- a/include/programmer.h +++ b/include/programmer.h @@ -368,7 +368,7 @@ /* ichspi.c */ #if CONFIG_INTERNAL == 1 int ich_init_spi(const struct programmer_cfg *cfg, void *spibar, enum ich_chipset ich_generation); -int via_init_spi(uint32_t mmio_base); +int via_init_spi(const struct programmer_cfg *cfg, uint32_t mmio_base); /* amd_imc.c */ int amd_imc_shutdown(struct pci_dev *dev); -- To view, visit https://review.coreboot.org/c/flashrom/+/73462 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I965645432a2d077fbdb3c4cdc36d008a5846ff8d Gerrit-Change-Number: 73462 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

[M] Change in flashrom[master]: internal: Move laptop_ok into board_cfg
by Edward O'Callaghan (Code Review) 06 Mar '23

06 Mar '23

Attention is currently required from: Thomas Heijligen, Angel Pons. Hello build bot (Jenkins), Thomas Heijligen, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/73456 to look at the new patch set (#2). Change subject: internal: Move laptop_ok into board_cfg ...................................................................... internal: Move laptop_ok into board_cfg Due to how internal is structured around chipset_flash_enable() entry we need to prepare a crafted programmer_cfg that contains a board_enable substructure with data derived from the board_enable subsystem. While this is certainly not perfection, it does make clear the relationships between board_enable into chipset_flash_enable and subsequently the overall internal programmer initialisation in a RAII fashion at the type level over closure upon global state that is impossible to reason about. Also flip predicate in report_nonwl_laptop_detected() and return early with the trivial base-case. TEST=`$ sudo ./flashrom -p internal --flash-name`. Change-Id: I459215253845c2af73262943ce91a36464e9eb06 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M board_enable.c M chipset_enable.c M flashrom.c M include/programmer.h M internal.c 5 files changed, 69 insertions(+), 44 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/56/73456/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/73456 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I459215253845c2af73262943ce91a36464e9eb06 Gerrit-Change-Number: 73456 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[L] Change in flashrom[master]: doc: Add build instructions
by Peter Marheine (Code Review) 06 Mar '23

06 Mar '23

Attention is currently required from: Thomas Heijligen, Stefan Reinauer, Anastasia Klimchuk. Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73359 ) Change subject: doc: Add build instructions ...................................................................... Patch Set 3: (1 comment) File doc/developers_doc/building_from_source.rst: https://review.coreboot.org/c/flashrom/+/73359/comment/218e1cf7_861729a0 PS3, Line 175: .. todo:: Write a sphinx extension to render ``meson_options.txt`` here `meson configure` will print out all options including project-specific ones, and the inline documentation for options isn't very useful right now (it doesn't seem useful unless you already have some idea of what the options do). I think the important options should either be documented manually here (providing more context for those that are typically interesting), or document that `meson configure` will show them (assuming that the plain information you get from there isn't very useful and it's not worth much effort to provide it inline here). -- To view, visit https://review.coreboot.org/c/flashrom/+/73359 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I96771e98b313a6d26dd2be940ff37998d4124324 Gerrit-Change-Number: 73359 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 06 Mar 2023 02:04:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: internal: Move laptop_ok into board_cfg
by Edward O'Callaghan (Code Review) 06 Mar '23

06 Mar '23

Attention is currently required from: Thomas Heijligen, Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73456 ) Change subject: internal: Move laptop_ok into board_cfg ...................................................................... Patch Set 1: (1 comment) File internal.c: https://review.coreboot.org/c/flashrom/+/73456/comment/b07d9582_4fdc19b7 PS1, Line 158: : /* Unconditionally reset global state from previous operation. */ : bcfg.laptop_ok = false; actually can delete this now. -- To view, visit https://review.coreboot.org/c/flashrom/+/73456 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I459215253845c2af73262943ce91a36464e9eb06 Gerrit-Change-Number: 73456 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 06 Mar 2023 00:38:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: board_enables: Allow for prog cfg coupling with board cfg
by Edward O'Callaghan (Code Review) 06 Mar '23

06 Mar '23

Attention is currently required from: Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71623 ) Change subject: board_enables: Allow for prog cfg coupling with board cfg ...................................................................... Patch Set 3: This change is ready for review. -- To view, visit https://review.coreboot.org/c/flashrom/+/71623 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7058a693e714a6966a842ae97cc8da7296e63e5e Gerrit-Change-Number: 71623 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 06 Mar 2023 00:31:00 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: internal: Move laptop_ok into board_cfg
by Edward O'Callaghan (Code Review) 06 Mar '23

06 Mar '23

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/73456 ) Change subject: internal: Move laptop_ok into board_cfg ...................................................................... internal: Move laptop_ok into board_cfg Due to how internal is structured around chipset_flash_enable() entry we need to prepare a crafted programmer_cfg that contains a board_enable substructure with data derived from the board_enable subsystem. While this is certainly not perfection, it does make clear the relationships between board_enable into chipset_flash_enable and subsequently the overall internal programmer initialisation in a RAII fashion at the type level over closure upon global state that is impossible to reason about. TEST=`$ sudo ./flashrom -p internal --flash-name`. Change-Id: I459215253845c2af73262943ce91a36464e9eb06 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M board_enable.c M chipset_enable.c M flashrom.c M include/programmer.h M internal.c 5 files changed, 40 insertions(+), 19 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/56/73456/1 diff --git a/board_enable.c b/board_enable.c index f10f5fc..d9af44e 100644 --- a/board_enable.c +++ b/board_enable.c @@ -2293,7 +2293,7 @@ static int p2_whitelist_laptop(struct board_cfg *cfg) { cfg->is_laptop = 1; - laptop_ok = true; + cfg->laptop_ok = true; msg_pdbg("Whitelisted laptop detected.\n"); return 0; } diff --git a/chipset_enable.c b/chipset_enable.c index e1dd6a3..16ef0f4 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -826,7 +826,7 @@ /* Suppress unknown laptop warning if we booted from SPI. */ if (boot_buses & BUS_SPI) - laptop_ok = true; + cfg->bcfg->laptop_ok = true; return 0; } @@ -971,7 +971,7 @@ /* Suppress unknown laptop warning if we booted from SPI. */ if (!ret && (boot_buses & BUS_SPI)) - laptop_ok = true; + cfg->bcfg->laptop_ok = true; _freepci_ret: pci_free_dev(spi_dev); @@ -1087,7 +1087,7 @@ /* Suppress unknown laptop warning if we booted from SPI. */ if (boot_buses & BUS_SPI) - laptop_ok = true; + cfg->bcfg->laptop_ok = true; return 0; } @@ -1676,7 +1676,7 @@ /* Suppress unknown laptop warning if we booted from SPI. */ if (!ret && want_spi) - laptop_ok = true; + cfg->bcfg->laptop_ok = true; return ret; } diff --git a/flashrom.c b/flashrom.c index 3cf67f1..da46354 100644 --- a/flashrom.c +++ b/flashrom.c @@ -65,10 +65,6 @@ */ static bool may_register_shutdown = false; -struct programmer_cfg { - char *params; -}; - /* Register a function to be executed on programmer shutdown. * The advantage over atexit() is that you can supply a void pointer which will * be used as parameter to the registered function upon programmer shutdown. diff --git a/include/programmer.h b/include/programmer.h index ec92702..5b304f5 100644 --- a/include/programmer.h +++ b/include/programmer.h @@ -30,7 +30,11 @@ USB, OTHER, }; -struct programmer_cfg; +struct board_cfg; +struct programmer_cfg { + char *params; + struct board_cfg *bcfg; +}; struct dev_entry { uint16_t vendor_id; @@ -162,6 +166,7 @@ struct board_cfg { int is_laptop; + bool laptop_ok; }; struct board_match { @@ -267,7 +272,6 @@ #endif #if CONFIG_INTERNAL == 1 -extern bool laptop_ok; extern bool force_boardmismatch; void probe_superio(void); int register_superio(struct superio s); diff --git a/internal.c b/internal.c index 7f0e0e9..79d6352 100644 --- a/internal.c +++ b/internal.c @@ -27,8 +27,6 @@ #include "hwaccess_x86_io.h" #endif -bool laptop_ok = false; - bool force_boardmismatch = false; enum chipbustype internal_buses_supported = BUS_NONE; @@ -108,9 +106,9 @@ } // FIXME: remove '_' suffix from parameters once global shadowing is fixed. -static void report_nonwl_laptop_detected(int is_laptop, bool laptop_ok_) +static void report_nonwl_laptop_detected(int is_laptop, bool laptop_ok) { - if (is_laptop && !laptop_ok_) { + if (is_laptop && !laptop_ok) { msg_pinfo("========================================================================\n"); if (is_laptop == 1) { msg_pinfo("You seem to be running flashrom on an unknown laptop. Some\n" @@ -159,7 +157,7 @@ return ret; /* Unconditionally reset global state from previous operation. */ - laptop_ok = false; + bcfg.laptop_ok = false; /* Default to Parallel/LPC/FWH flash devices. If a known host controller * is found, the host controller init routine sets the @@ -229,13 +227,15 @@ * this isn't a laptop. Board-enables may override this, * non-legacy buses (SPI and opaque atm) are probed anyway. */ - if (bcfg.is_laptop && !(laptop_ok || force_laptop || (not_a_laptop && bcfg.is_laptop == 2))) + if (bcfg.is_laptop && !(bcfg.laptop_ok || force_laptop || (not_a_laptop && bcfg.is_laptop == 2))) internal_buses_supported = BUS_NONE; /* try to enable it. Failure IS an option, since not all motherboards * really need this to be done, etc., etc. */ - ret = chipset_flash_enable(cfg); + struct programmer_cfg icfg = *cfg; + icfg.bcfg = &bcfg; + ret = chipset_flash_enable(&icfg); if (ret == -2) { msg_perr("WARNING: No chipset found. Flash detection " "will most likely fail.\n"); @@ -259,7 +259,7 @@ internal_par_init(internal_buses_supported); /* Report if a non-whitelisted laptop is detected that likely uses a legacy bus. */ - report_nonwl_laptop_detected(bcfg.is_laptop, laptop_ok); + report_nonwl_laptop_detected(bcfg.is_laptop, bcfg.laptop_ok); ret = 0; -- To view, visit https://review.coreboot.org/c/flashrom/+/73456 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I459215253845c2af73262943ce91a36464e9eb06 Gerrit-Change-Number: 73456 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: internal: Move is_laptop into board_cfg
by Edward O'Callaghan (Code Review) 06 Mar '23

06 Mar '23

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/73455 ) Change subject: internal: Move is_laptop into board_cfg ...................................................................... internal: Move is_laptop into board_cfg Change-Id: I24e38e4457299934acdcd70325d0bf0f4b139e5f Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M board_enable.c M include/programmer.h M internal.c 3 files changed, 18 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/55/73455/1 diff --git a/board_enable.c b/board_enable.c index 146b40f..f10f5fc 100644 --- a/board_enable.c +++ b/board_enable.c @@ -2282,7 +2282,7 @@ static int p2_not_a_laptop(struct board_cfg *cfg) { /* label this board as not a laptop */ - g_is_laptop = 0; + cfg->is_laptop = 0; msg_pdbg("Laptop detection overridden by P2 board enable.\n"); return 0; } @@ -2292,7 +2292,7 @@ */ static int p2_whitelist_laptop(struct board_cfg *cfg) { - g_is_laptop = 1; + cfg->is_laptop = 1; laptop_ok = true; msg_pdbg("Whitelisted laptop detected.\n"); return 0; diff --git a/include/programmer.h b/include/programmer.h index bc5e8ba..ec92702 100644 --- a/include/programmer.h +++ b/include/programmer.h @@ -161,6 +161,7 @@ }; struct board_cfg { + int is_laptop; }; struct board_match { @@ -266,7 +267,6 @@ #endif #if CONFIG_INTERNAL == 1 -extern int g_is_laptop; extern bool laptop_ok; extern bool force_boardmismatch; void probe_superio(void); diff --git a/internal.c b/internal.c index d59fcf2..7f0e0e9 100644 --- a/internal.c +++ b/internal.c @@ -27,7 +27,6 @@ #include "hwaccess_x86_io.h" #endif -int g_is_laptop = 0; bool laptop_ok = false; bool force_boardmismatch = false; @@ -150,7 +149,7 @@ const char *cb_model = NULL; #endif bool force_boardenable = false; - struct board_cfg bcfg; + struct board_cfg bcfg = {0}; ret = get_params(cfg, &force_boardenable, &force_boardmismatch, @@ -205,9 +204,9 @@ } } - g_is_laptop = 2; /* Assume that we don't know by default. */ + bcfg.is_laptop = 2; /* Assume that we don't know by default. */ - dmi_init(&g_is_laptop); + dmi_init(&bcfg.is_laptop); /* In case Super I/O probing would cause pretty explosions. */ board_handle_before_superio(&bcfg, force_boardenable); @@ -230,7 +229,7 @@ * this isn't a laptop. Board-enables may override this, * non-legacy buses (SPI and opaque atm) are probed anyway. */ - if (g_is_laptop && !(laptop_ok || force_laptop || (not_a_laptop && g_is_laptop == 2))) + if (bcfg.is_laptop && !(laptop_ok || force_laptop || (not_a_laptop && bcfg.is_laptop == 2))) internal_buses_supported = BUS_NONE; /* try to enable it. Failure IS an option, since not all motherboards @@ -260,7 +259,7 @@ internal_par_init(internal_buses_supported); /* Report if a non-whitelisted laptop is detected that likely uses a legacy bus. */ - report_nonwl_laptop_detected(g_is_laptop, laptop_ok); + report_nonwl_laptop_detected(bcfg.is_laptop, laptop_ok); ret = 0; -- To view, visit https://review.coreboot.org/c/flashrom/+/73455 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I24e38e4457299934acdcd70325d0bf0f4b139e5f Gerrit-Change-Number: 73455 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

[M] Change in flashrom[master]: wbsio_spi.c: Convert to direct driver entry
by Edward O'Callaghan (Code Review) 05 Mar '23

05 Mar '23

Attention is currently required from: Felix Singer, Thomas Heijligen. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72864 ) Change subject: wbsio_spi.c: Convert to direct driver entry ...................................................................... Patch Set 2: (3 comments) Patchset: PS2: > The entries in `test_build. […] Done File wbsio_spi.c: https://review.coreboot.org/c/flashrom/+/72864/comment/742defac_c885f0fb PS2, Line 240: msg_perr("There is currently no way to determine if the programmer works on a board. " > This programmer is known to work only on the 'Intel D201GLY' mainboard. […] Done https://review.coreboot.org/c/flashrom/+/72864/comment/9c634650_3c577ec5 PS2, Line 265: const struct programmer_entry programmer_wbsio_spi = { > ``devs. […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/72864 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I54b166048089bc502b99a345c02e91894590894e Gerrit-Change-Number: 72864 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Sun, 05 Mar 2023 23:24:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit March 2023