Messages in this thread | | | From | Grant Likely <> | Date | Sun, 16 Jan 2011 23:56:35 -0700 | Subject | Re: [PATCH v2] spi: New driver for Altera SPI |
| |
On Sun, Jan 16, 2011 at 10:46 PM, Thomas Chou <thomas@wytron.com.tw> wrote: > This patch adds a new SPI driver to support the Altera SOPC Builder > SPI component. It uses the bitbanging library. > > Signed-off-by: Thomas Chou <thomas@wytron.com.tw>
Hi Thomas, some minor comments below. I've not dug any deeper, so I have not verified locking or the way the driver uses the bitbang library, or anything like that.
g.
> --- > v2 add devicetree support. > > drivers/spi/Kconfig | 6 + > drivers/spi/Makefile | 1 + > drivers/spi/spi_altera.c | 398 ++++++++++++++++++++++++++++++++++++++++ > include/linux/spi/spi_altera.h | 12 ++ > 4 files changed, 417 insertions(+), 0 deletions(-) > create mode 100644 drivers/spi/spi_altera.c > create mode 100644 include/linux/spi/spi_altera.h > > diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig > index 13bfa9d..2b5b8d6 100644 > --- a/drivers/spi/Kconfig > +++ b/drivers/spi/Kconfig > @@ -53,6 +53,12 @@ if SPI_MASTER > > comment "SPI Master Controller Drivers" > > +config SPI_ALTERA > + tristate "Altera SPI Controller" > + select SPI_BITBANG > + help > + This is the driver for the Altera SPI Controller. > + > config SPI_ATMEL > tristate "Atmel SPI Controller" > depends on (ARCH_AT91 || AVR32) > diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile > index 3a42463..c2675a4 100644 > --- a/drivers/spi/Makefile > +++ b/drivers/spi/Makefile > @@ -9,6 +9,7 @@ ccflags-$(CONFIG_SPI_DEBUG) := -DDEBUG > obj-$(CONFIG_SPI_MASTER) += spi.o > > # SPI master controller drivers (bus) > +obj-$(CONFIG_SPI_ALTERA) += spi_altera.o > obj-$(CONFIG_SPI_ATMEL) += atmel_spi.o > obj-$(CONFIG_SPI_BFIN) += spi_bfin5xx.o > obj-$(CONFIG_SPI_BITBANG) += spi_bitbang.o > diff --git a/drivers/spi/spi_altera.c b/drivers/spi/spi_altera.c > new file mode 100644 > index 0000000..28c78c3 > --- /dev/null > +++ b/drivers/spi/spi_altera.c > @@ -0,0 +1,398 @@ > +/* > + * Altera SPI driver > + * > + * Copyright (C) 2008 Thomas Chou <thomas@wytron.com.tw> > + * > + * Based on spi_s3c24xx.c, which is: > + * Copyright (c) 2006 Ben Dooks > + * Copyright (c) 2006 Simtec Electronics > + * Ben Dooks <ben@simtec.co.uk> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + */ > + > +#include <linux/init.h> > +#include <linux/spinlock.h> > +#include <linux/workqueue.h> > +#include <linux/interrupt.h> > +#include <linux/delay.h> > +#include <linux/errno.h> > +#include <linux/err.h> > +#include <linux/clk.h> > +#include <linux/platform_device.h> > +#include <linux/spi/spi.h> > +#include <linux/spi/spi_bitbang.h> > +#include <linux/spi/spi_altera.h> > +#include <linux/io.h> > +#include <linux/of.h> > + > +#define DRV_NAME "spi_altera" > + > +#define ALTERA_SPI_RXDATA 0 > +#define ALTERA_SPI_TXDATA 4 > +#define ALTERA_SPI_STATUS 8 > +#define ALTERA_SPI_CONTROL 12 > +#define ALTERA_SPI_SLAVE_SEL 20 > + > +#define ALTERA_SPI_STATUS_ROE_MSK 0x8 > +#define ALTERA_SPI_STATUS_TOE_MSK 0x10 > +#define ALTERA_SPI_STATUS_TMT_MSK 0x20 > +#define ALTERA_SPI_STATUS_TRDY_MSK 0x40 > +#define ALTERA_SPI_STATUS_RRDY_MSK 0x80 > +#define ALTERA_SPI_STATUS_E_MSK 0x100 > + > +#define ALTERA_SPI_CONTROL_IROE_MSK 0x8 > +#define ALTERA_SPI_CONTROL_ITOE_MSK 0x10 > +#define ALTERA_SPI_CONTROL_ITRDY_MSK 0x40 > +#define ALTERA_SPI_CONTROL_IRRDY_MSK 0x80 > +#define ALTERA_SPI_CONTROL_IE_MSK 0x100 > +#define ALTERA_SPI_CONTROL_SSO_MSK 0x400 > + > +struct altera_spi { > + /* bitbang has to be first */ > + struct spi_bitbang bitbang; > + struct completion done; > + > + void __iomem *base; > + int irq; > + int interrupt; /* use interrupt driven data transfer, slow */
Is this a flag? If so, use type 'bool'.
> + int len; > + int count; > + int bytes_per_word; > + unsigned long imr; > + > + /* data buffers */ > + const unsigned char *tx; > + unsigned char *rx; > + > +};
Nit: drop the blank line at the end of the structure.
> + > +static inline struct altera_spi *to_hw(struct spi_device *sdev)
All file scope symbols should have a driver prefix. In this case, it should be "altera_spi_to_hw()".
> +{ > + return spi_master_get_devdata(sdev->master); > +} > + > +static void altera_spi_chipsel(struct spi_device *spi, int value) > +{ > + struct altera_spi *hw = to_hw(spi); > + > + if (spi->mode & SPI_CS_HIGH) { > + switch (value) { > + case BITBANG_CS_INACTIVE: > + writel(1 << spi->chip_select, > + hw->base + ALTERA_SPI_SLAVE_SEL); > + hw->imr |= ALTERA_SPI_CONTROL_SSO_MSK; > + writel(hw->imr, hw->base + ALTERA_SPI_CONTROL); > + break; > + > + case BITBANG_CS_ACTIVE: > + hw->imr &= ~ALTERA_SPI_CONTROL_SSO_MSK; > + writel(hw->imr, hw->base + ALTERA_SPI_CONTROL); > + writel(0, hw->base + ALTERA_SPI_SLAVE_SEL); > + break; > + } > + } else { > + switch (value) { > + case BITBANG_CS_INACTIVE: > + hw->imr &= ~ALTERA_SPI_CONTROL_SSO_MSK; > + writel(hw->imr, hw->base + ALTERA_SPI_CONTROL); > + break; > + > + case BITBANG_CS_ACTIVE: > + writel(1 << spi->chip_select, > + hw->base + ALTERA_SPI_SLAVE_SEL); > + hw->imr |= ALTERA_SPI_CONTROL_SSO_MSK; > + writel(hw->imr, hw->base + ALTERA_SPI_CONTROL); > + break; > + } > + } > +} > + > +static int altera_spi_setupxfer(struct spi_device *spi, struct spi_transfer *t) > +{ > + return 0; > +} > + > +static int altera_spi_setup(struct spi_device *spi) > +{ > + return 0; > +} > + > +static inline unsigned int hw_txbyte(struct altera_spi *hw, int count) > +{ > + if (hw->tx) { > + switch (hw->bytes_per_word) { > + case 1: > + return hw->tx[count]; > + case 2: > + return (hw->tx[count * 2] > + | (hw->tx[count * 2 + 1] << 8)); > + } > + } > + > + return 0; > +} > + > +static int altera_spi_txrx(struct spi_device *spi, struct spi_transfer *t) > +{ > + struct altera_spi *hw = to_hw(spi); > + > + hw->tx = t->tx_buf; > + hw->rx = t->rx_buf; > + hw->count = 0; > + hw->bytes_per_word = (t->bits_per_word ? : spi->bits_per_word) / 8; > + hw->len = t->len / hw->bytes_per_word; > + > + if (hw->irq >= 0 && hw->interrupt) { > + init_completion(&hw->done); > + /* enable receive interrupt */ > + hw->imr |= ALTERA_SPI_CONTROL_IRRDY_MSK; > + writel(hw->imr, hw->base + ALTERA_SPI_CONTROL); > + > + /* send the first byte */ > + writel(hw_txbyte(hw, 0), hw->base + ALTERA_SPI_TXDATA); > + > + wait_for_completion(&hw->done); > + /* disable receive interrupt */ > + hw->imr &= ~ALTERA_SPI_CONTROL_IRRDY_MSK; > + writel(hw->imr, hw->base + ALTERA_SPI_CONTROL); > + } else { > + /* send the first byte */ > + writel(hw_txbyte(hw, 0), hw->base + ALTERA_SPI_TXDATA); > + > + while (1) { > + unsigned int rxd; > + > + while (!(readl(hw->base + ALTERA_SPI_STATUS) & > + ALTERA_SPI_STATUS_RRDY_MSK)) > + cpu_relax(); > + > + rxd = readl(hw->base + ALTERA_SPI_RXDATA); > + if (hw->rx) { > + switch (hw->bytes_per_word) { > + case 1: > + hw->rx[hw->count] = rxd; > + break; > + case 2: > + hw->rx[hw->count * 2] = rxd; > + hw->rx[hw->count * 2 + 1] = rxd >> 8; > + break; > + } > + } > + > + hw->count++; > + > + if (hw->count < hw->len) > + writel(hw_txbyte(hw, hw->count), > + hw->base + ALTERA_SPI_TXDATA); > + else > + break; > + } > + > + } > + > + return hw->count * hw->bytes_per_word; > +} > + > +static irqreturn_t altera_spi_irq(int irq, void *dev) > +{ > + struct altera_spi *hw = dev; > + unsigned int rxd; > + > + rxd = readl(hw->base + ALTERA_SPI_RXDATA); > + if (hw->rx) { > + switch (hw->bytes_per_word) { > + case 1: > + hw->rx[hw->count] = rxd; > + break; > + case 2: > + hw->rx[hw->count * 2] = rxd; > + hw->rx[hw->count * 2 + 1] = rxd >> 8; > + break; > + } > + } > + > + hw->count++; > + > + if (hw->count < hw->len) > + writel(hw_txbyte(hw, hw->count), hw->base + ALTERA_SPI_TXDATA); > + else > + complete(&hw->done); > + > + return IRQ_HANDLED; > +} > + > +#ifdef CONFIG_OF > +static int __devinit altera_spi_of_probe(struct platform_device *pdev, > + struct altera_spi *hw) > +{ > + const __be32 *val; > + > + hw->bitbang.master->dev.of_node = pdev->dev.of_node; > + val = of_get_property(pdev->dev.of_node, "interrupt-driven", NULL); > + if (val) > + hw->interrupt = be32_to_cpup(val); > + return 0; > +} > +#else > +static int __devinit altera_spi_of_probe(struct platform_device *pdev, > + struct altera_spi *hw) > +{ > + return 0; > +} > +#endif > + > +static int __devinit altera_spi_probe(struct platform_device *pdev) > +{ > + struct altera_spi_platform_data *platp = pdev->dev.platform_data; > + struct altera_spi *hw; > + struct spi_master *master; > + struct resource *res; > + int err = 0; > + > + master = spi_alloc_master(&pdev->dev, sizeof(struct altera_spi)); > + if (master == NULL) { > + dev_err(&pdev->dev, "No memory for spi_master\n"); > + err = -ENOMEM; > + goto err_no_mem; > + } > + > + /* setup the master state. */ > + master->bus_num = pdev->id; > + master->num_chipselect = 16; > + master->mode_bits = SPI_CS_HIGH; > + master->setup = altera_spi_setup; > + > + hw = spi_master_get_devdata(master); > + platform_set_drvdata(pdev, hw); > + > + /* setup the state for the bitbang driver */ > + hw->bitbang.master = spi_master_get(master); > + if (hw->bitbang.master == NULL) { > + dev_err(&pdev->dev, "Cannot get device\n"); > + err = -ENODEV; > + goto err_no_dev; > + } > + hw->bitbang.setup_transfer = altera_spi_setupxfer; > + hw->bitbang.chipselect = altera_spi_chipsel; > + hw->bitbang.txrx_bufs = altera_spi_txrx; > + > + /* find and map our resources */ > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (res == NULL) { > + dev_err(&pdev->dev, "Cannot get IORESOURCE_MEM\n"); > + err = -ENOENT; > + goto err_no_iores; > + } > + hw->base = ioremap(res->start, (res->end - res->start) + 1); > + if (hw->base == 0) { > + dev_err(&pdev->dev, "Cannot map IO\n"); > + err = -ENXIO; > + goto err_no_iomap; > + } > + /* program defaults into the registers */ > + hw->imr = 0; /* disable spi interrupts */ > + writel(hw->imr, hw->base + ALTERA_SPI_CONTROL); > + writel(0, hw->base + ALTERA_SPI_STATUS); /* clear status reg */ > + if (readl(hw->base + ALTERA_SPI_STATUS) & ALTERA_SPI_STATUS_RRDY_MSK) > + readl(hw->base + ALTERA_SPI_RXDATA); /* flush rxdata */ > + /* irq is optional */ > + hw->irq = platform_get_irq(pdev, 0); > + if (hw->irq >= 0) { > + init_completion(&hw->done); > + err = request_irq(hw->irq, altera_spi_irq, 0, pdev->name, hw); > + if (err) { > + dev_err(&pdev->dev, "Cannot claim IRQ\n"); > + goto err_no_irq; > + } > + } > + /* find platform data */ > + if (platp) { > + hw->interrupt = platp->interrupt; > + } else { > + err = altera_spi_of_probe(pdev, hw); > + if (err) > + goto err_no_of; > + } > + > + /* register our spi controller */ > + err = spi_bitbang_start(&hw->bitbang); > + if (err) { > + dev_err(&pdev->dev, "Failed to register SPI master\n"); > + goto err_register; > + } > + dev_info(&pdev->dev, "base %p, irq %d\n", hw->base, hw->irq); > + > + return 0; > + > +err_register: > +err_no_of: > + if (hw->irq >= 0) > + free_irq(hw->irq, hw); > +err_no_irq: > + iounmap((void *)hw->base);
The cast should not be necessary.
> +err_no_iomap: > +err_no_iores: > + spi_master_put(master);; > +err_no_mem: > +err_no_dev: > + return err; > +} > + > +static int __devexit altera_spi_remove(struct platform_device *dev) > +{ > + struct altera_spi *hw = platform_get_drvdata(dev); > + struct spi_master *master = hw->bitbang.master; > + > + spi_bitbang_stop(&hw->bitbang); > + > + if (hw->irq >= 0) > + free_irq(hw->irq, hw); > + iounmap((void *)hw->base);
Ditto here.
> + > + platform_set_drvdata(dev, NULL); > + spi_master_put(master); > + return 0; > +} > + > +#ifdef CONFIG_OF > +static struct of_device_id altera_spi_match[] = { > + { > + .compatible = "altera,spi_altera", > + }, > + {}, > +} > +MODULE_DEVICE_TABLE(of, altera_spi_match); > +#endif > + > +static struct platform_driver altera_spidrv = { > + .remove = __devexit_p(altera_spi_remove), > + .driver = { > + .name = DRV_NAME, > + .owner = THIS_MODULE, > + .pm = NULL, > +#ifdef CONFIG_OF > + .of_match_table = altera_spi_match, > +#endif > + }, > +}; > + > +static int __init altera_spi_init(void) > +{ > + return platform_driver_probe(&altera_spidrv, altera_spi_probe); > +} > + > +static void __exit altera_spi_exit(void) > +{ > + platform_driver_unregister(&altera_spidrv); > +} > + > +module_init(altera_spi_init);
Move module_init line to be immediately after altera_spi_init() definition.
> +module_exit(altera_spi_exit); > + > +MODULE_DESCRIPTION("Altera SPI driver"); > +MODULE_AUTHOR("Thomas Chou <thomas@wytron.com.tw>"); > +MODULE_LICENSE("GPL"); > +MODULE_ALIAS("platform:" DRV_NAME); > diff --git a/include/linux/spi/spi_altera.h b/include/linux/spi/spi_altera.h > new file mode 100644 > index 0000000..b3d7f69 > --- /dev/null > +++ b/include/linux/spi/spi_altera.h > @@ -0,0 +1,12 @@ > +#ifndef _LINUX_SPI_SPI_ALTERA_H > +#define _LINUX_SPI_SPI_ALTERA_H > + > +/* > + * struct altera_spi_platform_data - platform data of the Altera SPI > + * @interrupt: use intrrupt driven data transfer. > + */ > +struct altera_spi_platform_data { > + int interrupt; > +}; > + > +#endif /* _LINUX_SPI_SPI_ALTERA_H */ > -- > 1.7.3.4 > >
-- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd.
| |