[coreboot] [RFC]flashrom: improve basic design

FENG Yu Ning fengyuning1984 at gmail.com
Thu Jan 1 11:21:23 CET 2009


There is a defect in basic design of flashrom. It leads to problems
some developers are facing or going to face. A partial solution is
proposed.

1. The Defect

struct flashchip {

        /* ... */

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

        /* ... */

};

The struct specifies drivers for operations. It is a good design if
the drivers deal with flash chips directly and provide interfaces for
upper layer program to use. However, those drivers deal with every
component in the communication chain. They do not fit into a structure
storing information closely related to flash chips.


2. Problems

Supporting non-standard flash chips needs to write a new driver. It
requires the developer to be familiar with the internals of flashrom.

Probe functions and IDs are seperated. They are associated by the
flashchip structure. In nature, probe functions and IDs are questions
and answers, and should be associated more closely. Seperated ID
fields in struct flashchip are not very meaningful.

flashrom will probably have plugins in the future. Though chips are
operated in the same way from the view of their host devices, there is
not much useful information for plugins in struct flashchip.


3. Solution

We need an abstract specification of operations in struct flashchip,
not actual code. If there have to be some information not related to
flash chips, the less there is, the better.

Abstract operation specification for SPI flash chips:

/* BEGINNING OF SPECIFICATION */

struct cycles {
        u8 type;	/* CYCLETYPE_PREOPCODE, CYCLETYPE_OPCODE, */
	   		/* CYCLETYPE_ADDRESS, CYCLETYPE_DUMMY,	  */
			/* CYCLETYPE_DATA_IN, CYCLETYPE_DATA_OUT  */
        u8 length;	/* in bytes */
	u8 value_type;	/* VALUETYPE_VARIABLE, VALUETYPE_FIXED	  */
	u32 value;
};

struct cycles byte_read[] =
        {{CYCLETYPE_OPCODE, 1, VALUETYPE_FIXED, 0x3},
       	 {CYCLETYPE_ADDRESS, 3, VALUETYPE_VARIABLE},
	 {CYCLETYPE_DATA_IN, 1, VALUETYPE_VARIABLE}
	};

struct cycles byte_program[] =
        {{CYCLETYPE_PREOPCODE, 1, VALUETYPE_FIXED, 0x6},
	 {CYCLETYPE_OPCODE, 1, VALUETYPE_FIXED, 0x2},
       	 {CYCLETYPE_ADDRESS, 3, VALUETYPE_VARIABLE},
	 {CYCLETYPE_DATA_OUT, 1, VALUETYPE_VARIABLE}
	};

struct cycles SST25VF040B_probe_rdid[] =
        {{CYCLETYPE_OPCODE, 1, VALUETYPE_FIXED, 0x90},
	 {CYCLETYPE_ADDRESS, 3, VALUETYPE_FIXED, 0},
	 {CYCLETYPE_DATA_IN, 2, VALUETYPE_FIXED, 0xbf8d}
	};

struct cycles SST25VF040B_probe_jedec[] =
        {{CYCLETYPE_OPCODE, 1, VALUETYPE_FIXED, 0x9f},
	 {CYCLETYPE_DATA_IN, 3, VALUETYPE_FIXED, 0xbf258d}
	};

/* END OF SPECIFICATION */

Specifications for LPC and FWH flash chips will be different, but the
point is abstraction.

Comments are appreciated.

New year's greetings,
yu ning




More information about the coreboot mailing list