Stephen,
-----Original Message-----
From: Stephen Warren [mailto:swar...@wwwdotorg.org]
Sent: Friday, October 09, 2015 11:51 AM
To: Tom Warren <twar...@nvidia.com>
Cc: u-boot@lists.denx.de; Jagan Teki <jt...@openedev.com>; Stephen Warren
<swar...@nvidia.com>; tomcwarren3...@gmail.com
Subject: Re: [U-Boot] [PATCH] Tegra210: add QSPI driver
On 10/09/2015 12:20 PM, Tom Warren wrote:
> From: Yen Lin <ye...@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.
True, my bad. I'll resubmit.
> 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.
Valid point, I'm still used to changing the pinmux as desired to get new
devices up. I'll pull down the latest P2571 spreadsheet and have it changed if
it hasn't already been. Once it's corrected, I assume you can generate a new
pinmux header and post a patch that I can put in ahead of the QSPI patches.
> 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.
Ok, I'll take a look. Thanks.
> +static int tegra210_qspi_claim_bus(struct udevice *bus) {
> + /* Clear stale status here */
> + setbits_le32(®s->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(®s->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.
Yen wrote and debugged this driver awhile back - I don't know why he wasn't
included by 'send-email', but I've added him to the CC list.
Thanks,
Tom
--
nvpublic
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
