[flashrom] [PATCH 1/5] Add opaque programmer registration infrastructure.
Carl-Daniel Hailfinger
c-d.hailfinger.devel.2006 at gmx.net
Wed Nov 2 23:20:17 CET 2011
Am 02.11.2011 18:29 schrieb Michael Karcher:
> Am Freitag, den 21.10.2011, 00:39 +0200 schrieb Stefan Tauner:
>> diff --git a/flash.h b/flash.h
>> index 535c1b8..8ad2845 100644
>> --- a/flash.h
>> +++ b/flash.h
>> @@ -62,8 +62,9 @@ enum chipbustype {
>> BUS_LPC = 1 << 1,
>> BUS_FWH = 1 << 2,
>> BUS_SPI = 1 << 3,
>> + BUS_PROG = 1 << 4,
>> BUS_NONSPI = BUS_PARALLEL | BUS_LPC | BUS_FWH,
>> - BUS_UNKNOWN = BUS_PARALLEL | BUS_LPC | BUS_FWH | BUS_SPI,
>> + BUS_UNKNOWN = BUS_PARALLEL | BUS_LPC | BUS_FWH | BUS_SPI | BUS_PROG,
> We don't need BUS_UNKNOWN. It is referenced only in flashbuses_to_text.
> Having a programmer being both opaque and non-opaque seems to be as
> useless as defining that a chip "might also be accessed through an
> opaque interface". In the case of opaque interfaces, we don't care about
> the chip type - so I don't see any use case for BUS_UNKNOWN, even more
> after adding BUS_PROG.
Killed.
>> };
>> +#if defined(CONFIG_INTERNAL) && (defined(__i386__) || defined(__x86_64__))
>> + {
>> + .vendor = "Programmer",
>> + .name = "Opaque flash chip",
>> + .bustype = BUS_PROG,
>> + .manufacture_id = PROGMANUF_ID,
>> + .model_id = PROGDEV_ID,
>> + .total_size = 0,
>> + .page_size = 256,
>> + /* probe is assumed to work, rest will be filled in by probe */
>> + .tested = TEST_OK_PROBE,
>> + .probe = probe_opaque,
>> + /* eraseblock sizes will be set by the probing function */
>> + .block_erasers =
>> + {
>> + {
>> + .block_erase = erase_opaque,
>> + }
>> + },
>> + .write = write_opaque,
>> + .read = read_opaque,
>> + },
>> +#endif // defined(CONFIG_INTERNAL) && (defined(__i386__) || defined(__x86_64__))
> Why do we need any prerequisite for the opaque programmer? It should
> work outside of x86(-64), too, depending on which opaque backend is
> used. I guess this #if is there because at the moment, the only opaque
> programmer is the ICH9 stuff, but wouldn't a define NEED_OPAQUE that is
> set in the include file as soon as any programmer based on the opaque
> programmer is selected be more sensible?
We don't use #ifdef for all other chips, so the sane solution is to
avoid the #ifdef completely.
>> @@ -9005,6 +9027,7 @@ const struct flashchip flashchips[] = {
>> .probe = probe_spi_rdid,
>> .write = NULL,
>> },
>> +
>> {
>> .vendor = "Generic",
>> .name = "unknown SPI chip (REMS)",
> Unrelated change - you might have it in accidentally.
Right. Stefan, do you plan to move this to your tested_stuff branch?
>> +/*
>> + * Contains the opaque programmer framework.
>> + * An opaque programmer is a programmer which does not provide direct access
>> + * to the flash chip and which abstracts all flash chip properties into a
>> + * programmer specific interface.
>> + */
>> +
>> +#include <strings.h>
>> +#include <string.h>
>> +#include "flash.h"
>> +#include "flashchips.h"
>> +#include "chipdrivers.h"
>> +#include "programmer.h"
>> +
>> +const struct opaque_programmer opaque_programmer_none = {
>> + .max_data_read = MAX_DATA_UNSPECIFIED,
>> + .max_data_write = MAX_DATA_UNSPECIFIED,
>> + .probe = NULL,
>> + .read = NULL,
> You are missing ".erase = NULL" here.
Fixed.
>> + .write = NULL,
>> +};
>> +
>> +const struct opaque_programmer *opaque_programmer = &opaque_programmer_none;
>> +
>> +int probe_opaque(struct flashchip *flash)
>> +{
>> + if (!opaque_programmer->probe) {
>> + msg_perr("%s called, but this is not an opaque programmer. "
>> + "Please report a bug at flashrom at flashrom.org\n",
>> + __func__);
> I don't really like the error message, I would prefer "%s called without
> register_opaque" or something like that.
What about "%s called before register_opaque_programmer" ?
>> +void register_opaque_programmer(const struct opaque_programmer *pgm)
>> +{
>> + opaque_programmer = pgm;
>> + buses_supported |= BUS_PROG;
>> +}
> You might want to check that the function pointers in pgm are not NULL,
> to make the suggested error message "without register_opaque" more apt.
Fixed.
> Otherwise, the patch looks good to me. This is
> Acked-By: Michael Karcher <flashrom at mkarcher.dialup.fu-berlin.de>
Thanks for the review!
> If you feel confident you didn't mess up anything, feel free to re-use
> the ack after minor edits.
While I tested compilation, I'd still appreciate a second look at the
result.
An opaque programmer does not allow direct flash access and only offers
abstract probe/read/erase/write methods.
Due to that, opaque programmers need their own infrastructure and
registration framework.
Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
Index: flashrom-register_opaque_programmer/flash.h
===================================================================
--- flashrom-register_opaque_programmer/flash.h (Revision 1458)
+++ flashrom-register_opaque_programmer/flash.h (Arbeitskopie)
@@ -62,8 +62,8 @@
BUS_LPC = 1 << 1,
BUS_FWH = 1 << 2,
BUS_SPI = 1 << 3,
+ BUS_PROG = 1 << 4,
BUS_NONSPI = BUS_PARALLEL | BUS_LPC | BUS_FWH,
- BUS_UNKNOWN = BUS_PARALLEL | BUS_LPC | BUS_FWH | BUS_SPI,
};
/*
Index: flashrom-register_opaque_programmer/Makefile
===================================================================
--- flashrom-register_opaque_programmer/Makefile (Revision 1458)
+++ flashrom-register_opaque_programmer/Makefile (Arbeitskopie)
@@ -242,7 +242,7 @@
CHIP_OBJS = jedec.o stm50flw0x0x.o w39.o w29ee011.o \
sst28sf040.o m29f400bt.o 82802ab.o pm49fl00x.o \
sst49lfxxxc.o sst_fwhub.o flashchips.o spi.o spi25.o sharplhf00l04.o \
- a25.o at25.o
+ a25.o at25.o opaque.o
LIB_OBJS = layout.o
Index: flashrom-register_opaque_programmer/opaque.c
===================================================================
--- flashrom-register_opaque_programmer/opaque.c (Revision 0)
+++ flashrom-register_opaque_programmer/opaque.c (Revision 0)
@@ -0,0 +1,100 @@
+/*
+ * This file is part of the flashrom project.
+ *
+ * Copyright (C) 2011 Carl-Daniel Hailfinger
+ *
+ * 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; version 2 of the License.
+ *
+ * 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
+ */
+
+/*
+ * Contains the opaque programmer framework.
+ * An opaque programmer is a programmer which does not provide direct access
+ * to the flash chip and which abstracts all flash chip properties into a
+ * programmer specific interface.
+ */
+
+#include <strings.h>
+#include <string.h>
+#include "flash.h"
+#include "flashchips.h"
+#include "chipdrivers.h"
+#include "programmer.h"
+
+const struct opaque_programmer opaque_programmer_none = {
+ .max_data_read = MAX_DATA_UNSPECIFIED,
+ .max_data_write = MAX_DATA_UNSPECIFIED,
+ .probe = NULL,
+ .read = NULL,
+ .write = NULL,
+ .erase = NULL,
+};
+
+const struct opaque_programmer *opaque_programmer = &opaque_programmer_none;
+
+int probe_opaque(struct flashchip *flash)
+{
+ if (!opaque_programmer->probe) {
+ msg_perr("%s called before register_opaque_programmer. "
+ "Please report a bug at flashrom at flashrom.org\n",
+ __func__);
+ return 0;
+ }
+
+ return opaque_programmer->probe(flash);
+}
+
+int read_opaque(struct flashchip *flash, uint8_t *buf, int start, int len)
+{
+ if (!opaque_programmer->read) {
+ msg_perr("%s called before register_opaque_programmer. "
+ "Please report a bug at flashrom at flashrom.org\n",
+ __func__);
+ return 1;
+ }
+ return opaque_programmer->read(flash, buf, start, len);
+}
+
+int write_opaque(struct flashchip *flash, uint8_t *buf, int start, int len)
+{
+ if (!opaque_programmer->write) {
+ msg_perr("%s called before register_opaque_programmer. "
+ "Please report a bug at flashrom at flashrom.org\n",
+ __func__);
+ return 1;
+ }
+ return opaque_programmer->write(flash, buf, start, len);
+}
+
+int erase_opaque(struct flashchip *flash, unsigned int blockaddr, unsigned int blocklen)
+{
+ if (!opaque_programmer->erase) {
+ msg_perr("%s called before register_opaque_programmer. "
+ "Please report a bug at flashrom at flashrom.org\n",
+ __func__);
+ return 1;
+ }
+ return opaque_programmer->erase(flash, blockaddr, blocklen);
+}
+
+void register_opaque_programmer(const struct opaque_programmer *pgm)
+{
+ if (!pgm->probe || !pgm->read || !pgm->write || !pgm->erase) {
+ msg_perr("%s called with one of probe/read/write/erase being "
+ "NULL. Please report a bug at flashrom at flashrom.org\n",
+ __func__);
+ return;
+ }
+ opaque_programmer = pgm;
+ buses_supported |= BUS_PROG;
+}
Index: flashrom-register_opaque_programmer/flashchips.c
===================================================================
--- flashrom-register_opaque_programmer/flashchips.c (Revision 1458)
+++ flashrom-register_opaque_programmer/flashchips.c (Arbeitskopie)
@@ -8874,6 +8874,28 @@
},
{
+ .vendor = "Programmer",
+ .name = "Opaque flash chip",
+ .bustype = BUS_PROG,
+ .manufacture_id = PROGMANUF_ID,
+ .model_id = PROGDEV_ID,
+ .total_size = 0,
+ .page_size = 256,
+ /* probe is assumed to work, rest will be filled in by probe */
+ .tested = TEST_OK_PROBE,
+ .probe = probe_opaque,
+ /* eraseblock sizes will be set by the probing function */
+ .block_erasers =
+ {
+ {
+ .block_erase = erase_opaque,
+ }
+ },
+ .write = write_opaque,
+ .read = read_opaque,
+ },
+
+ {
.vendor = "AMIC",
.name = "unknown AMIC SPI chip",
.bustype = BUS_SPI,
@@ -9005,6 +9027,7 @@
.probe = probe_spi_rdid,
.write = NULL,
},
+
{
.vendor = "Generic",
.name = "unknown SPI chip (REMS)",
Index: flashrom-register_opaque_programmer/flashchips.h
===================================================================
--- flashrom-register_opaque_programmer/flashchips.h (Revision 1458)
+++ flashrom-register_opaque_programmer/flashchips.h (Arbeitskopie)
@@ -646,4 +646,7 @@
#define WINBOND_W49V002A 0xB0
#define WINBOND_W49V002FA 0x32
+#define PROGMANUF_ID 0xFFFE /* dummy ID for opaque chips behind a programmer */
+#define PROGDEV_ID 0x01 /* dummy ID for opaque chips behind a programmer */
+
#endif /* !FLASHCHIPS_H */
Index: flashrom-register_opaque_programmer/programmer.h
===================================================================
--- flashrom-register_opaque_programmer/programmer.h (Revision 1458)
+++ flashrom-register_opaque_programmer/programmer.h (Arbeitskopie)
@@ -601,6 +601,19 @@
int wbsio_check_for_spi(void);
#endif
+/* opaque.c */
+struct opaque_programmer {
+ int max_data_read;
+ int max_data_write;
+ /* Specific functions for this programmer */
+ int (*probe) (struct flashchip *flash);
+ int (*read) (struct flashchip *flash, uint8_t *buf, int start, int len);
+ int (*write) (struct flashchip *flash, uint8_t *buf, int start, int len);
+ int (*erase) (struct flashchip *flash, unsigned int blockaddr, unsigned int blocklen);
+};
+extern const struct opaque_programmer *opaque_programmer;
+void register_opaque_programmer(const struct opaque_programmer *pgm);
+
/* serprog.c */
#if CONFIG_SERPROG == 1
int serprog_init(void);
Index: flashrom-register_opaque_programmer/chipdrivers.h
===================================================================
--- flashrom-register_opaque_programmer/chipdrivers.h (Revision 1458)
+++ flashrom-register_opaque_programmer/chipdrivers.h (Arbeitskopie)
@@ -58,6 +58,12 @@
int spi_write_chunked(struct flashchip *flash, uint8_t *buf, int start, int len, int chunksize);
int spi_aai_write(struct flashchip *flash, uint8_t *buf, int start, int len);
+/* opaque.c */
+int probe_opaque(struct flashchip *flash);
+int read_opaque(struct flashchip *flash, uint8_t *buf, int start, int len);
+int write_opaque(struct flashchip *flash, uint8_t *buf, int start, int len);
+int erase_opaque(struct flashchip *flash, unsigned int blockaddr, unsigned int blocklen);
+
/* a25.c */
int spi_prettyprint_status_register_amic_a25l05p(struct flashchip *flash);
int spi_prettyprint_status_register_amic_a25l40p(struct flashchip *flash);
Index: flashrom-register_opaque_programmer/print.c
===================================================================
--- flashrom-register_opaque_programmer/print.c (Revision 1458)
+++ flashrom-register_opaque_programmer/print.c (Arbeitskopie)
@@ -32,13 +32,11 @@
char *flashbuses_to_text(enum chipbustype bustype)
{
char *ret = calloc(1, 1);
- if (bustype == BUS_UNKNOWN) {
- ret = strcat_realloc(ret, "Unknown, ");
/*
* FIXME: Once all chipsets and flash chips have been updated, NONSPI
* will cease to exist and should be eliminated here as well.
*/
- } else if (bustype == BUS_NONSPI) {
+ if (bustype == BUS_NONSPI) {
ret = strcat_realloc(ret, "Non-SPI, ");
} else {
if (bustype & BUS_PARALLEL)
@@ -49,6 +47,8 @@
ret = strcat_realloc(ret, "FWH, ");
if (bustype & BUS_SPI)
ret = strcat_realloc(ret, "SPI, ");
+ if (bustype & BUS_PROG)
+ ret = strcat_realloc(ret, "Programmer-specific, ");
if (bustype == BUS_NONE)
ret = strcat_realloc(ret, "None, ");
}
--
http://www.hailfinger.org/
More information about the flashrom
mailing list