[3/5] fpga: add basic CARMA board support

This adds basic support for the system controller FPGA on the OVRO CARMA
board. This patch only adds infrastructure that will be used by later
drivers.

Signed-off-by: Ira W. Snyder <iws@ovro.caltech.edu>
---
 drivers/Kconfig             |    2 +
 drivers/Makefile            |    1 +
 drivers/fpga/Kconfig        |    1 +
 drivers/fpga/Makefile       |    1 +
 drivers/fpga/carma/Kconfig  |   21 ++++++++++++
 drivers/fpga/carma/Makefile |    1 +
 drivers/fpga/carma/carma.c  |   73 +++++++++++++++++++++++++++++++++++++++++++
 drivers/fpga/carma/carma.h  |   22 +++++++++++++
 8 files changed, 122 insertions(+), 0 deletions(-)
 create mode 100644 drivers/fpga/Kconfig
 create mode 100644 drivers/fpga/Makefile
 create mode 100644 drivers/fpga/carma/Kconfig
 create mode 100644 drivers/fpga/carma/Makefile
 create mode 100644 drivers/fpga/carma/carma.c
 create mode 100644 drivers/fpga/carma/carma.h

On Wed, 2010-09-08 at 09:41 -0700, Ira W. Snyder wrote:
> This adds basic support for the system controller FPGA on the OVRO CARMA
> board. This patch only adds infrastructure that will be used by later
> drivers.

One comment I have is do you really need to create a class ? Do you
provide a specific API/interface that would make it useful ?

Cheers,
Ben.

