<div class="gmail_quote">On Mon, Jun 13, 2011 at 5:13 PM, Carl-Daniel Hailfinger <span dir="ltr"><<a href="mailto:c-d.hailfinger.devel.2006@gmx.net">c-d.hailfinger.devel.2006@gmx.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
Am 14.06.2011 00:45 schrieb David Hendricks:<br>
<div class="im">> Thanks again for the additional comments. I made the changes (comments<br>
> below) and re-compiled with the extra programmers enabled.<br>
><br>
> New version of the patch attached.<br>
> Signed-off-by: David Hendricks <<a href="mailto:dhendrix@google.com">dhendrix@google.com</a>><br>
><br>
<br>
</div>Thanks, looks really good.<br>
<div class="im"><br>
<br>
> On Sat, Jun 11, 2011 at 5:09 AM, Carl-Daniel Hailfinger wrote:<br>
><br>
>> nicintel_spi has the register_shutdown too early.<br>
>> nicintel will not unmap the nicintel_bar if mapping nicintel_control_bar<br>
>> fails (error case handling needs a second label which unmaps nicintel_bar).<br>
>><br>
> Good catch -- I moved the register_shutdown call toward the bottom of the<br>
> function, but before the bitbang_spi_init call so that failure in<br>
> bitbang_spi_init will not prevent nicintel_spi_shutdown from being called.<br>
><br>
<br>
</div>Not exactly what I meant. I'll try to explain it better with a diff on<br>
top of your current diff. Apply that, and we're good to go:<br>
<br>
> --- nicintel.c~ 2011-06-14 01:58:17.000000000 +0200<br>
> +++ nicintel.c 2011-06-14 01:59:57.000000000 +0200<br>
> @@ -77,7 +77,7 @@<br>
> nicintel_control_bar = physmap("Intel NIC control/status reg",<br>
> addr, NICINTEL_CONTROL_MEMMAP_SIZE);<br>
> if (nicintel_control_bar == ERROR_PTR)<br>
> - goto error_out;<br>
> + goto error_out_unmap;<br>
<div class="im">><br>
> if (register_shutdown(nicintel_shutdown, NULL))<br>
</div>> return 1;<br>
> @@ -99,6 +99,8 @@<br>
><br>
> return 0;<br>
><br>
> +error_out_unmap:<br>
> + physunmap(nicintel_bar, NICINTEL_MEMMAP_SIZE);<br>
> error_out:<br>
<div class="im">> pci_cleanup(pacc);<br>
> release_io_perms();<br>
><br></div></blockquote><div><br></div><div>D'oh, I should've read your comment more closely. I've added your changes.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div class="im">
<br>
<br>
</div>One comment about satasii.c:<br>
> Index: satasii.c<br>
> ===================================================================<br>
> --- satasii.c (revision 1327)<br>
> +++ satasii.c (working copy)<br>
> @@ -59,7 +69,8 @@<br>
> reg_offset = 0x50;<br>
> }<br>
><br>
> - sii_bar = physmap("SATA SIL registers", addr, 0x100) + reg_offset;<br>
> + sii_bar = reg_offset +<br>
> + physmap("SATA SIL registers", addr, SATASII_MEMMAP_SIZE);<br>
><br>
<br>
Could you keep the old ordering? AFAIK the usual way to specify a<br>
location is base+offset, not the other way round.<br></blockquote><div><br></div><div>Done.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<br>
<br>
> Index: flashrom.c<br>
> ===================================================================<br>
> --- flashrom.c (revision 1327)<br>
<div class="im">> +++ flashrom.c (working copy)<br>
> @@ -543,13 +525,15 @@<br>
><br>
> int programmer_shutdown(void)<br>
</div>> {<br>
> + int rc = 0;<br>
><br>
<br>
int ret please.<br></blockquote><div><br></div><div>Done.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div class="im"><br>
<br>
> +<br>
> /* Registering shutdown functions is no longer allowed. */<br>
> may_register_shutdown = 0;<br>
> while (shutdown_fn_count > 0) {<br>
> int i = --shutdown_fn_count;<br>
> - shutdown_fn[i].func(shutdown_fn[i].data);<br>
> + rc |= shutdown_fn[i].func(shutdown_fn[i].data);<br>
> }<br>
</div><div class="im">> - return programmer_table[programmer].shutdown();<br>
> + return rc;<br>
> }<br>
><br>
> void *programmer_map_flash_region(const char *descr, unsigned long phys_addr,<br>
><br>
<br>
<br>
</div>With those minor changes, the patch is<br>
Acked-by: Carl-Daniel Hailfinger <<a href="mailto:c-d.hailfinger.devel.2006@gmx.net">c-d.hailfinger.devel.2006@gmx.net</a>><br>
<br>
Please go ahead and commit.<br></blockquote><div><br></div><div>Thanks, committed in r1338.</div><div><br></div></div>-- <br>David Hendricks (dhendrix)<br>Systems Software Engineer, Google Inc.<br>