Second try, with the patch actually attached this time.
Signed-off-by: David Hendricks <[email protected]>
On Thu, Apr 21, 2011 at 9:28 PM, David Hendricks <[email protected]>wrote:
> Hi everyone,
> Lately we've been looking into reverse
> PCI<http://flashrom.org/trac/flashrom/changeset/1232>and
> MMIO <http://patchwork.coreboot.org/patch/2331/> writes, along with the
> shutdown callback mechanism.
>
> The main usage case for Flashrom makes this a pretty simple matter, since
> most users only worry about a single programmer being used. However, when
> external devices are used which depend on internal programmer settings,
> things can get messy.
>
> Consider the case of a laptop with an x86 southbridge and an EC -- To
> communicate with the EC thru the LPC/FWH interface, some register settings
> in the southbridge might need to be changed. The code flow should be:
> 1. Set up SB, use rpci_* and rmmio_* to register reverse PCI and MMIO
> callbacks wherever necessary.
> 2. Set up EC.
> 3. Run code.
> 4. Call EC shutdown routine.
> 5. Reverse SB PCI/MMIO writes.
>
> However, the current code calls the programmer shutdown routine in step 4
> *after* doing all shutdown callbacks in step 5. This causes southbridge
> registers to be reverted before the EC's shutdown routine is called,
> potentially making the EC unreachable.
>
> The attached patch addresses this by using programmer shutdown functions
> the same as any other shutdown callback. The internal_shutdown function is
> registered early in the internal_init() function so that newly discovered
> external programmers (ie SuperIO/EC LPC -> SPI bridges) can be discovered
> and have their shutdown callbacks added in the correct order.
>
> If this seems useful, we should make necessary changes to other programmers
> as well before committing this patch.
>
> Signed-off-by: David Hendricks <[email protected]>
>
> --
> David Hendricks (dhendrix)
> Systems Software Engineer, Google Inc.
>
--
David Hendricks (dhendrix)
Systems Software Engineer, Google Inc.
Index: cli_classic.c
===================================================================
--- cli_classic.c (revision 1288)
+++ cli_classic.c (working copy)
@@ -373,7 +373,7 @@
for (i = 0; i < ARRAY_SIZE(flashes) && flashes[i]; i++)
printf(" %s", flashes[i]->name);
printf("\nPlease specify which chip to use with the -c <chipname> option.\n");
- programmer_shutdown();
+ flashrom_shutdown();
exit(1);
} else if (!flashes[0]) {
printf("No EEPROM/flash device found.\n");
@@ -385,14 +385,14 @@
flashes[0] = probe_flash(flashchips, 1);
if (!flashes[0]) {
printf("Probing for flash chip '%s' failed.\n", chip_to_probe);
- programmer_shutdown();
+ flashrom_shutdown();
exit(1);
}
printf("Please note that forced reads most likely contain garbage.\n");
return read_flash_to_file(flashes[0], filename);
}
// FIXME: flash writes stay enabled!
- programmer_shutdown();
+ flashrom_shutdown();
exit(1);
}
@@ -405,21 +405,21 @@
(!force)) {
fprintf(stderr, "Chip is too big for this programmer "
"(-V gives details). Use --force to override.\n");
- programmer_shutdown();
+ flashrom_shutdown();
return 1;
}
if (!(read_it | write_it | verify_it | erase_it)) {
printf("No operations were specified.\n");
// FIXME: flash writes stay enabled!
- programmer_shutdown();
+ flashrom_shutdown();
exit(0);
}
if (!filename && !erase_it) {
printf("Error: No filename specified.\n");
// FIXME: flash writes stay enabled!
- programmer_shutdown();
+ flashrom_shutdown();
exit(1);
}
Index: internal.c
===================================================================
--- internal.c (revision 1288)
+++ internal.c (working copy)
@@ -164,6 +164,7 @@
}
free(arg);
+ register_shutdown(internal_shutdown, NULL);
get_io_perms();
/* Initialize PCI access for flash enables */
@@ -264,11 +265,9 @@
#endif
}
-int internal_shutdown(void)
+void internal_shutdown(void *data)
{
release_io_perms();
-
- return 0;
}
#endif
Index: it85spi.c
===================================================================
--- it85spi.c (revision 1288)
+++ it85spi.c (working copy)
@@ -80,6 +80,8 @@
#define INDIRECT_WRITE(base, value) OUTB(value, (base) + 4)
#endif /* LPC_IO */
+void it85xx_shutdown(void *);
+
#ifdef LPC_IO
unsigned int shm_io_base;
#endif
@@ -308,6 +310,7 @@
/* Set this as spi controller. */
spi_controller = SPI_CONTROLLER_IT85XX;
+ register_shutdown(it85xx_shutdown, NULL);
return 0;
}
@@ -349,11 +352,10 @@
return ret;
}
-int it85xx_shutdown(void)
+void it85xx_shutdown(void *data)
{
msg_pdbg("%s():%d\n", __func__, __LINE__);
it85xx_exit_scratch_rom();
- return 0;
}
/* According to ITE 8502 document, the procedure to follow mode is following:
Index: flashrom.c
===================================================================
--- flashrom.c (revision 1288)
+++ flashrom.c (working copy)
@@ -126,7 +126,8 @@
{
.name = "internal",
.init = internal_init,
- .shutdown = internal_shutdown,
+ /* called implicitly using shutdown callback */
+// .shutdown = internal_shutdown,
.map_flash_region = physmap,
.unmap_flash_region = physunmap,
.chip_readb = internal_chip_readb,
@@ -543,7 +544,7 @@
return ret;
}
-int programmer_shutdown(void)
+int flashrom_shutdown(void)
{
/* Registering shutdown functions is no longer allowed. */
may_register_shutdown = 0;
@@ -551,7 +552,7 @@
int i = --shutdown_fn_count;
shutdown_fn[i].func(shutdown_fn[i].data);
}
- return programmer_table[programmer].shutdown();
+ return 0;
}
void *programmer_map_flash_region(const char *descr, unsigned long phys_addr,
@@ -1939,6 +1940,6 @@
free(oldcontents);
free(newcontents);
out_nofree:
- programmer_shutdown();
+ flashrom_shutdown();
return ret;
}
Index: programmer.h
===================================================================
--- programmer.h (revision 1288)
+++ programmer.h (working copy)
@@ -111,7 +111,7 @@
extern const struct programmer_entry programmer_table[];
int programmer_init(char *param);
-int programmer_shutdown(void);
+int flashrom_shutdown(void);
enum bitbang_spi_master_type {
BITBANG_SPI_INVALID = 0, /* This must always be the first entry. */
@@ -290,7 +290,7 @@
extern int force_boardmismatch;
void probe_superio(void);
int internal_init(void);
-int internal_shutdown(void);
+void internal_shutdown(void *);
void internal_chip_writeb(uint8_t val, chipaddr addr);
void internal_chip_writew(uint16_t val, chipaddr addr);
void internal_chip_writel(uint32_t val, chipaddr addr);
@@ -584,7 +584,6 @@
/* it85spi.c */
struct superio probe_superio_ite85xx(void);
int it85xx_spi_init(void);
-int it85xx_shutdown(void);
int it85xx_probe_spi_flash(void);
int it85xx_spi_send_command(unsigned int writecnt, unsigned int readcnt,
const unsigned char *writearr, unsigned char *readarr);
_______________________________________________
flashrom mailing list
[email protected]
http://www.flashrom.org/mailman/listinfo/flashrom
