[RFC] FPGA: add a simple framework for firmware programming

[RFC] FPGA: add a simple framework for firmware programming

[Juergen Beisert]

This is just a simple API to be used within C programs or via a shell command
to be able to program firmware into an attached FPGA. To hide the details how
to deal with the specific FPGA a handler register itself which does the real
work.
All users of this API just have to provide a handler identifier and the
firmware file.
As an example I added a firmware handler for an ALTERA FPGA connected via SPI
as its programming interface.

Comments are welcome.

Juergen


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/3] FPGA: add a simple programming handler framework

[Juergen Beisert]

This framework handles a list of registered FPGA programming handlers
to unify a firmware programming interface by hiding the details how
to program a specific FPGA in its handler.

Signed-off-by: Juergen Beisert <jbe@pengutronix.de>
---
 common/Kconfig    |   3 ++
 common/Makefile   |   1 +
 common/fpgamgr.c  | 122 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/fpgamgr.h |  58 ++++++++++++++++++++++++++
 4 files changed, 184 insertions(+)
 create mode 100644 common/fpgamgr.c
 create mode 100644 include/fpgamgr.h

diff --git a/common/Kconfig b/common/Kconfig
index ccfbc80..d87e59b 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -297,6 +297,9 @@ config MAXARGS
 	prompt "max. Number of arguments accepted for monitor commands"
 	default 16
 
+config FPGAMANAGER
+	bool
+
 choice
 	prompt "Select your shell"
 
diff --git a/common/Makefile b/common/Makefile
index 6f6e360..537dea4 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -48,6 +48,7 @@ obj-y += bootsource.o
 obj-$(CONFIG_BOOTM) += bootm.o
 extra-$(CONFIG_MODULES) += module.lds
 extra-y += barebox_default_env barebox_default_env.h
+obj-$(CONFIG_FPGAMANAGER) += fpgamgr.o
 
 ifdef CONFIG_DEFAULT_ENVIRONMENT
 $(obj)/startup.o: $(obj)/barebox_default_env.h
diff --git a/common/fpgamgr.c b/common/fpgamgr.c
new file mode 100644
index 0000000..4e36330
--- /dev/null
+++ b/common/fpgamgr.c
@@ -0,0 +1,122 @@
+/*
+ * Copyright (c) 2013 Juergen Beisert <kernel@pengutronix.de>, Pengutronix
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that 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.
+ */
+
+#include <common.h>
+#include <fpgamgr.h>
+#include <linux/list.h>
+#include <xfuncs.h>
+#include <malloc.h>
+
+struct fpga_mgr {
+	struct list_head list;
+	struct fpga_handler *handler; /* the program handler */
+	unsigned index;
+	unsigned flags;
+};
+
+static LIST_HEAD(fpgamgr_prog_handlers);
+
+static struct fpga_mgr *fpgamgr_init(void)
+{
+	struct fpga_mgr *handler;
+
+	handler = xzalloc(sizeof(struct fpga_mgr));
+	return handler;
+}
+
+struct fpga_mgr *fpgamgr_find_handler(const char *name, int index)
+{
+	struct fpga_mgr *mgr;
+
+	if (!name)
+		return -EINVAL;
+
+	list_for_each_entry(mgr, &fpgamgr_prog_handlers, list) {
+		if (!strcmp(mgr->handler->name, name)) {
+			if (index == -1 || index == mgr->index)
+				return mgr;
+		}
+	}
+
+	return NULL;
+}
+
+void fpgamgr_handlers_list(void)
+{
+	struct fpga_mgr *mgr;
+
+	if (list_empty(&fpgamgr_prog_handlers))
+		printf("(none)\n");
+
+	list_for_each_entry(mgr, &fpgamgr_prog_handlers, list)
+		printf("%s%-11s idx %u\n",
+				mgr->flags & FPGAMGR_HANDLER_FLAG_DEFAULT ?
+				"* " : "  ", mgr->handler->name, mgr->index);
+}
+
+/* make the device accessible to the public via its handler */
+int fpgamgr_register_handler(struct fpga_handler *h)
+{
+	struct fpga_mgr *mgr;
+	int index = 0;
+
+	if (fpgamgr_find_handler(h->name, -1) != 0) {
+		/* find the next free index for this name */
+		do
+			index++;
+		while (fpgamgr_find_handler(h->name, index) != NULL);
+	}
+
+	mgr = fpgamgr_init();
+	mgr->handler = h;
+	mgr->index = index;
+
+	list_add_tail(&mgr->list, &fpgamgr_prog_handlers);
+
+	return 0;
+}
+
+int fpgamgr_open_fpga(struct fpga_mgr *m)
+{
+	struct fpga_handler *h = m->handler;
+
+	if (h->open)
+		return h->open(h);
+
+	return -ENOSYS;
+}
+
+/*
+ * Expectation is 'cnt' in bytes and it *must* be a multiple of 8 bytes.
+ * Only the last call prior calling fpgamgr_close_fpga() can violate
+ * this rule.
+ */
+int fpgamgr_prog_fpga(struct fpga_mgr *m, const void *data, size_t cnt)
+{
+	struct fpga_handler *h = m->handler;
+
+	if (h->write)
+		return h->write(h, data, cnt);
+
+	return -ENOSYS;
+}
+
+int fpgamgr_close_fpga(struct fpga_mgr *m)
+{
+	struct fpga_handler *h = m->handler;
+
+	if (h->close)
+		return h->close(h);
+
+	return -ENOSYS;
+}
diff --git a/include/fpgamgr.h b/include/fpgamgr.h
new file mode 100644
index 0000000..8f94998
--- /dev/null
+++ b/include/fpgamgr.h
@@ -0,0 +1,58 @@
+/*
+ * Copyright (c) 2013 Juergen Beisert <kernel@pengutronix.de>, Pengutronix
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that 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.
+ */
+
+#include <types.h>
+
+struct fpga_handler {
+	const char *name; /* the name of the FPGA */
+	void *private_data; /* handler's private data */
+	/* called once to prepare the FPGA's programming cycle */
+	int (*open)(struct fpga_handler*);
+	/* called multiple times to program the FPGA with the given data */
+	int (*write)(struct fpga_handler*, const void*, size_t);
+	/* called once to finish programming cycle */
+	int (*close)(struct fpga_handler*);
+};
+
+struct fpga_mgr;
+
+int fpgamgr_register_handler(struct fpga_handler *);
+
+/* second argument can be -1 to be ignored */
+struct fpga_mgr *fpgamgr_find_handler(const char *, int);
+
+/*
+ * returns:
+ * 0 success
+ * -EIO preparing the FPGA for programming has failed
+ * -ENOSYS no such function available
+ */
+int fpgamgr_open_fpga(struct fpga_mgr *);
+
+/*
+ * returns:
+ * 0 success
+ * -EIO programming the FPGA has failed
+ * -ENOSYS no such function available
+ */
+int fpgamgr_prog_fpga(struct fpga_mgr *, const void *, size_t);
+
+/*
+ * returns:
+ * 0 success
+ * -EIO program cycle hasn't finished successfully
+ * -ENOSYS no such function available
+ */
+int fpgamgr_close_fpga(struct fpga_mgr *);
+
+void fpgamgr_handlers_list(void);
-- 
1.8.4.rc3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/3] FPGA: add a simple programming handler framework

