From mboxrd@z Thu Jan 1 00:00:00 1970 From: Grant Likely Subject: Re: [PATCH v2] spi: New driver for Altera SPI Date: Sun, 16 Jan 2011 23:56:35 -0700 Message-ID: References: <1254981838-20584-1-git-send-email-thomas@wytron.com.tw> <1295243200-28230-1-git-send-email-thomas@wytron.com.tw> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1295243200-28230-1-git-send-email-thomas-SDxUXYEhEBiCuPEqFHbRBg@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: devicetree-discuss-bounces+gldd-devicetree-discuss=m.gmane.org-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org Errors-To: devicetree-discuss-bounces+gldd-devicetree-discuss=m.gmane.org-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org To: Thomas Chou Cc: nios2-dev-1eJk0qcHJCcaeqlQEoCUNoJY59XmG8rH@public.gmane.org, David Brownell , Mike Frysinger , devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org List-Id: devicetree@vger.kernel.org On Sun, Jan 16, 2011 at 10:46 PM, Thomas Chou 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 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 > + * > + * Based on spi_s3c24xx.c, which is: > + * Copyright (c) 2006 Ben Dooks > + * Copyright (c) 2006 Simtec Electronics > + * Ben Dooks > + * > + * 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 > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#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 "); > +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.