[LinuxBIOS] GA-M57SLI SPI support

Uwe Hermann uwe at hermann-uwe.de
Tue Sep 25 01:17:20 CEST 2007


On Mon, Sep 24, 2007 at 12:10:15PM +0200, Carl-Daniel Hailfinger wrote:
> +uint8_t regval(uint16_t port, uint8_t reg)

Why not regread() btw? Just a minor issue.


> +	if (id == 0x8716) {
> +		/* LDN 0x0, reg 0x24, mask out lowest bit (suspend) */
> +		regwrite(port, 0x07, 0x0);
> +		tmp = regval(port, 0x24) & 0xFE;

Why LDN 0? Isn't that for floppy stuff? 0x24 can be manipulated
regardless of any LDN, IIRC.


> +static void it8716_serial_rdid(uint16_t port)
> +{
> +	uint8_t busy, data0, data1, data2;
> +	do {
> +		busy = inb(0x0820) & 0x80;
> +	} while (busy);
> +	/* RDID */
> +	outb(0x9f, 0x0820 + 1);

Should this be a #define?


> +	/* Start IO, 33MHz, 3 input bytes, 0 output bytes*/
> +	outb((0x5<<4)|(0x3<<2)|(0x0), 0x0820);
> +	do {
> +		busy = inb(0x0820) & 0x80;
> +	} while (busy);
> +	data0 = inb(0x0820 + 5);
> +	data1 = inb(0x0820 + 6);
> +	data2 = inb(0x0820 + 7);

#defines? SPI_IDATA0 etc.


> +	printf("RDID returned %02x %02x %02x\n", data0, data1, data2);

> +	return;

Not needed, the function returns void.


> +static int gam57sli_probe_serial_flash(const char *name)

Let's use the full/correct name for the board. In this case:
ga_m57sli_s4_probe_serial_flash or gam57slis4_probe_serial_flash
(I think the first version is more readable)


> +{
> +	uint16_t flashport;
> +	flashport = find_ite_serial_flash_port(0x2e);
> +	if (flashport)
> +		it8716_serial_rdid(flashport);
> +	flashport = find_ite_serial_flash_port(0x4e);
> +	if (flashport)
> +		it8716_serial_rdid(flashport);
> +	return 0;

If the function always returns the same value, maybe we should make
it return void?


>  struct board_pciid_enable board_pciid_enables[] = {
> +	{0x10de, 0x0360, 0x0000, 0x0000, 0x0000, 0x0000, 0x0000, 0x0000,
> +	 "gigabyte", "gam57sli", "Gigabyte GA-M57SLI", gam57sli_probe_serial_flash},

"gigabyte", "ga-m57sli-s4", "GIGABYTE GA-M57SLI-S4", ga_m57sli_s4_probe_serial_flash},

(or similar, depending on the above function name)

GIGABYTE is all-caps as they use that form on their website.


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20070925/37853f75/attachment.sig>


More information about the coreboot mailing list