- flashrom-gerrit - mail.coreboot.org

Change in flashrom[stable]: ichspi: Add support for Intel Skylake
by Nico Huber (Code Review) 30 Sep '17

30 Sep '17

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/21806 Change subject: ichspi: Add support for Intel Skylake ...................................................................... ichspi: Add support for Intel Skylake The Sunrise Point PCH, paired with Skylake, has some minor changes in the HW sequencing interface: * Support for more flash regions moved PR* registers * Only 4KiB erase blocks are supported by the primary erase command * A second erase command for 64KiB pages was added * More commands were added for status register access etc. * A "Dedicated Lock Bits" register was added No support for the new commands was added. The SW sequencing interface seems to have moved register location and is not supported any more officially. It's also untested. Changes are loosely based on the Skylake support commit in Chromium OS by Ramya Vijaykumar: commit a9a64f9e4d52c39fcd3c5f7d7b88065baed189b1 Author: Ramya Vijaykumar <ramya.vijaykumar(a)intel.com> flashrom: Add Skylake platform support Original-Change-Id: I0f4565a3c39f5fe3aec4fc8863605cebed1ad4ee Original-Reviewed-on: https://review.coreboot.org/18962 Original-Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Original-Reviewed-by: David Hendricks <david.hendricks(a)gmail.com> Original-Reviewed-by: Youness Alaoui <snifikino(a)gmail.com> Change-Id: Ifc5a5fc9ad22a8151e66850ef70c5cf406a83a37 Signed-off-by: Nico Huber <nico.huber(a)secunet.com> --- M ich_descriptors.c M ich_descriptors.h M ichspi.c 3 files changed, 233 insertions(+), 87 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/06/21806/1 diff --git a/ich_descriptors.c b/ich_descriptors.c index a12022c..1fc8835 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -824,6 +824,7 @@ case CHIPSET_8_SERIES_WELLSBURG: case CHIPSET_9_SERIES_WILDCAT_POINT: case CHIPSET_9_SERIES_WILDCAT_POINT_LP: + case CHIPSET_100_SERIES_SUNRISE_POINT: if (idx == 0) { size_enc = desc->component.dens_new.comp1_density; } else { @@ -849,16 +850,22 @@ /* Only used by ichspi.c */ #if CONFIG_INTERNAL == 1 && (defined(__i386__) || defined(__x86_64__)) -static uint32_t read_descriptor_reg(uint8_t section, uint16_t offset, void *spibar) +static uint32_t read_descriptor_reg(enum ich_chipset cs, uint8_t section, uint16_t offset, void *spibar) { uint32_t control = 0; control |= (section << FDOC_FDSS_OFF) & FDOC_FDSS; control |= (offset << FDOC_FDSI_OFF) & FDOC_FDSI; - mmio_le_writel(control, spibar + ICH9_REG_FDOC); - return mmio_le_readl(spibar + ICH9_REG_FDOD); + if (cs == CHIPSET_100_SERIES_SUNRISE_POINT) { + mmio_le_writel(control, spibar + PCH100_REG_FDOC); + return mmio_le_readl(spibar + PCH100_REG_FDOD); + } else { + mmio_le_writel(control, spibar + ICH9_REG_FDOC); + return mmio_le_readl(spibar + ICH9_REG_FDOD); + } + } -int read_ich_descriptors_via_fdo(void *spibar, struct ich_descriptors *desc) +int read_ich_descriptors_via_fdo(enum ich_chipset cs, void *spibar, struct ich_descriptors *desc) { uint8_t i; uint8_t nr; @@ -888,15 +895,15 @@ msg_pdbg2("Reading flash descriptors mapped by the chipset via FDOC/FDOD..."); /* content section */ - desc->content.FLVALSIG = read_descriptor_reg(0, 0, spibar); - desc->content.FLMAP0 = read_descriptor_reg(0, 1, spibar); - desc->content.FLMAP1 = read_descriptor_reg(0, 2, spibar); - desc->content.FLMAP2 = read_descriptor_reg(0, 3, spibar); + desc->content.FLVALSIG = read_descriptor_reg(cs, 0, 0, spibar); + desc->content.FLMAP0 = read_descriptor_reg(cs, 0, 1, spibar); + desc->content.FLMAP1 = read_descriptor_reg(cs, 0, 2, spibar); + desc->content.FLMAP2 = read_descriptor_reg(cs, 0, 3, spibar); /* component section */ - desc->component.FLCOMP = read_descriptor_reg(1, 0, spibar); - desc->component.FLILL = read_descriptor_reg(1, 1, spibar); - desc->component.FLPB = read_descriptor_reg(1, 2, spibar); + desc->component.FLCOMP = read_descriptor_reg(cs, 1, 0, spibar); + desc->component.FLILL = read_descriptor_reg(cs, 1, 1, spibar); + desc->component.FLPB = read_descriptor_reg(cs, 1, 2, spibar); /* region section */ nr = desc->content.NR + 1; @@ -906,12 +913,12 @@ return ICH_RET_ERR; } for (i = 0; i < 5; i++) - desc->region.FLREGs[i] = read_descriptor_reg(2, i, spibar); + desc->region.FLREGs[i] = read_descriptor_reg(cs, 2, i, spibar); /* master section */ - desc->master.FLMSTR1 = read_descriptor_reg(3, 0, spibar); - desc->master.FLMSTR2 = read_descriptor_reg(3, 1, spibar); - desc->master.FLMSTR3 = read_descriptor_reg(3, 2, spibar); + desc->master.FLMSTR1 = read_descriptor_reg(cs, 3, 0, spibar); + desc->master.FLMSTR2 = read_descriptor_reg(cs, 3, 1, spibar); + desc->master.FLMSTR3 = read_descriptor_reg(cs, 3, 2, spibar); /* Accessing the strap section via FDOC/D is only possible on ICH8 and * reading the upper map is impossible on all chipsets, so don't bother. diff --git a/ich_descriptors.h b/ich_descriptors.h index ecf44bf..831341a 100644 --- a/ich_descriptors.h +++ b/ich_descriptors.h @@ -33,6 +33,7 @@ #define ICH_RET_OOB -4 #define ICH9_REG_FDOC 0xB0 /* 32 Bits Flash Descriptor Observability Control */ +#define PCH100_REG_FDOC 0xB4 /* New offset from Sunrise Point on */ /* 0-1: reserved */ #define FDOC_FDSI_OFF 2 /* 2-11: Flash Descriptor Section Index */ #define FDOC_FDSI (0x3f << FDOC_FDSI_OFF) @@ -41,6 +42,7 @@ /* 15-31: reserved */ #define ICH9_REG_FDOD 0xB4 /* 32 Bits Flash Descriptor Observability Data */ +#define PCH100_REG_FDOD 0xB8 /* New offset from Sunrise Point on */ /* Field locations and semantics for LVSCC, UVSCC and related words in the flash * descriptor are equal therefore they all share the same macros below. */ @@ -581,7 +583,7 @@ void prettyprint_ich_descriptor_straps(enum ich_chipset cs, const struct ich_descriptors *desc); int read_ich_descriptors_from_dump(const uint32_t *dump, unsigned int len, struct ich_descriptors *desc); -int read_ich_descriptors_via_fdo(void *spibar, struct ich_descriptors *desc); +int read_ich_descriptors_via_fdo(enum ich_chipset cs, void *spibar, struct ich_descriptors *desc); int getFCBA_component_density(enum ich_chipset cs, const struct ich_descriptors *desc, uint8_t idx); int layout_from_ich_descriptors(struct ich_layout *, const void *dump, size_t len); diff --git a/ichspi.c b/ichspi.c index be13f91..ecf0c42 100644 --- a/ichspi.c +++ b/ichspi.c @@ -33,6 +33,60 @@ #include "spi.h" #include "ich_descriptors.h" +/* Sunrise Point */ + +/* Added HSFS Status bits */ +#define HSFS_WRSDIS_OFF 11 /* 11: Flash Configuration Lock-Down */ +#define HSFS_WRSDIS (0x1 << HSFS_WRSDIS_OFF) +#define HSFS_PRR34_LOCKDN_OFF 12 /* 12: PRR3 PRR4 Lock-Down */ +#define HSFS_PRR34_LOCKDN (0x1 << HSFS_PRR34_LOCKDN_OFF) +/* HSFS_BERASE vanished */ + +/* + * HSFC and HSFS 16-bit registers are combined into the 32-bit + * BIOS_HSFSTS_CTL register in the Sunrise Point datasheet, + * however we still treat them separately in order to reuse code. + */ + +/* Changed HSFC Control bits */ +#define PCH100_HSFC_FCYCLE_OFF (17 - 16) /* 1-4: FLASH Cycle */ +#define PCH100_HSFC_FCYCLE (0xf << PCH100_HSFC_FCYCLE_OFF) +/* New HSFC Control bit */ +#define HSFC_WET_OFF (21 - 16) /* 5: Write Enable Type */ +#define HSFC_WET (0x1 << HSFC_WET_OFF) + +#define PCH100_FADDR_FLA 0x07ffffff + +#define PCH100_REG_DLOCK 0x0c /* 32 Bits Discrete Lock Bits */ +#define DLOCK_BMWAG_LOCKDN_OFF 0 +#define DLOCK_BMWAG_LOCKDN (0x1 << DLOCK_BMWAG_LOCKDN_OFF) +#define DLOCK_BMRAG_LOCKDN_OFF 1 +#define DLOCK_BMRAG_LOCKDN (0x1 << DLOCK_BMRAG_LOCKDN_OFF) +#define DLOCK_SBMWAG_LOCKDN_OFF 2 +#define DLOCK_SBMWAG_LOCKDN (0x1 << DLOCK_SBMWAG_LOCKDN_OFF) +#define DLOCK_SBMRAG_LOCKDN_OFF 3 +#define DLOCK_SBMRAG_LOCKDN (0x1 << DLOCK_SBMRAG_LOCKDN_OFF) +#define DLOCK_PR0_LOCKDN_OFF 8 +#define DLOCK_PR0_LOCKDN (0x1 << DLOCK_PR0_LOCKDN_OFF) +#define DLOCK_PR1_LOCKDN_OFF 9 +#define DLOCK_PR1_LOCKDN (0x1 << DLOCK_PR1_LOCKDN_OFF) +#define DLOCK_PR2_LOCKDN_OFF 10 +#define DLOCK_PR2_LOCKDN (0x1 << DLOCK_PR2_LOCKDN_OFF) +#define DLOCK_PR3_LOCKDN_OFF 11 +#define DLOCK_PR3_LOCKDN (0x1 << DLOCK_PR3_LOCKDN_OFF) +#define DLOCK_PR4_LOCKDN_OFF 12 +#define DLOCK_PR4_LOCKDN (0x1 << DLOCK_PR4_LOCKDN_OFF) +#define DLOCK_SSEQ_LOCKDN_OFF 16 +#define DLOCK_SSEQ_LOCKDN (0x1 << DLOCK_SSEQ_LOCKDN_OFF) + +#define PCH100_REG_FPR0 0x84 /* 32 Bits Protected Range 0 */ +#define PCH100_REG_GPR0 0x98 /* 32 Bits Global Protected Range 0 */ + +#define PCH100_REG_SSFSC 0xA0 /* 32 Bits Status (8) + Control (24) */ +#define PCH100_REG_PREOP 0xA4 /* 16 Bits */ +#define PCH100_REG_OPTYPE 0xA6 /* 16 Bits */ +#define PCH100_REG_OPMENU 0xA8 /* 64 Bits */ + /* ICH9 controller register definition */ #define ICH9_REG_HSFS 0x04 /* 16 Bits Hardware Sequencing Flash Status */ #define HSFS_FDONE_OFF 0 /* 0: Flash Cycle Done */ @@ -66,6 +120,7 @@ #define HSFC_SME (0x1 << HSFC_SME_OFF) #define ICH9_REG_FADDR 0x08 /* 32 Bits */ +#define ICH9_FADDR_FLA 0x01ffffff #define ICH9_REG_FDATA0 0x10 /* 64 Bytes */ #define ICH9_REG_FRAP 0x50 /* 32 Bytes Flash Region Access Permissions */ @@ -325,7 +380,8 @@ ops->preop[1]); } -#define pprint_reg(reg, bit, val, sep) msg_pdbg("%s=%d" sep, #bit, (val & reg##_##bit)>>reg##_##bit##_OFF) +#define _pprint_reg(bit, mask, off, val, sep) msg_pdbg("%s=%d" sep, #bit, (val & mask) >> off) +#define pprint_reg(reg, bit, val, sep) _pprint_reg(bit, reg##_##bit, reg##_##bit##_OFF, val, sep) static void prettyprint_ich9_reg_hsfs(uint16_t reg_val) { @@ -333,8 +389,14 @@ pprint_reg(HSFS, FDONE, reg_val, ", "); pprint_reg(HSFS, FCERR, reg_val, ", "); pprint_reg(HSFS, AEL, reg_val, ", "); - pprint_reg(HSFS, BERASE, reg_val, ", "); + if (ich_generation != CHIPSET_100_SERIES_SUNRISE_POINT) { + pprint_reg(HSFS, BERASE, reg_val, ", "); + } pprint_reg(HSFS, SCIP, reg_val, ", "); + if (ich_generation == CHIPSET_100_SERIES_SUNRISE_POINT) { + pprint_reg(HSFS, PRR34_LOCKDN, reg_val, ", "); + pprint_reg(HSFS, WRSDIS, reg_val, ", "); + } pprint_reg(HSFS, FDOPSS, reg_val, ", "); pprint_reg(HSFS, FDV, reg_val, ", "); pprint_reg(HSFS, FLOCKDN, reg_val, "\n"); @@ -344,7 +406,12 @@ { msg_pdbg("HSFC: "); pprint_reg(HSFC, FGO, reg_val, ", "); - pprint_reg(HSFC, FCYCLE, reg_val, ", "); + if (ich_generation != CHIPSET_100_SERIES_SUNRISE_POINT) { + pprint_reg(HSFC, FCYCLE, reg_val, ", "); + } else { + _pprint_reg(HSFC, PCH100_HSFC_FCYCLE, PCH100_HSFC_FCYCLE_OFF, reg_val, ", "); + pprint_reg(HSFC, WET, reg_val, ", "); + } pprint_reg(HSFC, FDBC, reg_val, ", "); pprint_reg(HSFC, SME, reg_val, "\n"); } @@ -369,6 +436,28 @@ pprint_reg(SSFC, SME, reg_val, ", "); pprint_reg(SSFC, SCF, reg_val, "\n"); } + +static void prettyprint_pch100_reg_dlock(const uint32_t reg_val) +{ + msg_pdbg("DLOCK: "); + pprint_reg(DLOCK, BMWAG_LOCKDN, reg_val, ", "); + pprint_reg(DLOCK, BMRAG_LOCKDN, reg_val, ", "); + pprint_reg(DLOCK, SBMWAG_LOCKDN, reg_val, ", "); + pprint_reg(DLOCK, SBMRAG_LOCKDN, reg_val, ",\n "); + pprint_reg(DLOCK, PR0_LOCKDN, reg_val, ", "); + pprint_reg(DLOCK, PR1_LOCKDN, reg_val, ", "); + pprint_reg(DLOCK, PR2_LOCKDN, reg_val, ", "); + pprint_reg(DLOCK, PR3_LOCKDN, reg_val, ", "); + pprint_reg(DLOCK, PR4_LOCKDN, reg_val, ",\n "); + pprint_reg(DLOCK, SSEQ_LOCKDN, reg_val, "\n"); +} + +static struct { + size_t reg_ssfsc; + size_t reg_preop; + size_t reg_optype; + size_t reg_opmenu; +} swseq_data; static uint8_t lookup_spi_type(uint8_t opcode) { @@ -475,10 +564,10 @@ break; case CHIPSET_ICH8: default: /* Future version might behave the same */ - preop = REGREAD16(ICH9_REG_PREOP); - optype = REGREAD16(ICH9_REG_OPTYPE); - opmenu[0] = REGREAD32(ICH9_REG_OPMENU); - opmenu[1] = REGREAD32(ICH9_REG_OPMENU + 4); + preop = REGREAD16(swseq_data.reg_preop); + optype = REGREAD16(swseq_data.reg_optype); + opmenu[0] = REGREAD32(swseq_data.reg_opmenu); + opmenu[1] = REGREAD32(swseq_data.reg_opmenu + 4); break; } @@ -558,15 +647,15 @@ default: /* Future version might behave the same */ /* Register undo only for enable_undo=1, i.e. first call. */ if (enable_undo) { - rmmio_valw(ich_spibar + ICH9_REG_PREOP); - rmmio_valw(ich_spibar + ICH9_REG_OPTYPE); - rmmio_vall(ich_spibar + ICH9_REG_OPMENU); - rmmio_vall(ich_spibar + ICH9_REG_OPMENU + 4); + rmmio_valw(ich_spibar + swseq_data.reg_preop); + rmmio_valw(ich_spibar + swseq_data.reg_optype); + rmmio_vall(ich_spibar + swseq_data.reg_opmenu); + rmmio_vall(ich_spibar + swseq_data.reg_opmenu + 4); } - mmio_writew(preop, ich_spibar + ICH9_REG_PREOP); - mmio_writew(optype, ich_spibar + ICH9_REG_OPTYPE); - mmio_writel(opmenu[0], ich_spibar + ICH9_REG_OPMENU); - mmio_writel(opmenu[1], ich_spibar + ICH9_REG_OPMENU + 4); + mmio_writew(preop, ich_spibar + swseq_data.reg_preop); + mmio_writew(optype, ich_spibar + swseq_data.reg_optype); + mmio_writel(opmenu[0], ich_spibar + swseq_data.reg_opmenu); + mmio_writel(opmenu[1], ich_spibar + swseq_data.reg_opmenu + 4); break; } @@ -853,7 +942,7 @@ } timeout = 100 * 60; /* 60 ms are 9.6 million cycles at 16 MHz. */ - while ((REGREAD8(ICH9_REG_SSFS) & SSFS_SCIP) && --timeout) { + while ((REGREAD8(swseq_data.reg_ssfsc) & SSFS_SCIP) && --timeout) { programmer_delay(10); } if (!timeout) { @@ -871,12 +960,12 @@ ich_fill_data(data, datalength, ICH9_REG_FDATA0); /* Assemble SSFS + SSFC */ - temp32 = REGREAD32(ICH9_REG_SSFS); + temp32 = REGREAD32(swseq_data.reg_ssfsc); /* Keep reserved bits only */ temp32 &= SSFS_RESERVED_MASK | SSFC_RESERVED_MASK; /* Clear cycle done and cycle error status registers */ temp32 |= (SSFS_FDONE | SSFS_FCERR); - REGWRITE32(ICH9_REG_SSFS, temp32); + REGWRITE32(swseq_data.reg_ssfsc, temp32); /* Use 20 MHz */ temp32 |= SSFC_SCF_20MHZ; @@ -930,21 +1019,21 @@ temp32 |= SSFC_SCGO; /* write it */ - REGWRITE32(ICH9_REG_SSFS, temp32); + REGWRITE32(swseq_data.reg_ssfsc, temp32); /* Wait for Cycle Done Status or Flash Cycle Error. */ - while (((REGREAD32(ICH9_REG_SSFS) & (SSFS_FDONE | SSFS_FCERR)) == 0) && + while (((REGREAD32(swseq_data.reg_ssfsc) & (SSFS_FDONE | SSFS_FCERR)) == 0) && --timeout) { programmer_delay(10); } if (!timeout) { - msg_perr("timeout, ICH9_REG_SSFS=0x%08x\n", - REGREAD32(ICH9_REG_SSFS)); + msg_perr("timeout, REG_SSFS=0x%08x\n", + REGREAD32(swseq_data.reg_ssfsc)); return 1; } /* FIXME make sure we do not needlessly cause transaction errors. */ - temp32 = REGREAD32(ICH9_REG_SSFS); + temp32 = REGREAD32(swseq_data.reg_ssfsc); if (temp32 & SSFS_FCERR) { msg_perr("Transaction error!\n"); prettyprint_ich9_reg_ssfs(temp32); @@ -952,7 +1041,7 @@ /* keep reserved bits */ temp32 &= SSFS_RESERVED_MASK | SSFC_RESERVED_MASK; /* Clear the transaction error. */ - REGWRITE32(ICH9_REG_SSFS, temp32 | SSFS_FCERR); + REGWRITE32(swseq_data.reg_ssfsc, temp32 | SSFS_FCERR); return 1; } @@ -1117,13 +1206,16 @@ static struct hwseq_data { uint32_t size_comp0; uint32_t size_comp1; + uint32_t addr_mask; + bool only_4k; + uint32_t hsfc_fcycle; } hwseq_data; -/* Sets FLA in FADDR to (addr & 0x01FFFFFF) without touching other bits. */ +/* Sets FLA in FADDR to (addr & hwseq_data.addr_mask) without touching other bits. */ static void ich_hwseq_set_addr(uint32_t addr) { - uint32_t addr_old = REGREAD32(ICH9_REG_FADDR) & ~0x01FFFFFF; - REGWRITE32(ICH9_REG_FADDR, (addr & 0x01FFFFFF) | addr_old); + uint32_t addr_old = REGREAD32(ICH9_REG_FADDR) & ~hwseq_data.addr_mask; + REGWRITE32(ICH9_REG_FADDR, (addr & hwseq_data.addr_mask) | addr_old); } /* Sets FADDR.FLA to 'addr' and returns the erase block size in bytes @@ -1135,18 +1227,21 @@ */ static uint32_t ich_hwseq_get_erase_block_size(unsigned int addr) { - uint8_t enc_berase; - static const uint32_t dec_berase[4] = { - 256, - 4 * 1024, - 8 * 1024, - 64 * 1024 - }; + if (hwseq_data.only_4k) { + return 4 * 1024; + } else { + uint8_t enc_berase; + static const uint32_t dec_berase[4] = { + 256, + 4 * 1024, + 8 * 1024, + 64 * 1024 + }; - ich_hwseq_set_addr(addr); - enc_berase = (REGREAD16(ICH9_REG_HSFS) & HSFS_BERASE) >> - HSFS_BERASE_OFF; - return dec_berase[enc_berase]; + ich_hwseq_set_addr(addr); + enc_berase = (REGREAD16(ICH9_REG_HSFS) & HSFS_BERASE) >> HSFS_BERASE_OFF; + return dec_berase[enc_berase]; + } } /* Polls for Cycle Done Status, Flash Cycle Error or timeout in 8 us intervals. @@ -1167,7 +1262,7 @@ } REGWRITE16(ICH9_REG_HSFS, REGREAD16(ICH9_REG_HSFS)); if (!timeout) { - addr = REGREAD32(ICH9_REG_FADDR) & 0x01FFFFFF; + addr = REGREAD32(ICH9_REG_FADDR) & hwseq_data.addr_mask; msg_perr("Timeout error between offset 0x%08x and " "0x%08x (= 0x%08x + %d)!\n", addr, addr + len - 1, addr, len - 1); @@ -1177,7 +1272,7 @@ } if (hsfs & HSFS_FCERR) { - addr = REGREAD32(ICH9_REG_FADDR) & 0x01FFFFFF; + addr = REGREAD32(ICH9_REG_FADDR) & hwseq_data.addr_mask; msg_perr("Transaction error between offset 0x%08x and " "0x%08x (= 0x%08x + %d)!\n", addr, addr + len - 1, addr, len - 1); @@ -1205,7 +1300,10 @@ flash->chip->total_size = total_size / 1024; eraser = &(flash->chip->block_erasers[0]); - boundary = (REGREAD32(ICH9_REG_FPB) & FPB_FPBA) << 12; + if (!hwseq_data.only_4k) + boundary = (REGREAD32(ICH9_REG_FPB) & FPB_FPBA) << 12; + else + boundary = 0; size_high = total_size - boundary; erase_size_high = ich_hwseq_get_erase_block_size(boundary); @@ -1279,7 +1377,7 @@ REGWRITE16(ICH9_REG_HSFS, REGREAD16(ICH9_REG_HSFS)); hsfc = REGREAD16(ICH9_REG_HSFC); - hsfc &= ~HSFC_FCYCLE; /* clear operation */ + hsfc &= ~hwseq_data.hsfc_fcycle; /* clear operation */ hsfc |= (0x3 << HSFC_FCYCLE_OFF); /* set erase operation */ hsfc |= HSFC_FGO; /* start */ msg_pdbg("HSFC used for block erasing: "); @@ -1316,7 +1414,7 @@ ich_hwseq_set_addr(addr); hsfc = REGREAD16(ICH9_REG_HSFC); - hsfc &= ~HSFC_FCYCLE; /* set read operation */ + hsfc &= ~hwseq_data.hsfc_fcycle; /* set read operation */ hsfc &= ~HSFC_FDBC; /* clear byte count */ /* set byte count */ hsfc |= (((block_len - 1) << HSFC_FDBC_OFF) & HSFC_FDBC); @@ -1357,7 +1455,7 @@ block_len = min(block_len, 256 - (addr & 0xFF)); ich_fill_data(buf, block_len, ICH9_REG_FDATA0); hsfc = REGREAD16(ICH9_REG_HSFC); - hsfc &= ~HSFC_FCYCLE; /* clear operation */ + hsfc &= ~hwseq_data.hsfc_fcycle; /* clear operation */ hsfc |= (0x2 << HSFC_FCYCLE_OFF); /* set write operation */ hsfc &= ~HSFC_FDBC; /* clear byte count */ /* set byte count */ @@ -1449,6 +1547,8 @@ "Flash Descriptor", "BIOS", "Management Engine", "Gigabit Ethernet", "Platform Data" }; + const char *const region_name = i < ARRAY_SIZE(region_names) ? region_names[i] : "unknown"; + uint32_t base, limit; int rwperms = (((ICH_BRWA(frap) >> i) & 1) << 1) | (((ICH_BRRA(frap) >> i) & 1) << 0); @@ -1460,19 +1560,19 @@ if (base > limit || (freg == 0 && i > 0)) { /* this FREG is disabled */ msg_pdbg2("0x%02X: 0x%08x FREG%i: %s region is unused.\n", - offset, freg, i, region_names[i]); + offset, freg, i, region_name); return 0; } msg_pdbg("0x%02X: 0x%08x ", offset, freg); if (rwperms == 0x3) { msg_pdbg("FREG%i: %s region (0x%08x-0x%08x) is %s.\n", i, - region_names[i], base, (limit | 0x0fff), + region_name, base, (limit | 0x0fff), access_names[rwperms]); return 0; } msg_pwarn("FREG%i: Warning: %s region (0x%08x-0x%08x) is %s.\n", i, - region_names[i], base, (limit | 0x0fff), + region_name, base, (limit | 0x0fff), access_names[rwperms]); return 1; } @@ -1487,31 +1587,36 @@ ((~((pr) >> PR_WP_OFF) & 1) << 1)) /* returns 0 if range is unused (i.e. r/w) */ -static int ich9_handle_pr(int i) +static int ich9_handle_pr(const size_t reg_pr0, int i) { static const char *const access_names[3] = { "locked", "read-only", "write-only" }; - uint8_t off = ICH9_REG_PR0 + (i * 4); + uint8_t off = reg_pr0 + (i * 4); uint32_t pr = mmio_readl(ich_spibar + off); unsigned int rwperms = ICH_PR_PERMS(pr); + /* From 5 on we have GPR registers and start from 0 again. */ + const char *const prefix = i >= 5 ? "G" : ""; + if (i >= 5) + i -= 5; + if (rwperms == 0x3) { - msg_pdbg2("0x%02X: 0x%08x (PR%u is unused)\n", off, pr, i); + msg_pdbg2("0x%02X: 0x%08x (%sPR%u is unused)\n", off, pr, prefix, i); return 0; } msg_pdbg("0x%02X: 0x%08x ", off, pr); - msg_pwarn("PR%u: Warning: 0x%08x-0x%08x is %s.\n", i, ICH_FREG_BASE(pr), + msg_pwarn("%sPR%u: Warning: 0x%08x-0x%08x is %s.\n", prefix, i, ICH_FREG_BASE(pr), ICH_FREG_LIMIT(pr) | 0x0fff, access_names[rwperms]); return 1; } /* Set/Clear the read and write protection enable bits of PR register @i * according to @read_prot and @write_prot. */ -static void ich9_set_pr(int i, int read_prot, int write_prot) +static void ich9_set_pr(const size_t reg_pr0, int i, int read_prot, int write_prot) { - void *addr = ich_spibar + ICH9_REG_PR0 + (i * 4); + void *addr = ich_spibar + reg_pr0 + (i * 4); uint32_t old = mmio_readl(addr); uint32_t new; @@ -1576,9 +1681,35 @@ ich_hwseq, ich_swseq } ich_spi_mode = ich_auto; + size_t num_freg, num_pr, reg_pr0; ich_generation = ich_gen; ich_spibar = spibar; + + /* Moving registers / bits */ + if (ich_generation == CHIPSET_100_SERIES_SUNRISE_POINT) { + num_freg = 10; + num_pr = 6; + 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; + } else { + num_freg = 5; + 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; + } switch (ich_generation) { case CHIPSET_ICH7: @@ -1679,6 +1810,12 @@ tmp = mmio_readl(ich_spibar + ICH9_REG_FADDR); msg_pdbg2("0x08: 0x%08x (FADDR)\n", tmp); + if (ich_gen == CHIPSET_100_SERIES_SUNRISE_POINT) { + const uint32_t dlock = mmio_readl(ich_spibar + PCH100_REG_DLOCK); + msg_pdbg("0x0c: 0x%08x (DLOCK)\n", dlock); + prettyprint_pch100_reg_dlock(dlock); + } + if (desc_valid) { tmp = mmio_readl(ich_spibar + ICH9_REG_FRAP); msg_pdbg("0x50: 0x%08x (FRAP)\n", tmp); @@ -1688,7 +1825,7 @@ msg_pdbg("BRRA 0x%02x\n", ICH_BRRA(tmp)); /* Handle FREGx and FRAP registers */ - for (i = 0; i < 5; i++) + for (i = 0; i < num_freg; i++) ich_spi_rw_restricted |= ich9_handle_frap(tmp, i); if (ich_spi_rw_restricted) msg_pwarn("Not all flash regions are freely accessible by flashrom. This is " @@ -1697,11 +1834,11 @@ } /* Handle PR registers */ - for (i = 0; i < 5; i++) { + for (i = 0; i < num_pr; i++) { /* if not locked down try to disable PR locks first */ if (!ichspi_lock) - ich9_set_pr(i, 0, 0); - ich_spi_rw_restricted |= ich9_handle_pr(i); + ich9_set_pr(reg_pr0, i, 0, 0); + ich_spi_rw_restricted |= ich9_handle_pr(reg_pr0, i); } if (ich_spi_rw_restricted) { @@ -1716,30 +1853,30 @@ msg_pinfo("Continuing with write support because the user forced us to!\n"); } - tmp = mmio_readl(ich_spibar + ICH9_REG_SSFS); - msg_pdbg("0x90: 0x%02x (SSFS)\n", tmp & 0xff); + tmp = mmio_readl(ich_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, ich_spibar + ICH9_REG_SSFS); + mmio_writeb(SSFS_FCERR, ich_spibar + swseq_data.reg_ssfsc); } - msg_pdbg("0x91: 0x%06x (SSFC)\n", tmp >> 8); + msg_pdbg("0x%zx: 0x%06x (SSFC)\n", swseq_data.reg_ssfsc + 1, tmp >> 8); prettyprint_ich9_reg_ssfc(tmp); - msg_pdbg("0x94: 0x%04x (PREOP)\n", - mmio_readw(ich_spibar + ICH9_REG_PREOP)); - msg_pdbg("0x96: 0x%04x (OPTYPE)\n", - mmio_readw(ich_spibar + ICH9_REG_OPTYPE)); - msg_pdbg("0x98: 0x%08x (OPMENU)\n", - mmio_readl(ich_spibar + ICH9_REG_OPMENU)); - msg_pdbg("0x9C: 0x%08x (OPMENU+4)\n", - mmio_readl(ich_spibar + ICH9_REG_OPMENU + 4)); + msg_pdbg("0x%zx: 0x%04x (PREOP)\n", + swseq_data.reg_preop, mmio_readw(ich_spibar + swseq_data.reg_preop)); + msg_pdbg("0x%zx: 0x%04x (OPTYPE)\n", + swseq_data.reg_optype, mmio_readw(ich_spibar + swseq_data.reg_optype)); + msg_pdbg("0x%zx: 0x%08x (OPMENU)\n", + swseq_data.reg_opmenu, mmio_readl(ich_spibar + swseq_data.reg_opmenu)); + msg_pdbg("0x%zx: 0x%08x (OPMENU+4)\n", + swseq_data.reg_opmenu + 4, mmio_readl(ich_spibar + swseq_data.reg_opmenu + 4)); if (ich_generation == CHIPSET_ICH8 && desc_valid) { tmp = mmio_readl(ich_spibar + ICH8_REG_VSCC); msg_pdbg("0xC1: 0x%08x (VSCC)\n", tmp); msg_pdbg("VSCC: "); prettyprint_ich_reg_vscc(tmp, FLASHROM_MSG_DEBUG, true); - } else { + } else if (ich_generation != CHIPSET_100_SERIES_SUNRISE_POINT) { if (ich_generation != CHIPSET_BAYTRAIL && desc_valid) { ichspi_bbar = mmio_readl(ich_spibar + ICH9_REG_BBAR); msg_pdbg("0xA0: 0x%08x (BBAR)\n", @@ -1764,7 +1901,7 @@ } if (desc_valid) { - if (read_ich_descriptors_via_fdo(ich_spibar, &desc) == ICH_RET_OK) + if (read_ich_descriptors_via_fdo(ich_gen, ich_spibar, &desc) == ICH_RET_OK) prettyprint_ich_descriptors(ich_gen, &desc); /* If the descriptor is valid and indicates multiple -- To view, visit https://review.coreboot.org/21806 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-MessageType: newchange Gerrit-Change-Id: Ifc5a5fc9ad22a8151e66850ef70c5cf406a83a37 Gerrit-Change-Number: 21806 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>

