On 18.10.2010, at 07:26, Idwer Vollering vidwer@gmail.com wrote:
2010/10/18 Warren Turkal wt@penguintechs.org On Sunday, October 17, 2010 09:37:31 am Idwer Vollering wrote:
Add support for FreeBSD.
Signed-off-by: Idwer Vollering vidwer@gmail.com
Updated patch attached. Signed-off-by: Idwer Vollering vidwer@gmail.com
Acked-by Stefan Reinauer stepan@coreboot.org
In inteltool.h:
- Can you please briefly explain the need for the macros for {IN,OUT}{B,W,L}
when I don't seen them called from anywhere in the code?
Dropped, thanks.
- If you mean to use them, why are they implemented as macros instead of
functions. I think it'd be easier to read if they were function, and gcc could possibly even inline such a simple function.
See above.
In inteltool.c:
- Why not just include unistd.h on all platforms?
- I think the #ifdef __FREEBSD__ just makes the code difficult to read. I think
the platform specific code need to be factored out somehow.
- The io_fd variable doesn't appear to be used anywhere after opening the
/dev/io file. Doesn't it need to be closed somewhere? If not, why even bother creating a variable to hold the value of the open instead of just testing it directly?
Is this a possible memory leak ? If so, it needs to be fixed in flashrom too.
The Makefile change looks ok.
Thanks, wt
<inteltool_freebsd.v2.patch>
coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot