[coreboot] [PATCH] flashrom: External flasher support

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Fri May 8 15:58:01 CEST 2009


On 08.05.2009 13:41, Uwe Hermann wrote:
> On Fri, May 08, 2009 at 02:57:27AM +0200, Carl-Daniel Hailfinger wrote:
>   
>> To prepare flashrom for Paraflasher (or other external flasher) support,
>> each chip read/write access is either handled as memory mapped access or
>> external flasher cycle.
>>     
>
> This part is ok. These functions certainly don't belong in flash.h.
>   

read_memmapped definitely does not belong in flash.h, true.


>> External flashers can set the flasher variable to their own ID/number
>> during startup and handle accesses in the switch statement inside
>> chip_{read,write}[bwl].
>>     
>
> I'd like to solve this part differently, however. A switch statement
> becomes very big and ugly very soon if we suport more than one flasher.
> In addition all flasher code for all of them must be in one single file.
>
> The attached draft (non-compiling patch) uses a struct and functions to
> solve the issue, then each programmer can have it's own file where it
> provides the required API functions (read, write, init, shutdown, etc).
> E.g. we then have paraflasher.c, usbprog.c, etc.
>   

Agreed. I really like your function pointer approach. I had used it
myself, but I somehow lost the code and started again from scratch.


>> Index: flashrom-external_infrastructure/flash.h
>> ===================================================================
>> --- flashrom-external_infrastructure/flash.h	(Revision 471)
>> +++ flashrom-external_infrastructure/flash.h	(Arbeitskopie)
>>     
> [...]
>   
>> +/* external.c */
>>     
>
> I'd name this rather "internal.c" (for internal flasher) or "board.c"
> (for flashing in the mainboard, vs. using an external flasher).
>   

With your function pointer approach, we have to do it differently anyway.


>> Index: flashrom-external_infrastructure/external.c
>> ===================================================================
>> --- flashrom-external_infrastructure/external.c	(Revision 0)
>> +++ flashrom-external_infrastructure/external.c	(Revision 0)
>> @@ -0,0 +1,111 @@
>>     
> [...]
>   
>> +void chip_writew(uint16_t b, volatile void *addr)
>> +void chip_writel(uint32_t b, volatile void *addr)
>>     
>
> Will we ever need u16 and u32 accessor functions btw? I think they're
> unused right now, not sure if future plans require them(?)
>   

I think they can be used to speed up reading (less overhead for some
protocols).


