[flashrom] [PATCH 08/12] simplify ich_set_bbar

[Carl-Daniel Hailfinger]

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.


> 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/