June 2011 - flashrom - mail.coreboot.org

[PATCH 2/2] Bonus payload reads flash, sends on serial console
by Tadas Slotkus June 12, 2011

June 12, 2011

Signed-off-by: Tadas Slotkus <devtadas(a)gmail.com> --- flashrom.c | 14 ++++++++++++++ 1 files changed, 14 insertions(+), 0 deletions(-) diff --git a/flashrom.c b/flashrom.c index b145037..f66a85d 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1291,6 +1291,12 @@ int write_buf_to_file(unsigned char *buf, unsigned long size, char *filename) filename); return 1; } +#else + unsigned long i; + serial_putcmd("\n\nRomStartMarker\n\n\0"); + for (i = 0; i < size; i++) + serial_putchar(*(buf++)); + serial_putcmd("\n\nRomEndMarker\n\n\0"); #endif return 0; } @@ -1800,6 +1806,14 @@ void check_chip_supported(const struct flashchip *flash) int main(int argc, char *argv[]) { +#ifdef __LIBPAYLOAD__ + while (1) { + argc=3; + char *argt[3] = {"./flashrom\0", "-r\0", "out.txt\0"}; + argt[1][1] = getchar(); + cli_classic(argc, argt); + } +#endif return cli_classic(argc, argv); } -- 1.7.0.4

1 0

Re: [flashrom] [PATCH 04/12] use the max_data_read field of the new spi_programmer struct to simplify run_opcode.
by Carl-Daniel Hailfinger June 11, 2011

June 11, 2011

