Altera Cyclone Passive Serial SPI FPGA Manager

Implement faster bitrev8x4() for arm, arm64 and mips, all the platforms with CONFIG_HAVE_ARCH_BITREVERSE.
ARM platforms just need a byteswap added to the existing __arch_bitrev32() Amusingly, the mips implementation is exactly the opposite, requiring removal of the byteswap from its __arch_bitrev32()

Signed-off-by Joshua Clayton <[email protected]>

---
 arch/arm/include/asm/bitrev.h   | 6 ++++++
 arch/arm64/include/asm/bitrev.h | 6 ++++++
 arch/mips/include/asm/bitrev.h  | 6 ++++++
 include/linux/bitrev.h          | 1 +
 4 files changed, 19 insertions(+)

arch/arm/include/asm/bitrev.h +6 -0

index ec291c3..9482f78 100644
@@ -17,4 +17,10 @@ static __always_inline __attribute_const__ u8 __arch_bitrev8(u8 x)
 	return __arch_bitrev32((u32)x) >> 24;
 }
 
+static __always_inline __attribute_const__ u32 __arch_bitrev8x4(u32 x)
+{
+	__asm__ ("rbit %0, %1; rev %0, %0" : "=r" (x) : "r" (x));
+	return x;
+}
+
 #endif

arch/arm64/include/asm/bitrev.h +0 -0

index a5a0c36..1801078 100644

Quoted 37 lines · Joshua Clayton

Implement faster bitrev8x4() for arm, arm64 and mips, all the platforms with CONFIG_HAVE_ARCH_BITREVERSE.
ARM platforms just need a byteswap added to the existing __arch_bitrev32() Amusingly, the mips implementation is exactly the opposite, requiring removal of the byteswap from its __arch_bitrev32()

Signed-off-by Joshua Clayton <[email protected]>

---
 arch/arm/include/asm/bitrev.h   | 6 ++++++
 arch/arm64/include/asm/bitrev.h | 6 ++++++
 arch/mips/include/asm/bitrev.h  | 6 ++++++
 include/linux/bitrev.h          | 1 +
 4 files changed, 19 insertions(+)

arch/arm/include/asm/bitrev.h +6 -0

index ec291c3..9482f78 100644
@@ -17,4 +17,10 @@ static __always_inline __attribute_const__ u8 __arch_bitrev8(u8 x)
 	return __arch_bitrev32((u32)x) >> 24;
 }
 
+static __always_inline __attribute_const__ u32 __arch_bitrev8x4(u32 x)
+{
+	__asm__ ("rbit %0, %1; rev %0, %0" : "=r" (x) : "r" (x));
+	return x;
+}
+
 #endif

arch/arm64/include/asm/bitrev.h +0 -0

index a5a0c36..1801078 100644
@@ -16,4 +16,10 @@ static __always_inline __attribute_const__ u8 __arch_bitrev8(u8 x)
 	return __arch_bitrev32((u32)x) >> 24;
 }
 

arch/arm64/include/asm/bitrev.h static __always_inline __attribute_const__ u8 __arch_bitrev8(u8 x) · line 21

Quoted 3 lines

arch/arm64/include/asm/bitrev.h +3 -0

