2016-12-18 05:34:25

by Joel Holdsworth

[permalink] [raw]
Subject: [PATCH v10 1/3] of: Add vendor prefix for Lattice Semiconductor

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: Alan Tull <[email protected]>
Acked-by: Moritz Fischer <[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 64fdc8c..7a87932 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -158,6 +158,7 @@ kosagi Sutajio Ko-Usagi PTE Ltd.
kyo Kyocera Corporation
lacie LaCie
lantiq Lantiq Semiconductor
+lattice Lattice Semiconductor
lenovo Lenovo Group Ltd.
lg LG Corporation
linux Linux-specific binding
--
2.7.4


2016-12-18 05:34:29

by Joel Holdsworth

[permalink] [raw]
Subject: [PATCH v10 2/3] Documentation: Add binding document for Lattice iCE40 FPGA manager

This adds documentation of the device tree bindings of the Lattice iCE40
FPGA driver for the FPGA manager framework.

Signed-off-by: Joel Holdsworth <[email protected]>
Acked-by: Rob Herring <[email protected]>
Acked-by: Alan Tull <[email protected]>
Acked-by: Moritz Fischer <[email protected]>
Acked-by: Marek Vasut <[email protected]>
---
.../bindings/fpga/lattice-ice40-fpga-mgr.txt | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
create mode 100644 Documentation/devicetree/bindings/fpga/lattice-ice40-fpga-mgr.txt

diff --git a/Documentation/devicetree/bindings/fpga/lattice-ice40-fpga-mgr.txt b/Documentation/devicetree/bindings/fpga/lattice-ice40-fpga-mgr.txt
new file mode 100644
index 0000000..7e7a78b
--- /dev/null
+++ b/Documentation/devicetree/bindings/fpga/lattice-ice40-fpga-mgr.txt
@@ -0,0 +1,21 @@
+Lattice iCE40 FPGA Manager
+
+Required properties:
+- compatible: Should contain "lattice,ice40-fpga-mgr"
+- reg: SPI chip select
+- spi-max-frequency: Maximum SPI frequency (>=1000000, <=25000000)
+- cdone-gpios: GPIO input connected to CDONE pin
+- reset-gpios: Active-low GPIO output connected to CRESET_B pin. Note
+ that unless the GPIO is held low during startup, the
+ FPGA will enter Master SPI mode and drive SCK with a
+ clock signal potentially jamming other devices on the
+ bus until the firmware is loaded.
+
+Example:
+ ice40: ice40@0 {
+ compatible = "lattice,ice40-fpga-mgr";
+ reg = <0>;
+ spi-max-frequency = <1000000>;
+ cdone-gpios = <&gpio 24 GPIO_ACTIVE_HIGH>;
+ reset-gpios = <&gpio 22 GPIO_ACTIVE_LOW>;
+ };
--
2.7.4

2016-12-18 05:34:33

by Joel Holdsworth

[permalink] [raw]
Subject: [PATCH v10 3/3] fpga: Add support for Lattice iCE40 FPGAs

This patch adds support to the FPGA manager for configuring the SRAM of
iCE40LM, iCE40LP, iCE40HX, iCE40 Ultra, iCE40 UltraLite and iCE40
UltraPlus devices, through slave SPI.

Signed-off-by: Joel Holdsworth <[email protected]>
Reviewed-by: Marek Vasut <[email protected]>
Reviewed-by: Moritz Fischer <[email protected]>
Acked-by: Alan Tull <[email protected]>
---
drivers/fpga/Kconfig | 6 ++
drivers/fpga/Makefile | 1 +
drivers/fpga/ice40-spi.c | 219 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 226 insertions(+)
create mode 100644 drivers/fpga/ice40-spi.c

diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index ce861a2..967cda4 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -20,6 +20,12 @@ config FPGA_REGION
FPGA Regions allow loading FPGA images under control of
the Device Tree.

+config FPGA_MGR_ICE40_SPI
+ tristate "Lattice iCE40 SPI"
+ depends on OF && SPI
+ help
+ FPGA manager driver support for Lattice iCE40 FPGAs 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 8df07bc..cc0d364 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_ICE40_SPI) += ice40-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
diff --git a/drivers/fpga/ice40-spi.c b/drivers/fpga/ice40-spi.c
new file mode 100644
index 0000000..16ea2c6
--- /dev/null
+++ b/drivers/fpga/ice40-spi.c
@@ -0,0 +1,219 @@
+/*
+ * FPGA Manager Driver for Lattice iCE40.
+ *
+ * Copyright (c) 2016 Joel Holdsworth
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This driver adds support to the FPGA manager for configuring the SRAM of
+ * Lattice iCE40 FPGAs through slave SPI.
+ */
+
+#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/stringify.h>
+
+#define ICE40_SPI_MAX_SPEED 25000000 /* Hz */
+#define ICE40_SPI_MIN_SPEED 1000000 /* Hz */
+
+#define ICE40_SPI_RESET_DELAY 1 /* us (>200ns) */
+#define ICE40_SPI_HOUSEKEEPING_DELAY 1200 /* us */
+
+#define ICE40_SPI_NUM_ACTIVATION_BYTES DIV_ROUND_UP(49, 8)
+
+struct ice40_fpga_priv {
+ struct spi_device *dev;
+ struct gpio_desc *reset;
+ struct gpio_desc *cdone;
+};
+
+static enum fpga_mgr_states ice40_fpga_ops_state(struct fpga_manager *mgr)
+{
+ struct ice40_fpga_priv *priv = mgr->priv;
+
+ return gpiod_get_value(priv->cdone) ? FPGA_MGR_STATE_OPERATING :
+ FPGA_MGR_STATE_UNKNOWN;
+}
+
+static int ice40_fpga_ops_write_init(struct fpga_manager *mgr,
+ struct fpga_image_info *info,
+ const char *buf, size_t count)
+{
+ struct ice40_fpga_priv *priv = mgr->priv;
+ struct spi_device *dev = priv->dev;
+ struct spi_message message;
+ struct spi_transfer assert_cs_then_reset_delay = {
+ .cs_change = 1,
+ .delay_usecs = ICE40_SPI_RESET_DELAY
+ };
+ struct spi_transfer housekeeping_delay_then_release_cs = {
+ .delay_usecs = ICE40_SPI_HOUSEKEEPING_DELAY
+ };
+ int ret;
+
+ if ((info->flags & FPGA_MGR_PARTIAL_RECONFIG)) {
+ dev_err(&dev->dev,
+ "Partial reconfiguration is not supported\n");
+ return -ENOTSUPP;
+ }
+
+ /* Lock the bus, assert CRESET_B and SS_B and delay >200ns */
+ spi_bus_lock(dev->master);
+
+ gpiod_set_value(priv->reset, 1);
+
+ spi_message_init(&message);
+ spi_message_add_tail(&assert_cs_then_reset_delay, &message);
+ ret = spi_sync_locked(dev, &message);
+
+ /* Come out of reset */
+ gpiod_set_value(priv->reset, 0);
+
+ /* Abort if the chip-select failed */
+ if (ret)
+ goto fail;
+
+ /* Check CDONE is de-asserted i.e. the FPGA is reset */
+ if (gpiod_get_value(priv->cdone)) {
+ dev_err(&dev->dev, "Device reset failed, CDONE is asserted\n");
+ ret = -EIO;
+ goto fail;
+ }
+
+ /* Wait for the housekeeping to complete, and release SS_B */
+ spi_message_init(&message);
+ spi_message_add_tail(&housekeeping_delay_then_release_cs, &message);
+ ret = spi_sync_locked(dev, &message);
+
+fail:
+ spi_bus_unlock(dev->master);
+
+ return ret;
+}
+
+static int ice40_fpga_ops_write(struct fpga_manager *mgr,
+ const char *buf, size_t count)
+{
+ struct ice40_fpga_priv *priv = mgr->priv;
+
+ return spi_write(priv->dev, buf, count);
+}
+
+static int ice40_fpga_ops_write_complete(struct fpga_manager *mgr,
+ struct fpga_image_info *info)
+{
+ struct ice40_fpga_priv *priv = mgr->priv;
+ struct spi_device *dev = priv->dev;
+ const u8 padding[ICE40_SPI_NUM_ACTIVATION_BYTES] = {0};
+
+ /* Check CDONE is asserted */
+ if (!gpiod_get_value(priv->cdone)) {
+ dev_err(&dev->dev,
+ "CDONE was not asserted after firmware transfer\n");
+ return -EIO;
+ }
+
+ /* Send of zero-padding to activate the firmware */
+ return spi_write(dev, padding, sizeof(padding));
+}
+
+static const struct fpga_manager_ops ice40_fpga_ops = {
+ .state = ice40_fpga_ops_state,
+ .write_init = ice40_fpga_ops_write_init,
+ .write = ice40_fpga_ops_write,
+ .write_complete = ice40_fpga_ops_write_complete,
+};
+
+static int ice40_fpga_probe(struct spi_device *spi)
+{
+ struct device *dev = &spi->dev;
+ struct device_node *np = spi->dev.of_node;
+ struct ice40_fpga_priv *priv;
+ int ret;
+
+ if (!np) {
+ dev_err(dev, "No Device Tree entry\n");
+ return -EINVAL;
+ }
+
+ priv = devm_kzalloc(&spi->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->dev = spi;
+
+ /* Check board setup data. */
+ if (spi->max_speed_hz > ICE40_SPI_MAX_SPEED) {
+ dev_err(dev, "SPI speed is too high, maximum speed is "
+ __stringify(ICE40_SPI_MAX_SPEED) "\n");
+ return -EINVAL;
+ }
+
+ if (spi->max_speed_hz < ICE40_SPI_MIN_SPEED) {
+ dev_err(dev, "SPI speed is too low, minimum speed is "
+ __stringify(ICE40_SPI_MIN_SPEED) "\n");
+ return -EINVAL;
+ }
+
+ if (spi->mode & SPI_CPHA) {
+ dev_err(dev, "Bad SPI mode, CPHA not supported\n");
+ return -EINVAL;
+ }
+
+ /* Set up the GPIOs */
+ priv->cdone = devm_gpiod_get(dev, "cdone", GPIOD_IN);
+ if (IS_ERR(priv->cdone)) {
+ dev_err(dev, "Failed to get CDONE GPIO: %ld\n",
+ PTR_ERR(priv->cdone));
+ return -EINVAL;
+ }
+
+ priv->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(priv->reset)) {
+ dev_err(dev, "Failed to get CRESET_B GPIO: %ld\n",
+ PTR_ERR(priv->reset));
+ return -EINVAL;
+ }
+
+ /* Register with the FPGA manager */
+ ret = fpga_mgr_register(dev, "Lattice iCE40 FPGA Manager",
+ &ice40_fpga_ops, priv);
+ if (ret) {
+ dev_err(dev, "Unable to register FPGA manager");
+ return ret;
+ }
+
+ return 0;
+}
+
+static int ice40_fpga_remove(struct spi_device *spi)
+{
+ fpga_mgr_unregister(&spi->dev);
+ return 0;
+}
+
+static const struct of_device_id ice40_fpga_of_match[] = {
+ { .compatible = "lattice,ice40-fpga-mgr", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, ice40_fpga_of_match);
+
+static struct spi_driver ice40_fpga_driver = {
+ .probe = ice40_fpga_probe,
+ .remove = ice40_fpga_remove,
+ .driver = {
+ .name = "ice40spi",
+ .of_match_table = of_match_ptr(ice40_fpga_of_match),
+ },
+};
+
+module_spi_driver(ice40_fpga_driver);
+
+MODULE_AUTHOR("Joel Holdsworth <[email protected]>");
+MODULE_DESCRIPTION("Lattice iCE40 FPGA Manager");
+MODULE_LICENSE("GPL v2");
--
2.7.4

2016-12-18 19:18:12

by Vladimir Zapolskiy

[permalink] [raw]
Subject: Re: [PATCH v10 3/3] fpga: Add support for Lattice iCE40 FPGAs

Hi Joel,

On 12/18/2016 07:33 AM, Joel Holdsworth wrote:
> This patch adds support to the FPGA manager for configuring the SRAM of
> iCE40LM, iCE40LP, iCE40HX, iCE40 Ultra, iCE40 UltraLite and iCE40
> UltraPlus devices, through slave SPI.
>
> Signed-off-by: Joel Holdsworth <[email protected]>
> Reviewed-by: Marek Vasut <[email protected]>
> Reviewed-by: Moritz Fischer <[email protected]>
> Acked-by: Alan Tull <[email protected]>
> ---

[snip]

> +
> +static int ice40_fpga_probe(struct spi_device *spi)
> +{
> + struct device *dev = &spi->dev;
> + struct device_node *np = spi->dev.of_node;

struct device_node *np = dev->of_node?

And actually I don't see why local variable 'np' is needed in the function,
see the next comment.

> + struct ice40_fpga_priv *priv;
> + int ret;
> +
> + if (!np) {
> + dev_err(dev, "No Device Tree entry\n");
> + return -EINVAL;
> + }

I would suggest to remove this check completely, I don't see a good
reason why it is needed.

> +
> + priv = devm_kzalloc(&spi->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + priv->dev = spi;
> +
> + /* Check board setup data. */
> + if (spi->max_speed_hz > ICE40_SPI_MAX_SPEED) {
> + dev_err(dev, "SPI speed is too high, maximum speed is "
> + __stringify(ICE40_SPI_MAX_SPEED) "\n");
> + return -EINVAL;
> + }
> +
> + if (spi->max_speed_hz < ICE40_SPI_MIN_SPEED) {
> + dev_err(dev, "SPI speed is too low, minimum speed is "
> + __stringify(ICE40_SPI_MIN_SPEED) "\n");
> + return -EINVAL;
> + }
> +
> + if (spi->mode & SPI_CPHA) {
> + dev_err(dev, "Bad SPI mode, CPHA not supported\n");
> + return -EINVAL;
> + }
> +
> + /* Set up the GPIOs */
> + priv->cdone = devm_gpiod_get(dev, "cdone", GPIOD_IN);
> + if (IS_ERR(priv->cdone)) {
> + dev_err(dev, "Failed to get CDONE GPIO: %ld\n",
> + PTR_ERR(priv->cdone));
> + return -EINVAL;

You should do 'return PTR_ERR(priv->cdone)' here, this will allow
to handle at lease -EPROBE_DEFER correctly.

> + }
> +
> + priv->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
> + if (IS_ERR(priv->reset)) {
> + dev_err(dev, "Failed to get CRESET_B GPIO: %ld\n",
> + PTR_ERR(priv->reset));
> + return -EINVAL;

Same as above.

> + }
> +
> + /* Register with the FPGA manager */
> + ret = fpga_mgr_register(dev, "Lattice iCE40 FPGA Manager",
> + &ice40_fpga_ops, priv);
> + if (ret) {
> + dev_err(dev, "Unable to register FPGA manager");
> + return ret;

I would suggest to do just 'return fpga_mgr_register(dev, ....);' here,
if the driver is not registered, the core will let the user know about it.

> + }
> +
> + return 0;
> +}
> +

--
With best wishes,
Vladimir

2016-12-19 11:16:53

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v10 2/3] Documentation: Add binding document for Lattice iCE40 FPGA manager

Hi Joel,

On Sun, Dec 18, 2016 at 6:33 AM, Joel Holdsworth
<[email protected]> wrote:
> This adds documentation of the device tree bindings of the Lattice iCE40
> FPGA driver for the FPGA manager framework.
>
> Signed-off-by: Joel Holdsworth <[email protected]>
> Acked-by: Rob Herring <[email protected]>
> Acked-by: Alan Tull <[email protected]>
> Acked-by: Moritz Fischer <[email protected]>
> Acked-by: Marek Vasut <[email protected]>

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/fpga/lattice-ice40-fpga-mgr.txt
> @@ -0,0 +1,21 @@
> +Lattice iCE40 FPGA Manager
> +
> +Required properties:
> +- compatible: Should contain "lattice,ice40-fpga-mgr"
> +- reg: SPI chip select
> +- spi-max-frequency: Maximum SPI frequency (>=1000000, <=25000000)
> +- cdone-gpios: GPIO input connected to CDONE pin
> +- reset-gpios: Active-low GPIO output connected to CRESET_B pin. Note
> + that unless the GPIO is held low during startup, the
> + FPGA will enter Master SPI mode and drive SCK with a
> + clock signal potentially jamming other devices on the
> + bus until the firmware is loaded.
> +
> +Example:
> + ice40: ice40@0 {

As per ePAPR, node names should be generic names, e.g. "fpga@0".
Sorry for not noticing before.

> + compatible = "lattice,ice40-fpga-mgr";
> + reg = <0>;
> + spi-max-frequency = <1000000>;
> + cdone-gpios = <&gpio 24 GPIO_ACTIVE_HIGH>;
> + reset-gpios = <&gpio 22 GPIO_ACTIVE_LOW>;
> + };

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds