
Hi Mike,
On Thu, Oct 20, 2011 at 1:03 PM, Mike Frysinger vapier@gentoo.org wrote:
On Thursday 20 October 2011 15:03:24 Simon Glass wrote:
This driver supports SPI on Tegra2, running at 48MHz.
the summary says "SPIFLASH" and "Seaboard". sounds like this is a tegra2 SoC issue, and so driver/board specific info shouldn't be in the summary.
adding notes to the changelog as to what boards/setups have been tested is OK though ...
--- /dev/null +++ b/arch/arm/include/asm/arch-tegra2/tegra2_spi.h
+struct spi_tegra {
- u32 command; /* SPI_COMMAND_0 register */
- u32 status; /* SPI_STATUS_0 register */
- u32 rx_cmp; /* SPI_RX_CMP_0 register */
- u32 dma_ctl; /* SPI_DMA_CTL_0 register */
- u32 tx_fifo; /* SPI_TX_FIFO_0 register */
- u32 rsvd[3]; /* offsets 0x14 to 0x1F reserved */
- u32 rx_fifo; /* SPI_RX_FIFO_0 register */
+};
don't need that newline before the closing brace
--- /dev/null +++ b/drivers/spi/tegra2_spi.c
+int spi_cs_is_valid(unsigned int bus, unsigned int cs) +{
- /* Tegra2 SPI-Flash - only 1 device ('bus/cs') */
- if (bus > 0 && cs != 0)
- return 0;
- else
- return 1;
+}
only need one new line here
+struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
- unsigned int max_hz, unsigned int mode)
+{
- struct spi_slave *slave;
- if (!spi_cs_is_valid(bus, cs))
- return NULL;
- slave = malloc(sizeof(struct spi_slave));
- if (!slave)
- return NULL;
- slave->bus = bus;
- slave->cs = cs;
- /*
- * Currently, Tegra2 SFLASH uses mode 0 & a 24MHz clock.
- * Use 'mode' and 'maz_hz' to change that here, if needed.
- */
- return slave;
+}
this should be respecting hz/mode ...
Thanks for the comments.
OK I will need to implement this. There will be a short delay....
Regards, Simon
+void spi_init(void) +{
- struct spi_tegra *spi = (struct spi_tegra *)TEGRA2_SPI_BASE;
- u32 reg;
- /* Change SPI clock to 48MHz, PLLP_OUT0 source */
- clock_start_periph_pll(PERIPH_ID_SPI1, CLOCK_ID_PERIPH, 48000000);
- /* Clear stale status here */
- reg = SPI_STAT_RDY | SPI_STAT_RXF_FLUSH | SPI_STAT_TXF_FLUSH | \
- SPI_STAT_RXF_UNR | SPI_STAT_TXF_OVF;
- writel(reg, &spi->status);
- debug("spi_init: STATUS = %08x\n", readl(&spi->status));
- /*
- * Use sw-controlled CS, so we can clock in data after ReadID, etc.
- */
- reg = readl(&spi->command);
- writel(reg | SPI_CMD_CS_SOFT, &spi->command);
- debug("spi_init: COMMAND = %08x\n", readl(&spi->command));
- /*
- * SPI pins on Tegra2 are muxed - change pinmux later due to UART
- * issue.
- */
+}
shouldn't this be in spi_claim_bus() ? and configured using the values requested at spi_setup_slave() ?
+int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void
- *dout, void *din, unsigned long flags)
+{
- int num_bytes = (bitlen + 7) / 8;
if you can only handle multiples of 8 bits, then return an error. many (most) of the spi buses do just that.
- reg = readl(&spi->command);
- writel((reg |= (SPI_CMD_TXEN | SPI_CMD_RXEN)), &spi->command);
ugh! pull that "|=" operation out and move it up to the readl() line above.
- debug("spi_xfer: COMMAND = %08x\n", readl(&spi->command));
the debug() should use "reg" rather than readl().
- debug("spi_xfer: slave %u:%u dout %08X din %08X bitlen %u\n",
- slave->bus, slave->cs, *(u8 *)dout, *(u8 *)din, bitlen);
...
- tmpdout = (tmpdout << 8) | ((u8 *) dout)[i];
...
- ((u8 *)din)[i] =
- (tmpdin & 0xff);
please setup local vars with u8* type and set din/dout to that to avoid all these inline casts
- writel((reg |= SPI_CMD_GO), &spi->command);
hoist that |= operation up and out -mike