[v4 0/3] Add Efinix FPGA SPI programming support

[v4 0/3] Add Efinix FPGA SPI programming support

[iansdannapel]

From: Ian Dannapel <iansdannapel@gmail.com>

This patch series introduces support for Efinix FPGA devices
through SPI-based programming.

Currently, only Trion devices are tested. Topaz and Titanium series are 
theoretically supported because of the similarity with Trion, but the
driver is only documentation based.

Changes since v3:
- major rework on the driver spi write approach
    - indirect CS assert using spi_transfer instead of duplicating the
  SPI controller property
  - locked SPI bus transfer to avoid possible conflicts with other
  devices on the bus
- file name and compatible strings renamed to better match device
- minor improvements/fixes


Ian Dannapel (3):
  dt-bindings: vendor-prefix: Add prefix for Efinix, Inc.
  dt-bindings: fpga: Add Efinix SPI programming bindings
  fpga-mgr: Add Efinix SPI programming driver

 .../devicetree/bindings/fpga/efinix,spi.yaml  |  81 +++++++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 drivers/fpga/Kconfig                          |   7 +
 drivers/fpga/Makefile                         |   1 +
 drivers/fpga/efinix-spi.c                     | 212 ++++++++++++++++++
 5 files changed, 303 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/fpga/efinix,spi.yaml
 create mode 100644 drivers/fpga/efinix-spi.c

-- 
2.43.0

[v4 1/3] dt-bindings: vendor-prefix: Add prefix for Efinix, Inc.

[iansdannapel]

From: Ian Dannapel <iansdannapel@gmail.com>

Add entry for Efinix, Inc. (https://www.efinixinc.com/)

Signed-off-by: Ian Dannapel <iansdannapel@gmail.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 5079ca6ce1d1..f0e0da4e1ec0 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -438,6 +438,8 @@ patternProperties:
     description: Emtop Embedded Solutions
   "^eeti,.*":
     description: eGalax_eMPIA Technology Inc
+  "^efinix,.*":
+    description: Efinix, Inc.
   "^einfochips,.*":
     description: Einfochips
   "^eink,.*":
-- 
2.43.0

[v4 2/3] dt-bindings: fpga: Add Efinix SPI programming bindings

[iansdannapel]

From: Ian Dannapel <iansdannapel@gmail.com>

Add device tree bindings documentation for configuring Efinix FPGA
using serial SPI passive programming mode.

Signed-off-by: Ian Dannapel <iansdannapel@gmail.com>
---
 .../devicetree/bindings/fpga/efinix,spi.yaml  | 81 +++++++++++++++++++
 1 file changed, 81 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/fpga/efinix,spi.yaml

diff --git a/Documentation/devicetree/bindings/fpga/efinix,spi.yaml b/Documentation/devicetree/bindings/fpga/efinix,spi.yaml
new file mode 100644
index 000000000000..145c96f38e45
--- /dev/null
+++ b/Documentation/devicetree/bindings/fpga/efinix,spi.yaml
@@ -0,0 +1,81 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/fpga/efinix,spi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Efinix SPI FPGA Manager
+
+maintainers:
+  - Ian Dannapel <iansdannapel@gmail.com>
+
+description: |
+  Efinix FPGAs (Trion, Topaz, and Titanium families) support loading bitstreams
+  through "SPI Passive Mode".
+  Note 1: Only bus width 1x is supported.
+  Note 2: Additional pins hogs for bus width configuration must be set
+  elsewhere, if necessary.
+  Note 3: Topaz and Titanium support is based on documentation but remains
+  untested.
+
+  References:
+  - https://www.efinixinc.com/docs/an006-configuring-trion-fpgas-v6.3.pdf
+  - https://www.efinixinc.com/docs/an033-configuring-titanium-fpgas-v2.8.pdf
+  - https://www.efinixinc.com/docs/an061-configuring-topaz-fpgas-v1.1.pdf
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+  compatible:
+    enum:
+      - efinix,trion-spi
+      - efinix,titanium-spi
+      - efinix,topaz-spi
+      - efinix,fpga-spi
+
+  spi-cpha: true
+
+  spi-cpol: true
+
+  spi-max-frequency:
+    maximum: 25000000
+
+  reg:
+    maxItems: 1
+
+  reset-gpios:
+    description:
+      reset and re-configuration trigger pin (low active)
+    maxItems: 1
+
+  cdone-gpios:
+    description:
+      optional configuration done status pin (high active)
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - reset-gpios
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    spi {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      cs-gpios = <&gpio5 13 GPIO_ACTIVE_LOW>;
+      fpga-mgr@0 {
+        compatible = "efinix,trion-spi";
+        reg = <0>;
+        spi-max-frequency = <25000000>;
+        spi-cpha;
+        spi-cpol;
+        reset-gpios = <&gpio4 17 GPIO_ACTIVE_LOW>;
+        cdone-gpios = <&gpio0 9 GPIO_ACTIVE_HIGH>;
+      };
+    };
+...
-- 
2.43.0

[v4 3/3] fpga-mgr: Add Efinix SPI programming driver

[iansdannapel]

From: Ian Dannapel <iansdannapel@gmail.com>

Add a new driver for loading binary firmware to configuration
RAM using "SPI passive mode" on Efinix FPGAs.

Signed-off-by: Ian Dannapel <iansdannapel@gmail.com>
---
 drivers/fpga/Kconfig      |   7 ++
 drivers/fpga/Makefile     |   1 +
 drivers/fpga/efinix-spi.c | 212 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 220 insertions(+)
 create mode 100644 drivers/fpga/efinix-spi.c

diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index 37b35f58f0df..b5d60ba62900 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -83,6 +83,13 @@ config FPGA_MGR_XILINX_SPI
 	  FPGA manager driver support for Xilinx FPGA configuration
 	  over slave serial interface.
 
+config FPGA_MGR_EFINIX_SPI
+	tristate "Efinix FPGA configuration over SPI"
+	depends on SPI
+	help
+	  FPGA manager driver support for Efinix FPGAs configuration over SPI
+	  (passive mode only).
+
 config FPGA_MGR_ICE40_SPI
 	tristate "Lattice iCE40 SPI"
 	depends on OF && SPI
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index aeb89bb13517..adbd51d2cd1e 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_FPGA_MGR_TS73XX)		+= ts73xx-fpga.o
 obj-$(CONFIG_FPGA_MGR_XILINX_CORE)	+= xilinx-core.o
 obj-$(CONFIG_FPGA_MGR_XILINX_SELECTMAP)	+= xilinx-selectmap.o
 obj-$(CONFIG_FPGA_MGR_XILINX_SPI)	+= xilinx-spi.o