[Alexander Shiyan]

> This framework handles a list of registered FPGA programming handlers
> to unify a firmware programming interface by hiding the details how
> to program a specific FPGA in its handler.
> 
> Signed-off-by: Juergen Beisert <jbe@pengutronix.de>

Seems this can be used not only for FPGA, so I suggest remove all FPGA_-prefix in
this framework. Maybe "firmware" is more flexible name here?

---
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/3] FPGA: add a programming command

[Juergen Beisert]

This command is a simple frontend to the FPGA programming handler manager.

Signed-off-by: Juergen Beisert <jbe@pengutronix.de>
---
 commands/Kconfig    |  10 +++++
 commands/Makefile   |   1 +
 commands/fpgaload.c | 117 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 128 insertions(+)
 create mode 100644 commands/fpgaload.c

diff --git a/commands/Kconfig b/commands/Kconfig
index 9738ec4..bb4ccaf 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -605,6 +605,16 @@ config CMD_BAREBOX_UPDATE
 	select BAREBOX_UPDATE
 	prompt "barebox-update"
 
+config CMD_FPGALOAD
+	bool
+	select FPGAMANAGER
+	prompt "fpgaload"
+	help
+	  Provides the "fpgaload" command which deals with FPGA firmware to
+	  download it into an FPGA device. This command uses the FPGA manager
+	  framework to hide the details about how program a specific FPGA
+	  device.
+
 config CMD_TIMEOUT
 	tristate
 	prompt "timeout"
diff --git a/commands/Makefile b/commands/Makefile
index 58d27fa..864ca0c 100644
--- a/commands/Makefile
+++ b/commands/Makefile
@@ -93,3 +93,4 @@ obj-$(CONFIG_CMD_MIITOOL)	+= miitool.o
 obj-$(CONFIG_CMD_DETECT)	+= detect.o
 obj-$(CONFIG_CMD_BOOT)		+= boot.o
 obj-$(CONFIG_CMD_DEVINFO)	+= devinfo.o
