[U-Boot] [PATCH v2 8/9] tegra: add SPI SLINK driver
Allen Martin
amartin at nvidia.com
Tue Jan 15 05:27:03 CET 2013
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 <amartin at 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
More information about the U-Boot
mailing list