On 28.09.2007 16:55, Uwe Hermann wrote:
On Fri, Sep 28, 2007 at 03:35:54PM +0200, Carl-Daniel Hailfinger wrote:
On 27.09.2007 16:30, Ward Vandewege wrote:
On Thu, Sep 27, 2007 at 11:31:14AM +0200, Carl-Daniel Hailfinger wrote:
No problem. Can you commit with your changes? Changelog follows:
Add preliminary SPI flash identification support for SPI chips attached to ITE IT8716F Super I/O. Right now this is hardcoded to the Gigabyte M57SLI board. It works only with rev 2.0 of the board, but it will bail out on earlier versions, so no damage can occur.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Acked-by: Ward Vandewege ward@gnu.org
and committed in r2811
Thanks for committing!
Here is a patch against current svn which aims to restructure SPI flash support in a more reasonable way. Please note that the patch is not ready to be committed, but it should work much better than the existing
Yep, Signed-off-by missing for this one (yeah, there's one for the older version, but still)...
Intentional. Was not ready to be applied.
one, namely without the dirty "do everything in board_enable functions" hack. Functions still have to be moved to another file, but for initial testing, this should be fine.
Regards, Carl-Daniel
Index: util/flashrom/flash.h
--- util/flashrom/flash.h (Revision 2811) +++ util/flashrom/flash.h (Arbeitskopie) @@ -68,8 +68,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 */
+#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 */
+#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
Maybe we should separate out the IDs for SPI chips in a different section of the header file (to not confuse people into assuing they're LPC parts or so)?
All SPI parts have 16-bit device IDs, I'll add a comment about that.
#define SHARP_ID 0xB0 /* Sharp */ #define SHARP_LHF00L04 0xCF @@ -182,6 +205,8 @@ int linuxbios_init(void); extern char *lb_part, *lb_vendor;
+int probe_spi(struct flashchip *flash);
Can be int probe_spi(const struct flashchip *flash); I think.
Maybe, but that's a separate cleanup. This function prototype matches the others.
Index: util/flashrom/board_enable.c
--- util/flashrom/board_enable.c (Revision 2811) +++ util/flashrom/board_enable.c (Arbeitskopie) @@ -96,35 +96,100 @@ return flashport; }
-static void it8716_serial_rdid(uint16_t port) +/* The IT8716 only supports commands with length 1,2,4,5 bytes including
IT8716F please, as per datasheet. AFAICS all ITEs should have the F at the end of the name. Applies to several places in the code and comments.
OK.
- 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 IT8716 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. */
+static int it8716_spi_command(uint16_t port, unsigned char writecnt, unsigned char readcnt, const unsigned char *writearr, unsigned char *readarr)
If the size of writecnt/readcnt/writearr/readarr etc. is fixed/specified in the datasheet then please make it uint16_t etc. instead of the "generic" unsigned char and friends.
{read,write}cnt are between 0 and 5, so they could as well be int. No datasheet specification for their variable size. {read,write}arr are just helper constructs to abstract the horrible datasheet variable types. Or do you honestly want a 24-bit type?
+static int it8716_serial_rdid(uint16_t port, unsigned char *readarr) +{
- /* RDID is 0x9f */
- const unsigned char cmd[] = {0x9f};
#define RDID 0x9f
at the top of the file?
Can do, but it will get messy very soon because there is no way to sensibly specify command value, input size, output size in one #define. Furthermore, depending on the state of the chip, the commands change values/input/output size. So I'd prefer to keep it that way right now until we figure out a better abstraction which doesn't complicate stuff.
- if (it8716_spi_command(port, 1, 3, cmd, readarr))
return 1;- printf("RDID returned %02x %02x %02x\n", readarr[0], readarr[1], readarr[2]);
- return 0;
+}
+static uint16_t it8716_flashport = 0;
Move this to the top of the file, please.
OK.
static int it87xx_probe_serial_flash(const char *name) {
- uint16_t flashport;
- flashport = find_ite_serial_flash_port(0x2e);
- if (flashport)
it8716_serial_rdid(flashport);- flashport = find_ite_serial_flash_port(0x4e);
- if (flashport)
it8716_serial_rdid(flashport);
- it8716_flashport = find_ite_serial_flash_port(0x2e);
- if (!it8716_flashport)
it8716_flashport = find_ite_serial_flash_port(0x4e);#defines for 0x2e and 0x4e.
OK.
- return (!it8716_flashport);
+}
+int probe_spi(struct flashchip *flash) +{
- unsigned char readarr[3];
- uint8_t manuf_id;
- uint16_t model_id;
- if (it8716_flashport) {
it8716_serial_rdid(it8716_flashport, readarr);manuf_id = readarr[0];model_id = (readarr[1] << 8) | readarr[2];printf_debug("%s: id1 0x%x, id2 0x%x\n", __FUNCTION__, manuf_id, model_id);if (manuf_id == flash->manufacture_id && model_id == flash->model_id)return 1;- }
- return 0;
}
Looks good otherwise, but should be tested of course.
Yes, looking for testers.
Carl-Daniel