[coreboot] [PATCH] flashrom: clean up SPI probing, bus handling

[Carl-Daniel Hailfinger]

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/