On Wed, Nov 16, 2011 at 03:37:17PM +0100, John Crispin wrote:
> The external bus unit (EBU) found on the FALC-ON SoC has spi emulation that is
> designed for serial flash access. This driver has only been tested with m25p80
> type chips. The hardware has no support for other types of spi peripherals.
>
> Signed-off-by: Thomas Langer <thomas.langer@xxxxxxxxxx>
> Signed-off-by: John Crispin <blogic@xxxxxxxxxxx>
> Cc: spi-devel-general@xxxxxxxxxxxxxxxxxxxxx
Rough review.
> ---
>
> arch/mips/lantiq/falcon/devices.c | 13 +
> arch/mips/lantiq/falcon/devices.h | 4 +
> arch/mips/lantiq/falcon/mach-easy98000.c | 27 ++
> drivers/spi/Kconfig | 4 +
> drivers/spi/Makefile | 1 +
> drivers/spi/spi-falcon.c | 483 ++++++++++++++++++++++++++++++
> 6 files changed, 532 insertions(+), 0 deletions(-)
> create mode 100644 drivers/spi/spi-falcon.c
You need to split up the arch-specific changes and driver-specific
changes into seperate patches.
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index a1fd73d..f244553 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -180,6 +180,10 @@ config SPI_MPC52xx
> This drivers supports the MPC52xx SPI controller in master SPI
> mode.
>
> +config SPI_FALCON
> + tristate "Falcon SPI controller support"
> + depends on SOC_FALCON
> +
Kconfig is alphabetically sorted. Please keep that.
> config SPI_MPC52xx_PSC
> tristate "Freescale MPC52xx PSC SPI controller"
> depends on PPC_MPC52xx && EXPERIMENTAL
> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
> index 61c3261..570894c 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -25,6 +25,7 @@ obj-$(CONFIG_SPI_DW_MMIO) += spi-dw-mmio.o
> obj-$(CONFIG_SPI_DW_PCI) += spi-dw-midpci.o
> spi-dw-midpci-objs := spi-dw-pci.o spi-dw-mid.o
> obj-$(CONFIG_SPI_EP93XX) += spi-ep93xx.o
> +obj-$(CONFIG_SPI_FALCON) += spi-falcon.o
> obj-$(CONFIG_SPI_FSL_LIB) += spi-fsl-lib.o
> obj-$(CONFIG_SPI_FSL_ESPI) += spi-fsl-espi.o
> obj-$(CONFIG_SPI_FSL_SPI) += spi-fsl-spi.o
> diff --git a/drivers/spi/spi-falcon.c b/drivers/spi/spi-falcon.c
> new file mode 100644
> index 0000000..8b81aa2
> --- /dev/null
> +++ b/drivers/spi/spi-falcon.c
> @@ -0,0 +1,483 @@
> +/*
> + * 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.
> + *
> + * Copyright (C) 2010 Thomas Langer <thomas.langer@xxxxxxxxxx>
> + */
> +
...
> +int
> +falcon_spi_xfer(struct spi_device *spi,
> + struct spi_transfer *t,
> + unsigned long flags)
I'd prefer to squash the lines, dunno Grant's preference.
> +{
> + struct device *dev = &spi->dev;
> + struct falcon_spi *priv = spi_master_get_devdata(spi->master);
> + const u8 *txp = t->tx_buf;
> + u8 *rxp = t->rx_buf;
> + unsigned int bytelen = ((8 * t->len + 7) / 8);
> + unsigned int len, alen, dumlen;
> + u32 val;
> + enum {
> + state_init,
> + state_command_prepare,
> + state_write,
> + state_read,
> + state_disable_cs,
> + state_end
> + } state = state_init;
> +
> + do {
> + switch (state) {
> + case state_init: /* detect phase of upper layer sequence */
> + {
> + /* initial write ? */
> + if (flags & FALCON_SPI_XFER_BEGIN) {
> + if (!txp) {
> + dev_err(dev,
> + "BEGIN without tx data!\n");
> + return -1;
-Esomething perhaps (for all occurences)?
> + }
> + /*
> + * Prepare the parts of the sfcmd register,
> + * which should not
> + * change during a sequence!
> + * Only exception are the length fields,
> + * especially alen and dumlen.
> + */
I think some lines could be squashed again.
> +
> + priv->sfcmd = ((spi->chip_select
> + << SFCMD_CS_OFFSET)
> + & SFCMD_CS_MASK);
> + priv->sfcmd |= SFCMD_KEEP_CS_KEEP_SELECTED;
> + priv->sfcmd |= *txp;
> + txp++;
> + bytelen--;
> + if (bytelen) {
> + /*
> + * more data:
> + * maybe address and/or dummy
> + */
> + state = state_command_prepare;
> + break;
> + } else {
> + dev_dbg(dev, "write cmd %02X\n",
> + priv->sfcmd & SFCMD_OPC_MASK);
> + }
> + }
> + /* continued write ? */
> + if (txp && bytelen) {
> + state = state_write;
> + break;
> + }
> + /* read data? */
> + if (rxp && bytelen) {
> + state = state_read;
> + break;
> + }
> + /* end of sequence? */
> + if (flags & FALCON_SPI_XFER_END)
> + state = state_disable_cs;
> + else
> + state = state_end;
> + break;
> + }
> + /* collect tx data for address and dummy phase */
> + case state_command_prepare:
> + {
> + /* txp is valid, already checked */
> + val = 0;
> + alen = 0;
> + dumlen = 0;
> + while (bytelen > 0) {
> + if (alen < 3) {
> + val = (val<<8)|(*txp++);
Spaces around operators.
> + alen++;
> + } else if ((dumlen < 15) && (*txp == 0)) {
> + /*
> + * assume dummy bytes are set to 0
> + * from upper layer
> + */
> + dumlen++;
> + txp++;
> + } else
> + break;
> + bytelen--;
> + }
...
> +static int
> +falcon_spi_setup(struct spi_device *spi)
> +{
> + struct device *dev = &spi->dev;
> + const u32 ebuclk = CLOCK_100M;
> + unsigned int i;
> + unsigned long flags;
> +
> + dev_dbg(dev, "setup\n");
I'd think these debug outputs at the beginning of functions could simply
go.
> +
> + if (spi->master->bus_num > 0 || spi->chip_select > 0)
> + return -ENODEV;
> +
...
> +static int
> +falcon_spi_transfer(struct spi_device *spi, struct spi_message *m)
> +{
> + struct falcon_spi *priv = spi_master_get_devdata(spi->master);
> + struct spi_transfer *t;
> + unsigned long spi_flags;
> + unsigned long flags;
> + int ret = 0;
> +
> + priv->sfcmd = 0;
> + m->actual_length = 0;
> +
> + spi_flags = FALCON_SPI_XFER_BEGIN;
> + list_for_each_entry(t, &m->transfers, transfer_list) {
> + if (list_is_last(&t->transfer_list, &m->transfers))
> + spi_flags |= FALCON_SPI_XFER_END;
> +
> + spin_lock_irqsave(&ebu_lock, flags);
> + ret = falcon_spi_xfer(spi, t, spi_flags);
> + spin_unlock_irqrestore(&ebu_lock, flags);
> +
> + if (ret)
> + break;
> +
> + m->actual_length += t->len;
> +
> + if (t->delay_usecs || t->cs_change)
> + BUG();
No garceful way of handling that?
> +
> + spi_flags = 0;
> + }
> +
> + m->status = ret;
> + m->complete(m->context);
> +
> + return 0;
> +}
> +
> +static void
> +falcon_spi_cleanup(struct spi_device *spi)
> +{
> + struct device *dev = &spi->dev;
> +
> + dev_dbg(dev, "cleanup\n");
> +}
> +
> +static int __devinit
> +falcon_spi_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct falcon_spi *priv;
> + struct spi_master *master;
> + int ret;
> +
> + dev_dbg(dev, "probing\n");
> +
> + master = spi_alloc_master(&pdev->dev, sizeof(*priv));
> + if (!master) {
> + dev_err(dev, "no memory for spi_master\n");
I think -ENOMEM tells enough.
> + return -ENOMEM;
> + }
> +
> + priv = spi_master_get_devdata(master);
> + priv->master = master;
> +
> + master->mode_bits = SPI_MODE_3;
> + master->num_chipselect = 1;
> + master->bus_num = 0;
> +
> + master->setup = falcon_spi_setup;
> + master->transfer = falcon_spi_transfer;
> + master->cleanup = falcon_spi_cleanup;
> +
> + platform_set_drvdata(pdev, priv);
> +
> + ret = spi_register_master(master);
> + if (ret)
> + spi_master_put(master);
> +
> + return ret;
> +}
> +
> +static int __devexit
> +falcon_spi_remove(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct falcon_spi *priv = platform_get_drvdata(pdev);
> +
> + dev_dbg(dev, "removed\n");
> +
> + spi_unregister_master(priv->master);
> +
> + return 0;
> +}
> +
> +static struct platform_driver falcon_spi_driver = {
> + .probe = falcon_spi_probe,
> + .remove = __devexit_p(falcon_spi_remove),
> + .driver = {
> + .name = DRV_NAME,
> + .owner = THIS_MODULE
> + }
> +};
Use module_platform_driver here.
> +
> +static int __init
> +falcon_spi_init(void)
> +{
> + return platform_driver_register(&falcon_spi_driver);
> +}
> +
> +static void __exit
> +falcon_spi_exit(void)
> +{
> + platform_driver_unregister(&falcon_spi_driver);
> +}
> +
> +module_init(falcon_spi_init);
> +module_exit(falcon_spi_exit);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Lantiq Falcon SPI controller driver");
Thanks,
Wolfram
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Attachment:
signature.asc
Description: Digital signature
