On 08.05.2009 19:27, Uwe Hermann wrote: > On Fri, May 08, 2009 at 06:29:12PM +0200, Carl-Daniel Hailfinger wrote: > >> Signed-off-by: Carl-Daniel Hailfinger <[email protected]> >> > > With the change below: > Acked-by: Uwe Hermann <[email protected]> >
Thanks, committed in r476. >> Index: flashrom-external_functionpointer/flash.h >> =================================================================== >> --- flashrom-external_functionpointer/flash.h (Revision 473) >> +++ flashrom-external_functionpointer/flash.h (Arbeitskopie) >> @@ -76,34 +76,64 @@ >> > [...] > >> +extern const struct programmer_entry programmer_table[]; >> + >> +static inline int programmer_init(void) >> > > I'd drop all those 'inline's, they're pretty useless and they're > not enforced by the compiler anyway. And we couldn't care less > about potential speed improvements here anyway. > Since these functions are one-liners calling other functions, I think inlining them is a good choice. Inlining also allows us to avoid prototypes and outsourcing these small wrappers to a separate file. I don't feel strongly about the issue, though. > Let's postpone that to another patch though. > Yes. >> +/* internal.c */ >> +int internal_init(void); >> +int internal_shutdown(void); >> +void internal_chip_writeb(uint8_t b, volatile void *addr); >> +void internal_chip_writew(uint16_t b, volatile void *addr); >> +void internal_chip_writel(uint32_t b, volatile void *addr); > > b -> val > Thanks, fixed. >> Index: flashrom-external_functionpointer/flashrom.c >> =================================================================== >> --- flashrom-external_functionpointer/flashrom.c (Revision 473) >> + ret = programmer_init(); >> > > Unused right now, we should add some error handling in another patch. > I think programer_init() already terminates the program in case of a fatal error. Non-fatal errors return ret and become the return code of main(). Regards, Carl-Daniel -- http://www.hailfinger.org/ -- coreboot mailing list: [email protected] http://www.coreboot.org/mailman/listinfo/coreboot
