Hi Mark,
thank you for your patch.
On 01.12.2010 19:12, Mark Marshall wrote:
The attached patch adds support for the Open Graphics Project development card, OGD1, as a SPI flash programmer. The project is in the the process of designing and making a complete, open source, graphics card. Check out http://wiki.opengraphics.org.
The first development card is a PCI add in card containing a couple of FPGAs and a couple of serial flash chips (amongst other things). The FPGA's are called XP10 and S3 (their part numbers). The XP10 contains it's own flash and does not need to be programmed by flashrom - it ensures that the device can enumerate on the PCI bus without needing further configuration.
The larger FPGA is the S3. This is configured from a large serial flash (2M bytes). The second serial flash is used to store the VGA BIOS. It is smaller (128K bytes).
The attached patch adds support for programming either of the two serial flash chips. They are both serial SPI devices.
The programmer device takes one configuration option which selects which of the two flash chips is accessed. This must be set to either "cprom" or "bprom". (The project refers to the two chips as "cprom" / "bprom", "s3" and "bios" are more readable alternatives).
Should we use strcasecmp for case-insensitive comparison of those names? I guess it depends on the user population you expect.
I have also attached a log of the modified flashrom talking to both devices. I can read, erase, write and verify to both. The chips installed on my card are a SST25VF010 and a SST25VF016B. I have added a section to flashchips.c for the former. I have modified the flags of the later to say that writing works.
I made a few small changes to your patch (whitespace, some code simplifications, added error messages). Please note that the bitbanging core in flashrom guarantees that val is either 0 or 1, and val will never be anything else.
What has me a bit stumped is the naming. The file is called oga1_spi.c and the programmer is called oga1_spi, so that part is consistent. The PCI device seems to be called OGD1, not OGA1. And looking at the naming scheme the rest of flashrom uses, we should probably use OGP (vendor) as name for the programmer instead of OGA1 or OGD1.
You may want to use the long name "Open Graphics Project" in the PCI ID table, and expand the name of the device as well. Compare: Supported devices for the oga1_spi programmer: PCI devices: OGP OGD1 [1227:0000] and Supported devices for the ft2232_spi programmer: USB devices: First International Computer, Inc. OpenMoko Neo1973 Debug board (V2+) [1457:5118]
A man page entry in flashrom.8 would be appreciated. Feel free to link to your project in the man page entry (rayer_spi would be a good template). You can either send a combined patch, or I can go ahead and commit this and commit the man page patch separately.
Signed-off-by: Mark Marshall mark.marshall@csr.com
Acked-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Modified patch follows. If it is ok with you and if you are confident that the naming is right, I'll commit.
Index: flashrom-mmarshall_oga1_spi/oga1_spi.c =================================================================== --- flashrom-mmarshall_oga1_spi/oga1_spi.c (Revision 0) +++ flashrom-mmarshall_oga1_spi/oga1_spi.c (Revision 0) @@ -0,0 +1,143 @@ +/* + * This file is part of the flashrom project. + * + * Copyright (C) 2010 Mark Marshall + * + * 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 + */ + +#include <stdlib.h> +#include <string.h> +#include "flash.h" +#include "programmer.h" + +#define PCI_VENDOR_ID_OGP 0x1227 + +#define OGA1_XP10_BPROM_SI 0x0040 /* W */ +#define OGA1_XP10_BPROM_SO 0x0040 /* R */ +#define OGA1_XP10_BPROM_CE_BAR 0x0044 /* W */ +#define OGA1_XP10_BPROM_SCK 0x0048 /* W */ +#define OGA1_XP10_BPROM_REG_SEL 0x004C /* W */ +#define OGA1_XP10_CPROM_SI 0x0050 /* W */ +#define OGA1_XP10_CPROM_SO 0x0050 /* R */ +#define OGA1_XP10_CPROM_CE_BAR 0x0054 /* W */ +#define OGA1_XP10_CPROM_SCK 0x0058 /* W */ +#define OGA1_XP10_CPROM_REG_SEL 0x005C /* W */ + +static uint8_t *oga1_spibar; + +static uint32_t oga1_reg_sel; +static uint32_t oga1_reg_siso; +static uint32_t oga1_reg__ce; +static uint32_t oga1_reg_sck; + +const struct pcidev_status oga1_spi[] = { + {PCI_VENDOR_ID_OGP, 0x0000, OK, "OGP", "OGD1"}, + {}, +}; + +static void oga1_request_spibus(void) +{ + pci_mmio_writel(1, oga1_spibar + oga1_reg_sel); +} + +static void oga1_release_spibus(void) +{ + pci_mmio_writel(0, oga1_spibar + oga1_reg_sel); +} + +static void oga1_bitbang_set_cs(int val) +{ + pci_mmio_writel(val, oga1_spibar + oga1_reg__ce); +} + +static void oga1_bitbang_set_sck(int val) +{ + pci_mmio_writel(val, oga1_spibar + oga1_reg_sck); +} + +static void oga1_bitbang_set_mosi(int val) +{ + pci_mmio_writel(val, oga1_spibar + oga1_reg_siso); +} + +static int oga1_bitbang_get_miso(void) +{ + uint32_t tmp; + + tmp = pci_mmio_readl(oga1_spibar + oga1_reg_siso); + return tmp & 0x1; +} + +static const struct bitbang_spi_master bitbang_spi_master_oga1 = { + .type = BITBANG_SPI_MASTER_OGA1, + .set_cs = oga1_bitbang_set_cs, + .set_sck = oga1_bitbang_set_sck, + .set_mosi = oga1_bitbang_set_mosi, + .get_miso = oga1_bitbang_get_miso, + .request_bus = oga1_request_spibus, + .release_bus = oga1_release_spibus, +}; + +int oga1_spi_init(void) +{ + char *type; + + type = extract_programmer_param("rom"); + + if (!type) { + msg_perr("Please use flashrom -p oga1_spi:rom=... to specify " + "which flashchip you want to access.\n"); + return 1; + } else if (!strcmp(type, "bprom") || !strcmp(type, "bios")) { + oga1_reg_sel = OGA1_XP10_BPROM_REG_SEL; + oga1_reg_siso = OGA1_XP10_BPROM_SI; + oga1_reg__ce = OGA1_XP10_BPROM_CE_BAR; + oga1_reg_sck = OGA1_XP10_BPROM_SCK; + } else if (!strcmp(type, "cprom") || !strcmp(type, "s3")) { + oga1_reg_sel = OGA1_XP10_CPROM_REG_SEL; + oga1_reg_siso = OGA1_XP10_CPROM_SI; + oga1_reg__ce = OGA1_XP10_CPROM_CE_BAR; + oga1_reg_sck = OGA1_XP10_CPROM_SCK; + } else { + msg_perr("Invalid or missing rom= parameter.\n"); + return 1; + } + + get_io_perms(); + + io_base_addr = pcidev_init(PCI_VENDOR_ID_OGP, PCI_BASE_ADDRESS_0, + oga1_spi); + + oga1_spibar = physmap("OGD1 registers", + io_base_addr, 4096); + + /* no delay for now. */ + if (bitbang_spi_init(&bitbang_spi_master_oga1, 0)) + return 1; + + buses_supported = CHIP_BUSTYPE_SPI; + spi_controller = SPI_CONTROLLER_OGA1; + + return 0; +} + +int oga1_spi_shutdown(void) +{ + physunmap(oga1_spibar, 4096); + pci_cleanup(pacc); + release_io_perms(); + + return 0; +} Index: flashrom-mmarshall_oga1_spi/Makefile =================================================================== --- flashrom-mmarshall_oga1_spi/Makefile (Revision 1237) +++ flashrom-mmarshall_oga1_spi/Makefile (Arbeitskopie) @@ -146,6 +146,9 @@ # Always enable SPI on Intel NICs for now. CONFIG_NICINTEL_SPI ?= yes
+# Always enable SPI on OGP cards for now. +CONFIG_OGA1_SPI ?= yes + # Always enable Bus Pirate SPI for now. CONFIG_BUSPIRATE_SPI ?= yes
@@ -165,10 +168,14 @@ ifeq ($(CONFIG_NICINTEL_SPI), yes) override CONFIG_BITBANG_SPI = yes else +ifeq ($(CONFIG_OGA1_SPI), yes) +override CONFIG_BITBANG_SPI = yes +else CONFIG_BITBANG_SPI ?= no endif endif endif +endif
ifeq ($(CONFIG_INTERNAL), yes) FEATURE_CFLAGS += -D'CONFIG_INTERNAL=1' @@ -258,6 +265,12 @@ NEED_PCI := yes endif
+ifeq ($(CONFIG_OGA1_SPI), yes) +FEATURE_CFLAGS += -D'CONFIG_OGA1_SPI=1' +PROGRAMMER_OBJS += oga1_spi.o +NEED_PCI := yes +endif + ifeq ($(CONFIG_BUSPIRATE_SPI), yes) FEATURE_CFLAGS += -D'CONFIG_BUSPIRATE_SPI=1' PROGRAMMER_OBJS += buspirate_spi.o Index: flashrom-mmarshall_oga1_spi/print_wiki.c =================================================================== --- flashrom-mmarshall_oga1_spi/print_wiki.c (Revision 1237) +++ flashrom-mmarshall_oga1_spi/print_wiki.c (Arbeitskopie) @@ -299,6 +299,9 @@ #if CONFIG_NICINTEL_SPI == 1 print_supported_pcidevs_wiki(nics_intel_spi); #endif +#if CONFIG_OGA1_SPI == 1 + print_supported_pcidevs_wiki(oga1_spi); +#endif printf("\n|}\n"); }
Index: flashrom-mmarshall_oga1_spi/flashchips.c =================================================================== --- flashrom-mmarshall_oga1_spi/flashchips.c (Revision 1237) +++ flashrom-mmarshall_oga1_spi/flashchips.c (Arbeitskopie) @@ -4903,7 +4903,7 @@ .model_id = SST_SST25VF016B, .total_size = 2048, .page_size = 256, - .tested = TEST_OK_PRE, + .tested = TEST_OK_PREW, .probe = probe_spi_rdid, .probe_timing = TIMING_ZERO, .block_erasers = @@ -5002,6 +5002,35 @@
{ .vendor = "SST", + .name = "SST25VF010.REMS", + .bustype = CHIP_BUSTYPE_SPI, + .manufacture_id = SST_ID, + .model_id = SST_SST25VF010_REMS, + .total_size = 128, + .page_size = 256, + .tested = TEST_OK_PREW, + .probe = probe_spi_rems, + .probe_timing = TIMING_ZERO, + .block_erasers = + { + { + .eraseblocks = { {4 * 1024, 32} }, + .block_erase = spi_block_erase_20, + }, { + .eraseblocks = { {32 * 1024, 4} }, + .block_erase = spi_block_erase_52, + }, { + .eraseblocks = { {128 * 1024, 1} }, + .block_erase = spi_block_erase_60, + }, + }, + .unlock = spi_disable_blockprotect, + .write = spi_chip_write_1, + .read = spi_chip_read, + }, + + { + .vendor = "SST", .name = "SST25VF040.REMS", .bustype = CHIP_BUSTYPE_SPI, .manufacture_id = SST_ID, Index: flashrom-mmarshall_oga1_spi/spi.c =================================================================== --- flashrom-mmarshall_oga1_spi/spi.c (Revision 1237) +++ flashrom-mmarshall_oga1_spi/spi.c (Arbeitskopie) @@ -146,6 +146,15 @@ }, #endif
+#if CONFIG_OGA1_SPI == 1 + { /* SPI_CONTROLLER_OGA1 */ + .command = bitbang_spi_send_command, + .multicommand = default_spi_send_multicommand, + .read = bitbang_spi_read, + .write_256 = bitbang_spi_write_256, + }, +#endif + {}, /* This entry corresponds to SPI_CONTROLLER_INVALID. */ };
Index: flashrom-mmarshall_oga1_spi/flashrom.c =================================================================== --- flashrom-mmarshall_oga1_spi/flashrom.c (Revision 1237) +++ flashrom-mmarshall_oga1_spi/flashrom.c (Arbeitskopie) @@ -52,7 +52,7 @@ * if more than one of them is selected. If only one is selected, it is clear * that the user wants that one to become the default. */ -#if CONFIG_NIC3COM+CONFIG_NICREALTEK+CONFIG_NICNATSEMI+CONFIG_GFXNVIDIA+CONFIG_DRKAISER+CONFIG_SATASII+CONFIG_ATAHPT+CONFIG_FT2232_SPI+CONFIG_SERPROG+CONFIG_BUSPIRATE_SPI+CONFIG_DEDIPROG+CONFIG_RAYER_SPI+CONFIG_NICINTEL_SPI > 1 +#if CONFIG_NIC3COM+CONFIG_NICREALTEK+CONFIG_NICNATSEMI+CONFIG_GFXNVIDIA+CONFIG_DRKAISER+CONFIG_SATASII+CONFIG_ATAHPT+CONFIG_FT2232_SPI+CONFIG_SERPROG+CONFIG_BUSPIRATE_SPI+CONFIG_DEDIPROG+CONFIG_RAYER_SPI+CONFIG_NICINTEL_SPI+CONFIG_OGA1_SPI > 1 #error Please enable either CONFIG_DUMMY or CONFIG_INTERNAL or disable support for all programmers except one. #endif enum programmer programmer = @@ -96,6 +96,9 @@ #if CONFIG_NICINTEL_SPI == 1 PROGRAMMER_NICINTEL_SPI #endif +#if CONFIG_OGA1_SPI == 1 + PROGRAMMER_OGA1_SPI +#endif ; #endif
@@ -439,6 +442,25 @@ }, #endif
+#if CONFIG_OGA1_SPI == 1 + { + .name = "oga1_spi", + .init = oga1_spi_init, + .shutdown = oga1_spi_shutdown, + .map_flash_region = fallback_map, + .unmap_flash_region = fallback_unmap, + .chip_readb = noop_chip_readb, + .chip_readw = fallback_chip_readw, + .chip_readl = fallback_chip_readl, + .chip_readn = fallback_chip_readn, + .chip_writeb = noop_chip_writeb, + .chip_writew = fallback_chip_writew, + .chip_writel = fallback_chip_writel, + .chip_writen = fallback_chip_writen, + .delay = internal_delay, + }, +#endif + {}, /* This entry corresponds to PROGRAMMER_INVALID. */ };
Index: flashrom-mmarshall_oga1_spi/programmer.h =================================================================== --- flashrom-mmarshall_oga1_spi/programmer.h (Revision 1237) +++ flashrom-mmarshall_oga1_spi/programmer.h (Arbeitskopie) @@ -76,6 +76,9 @@ #if CONFIG_NICINTEL_SPI == 1 PROGRAMMER_NICINTEL_SPI, #endif +#if CONFIG_OGA1_SPI == 1 + PROGRAMMER_OGA1_SPI, +#endif PROGRAMMER_INVALID /* This must always be the last entry. */ };
@@ -121,6 +124,9 @@ BITBANG_SPI_MASTER_MCP, #endif #endif +#if CONFIG_OGA1_SPI == 1 + BITBANG_SPI_MASTER_OGA1, +#endif };
struct bitbang_spi_master { @@ -221,7 +227,7 @@ #endif
/* print.c */ -#if CONFIG_NIC3COM+CONFIG_NICREALTEK+CONFIG_NICNATSEMI+CONFIG_GFXNVIDIA+CONFIG_DRKAISER+CONFIG_SATASII+CONFIG_ATAHPT+CONFIG_NICINTEL_SPI >= 1 +#if CONFIG_NIC3COM+CONFIG_NICREALTEK+CONFIG_NICNATSEMI+CONFIG_GFXNVIDIA+CONFIG_DRKAISER+CONFIG_SATASII+CONFIG_ATAHPT+CONFIG_NICINTEL_SPI+CONFIG_OGA1_SPI >= 1 void print_supported_pcidevs(const struct pcidev_status *devs); #endif
@@ -402,6 +408,16 @@ extern const struct pcidev_status nics_intel_spi[]; #endif
+/* oga1_spi.c */ +#if CONFIG_OGA1_SPI == 1 +int oga1_spi_init(void); +int oga1_spi_shutdown(void); +int oga1_spi_send_command(unsigned int writecnt, unsigned int readcnt, + const unsigned char *writearr, unsigned char *readarr); +void oga1_spi_chip_writeb(uint8_t val, chipaddr addr); +extern const struct pcidev_status oga1_spi[]; +#endif + /* satasii.c */ #if CONFIG_SATASII == 1 int satasii_init(void); @@ -523,6 +539,9 @@ #if CONFIG_NICINTEL_SPI == 1 SPI_CONTROLLER_NICINTEL, #endif +#if CONFIG_OGA1_SPI == 1 + SPI_CONTROLLER_OGA1, +#endif SPI_CONTROLLER_INVALID /* This must always be the last entry. */ }; extern const int spi_programmer_count; Index: flashrom-mmarshall_oga1_spi/print.c =================================================================== --- flashrom-mmarshall_oga1_spi/print.c (Revision 1237) +++ flashrom-mmarshall_oga1_spi/print.c (Arbeitskopie) @@ -312,6 +312,11 @@ programmer_table[PROGRAMMER_NICINTEL_SPI].name); print_supported_pcidevs(nics_intel_spi); #endif +#if CONFIG_OGA1_SPI == 1 + printf("\nSupported devices for the %s programmer:\n", + programmer_table[PROGRAMMER_OGA1_SPI].name); + print_supported_pcidevs(oga1_spi); +#endif }
#if CONFIG_INTERNAL == 1