[flashrom] [PATCH] MCP67 SPI detection/debugging

[Michael Karcher]

Am Montag, den 01.02.2010, 03:31 +0100 schrieb Carl-Daniel Hailfinger:
> +/**
> + * The MCP67 code is guesswork based on cleanroom reverse engineering.
> + * Due to that, it only reads info and doesn't change any settings.
> + * It is assumed that LPC chips need the MCP55 code and SPI chips need the
> + * code provided in this function. Until we know for sure, call
> + * enable_flash_mcp55 from this function.
> + */
We don't know whether we can switch the chip between SPI and LPC mode
(does it support LPC at all?), and running correct initialization for
SPI might make LPC inaccessible.
[...]
> +	msg_pdbg("ISA bridge reg 0x8a contents: 0x%02x, bit 6 is %i, bit 5 is "
> +		 "%i\n", byte, (byte >> 6) & 0x1, (byte >> 5) & 0x1);
> +	msg_pdbg("Guessed flash bus type is %s\n", ((byte >> 5) & 0x3) == 0x2 ?
> +		 "SPI" : "unknown, probably LPC");
> +	/* Disable the write code for now until we have more info. */
Looks fine.

> +	/* Locate the BAR where the GPIOs live. */
Maybe GPIO should be changed to something else, as the standard GPIOs
are I/O mapped. Make that "where the SPI interface lives"
> +	gpiobaraddr = pci_read_long(smbusdev, 0x74);
> +	msg_pdbg("GPIO BAR is at 0x%08x, ", gpiobaraddr);
> +	/* We hope this has native alignment. We know the GPIOs are at offset
> +	 * 0x530, so we expect a size of at least 0x800. Clear the lower bits.
> +	 * It is entirely possible that the BAR is 64k big and the low bits are
> +	 * reserved for an entirely different purpose.
> +	 */
> +	gpiobaraddr &= ~0x7ff;
> +	msg_pdbg("after clearing low bits BAR is at 0x%08x\n", gpiobaraddr);
> +
> +	/* Accessing a NULL pointer BAR is evil. Don't do it. */
> +	if (gpiobaraddr) {
> +		/* Map the BAR. We access bytewise and wordwise at 0x530. */
> +		gpiobar = physmap("MCP67 GPIO", gpiobaraddr, 0x534);
We might need gpiobaraddr + 0x10 later. Some BIOSses access it, too. Go
directly for 0x544.

> +/* Guessed. If this is correct, migrate to a separate MCP67 SPI driver. */
> +#define MCP67_SPI_CS		(1 << 1)
> +#define MCP67_SPI_SCK		(1 << 2)
> +#define MCP67_SPI_MOSI		(1 << 3)
> +#define MCP67_SPI_MISO		(1 << 4)
> +#define MCP67_SPI_ENABLE	(1 << 0)
> +#define MCP67_SPI_IDLE		(1 << 8)
> +		status = mmio_readw(gpiobar + 0x530);
> +		msg_pdbg("SPI control is 0x%04x, enable=%i, idle=%i\n",
> +			 status, status & 0x1, (status >> 8) & 0x1);
> +	}
Add error handling here [else msg_perr("bad base address of memory
mapped interface")]

> +/* This is a shot in the dark. Even if the code is totally bogus for some
> + * chipsets, users will at least start to send in reports.
> + */
Mostly a ripoff of the mcp67 function, bit without any writing. OK.

Acked-By: Michael Karcher <flashrom at mkarcher.dialup.fu-berlin.de>