+static __always_inline __attribute_const__ u32 __arch_bitrev8x4(u32 x)
+{
+	__asm__ ("rbit %0, %1; rev %0, %0" : "=r" (x) : "r" (x));

This is broken -- you're operating on 64-bit registers. I only noticed because if you write:

swab32(bitrev32(x))

then GCC generates:

rbit w0, w0
rev w0, w0

so perhaps we should just implement the asm-generic version like that and not bother with the arch-specific stuff?

Will

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

Signed-off-by Joshua Clayton <[email protected]>

Acked-by Rob Herring <[email protected]>

---
 .../bindings/fpga/cyclone-ps-spi-fpga-mgr.txt      | 25 ++++++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/fpga/cyclone-ps-spi-fpga-mgr.txt

Documentation/devicetree/bindings/fpga/cyclone-ps-spi-fpga-mgr.txt new file +25 -0

new file mode 100644
index 0000000..3f515c7
@@ -0,0 +1,25 @@
+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,cyclone-ps-spi-fpga-mgr"
+- reg         : spi slave id of the fpga
+- config-gpios : config pin (referred to as nCONFIG in the cyclone manual)
+- status-gpios : status pin (referred to as nSTATUS in the cyclone manual)
+
+both gpios pins are normally active low open drain.
+
+Example:
+	fpga_spi: evi-fpga-spi at 0 {
+		compatible = "altr,cyclone-ps-spi-fpga-mgr";
+		spi-max-frequency = <20000000>;
+		reg = <0>;
+		config-gpios = <&gpio4 9 GPIO_ACTIVE_LOW>;
+		status-gpios = <&gpio4 11 GPIO_ACTIVE_LOW>;
+	};

Signature · 1 line

2.9.3

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.

Signed-off-by Joshua Clayton <[email protected]>

---
 drivers/fpga/Kconfig          |   7 ++
 drivers/fpga/Makefile         |   1 +
 drivers/fpga/cyclone-ps-spi.c | 186 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 194 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 +0 -0

new file mode 100644
index 0000000..f9126f9

drivers/fpga/cyclone-ps-spi.c new file +17 -0

+ *
+ */
+
+#include <linux/bitrev.h>
+#include <linux/delay.h>
+#include <linux/fpga/fpga-mgr.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/of_gpio.h>
+#include <linux/spi/spi.h>
+#include <linux/sizes.h>
+
+#define FPGA_RESET_TIME		50   /* time in usecs to trigger FPGA config */
+#define FPGA_MIN_DELAY		50   /* min usecs to wait for config status */
+#define FPGA_MAX_DELAY		1000 /* max usecs to wait for config status */
+
+struct cyclonespi_conf {

drivers/fpga/cyclone-ps-spi.c new file +16 -0

+static enum fpga_mgr_states cyclonespi_state(struct fpga_manager *mgr)
+{
+	struct cyclonespi_conf *conf = (struct cyclonespi_conf *)mgr->priv;
+
+	if (gpiod_get_value(conf->status))
+		return FPGA_MGR_STATE_RESET;
+
+	return FPGA_MGR_STATE_UNKNOWN;
+}
+
+static int cyclonespi_write_init(struct fpga_manager *mgr,
+				 struct fpga_image_info *info,
+				 const char *buf, size_t count)
+{
+	struct cyclonespi_conf *conf = (struct cyclonespi_conf *)mgr->priv;
+	int i;

drivers/fpga/cyclone-ps-spi.c new file +4 -0

+}
+
+static int cyclonespi_write(struct fpga_manager *mgr, const char *buf,
+			    size_t count)

drivers/fpga/cyclone-ps-spi.c new file +6 -0

+		ret = spi_write(conf->spi, fw_data, stride);
+		if (ret) {
+			dev_err(&mgr->dev, "spi error in firmware write: %d\n",
+				ret);
+			return ret;
+		}

drivers/fpga/cyclone-ps-spi.c new file +30 -0

+
+	if (!conf)
+		return -ENOMEM;
+
+	conf->spi = spi;
+	conf->config = devm_gpiod_get(&spi->dev, "config", GPIOD_OUT_HIGH);
+	if (IS_ERR(conf->config)) {
+		dev_err(&spi->dev, "Failed to get config gpio: %ld\n",
+			PTR_ERR(conf->config));
+		return PTR_ERR(conf->config);
+	}
+
+	conf->status = devm_gpiod_get(&spi->dev, "status", GPIOD_IN);
+	if (IS_ERR(conf->status)) {
+		dev_err(&spi->dev, "Failed to get status gpio: %ld\n",
+			PTR_ERR(conf->status));
+		return PTR_ERR(conf->status);
+	}
+
+	return fpga_mgr_register(&spi->dev,
+				 "Altera Cyclone PS SPI FPGA Manager",
+				 &cyclonespi_ops, conf);
+}
+
+static int cyclonespi_remove(struct spi_device *spi)
+{
+	fpga_mgr_unregister(&spi->dev);
+
+	return 0;
+}

Quoted 39 lines · 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.

Signed-off-by Joshua Clayton <[email protected]>

---
 drivers/fpga/Kconfig          |   7 ++
 drivers/fpga/Makefile         |   1 +
 drivers/fpga/cyclone-ps-spi.c | 186 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 194 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 +0 -0

index 8df07bc..a112bef 100644
@@ -6,6 +6,7 @@
 obj-$(CONFIG_FPGA)			+= fpga-mgr.o
 

Quoted 11 lines

drivers/fpga/Makefile +1 -0

 # 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 +0 -0

new file mode 100644
index 0000000..f9126f9

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

Quoted 4 lines

drivers/fpga/cyclone-ps-spi.c new file +4 -0

@@ -0,0 +1,186 @@
+/**
+ * Altera Cyclone Passive Serial SPI Driver
+ *
+ *  Copyright (c) 2017 United Western Technologies, Corporation

In which timezone it's already 2017? s/ / /

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

Quoted 8 lines

drivers/fpga/cyclone-ps-spi.c new file +8 -0

+ *
+ *  Joshua Clayton <[email protected]>
+ *
+ * Manage Altera FPGA firmware that is loaded over spi using the passive
+ * serial configuration method.
+ * Firmware must be in binary "rbf" format.
+ * Works on Cyclone V. Should work on cyclone series.
+ * May work on other Altera FPGAs.

I can test this later on an Arria 10. I'm not sure what the connection between "Cyclone" and "Arria" is, but the protocol looks similar.

Quoted 17 lines

drivers/fpga/cyclone-ps-spi.c new file +17 -0

+ *
+ */
+
+#include <linux/bitrev.h>
+#include <linux/delay.h>
+#include <linux/fpga/fpga-mgr.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/of_gpio.h>
+#include <linux/spi/spi.h>
+#include <linux/sizes.h>
+
+#define FPGA_RESET_TIME		50   /* time in usecs to trigger FPGA config */
+#define FPGA_MIN_DELAY		50   /* min usecs to wait for config status */
+#define FPGA_MAX_DELAY		1000 /* max usecs to wait for config status */
+
+struct cyclonespi_conf {

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

Quoted 10 lines

drivers/fpga/cyclone-ps-spi.c new file +10 -0

+	struct gpio_desc *config;
+	struct gpio_desc *status;
+	struct spi_device *spi;
+};
+
+static const struct of_device_id of_ef_match[] = {
+	{ .compatible = "altr,cyclone-ps-spi-fpga-mgr", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, of_ef_match);

barebox already has such a driver, the binding is available at

https://git.pengutronix.de/cgit/barebox/tree/Documentation/devicetree/bindings/firmware/altr,passive-serial.txt

(This isn't completely accurate because nstat is optional in the driver.)

Quoted 19 lines

drivers/fpga/cyclone-ps-spi.c new file +19 -0

+static enum fpga_mgr_states cyclonespi_state(struct fpga_manager *mgr)
+{
+	struct cyclonespi_conf *conf = (struct cyclonespi_conf *)mgr->priv;
+
+	if (gpiod_get_value(conf->status))
+		return FPGA_MGR_STATE_RESET;
+
+	return FPGA_MGR_STATE_UNKNOWN;
+}
+
+static int cyclonespi_write_init(struct fpga_manager *mgr,
+				 struct fpga_image_info *info,
+				 const char *buf, size_t count)
+{
+	struct cyclonespi_conf *conf = (struct cyclonespi_conf *)mgr->priv;
+	int i;
+
+	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
+		dev_err(&mgr->dev, "Partial reconfiguration not supported.\n");

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

Quoted 7 lines

drivers/fpga/cyclone-ps-spi.c new file +7 -0

+		return -EINVAL;
+	}
+
+	gpiod_set_value(conf->config, 1);
+	usleep_range(FPGA_RESET_TIME, FPGA_RESET_TIME + 20);
+	if (!gpiod_get_value(conf->status)) {
+		dev_err(&mgr->dev, "Status pin should be low.\n");

You write this when get_value returns 0. There is something fishy.

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

Quoted 12 lines

drivers/fpga/cyclone-ps-spi.c new file +12 -0

+		return -EIO;
+	}
+
+	gpiod_set_value(conf->config, 0);
+	for (i = 0; i < (FPGA_MAX_DELAY / FPGA_MIN_DELAY); i++) {
+		usleep_range(FPGA_MIN_DELAY, FPGA_MIN_DELAY + 20);
+		if (!gpiod_get_value(conf->status))
+			return 0;
+	}
+
+	dev_err(&mgr->dev, "Status pin not ready.\n");
+	return -EIO;

For Arria 10 the documentation has:

To ensure a successful configuration, send the entire configuration data to the device. CONF_DONE is released high when the device receives all the configuration data successfully. After CONF_DONE goes high, send two additional falling edges on DCLK to begin initialization and enter user mode.

ISTR this is necessary for Arria V, too.

Quoted 6 lines

drivers/fpga/cyclone-ps-spi.c new file +6 -0

+}
+
+static void rev_buf(void *buf, size_t len)
+{
+	u32 *fw32 = (u32 *)buf;
+	const u32 *fw_end = (u32 *)(buf + len);

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

Quoted 6 lines

drivers/fpga/cyclone-ps-spi.c new file +6 -0

+
+	/* set buffer to lsb first */
+	while (fw32 < fw_end) {
+		*fw32 = bitrev8x4(*fw32);
+		fw32++;
+	}

Is the size of the firmware always a multiple of 32 bit? If len isn't a multiple of 4 you access data after the end of buf.

Quoted 9 lines

drivers/fpga/cyclone-ps-spi.c new file +9 -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;
+

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

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((void *)fw_data, stride);

This isn't necessary if the spi controller supports SPI_LSB_FIRST. At least the mvebu spi core can do this for you. (The driver doesn't support it yet, though.)

Quoted 9 lines

drivers/fpga/cyclone-ps-spi.c new file +9 -0

+		ret = spi_write(conf->spi, fw_data, stride);
+		if (ret) {
+			dev_err(&mgr->dev, "spi error in firmware write: %d\n",
+				ret);
+			return ret;
+		}
+		fw_data += stride;
+	}
+

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

Quoted 7 lines

drivers/fpga/cyclone-ps-spi.c new file +6 -0

+	return 0;
+}
[...]
+static int cyclonespi_probe(struct spi_device *spi)
+{
+	struct cyclonespi_conf *conf = devm_kzalloc(&spi->dev, sizeof(*conf),
+						GFP_KERNEL);

please indent to the opening (.

Quoted 36 lines

drivers/fpga/cyclone-ps-spi.c new file +36 -0

+
+	if (!conf)
+		return -ENOMEM;
+
+	conf->spi = spi;
+	conf->config = devm_gpiod_get(&spi->dev, "config", GPIOD_OUT_HIGH);
+	if (IS_ERR(conf->config)) {
+		dev_err(&spi->dev, "Failed to get config gpio: %ld\n",
+			PTR_ERR(conf->config));
+		return PTR_ERR(conf->config);
+	}
+
+	conf->status = devm_gpiod_get(&spi->dev, "status", GPIOD_IN);
+	if (IS_ERR(conf->status)) {
+		dev_err(&spi->dev, "Failed to get status gpio: %ld\n",
+			PTR_ERR(conf->status));
+		return PTR_ERR(conf->status);
+	}
+
+	return fpga_mgr_register(&spi->dev,
+				 "Altera Cyclone PS SPI FPGA Manager",
+				 &cyclonespi_ops, conf);
+}
+
+static int cyclonespi_remove(struct spi_device *spi)
+{
+	fpga_mgr_unregister(&spi->dev);
+
+	return 0;
+}
+
+static struct spi_driver cyclonespi_driver = {
+	.driver = {
+		.name   = "cyclone-ps-spi",
+		.owner  = THIS_MODULE,
+		.of_match_table = of_match_ptr(of_ef_match),

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

Quoted 4 lines

drivers/fpga/cyclone-ps-spi.c new file +4 -0

+	},
+	.probe  = cyclonespi_probe,
+	.remove = cyclonespi_remove,
+};

I'm not a big fan of aligning the assignment operators. This tends to get out of sync or results in bigger than necessary changes in follow up patches. Note that it's out of sync already now, so I suggest to use a single space before =.

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

Quoted 7 lines

drivers/fpga/cyclone-ps-spi.c new file +6 -0

+
+module_spi_driver(cyclonespi_driver)
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Joshua Clayton <[email protected]>");
+MODULE_DESCRIPTION("Module to load Altera FPGA firmware over spi");

Best regards
Uwe

Quoted 3 lines

Quoted 1 line

+ * Copyright (c) 2017 United Western Technologies, Corporation

In which timezone it's already 2017? s/ / /

LOL. It will be 2017 long before 4.11 was my thinking.
I guess I've never spent much time on time stamp etiquette for copyright. It said "2015" in v5, despite still being revised.

Quoted 8 lines

Quoted 8 lines

+ *
+ * Joshua Clayton <[email protected]>
+ *
+ * Manage Altera FPGA firmware that is loaded over spi using the passive
+ * serial configuration method.
+ * Firmware must be in binary "rbf" format.
+ * Works on Cyclone V. Should work on cyclone series.
+ * May work on other Altera FPGAs.

Quoted 2 lines

I can test this later on an Arria 10. I'm not sure what the connection between "Cyclone" and "Arria" is, but the protocol looks similar.

My guess was it would be.
Would be wonderful to have someone to test.

Quoted 29 lines

Quoted 17 lines

+ *
+ */

drivers/fpga/cyclone-ps-spi.c new file +15 -0

+
+#include <linux/bitrev.h>
+#include <linux/delay.h>
+#include <linux/fpga/fpga-mgr.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/of_gpio.h>
+#include <linux/spi/spi.h>
+#include <linux/sizes.h>
+
+#define FPGA_RESET_TIME		50   /* time in usecs to trigger FPGA config */
+#define FPGA_MIN_DELAY		50   /* min usecs to wait for config status */
+#define FPGA_MAX_DELAY		1000 /* max usecs to wait for config status */
+
+struct cyclonespi_conf {

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

Quoted 10 lines

drivers/fpga/cyclone-ps-spi.c new file +10 -0

+	struct gpio_desc *config;
+	struct gpio_desc *status;
+	struct spi_device *spi;
+};
+
+static const struct of_device_id of_ef_match[] = {
+	{ .compatible = "altr,cyclone-ps-spi-fpga-mgr", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, of_ef_match);

barebox already has such a driver, the binding is available at

Quoted 3 lines

https://git.pengutronix.de/cgit/barebox/tree/Documentation/devicetree/bindings/firmware/altr,passive-serial.txt

(This isn't completely accurate because nstat is optional in the driver.)

Interesting.
The binding looks ... like we should synchronize those bindings.

Quoted 5 lines

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

Quoted 4 lines

drivers/fpga/cyclone-ps-spi.c new file +4 -0

+	gpiod_set_value(conf->config, 1);
+	usleep_range(FPGA_RESET_TIME, FPGA_RESET_TIME + 20);
+	if (!gpiod_get_value(conf->status)) {
+		dev_err(&mgr->dev, "Status pin should be low.\n");

You write this when get_value returns 0. There is something fishy.

I'll take a look. These gpios are "active low", so a logical 1 is a physical low, IIRC. Maybe I should change the wording:
Something like dev_err(&mgr->dev, "Status pin should be in reset.\n");

Quoted 21 lines

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

Quoted 12 lines

drivers/fpga/cyclone-ps-spi.c new file +12 -0

+		return -EIO;
+	}
+
+	gpiod_set_value(conf->config, 0);
+	for (i = 0; i < (FPGA_MAX_DELAY / FPGA_MIN_DELAY); i++) {
+		usleep_range(FPGA_MIN_DELAY, FPGA_MIN_DELAY + 20);
+		if (!gpiod_get_value(conf->status))
+			return 0;
+	}
+
+	dev_err(&mgr->dev, "Status pin not ready.\n");
+	return -EIO;

For Arria 10 the documentation has:

To ensure a successful configuration, send the entire configuration data to the device. CONF_DONE is released high when the device receives all the configuration data successfully. After CONF_DONE goes high, send two additional falling edges on DCLK to begin initialization and enter user mode.

Quoted 1 line

ISTR this is necessary for Arria V, too.

DCLK is the spi clock, yes?
Would sending an extra byte after CONF_DONE is released suffice?

Quoted 9 lines

Quoted 9 lines

drivers/fpga/cyclone-ps-spi.c new file +9 -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 104

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((void *)fw_data, stride);

This isn't necessary if the spi controller supports SPI_LSB_FIRST. At least the mvebu spi core can do this for you. (The driver doesn't support it yet, though.)

This is true, but many of them do not.

Moritz Fischer had proposal to add things like this with a flag. It could then be part of the library, rather than part of the driver

Speaking of which,
I made an unsuccessful attempt to hack generic lsb first SPI with an extra bounce buffer.
Sending was fine, but I ran into trouble with LSB first rx (I think) because of dma.

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

Signed-off-by Joshua Clayton <[email protected]>

---
 arch/arm/boot/dts/imx6q-evi.dts | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

arch/arm/boot/dts/imx6q-evi.dts +16 -0

index 7c7c1a8..ec4d365 100644
@@ -95,6 +95,15 @@
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_ecspi1 &pinctrl_ecspi1cs>;
 	status = "okay";
+
+	fpga_spi: cyclonespi at 0 {
+		compatible = "altr,cyclone-ps-spi-fpga-mgr";
+		spi-max-frequency = <20000000>;
+		reg = <0>;
+		pinctrl-0 = <&pinctrl_fpgaspi>;
+		config-gpios = <&gpio4 9 GPIO_ACTIVE_LOW>;
+		status-gpios = <&gpio4 11 GPIO_ACTIVE_LOW>;
+	};
 };
 
 &ecspi3 {

@@ -322,6 +331,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

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.

Need ACKs from ARCH maintainers for ARCH specific implementations of __arch_bitrev8x4(), and I've added more ARCHes, so will need more ACKS, but the generic code will work without that patch, so if there is a holdup the rest of the series can go in without patch 2

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

Joshua Clayton (5):
lib: add bitrev8x4()
lib: implement __arch_bitrev8x4()
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/cyclone-ps-spi-fpga-mgr.txt      |  25 +++
 arch/arm/boot/dts/imx6q-evi.dts                    |  16 ++
 arch/arm/include/asm/bitrev.h                      |   6 +
 arch/arm64/include/asm/bitrev.h                    |   6 +
 arch/mips/include/asm/bitrev.h                     |   6 +
 drivers/fpga/Kconfig                               |   7 +
 drivers/fpga/Makefile                              |   1 +
 drivers/fpga/cyclone-ps-spi.c                      | 186 +++++++++++++++++++++
 include/linux/bitrev.h                             |  26 +++
 9 files changed, 279 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/fpga/cyclone-ps-spi-fpga-mgr.txt
 create mode 100644 drivers/fpga/cyclone-ps-spi.c

Signature · 6 lines

2.9.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html