2017-06-05 19:07:11

by Alan Tull

[permalink] [raw]
Subject: [PATCH 00/10] patches for fpga

Hi Greg,

Please take these patches for FPGA that have been
reviewed on the mailing lists.

Thanks,
Alan

Anatolij Gustschin (1):
fpga: Add flag to indicate SPI bitstream is bit-reversed

Joel Holdsworth (1):
of: Add vendor prefix for Lattice Semiconductor

Joshua Clayton (5):
doc: dt: document altera-passive-serial binding
fpga manager: Add altera-ps-spi driver for Altera FPGAs
ARM: dts: imx6q-evi: support altera-ps-spi
lib: add bitrev8x4()
fpga-manager: altera-ps-spi: use bitrev8x4

Moritz Fischer (1):
dt-bindings: fpga: Add bindings document for Xilinx LogiCore PR
Decoupler

Tobias Klauser (1):
fpga: allow to compile-test Altera FPGA bridge drivers

Vincent Legoll (1):
Make FPGA a menuconfig to ease disabling it all

.../bindings/fpga/altera-passive-serial.txt | 29 ++
.../bindings/fpga/xilinx-pr-decoupler.txt | 36 +++
.../devicetree/bindings/vendor-prefixes.txt | 1 +
arch/arm/boot/dts/imx6q-evi.dts | 16 ++
drivers/fpga/Kconfig | 17 +-
drivers/fpga/Makefile | 1 +
drivers/fpga/altera-ps-spi.c | 308 +++++++++++++++++++++
include/linux/bitrev.h | 19 ++
include/linux/fpga/fpga-mgr.h | 2 +
9 files changed, 422 insertions(+), 7 deletions(-)
create mode 100644 Documentation/devicetree/bindings/fpga/altera-passive-serial.txt
create mode 100644 Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt
create mode 100644 drivers/fpga/altera-ps-spi.c

--
2.11.0


2017-06-05 19:07:16

by Alan Tull

[permalink] [raw]
Subject: [PATCH 04/10] fpga: Add flag to indicate SPI bitstream is bit-reversed

From: Anatolij Gustschin <[email protected]>

Add a flag that is passed to the write_init() callback,
indicating that the SPI bitstream starts with LSB first.
SPI controllers usually send data with MSB first. If an
FPGA expects bitstream data as LSB first, the data must
be reversed either by the SPI controller or by the driver.

Alternatively the bitstream could be prepared as bit-reversed
to avoid the bit-swapping while sending. This flag indicates
such bit-reversed SPI bitstream. The low-level driver will
deal with the flag and perform bit-reversing if needed.

Signed-off-by: Anatolij Gustschin <[email protected]>
Signed-off-by: Joshua Clayton <[email protected]>
Signed-off-by: Alan Tull <[email protected]>
---
include/linux/fpga/fpga-mgr.h | 2 ++
1 file changed, 2 insertions(+)

diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h
index b4ac24c4411d..01c348ca38b7 100644
--- a/include/linux/fpga/fpga-mgr.h
+++ b/include/linux/fpga/fpga-mgr.h
@@ -67,10 +67,12 @@ enum fpga_mgr_states {
* FPGA Manager flags
* FPGA_MGR_PARTIAL_RECONFIG: do partial reconfiguration if supported
* FPGA_MGR_EXTERNAL_CONFIG: FPGA has been configured prior to Linux booting
+ * FPGA_MGR_BITSTREAM_LSB_FIRST: SPI bitstream bit order is LSB first
*/
#define FPGA_MGR_PARTIAL_RECONFIG BIT(0)
#define FPGA_MGR_EXTERNAL_CONFIG BIT(1)
#define FPGA_MGR_ENCRYPTED_BITSTREAM BIT(2)
+#define FPGA_MGR_BITSTREAM_LSB_FIRST BIT(3)

/**
* struct fpga_image_info - information specific to a FPGA image
--
2.11.0

2017-06-05 19:07:27

by Alan Tull

[permalink] [raw]
Subject: [PATCH 09/10] fpga-manager: altera-ps-spi: use bitrev8x4

From: Joshua Clayton <[email protected]>

Speed up bit reversal by using hardware bit reversal
Add extra code to handle less than 4byte remnants, if any

Signed-off-by: Joshua Clayton <[email protected]>
Signed-off-by: Alan Tull <[email protected]>
---
drivers/fpga/altera-ps-spi.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/fpga/altera-ps-spi.c b/drivers/fpga/altera-ps-spi.c
index 0db8def668ed..14f14efdf0d5 100644
--- a/drivers/fpga/altera-ps-spi.c
+++ b/drivers/fpga/altera-ps-spi.c
@@ -149,12 +149,23 @@ static int altera_ps_write_init(struct fpga_manager *mgr,

static void rev_buf(char *buf, size_t len)
{
- const char *fw_end = (buf + len);
+ u32 *fw32 = (u32 *)buf;
+ size_t extra_bytes = (len & 0x03);
+ const u32 *fw_end = (u32 *)(buf + len - extra_bytes);

/* set buffer to lsb first */
- while (buf < fw_end) {
- *buf = bitrev8(*buf);
- buf++;
+ while (fw32 < fw_end) {
+ *fw32 = bitrev8x4(*fw32);
+ fw32++;
+ }
+
+ if (extra_bytes) {
+ buf = (char *)fw_end;
+ while (extra_bytes) {
+ *buf = bitrev8(*buf);
+ buf++;
+ extra_bytes--;
+ }
}
}

--
2.11.0

2017-06-05 19:07:18

by Alan Tull

[permalink] [raw]
Subject: [PATCH 06/10] fpga manager: Add altera-ps-spi driver for Altera FPGAs

From: Joshua Clayton <[email protected]>

altera-ps-spi loads FPGA firmware over SPI, using the "passive serial"
interface on Altera Arria 10, Cyclone V or Stratix V 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]>
Signed-off-by: Anatolij Gustschin <[email protected]>
Signed-off-by: Alan Tull <[email protected]>
---
drivers/fpga/Kconfig | 7 +
drivers/fpga/Makefile | 1 +
drivers/fpga/altera-ps-spi.c | 297 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 305 insertions(+)
create mode 100644 drivers/fpga/altera-ps-spi.c

diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index d6493655de1b..942d276bff90 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -24,6 +24,13 @@ config FPGA_MGR_ICE40_SPI
help
FPGA manager driver support for Lattice iCE40 FPGAs over SPI.

+config FPGA_MGR_ALTERA_PS_SPI
+ tristate "Altera FPGA Passive Serial over SPI"
+ depends on SPI || COMPILE_TEST
+ help
+ FPGA manager driver support for Altera Arria/Cyclone/Stratix
+ using the passive serial interface over SPI.
+
config FPGA_MGR_SOCFPGA
tristate "Altera SOCFPGA FPGA Manager"
depends on ARCH_SOCFPGA || COMPILE_TEST
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index 2a4f0218145c..e75d3570e26a 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -6,6 +6,7 @@
obj-$(CONFIG_FPGA) += fpga-mgr.o

# FPGA Manager Drivers
+obj-$(CONFIG_FPGA_MGR_ALTERA_PS_SPI) += altera-ps-spi.o
obj-$(CONFIG_FPGA_MGR_ICE40_SPI) += ice40-spi.o
obj-$(CONFIG_FPGA_MGR_SOCFPGA) += socfpga.o
obj-$(CONFIG_FPGA_MGR_SOCFPGA_A10) += socfpga-a10.o
diff --git a/drivers/fpga/altera-ps-spi.c b/drivers/fpga/altera-ps-spi.c
new file mode 100644
index 000000000000..0db8def668ed
--- /dev/null
+++ b/drivers/fpga/altera-ps-spi.c
@@ -0,0 +1,297 @@
+/*
+ * Altera Passive Serial SPI Driver
+ *
+ * Copyright (c) 2017 United Western Technologies, Corporation
+ *
+ * Joshua Clayton <[email protected]>
+ *
+ * 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.
+ *
+ * Manage Altera FPGA firmware that is loaded over SPI using the passive
+ * serial configuration method.
+ * Firmware must be in binary "rbf" format.
+ * Works on Arria 10, Cyclone V and Stratix V. Should work on Cyclone series.
+ * May work on other Altera FPGAs.
+ */
+
+#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/of_device.h>
+#include <linux/spi/spi.h>
+#include <linux/sizes.h>
+
+enum altera_ps_devtype {
+ CYCLONE5,
+ ARRIA10,
+};
+
+struct altera_ps_data {
+ enum altera_ps_devtype devtype;
+ int status_wait_min_us;
+ int status_wait_max_us;
+ int t_cfg_us;
+ int t_st2ck_us;
+};
+
+struct altera_ps_conf {
+ struct gpio_desc *config;
+ struct gpio_desc *confd;
+ struct gpio_desc *status;
+ struct spi_device *spi;
+ const struct altera_ps_data *data;
+ u32 info_flags;
+ char mgr_name[64];
+};
+
+/* | Arria 10 | Cyclone5 | Stratix5 |
+ * t_CF2ST0 | [; 600] | [; 600] | [; 600] |ns
+ * t_CFG | [2;] | [2;] | [2;] |µs
+ * t_STATUS | [268; 3000] | [268; 1506] | [268; 1506] |µs
+ * t_CF2ST1 | [; 3000] | [; 1506] | [; 1506] |µs
+ * t_CF2CK | [3010;] | [1506;] | [1506;] |µs
+ * t_ST2CK | [10;] | [2;] | [2;] |µs
+ * t_CD2UM | [175; 830] | [175; 437] | [175; 437] |µs
+ */
+static struct altera_ps_data c5_data = {
+ /* these values for Cyclone5 are compatible with Stratix5 */
+ .devtype = CYCLONE5,
+ .status_wait_min_us = 268,
+ .status_wait_max_us = 1506,
+ .t_cfg_us = 2,
+ .t_st2ck_us = 2,
+};
+
+static struct altera_ps_data a10_data = {
+ .devtype = ARRIA10,
+ .status_wait_min_us = 268, /* min(t_STATUS) */
+ .status_wait_max_us = 3000, /* max(t_CF2ST1) */
+ .t_cfg_us = 2, /* max { min(t_CFG), max(tCF2ST0) } */
+ .t_st2ck_us = 10, /* min(t_ST2CK) */
+};
+
+static const struct of_device_id of_ef_match[] = {
+ { .compatible = "altr,fpga-passive-serial", .data = &c5_data },
+ { .compatible = "altr,fpga-arria10-passive-serial", .data = &a10_data },
+ {}
+};
+MODULE_DEVICE_TABLE(of, of_ef_match);
+
+static enum fpga_mgr_states altera_ps_state(struct fpga_manager *mgr)
+{
+ struct altera_ps_conf *conf = mgr->priv;
+
+ if (gpiod_get_value_cansleep(conf->status))
+ return FPGA_MGR_STATE_RESET;
+
+ return FPGA_MGR_STATE_UNKNOWN;
+}
+
+static inline void altera_ps_delay(int delay_us)
+{
+ if (delay_us > 10)
+ usleep_range(delay_us, delay_us + 5);
+ else
+ udelay(delay_us);
+}
+
+static int altera_ps_write_init(struct fpga_manager *mgr,
+ struct fpga_image_info *info,
+ const char *buf, size_t count)
+{
+ struct altera_ps_conf *conf = mgr->priv;
+ int min, max, waits;
+ int i;
+
+ conf->info_flags = info->flags;
+
+ if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
+ dev_err(&mgr->dev, "Partial reconfiguration not supported.\n");
+ return -EINVAL;
+ }
+
+ gpiod_set_value_cansleep(conf->config, 1);
+
+ /* wait min reset pulse time */
+ altera_ps_delay(conf->data->t_cfg_us);
+
+ if (!gpiod_get_value_cansleep(conf->status)) {
+ dev_err(&mgr->dev, "Status pin failed to show a reset\n");
+ return -EIO;
+ }
+
+ gpiod_set_value_cansleep(conf->config, 0);
+
+ min = conf->data->status_wait_min_us;
+ max = conf->data->status_wait_max_us;
+ waits = max / min;
+ if (max % min)
+ waits++;
+
+ /* wait for max { max(t_STATUS), max(t_CF2ST1) } */
+ for (i = 0; i < waits; i++) {
+ usleep_range(min, min + 10);
+ if (!gpiod_get_value_cansleep(conf->status)) {
+ /* wait for min(t_ST2CK)*/
+ altera_ps_delay(conf->data->t_st2ck_us);
+ return 0;
+ }
+ }
+
+ dev_err(&mgr->dev, "Status pin not ready.\n");
+ return -EIO;
+}
+
+static void rev_buf(char *buf, size_t len)
+{
+ const char *fw_end = (buf + len);
+
+ /* set buffer to lsb first */
+ while (buf < fw_end) {
+ *buf = bitrev8(*buf);
+ buf++;
+ }
+}
+
+static int altera_ps_write(struct fpga_manager *mgr, const char *buf,
+ size_t count)
+{
+ struct altera_ps_conf *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_t(size_t, fw_data_end - fw_data, SZ_4K);
+
+ if (!(conf->info_flags & FPGA_MGR_BITSTREAM_LSB_FIRST))
+ rev_buf((char *)fw_data, stride);
+
+ 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;
+ }
+
+ return 0;
+}
+
+static int altera_ps_write_complete(struct fpga_manager *mgr,
+ struct fpga_image_info *info)
+{
+ struct altera_ps_conf *conf = mgr->priv;
+ const char dummy[] = {0};
+ int ret;
+
+ if (gpiod_get_value_cansleep(conf->status)) {
+ dev_err(&mgr->dev, "Error during configuration.\n");
+ return -EIO;
+ }
+
+ if (!IS_ERR(conf->confd)) {
+ if (!gpiod_get_raw_value_cansleep(conf->confd)) {
+ dev_err(&mgr->dev, "CONF_DONE is inactive!\n");
+ return -EIO;
+ }
+ }
+
+ /*
+ * After CONF_DONE goes high, send two additional falling edges on DCLK
+ * to begin initialization and enter user mode
+ */
+ ret = spi_write(conf->spi, dummy, 1);
+ if (ret) {
+ dev_err(&mgr->dev, "spi error during end sequence: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct fpga_manager_ops altera_ps_ops = {
+ .state = altera_ps_state,
+ .write_init = altera_ps_write_init,
+ .write = altera_ps_write,
+ .write_complete = altera_ps_write_complete,
+};
+
+static int altera_ps_probe(struct spi_device *spi)
+{
+ struct altera_ps_conf *conf;
+ const struct of_device_id *of_id;
+
+ conf = devm_kzalloc(&spi->dev, sizeof(*conf), GFP_KERNEL);
+ if (!conf)
+ return -ENOMEM;
+
+ of_id = of_match_device(of_ef_match, &spi->dev);
+ if (!of_id)
+ return -ENODEV;
+
+ conf->data = of_id->data;
+ conf->spi = spi;
+ conf->config = devm_gpiod_get(&spi->dev, "nconfig", 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, "nstat", 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);
+ }
+
+ conf->confd = devm_gpiod_get(&spi->dev, "confd", GPIOD_IN);
+ if (IS_ERR(conf->confd)) {
+ dev_warn(&spi->dev, "Not using confd gpio: %ld\n",
+ PTR_ERR(conf->confd));
+ }
+
+ /* Register manager with unique name */
+ snprintf(conf->mgr_name, sizeof(conf->mgr_name), "%s %s",
+ dev_driver_string(&spi->dev), dev_name(&spi->dev));
+
+ return fpga_mgr_register(&spi->dev, conf->mgr_name,
+ &altera_ps_ops, conf);
+}
+
+static int altera_ps_remove(struct spi_device *spi)
+{
+ fpga_mgr_unregister(&spi->dev);
+
+ return 0;
+}
+
+static const struct spi_device_id altera_ps_spi_ids[] = {
+ {"cyclone-ps-spi", 0},
+ {}
+};
+MODULE_DEVICE_TABLE(spi, altera_ps_spi_ids);
+
+static struct spi_driver altera_ps_driver = {
+ .driver = {
+ .name = "altera-ps-spi",
+ .owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(of_ef_match),
+ },
+ .id_table = altera_ps_spi_ids,
+ .probe = altera_ps_probe,
+ .remove = altera_ps_remove,
+};
+
+module_spi_driver(altera_ps_driver)
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Joshua Clayton <[email protected]>");
+MODULE_DESCRIPTION("Module to load Altera FPGA firmware over SPI");
--
2.11.0

2017-06-05 19:07:21

by Alan Tull

[permalink] [raw]
Subject: [PATCH 08/10] lib: add bitrev8x4()

From: Joshua Clayton <[email protected]>

Add a function to reverse bytes within a 32 bit word.
Operate on a u32 rather than individual bytes.

Signed-off-by: Joshua Clayton <[email protected]>
Signed-off-by: Alan Tull <[email protected]>
---
include/linux/bitrev.h | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)