+obj-$(CONFIG_CMD_FPGALOAD)	+= fpgaload.o
diff --git a/commands/fpgaload.c b/commands/fpgaload.c
new file mode 100644
index 0000000..677ff73
--- /dev/null
+++ b/commands/fpgaload.c
@@ -0,0 +1,117 @@
+/*
+ * Copyright (c) 2013 Juergen Beisert <kernel@pengutronix.de>, Pengutronix
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that 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.
+ */
+
+#include <common.h>
+#include <command.h>
+#include <getopt.h>
+#include <fpgamgr.h>
+#include <fs.h>
+#include <fcntl.h>
+#include <linux/stat.h>
+
+static int fpgaload_write_data(struct fpga_mgr *mgr, const char *firmware)
+{
+	int fd, ret;
+	struct stat s;
+	size_t sz;
+	unsigned char buffer[256]; /* must always be a multiple of 8 bytes! */
+
+	ret = stat(firmware, &s);
+	if (ret != 0) {
+		printf("Unable to access file '%s'\n", firmware);
+		return -EINVAL;
+	}
+
+	fd = open(firmware, O_RDONLY);
+	if (fd < 0)
+		return fd;
+
+	do {
+		sz = read(fd, buffer, sizeof(buffer));
+		if (sz == 0)
+			break;
+		ret = fpgamgr_prog_fpga(mgr, buffer, sz);
+		if (ret < 0)
+			break;
+	} while (1);
+
+	close(fd);
+	return 0;
+}
+
+static int do_fpgaload(int argc, char *argv[])
+{
+	int ret, opt, index = -1;
+	const char *name = NULL, *firmware;
+	struct fpga_mgr *mgr;
+
+	while ((opt = getopt(argc, argv, "t:i:l")) > 0) {
+		switch (opt) {
+		case 't':
+			name = optarg;
+			break;
+		case 'i':
+			index = simple_strtoul(optarg, NULL, 0);
+			break;
+		case 'l':
+			printf("registered programming handlers:\n");
+			fpgamgr_handlers_list();
+			return 0;
+		default:
+			return COMMAND_ERROR_USAGE;
+		}
+	}
+
+	if (!(argc - optind))
+		return COMMAND_ERROR_USAGE;
+
+	firmware = argv[optind];
+
+	mgr = fpgamgr_find_handler(name, index);
+	if (mgr == NULL) {
+		printf("No such programming handler found\n");
+		return 1;
+	}
+
+	ret = fpgamgr_open_fpga(mgr);
+	if (ret == -ENOSYS) {
+		/* this might be a bug... */
+		pr_debug("No programming initiater function defined\n");
+	}
+
+	ret = fpgaload_write_data(mgr, firmware);
+	if (ret != 0)
+		return 1;
+
+	ret = fpgamgr_close_fpga(mgr);
+	if (ret == -ENOSYS) {
+		/* this might be a bug... */
+		pr_debug("No programming finisher function defined\n");
+	}
+
+	return 0;
+}
+
+BAREBOX_CMD_HELP_START(fpgaload)
+BAREBOX_CMD_HELP_USAGE("fpgaload [OPTIONS] <firmware>\n")
+BAREBOX_CMD_HELP_SHORT("Program a firmware file content into an FPGA\n")
+BAREBOX_CMD_HELP_OPT("-t <target>", "define the FPGA handler by name\n")
+BAREBOX_CMD_HELP_OPT("-i <index>", "define the FPGA handler by index\n")
+BAREBOX_CMD_HELP_OPT("-l\t", "list registered FPGAs\n")
+BAREBOX_CMD_HELP_END
+
+BAREBOX_CMD_START(fpgaload)
+	.cmd = do_fpgaload,
+	.usage = "program an FPGA",
+	BAREBOX_CMD_HELP(cmd_fpgaload_help)
+BAREBOX_CMD_END
-- 
1.8.4.rc3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [SPAM] [PATCH 2/3] FPGA: add a programming command

[Jean-Christophe PLAGNIOL-VILLARD]

Hi,

	I really do not like it

	we need to have an API to load firmware same a Linux

	and then provide the file name to the dev via params

Best Regards,
J.
On 15:24 Wed 06 Nov     , Juergen Beisert wrote:
> This command is a simple frontend to the FPGA programming handler manager.
> 
> Signed-off-by: Juergen Beisert <jbe@pengutronix.de>
> ---
>  commands/Kconfig    |  10 +++++
>  commands/Makefile   |   1 +
>  commands/fpgaload.c | 117 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 128 insertions(+)
>  create mode 100644 commands/fpgaload.c
> 
> diff --git a/commands/Kconfig b/commands/Kconfig
> index 9738ec4..bb4ccaf 100644
> --- a/commands/Kconfig
> +++ b/commands/Kconfig
> @@ -605,6 +605,16 @@ config CMD_BAREBOX_UPDATE
>  	select BAREBOX_UPDATE
>  	prompt "barebox-update"
>  
> +config CMD_FPGALOAD
> +	bool
> +	select FPGAMANAGER
> +	prompt "fpgaload"
> +	help
> +	  Provides the "fpgaload" command which deals with FPGA firmware to
> +	  download it into an FPGA device. This command uses the FPGA manager
> +	  framework to hide the details about how program a specific FPGA
> +	  device.
> +
>  config CMD_TIMEOUT
>  	tristate
>  	prompt "timeout"
> diff --git a/commands/Makefile b/commands/Makefile
> index 58d27fa..864ca0c 100644
> --- a/commands/Makefile
> +++ b/commands/Makefile
> @@ -93,3 +93,4 @@ obj-$(CONFIG_CMD_MIITOOL)	+= miitool.o
>  obj-$(CONFIG_CMD_DETECT)	+= detect.o
>  obj-$(CONFIG_CMD_BOOT)		+= boot.o
>  obj-$(CONFIG_CMD_DEVINFO)	+= devinfo.o
> +obj-$(CONFIG_CMD_FPGALOAD)	+= fpgaload.o
> diff --git a/commands/fpgaload.c b/commands/fpgaload.c
> new file mode 100644
> index 0000000..677ff73
> --- /dev/null
> +++ b/commands/fpgaload.c
> @@ -0,0 +1,117 @@
> +/*
> + * Copyright (c) 2013 Juergen Beisert <kernel@pengutronix.de>, Pengutronix
> + *
> + * 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.
> + *
> + * This program is distributed in the hope that 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.
> + */
> +
> +#include <common.h>
> +#include <command.h>
> +#include <getopt.h>
> +#include <fpgamgr.h>
> +#include <fs.h>
> +#include <fcntl.h>
> +#include <linux/stat.h>
> +
> +static int fpgaload_write_data(struct fpga_mgr *mgr, const char *firmware)
> +{
> +	int fd, ret;
> +	struct stat s;
> +	size_t sz;
> +	unsigned char buffer[256]; /* must always be a multiple of 8 bytes! */
> +
> +	ret = stat(firmware, &s);
> +	if (ret != 0) {
> +		printf("Unable to access file '%s'\n", firmware);
> +		return -EINVAL;
> +	}
> +
> +	fd = open(firmware, O_RDONLY);
> +	if (fd < 0)
> +		return fd;
> +
> +	do {
> +		sz = read(fd, buffer, sizeof(buffer));
> +		if (sz == 0)
> +			break;
> +		ret = fpgamgr_prog_fpga(mgr, buffer, sz);
> +		if (ret < 0)
> +			break;
> +	} while (1);
> +
> +	close(fd);
> +	return 0;
> +}
> +
> +static int do_fpgaload(int argc, char *argv[])
> +{
> +	int ret, opt, index = -1;
> +	const char *name = NULL, *firmware;
> +	struct fpga_mgr *mgr;
> +
> +	while ((opt = getopt(argc, argv, "t:i:l")) > 0) {
> +		switch (opt) {
> +		case 't':
> +			name = optarg;
> +			break;
> +		case 'i':
> +			index = simple_strtoul(optarg, NULL, 0);
> +			break;
> +		case 'l':
> +			printf("registered programming handlers:\n");
> +			fpgamgr_handlers_list();
> +			return 0;
> +		default:
> +			return COMMAND_ERROR_USAGE;
> +		}
> +	}
> +
> +	if (!(argc - optind))
> +		return COMMAND_ERROR_USAGE;
> +
> +	firmware = argv[optind];
> +
> +	mgr = fpgamgr_find_handler(name, index);
> +	if (mgr == NULL) {
> +		printf("No such programming handler found\n");
> +		return 1;
> +	}
> +
> +	ret = fpgamgr_open_fpga(mgr);
> +	if (ret == -ENOSYS) {
> +		/* this might be a bug... */
> +		pr_debug("No programming initiater function defined\n");
> +	}
> +
> +	ret = fpgaload_write_data(mgr, firmware);
> +	if (ret != 0)
> +		return 1;
> +
> +	ret = fpgamgr_close_fpga(mgr);
> +	if (ret == -ENOSYS) {
> +		/* this might be a bug... */
> +		pr_debug("No programming finisher function defined\n");
> +	}
> +
> +	return 0;
> +}
> +
> +BAREBOX_CMD_HELP_START(fpgaload)
> +BAREBOX_CMD_HELP_USAGE("fpgaload [OPTIONS] <firmware>\n")
> +BAREBOX_CMD_HELP_SHORT("Program a firmware file content into an FPGA\n")
> +BAREBOX_CMD_HELP_OPT("-t <target>", "define the FPGA handler by name\n")
> +BAREBOX_CMD_HELP_OPT("-i <index>", "define the FPGA handler by index\n")
> +BAREBOX_CMD_HELP_OPT("-l\t", "list registered FPGAs\n")
> +BAREBOX_CMD_HELP_END
> +
> +BAREBOX_CMD_START(fpgaload)
> +	.cmd = do_fpgaload,
> +	.usage = "program an FPGA",
> +	BAREBOX_CMD_HELP(cmd_fpgaload_help)
> +BAREBOX_CMD_END
> -- 
> 1.8.4.rc3
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [SPAM] [PATCH 2/3] FPGA: add a programming command

[Sascha Hauer]