1 0

Change in flashrom[stable]: chipset_enable: Add support for Intel Skylake / Kabylake
by Nico Huber (Code Review) 30 Sep '17

30 Sep '17

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/21805 Change subject: chipset_enable: Add support for Intel Skylake / Kabylake ...................................................................... chipset_enable: Add support for Intel Skylake / Kabylake All publicly known Skylake / Kabylake / Sunrise Point PCH variants share the same register interface [1..6]. Although all SPI configu- ration is now done through the SPI PCI device 1f.5, we can't probe for it directly since its PCI vendor and device IDs are usually hid- den. To work around the hidden IDs, we use another PCI accessor that doesn't rely on the OS seeing the PCI device. This handles SPI flashes only. While booting from LPC is still sup- ported, it seems nobody uses it any more. Some additional PCI IDs were gathered from driveridentifier.com. TEST=Compiled with B150 set to NT (instead of BAD) and checked for sane register readings. [1] 6th Generation Intel® Core(TM) Processor Families I/O Platform Datasheet - Volume 1 of 2 Revision 002EN Document Number 332995 [2] 6th Generation Intel® Processor I/O Datasheet for U/Y Platforms Volume 2 of 2 Revision 001EN Document Number 332996 [3] 7th Generation Intel® Processor Families I/O Platform Datasheet - Volume 1 of 2 Revision 002 Document Number 334658 [4] 7th Generation Intel® Processor Families I/O for U/Y Platforms Datasheet - Volume 2 of 2 Revision 002 Document Number 334659 [5] Intel® 100 Series and Intel® C230 Series Chipset Family Platform Controller Hub (PCH) Datasheet - Volume 1 of 2 Revision 004EN Document Number 332690 [6] Intel® 100 Series Chipset Family Platform Controller Hub (PCH) Datasheet - Volume 2 of 2 Revision 001EN Document Number 332691 Original-Change-Id: I000819aff25fbe9764f33df85f040093b82cd948 Original-Reviewed-on: https://review.coreboot.org/18925 Original-Reviewed-by: David Hendricks <david.hendricks(a)gmail.com> Original-Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Original-Reviewed-by: Youness Alaoui <snifikino(a)gmail.com> Change-Id: I29ff1546c6236d948c6136ea2acca5017c4f284f Signed-off-by: Nico Huber <nico.huber(a)secunet.com> --- M chipset_enable.c M programmer.h 2 files changed, 111 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/05/21805/1 diff --git a/chipset_enable.c b/chipset_enable.c index 83c470f..a85ae21 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -7,6 +7,8 @@ * Copyright (C) 2007,2008,2009 Carl-Daniel Hailfinger * Copyright (C) 2009 Kontron Modular Computers GmbH * Copyright (C) 2011, 2012 Stefan Tauner + * Copyright (C) 2017 secunet Security Networks AG + * (Written by Nico Huber <nico.huber(a)secunet.com> for secunet) * * 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 @@ -588,21 +590,32 @@ const uint8_t *const rcrb) { uint32_t gcs; - bool top_swap; + const char *reg_name; + bool bild, top_swap; switch (ich_generation) { case CHIPSET_BAYTRAIL: + reg_name = "GCS"; gcs = mmio_readl(rcrb + 0); + bild = gcs & 1; top_swap = (gcs & 2) >> 1; break; + case CHIPSET_100_SERIES_SUNRISE_POINT: + reg_name = "BIOS_SPI_BC"; + gcs = pci_read_long(dev, 0xdc); + bild = (gcs >> 7) & 1; + top_swap = (gcs >> 4) & 1; + break; default: + reg_name = "GCS"; gcs = mmio_readl(rcrb + 0x3410); + bild = gcs & 1; top_swap = mmio_readb(rcrb + 0x3414) & 1; break; } - msg_pdbg("GCS = 0x%x: ", gcs); - msg_pdbg("BIOS Interface Lock-Down: %sabled, ", (gcs & 0x1) ? "en" : "dis"); + msg_pdbg("%s = 0x%x: ", reg_name, gcs); + msg_pdbg("BIOS Interface Lock-Down: %sabled, ", bild ? "en" : "dis"); static const char *const straps_names_EP80579[] = { "SPI", "reserved", "reserved", "LPC" }; static const char *const straps_names_ich7_nm10[] = { "reserved", "SPI", "PCI", "LPC" }; @@ -644,6 +657,7 @@ break; case CHIPSET_8_SERIES_LYNX_POINT_LP: case CHIPSET_9_SERIES_WILDCAT_POINT_LP: + case CHIPSET_100_SERIES_SUNRISE_POINT: straps_names = straps_names_pch8_lp; break; case CHIPSET_8_SERIES_WELLSBURG: // FIXME: check datasheet @@ -665,6 +679,9 @@ case CHIPSET_9_SERIES_WILDCAT_POINT_LP: /* LP PCHs use a single bit for BBS */ bbs = (gcs >> 10) & 0x1; + break; + case CHIPSET_100_SERIES_SUNRISE_POINT: + bbs = (gcs >> 6) & 0x1; break; default: /* Other chipsets use two bits for BBS */ @@ -804,6 +821,66 @@ static int enable_flash_pch9_lp(struct pci_dev *dev, const char *name) { return enable_flash_ich_spi(dev, CHIPSET_9_SERIES_WILDCAT_POINT_LP, 0xdc); +} + +/* Sunrise Point */ +static int enable_flash_pch100_shutdown(void *const pci_acc) +{ + pci_cleanup(pci_acc); + return 0; +} + +static int enable_flash_pch100(struct pci_dev *const dev, const char *const name) +{ + const enum ich_chipset pch_generation = CHIPSET_100_SERIES_SUNRISE_POINT; + int ret = ERROR_FATAL; + + /* + * The SPI PCI device is usually hidden (by hiding PCI vendor + * and device IDs). So we need a PCI access method that works + * even when the OS doesn't know the PCI device. We can't use + * this method globally since it would bring along other con- + * straints (e.g. on PCI domains, extended PCIe config space). + */ + struct pci_access *const pci_acc = pci_alloc(); + if (!pci_acc) { + msg_perr("Can't allocate PCI accessor.\n"); + return ret; + } + pci_acc->method = PCI_ACCESS_I386_TYPE1; + pci_init(pci_acc); + register_shutdown(enable_flash_pch100_shutdown, pci_acc); + + struct pci_dev *const spi_dev = pci_get_dev(pci_acc, dev->domain, dev->bus, 0x1f, 5); + if (!spi_dev) { + msg_perr("Can't allocate PCI device.\n"); + return ret; + } + + enable_flash_ich_report_gcs(spi_dev, pch_generation, NULL); + + const int ret_bc = enable_flash_ich_bios_cntl_config_space(spi_dev, pch_generation, 0xdc); + if (ret_bc == ERROR_FATAL) + goto _freepci_ret; + + const uint32_t phys_spibar = pci_read_long(spi_dev, PCI_BASE_ADDRESS_0) & 0xfffff000; + void *const spibar = rphysmap("SPIBAR", phys_spibar, 0x1000); + if (spibar == ERROR_PTR) + goto _freepci_ret; + msg_pdbg("SPIBAR = 0x%0*" PRIxPTR " (phys = 0x%08x)\n", PRIxPTR_WIDTH, (uintptr_t)spibar, phys_spibar); + + /* This adds BUS_SPI */ + const int ret_spi = ich_init_spi(spibar, pch_generation); + if (ret_spi != ERROR_FATAL) { + if (ret_bc || ret_spi) + ret = ERROR_NONFATAL; + else + ret = 0; + } + +_freepci_ret: + pci_free_dev(spi_dev); + return ret; } /* Silvermont architecture: Bay Trail(-T/-I), Avoton/Rangeley. @@ -1803,10 +1880,36 @@ {0x8086, 0x9cc7, NT, "Intel", "Broadwell Y Premium", enable_flash_pch9_lp}, {0x8086, 0x9cc9, NT, "Intel", "Broadwell Y Base", enable_flash_pch9_lp}, {0x8086, 0x9ccb, NT, "Intel", "Broadwell H", enable_flash_pch9}, - {0x8086, 0x9d41, BAD, "Intel", "Sunrise Point (Skylake LP Sample)", NULL}, - {0x8086, 0x9d43, BAD, "Intel", "Sunrise Point (Skylake-U Base)", NULL}, - {0x8086, 0x9d48, BAD, "Intel", "Sunrise Point (Skylake-U Premium)", NULL}, - {0x8086, 0x9d46, BAD, "Intel", "Sunrise Point (Skylake-Y Premium)", NULL}, + {0x8086, 0x9d41, BAD, "Intel", "Skylake / Kaby Lake Sample", enable_flash_pch100}, + {0x8086, 0x9d43, BAD, "Intel", "Skylake U Base", enable_flash_pch100}, + {0x8086, 0x9d46, BAD, "Intel", "Skylake Y Premium", enable_flash_pch100}, + {0x8086, 0x9d48, BAD, "Intel", "Skylake U Premium", enable_flash_pch100}, + {0x8086, 0x9d4b, BAD, "Intel", "Kaby Lake Y w/ iHDCP2.2 Prem.", enable_flash_pch100}, + {0x8086, 0x9d4e, BAD, "Intel", "Kaby Lake U w/ iHDCP2.2 Prem.", enable_flash_pch100}, + {0x8086, 0x9d50, BAD, "Intel", "Kaby Lake U w/ iHDCP2.2 Base", enable_flash_pch100}, + {0x8086, 0x9d51, BAD, "Intel", "Kabe Lake w/ iHDCP2.2 Sample", enable_flash_pch100}, + {0x8086, 0x9d53, BAD, "Intel", "Kaby Lake U Base", enable_flash_pch100}, + {0x8086, 0x9d56, BAD, "Intel", "Kaby Lake Y Premium", enable_flash_pch100}, + {0x8086, 0x9d58, BAD, "Intel", "Kaby Lake U Premium", enable_flash_pch100}, + {0x8086, 0xa141, BAD, "Intel", "Sunrise Point Desktop Sample", enable_flash_pch100}, + {0x8086, 0xa142, BAD, "Intel", "Sunrise Point Unknown Sample", enable_flash_pch100}, + {0x8086, 0xa143, BAD, "Intel", "H110", enable_flash_pch100}, + {0x8086, 0xa144, BAD, "Intel", "H170", enable_flash_pch100}, + {0x8086, 0xa145, BAD, "Intel", "Z170", enable_flash_pch100}, + {0x8086, 0xa146, BAD, "Intel", "Q170", enable_flash_pch100}, + {0x8086, 0xa147, BAD, "Intel", "Q150", enable_flash_pch100}, + {0x8086, 0xa148, BAD, "Intel", "B150", enable_flash_pch100}, + {0x8086, 0xa149, BAD, "Intel", "C236", enable_flash_pch100}, + {0x8086, 0xa14a, BAD, "Intel", "C232", enable_flash_pch100}, + {0x8086, 0xa14b, BAD, "Intel", "Sunrise Point Server Sample", enable_flash_pch100}, + {0x8086, 0xa14d, BAD, "Intel", "QM170", enable_flash_pch100}, + {0x8086, 0xa14e, BAD, "Intel", "HM170", enable_flash_pch100}, + {0x8086, 0xa150, BAD, "Intel", "CM236", enable_flash_pch100}, + {0x8086, 0xa151, BAD, "Intel", "QMS180", enable_flash_pch100}, + {0x8086, 0xa152, BAD, "Intel", "HM175", enable_flash_pch100}, + {0x8086, 0xa153, BAD, "Intel", "QM175", enable_flash_pch100}, + {0x8086, 0xa154, BAD, "Intel", "CM238", enable_flash_pch100}, + {0x8086, 0xa155, BAD, "Intel", "QMU185", enable_flash_pch100}, #endif {0}, }; diff --git a/programmer.h b/programmer.h index fd9da96..ec00bd9 100644 --- a/programmer.h +++ b/programmer.h @@ -650,6 +650,7 @@ CHIPSET_8_SERIES_WELLSBURG, CHIPSET_9_SERIES_WILDCAT_POINT, CHIPSET_9_SERIES_WILDCAT_POINT_LP, + CHIPSET_100_SERIES_SUNRISE_POINT, /* also 6th/7th gen Core i/o (LP) variants */ }; /* ichspi.c */ -- To view, visit https://review.coreboot.org/21805 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-MessageType: newchange Gerrit-Change-Id: I29ff1546c6236d948c6136ea2acca5017c4f284f Gerrit-Change-Number: 21805 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>

1 0

Change in flashrom[stable]: hwaccess: Constify `addr` argument of MMIO read functions
by build bot (Jenkins) (Code Review) 30 Sep '17

30 Sep '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/21784 ) Change subject: hwaccess: Constify `addr` argument of MMIO read functions ...................................................................... Patch Set 1: Verified-1 Build Failed https://qa.coreboot.org/job/flashrom-customrules/578/ : FAILURE https://qa.coreboot.org/job/flashrom_gerrit/523/ : SUCCESS -- To view, visit https://review.coreboot.org/21784 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-MessageType: comment Gerrit-Change-Id: I9158e8a7b994fe84e71a39483db651843adcf892 Gerrit-Change-Number: 21784 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sat, 30 Sep 2017 20:48:44 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[stable]: Drop redundant `enum msglevel`
by Nico Huber (Code Review) 30 Sep '17

30 Sep '17

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/21804 Change subject: Drop redundant `enum msglevel` ...................................................................... Drop redundant `enum msglevel` Use `enum flashrom_log_level` instead to avoid further confusion. Original-Change-Id: I1895cb8f60da3abf70c9c2953f52414cd2cc10a9 Original-Reviewed-on: https://review.coreboot.org/20268 Original-Reviewed-by: Paul Menzel <paulepanter(a)users.sourceforge.net> Original-Reviewed-by: Philippe Mathieu-Daudé <f4bug(a)amsat.org> Original-Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Original-Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Change-Id: I70df0d6564b8f0ea2f597d4d5346bc1bf4de006d Signed-off-by: Nico Huber <nico.huber(a)secunet.com> --- M cli_classic.c M cli_output.c M flash.h M ichspi.c M libflashrom.c M libflashrom.h 6 files changed, 38 insertions(+), 45 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/04/21804/1 diff --git a/cli_classic.c b/cli_classic.c index 4c3db4d..441fc91 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -203,7 +203,7 @@ break; case 'V': verbose_screen++; - if (verbose_screen > MSG_DEBUG2) + if (verbose_screen > FLASHROM_MSG_DEBUG2) verbose_logfile = verbose_screen; break; case 'E': diff --git a/cli_output.c b/cli_output.c index e697985..61a9af6 100644 --- a/cli_output.c +++ b/cli_output.c @@ -25,8 +25,8 @@ #include <errno.h> #include "flash.h" -int verbose_screen = MSG_INFO; -int verbose_logfile = MSG_DEBUG2; +enum flashrom_log_level verbose_screen = FLASHROM_MSG_INFO; +enum flashrom_log_level verbose_logfile = FLASHROM_MSG_DEBUG2; #ifndef STANDALONE static FILE *logfile = NULL; @@ -61,17 +61,17 @@ void start_logging(void) { - enum msglevel oldverbose_screen = verbose_screen; + enum flashrom_log_level oldverbose_screen = verbose_screen; /* Shut up the console. */ - verbose_screen = MSG_ERROR; + verbose_screen = FLASHROM_MSG_ERROR; print_version(); verbose_screen = oldverbose_screen; } #endif /* !STANDALONE */ /* Please note that level is the verbosity, not the importance of the message. */ -int flashrom_print_cb(enum msglevel level, const char *fmt, va_list ap) +int flashrom_print_cb(enum flashrom_log_level level, const char *fmt, va_list ap) { int ret = 0; FILE *output_type = stdout; @@ -79,20 +79,20 @@ va_list logfile_args; va_copy(logfile_args, ap); - if (level < MSG_INFO) + if (level < FLASHROM_MSG_INFO) output_type = stderr; if (level <= verbose_screen) { ret = vfprintf(output_type, fmt, ap); /* msg_*spew often happens inside chip accessors in possibly * time-critical operations. Don't slow them down by flushing. */ - if (level != MSG_SPEW) + if (level != FLASHROM_MSG_SPEW) fflush(output_type); } #ifndef STANDALONE if ((level <= verbose_logfile) && logfile) { ret = vfprintf(logfile, fmt, logfile_args); - if (level != MSG_SPEW) + if (level != FLASHROM_MSG_SPEW) fflush(logfile); } #endif /* !STANDALONE */ diff --git a/flash.h b/flash.h index b5eb99b..67c7d20 100644 --- a/flash.h +++ b/flash.h @@ -38,6 +38,7 @@ #undef max #endif +#include "libflashrom.h" #include "layout.h" #define ERROR_PTR ((void*)-1) @@ -312,47 +313,39 @@ void print_chip_support_status(const struct flashchip *chip); /* cli_output.c */ -extern int verbose_screen; -extern int verbose_logfile; +extern enum flashrom_log_level verbose_screen; +extern enum flashrom_log_level verbose_logfile; #ifndef STANDALONE int open_logfile(const char * const filename); int close_logfile(void); void start_logging(void); #endif -enum msglevel { - MSG_ERROR = 0, - MSG_WARN = 1, - MSG_INFO = 2, - MSG_DEBUG = 3, - MSG_DEBUG2 = 4, - MSG_SPEW = 5, -}; -int flashrom_print_cb(enum msglevel level, const char *fmt, va_list ap); +int flashrom_print_cb(enum flashrom_log_level level, const char *fmt, va_list ap); /* Let gcc and clang check for correct printf-style format strings. */ -int print(enum msglevel level, const char *fmt, ...) +int print(enum flashrom_log_level level, const char *fmt, ...) #ifdef __MINGW32__ __attribute__((format(gnu_printf, 2, 3))); #else __attribute__((format(printf, 2, 3))); #endif -#define msg_gerr(...) print(MSG_ERROR, __VA_ARGS__) /* general errors */ -#define msg_perr(...) print(MSG_ERROR, __VA_ARGS__) /* programmer errors */ -#define msg_cerr(...) print(MSG_ERROR, __VA_ARGS__) /* chip errors */ -#define msg_gwarn(...) print(MSG_WARN, __VA_ARGS__) /* general warnings */ -#define msg_pwarn(...) print(MSG_WARN, __VA_ARGS__) /* programmer warnings */ -#define msg_cwarn(...) print(MSG_WARN, __VA_ARGS__) /* chip warnings */ -#define msg_ginfo(...) print(MSG_INFO, __VA_ARGS__) /* general info */ -#define msg_pinfo(...) print(MSG_INFO, __VA_ARGS__) /* programmer info */ -#define msg_cinfo(...) print(MSG_INFO, __VA_ARGS__) /* chip info */ -#define msg_gdbg(...) print(MSG_DEBUG, __VA_ARGS__) /* general debug */ -#define msg_pdbg(...) print(MSG_DEBUG, __VA_ARGS__) /* programmer debug */ -#define msg_cdbg(...) print(MSG_DEBUG, __VA_ARGS__) /* chip debug */ -#define msg_gdbg2(...) print(MSG_DEBUG2, __VA_ARGS__) /* general debug2 */ -#define msg_pdbg2(...) print(MSG_DEBUG2, __VA_ARGS__) /* programmer debug2 */ -#define msg_cdbg2(...) print(MSG_DEBUG2, __VA_ARGS__) /* chip debug2 */ -#define msg_gspew(...) print(MSG_SPEW, __VA_ARGS__) /* general debug spew */ -#define msg_pspew(...) print(MSG_SPEW, __VA_ARGS__) /* programmer debug spew */ -#define msg_cspew(...) print(MSG_SPEW, __VA_ARGS__) /* chip debug spew */ +#define msg_gerr(...) print(FLASHROM_MSG_ERROR, __VA_ARGS__) /* general errors */ +#define msg_perr(...) print(FLASHROM_MSG_ERROR, __VA_ARGS__) /* programmer errors */ +#define msg_cerr(...) print(FLASHROM_MSG_ERROR, __VA_ARGS__) /* chip errors */ +#define msg_gwarn(...) print(FLASHROM_MSG_WARN, __VA_ARGS__) /* general warnings */ +#define msg_pwarn(...) print(FLASHROM_MSG_WARN, __VA_ARGS__) /* programmer warnings */ +#define msg_cwarn(...) print(FLASHROM_MSG_WARN, __VA_ARGS__) /* chip warnings */ +#define msg_ginfo(...) print(FLASHROM_MSG_INFO, __VA_ARGS__) /* general info */ +#define msg_pinfo(...) print(FLASHROM_MSG_INFO, __VA_ARGS__) /* programmer info */ +#define msg_cinfo(...) print(FLASHROM_MSG_INFO, __VA_ARGS__) /* chip info */ +#define msg_gdbg(...) print(FLASHROM_MSG_DEBUG, __VA_ARGS__) /* general debug */ +#define msg_pdbg(...) print(FLASHROM_MSG_DEBUG, __VA_ARGS__) /* programmer debug */ +#define msg_cdbg(...) print(FLASHROM_MSG_DEBUG, __VA_ARGS__) /* chip debug */ +#define msg_gdbg2(...) print(FLASHROM_MSG_DEBUG2, __VA_ARGS__) /* general debug2 */ +#define msg_pdbg2(...) print(FLASHROM_MSG_DEBUG2, __VA_ARGS__) /* programmer debug2 */ +#define msg_cdbg2(...) print(FLASHROM_MSG_DEBUG2, __VA_ARGS__) /* chip debug2 */ +#define msg_gspew(...) print(FLASHROM_MSG_SPEW, __VA_ARGS__) /* general debug spew */ +#define msg_pspew(...) print(FLASHROM_MSG_SPEW, __VA_ARGS__) /* programmer debug spew */ +#define msg_cspew(...) print(FLASHROM_MSG_SPEW, __VA_ARGS__) /* chip debug spew */ /* layout.c */ int register_include_arg(char *name); diff --git a/ichspi.c b/ichspi.c index 218e3b1..be13f91 100644 --- a/ichspi.c +++ b/ichspi.c @@ -1738,7 +1738,7 @@ tmp = mmio_readl(ich_spibar + ICH8_REG_VSCC); msg_pdbg("0xC1: 0x%08x (VSCC)\n", tmp); msg_pdbg("VSCC: "); - prettyprint_ich_reg_vscc(tmp, MSG_DEBUG, true); + prettyprint_ich_reg_vscc(tmp, FLASHROM_MSG_DEBUG, true); } else { if (ich_generation != CHIPSET_BAYTRAIL && desc_valid) { ichspi_bbar = mmio_readl(ich_spibar + ICH9_REG_BBAR); @@ -1751,12 +1751,12 @@ tmp = mmio_readl(ich_spibar + ICH9_REG_LVSCC); msg_pdbg("0xC4: 0x%08x (LVSCC)\n", tmp); msg_pdbg("LVSCC: "); - prettyprint_ich_reg_vscc(tmp, MSG_DEBUG, true); + prettyprint_ich_reg_vscc(tmp, FLASHROM_MSG_DEBUG, true); tmp = mmio_readl(ich_spibar + ICH9_REG_UVSCC); msg_pdbg("0xC8: 0x%08x (UVSCC)\n", tmp); msg_pdbg("UVSCC: "); - prettyprint_ich_reg_vscc(tmp, MSG_DEBUG, false); + prettyprint_ich_reg_vscc(tmp, FLASHROM_MSG_DEBUG, false); tmp = mmio_readl(ich_spibar + ICH9_REG_FPB); msg_pdbg("0xD0: 0x%08x (FPB)\n", tmp); diff --git a/libflashrom.c b/libflashrom.c index 962e96f..6e0f42c 100644 --- a/libflashrom.c +++ b/libflashrom.c @@ -84,13 +84,13 @@ global_log_callback = log_callback; } /** @private */ -int print(const enum msglevel level, const char *const fmt, ...) +int print(const enum flashrom_log_level level, const char *const fmt, ...) { if (global_log_callback) { int ret; va_list args; va_start(args, fmt); - ret = global_log_callback((enum flashrom_log_level)level, fmt, args); + ret = global_log_callback(level, fmt, args); va_end(args); return ret; } diff --git a/libflashrom.h b/libflashrom.h index c5d972e..d3f3ded 100644 --- a/libflashrom.h +++ b/libflashrom.h @@ -27,7 +27,7 @@ int flashrom_init(int perform_selfcheck); int flashrom_shutdown(void); /** @ingroup flashrom-general */ -enum flashrom_log_level { /* This has to match enum msglevel. */ +enum flashrom_log_level { FLASHROM_MSG_ERROR = 0, FLASHROM_MSG_WARN = 1, FLASHROM_MSG_INFO = 2, -- To view, visit https://review.coreboot.org/21804 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-MessageType: newchange Gerrit-Change-Id: I70df0d6564b8f0ea2f597d4d5346bc1bf4de006d Gerrit-Change-Number: 21804 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>

