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

[Nico Huber]

Dear flashrom maintainers, ;)

I have one last concern with this patch. It makes flashrom probe auto-
matically by emitting SPI 0x30 commands and I have no idea if that may
harm SPI chips that interpret it in any non-obvious way. Do we have a
policy for adding new probe commands?

If you share my concerns, is there a way to disable the automatic probe?
If not, well, it's acked-by :)

Nico

On 11.11.2015 16:55, 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

Acked-by: Nico Huber
> Makefile      |   2 +-
>  chipdrivers.h |   6 +
>  edi.c         | 497 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  edi.h         |  30 ++++
>  ene.h         |  51 ++++++
>  flashchips.c  |  23 +++
>  6 files changed, 608 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..0ca1704
> --- /dev/null
> +++ b/edi.c
> @@ -0,0 +1,497 @@
> +/*
> 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.
> */
>
> +#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 const struct ene_chip ene_kb9012 = {
> .hwversion = ENE_KB9012_HWVERSION,
> .ediid = ENE_KB9012_EDIID,
>
> +};
>
> +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];
> int rc;
> edi_write_cmd(cmd, address, data);
> rc = spi_send_command(flash, sizeof(cmd), 0, cmd, NULL);
> if (rc)
> return -1;
> return 0;
>
> +}
>
> +static int edi_read_byte(struct flashctx *flash, unsigned short address, unsigned char *data)
> +{
> unsigned char cmd[4];
> unsigned char buffer[edi_read_buffer_length];
> unsigned int index;
> unsigned int i;
> int rc;
> edi_read_cmd(cmd, address);
> rc = spi_send_command(flash, sizeof(cmd), sizeof(buffer), cmd, buffer);
> if (rc)
> return -1;
> index = 0;
> for (i = 0; i < sizeof(buffer); i++) {
> index = 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;
> 	}
> 	*data = buffer[i + 1];
> 	return 0;
> }
> }
> if (buffer[index] == EDI_NOT_READY)
> return -EDI_NOT_READY;
> return -1;
>
> +}
>
> +static int edi_read(struct flashctx *flash, unsigned short address, unsigned char *data)
> +{
> int rc;
> do {
> rc = edi_read_byte(flash, address, data);
> if (rc == -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 < EDI_READ_BUFFER_LENGTH_MAX) {
> 		msg_pwarn("%s: Retrying read with greater buffer length!\n", __func__);
> 		edi_read_buffer_length++;
> 	} else {
> 		msg_perr("%s: Maximum buffer length reached and data still not ready!\n", __func__);
> 		return -1;
> 	}
> } else if (rc < 0) {
> 	return -1;
> }
> } while (rc == -EDI_NOT_READY);
> return 0;
>
> +}
>
> +static int edi_disable(struct flashctx *flash)
> +{
> unsigned char cmd = EDI_DISABLE;
> int rc;
> rc = spi_send_command(flash, sizeof(cmd), 0, &cmd, NULL);
> if (rc)
> return -1;
> return 0;
>
> +}
>
> +static int edi_chip_probe(struct flashctx *flash, const struct ene_chip *chip)
> +{
> unsigned char hwversion;
> unsigned char ediid;
> 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_spi_enable(struct flashctx *flash)
> +{
> unsigned char buffer;
> int rc;
> rc = edi_read(flash, ENE_XBI_EFCFG, &buffer);
> if (rc < 0)
> return -1;
> buffer |= ENE_XBI_EFCFG_CMD_WE;
> rc = edi_write(flash, ENE_XBI_EFCFG, buffer);
> if (rc < 0)
> return -1;
> return 0;
>
> +}
>
> +static int edi_spi_disable(struct flashctx *flash)
> +{
> unsigned char buffer;
> int rc;
> rc = edi_read(flash, ENE_XBI_EFCFG, &buffer);
> if (rc < 0)
> return -1;
> buffer &= ~ENE_XBI_EFCFG_CMD_WE;
> rc = edi_write(flash, ENE_XBI_EFCFG, buffer);
> if (rc < 0)
> return -1;
> return 0;
>
> +}
>
> +static int edi_spi_busy(struct flashctx *flash)
> +{
> unsigned char buffer;
> int rc;
> rc = edi_read(flash, ENE_XBI_EFCFG, &buffer);
> if (rc < 0)
> return -1;
> return !!(buffer & ENE_XBI_EFCFG_BUSY);
>
> +}
>
> +static int edi_spi_address(struct flashctx *flash, unsigned int start, unsigned int address)
> +{
> int rc;
> if ((address == start) || (((address - 1) & 0xff) != (address & 0xff))) {
> rc = edi_write(flash, ENE_XBI_EFA0, ((address & 0xff) >> 0));
> if (rc < 0)
> 	return -1;
> }
> if ((address == start) || (((address - 1) & 0xff00) != (address & 0xff00))) {
> rc = edi_write(flash, ENE_XBI_EFA1, ((address & 0xff00) >> 8));
> if (rc < 0)
> 	return -1;
> }
> if ((address == start) || (((address - 1) & 0xff0000) != (address & 0xff0000))) {
> rc = edi_write(flash, ENE_XBI_EFA2, ((address & 0xff0000) >> 16));
> if (rc < 0)
> 	return -1;
> }
> return 0;
>
> +}
>
> +static int edi_8051_reset(struct flashctx *flash)
> +{
> unsigned char buffer;
> int rc;
> rc = edi_read(flash, ENE_EC_PXCFG, &buffer);
> if (rc < 0)
> return -1;
> buffer |= ENE_EC_PXCFG_8051_RESET;
> rc = edi_write(flash, ENE_EC_PXCFG, buffer);
> if (rc < 0)
> return -1;
> return 0;
>
> +}
>
> +static int edi_8051_execute(struct flashctx *flash)
> +{
> unsigned char buffer;
> int rc;
> rc = edi_read(flash, ENE_EC_PXCFG, &buffer);
> if (rc < 0)
> return -1;
> buffer &= ~ENE_EC_PXCFG_8051_RESET;
> rc = edi_write(flash, ENE_EC_PXCFG, buffer);
> if (rc < 0)
> return -1;
> return 0;
>
> +}
>
> +int edi_chip_block_erase(struct flashctx *flash, unsigned int page, unsigned int size)
> +{
> unsigned int timeout = 64;
> int rc;
> if (size != flash->chip->page_size) {
> msg_perr("%s: Block erase size is not page size!\n", __func__);
> return -1;
> }
> rc = edi_spi_enable(flash);
> if (rc < 0) {
> msg_perr("%s: Unable to enable SPI!\n", __func__);
> return -1;
> }
> rc = edi_spi_address(flash, page, page);
> if (rc < 0)
> return -1;
> rc = edi_write(flash, ENE_XBI_EFCMD, ENE_XBI_EFCMD_ERASE);
> if (rc < 0)
> return -1;
> while (edi_spi_busy(flash) == 1 && timeout) {
> programmer_delay(10);
> timeout--;
> }
> if (!timeout) {
> msg_perr("%s: Timed out waiting for SPI not busy!\n", __func__);
> return -1;
> }
> rc = edi_spi_disable(flash);
> if (rc < 0) {
> msg_perr("%s: Unable to disable SPI!\n", __func__);
> return -1;
> }
> return 0;
>
> +}
>
> +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;
> int rc;
> 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;
> rc = edi_spi_enable(flash);
> if (rc < 0) {
> msg_perr("%s: Unable to enable SPI!\n", __func__);
> return -1;
> }
> for (i = 0; i < pages; i++) {
> timeout = 64;
> /* Clear page buffer. */
> rc = edi_write(flash, ENE_XBI_EFCMD, ENE_XBI_EFCMD_HVPL_CLEAR);
> if (rc < 0)
> 	return -1;
> for (j = 0; j < flash->chip->page_size; j++) {
> 	rc = edi_spi_address(flash, start, address);
> 	if (rc < 0)
> 		return -1;
> 	rc = edi_write(flash, ENE_XBI_EFDAT, *buf);
> 	if (rc < 0)
> 		return -1;
> 	rc = edi_write(flash, ENE_XBI_EFCMD, ENE_XBI_EFCMD_HVPL_LATCH);
> 	if (rc < 0)
> 		return -1;
> 	buf++;
> 	address++;
> }
> /* Program page buffer to flash. */
> rc = edi_write(flash, ENE_XBI_EFCMD, ENE_XBI_EFCMD_PROGRAM);
> if (rc < 0)
> 	return -1;
> while (edi_spi_busy(flash) == 1 && timeout) {
> 	programmer_delay(10);
> 	timeout--;
> }
> if (!timeout) {
> 	msg_perr("%s: Timed out waiting for SPI not busy!\n", __func__);
> 	return -1;
> }
> }
> rc = edi_spi_disable(flash);
> if (rc < 0) {
> msg_perr("%s: Unable to disable SPI!\n", __func__);
> return -1;
> }
> return 0;
>
> +}
>
> +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;
> rc = edi_spi_enable(flash);
> if (rc < 0) {
> msg_perr("%s: Unable to enable SPI!\n", __func__);
> return -1;
> }
> /*
> * 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;
> rc = edi_spi_address(flash, start, address);
> if (rc < 0)
> 	return -1;
> rc = edi_write(flash, ENE_XBI_EFCMD, ENE_XBI_EFCMD_READ);
> if (rc < 0)
> 	return -1;
> do {
> 	rc = edi_read(flash, ENE_XBI_EFDAT, buf);
> 	if (rc == 0)
> 		break;
> 	/* Just in case. */
> 	while (edi_spi_busy(flash) == 1 && timeout) {
> 		programmer_delay(10);
> 		timeout--;
> 	}
> 	if (!timeout) {
> 		msg_perr("%s: Timed out waiting for SPI not busy!\n", __func__);
> 		return -1;
> 	}
> } while (1);
> buf++;
> address++;
> }
> rc = edi_spi_disable(flash);
> if (rc < 0) {
> msg_perr("%s: Unable to disable SPI!\n", __func__);
> return -1;
> }
> 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..542bf26
> --- /dev/null
> +++ b/edi.h
> @@ -0,0 +1,30 @@
> +/*
> 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.
> */
>
> +#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..e03e49b
> --- /dev/null
> +++ b/ene.h
> @@ -0,0 +1,51 @@
> +/*
> 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.
> */
>
> +#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,