> Signed-off-by: Ira W. Snyder <iws@ovro.caltech.edu>
> ---
>  drivers/Kconfig             |    2 +
>  drivers/Makefile            |    1 +
>  drivers/fpga/Kconfig        |    1 +
>  drivers/fpga/Makefile       |    1 +
>  drivers/fpga/carma/Kconfig  |   21 ++++++++++++
>  drivers/fpga/carma/Makefile |    1 +
>  drivers/fpga/carma/carma.c  |   73 +++++++++++++++++++++++++++++++++++++++++++
>  drivers/fpga/carma/carma.h  |   22 +++++++++++++
>  8 files changed, 122 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/fpga/Kconfig
>  create mode 100644 drivers/fpga/Makefile
>  create mode 100644 drivers/fpga/carma/Kconfig
>  create mode 100644 drivers/fpga/carma/Makefile
>  create mode 100644 drivers/fpga/carma/carma.c
>  create mode 100644 drivers/fpga/carma/carma.h
> 
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index a2b902f..8945ae6 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -111,4 +111,6 @@ source "drivers/xen/Kconfig"
>  source "drivers/staging/Kconfig"
>  
>  source "drivers/platform/Kconfig"
> +
> +source "drivers/fpga/Kconfig"
>  endmenu
> diff --git a/drivers/Makefile b/drivers/Makefile
> index ae47344..c0b05de 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -115,3 +115,4 @@ obj-$(CONFIG_VLYNQ)		+= vlynq/
>  obj-$(CONFIG_STAGING)		+= staging/
>  obj-y				+= platform/
>  obj-y				+= ieee802154/
> +obj-$(CONFIG_FPGA_DRIVERS)	+= fpga/
> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> new file mode 100644
> index 0000000..c85c2cc
> --- /dev/null
> +++ b/drivers/fpga/Kconfig
> @@ -0,0 +1 @@
> +source "drivers/fpga/carma/Kconfig"
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> new file mode 100644
> index 0000000..409a5f9
> --- /dev/null
> +++ b/drivers/fpga/Makefile
> @@ -0,0 +1 @@
> +obj-y		+= carma/
> diff --git a/drivers/fpga/carma/Kconfig b/drivers/fpga/carma/Kconfig
> new file mode 100644
> index 0000000..448885e
> --- /dev/null
> +++ b/drivers/fpga/carma/Kconfig
> @@ -0,0 +1,21 @@
> +
> +menuconfig FPGA_DRIVERS
> +	bool "FPGA Drivers"
> +	default n
> +	help
> +	  Say Y here to see options for devices used with custom FPGAs.
> +	  This option alone does not add any kernel code.
> +
> +	  If you say N, all options in this submenu will be skipped and disabled.
> +
> +if FPGA_DRIVERS
> +
> +config CARMA
> +	tristate "CARMA System Controller FPGA support"
> +	depends on FSL_SOC && PPC_83xx
> +	default n
> +	help
> +	  Say Y here to include basic support for the CARMA System Controller
> +	  FPGA. This option allows the other more advanced drivers to be built.
> +
> +endif # FPGA_DRIVERS
> diff --git a/drivers/fpga/carma/Makefile b/drivers/fpga/carma/Makefile
> new file mode 100644
> index 0000000..90d0594
> --- /dev/null
> +++ b/drivers/fpga/carma/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_CARMA)			+= carma.o
> diff --git a/drivers/fpga/carma/carma.c b/drivers/fpga/carma/carma.c
> new file mode 100644
> index 0000000..97549d2
> --- /dev/null
> +++ b/drivers/fpga/carma/carma.c
> @@ -0,0 +1,73 @@
> +/*
> + * CARMA Board Utility Driver
> + *
> + * Copyright (c) 2009-2010 Ira W. Snyder <iws@ovro.caltech.edu>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +
> +#include "carma.h"
> +
> +static struct class *carma_class;
> +static const char drv_name[] = "carma";
> +
> +/*
> + * CARMA Device Class Functions
> + */
> +
> +struct device *carma_device_create(struct device *parent, dev_t devno,
> +				   const char *fmt, ...)
> +{
> +	struct device *dev;
> +	va_list vargs;
> +
> +	va_start(vargs, fmt);
> +	dev = device_create_vargs(carma_class, parent, devno, NULL, fmt, vargs);
> +	va_end(vargs);
> +
> +	return dev;
> +}
> +EXPORT_SYMBOL_GPL(carma_device_create);
> +
> +void carma_device_destroy(dev_t devno)
> +{
> +	device_destroy(carma_class, devno);
> +}
> +EXPORT_SYMBOL_GPL(carma_device_destroy);
> +
> +/*
> + * Module Init / Exit
> + */
> +
> +static int __init carma_init(void)
> +{
> +	/* Register the CARMA device class */
> +	carma_class = class_create(THIS_MODULE, "carma");
> +	if (IS_ERR(carma_class)) {
> +		pr_err("%s: unable to create CARMA class\n", drv_name);
> +		return PTR_ERR(carma_class);
> +	}
> +
> +	return 0;
> +}
> +
> +static void __exit carma_exit(void)
> +{
> +	class_destroy(carma_class);
> +}
> +
> +MODULE_AUTHOR("Ira W. Snyder <iws@ovro.caltech.edu>");
> +MODULE_DESCRIPTION("CARMA Device Class Driver");
> +MODULE_LICENSE("GPL");
> +
> +module_init(carma_init);
> +module_exit(carma_exit);
> diff --git a/drivers/fpga/carma/carma.h b/drivers/fpga/carma/carma.h
> new file mode 100644
> index 0000000..f556dc8
> --- /dev/null
> +++ b/drivers/fpga/carma/carma.h
> @@ -0,0 +1,22 @@
> +/*
> + * CARMA Board Utilities
> + *
> + * Copyright (c) 2009-2010 Ira W. Snyder <iws@ovro.caltech.edu>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + */
> +
> +#ifndef CARMA_DEVICE_H
> +#define CARMA_DEVICE_H
> +
> +#include <linux/device.h>
> +
> +struct device *carma_device_create(struct device *parent, dev_t devno,
> +				   const char *fmt, ...)
> +				   __attribute__((format(printf, 3, 4)));
> +void carma_device_destroy(dev_t devno);
> +
> +#endif /* CARMA_DEVICE_H */