On Thu, Nov 07, 2013 at 11:04:56AM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> Hi,
> 
> 	I really do not like it
> 
> 	we need to have an API to load firmware same a Linux

The firmware loading mechanism in Linux is driven by the driver
requesting a firmware. This is appropriate for WiFi drivers which can't
continue without a firmware. For FPGAs which can be loaded, unloaded, or
even partially loaded, it's the user that should trigger firmware
loading, not the driver.

Also, in barebox a user should decide if and when a firmware is loaded.
We have cases where a single board requires different Firmwares
depending on bootstrapping. In this case You don't want to have fixed
firmware names.

So no, the Linux Firmware model is not suitable for barebox (it sucks
for Linux aswell in many cases).

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [SPAM] [PATCH 2/3] FPGA: add a programming command

[Jean-Christophe PLAGNIOL-VILLARD]

On 12:09 Thu 07 Nov     , Sascha Hauer wrote:
> On Thu, Nov 07, 2013 at 11:04:56AM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > Hi,
> > 
> > 	I really do not like it
> > 
> > 	we need to have an API to load firmware same a Linux
> 
> The firmware loading mechanism in Linux is driven by the driver
> requesting a firmware. This is appropriate for WiFi drivers which can't
> continue without a firmware. For FPGAs which can be loaded, unloaded, or
> even partially loaded, it's the user that should trigger firmware
> loading, not the driver.
> 
> Also, in barebox a user should decide if and when a firmware is loaded.
> We have cases where a single board requires different Firmwares
> depending on bootstrapping. In this case You don't want to have fixed
> firmware names.
> 
> So no, the Linux Firmware model is not suitable for barebox (it sucks
> for Linux aswell in many cases).

and command is horrible as you need to known the protocol which you do not
care

you just need to known the fpga device and firmware you want to use

then the fpga driver will handle

Best Regards,
J.
> 
> Sascha
> 
> -- 
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [SPAM] [PATCH 2/3] FPGA: add a programming command

[Lucas Stach]

Am Donnerstag, den 07.11.2013, 15:37 +0100 schrieb Jean-Christophe
PLAGNIOL-VILLARD:
> On 12:09 Thu 07 Nov     , Sascha Hauer wrote:
> > On Thu, Nov 07, 2013 at 11:04:56AM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > > Hi,
> > > 
> > > 	I really do not like it
> > > 
> > > 	we need to have an API to load firmware same a Linux
> > 
> > The firmware loading mechanism in Linux is driven by the driver
> > requesting a firmware. This is appropriate for WiFi drivers which can't
> > continue without a firmware. For FPGAs which can be loaded, unloaded, or
> > even partially loaded, it's the user that should trigger firmware
> > loading, not the driver.
> > 
> > Also, in barebox a user should decide if and when a firmware is loaded.
> > We have cases where a single board requires different Firmwares
> > depending on bootstrapping. In this case You don't want to have fixed
> > firmware names.
> > 
> > So no, the Linux Firmware model is not suitable for barebox (it sucks
> > for Linux aswell in many cases).
> 
> and command is horrible as you need to known the protocol which you do not
> care
> 
> you just need to known the fpga device and firmware you want to use
> 
> then the fpga driver will handle
> 
Did you take the time to read the patches?

From a user perspective you only specify which FPGA you want to program
and tell the command which firmware file to use. The protocol and other
lowlevel stuff is taken care of in the handler.

Regards,
Lucas
-- 
Pengutronix e.K.                           | Lucas Stach                 |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [SPAM] [PATCH 2/3] FPGA: add a programming command

[Jean-Christophe PLAGNIOL-VILLARD]

On Nov 7, 2013, at 10:37 PM, Lucas Stach <l.stach@pengutronix.de> wrote:

> Am Donnerstag, den 07.11.2013, 15:37 +0100 schrieb Jean-Christophe
> PLAGNIOL-VILLARD:
>> On 12:09 Thu 07 Nov     , Sascha Hauer wrote:
>>> On Thu, Nov 07, 2013 at 11:04:56AM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>>> Hi,
>>>> 
>>>> 	I really do not like it
>>>> 
>>>> 	we need to have an API to load firmware same a Linux
>>> 
>>> The firmware loading mechanism in Linux is driven by the driver
>>> requesting a firmware. This is appropriate for WiFi drivers which can't
>>> continue without a firmware. For FPGAs which can be loaded, unloaded, or
>>> even partially loaded, it's the user that should trigger firmware
>>> loading, not the driver.
>>> 
>>> Also, in barebox a user should decide if and when a firmware is loaded.
>>> We have cases where a single board requires different Firmwares
>>> depending on bootstrapping. In this case You don't want to have fixed
>>> firmware names.
>>> 
>>> So no, the Linux Firmware model is not suitable for barebox (it sucks
>>> for Linux aswell in many cases).
>> 
>> and command is horrible as you need to known the protocol which you do not
>> care
>> 
>> you just need to known the fpga device and firmware you want to use
>> 
>> then the fpga driver will handle
>> 
> Did you take the time to read the patches?
> 
> From a user perspective you only specify which FPGA you want to program
> and tell the command which firmware file to use. The protocol and other
> lowlevel stuff is taken care of in the handler.