diff --git a/include/linux/bitrev.h b/include/linux/bitrev.h
index fb790b8449c1..b97be27e5a85 100644
--- a/include/linux/bitrev.h
+++ b/include/linux/bitrev.h
@@ -29,6 +29,8 @@ static inline u32 __bitrev32(u32 x)

#endif /* CONFIG_HAVE_ARCH_BITREVERSE */

+#define __bitrev8x4(x) (__bitrev32(swab32(x)))
+
#define __constant_bitrev32(x) \
({ \
u32 __x = x; \
@@ -50,6 +52,15 @@ static inline u32 __bitrev32(u32 x)
__x; \
})

+#define __constant_bitrev8x4(x) \
+({ \
+ u32 __x = x; \
+ __x = ((__x & (u32)0xF0F0F0F0UL) >> 4) | ((__x & (u32)0x0F0F0F0FUL) << 4); \
+ __x = ((__x & (u32)0xCCCCCCCCUL) >> 2) | ((__x & (u32)0x33333333UL) << 2); \
+ __x = ((__x & (u32)0xAAAAAAAAUL) >> 1) | ((__x & (u32)0x55555555UL) << 1); \
+ __x; \
+})
+
#define __constant_bitrev8(x) \
({ \
u8 __x = x; \
@@ -75,6 +86,14 @@ static inline u32 __bitrev32(u32 x)
__bitrev16(__x); \
})

