Am 26.10.2011 15:35 schrieb Stefan Tauner: > Based on the new opaque programmer framework this patch adds support > for Intel Hardware Sequencing on ICH8 and its successors. > > By default (or when setting the ich_spi_mode option to auto) > the module tries to use swseq and only activates hwseq if need be: > - if important opcodes are inaccessible due to lockdown > - if more than one flash chip is attached. > The other options (swseq, hwseq) select the respective mode (if possible). > > A general description of Hardware Sequencing can be found in this blog entry: > http://blogs.coreboot.org/blog/2011/06/11/gsoc-2011-flashrom-part-1/ > > TODO: adding real documentation when we have a directory for it > > Signed-off-by: Stefan Tauner <[email protected]>
Acked-by: Carl-Daniel Hailfinger <[email protected]> with a few small comments. > --- > flashrom.8 | 20 +++++ > ichspi.c | 268 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 283 insertions(+), 5 deletions(-) > > diff --git a/flashrom.8 b/flashrom.8 > index a8f4660..66cde4f 100644 > --- a/flashrom.8 > +++ b/flashrom.8 > @@ -303,6 +303,26 @@ is the I/O port number (must be a multiple of 8). In the > unlikely case > flashrom doesn't detect an active IT87 LPC<->SPI bridge, please send a bug > report so we can diagnose the problem. > .sp > +If you have an Intel chipset with an ICH8 or later southbridge with SPI flash > +attached, and if a valid descriptor was written to it (e.g. by the vendor), > the > +chipset provides an alternative way to access the flash chip(s) named > +.BR "Hardware Sequencing" . > +It is much simpler than the normal access method (called > +.BR "Software Sequencing" ")," > +but does not allow the software to choose the SPI commands to be sent. > +You can use the > +.sp > +.B " flashrom \-p internal:ich_spi_mode=value" > +.sp > +syntax where value can be > +.BR auto ", " swseq " or " hwseq . > +By default > +.RB "(or when setting " ich_spi_mode=auto ) > +the module tries to use swseq and only activates hwseq if need be (e.g. if > +important opcodes are inaccessible due to lockdown; or if more than one flash > +chip is attached). The other options (swseq, hwseq) select the respective > mode > +(if possible). > +.sp > If you have an Intel chipset with an ICH6 or later southbridge and if you > want > to set specific IDSEL values for a non-default flash chip or an embedded > controller (EC), you can use the > diff --git a/ichspi.c b/ichspi.c > index bc03554..1d5dd34 100644 > --- a/ichspi.c > +++ b/ichspi.c > @@ -575,6 +575,25 @@ static int program_opcodes(int ich_generation, OPCODES > *op, int enable_undo) > return 0; > } > > +/* Returns true if the most important opcodes are accessible. */ You assume that some erase opcode will be available if BYTE_PROGRAM is available. I think that assumption is reasonable, but it could be documented in this comment above the function. > +static int ich_check_opcodes() > +{ > + uint8_t ops[] = { > + JEDEC_READ, > + JEDEC_BYTE_PROGRAM, > + JEDEC_RDSR, > + 0 > + }; > + int i = 0; > + while (ops[i] != 0) { > + msg_pspew("checking for opcode 0x%02x\n", ops[i]); > + if (find_opcode(curopcodes, ops[i]) == -1) > + return 0; > + i++; > + } > + return 1; > +} > + > /* > * Try to set BBAR (BIOS Base Address Register), but read back the value in > case > * it didn't stick. > @@ -1325,6 +1525,14 @@ static const struct spi_programmer spi_programmer_ich9 > = { > .write_256 = default_spi_write_256, > }; > > +static const struct opaque_programmer opaque_programmer_ich_hwseq = { > + .max_data_read = 64, > + .max_data_write = 64, > + .probe = ich_hwseq_probe, > + .read = ich_hwseq_read, > + .write = ich_hwseq_write, Please add ich_hwseq_block_erase here. > +}; > + > int ich_init_spi(struct pci_dev *dev, uint32_t base, void *rcrb, > int ich_generation) > { > @@ -1332,7 +1540,14 @@ int ich_init_spi(struct pci_dev *dev, uint32_t base, > void *rcrb, > uint8_t old, new; > uint16_t spibar_offset, tmp2; > uint32_t tmp; > + char *arg; > int desc_valid = 0; > + struct ich_descriptors desc = {{ 0 }}; > + enum ich_spi_mode { > + ich_auto, > + ich_hwseq, > + ich_swseq > + } ich_spi_mode = ich_auto; > > switch (ich_generation) { > case 7: > @@ -1399,6 +1614,22 @@ int ich_init_spi(struct pci_dev *dev, uint32_t base, > void *rcrb, > case 9: > case 10: > default: /* Future version might behave the same */ > + arg = extract_programmer_param("ich_spi_mode"); > + if (arg && !strcmp(arg, "hwseq")) { > + ich_spi_mode = ich_hwseq; > + msg_pspew("user selected hwseq\n"); > + } else if (arg && !strcmp(arg, "swseq")) { > + ich_spi_mode = ich_swseq; > + msg_pspew("user selected swseq\n"); > + } else if (arg && !strcmp(arg, "auto")) { > + msg_pspew("user selected auto\n"); > + ich_spi_mode = ich_auto; > + } else if (arg && !strlen(arg)) > + msg_pinfo("Missing argument for ich_spi_mode.\n"); > + else if (arg) > + msg_pinfo("Unknown argument for ich_spi_mode: %s\n", > arg); We should return an error all the way up to programmer init for both cases (and clean up everything). I know that this is a complicated code path, so if you decide not to fail programmer init here, please add a comment like the one below: /* FIXME: Return an error to programmer_init. */ > + free(arg); > + > tmp2 = mmio_readw(ich_spibar + ICH9_REG_HSFS); > msg_pdbg("0x04: 0x%04x (HSFS)\n", tmp2); > prettyprint_ich9_reg_hsfs(tmp2); > @@ -1484,14 +1715,41 @@ int ich_init_spi(struct pci_dev *dev, uint32_t base, > void *rcrb, > > msg_pdbg("\n"); > if (desc_valid) { > - struct ich_descriptors desc = {{ 0 }}; > if (read_ich_descriptors_via_fdo(ich_spibar, &desc) == > ICH_RET_OK) > prettyprint_ich_descriptors(CHIPSET_ICH_UNKNOWN, > &desc); > + /* If the descriptor is valid and indicates multiple > + * flash devices we need to use hwseq to be able to > + * access the second flash device. > + */ > + if (ich_spi_mode == ich_auto && desc.content.NC != 0) { > + msg_pinfo("Enabling hardware sequencing due to " > + "multiple flash chips detected.\n"); > + ich_spi_mode = ich_hwseq; > + } > + } > + > + if (ich_spi_mode == ich_auto && ichspi_lock && > + !ich_check_opcodes()) { > + msg_pinfo("Enabling hardware sequencing because " > + "some important opcodes are locked.\n"); > + ich_spi_mode = ich_hwseq; > + } > + > + if (ich_spi_mode == ich_hwseq) { > + if (!desc_valid) { > + msg_perr("Hardware sequencing was requested " > + "but the flash descriptor is not " > + "valid. Aborting.\n"); > + return 1; Can you check that this indeed causes flashrom to return an error from programmer_init? Chipset init IIRC ignores most errors unless they are somehow declared to be fatal. > + } > + hwseq_data.size_comp0 = > getFCBA_component_density(&desc, 0); > + hwseq_data.size_comp1 = > getFCBA_component_density(&desc, 1); > + > register_opaque_programmer(&opaque_programmer_ich_hwseq); > + } else { > + register_spi_programmer(&spi_programmer_ich9); > } > - register_spi_programmer(&spi_programmer_ich9); > - ich_init_opcodes(); > break; > } > Regards, Carl-Daniel -- http://www.hailfinger.org/ _______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
