Various IT85* cleanups and fixes.
Fix a few typos. Change the EC memory region mapping name. Drop unused function parameter. Use mmio_writeb()/mmio_readb() to get reliable access to volatile memory locations instead of plain pointer access which gets optimized away by gcc. Use own it85_* SPI high-level chip read/write functions instead of relying on unrelated ICH functions.
AFAICS this should be mergeable both in the chromium tree and vanilla flashrom.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-it85spi_cleanup/it85spi.c =================================================================== --- flashrom-it85spi_cleanup/it85spi.c (Revision 1263) +++ flashrom-it85spi_cleanup/it85spi.c (Arbeitskopie) @@ -37,7 +37,7 @@ #define MAX_TIMEOUT 100000 #define MAX_TRY 5
-/* Constans for I/O ports */ +/* Constants for I/O ports */ #define ITE_SUPERIO_PORT1 0x2e #define ITE_SUPERIO_PORT2 0x4e
@@ -52,13 +52,14 @@ #define CHIP_CHIP_VER_REG 0x22
/* These are standard Super I/O 16-bit base address registers */ -#define SHM_IO_BAD0 0x60 /* big-endian, this is high bits */ -#define SHM_IO_BAD1 0x61 +#define SHM_IO_BAR0 0x60 /* big-endian, this is high bits */ +#define SHM_IO_BAR1 0x61
-/* 8042 keyboard controller uses an input buffer and an output buffer to - * communicate with host CPU. Both buffers are 1-byte depth. That means the - * IBF is set to 1 when host CPU sends a command to input buffer (standing on - * the EC side). IBF is cleared to 0 once the command is read by EC. */ +/* The 8042 keyboard controller uses an input buffer and an output buffer to + * communicate with the host CPU. Both buffers are 1-byte depth. That means + * IBF is set to 1 when the host CPU sends a command to the input buffer + * of the EC. IBF is cleared to 0 once the command is read by the EC. + */ #define KB_IBF (1 << 1) /* Input Buffer Full */ #define KB_OBF (1 << 0) /* Output Buffer Full */
@@ -278,8 +279,8 @@ #ifdef LPC_IO /* Get LPCPNP of SHM. That's big-endian */ sio_write(superio.port, LDNSEL, 0x0F); /* Set LDN to SHM (0x0F) */ - shm_io_base = (sio_read(superio.port, SHM_IO_BAD0) << 8) + - sio_read(superio.port, SHM_IO_BAD1); + shm_io_base = (sio_read(superio.port, SHM_IO_BAR0) << 8) + + sio_read(superio.port, SHM_IO_BAR1); msg_pdbg("%s():%d shm_io_base=0x%04x\n", __func__, __LINE__, shm_io_base);
@@ -296,8 +297,8 @@ INDIRECT_A3(shm_io_base, (base >> 24)); #endif #ifdef LPC_MEMORY - base = (chipaddr)programmer_map_flash_region("flash base", 0xFFFFF000, - 0x1000); + base = (chipaddr)programmer_map_flash_region("it85 communication", + 0xFFFFF000, 0x1000); msg_pdbg("%s():%d base=0x%08x\n", __func__, __LINE__, (unsigned int)base); ce_high = (unsigned char*)(base + 0xE00); /* 0xFFFFFE00 */ @@ -328,7 +329,7 @@ }
/* Called by internal_init() */ -int it85xx_probe_spi_flash(const char *name) +int it85xx_probe_spi_flash(void) { int ret;
@@ -377,14 +378,14 @@ INDIRECT_A1(shm_io_base, (((unsigned long int)ce_low) >> 8) & 0xff); #endif #ifdef LPC_MEMORY - *ce_high = 0; + mmio_writeb(0, ce_high); #endif for (i = 0; i < writecnt; ++i) { #ifdef LPC_IO INDIRECT_WRITE(shm_io_base, writearr[i]); #endif #ifdef LPC_MEMORY - *ce_low = writearr[i]; + mmio_writeb(writearr[i], ce_low); #endif } for (i = 0; i < readcnt; ++i) { @@ -392,7 +393,7 @@ readarr[i] = INDIRECT_READ(shm_io_base); #endif #ifdef LPC_MEMORY - readarr[i] = *ce_low; + readarr[i] = mmio_readb(ce_low); #endif } #ifdef LPC_IO @@ -400,10 +401,20 @@ INDIRECT_WRITE(shm_io_base, 0xFF); /* Write anything to this address.*/ #endif #ifdef LPC_MEMORY - *ce_high = 0; + mmio_writeb(0, ce_high); #endif
return 0; }
+int it85_spi_read(struct flashchip *flash, uint8_t * buf, int start, int len) +{ + return spi_read_chunked(flash, buf, start, len, 64); +} + +int it85_spi_write_256(struct flashchip *flash, uint8_t * buf, int start, int len) +{ + return spi_write_chunked(flash, buf, start, len, 64); +} + #endif Index: flashrom-it85spi_cleanup/spi.c =================================================================== --- flashrom-it85spi_cleanup/spi.c (Revision 1263) +++ flashrom-it85spi_cleanup/spi.c (Arbeitskopie) @@ -58,8 +58,8 @@ { /* SPI_CONTROLLER_IT85XX */ .command = it85xx_spi_send_command, .multicommand = default_spi_send_multicommand, - .read = ich_spi_read, - .write_256 = ich_spi_write_256, + .read = it85_spi_read, + .write_256 = it85_spi_write_256, },
{ /* SPI_CONTROLLER_IT87XX */ Index: flashrom-it85spi_cleanup/programmer.h =================================================================== --- flashrom-it85spi_cleanup/programmer.h (Revision 1263) +++ flashrom-it85spi_cleanup/programmer.h (Arbeitskopie) @@ -588,9 +588,11 @@ struct superio probe_superio_ite85xx(void); int it85xx_spi_init(void); int it85xx_shutdown(void); -int it85xx_probe_spi_flash(const char *name); +int it85xx_probe_spi_flash(void); int it85xx_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); +int it85_spi_read(struct flashchip *flash, uint8_t * buf, int start, int len); +int it85_spi_write_256(struct flashchip *flash, uint8_t * buf, int start, int len);
/* it87spi.c */ void enter_conf_mode_ite(uint16_t port);
Looks great! I applied your patch against the Chromium OS branch and successfully tested read and write operations on a Cr48.
Acked-by: David Hendricks dhendrix@google.com
On Thu, Mar 3, 2011 at 4:38 PM, Carl-Daniel Hailfinger < c-d.hailfinger.devel.2006@gmx.net> wrote:
Various IT85* cleanups and fixes.
Fix a few typos. Change the EC memory region mapping name. Drop unused function parameter. Use mmio_writeb()/mmio_readb() to get reliable access to volatile memory locations instead of plain pointer access which gets optimized away by gcc. Use own it85_* SPI high-level chip read/write functions instead of relying on unrelated ICH functions.
AFAICS this should be mergeable both in the chromium tree and vanilla flashrom.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-it85spi_cleanup/it85spi.c
--- flashrom-it85spi_cleanup/it85spi.c (Revision 1263) +++ flashrom-it85spi_cleanup/it85spi.c (Arbeitskopie) @@ -37,7 +37,7 @@ #define MAX_TIMEOUT 100000 #define MAX_TRY 5
-/* Constans for I/O ports */ +/* Constants for I/O ports */ #define ITE_SUPERIO_PORT1 0x2e #define ITE_SUPERIO_PORT2 0x4e
@@ -52,13 +52,14 @@ #define CHIP_CHIP_VER_REG 0x22
/* These are standard Super I/O 16-bit base address registers */ -#define SHM_IO_BAD0 0x60 /* big-endian, this is high bits */ -#define SHM_IO_BAD1 0x61 +#define SHM_IO_BAR0 0x60 /* big-endian, this is high bits */ +#define SHM_IO_BAR1 0x61
-/* 8042 keyboard controller uses an input buffer and an output buffer to
- communicate with host CPU. Both buffers are 1-byte depth. That means
the
- IBF is set to 1 when host CPU sends a command to input buffer (standing
on
- the EC side). IBF is cleared to 0 once the command is read by EC. */
+/* The 8042 keyboard controller uses an input buffer and an output buffer to
- communicate with the host CPU. Both buffers are 1-byte depth. That
means
- IBF is set to 1 when the host CPU sends a command to the input buffer
- of the EC. IBF is cleared to 0 once the command is read by the EC.
- */
#define KB_IBF (1 << 1) /* Input Buffer Full */ #define KB_OBF (1 << 0) /* Output Buffer Full */
@@ -278,8 +279,8 @@ #ifdef LPC_IO /* Get LPCPNP of SHM. That's big-endian */ sio_write(superio.port, LDNSEL, 0x0F); /* Set LDN to SHM (0x0F) */
shm_io_base = (sio_read(superio.port, SHM_IO_BAD0) << 8) +sio_read(superio.port, SHM_IO_BAD1);
shm_io_base = (sio_read(superio.port, SHM_IO_BAR0) << 8) +sio_read(superio.port, SHM_IO_BAR1); msg_pdbg("%s():%d shm_io_base=0x%04x\n", __func__, __LINE__, shm_io_base);@@ -296,8 +297,8 @@ INDIRECT_A3(shm_io_base, (base >> 24)); #endif #ifdef LPC_MEMORY
base = (chipaddr)programmer_map_flash_region("flash base",0xFFFFF000,
0x1000);
base = (chipaddr)programmer_map_flash_region("it85 communication",0xFFFFF000, 0x1000); msg_pdbg("%s():%d base=0x%08x\n", __func__, __LINE__, (unsigned int)base); ce_high = (unsigned char*)(base + 0xE00); /* 0xFFFFFE00 */@@ -328,7 +329,7 @@ }
/* Called by internal_init() */ -int it85xx_probe_spi_flash(const char *name) +int it85xx_probe_spi_flash(void) { int ret;
@@ -377,14 +378,14 @@ INDIRECT_A1(shm_io_base, (((unsigned long int)ce_low) >> 8) & 0xff); #endif #ifdef LPC_MEMORY
*ce_high = 0;
mmio_writeb(0, ce_high);#endif for (i = 0; i < writecnt; ++i) { #ifdef LPC_IO INDIRECT_WRITE(shm_io_base, writearr[i]); #endif #ifdef LPC_MEMORY
*ce_low = writearr[i];
mmio_writeb(writearr[i], ce_low);#endif } for (i = 0; i < readcnt; ++i) { @@ -392,7 +393,7 @@ readarr[i] = INDIRECT_READ(shm_io_base); #endif #ifdef LPC_MEMORY
readarr[i] = *ce_low;
readarr[i] = mmio_readb(ce_low);#endif } #ifdef LPC_IO @@ -400,10 +401,20 @@ INDIRECT_WRITE(shm_io_base, 0xFF); /* Write anything to this address.*/ #endif #ifdef LPC_MEMORY
*ce_high = 0;
mmio_writeb(0, ce_high);#endif
return 0;}
+int it85_spi_read(struct flashchip *flash, uint8_t * buf, int start, int len) +{
return spi_read_chunked(flash, buf, start, len, 64);+}
+int it85_spi_write_256(struct flashchip *flash, uint8_t * buf, int start, int len) +{
return spi_write_chunked(flash, buf, start, len, 64);+}
#endif Index: flashrom-it85spi_cleanup/spi.c =================================================================== --- flashrom-it85spi_cleanup/spi.c (Revision 1263) +++ flashrom-it85spi_cleanup/spi.c (Arbeitskopie) @@ -58,8 +58,8 @@ { /* SPI_CONTROLLER_IT85XX */ .command = it85xx_spi_send_command, .multicommand = default_spi_send_multicommand,
.read = ich_spi_read,.write_256 = ich_spi_write_256,
.read = it85_spi_read,.write_256 = it85_spi_write_256, }, { /* SPI_CONTROLLER_IT87XX */Index: flashrom-it85spi_cleanup/programmer.h
--- flashrom-it85spi_cleanup/programmer.h (Revision 1263) +++ flashrom-it85spi_cleanup/programmer.h (Arbeitskopie) @@ -588,9 +588,11 @@ struct superio probe_superio_ite85xx(void); int it85xx_spi_init(void); int it85xx_shutdown(void); -int it85xx_probe_spi_flash(const char *name); +int it85xx_probe_spi_flash(void); int it85xx_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr); +int it85_spi_read(struct flashchip *flash, uint8_t * buf, int start, int len); +int it85_spi_write_256(struct flashchip *flash, uint8_t * buf, int start, int len);
/* it87spi.c */ void enter_conf_mode_ite(uint16_t port);
Auf 08.03.2011 00:18, David Hendricks schrieb:
Looks great! I applied your patch against the Chromium OS branch and successfully tested read and write operations on a Cr48.
Acked-by: David Hendricks dhendrix@google.com
Thanks for the review! Committed in r1279.
Regards, Carl-Daniel
Hi David,
Auf 08.03.2011 00:18, David Hendricks schrieb:
tested read and write operations on a Cr48.
While you have that Cr-48 nearby, could you please run lspci -nnvvvxxx superiotool -deV dmidecode
Is there an EFI equivalent of dmidecode? If yes, please run that as well in verbose mode. If you know of any other way to detect that we're running on a Cr-48, please tell us.
Regards, Carl-Daniel
On Mon, Mar 7, 2011 at 4:43 PM, Carl-Daniel Hailfinger < c-d.hailfinger.devel.2006@gmx.net> wrote:
Hi David,
Auf 08.03.2011 00:18, David Hendricks schrieb:
tested read and write operations on a Cr48.
While you have that Cr-48 nearby, could you please run lspci -nnvvvxxx superiotool -deV dmidecode
Attached.
The SMBIOS stuff is a bit tricky. I ran into dmidecode issues due to some EFI weirdness (we discussed this on IRC a while back), so instead I dumped the raw SMBIOS tables from memory and faked the entry point structure which is prepended in smbios_raw.bin. Newer versions of dmidecode (I'm using 2.11) have a --from-dump option you may use to get the fully decoded output from the binary file. As you might expect, about the only useful info is the type 1 table's manufacturer (IEC) and product name (Mario). The BIOS on this particular machine is non-production, so the serial numbers and stuff will look weird.
On Mon, Mar 7, 2011 at 4:43 PM, Carl-Daniel Hailfinger < c-d.hailfinger.devel.2006@gmx.net> wrote:
Is there an EFI equivalent of dmidecode? If yes, please run that as well in verbose mode.
dmidecode works with EFI, however there is an interesting constraint -- the efi_vars kernel module will explode if the BIOS is 64-bit and the kernel is 32-bit (more details here: http://code.google.com/p/chromium-os/issues/detail?id=3146)
-
Carl-Daniel Hailfinger -
David Hendricks