Finally got a chance to revise this. There are a lot of small, tedious changes to all the programmer .c files and some interesting changes to flashrom.c. Otherwise it's a pretty simple patch.
To recap from earlier discussion, this version does the following:
- Registers a shutdown callback during initialization for each programmer. See notes below.
- Kills the .shutdown function pointer from programmer_entry struct
- Updates all programmer shutdown functions to return an int and take a void *data arg. Also, make them static.
- programmer_shutdown() checks the return code of all shutdown callbacks.
A few minor questions:
- If programmer_init() fails when called in cli_classic(), should we call programmer_shutdown()? As Carl-Daniel noted earlier, this could potentially be used to do stuff like release_io_perms(), or perhaps free resources allocated by an init routine that fails.
- nicintel_spi -- Looks like flash write enable/disable should be done using rpci?
- drkaiser - shutdown function missing a physunmap?
Additional notes:
- Most programmers have a simple init routines with no failure condition, so placement of register_shutdown() doesn't really matter.
- serialport_shutdown non-static since it's used by buspirate.
- There were cases where the programmer's init routine would do something less simple, like open a file or map physical memory, and the shutdown callback would undo it. In those cases, placement took a little more thought. Someone more familiar with those should double-check that this patch does the right thing for buspirate, serprog, and satamv.
I did a quick test using an NM10/ICH7 platform and callbacks were shown in verbose mode output as expected.
--
David Hendricks (dhendrix)
Systems Software Engineer, Google Inc.