+obj-$(CONFIG_FPGA_MGR_EFINIX_SPI)	+= efinix-spi.o
 obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA)	+= zynq-fpga.o
 obj-$(CONFIG_FPGA_MGR_ZYNQMP_FPGA)	+= zynqmp-fpga.o
 obj-$(CONFIG_FPGA_MGR_VERSAL_FPGA)	+= versal-fpga.o
diff --git a/drivers/fpga/efinix-spi.c b/drivers/fpga/efinix-spi.c
new file mode 100644
index 000000000000..07885110a8a8
--- /dev/null
+++ b/drivers/fpga/efinix-spi.c
@@ -0,0 +1,212 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * FPGA Manager Driver for Efinix
+ *
+ * Copyright (C) 2025 iris-GmbH infrared & intelligent sensors
+ *
+ * Ian Dannapel <iansdannapel@gmail.com>
+ *
+ * Load Efinix FPGA firmware over SPI using the serial configuration interface.
+ *
+ * Note 1: Only passive mode (host initiates transfer) is currently supported.
+ * Note 2: Topaz and Titanium support is based on documentation but remains
+ * untested.
+ */
+
+#include <linux/delay.h>
+#include <linux/fpga/fpga-mgr.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/spi/spi.h>
+
+struct efinix_spi_conf {
+	struct spi_device *spi;
+	struct gpio_desc *cdone;
+	struct gpio_desc *reset;
+};
+
+static void efinix_spi_reset(struct efinix_spi_conf *conf)
+{
+	gpiod_set_value(conf->reset, 1);
+	/* tCRESET_N > 320 ns */
+	usleep_range(1, 2);
+	gpiod_set_value(conf->reset, 0);
+
+	/* tDMIN > 32 us */
+	usleep_range(35, 40);
+}
+
+static enum fpga_mgr_states efinix_spi_state(struct fpga_manager *mgr)
+{
+	struct efinix_spi_conf *conf = mgr->priv;
+
+	if (conf->cdone && gpiod_get_value(conf->cdone) == 1)
+		return FPGA_MGR_STATE_OPERATING;
+
+	return FPGA_MGR_STATE_UNKNOWN;
+}
+
+static int efinix_spi_write_init(struct fpga_manager *mgr,
+				 struct fpga_image_info *info,
+				 const char *buf, size_t count)
+{
+	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
+		dev_err(&mgr->dev, "Partial reconfiguration not supported\n");
+		return -EOPNOTSUPP;
+	}
+	return 0;
+}
+
+static int efinix_spi_write(struct fpga_manager *mgr, const char *buf,
+			    size_t count)
+{
+	struct efinix_spi_conf *conf = mgr->priv;
+	int ret;
+	struct spi_message message;
+	struct spi_transfer assert_cs = {
+		.cs_change = 1
+	};
+	struct spi_transfer write_xfer = {
+		.tx_buf = buf,
+		.len = count
+	};
+	struct spi_transfer clk_cycles = {
+		.len = 13,  // > 100 clock cycles
+		.tx_buf = NULL
+	};
+	u8 *dummy_buf;
+
+	dummy_buf = kzalloc(13, GFP_KERNEL);
+	if (!dummy_buf) {
+		ret = -ENOMEM;
+		goto fail;
+	}
+
+	spi_bus_lock(conf->spi->controller);
+	spi_message_init(&message);
+	spi_message_add_tail(&assert_cs, &message);
+	ret = spi_sync_locked(conf->spi, &message);
+	if (ret)
+		goto fail_unlock;
+
+	/* reset with asserted cs */
+	efinix_spi_reset(conf);
+
+	spi_message_init(&message);
+	spi_message_add_tail(&write_xfer, &message);
+
+	clk_cycles.tx_buf = dummy_buf;
+	spi_message_add_tail(&clk_cycles, &message);
+
+	ret = spi_sync_locked(conf->spi, &message);
+	if (ret)
+		dev_err(&mgr->dev, "SPI error in firmware write: %d\n", ret);
+
+fail_unlock:
+	spi_bus_unlock(conf->spi->controller);
+	kfree(dummy_buf);
+fail:
+	return ret;
+}
+
+static int efinix_spi_write_complete(struct fpga_manager *mgr,
+				     struct fpga_image_info *info)
+{
+	struct efinix_spi_conf *conf = mgr->priv;
+	unsigned long timeout =
+		jiffies + usecs_to_jiffies(info->config_complete_timeout_us);
+	bool expired = false;
+	int done;
+
+	if (conf->cdone) {
+		while (!expired) {
+			expired = time_after(jiffies, timeout);
+
+			done = gpiod_get_value(conf->cdone);
+			if (done < 0)
+				return done;
+
+			if (done)
+				break;
+		}
+	}
+
+	if (expired)
+		return -ETIMEDOUT;
+
+	/* tUSER > 25 us */
+	usleep_range(30, 35);
+	return 0;
+}
+
+static const struct fpga_manager_ops efinix_spi_ops = {
+	.state = efinix_spi_state,
+	.write_init = efinix_spi_write_init,
+	.write = efinix_spi_write,
+	.write_complete = efinix_spi_write_complete,
+};
+
+static int efinix_spi_probe(struct spi_device *spi)
+{
+	struct efinix_spi_conf *conf;
+	struct fpga_manager *mgr;
+
+	conf = devm_kzalloc(&spi->dev, sizeof(*conf), GFP_KERNEL);
+	if (!conf)
+		return -ENOMEM;
+
+	conf->spi = spi;
+
+	conf->reset = devm_gpiod_get(&spi->dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(conf->reset))
+		return dev_err_probe(&spi->dev, PTR_ERR(conf->reset),
+				     "Failed to get RESET gpio\n");
+
+	if (!(spi->mode & SPI_CPHA) || !(spi->mode & SPI_CPOL))
+		return dev_err_probe(&spi->dev, -EINVAL,
+				     "Unsupported SPI mode, set CPHA and CPOL\n");
+
+	conf->cdone = devm_gpiod_get_optional(&spi->dev, "cdone", GPIOD_IN);
+	if (IS_ERR(conf->cdone))
+		return dev_err_probe(&spi->dev, PTR_ERR(conf->cdone),
+				     "Failed to get CDONE gpio\n");
+
+	mgr = devm_fpga_mgr_register(&spi->dev,
+				     "Efinix FPGA Manager",
+				     &efinix_spi_ops, conf);
+
+	return PTR_ERR_OR_ZERO(mgr);
+}
+
+static const struct of_device_id efinix_spi_of_match[] = {
+	{ .compatible = "efinix,trion-spi", },
+	{ .compatible = "efinix,titanium-spi", },
+	{ .compatible = "efinix,topaz-spi", },
+	{ .compatible = "efinix,fpga-spi", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, efinix_spi_of_match);
+
+static const struct spi_device_id efinix_ids[] = {
+	{ "trion-spi", 0 },
+	{ "titanium-spi", 0 },
+	{ "topaz-spi", 0 },
+	{},
+};
+MODULE_DEVICE_TABLE(spi, efinix_ids);
+
+static struct spi_driver efinix_spi_driver = {
+	.driver = {
+		.name = "efinix-spi",
+		.of_match_table = efinix_spi_of_match,
+	},
+	.probe = efinix_spi_probe,
+	.id_table = efinix_ids,
+};
+
+module_spi_driver(efinix_spi_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Ian Dannapel <iansdannapel@gmail.com>");
+MODULE_DESCRIPTION("Efinix FPGA SPI Programming Driver (Topaz/Titanium untested)");
-- 
2.43.0

Re: [v4 2/3] dt-bindings: fpga: Add Efinix SPI programming bindings

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3361 bytes --]

On Fri, Feb 28, 2025 at 10:47:31AM +0100, iansdannapel@gmail.com wrote:
> From: Ian Dannapel <iansdannapel@gmail.com>
> 
> Add device tree bindings documentation for configuring Efinix FPGA
> using serial SPI passive programming mode.
> 
> Signed-off-by: Ian Dannapel <iansdannapel@gmail.com>
> ---
>  .../devicetree/bindings/fpga/efinix,spi.yaml  | 81 +++++++++++++++++++
>  1 file changed, 81 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/fpga/efinix,spi.yaml
> 
> diff --git a/Documentation/devicetree/bindings/fpga/efinix,spi.yaml b/Documentation/devicetree/bindings/fpga/efinix,spi.yaml
> new file mode 100644
> index 000000000000..145c96f38e45
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/fpga/efinix,spi.yaml

Filename matching a compatible please.

> @@ -0,0 +1,81 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/fpga/efinix,spi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Efinix SPI FPGA Manager
> +
> +maintainers:
> +  - Ian Dannapel <iansdannapel@gmail.com>
> +
> +description: |
> +  Efinix FPGAs (Trion, Topaz, and Titanium families) support loading bitstreams
> +  through "SPI Passive Mode".
> +  Note 1: Only bus width 1x is supported.
> +  Note 2: Additional pins hogs for bus width configuration must be set
> +  elsewhere, if necessary.
> +  Note 3: Topaz and Titanium support is based on documentation but remains
> +  untested.

Points 1 and 3 here seem to be driver limitations, and shouldn't really
be present in a document describing the hardware?

> +
> +  References:
> +  - https://www.efinixinc.com/docs/an006-configuring-trion-fpgas-v6.3.pdf
> +  - https://www.efinixinc.com/docs/an033-configuring-titanium-fpgas-v2.8.pdf
> +  - https://www.efinixinc.com/docs/an061-configuring-topaz-fpgas-v1.1.pdf
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - efinix,trion-spi
> +      - efinix,titanium-spi
> +      - efinix,topaz-spi

> +      - efinix,fpga-spi

What hardware does this device represent? Other ones are obvious matches
to the families you mention, but what is this one?

Cheers,
Conor.

> +
> +  spi-cpha: true
> +
> +  spi-cpol: true
> +
> +  spi-max-frequency:
> +    maximum: 25000000
> +
> +  reg:
> +    maxItems: 1
> +
> +  reset-gpios:
> +    description:
> +      reset and re-configuration trigger pin (low active)
> +    maxItems: 1
> +
> +  cdone-gpios:
> +    description:
> +      optional configuration done status pin (high active)
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - reset-gpios
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    spi {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      cs-gpios = <&gpio5 13 GPIO_ACTIVE_LOW>;
> +      fpga-mgr@0 {
> +        compatible = "efinix,trion-spi";
> +        reg = <0>;
> +        spi-max-frequency = <25000000>;
> +        spi-cpha;
> +        spi-cpol;
> +        reset-gpios = <&gpio4 17 GPIO_ACTIVE_LOW>;
> +        cdone-gpios = <&gpio0 9 GPIO_ACTIVE_HIGH>;
> +      };
> +    };
> +...
> -- 
> 2.43.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [v4 1/3] dt-bindings: vendor-prefix: Add prefix for Efinix, Inc.

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1016 bytes --]

On Fri, Feb 28, 2025 at 10:47:30AM +0100, iansdannapel@gmail.com wrote:
> From: Ian Dannapel <iansdannapel@gmail.com>
> 
> Add entry for Efinix, Inc. (https://www.efinixinc.com/)
> 
> Signed-off-by: Ian Dannapel <iansdannapel@gmail.com>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

> ---
>  Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> index 5079ca6ce1d1..f0e0da4e1ec0 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> @@ -438,6 +438,8 @@ patternProperties:
>      description: Emtop Embedded Solutions
>    "^eeti,.*":
>      description: eGalax_eMPIA Technology Inc
> +  "^efinix,.*":
> +    description: Efinix, Inc.
>    "^einfochips,.*":
>      description: Einfochips
>    "^eink,.*":
> -- 
> 2.43.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [v4 1/3] dt-bindings: vendor-prefix: Add prefix for Efinix, Inc.

[Krzysztof Kozlowski]

On 28/02/2025 10:47, iansdannapel@gmail.com wrote:
> From: Ian Dannapel <iansdannapel@gmail.com>
> 
> Add entry for Efinix, Inc. (https://www.efinixinc.com/)
> 
> Signed-off-by: Ian Dannapel <iansdannapel@gmail.com>

<form letter>
This is a friendly reminder during the review process.

It looks like you received a tag and forgot to add it.

If you do not know the process, here is a short explanation:
Please add Acked-by/Reviewed-by/Tested-by tags when posting new versions
of patchset, under or above your Signed-off-by tag, unless patch changed
significantly (e.g. new properties added to the DT bindings). Tag is
"received", when provided in a message replied to you on the mailing
list. Tools like b4 can help here. However, there's no need to repost
patches *only* to add the tags. The upstream maintainer will do that for
tags received on the version they apply.

Please read:
https://elixir.bootlin.com/linux/v6.12-rc3/source/Documentation/process/submitting-patches.rst#L577

If a tag was not added on purpose, please state why and what changed.
</form letter>

Best regards,
Krzysztof

Re: [v4 2/3] dt-bindings: fpga: Add Efinix SPI programming bindings

[Krzysztof Kozlowski]

On 28/02/2025 10:47, iansdannapel@gmail.com wrote:
> +
> +  References:
> +  - https://www.efinixinc.com/docs/an006-configuring-trion-fpgas-v6.3.pdf
> +  - https://www.efinixinc.com/docs/an033-configuring-titanium-fpgas-v2.8.pdf
> +  - https://www.efinixinc.com/docs/an061-configuring-topaz-fpgas-v1.1.pdf
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - efinix,trion-spi
> +      - efinix,titanium-spi
> +      - efinix,topaz-spi


Same comments as before about compatibility. Address or implement.

> +      - efinix,fpga-spi


And this one is for which device? It is not even used.


Best regards,
Krzysztof

Re: [v4 2/3] dt-bindings: fpga: Add Efinix SPI programming bindings

[Ian Dannapel]

Hi Conor, thanks for the quick response.

On Fri, Feb 28, 2025 at 7:28 PM Conor Dooley <conor@kernel.org> wrote:
> > +description: |
> > +  Efinix FPGAs (Trion, Topaz, and Titanium families) support loading bitstreams
> > +  through "SPI Passive Mode".
> > +  Note 1: Only bus width 1x is supported.
> > +  Note 2: Additional pins hogs for bus width configuration must be set
> > +  elsewhere, if necessary.
> > +  Note 3: Topaz and Titanium support is based on documentation but remains
> > +  untested.
>
> Points 1 and 3 here seem to be driver limitations, and shouldn't really
> be present in a document describing the hardware?
>
Yes, they are driver limitations and probably do not belong here.

> > +properties:
> > +  compatible:
> > +    enum:
> > +      - efinix,trion-spi
> > +      - efinix,titanium-spi
> > +      - efinix,topaz-spi
>
> > +      - efinix,fpga-spi
>
> What hardware does this device represent? Other ones are obvious matches
> to the families you mention, but what is this one?
The proposed compatible is a generic fallback for any Efinix FPGA Series.

Regards
Ian

Re: [v4 2/3] dt-bindings: fpga: Add Efinix SPI programming bindings

[Ian Dannapel]

Hi Krzysztof, thanks for the quick response.

On Sat, Mar 1, 2025 at 2:13 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 28/02/2025 10:47, iansdannapel@gmail.com wrote:
> > +
> > +  References:
> > +  - https://www.efinixinc.com/docs/an006-configuring-trion-fpgas-v6.3.pdf
> > +  - https://www.efinixinc.com/docs/an033-configuring-titanium-fpgas-v2.8.pdf
> > +  - https://www.efinixinc.com/docs/an061-configuring-topaz-fpgas-v1.1.pdf
> > +
> > +allOf:
> > +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - efinix,trion-spi
> > +      - efinix,titanium-spi
> > +      - efinix,topaz-spi
>
>
> Same comments as before about compatibility. Address or implement.
>
The compatibles are implemented in the device match table, what
exactly should be addressed or implemented here?

> > +      - efinix,fpga-spi
>
>
> And this one is for which device? It is not even used.
The proposed compatible is a generic fallback for any Efinix FPGA
Series. Isn't it used if the compatible is part of the drivers match
table?

Regards,
Ian

Re: [v4 2/3] dt-bindings: fpga: Add Efinix SPI programming bindings

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1831 bytes --]

On Mon, Mar 03, 2025 at 11:10:53AM +0100, Ian Dannapel wrote:
> Hi Conor, thanks for the quick response.
> 
> On Fri, Feb 28, 2025 at 7:28 PM Conor Dooley <conor@kernel.org> wrote:
> > > +description: |
> > > +  Efinix FPGAs (Trion, Topaz, and Titanium families) support loading bitstreams
> > > +  through "SPI Passive Mode".
> > > +  Note 1: Only bus width 1x is supported.
> > > +  Note 2: Additional pins hogs for bus width configuration must be set
> > > +  elsewhere, if necessary.
> > > +  Note 3: Topaz and Titanium support is based on documentation but remains
> > > +  untested.
> >
> > Points 1 and 3 here seem to be driver limitations, and shouldn't really
> > be present in a document describing the hardware?
> >
> Yes, they are driver limitations and probably do not belong here.
> 
> > > +properties:
> > > +  compatible:
> > > +    enum:
> > > +      - efinix,trion-spi
> > > +      - efinix,titanium-spi
> > > +      - efinix,topaz-spi
> >
> > > +      - efinix,fpga-spi
> >
> > What hardware does this device represent? Other ones are obvious matches
> > to the families you mention, but what is this one?

> The proposed compatible is a generic fallback for any Efinix FPGA Series.

If it is a fallback, your binding should look like:
compatible:
  items:
    - enum:
        - efinix,trion-spi
        - efinix,titanium-spi
        - efinix,topaz-spi
    - const: efinix,fpga-spi

|+static const struct of_device_id efinix_spi_of_match[] = {
|+       { .compatible = "efinix,trion-spi", },
|+       { .compatible = "efinix,titanium-spi", },
|+       { .compatible = "efinix,topaz-spi", },

And these three compatibles can/should be removed from the driver, since
the fallback is required.

|+       { .compatible = "efinix,fpga-spi", },
|+       {}
|+};


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [v4 2/3] dt-bindings: fpga: Add Efinix SPI programming bindings

[Krzysztof Kozlowski]

On 03/03/2025 11:29, Ian Dannapel wrote:
> Hi Krzysztof, thanks for the quick response.
> 
> On Sat, Mar 1, 2025 at 2:13 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On 28/02/2025 10:47, iansdannapel@gmail.com wrote:
>>> +
>>> +  References:
>>> +  - https://www.efinixinc.com/docs/an006-configuring-trion-fpgas-v6.3.pdf
>>> +  - https://www.efinixinc.com/docs/an033-configuring-titanium-fpgas-v2.8.pdf
>>> +  - https://www.efinixinc.com/docs/an061-configuring-topaz-fpgas-v1.1.pdf
>>> +
>>> +allOf:
>>> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - efinix,trion-spi
>>> +      - efinix,titanium-spi
>>> +      - efinix,topaz-spi
>>
>>
>> Same comments as before about compatibility. Address or implement.
>>
> The compatibles are implemented in the device match table, what
> exactly should be addressed or implemented here?

Comments from previous revision - they look compatible, so define this
as list of compatibles where one is of above is used as fallback. See
example schema or 90% of other bindings (oneOf:).


> 
>>> +      - efinix,fpga-spi
>>
>>
>> And this one is for which device? It is not even used.
> The proposed compatible is a generic fallback for any Efinix FPGA

You do not use here fallback at all. Fallback means last compatible in
the list, but you have here only one item.

> Series. Isn't it used if the compatible is part of the drivers match
> table?

Drop, compatibles are supposed to be specific.


Best regards,
Krzysztof

Re: [v4 2/3] dt-bindings: fpga: Add Efinix SPI programming bindings

[Krzysztof Kozlowski]

On 03/03/2025 11:31, Conor Dooley wrote:
> On Mon, Mar 03, 2025 at 11:10:53AM +0100, Ian Dannapel wrote:
>> Hi Conor, thanks for the quick response.
>>
>> On Fri, Feb 28, 2025 at 7:28 PM Conor Dooley <conor@kernel.org> wrote:
>>>> +description: |
>>>> +  Efinix FPGAs (Trion, Topaz, and Titanium families) support loading bitstreams
>>>> +  through "SPI Passive Mode".
>>>> +  Note 1: Only bus width 1x is supported.
>>>> +  Note 2: Additional pins hogs for bus width configuration must be set
>>>> +  elsewhere, if necessary.
>>>> +  Note 3: Topaz and Titanium support is based on documentation but remains
>>>> +  untested.
>>>
>>> Points 1 and 3 here seem to be driver limitations, and shouldn't really
>>> be present in a document describing the hardware?
>>>
>> Yes, they are driver limitations and probably do not belong here.
>>
>>>> +properties:
>>>> +  compatible:
>>>> +    enum:
>>>> +      - efinix,trion-spi
>>>> +      - efinix,titanium-spi
>>>> +      - efinix,topaz-spi
>>>
>>>> +      - efinix,fpga-spi
>>>
>>> What hardware does this device represent? Other ones are obvious matches
>>> to the families you mention, but what is this one?
> 
>> The proposed compatible is a generic fallback for any Efinix FPGA Series.
> 
> If it is a fallback, your binding should look like:
> compatible:
>   items:
>     - enum:
>         - efinix,trion-spi
>         - efinix,titanium-spi
>         - efinix,topaz-spi
>     - const: efinix,fpga-spi
> 
> |+static const struct of_device_id efinix_spi_of_match[] = {
> |+       { .compatible = "efinix,trion-spi", },
> |+       { .compatible = "efinix,titanium-spi", },
> |+       { .compatible = "efinix,topaz-spi", },
> 
> And these three compatibles can/should be removed from the driver, since
> the fallback is required.
> 
> |+       { .compatible = "efinix,fpga-spi", },
> |+       {}

Yes, except that one of the devices should be the fallback, not generic
"fpga".


Best regards,
Krzysztof

RE: [v4 3/3] fpga-mgr: Add Efinix SPI programming driver

[Manne, Nava kishore]

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: iansdannapel@gmail.com <iansdannapel@gmail.com>
> Sent: Friday, February 28, 2025 3:18 PM
> To: linux-fpga@vger.kernel.org
> Cc: Moritz Fischer <mdf@kernel.org>; Wu Hao <hao.wu@intel.com>; Xu Yilun
> <yilun.xu@intel.com>; Tom Rix <trix@redhat.com>; Rob Herring
> <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>; Neil Armstrong <neil.armstrong@linaro.org>; Jonathan
> Cameron <Jonathan.Cameron@huawei.com>; Rafał Miłecki <rafal@milecki.pl>;
> Aradhya Bhatia <a-bhatia1@ti.com>; Ian Dannapel <iansdannapel@gmail.com>;
> open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
> <devicetree@vger.kernel.org>; open list <linux-kernel@vger.kernel.org>
> Subject: [v4 3/3] fpga-mgr: Add Efinix SPI programming driver
>
> From: Ian Dannapel <iansdannapel@gmail.com>
>
> Add a new driver for loading binary firmware to configuration RAM using "SPI
> passive mode" on Efinix FPGAs.
>
> Signed-off-by: Ian Dannapel <iansdannapel@gmail.com>
> ---
>  drivers/fpga/Kconfig      |   7 ++
>  drivers/fpga/Makefile     |   1 +
>  drivers/fpga/efinix-spi.c | 212 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 220 insertions(+)
>  create mode 100644 drivers/fpga/efinix-spi.c
>
> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig index
> 37b35f58f0df..b5d60ba62900 100644
> --- a/drivers/fpga/Kconfig
> +++ b/drivers/fpga/Kconfig
> @@ -83,6 +83,13 @@ config FPGA_MGR_XILINX_SPI
>         FPGA manager driver support for Xilinx FPGA configuration
>         over slave serial interface.
>
> +config FPGA_MGR_EFINIX_SPI
> +     tristate "Efinix FPGA configuration over SPI"
> +     depends on SPI
> +     help
> +       FPGA manager driver support for Efinix FPGAs configuration over SPI
> +       (passive mode only).
> +
>  config FPGA_MGR_ICE40_SPI
>       tristate "Lattice iCE40 SPI"
>       depends on OF && SPI
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile index
> aeb89bb13517..adbd51d2cd1e 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -18,6 +18,7 @@ obj-$(CONFIG_FPGA_MGR_TS73XX)               +=
> ts73xx-fpga.o
>  obj-$(CONFIG_FPGA_MGR_XILINX_CORE)   += xilinx-core.o
>  obj-$(CONFIG_FPGA_MGR_XILINX_SELECTMAP)      += xilinx-selectmap.o
>  obj-$(CONFIG_FPGA_MGR_XILINX_SPI)    += xilinx-spi.o
> +obj-$(CONFIG_FPGA_MGR_EFINIX_SPI)    += efinix-spi.o
>  obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA)     += zynq-fpga.o
>  obj-$(CONFIG_FPGA_MGR_ZYNQMP_FPGA)   += zynqmp-fpga.o
>  obj-$(CONFIG_FPGA_MGR_VERSAL_FPGA)   += versal-fpga.o
> diff --git a/drivers/fpga/efinix-spi.c b/drivers/fpga/efinix-spi.c new file mode 100644
> index 000000000000..07885110a8a8
> --- /dev/null
> +++ b/drivers/fpga/efinix-spi.c
> @@ -0,0 +1,212 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * FPGA Manager Driver for Efinix
> + *
> + * Copyright (C) 2025 iris-GmbH infrared & intelligent sensors
> + *
> + * Ian Dannapel <iansdannapel@gmail.com>
> + *
> + * Load Efinix FPGA firmware over SPI using the serial configuration interface.
> + *
> + * Note 1: Only passive mode (host initiates transfer) is currently supported.
> + * Note 2: Topaz and Titanium support is based on documentation but
> +remains
> + * untested.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/fpga/fpga-mgr.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/spi/spi.h>
> +
> +struct efinix_spi_conf {
> +     struct spi_device *spi;
> +     struct gpio_desc *cdone;
> +     struct gpio_desc *reset;
> +};
> +
> +static void efinix_spi_reset(struct efinix_spi_conf *conf) {
> +     gpiod_set_value(conf->reset, 1);
> +     /* tCRESET_N > 320 ns */
> +     usleep_range(1, 2);
> +     gpiod_set_value(conf->reset, 0);
> +
> +     /* tDMIN > 32 us */
> +     usleep_range(35, 40);

Use macros instead of hardcoded values.

> +}
> +
> +static enum fpga_mgr_states efinix_spi_state(struct fpga_manager *mgr)
> +{
> +     struct efinix_spi_conf *conf = mgr->priv;
> +
> +     if (conf->cdone && gpiod_get_value(conf->cdone) == 1)
> +             return FPGA_MGR_STATE_OPERATING;
> +
> +     return FPGA_MGR_STATE_UNKNOWN;
> +}
> +
> +static int efinix_spi_write_init(struct fpga_manager *mgr,
> +                              struct fpga_image_info *info,
> +                              const char *buf, size_t count)
> +{
> +     if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
> +             dev_err(&mgr->dev, "Partial reconfiguration not supported\n");
> +             return -EOPNOTSUPP;
> +     }
> +     return 0;
> +}
> +
> +static int efinix_spi_write(struct fpga_manager *mgr, const char *buf,
> +                         size_t count)
> +{
> +     struct efinix_spi_conf *conf = mgr->priv;
> +     int ret;
> +     struct spi_message message;
> +     struct spi_transfer assert_cs = {
> +             .cs_change = 1
> +     };
> +     struct spi_transfer write_xfer = {
> +             .tx_buf = buf,
> +             .len = count
> +     };
> +     struct spi_transfer clk_cycles = {
> +             .len = 13,  // > 100 clock cycles

The .len = 13 is based on documentation stating?
Consider using macro.

> +             .tx_buf = NULL
> +     };
> +     u8 *dummy_buf;
> +
> +     dummy_buf = kzalloc(13, GFP_KERNEL);

Same - use macro

> +     if (!dummy_buf) {
> +             ret = -ENOMEM;
> +             goto fail;
> +     }
> +
> +     spi_bus_lock(conf->spi->controller);
> +     spi_message_init(&message);
> +     spi_message_add_tail(&assert_cs, &message);
> +     ret = spi_sync_locked(conf->spi, &message);
> +     if (ret)
> +             goto fail_unlock;
> +
> +     /* reset with asserted cs */
> +     efinix_spi_reset(conf);
> +
> +     spi_message_init(&message);
> +     spi_message_add_tail(&write_xfer, &message);
> +
> +     clk_cycles.tx_buf = dummy_buf;
> +     spi_message_add_tail(&clk_cycles, &message);
> +
> +     ret = spi_sync_locked(conf->spi, &message);
> +     if (ret)
> +             dev_err(&mgr->dev, "SPI error in firmware write: %d\n", ret);
> +
> +fail_unlock:
> +     spi_bus_unlock(conf->spi->controller);
> +     kfree(dummy_buf);
> +fail:
> +     return ret;
> +}
> +
> +static int efinix_spi_write_complete(struct fpga_manager *mgr,
> +                                  struct fpga_image_info *info)
> +{
> +     struct efinix_spi_conf *conf = mgr->priv;
> +     unsigned long timeout =
> +             jiffies + usecs_to_jiffies(info->config_complete_timeout_us);
> +     bool expired = false;
> +     int done;
> +
> +     if (conf->cdone) {
> +             while (!expired) {
> +                     expired = time_after(jiffies, timeout);
> +
> +                     done = gpiod_get_value(conf->cdone);
> +                     if (done < 0)
> +                             return done;
> +
> +                     if (done)
> +                             break;
> +             }
> +     }
> +
> +     if (expired)
> +             return -ETIMEDOUT;
> +
> +     /* tUSER > 25 us */
> +     usleep_range(30, 35);

Same - use macros.

> +     return 0;
> +}
> +
> +static const struct fpga_manager_ops efinix_spi_ops = {
> +     .state = efinix_spi_state,
> +     .write_init = efinix_spi_write_init,
> +     .write = efinix_spi_write,
> +     .write_complete = efinix_spi_write_complete, };
> +
> +static int efinix_spi_probe(struct spi_device *spi) {
> +     struct efinix_spi_conf *conf;
> +     struct fpga_manager *mgr;
> +
> +     conf = devm_kzalloc(&spi->dev, sizeof(*conf), GFP_KERNEL);
> +     if (!conf)
> +             return -ENOMEM;
> +
> +     conf->spi = spi;
> +
> +     conf->reset = devm_gpiod_get(&spi->dev, "reset", GPIOD_OUT_HIGH);
> +     if (IS_ERR(conf->reset))
> +             return dev_err_probe(&spi->dev, PTR_ERR(conf->reset),
> +                                  "Failed to get RESET gpio\n");
> +
> +     if (!(spi->mode & SPI_CPHA) || !(spi->mode & SPI_CPOL))
> +             return dev_err_probe(&spi->dev, -EINVAL,
> +                                  "Unsupported SPI mode, set CPHA and CPOL\n");
> +
> +     conf->cdone = devm_gpiod_get_optional(&spi->dev, "cdone", GPIOD_IN);
> +     if (IS_ERR(conf->cdone))
> +             return dev_err_probe(&spi->dev, PTR_ERR(conf->cdone),
> +                                  "Failed to get CDONE gpio\n");
> +
> +     mgr = devm_fpga_mgr_register(&spi->dev,
> +                                  "Efinix FPGA Manager",
> +                                  &efinix_spi_ops, conf);
> +
> +     return PTR_ERR_OR_ZERO(mgr);
> +}
> +
> +static const struct of_device_id efinix_spi_of_match[] = {
> +     { .compatible = "efinix,trion-spi", },
> +     { .compatible = "efinix,titanium-spi", },
> +     { .compatible = "efinix,topaz-spi", },
> +     { .compatible = "efinix,fpga-spi", },
> +     {}
> +};
> +MODULE_DEVICE_TABLE(of, efinix_spi_of_match);
> +
> +static const struct spi_device_id efinix_ids[] = {
> +     { "trion-spi", 0 },
> +     { "titanium-spi", 0 },
> +     { "topaz-spi", 0 },
> +     {},
> +};
> +MODULE_DEVICE_TABLE(spi, efinix_ids);
> +
> +static struct spi_driver efinix_spi_driver = {
> +     .driver = {
> +             .name = "efinix-spi",
> +             .of_match_table = efinix_spi_of_match,
> +     },
> +     .probe = efinix_spi_probe,
> +     .id_table = efinix_ids,
> +};
> +
> +module_spi_driver(efinix_spi_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Ian Dannapel <iansdannapel@gmail.com>");
> +MODULE_DESCRIPTION("Efinix FPGA SPI Programming Driver (Topaz/Titanium
> +untested)");

If untested, it might be useful to mark them as experimental in Kconfig.

Regards,
Navakishore.

Re: [v4 3/3] fpga-mgr: Add Efinix SPI programming driver

[Ian Dannapel]

Hi Navakishore, thanks for the review.

On Mon, Mar 3, 2025 at 12:57 PM Manne, Nava kishore
<nava.kishore.manne@amd.com> wrote:

> > +MODULE_DESCRIPTION("Efinix FPGA SPI Programming Driver (Topaz/Titanium
> > +untested)");
>
> If untested, it might be useful to mark them as experimental in Kconfig.
Is a separate Kconfig for that preferred or maybe just a note in the
help description?

Regards, Ian

Re: [v4 1/3] dt-bindings: vendor-prefix: Add prefix for Efinix, Inc.

[Alexander Dahl]

Hello Ian,

Am Sat, Mar 01, 2025 at 02:10:38PM +0100 schrieb Krzysztof Kozlowski:
> On 28/02/2025 10:47, iansdannapel@gmail.com wrote:
> > From: Ian Dannapel <iansdannapel@gmail.com>
> > 
> > Add entry for Efinix, Inc. (https://www.efinixinc.com/)
> > 
> > Signed-off-by: Ian Dannapel <iansdannapel@gmail.com>
> 
> <form letter>
> This is a friendly reminder during the review process.
> 
> It looks like you received a tag and forgot to add it.
> 
> If you do not know the process, here is a short explanation:
> Please add Acked-by/Reviewed-by/Tested-by tags when posting new versions
> of patchset, under or above your Signed-off-by tag, unless patch changed
> significantly (e.g. new properties added to the DT bindings). Tag is
> "received", when provided in a message replied to you on the mailing
> list. Tools like b4 can help here. However, there's no need to repost
> patches *only* to add the tags. The upstream maintainer will do that for
> tags received on the version they apply.
> 
> Please read:
> https://elixir.bootlin.com/linux/v6.12-rc3/source/Documentation/process/submitting-patches.rst#L577
> 
> If a tag was not added on purpose, please state why and what changed.
> </form letter>

FWIW I guess this might refer to:

https://lore.kernel.org/linux-fpga/20240930-tranquil-glitch-f48685f77942@thorsis.com/

Greets
Alex

> 
> Best regards,
> Krzysztof
> 

Re: [v4 3/3] fpga-mgr: Add Efinix SPI programming driver

[Xu Yilun]

On Fri, Feb 28, 2025 at 10:47:32AM +0100, iansdannapel@gmail.com wrote:
> From: Ian Dannapel <iansdannapel@gmail.com>
> 
> Add a new driver for loading binary firmware to configuration
> RAM using "SPI passive mode" on Efinix FPGAs.
> 
> Signed-off-by: Ian Dannapel <iansdannapel@gmail.com>
> ---
>  drivers/fpga/Kconfig      |   7 ++
>  drivers/fpga/Makefile     |   1 +
>  drivers/fpga/efinix-spi.c | 212 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 220 insertions(+)
>  create mode 100644 drivers/fpga/efinix-spi.c
> 
> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> index 37b35f58f0df..b5d60ba62900 100644
> --- a/drivers/fpga/Kconfig
> +++ b/drivers/fpga/Kconfig
> @@ -83,6 +83,13 @@ config FPGA_MGR_XILINX_SPI
>  	  FPGA manager driver support for Xilinx FPGA configuration
>  	  over slave serial interface.
>  
> +config FPGA_MGR_EFINIX_SPI
> +	tristate "Efinix FPGA configuration over SPI"
> +	depends on SPI
> +	help
> +	  FPGA manager driver support for Efinix FPGAs configuration over SPI
> +	  (passive mode only).
> +
>  config FPGA_MGR_ICE40_SPI
>  	tristate "Lattice iCE40 SPI"
>  	depends on OF && SPI
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index aeb89bb13517..adbd51d2cd1e 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -18,6 +18,7 @@ obj-$(CONFIG_FPGA_MGR_TS73XX)		+= ts73xx-fpga.o
>  obj-$(CONFIG_FPGA_MGR_XILINX_CORE)	+= xilinx-core.o
>  obj-$(CONFIG_FPGA_MGR_XILINX_SELECTMAP)	+= xilinx-selectmap.o
>  obj-$(CONFIG_FPGA_MGR_XILINX_SPI)	+= xilinx-spi.o
> +obj-$(CONFIG_FPGA_MGR_EFINIX_SPI)	+= efinix-spi.o
>  obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA)	+= zynq-fpga.o
>  obj-$(CONFIG_FPGA_MGR_ZYNQMP_FPGA)	+= zynqmp-fpga.o
>  obj-$(CONFIG_FPGA_MGR_VERSAL_FPGA)	+= versal-fpga.o
> diff --git a/drivers/fpga/efinix-spi.c b/drivers/fpga/efinix-spi.c
> new file mode 100644
> index 000000000000..07885110a8a8
> --- /dev/null
> +++ b/drivers/fpga/efinix-spi.c
> @@ -0,0 +1,212 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * FPGA Manager Driver for Efinix
> + *
> + * Copyright (C) 2025 iris-GmbH infrared & intelligent sensors
> + *
> + * Ian Dannapel <iansdannapel@gmail.com>
> + *
> + * Load Efinix FPGA firmware over SPI using the serial configuration interface.
> + *
> + * Note 1: Only passive mode (host initiates transfer) is currently supported.
> + * Note 2: Topaz and Titanium support is based on documentation but remains
> + * untested.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/fpga/fpga-mgr.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/spi/spi.h>
> +
> +struct efinix_spi_conf {
> +	struct spi_device *spi;
> +	struct gpio_desc *cdone;
> +	struct gpio_desc *reset;
> +};
> +
> +static void efinix_spi_reset(struct efinix_spi_conf *conf)
> +{
> +	gpiod_set_value(conf->reset, 1);
> +	/* tCRESET_N > 320 ns */
> +	usleep_range(1, 2);
> +	gpiod_set_value(conf->reset, 0);
> +
> +	/* tDMIN > 32 us */
> +	usleep_range(35, 40);
> +}
> +
> +static enum fpga_mgr_states efinix_spi_state(struct fpga_manager *mgr)
> +{
> +	struct efinix_spi_conf *conf = mgr->priv;
> +
> +	if (conf->cdone && gpiod_get_value(conf->cdone) == 1)
> +		return FPGA_MGR_STATE_OPERATING;
> +
> +	return FPGA_MGR_STATE_UNKNOWN;
> +}
> +
> +static int efinix_spi_write_init(struct fpga_manager *mgr,
> +				 struct fpga_image_info *info,
> +				 const char *buf, size_t count)
> +{
> +	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
> +		dev_err(&mgr->dev, "Partial reconfiguration not supported\n");
> +		return -EOPNOTSUPP;
> +	}
> +	return 0;
> +}
> +
> +static int efinix_spi_write(struct fpga_manager *mgr, const char *buf,
> +			    size_t count)
> +{
> +	struct efinix_spi_conf *conf = mgr->priv;
> +	int ret;
> +	struct spi_message message;
> +	struct spi_transfer assert_cs = {
> +		.cs_change = 1
> +	};
> +	struct spi_transfer write_xfer = {
> +		.tx_buf = buf,
> +		.len = count
> +	};
> +	struct spi_transfer clk_cycles = {
> +		.len = 13,  // > 100 clock cycles

Please define a meaning for Macro name for this.

> +		.tx_buf = NULL
> +	};
> +	u8 *dummy_buf;
> +
> +	dummy_buf = kzalloc(13, GFP_KERNEL);

and here.

> +	if (!dummy_buf) {
> +		ret = -ENOMEM;
> +		goto fail;
> +	}
> +
> +	spi_bus_lock(conf->spi->controller);
> +	spi_message_init(&message);
> +	spi_message_add_tail(&assert_cs, &message);

Is it better use spi_message_init_with_transfers()?

> +	ret = spi_sync_locked(conf->spi, &message);
> +	if (ret)
> +		goto fail_unlock;
> +
> +	/* reset with asserted cs */
> +	efinix_spi_reset(conf);

Recall we've discussed the write() operation may be called multiple
times, each writes a part of the image data. So with this
implementation, the cs & reset may be toggled several times during the
whole reprogramming, is it? Could the fpga be correctly reprogramed?
I remember you said no.

> +
> +	spi_message_init(&message);
> +	spi_message_add_tail(&write_xfer, &message);
> +
> +	clk_cycles.tx_buf = dummy_buf;
> +	spi_message_add_tail(&clk_cycles, &message);

use spi_message_init_with_transfers when possible.

Thanks,
Yilun

> +
> +	ret = spi_sync_locked(conf->spi, &message);
> +	if (ret)
> +		dev_err(&mgr->dev, "SPI error in firmware write: %d\n", ret);
> +
> +fail_unlock:
> +	spi_bus_unlock(conf->spi->controller);
> +	kfree(dummy_buf);
> +fail:
> +	return ret;
> +}
> +
> +static int efinix_spi_write_complete(struct fpga_manager *mgr,
> +				     struct fpga_image_info *info)
> +{
> +	struct efinix_spi_conf *conf = mgr->priv;
> +	unsigned long timeout =
> +		jiffies + usecs_to_jiffies(info->config_complete_timeout_us);
> +	bool expired = false;
> +	int done;
> +
> +	if (conf->cdone) {
> +		while (!expired) {
> +			expired = time_after(jiffies, timeout);
> +
> +			done = gpiod_get_value(conf->cdone);
> +			if (done < 0)
> +				return done;
> +
> +			if (done)
> +				break;
> +		}
> +	}
> +
> +	if (expired)
> +		return -ETIMEDOUT;
> +
> +	/* tUSER > 25 us */
> +	usleep_range(30, 35);
> +	return 0;
> +}
> +
> +static const struct fpga_manager_ops efinix_spi_ops = {
> +	.state = efinix_spi_state,
> +	.write_init = efinix_spi_write_init,
> +	.write = efinix_spi_write,
> +	.write_complete = efinix_spi_write_complete,
> +};
> +
> +static int efinix_spi_probe(struct spi_device *spi)
> +{
> +	struct efinix_spi_conf *conf;
> +	struct fpga_manager *mgr;
> +
> +	conf = devm_kzalloc(&spi->dev, sizeof(*conf), GFP_KERNEL);
> +	if (!conf)
> +		return -ENOMEM;
> +
> +	conf->spi = spi;
> +
> +	conf->reset = devm_gpiod_get(&spi->dev, "reset", GPIOD_OUT_HIGH);
> +	if (IS_ERR(conf->reset))
> +		return dev_err_probe(&spi->dev, PTR_ERR(conf->reset),
> +				     "Failed to get RESET gpio\n");
> +
> +	if (!(spi->mode & SPI_CPHA) || !(spi->mode & SPI_CPOL))
> +		return dev_err_probe(&spi->dev, -EINVAL,
> +				     "Unsupported SPI mode, set CPHA and CPOL\n");
> +
> +	conf->cdone = devm_gpiod_get_optional(&spi->dev, "cdone", GPIOD_IN);
> +	if (IS_ERR(conf->cdone))
> +		return dev_err_probe(&spi->dev, PTR_ERR(conf->cdone),
> +				     "Failed to get CDONE gpio\n");
> +
> +	mgr = devm_fpga_mgr_register(&spi->dev,
> +				     "Efinix FPGA Manager",
> +				     &efinix_spi_ops, conf);
> +
> +	return PTR_ERR_OR_ZERO(mgr);
> +}
> +
> +static const struct of_device_id efinix_spi_of_match[] = {
> +	{ .compatible = "efinix,trion-spi", },
> +	{ .compatible = "efinix,titanium-spi", },
> +	{ .compatible = "efinix,topaz-spi", },
> +	{ .compatible = "efinix,fpga-spi", },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, efinix_spi_of_match);
> +
> +static const struct spi_device_id efinix_ids[] = {
> +	{ "trion-spi", 0 },
> +	{ "titanium-spi", 0 },
> +	{ "topaz-spi", 0 },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(spi, efinix_ids);
> +
> +static struct spi_driver efinix_spi_driver = {
> +	.driver = {
> +		.name = "efinix-spi",
> +		.of_match_table = efinix_spi_of_match,
> +	},
> +	.probe = efinix_spi_probe,
> +	.id_table = efinix_ids,
> +};
> +
> +module_spi_driver(efinix_spi_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Ian Dannapel <iansdannapel@gmail.com>");
> +MODULE_DESCRIPTION("Efinix FPGA SPI Programming Driver (Topaz/Titanium untested)");
> -- 
> 2.43.0
> 
>