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

[S] Change in flashrom[master]: board_enable.c: Invert branch for readability
by Angel Pons (Code Review) 02 Apr '23

02 Apr '23

Attention is currently required from: Edward O'Callaghan. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/74164 ) Change subject: board_enable.c: Invert branch for readability ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/74164 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9bd96d4d077270697c329d7e6cac1d32ed513ed3 Gerrit-Change-Number: 74164 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Sun, 02 Apr 2023 16:10:19 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: board_enable.c: Avoid excessive forward declarations
by Angel Pons (Code Review) 02 Apr '23

02 Apr '23

Attention is currently required from: Edward O'Callaghan. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/74163 ) Change subject: board_enable.c: Avoid excessive forward declarations ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/74163 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5d416a023c17568c6275426edc1edb77268ea331 Gerrit-Change-Number: 74163 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Sun, 02 Apr 2023 16:09:45 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[L] Change in flashrom[master]: util/flashchips_db_parser: Add initial rust flashchip json parser
by Edward O'Callaghan (Code Review) 02 Apr '23

02 Apr '23

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/74166 ) Change subject: util/flashchips_db_parser: Add initial rust flashchip json parser ...................................................................... util/flashchips_db_parser: Add initial rust flashchip json parser Change-Id: I48346a3da5923e614728a1cc0567ed2bb1ba2e2f Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- A util/flashchips_db_parser/Cargo.lock A util/flashchips_db_parser/Cargo.toml A util/flashchips_db_parser/src/main.rs A util/flashchips_db_parser/src/parser.rs 4 files changed, 403 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/66/74166/1 diff --git a/util/flashchips_db_parser/Cargo.lock b/util/flashchips_db_parser/Cargo.lock new file mode 100644 index 0000000..b8e7070 --- /dev/null +++ b/util/flashchips_db_parser/Cargo.lock @@ -0,0 +1,89 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 3 + +[[package]] +name = "flashchips_db_parser_rust" +version = "0.1.0" +dependencies = [ + "serde", + "serde_json", +] + +[[package]] +name = "itoa" +version = "1.0.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "453ad9f582a441959e5f0d088b02ce04cfe8d51a8eaf077f12ac6d3e94164ca6" + +[[package]] +name = "proc-macro2" +version = "1.0.54" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e472a104799c74b514a57226160104aa483546de37e839ec50e3c2e41dd87534" +dependencies = [ + "unicode-ident", +] + +[[package]] +name = "quote" +version = "1.0.26" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4424af4bf778aae2051a77b60283332f386554255d722233d09fbfc7e30da2fc" +dependencies = [ + "proc-macro2", +] + +[[package]] +name = "ryu" +version = "1.0.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f91339c0467de62360649f8d3e185ca8de4224ff281f66000de5eb2a77a79041" + +[[package]] +name = "serde" +version = "1.0.159" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3c04e8343c3daeec41f58990b9d77068df31209f2af111e059e9fe9646693065" +dependencies = [ + "serde_derive", +] + +[[package]] +name = "serde_derive" +version = "1.0.159" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4c614d17805b093df4b147b51339e7e44bf05ef59fba1e45d83500bcfb4d8585" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "serde_json" +version = "1.0.95" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d721eca97ac802aa7777b701877c8004d950fc142651367300d21c1cc0194744" +dependencies = [ + "itoa", + "ryu", + "serde", +] + +[[package]] +name = "syn" +version = "2.0.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "79d9531f94112cfc3e4c8f5f02cb2b58f72c97b7efd85f70203cc6d8efda5927" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + +[[package]] +name = "unicode-ident" +version = "1.0.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e5464a87b239f13a63a501f2701565754bae92d243d4bb7eb12f6d57d2269bf4" diff --git a/util/flashchips_db_parser/Cargo.toml b/util/flashchips_db_parser/Cargo.toml new file mode 100644 index 0000000..a402ebc --- /dev/null +++ b/util/flashchips_db_parser/Cargo.toml @@ -0,0 +1,10 @@ +[package] +name = "flashchips_db_parser_rust" +version = "0.1.0" +edition = "2021" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] +serde = { version = "1.0.159", features = ["derive"] } +serde_json = "1.0.95" diff --git a/util/flashchips_db_parser/src/main.rs b/util/flashchips_db_parser/src/main.rs new file mode 100644 index 0000000..9ac1d4b --- /dev/null +++ b/util/flashchips_db_parser/src/main.rs @@ -0,0 +1,30 @@ +// +// This file is part of the flashrom project. +// +// Copyright (C) 2023 Google Australia. +// +// This program is free software; you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation; either version 2 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// + +use std::fs; + +pub mod parser; + +fn main() { + let file_path = "../../data/flashchips.json".to_owned(); + let contents = fs::read_to_string(file_path).expect("Couldn't find or load that file."); + + let result = parser::flashchips(&contents); + match result { + Ok(result) => (), + Err(error) => print!("{}", error), + } +} diff --git a/util/flashchips_db_parser/src/parser.rs b/util/flashchips_db_parser/src/parser.rs new file mode 100644 index 0000000..c02799c --- /dev/null +++ b/util/flashchips_db_parser/src/parser.rs @@ -0,0 +1,264 @@ +// +// This file is part of the flashrom project. +// +// Copyright (C) 2023 Google Australia. +// +// This program is free software; you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation; either version 2 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// + +use std::collections::HashMap; + +use serde::{Deserialize, Serialize}; +use serde_json::Result; //{Result, Value}; + +//#[derive(Serialize, Deserialize)] +//enum ChipBusType { +// BUS_NONE = 0, +// BUS_PARALLEL = 1 << 0, +// BUS_LPC = 1 << 1, +// BUS_FWH = 1 << 2, +// BUS_SPI = 1 << 3, +// BUS_PROG = 1 << 4, +// BUS_NONSPI = (1 << 0) | (1 << 1) | (1 << 2), // BUS_PARALLEL | BUS_LPC | BUS_FWH, +//} + +#[derive(Serialize, Deserialize)] +enum WriteGranularity { + WRITE_GRAN_256BYTES = 0, + WRITE_GRAN_1BIT, + WRITE_GRAN_1BYTE, + WRITE_GRAN_128BYTES, + WRITE_GRAN_264BYTES, + WRITE_GRAN_512BYTES, + WRITE_GRAN_528BYTES, + WRITE_GRAN_1024BYTES, + WRITE_GRAN_1056BYTES, + WRITE_GRAN_1BYTE_IMPLICIT_ERASE, +} + +#[derive(Serialize, Deserialize)] +struct EraseBlock { + size: u32, + count: u32, +} + +#[derive(Serialize, Deserialize)] +enum BlockEraseFunc { + NO_BLOCK_ERASE_FUNC = 0, /* 0 indicates no block erase function set. */ + SPI_BLOCK_ERASE_EMULATION = 1, + SPI_BLOCK_ERASE_20, + SPI_BLOCK_ERASE_21, + SPI_BLOCK_ERASE_40, + SPI_BLOCK_ERASE_50, + SPI_BLOCK_ERASE_52, + SPI_BLOCK_ERASE_53, + SPI_BLOCK_ERASE_5C, + SPI_BLOCK_ERASE_60, + SPI_BLOCK_ERASE_62, + SPI_BLOCK_ERASE_81, + SPI_BLOCK_ERASE_C4, + SPI_BLOCK_ERASE_C7, + SPI_BLOCK_ERASE_D7, + SPI_BLOCK_ERASE_D8, + SPI_BLOCK_ERASE_DB, + SPI_BLOCK_ERASE_DC, + S25FL_BLOCK_ERASE, + S25FS_BLOCK_ERASE_D8, + JEDEC_SECTOR_ERASE, + JEDEC_BLOCK_ERASE, + JEDEC_CHIP_BLOCK_ERASE, + OPAQUE_ERASE, + SPI_ERASE_AT45CS_SECTOR, + SPI_ERASE_AT45DB_BLOCK, + SPI_ERASE_AT45DB_CHIP, + SPI_ERASE_AT45DB_PAGE, + SPI_ERASE_AT45DB_SECTOR, + ERASE_CHIP_28SF040, + ERASE_SECTOR_28SF040, + ERASE_BLOCK_82802AB, + ERASE_SECTOR_49LFXXXC, + STM50_SECTOR_ERASE, + EDI_CHIP_BLOCK_ERASE, +} + +#[derive(Serialize, Deserialize)] +struct BlockEraser { + eraseblocks: Option<Vec<EraseBlock>>, + block_erase: BlockEraseFunc, +} + +#[derive(Serialize, Deserialize)] +struct Voltage { + min: u16, + max: u16, +} + +#[derive(Serialize, Deserialize, Debug)] +enum FlashReg { + INVALID_REG = 0, + STATUS1, + STATUS2, + STATUS3, + SECURITY, + CONFIG, + MAX_REGISTERS, +} + +#[derive(Serialize, Deserialize, Debug)] +enum Writability { + RO, + RW, + OTP, +} + +#[derive(Serialize, Deserialize, Debug)] +struct RegBitInfo { + reg: FlashReg, + bit_index: u8, + writability: Writability, +} + +#[derive(Serialize, Deserialize, Debug)] +struct RegBitMap { + srp: Option<RegBitInfo>, + srl: Option<RegBitInfo>, + bp: Vec<RegBitInfo>, + tb: Option<RegBitInfo>, + sec: Option<RegBitInfo>, + cmp: Option<RegBitInfo>, + wps: Option<RegBitInfo>, +} + +#[derive(Serialize, Deserialize)] +enum DecodeRangeFunc { + NO_DECODE_RANGE_FUNC = 0, /* 0 indicates no range decode function is set. */ + DECODE_RANGE_SPI25 = 1, + DECODE_RANGE_SPI25_64K_BLOCK = 2, + DECODE_RANGE_SPI25_BIT_CMP = 3, + DECODE_RANGE_SPI25_2X_BLOCK = 4, +} + +#[derive(Serialize, Deserialize)] +struct Tested { + probe: u32, // enum test_state probe; + read: u32, // enum test_state read; + erase: u32, // enum test_state erase; + write: u32, // enum test_state write; + wp: u32, // enum test_state wp; +} + +#[derive(Serialize, Deserialize)] +enum ProbeFunc { + NO_PROBE_FUNC = 0, /* 0 indicates no probe function set. */ + PROBE_JEDEC = 1, + PROBE_JEDEC_29GL, + PROBE_OPAQUE, + PROBE_EDI_KB9012, + PROBE_AT82802AB, + PROBE_W29EE011, + PROBE_EN29LV640B, + PROBE_SPI_AT25F, + PROBE_SPI_AT45DB, + PROBE_SPI_BIG_SPANSION, + PROBE_SPI_RDID, + PROBE_SPI_RDID4, + PROBE_SPI_REMS, + PROBE_SPI_RES1, + PROBE_SPI_RES2, + PROBE_SPI_SFDP, + PROBE_SPI_ST95, +} + +#[derive(Serialize, Deserialize)] +enum WriteFunc { + NO_WRITE_FUNC = 0, /* 0 indicates no write function set. */ + WRITE_JEDEC = 1, + WRITE_JEDEC1, + WRITE_OPAQUE, + SPI_CHIP_WRITE1, + SPI_CHIP_WRITE256, + SPI_WRITE_AAI, + SPI_WRITE_AT45DB, + WRITE_28SF040, + WRITE_82802AB, + WRITE_EN29LV640B, + EDI_CHIP_WRITE, +} + +#[derive(Serialize, Deserialize, Debug)] // XXX +enum ReadFunc { + NO_READ_FUNC = 0, /* 0 indicates no read function set. */ + SPI_CHIP_READ = 1, + READ_OPAQUE, + READ_MEMMAPPED, + EDI_CHIP_READ, + SPI_READ_AT45DB, + SPI_READ_AT45DB_E8, +} + +#[derive(Serialize, Deserialize)] +enum SpiCmdSet { + SPI25 = 0, + SPI_EDI = 1, +} + +#[derive(Serialize, Deserialize)] +struct FlashChip { + vendor: String, + name: String, + + bustype: String, // XXX Vec<ChipBusType>, + + manufacture_id: u32, + model_id: u32, + + total_size: u32, + page_size: u32, + + feature_bits: u32, + + tested: Option<Tested>, + + spi_cmd_set: SpiCmdSet, + + probe: ProbeFunc, + + probe_timing: i32, + + block_erasers: Vec<BlockEraser>, + + printlock: Option<u32>, // Obsolete printlock_func + unlock: Option<u32>, // Obsolete blockprotect_func + + write: WriteFunc, + read: ReadFunc, + + voltage: Voltage, + + gran: WriteGranularity, + + reg_bits: Option<RegBitMap>, + + decode_range: Option<DecodeRangeFunc>, +} + +pub fn flashchips(json_data: &str) -> Result<()> { + //let v: Value = serde_json::from_str(json_data)?; + let m: HashMap<String, Vec<FlashChip>> = serde_json::from_str(json_data)?; + let v: &Vec<FlashChip> = &m["flashchips"]; + + for fc in v { + println!("{}", fc.name); +// println!("read func {:#?}", fc.read); + } + + Ok(()) +} -- To view, visit https://review.coreboot.org/c/flashrom/+/74166 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I48346a3da5923e614728a1cc0567ed2bb1ba2e2f Gerrit-Change-Number: 74166 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

