Hi
i am a newbie and am doing a project on beagle board(running
omap3530). i am interfacing an adc(ads7886) to the beagleboard via
spi. need to know how the spi works and how a module can access the
spi registers of the omap. if someones already made an adc driver can
they mail me?
thanks
arnav
On 4/23/09, David Brownell <davi...@pacbell.net> wrote:
> On Thursday 08 January 2009, Stefan Roese wrote:
>> This adds a SPI driver for the SPI controller found in the IBM/AMCC
>> 4xx PowerPC's.
>
>
>> +/*
>> + * The PPC4xx SPI controller has no FIFO so each sent/received byte will
>> + * generate an interrupt to the CPU. This can cause high CPU utilization.
>> + * This driver allows platforms to reduce the interrupt load on the CPU
>> + * during SPI transfers by setting max_speed_hz via the device tree.
>
> Note that an alternate strategy is to use polling instead of IRQs,
> at least when the data rate is high enough that the IRQ processing
> is also slowing down the data transfers.
>
>
>> +/* bits in mode register - bit 0 ist MSb */
>> +/* data latched on leading edge of clock, else trailing edge */
>> +#define SPI_PPC4XX_MODE_SCP (0x80 >> 3)
>
> Or in short, SCP == CPHA.
>
>> +/* port enabled */
>> +#define SPI_PPC4XX_MODE_SPE (0x80 >> 4)
>> +/* MSB first, else LSB first */
>> +#define SPI_PPC4XX_MODE_RD (0x80 >> 5)
>> +/* clock invert - idle clock = 1, active clock = 0; else reversed */
>> +#define SPI_PPC4XX_MODE_CI (0x80 >> 6)
>
> Or in short, CI == CPOL.
>
>> +/* loopback enable */
>> +#define SPI_PPC4XX_MODE_IL (0x80 >> 7)
>> +/* bits in control register */
>> +/* starts a transfer when set */
>> +#define SPI_PPC4XX_CR_STR (0x80 >> 7)
>> +/* bits in status register */
>> +/* port is busy with a transfer */
>> +#define SPI_PPC4XX_SR_BSY (0x80 >> 6)
>> +/* RxD ready */
>> +#define SPI_PPC4XX_SR_RBR (0x80 >> 7)
>> +
>> +/* the spi->mode bits understood by this driver: */
>> +#define MODEBITS (SPI_CPHA | SPI_CPOL | SPI_CS_HIGH | SPI_LSB_FIRST)
>> +
>> +/* clock settings (SCP and CI) for various SPI modes */
>> +#define SPI_CLK_MODE0 SPI_PPC4XX_MODE_SCP
>> +#define SPI_CLK_MODE1 0
>> +#define SPI_CLK_MODE2 SPI_PPC4XX_MODE_CI
>> +#define SPI_CLK_MODE3 (SPI_PPC4XX_MODE_SCP | SPI_PPC4XX_MODE_CI)
>
> Color me puzzled then. Either the definitions of MODE0 and MODE1
> are swapped here ... or the comments above are wrong, and SCP should
> describe "falling" vs "rising" instead of "leading" vs "trailing".
>
>
>> +static int spi_ppc4xx_setupxfer(struct spi_device *spi, struct
>> spi_transfer *t)
>> +{
>> + struct ppc4xx_spi *hw = spi_master_get_devdata(spi->master);
>> + struct spi_ppc4xx_cs *cs = spi->controller_state;
>> + int scr;
>> + u8 cdm = 0;
>> + u32 speed;
>> + u8 bits_per_word;
>> +
>> + bits_per_word = (t) ? t->bits_per_word : spi->bits_per_word;
>> + speed = (t) ? t->speed_hz : spi->max_speed_hz;
>> +
>> + ...
>> +
>> + if (!speed || (speed > spi->max_speed_hz)) {
>
> This is wrong. Typical case is that t->speed_hz is zero,
> meaning "use spi->max_speed_hz" ... you treat that as an error.
>
>> + dev_err(&spi->dev, "invalid speed_hz (%d)\n", speed);
>> + return -EINVAL;
>> + }
>> +
>> + ...
>> +}
>> +
>> +static int spi_ppc4xx_setup(struct spi_device *spi)
>> +{
>> + int ret;
>> + struct spi_ppc4xx_cs *cs = spi->controller_state;
>> + int init = 0;
>
> This "init" thing is still wrong.
>
> Consider: board gets set up with one device. Later,
> *WHILE BUSY TRANSFERRING DATA TO/FROM THAT DEVICE*
> some other device gets instantiated. Then ...
>
>
>> + if (cs == NULL) {
>> + cs = kzalloc(sizeof *cs, GFP_KERNEL);
>> + if (!cs)
>> + return -ENOMEM;
>> + spi->controller_state = cs;
>> +
>> + /*
>> + * First time called, so let's init the SPI controller
>> + * at the end of this function
>> + */
>> + init = 1;
>
> "init" becomes true here, although the controller has
> already been initialized. If it needs to be set up,
> do it in probe() before registering the spi_master.
>
>
>> + }
>> +
>> + ...
>> +
>> + /*
>> + * New configuration (mode, speed etc) will be written to the
>> + * controller in spi_ppc4xx_setupxfer(). Only call
>> + * spi_ppc4xx_setupxfer() directly upon first initialization.
>> + */
>> + if (init) {
>> + ret = spi_ppc4xx_setupxfer(spi, NULL);
>
> ... so blam, you clobber the settings currently being used for
> the active transfer. So this would be a bug.
>
> If not ... this driver deserves a comment on exactly how
> unusual this driver is. Specifically, that all spi_device
> children must be set up *in advance* so that standard calls
> like spi_new_device() don't work with it; and why.
>
> I don't think I see anything in this driver which would
> prevent that from working, though. Sure you've got to
> have a list of chipselect GPIOs in advance, but that's
> distinct from being unable to use spi_new_device().
>
>
>> + ...
>> +}
>> +
>> +static void spi_ppc4xx_chipsel(struct spi_device *spi, int value)
>> +{
>> + struct ppc4xx_spi *hw = spi_master_get_devdata(spi->master);
>> + unsigned int cspol = spi->mode & SPI_CS_HIGH ? 1 : 0;
>> + unsigned int cs = spi->chip_select;
>> +
>> + if (!hw->num_gpios)
>> + return;
>
> Hmm, num_gpios ... can never be zero, right? You always
> set it up so that of_gpio_count() GPIOs can all be used
> as chipselects. Looks like this check is not needed.
>
>> +
>> + if (cs >= hw->num_gpios)
>> + return;
>
> I guess I don't see why you have num_gpio instead of just
> using the chipselect count, either. (Assuming that the
> of_gpio_count routine isn't counting *all* gpios, instead
> of just ones allocated to this controller, which would be
> a different issue.)
>
>
>> +
>> + if (value != BITBANG_CS_INACTIVE && value != BITBANG_CS_ACTIVE)
>> + return;
>
> Strange, what other values could be passed??
>
>
>> +
>> + if (value == BITBANG_CS_INACTIVE)
>> + cspol = !cspol;
>> +
>> + gpio_set_value(hw->gpios[cs], cspol);
>> +}
>> +
>> +static irqreturn_t spi_ppc4xx_int(int irq, void *dev_id)
>> +{
>> + struct ppc4xx_spi *hw;
>> + u8 status;
>> + u8 data;
>> + unsigned int count;
>> +
>> + hw = (struct ppc4xx_spi *)dev_id;
>> +
>> + status = in_8(&hw->regs->sr);
>> + if (!status)
>> + return IRQ_NONE;
>> +
>> + /* should never happen but check anyway */
>> + if (status & SPI_PPC4XX_SR_BSY) {
>
> Maybe "if (unlikely(...)) {" then.
>
>
>> + u8 lstatus;
>> + int cnt = 0;
>> +
>> + dev_dbg(hw->dev, "got interrupt but spi still busy?\n");
>
> But ... *does* this ever happen? If not, I'd strike this
> code. And if it does, I'd add a comment explaining what's
> known about this hardware bug. Without this, the IRQ handler
> would look pretty tight.
>
>> + do {
>> + ndelay(10);
>> + lstatus = in_8(&hw->regs->sr);
>> + } while (++cnt < 100 && lstatus & SPI_PPC4XX_SR_BSY);
>> +
>> + if (cnt >= 100) {
>> + dev_err(hw->dev, "busywait: too many loops!\n");
>> + complete(&hw->done);
>> + return IRQ_HANDLED;
>> + } else {
>> + /* status is always 1 (RBR) here */
>> + status = in_8(&hw->regs->sr);
>> + dev_dbg(hw->dev, "loops %d status %x\n", cnt, status);
>> + }
>> + }
>> +
>> + count = hw->count;
>> + hw->count++;
>> +
>> + if (status & SPI_PPC4XX_SR_RBR) {
>> + /* Data Ready */
>
> Most SPI hardware I've seen has at most a weak distinction
> between "tx done" and "rx done" IRQs ... there's little
> point to separate IRQs, since shifting out the last TX bit
> is what shifts in the last RX bit.
>
> Unless this is unusual hardware you can probably just assume
> that there's RX data, and only read it if hw->rx is non-null.
> Or, if this is *not* set, report the odd behavior...
>
>
>> + data = in_8(&hw->regs->rxd);
>> + if (hw->rx)
>> + hw->rx[count] = data;
>> + }
>> +
>> + count++;
>> +
>> + if (count < hw->len) {
>> + data = hw->tx ? hw->tx[count] : 0;
>> + out_8(&hw->regs->txd, data);
>> + out_8(&hw->regs->cr, SPI_PPC4XX_CR_STR);
>> + } else {
>> + complete(&hw->done);
>> + }
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> + ...
>> +
>
>> +static int __init spi_ppc4xx_of_probe(struct of_device *op,
>> + const struct of_device_id *match)
>> +{
>> + struct ppc4xx_spi *hw;
>> + struct spi_master *master;
>> + struct spi_bitbang *bbp;
>> + struct resource resource;
>> + struct device_node *np = op->node;
>> + struct device *dev = &op->dev;
>> + struct device_node *opbnp;
>> + int ret;
>> + const unsigned int *clk;
>> +
>> + master = spi_alloc_master(dev, sizeof *hw);
>> + if (master == NULL)
>> + return -ENOMEM;
>> + dev_set_drvdata(dev, master);
>> + hw = spi_master_get_devdata(master);
>> + memset(hw, 0, sizeof(struct ppc4xx_spi));
>
> That memset is not required, it comes to you pre-zeroed.
>
>> +
>> + hw->master = spi_master_get(master);
>> + hw->dev = dev;
>> +
>> + init_completion(&hw->done);
>> +
>> + hw->num_gpios = of_gpio_count(np);
>
> Worth a comment what's going on there. I'm assuming this
> of_gpio_count() thing returns a count of GPIOs somehow
> associated with this specific device tree node, rather than
> say all GPIOs known to the device tree...
>
>> + if (hw->num_gpios) {
>> + int i;
>> +
>> + hw->gpios = kzalloc(sizeof(int) * hw->num_gpios,
>> + GFP_KERNEL);
>> + if (!hw->gpios) {
>> + ret = -ENOMEM;
>> + goto free_master;
>> + }
>> +
>> + for (i = 0; i < hw->num_gpios; i++) {
>> + int gpio = of_get_gpio(np, i);
>> + if (gpio < 0) {
>> + dev_err(dev, "Invalid gpio spec %d\n", i);
>> + ret = gpio;
>> + goto free_gpios;
>> + }
>> +
>> + ret = gpio_request(gpio, np->name);
>> + if (ret < 0) {
>> + dev_err(dev, "gpio %d already in use\n", i);
>> + ret = gpio;
>> + goto free_gpios;
>> + }
>> +
>> + gpio_direction_output(gpio, 0);
>> + hw->gpios[i] = gpio;
>> + }
>> + }
>
> Else ... error, if there are no chipselects?
>
> I've got a patch pending to add a "no chipselect" spi->mode flag.
> If you expect to support that, please let me know.
>
>
>> +
>> + /* Setup the state for the bitbang driver */
>> + bbp = &hw->bitbang;
>> + bbp->master = hw->master;
>> + bbp->setup_transfer = spi_ppc4xx_setupxfer;
>> + bbp->chipselect = spi_ppc4xx_chipsel;
>> + bbp->txrx_bufs = spi_ppc4xx_txrx;
>> + bbp->use_dma = 0;
>> + bbp->master->setup = spi_ppc4xx_setup;
>> + bbp->master->cleanup = spi_ppc4xx_cleanup;
>> + /* only one SPI controller */
>> + bbp->master->bus_num = 0;
>
> Bad assumption. There could be GPIO-based adapters,
> of course. And aren't these the cores which can be
> found on higher end Xilinx FPGAs? And, accordingly,
> integrated with any number of additional controllers?
>
> If so, the restriction is just that this controller
> be bus #0. If not ... the Kconfig should say more
> about *which* PPC4xx chips have this controller.
> (I suspect it's the latter.)
>
>
>> + if (bbp->master->num_chipselect == 0) {
>
> You didn't set it, so of course it's still zeroed.
>
>
>> + /* this many pins in al GPIO controllers */
>> + bbp->master->num_chipselect = hw->num_gpios;
>
> So num_chipselect is alays num_gpios, and there's no point
> to having num_gpios since you could always use num_chipselect.
>
>
>> + }
>> +
>> + ...
>> +}
>
> ------------------------------------------------------------------------------
> Stay on top of everything new and different, both inside and
> around Java (TM) technology - register by April 22, and save
> $200 on the JavaOne (SM) conference, June 2-5, 2009, San Francisco.
> 300 plus technical and hands-on sessions. Register today.
> Use priority code J9JMT32. http://p.sf.net/sfu/p
> _______________________________________________
> spi-devel-general mailing list
> spi-devel-gene...@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/spi-devel-general
>
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
