This patch adds support for the SPI Chips Atmel AT25DF32, ST M25P80, ST M25P16, M25P32, ST M25P64 attached to the ICH9. Because chipset plays an important role in communicating with these chips, it was not possible to integrate this support into the already existing spi.c This module was tested with the devices as mentioned above, but it should be easy to expand it for other SPI chips attached to an ICH.
Signed-off-by: Claus Gindhart claus.gindhart@kontron.com
---
The previous patch was buggy; there was a merging conflict in my local code repository, which was incuded into the patch; sorry for that.
I did not yet set the test status flag to any TEST_OK for my supporting function, because yet i dont know, who has the permission to do that; is it the initiator of this code, or any acknowledging person ?
Mit freundlichen Grüßen / Best regards
Claus Gindhart SW R&D Kontron Modular Computers phone :++49 (0)8341-803-374 mailto:claus.gindhart@kontron.com http://www.kontron.com
Kontron Modular Computers GmbH Geschaeftsfuehrer / Managing Directors: Ulrich Gehrmann, Thomas Sabisch Sitz der Gesellschaft / Registered Office: Kaufbeuren, Rechtsform / Legal: GmbH Amtsgericht / Local District Court Kempten, HRB Nr. / Trade Register No. 6195
The information contained in this document is CONFIDENTIAL and property of Kontron. Any unauthorized review, use, disclosure or distribution is prohibited without express written consent of Kontron. If you are not the intended recipient, please contact the sender and destroy all copies of the original message and enclosed attachments.
Diese E-Mail enthält vertrauliche und/oder rechtlich geschützte Informationen und ist Eigentum von Kontron. Die Verwendung und Weitergabe von jeglichen Inhalten ist ohne ausdrückliche schriftliche Genehmigung von Kontron strikt untersagt. Wenn Sie diese E-Mail irrtümlich erhalten haben, informieren Sie bitte sofort den Absender und vernichten diese Mail und enthaltene Dokumente.
Hi Claus,
On Tue, May 13, 2008 at 09:56:06AM +0200, Claus Gindhart wrote:
This patch adds support for the SPI Chips Atmel AT25DF32, ST M25P80, ST M25P16, M25P32, ST M25P64 attached to the ICH9. Because chipset plays an important role in communicating with these chips, it was not possible to integrate this support into the already existing spi.c This module was tested with the devices as mentioned above, but it should be easy to expand it for other SPI chips attached to an ICH.
Signed-off-by: Claus Gindhart claus.gindhart@kontron.com
Thanks for the patch!
This is a big improvement over the last one!
The previous patch was buggy; there was a merging conflict in my local code repository, which was incuded into the patch; sorry for that.
No problem as long as you can send a new one. :)
I did not yet set the test status flag to any TEST_OK for my supporting function, because yet i dont know, who has the permission to do that; is it the initiator of this code, or any acknowledging person ?
It is quite simple; anyone who has tested the program with a chip can send a notice to the list or flashrom@coreboot.org (though I don't know for sure that that address works yet) and that information will be trusted and commited immediately. No need for Ack since that does not make much sense for a test result.
So please do add TEST_OK_ flags for any chips that you have used with flashrom.
If you prefer to just list the chips and what you have tested that is fine, but a patch would be ideal.
Index: sst_fwhub.c
This file should not be included in the commit, but it can be removed manually. Please update your working copy too Claus so the same hunk isn't included by mistake in future patches. Thanks!
Index: ichspi.c
..
+#undef DEVELOPPERS_DEBUG
There is a printf_debug() function that could be used instead of this define. It prints output when -V is specified on the command line.
+static OPCODES *curopcodes; +static inline uint8_t REGREAD8(int X) +static inline uint16_t REGREAD16(int X) +static inline uint32_t REGREAD32(int X) +#define REGWRITE32(X,Y) (*(uint32_t *)(curflash->virtual_registers+X)=Y) +#define REGWRITE16(X,Y) (*(uint16_t *)(curflash->virtual_registers+X)=Y) +#define REGWRITE8(X,Y) (*(uint8_t *)(curflash->virtual_registers+X)=Y)
+/* Common SPI functions */ +static int ProgramOpcodes(OPCODES * op); +static int RunOpcode(uint8_t nr, OPCODE op, uint32_t offset, uint8_t datalength,
uint8_t * data);+static int read_page_ichspi(struct flashchip *flash, uint8_t * buf, int Offset); +static int is_supported_chipset();
Maybe this will feel very petty, but I do not like this naming style.
Please use only lowercase letters, and use underscore to separate words, for all types, functions and parameters.
Restrict uppercase names for #defines only.
I think this is important for readability of the program as a whole, though since this is one isolated file that will eventually be replaced anyway with better generic SPI support in flashrom, it might be possible to convince me to commit this as is. :)
Index: ichspiacc.h
..
+typedef struct _SPIFLASH {
- const char *name;
- OPCODES opcodes;
- uint32_t pagesize;
- uint32_t size;
+#if 0
- int (*erase) (struct _SPIFLASH * myflash, uint32 Offset, uint32 sectors)
- int (*write) (struct _SPIFLASH * myflash, uint32 Offset, uint32 number,
uint8_t * bytes)- int (*read) (struct _SPIFLASH * myflash, uint32 Offset, uint32 number,
uint8_t * bytes)+#endif +} SPIFLASH;
This struct is never used - please remove it.
Claus, if you can address my comments above, I will be happy to Ack and commit this patch!
//Peter
Hi,
On Tue, May 13, 2008 at 09:56:06AM +0200, Claus Gindhart wrote:
This patch adds support for the SPI Chips Atmel AT25DF32, ST M25P80, ST M25P16, M25P32, ST M25P64 attached to the ICH9. Because chipset plays an important role in communicating with these chips, it was not possible to integrate this support into the already existing spi.c This module was tested with the devices as mentioned above, but it should be easy to expand it for other SPI chips attached to an ICH.
Signed-off-by: Claus Gindhart claus.gindhart@kontron.com
Thanks, nice work! I'll let someone else review the SPI-related parts, I don't have much experience with that, but see a few other random comments below.
The previous patch was buggy; there was a merging conflict in my local code repository, which was incuded into the patch; sorry for that.
I did not yet set the test status flag to any TEST_OK for my supporting function, because yet i dont know, who has the permission to do that; is it the initiator of this code, or any acknowledging person ?
I'd say anybody who has actually properly tested all operations on hardware.
Index: flash.h
--- flash.h (revision 3300) +++ flash.h (working copy) @@ -107,6 +107,7 @@ #define AT_29C020 0xDA #define AT_49F002N 0x07 /* for AT49F002(N) */ #define AT_49F002NT 0x08 /* for AT49F002(N)T */ +#define AT_25DF321 0x4700
#define CATALYST_ID 0x31 /* Catalyst */
@@ -455,4 +456,13 @@ int probe_stm50flw0x0x(struct flashchip *flash); int erase_stm50flw0x0x(struct flashchip *flash); int write_stm50flw0x0x(struct flashchip *flash, uint8_t *buf);
+/* ichspi.c */ +int probe_ichspi_stm25(struct flashchip *flash); +int probe_ichspi_stm25_sig(struct flashchip *flash); +int probe_ichspi_at25df(struct flashchip *flash); +int erase_ichspi(struct flashchip *flash); +int write_ichspi(struct flashchip *flash, uint8_t *buf); +int read_ichspi(struct flashchip *flash, uint8_t *buf);
#endif /* !__FLASH_H__ */ Index: flashchips.c =================================================================== --- flashchips.c (revision 3300) +++ flashchips.c (working copy) @@ -43,6 +43,7 @@ {"Atmel", "AT29C040A", ATMEL_ID, AT_29C040A, 512, 256, TEST_UNTESTED, probe_jedec, erase_chip_jedec, write_jedec}, {"Atmel", "AT49F002(N)", ATMEL_ID, AT_49F002N, 256, 256, TEST_UNTESTED, probe_jedec, erase_chip_jedec, write_jedec}, {"Atmel", "AT49F002(N)T", ATMEL_ID, AT_49F002NT, 256, 256, TEST_UNTESTED, probe_jedec, erase_chip_jedec, write_jedec},
- {"Atmel", "AT25DF321@ICH9", ATMEL_ID, AT_25DF321, 4096, 64 * 1024, TEST_UNTESTED, probe_ichspi_at25df, erase_ichspi, write_ichspi, read_ichspi}, {"EMST", "F49B002UA", EMST_ID, EMST_F49B002UA, 256, 4096, TEST_UNTESTED, probe_jedec, erase_chip_jedec, write_49f002}, {"EON", "EN29F002(A)(N)B", EON_ID, EN_29F002B, 256, 256, TEST_UNTESTED, probe_jedec, erase_chip_jedec, write_jedec}, {"EON", "EN29F002(A)(N)T", EON_ID, EN_29F002T, 256, 256, TEST_UNTESTED, probe_jedec, erase_chip_jedec, write_jedec},
@@ -113,6 +114,11 @@ {"ST", "M50FW040", ST_ID, ST_M50FW040, 512, 64 * 1024, TEST_UNTESTED, probe_82802ab, erase_82802ab, write_82802ab}, {"ST", "M50FW080", ST_ID, ST_M50FW080, 1024, 64 * 1024, TEST_UNTESTED, probe_82802ab, erase_82802ab, write_82802ab}, {"ST", "M50LPW116", ST_ID, ST_M50LPW116, 2048, 64 * 1024, TEST_UNTESTED, probe_jedec, erase_chip_jedec, write_jedec},
- {"ST", "M25P40@ICH9", ST_ID, ST_M25P40, 512, 64 * 1024, TEST_UNTESTED, probe_ichspi_stm25_sig, erase_ichspi, write_ichspi, read_ichspi},
- {"ST", "M25P80@ICH9", ST_ID, ST_M25P80, 1024, 64 * 1024, TEST_UNTESTED, probe_ichspi_stm25_sig, erase_ichspi, write_ichspi, read_ichspi},
- {"ST", "M25P16@ICH9", ST_ID, ST_M25P16, 2048, 64 * 1024, TEST_UNTESTED, probe_ichspi_stm25, erase_ichspi, write_ichspi, read_ichspi},
- {"ST", "M25P32@ICH9", ST_ID, ST_M25P32, 4096, 64 * 1024, TEST_UNTESTED, probe_ichspi_stm25, erase_ichspi, write_ichspi, read_ichspi},
- {"ST", "M25P64@ICH9", ST_ID, ST_M25P64, 8192, 64 * 1024, TEST_UNTESTED, probe_ichspi_stm25, erase_ichspi, write_ichspi, read_ichspi},
Why "@ICH9", does the chip behave differently depending on where you plug it in?
{"SyncMOS", "S29C31004T", SYNCMOS_ID, S29C31004T, 512, 128, TEST_UNTESTED, probe_jedec, erase_chip_jedec, write_49f002}, {"SyncMOS", "S29C51001T", SYNCMOS_ID, S29C51001T, 128, 128, TEST_UNTESTED, probe_jedec, erase_chip_jedec, write_49f002}, {"SyncMOS", "S29C51002T", SYNCMOS_ID, S29C51002T, 256, 128, TEST_UNTESTED, probe_jedec, erase_chip_jedec, write_49f002}, Index: Makefile =================================================================== --- Makefile (revision 3300) +++ Makefile (working copy) @@ -24,7 +24,7 @@ sst28sf040.o am29f040b.o mx29f002.o sst39sf020.o m29f400bt.o \ w49f002u.o 82802ab.o msys_doc.o pm49fl004.o sst49lf040.o \ sst49lfxxxc.o sst_fwhub.o layout.o cbtable.o flashchips.o \
- flashrom.o w39v080fa.o sharplhf00l04.o w29ee011.o spi.o
- flashrom.o w39v080fa.o sharplhf00l04.o w29ee011.o spi.o ichspi.o
all: pciutils dep $(PROGRAM)
Index: sst_fwhub.c
--- sst_fwhub.c (revision 3300) +++ sst_fwhub.c (working copy) @@ -21,6 +21,7 @@ /* Adapted from the Intel FW hub stuff for 82802ax parts. */
#include <stdio.h> +#include <string.h> #include "flash.h"
// I need that Berkeley bit-map printer @@ -56,6 +57,8 @@ erase_block_jedec(flash->virtual_memory, offset); toggle_ready_jedec(flash->virtual_memory);
- printf("DONE BLOCK 0x%x\n", offset);
- return 0;
}
@@ -81,23 +84,34 @@
int write_sst_fwhub(struct flashchip *flash, uint8_t *buf) {
- int i;
- int i,rc=0; int total_size = flash->total_size * 1024; int page_size = flash->page_size; volatile uint8_t *bios = flash->virtual_memory;
- // FIXME: We want block wide erase instead of ironing the whole chip
- if (erase_sst_fwhub(flash))
return -1;- printf("Programming page: "); for (i = 0; i < total_size / page_size; i++) {
printf("\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b");
write_sector_jedec(bios, buf + i * page_size,
/* Auto Skip Blocks, which already contain the desired data* Faster, because we only write, what has changed* More secure, because blocks, which are excluded* (with the exclude or layout feature)* are not erased and rewritten; data is retained also* in sudden power off situations*/if (!memcmp((void *)(buf + i * page_size),(void *)(bios + i * page_size), page_size)) {printf("SKIPPED\n");continue;}rc = erase_sst_fwhub_block(flash, i * page_size);if (!rc) write_sector_jedec(bios, buf + i * page_size, bios + i * page_size, page_size);
printf("\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b");} printf("\n");
return 0;
- return rc;
} Index: ichspi.c =================================================================== --- ichspi.c (revision 0) +++ ichspi.c (revision 0) @@ -0,0 +1,654 @@ +/*
- This file is part of the flashrom project.
- Initially implemented by Stefan Wildemann stefan.wildemann@kontron.com
- Adapted to flashrom by Claus Gindhart claus.gindhart@kontron.com
Please change these two lines to the usual Copyright lines, i.e.
Copyright (C) 200x Stefan Wildemann stefan.wildemann@kontron.com Copyright (C) 200y Claus Gindhart claus.gindhart@kontron.com
replacing the 200x/200y years appropriately.
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License as published by
- the Free Software Foundation; either version 2 of the License, or
- (at your option) any later version.
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- You should have received a copy of the GNU General Public License
- along with this program; if not, write to the Free Software
- Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
- */
+/*
- This module is designed for supporting the devices
- ST M25P40
- ST M25P80
- ST M25P16
- ST M25P32
- ST M25P64
- AT 25DF321
- */
+#undef DEVELOPPERS_DEBUG
I'd drop this, see below.
+#include <stdio.h> +#include <string.h> +#include <stdint.h> +#include <sys/mman.h> +#include <pci/pci.h> +#include "flash.h"
+/* Includes from the testsw_ng module */
What's testsw_ng? Drop this comment?
+#include "ichspiacc.h"
+/* HW access functions */
+/* Current flash descriptor, to avoid passing it as argument everywhere */ +static struct flashchip *curflash; +static OPCODES *curopcodes;
+static inline uint8_t REGREAD8(int X) +{
- volatile uint8_t regval;
- regval = *(volatile uint8_t *)(curflash->virtual_registers + X);
- return regval;
+}
+static inline uint16_t REGREAD16(int X) +{
- volatile uint16_t regval;
- regval = *(volatile uint16_t *)(curflash->virtual_registers + X);
- return regval;
+}
+static inline uint32_t REGREAD32(int X) +{
- volatile uint32_t regval;
- regval = *(volatile uint32_t *)(curflash->virtual_registers + X);
- return regval;
+}
+#define REGWRITE32(X,Y) (*(uint32_t *)(curflash->virtual_registers+X)=Y) +#define REGWRITE16(X,Y) (*(uint16_t *)(curflash->virtual_registers+X)=Y) +#define REGWRITE8(X,Y) (*(uint8_t *)(curflash->virtual_registers+X)=Y)
+/* Common SPI functions */ +static int ProgramOpcodes(OPCODES * op); +static int RunOpcode(uint8_t nr, OPCODE op, uint32_t offset, uint8_t datalength,
uint8_t * data);
Please rename the functions to follow our usual coding guidelines is possible, i.e. program_opcodes() etc. See http://www.coreboot.org/Development_Guidelines#Coding_Style http://lxr.linux.no/linux/Documentation/CodingStyle for details.
+static int read_page_ichspi(struct flashchip *flash, uint8_t * buf, int Offset); +static int is_supported_chipset();
+OPCODES O_ST_M25P = {
- {
SPI_ST_M25P_COMMAND_WRITE_ENABLE,0- },
- {
{SPI_ST_M25P_COMMAND_WRITE, SPI_OPCODE_TYPE_WRITE_WITH_ADDRESS, 1}, // Write Byte{SPI_ST_M25P_COMMAND_READ, SPI_OPCODE_TYPE_READ_WITH_ADDRESS, 0}, // Read Data{SPI_ST_M25P_COMMAND_ERASE, SPI_OPCODE_TYPE_WRITE_WITH_ADDRESS, 1}, // Erase 4k Sector{SPI_ST_M25P_COMMAND_READ_STATUS, SPI_OPCODE_TYPE_READ_NO_ADDRESS, 0}, // Read Device Status Reg{SPI_ST_M25P_COMMAND_RES_PD, SPI_OPCODE_TYPE_READ_WITH_ADDRESS, 0}, // Resume Deep Power-Down{SPI_ST_M25P_COMMAND_WRITE_S, SPI_OPCODE_TYPE_WRITE_NO_ADDRESS, 1}, // Write Status Register{SPI_ST_M25P_COMMAND_JDEC, SPI_OPCODE_TYPE_READ_NO_ADDRESS, 0}, // Read JDEC ID{SPI_ST_M25P_COMMAND_BULK_ERASE, SPI_OPCODE_TYPE_WRITE_NO_ADDRESS, 1}, // Bulk erase}+};
+OPCODES O_AT_25DF = {
- {
SPI_ST_M25P_COMMAND_WRITE_ENABLE,0- },
- {
{SPI_ST_M25P_COMMAND_WRITE, SPI_OPCODE_TYPE_WRITE_WITH_ADDRESS, 1}, // Write Byte{SPI_ST_M25P_COMMAND_READ, SPI_OPCODE_TYPE_READ_WITH_ADDRESS, 0}, // Read Data{SPI_ST_M25P_COMMAND_ERASE, SPI_OPCODE_TYPE_WRITE_WITH_ADDRESS, 1}, // Erase Sector{SPI_ST_M25P_COMMAND_READ_STATUS, SPI_OPCODE_TYPE_READ_NO_ADDRESS, 0}, // Read Device Status Reg{SPI_ST_M25P_COMMAND_RES_PD, SPI_OPCODE_TYPE_READ_WITH_ADDRESS, 0}, // Resume Deep Power-Down{SPI_ST_M25P_COMMAND_WRITE_S, SPI_OPCODE_TYPE_WRITE_NO_ADDRESS, 1}, // Write Status Register{SPI_ST_M25P_COMMAND_JDEC, SPI_OPCODE_TYPE_READ_NO_ADDRESS, 0}, // Read JDEC ID{SPI_ST_M25P_COMMAND_BULK_ERASE, SPI_OPCODE_TYPE_WRITE_NO_ADDRESS, 1}, // Bulk erase}+};
+int ProgramOpcodes(OPCODES * op) +{
- uint8_t a;
+#ifdef DEVELOPPERS_DEBUG
- printf_debug("ProgramOpcodes:");
+#endif
I'd change all DEVELOPPERS_DEBUG to only be printed when -V is supplied to flashrom, no need for another #define. I.e. drop DEVELOPPERS_DEBUG.
- uint16_t temp16;
- uint32_t temp32;
- /* Program Prefix Opcodes */
- temp16 = 0;
- /* 0:7 Prefix Opcode 1 */
- temp16 = (op->preop[0]);
- /* 8:16 Prefix Opcode 2 */
- temp16 |= ((uint16_t) op->preop[1]) << 8;
- REGWRITE16(REG_PREOP, temp16);
+#ifdef DEVELOPPERS_DEBUG
- printf("REG_PREOP == 0x%04X (0x%04X)\n", REGREAD16(REG_PREOP), temp16);
+#endif
- /*Program Opcode Types 0 - 7 */
- temp16 = 0;
- for (a = 0; a < 8; a++) {
temp16 |= ((uint16_t) op->opcode[a].spi_type) << (a * 2);- }
- REGWRITE16(REG_OPTYPE, temp16);
+#ifdef DEVELOPPERS_DEBUG
- printf("REG_OPTYPE = 0x%04X (0x%04X)\n", REGREAD16(REG_OPTYPE), temp16);
+#endif
- /*Program Allowable Opcodes 0 - 3 */
- temp32 = 0;
- for (a = 0; a < 4; a++) {
temp32 |= ((uint32_t) op->opcode[a].opcode) << (a * 8);- }
- REGWRITE32(REG_OPMENU, temp32);
- /*Program Allowable Opcodes 4 - 7 */
- temp32 = 0;
- for (a = 4; a < 8; a++) {
temp32 |= ((uint32_t) op->opcode[a].opcode) << ((a - 4) * 8);- }
- REGWRITE32(REG_OPMENU + 4, temp32);
+#ifdef DEVELOPPERS_DEBUG
- printf("REG_OPMENU = 0x%08X%08X\n", REGREAD32(REG_OPMENU + 4),
REGREAD32(REG_OPMENU));- printf("ProgramOpcodes end\n");
+#endif
- return 0;
+}
+int RunOpcode(uint8_t nr, OPCODE op, uint32_t offset, uint8_t datalength,
uint8_t * data)+{ +#ifdef DEVELOPPERS_DEBUG
- printf_debug("RunOpcode::");
+#endif
- int write_cmd = 0;
- uint32_t temp32;
- uint32_t a;
- /* Is it a write command? */
- if ((op.spi_type == SPI_OPCODE_TYPE_WRITE_NO_ADDRESS)
|| (op.spi_type == SPI_OPCODE_TYPE_WRITE_WITH_ADDRESS)) {write_cmd = 1;printf_debug("Write command detected!");- }
- /* Programm Offset in Flash into FADDR */
- REGWRITE32(REG_FADDR, (offset & 0x00FFFFFF)); /*SPI addresses are 24 BIT only */
- printf_debug("REG_FADDR == 0x%08X (0x%08X)", REGREAD32(REG_FADDR),
offset);- /* Program data into FDATA0 to N */
- if (write_cmd && (datalength != 0)) {
printf_debug("Going to write data bytes to FDATA0");temp32 = 0;for (a = 0; a < datalength; a++) {if ((a % 4) == 0) {temp32 = 0;}temp32 |= ((uint32_t) data[a]) << ((a % 4) * 8);if ((a % 4) == 3) {REGWRITE32(REG_FDATA0 + (a - (a % 4)), temp32);printf_debug("REG_FDATA0 + 0x%X) == 0x%08X",a - (a % 4),REGREAD32(REG_FDATA0 +(a - (a % 4))));}}if (((a - 1) % 4) != 3) {printf_debug("writing rest");REGWRITE32(REG_FDATA0 + ((a - 1) - ((a - 1) % 4)),temp32);printf_debug("REG_FDATA0 + 0x%X) == 0x%08X",(a - 1) - ((a - 1) % 4),REGREAD32(REG_FDATA0 +((a - 1) - ((a - 1) % 4))));}- }
- /* Assemble SSFS + SSFC */
- temp32 = 0;
- /* clear error status registers */
- temp32 |= (SSFS_CDS + SSFS_FCERR);
- /* USE 20 MhZ */
- temp32 |= SSFC_SCF_20MHZ;
- if (datalength != 0) {
uint32_t datatemp;temp32 |= SSFC_DS;datatemp = ((uint32_t) ((datalength - 1) & 0x3f)) << (8 + 8);//printf_debug("datatemp = 0x%08X", datatemp);temp32 |= datatemp;- }
- /* Select opcode */
- temp32 |= ((uint32_t) (nr & 0x07)) << (8 + 4);
- /* Handle Atomic */
- if (op.atomic != 0) {
printf_debug("Atomic operation detected");/* Select atomic command */temp32 |= SSFC_ACS;/* Selct prefix opcode */if ((op.atomic - 1) == 1) {/*Select prefix opcode 2 */temp32 |= SSFC_SPOP;}- }
- /* Start */
- temp32 |= SSFC_SCGO;
- /* write it */
- //printf_debug("going to write 0x%08X to REG_SSFS", temp32);
- REGWRITE32(REG_SSFS, temp32);
- /*wait for cycle complete */
- while ((REGREAD32(REG_SSFS) & SSFS_CDS) == 0) {
/*TODO; Do something that this can't lead into an endless loop. but some* commands may cause this to be last more than 30 seconds */- }
- //printf_debug("REG_SSFS == 0x%08X", REGREAD32(REG_SSFS));
- if ((REGREAD32(REG_SSFS) & SSFS_FCERR) != 0) {
printf_debug("Transaction error!");return 1;- }
- if ((!write_cmd) && (datalength != 0)) {
printf_debug("Going to read data from FDATA0");for (a = 0; a < datalength; a++) {if ((a % 4) == 0) {temp32 = REGREAD32(REG_FDATA0 + (a));printf_debug("REG_FDATA0 + 0x%X == 0x%08X", (a),REGREAD32(REG_FDATA0 + (a)));}data[a] =(temp32 & (((uint32_t) 0xff) << ((a % 4) * 8))) >>((a % 4) * 8);}- }
- return 0;
+}
+static volatile uint8_t *mmap_base = 0;
+int map_ich_registers(struct flashchip *flash) +{
- int offset_from_page = (RCRB + SPIBAR) % 0x1000;
- int base_of_page = (RCRB + SPIBAR) & ~0xFFF;
- mmap_base = mmap(0, 0x1000, PROT_WRITE | PROT_READ, MAP_SHARED,
fd_mem, (off_t) base_of_page);- if (mmap_base == MAP_FAILED) {
perror("Can't mmap registers using " MEM_DEV);return -1;- }
- flash->virtual_registers = mmap_base + offset_from_page;
- return 0;
+}
+void munmap_ich_registers(void) +{
- if (mmap_base) {
munmap((void *)mmap_base, 0x1000);mmap_base = 0;- }
+}
+/* check for ICH8/9 chipset */ +static int is_supported_chipset() +{
- struct pci_dev *temp;
- struct pci_filter filter;
- uint8_t tmpid;
- pci_filter_init(NULL, &filter);
- filter.vendor = 0x8086; /* Vendor Intel */
I'd make this...
- for (temp = pacc->devices; temp; temp = temp->next) {
if (pci_filter_match(&filter, temp)) {tmpid = (temp->device_id & 0xff00) >> 8; /* upper byte of device-id *//* return success if ICH8/9 found */if ((tmpid == 0x28) || (tmpid == 0x29))
... and these two values #defines such as
VENDOR_INTEL, SOUTHBRIDGE_ICH8/9 or similar, for better readability.
return 1;}- }
- return 0;
+}
+/* ST-chips with deprecated 'Electronic Signature' and no JEDEC id support */ +/* untested */ +int probe_ichspi_stm25_sig(struct flashchip *flash) +{
- uint8_t byte[65];
- curflash = flash;
- curopcodes = &O_ST_M25P;
- if (!is_supported_chipset())
return 0;- if (map_ich_registers(flash)) {
perror("Error mapping ICH registers\n");return 0;- }
- ProgramOpcodes(curopcodes);
- /* resume from Deep Power-Down mode and read Electronic Signature (4) */
- if (RunOpcode(4, curopcodes->opcode[4], 0x00000000, 1, byte) != 0) {
perror("RunOpcode");munmap_ich_registers();return 0;- }
- printf_debug("ReadID Result %02x", byte[0]);
- /* FIXME: use defines for the signatures */
- if (!
((byte[0] == 0x12) || (byte[0] == 0x13) || (byte[0] == 0x14)|| (byte[0] == 0x15) || (byte[0] == 0x16))) {munmap_ich_registers();return 0;- }
- printf_debug("Chip detected, Electronic Signature 0x%02x\n", byte[0]);
- return 1;
+}
+/* untested */ +int probe_ichspi_stm25(struct flashchip *flash) +{
- uint8_t byte[65];
- uint32_t largeid1;
- curflash = flash;
- curopcodes = &O_ST_M25P;
- if (!is_supported_chipset())
return 0;- if (map_ich_registers(flash)) {
perror("Error mapping ICH registers\n");return 0;- }
- /* initialize controller with opcodes */
- ProgramOpcodes(curopcodes);
- /* resume from Deep Power-Down mode (4) */
- if (RunOpcode(4, curopcodes->opcode[4], 0x00000000, 0, byte) != 0) {
perror("RunOpcode");munmap_ich_registers();return 0;- }
- /* do a read JEDEC id (6) */
- if (RunOpcode(6, curopcodes->opcode[6], 0x00000000, 3, byte) != 0) {
perror("RunOpcode");munmap_ich_registers();return 0;- }
+#ifdef DEVELOPPERS_DEBUG
- printf_debug("JEDEC-ID Result %02x %02x %02x\n", byte[0], byte[1],
byte[2]);+#endif
- if (byte[0] != flash->manufacture_id) {
munmap_ich_registers();return 0;- }
- largeid1 = (byte[1] << 8) | byte[2];
- if (largeid1 != flash->model_id) {
munmap_ich_registers();return 0;- }
- printf_debug("Chip detected, Jedec ID 0x%x\n", largeid1);
- return 1;
+}
+int probe_ichspi_at25df(struct flashchip *flash) +{
- uint8_t byte[65];
- uint32_t largeid1;
- curflash = flash;
- curopcodes = &O_AT_25DF;
- if (!is_supported_chipset())
return 0;- if (map_ich_registers(flash)) {
perror("Error mapping ICH registers\n");return 0;- }
- /* initialize controller with opcodes */
- ProgramOpcodes(curopcodes);
- /* resume from Deep Power-Down mode (4) */
- if (RunOpcode(4, curopcodes->opcode[4], 0x00000000, 0, byte) != 0) {
perror("RunOpcode");munmap_ich_registers();return 0;- }
- /* do a read JEDEC id (6) */
- if (RunOpcode(6, curopcodes->opcode[6], 0x00000000, 3, byte) != 0) {
perror("RunOpcode");munmap_ich_registers();return 0;- }
+#ifdef DEVELOPPERS_DEBUG
- printf_debug("JEDEC-ID Result %02x %02x %02x\n", byte[0], byte[1],
byte[2]);+#endif
- if (byte[0] != flash->manufacture_id) {
munmap_ich_registers();return 0;- }
- largeid1 = (byte[1] << 8) | byte[2];
- if (largeid1 != flash->model_id) {
munmap_ich_registers();return 0;- }
- /* perform global unprotect by writing 0 into whole status reg */
- byte[0] = 0;
- if (RunOpcode(5, curopcodes->opcode[5], 0x00000000, 1, byte) != 0) {
printf_debug("Error global unprotect\n");return -1;- }
- printf_debug("Chip detected, Jedec ID 0x%x\n", largeid1);
- return 1;
+}
+int erase_block_ichspi(struct flashchip *flash, int offset) +{ +#ifdef DEVELOPPERS_DEBUG
- printf("Spi_Erase,Offset=%d,sectors=%d\n", offset, 1);
+#endif
- // TODO: Is this correct ?
- if (RunOpcode(2, curopcodes->opcode[2], offset, 0, NULL) != 0) {
printf_debug("Error erasing sector at 0x%x", offset);return -1;- }
- printf("DONE BLOCK 0x%x\n", offset);
- return 0;
+}
+static int read_page_ichspi(struct flashchip *flash, uint8_t * buf, int Offset) +{
- int page_size = flash->page_size;
- uint32_t remaining = flash->page_size;
- int a;
+#ifdef DEVELOPPERS_DEBUG
- printf("Spi_Read,Offset=%d,number=%d,buf=%p\n", Offset, page_size, buf);
+#endif
- for (a = 0; a < page_size; a += MAXDATABYTES) {
if (remaining < MAXDATABYTES) {if (RunOpcode(1, curopcodes->opcode[1],Offset + (page_size - remaining), remaining,&buf[page_size - remaining]) != 0) {printf_debug("Error reading");return 1;}remaining = 0;} else {if (RunOpcode(1, curopcodes->opcode[1],Offset + (page_size - remaining), MAXDATABYTES,&buf[page_size - remaining]) != 0) {printf_debug("Error reading");return 1;}remaining -= MAXDATABYTES;}- }
- return 0;
+}
+static int write_page_ichspi(struct flashchip *flash, uint8_t * bytes,
int Offset)+{
- int page_size = flash->page_size;
- uint32_t remaining = page_size;
- int a;
+#ifdef DEVELOPPERS_DEBUG
- printf("Spi_Write,Offset=%d,number=%d,buf=%p\n", Offset, page_size,
bytes);+#endif
- for (a = 0; a < page_size; a += MAXDATABYTES) {
if (remaining < MAXDATABYTES) {if (RunOpcode(0, curopcodes->opcode[0],Offset + (page_size - remaining), remaining,&bytes[page_size - remaining]) != 0) {printf_debug("Error writing");return 1;}remaining = 0;} else {if (RunOpcode(0, curopcodes->opcode[0],Offset + (page_size - remaining), MAXDATABYTES,&bytes[page_size - remaining]) != 0) {printf_debug("Error writing");return 1;}remaining -= MAXDATABYTES;}- }
- return 0;
+}
+/* Bulk erase
- */
+int erase_ichspi(struct flashchip *flash) +{
- printf_debug("Bulk erase requested");
- if (RunOpcode(7, curopcodes->opcode[7], 0, 0, NULL) != 0) {
printf_debug("Error erasing sector");return 1;- }
- return 0;
+}
+int read_ichspi(struct flashchip *flash, uint8_t * buf) +{
- int i, rc = 0;
- int total_size = flash->total_size * 1024;
- int page_size = flash->page_size;
- for (i = 0; (i < total_size / page_size) && (rc == 0); i++) {
rc = read_page_ichspi(flash, (void *)(buf + i * page_size),i * page_size);- }
- return rc;
+}
+int write_ichspi(struct flashchip *flash, uint8_t * buf) +{
- int i, rc = 0;
- int total_size = flash->total_size * 1024;
- int page_size = flash->page_size;
- printf("Programming page: \n");
- for (i = 0; (i < total_size / page_size) && (rc == 0); i++) {
printf("\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b");/* Auto Skip Blocks, which already contain the desired data* Faster, because we only write, what has changed* More secure, because blocks, which are excluded* (with the exclude or layout feature)* are not erased and rewritten; data is retained also* in sudden power off situations*/if (!memcmp((void *)(buf + i * page_size),(void *)(flash->virtual_memory + i * page_size),page_size)) {printf("SKIPPED\n");continue;}rc = erase_block_ichspi(flash, i * page_size);if (!rc)write_page_ichspi(flash, (void *)(buf + i * page_size),i * page_size);- }
- printf("\n");
- munmap_ich_registers();
- return rc;
+} Index: ichspiacc.h =================================================================== --- ichspiacc.h (revision 0) +++ ichspiacc.h (revision 0) @@ -0,0 +1,123 @@
Please add the same copyright header here as is the *.c files. Every file should have one.
+#ifndef __ICHSPIACC +#define __ICHSPIACC
+#define RCRB 0x0FED1C000 +#define SPIBAR 0x000003800
+#define MAXDATABYTES 0x40
+// Bit defs +#define BIT00 0x0001 +#define BIT01 0x0002 +#define BIT02 0x0004 +#define BIT03 0x0008 +#define BIT04 0x0010 +#define BIT05 0x0020 +#define BIT06 0x0040 +#define BIT07 0x0080 +#define BIT08 0x0100 +#define BIT09 0x0200 +#define BIT10 0x0400 +#define BIT11 0x0800 +#define BIT12 0x1000 +#define BIT13 0x2000 +#define BIT14 0x4000 +#define BIT15 0x8000 +#define BIT16 0x00010000 +#define BIT17 0x00020000 +#define BIT18 0x00040000 +#define BIT19 0x00080000 +#define BIT20 0x00100000 +#define BIT21 0x00200000 +#define BIT22 0x00400000 +#define BIT23 0x00800000 +#define BIT24 0x01000000 +#define BIT25 0x02000000 +#define BIT26 0x04000000 +#define BIT27 0x08000000 +#define BIT28 0x10000000 +#define BIT29 0x20000000 +#define BIT30 0x40000000 +#define BIT31 0x80000000
This is a bit elaborate. I'd drop it and just use (1 << 5) , (1 << 17) etc. as needed. Besides, they don't seem to be used currently anyway.
+/*ICH9 controller register definition*/ +#define REG_FADDR 0x08 /* 32 Bits */ +#define REG_FDATA0 0x10 /* 64 Bytes */ +#define REG_SSFS 0x90 /* 08 Bits */ +#define SSFS_SCIP 0x00000001 +#define SSFS_CDS 0x00000004 +#define SSFS_FCERR 0x00000008 +#define SSFS_AEL 0x00000010 +#define REG_SSFC 0x91 /* 24 Bits */ +#define SSFC_SCGO 0x00000200 +#define SSFC_ACS 0x00000400 +#define SSFC_SPOP 0x00000800 +#define SSFC_COP 0x00001000 +#define SSFC_DBC 0x00010000 +#define SSFC_DS 0x00400000 +#define SSFC_SME 0x00800000 +#define SSFC_SCF 0x01000000 +#define SSFC_SCF_20MHZ 0x00000000 +#define SSFC_SCF_33MHZ 0x01000000 +#define REG_PREOP 0x94 /* 16 Bits */ +#define REG_OPTYPE 0x96 /* 16 Bits */ +#define REG_OPMENU 0x98 /* 64 BITS */
+// ICH9R SPI commands +#define SPI_OPCODE_TYPE_READ_NO_ADDRESS 0 +#define SPI_OPCODE_TYPE_WRITE_NO_ADDRESS 1 +#define SPI_OPCODE_TYPE_READ_WITH_ADDRESS 2 +#define SPI_OPCODE_TYPE_WRITE_WITH_ADDRESS 3
+// Commands for ST M25Pxx parts +#define SPI_ST_M25P_COMMAND_WRITE 0x02 +#define SPI_ST_M25P_COMMAND_READ 0x03 +#define SPI_ST_M25P_COMMAND_ERASE 0xd8 +#define SPI_ST_M25P_COMMAND_WRITE_DISABLE 0x04 +#define SPI_ST_M25P_COMMAND_READ_STATUS 0x05 +#define SPI_ST_M25P_COMMAND_WRITE_ENABLE 0x06 +#define SPI_ST_M25P_COMMAND_JDEC 0x9F +#define SPI_ST_M25P_COMMAND_RES_PD 0xab +#define SPI_ST_M25P_COMMAND_WRITE_S_EN 0x50 +#define SPI_ST_M25P_COMMAND_WRITE_S 0x01 +#define SPI_ST_M25P_COMMAND_BULK_ERASE 0xc7
+typedef struct _OPCODE {
- uint8_t opcode; //This commands spi opcode
- uint8_t spi_type; //This commands spi type
- uint8_t atomic; //Use preop: (0: none, 1: preop0, 2: preop1
+} OPCODE;
+/* Opcode definition:
- Preop 1: Write Enable
- Preop 2: Write Status register enable
- OP 0: Write address
- OP 1: Read Address
- OP 2: ERASE block
- OP 3: Read Status register
- OP 4: Read ID
- OP 5: Write Status register
- OP 6: chip private (read JDEC id)
- OP 7: Chip erase
- */
+typedef struct _OPCODES {
- uint8_t preop[2];
- OPCODE opcode[8];
+} OPCODES;
+typedef struct _SPIFLASH {
- const char *name;
- OPCODES opcodes;
- uint32_t pagesize;
- uint32_t size;
+#if 0
- int (*erase) (struct _SPIFLASH * myflash, uint32 Offset, uint32 sectors)
- int (*write) (struct _SPIFLASH * myflash, uint32 Offset, uint32 number,
uint8_t * bytes)- int (*read) (struct _SPIFLASH * myflash, uint32 Offset, uint32 number,
uint8_t * bytes)+#endif +} SPIFLASH;
+#endif
Uwe.
On Tue, May 13, 2008 at 01:56:38PM +0200, Uwe Hermann wrote:
- {"ST", "M25P40@ICH9", ST_ID, ST_M25P40, 512, 64 * 1024, TEST_UNTESTED, probe_ichspi_stm25_sig, erase_ichspi, write_ichspi, read_ichspi},
- {"ST", "M25P80@ICH9", ST_ID, ST_M25P80, 1024, 64 * 1024, TEST_UNTESTED, probe_ichspi_stm25_sig, erase_ichspi, write_ichspi, read_ichspi},
- {"ST", "M25P16@ICH9", ST_ID, ST_M25P16, 2048, 64 * 1024, TEST_UNTESTED, probe_ichspi_stm25, erase_ichspi, write_ichspi, read_ichspi},
- {"ST", "M25P32@ICH9", ST_ID, ST_M25P32, 4096, 64 * 1024, TEST_UNTESTED, probe_ichspi_stm25, erase_ichspi, write_ichspi, read_ichspi},
- {"ST", "M25P64@ICH9", ST_ID, ST_M25P64, 8192, 64 * 1024, TEST_UNTESTED, probe_ichspi_stm25, erase_ichspi, write_ichspi, read_ichspi},
Why "@ICH9", does the chip behave differently depending on where you plug it in?
How to program an SPI flash chip depends on both the chip type, and the SPI master type. (Ie which superio/southbridge it is connected to.)
Currently, flashrom does not have infrastructure that can deal with a non-transparent flash memory bus master, so the support for these flash chips that Claus is proposing will only work on ICH9 systems.
This sucks of course, but it is a flaw of flashrom and I do think it is OK to include Claus' changes because they do work for him.
It is too much to expect a brand new contributor to rewrite the SPI infrastructure in flashrom. That will come, I know Carl-Daniel has ideas.
Index: ichspi.c
..
- Initially implemented by Stefan Wildemann stefan.wildemann@kontron.com
- Adapted to flashrom by Claus Gindhart claus.gindhart@kontron.com
Please change these two lines to the usual Copyright lines, i.e.
Copyright (C) 200x Stefan Wildemann stefan.wildemann@kontron.com Copyright (C) 200y Claus Gindhart claus.gindhart@kontron.com
replacing the 200x/200y years appropriately.
Good point.
+#undef DEVELOPPERS_DEBUG
I'd drop this, see below.
Yes, I agree.
Index: ichspiacc.h
..
Please add the same copyright header here as is the *.c files. Every file should have one.
Yep.
+// Bit defs +#define BIT00 0x0001
..
+#define BIT31 0x80000000
This is a bit elaborate. I'd drop it and just use (1 << 5) , (1 << 17) etc. as needed. Besides, they don't seem to be used currently anyway.
Hehe, nice one. I missed that.
//Peter
On 13.05.2008 15:14, Peter Stuge wrote:
On Tue, May 13, 2008 at 01:56:38PM +0200, Uwe Hermann wrote:
- {"ST", "M25P40@ICH9", ST_ID, ST_M25P40, 512, 64 * 1024, TEST_UNTESTED, probe_ichspi_stm25_sig, erase_ichspi, write_ichspi, read_ichspi},
- {"ST", "M25P80@ICH9", ST_ID, ST_M25P80, 1024, 64 * 1024, TEST_UNTESTED, probe_ichspi_stm25_sig, erase_ichspi, write_ichspi, read_ichspi},
- {"ST", "M25P16@ICH9", ST_ID, ST_M25P16, 2048, 64 * 1024, TEST_UNTESTED, probe_ichspi_stm25, erase_ichspi, write_ichspi, read_ichspi},
- {"ST", "M25P32@ICH9", ST_ID, ST_M25P32, 4096, 64 * 1024, TEST_UNTESTED, probe_ichspi_stm25, erase_ichspi, write_ichspi, read_ichspi},
- {"ST", "M25P64@ICH9", ST_ID, ST_M25P64, 8192, 64 * 1024, TEST_UNTESTED, probe_ichspi_stm25, erase_ichspi, write_ichspi, read_ichspi},
Why "@ICH9", does the chip behave differently depending on where you plug it in?
How to program an SPI flash chip depends on both the chip type, and the SPI master type. (Ie which superio/southbridge it is connected to.)
Currently, flashrom does not have infrastructure that can deal with a non-transparent flash memory bus master, so the support for these flash chips that Claus is proposing will only work on ICH9 systems.
This sucks of course, but it is a flaw of flashrom and I do think it is OK to include Claus' changes because they do work for him.
It is too much to expect a brand new contributor to rewrite the SPI infrastructure in flashrom. That will come, I know Carl-Daniel has ideas.
Once the RES support patch is merged (I'm waiting for Fredrik Tolf to signoff it and Peter Stuge to ack it) and we have a deal about the "flags" field in struct flashchip, it will be easy to integrate ICH9 support into flashrom.
Regards, Carl-Daniel
-
Carl-Daniel Hailfinger -
Claus Gindhart -
Peter Stuge -
Uwe Hermann