Skip to contentSkip to conversation

Altera Cyclone Passive Serial SPI FPGA Manager

PATCH In review Joshua Clayton 4 participants 9 messages Jan 24, 2017 – Feb 15, 2017

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

v9 cover Altera Cyclone Passive Serial SPI FPGA Manager

Quoted 1 line

From c429cec34a880c0d5438091f436a5946a4db3fc0 Mon Sep 17 00:00:00 2001

Message-Id: <[email protected]>
From: Joshua Clayton <[email protected]>
Date: Mon, 23 Jan 2017 14:43:20 -0800
Subject: [PATCH v8 0/3] Altera Cyclone Passive Serial SPI FPGA Manager

This series adds an FPGA manager for Altera cyclone FPGAs that can program them using an spi port and a couple of gpios, using Alteras passive serial protocol.

Changes from v8:
- List the confd pin as optional (it is an additional way to confirm the FPGA is done loading, but is not accessible in my hardare)

Changes from v7:
Add Rob Herrings Ack on the bindings change (Thanks!) Change const u8* to const char * to make sparse happy

Changes from v6:
- moved bitrev8x4() out (I'll submit it in a separate patch set)
- Changed the dts format to match what is already done in barebox
(see https://git.pengutronix.de/cgit/barebox/tree/Documentation/devicetree/bindings/firmware/altr,passive-serial.txt)
- Changed error message from "Status pin should be low" to "Status
pin failed to show a reset" for better clarity
- Fixed any whitespace problems that had crept in.

Changes from v5:
- Rebased on next-20161214xi
- Corrected for FPGA Mgr API change in write_init() and write_complete()
- Better describe the device cyclone-ps-spi runs on in the file header.
- Split the bitrev8x4 patch into generic and arch specific patches...
- Added AARCH64 and MIPS implementations of bitrev8x4()... they all have to have an implementation for it to compile cleanly across platforms
- Added the changes to imx6q-evi.dts to the patch set.

Changes from v4:
- Added the needed return statement to __arch_bitrev8x4()
- Added Rob Herrings ACK for and fix a typo in the commit log of patch 2

Changes from v3:
- Fixed up the state() function to return the state of the status pin reqested by Alan Tull
- Switched the pin to ACTIVE_LOW and coresponding logic level, and updated the corresponding documentation. Thanks Rob Herring for pointing out my mistake.
- Per Rob Herring, switched from "gpio" to "gpios" in dts

Changes from v2:
- Merged patch 3 and 4 as suggested in review by Moritz Fischer
- Changed FPGA_MIN_DELAY from 250 to 50 ms is the time advertized by Altera. This now works, as we don't assume it is done

Changes from v1:
- Changed the name from cyclone-spi-fpga-mgr to cyclone-ps-spi-fpga-mgr This name change was requested by Alan Tull, to be specific about which programming method is being employed on the fpga.
- Changed the name of the reset-gpio to config-gpio to closer match the way the pins are described in the Altera manual
- Moved MODULE_LICENCE, _AUTHOR, and _DESCRIPTION to the bottom

- Added a bitrev8x4() function to the bitrev headers and implemented ARM const, runtime, and ARM specific faster versions (This may end up needing to be a standalone patch)

- Moved the bitswapping into cyclonespi_write(), as requested. This falls short of my desired generic lsb first spi support, but is a step in that direction.

- Fixed whitespace problems introduced during refactoring

- Replaced magic number for initial delay with a descriptive macro
- Poll the fpga to see when it is ready rather than a fixed 1 ms sleep

*** BLURB HERE ***

Joshua Clayton (3):
doc: dt: add cyclone-ps-spi binding document fpga manager: Add cyclone-ps-spi driver for Altera FPGAs
ARM: dts: imx6q-evi: support cyclone-ps-spi

 .../bindings/fpga/altera-passive-serial.txt        |  29 ++++
 arch/arm/boot/dts/imx6q-evi.dts                    |  16 ++
 drivers/fpga/Kconfig                               |   7 +
 drivers/fpga/Makefile                              |   1 +
 drivers/fpga/cyclone-ps-spi.c                      | 185 +++++++++++++++++++++
 5 files changed, 238 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/fpga/altera-passive-serial.txt
 create mode 100644 drivers/fpga/cyclone-ps-spi.c
Signature · 1 line
2.9.3
#1 lore raw

PATCH v9 1/3 doc: dt: add cyclone-ps-spi binding document

Describe a cyclone-ps-spi devicetree entry, required features

---
 .../bindings/fpga/altera-passive-serial.txt        | 29 ++++++++++++++++++++++
 1 file changed, 29 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/fpga/altera-passive-serial.txt
Documentation/devicetree/bindings/fpga/altera-passive-serial.txt new file +29 -0
new file mode 100644
index 0000000..774e6a7
@@ -0,0 +1,29 @@
+Altera Cyclone Passive Serial SPI FPGA Manager
+
+Altera Cyclone FPGAs support a method of loading the bitstream over what is
+referred to as "passive serial".
+The passive serial link is not technically spi, and might require extra
+circuits in order to play nicely with other spi slaves on the same bus.
+
+See https://www.altera.com/literature/hb/cyc/cyc_c51013.pdf
+
+Required properties:
+- compatible: should contain "altr,fpga-passive-serial"
+- reg: spi chip select of the FPGA
+- nconfig-gpios: config pin (referred to as nCONFIG in the cyclone manual)
+- nstat-gpios: status pin (referred to as nSTATUS in the cyclone manual)
+
+Optional properties:
+- confd-gpios: confd pin (referred to as CONF_DONE in the cyclone manual)
+
+
+
+Example:
+	fpga_spi: evi-fpga-spi at 0 {
+		compatible = "altr,cyclone-ps-spi-fpga-mgr";
+		spi-max-frequency = <20000000>;
+		reg = <0>;
+		nconfig-gpios = <&gpio4 9 GPIO_ACTIVE_LOW>;
+		nstat-gpios = <&gpio4 11 GPIO_ACTIVE_LOW>;
+		confd-gpios = <&gpio4 12 GPIO_ACTIVE_LOW>;
+	};
Signature · 1 line
2.9.3
maintainer Acked-by on v9 1/3
#2 lore raw

Re: [PATCH v9 1/3] doc: dt: add cyclone-ps-spi binding document

Quoted 7 lines · Joshua Clayton

Describe a cyclone-ps-spi devicetree entry, required features

---
 .../bindings/fpga/altera-passive-serial.txt        | 29 ++++++++++++++++++++++
 1 file changed, 29 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/fpga/altera-passive-serial.txt

This is a minor change, you could have kept my ack.

#3 lore raw

Re: [PATCH v9 1/3] doc: dt: add cyclone-ps-spi binding document

Thanks, Rob.

Quoted 11 lines · Rob Herring
Quoted 7 lines · Joshua Clayton

Describe a cyclone-ps-spi devicetree entry, required features

---
 .../bindings/fpga/altera-passive-serial.txt        | 29 ++++++++++++++++++++++
 1 file changed, 29 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/fpga/altera-passive-serial.txt

This is a minor change, you could have kept my ack.

I know you have a lot of these to wade through.
I'll remember that going forward.

#4 lore raw

PATCH v9 2/3 fpga manager: Add cyclone-ps-spi driver for Altera FPGAs

In reply to Joshua Clayton

cyclone-ps-spi loads FPGA firmware over spi, using the "passive serial" interface on Altera Cyclone FPGAS.

This is one of the simpler ways to set up an FPGA at runtime. The signal interface is close to unidirectional spi with lsb first.

---
 drivers/fpga/Kconfig          |   7 ++
 drivers/fpga/Makefile         |   1 +
 drivers/fpga/cyclone-ps-spi.c | 185 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 193 insertions(+)
 create mode 100644 drivers/fpga/cyclone-ps-spi.c
drivers/fpga/Kconfig +7 -0
index ce861a2..e6c032d 100644
@@ -20,6 +20,13 @@ config FPGA_REGION
 	  FPGA Regions allow loading FPGA images under control of
 	  the Device Tree.
 
+config FPGA_MGR_CYCLONE_PS_SPI
+	tristate "Altera Cyclone FPGA Passive Serial over SPI"
+	depends on SPI
+	help
+	  FPGA manager driver support for Altera Cyclone using the
+	  passive serial interface over SPI
+
 config FPGA_MGR_SOCFPGA
 	tristate "Altera SOCFPGA FPGA Manager"
 	depends on ARCH_SOCFPGA || COMPILE_TEST
drivers/fpga/Makefile +1 -0
index 8df07bc..a112bef 100644
@@ -6,6 +6,7 @@
 obj-$(CONFIG_FPGA)			+= fpga-mgr.o
 
 # FPGA Manager Drivers
+obj-$(CONFIG_FPGA_MGR_CYCLONE_PS_SPI)	+= cyclone-ps-spi.o
 obj-$(CONFIG_FPGA_MGR_SOCFPGA)		+= socfpga.o
 obj-$(CONFIG_FPGA_MGR_SOCFPGA_A10)	+= socfpga-a10.o
 obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA)	+= zynq-fpga.o
drivers/fpga/cyclone-ps-spi.c new file +185 -0

185 lines not on this page · raw message

Signature · 1 line
2.9.3
#5 lore raw

Re: [PATCH v9 2/3] fpga manager: Add cyclone-ps-spi driver for Altera FPGAs

Hi Joshua,

On Tue, 24 Jan 2017 14:19:33 -0800
Joshua Clayton stillcompiling at gmail.com wrote:
...

drivers/fpga/cyclone-ps-spi.c line 103

Quoted 12 lines
drivers/fpga/cyclone-ps-spi.c new file +12 -0
+static int cyclonespi_write(struct fpga_manager *mgr, const char *buf,
+			    size_t count)
+{
+	struct cyclonespi_conf *conf = (struct cyclonespi_conf *)mgr->priv;
+	const char *fw_data = buf;
+	const char *fw_data_end = fw_data + count;
+
+	while (fw_data < fw_data_end) {
+		int ret;
+		size_t stride = min(fw_data_end - fw_data, SZ_4K);
+
+		rev_buf(fw_data, stride);

Can we make rev_buf() optional here? I've tested this driver with Stratix-V FPGA using orion_spi driver. It works without swapping the data.

Thanks,
Anatolij

#6 lore raw

Re: [PATCH v9 2/3] fpga manager: Add cyclone-ps-spi driver for Altera FPGAs

Quoted 5 lines · Anatolij Gustschin

Hi Joshua,

On Tue, 24 Jan 2017 14:19:33 -0800
Joshua Clayton stillcompiling at gmail.com wrote:
...

Quoted 16 lines

drivers/fpga/cyclone-ps-spi.c line 103

Quoted 12 lines
drivers/fpga/cyclone-ps-spi.c new file +12 -0
+static int cyclonespi_write(struct fpga_manager *mgr, const char *buf,
+                          size_t count)
+{
+      struct cyclonespi_conf *conf = (struct cyclonespi_conf *)mgr->priv;
+      const char *fw_data = buf;
+      const char *fw_data_end = fw_data + count;
+
+      while (fw_data < fw_data_end) {
+              int ret;
+              size_t stride = min(fw_data_end - fw_data, SZ_4K);
+
+              rev_buf(fw_data, stride);

Can we make rev_buf() optional here? I've tested this driver with Stratix-V FPGA using orion_spi driver. It works without swapping the data.

I replied earlier, but something in my email settings bounced.

The way to make it optional is to add a bit to the flags of the fpga_image_info struct.

Alan

Quoted 3 lines

Thanks,
Anatolij

#7 lore raw

Re: [PATCH v9 2/3] fpga manager: Add cyclone-ps-spi driver for Altera FPGAs

Quoted 6 lines · Alan Tull
Quoted 5 lines · Anatolij Gustschin

Hi Joshua,

On Tue, 24 Jan 2017 14:19:33 -0800
Joshua Clayton stillcompiling at gmail.com wrote:
...

Quoted 15 lines
Quoted 7 lines
Quoted 7 lines
drivers/fpga/cyclone-ps-spi.c new file +7 -0
+static int cyclonespi_write(struct fpga_manager *mgr, const char *buf,
+                          size_t count)
+{
+      struct cyclonespi_conf *conf = (struct cyclonespi_conf *)mgr->priv;
+      const char *fw_data = buf;
+      const char *fw_data_end = fw_data + count;
+
Quoted 8 lines

drivers/fpga/cyclone-ps-spi.c line 103

Quoted 5 lines
drivers/fpga/cyclone-ps-spi.c new file +5 -0
+      while (fw_data < fw_data_end) {
+              int ret;
+              size_t stride = min(fw_data_end - fw_data, SZ_4K);
+
+              rev_buf(fw_data, stride);

Can we make rev_buf() optional here? I've tested this driver with Stratix-V FPGA using orion_spi driver. It works without swapping the data.

Thanks for testing!
I'd be happy to make it an optional feature, rather than integral.

Quoted 6 lines

I replied earlier, but something in my email settings bounced.

The way to make it optional is to add a bit to the flags of the fpga_image_info struct.

Alan

OK.

Quoted 3 lines
Quoted 3 lines

Thanks,
Anatolij

#8 lore raw

PATCH v9 3/3 ARM: dts: imx6q-evi: support cyclone-ps-spi

In reply to Joshua Clayton

Add support for Altera cyclone V FPGA connected to an spi port to the evi devicetree file

---
 arch/arm/boot/dts/imx6q-evi.dts | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
arch/arm/boot/dts/imx6q-evi.dts +16 -0
index 79a0bd5..e531bf7 100644
@@ -83,6 +83,15 @@
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi1 &pinctrl_ecspi1cs>;
 	status = "okay";
+
+	fpga_spi: cyclonespi at 0 {
+		compatible = "altr,fpga-passive-serial";
+		spi-max-frequency = <20000000>;
+		reg = <0>;
+		pinctrl-0 = <&pinctrl_fpgaspi>;
+		nconfig-gpios = <&gpio4 9 GPIO_ACTIVE_LOW>;
+		nstat-gpios = <&gpio4 11 GPIO_ACTIVE_LOW>;
+	};
 };
 
 &ecspi3 {
@@ -316,6 +325,13 @@
 		>;
 	};
 
+	pinctrl_fpgaspi: fpgaspigrp {
+		fsl,pins = <
+			MX6QDL_PAD_KEY_ROW1__GPIO4_IO09 0x1b0b0
+			MX6QDL_PAD_KEY_ROW2__GPIO4_IO11 0x1b0b0
+		>;
+	};
+
 	pinctrl_gpminand: gpminandgrp {
 		fsl,pins = <
 			MX6QDL_PAD_NANDF_CLE__NAND_CLE 0xb0b1
Signature · 1 line
2.9.3