Am 13.06.2011 20:31 schrieb Stefan Tauner: > On Mon, 13 Jun 2011 19:24:55 +0200 > Carl-Daniel Hailfinger <[email protected]> wrote: > > >> Am 08.06.2011 04:55 schrieb Stefan Tauner: >> >>> ~10 lines less, documenting better what the differences are (i.e. offset of >>> BBAR only). >>> >>> Signed-off-by: Stefan Tauner <[email protected]> >>> >>> >> Nice cleanup! >> Acked-by: Carl-Daniel Hailfinger <[email protected]> >> > ahem! :) > > because of idwer's remark that "msg_pdbg("Reserved bits in BBAR not > zero: 0x%04x"..." should get a \n (thx!), i reviewed that patch again > and noticed that it is quite broken. > > in the original code the real bbar is read from the register and > checked for reserved bits before we try to set the requested min > address. i somehow missed that, sorry. :/ >
Ouch. I overlooked that during review. The patch looked so simple. > the new, attached patch has following changes: > - check for reserved bits > - output 8 hex characters instead of 4 (i.e. 32b instead of 16b) > - add \n to the message above > Since you're already changing the function prototype of ich_set_bbar, can you make it static? And another point about the coding style: Can you use int bbar_offs instead of void *bbar_addr? That way, it is easier to check directly which physmap region the accesses correspond to. Regards, Carl-Daniel -- http://www.hailfinger.org/ _______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
