On Sat, Jan 12, 2013 at 08:56:23AM -0800, Simon Glass wrote: > Hi, > > On Sat, Jan 12, 2013 at 1:07 AM, Allen Martin <amar...@nvidia.com> wrote: > > Add driver for tegra SPI "SLINK" style driver. This controller is > > similar to the tegra20 SPI "SFLASH" controller. The difference is > > that the SLINK controller is a genernal purpose SPI controller and the > > SFLASH controller is special purpose and can only talk to FLASH > > devices. In addition there are potentially many instances of an SLINK > > controller on tegra and only a single instance of SFLASH. Tegra20 is > > currently ths only version of tegra that instantiates an SFLASH > > controller. > > > > This driver supports basic PIO mode of operation and is configurable > > (CONFIG_OF_CONTROL) to be driven off devicetree bindings. Up to 4 > > devices per controller may be attached, although typically only a > > single chip select line is exposed from tegra per controller so in > > reality this is usually limited to 1. > > > > To enable this driver, use CONFIG_TEGRA_SLINK > > A few comments - note I am on holiday next week so please don't wait > for my response on the next version. > > > ... > > +#include <spi.h> > > +#ifdef CONFIG_OF_CONTROL > > You probably don't need this ifdef >
ok > > + > > + spi = malloc(sizeof(struct tegra_spi_slave)); > > Please also look at my SPI series where I added an allocate function for this. > > http://patchwork.ozlabs.org/patch/208226/ > http://patchwork.ozlabs.org/patch/208229/ > Nice, thanks. I propose I should wait for that to land in u-boot/master and trick back down to u-boot-arm and u-boot-tegra, and then add it in a separate patch. That way I don't have to add a cross repo dependency. > > +{ > > + int node = 0, i; > > + struct tegra_spi_ctrl *ctrl; > > blank line here ok > > > + for (i = 0; i < CONFIG_TEGRA_SLINK_CTRLS; i++) { > > + ctrl = &spi_ctrls[i]; > > +#ifdef CONFIG_OF_CONTROL > > + node = fdtdec_next_compatible(gd->fdt_blob, node, > > + COMPAT_NVIDIA_TEGRA20_SLINK); > > + if (!node) > > + break; > > I think you should be using fdtdec_find_aliases_for_id() so that aliases work. I'll reply in Stephen's follow-up on this. > > + (slave->cs << SLINK_CMD2_SS_EN_SHIFT); > > + writel(reg, ®s->command2); > > Could use clrsetbits_le32() if you like Ok > > + bytes = (num_bytes > 4) ? 4 : num_bytes; > > + > > + if (dout != NULL) { > > + for (i = 0; i < bytes; ++i) > > + tmpdout = (tmpdout << 8) | dout[i]; > > dout += bytes here... > > > + } > > + > > + num_bytes -= bytes; > > + if (dout) > > + dout += bytes; > > instead of here? ok > > > + > > + clrsetbits_le32(®s->command, SLINK_CMD_BIT_LENGTH_MASK, > > + bytes * 8 - 1); > > + writel(tmpdout, ®s->tx_fifo); > > + setbits_le32(®s->command, SLINK_CMD_GO); > > + > > + /* > > + * Wait for SPI transmit FIFO to empty, or to time out. > > + * The RX FIFO status will be read and cleared last > > + */ > > + for (tm = 0, is_read = 0; tm < SPI_TIMEOUT; ++tm) { > > + u32 status; > > + > > This says timeout but doesn't seem to actually check get_timer(). Also > is it possible to separate the code that waits for completion from the > code below? You're right about get_timer(), I'll fix that. I pulled this loop directly from the tegra20 tegra_spi driver, so I'm not the original author, but I believe the reason it's written this way is so there's a single timeout loop around the wait for STAT_BSY to drop, RXF_EMPTY to drop, and TXF_EMPTY to go high. It *should* be ok to move the TXF_EMPTY wait to a separate wait loop, but I'm a little hesitant to touch the code since it's beeen well tested in the tegra20 driver, and this part of the driver is identical. > > > + status = readl(®s->status); > > + > > + /* We can exit when we've had both RX and TX > > activity */ > > + if (is_read && (status & SLINK_STAT_TXF_EMPTY)) > > + break; > > + > > + if ((status & (SLINK_STAT_BSY | SLINK_STAT_RDY)) != > > + SLINK_STAT_RDY) > > + tm++; > > + > > + else if (!(status & SLINK_STAT_RXF_EMPTY)) { > > + tmpdin = readl(®s->rx_fifo); > > + is_read = 1; > > + > > + /* swap bytes read in */ > > + if (din != NULL) { > > + for (i = bytes - 1; i >= 0; --i) { > > + din[i] = tmpdin & 0xff; > > + tmpdin >>= 8; > > + } > > + din += bytes; > > + } > > + } > > + } > > + > > + if (tm >= SPI_TIMEOUT) > > + ret = tm; > > + > > + /* clear ACK RDY, etc. bits */ > > + writel(readl(®s->status), ®s->status); > > + } > > + > > + if (flags & SPI_XFER_END) > > + spi_cs_deactivate(slave); > > + > > + debug("%s: transfer ended. Value=%08x, status = %08x\n", > > + __func__, tmpdin, readl(®s->status)); > > + > > + if (ret) { > > + printf("%s: timeout during SPI transfer, tm %d\n", > > + __func__, ret); > > + return -1; > > + } > > + > > + return 0; > > +} > > diff --git a/include/fdtdec.h b/include/fdtdec.h > > index 1504336..14aa308 100644 > > --- a/include/fdtdec.h > > +++ b/include/fdtdec.h > > @@ -71,6 +71,7 @@ enum fdt_compat_id { > > COMPAT_NVIDIA_TEGRA20_PWM, /* Tegra 2 PWM controller */ > > COMPAT_NVIDIA_TEGRA20_DC, /* Tegra 2 Display controller */ > > COMPAT_NVIDIA_TEGRA20_SFLASH, /* Tegra 2 SPI flash controller */ > > + COMPAT_NVIDIA_TEGRA20_SLINK, /* Tegra 2 SPI SLINK controller */ > > > > COMPAT_COUNT, > > }; > > diff --git a/lib/fdtdec.c b/lib/fdtdec.c > > index 6779278..4fef428 100644 > > --- a/lib/fdtdec.c > > +++ b/lib/fdtdec.c > > @@ -46,6 +46,7 @@ static const char * const compat_names[COMPAT_COUNT] = { > > COMPAT(NVIDIA_TEGRA20_PWM, "nvidia,tegra20-pwm"), > > COMPAT(NVIDIA_TEGRA20_DC, "nvidia,tegra20-dc"), > > COMPAT(NVIDIA_TEGRA20_SFLASH, "nvidia,tegra20-sflash"), > > + COMPAT(NVIDIA_TEGRA20_SLINK, "nvidia,tegra20-slink"), > > }; > > > > const char *fdtdec_get_compatible(enum fdt_compat_id id) > > -- > > 1.7.10.4 > > > > Regards, > Simon -Allen -- nvpublic _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot