Re: [flashrom] [PATCH 3/3] ENE Embedded Debug Interface (EDI) and ENE KB9012 EC internal flash support

[Paul Kocialkowski]

Hi Nico,

Le mercredi 14 octobre 2015 à 23:39 +0200, Nico Huber a écrit :
> On 10.10.2015 16:20, Paul Kocialkowski wrote:
> > The ENE Embedded Debug Interface (EDI) is a SPI-based interface for
> > accessing
> > the memory of ENE embedded controllers.
> >
> > The ENE KB9012 EC is an embedded controller found on various
> > laptops such as
> > the Lenovo G505s. It features a 8051 microcontroller and has 128
> > KiB of internal
> > storage for program data.
> >
> > EDI can be accessed on the KB9012 through pins 59-62 (CS-CLK-MOSI
> > -MISO) when
> > flash direct access is not in use. Some firmwares disable EDI at
> > run-time, so
> > it might be necessary to ground pin 42 to reset the 8051
> > microcontroller before
> > accessing the KB9012 via EDI.
> >
> > Signed-off-by: Paul Kocialkowski
>
> Thank you for taking the time to write a clean implementation. I had
> a
> good time reading it and learning about that EDI protocol :)
> I don't know the hardware, so I've only commented on general stuff.

Thanks for reviewing this!

The EDI protocol is described in the KB9012 datasheet, application
apendix A.5 Embedded Flash Brief Description and the internal SPI
controller in part 4.14 X-Bus Interface (XBI).
> It looks good on the easy to test, positive paths. Failure handling 
> OTOH needs more work.

I will probably send v2 right away, feel free to follow up the
discussion on some of these comments in there, it'll probably be
easier.
> Nico
> > Makefile      |   2 +-
> >  chipdrivers.h |   6 +
> >  edi.c         | 418
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  edi.h         |  34 +++++
> >  ene.h         |  55 ++++++++
> >  flashchips.c  |  23 ++++
> >  6 files changed, 537 insertions(+), 1 deletion(-)
> >  create mode 100644 edi.c
> >  create mode 100644 edi.h
> >  create mode 100644 ene.h
> >
> > diff --git a/Makefile b/Makefile
> > index c439d8d..661c52a 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -368,7 +368,7 @@ endif
> >
> > CHIP_OBJS = jedec.o stm50.o w39.o w29ee011.o \
> >    sst28sf040.o 82802ab.o \
> > sst49lfxxxc.o sst_fwhub.o flashchips.o spi.o spi25.o
> >
> > spi25_statusreg.o \
> > sst49lfxxxc.o sst_fwhub.o edi.o flashchips.o spi.o spi25.o
> >
> > spi25_statusreg.o \
> >    opaque.o sfdp.o en29lv640b.o at45db.o
> >
> > ##################################################################
> > #############
> > diff --git a/chipdrivers.h b/chipdrivers.h
> > index cac94f3..8015b52 100644
> > --- a/chipdrivers.h
> > +++ b/chipdrivers.h
> > @@ -194,4 +194,10 @@ int erase_sector_stm50(struct flashctx *flash,
> > unsigned int block, unsigned int
> >  int probe_en29lv640b(struct flashctx *flash);
> >  int write_en29lv640b(struct flashctx *flash, const uint8_t *buf,
> > unsigned int start, unsigned int len);
> >
> > +/* edi.c */
> > +int edi_chip_block_erase(struct flashctx *flash, unsigned int
> > page, unsigned int size);
> > +int edi_chip_write(struct flashctx *flash, const uint8_t *buf,
> > unsigned int start, unsigned int len);
> > +int edi_chip_read(struct flashctx *flash, uint8_t *buf, unsigned
> > int start, unsigned int len);
> > +int edi_probe_kb9012(struct flashctx *flash);
> >
> > #endif /* !__CHIPDRIVERS_H__ */
> > diff --git a/edi.c b/edi.c
> > new file mode 100644
> > index 0000000..a3e0539
> > --- /dev/null
> > +++ b/edi.c
> > @@ -0,0 +1,418 @@
> > +/*
> > This file is part of the flashrom project.
> > Copyright (C) 2015 Paul Kocialkowski
> > 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
>
> Please drop that last paragraph from all new files. FSF's address
> used
> to change and might again.

Right, perhaps it would be worth fixing this for each flashrom file,
too.
> > */
> >
> > +#include <string.h>
> > +#include "flash.h"
> > +#include "ene.h"
> > +#include "edi.h"
> >
> > +static unsigned int edi_read_buffer_length =
> > EDI_READ_BUFFER_LENGTH_DEFAULT;
> >
> > +static struct ene_chip ene_kb9012 = {
> > .hwversion = ENE_KB9012_HWVERSION,
> > .ediid = ENE_KB9012_EDIID,
> >
> > +};
>
> Could be `const` from what I've seen.

Correct.
> > +static void edi_write_cmd(unsigned char *cmd, unsigned short
> > address, unsigned char data)
> > +{
> > cmd[0] = EDI_WRITE; /* EDI write command. */
> > cmd[1] = 0x00; /* Address is only 2 bytes. */
> > cmd[2] = (address >> 8) & 0xff; /* Address higher byte. */
> > cmd[3] = (address >> 0) & 0xff; /* Address lower byte. */
> > cmd[4] = data; /* Write data. */
> >
> > +}
> >
> > +static void edi_read_cmd(unsigned char *cmd, unsigned short
> > address)
> > +{
> > cmd[0] = EDI_READ; /* EDI read command. */
> > cmd[1] = 0x00; /* Address is only 2 bytes. */
> > cmd[2] = (address >> 8) & 0xff; /* Address higher byte. */
> > cmd[3] = (address >> 0) & 0xff; /* Address lower byte. */
> >
> > +}
> >
> > +static int edi_write(struct flashctx *flash, unsigned short
> > address, unsigned char data)
> > +{
> > unsigned char cmd[5] = { 0 };
>
> edi_write_cmd() below already fully initializes `cmd`.

Right, but I see that as a good programming habit, initializing memory
that will be filled by another function, because we might be using a
buffer larger than what the function will fill, etc.

Of course here, it's so simple that it's 100% safe to remove the
initialization, but I don't think it's a problem to keep it.
> > int rc;
> > edi_write_cmd((unsigned char *)cmd, address, data);
> > rc = spi_send_command(flash, sizeof(cmd), 0, (unsigned
> >
> > char *)cmd, NULL);
>
> I don't see a reason to cast `cmd`. But maybe it's just my C.

That's true, it's not required at all.
> Also you could just return spi_send_command(...)...

I prefer to always have a rc variable and check the return code
explicitly. Actually, I prefer to use known return codes for my local
(static) functions and not return the return code of the function
causing the error. This is not what I did in this patch and I'll
correct this.

This is simply my coding style and while there are some minor
advantages (moving code blocks around more easily, only bothering once
about what those functions return as good return code) and
disadvantages (longer code, very redundant), I'd like to keep things
this way unless there is an explicit policy in flashrom not to do this.

Also, I very much dislike checking functions' return codes with inline
calls in conditional statements or when returning, it simply looks
somewhat ugly to me, unless the function returns a boolean value.
> > if (rc)
> > return rc;
> > return 0;
>
> ... or return rc unconditionally. It looks very weird that way.

See above.
> > +}
> >
> > +static int edi_read(struct flashctx *flash, unsigned short
> > address, unsigned char *data)
> > +{
> > unsigned char cmd[4] = { 0 };
>
> Again, `cmd` gets fully initialized later.

See above.
> > unsigned char buffer[edi_read_buffer_length];
> > unsigned int i;
> > int rc;
> > edi_read_cmd((unsigned char *)cmd, address);
> > memset(buffer, 0, sizeof(buffer));
>
> Why?

Same thing here, spi_send_command *should* fill-in the whole buffer,
but in case it doesn't, I'd rather have known values instead, just in
case garbage turns out to take one of EDI_READY or EDI_NOT_READY.

This is probably not a hard requirement either, but seems like a good
programming habit to produce reliable code to me.
> > rc = spi_send_command(flash, sizeof(cmd), sizeof(buffer),
> >
> > (unsigned char *)cmd, (unsigned char *)buffer);
>
> Maybe unnecessary casts.
> > if (rc)
> > return rc;
> > for (i = 0; i < sizeof(buffer); i++) {
> > if (buffer[i] == EDI_NOT_READY)
> > 	continue;
> > if (buffer[i] == EDI_READY) {
> > 	if (i == (sizeof(buffer) - 1)) {
> > 		/*
> > 		 * Buffer size was too small for
> >
> > receiving the value.
> > 		 * This is as good as getting only
> >
> > EDI_NOT_READY.
> > 		 */
> > 		buffer[i] = EDI_NOT_READY;
> > 		break;
>
> If you break here, `i` won't get increased and `buffer[i]` is never
> read.

Well spotted, I missed that one. Of course, I need to increase i here.
I'll probably go with another variable "index" that keeps that last
value of i seen in the loop, since increasing i before breaking looks
too artificial to really be straightforward.
> > 	}
> > 	*data = buffer[i + 1];
> > 	return 0;
> > }
>
> So you're ignoring everything but EDI_READY and EDI_NOT_READY. Are
> there
> other valid values that might occur? Or could we just bail out?
> return
> failure here?

Well, as far as I could see from the documentation, the only valid byte
should be read after EDI_READY. EDI_NOT_READY indicates that we should
keep reading and other values should be treated as garbage.

I think this implementation is consistent with that. Especially, values
of 0 usually occur when the kb9012 is not responding (not powered, etc)
so I don't want to do anything in particular when it happens (such as
increasing the buffer).
> > }
> > if (buffer[i - 1] == EDI_NOT_READY) {
> > /*
> >  * Buffer size is increased, one step at a time,
> >  * to hold more data if we only catch
> >
> > EDI_NOT_READY.
> >  * Once CS is deasserted, no more data will be
> >
> > sent by the EC,
> >  * so we cannot keep reading afterwards and have
> >
> > to start a new
> >  * transaction with a longer buffer, to be safe.
> >  */
> > if ((edi_read_buffer_length + 1) <=
> >
> > EDI_READ_BUFFER_LENGTH_MAX) {
>
> So that's equivalent to `edi_read_buffer_length <
> EDI_READ_BUFFER_LENGTH_MAX`.

Quite right! This one is quite ugly.
> > 	msg_pwarn("%s: Retrying read with greater
> >
> > buffer length!\n", __func__);
> > 	edi_read_buffer_length++;
> > 	return edi_read(flash, address, data);
>
> Oh, recursion... stack usage looks not that bad, but could you live
> without it? For example write a edi_retry_read() that calls a non
> -recur-
> sive edi_read() in a loop?

Fair enough, let's go with renaming edi_read to edi_read_byte and
having edi_read wrap the call to edi_read_byte, check its rc for
 EDI_NOT_READY and implement the buffer increase logic in a non
-recursive way.
> > } else {
> > 	msg_perr("%s: Maximum buffer length
> >
> > reached and data still not ready!\n", __func__);
> > 	return -1;
>
> You'd return -1 anyway below, but that's ok, it looks more balanced
> this way...

This becomes more consistent with the new code flow suggested above.
> > }
> > }
> > return -1;
> >
> > +}
> >
> > +static int edi_chip_probe(struct flashctx *flash, struct ene_chip
> > *chip)
>
> `chip` could be const.

Ack.
> > +{
> > unsigned char hwversion = 0;
> > unsigned char ediid = 0;
> > int rc;
> > rc = edi_read(flash, ENE_EC_HWVERSION, &hwversion);
> > if (rc < 0)
> > return 0;
> > rc = edi_read(flash, ENE_EC_EDIID, &ediid);
> > if (rc < 0)
> > return 0;
> > if (chip->hwversion == hwversion && chip->ediid == ediid)
> > return 1;
> > return 0;
> >
> > +}
> >
> > +static int edi_disable(struct flashctx *flash)
> > +{
> > unsigned char cmd = EDI_DISABLE;
>
> const?

Would make sense, but since other definitions of cmd in
edi_read/edi_write are not const, I'd prefer to have this one not be
const for consistency.
> > int rc;
>
> There's no need for a variable here. Just return
> spi_send_command(...).

Not my coding style, see above.
> > rc = spi_send_command(flash, sizeof(cmd), 0, (unsigned
> >
> > char *)&cmd, NULL);
> > if (rc)
> > return rc;
> > return 0;
> >
> > +}
> >
> > +static int edi_spi_enable(struct flashctx *flash)
> > +{
> > unsigned char buffer = 0;
> > int rc;
> > rc = edi_read(flash, ENE_XBI_EFCFG, &buffer);
> > if (rc < 0)
>
> It looks odd if you know, that it won't be > 0.

I don't find that surprising. Negative rc for error and zero for
success is what I usually do.
> > return rc;
> > buffer |= ENE_XBI_EFCFG_CMD_WE;
> > rc = edi_write(flash, ENE_XBI_EFCFG, buffer);
> > if (rc < 0)
> > return rc;
> > return 0;
>
> return edi_write(...)?

See above.
> > +}
> >
> > +static int edi_spi_disable(struct flashctx *flash)
> > +{
> > unsigned char buffer = 0;
> > int rc;
> > rc = edi_read(flash, ENE_XBI_EFCFG, &buffer);
> > if (rc < 0)
> > return rc;
> > buffer &= ~ENE_XBI_EFCFG_CMD_WE;
> > rc = edi_write(flash, ENE_XBI_EFCFG, buffer);
> > if (rc < 0)
> > return rc;
> > return 0;
> >
> > +}
> >
> > +static int edi_spi_busy(struct flashctx *flash)
> > +{
> > unsigned char buffer = 0;
> > int rc;
> > rc = edi_read(flash, ENE_XBI_EFCFG, &buffer);
> > if (rc < 0)
> > return rc;
> > return !!(buffer & ENE_XBI_EFCFG_BUSY);
> >
> > +}
> >
> > +static int edi_8051_reset(struct flashctx *flash)
> > +{
> > unsigned char buffer = 0;
> > int rc;
> > rc = edi_read(flash, ENE_EC_PXCFG, &buffer);
> > if (rc < 0)
> > return rc;
> > buffer |= ENE_EC_PXCFG_8051_RESET;
> > rc = edi_write(flash, ENE_EC_PXCFG, buffer);
> > if (rc < 0)
> > return rc;
> > return 0;
> >
> > +}
> >
> > +static int edi_8051_execute(struct flashctx *flash)
> > +{
> > unsigned char buffer = 0;
> > int rc;
> > rc = edi_read(flash, ENE_EC_PXCFG, &buffer);
> > if (rc < 0)
> > return rc;
> > buffer &= ~ENE_EC_PXCFG_8051_RESET;
> > rc = edi_write(flash, ENE_EC_PXCFG, buffer);
> > if (rc < 0)
> > return rc;
> > return 0;
> >
> > +}
> >
> > +int edi_chip_block_erase(struct flashctx *flash, unsigned int
> > page, unsigned int size)
>
> Pedantic me: unsigned int is not assured to be wider than 16-bit ;)
> but
> that's a flashrom interface, isn't it?

Yep, I copied the prototype as-is.
> > +{
> > unsigned int timeout = 64;
> > if (size != flash->chip->page_size) {
>
> Is this even possible? Or some kind of assertion?

I haven't seen flashrom ask for this, but it could. When that happens,
some bits on the page are left unprogrammed and when the whole page is
written to memory, we're losing data.

Handling that in edi.c causes way way too much overhead, so I'm just
enforcing these assumptions to be safe.
> > msg_perr("%s: Block erase size is not page
> >
> > size!\n", __func__);
> > return -1;
> > }
> > edi_spi_enable(flash);
> > edi_write(flash, ENE_XBI_EFA0, ((page & 0xff) >> 0));
> > edi_write(flash, ENE_XBI_EFA1, ((page & 0xff00) >> 8));
> > edi_write(flash, ENE_XBI_EFA2, ((page & 0xff0000) >> 16));
> > edi_write(flash, ENE_XBI_EFCMD, ENE_XBI_EFCMD_ERASE);
>
> Every edi_write() may fail...[1]

Indeed, I have added all the rc checking at a later point when coding
and apparently missed those. Good spotting!
> > while (edi_spi_busy(flash) && timeout--)
>
> Um, edi_spi_busy() also returns true (!= 0) if it failed to read at
> all.

Quite right, let's make this edi_spi_busy(flash) == 1 and nevermind the
error just this one time.
> > programmer_delay(10);
> > edi_spi_disable(flash);
> > return 0;
>
> [1]...returning 0 anyway?
> > +}
> >
> > +int edi_chip_write(struct flashctx *flash, const uint8_t *buf,
> > unsigned int start, unsigned int len)
> > +{
> > unsigned int address = start;
> > unsigned int pages;
> > unsigned int timeout;
> > unsigned int i, j;
> > if ((start % flash->chip->page_size) != 0) {
> > msg_perr("%s: Start address is not page
> >
> > -aligned!\n", __func__);
> > return -1;
> > }
> > if ((len % flash->chip->page_size) != 0) {
> > msg_perr("%s: Length is not page-aligned!\n",
> >
> > __func__);
> > return -1;
> > }
> > pages = len / flash->chip->page_size;
> > edi_spi_enable(flash);
> > edi_write(flash, ENE_XBI_EFA0, ((address & 0xff) >> 0));
> > edi_write(flash, ENE_XBI_EFA1, ((address & 0xff00) >> 8));
> > edi_write(flash, ENE_XBI_EFA2, ((address & 0xff0000) >>
> >
> > 16));
> > for (i = 0; i < pages; i++) {
> > timeout = 64;
> > /* Clear page buffer. */
> > edi_write(flash, ENE_XBI_EFCMD,
> >
> > ENE_XBI_EFCMD_HVPL_CLEAR);
> > for (j = 0; j < flash->chip->page_size; j++) {
> > 	if ((address - start) > 0) {
>
> Just got confused here, so `start` is the overall start not the start 
> of the page.

Indeed. The point here is simply to check that address - 1 is within
the requested range, otherwise we end up setting the address twice the
first time.

Also, note that the address has to be set each time we write a byte to
the page buffer. Unfortunately, it doesn't append the byte we just
wrote to the page buffer.
> Would have known that, when `start` would have been declared const ;)

Sure, but again, I copied the prototype as-is from struct flashchip.
Feel free to change that for the whole flashrom ;)
> Also it's equivalent to `address > start`.

Good point, that's more readable.
> > 		if (((address - 1) & 0xff) !=
> >
> > (address & 0xff))
> > 			edi_write(flash,
> >
> > ENE_XBI_EFA0, ((address & 0xff) >> 0));
> > 		if (((address - 1) & 0xff00) !=
> >
> > (address & 0xff00))
> > 			edi_write(flash,
> >
> > ENE_XBI_EFA1, ((address & 0xff00) >> 8));
> > 		if (((address - 1) & 0xff0000) !=
> >
> > (address & 0xff0000))
> > 			edi_write(flash,
> >
> > ENE_XBI_EFA2, ((address & 0xff0000) >> 16));
> > 	}
> > 	edi_write(flash, ENE_XBI_EFDAT, *buf);
> > 	edi_write(flash, ENE_XBI_EFCMD,
> >
> > ENE_XBI_EFCMD_HVPL_LATCH);
> > 	buf++;
> > 	address++;
> > }
> > /* Program page buffer to flash. */
> > edi_write(flash, ENE_XBI_EFCMD,
> >
> > ENE_XBI_EFCMD_PROGRAM);
> > while (edi_spi_busy(flash) && timeout--)
> > 	programmer_delay(10);
> > }
> > edi_spi_disable(flash);
> > return 0;
>
> Again, every edi_write() may fail, edi_spi_{en,dis}able() also. Not
> checking on single calls might be ok, but in a sheer endless loop?
> that's not good. Guess, the SPI programmer driver runs into a timeout 
> of one second for every call to spi_send_command(), you'd be waiting 
> ages for this to end.

Sure, that will be fixed.
> > +}
> >
> > +int edi_chip_read(struct flashctx *flash, uint8_t *buf, unsigned
> > int start, unsigned int len)
> > +{
> > unsigned int address = start;
> > unsigned int i;
> > unsigned int timeout;
> > int rc;
> > edi_spi_enable(flash);
> > edi_write(flash, ENE_XBI_EFA0, ((address & 0xff) >> 0));
> > edi_write(flash, ENE_XBI_EFA1, ((address & 0xff00) >> 8));
> > edi_write(flash, ENE_XBI_EFA2, ((address & 0xff0000) >>
> >
> > 16));
> > /*
> > * EDI brings such a drastic overhead that there is about
> >
> > no need to
> > * have any delay in between calls. The EDI protocol will
> >
> > handle wait
> > * I/O times on its own anyway.
> > */
> > for (i = 0; i < len; i++) {
> > timeout = 64;
> > if ((address - start) > 0) {
> > 	if (((address - 1) & 0xff) != (address &
> >
> > 0xff))
> > 		edi_write(flash, ENE_XBI_EFA0,
> >
> > ((address & 0xff) >> 0));
> > 	if (((address - 1) & 0xff00) != (address &
> >
> > 0xff00))
> > 		edi_write(flash, ENE_XBI_EFA1,
> >
> > ((address & 0xff00) >> 8));
> > 	if (((address - 1) & 0xff0000) != (address
> >
> > & 0xff0000))
> > 		edi_write(flash, ENE_XBI_EFA2,
> >
> > ((address & 0xff0000) >> 16));
> > }
> > edi_write(flash, ENE_XBI_EFCMD,
> >
> > ENE_XBI_EFCMD_READ);
> > do {
> > 	rc = edi_read(flash, ENE_XBI_EFDAT, buf);
> > 	if (!rc)
> > 		break;
>
> if (!timeout) return ...? or loop for ever!

Thanks, sorry for such carelessness, there are other places where
timeout is not checked (even though it's more critical in this spot).
> > 	/* Just in case. */
> > 	while (edi_spi_busy(flash) && timeout--)
> > 		programmer_delay(10);
> > } while (rc);
>
> Redundant check (rc can't be zero when we reach it) hides the endless
> loop. Nice try! hehe

That's correct, I simply prefer to have a dummy condition loosely
related to the code logic (that is never actually reached) because,
well, it doesn't hurt and looks better then while (1).

If you believe it obfuscates the code, I'd be fine with removing that
form.
> > buf++;
> > address++;
> > }
> > edi_spi_disable(flash);
> > return 0;
> >
> > +}
> >
> > +int edi_shutdown(void *data)
> > +{
> > struct flashctx *flash;
> > int rc;
> > if (data == NULL)
> > return -1;
> > flash = (struct flashctx *)data;
> > rc = edi_8051_execute(flash);
> > if (rc < 0) {
> > msg_perr("%s: Unable to execute 8051!\n",
> >
> > __func__);
> > return -1;
> > }
> > rc = edi_disable(flash);
> > if (rc < 0) {
> > msg_perr("%s: Unable to disable EDI!\n",
> >
> > __func__);
> > return -1;
> > }
> > return 0;
> >
> > +}
> >
> > +int edi_probe_kb9012(struct flashctx *flash)
> > +{
> > int probe;
> > int rc;
> > probe = edi_chip_probe(flash, &ene_kb9012);
> > if (!probe)
> > return 0;
> > rc = edi_8051_reset(flash);
> > if (rc < 0) {
> > msg_perr("%s: Unable to reset 8051!\n", __func__);
> > return 0;
> > }
> > register_shutdown(edi_shutdown, (void *)flash);
> > return 1;
> >
> > +}
> > diff --git a/edi.h b/edi.h
> > new file mode 100644
> > index 0000000..d9387b1
> > --- /dev/null
> > +++ b/edi.h
> > @@ -0,0 +1,34 @@
> > +/*
> > This file is part of the flashrom project.
> > Copyright (C) 2015 Paul Kocialkowski
> > 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
> > */
> >
> > +#ifndef __EDI_H__
> > +#define __EDI_H__ 1
> >
> > +#define EDI_READ			0x30
> > +#define EDI_WRITE			0x40
> > +#define EDI_DISABLE			0xf3
> >
> > +#define EDI_NOT_READY			0x5f
> > +#define EDI_READY			0x50
> >
> > +#define EDI_READ_BUFFER_LENGTH_DEFAULT	3
> > +#define EDI_READ_BUFFER_LENGTH_MAX	32
> >
> > +#endif
> > diff --git a/ene.h b/ene.h
> > new file mode 100644
> > index 0000000..445d28b
> > --- /dev/null
> > +++ b/ene.h
> > @@ -0,0 +1,55 @@
> > +/*
> > This file is part of the flashrom project.
> > Copyright (C) 2015 Paul Kocialkowski
> > 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
> > */
> >
> > +#ifndef __ENE_H__
> > +#define __ENE_H__ 1
> >
> > +#define ENE_XBI_EFA0			0xfea8
> > +#define ENE_XBI_EFA1			0xfea9
> > +#define ENE_XBI_EFA2			0xfeaa
> > +#define ENE_XBI_EFDAT			0xfeab
> > +#define ENE_XBI_EFCMD			0xfeac
> > +#define ENE_XBI_EFCFG			0xfead
> >
> > +#define ENE_XBI_EFCFG_CMD_WE		(1 << 3)
> > +#define ENE_XBI_EFCFG_BUSY		(1 << 1)
> >
> > +#define ENE_XBI_EFCMD_HVPL_LATCH	0x02
> > +#define ENE_XBI_EFCMD_READ		0x03
> > +#define ENE_XBI_EFCMD_ERASE		0x20
> > +#define ENE_XBI_EFCMD_PROGRAM		0x70
> > +#define ENE_XBI_EFCMD_HVPL_CLEAR	0x80
> >
> > +#define ENE_EC_PXCFG			0xff14
> >
> > +#define ENE_EC_PXCFG_8051_RESET		0x01
> >
> > +#define ENE_EC_HWVERSION		0xff00
> > +#define ENE_EC_EDIID			0xff24
> >
> > +#define ENE_KB9012_HWVERSION		0xc3
> > +#define ENE_KB9012_EDIID		0x04
> >
> > +struct ene_chip {
> > unsigned char hwversion;
> > unsigned char ediid;
> >
> > +};
> >
> > +#endif
> > diff --git a/flashchips.c b/flashchips.c
> > index 574ad74..13f0574 100644
> > --- a/flashchips.c
> > +++ b/flashchips.c
> > @@ -3201,6 +3201,29 @@ const struct flashchip flashchips[] = {
> >    },
> >
> > {
> > .vendor		= "ENE",
> > .name		= "KB9012 (EDI)",
> > .bustype	= BUS_SPI,
> > .total_size	= 128,
> > .page_size	= 128,
> > .feature_bits	= FEATURE_ERASED_ZERO,
> > .tested		= TEST_OK_PREW,
> > .probe		= edi_probe_kb9012,
> > .probe_timing	= TIMING_ZERO,
> > .block_erasers	=
> > {
> > 	{
> > 		.eraseblocks = { {128, 1024} },
> > 		.block_erase =
> >
> > edi_chip_block_erase,
> > 	},
> > },
> > .gran		= write_gran_128bytes,
> > .write		= edi_chip_write,
> > .read		= edi_chip_read,
> > .voltage	= {2700, 3600},
> > },
> > {
> > .vendor		= "ESMT",
> > .name		= "F49B002UA",
> > .bustype	= BUS_PARALLEL,