[U-Boot] [PATCH] Tegra210: add QSPI driver

[Stephen Warren]

On 10/09/2015 12:20 PM, Tom Warren wrote:
> From: Yen Lin <yelin at nvidia.com>
>
> Tested on P2571 board with Winbond SPI ROM.
> sf probe 2:0 to see SPI chip. Flashed U-Boot image to SPI, rebooted, read back data to RAM with sf read, then verified crc against original binary file.

>   board/nvidia/p2571/pinmux-config-p2571.h |  14 +-
>   configs/p2571_defconfig                  |   1 +

As a general comment, driver changes should be in separate patches to 
board changes to enable the driver. The two things are logically 
separate, and this also helps when cherry-picking patches between 
branches that might not have p2571 support.

> diff --git a/board/nvidia/p2571/pinmux-config-p2571.h b/board/nvidia/p2571/pinmux-config-p2571.h

> -	PINCFG(QSPI_SCK_PEE0,        RSVD1,    DOWN,   TRISTATE, OUTPUT,  DISABLE, DEFAULT),
> -	PINCFG(QSPI_CS_N_PEE1,       RSVD1,    DOWN,   TRISTATE, OUTPUT,  DISABLE, DEFAULT),
> -	PINCFG(QSPI_IO0_PEE2,        RSVD1,    DOWN,   TRISTATE, OUTPUT,  DISABLE, DEFAULT),
> -	PINCFG(QSPI_IO1_PEE3,        RSVD1,    DOWN,   TRISTATE, OUTPUT,  DISABLE, DEFAULT),
> -	PINCFG(QSPI_IO2_PEE4,        RSVD1,    DOWN,   TRISTATE, OUTPUT,  DISABLE, DEFAULT),
> -	PINCFG(QSPI_IO3_PEE5,        RSVD1,    DOWN,   TRISTATE, OUTPUT,  DISABLE, DEFAULT),
> +
> +	PINCFG(QSPI_SCK_PEE0,        QSPI,     NORMAL, NORMAL,   INPUT,   DISABLE, DEFAULT),
> +	PINCFG(QSPI_CS_N_PEE1,       QSPI,     NORMAL, NORMAL,   INPUT,   DISABLE, DEFAULT),
> +	PINCFG(QSPI_IO0_PEE2,        QSPI,     NORMAL, NORMAL,   INPUT,   DISABLE, DEFAULT),
> +	PINCFG(QSPI_IO1_PEE3,        QSPI,     NORMAL, NORMAL,   INPUT,   DISABLE, DEFAULT),
> +	PINCFG(QSPI_IO2_PEE4,        QSPI,     UP,     TRISTATE, OUTPUT,  DISABLE, DEFAULT),
> +	PINCFG(QSPI_IO3_PEE5,        QSPI,     UP,     TRISTATE, OUTPUT,  DISABLE, DEFAULT),

The current pinmux matches the syseng-supplied spreadsheet. If the 
spreadsheet is wrong, please get syseng to update the spreadsheet first. 
The pinmux files must not be manually edited, so we ensure that the 
mandated pinmux is used.

> diff --git a/drivers/spi/tegra210_qspi.c b/drivers/spi/tegra210_qspi.c new file mode 100644 index 0000000..d827667


> +static int tegra210_qspi_ofdata_to_platdata(struct udevice *bus) {
> +	struct tegra_spi_platdata *plat = bus->platdata;
> +	const void *blob = gd->fdt_blob;
> +	int node = bus->of_offset;
> +
> +	plat->base = fdtdec_get_addr(blob, node, "reg");

I think that should use dev_get_addr() now, since that does the correct 
thing w.r.t. #address-cells/#size-cells.

> +static int tegra210_qspi_claim_bus(struct udevice *bus) {

> +	/* Clear stale status here */
> +	setbits_le32(&regs->fifo_status,
> +		     QSPI_FIFO_STS_ERR		|
> +		     QSPI_FIFO_STS_TX_FIFO_OVF	|
> +		     QSPI_FIFO_STS_TX_FIFO_UNR	|
> +		     QSPI_FIFO_STS_RX_FIFO_OVF	|
> +		     QSPI_FIFO_STS_RX_FIFO_UNR	|
> +		     QSPI_FIFO_STS_TX_FIFO_FULL	|
> +		     QSPI_FIFO_STS_TX_FIFO_EMPTY	|
> +		     QSPI_FIFO_STS_RX_FIFO_FULL	|
> +		     QSPI_FIFO_STS_RX_FIFO_EMPTY);
> +	debug("%s: FIFO STATUS = %08x\n", __func__,
> +readl(&regs->fifo_status));

Indentation issue.

I would expect at least that setbits_le32() to happen inside the 
transfer function so that every transfer is immune to stale status, not 
just the first transfer after the bus is cleared.

Actually, it looks like the transfer function already does this, so the 
above code can simply be dropped.

Perhaps the other HW-touching code might be moved to, but it's probably 
fine to leave it here.