yes I did but the issue is that you need to use a command instead just simply
set a parameter to the fpga device

that why I hate the idea of command you use the device to set the firmware you want

Best Regards,
J.

> 
> Regards,
> Lucas
> -- 
> Pengutronix e.K.                           | Lucas Stach                 |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [SPAM] [PATCH 2/3] FPGA: add a programming command

[Sascha Hauer]

On Thu, Nov 07, 2013 at 11:27:39PM +0800, Jean-Christophe PLAGNIOL-VILLARD wrote:
> 
> On Nov 7, 2013, at 10:37 PM, Lucas Stach <l.stach@pengutronix.de> wrote:
> 
> > Am Donnerstag, den 07.11.2013, 15:37 +0100 schrieb Jean-Christophe
> > PLAGNIOL-VILLARD:
> >> On 12:09 Thu 07 Nov     , Sascha Hauer wrote:
> >>> On Thu, Nov 07, 2013 at 11:04:56AM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> >>>> Hi,
> >>>> 
> >>>> 	I really do not like it
> >>>> 
> >>>> 	we need to have an API to load firmware same a Linux
> >>> 
> >>> The firmware loading mechanism in Linux is driven by the driver
> >>> requesting a firmware. This is appropriate for WiFi drivers which can't
> >>> continue without a firmware. For FPGAs which can be loaded, unloaded, or
> >>> even partially loaded, it's the user that should trigger firmware
> >>> loading, not the driver.
> >>> 
> >>> Also, in barebox a user should decide if and when a firmware is loaded.
> >>> We have cases where a single board requires different Firmwares
> >>> depending on bootstrapping. In this case You don't want to have fixed
> >>> firmware names.
> >>> 
> >>> So no, the Linux Firmware model is not suitable for barebox (it sucks
> >>> for Linux aswell in many cases).
> >> 
> >> and command is horrible as you need to known the protocol which you do not
> >> care
> >> 
> >> you just need to known the fpga device and firmware you want to use
> >> 
> >> then the fpga driver will handle
> >> 
> > Did you take the time to read the patches?
> > 
> > From a user perspective you only specify which FPGA you want to program
> > and tell the command which firmware file to use. The protocol and other
> > lowlevel stuff is taken care of in the handler.
> 
> yes I did but the issue is that you need to use a command instead just simply
> set a parameter to the fpga device
> 
> that why I hate the idea of command you use the device to set the firmware you want

I'm fine with a /dev/fpga.x interface to load firmware. However, I find
a command still useful. A command can list all available FPGAs, can
handle partitioned FPGAs and has better possibilities to react to
unusual situations. Right now Jürgens interface to the FPGA driver has
a open/write/close interface. Integrating a cdev into this should be
simple. Then time can decide whether a /dev/ interface or a command or
both is more useful.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 3/3] FPGA: provide a handler to program ALTERA FPGAs

[Juergen Beisert]

This handler uses a regular SPI master and a few GPIO to program an ALTERA FPGA
in serial mode.

Signed-off-by: Juergen Beisert <jbe@pengutronix.de>
---
 drivers/Kconfig              |   1 +
 drivers/Makefile             |   1 +
 drivers/fpga/Kconfig         |  10 ++
 drivers/fpga/Makefile        |   2 +
 drivers/fpga/altera_serial.c | 278 +++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 292 insertions(+)
 create mode 100644 drivers/fpga/Kconfig
 create mode 100644 drivers/fpga/Makefile
 create mode 100644 drivers/fpga/altera_serial.c

diff --git a/drivers/Kconfig b/drivers/Kconfig
index d34d2c7..ac3c699 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -25,5 +25,6 @@ source "drivers/gpio/Kconfig"
 source "drivers/w1/Kconfig"
 source "drivers/pinctrl/Kconfig"
 source "drivers/bus/Kconfig"
+source "drivers/fpga/Kconfig"
 
 endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index ba1dc6d..fa4aace 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -24,3 +24,4 @@ obj-$(CONFIG_OFTREE) += of/
 obj-$(CONFIG_W1) += w1/
 obj-y += pinctrl/
 obj-y += bus/
