[LinuxBIOS] [PATCH] improved SPI flash support (restructured)

Uwe Hermann uwe at hermann-uwe.de
Wed Oct 3 00:17:02 CEST 2007


On Mon, Oct 01, 2007 at 02:53:35PM +0200, Carl-Daniel Hailfinger wrote:
> Index: util/flashrom/flash.h
> ===================================================================
> --- util/flashrom/flash.h	(Revision 2814)
> +++ util/flashrom/flash.h	(Arbeitskopie)
> @@ -55,6 +55,8 @@
>  /* Please keep this list sorted alphabetically by manufacturer. The first
>   * entry of each section should be the manufacturer ID, followed by the
>   * list of devices from that manufacturer (sorted by device IDs).
> + * All LPC/FWH parts (parallel flash) have 8-bit device IDs.
> + * All SPI parts have 16-bit device IDs.
>   */
>  
>  #define AMD_ID			0x01	/* AMD */
> @@ -68,8 +70,31 @@
>  #define AT_29C040A		0xA4
>  #define AT_29C020		0xDA
>  
> +#define EON_ID			0x1C
> +/* EN25 chips are SPI, first byte of device id is memory type,
> +   second byte of device id is log(bitsize)-9 */

id -> ID


> +#define EN_25B05		0x2010	/* 2^19 kbit or 2^16 kByte */
> +#define EN_25B10		0x2011
> +#define EN_25B20		0x2012
> +#define EN_25B40		0x2013
> +#define EN_25B80		0x2014
> +#define EN_25B16		0x2015
> +#define EN_25B32		0x2016
> +
>  #define MX_ID			0xC2	/* Macronix (MX) */
>  #define MX_29F002		0xB0
> +/* MX25L chips are SPI, first byte of device id is memory type,
> +   second byte of device id is log(bitsize)-9 */

id -> ID


> +#define MX_25L512		0x2010	/* 2^19 kbit or 2^16 kByte */
> +#define MX_25L1005		0x2011
> +#define MX_25L2005		0x2012
> +#define MX_25L4005		0x2013	/* MX25L4005{,A} */
> +#define MX_25L8005		0x2014
> +#define MX_25L1605		0x2015	/* MX25L1605{,A,D} */
> +#define MX_25L3205		0x2016	/* MX25L3205{,A} */
> +#define MX_25L6405		0x2017	/* MX25L3205{,D} */
> +#define MX_25L1635D		0x2415
> +#define MX_25L3235D		0x2416
>  
>  #define SHARP_ID		0xB0	/* Sharp */
>  #define SHARP_LHF00L04		0xCF
> @@ -182,6 +207,8 @@
>  int linuxbios_init(void);
>  extern char *lb_part, *lb_vendor;
>  
> +int probe_spi(struct flashchip *flash);
> +
>  /* 82802ab.c */
>  int probe_82802ab(struct flashchip *flash);
>  int erase_82802ab(struct flashchip *flash);
> Index: util/flashrom/board_enable.c
> ===================================================================
> --- util/flashrom/board_enable.c	(Revision 2814)
> +++ util/flashrom/board_enable.c	(Arbeitskopie)
> @@ -30,6 +30,15 @@
>  #include <string.h>
>  #include "flash.h"
>  
> +#define ITE_SUPERIO_PORT1	0x2e
> +#define ITE_SUPERIO_PORT2	0x4e
> +
> +#define JEDEC_RDID	{0x9f}
> +#define JEDEC_RDID_OUTSIZE	0x01
> +#define JEDEC_RDID_INSIZE	0x03
> +
> +static uint16_t it8716f_flashport = 0;
> +
>  /* Generic Super I/O helper functions */
>  uint8_t regval(uint16_t port, uint8_t reg)
>  {
> @@ -51,7 +60,7 @@
>  	outb(0x87, port);
>  	outb(0x01, port);
>  	outb(0x55, port);
> -	if (port == 0x2e)
> +	if (port == ITE_SUPERIO_PORT1)
>  		outb(0x55, port);
>  	else
>  		outb(0xaa, port);
> @@ -96,35 +105,97 @@
>  	return flashport;
>  }
>  
> -static void it8716_serial_rdid(uint16_t port)
> +/* The IT8716F only supports commands with length 1,2,4,5 bytes including
> +   command byte and can not read more than 3 bytes from the device.
> +   This function expects writearr[0] to be the first byte sent to the device,
> +   whereas the IT8716F splits commands internally into address and non-address
> +   commands with the address in inverse wire order. That's why the register
> +   ordering in case 4 and 5 may seem strange. */

Please make all multi-line comments look the same as in the rest of the
code:

/* Foo
 * Bar
 */

Or, if we want to use the exact Linux-style:

/*
 * Foo
 * Bar
 */


> +static int it8716f_spi_command(uint16_t port, unsigned char writecnt, unsigned char readcnt, const unsigned char *writearr, unsigned char *readarr)

As per another mail, writecnt/readcnt/writearr/readarr seems to have a
dont-care size, so I think we can make them all 'int' (or 'unsigned int'
if they can't or shouldn't get negative).

I think using the "native" int type is the best solution in general, it
gives the compiler maximum freedom to optimize (not that it would matter
much here).


> Index: util/flashrom/flashchips.c
> ===================================================================
> --- util/flashrom/flashchips.c	(Revision 2814)
> +++ util/flashrom/flashchips.c	(Arbeitskopie)
> @@ -38,6 +38,8 @@
>  	 probe_jedec,	erase_chip_jedec, write_jedec},
>  	{"Mx29f002",	MX_ID,		MX_29F002,	256, 64 * 1024,
>  	 probe_29f002,	erase_29f002, 	write_29f002},
> +	{"MX25L4005",	MX_ID,		MX_25L4005,	512, 4 * 1024,
> +	 probe_spi,	NULL,		NULL},

All the other chips from above are not added?


>  	{"SST29EE020A", SST_ID,		SST_29EE020A,	256, 128,
>  	 probe_jedec,	erase_chip_jedec, write_jedec},
>  	{"SST28SF040A", SST_ID,		SST_28SF040,	512, 256,


With the above fixes:
Acked-by: Uwe Hermann <uwe at hermann-uwe.de>


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20071003/a0e04a46/attachment.sig>


More information about the coreboot mailing list