Am 22.04.2011 06:31 schrieb David Hendricks:
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();
Wrong order. We want to register the shutdown function only after we got
IO permissions. That said, should get_io_perms automatically register
release_io_perms? This would kill a few useless shutdown functions.
/* 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:
It would be great if you could forward-port this patch to the current
IT85* SPI code.
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,
The same for all other programmers, please.
And kill the .shutdown struct member everywhere (even in the header
file) while you're at it.
@@ -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);
We should care about the result of the shutdown function, even if it is
only for printing diagnostic messages.
}
- return programmer_table[programmer].shutdown();
+ return 0;
And return the error code of the failing shutdown function (if one
failed), zero (if none failed) or some arbitrary value if more than one
shutdown function failed.
}
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);
Given that this is still a programmer shutdown (as opposed to a flash
chip shutdown which may be necessary in the future), I'd like to keep
the name.
Overall, I like this patch. If you send a variant which converts all
programmers and addresses the comments, I'll ack.
Regards,
Carl-Daniel
--
http://www.hailfinger.org/
_______________________________________________
flashrom mailing list
[email protected]
http://www.flashrom.org/mailman/listinfo/flashrom