+obj-y += fpga/
diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
new file mode 100644
index 0000000..204e852
--- /dev/null
+++ b/drivers/fpga/Kconfig
@@ -0,0 +1,10 @@
+menu "FPGA programming"
+
+config ALTERA_SERIAL
+	bool "Altera SPI programming"
+	depends on FPGAMANAGER && OFDEVICE
+	help
+	  Programming an Altera FPGA via a few GPIOs for the control lines and
+	  MOSI, MISO and clock from an SPI interface for the data lines
+
+endmenu
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
new file mode 100644
index 0000000..3147634
--- /dev/null
+++ b/drivers/fpga/Makefile
@@ -0,0 +1,2 @@
+
+obj-$(CONFIG_ALTERA_SERIAL) += altera_serial.o
diff --git a/drivers/fpga/altera_serial.c b/drivers/fpga/altera_serial.c
new file mode 100644
index 0000000..cf05845
--- /dev/null
+++ b/drivers/fpga/altera_serial.c
@@ -0,0 +1,278 @@
+/*
+ * Copyright (c) 2013 Juergen Beisert <kernel@pengutronix.de>, Pengutronix
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that 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.
+ */
+#include <common.h>
+#include <init.h>
+#include <driver.h>
+#include <fpgamgr.h>
+#include <of_gpio.h>
+#include <xfuncs.h>
+#include <malloc.h>
+#include <gpio.h>
+#include <clock.h>
+#include <spi/spi.h>
+
+/*
+ * Physical requirements:
+ * - three free GPIOs for the signals nCONFIG, CONFIGURE_DONE, nSTATUS
+ * - 32 bit per word, LSB first capable SPI master (MOSI + clock)
+ *
+ * Example how to configure this driver via device tree
+ *
+ *	fpga@0 {
+ *		compatible = "altera_serial";
+ *		nstat-gpio = <&gpio4 18 0>;
+ *		confd-gpio = <&gpio4 19 0>;
+ *		nconfig-gpio = <&gpio4 20 0>;
+ *		spi-max-frequency = <10000000>;
+ *		reg = <0>;
+ *	};
+ */
+
+struct fpga_spi {
+	int nstat_gpio; /* input GPIO to read the status line */
+	int confd_gpio; /* input GPIO to read the config done line */
+	int nconfig_gpio; /* output GPIO to start the FPGA's config */
+	struct device_d *dev;
+	struct spi_device *spi;
+	bool padding_done;
+};
+
+static int altera_spi_open(struct fpga_handler *h)
+{
+	struct fpga_spi *this = (struct fpga_spi *)h->private_data;
+	struct device_d *dev = this->dev;
+	int ret;
+
+	dev_dbg(dev, "Initiating programming\n");
+
+	/* initiate an FPGA programming */
+	gpio_set_value(this->nconfig_gpio, 0);
+
+	/*
+	 * after about 2 µs the FPGA must acknowledge with
+	 * STATUS and CONFIG DONE lines at low level
+	 */
+	ret = wait_on_timeout(2 * 1000,
+				(gpio_get_value(this->nstat_gpio) == 0) &&
+				(gpio_get_value(this->confd_gpio) == 0));
+
+	if (ret != 0) {
+		dev_err(dev, "FPGA does not acknowledge the programming initiation\n");
+		if (gpio_get_value(this->nstat_gpio))
+			dev_err(dev, "STATUS is still high!\n");
+		if (gpio_get_value(this->confd_gpio))
+			dev_err(dev, "CONFIG DONE is still high!\n");
+		return ret;
+	}
+
+	/* arm the FPGA to await its new firmware */
+	gpio_set_value(this->nconfig_gpio, 1);
+
+	/* once again, we might need padding the data */
+	this->padding_done = false;
+
+	/*
+	 * after about 1506 µs the FPGA must acknowledge this step
+	 * with the STATUS line at high level
+	 */
+	ret = wait_on_timeout(1600 * 1000,
+				gpio_get_value(this->nstat_gpio) == 1);
+	if (ret != 0) {
+		dev_err(dev, "FPGA does not acknowledge the programming start\n");
+		return ret;
+	}
+
+	dev_dbg(dev, "Initiating passed\n");
+	/* at the end, wait at least 2 µs prior beginning writing data */
+	ndelay(2 * 1000);
+
+	return 0;
+}
+
+static int altera_spi_write(struct fpga_handler *h, const void *buf, size_t sz)
+{
+	struct fpga_spi *this = (struct fpga_spi *)h->private_data;
+	struct device_d *dev = this->dev;
+	struct spi_transfer t[2];
+	struct spi_message m;
+	u32 dummy;
+	int ret;
+
+	spi_message_init(&m);
+
+	if (sz < sizeof(u32)) {
+		/* simple padding */
+		dummy = 0;
+		memcpy(&dummy, buf, sz);
+		buf = &dummy;
+		sz = sizeof(u32);
+		this->padding_done = true;
+	}
+
+	t[0].tx_buf = buf;
+	t[0].rx_buf = NULL;
+	t[0].len = sz;
+	spi_message_add_tail(&t[0], &m);
+
+	if (sz & 0x3) { /* padding required? */
+		u32 *word_buf = (u32 *)buf;
+		dummy = 0;
+		memcpy(&dummy, &word_buf[sz >> 2], sz & 0x3);
+		t[0].len &= ~0x03;
+		t[1].tx_buf = &dummy;
+		t[1].rx_buf = NULL;
+		t[1].len = sizeof(u32);
+		spi_message_add_tail(&t[1], &m);
+		this->padding_done = true;
+	}
+
+	ret = spi_sync(this->spi, &m);
+	if (ret != 0)
+		dev_err(dev, "programming failure\n");
+
+	return ret;
+}
+
+static int altera_spi_close(struct fpga_handler *h)
+{
+	struct fpga_spi *this = (struct fpga_spi *)h->private_data;
+	struct device_d *dev = this->dev;
+	struct spi_transfer t;
+	struct spi_message m;
+	u32 dummy = 0;
+	int ret;
+
+	dev_dbg(dev, "Finalize programming\n");
+
+	if (this->padding_done == false) {
+		spi_message_init(&m);
+		t.tx_buf = &dummy;
+		t.rx_buf = NULL;
+		t.len = sizeof(dummy);
+		spi_message_add_tail(&t, &m);
+
+		ret = spi_sync(this->spi, &m);
+		if (ret != 0)
+			dev_err(dev, "programming failure\n");
+	}
+
+	/*
+	 * when programming was successfully,
+	 * both status lines should be at high level
+	 */
+	ret = wait_on_timeout(10 * 1000,
+				(gpio_get_value(this->nstat_gpio) == 1) &&
+				(gpio_get_value(this->confd_gpio) == 1));
+	if (ret == 0) {
+		dev_dbg(dev, "Programming successfull\n");
+		return ret;
+	}
+
+	dev_err(dev, "Programming failed due to time out\n");
+	if (gpio_get_value(this->nstat_gpio) == 0)
+		dev_err(dev, "STATUS is still low!\n");
+	if (gpio_get_value(this->confd_gpio) == 0)
+		dev_err(dev, "CONFIG DONE is still low!\n");
+
+	return -EIO;
+}
+
+static int altera_spi_of(struct device_d *dev, struct fpga_spi *this)
+{
+	struct device_node *n = dev->device_node;
+
+	this->nstat_gpio = of_get_named_gpio(n, "nstat-gpio", 0);
+	if (this->nstat_gpio < 0) {
+		dev_err(dev, "Illegal value for the STATUS input GPIO\n");
+		return this->nstat_gpio;
+	}
+	this->confd_gpio = of_get_named_gpio(n, "confd-gpio", 0);
+	if (this->confd_gpio < 0) {
+		dev_err(dev, "Illegal value for the CONFIG DONE input GPIO\n");
+		return this->confd_gpio;
+	}
+	this->nconfig_gpio = of_get_named_gpio(n, "nconfig-gpio", 0);
+	if (this->nconfig_gpio < 0) {
+		dev_err(dev, "Illegal value for the CONFIGURE output GPIO\n");
+		return this->nconfig_gpio;
+	}
+
+	/* init to passive and sane values */
+	gpio_direction_output(this->nconfig_gpio, 1);
+	gpio_direction_input(this->nstat_gpio);
+	gpio_direction_input(this->confd_gpio);
+	
+	return 0;
+}
+
+static void altera_spi_init_mode(struct spi_device *spi)
+{
+	spi->bits_per_word = 32;
+	/*
+	 * CPHA = CPOL = 0
+	 * the FPGA expects its firmware data with LSB first
+	 */
+	spi->mode = SPI_MODE_0 | SPI_LSB_FIRST;
+}
+
+static int altera_spi_probe(struct device_d *dev)
+{
+	int rc;
+	struct fpga_spi *this;
+	struct fpga_handler *h;
+	const char *alias = of_alias_get(dev->device_node);
+
+	dev_dbg(dev, "Probing FPGA firmware programmer\n");
+	h = xzalloc(sizeof(struct fpga_spi) + sizeof(struct fpga_handler));
+	this = (struct fpga_spi *)&h[1];
+	h->private_data = this;
+
+	rc = altera_spi_of(dev, this);
+	if (rc != 0)
+		return rc;
+
+	if (alias)
+		h->name = xstrdup(alias);
+	else
+		h->name = "fpga";
+
+	h->open = altera_spi_open;
+	h->write = altera_spi_write;
+	h->close = altera_spi_close;
+
+	this->spi = (struct spi_device *)dev->type_data;
+	altera_spi_init_mode(this->spi);
+	this->dev = dev;
+
+	dev_dbg(dev, "Registering FPGA firmware programmer\n");
+	rc = fpgamgr_register_handler(h);
+	if (rc != 0) {
+		free(h);
+		return rc;
+	}
+
+	return 0;
+}
+
+static struct of_device_id altera_spi_id_table[] = {
+	{
+		.compatible = "altera_serial",
+	},
+};
+
+static struct driver_d altera_spi_driver = {
+	.name = "fpga_spi",
+	.of_compatible = DRV_OF_COMPAT(altera_spi_id_table),
+	.probe = altera_spi_probe,
+};
+device_spi_driver(altera_spi_driver);
-- 
1.8.4.rc3


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox