You goofed on the changes to spi_setup() ...
On Friday 17 July 2009, s-paul...@ti.com wrote:
> +static int davinci_spi_setup(struct spi_device *spi)
> +{
> + int retval;
> + int op_mode = 0;
> + struct davinci_spi *davinci_spi;
> +
> + davinci_spi = spi_master_get_devdata(spi->master);
> +
> + /*
> + * Set up device-specific SPI configuration parameters.
> + * Most of these would normally be handled in spi_setup(),
... by just moving all of the old davinci_spi_bufs_prep() here.
Example, that comment is specific to the old davinci_spi_bufs_prep()
code not this version. That's an easy fix (although I'd still
rather see this have a single read and write to the FMT register).
Except the global options (loopback, READY handsdhaking) should
not be here ... in this hardware they are global, so that by
setting them here you could trash an ongoing transfer.
The rule of thumb for the setup() methods is that you must expect
they are called in the middle of a transfer for some *OTHER* chip
select line. See 6e538aaf50ae782a890cbc02c27950448d8193e1 ...
> + * updating the per-chipselect FMT registers, but some of
> + * them use global controls like SPI_LOOP and SPI_READY.
> + */
> +
> + if (spi->mode & SPI_LSB_FIRST)
> + set_fmt_bits(davinci_spi->base, SPIFMT_SHIFTDIR_MASK,
> + spi->chip_select);
> + else
> + clear_fmt_bits(davinci_spi->base, SPIFMT_SHIFTDIR_MASK,
> + spi->chip_select);
> +
> + if (spi->mode & SPI_CPOL)
> + set_fmt_bits(davinci_spi->base, SPIFMT_POLARITY_MASK,
> + spi->chip_select);
> + else
> + clear_fmt_bits(davinci_spi->base, SPIFMT_POLARITY_MASK,
> + spi->chip_select);
> +
> + if (!(spi->mode & SPI_CPHA))
> + set_fmt_bits(davinci_spi->base, SPIFMT_PHASE_MASK,
> + spi->chip_select);
> + else
> + clear_fmt_bits(davinci_spi->base, SPIFMT_PHASE_MASK,
> + spi->chip_select);
> +
> + if (davinci_spi->version == SPI_VERSION_2) {
> + clear_fmt_bits(davinci_spi->base, SPIFMT_WDELAY_MASK,
> + spi->chip_select);
> + set_fmt_bits(davinci_spi->base,
> + (davinci_spi->pdata->wdelay
> + << SPIFMT_WDELAY_SHIFT)
> + & SPIFMT_WDELAY_MASK,
> + spi->chip_select);
> +
> + if (davinci_spi->pdata->odd_parity)
> + set_fmt_bits(davinci_spi->base,
> + SPIFMT_ODD_PARITY_MASK,
> + spi->chip_select);
> + else
> + clear_fmt_bits(davinci_spi->base,
> + SPIFMT_ODD_PARITY_MASK,
> + spi->chip_select);
> +
> + if (davinci_spi->pdata->parity_enable)
> + set_fmt_bits(davinci_spi->base,
> + SPIFMT_PARITYENA_MASK,
> + spi->chip_select);
> + else
> + clear_fmt_bits(davinci_spi->base,
> + SPIFMT_PARITYENA_MASK,
> + spi->chip_select);
> +
> + if (davinci_spi->pdata->wait_enable)
> + set_fmt_bits(davinci_spi->base,
> + SPIFMT_WAITENA_MASK,
> + spi->chip_select);
> + else
> + clear_fmt_bits(davinci_spi->base,
> + SPIFMT_WAITENA_MASK,
> + spi->chip_select);
> +
> + if (davinci_spi->pdata->timer_disable)
> + set_fmt_bits(davinci_spi->base,
> + SPIFMT_DISTIMER_MASK,
> + spi->chip_select);
> + else
> + clear_fmt_bits(davinci_spi->base,
> + SPIFMT_DISTIMER_MASK,
> + spi->chip_select);
> + }
All those *_fmt_bits() ops are OK; the FMT register is specific to
a given chipselect line. And you can trust that *THIS* chipselect
is not in use when this method is called.
> +
> + /* Clock internal */
> + if (davinci_spi->pdata->clk_internal)
> + set_io_bits(davinci_spi->base + SPIGCR1,
> + SPIGCR1_CLKMOD_MASK);
> + else
> + clear_io_bits(davinci_spi->base + SPIGCR1,
> + SPIGCR1_CLKMOD_MASK);
> +
> + /* master mode default */
> + set_io_bits(davinci_spi->base + SPIGCR1, SPIGCR1_MASTER_MASK);
> +
> + if (davinci_spi->pdata->intr_level)
> + iowrite32(SPI_INTLVL_1, davinci_spi->base + SPILVL);
> + else
> + iowrite32(SPI_INTLVL_0, davinci_spi->base + SPILVL);
But the *_io_bits() calls can crap on a pending transfer for some
other chipselect ... and the interrupt config bit should be done
in the probe(), since it never changes. In fact all three of
those chunks seem like they should be in probe().
> +
> + /*
> + * Version 1 hardware supports two basic SPI modes:
> + * - Standard SPI mode uses 4 pins, with chipselect
> + * - 3 pin SPI is a 4 pin variant without CS (SPI_NO_CS)
> + * (distinct from SPI_3WIRE, with just one data wire;
> + * or similar variants without MOSI or without MISO)
> + *
> + * Version 2 hardware supports an optional handshaking signal,
> + * so it can support two more modes:
> + * - 5 pin SPI variant is standard SPI plus SPI_READY
> + * - 4 pin with enable is (SPI_READY | SPI_NO_CS)
> + */
> +
> + op_mode = SPIPC0_DIFUN_MASK
> + | SPIPC0_DOFUN_MASK
> + | SPIPC0_CLKFUN_MASK;
> + if (!(spi->mode & SPI_NO_CS))
> + op_mode |= 1 << spi->chip_select;
> + if (spi->mode & SPI_READY)
> + op_mode |= SPIPC0_SPIENA_MASK;
> +
> + iowrite32(op_mode, davinci_spi->base + SPIPC0);
> +
> + if (spi->mode & SPI_LOOP)
> + set_io_bits(davinci_spi->base + SPIGCR1,
> + SPIGCR1_LOOPBACK_MASK);
> + else
> + clear_io_bits(davinci_spi->base + SPIGCR1,
> + SPIGCR1_LOOPBACK_MASK);
These are all global. I suggest moving this code so it kicks
in immediately before a message is processed for that
particular device.
> +
> + /* if bits per word length is zero then set it default 8 */
> + if (!spi->bits_per_word)
> + spi->bits_per_word = 8;
> +
> + /*
> + * SPI in DaVinci and DA8xx operate between
> + * 600 KHz and 50 MHz
> + */
> + if (spi->max_speed_hz < 600000 || spi->max_speed_hz > 50000000)
> + return -EINVAL;
> +
> + retval = davinci_spi_setup_transfer(spi, NULL);
> +
> + return retval;
> +}
> +
_______________________________________________
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
