Re: [PATCH] Memory-mapped Dwsignware SPI driver

[Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>]

On Mon, Jan 18, 2010 at 9:50 AM, Jean-Hugues Deschenes
<jean-hugues.deschenes-YGVykHU+fedBDgjK7y7TUQ@public.gmane.org> wrote:
> Adds a memory-mapped I/O dw_spi platform device.
>
> Signed-off-by: Jean-Hugues Deschenes <jean-hugues.deschenes-YGVykHU+fedBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/spi/Kconfig       |   16 ++++
>  drivers/spi/Makefile      |    1
>  drivers/spi/dw_spi_mmio.c |  164 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 178 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/spi/dw_spi_mmio.c
>
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index f55eb01..79c8d5f 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -308,14 +308,24 @@ config SPI_NUC900
>  #
>
>  config SPI_DESIGNWARE
> -       bool "DesignWare SPI controller core support"
> +       tristate "DesignWare SPI controller core support"
>        depends on SPI_MASTER
>        help
>          general driver for SPI controller core from DesignWare
>
> +choice
> +       prompt "Designware bus type"
> +       depends on SPI_DESIGNWARE
> +       default SPI_DW_PCI
> +
>  config SPI_DW_PCI
> -       tristate "PCI interface driver for DW SPI core"
> -       depends on SPI_DESIGNWARE && PCI
> +       bool "PCI interface driver for DW SPI core"
> +       depends on PCI
> +
> +config SPI_DW_MMIO
> +       bool "Memory-mapped io interface driver for DW SPI core"
> +
> +endchoice

The MMIO patch looks non-orthogonal to the PCI stuff so I don't
understand why a choice/endchoice is being used here.  Is there any
reason that both bus attachments cannot be compiled as modules at the
same time?

> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
> index f3d2810..c3e5ce7 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -18,6 +18,7 @@ obj-$(CONFIG_SPI_AU1550)              += au1550_spi.o
>  obj-$(CONFIG_SPI_BUTTERFLY)            += spi_butterfly.o
>  obj-$(CONFIG_SPI_DESIGNWARE)           += dw_spi.o
>  obj-$(CONFIG_SPI_DW_PCI)               += dw_spi_pci.o
> +obj-$(CONFIG_SPI_DW_MMIO)              += dw_spi_mmio.o
>  obj-$(CONFIG_SPI_GPIO)                 += spi_gpio.o
>  obj-$(CONFIG_SPI_IMX)                  += spi_imx.o
>  obj-$(CONFIG_SPI_LM70_LLP)             += spi_lm70llp.o
> diff --git a/drivers/spi/dw_spi_mmio.c b/drivers/spi/dw_spi_mmio.c
> new file mode 100644
> index 0000000..9abeb45
> --- /dev/null
> +++ b/drivers/spi/dw_spi_mmio.c
> @@ -0,0 +1,164 @@
> +/*
> + * dw_spi_mmio.c - Memory-mapped interface driver for DW SPI Core
> + *
> + * Copyright (c) 2010, Octasic semiconductor.
> + *
> + * Somewhat derived from PCI implementation,
> + * Copyright (c) 2009, Intel Corporation.
> + *
> + * ...and from designware i2c driver
> + * Copyright (C) 2006 Texas Instruments.
> + * Copyright (C) 2007 MontaVista Software Inc.
> + * Copyright (C) 2009 Provigent Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, write to the Free Software Foundation,
> + * Inc., 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.

Do you really need these three paragraphs of preamble?  IMNSOH, the
first paragraph stating GPLv2 should be sufficient.

Otherwise, the patch looks good to me.  I don't have any technical
comments on the code.

Thanks,
g.



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

------------------------------------------------------------------------------
Throughout its 18-year history, RSA Conference consistently attracts the
world's best and brightest in the field, creating opportunities for Conference
attendees to learn about information security's most important issues through
interactions with peers, luminaries and emerging and established companies.
http://p.sf.net/sfu/rsaconf-dev2dev