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 <[email protected]>
>
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 <[email protected]>
Please go ahead and commit.
Regards,
Carl-Daniel
--
http://www.hailfinger.org/
_______________________________________________
flashrom mailing list
[email protected]
http://www.flashrom.org/mailman/listinfo/flashrom
