[flashrom] Improving pcidev.c
Carl-Daniel Hailfinger
c-d.hailfinger.devel.2006 at gmx.net
Tue Jan 1 19:46:48 CET 2013
Am 04.09.2012 12:14 schrieb Stefan Tauner:
> On Tue, 31 Jul 2012 09:27:47 +0200
> Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net> wrote:
>
>> Ping?
>>
>> IMHO this patch fixes a few structural problems, and although it
>> probably isn't the final desired result, it is a step in the right
>> direction.
> After my short adventure into trying to understand the PCI code while
> refining the atavia patch by Jonathan, i have to agree 100% with
> Michael's first mail regarding the old code base.
>
> Something that hasn't been mentioned (or i have missed) is that there
> are some drivers that don't need a BAR at all because they work by
> accessing the configuration space only. atavia is one case (at least i
> think so). and satasii is even more special as it needs different BARs
> for different PCI IDs, but you are aware of that afaics.
>
> I still don't have a complete overview on how all pcidev_init
> callers work, but the current patch seems to improve things and hence
> should go in ASAP (you can read that as an ACK if you think it is safe).
>
> Since you touch all pcidev_init calls in this patch, it would be great
> if you could switch the parameters though. The PCI dev table should be
> first since it is the most important argument and the bar should IMHO
> even be optional in the future or some kind of flag/mask as you
> discussed... that argument apparently confused not only me but also
> Jonathan and Idwer in the past.
> A comment explaining the parameters would certainly improve the
> situation too (e.g. mention that the bar parameter is not the number of
> the BAR!).
OK, so I did address only some of the comments, but I wanted to send the
current version out there (and it kills a boatload of exit(1) as well).
Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
Index: flashrom-pcidev_init_do_not_return_bar/ogp_spi.c
===================================================================
--- flashrom-pcidev_init_do_not_return_bar/ogp_spi.c (Revision 1640)
+++ flashrom-pcidev_init_do_not_return_bar/ogp_spi.c (Arbeitskopie)
@@ -107,6 +107,7 @@
int ogp_spi_init(void)
{
+ struct pci_dev *dev = NULL;
char *type;
type = extract_programmer_param("rom");
@@ -133,7 +134,8 @@
if (rget_io_perms())
return 1;
- io_base_addr = pcidev_init(PCI_BASE_ADDRESS_0, ogp_spi);
+ dev = pcidev_init(PCI_BASE_ADDRESS_0, ogp_spi);
+ io_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0);
ogp_spibar = physmap("OGP registers", io_base_addr, 4096);
Index: flashrom-pcidev_init_do_not_return_bar/drkaiser.c
===================================================================
--- flashrom-pcidev_init_do_not_return_bar/drkaiser.c (Revision 1640)
+++ flashrom-pcidev_init_do_not_return_bar/drkaiser.c (Arbeitskopie)
@@ -66,16 +66,17 @@
int drkaiser_init(void)
{
+ struct pci_dev *dev = NULL;
uint32_t addr;
if (rget_io_perms())
return 1;
- addr = pcidev_init(PCI_BASE_ADDRESS_2, drkaiser_pcidev);
+ dev = pcidev_init(PCI_BASE_ADDRESS_2, drkaiser_pcidev);
+ addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_2);
/* Write magic register to enable flash write. */
- rpci_write_word(pcidev_dev, PCI_MAGIC_DRKAISER_ADDR,
- PCI_MAGIC_DRKAISER_VALUE);
+ rpci_write_word(dev, PCI_MAGIC_DRKAISER_ADDR, PCI_MAGIC_DRKAISER_VALUE);
/* Map 128kB flash memory window. */
drkaiser_bar = physmap("Dr. Kaiser PC-Waechter flash memory",
Index: flashrom-pcidev_init_do_not_return_bar/pcidev.c
===================================================================
--- flashrom-pcidev_init_do_not_return_bar/pcidev.c (Revision 1640)
+++ flashrom-pcidev_init_do_not_return_bar/pcidev.c (Arbeitskopie)
@@ -27,7 +27,6 @@
uint32_t io_base_addr;
struct pci_access *pacc;
-struct pci_dev *pcidev_dev = NULL;
enum pci_bartype {
TYPE_MEMBAR,
@@ -154,18 +153,31 @@
return (uintptr_t)addr;
}
-uintptr_t pcidev_init(int bar, const struct dev_entry *devs)
+int pci_cleanup_wrapper(void *pa)
{
+ pci_cleanup(pa);
+ return 0;
+}
+
+/* pcidev_init gets an array of allowed PCI device IDs and returns a pointer to struct pci_dev iff exactly one
+ * match was found. If the "pci=bb:dd.f" programmer parameter was specified, a match is only considered if it
+ * also matches the specified bus:device.function.
+ * For convenience, this function also registers its own undo handlers.
+ */
+struct pci_dev *pcidev_init(int bar, const struct dev_entry *devs)
+{
struct pci_dev *dev;
+ struct pci_dev *found_dev = NULL;
struct pci_filter filter;
char *pcidev_bdf;
char *msg = NULL;
int found = 0;
int i;
- uintptr_t addr = 0, curaddr = 0;
+ uintptr_t addr = 0;
pacc = pci_alloc(); /* Get the pci_access structure */
pci_init(pacc); /* Initialize the PCI library */
+ register_shutdown(pci_cleanup_wrapper, pacc);
pci_scan_bus(pacc); /* We want to get the list of devices */
pci_filter_init(pacc, &filter);
@@ -174,7 +186,7 @@
if (pcidev_bdf != NULL) {
if ((msg = pci_filter_parse_slot(&filter, pcidev_bdf))) {
msg_perr("Error: %s\n", msg);
- exit(1);
+ return NULL;
}
}
free(pcidev_bdf);
@@ -204,8 +216,7 @@
* just those with a valid BAR.
*/
if ((addr = pcidev_readbar(dev, bar)) != 0) {
- curaddr = addr;
- pcidev_dev = dev;
+ found_dev = dev;
found++;
}
}
@@ -214,14 +225,14 @@
/* Only continue if exactly one supported PCI dev has been found. */
if (found == 0) {
msg_perr("Error: No supported PCI device found.\n");
- exit(1);
+ return NULL;
} else if (found > 1) {
msg_perr("Error: Multiple supported PCI devices found. Use 'flashrom -p xxxx:pci=bb:dd.f' \n"
"to explicitly select the card with the given BDF (PCI bus, device, function).\n");
- exit(1);
+ return NULL;
}
- return curaddr;
+ return found_dev;
}
enum pci_write_type {
Index: flashrom-pcidev_init_do_not_return_bar/gfxnvidia.c
===================================================================
--- flashrom-pcidev_init_do_not_return_bar/gfxnvidia.c (Revision 1640)
+++ flashrom-pcidev_init_do_not_return_bar/gfxnvidia.c (Arbeitskopie)
@@ -89,12 +89,17 @@
int gfxnvidia_init(void)
{
+ struct pci_dev *dev = NULL;
uint32_t reg32;
if (rget_io_perms())
return 1;
- io_base_addr = pcidev_init(PCI_BASE_ADDRESS_0, gfx_nvidia);
+ dev = pcidev_init(PCI_BASE_ADDRESS_0, gfx_nvidia);
+ if (!dev) {
+ return 1;
+ }
+ io_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0);
io_base_addr += 0x300000;
msg_pinfo("Detected NVIDIA I/O base address: 0x%x.\n", io_base_addr);
@@ -106,9 +111,9 @@
return 1;
/* Allow access to flash interface (will disable screen). */
- reg32 = pci_read_long(pcidev_dev, 0x50);
+ reg32 = pci_read_long(dev, 0x50);
reg32 &= ~(1 << 0);
- rpci_write_long(pcidev_dev, 0x50, reg32);
+ rpci_write_long(dev, 0x50, reg32);
/* Write/erase doesn't work. */
programmer_may_write = 0;
Index: flashrom-pcidev_init_do_not_return_bar/nicrealtek.c
===================================================================
--- flashrom-pcidev_init_do_not_return_bar/nicrealtek.c (Revision 1640)
+++ flashrom-pcidev_init_do_not_return_bar/nicrealtek.c (Arbeitskopie)
@@ -60,16 +60,19 @@
int nicrealtek_init(void)
{
+ struct pci_dev *dev = NULL;
+
if (rget_io_perms())
return 1;
- io_base_addr = pcidev_init(PCI_BASE_ADDRESS_0, nics_realtek);
+ dev = pcidev_init(PCI_BASE_ADDRESS_0, nics_realtek);
+ io_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0);
if (register_shutdown(nicrealtek_shutdown, NULL))
return 1;
/* Beware, this ignores the vendor ID! */
- switch (pcidev_dev->device_id) {
+ switch (dev->device_id) {
case 0x8139: /* RTL8139 */
case 0x1211: /* SMC 1211TX */
default:
Index: flashrom-pcidev_init_do_not_return_bar/satamv.c
===================================================================
--- flashrom-pcidev_init_do_not_return_bar/satamv.c (Revision 1640)
+++ flashrom-pcidev_init_do_not_return_bar/satamv.c (Arbeitskopie)
@@ -82,6 +82,7 @@
*/
int satamv_init(void)
{
+ struct pci_dev *dev = NULL;
uintptr_t addr;
uint32_t tmp;
@@ -92,7 +93,8 @@
/* No need to check for errors, pcidev_init() will not return in case
* of errors.
*/
- addr = pcidev_init(PCI_BASE_ADDRESS_0, satas_mv);
+ dev = pcidev_init(PCI_BASE_ADDRESS_0, satas_mv);
+ addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0);
mv_bar = physmap("Marvell 88SX7042 registers", addr, 0x20000);
if (mv_bar == ERROR_PTR)
@@ -144,8 +146,7 @@
pci_rmmio_writel(tmp, mv_bar + GPIO_PORT_CONTROL);
/* Get I/O BAR location. */
- tmp = pci_read_long(pcidev_dev, PCI_BASE_ADDRESS_2) &
- PCI_BASE_ADDRESS_IO_MASK;
+ tmp = pcidev_readbar(dev, PCI_BASE_ADDRESS_2);
/* Truncate to reachable range.
* FIXME: Check if the I/O BAR is actually reachable.
* This is an arch specific check.
Index: flashrom-pcidev_init_do_not_return_bar/nicintel_spi.c
===================================================================
--- flashrom-pcidev_init_do_not_return_bar/nicintel_spi.c (Revision 1640)
+++ flashrom-pcidev_init_do_not_return_bar/nicintel_spi.c (Arbeitskopie)
@@ -167,12 +167,14 @@
int nicintel_spi_init(void)
{
+ struct pci_dev *dev = NULL;
uint32_t tmp;
if (rget_io_perms())
return 1;
- io_base_addr = pcidev_init(PCI_BASE_ADDRESS_0, nics_intel_spi);
+ dev = pcidev_init(PCI_BASE_ADDRESS_0, nics_intel_spi);
+ io_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0);
nicintel_spibar = physmap("Intel Gigabit NIC w/ SPI flash",
io_base_addr, MEMMAP_SIZE);
Index: flashrom-pcidev_init_do_not_return_bar/nicnatsemi.c
===================================================================
--- flashrom-pcidev_init_do_not_return_bar/nicnatsemi.c (Revision 1640)
+++ flashrom-pcidev_init_do_not_return_bar/nicnatsemi.c (Arbeitskopie)
@@ -60,10 +60,13 @@
int nicnatsemi_init(void)
{
+ struct pci_dev *dev = NULL;
+
if (rget_io_perms())
return 1;
- io_base_addr = pcidev_init(PCI_BASE_ADDRESS_0, nics_natsemi);
+ dev = pcidev_init(PCI_BASE_ADDRESS_0, nics_natsemi);
+ io_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0);
if (register_shutdown(nicnatsemi_shutdown, NULL))
return 1;
Index: flashrom-pcidev_init_do_not_return_bar/atahpt.c
===================================================================
--- flashrom-pcidev_init_do_not_return_bar/atahpt.c (Revision 1640)
+++ flashrom-pcidev_init_do_not_return_bar/atahpt.c (Arbeitskopie)
@@ -65,20 +65,22 @@
int atahpt_init(void)
{
+ struct pci_dev *dev = NULL;
uint32_t reg32;
if (rget_io_perms())
return 1;
- io_base_addr = pcidev_init(PCI_BASE_ADDRESS_4, ata_hpt);
+ dev = pcidev_init(PCI_BASE_ADDRESS_4, ata_hpt);
+ io_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_4);
if (register_shutdown(atahpt_shutdown, NULL))
return 1;
/* Enable flash access. */
- reg32 = pci_read_long(pcidev_dev, REG_FLASH_ACCESS);
+ reg32 = pci_read_long(dev, REG_FLASH_ACCESS);
reg32 |= (1 << 24);
- rpci_write_long(pcidev_dev, REG_FLASH_ACCESS, reg32);
+ rpci_write_long(dev, REG_FLASH_ACCESS, reg32);
register_par_programmer(&par_programmer_atahpt, BUS_PARALLEL);
Index: flashrom-pcidev_init_do_not_return_bar/nic3com.c
===================================================================
--- flashrom-pcidev_init_do_not_return_bar/nic3com.c (Revision 1640)
+++ flashrom-pcidev_init_do_not_return_bar/nic3com.c (Arbeitskopie)
@@ -87,12 +87,15 @@
int nic3com_init(void)
{
+ struct pci_dev *dev = NULL;
+
if (rget_io_perms())
return 1;
- io_base_addr = pcidev_init(PCI_BASE_ADDRESS_0, nics_3com);
+ dev = pcidev_init(PCI_BASE_ADDRESS_0, nics_3com);
+ io_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0);
- id = pcidev_dev->device_id;
+ id = dev->device_id;
/* 3COM 3C90xB cards need a special fixup. */
if (id == 0x9055 || id == 0x9001 || id == 0x9004 || id == 0x9005
Index: flashrom-pcidev_init_do_not_return_bar/satasii.c
===================================================================
--- flashrom-pcidev_init_do_not_return_bar/satasii.c (Revision 1640)
+++ flashrom-pcidev_init_do_not_return_bar/satasii.c (Arbeitskopie)
@@ -77,21 +77,22 @@
int satasii_init(void)
{
+ struct pci_dev *dev = NULL;
uint32_t addr;
uint16_t reg_offset;
if (rget_io_perms())
return 1;
- pcidev_init(PCI_BASE_ADDRESS_0, satas_sii);
+ dev = pcidev_init(PCI_BASE_ADDRESS_0, satas_sii);
- id = pcidev_dev->device_id;
+ id = dev->device_id;
if ((id == 0x3132) || (id == 0x3124)) {
- addr = pci_read_long(pcidev_dev, PCI_BASE_ADDRESS_0) & ~0x07;
+ addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0);
reg_offset = 0x70;
} else {
- addr = pci_read_long(pcidev_dev, PCI_BASE_ADDRESS_5) & ~0x07;
+ addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_5);
reg_offset = 0x50;
}
Index: flashrom-pcidev_init_do_not_return_bar/nicintel.c
===================================================================
--- flashrom-pcidev_init_do_not_return_bar/nicintel.c (Revision 1640)
+++ flashrom-pcidev_init_do_not_return_bar/nicintel.c (Arbeitskopie)
@@ -69,6 +69,7 @@
int nicintel_init(void)
{
+ struct pci_dev *dev = NULL;
uintptr_t addr;
/* Needed only for PCI accesses on some platforms.
@@ -81,14 +82,14 @@
* of errors.
* FIXME: BAR2 is not available if the device uses the CardBus function.
*/
- addr = pcidev_init(PCI_BASE_ADDRESS_2, nics_intel);
+ dev = pcidev_init(PCI_BASE_ADDRESS_2, nics_intel);
+ addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_2);
nicintel_bar = physmap("Intel NIC flash", addr, NICINTEL_MEMMAP_SIZE);
if (nicintel_bar == ERROR_PTR)
goto error_out_unmap;
- /* FIXME: Using pcidev_dev _will_ cause pretty explosions in the future. */
- addr = pcidev_readbar(pcidev_dev, PCI_BASE_ADDRESS_0);
+ addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0);
/* FIXME: This is not an aligned mapping. Use 4k? */
nicintel_control_bar = physmap("Intel NIC control/status reg",
addr, NICINTEL_CONTROL_MEMMAP_SIZE);
Index: flashrom-pcidev_init_do_not_return_bar/programmer.h
===================================================================
--- flashrom-pcidev_init_do_not_return_bar/programmer.h (Revision 1640)
+++ flashrom-pcidev_init_do_not_return_bar/programmer.h (Arbeitskopie)
@@ -237,9 +237,8 @@
// FIXME: These need to be local, not global
extern uint32_t io_base_addr;
extern struct pci_access *pacc;
-extern struct pci_dev *pcidev_dev;
uintptr_t pcidev_readbar(struct pci_dev *dev, int bar);
-uintptr_t pcidev_init(int bar, const struct dev_entry *devs);
+struct pci_dev *pcidev_init(int bar, const struct dev_entry *devs);
/* rpci_write_* are reversible writes. The original PCI config space register
* contents will be restored on shutdown.
*/
--
http://www.hailfinger.org/
More information about the flashrom
mailing list