Am 14.06.2011 00:45 schrieb David Hendricks:

> Thanks again for the additional comments. I made the changes (comments
> below) and re-compiled with the extra programmers enabled.
>
> New version of the patch attached.
> Signed-off-by: David Hendricks <dhendrix@google.com>
>

Thanks, looks really good.

> On Sat, Jun 11, 2011 at 5:09 AM, Carl-Daniel Hailfinger wrote:
>
>> nicintel_spi has the register_shutdown too early.
>> nicintel will not unmap the nicintel_bar if mapping nicintel_control_bar
>> fails (error case handling needs a second label which unmaps nicintel_bar).
>>
> Good catch -- I moved the register_shutdown call toward the bottom of the
> function, but before the bitbang_spi_init call so that failure in
> bitbang_spi_init will not prevent nicintel_spi_shutdown from being called.
>

Not exactly what I meant. I'll try to explain it better with a diff on
top of your current diff. Apply that, and we're good to go:

> --- nicintel.c~       2011-06-14 01:58:17.000000000 +0200
> +++ nicintel.c        2011-06-14 01:59:57.000000000 +0200
> @@ -77,7 +77,7 @@
>       nicintel_control_bar = physmap("Intel NIC control/status reg",
>                                      addr, NICINTEL_CONTROL_MEMMAP_SIZE);
>       if (nicintel_control_bar == ERROR_PTR)
> -             goto error_out;
> +             goto error_out_unmap;

>
>       if (register_shutdown(nicintel_shutdown, NULL))

>               return 1;
> @@ -99,6 +99,8 @@
>
>       return 0;
>
> +error_out_unmap:
> +     physunmap(nicintel_bar, NICINTEL_MEMMAP_SIZE);
>  error_out:

>       pci_cleanup(pacc);
>       release_io_perms();
>

One comment about satasii.c:
> Index: satasii.c
> ===================================================================
> --- satasii.c (revision 1327)
> +++ satasii.c (working copy)
> @@ -59,7 +69,8 @@
>               reg_offset = 0x50;
>       }
>
> -     sii_bar = physmap("SATA SIL registers", addr, 0x100) + reg_offset;
> +     sii_bar = reg_offset +
> +               physmap("SATA SIL registers", addr, SATASII_MEMMAP_SIZE);
>

Could you keep the old ordering? AFAIK the usual way to specify a
location is base+offset, not the other way round.

> Index: flashrom.c
> ===================================================================
> --- flashrom.c        (revision 1327)

> +++ flashrom.c        (working copy)
> @@ -543,13 +525,15 @@
>
>  int programmer_shutdown(void)

>  {
> +     int rc = 0;
>

int ret please.

> +
>       /* Registering shutdown functions is no longer allowed. */
>       may_register_shutdown = 0;
>       while (shutdown_fn_count > 0) {
>               int i = --shutdown_fn_count;
> -             shutdown_fn[i].func(shutdown_fn[i].data);
> +             rc |= shutdown_fn[i].func(shutdown_fn[i].data);
>       }

> -     return programmer_table[programmer].shutdown();
> +     return rc;
>  }
>
>  void *programmer_map_flash_region(const char *descr, unsigned long phys_addr,
>

With those minor changes, the patch is
Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>

Please go ahead and commit.