[M] Change in flashrom[master]: tree/: Case write_granularity enum values
by Edward O'Callaghan (Code Review) 02 Apr '23

02 Apr '23

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/74165 ) Change subject: tree/: Case write_granularity enum values ...................................................................... tree/: Case write_granularity enum values Change-Id: Ic8c655225abe477c1b618dc685b743e691c16ebd Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M at45db.c M flashchips.c M flashrom.c M include/flash.h M nicintel_eeprom.c 5 files changed, 51 insertions(+), 41 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/65/74165/1 diff --git a/at45db.c b/at45db.c index 7877330..6d66a6e 100644 --- a/at45db.c +++ b/at45db.c @@ -200,12 +200,12 @@ } switch (chip->page_size) { - case 256: chip->gran = write_gran_256bytes; break; - case 264: chip->gran = write_gran_264bytes; break; - case 512: chip->gran = write_gran_512bytes; break; - case 528: chip->gran = write_gran_528bytes; break; - case 1024: chip->gran = write_gran_1024bytes; break; - case 1056: chip->gran = write_gran_1056bytes; break; + case 256: chip->gran = WRITE_GRAN_256BYTES; break; + case 264: chip->gran = WRITE_GRAN_264BYTES; break; + case 512: chip->gran = WRITE_GRAN_512BYTES; break; + case 528: chip->gran = WRITE_GRAN_528BYTES; break; + case 1024: chip->gran = WRITE_GRAN_1024BYTES; break; + case 1056: chip->gran = WRITE_GRAN_1056BYTES; break; default: msg_cerr("%s: unknown page size %d.\n", __func__, chip->page_size); return 0; diff --git a/flashchips.c b/flashchips.c index dc6e882..6a5cf49 100644 --- a/flashchips.c +++ b/flashchips.c @@ -2762,7 +2762,7 @@ .write = SPI_WRITE_AT45DB, .read = SPI_READ_AT45DB, .voltage = {2700, 3600}, - .gran = write_gran_1056bytes, + .gran = WRITE_GRAN_1056BYTES, }, { @@ -3015,7 +3015,7 @@ .write = SPI_WRITE_AT45DB, .read = SPI_READ_AT45DB_E8, /* 3 address and 4 dummy bytes */ .voltage = {2700, 3600}, - .gran = write_gran_528bytes, + .gran = WRITE_GRAN_528BYTES, }, { @@ -3638,7 +3638,7 @@ .write = EDI_CHIP_WRITE, .read = EDI_CHIP_READ, .voltage = {2700, 3600}, - .gran = write_gran_128bytes, + .gran = WRITE_GRAN_128BYTES, }, { diff --git a/flashrom.c b/flashrom.c index b135e58..cee96d6 100644 --- a/flashrom.c +++ b/flashrom.c @@ -723,42 +723,42 @@ unsigned int i; switch (gran) { - case write_gran_1bit: + case WRITE_GRAN_1BIT: for (i = 0; i < len; i++) if ((have[i] & want[i]) != want[i]) { result = 1; break; } break; - case write_gran_1byte: + case WRITE_GRAN_1BYTE: for (i = 0; i < len; i++) if ((have[i] != want[i]) && (have[i] != erased_value)) { result = 1; break; } break; - case write_gran_128bytes: + case WRITE_GRAN_128BYTES: result = need_erase_gran_bytes(have, want, len, 128, erased_value); break; - case write_gran_256bytes: + case WRITE_GRAN_256BYTES: result = need_erase_gran_bytes(have, want, len, 256, erased_value); break; - case write_gran_264bytes: + case WRITE_GRAN_264BYTES: result = need_erase_gran_bytes(have, want, len, 264, erased_value); break; - case write_gran_512bytes: + case WRITE_GRAN_512BYTES: result = need_erase_gran_bytes(have, want, len, 512, erased_value); break; - case write_gran_528bytes: + case WRITE_GRAN_528BYTES: result = need_erase_gran_bytes(have, want, len, 528, erased_value); break; - case write_gran_1024bytes: + case WRITE_GRAN_1024BYTES: result = need_erase_gran_bytes(have, want, len, 1024, erased_value); break; - case write_gran_1056bytes: + case WRITE_GRAN_1056BYTES: result = need_erase_gran_bytes(have, want, len, 1056, erased_value); break; - case write_gran_1byte_implicit_erase: + case WRITE_GRAN_1BYTE_IMPLICIT_ERASE: /* Do not erase, handle content changes from anything->0xff by writing 0xff. */ result = 0; break; @@ -801,30 +801,30 @@ unsigned int i, limit, stride; switch (gran) { - case write_gran_1bit: - case write_gran_1byte: - case write_gran_1byte_implicit_erase: + case WRITE_GRAN_1BIT: + case WRITE_GRAN_1BYTE: + case WRITE_GRAN_1BYTE_IMPLICIT_ERASE: stride = 1; break; - case write_gran_128bytes: + case WRITE_GRAN_128BYTES: stride = 128; break; - case write_gran_256bytes: + case WRITE_GRAN_256BYTES: stride = 256; break; - case write_gran_264bytes: + case WRITE_GRAN_264BYTES: stride = 264; break; - case write_gran_512bytes: + case WRITE_GRAN_512BYTES: stride = 512; break; - case write_gran_528bytes: + case WRITE_GRAN_528BYTES: stride = 528; break; - case write_gran_1024bytes: + case WRITE_GRAN_1024BYTES: stride = 1024; break; - case write_gran_1056bytes: + case WRITE_GRAN_1056BYTES: stride = 1056; break; default: diff --git a/include/flash.h b/include/flash.h index f0357cb..3e9c885 100644 --- a/include/flash.h +++ b/include/flash.h @@ -83,17 +83,17 @@ */ enum write_granularity { /* We assume 256 byte granularity by default. */ - write_gran_256bytes = 0,/* If less than 256 bytes are written, the unwritten bytes are undefined. */ - write_gran_1bit, /* Each bit can be cleared individually. */ - write_gran_1byte, /* A byte can be written once. Further writes to an already written byte cause + WRITE_GRAN_256BYTES = 0,/* If less than 256 bytes are written, the unwritten bytes are undefined. */ + WRITE_GRAN_1BIT, /* Each bit can be cleared individually. */ + WRITE_GRAN_1BYTE, /* A byte can be written once. Further writes to an already written byte cause * its contents to be either undefined or to stay unchanged. */ - write_gran_128bytes, /* If less than 128 bytes are written, the unwritten bytes are undefined. */ - write_gran_264bytes, /* If less than 264 bytes are written, the unwritten bytes are undefined. */ - write_gran_512bytes, /* If less than 512 bytes are written, the unwritten bytes are undefined. */ - write_gran_528bytes, /* If less than 528 bytes are written, the unwritten bytes are undefined. */ - write_gran_1024bytes, /* If less than 1024 bytes are written, the unwritten bytes are undefined. */ - write_gran_1056bytes, /* If less than 1056 bytes are written, the unwritten bytes are undefined. */ - write_gran_1byte_implicit_erase, /* EEPROMs and other chips with implicit erase and 1-byte writes. */ + WRITE_GRAN_128BYTES, /* If less than 128 bytes are written, the unwritten bytes are undefined. */ + WRITE_GRAN_264BYTES, /* If less than 264 bytes are written, the unwritten bytes are undefined. */ + WRITE_GRAN_512BYTES, /* If less than 512 bytes are written, the unwritten bytes are undefined. */ + WRITE_GRAN_528BYTES, /* If less than 528 bytes are written, the unwritten bytes are undefined. */ + WRITE_GRAN_1024BYTES, /* If less than 1024 bytes are written, the unwritten bytes are undefined. */ + WRITE_GRAN_1056BYTES, /* If less than 1056 bytes are written, the unwritten bytes are undefined. */ + WRITE_GRAN_1BYTE_IMPLICIT_ERASE, /* EEPROMs and other chips with implicit erase and 1-byte writes. */ }; /* diff --git a/nicintel_eeprom.c b/nicintel_eeprom.c index 80ccd88..2c351da 100644 --- a/nicintel_eeprom.c +++ b/nicintel_eeprom.c @@ -116,7 +116,7 @@ flash->chip->total_size = 4; flash->chip->page_size = flash->chip->total_size * 1024; flash->chip->tested = TEST_OK_PREWB; - flash->chip->gran = write_gran_1byte_implicit_erase; + flash->chip->gran = WRITE_GRAN_1BYTE_IMPLICIT_ERASE; flash->chip->block_erasers->eraseblocks[0].size = flash->chip->page_size; flash->chip->block_erasers->eraseblocks[0].count = 1; @@ -147,7 +147,7 @@ flash->chip->page_size = EE_PAGE_MASK + 1; flash->chip->tested = TEST_OK_PREWB; - flash->chip->gran = write_gran_1byte_implicit_erase; + flash->chip->gran = WRITE_GRAN_1BYTE_IMPLICIT_ERASE; flash->chip->block_erasers->eraseblocks[0].size = (EE_PAGE_MASK + 1); flash->chip->block_erasers->eraseblocks[0].count = (flash->chip->total_size * 1024) / (EE_PAGE_MASK + 1); -- To view, visit https://review.coreboot.org/c/flashrom/+/74165 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic8c655225abe477c1b618dc685b743e691c16ebd Gerrit-Change-Number: 74165 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

[XL] Change in flashrom[master]: data/flashchips.json: Add JSON flashchips db variant
by Edward O'Callaghan (Code Review) 02 Apr '23

02 Apr '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/+/74071 to look at the new patch set (#4). Change subject: data/flashchips.json: Add JSON flashchips db variant ...................................................................... data/flashchips.json: Add JSON flashchips db variant Generated from flashchips.c at commit 73e47091. Change-Id: I7fceac8a285c8ecf3f3a09a90cc1ae369e8a9429 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- A data/flashchips.json 1 file changed, 176,109 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/71/74071/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/74071 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7fceac8a285c8ecf3f3a09a90cc1ae369e8a9429 Gerrit-Change-Number: 74071 Gerrit-PatchSet: 4 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

[M] Change in flashrom[master]: util/flashchips_db_jsoniser: Fix bugs
by Edward O'Callaghan (Code Review) 02 Apr '23

02 Apr '23

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/74083 to look at the new patch set (#2). Change subject: util/flashchips_db_jsoniser: Fix bugs ...................................................................... util/flashchips_db_jsoniser: Fix bugs Also add write granularity support. One bug spotted by Stefan. Spotted-by: Stefan Reinauer <reinauer(a)google.com> Change-Id: Ie514f512b5299290a253a56e950c30127e55ca84 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M util/flashchips_db_jsoniser/main.c 1 file changed, 45 insertions(+), 13 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/83/74083/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/74083 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie514f512b5299290a253a56e950c30127e55ca84 Gerrit-Change-Number: 74083 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

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

02 Apr '23

Attention is currently required from: Sam McNally, 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 5: (1 comment) File internal.c: https://review.coreboot.org/c/flashrom/+/73456/comment/f71b4a57_e8d297e0 PS3, Line 108: struct > const Done -- 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: 5 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Sam McNally <sammc(a)google.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sun, 02 Apr 2023 08:44:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Sam McNally <sammc(a)google.com> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: board_enable.c: Invert branch for readability
by Edward O'Callaghan (Code Review) 02 Apr '23

02 Apr '23

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/74164 ) Change subject: board_enable.c: Invert branch for readability ...................................................................... board_enable.c: Invert branch for readability Just use a early return for a base condition of the function to aid in removing one layer of logic embedding within a branch. Change-Id: I9bd96d4d077270697c329d7e6cac1d32ed513ed3 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M board_enable.c 1 file changed, 23 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/64/74164/1 diff --git a/board_enable.c b/board_enable.c index 795d0f3..121b211 100644 --- a/board_enable.c +++ b/board_enable.c @@ -2661,7 +2661,6 @@ bool force_boardenable) { const struct board_match *board = NULL; - int ret = 0; if (vendor && model) { board = board_match_name(vendor, model, false); @@ -2691,16 +2690,17 @@ if (board->max_rom_decode_parallel) max_rom_decode.parallel = board->max_rom_decode_parallel * 1024; - if (board->enable) { - msg_pinfo("Enabling full flash access for board \"%s %s\"... ", - board->vendor_name, board->board_name); + if (!board->enable) + return 0; - ret = board->enable(cfg); - if (ret) - msg_pinfo("FAILED!\n"); - else - msg_pinfo("OK.\n"); - } + msg_pinfo("Enabling full flash access for board \"%s %s\"... ", + board->vendor_name, board->board_name); + + int ret = board->enable(cfg); + if (ret) + msg_pinfo("FAILED!\n"); + else + msg_pinfo("OK.\n"); return ret; } -- To view, visit https://review.coreboot.org/c/flashrom/+/74164 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9bd96d4d077270697c329d7e6cac1d32ed513ed3 Gerrit-Change-Number: 74164 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

[M] Change in flashrom[master]: board_enable.c: Avoid excessive forward declarations
by Edward O'Callaghan (Code Review) 02 Apr '23

02 Apr '23

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/74163 ) Change subject: board_enable.c: Avoid excessive forward declarations ...................................................................... board_enable.c: Avoid excessive forward declarations Dispense with unnecessary C89'isms make the code dramaticaly shorter and to the point. Change-Id: I5d416a023c17568c6275426edc1edb77268ea331 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M board_enable.c 1 file changed, 85 insertions(+), 148 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/63/74163/1 diff --git a/board_enable.c b/board_enable.c index d9af44e..795d0f3 100644 --- a/board_enable.c +++ b/board_enable.c @@ -64,20 +64,16 @@ void sio_mask(uint16_t port, uint8_t reg, uint8_t data, uint8_t mask) { - uint8_t tmp; - OUTB(reg, port); - tmp = INB(port + 1) & ~mask; + uint8_t tmp = INB(port + 1) & ~mask; OUTB(tmp | (data & mask), port + 1); } /* Winbond W83697 documentation indicates that the index register has to be written for each access. */ static void sio_mask_alzheimer(uint16_t port, uint8_t reg, uint8_t data, uint8_t mask) { - uint8_t tmp; - OUTB(reg, port); - tmp = INB(port + 1) & ~mask; + uint8_t tmp = INB(port + 1) & ~mask; OUTB(reg, port); OUTB(tmp | (data & mask), port + 1); } @@ -116,10 +112,8 @@ */ static int fdc37b787_gpio50_raise(uint16_t port) { - uint8_t id, val; - OUTB(0x55, port); /* enter conf mode */ - id = sio_read(port, 0x20); + uint8_t id = sio_read(port, 0x20); if (id != 0x44) { msg_perr("\nERROR: FDC37B787: Wrong ID 0x%02X.\n", id); OUTB(0xAA, port); /* leave conf mode */ @@ -128,9 +122,8 @@ sio_write(port, 0x07, 0x08); /* Select Aux I/O subdevice */ - val = sio_read(port, 0xC8); /* GP50 */ - if ((val & 0x1B) != 0x10) /* output, no invert, GPIO */ - { + uint8_t val = sio_read(port, 0xC8); /* GP50 */ + if ((val & 0x1B) != 0x10) { /* output, no invert, GPIO */ msg_perr("\nERROR: GPIO50 mode 0x%02X unexpected.\n", val); OUTB(0xAA, port); return -1; @@ -252,17 +245,13 @@ */ static uint8_t w836xx_deviceid_hwmon(uint16_t sio_port) { - uint16_t hwmport; - uint16_t hwm_vendorid; - uint8_t hwm_deviceid; - sio_write(sio_port, 0x07, 0x0b); /* Select LDN 0xb (HWM). */ if ((sio_read(sio_port, 0x30) & (1 << 0)) != (1 << 0)) { msg_pinfo("W836xx hardware monitor disabled or does not exist.\n"); return 0; } /* Get HWM base address (stored in LDN 0xb, index 0x60/0x61). */ - hwmport = sio_read(sio_port, 0x60) << 8; + uint16_t hwmport = sio_read(sio_port, 0x60) << 8; hwmport |= sio_read(sio_port, 0x61); /* HWM address register = HWM base address + 5. */ hwmport += 5; @@ -275,7 +264,7 @@ /* Set HBACS=1. */ sio_mask_alzheimer(hwmport, 0x4e, 0x80, 0x80); /* Read upper byte of vendor ID. */ - hwm_vendorid = sio_read(hwmport, 0x4f) << 8; + uint16_t hwm_vendorid = sio_read(hwmport, 0x4f) << 8; /* Set HBACS=0. */ sio_mask_alzheimer(hwmport, 0x4e, 0x00, 0x80); /* Read lower byte of vendor ID. */ @@ -288,24 +277,22 @@ /* Set Bank=0. */ sio_mask_alzheimer(hwmport, 0x4e, 0x00, 0x07); /* Read "chip" ID. We call this one the device ID. */ - hwm_deviceid = sio_read(hwmport, 0x58); + uint8_t hwm_deviceid = sio_read(hwmport, 0x58); return hwm_deviceid; } void probe_superio_winbond(void) { - struct superio s = {0}; uint16_t winbond_ports[] = {WINBOND_SUPERIO_PORT1, WINBOND_SUPERIO_PORT2, 0}; - uint16_t *i = winbond_ports; - uint8_t model; uint8_t tmp; + struct superio s = {0}; s.vendor = SUPERIO_VENDOR_WINBOND; - for (; *i; i++) { + for (uint16_t *i = winbond_ports; *i; i++) { s.port = *i; /* If we're already in Super I/O config more, the W836xx enter sequence won't hurt. */ w836xx_ext_enter(s.port); - model = sio_read(s.port, 0x20); + uint8_t model = sio_read(s.port, 0x20); /* No response, no point leaving the config mode. */ if (model == 0xff) continue; @@ -373,18 +360,14 @@ } w836xx_ext_leave(s.port); } - return; } static const struct winbond_chip *winbond_superio_chipdef(void) { - int i; - unsigned int j; - - for (i = 0; i < superio_count; i++) { + for (int i = 0; i < superio_count; i++) { if (superios[i].vendor != SUPERIO_VENDOR_WINBOND) continue; - for (j = 0; j < ARRAY_SIZE(winbond_chips); j++) + for (unsigned int j = 0; j < ARRAY_SIZE(winbond_chips); j++) if (winbond_chips[j].device_id == superios[i].model) return &winbond_chips[j]; } @@ -399,12 +382,10 @@ static int winbond_gpio_set(uint16_t base, enum winbond_id chipid, int pin, int raise) { - const struct winbond_chip *chip = NULL; - const struct winbond_port *gpio; int port = pin / 10; int bit = pin % 10; - chip = winbond_superio_chipdef(); + const struct winbond_chip *chip = winbond_superio_chipdef(); if (!chip) { msg_perr("\nERROR: No supported Winbond Super I/O found\n"); return -1; @@ -420,7 +401,7 @@ return -1; } - gpio = &chip->port[port - 1]; + const struct winbond_port *gpio = &chip->port[port - 1]; if (gpio->ldn == 0) { msg_perr("\nERROR: GPIO%d is not supported yet on this" @@ -603,14 +584,13 @@ */ int it8705f_write_enable(uint8_t port) { - uint8_t tmp; int ret = 0; if (!(internal_buses_supported & BUS_PARALLEL)) return 1; enter_conf_mode_ite(port); - tmp = sio_read(port, 0x24); + uint8_t tmp = sio_read(port, 0x24); /* Check if at least one flash segment is enabled. */ if (tmp & 0xf0) { /* The IT8705F will respond to LPC cycles and translate them. */ @@ -697,12 +677,10 @@ */ static int it8707f_write_enable(uint8_t port) { - uint8_t tmp; - w836xx_ext_enter(port); /* Set bit 3 (GLB_REG_WE) of reg 0x23: Makes reg 0x24-0x2A rw */ - tmp = sio_read(port, 0x23); + uint8_t tmp = sio_read(port, 0x23); tmp |= (1 << 3); sio_write(port, 0x23, tmp); @@ -738,15 +716,13 @@ static const int bankbase[] = {0, 4, 8, 10, 12}; int gpio_bank = gpio / 8; int gpio_pin = gpio % 8; - uint16_t baseport; - uint8_t id, val; if (gpio_bank > 4) { msg_perr("PC8736x: Invalid GPIO %d\n", gpio); return -1; } - id = sio_read(0x2E, 0x20); + uint8_t id = sio_read(0x2E, 0x20); if (id != chipid) { msg_perr("PC8736x: unexpected ID %02x (expected %02x)\n", id, chipid); @@ -754,7 +730,7 @@ } sio_write(0x2E, 0x07, 0x07); /* Select GPIO device. */ - baseport = (sio_read(0x2E, 0x60) << 8) | sio_read(0x2E, 0x61); + uint16_t baseport = (sio_read(0x2E, 0x60) << 8) | sio_read(0x2E, 0x61); if ((baseport & 0xFFF0) == 0xFFF0 || baseport == 0) { msg_perr("PC87360: invalid GPIO base address %04x\n", baseport); @@ -764,7 +740,7 @@ sio_write(0x2E, 0xF0, gpio_bank * 16 + gpio_pin); sio_mask (0x2E, 0xF1, 0x01, 0x01); /* Make pin output. */ - val = INB(baseport + bankbase[gpio_bank]); + uint8_t val = INB(baseport + bankbase[gpio_bank]); if (raise) val |= 1 << gpio_pin; else @@ -779,11 +755,9 @@ */ static int via_vt823x_gpio_set(uint8_t gpio, int raise) { - struct pci_dev *dev; - uint16_t base; - uint8_t val, bit, offset; + uint8_t val; - dev = pcidev_find_vendorclass(0x1106, 0x0601); + struct pci_dev *dev = pcidev_find_vendorclass(0x1106, 0x0601); switch (dev->device_id) { case 0x3177: /* VT8235 */ case 0x3227: /* VT8237/VT8237R */ @@ -815,10 +789,10 @@ } /* We need the I/O Base Address for this board's flash enable. */ - base = pci_read_word(dev, 0x88) & 0xff80; + uint16_t base = pci_read_word(dev, 0x88) & 0xff80; - offset = 0x4C + gpio / 8; - bit = 0x01 << (gpio % 8); + uint8_t offset = 0x4C + gpio / 8; + uint8_t bit = 0x01 << (gpio % 8); val = INB(base + offset); if (raise) @@ -870,9 +844,7 @@ */ static int board_msi_kt4v(struct board_cfg *cfg) { - int ret; - - ret = via_vt823x_gpio_set(12, 1); + int ret = via_vt823x_gpio_set(12, 1); w836xx_memw_enable(0x2E); return ret; @@ -890,17 +862,14 @@ /* * Find where the SMBus host is. ASUS sets it to 0xE800; coreboot sets it to 0x0F00. */ - struct pci_dev *dev; - uint16_t smbba; - uint8_t b; - dev = pcidev_find(0x8086, 0x7113); /* Intel PIIX4, PM/SMBus function. */ + struct pci_dev *dev = pcidev_find(0x8086, 0x7113); /* Intel PIIX4, PM/SMBus function. */ if (!dev) { msg_perr("\nERROR: Intel PIIX4 PM not found.\n"); return -1; } - smbba = pci_read_word(dev, 0x90) & 0xfff0; + uint16_t smbba = pci_read_word(dev, 0x90) & 0xfff0; OUTB(0xFF, smbba); /* Clear previous SMBus status. */ OUTB(0x48 << 1, smbba + 4); @@ -909,7 +878,7 @@ OUTB(0x48, smbba + 2); /* Wait until SMBus transaction is complete. */ - b = 0x1; + uint8_t b = 0x1; while (b & 0x01) { INB(0x80); b = INB(smbba); @@ -1043,16 +1012,13 @@ */ static int board_ecs_geforce6100sm_m(struct board_cfg *cfg) { - struct pci_dev *dev; - uint32_t tmp; - - dev = pcidev_find(0x10DE, 0x03EB); /* NVIDIA MCP61 SMBus. */ + struct pci_dev *dev = pcidev_find(0x10DE, 0x03EB); /* NVIDIA MCP61 SMBus. */ if (!dev) { msg_perr("\nERROR: NVIDIA MCP61 SMBus not found.\n"); return -1; } - tmp = pci_read_byte(dev, 0xE0); + uint32_t tmp = pci_read_byte(dev, 0xE0); tmp &= ~(1 << 3); pci_write_byte(dev, 0xE0, tmp); @@ -1064,17 +1030,13 @@ */ static int nvidia_mcp_gpio_set(int gpio, int raise) { - struct pci_dev *dev; - uint16_t base, devclass; - uint8_t tmp; - if ((gpio < 0) || (gpio >= 0x40)) { msg_perr("\nERROR: unsupported GPIO: %d.\n", gpio); return -1; } /* Check for the ISA bridge first. */ - dev = pcidev_find_vendorclass(0x10DE, 0x0601); + struct pci_dev *dev = pcidev_find_vendorclass(0x10DE, 0x0601); switch (dev->device_id) { case 0x0030: /* CK804 */ case 0x0050: /* MCP04 */ @@ -1100,7 +1062,7 @@ msg_perr("MCP SMBus controller could not be found\n"); return -1; } - devclass = pci_read_word(dev, PCI_CLASS_DEVICE); + uint16_t devclass = pci_read_word(dev, PCI_CLASS_DEVICE); if (devclass != 0x0C05) { msg_perr("Unexpected device class %04x for SMBus" " controller\n", devclass); @@ -1112,10 +1074,10 @@ return -1; } - base = pci_read_long(dev, 0x64) & 0x0000FF00; /* System control area */ + uint16_t base = pci_read_long(dev, 0x64) & 0x0000FF00; /* System control area */ base += 0xC0; - tmp = INB(base + gpio); + uint8_t tmp = INB(base + gpio); tmp &= ~0x0F; /* null lower nibble */ tmp |= 0x04; /* gpio -> output. */ if (raise) @@ -1265,26 +1227,21 @@ */ static int board_sun_ultra_40_m2(struct board_cfg *cfg) { - int ret; - uint8_t reg; - uint16_t base; - struct pci_dev *dev; - - ret = nvidia_mcp_gpio4_lower(cfg); + int ret = nvidia_mcp_gpio4_lower(cfg); if (ret) return ret; - dev = pcidev_find(0x10de, 0x0364); /* NVIDIA MCP55 LPC bridge */ + struct pci_dev *dev = pcidev_find(0x10de, 0x0364); /* NVIDIA MCP55 LPC bridge */ if (!dev) { msg_perr("\nERROR: NVIDIA MCP55 LPC bridge not found.\n"); return -1; } - base = pci_read_word(dev, 0xb4); /* some IO BAR? */ + uint16_t base = pci_read_word(dev, 0xb4); /* some IO BAR? */ if (!base) return -1; - reg = INB(base + 0x4b); + uint8_t reg = INB(base + 0x4b); reg |= 0x10; OUTB(reg, base + 0x4b); @@ -1307,14 +1264,13 @@ #define DBE6x_BOOT_LOC_FLASH 2 #define DBE6x_BOOT_LOC_FWHUB 3 - msr_t msr; unsigned long boot_loc; /* Geode only has a single core */ if (msr_setup(0)) return -1; - msr = msr_read(DBE6x_MSR_DIVIL_BALL_OPTS); + msr_t msr = msr_read(DBE6x_MSR_DIVIL_BALL_OPTS); if ((msr.lo & (DBE6x_BOOT_OP_LATCHED)) == (DBE6x_BOOT_LOC_FWHUB << DBE6x_BOOT_OP_LATCHED_SHIFT)) @@ -1341,16 +1297,13 @@ */ static int amd_sbxxx_gpio9_raise(struct board_cfg *cfg) { - struct pci_dev *dev; - uint32_t reg; - - dev = pcidev_find(0x1002, 0x4372); /* AMD SMBus controller */ + struct pci_dev *dev = pcidev_find(0x1002, 0x4372); /* AMD SMBus controller */ if (!dev) { msg_perr("\nERROR: AMD SMBus Controller (0x4372) not found.\n"); return -1; } - reg = pci_read_long(dev, 0xA8); /* GPIO_12_to_4_Cntrl CI_Reg: A8h-ABh */ + uint32_t reg = pci_read_long(dev, 0xA8); /* GPIO_12_to_4_Cntrl CI_Reg: A8h-ABh */ /* enable output (0: enable, 1: tristate): GPIO9 output enable is at bit 5 in 0xA9 */ reg &= ~((uint32_t)1<<(8+5)); @@ -1367,10 +1320,6 @@ */ static int intel_piix4_gpo_set(unsigned int gpo, int raise) { - unsigned int gpo_byte, gpo_bit; - struct pci_dev *dev; - uint32_t tmp, base; - /* GPO{0,8,27,28,30} are always available. */ static const uint32_t nonmuxed_gpos = 0x58000101; @@ -1408,7 +1357,7 @@ {0} }; - dev = pcidev_find(0x8086, 0x7110); /* Intel PIIX4 ISA bridge */ + struct pci_dev *dev = pcidev_find(0x8086, 0x7110); /* Intel PIIX4 ISA bridge */ if (!dev) { msg_perr("\nERROR: Intel PIIX4 ISA bridge not found.\n"); return -1; @@ -1434,11 +1383,11 @@ } /* PM IO base */ - base = pci_read_long(dev, 0x40) & 0x0000FFC0; + uint32_t base = pci_read_long(dev, 0x40) & 0x0000FFC0; - gpo_byte = gpo >> 3; - gpo_bit = gpo & 7; - tmp = INB(base + 0x34 + gpo_byte); /* GPO register */ + unsigned int gpo_byte = gpo >> 3; + unsigned int gpo_bit = gpo & 7; + uint32_t tmp = INB(base + 0x34 + gpo_byte); /* GPO register */ if (raise) tmp |= 0x01 << gpo_bit; else @@ -1550,13 +1499,11 @@ {0, 0, 0, 0, 0} /* end marker */ }; - struct pci_dev *dev; - uint16_t base; uint32_t tmp; int i, allowed; /* First, look for a known LPC bridge */ - dev = pcidev_find_vendorclass(0x8086, 0x0601); /* ISA bridge */ + struct pci_dev *dev = pcidev_find_vendorclass(0x8086, 0x0601); /* ISA bridge */ if (!dev) { msg_perr("\nERROR: No known Intel LPC bridge found.\n"); return -1; @@ -1576,7 +1523,7 @@ * strapped to zero. From some mobile ICH9 version on, this becomes * 6:1. The mask below catches all. */ - base = pci_read_word(dev, intel_ich_gpio_table[i].base_reg) & 0xFFC0; + uint16_t base = pci_read_word(dev, intel_ich_gpio_table[i].base_reg) & 0xFFC0; /* Check whether the line is allowed. */ if (gpio < 32) @@ -1808,8 +1755,7 @@ */ static int board_hp_vl400(struct board_cfg *cfg) { - int ret; - ret = intel_ich_gpio_set(25, 1); /* Master write enable ? */ + int ret = intel_ich_gpio_set(25, 1); /* Master write enable ? */ if (!ret) ret = pc8736x_gpio_set(PC87360_ID, 0x09, 1); /* #WP ? */ if (!ret) @@ -1902,10 +1848,8 @@ */ static int board_acorp_6a815epd(struct board_cfg *cfg) { - int ret; - /* Lower Blocks Lock -- pin 7 of PLCC32 */ - ret = intel_ich_gpio_set(22, 1); + int ret = intel_ich_gpio_set(22, 1); if (!ret) /* Top Block Lock -- pin 8 of PLCC32 */ ret = intel_ich_gpio_set(23, 1); @@ -1918,9 +1862,7 @@ */ static int board_kontron_986lcd_m(struct board_cfg *cfg) { - int ret; - - ret = intel_ich_gpio_set(34, 1); /* #TBL */ + int ret = intel_ich_gpio_set(34, 1); /* #TBL */ if (!ret) ret = intel_ich_gpio_set(35, 1); /* #WP */ @@ -1933,11 +1875,8 @@ */ static int via_apollo_gpo_set(int gpio, int raise) { - struct pci_dev *dev; - uint32_t base, tmp; - /* VT82C686 power management */ - dev = pcidev_find(0x1106, 0x3057); + struct pci_dev *dev = pcidev_find(0x1106, 0x3057); if (!dev) { msg_perr("\nERROR: VT82C686 PM device not found.\n"); return -1; @@ -1947,7 +1886,7 @@ raise ? "Rais" : "Dropp", gpio); /* Select GPO function on multiplexed pins. */ - tmp = pci_read_byte(dev, 0x54); + uint32_t tmp = pci_read_byte(dev, 0x54); switch (gpio) { case 0: tmp &= ~0x03; @@ -1965,7 +1904,7 @@ pci_write_byte(dev, 0x54, tmp); /* PM IO base */ - base = pci_read_long(dev, 0x48) & 0x0000FF00; + uint32_t base = pci_read_long(dev, 0x48) & 0x0000FF00; /* Drop GPO0 */ tmp = INL(base + 0x4C); @@ -2007,17 +1946,14 @@ */ static int sis_gpio0_raise_and_w836xx_memw(struct board_cfg *cfg) { - struct pci_dev *dev; - uint16_t base, temp; - - dev = pcidev_find(0x1039, 0x0962); + struct pci_dev *dev = pcidev_find(0x1039, 0x0962); if (!dev) { msg_perr("Expected south bridge not found\n"); return 1; } - base = pci_read_word(dev, 0x74); - temp = INW(base + 0x68); + uint16_t base = pci_read_word(dev, 0x74); + uint16_t temp = INW(base + 0x68); temp &= ~(1 << 0); /* Make pin output? */ OUTW(temp, base + 0x68); @@ -2070,22 +2006,18 @@ */ static int board_mitac_6513wu(struct board_cfg *cfg) { - struct pci_dev *dev; - uint16_t rt_port; - uint8_t val; - - dev = pcidev_find(0x8086, 0x2410); /* Intel 82801AA ISA bridge */ + struct pci_dev *dev = pcidev_find(0x8086, 0x2410); /* Intel 82801AA ISA bridge */ if (!dev) { msg_perr("\nERROR: Intel 82801AA ISA bridge not found.\n"); return -1; } - rt_port = smsc_find_runtime(0x4e, 0x54 /* LPC47U33x */, 0xa); + uint16_t rt_port = smsc_find_runtime(0x4e, 0x54 /* LPC47U33x */, 0xa); if (rt_port == 0) return -1; /* Configure the GPIO pin. */ - val = INB(rt_port + 0x33); /* GP30 config */ + uint8_t val = INB(rt_port + 0x33); /* GP30 config */ val &= ~0x87; /* Output, non-inverted, GPIO, push/pull */ OUTB(val, rt_port + 0x33); @@ -2103,10 +2035,8 @@ */ static int board_abit_av8(struct board_cfg *cfg) { - uint8_t val; - /* Raise GPO pins GP22 & GP23 */ - val = INB(0x404E); + uint8_t val = INB(0x404E); val |= 0xC0; OUTB(val, 0x404E); @@ -2120,12 +2050,9 @@ */ static int it8703f_gpio51_raise(struct board_cfg *cfg) { - uint16_t id, base; - uint8_t tmp; - /* Find the IT8703F. */ w836xx_ext_enter(0x2E); - id = (sio_read(0x2E, 0x20) << 8) | sio_read(0x2E, 0x21); + uint16_t id = (sio_read(0x2E, 0x20) << 8) | sio_read(0x2E, 0x21); w836xx_ext_leave(0x2E); if (id != 0x8701) { @@ -2136,7 +2063,7 @@ /* Get the GP567 I/O base. */ w836xx_ext_enter(0x2E); sio_write(0x2E, 0x07, 0x0C); - base = (sio_read(0x2E, 0x60) << 8) | sio_read(0x2E, 0x61); + uint16_t base = (sio_read(0x2E, 0x60) << 8) | sio_read(0x2E, 0x61); w836xx_ext_leave(0x2E); if (!base) { @@ -2146,7 +2073,7 @@ } /* Raise GP51. */ - tmp = INB(base); + uint8_t tmp = INB(base); tmp |= 0x02; OUTB(tmp, base); @@ -2160,8 +2087,6 @@ { int allowed, sio; unsigned int port; - uint16_t base, sioport; - uint8_t tmp; /* IT87 GPIO configuration table */ static const struct it87cfg { @@ -2179,19 +2104,19 @@ /* Find the Super I/O in the probed list */ for (sio = 0; sio < superio_count; sio++) { - int i; if (superios[sio].vendor != SUPERIO_VENDOR_ITE) continue; /* Is this device in our list? */ - for (i = 0; it87_gpio_table[i].id; i++) + for (int i = 0; it87_gpio_table[i].id; i++) { if (superios[sio].model == it87_gpio_table[i].id) { cfg = &it87_gpio_table[i]; goto found; } + } } - if (cfg == NULL) { + if (!cfg) { msg_perr("\nERROR: No IT87 Super I/O GPIO configuration " "found.\n"); return -1; @@ -2215,10 +2140,10 @@ } /* Read the Simple I/O Base Address Register */ - sioport = superios[sio].port; + uint16_t sioport = superios[sio].port; enter_conf_mode_ite(sioport); sio_write(sioport, 0x07, 0x07); - base = (sio_read(sioport, cfg->base_reg) << 8) | + uint16_t base = (sio_read(sioport, cfg->base_reg) << 8) | sio_read(sioport, cfg->base_reg + 1); exit_conf_mode_ite(sioport); @@ -2233,7 +2158,7 @@ gpio %= 10; /* set GPIO. */ - tmp = INB(base + port); + uint8_t tmp = INB(base + port); if (raise) tmp |= 1 << gpio; else @@ -2534,8 +2459,7 @@ } int ret = 0; - unsigned int i; - for (i = 0; i + 1 < ARRAY_SIZE(board_matches); i++) { + for (unsigned int i = 0; i + 1 < ARRAY_SIZE(board_matches); i++) { const struct board_match *b = &board_matches[i]; if (b->vendor_name == NULL || b->board_name == NULL) { msg_gerr("ERROR: Board enable #%d does not define a vendor and board name.\n" -- To view, visit https://review.coreboot.org/c/flashrom/+/74163 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5d416a023c17568c6275426edc1edb77268ea331 Gerrit-Change-Number: 74163 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) 02 Apr '23

02 Apr '23

Attention is currently required from: Thomas Heijligen, Edward O'Callaghan, Angel Pons. Hello Sam McNally, 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 (#5). 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/5 -- 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: 5 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Sam McNally <sammc(a)google.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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit April 2023