On Wed, 2010-09-08 at 09:41 -0700, Ira W. Snyder wrote:
> This adds basic support for the system controller FPGA on the OVRO CARMA
> board. This patch only adds infrastructure that will be used by later
> drivers.

Oh and another comment ...

I'm not sure about drivers/fpga ... in the end, one would expect such a
directory to contain stuff to manipulate FPGAs in the sense of
downloading bitfiles, instanciating devices (device-tree manipulation ?)
etc...

From what I see in your code, the fact that these are FPGAs is almost
irrelevant, you are providing support for "carma" devices, and such are
no different than some other platform device, they just happen to be
implemented as FPGAs. Or am I missing something ?

Cheers,
Ben.

On Sun, Nov 28, 2010 at 4:38 PM, Benjamin Herrenschmidt <
benh@kernel.crashing.org> wrote:

> On Wed, 2010-09-08 at 09:41 -0700, Ira W. Snyder wrote:
> > This adds basic support for the system controller FPGA on the OVRO CARMA
> > board. This patch only adds infrastructure that will be used by later
> > drivers.
>
> Oh and another comment ...
>
> I'm not sure about drivers/fpga ... in the end, one would expect such a
> directory to contain stuff to manipulate FPGAs in the sense of
> downloading bitfiles, instanciating devices (device-tree manipulation ?)
> etc...
>
> From what I see in your code, the fact that these are FPGAs is almost
> irrelevant, you are providing support for "carma" devices, and such are
> no different than some other platform device, they just happen to be
> implemented as FPGAs. Or am I missing something ?
>
> Cheers,
> Ben.
>
>
Generally, I agree with this: There doesn't seem to be anything generic
about
the FPGA support you are adding, it seems very specific to the CARMA
hardware.

I'm generally not opposed to drivers/fpga: I think that colocating drivers
for things
that are basically FPGA compute platforms isn't necessarily a bad idea, but
I think it would be better if those devices were colocated because they
shared
some FPGA-oriented infrastructure, which this doesn't seem to do.
Currently,
generic infrastructure has been going into drivers/char, and drivers
themselves have
been going wherever they seem best.  Really what you have is 2 character
devices: one
for configuration, and one for access.

Steve

On Mon, Nov 29, 2010 at 11:38:14AM +1100, Benjamin Herrenschmidt wrote:
> On Wed, 2010-09-08 at 09:41 -0700, Ira W. Snyder wrote:
> > This adds basic support for the system controller FPGA on the OVRO CARMA
> > board. This patch only adds infrastructure that will be used by later
> > drivers.
> 
> Oh and another comment ...
> 
> I'm not sure about drivers/fpga ... in the end, one would expect such a
> directory to contain stuff to manipulate FPGAs in the sense of
> downloading bitfiles, instanciating devices (device-tree manipulation ?)
> etc...
> 
> From what I see in your code, the fact that these are FPGAs is almost
> irrelevant, you are providing support for "carma" devices, and such are
> no different than some other platform device, they just happen to be
> implemented as FPGAs. Or am I missing something ?
> 

You are exactly right. They are just regular platform devices. One
devices does happen to be a bitfile downloading driver
(carma-fpga-program), but it does not create any generic infrastructure
for downloading bitfiles.

Regarding your earlier comment about the carma class: no, it isn't
necessary. I found it convenient to have everything related to this
hardware appear in /sys/class/carma/, nothing more. It just wasn't as
easy to remember something like:
/sys/bus/platform/devices/f0000000.carma-fpga/.

I was thinking about changing the drivers from generic char devices into
misc devices instead. The sysfs interface would move from
/sys/class/carma/carma-fpga to /sys/class/misc/carma-fpga (for example),
but that is easy enough to remember.

Rather than putting the source code in drivers/fpga/carma, what about
drivers/misc/carma instead? I've already done that in my local tree, and
I'm much happier with the result.

Thanks for the comments.

Ira