[coreboot] [patch] inteltool support for FreeBSD

Warren Turkal wt at penguintechs.org
Mon Oct 18 07:07:41 CEST 2010


On Sunday, October 17, 2010 09:37:31 am Idwer Vollering wrote:
> Add support for FreeBSD.
> 
> Signed-off-by: Idwer Vollering <vidwer at gmail.com>

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?
* 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.

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?

The Makefile change looks ok.

Thanks,
wt




More information about the coreboot mailing list