[coreboot] [PATCH] flashrom: clean up SPI probing, bus handling
Carl-Daniel Hailfinger
c-d.hailfinger.devel.2006 at gmx.net
Sun Jun 29 13:11:52 CEST 2008
On 29.06.2008 12:35, Stefan Reinauer wrote:
> Carl-Daniel Hailfinger wrote:
>
>>> Yes, something along the line. Alongside with this I want to clean up
>>> use of struct flashchip
>>> for flashes[] but I would really like to get the current change in the
>>> tree yet.
>>>
>>>
>> I'm not a fan of committing code which will be changed shortly
>> afterwards, but...
>>
>>
> Release early, release often, yes? ;-) I can keep this in our internal
> repository for a while until it's ripened, but I fear it will decay if
> it's only floating around as a patch and not in the repo (see Rudolf's
> SPI patch, though that can still easily be applied for VIA systems)
>
Actually, to give Rudolf the chance to adapt VIA chipset support in
flashrom to the new code I'd like to keep big commits away from ichspi.c
until VIA support is merged.
>>> struct flashchip {
>>> const char *vendor;
>>> const char *name;
>>> /* With 32bit manufacture_id and model_id we can cover IDs up to
>>> * (including) the 4th bank of JEDEC JEP106W Standard
>>> Manufacturer's
>>> * Identification code.
>>> */
>>> uint32_t manufacture_id;
>>> uint32_t model_id;
>>>
>>> int total_size;
>>> int page_size;
>>>
>>> /* Indicate if flashrom has been tested with this flash chip
>>> and if
>>> * everything worked correctly.
>>> */
>>> uint32_t tested;
>>>
>>> int (*probe) (struct flashchip *flash);
>>> int (*erase) (struct flashchip *flash);
>>> int (*write) (struct flashchip *flash, uint8_t *buf);
>>> int (*read) (struct flashchip *flash, uint8_t *buf);
>>>
>>>
>> Maybe change struct flashchip to struct flashchip_instance for the 4
>> functions above?
>>
>>
> The parameter? Yes, of course.
>
Good.
>>> uint32_t compatible_busses;
>>> }
>>>
>>>
>>> // now this is what flashes is going to be:
>>> struct flashchip_instance {
>>> struct flashchip *flash;
>>>
>>> flashbus_t bus;
>>> /* Some flash devices have an additional register space. */
>>> volatile uint8_t *virtual_memory;
>>> volatile uint8_t *virtual_registers;
>>>
>>> unsigned long physical_memory;
>>>
>>>
>> What does that struct member mean?
>>
>>
>>
> physical address of the flash chip. we'll need that to determine where
> to look for flash chips in our memory maps. The virtual addresses are
> not sufficient
>
I always thought that virtual_memory is exactly the location of the
flash chip in the memory map. If not, what's the point of virtual_memory?
Regards,
Carl-Daniel
--
http://www.hailfinger.org/
More information about the coreboot
mailing list