1 0

Change in flashrom[stable]: ichspi: Drop `dev` parameter from init functions
by Nico Huber (Code Review) 30 Sep '17

30 Sep '17

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/21803 Change subject: ichspi: Drop `dev` parameter from init functions ...................................................................... ichspi: Drop `dev` parameter from init functions It's never used and has no clear contract (e.g. will the pointer stay valid beyond the call?). Original-Change-Id: I0d4e7cc731364e86eff214b9022b842a577f9ef4 Original-Reviewed-on: https://review.coreboot.org/19460 Original-Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Original-Reviewed-by: Philippe Mathieu-Daudé <philippe.mathieu.daude(a)gmail.com> Original-Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Change-Id: I01978e3e45332021f8967034bbaa907ef5f48d19 Signed-off-by: Nico Huber <nico.huber(a)secunet.com> --- M chipset_enable.c M ichspi.c M programmer.h 3 files changed, 8 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/03/21803/1 diff --git a/chipset_enable.c b/chipset_enable.c index 208cd3a..83c470f 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -718,7 +718,7 @@ void *spibar = rcrb + spibar_offset; /* This adds BUS_SPI */ - int ret_spi = ich_init_spi(dev, spibar, ich_generation); + int ret_spi = ich_init_spi(spibar, ich_generation); if (ret_spi == ERROR_FATAL) return ret_spi; @@ -846,7 +846,7 @@ */ enable_flash_ich_bios_cntl_memmapped(ich_generation, spibar + 0xFC); - int ret_spi = ich_init_spi(dev, spibar, ich_generation); + int ret_spi = ich_init_spi(spibar, ich_generation); if (ret_spi == ERROR_FATAL) return ret_spi; @@ -949,12 +949,12 @@ return ERROR_FATAL; } - return via_init_spi(dev, spi0_mm_base); + return via_init_spi(spi0_mm_base); } static int enable_flash_vt8237s_spi(struct pci_dev *dev, const char *name) { - return via_init_spi(dev, pci_read_long(dev, 0xbc) << 8); + return via_init_spi(pci_read_long(dev, 0xbc) << 8); } static int enable_flash_cs5530(struct pci_dev *dev, const char *name) diff --git a/ichspi.c b/ichspi.c index e1395ee..218e3b1 100644 --- a/ichspi.c +++ b/ichspi.c @@ -1561,7 +1561,7 @@ .erase = ich_hwseq_block_erase, }; -int ich_init_spi(struct pci_dev *dev, void *spibar, enum ich_chipset ich_gen) +int ich_init_spi(void *spibar, enum ich_chipset ich_gen) { int i; uint16_t tmp2; @@ -1828,7 +1828,7 @@ .write_aai = default_spi_write_aai, }; -int via_init_spi(struct pci_dev *dev, uint32_t mmio_base) +int via_init_spi(uint32_t mmio_base) { int i; diff --git a/programmer.h b/programmer.h index 9beaf98..fd9da96 100644 --- a/programmer.h +++ b/programmer.h @@ -655,8 +655,8 @@ /* ichspi.c */ #if CONFIG_INTERNAL == 1 extern uint32_t ichspi_bbar; -int ich_init_spi(struct pci_dev *dev, void *spibar, enum ich_chipset ich_generation); -int via_init_spi(struct pci_dev *dev, uint32_t mmio_base); +int ich_init_spi(void *spibar, enum ich_chipset ich_generation); +int via_init_spi(uint32_t mmio_base); /* amd_imc.c */ int amd_imc_shutdown(struct pci_dev *dev); -- To view, visit https://review.coreboot.org/21803 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-MessageType: newchange Gerrit-Change-Id: I01978e3e45332021f8967034bbaa907ef5f48d19 Gerrit-Change-Number: 21803 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>