Am 08.06.2011 04:55 schrieb Stefan Tauner: > Signed-off-by: Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at> > --- > ichspi.c | 41 +++++++++++++++++------------------------ > 1 files changed, 17 insertions(+), 24 deletions(-) > > diff --git a/ichspi.c b/ichspi.c > index 12727d1..dff6f32 100644 > --- a/ichspi.c > +++ b/ichspi.c > @@ -913,37 +913,30 @@ static int ich9_run_opcode(OPCODE op, uint32_t offset, > static int run_opcode(OPCODE op, uint32_t offset, > uint8_t datalength, uint8_t * data) > { > + uint8_t maxlength = spi_programmer->max_data_read; > Mh. I see what you're doing here, and it makes sense, but a comment would be appreciated in the code: /* The maximum data length is identical on ICH/VIA for the maximum read length and for the maximum write length without opcode and address. Opcode and address are stored in separate registers, not in the data registers. The only exception applies if the opcode definition (un)intentionally classifies said opcode incorrectly as non-address opcode or vice versa. */ Any suggestions on how to improve that comment? > + > + if (spi_programmer->type == SPI_CONTROLLER_NONE) { > + msg_perr("%s: unsupported chipset\n", __func__); > + return -1; > + } > + > + if (datalength > maxlength) { > + msg_perr("%s: Internal command size error for " > + "opcode 0x%02x, got datalength=%i, want <=%i\n", > + __func__, op.opcode, datalength, maxlength); > + return SPI_INVALID_LENGTH; > + } > + > switch (spi_programmer->type) { > case SPI_CONTROLLER_VIA: > - if (datalength > 16) { > - msg_perr("%s: Internal command size error for " > - "opcode 0x%02x, got datalength=%i, want <=16\n", > - __func__, op.opcode, datalength); > - return SPI_INVALID_LENGTH; > - } > - return ich7_run_opcode(op, offset, datalength, data, 16); > case SPI_CONTROLLER_ICH7: > - if (datalength > 64) { > - msg_perr("%s: Internal command size error for " > - "opcode 0x%02x, got datalength=%i, want <=16\n", > - __func__, op.opcode, datalength); > - return SPI_INVALID_LENGTH; > - } > - return ich7_run_opcode(op, offset, datalength, data, 64); > + return ich7_run_opcode(op, offset, datalength, data, maxlength); > case SPI_CONTROLLER_ICH9: > - if (datalength > 64) { > - msg_perr("%s: Internal command size error for " > - "opcode 0x%02x, got datalength=%i, want <=16\n", > - __func__, op.opcode, datalength); > - return SPI_INVALID_LENGTH; > - } > return ich9_run_opcode(op, offset, datalength, data); > default: > - msg_perr("%s: unsupported chipset\n", __func__); > + /* If we ever get here, something really weird happened */ > + return -1; > } > - > - /* If we ever get here, something really weird happened */ > - return -1; > } > > static int ich_spi_send_command(unsigned int writecnt, unsigned int readcnt, > Apart from the comment issue (which can be fixed without resubmitting), this is Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Regards, Carl-Daniel -- http://www.hailfinger.org/

2 4

[commit] r1333 - trunk
by repository service June 11, 2011

June 11, 2011

Author: stefanct Date: Sat Jun 11 21:44:31 2011 New Revision: 1333 URL: http://flashrom.org/trac/flashrom/changeset/1333 Log: use the max_data_read field of the new spi_programmer struct to simplify run_opcode also, explain the transformation of the read/write array/count in ich_spi_send_command better. Signed-off-by: Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at> Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Modified: trunk/ichspi.c Modified: trunk/ichspi.c ============================================================================== --- trunk/ichspi.c Sat Jun 11 20:16:50 2011 (r1332) +++ trunk/ichspi.c Sat Jun 11 21:44:31 2011 (r1333) @@ -922,37 +922,31 @@ static int run_opcode(OPCODE op, uint32_t offset, uint8_t datalength, uint8_t * data) { + /* max_data_read == max_data_write for all Intel/VIA SPI masters */ + uint8_t maxlength = spi_programmer->max_data_read; + + if (spi_programmer->type == SPI_CONTROLLER_NONE) { + msg_perr("%s: unsupported chipset\n", __func__); + return -1; + } + + if (datalength > maxlength) { + msg_perr("%s: Internal command size error for " + "opcode 0x%02x, got datalength=%i, want <=%i\n", + __func__, op.opcode, datalength, maxlength); + return SPI_INVALID_LENGTH; + } + switch (spi_programmer->type) { case SPI_CONTROLLER_VIA: - if (datalength > 16) { - msg_perr("%s: Internal command size error for " - "opcode 0x%02x, got datalength=%i, want <=16\n", - __func__, op.opcode, datalength); - return SPI_INVALID_LENGTH; - } - return ich7_run_opcode(op, offset, datalength, data, 16); case SPI_CONTROLLER_ICH7: - if (datalength > 64) { - msg_perr("%s: Internal command size error for " - "opcode 0x%02x, got datalength=%i, want <=16\n", - __func__, op.opcode, datalength); - return SPI_INVALID_LENGTH; - } - return ich7_run_opcode(op, offset, datalength, data, 64); + return ich7_run_opcode(op, offset, datalength, data, maxlength); case SPI_CONTROLLER_ICH9: - if (datalength > 64) { - msg_perr("%s: Internal command size error for " - "opcode 0x%02x, got datalength=%i, want <=16\n", - __func__, op.opcode, datalength); - return SPI_INVALID_LENGTH; - } return ich9_run_opcode(op, offset, datalength, data); default: - msg_perr("%s: unsupported chipset\n", __func__); + /* If we ever get here, something really weird happened */ + return -1; } - - /* If we ever get here, something really weird happened */ - return -1; } static int ich_spi_send_command(unsigned int writecnt, unsigned int readcnt, @@ -1038,7 +1032,13 @@ } } - /* translate read/write array/count */ + /* Translate read/write array/count. + * The maximum data length is identical for the maximum read length and + * for the maximum write length excluding opcode and address. Opcode and + * address are stored in separate registers, not in the data registers + * and are thus not counted towards data length. The only exception + * applies if the opcode definition (un)intentionally classifies said + * opcode incorrectly as non-address opcode or vice versa. */ if (opcode->spi_type == SPI_OPCODE_TYPE_WRITE_NO_ADDRESS) { data = (uint8_t *) (writearr + 1); count = writecnt - 1;

1 0

Re: [flashrom] [PATCH 05/12] warn if SMM BIOS Write Protection is detected in BIOS_CNTL
by Stefan Tauner June 11, 2011

June 11, 2011

already committed (in r1332), dont bother to review :) -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner

1 0

Re: [flashrom] [PATCH 05/11] warn if SMM BIOS Write Protection is detected in BIOS_CNTL
by Carl-Daniel Hailfinger June 11, 2011

June 11, 2011

Am 28.05.2011 05:38 schrieb Stefan Tauner: > Signed-off-by: Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at> > With minor changes: Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> > --- > chipset_enable.c | 11 ++++++++++- > 1 files changed, 10 insertions(+), 1 deletions(-) > > diff --git a/chipset_enable.c b/chipset_enable.c > index 83b49ad..1f6f74c 100644 > --- a/chipset_enable.c > +++ b/chipset_enable.c > @@ -264,8 +264,17 @@ static int enable_flash_ich(struct pci_dev *dev, const char *name, > (old & (1 << 0)) ? "en" : "dis"); > msg_pdbg("BIOS_CNTL is 0x%x\n", old); > > - new = old | 1; > + /* > + * Quote from the 6 Series datasheet: > Please include a datasheet number so others can find it easily. > + * "5: SMM BIOS Write Protect Disable (SMM_BWP) > Maybe "Bit 5: SMM..." > + * 1 = BIOS region SMM protection is enabled. > + * The BIOS Region is not writable unless all processors are in SMM." > + * In earlier chipsets this bit is reserved. */ > + if (old & (1 << 5)) { > + msg_pinfo("WARNING: BIOS region SMM protection is enabled!\n"); > + } > > + new = old | 1; > if (new == old) > return 0; > > Regards, Carl-Daniel -- http://www.hailfinger.org/

2 1

[commit] r1332 - trunk
by repository service June 11, 2011

June 11, 2011

Author: stefanct Date: Sat Jun 11 20:16:50 2011 New Revision: 1332 URL: http://flashrom.org/trac/flashrom/changeset/1332 Log: enable_flash_ich: warn if SMM BIOS Write Protection is detected in BIOS_CNTL Signed-off-by: Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at> Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Modified: trunk/chipset_enable.c Modified: trunk/chipset_enable.c ============================================================================== --- trunk/chipset_enable.c Sat Jun 11 14:21:37 2011 (r1331) +++ trunk/chipset_enable.c Sat Jun 11 20:16:50 2011 (r1332) @@ -264,8 +264,17 @@ (old & (1 << 0)) ? "en" : "dis"); msg_pdbg("BIOS_CNTL is 0x%x\n", old); - new = old | 1; + /* + * Quote from the 6 Series datasheet (Document Number: 324645-004): + * "Bit 5: SMM BIOS Write Protect Disable (SMM_BWP) + * 1 = BIOS region SMM protection is enabled. + * The BIOS Region is not writable unless all processors are in SMM." + * In earlier chipsets this bit is reserved. */ + if (old & (1 << 5)) { + msg_pinfo("WARNING: BIOS region SMM protection is enabled!\n"); + } + new = old | 1; if (new == old) return 0;

1 0

[commit] r1331 - trunk
by repository service June 11, 2011

June 11, 2011

Author: stefanct Date: Sat Jun 11 14:21:37 2011 New Revision: 1331 URL: http://flashrom.org/trac/flashrom/changeset/1331 Log: add support for Olimex programmers to ft2232_spi - add support for Olimex' ARM-USB-TINY, ARM-USB-TINY-H, ARM-USB-OCD AND ARM-USB-OCD-H and adjust man page - minor string change ("First International Computer, Inc." -> "FIC") Signed-off-by: Pete Batard <pbatard(a)gmail.com> Acked-by: Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at> Modified: trunk/flashrom.8 trunk/ft2232_spi.c Modified: trunk/flashrom.8 ============================================================================== --- trunk/flashrom.8 Sat Jun 11 11:53:22 2011 (r1330) +++ trunk/flashrom.8 Sat Jun 11 14:21:37 2011 (r1331) @@ -183,7 +183,7 @@ .BR "* it87spi" " (for flash ROMs behind an ITE IT87xx Super I/O LPC/SPI \ translation unit)" .sp -.BR "* ft2232_spi" " (for SPI flash ROMs attached to a FT2232H/FT4232H/JTAGkey \ +.BR "* ft2232_spi" " (for SPI flash ROMs attached to an FT2232/FT4232H family \ based USB SPI programmer)" .sp .BR "* serprog" " (for flash ROMs attached to a programmer speaking serprog)" @@ -361,8 +361,9 @@ .sp syntax where .B model -can be any of -.BR 2232H ", " JTAGkey ", or " 4232H +can be one of +.BR 2232H ", " 4232H ", " jtagkey ", " openmoko ", " arm-usb-tiny ", " \ +arm-usb-tiny-h ", " arm-usb-ocd " or " arm-usb-ocd-h and .B interface can be any of Modified: trunk/ft2232_spi.c ============================================================================== --- trunk/ft2232_spi.c Sat Jun 11 11:53:22 2011 (r1330) +++ trunk/ft2232_spi.c Sat Jun 11 14:21:37 2011 (r1331) @@ -38,13 +38,22 @@ #define FIC_VID 0x1457 #define OPENMOKO_DBGBOARD_PID 0x5118 +#define OLIMEX_VID 0x15BA +#define OLIMEX_ARM_OCD_PID 0x0003 +#define OLIMEX_ARM_TINY_PID 0x0004 +#define OLIMEX_ARM_OCD_H_PID 0x002B +#define OLIMEX_ARM_TINY_H_PID 0x002A + const struct usbdev_status devs_ft2232spi[] = { {FTDI_VID, FTDI_FT2232H_PID, OK, "FTDI", "FT2232H"}, {FTDI_VID, FTDI_FT4232H_PID, OK, "FTDI", "FT4232H"}, {FTDI_VID, AMONTEC_JTAGKEY_PID, OK, "Amontec", "JTAGkey"}, - {FIC_VID, OPENMOKO_DBGBOARD_PID, OK, - "First International Computer, Inc.", + {FIC_VID, OPENMOKO_DBGBOARD_PID, OK, "FIC", "OpenMoko Neo1973 Debug board (V2+)"}, + {OLIMEX_VID, OLIMEX_ARM_OCD_PID, NT, "Olimex", "ARM-USB-OCD"}, + {OLIMEX_VID, OLIMEX_ARM_TINY_PID, OK, "Olimex", "ARM-USB-TINY"}, + {OLIMEX_VID, OLIMEX_ARM_OCD_H_PID, NT, "Olimex", "ARM-USB-OCD-H"}, + {OLIMEX_VID, OLIMEX_ARM_TINY_H_PID, NT, "Olimex", "ARM-USB-TINY-H"}, {}, }; @@ -167,6 +176,22 @@ ft2232_vid = FIC_VID; ft2232_type = OPENMOKO_DBGBOARD_PID; ft2232_interface = INTERFACE_A; + } else if (!strcasecmp(arg, "arm-usb-ocd")) { + ft2232_vid = OLIMEX_VID; + ft2232_type = OLIMEX_ARM_OCD_PID; + ft2232_interface = INTERFACE_A; + } else if (!strcasecmp(arg, "arm-usb-tiny")) { + ft2232_vid = OLIMEX_VID; + ft2232_type = OLIMEX_ARM_TINY_PID; + ft2232_interface = INTERFACE_A; + } else if (!strcasecmp(arg, "arm-usb-ocd-h")) { + ft2232_vid = OLIMEX_VID; + ft2232_type = OLIMEX_ARM_OCD_H_PID; + ft2232_interface = INTERFACE_A; + } else if (!strcasecmp(arg, "arm-usb-tiny-h")) { + ft2232_vid = OLIMEX_VID; + ft2232_type = OLIMEX_ARM_TINY_H_PID; + ft2232_interface = INTERFACE_A; } else { msg_perr("Error: Invalid device type specified.\n"); free(arg);

1 0

Re: [flashrom] [PATCH 03/12] add macros and pretty printing for HSFS and HSFC
by Carl-Daniel Hailfinger June 11, 2011

June 11, 2011

Am 08.06.2011 04:55 schrieb Stefan Tauner: > Signed-off-by: Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at> > --- > ichspi.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++---------- > 1 files changed, 62 insertions(+), 12 deletions(-) > > diff --git a/ichspi.c b/ichspi.c > index 7e4a363..12727d1 100644 > --- a/ichspi.c > +++ b/ichspi.c > @@ -43,6 +43,37 @@ > #include "spi.h" > > /* ICH9 controller register definition */ > +#define ICH9_REG_HSFS 0x04 /* 16 Bits Hardware Sequencing Flash Status */ > +#define HSFS_FDONE_OFF 0 /* 0: Flash Cycle Done */ > +#define HSFS_FDONE (0x1 << HSFS_FDONE_OFF) > +#define HSFS_FCERR_OFF 1 /* 1: Flash Cycle Error */ > +#define HSFS_FCERR (0x1 << HSFS_FCERR_OFF) > +#define HSFS_AEL_OFF 2 /* 2: Access Error Log */ > +#define HSFS_AEL (0x1 << HSFS_AEL_OFF) > +#define HSFS_BERASE_OFF 3 /* 3-4: Block/Sector Erase Size */ > +#define HSFS_BERASE (0x1 << HSFS_BERASE_OFF) > Code change! Please explain/fix. > +#define HSFS_SCIP_OFF 5 /* 5: SPI Cycle In Progress */ > +#define HSFS_SCIP (0x1 << HSFS_SCIP_OFF) > + /* 6-12: reserved */ > +#define HSFS_FDOPSS_OFF 13 /* 13: Flash Descriptor Override Pin-Strap Status */ > +#define HSFS_FDOPSS (0x1 << HSFS_FDOPSS_OFF) > +#define HSFS_FDV_OFF 14 /* 14: Flash Descriptor Valid */ > +#define HSFS_FDV (0x1 << HSFS_FDV_OFF) > +#define HSFS_FLOCKDN_OFF 15 /* 15: Flash Configuration Lock-Down */ > +#define HSFS_FLOCKDN (0x1 << HSFS_FLOCKDN_OFF) > + > +#define ICH9_REG_HSFC 0x06 /* 16 Bits Hardware Sequencing Flash Control */ > +#define HSFC_FGO_OFF 0 /* 0: Flash Cycle Go */ > +#define HSFC_FGO (0x1 << HSFC_FGO_OFF) > +#define HSFC_FCYCLE_OFF 1 /* 1-2: FLASH Cycle */ > +#define HSFC_FCYCLE (0x3 << HSFC_FCYCLE_OFF) > + /* 3-7: reserved */ > +#define HSFC_FDBC_OFF 8 /* 8-13: Flash Data Byte Count */ > +#define HSFC_FDBC (0x3f << HSFC_FDBC_OFF) > + /* 14: reserved */ > +#define HSFC_SME_OFF 15 /* 15: SPI SMI# Enable */ > +#define HSFC_SME (0x1 << HSFC_SME_OFF) > + > #define ICH9_REG_FADDR 0x08 /* 32 Bits */ > #define ICH9_REG_FDATA0 0x10 /* 64 Bytes */ > > @@ -260,6 +291,28 @@ static void pretty_print_opcodes(OPCODES *ops) > } > > #define pprint_reg(reg, bit, val, sep) msg_pdbg("%s=%d" sep, #bit, (val & reg##_##bit)>>reg##_##bit##_OFF) > +static void prettyprint_ich9_reg_hsfs(uint16_t reg_val) > +{ > + msg_pdbg("HSFS: "); > + pprint_reg(HSFS, FDONE, reg_val, ", "); > + pprint_reg(HSFS, FCERR, reg_val, ", "); > + pprint_reg(HSFS, AEL, reg_val, ", "); > + pprint_reg(HSFS, BERASE, reg_val, ", "); > + pprint_reg(HSFS, SCIP, reg_val, ", "); > + pprint_reg(HSFS, FDOPSS, reg_val, ", "); > + pprint_reg(HSFS, FDV, reg_val, ", "); > + pprint_reg(HSFS, FLOCKDN, reg_val, "\n"); > +} > + > +static void prettyprint_ich9_reg_hsfc(uint16_t reg_val) > +{ > + msg_pdbg("HSFC: "); > + pprint_reg(HSFC, FGO, reg_val, ", "); > + pprint_reg(HSFC, FCYCLE, reg_val, ", "); > + pprint_reg(HSFC, FDBC, reg_val, ", "); > + pprint_reg(HSFC, SME, reg_val, "\n"); > +} > + > static void prettyprint_ich9_reg_ssfs(uint32_t reg_val) > { > msg_pdbg("SSFS: "); > @@ -1204,14 +1257,15 @@ int ich_init_spi(struct pci_dev *dev, uint32_t base, void *rcrb, > case SPI_CONTROLLER_ICH9: > tmp2 = mmio_readw(ich_spibar + 4); > msg_pdbg("0x04: 0x%04x (HSFS)\n", tmp2); > - msg_pdbg("FLOCKDN %i, ", (tmp2 >> 15 & 1)); > - msg_pdbg("FDV %i, ", (tmp2 >> 14) & 1); > - msg_pdbg("FDOPSS %i, ", (tmp2 >> 13) & 1); > - msg_pdbg("SCIP %i, ", (tmp2 >> 5) & 1); > - msg_pdbg("BERASE %i, ", (tmp2 >> 3) & 3); > - msg_pdbg("AEL %i, ", (tmp2 >> 2) & 1); > - msg_pdbg("FCERR %i, ", (tmp2 >> 1) & 1); > - msg_pdbg("FDONE %i\n", (tmp2 >> 0) & 1); > + prettyprint_ich9_reg_hsfs(tmp2); > + if (tmp2 & (1 << 15)) { > + msg_pinfo("WARNING: SPI Configuration Lockdown activated.\n"); > + ichspi_lock = 1; > + } > + > + tmp2 = mmio_readw(ich_spibar + 6); > + msg_pdbg("0x06: 0x%04x (HSFC)\n", tmp2); > + prettyprint_ich9_reg_hsfc(tmp2); > > tmp = mmio_readl(ich_spibar + 0x50); > msg_pdbg("0x50: 0x%08x (FRAP)\n", tmp); > @@ -1258,10 +1312,6 @@ int ich_init_spi(struct pci_dev *dev, uint32_t base, void *rcrb, > ichspi_bbar); > msg_pdbg("0xB0: 0x%08x (FDOC)\n", > mmio_readl(ich_spibar + 0xB0)); > - if (tmp2 & (1 << 15)) { > - msg_pinfo("WARNING: SPI Configuration Lockdown activated.\n"); > - ichspi_lock = 1; > - } > Was it intentional to move that code a few lines up? > ich_init_opcodes(); > break; > default: > With an explanation/fix for the HSFS_BERASE code change this is Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Regards, Carl-Daniel -- http://www.hailfinger.org/

2 2

Re: [flashrom] [PATCH 02/12] add pretty printing functions for SSFS+SSFC and the OPCODE struct and use them
by Carl-Daniel Hailfinger June 11, 2011

June 11, 2011

Am 08.06.2011 04:55 schrieb Stefan Tauner: > Signed-off-by: Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at> > Thanks for your patch! > --- > ichspi.c | 51 +++++++++++++++++++++++++++++++++++++++++++++------ > 1 files changed, 45 insertions(+), 6 deletions(-) > > diff --git a/ichspi.c b/ichspi.c > index 676b93a..7e4a363 100644 > --- a/ichspi.c > +++ b/ichspi.c > @@ -243,6 +243,44 @@ static OPCODE POSSIBLE_OPCODES[] = { > > static OPCODES O_EXISTING = {}; > > +/* pretty printing functions */ > +static void pretty_print_opcodes(OPCODES *ops) > +{ > + if(ops == NULL) > + return; > + > + msg_pdbg("preop0=0x%02x, preop1=0x%02x\n", ops->preop[0], ops->preop[1]); > 80 columns > + > + OPCODE oc; > + uint8_t i; > + for (i = 0; i < 8; i++) { > + oc = ops->opcode[i]; > + msg_pdbg("op[%d]=0x%02x, %d, %d\n", i, oc.opcode, oc.spi_type, oc.atomic); > 80 columns > + } > +} > + > +#define pprint_reg(reg, bit, val, sep) msg_pdbg("%s=%d" sep, #bit, (val & reg##_##bit)>>reg##_##bit##_OFF) > For this macro, 80 col violations are OK. But please insert an empty line afterwards. > +static void prettyprint_ich9_reg_ssfs(uint32_t reg_val) > +{ > + msg_pdbg("SSFS: "); > + pprint_reg(SSFS, SCIP, reg_val, ", "); > + pprint_reg(SSFS, FDONE, reg_val, ", "); > + pprint_reg(SSFS, FCERR, reg_val, ", "); > + pprint_reg(SSFS, AEL, reg_val, "\n"); > +} > + > +static void prettyprint_ich9_reg_ssfc(uint32_t reg_val) > +{ > + msg_pdbg("SSFC: "); > + pprint_reg(SSFC, SCGO, reg_val, ", "); > + pprint_reg(SSFC, ACS, reg_val, ", "); > + pprint_reg(SSFC, SPOP, reg_val, ", "); > + pprint_reg(SSFC, COP, reg_val, ", "); > + pprint_reg(SSFC, DBC, reg_val, ", "); > + pprint_reg(SSFC, SME, reg_val, ", "); > + pprint_reg(SSFC, SCF, reg_val, "\n"); > +} > + > static uint8_t lookup_spi_type(uint8_t opcode) > { > int a; > @@ -519,6 +557,8 @@ static int ich_init_opcodes(void) > } else { > curopcodes = curopcodes_done; > msg_pdbg("done\n"); > + pretty_print_opcodes(curopcodes); > + msg_pdbg("\n"); > return 0; > } > } > @@ -793,6 +833,8 @@ static int ich9_run_opcode(OPCODE op, uint32_t offset, > temp32 = REGREAD32(ICH9_REG_SSFS); > if (temp32 & SSFS_FCERR) { > msg_perr("Transaction error!\n"); > + prettyprint_ich9_reg_ssfs(temp32); > + prettyprint_ich9_reg_ssfc(temp32); > /* keep reserved bits */ > temp32 &= SSFS_RESERVED_MASK | SSFC_RESERVED_MASK; > /* Clear the transaction error. */ > @@ -1195,16 +1237,13 @@ int ich_init_spi(struct pci_dev *dev, uint32_t base, void *rcrb, > > tmp = mmio_readl(ich_spibar + 0x90); > msg_pdbg("0x90: 0x%02x (SSFS)\n", tmp & 0xff); > - msg_pdbg("AEL %i, ", (tmp >> 4) & 1); > - msg_pdbg("FCERR %i, ", (tmp >> 3) & 1); > - msg_pdbg("FDONE %i, ", (tmp >> 2) & 1); > - msg_pdbg("SCIP %i\n", (tmp >> 0) & 1); > + prettyprint_ich9_reg_ssfs(tmp); > if (tmp & (1 << 3)) { > msg_pdbg("Clearing SSFS.FCERR\n"); > mmio_writeb(1 << 3, ich_spibar + 0x90); > } > - tmp >>= 8; > - msg_pdbg("0x91: 0x%06x (SSFC)\n", tmp); > + msg_pdbg("0x91: 0x%06x (SSFC)\n", tmp >> 8); > + prettyprint_ich9_reg_ssfc(tmp); > > msg_pdbg("0x94: 0x%04x (PREOP)\n", > mmio_readw(ich_spibar + 0x94)); > Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Regards, Carl-Daniel -- http://www.hailfinger.org/

2 1

Re: [flashrom] [PATCH 01/12] improve macros for SSFS and SSFC bits
by Carl-Daniel Hailfinger June 11, 2011

June 11, 2011

One additional comment... Am 08.06.2011 04:55 schrieb Stefan Tauner: > - introduce offset macros and use them to (re)define the existing mask macros > - add comments > - rename SSFS_CDS to SSFS_FDONE (abbr. used in datasheet not in SSFS but HSFS) > - use those for refactoring and magic number elemination. > - following patch uses them for pretty printing > > --- a/ichspi.c > +++ b/ichspi.c > @@ -709,8 +721,8 @@ static int ich9_run_opcode(OPCODE op, uint32_t offset, > temp32 = REGREAD32(ICH9_REG_SSFS); > /* Keep reserved bits only */ > temp32 &= SSFS_RESERVED_MASK | SSFC_RESERVED_MASK; > - /* clear error status registers */ > - temp32 |= (SSFS_CDS + SSFS_FCERR); > + /* Clear cycle done and cycle error status registers */ > + temp32 |= (SSFS_FDONE + SSFS_FCERR); > Please use | instead of + here. > REGWRITE32(ICH9_REG_SSFS, temp32); > > /* Use 20 MHz */ > Regards, Carl-Daniel -- http://www.hailfinger.org/

2 2