> Draft for external flasher support.
>
> Signed-off-by: Uwe Hermann <uwe at hermann-uwe.de>
>
> Index: flash.h
> ===================================================================
> --- flash.h	(Revision 472)
> +++ flash.h	(Arbeitskopie)
> +
> +struct programmer {
>   

Maybe call it struct programmer_entry to avoid confusion with int
programmer.

Anyway, I took your code as inspiration and source and created an
alternative patch which should work fine.

Compile tested.

Add external flasher support:
- Read/write accesses through function pointers
- Command line parameter for internal/external flasher
- Board and chipset setup moved to internal init function
- Shutdown stuff moved to internal shutdown function

As a side benefit, this will allow us to undo chipset write enable
during shutdown.

Runtime tests appreciated.

Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>

Index: flashrom-external_functionpointer/flash.h
===================================================================
--- flashrom-external_functionpointer/flash.h	(Revision 473)
+++ flashrom-external_functionpointer/flash.h	(Arbeitskopie)
@@ -76,34 +76,54 @@
 #endif
 #endif
 
+extern int programmer;
+#define PROGRAMMER_INTERNAL 0x00
+
+struct programmer_entry {
+	const char *vendor;
+	const char *name;
+
+	int (*init) (void);
+	int (*shutdown) (void);
+
+	void (*chip_writeb) (uint8_t b, volatile void *addr);
+	void (*chip_writew) (uint16_t b, volatile void *addr);
+	void (*chip_writel) (uint32_t b, volatile void *addr);
+	uint8_t (*chip_readb) (const volatile void *addr);
+	uint16_t (*chip_readw) (const volatile void *addr);
+	uint32_t (*chip_readl) (const volatile void *addr);
+};
+
+extern const struct programmer_entry programmer_table[];
+
 static inline void chip_writeb(uint8_t b, volatile void *addr)
 {
-	*(volatile uint8_t *) addr = b;
+	programmer_table[programmer].chip_writeb(b, addr);
 }
 
 static inline void chip_writew(uint16_t b, volatile void *addr)
 {
-	*(volatile uint16_t *) addr = b;
+	programmer_table[programmer].chip_writew(b, addr);
 }
 
 static inline void chip_writel(uint32_t b, volatile void *addr)
 {
-	*(volatile uint32_t *) addr = b;
+	programmer_table[programmer].chip_writel(b, addr);
 }
 
 static inline uint8_t chip_readb(const volatile void *addr)
 {
-	return *(volatile uint8_t *) addr;
+	return programmer_table[programmer].chip_readb(addr);
 }
 
 static inline uint16_t chip_readw(const volatile void *addr)
 {
-	return *(volatile uint16_t *) addr;
+	return programmer_table[programmer].chip_readw(addr);
 }
 
 static inline uint32_t chip_readl(const volatile void *addr)
 {
-	return *(volatile uint32_t *) addr;
+	return programmer_table[programmer].chip_readl(addr);
 }
 
 #define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0]))
@@ -535,6 +555,16 @@
 void *physmap(const char *descr, unsigned long phys_addr, size_t len);
 void physunmap(void *virt_addr, size_t len);
 
+/* intext.c */
+int internal_init(void);
+int internal_shutdown(void);
+void internal_chip_writeb(uint8_t b, volatile void *addr);
+void internal_chip_writew(uint16_t b, volatile void *addr);
+void internal_chip_writel(uint32_t b, volatile void *addr);
+uint8_t internal_chip_readb(const volatile void *addr);
+uint16_t internal_chip_readw(const volatile void *addr);
+uint32_t internal_chip_readl(const volatile void *addr);
+
 /* flashrom.c */
 extern int verbose;
 #define printf_debug(x...) { if (verbose) printf(x); }
Index: flashrom-external_functionpointer/intext.c
===================================================================
--- flashrom-external_functionpointer/intext.c	(Revision 0)
+++ flashrom-external_functionpointer/intext.c	(Revision 0)
@@ -0,0 +1,166 @@
+/*
+ * This file is part of the flashrom project.
+ *
+ * Copyright (C) 2009 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; 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
+ */
+
+#include <stdint.h>
+#include <string.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <pci/pci.h>
+#include "flash.h"
+
+#if defined(__FreeBSD__) || defined(__DragonFly__)
+int io_fd;
+#endif
+
+const struct programmer_entry programmer_table[] = {
+	{
+		.init		= internal_init,
+		.shutdown	= internal_shutdown,
+		.chip_readb	= internal_chip_readb,
+		.chip_readw	= internal_chip_readw,
+		.chip_readl	= internal_chip_readl,
+		.chip_writeb	= internal_chip_writeb,
+		.chip_writew	= internal_chip_writew,
+		.chip_writel	= internal_chip_writel,
+	},
+
+	{},
+};
+
+struct pci_access *pacc;	/* For board and chipset_enable */
+
+struct pci_dev *pci_dev_find(uint16_t vendor, uint16_t device)
+{
+	struct pci_dev *temp;
+	struct pci_filter filter;
+
+	pci_filter_init(NULL, &filter);
+	filter.vendor = vendor;
+	filter.device = device;
+
+	for (temp = pacc->devices; temp; temp = temp->next)
+		if (pci_filter_match(&filter, temp))
+			return temp;
+
+	return NULL;
+}
+
+struct pci_dev *pci_card_find(uint16_t vendor, uint16_t device,
+			      uint16_t card_vendor, uint16_t card_device)
+{
+	struct pci_dev *temp;
+	struct pci_filter filter;
+
+	pci_filter_init(NULL, &filter);
+	filter.vendor = vendor;
+	filter.device = device;
+
+	for (temp = pacc->devices; temp; temp = temp->next)
+		if (pci_filter_match(&filter, temp)) {
+			if ((card_vendor ==
+			     pci_read_word(temp, PCI_SUBSYSTEM_VENDOR_ID))
+			    && (card_device ==
+				pci_read_word(temp, PCI_SUBSYSTEM_ID)))
+				return temp;
+		}
+
+	return NULL;
+}
+
+int internal_init(void)
+{
+	int ret = 0;
+
+	/* First get full io access */
+#if defined (__sun) && (defined(__i386) || defined(__amd64))
+	if (sysi86(SI86V86, V86SC_IOPL, PS_IOPL) != 0) {
+#elif defined(__FreeBSD__) || defined (__DragonFly__)
+	if ((io_fd = open("/dev/io", O_RDWR)) < 0) {
+#else
+	if (iopl(3) != 0) {
+#endif
+		fprintf(stderr, "ERROR: Could not get IO privileges (%s).\nYou need to be root.\n", strerror(errno));
+		exit(1);
+	}
+
+	/* Initialize PCI access for flash enables */
+	pacc = pci_alloc();	/* Get the pci_access structure */
+	/* Set all options you want -- here we stick with the defaults */
+	pci_init(pacc);		/* Initialize the PCI library */
+	pci_scan_bus(pacc);	/* We want to get the list of devices */
+
+	/* We look at the lbtable first to see if we need a
+	 * mainboard specific flash enable sequence.
+	 */
+	coreboot_init();
+
+	/* try to enable it. Failure IS an option, since not all motherboards
+	 * really need this to be done, etc., etc.
+	 */
+	ret = chipset_flash_enable();
+	if (ret == -2) {
+		printf("WARNING: No chipset found. Flash detection "
+		       "will most likely fail.\n");
+	}
+
+	board_flash_enable(lb_vendor, lb_part);
+
+	return ret; 
+}
+
+int internal_shutdown(void)
+{
+#if defined(__FreeBSD__) || defined(__DragonFly__)
+	close(io_fd);
+#endif
+
+	return 0;
+}
+
+void internal_chip_writeb(uint8_t b, volatile void *addr)
+{
+	*(volatile uint8_t *) addr = b;
+}
+
+void internal_chip_writew(uint16_t b, volatile void *addr)
+{
+	*(volatile uint16_t *) addr = b;
+}
+
+void internal_chip_writel(uint32_t b, volatile void *addr)
+{
+	*(volatile uint32_t *) addr = b;
+}
+
+uint8_t internal_chip_readb(const volatile void *addr)
+{
+	return *(volatile uint8_t *) addr;
+}
+
+uint16_t internal_chip_readw(const volatile void *addr)
+{
+	return *(volatile uint16_t *) addr;
+}
+
+uint32_t internal_chip_readl(const volatile void *addr)
+{
+	return *(volatile uint32_t *) addr;
+}
+
Index: flashrom-external_functionpointer/Makefile
===================================================================
--- flashrom-external_functionpointer/Makefile	(Revision 473)
+++ flashrom-external_functionpointer/Makefile	(Arbeitskopie)
@@ -34,7 +34,7 @@
 	w49f002u.o 82802ab.o pm49fl00x.o sst49lf040.o en29f002a.o \
 	sst49lfxxxc.o sst_fwhub.o layout.o cbtable.o flashchips.o physmap.o \
 	flashrom.o w39v080fa.o sharplhf00l04.o w29ee011.o spi.o it87spi.o \
-	ichspi.o w39v040c.o sb600spi.o wbsio_spi.o m29f002.o
+	ichspi.o w39v040c.o sb600spi.o wbsio_spi.o m29f002.o intext.o
 
 all: pciutils dep $(PROGRAM)
 
Index: flashrom-external_functionpointer/flashrom.c
===================================================================
--- flashrom-external_functionpointer/flashrom.c	(Revision 473)
+++ flashrom-external_functionpointer/flashrom.c	(Arbeitskopie)
@@ -21,7 +21,6 @@
  * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
  */
 
-#include <errno.h>
 #include <fcntl.h>
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -30,52 +29,13 @@
 #include <string.h>
 #include <stdlib.h>
 #include <getopt.h>
-#include <pci/pci.h>
 #include "flash.h"
 
 char *chip_to_probe = NULL;
-struct pci_access *pacc;	/* For board and chipset_enable */
 int exclude_start_page, exclude_end_page;
 int verbose = 0;
+int programmer = PROGRAMMER_INTERNAL;
 
-struct pci_dev *pci_dev_find(uint16_t vendor, uint16_t device)
-{
-	struct pci_dev *temp;
-	struct pci_filter filter;
-
-	pci_filter_init(NULL, &filter);
-	filter.vendor = vendor;
-	filter.device = device;
-
-	for (temp = pacc->devices; temp; temp = temp->next)
-		if (pci_filter_match(&filter, temp))
-			return temp;
-
-	return NULL;
-}
-
-struct pci_dev *pci_card_find(uint16_t vendor, uint16_t device,
-			      uint16_t card_vendor, uint16_t card_device)
-{
-	struct pci_dev *temp;
-	struct pci_filter filter;
-
-	pci_filter_init(NULL, &filter);
-	filter.vendor = vendor;
-	filter.device = device;
-
-	for (temp = pacc->devices; temp; temp = temp->next)
-		if (pci_filter_match(&filter, temp)) {
-			if ((card_vendor ==
-			     pci_read_word(temp, PCI_SUBSYSTEM_VENDOR_ID))
-			    && (card_device ==
-				pci_read_word(temp, PCI_SUBSYSTEM_ID)))
-				return temp;
-		}
-
-	return NULL;
-}
-
 void map_flash_registers(struct flashchip *flash)
 {
 	size_t size = flash->total_size * 1024;
@@ -360,9 +320,6 @@
 	int force = 0;
 	int read_it = 0, write_it = 0, erase_it = 0, verify_it = 0;
 	int ret = 0, i;
-#if defined(__FreeBSD__) || defined(__DragonFly__)
-	int io_fd;
-#endif
 
 	static struct option long_options[] = {
 		{"read", 0, 0, 'r'},
@@ -378,6 +335,7 @@
 		{"layout", 1, 0, 'l'},
 		{"image", 1, 0, 'i'},
 		{"list-supported", 0, 0, 'L'},
+		{"programmer", 1, 0, 'p'},
 		{"help", 0, 0, 'h'},
 		{"version", 0, 0, 'R'},
 		{0, 0, 0, 0}
@@ -399,7 +357,7 @@
 	}
 
 	setbuf(stdout, NULL);
-	while ((opt = getopt_long(argc, argv, "rRwvVEfc:s:e:m:l:i:Lh",
+	while ((opt = getopt_long(argc, argv, "rRwvVEfc:s:e:m:l:i:p:Lh",
 				  long_options, &option_index)) != EOF) {
 		switch (opt) {
 		case 'r':
@@ -458,6 +416,14 @@
 			print_supported_boards();
 			exit(0);
 			break;
+		case 'p':
+			if (strncmp(optarg, "internal", 8) == 0) {
+				programmer = PROGRAMMER_INTERNAL;
+			} else {
+				printf("Error: Unknown programmer.\n");
+				exit(1);
+			}
+			break;
 		case 'R':
 			/* print_version() is always called during startup. */
 			exit(0);
@@ -477,42 +443,10 @@
 	if (optind < argc)
 		filename = argv[optind++];
 
-	/* First get full io access */
-#if defined (__sun) && (defined(__i386) || defined(__amd64))
-	if (sysi86(SI86V86, V86SC_IOPL, PS_IOPL) != 0) {
-#elif defined(__FreeBSD__) || defined (__DragonFly__)
-	if ((io_fd = open("/dev/io", O_RDWR)) < 0) {
-#else
-	if (iopl(3) != 0) {
-#endif
-		fprintf(stderr, "ERROR: Could not get IO privileges (%s).\nYou need to be root.\n", strerror(errno));
-		exit(1);
-	}
+	ret = programmer_table[programmer].init();
 
-	/* Initialize PCI access for flash enables */
-	pacc = pci_alloc();	/* Get the pci_access structure */
-	/* Set all options you want -- here we stick with the defaults */
-	pci_init(pacc);		/* Initialize the PCI library */
-	pci_scan_bus(pacc);	/* We want to get the list of devices */
-
 	myusec_calibrate_delay();
 
-	/* We look at the lbtable first to see if we need a
-	 * mainboard specific flash enable sequence.
-	 */
-	coreboot_init();
-
-	/* try to enable it. Failure IS an option, since not all motherboards
-	 * really need this to be done, etc., etc.
-	 */
-	ret = chipset_flash_enable();
-	if (ret == -2) {
-		printf("WARNING: No chipset found. Flash detection "
-		       "will most likely fail.\n");
-	}
-
-	board_flash_enable(lb_vendor, lb_part);
-
 	for (i = 0; i < ARRAY_SIZE(flashes); i++) {
 		flashes[i] =
 		    probe_flash(i ? flashes[i - 1] + 1 : flashchips, 0);
@@ -702,8 +636,7 @@
 	if (verify_it)
 		ret |= verify_flash(flash, buf);
 
-#ifdef __FreeBSD__
-	close(io_fd);
-#endif
+	programmer_table[programmer].shutdown();
+
 	return ret;
 }


-- 
http://www.hailfinger.org/

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: flashrom_external_infrastructure_functionpointer.diff
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20090508/6c5d0580/attachment.ksh>


More information about the coreboot mailing list