1 0

Change in flashrom[stable]: Handle Intel Wildcat Point *LP* like Lynx Point LP
by Nico Huber (Code Review) 30 Sep '17

30 Sep '17

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/21802 Change subject: Handle Intel Wildcat Point *LP* like Lynx Point LP ...................................................................... Handle Intel Wildcat Point *LP* like Lynx Point LP The subtle difference was ignored when adding these chipsets. The integrated Wildcat Point LP PCH is documented in [1]. I'm not sure how to account for "Broadwell H" which seems not publicly documented. Maybe it's an unreleased HM9*, in which case the non-LP path should be correct. [1] Mobile 5th Generation Intel® Core(TM) Processor Family I/O, Intel® Core(TM) M Processor Family I/O, Mobile Intel® Pentium® Processor Family I/O, and Mobile Intel® Celeron® Processor Family I/O Datasheet Revision 004 Document Number: 330837 Original-Change-Id: I6b7ca3c0bde111b04ed7c745ed76d28d3d05f01c Original-Reviewed-on: https://review.coreboot.org/18883 Original-Reviewed-by: Youness Alaoui <snifikino(a)gmail.com> Original-Reviewed-by: David Hendricks <david.hendricks(a)gmail.com> Original-Reviewed-by: Philippe Mathieu-Daudé <philippe.mathieu.daude(a)gmail.com> Original-Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Change-Id: Ib68704b4a720cb4539d4dd65495ccf6fead64724 Signed-off-by: Nico Huber <nico.huber(a)secunet.com> --- M chipset_enable.c M ich_descriptors.c M programmer.h 3 files changed, 21 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/02/21802/1 diff --git a/chipset_enable.c b/chipset_enable.c index 1191a4c..208cd3a 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -643,6 +643,7 @@ straps_names = straps_names_pch89_baytrail; break; case CHIPSET_8_SERIES_LYNX_POINT_LP: + case CHIPSET_9_SERIES_WILDCAT_POINT_LP: straps_names = straps_names_pch8_lp; break; case CHIPSET_8_SERIES_WELLSBURG: // FIXME: check datasheet @@ -661,7 +662,8 @@ bbs = (gcs >> 1) & 0x1; break; case CHIPSET_8_SERIES_LYNX_POINT_LP: - /* Lynx Point LP uses a single bit for BBS */ + case CHIPSET_9_SERIES_WILDCAT_POINT_LP: + /* LP PCHs use a single bit for BBS */ bbs = (gcs >> 10) & 0x1; break; default: @@ -796,6 +798,12 @@ static int enable_flash_pch9(struct pci_dev *dev, const char *name) { return enable_flash_ich_spi(dev, CHIPSET_9_SERIES_WILDCAT_POINT, 0xdc); +} + +/* Wildcat Point LP */ +static int enable_flash_pch9_lp(struct pci_dev *dev, const char *name) +{ + return enable_flash_ich_spi(dev, CHIPSET_9_SERIES_WILDCAT_POINT_LP, 0xdc); } /* Silvermont architecture: Bay Trail(-T/-I), Avoton/Rangeley. @@ -1787,13 +1795,13 @@ {0x8086, 0x9c43, NT, "Intel", "Lynx Point LP Premium", enable_flash_pch8_lp}, {0x8086, 0x9c45, NT, "Intel", "Lynx Point LP Mainstream", enable_flash_pch8_lp}, {0x8086, 0x9c47, NT, "Intel", "Lynx Point LP Value", enable_flash_pch8_lp}, - {0x8086, 0x9cc1, NT, "Intel", "Haswell U Sample", enable_flash_pch9}, - {0x8086, 0x9cc2, NT, "Intel", "Broadwell U Sample", enable_flash_pch9}, - {0x8086, 0x9cc3, NT, "Intel", "Broadwell U Premium", enable_flash_pch9}, - {0x8086, 0x9cc5, NT, "Intel", "Broadwell U Base", enable_flash_pch9}, - {0x8086, 0x9cc6, NT, "Intel", "Broadwell Y Sample", enable_flash_pch9}, - {0x8086, 0x9cc7, NT, "Intel", "Broadwell Y Premium", enable_flash_pch9}, - {0x8086, 0x9cc9, NT, "Intel", "Broadwell Y Base", enable_flash_pch9}, + {0x8086, 0x9cc1, NT, "Intel", "Haswell U Sample", enable_flash_pch9_lp}, + {0x8086, 0x9cc2, NT, "Intel", "Broadwell U Sample", enable_flash_pch9_lp}, + {0x8086, 0x9cc3, NT, "Intel", "Broadwell U Premium", enable_flash_pch9_lp}, + {0x8086, 0x9cc5, NT, "Intel", "Broadwell U Base", enable_flash_pch9_lp}, + {0x8086, 0x9cc6, NT, "Intel", "Broadwell Y Sample", enable_flash_pch9_lp}, + {0x8086, 0x9cc7, NT, "Intel", "Broadwell Y Premium", enable_flash_pch9_lp}, + {0x8086, 0x9cc9, NT, "Intel", "Broadwell Y Base", enable_flash_pch9_lp}, {0x8086, 0x9ccb, NT, "Intel", "Broadwell H", enable_flash_pch9}, {0x8086, 0x9d41, BAD, "Intel", "Sunrise Point (Skylake LP Sample)", NULL}, {0x8086, 0x9d43, BAD, "Intel", "Sunrise Point (Skylake-U Base)", NULL}, diff --git a/ich_descriptors.c b/ich_descriptors.c index 66dbb44..a12022c 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -139,7 +139,8 @@ case CHIPSET_8_SERIES_LYNX_POINT: case CHIPSET_8_SERIES_LYNX_POINT_LP: case CHIPSET_8_SERIES_WELLSBURG: - case CHIPSET_9_SERIES_WILDCAT_POINT: { + case CHIPSET_9_SERIES_WILDCAT_POINT: + case CHIPSET_9_SERIES_WILDCAT_POINT_LP: { uint8_t size_enc; if (idx == 0) { size_enc = desc->component.dens_new.comp1_density; @@ -183,6 +184,7 @@ case CHIPSET_8_SERIES_LYNX_POINT_LP: case CHIPSET_8_SERIES_WELLSBURG: case CHIPSET_9_SERIES_WILDCAT_POINT: + case CHIPSET_9_SERIES_WILDCAT_POINT_LP: return freq_str[value]; case CHIPSET_ICH_UNKNOWN: default: @@ -821,6 +823,7 @@ case CHIPSET_8_SERIES_LYNX_POINT_LP: case CHIPSET_8_SERIES_WELLSBURG: case CHIPSET_9_SERIES_WILDCAT_POINT: + case CHIPSET_9_SERIES_WILDCAT_POINT_LP: if (idx == 0) { size_enc = desc->component.dens_new.comp1_density; } else { diff --git a/programmer.h b/programmer.h index 1a6216a..9beaf98 100644 --- a/programmer.h +++ b/programmer.h @@ -649,6 +649,7 @@ CHIPSET_8_SERIES_LYNX_POINT_LP, CHIPSET_8_SERIES_WELLSBURG, CHIPSET_9_SERIES_WILDCAT_POINT, + CHIPSET_9_SERIES_WILDCAT_POINT_LP, }; /* ichspi.c */ -- To view, visit https://review.coreboot.org/21802 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-MessageType: newchange Gerrit-Change-Id: Ib68704b4a720cb4539d4dd65495ccf6fead64724 Gerrit-Change-Number: 21802 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>

