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

[Stefan Reinauer]

Carl-Daniel Hailfinger wrote:

On 29.06.2008 01:04, Stefan Reinauer wrote:

See patch. It's a first attempt, but I think we should get this in in
small enough step by step improvements.

First attempt to clean up SPI probing and create a common
construct: the flash bus.

At some point the flash bus will be part of struct flashchip.

Signed-off-by: Stefan Reinauer <[EMAIL PROTECTED]>

Nice, but I'd like a small design change:
* Keep a list of available buses: flashbuses[]
* Call all functions with an additional parameter: flashbus

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.

Per-bus probing is a lot easier with that change.

The cleanups unrelated to flash bus support are
Acked-by: Carl-Daniel Hailfinger <[EMAIL PROTECTED]>

For the rest, please consider my design change suggestions.

Thanks.. please allow me to come up with something that integrates your suggestions after we get the current state in the tree.

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);


       uint32_t compatible_busses;
}


//  now this is what flashes is going to be:
struct flashchip_instance {
       struct flashchip *flash;

       flashbus_t bus;

       volatile uint8_t *virtual_memory;
       volatile uint8_t *virtual_registers;

      unsigned long physical_memory;
};

I am thinking of splitting up struct flashchip into the description part and the instance part. /* Some flash devices have an additional register space. */

Index: flash.h
===================================================================
--- flash.h     (revision 3393)
+++ flash.h     (working copy)
@@ -370,10 +370,18 @@
 /* chipset_enable.c */
 int chipset_flash_enable(void);
 void print_supported_chipsets(void);
-extern int ich7_detected;
-extern int ich9_detected;
-extern void *ich_spibar;

+typedef enum {

BUS_TYPE_FWH
BUS_TYPE_PARALLEL

+       BUS_TYPE_LPC,
+       BUS_TYPE_ICH7_SPI,
+       BUS_TYPE_ICH9_SPI,
+       BUS_TYPE_IT87XX_SPI,
+       BUS_TYPE_VIA_SPI
+} flashbus_t;

+extern flashbus_t flashbus;

+

See my initial comment:

extern flashbus_t flashbuses[];

+
+flashbus_t flashbus = BUS_TYPE_LPC;

flashbus_t flashbuses[MAX_BUSES] = {BUS_TYPE_LPC, };
int flashbus_count=1;

(and matching changes below)

--
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
     Tel.: +49 761 7668825 • Fax: +49 761 7664613
Email: [EMAIL PROTECTED]  • http://www.coresystems.de/
Registergericht: Amtsgericht Freiburg • HRB 7656
Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866

signature.asc
Description: OpenPGP digital signature

-- 
coreboot mailing list
coreboot@coreboot.org
http://www.coreboot.org/mailman/listinfo/coreboot