[flashrom] [PATCH 08/12] simplify ich_set_bbar
stefan.tauner at student.tuwien.ac.at
Mon Jun 13 23:12:45 CEST 2011
On Mon, 13 Jun 2011 22:09:35 +0200
Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net> wrote:
> Am 13.06.2011 20:31 schrieb Stefan Tauner:
> > On Mon, 13 Jun 2011 19:24:55 +0200
> > Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net> 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 <stefan.tauner at student.tuwien.ac.at>
> >> Nice cleanup!
> >> Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
> > 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.
famous last words ;)
> > 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.
Kind regards/Mit freundlichen Grüßen, Stefan Tauner
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 2799 bytes
Desc: not available
More information about the flashrom