+#define bitrev8x4(x) \
+({ \
+ u32 __x = x; \
+ __builtin_constant_p(__x) ? \
+ __constant_bitrev8x4(__x) : \
+ __bitrev8x4(__x); \
+ })
+
#define bitrev8(x) \
({ \
u8 __x = x; \
--
2.11.0

2017-06-05 19:07:41

by Alan Tull

[permalink] [raw]
Subject: [PATCH 10/10] of: Add vendor prefix for Lattice Semiconductor

From: Joel Holdsworth <[email protected]>

Lattice Semiconductor Corporation is a manufacturer of integrated
circuits and IP products, including low-power FPGAs, video connectivity
devices and millimeter wave wireless products.

Website: http://latticesemi.com

Signed-off-by: Joel Holdsworth <[email protected]>
Acked-by: Rob Herring <[email protected]>
Acked-by: Moritz Fischer <[email protected]>
Signed-off-by: Alan Tull <[email protected]>
---
Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index d808f24af949..a1078d9f84af 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -172,6 +172,7 @@ kosagi Sutajio Ko-Usagi PTE Ltd.
kyo Kyocera Corporation
lacie LaCie
lantiq Lantiq Semiconductor
+lattice Lattice Semiconductor
lego LEGO Systems A/S
lenovo Lenovo Group Ltd.
lg LG Corporation
--
2.11.0

2017-06-05 19:08:07

by Alan Tull

[permalink] [raw]
Subject: [PATCH 07/10] ARM: dts: imx6q-evi: support altera-ps-spi

From: Joshua Clayton <[email protected]>

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

Signed-off-by: Joshua Clayton <[email protected]>
Signed-off-by: Alan Tull <[email protected]>
---
arch/arm/boot/dts/imx6q-evi.dts | 16 ++++++++++++++++
1 file changed, 16 insertions(+)

diff --git a/arch/arm/boot/dts/imx6q-evi.dts b/arch/arm/boot/dts/imx6q-evi.dts
index fd2220aa49e2..55ec775d4435 100644
--- a/arch/arm/boot/dts/imx6q-evi.dts
+++ b/arch/arm/boot/dts/imx6q-evi.dts
@@ -94,6 +94,15 @@
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_ecspi1 &pinctrl_ecspi1cs>;
status = "okay";
+
+ fpga: fpga@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 {
@@ -319,6 +328,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
--
2.11.0

2017-06-05 19:07:14

by Alan Tull

[permalink] [raw]
Subject: [PATCH 01/10] dt-bindings: fpga: Add bindings document for Xilinx LogiCore PR Decoupler

From: Moritz Fischer <[email protected]>

This adds the binding documentation for the Xilinx LogiCORE PR
Decoupler soft core.

Signed-off-by: Moritz Fischer <[email protected]>
Signed-off-by: Michal Simek <[email protected]>
Cc: Sören Brinkmann <[email protected]>
Cc: [email protected]
Cc: [email protected]
Acked-by: Rob Herring <[email protected]>
Signed off-by: Alan Tull <[email protected]>
---
.../bindings/fpga/xilinx-pr-decoupler.txt | 36 ++++++++++++++++++++++
1 file changed, 36 insertions(+)
create mode 100644 Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt

diff --git a/Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt b/Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt
new file mode 100644
index 000000000000..8dcfba926bc7
--- /dev/null
+++ b/Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt
@@ -0,0 +1,36 @@
+Xilinx LogiCORE Partial Reconfig Decoupler Softcore
+
+The Xilinx LogiCORE Partial Reconfig Decoupler manages one or more
+decouplers / fpga bridges.
+The controller can decouple/disable the bridges which prevents signal
+changes from passing through the bridge. The controller can also
+couple / enable the bridges which allows traffic to pass through the
+bridge normally.
+
+The Driver supports only MMIO handling. A PR region can have multiple
+PR Decouplers which can be handled independently or chained via decouple/
+decouple_status signals.
+
+Required properties:
+- compatible : Should contain "xlnx,pr-decoupler-1.00" followed by
+ "xlnx,pr-decoupler"
+- regs : base address and size for decoupler module
+- clocks : input clock to IP
+- clock-names : should contain "aclk"
+
+Optional properties:
+- bridge-enable : 0 if driver should disable bridge at startup
+ 1 if driver should enable bridge at startup
+ Default is to leave bridge in current state.
+
+See Documentation/devicetree/bindings/fpga/fpga-region.txt for generic bindings.
+
+Example:
+ fpga-bridge@100000450 {
+ compatible = "xlnx,pr-decoupler-1.00",
+ "xlnx-pr-decoupler";
+ regs = <0x10000045 0x10>;
+ clocks = <&clkc 15>;
+ clock-names = "aclk";
+ bridge-enable = <0>;
+ };
--
2.11.0

2017-06-05 19:08:45

by Alan Tull

[permalink] [raw]
Subject: [PATCH 05/10] doc: dt: document altera-passive-serial binding

From: Joshua Clayton <[email protected]>

Describe an altera-passive-serial devicetree entry, required features

Signed-off-by: Joshua Clayton <[email protected]>
Acked-by: Rob Herring <[email protected]>
Signed-off-by: Alan Tull <[email protected]>
---
.../bindings/fpga/altera-passive-serial.txt | 29 ++++++++++++++++++++++
1 file changed, 29 insertions(+)
create mode 100644 Documentation/devicetree/bindings/fpga/altera-passive-serial.txt

diff --git a/Documentation/devicetree/bindings/fpga/altera-passive-serial.txt b/Documentation/devicetree/bindings/fpga/altera-passive-serial.txt
new file mode 100644
index 000000000000..48478bc07e29
--- /dev/null
+++ b/Documentation/devicetree/bindings/fpga/altera-passive-serial.txt
@@ -0,0 +1,29 @@
+Altera Passive Serial SPI FPGA Manager
+
+Altera 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: Must be one of the following:
+ "altr,fpga-passive-serial",
+ "altr,fpga-arria10-passive-serial"
+- reg: SPI chip select of the FPGA
+- nconfig-gpios: config pin (referred to as nCONFIG in the manual)
+- nstat-gpios: status pin (referred to as nSTATUS in the manual)
+
+Optional properties:
+- confd-gpios: confd pin (referred to as CONF_DONE in the manual)
+
+Example:
+ fpga: fpga@0 {
+ compatible = "altr,fpga-passive-serial";
+ 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>;
+ };
--
2.11.0

2017-06-05 19:09:10

by Alan Tull

[permalink] [raw]
Subject: [PATCH 03/10] fpga: allow to compile-test Altera FPGA bridge drivers

From: Tobias Klauser <[email protected]>

Add COMPILE_TEST to the Kconfig entry for the Altera SoCFPGA FPGA
Bridge. The Altera FPGA Freeze Bridge can also be used on Altera PEIe
FPGAs, so the driver shouldn't depend on ARCH_SOCFPGA in the first place.
This allows test-compiling the drivers on other architectures to catch
compiler errors/warnings, e.g. due to API/header changes earlier on.

Signed-off-by: Tobias Klauser <[email protected]>
Signed-off-by: Alan Tull <[email protected]>
---
drivers/fpga/Kconfig | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index 10361902c830..d6493655de1b 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -67,14 +67,14 @@ config FPGA_BRIDGE

config SOCFPGA_FPGA_BRIDGE
tristate "Altera SoCFPGA FPGA Bridges"
- depends on ARCH_SOCFPGA && FPGA_BRIDGE
+ depends on (ARCH_SOCFPGA || COMPILE_TEST) && FPGA_BRIDGE
help
Say Y to enable drivers for FPGA bridges for Altera SOCFPGA
devices.

config ALTERA_FREEZE_BRIDGE
tristate "Altera FPGA Freeze Bridge"
- depends on ARCH_SOCFPGA && FPGA_BRIDGE
+ depends on FPGA_BRIDGE
help
Say Y to enable drivers for Altera FPGA Freeze bridges. A
freeze bridge is a bridge that exists in the FPGA fabric to
--
2.11.0

2017-06-05 19:09:24

by Alan Tull

[permalink] [raw]
Subject: [PATCH 02/10] Make FPGA a menuconfig to ease disabling it all

From: Vincent Legoll <[email protected]>

No need to get into the submenu to disable all FPGA-related config entries

Signed-off-by: Vincent Legoll <[email protected]>
Acked-by: Moritz Fischer <[email protected]>
Signed-off-by: Alan Tull <[email protected]>
---
drivers/fpga/Kconfig | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index 161ba9dccede..10361902c830 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -2,9 +2,7 @@
# FPGA framework configuration
#

-menu "FPGA Configuration Support"
-
-config FPGA
+menuconfig FPGA
tristate "FPGA Configuration Framework"
help
Say Y here if you want support for configuring FPGAs from the
@@ -106,5 +104,3 @@ config XILINX_PR_DECOUPLER
being reprogrammed during partial reconfig.

endif # FPGA
-
-endmenu
--
2.11.0

2017-06-06 13:46:24

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 03/10] fpga: allow to compile-test Altera FPGA bridge drivers

Hi Tobias,

[auto build test WARNING on linus/master]
[also build test WARNING on v4.12-rc4 next-20170606]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Alan-Tull/patches-for-fpga/20170606-072535
config: m32r-allyesconfig (attached as .config)
compiler: m32r-linux-gcc (GCC) 6.2.0
reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=m32r

All warnings (new ones prefixed by >>):

In file included from include/linux/printk.h:329:0,
from include/linux/kernel.h:13,
from include/linux/delay.h:21,
from drivers/fpga/altera-freeze-bridge.c:18:
drivers/fpga/altera-freeze-bridge.c: In function 'altera_freeze_br_do_freeze':
>> drivers/fpga/altera-freeze-bridge.c:108:15: warning: format '%d' expects argument of type 'int', but argument 6 has type 'long unsigned int' [-Wformat=]
dev_dbg(dev, "%s %d %d\n", __func__, status, readl(csr_ctrl_addr));
^
include/linux/dynamic_debug.h:134:39: note: in definition of macro 'dynamic_dev_dbg'
__dynamic_dev_dbg(&descriptor, dev, fmt, \
^~~
>> drivers/fpga/altera-freeze-bridge.c:108:2: note: in expansion of macro 'dev_dbg'
dev_dbg(dev, "%s %d %d\n", __func__, status, readl(csr_ctrl_addr));
^~~~~~~
drivers/fpga/altera-freeze-bridge.c: In function 'altera_freeze_br_do_unfreeze':
drivers/fpga/altera-freeze-bridge.c:145:15: warning: format '%d' expects argument of type 'int', but argument 6 has type 'long unsigned int' [-Wformat=]
dev_dbg(dev, "%s %d %d\n", __func__, status, readl(csr_ctrl_addr));
^
include/linux/dynamic_debug.h:134:39: note: in definition of macro 'dynamic_dev_dbg'
__dynamic_dev_dbg(&descriptor, dev, fmt, \
^~~
drivers/fpga/altera-freeze-bridge.c:145:2: note: in expansion of macro 'dev_dbg'
dev_dbg(dev, "%s %d %d\n", __func__, status, readl(csr_ctrl_addr));
^~~~~~~
drivers/fpga/altera-freeze-bridge.c:163:15: warning: format '%d' expects argument of type 'int', but argument 6 has type 'long unsigned int' [-Wformat=]
dev_dbg(dev, "%s %d %d\n", __func__, status, readl(csr_ctrl_addr));
^
include/linux/dynamic_debug.h:134:39: note: in definition of macro 'dynamic_dev_dbg'
__dynamic_dev_dbg(&descriptor, dev, fmt, \
^~~
drivers/fpga/altera-freeze-bridge.c:163:2: note: in expansion of macro 'dev_dbg'
dev_dbg(dev, "%s %d %d\n", __func__, status, readl(csr_ctrl_addr));
^~~~~~~

vim +108 drivers/fpga/altera-freeze-bridge.c

ca24a648 Alan Tull 2016-11-01 12 * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
ca24a648 Alan Tull 2016-11-01 13 * more details.
ca24a648 Alan Tull 2016-11-01 14 *
ca24a648 Alan Tull 2016-11-01 15 * You should have received a copy of the GNU General Public License along with
ca24a648 Alan Tull 2016-11-01 16 * this program. If not, see <http://www.gnu.org/licenses/>.
ca24a648 Alan Tull 2016-11-01 17 */
ca24a648 Alan Tull 2016-11-01 @18 #include <linux/delay.h>
ca24a648 Alan Tull 2016-11-01 19 #include <linux/io.h>
ca24a648 Alan Tull 2016-11-01 20 #include <linux/kernel.h>
ca24a648 Alan Tull 2016-11-01 21 #include <linux/of_device.h>
ca24a648 Alan Tull 2016-11-01 22 #include <linux/module.h>
ca24a648 Alan Tull 2016-11-01 23 #include <linux/fpga/fpga-bridge.h>
ca24a648 Alan Tull 2016-11-01 24
ca24a648 Alan Tull 2016-11-01 25 #define FREEZE_CSR_STATUS_OFFSET 0
ca24a648 Alan Tull 2016-11-01 26 #define FREEZE_CSR_CTRL_OFFSET 4
ca24a648 Alan Tull 2016-11-01 27 #define FREEZE_CSR_ILLEGAL_REQ_OFFSET 8
ca24a648 Alan Tull 2016-11-01 28 #define FREEZE_CSR_REG_VERSION 12
ca24a648 Alan Tull 2016-11-01 29
ca24a648 Alan Tull 2016-11-01 30 #define FREEZE_CSR_SUPPORTED_VERSION 2
dd17cc7b Matthew Gerlach 2017-04-24 31 #define FREEZE_CSR_OFFICIAL_VERSION 0xad000003
ca24a648 Alan Tull 2016-11-01 32
ca24a648 Alan Tull 2016-11-01 33 #define FREEZE_CSR_STATUS_FREEZE_REQ_DONE BIT(0)
ca24a648 Alan Tull 2016-11-01 34 #define FREEZE_CSR_STATUS_UNFREEZE_REQ_DONE BIT(1)
ca24a648 Alan Tull 2016-11-01 35
ca24a648 Alan Tull 2016-11-01 36 #define FREEZE_CSR_CTRL_FREEZE_REQ BIT(0)
ca24a648 Alan Tull 2016-11-01 37 #define FREEZE_CSR_CTRL_RESET_REQ BIT(1)
ca24a648 Alan Tull 2016-11-01 38 #define FREEZE_CSR_CTRL_UNFREEZE_REQ BIT(2)
ca24a648 Alan Tull 2016-11-01 39
ca24a648 Alan Tull 2016-11-01 40 #define FREEZE_BRIDGE_NAME "freeze"
ca24a648 Alan Tull 2016-11-01 41
ca24a648 Alan Tull 2016-11-01 42 struct altera_freeze_br_data {
ca24a648 Alan Tull 2016-11-01 43 struct device *dev;
ca24a648 Alan Tull 2016-11-01 44 void __iomem *base_addr;
ca24a648 Alan Tull 2016-11-01 45 bool enable;
ca24a648 Alan Tull 2016-11-01 46 };
ca24a648 Alan Tull 2016-11-01 47
ca24a648 Alan Tull 2016-11-01 48 /*
ca24a648 Alan Tull 2016-11-01 49 * Poll status until status bit is set or we have a timeout.
ca24a648 Alan Tull 2016-11-01 50 */
ca24a648 Alan Tull 2016-11-01 51 static int altera_freeze_br_req_ack(struct altera_freeze_br_data *priv,
ca24a648 Alan Tull 2016-11-01 52 u32 timeout, u32 req_ack)
ca24a648 Alan Tull 2016-11-01 53 {
ca24a648 Alan Tull 2016-11-01 54 struct device *dev = priv->dev;
ca24a648 Alan Tull 2016-11-01 55 void __iomem *csr_illegal_req_addr = priv->base_addr +
ca24a648 Alan Tull 2016-11-01 56 FREEZE_CSR_ILLEGAL_REQ_OFFSET;
ca24a648 Alan Tull 2016-11-01 57 u32 status, illegal, ctrl;
ca24a648 Alan Tull 2016-11-01 58 int ret = -ETIMEDOUT;
ca24a648 Alan Tull 2016-11-01 59
ca24a648 Alan Tull 2016-11-01 60 do {
ca24a648 Alan Tull 2016-11-01 61 illegal = readl(csr_illegal_req_addr);
ca24a648 Alan Tull 2016-11-01 62 if (illegal) {
ca24a648 Alan Tull 2016-11-01 63 dev_err(dev, "illegal request detected 0x%x", illegal);
ca24a648 Alan Tull 2016-11-01 64
ca24a648 Alan Tull 2016-11-01 65 writel(1, csr_illegal_req_addr);
ca24a648 Alan Tull 2016-11-01 66
ca24a648 Alan Tull 2016-11-01 67 illegal = readl(csr_illegal_req_addr);
ca24a648 Alan Tull 2016-11-01 68 if (illegal)
ca24a648 Alan Tull 2016-11-01 69 dev_err(dev, "illegal request not cleared 0x%x",
ca24a648 Alan Tull 2016-11-01 70 illegal);
ca24a648 Alan Tull 2016-11-01 71
ca24a648 Alan Tull 2016-11-01 72 ret = -EINVAL;
ca24a648 Alan Tull 2016-11-01 73 break;
ca24a648 Alan Tull 2016-11-01 74 }
ca24a648 Alan Tull 2016-11-01 75
ca24a648 Alan Tull 2016-11-01 76 status = readl(priv->base_addr + FREEZE_CSR_STATUS_OFFSET);
ca24a648 Alan Tull 2016-11-01 77 dev_dbg(dev, "%s %x %x\n", __func__, status, req_ack);
ca24a648 Alan Tull 2016-11-01 78 status &= req_ack;
ca24a648 Alan Tull 2016-11-01 79 if (status) {
ca24a648 Alan Tull 2016-11-01 80 ctrl = readl(priv->base_addr + FREEZE_CSR_CTRL_OFFSET);
ca24a648 Alan Tull 2016-11-01 81 dev_dbg(dev, "%s request %x acknowledged %x %x\n",
ca24a648 Alan Tull 2016-11-01 82 __func__, req_ack, status, ctrl);
ca24a648 Alan Tull 2016-11-01 83 ret = 0;
ca24a648 Alan Tull 2016-11-01 84 break;
ca24a648 Alan Tull 2016-11-01 85 }
ca24a648 Alan Tull 2016-11-01 86
ca24a648 Alan Tull 2016-11-01 87 udelay(1);
ca24a648 Alan Tull 2016-11-01 88 } while (timeout--);
ca24a648 Alan Tull 2016-11-01 89
ca24a648 Alan Tull 2016-11-01 90 if (ret == -ETIMEDOUT)
ca24a648 Alan Tull 2016-11-01 91 dev_err(dev, "%s timeout waiting for 0x%x\n",
ca24a648 Alan Tull 2016-11-01 92 __func__, req_ack);
ca24a648 Alan Tull 2016-11-01 93
ca24a648 Alan Tull 2016-11-01 94 return ret;
ca24a648 Alan Tull 2016-11-01 95 }
ca24a648 Alan Tull 2016-11-01 96
ca24a648 Alan Tull 2016-11-01 97 static int altera_freeze_br_do_freeze(struct altera_freeze_br_data *priv,
ca24a648 Alan Tull 2016-11-01 98 u32 timeout)
ca24a648 Alan Tull 2016-11-01 99 {
ca24a648 Alan Tull 2016-11-01 100 struct device *dev = priv->dev;
ca24a648 Alan Tull 2016-11-01 101 void __iomem *csr_ctrl_addr = priv->base_addr +
ca24a648 Alan Tull 2016-11-01 102 FREEZE_CSR_CTRL_OFFSET;
ca24a648 Alan Tull 2016-11-01 103 u32 status;
ca24a648 Alan Tull 2016-11-01 104 int ret;
ca24a648 Alan Tull 2016-11-01 105
ca24a648 Alan Tull 2016-11-01 106 status = readl(priv->base_addr + FREEZE_CSR_STATUS_OFFSET);
ca24a648 Alan Tull 2016-11-01 107
ca24a648 Alan Tull 2016-11-01 @108 dev_dbg(dev, "%s %d %d\n", __func__, status, readl(csr_ctrl_addr));
ca24a648 Alan Tull 2016-11-01 109
ca24a648 Alan Tull 2016-11-01 110 if (status & FREEZE_CSR_STATUS_FREEZE_REQ_DONE) {
ca24a648 Alan Tull 2016-11-01 111 dev_dbg(dev, "%s bridge already disabled %d\n",

:::::: The code at line 108 was first introduced by commit
:::::: ca24a648f535a02b4163ca4f4d2e51869f155a3a fpga: add altera freeze bridge support

:::::: TO: Alan Tull <[email protected]>
:::::: CC: Greg Kroah-Hartman <[email protected]>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (9.93 kB)
.config.gz (41.55 kB)
Download all attachments

2017-06-09 09:49:12

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 03/10] fpga: allow to compile-test Altera FPGA bridge drivers

On Mon, Jun 05, 2017 at 02:07:34PM -0500, Alan Tull wrote:
> From: Tobias Klauser <[email protected]>
>
> Add COMPILE_TEST to the Kconfig entry for the Altera SoCFPGA FPGA
> Bridge. The Altera FPGA Freeze Bridge can also be used on Altera PEIe
> FPGAs, so the driver shouldn't depend on ARCH_SOCFPGA in the first place.
> This allows test-compiling the drivers on other architectures to catch
> compiler errors/warnings, e.g. due to API/header changes earlier on.
>
> Signed-off-by: Tobias Klauser <[email protected]>
> Signed-off-by: Alan Tull <[email protected]>
> ---
> drivers/fpga/Kconfig | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)

I'll drop this patch from the series, as kbuild reports errors with it
:(

2017-06-09 09:51:28

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 06/10] fpga manager: Add altera-ps-spi driver for Altera FPGAs

On Mon, Jun 05, 2017 at 02:07:37PM -0500, Alan Tull wrote:
> From: Joshua Clayton <[email protected]>
>
> altera-ps-spi loads FPGA firmware over SPI, using the "passive serial"
> interface on Altera Arria 10, Cyclone V or Stratix V 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]>
> Signed-off-by: Anatolij Gustschin <[email protected]>
> Signed-off-by: Alan Tull <[email protected]>

I get the following build error with this patch:

ERROR: "__spi_register_driver" [drivers/fpga/altera-ps-spi.ko] undefined!

So I'll just ignore this whole series, can you fix these issues up and
resend?

thanks,

greg k-h

2017-06-09 13:18:53

by Anatolij Gustschin

[permalink] [raw]
Subject: Re: [PATCH 06/10] fpga manager: Add altera-ps-spi driver for Altera FPGAs

On Fri, 9 Jun 2017 11:51:12 +0200
Greg Kroah-Hartman [email protected] wrote:
...
>I get the following build error with this patch:
>
>ERROR: "__spi_register_driver" [drivers/fpga/altera-ps-spi.ko] undefined!

it is due to enabled COMPILE_TEST and disabled CONFIG_SPI.

>So I'll just ignore this whole series, can you fix these issues up and
>resend?

would "depends on SPI || (COMPILE_TEST && SPI)" in Kconfig be a proper
fix for this? Anyone an idea?

Thanks,
Anatolij

2017-06-09 13:44:09

by Tobias Klauser

[permalink] [raw]
Subject: Re: [PATCH 03/10] fpga: allow to compile-test Altera FPGA bridge drivers

On 2017-06-09 at 11:49:02 +0200, Greg Kroah-Hartman <[email protected]> wrote:
> On Mon, Jun 05, 2017 at 02:07:34PM -0500, Alan Tull wrote:
> > From: Tobias Klauser <[email protected]>
> >
> > Add COMPILE_TEST to the Kconfig entry for the Altera SoCFPGA FPGA
> > Bridge. The Altera FPGA Freeze Bridge can also be used on Altera PEIe
> > FPGAs, so the driver shouldn't depend on ARCH_SOCFPGA in the first place.
> > This allows test-compiling the drivers on other architectures to catch
> > compiler errors/warnings, e.g. due to API/header changes earlier on.
> >
> > Signed-off-by: Tobias Klauser <[email protected]>
> > Signed-off-by: Alan Tull <[email protected]>
> > ---
> > drivers/fpga/Kconfig | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
>
> I'll drop this patch from the series, as kbuild reports errors with it
> :(

These are warnings from the m32r cross compiler, not errors. They are
due to the way readl() is defined on m32r (returning unsigned long
instead of u32, as all other architectures do, see [1]). On all other
architectures the patch doesn't cause any issues. There are also other
cases which are already in mainline where this issue appers, so I'd say
this patch rather uncovers the symptom of a problem rather than causing
it ;)

[1] https://marc.info/?l=linux-kernel&m=149252925326414

2017-06-09 22:08:01

by Joshua Clayton

[permalink] [raw]
Subject: Re: [PATCH 06/10] fpga manager: Add altera-ps-spi driver for Altera FPGAs

On Friday, June 9, 2017 3:18:40 PM PDT Anatolij Gustschin wrote:
> On Fri, 9 Jun 2017 11:51:12 +0200
> Greg Kroah-Hartman [email protected] wrote:
> ...
>
> >I get the following build error with this patch:
> >
> >ERROR: "__spi_register_driver" [drivers/fpga/altera-ps-spi.ko] undefined!
>
> it is due to enabled COMPILE_TEST and disabled CONFIG_SPI.
>
> >So I'll just ignore this whole series, can you fix these issues up and
> >resend?
>
> would "depends on SPI || (COMPILE_TEST && SPI)" in Kconfig be a proper
> fix for this? Anyone an idea?
>

> Thanks,
> Anatolij
I don't think that ends up being any different than "depends on SPI"
But, it is an SPI slave driver driver, after all.
COMPILE_TEST is intended to build stuff that can't work due to a lack of
hardware support, or lack of crosscompiler.
"selects SPI" would work, but is discouraged.

I just built successfully on native x86_64 with no problem, by enabling SPI.
"depends on SPI" is the right thing here.
I'm not sure when || COMPILE_TEST sneaked in there.

I'll submit a v13 with that small change.
--
~Joshua A Clayton

2017-06-10 03:55:50

by Alan Tull

[permalink] [raw]
Subject: Re: [PATCH 03/10] fpga: allow to compile-test Altera FPGA bridge drivers

On Fri, Jun 9, 2017 at 8:44 AM, Tobias Klauser <[email protected]> wrote:
> On 2017-06-09 at 11:49:02 +0200, Greg Kroah-Hartman <[email protected]> wrote:
>> On Mon, Jun 05, 2017 at 02:07:34PM -0500, Alan Tull wrote:
>> > From: Tobias Klauser <[email protected]>
>> >
>> > Add COMPILE_TEST to the Kconfig entry for the Altera SoCFPGA FPGA
>> > Bridge. The Altera FPGA Freeze Bridge can also be used on Altera PEIe
>> > FPGAs, so the driver shouldn't depend on ARCH_SOCFPGA in the first place.
>> > This allows test-compiling the drivers on other architectures to catch
>> > compiler errors/warnings, e.g. due to API/header changes earlier on.
>> >
>> > Signed-off-by: Tobias Klauser <[email protected]>
>> > Signed-off-by: Alan Tull <[email protected]>
>> > ---
>> > drivers/fpga/Kconfig | 4 ++--
>> > 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> I'll drop this patch from the series, as kbuild reports errors with it
>> :(
>
> These are warnings from the m32r cross compiler, not errors. They are
> due to the way readl() is defined on m32r (returning unsigned long
> instead of u32, as all other architectures do, see [1]). On all other
> architectures the patch doesn't cause any issues. There are also other
> cases which are already in mainline where this issue appers, so I'd say
> this patch rather uncovers the symptom of a problem rather than causing
> it ;)
>
> [1] https://marc.info/?l=linux-kernel&m=149252925326414

Yes the issue is the way m32r defines readl(). Tobias has pointed out
[2] other kernel patches cause the same warning. There is a trivial
suggested workaround in [1] above (declare a local u32 to take the
readl() value). Doing that would be effort to mask a known issue that
lies in the m32r code though.

Alan

[2] https://marc.info/?l=linux-kernel&m=149260369012554&w=2

2017-06-10 14:44:28

by Alan Tull

[permalink] [raw]
Subject: Re: [PATCH 06/10] fpga manager: Add altera-ps-spi driver for Altera FPGAs

On Fri, Jun 9, 2017 at 4:51 AM, Greg Kroah-Hartman
<[email protected]> wrote:
> On Mon, Jun 05, 2017 at 02:07:37PM -0500, Alan Tull wrote:
>> From: Joshua Clayton <[email protected]>
>>
>> altera-ps-spi loads FPGA firmware over SPI, using the "passive serial"
>> interface on Altera Arria 10, Cyclone V or Stratix V 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]>
>> Signed-off-by: Anatolij Gustschin <[email protected]>
>> Signed-off-by: Alan Tull <[email protected]>
>
> I get the following build error with this patch:
>
> ERROR: "__spi_register_driver" [drivers/fpga/altera-ps-spi.ko] undefined!
>
> So I'll just ignore this whole series, can you fix these issues up and
> resend?

Yes, will do.

Alan

>
> thanks,
>
> greg k-h