1 0

Change in flashrom[stable]: ich_descriptors: Fix more odd +1s
by Nico Huber (Code Review) 30 Sep '17

30 Sep '17

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/21801 Change subject: ich_descriptors: Fix more odd +1s ...................................................................... ich_descriptors: Fix more odd +1s +1 on everything doesn't make software greater per se. v2: o Fix another +1. o Amend style of similar (not +1 suffering) code, too. Original-Change-Id: Ifa5455c999e90ff9121aed29f542d71ac9ca2b1c Original-Reviewed-on: https://review.coreboot.org/19044 Original-Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Original-Reviewed-by: Philippe Mathieu-Daudé <philippe.mathieu.daude(a)gmail.com> Original-Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Change-Id: I218475648010430f17942224f7a805e821115139 Signed-off-by: Nico Huber <nico.huber(a)secunet.com> --- M ich_descriptors.c 1 file changed, 7 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/01/21801/1 diff --git a/ich_descriptors.c b/ich_descriptors.c index ecc2b95..66dbb44 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -611,26 +611,24 @@ unsigned int i, max_count; msg_pdbg2("=== Softstraps ===\n"); - if (sizeof(desc->north.STRPs) / 4 + 1 < desc->content.MSL) { - max_count = sizeof(desc->north.STRPs) / 4 + 1; + max_count = min(ARRAY_SIZE(desc->north.STRPs), desc->content.MSL); + if (max_count < desc->content.MSL) { msg_pdbg2("MSL (%u) is greater than the current maximum of %u entries.\n", - desc->content.MSL, max_count + 1); + desc->content.MSL, max_count); msg_pdbg2("Only the first %u entries will be printed.\n", max_count); - } else - max_count = desc->content.MSL; + } msg_pdbg2("--- North/MCH/PROC (%d entries) ---\n", max_count); for (i = 0; i < max_count; i++) msg_pdbg2("STRP%-2d = 0x%08x\n", i, desc->north.STRPs[i]); msg_pdbg2("\n"); - if (sizeof(desc->south.STRPs) / 4 < desc->content.ISL) { - max_count = sizeof(desc->south.STRPs) / 4; + max_count = min(ARRAY_SIZE(desc->south.STRPs), desc->content.ISL); + if (max_count < desc->content.ISL) { msg_pdbg2("ISL (%u) is greater than the current maximum of %u entries.\n", desc->content.ISL, max_count); msg_pdbg2("Only the first %u entries will be printed.\n", max_count); - } else - max_count = desc->content.ISL; + } msg_pdbg2("--- South/ICH/PCH (%d entries) ---\n", max_count); for (i = 0; i < max_count; i++) -- To view, visit https://review.coreboot.org/21801 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-MessageType: newchange Gerrit-Change-Id: I218475648010430f17942224f7a805e821115139 Gerrit-Change-Number: 21801 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>

1 0

Change in flashrom[stable]: dediprog: Reimplement target chip option
by build bot (Jenkins) (Code Review) 30 Sep '17

30 Sep '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/21780 ) Change subject: dediprog: Reimplement target chip option ...................................................................... Patch Set 1: Verified-1 Build Failed https://qa.coreboot.org/job/flashrom-customrules/575/ : FAILURE https://qa.coreboot.org/job/flashrom_gerrit/520/ : SUCCESS -- To view, visit https://review.coreboot.org/21780 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-MessageType: comment Gerrit-Change-Id: I94576967e2892e08295daa829caabace5a367fa0 Gerrit-Change-Number: 21780 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sat, 30 Sep 2017 20:48:37 +0000 Gerrit-HasComments: No

1 0

Change in flashrom[stable]: ich_descriptors: Fix range checks for dumps
by Nico Huber (Code Review) 30 Sep '17

30 Sep '17

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/21800 Change subject: ich_descriptors: Fix range checks for dumps ...................................................................... ich_descriptors: Fix range checks for dumps These explicit off-by-one calculations were... off-by-one. Original-Change-Id: If57c92ba28f91c4d72123ef0cfd2d9d5ac0a0656 Original-Reviewed-on: https://review.coreboot.org/19031 Original-Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Original-Reviewed-by: David Hendricks <david.hendricks(a)gmail.com> Change-Id: I8ff9e54aba01aea6e2714a13a3efe0d2bb9714de Signed-off-by: Nico Huber <nico.huber(a)secunet.com> --- M ich_descriptors.c 1 file changed, 5 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/00/21800/1 diff --git a/ich_descriptors.c b/ich_descriptors.c index 4af8764..ecc2b95 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -719,7 +719,7 @@ } /* map */ - if (len < (4 + pch_bug_offset) * 4 - 1) + if (len < (4 + pch_bug_offset) * 4) return ICH_RET_OOB; desc->content.FLVALSIG = dump[0 + pch_bug_offset]; desc->content.FLMAP0 = dump[1 + pch_bug_offset]; @@ -727,14 +727,14 @@ desc->content.FLMAP2 = dump[3 + pch_bug_offset]; /* component */ - if (len < (getFCBA(&desc->content) + 3 * 4 - 1)) + if (len < getFCBA(&desc->content) + 3 * 4) return ICH_RET_OOB; desc->component.FLCOMP = dump[(getFCBA(&desc->content) >> 2) + 0]; desc->component.FLILL = dump[(getFCBA(&desc->content) >> 2) + 1]; desc->component.FLPB = dump[(getFCBA(&desc->content) >> 2) + 2]; /* region */ - if (len < (getFRBA(&desc->content) + 5 * 4 - 1)) + if (len < getFRBA(&desc->content) + 5 * 4) return ICH_RET_OOB; desc->region.FLREGs[0] = dump[(getFRBA(&desc->content) >> 2) + 0]; desc->region.FLREGs[1] = dump[(getFRBA(&desc->content) >> 2) + 1]; @@ -743,7 +743,7 @@ desc->region.FLREGs[4] = dump[(getFRBA(&desc->content) >> 2) + 4]; /* master */ - if (len < (getFMBA(&desc->content) + 3 * 4 - 1)) + if (len < getFMBA(&desc->content) + 3 * 4) return ICH_RET_OOB; desc->master.FLMSTR1 = dump[(getFMBA(&desc->content) >> 2) + 0]; desc->master.FLMSTR2 = dump[(getFMBA(&desc->content) >> 2) + 1]; @@ -758,7 +758,7 @@ * the maximum of 255 gives us 127.5 SPI components(!?) 8 bytes each. A * check ensures that the maximum offset actually accessed is available. */ - if (len < (getVTBA(&desc->upper) + (desc->upper.VTL / 2 * 8) - 1)) + if (len < getVTBA(&desc->upper) + (desc->upper.VTL / 2 * 8)) return ICH_RET_OOB; for (i = 0; i < desc->upper.VTL/2; i++) { -- To view, visit https://review.coreboot.org/21800 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-MessageType: newchange Gerrit-Change-Id: I8ff9e54aba01aea6e2714a13a3efe0d2bb9714de Gerrit-Change-Number: 21800 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>

1 0

Change in flashrom[stable]: dediprog: Fix bug where too many transfers would be queued
by build bot (Jenkins) (Code Review) 30 Sep '17

30 Sep '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/21781 ) Change subject: dediprog: Fix bug where too many transfers would be queued ...................................................................... Patch Set 1: Verified-1 Build Failed https://qa.coreboot.org/job/flashrom-customrules/576/ : FAILURE https://qa.coreboot.org/job/flashrom_gerrit/521/ : SUCCESS -- To view, visit https://review.coreboot.org/21781 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: stable Gerrit-MessageType: comment Gerrit-Change-Id: Ie0d516f0fb2923a772a0ca7020ca5118ab260dc5 Gerrit-Change-Number: 21781 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sat, 30 Sep 2017 20:48:34 +0000 Gerrit-HasComments: No

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit