[coreboot] [RFC] Adding struct flashchip * to all chip functions

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Mon Jun 8 20:52:25 CEST 2009


On 08.06.2009 19:07, Urja Rannikko wrote:
>> However, any external flasher supporting multiple bus protocols (and
>> such flashers are being worked on) really wants to know which protocol
>> to use. Unfortunately, chip_read* and chip_write* don't know about the
>> bus protocol.
>>     
>
> If you're referring to my work, yes the protocol is designed so to be
> able to support
> many bustypes, but i have no real hardware for anything else than parallel.
>   

I was thinking of Paraflasher.


> And then, i realized a missing feature in my protocol, it cannot tell
> the flasher of the bustype to
> be used. It's only an optional command away though.
>   

I think we have too many words for too many things.
"bus protocol" in my mail above refers to the bus tye/protocol between
flasher and flash chip.


>> struct flashchip * as parameter to chip_read* and chip_write*
>> [...]
>> using global variables for current flash chip [...]
>>     
>
> I think that having a simple global current_flashchip pointer would
> be simpler in many ways:
> - it would only need a single assign to operation to the loop at
> probe_flash to support it
>   

IMHO the loop would get less readable because it tries to handle
detection of multiple chips. You'd have to keep an array of found chips
which would be local, whereas the currently probed chip would be global.
After probing, the global variable would have to be overwritten by a
local variable... Not exactly my idea of fun.

> - it wouldnt be necessary to pass unnecessary parameters to chip_*
> functions ( most dont need this info - atleast atm ;P ),
> making for a smaller binary
>   

I doubt the binary is going to be smaller, but this suggestion does
warrant an investigation. I'll try to cook up a patch which adds struct
flashchip everywhere and measure the size differences.

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/





More information about the coreboot mailing list