[PATCH v8 0/2] Lattice sysCONFIG SPI FPGA manager

[PATCH v8 0/2] Lattice sysCONFIG SPI FPGA manager

[Ivan Bornyakov]

Add support to the FPGA manager for programming Lattice FPGAs over slave
SPI sysCONFIG interface. ECP5 and MachXO2 are supported.

WARNING: I only have HW with ECP5, so can't vouch for the MachXO2.
	 MachXO2's support is based on existing machxo2-spi.c and
	 publicly available documentation, it's not tested on real
	 hardware.

ChangeLog:
  v1 -> v2:
    * remove "spi" from compatible string
    * reword description in dt-bindings doc
    * add reference to spi-peripheral-props.yaml in dt-binding doc
    * fix DTS example in dt-bindings doc: 4-spaces indentations, no
      undersores in node names.
  v2 -> v3:
    * fix typo "##size-cells" -> "#size-cells" in dt-bindings example
  v3 -> v4:
    * dt-bindings: reword description
    * dt-bindings: revert props order
  v4 -> v5:
    * dt-bindings: remove trailing dot from title
    * dt-bindings: reword description to avoid driver reference
    * dt-bindings: add "Reviewed-by: Krzysztof Kozlowski" tag
  v5 -> v6:
    * ecp5-spi: lock SPI bus for exclusive usage in
      ecp5_ops_write_init(), release in ecp5_ops_write_complete()
      or on error
  v6 -> v7:
    * ecp5-spi.c -> lattice-sysconfig-spi.c. Reworked to represent
      generalized sysCONFIG port with implementations for ECP5 and
      MachXO2
    * lattice,ecp5-fpga-mgr.yaml -> lattice,sysconfig.yaml. Reworked to
      document both ECP5 and MachXO2 sysCONFIG.
    * dt-bindings: remove "Reviewed-by: Krzysztof Kozlowski" tag as doc
      was rewritten by a considerable amount.
  v7 -> v8:
    * dt-bindings: move "program-gpios", "init-gpios" and "done-gpios"
      to top-level properties and disallow them for MachXO2 variant.

Ivan Bornyakov (2):
  fpga: lattice-sysconfig-spi: add Lattice sysCONFIG FPGA manager
  dt-bindings: fpga: document Lattice sysCONFIG FPGA manager

 .../bindings/fpga/lattice,sysconfig.yaml      | 109 +++
 drivers/fpga/Kconfig                          |   7 +
 drivers/fpga/Makefile                         |   1 +
 drivers/fpga/lattice-sysconfig-spi.c          | 630 ++++++++++++++++++
 4 files changed, 747 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/fpga/lattice,sysconfig.yaml
 create mode 100644 drivers/fpga/lattice-sysconfig-spi.c

-- 
2.37.2

[PATCH v8 1/2] fpga: lattice-sysconfig-spi: add Lattice sysCONFIG FPGA manager

[Ivan Bornyakov]

Add support to the FPGA manager for programming Lattice ECP5 and MachXO2
FPGAs over slave SPI sysCONFIG interface.

Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
---
 drivers/fpga/Kconfig                 |   7 +
 drivers/fpga/Makefile                |   1 +
 drivers/fpga/lattice-sysconfig-spi.c | 630 +++++++++++++++++++++++++++
 3 files changed, 638 insertions(+)
 create mode 100644 drivers/fpga/lattice-sysconfig-spi.c

diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index 6c416955da53..991d9d976dca 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -263,4 +263,11 @@ config FPGA_MGR_MICROCHIP_SPI
 	  programming over slave SPI interface with .dat formatted
 	  bitstream image.
 
+config FPGA_MGR_LATTICE_SPI
+	tristate "Lattice sysCONFIG SPI FPGA manager"
+	depends on SPI
+	help
+	  FPGA manager driver support for Lattice FPGAs programming over slave
+	  SPI sysCONFIG interface.
+
 endif # FPGA
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index 42ae8b58abce..115dba916024 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -20,6 +20,7 @@ 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
 obj-$(CONFIG_FPGA_MGR_MICROCHIP_SPI)	+= microchip-spi.o
+obj-$(CONFIG_FPGA_MGR_LATTICE_SPI)	+= lattice-sysconfig-spi.o
 obj-$(CONFIG_ALTERA_PR_IP_CORE)		+= altera-pr-ip-core.o
 obj-$(CONFIG_ALTERA_PR_IP_CORE_PLAT)	+= altera-pr-ip-core-plat.o
 
diff --git a/drivers/fpga/lattice-sysconfig-spi.c b/drivers/fpga/lattice-sysconfig-spi.c
new file mode 100644
index 000000000000..145b5b27b88d
--- /dev/null
+++ b/drivers/fpga/lattice-sysconfig-spi.c
@@ -0,0 +1,630 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Lattice FPGA programming over slave SPI sysCONFIG interface.
+ */
+
+#include <linux/delay.h>
+#include <linux/fpga/fpga-mgr.h>
+#include <linux/of_device.h>
+#include <linux/spi/spi.h>
+
+#define	SYSCONFIG_ISC_ENABLE		{0xC6, 0x00, 0x00, 0x00}
+#define	SYSCONFIG_ISC_DISABLE		{0x26, 0x00, 0x00, 0x00}
+#define	SYSCONFIG_ISC_ERASE		{0x0E, 0x00, 0x00, 0x00}
+#define	SYSCONFIG_ISC_PROGRAM_DONE	{0x5E, 0x00, 0x00, 0x00}
+#define	SYSCONFIG_LSC_READ_STATUS	{0x3C, 0x00, 0x00, 0x00}
+#define	SYSCONFIG_LSC_CHECK_BUSY	{0xF0, 0x00, 0x00, 0x00}
+#define	SYSCONFIG_LSC_REFRESH		{0x79, 0x00, 0x00, 0x00}
+#define	SYSCONFIG_LSC_INIT_ADDR		{0x46, 0x00, 0x00, 0x00}
+#define	SYSCONFIG_LSC_BITSTREAM_BURST	{0x7a, 0x00, 0x00, 0x00}
+#define	SYSCONFIG_LSC_PROG_INCR_NV	{0x70, 0x00, 0x00, 0x01}
+
+#define	SYSCONFIG_STATUS_DONE		BIT(8)
+#define	SYSCONFIG_STATUS_BUSY		BIT(12)
+#define	SYSCONFIG_STATUS_FAIL		BIT(13)
+#define	SYSCONFIG_STATUS_ERR		(BIT(23) | BIT(24) | BIT(25))
+
+#define	SYSCONFIG_POLL_RETRIES		100000
+
+#define	ECP5_SPI_MAX_SPEED_HZ		60000000
+#define	ECP5_ISC_ERASE_OPERAND		0x01
+
+#define	MACHXO2_SPI_MAX_SPEED_HZ	66000000
+#define	MACHXO2_PAGE_SIZE		16
+#define	MACHXO2_ISC_ENABLE_OPERAND	0x08
+#define	MACHXO2_ISC_ERASE_OPERAND	0x04
+
+struct sysconfig_priv {
+	struct gpio_desc *program;
+	struct gpio_desc *init;
+	struct gpio_desc *done;
+	struct spi_device *spi;
+	u8 isc_enable_operand;
+	u8 isc_erase_operand;
+};
+
+static int sysconfig_poll_busy(struct sysconfig_priv *data)
+{
+	const u8 lsc_check_busy[] = SYSCONFIG_LSC_CHECK_BUSY;
+	int ret, retries = SYSCONFIG_POLL_RETRIES;
+	u8 busy;
+
+	while (retries--) {
+		ret = spi_write_then_read(data->spi,
+					  lsc_check_busy, sizeof(lsc_check_busy),
+					  &busy, sizeof(busy));
+		if (ret)
+			return ret;
+
+		if (!busy)
+			return 0;
+
+		usleep_range(50, 100);
+	}
+
+	return -EBUSY;
+}
+
+static int sysconfig_read_status(struct sysconfig_priv *data, u32 *status)
+{
+	const u8 lsc_read_status[] = SYSCONFIG_LSC_READ_STATUS;
+	__be32 device_status;
+	int ret;
+
+	ret = spi_write_then_read(data->spi,
+				  lsc_read_status, sizeof(lsc_read_status),
+				  &device_status, sizeof(device_status));
+	if (ret)
+		return ret;
+
+	*status = be32_to_cpu(device_status);
+
+	return 0;
+}
+
+static int sysconfig_poll_status(struct sysconfig_priv *data, u32 *status)
+{
+	int ret;
+
+	ret = sysconfig_poll_busy(data);
+	if (ret)
+		return ret;
+
+	return sysconfig_read_status(data, status);
+}
+
+static int sysconfig_poll_gpio(struct gpio_desc *gpio, bool is_active)
+{
+	int value, retries = SYSCONFIG_POLL_RETRIES;
+
+	while (retries--) {
+		value = gpiod_get_value(gpio);
+		if (value < 0)
+			return value;
+
+		if ((!is_active && !value) || (is_active && value))
+			return 0;
+
+		ndelay(10);
+	}
+
+	return -ETIMEDOUT;
+}
+
+static int sysconfig_refresh(struct sysconfig_priv *data)
+{
+	static const u8 lsc_refresh[] = SYSCONFIG_LSC_REFRESH;
+	int ret;
+
+	ret = spi_write(data->spi, lsc_refresh, sizeof(lsc_refresh));
+	if (ret)
+		return ret;
+
+	usleep_range(4000, 8000);
+
+	return 0;
+}
+
+static int sysconfig_isc_enable(struct sysconfig_priv *data)
+{
+	u8 isc_enable[] = SYSCONFIG_ISC_ENABLE;
+	u32 status;
+	int ret;
+
+	isc_enable[1] = data->isc_enable_operand;
+
+	ret = spi_write(data->spi, isc_enable, sizeof(isc_enable));
+	if (ret)
+		return ret;
+
+	ret = sysconfig_poll_status(data, &status);
+	if (ret || (status & SYSCONFIG_STATUS_FAIL))
+		return ret ? : -EFAULT;
+
+	return 0;
+}
+
+static int sysconfig_isc_erase(struct sysconfig_priv *data)
+{
+	u8 isc_erase[] = SYSCONFIG_ISC_ERASE;
+	u32 status;
+	int ret;
+
+	isc_erase[1] = data->isc_erase_operand;
+
+	ret = spi_write(data->spi, isc_erase, sizeof(isc_erase));
+	if (ret)
+		return ret;
+
+	ret = sysconfig_poll_status(data, &status);
+	if (ret || (status & SYSCONFIG_STATUS_FAIL))
+		return ret ? : -EFAULT;
+
+	return 0;
+}
+
+static int sysconfig_isc_init(struct sysconfig_priv *data)
+{
+	int ret;
+
+	ret = sysconfig_isc_enable(data);
+	if (ret)
+		return ret;
+
+	return sysconfig_isc_erase(data);
+}
+
+static int sysconfig_lsc_init_addr(struct sysconfig_priv *data)
+{
+	const u8 lsc_init_addr[] = SYSCONFIG_LSC_INIT_ADDR;
+
+	return spi_write(data->spi, lsc_init_addr, sizeof(lsc_init_addr));
+}
+
+static int sysconfig_lsc_bitstream_burst(struct sysconfig_priv *data)
+{
+	const u8 lsc_bitstream_burst[] = SYSCONFIG_LSC_BITSTREAM_BURST;
+	struct spi_transfer xfer = {
+		.tx_buf = lsc_bitstream_burst,
+		.len = sizeof(lsc_bitstream_burst),
+		.cs_change = 1,
+	};
+	struct spi_message msg;
+
+	spi_message_init_with_transfers(&msg, &xfer, 1);
+
+	return spi_sync_locked(data->spi, &msg);
+}
+
+static int sysconfig_isc_prog_done(struct sysconfig_priv *data)
+{
+	const u8 isc_prog_done[] = SYSCONFIG_ISC_PROGRAM_DONE;
+	u32 status;
+	int ret;
+
+	ret = spi_write(data->spi, isc_prog_done, sizeof(isc_prog_done));
+	if (ret)
+		return ret;
+
+	ret = sysconfig_poll_status(data, &status);
+	if (ret)
+		return ret;
+
+	if (status & SYSCONFIG_STATUS_DONE)
+		return 0;
+
+	return -EFAULT;
+}
+
+static int sysconfig_isc_disable(struct sysconfig_priv *data)
+{
+	const u8 isc_disable[] = SYSCONFIG_ISC_DISABLE;
+
+	return spi_write(data->spi, isc_disable, sizeof(isc_disable));
+}
+
+static enum fpga_mgr_states ecp5_ops_state(struct fpga_manager *mgr)
+{
+	struct sysconfig_priv *priv = mgr->priv;
+
+	return (gpiod_get_value(priv->done) > 0) ? FPGA_MGR_STATE_OPERATING :
+						   FPGA_MGR_STATE_UNKNOWN;
+}
+
+static int ecp5_ops_write_init(struct fpga_manager *mgr,
+			       struct fpga_image_info *info,
+			       const char *buf, size_t count)
+{
+	struct sysconfig_priv *priv = mgr->priv;
+	struct spi_device *spi = priv->spi;
+	struct device *dev = &mgr->dev;
+	struct gpio_desc *program;
+	struct gpio_desc *init;
+	struct gpio_desc *done;
+	int ret;
+
+	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
+		dev_err(dev, "Partial reconfiguration is not supported\n");
+		return -EOPNOTSUPP;
+	}
+
+	program = priv->program;
+	init = priv->init;
+	done = priv->done;
+
+	/* Enter init mode */
+	gpiod_set_value(program, 1);
+
+	ret = sysconfig_poll_gpio(init, true);
+	if (!ret)
+		ret = sysconfig_poll_gpio(done, false);
+
+	if (ret) {
+		dev_err(dev, "Failed to go to init mode\n");
+		return ret;
+	}
+
+	/* Enter program mode */
+	gpiod_set_value(program, 0);
+
+	ret = sysconfig_poll_gpio(init, false);
+	if (ret) {
+		dev_err(dev, "Failed to go to program mode\n");
+		return ret;
+	}
+
+	/* Enter ISC mode */
+	ret = sysconfig_isc_init(priv);
+	if (ret) {
+		dev_err(dev, "Failed to go to ISC mode\n");
+		return ret;
+	}
+
+	/* Initialize the Address Shift Register */
+	ret = sysconfig_lsc_init_addr(priv);
+	if (ret) {
+		dev_err(dev,
+			"Failed to initialize the Address Shift Register\n");
+		return ret;
+	}
+
+	/*
+	 * Lock SPI bus for exclusive usage until FPGA programming is done.
+	 * SPI bus will be released in ecp5_ops_write_complete() or on error.
+	 */
+	spi_bus_lock(spi->controller);
+
+	/* Prepare for bitstream burst write */
+	ret = sysconfig_lsc_bitstream_burst(priv);
+	if (ret) {
+		dev_err(dev, "Failed to prepare for bitstream burst write\n");
+		spi_bus_unlock(spi->controller);
+	}
+
+	return ret;
+}
+
+static int ecp5_ops_write(struct fpga_manager *mgr, const char *buf, size_t count)
+{
+	struct sysconfig_priv *priv = mgr->priv;
+	struct spi_device *spi = priv->spi;
+	struct spi_transfer xfer = {
+		.tx_buf = buf,
+		.len = count,
+		.cs_change = 1,
+	};
+	struct spi_message msg;
+	int ret;
+
+	spi_message_init_with_transfers(&msg, &xfer, 1);
+	ret = spi_sync_locked(spi, &msg);
+	if (ret)
+		spi_bus_unlock(spi->controller);
+
+	return ret;
+}
+
+static int ecp5_ops_write_complete(struct fpga_manager *mgr,
+				   struct fpga_image_info *info)
+{
+	struct sysconfig_priv *priv = mgr->priv;
+	struct spi_device *spi = priv->spi;
+	struct device *dev = &mgr->dev;
+	int ret;
+
+	/* Bitstream burst write is done, release SPI bus */
+	spi_bus_unlock(spi->controller);
+
+	/* Toggle CS and wait for bitstream write to finish */
+	ret = spi_write(spi, NULL, 0);
+	if (!ret)
+		ret = sysconfig_poll_busy(priv);
+
+	if (ret) {
+		dev_err(dev, "Error while waiting bitstream write to finish\n");
+		return ret;
+	}
+
+	/* Exit ISC mode */
+	ret = sysconfig_isc_disable(priv);
+	if (!ret)
+		ret = sysconfig_poll_gpio(priv->done, true);
+
+	if (ret)
+		dev_err(dev, "Failed to finish ISC\n");
+
+	return ret;
+}
+
+static const struct fpga_manager_ops ecp5_fpga_ops = {
+	.state = ecp5_ops_state,
+	.write_init = ecp5_ops_write_init,
+	.write = ecp5_ops_write,
+	.write_complete = ecp5_ops_write_complete,
+};
+
+static int ecp5_probe(struct sysconfig_priv *priv)
+{
+	struct spi_device *spi = priv->spi;
+	struct device *dev = &spi->dev;
+	struct fpga_manager *mgr;
+	int ret;
+
+	if (spi->max_speed_hz > ECP5_SPI_MAX_SPEED_HZ) {
+		dev_err(dev, "SPI speed %u is too high, maximum speed is %u\n",
+			spi->max_speed_hz, ECP5_SPI_MAX_SPEED_HZ);
+		return -EINVAL;
+	}
+
+	priv->isc_erase_operand = ECP5_ISC_ERASE_OPERAND;
+
+	priv->done = devm_gpiod_get(dev, "done", GPIOD_IN);
+	if (IS_ERR(priv->done)) {
+		ret = PTR_ERR(priv->done);
+		dev_err(dev, "Failed to get DONE GPIO: %d\n", ret);
+		return ret;
+	}
+
+	priv->init = devm_gpiod_get(dev, "init", GPIOD_IN);
+	if (IS_ERR(priv->init)) {
+		ret = PTR_ERR(priv->init);
+		dev_err(dev, "Failed to get INIT GPIO: %d\n", ret);
+		return ret;
+	}
+
+	priv->program = devm_gpiod_get(dev, "program", GPIOD_OUT_LOW);
+	if (IS_ERR(priv->program)) {
+		ret = PTR_ERR(priv->program);
+		dev_err(dev, "Failed to get PROGRAM GPIO: %d\n", ret);
+		return ret;
+	}
+
+	mgr = devm_fpga_mgr_register(dev, "Lattice ECP5 SPI FPGA Manager",
+				     &ecp5_fpga_ops, priv);
+
+	return PTR_ERR_OR_ZERO(mgr);
+}
+
+static enum fpga_mgr_states machxo2_ops_state(struct fpga_manager *mgr)
+{
+	struct sysconfig_priv *priv = mgr->priv;
+	u32 status;
+	int ret;
+
+	ret = sysconfig_read_status(priv, &status);
+	if (ret || !(status & SYSCONFIG_STATUS_DONE))
+		return FPGA_MGR_STATE_UNKNOWN;
+
+	return FPGA_MGR_STATE_OPERATING;
+}
+
+static int machxo2_ops_write_init(struct fpga_manager *mgr,
+				  struct fpga_image_info *info,
+				  const char *buf, size_t count)
+{
+	struct sysconfig_priv *priv = mgr->priv;
+	struct device *dev = &mgr->dev;
+	int ret;
+
+	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
+		dev_err(dev, "Partial reconfiguration is not supported\n");
+		return -EOPNOTSUPP;
+	}
+
+	/* Enter ISC mode */
+	ret = sysconfig_isc_init(priv);
+	if (ret) {
+		dev_err(dev, "Failed to go to ISC mode\n");
+		return ret;
+	}
+
+	/* Initialize the Address Shift Register */
+	ret = sysconfig_lsc_init_addr(priv);
+	if (ret)
+		dev_err(dev,
+			"Failed to initialize the Address Shift Register\n");
+
+	return ret;
+}
+
+static int machxo2_ops_write(struct fpga_manager *mgr, const char *buf, size_t count)
+{
+	const u8 lsc_progincr[] = SYSCONFIG_LSC_PROG_INCR_NV;
+	struct sysconfig_priv *priv = mgr->priv;
+	struct device *dev = &mgr->dev;
+	struct spi_transfer xfers[2] = {
+		{
+			.tx_buf = lsc_progincr,
+			.len = sizeof(lsc_progincr),
+		}, {
+			.len = MACHXO2_PAGE_SIZE,
+		},
+	};
+	size_t i;
+	int ret;
+
+	if (count % MACHXO2_PAGE_SIZE) {
+		dev_err(dev, "Malformed payload.\n");
+		return -EINVAL;
+	}
+
+	for (i = 0; i < count; i += MACHXO2_PAGE_SIZE) {
+		xfers[1].tx_buf = buf + i;
+
+		ret = spi_sync_transfer(priv->spi, xfers, 2);
+		if (!ret)
+			ret = sysconfig_poll_busy(priv);
+
+		if (ret) {
+			dev_err(dev, "Failed to write frame %zu of %zu\n",
+				i / MACHXO2_PAGE_SIZE, count / MACHXO2_PAGE_SIZE);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static void machxo2_cleanup(struct sysconfig_priv *data)
+{
+	sysconfig_isc_erase(data);
+	sysconfig_refresh(data);
+}
+
+static int machxo2_ops_write_complete(struct fpga_manager *mgr,
+				      struct fpga_image_info *info)
+{
+	int ret, retries = SYSCONFIG_POLL_RETRIES;
+	struct sysconfig_priv *priv = mgr->priv;
+	struct device *dev = &mgr->dev;
+	u32 status;
+
+	ret = sysconfig_isc_prog_done(priv);
+	if (ret) {
+		dev_err(dev, "Failed to enable Self-Download Mode\n");
+		goto fail;
+	}
+
+	ret = sysconfig_isc_disable(priv);
+	if (ret) {
+		dev_err(dev, "Failed to disable Configuration Interface\n");
+		goto fail;
+	}
+
+	while (retries--) {
+		ret = sysconfig_refresh(priv);
+		if (!ret)
+			ret = sysconfig_read_status(priv, &status);
+
+		if (ret) {
+			dev_err(dev, "Failed to refresh\n");
+			break;
+		}
+
+		if (status & SYSCONFIG_STATUS_DONE &&
+		    !(status & SYSCONFIG_STATUS_BUSY) &&
+		    !(status & SYSCONFIG_STATUS_ERR))
+			return 0;
+	}
+
+fail:
+	machxo2_cleanup(priv);
+
+	return -EFAULT;
+}
+
+static const struct fpga_manager_ops machxo2_fpga_ops = {
+	.state = machxo2_ops_state,
+	.write_init = machxo2_ops_write_init,
+	.write = machxo2_ops_write,
+	.write_complete = machxo2_ops_write_complete,
+};
+
+static int machxo2_probe(struct sysconfig_priv *priv)
+{
+	struct spi_device *spi = priv->spi;
+	struct device *dev = &spi->dev;
+	struct fpga_manager *mgr;
+
+	if (spi->max_speed_hz > MACHXO2_SPI_MAX_SPEED_HZ) {
+		dev_err(dev, "SPI speed %u is too high, maximum speed is %u\n",
+			spi->max_speed_hz, MACHXO2_SPI_MAX_SPEED_HZ);
+		return -EINVAL;
+	}
+
+	priv->isc_enable_operand = MACHXO2_ISC_ENABLE_OPERAND;
+	priv->isc_erase_operand = MACHXO2_ISC_ERASE_OPERAND;
+
+	mgr = devm_fpga_mgr_register(dev, "Lattice MachXO2 SPI FPGA Manager",
+				     &machxo2_fpga_ops, priv);
+
+	return PTR_ERR_OR_ZERO(mgr);
+}
+
+typedef int (*lattice_fpga_probe_func)(struct sysconfig_priv *);
+
+static int sysconfig_probe(struct spi_device *spi)
+{
+	const struct spi_device_id *dev_id;
+	lattice_fpga_probe_func probe_func;
+	struct device *dev = &spi->dev;
+	struct sysconfig_priv *priv;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->spi = spi;
+
+	probe_func = of_device_get_match_data(&spi->dev);
+	if (!probe_func) {
+		dev_id = spi_get_device_id(spi);
+		if (!dev_id)
+			return -ENODEV;
+
+		probe_func = (lattice_fpga_probe_func)dev_id->driver_data;
+	}
+
+	if (!probe_func)
+		return -EINVAL;
+
+	return probe_func(priv);
+}
+
+static const struct spi_device_id sysconfig_spi_ids[] = {
+	{
+		.name = "ecp5-fpga-mgr",
+		.driver_data = (kernel_ulong_t)ecp5_probe,
+	}, {
+		.name = "machxo2-fpga-mgr",
+		.driver_data = (kernel_ulong_t)machxo2_probe,
+	}, {},
+};
+MODULE_DEVICE_TABLE(spi, sysconfig_spi_ids);
+
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id sysconfig_of_ids[] = {
+	{
+		.compatible = "lattice,ecp5-fpga-mgr",
+		.data = ecp5_probe,
+	}, {
+		.compatible = "lattice,machxo2-fpga-mgr",
+		.data = machxo2_probe
+	}, {},
+};
+MODULE_DEVICE_TABLE(of, sysconfig_of_ids);
+#endif /* IS_ENABLED(CONFIG_OF) */
+
+static struct spi_driver lattice_sysconfig_driver = {
+	.probe = sysconfig_probe,
+	.id_table = sysconfig_spi_ids,
+	.driver = {
+		.name = "lattice_sysconfig_spi_fpga_mgr",
+		.of_match_table = of_match_ptr(sysconfig_of_ids),
+	},
+};
+
+module_spi_driver(lattice_sysconfig_driver);
+
+MODULE_DESCRIPTION("Lattice sysCONFIG Slave SPI FPGA Manager");
+MODULE_LICENSE("GPL");
-- 
2.37.2

[PATCH v8 2/2] dt-bindings: fpga: document Lattice sysCONFIG FPGA manager

[Ivan Bornyakov]

Add Device Tree Binding doc for configuring Lattice ECP5 and MachXO2
FPGAs over Slave SPI sysCONFIG interface.

Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
---
 .../bindings/fpga/lattice,sysconfig.yaml      | 109 ++++++++++++++++++
 1 file changed, 109 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/fpga/lattice,sysconfig.yaml

diff --git a/Documentation/devicetree/bindings/fpga/lattice,sysconfig.yaml b/Documentation/devicetree/bindings/fpga/lattice,sysconfig.yaml
new file mode 100644
index 000000000000..3ea338a05bb5
--- /dev/null
+++ b/Documentation/devicetree/bindings/fpga/lattice,sysconfig.yaml
@@ -0,0 +1,109 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/fpga/lattice,sysconfig.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Lattice Slave SPI sysCONFIG FPGA manager
+
+maintainers:
+  - Ivan Bornyakov <i.bornyakov@metrotek.ru>
+
+description: |
+  Lattice sysCONFIG port, which is used for FPGA configuration, among others,
+  have Slave Serial Peripheral Interface. Only full reconfiguration is
+  supported.
+
+  Programming of ECP5 is done by writing uncompressed bitstream image in .bit
+  format into FPGA's SRAM configuration memory.
+
+  Programming of MachXO2 is done by writing configuration data into device's
+  internal non-volatile Flash memory, then Self-Download of data from Flash
+  into SRAM is issued.
+
+properties:
+  compatible:
+    enum:
+      - lattice,ecp5-fpga-mgr
+      - lattice,machxo2-fpga-mgr
+
+  reg:
+    maxItems: 1
+
+  program-gpios:
+    description:
+      A GPIO line connected to PROGRAMN (active low) pin of the device.
+      Initiates configuration sequence.
+    maxItems: 1
+
+  init-gpios:
+    description:
+      A GPIO line connected to INITN (active low) pin of the device.
+      Indicates that the FPGA is ready to be configured.
+    maxItems: 1
+
+  done-gpios:
+    description:
+      A GPIO line connected to DONE (active high) pin of the device.
+      Indicates that the configuration sequence is complete.
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: lattice,machxo2-fpga-mgr
+    then:
+      properties:
+        spi-max-frequency:
+          maximum: 66000000
+        program-gpios: false
+        init-gpios: false
+        done-gpios: false
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: lattice,ecp5-fpga-mgr
+    then:
+      properties:
+        spi-max-frequency:
+          maximum: 60000000
+      required:
+        - program-gpios
+        - init-gpios
+        - done-gpios
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        fpga-mgr@0 {
+            compatible = "lattice,ecp5-fpga-mgr";
+            reg = <0>;
+            spi-max-frequency = <20000000>;
+            program-gpios = <&gpio3 4 GPIO_ACTIVE_LOW>;
+            init-gpios = <&gpio3 3 GPIO_ACTIVE_LOW>;
+            done-gpios = <&gpio3 2 GPIO_ACTIVE_HIGH>;
+        };
+
+        fpga-mgr@1 {
+            compatible = "lattice,machxo2-fpga-mgr";
+            reg = <1>;
+            spi-max-frequency = <20000000>;
+        };
+    };
-- 
2.37.2

Re: [PATCH v8 2/2] dt-bindings: fpga: document Lattice sysCONFIG FPGA manager

[Krzysztof Kozlowski]

On 25/08/2022 14:24, Ivan Bornyakov wrote:
> Add Device Tree Binding doc for configuring Lattice ECP5 and MachXO2
> FPGAs over Slave SPI sysCONFIG interface.
> 
> Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

Re: [PATCH v8 1/2] fpga: lattice-sysconfig-spi: add Lattice sysCONFIG FPGA manager

[Ivan Bornyakov]

On Thu, Aug 25, 2022 at 02:24:32PM +0300, Ivan Bornyakov wrote:
> Add support to the FPGA manager for programming Lattice ECP5 and MachXO2
> FPGAs over slave SPI sysCONFIG interface.
> 
> Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
> ---
>  drivers/fpga/Kconfig                 |   7 +
>  drivers/fpga/Makefile                |   1 +
>  drivers/fpga/lattice-sysconfig-spi.c | 630 +++++++++++++++++++++++++++
>  3 files changed, 638 insertions(+)
>  create mode 100644 drivers/fpga/lattice-sysconfig-spi.c
> 
> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> index 6c416955da53..991d9d976dca 100644
> --- a/drivers/fpga/Kconfig
> +++ b/drivers/fpga/Kconfig
> @@ -263,4 +263,11 @@ config FPGA_MGR_MICROCHIP_SPI
>  	  programming over slave SPI interface with .dat formatted
>  	  bitstream image.
>  
> +config FPGA_MGR_LATTICE_SPI
> +	tristate "Lattice sysCONFIG SPI FPGA manager"
> +	depends on SPI
> +	help
> +	  FPGA manager driver support for Lattice FPGAs programming over slave
> +	  SPI sysCONFIG interface.
> +
>  endif # FPGA
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index 42ae8b58abce..115dba916024 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -20,6 +20,7 @@ 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
>  obj-$(CONFIG_FPGA_MGR_MICROCHIP_SPI)	+= microchip-spi.o
> +obj-$(CONFIG_FPGA_MGR_LATTICE_SPI)	+= lattice-sysconfig-spi.o
>  obj-$(CONFIG_ALTERA_PR_IP_CORE)		+= altera-pr-ip-core.o
>  obj-$(CONFIG_ALTERA_PR_IP_CORE_PLAT)	+= altera-pr-ip-core-plat.o
>  
> diff --git a/drivers/fpga/lattice-sysconfig-spi.c b/drivers/fpga/lattice-sysconfig-spi.c
> new file mode 100644
> index 000000000000..145b5b27b88d
> --- /dev/null
> +++ b/drivers/fpga/lattice-sysconfig-spi.c
> @@ -0,0 +1,630 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Lattice FPGA programming over slave SPI sysCONFIG interface.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/fpga/fpga-mgr.h>
> +#include <linux/of_device.h>
> +#include <linux/spi/spi.h>
> +
> +#define	SYSCONFIG_ISC_ENABLE		{0xC6, 0x00, 0x00, 0x00}
> +#define	SYSCONFIG_ISC_DISABLE		{0x26, 0x00, 0x00, 0x00}
> +#define	SYSCONFIG_ISC_ERASE		{0x0E, 0x00, 0x00, 0x00}
> +#define	SYSCONFIG_ISC_PROGRAM_DONE	{0x5E, 0x00, 0x00, 0x00}
> +#define	SYSCONFIG_LSC_READ_STATUS	{0x3C, 0x00, 0x00, 0x00}
> +#define	SYSCONFIG_LSC_CHECK_BUSY	{0xF0, 0x00, 0x00, 0x00}
> +#define	SYSCONFIG_LSC_REFRESH		{0x79, 0x00, 0x00, 0x00}
> +#define	SYSCONFIG_LSC_INIT_ADDR		{0x46, 0x00, 0x00, 0x00}
> +#define	SYSCONFIG_LSC_BITSTREAM_BURST	{0x7a, 0x00, 0x00, 0x00}
> +#define	SYSCONFIG_LSC_PROG_INCR_NV	{0x70, 0x00, 0x00, 0x01}
> +
> +#define	SYSCONFIG_STATUS_DONE		BIT(8)
> +#define	SYSCONFIG_STATUS_BUSY		BIT(12)
> +#define	SYSCONFIG_STATUS_FAIL		BIT(13)
> +#define	SYSCONFIG_STATUS_ERR		(BIT(23) | BIT(24) | BIT(25))
> +
> +#define	SYSCONFIG_POLL_RETRIES		100000
> +
> +#define	ECP5_SPI_MAX_SPEED_HZ		60000000
> +#define	ECP5_ISC_ERASE_OPERAND		0x01
> +
> +#define	MACHXO2_SPI_MAX_SPEED_HZ	66000000
> +#define	MACHXO2_PAGE_SIZE		16
> +#define	MACHXO2_ISC_ENABLE_OPERAND	0x08
> +#define	MACHXO2_ISC_ERASE_OPERAND	0x04
> +
> +struct sysconfig_priv {
> +	struct gpio_desc *program;
> +	struct gpio_desc *init;
> +	struct gpio_desc *done;
> +	struct spi_device *spi;
> +	u8 isc_enable_operand;
> +	u8 isc_erase_operand;
> +};
> +
> +static int sysconfig_poll_busy(struct sysconfig_priv *data)
> +{
> +	const u8 lsc_check_busy[] = SYSCONFIG_LSC_CHECK_BUSY;
> +	int ret, retries = SYSCONFIG_POLL_RETRIES;
> +	u8 busy;
> +
> +	while (retries--) {
> +		ret = spi_write_then_read(data->spi,
> +					  lsc_check_busy, sizeof(lsc_check_busy),
> +					  &busy, sizeof(busy));
> +		if (ret)
> +			return ret;
> +
> +		if (!busy)
> +			return 0;
> +
> +		usleep_range(50, 100);
> +	}
> +
> +	return -EBUSY;
> +}
> +
> +static int sysconfig_read_status(struct sysconfig_priv *data, u32 *status)
> +{
> +	const u8 lsc_read_status[] = SYSCONFIG_LSC_READ_STATUS;
> +	__be32 device_status;
> +	int ret;
> +
> +	ret = spi_write_then_read(data->spi,
> +				  lsc_read_status, sizeof(lsc_read_status),
> +				  &device_status, sizeof(device_status));
> +	if (ret)
> +		return ret;
> +
> +	*status = be32_to_cpu(device_status);
> +
> +	return 0;
> +}
> +
> +static int sysconfig_poll_status(struct sysconfig_priv *data, u32 *status)
> +{
> +	int ret;
> +
> +	ret = sysconfig_poll_busy(data);
> +	if (ret)
> +		return ret;
> +
> +	return sysconfig_read_status(data, status);
> +}
> +
> +static int sysconfig_poll_gpio(struct gpio_desc *gpio, bool is_active)
> +{
> +	int value, retries = SYSCONFIG_POLL_RETRIES;
> +
> +	while (retries--) {
> +		value = gpiod_get_value(gpio);
> +		if (value < 0)
> +			return value;
> +
> +		if ((!is_active && !value) || (is_active && value))
> +			return 0;
> +
> +		ndelay(10);
> +	}
> +
> +	return -ETIMEDOUT;
> +}
> +
> +static int sysconfig_refresh(struct sysconfig_priv *data)
> +{
> +	static const u8 lsc_refresh[] = SYSCONFIG_LSC_REFRESH;
> +	int ret;
> +
> +	ret = spi_write(data->spi, lsc_refresh, sizeof(lsc_refresh));
> +	if (ret)
> +		return ret;
> +
> +	usleep_range(4000, 8000);
> +
> +	return 0;
> +}
> +
> +static int sysconfig_isc_enable(struct sysconfig_priv *data)
> +{
> +	u8 isc_enable[] = SYSCONFIG_ISC_ENABLE;
> +	u32 status;
> +	int ret;
> +
> +	isc_enable[1] = data->isc_enable_operand;
> +
> +	ret = spi_write(data->spi, isc_enable, sizeof(isc_enable));
> +	if (ret)
> +		return ret;
> +
> +	ret = sysconfig_poll_status(data, &status);
> +	if (ret || (status & SYSCONFIG_STATUS_FAIL))
> +		return ret ? : -EFAULT;
> +
> +	return 0;
> +}
> +
> +static int sysconfig_isc_erase(struct sysconfig_priv *data)
> +{
> +	u8 isc_erase[] = SYSCONFIG_ISC_ERASE;
> +	u32 status;
> +	int ret;
> +
> +	isc_erase[1] = data->isc_erase_operand;
> +
> +	ret = spi_write(data->spi, isc_erase, sizeof(isc_erase));
> +	if (ret)
> +		return ret;
> +
> +	ret = sysconfig_poll_status(data, &status);
> +	if (ret || (status & SYSCONFIG_STATUS_FAIL))
> +		return ret ? : -EFAULT;
> +
> +	return 0;
> +}
> +
> +static int sysconfig_isc_init(struct sysconfig_priv *data)
> +{
> +	int ret;
> +
> +	ret = sysconfig_isc_enable(data);
> +	if (ret)
> +		return ret;
> +
> +	return sysconfig_isc_erase(data);
> +}
> +
> +static int sysconfig_lsc_init_addr(struct sysconfig_priv *data)
> +{
> +	const u8 lsc_init_addr[] = SYSCONFIG_LSC_INIT_ADDR;
> +
> +	return spi_write(data->spi, lsc_init_addr, sizeof(lsc_init_addr));
> +}
> +
> +static int sysconfig_lsc_bitstream_burst(struct sysconfig_priv *data)
> +{
> +	const u8 lsc_bitstream_burst[] = SYSCONFIG_LSC_BITSTREAM_BURST;
> +	struct spi_transfer xfer = {
> +		.tx_buf = lsc_bitstream_burst,
> +		.len = sizeof(lsc_bitstream_burst),
> +		.cs_change = 1,
> +	};
> +	struct spi_message msg;
> +
> +	spi_message_init_with_transfers(&msg, &xfer, 1);
> +
> +	return spi_sync_locked(data->spi, &msg);
> +}
> +
> +static int sysconfig_isc_prog_done(struct sysconfig_priv *data)
> +{
> +	const u8 isc_prog_done[] = SYSCONFIG_ISC_PROGRAM_DONE;
> +	u32 status;
> +	int ret;
> +
> +	ret = spi_write(data->spi, isc_prog_done, sizeof(isc_prog_done));
> +	if (ret)
> +		return ret;
> +
> +	ret = sysconfig_poll_status(data, &status);
> +	if (ret)
> +		return ret;
> +
> +	if (status & SYSCONFIG_STATUS_DONE)
> +		return 0;
> +
> +	return -EFAULT;
> +}
> +
> +static int sysconfig_isc_disable(struct sysconfig_priv *data)
> +{
> +	const u8 isc_disable[] = SYSCONFIG_ISC_DISABLE;
> +
> +	return spi_write(data->spi, isc_disable, sizeof(isc_disable));
> +}
> +
> +static enum fpga_mgr_states ecp5_ops_state(struct fpga_manager *mgr)
> +{
> +	struct sysconfig_priv *priv = mgr->priv;
> +
> +	return (gpiod_get_value(priv->done) > 0) ? FPGA_MGR_STATE_OPERATING :
> +						   FPGA_MGR_STATE_UNKNOWN;
> +}
> +
> +static int ecp5_ops_write_init(struct fpga_manager *mgr,
> +			       struct fpga_image_info *info,
> +			       const char *buf, size_t count)
> +{
> +	struct sysconfig_priv *priv = mgr->priv;
> +	struct spi_device *spi = priv->spi;
> +	struct device *dev = &mgr->dev;
> +	struct gpio_desc *program;
> +	struct gpio_desc *init;
> +	struct gpio_desc *done;
> +	int ret;
> +
> +	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
> +		dev_err(dev, "Partial reconfiguration is not supported\n");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	program = priv->program;
> +	init = priv->init;
> +	done = priv->done;
> +
> +	/* Enter init mode */
> +	gpiod_set_value(program, 1);
> +
> +	ret = sysconfig_poll_gpio(init, true);
> +	if (!ret)
> +		ret = sysconfig_poll_gpio(done, false);
> +
> +	if (ret) {
> +		dev_err(dev, "Failed to go to init mode\n");
> +		return ret;
> +	}
> +
> +	/* Enter program mode */
> +	gpiod_set_value(program, 0);
> +
> +	ret = sysconfig_poll_gpio(init, false);
> +	if (ret) {
> +		dev_err(dev, "Failed to go to program mode\n");
> +		return ret;
> +	}
> +
> +	/* Enter ISC mode */
> +	ret = sysconfig_isc_init(priv);
> +	if (ret) {
> +		dev_err(dev, "Failed to go to ISC mode\n");
> +		return ret;
> +	}
> +
> +	/* Initialize the Address Shift Register */
> +	ret = sysconfig_lsc_init_addr(priv);
> +	if (ret) {
> +		dev_err(dev,
> +			"Failed to initialize the Address Shift Register\n");
> +		return ret;
> +	}
> +
> +	/*
> +	 * Lock SPI bus for exclusive usage until FPGA programming is done.
> +	 * SPI bus will be released in ecp5_ops_write_complete() or on error.
> +	 */
> +	spi_bus_lock(spi->controller);
> +
> +	/* Prepare for bitstream burst write */
> +	ret = sysconfig_lsc_bitstream_burst(priv);
> +	if (ret) {
> +		dev_err(dev, "Failed to prepare for bitstream burst write\n");
> +		spi_bus_unlock(spi->controller);
> +	}
> +
> +	return ret;
> +}
> +
> +static int ecp5_ops_write(struct fpga_manager *mgr, const char *buf, size_t count)
> +{
> +	struct sysconfig_priv *priv = mgr->priv;
> +	struct spi_device *spi = priv->spi;
> +	struct spi_transfer xfer = {
> +		.tx_buf = buf,
> +		.len = count,
> +		.cs_change = 1,
> +	};
> +	struct spi_message msg;
> +	int ret;
> +
> +	spi_message_init_with_transfers(&msg, &xfer, 1);
> +	ret = spi_sync_locked(spi, &msg);
> +	if (ret)
> +		spi_bus_unlock(spi->controller);
> +
> +	return ret;
> +}
> +
> +static int ecp5_ops_write_complete(struct fpga_manager *mgr,
> +				   struct fpga_image_info *info)
> +{
> +	struct sysconfig_priv *priv = mgr->priv;
> +	struct spi_device *spi = priv->spi;
> +	struct device *dev = &mgr->dev;
> +	int ret;
> +
> +	/* Bitstream burst write is done, release SPI bus */
> +	spi_bus_unlock(spi->controller);
> +
> +	/* Toggle CS and wait for bitstream write to finish */
> +	ret = spi_write(spi, NULL, 0);
> +	if (!ret)
> +		ret = sysconfig_poll_busy(priv);
> +
> +	if (ret) {
> +		dev_err(dev, "Error while waiting bitstream write to finish\n");
> +		return ret;
> +	}
> +
> +	/* Exit ISC mode */
> +	ret = sysconfig_isc_disable(priv);
> +	if (!ret)
> +		ret = sysconfig_poll_gpio(priv->done, true);
> +
> +	if (ret)
> +		dev_err(dev, "Failed to finish ISC\n");
> +
> +	return ret;
> +}
> +
> +static const struct fpga_manager_ops ecp5_fpga_ops = {
> +	.state = ecp5_ops_state,
> +	.write_init = ecp5_ops_write_init,
> +	.write = ecp5_ops_write,
> +	.write_complete = ecp5_ops_write_complete,
> +};
> +
> +static int ecp5_probe(struct sysconfig_priv *priv)
> +{
> +	struct spi_device *spi = priv->spi;
> +	struct device *dev = &spi->dev;
> +	struct fpga_manager *mgr;
> +	int ret;
> +
> +	if (spi->max_speed_hz > ECP5_SPI_MAX_SPEED_HZ) {
> +		dev_err(dev, "SPI speed %u is too high, maximum speed is %u\n",
> +			spi->max_speed_hz, ECP5_SPI_MAX_SPEED_HZ);
> +		return -EINVAL;
> +	}
> +
> +	priv->isc_erase_operand = ECP5_ISC_ERASE_OPERAND;
> +
> +	priv->done = devm_gpiod_get(dev, "done", GPIOD_IN);
> +	if (IS_ERR(priv->done)) {
> +		ret = PTR_ERR(priv->done);
> +		dev_err(dev, "Failed to get DONE GPIO: %d\n", ret);
> +		return ret;
> +	}
> +
> +	priv->init = devm_gpiod_get(dev, "init", GPIOD_IN);
> +	if (IS_ERR(priv->init)) {
> +		ret = PTR_ERR(priv->init);
> +		dev_err(dev, "Failed to get INIT GPIO: %d\n", ret);
> +		return ret;
> +	}
> +
> +	priv->program = devm_gpiod_get(dev, "program", GPIOD_OUT_LOW);
> +	if (IS_ERR(priv->program)) {
> +		ret = PTR_ERR(priv->program);
> +		dev_err(dev, "Failed to get PROGRAM GPIO: %d\n", ret);
> +		return ret;
> +	}
> +
> +	mgr = devm_fpga_mgr_register(dev, "Lattice ECP5 SPI FPGA Manager",
> +				     &ecp5_fpga_ops, priv);
> +
> +	return PTR_ERR_OR_ZERO(mgr);
> +}
> +
> +static enum fpga_mgr_states machxo2_ops_state(struct fpga_manager *mgr)
> +{
> +	struct sysconfig_priv *priv = mgr->priv;
> +	u32 status;
> +	int ret;
> +
> +	ret = sysconfig_read_status(priv, &status);
> +	if (ret || !(status & SYSCONFIG_STATUS_DONE))
> +		return FPGA_MGR_STATE_UNKNOWN;
> +
> +	return FPGA_MGR_STATE_OPERATING;
> +}
> +
> +static int machxo2_ops_write_init(struct fpga_manager *mgr,
> +				  struct fpga_image_info *info,
> +				  const char *buf, size_t count)
> +{
> +	struct sysconfig_priv *priv = mgr->priv;
> +	struct device *dev = &mgr->dev;
> +	int ret;
> +
> +	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
> +		dev_err(dev, "Partial reconfiguration is not supported\n");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	/* Enter ISC mode */
> +	ret = sysconfig_isc_init(priv);
> +	if (ret) {
> +		dev_err(dev, "Failed to go to ISC mode\n");
> +		return ret;
> +	}
> +
> +	/* Initialize the Address Shift Register */
> +	ret = sysconfig_lsc_init_addr(priv);
> +	if (ret)
> +		dev_err(dev,
> +			"Failed to initialize the Address Shift Register\n");
> +
> +	return ret;
> +}
> +
> +static int machxo2_ops_write(struct fpga_manager *mgr, const char *buf, size_t count)
> +{
> +	const u8 lsc_progincr[] = SYSCONFIG_LSC_PROG_INCR_NV;
> +	struct sysconfig_priv *priv = mgr->priv;
> +	struct device *dev = &mgr->dev;
> +	struct spi_transfer xfers[2] = {
> +		{
> +			.tx_buf = lsc_progincr,
> +			.len = sizeof(lsc_progincr),
> +		}, {
> +			.len = MACHXO2_PAGE_SIZE,
> +		},
> +	};
> +	size_t i;
> +	int ret;
> +
> +	if (count % MACHXO2_PAGE_SIZE) {
> +		dev_err(dev, "Malformed payload.\n");
> +		return -EINVAL;
> +	}
> +
> +	for (i = 0; i < count; i += MACHXO2_PAGE_SIZE) {
> +		xfers[1].tx_buf = buf + i;
> +
> +		ret = spi_sync_transfer(priv->spi, xfers, 2);
> +		if (!ret)
> +			ret = sysconfig_poll_busy(priv);
> +
> +		if (ret) {
> +			dev_err(dev, "Failed to write frame %zu of %zu\n",
> +				i / MACHXO2_PAGE_SIZE, count / MACHXO2_PAGE_SIZE);
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static void machxo2_cleanup(struct sysconfig_priv *data)
> +{
> +	sysconfig_isc_erase(data);
> +	sysconfig_refresh(data);
> +}
> +
> +static int machxo2_ops_write_complete(struct fpga_manager *mgr,
> +				      struct fpga_image_info *info)
> +{
> +	int ret, retries = SYSCONFIG_POLL_RETRIES;
> +	struct sysconfig_priv *priv = mgr->priv;
> +	struct device *dev = &mgr->dev;
> +	u32 status;
> +
> +	ret = sysconfig_isc_prog_done(priv);
> +	if (ret) {
> +		dev_err(dev, "Failed to enable Self-Download Mode\n");
> +		goto fail;
> +	}
> +
> +	ret = sysconfig_isc_disable(priv);
> +	if (ret) {
> +		dev_err(dev, "Failed to disable Configuration Interface\n");
> +		goto fail;
> +	}
> +
> +	while (retries--) {
> +		ret = sysconfig_refresh(priv);
> +		if (!ret)
> +			ret = sysconfig_read_status(priv, &status);
> +
> +		if (ret) {
> +			dev_err(dev, "Failed to refresh\n");
> +			break;
> +		}
> +
> +		if (status & SYSCONFIG_STATUS_DONE &&
> +		    !(status & SYSCONFIG_STATUS_BUSY) &&
> +		    !(status & SYSCONFIG_STATUS_ERR))
> +			return 0;
> +	}
> +
> +fail:
> +	machxo2_cleanup(priv);
> +
> +	return -EFAULT;
> +}
> +
> +static const struct fpga_manager_ops machxo2_fpga_ops = {
> +	.state = machxo2_ops_state,
> +	.write_init = machxo2_ops_write_init,
> +	.write = machxo2_ops_write,
> +	.write_complete = machxo2_ops_write_complete,
> +};
> +
> +static int machxo2_probe(struct sysconfig_priv *priv)
> +{
> +	struct spi_device *spi = priv->spi;
> +	struct device *dev = &spi->dev;
> +	struct fpga_manager *mgr;
> +
> +	if (spi->max_speed_hz > MACHXO2_SPI_MAX_SPEED_HZ) {
> +		dev_err(dev, "SPI speed %u is too high, maximum speed is %u\n",
> +			spi->max_speed_hz, MACHXO2_SPI_MAX_SPEED_HZ);
> +		return -EINVAL;
> +	}
> +
> +	priv->isc_enable_operand = MACHXO2_ISC_ENABLE_OPERAND;
> +	priv->isc_erase_operand = MACHXO2_ISC_ERASE_OPERAND;
> +
> +	mgr = devm_fpga_mgr_register(dev, "Lattice MachXO2 SPI FPGA Manager",
> +				     &machxo2_fpga_ops, priv);
> +
> +	return PTR_ERR_OR_ZERO(mgr);
> +}
> +
> +typedef int (*lattice_fpga_probe_func)(struct sysconfig_priv *);
> +
> +static int sysconfig_probe(struct spi_device *spi)
> +{
> +	const struct spi_device_id *dev_id;
> +	lattice_fpga_probe_func probe_func;
> +	struct device *dev = &spi->dev;
> +	struct sysconfig_priv *priv;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->spi = spi;
> +
> +	probe_func = of_device_get_match_data(&spi->dev);
> +	if (!probe_func) {
> +		dev_id = spi_get_device_id(spi);
> +		if (!dev_id)
> +			return -ENODEV;
> +
> +		probe_func = (lattice_fpga_probe_func)dev_id->driver_data;
> +	}
> +
> +	if (!probe_func)
> +		return -EINVAL;
> +
> +	return probe_func(priv);
> +}
> +
> +static const struct spi_device_id sysconfig_spi_ids[] = {
> +	{
> +		.name = "ecp5-fpga-mgr",
> +		.driver_data = (kernel_ulong_t)ecp5_probe,
> +	}, {
> +		.name = "machxo2-fpga-mgr",
> +		.driver_data = (kernel_ulong_t)machxo2_probe,
> +	}, {},
> +};
> +MODULE_DEVICE_TABLE(spi, sysconfig_spi_ids);
> +
> +#if IS_ENABLED(CONFIG_OF)
> +static const struct of_device_id sysconfig_of_ids[] = {
> +	{
> +		.compatible = "lattice,ecp5-fpga-mgr",
> +		.data = ecp5_probe,
> +	}, {
> +		.compatible = "lattice,machxo2-fpga-mgr",
> +		.data = machxo2_probe
> +	}, {},
> +};
> +MODULE_DEVICE_TABLE(of, sysconfig_of_ids);
> +#endif /* IS_ENABLED(CONFIG_OF) */
> +
> +static struct spi_driver lattice_sysconfig_driver = {
> +	.probe = sysconfig_probe,
> +	.id_table = sysconfig_spi_ids,
> +	.driver = {
> +		.name = "lattice_sysconfig_spi_fpga_mgr",
> +		.of_match_table = of_match_ptr(sysconfig_of_ids),
> +	},
> +};
> +
> +module_spi_driver(lattice_sysconfig_driver);
> +
> +MODULE_DESCRIPTION("Lattice sysCONFIG Slave SPI FPGA Manager");
> +MODULE_LICENSE("GPL");
> -- 
> 2.37.2
> 

Cc: Johannes Zink <j.zink@pengutronix.de>

Re: [PATCH v8 1/2] fpga: lattice-sysconfig-spi: add Lattice sysCONFIG FPGA manager

[Ivan Bornyakov]

On Thu, Aug 25, 2022 at 02:24:32PM +0300, Ivan Bornyakov wrote:
> Add support to the FPGA manager for programming Lattice ECP5 and MachXO2
> FPGAs over slave SPI sysCONFIG interface.
> 
> Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
> ---
>  drivers/fpga/Kconfig                 |   7 +
>  drivers/fpga/Makefile                |   1 +
>  drivers/fpga/lattice-sysconfig-spi.c | 630 +++++++++++++++++++++++++++
>  3 files changed, 638 insertions(+)
>  create mode 100644 drivers/fpga/lattice-sysconfig-spi.c
> 
> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> index 6c416955da53..991d9d976dca 100644
> --- a/drivers/fpga/Kconfig
> +++ b/drivers/fpga/Kconfig
> @@ -263,4 +263,11 @@ config FPGA_MGR_MICROCHIP_SPI
>  	  programming over slave SPI interface with .dat formatted
>  	  bitstream image.
>  
> +config FPGA_MGR_LATTICE_SPI
> +	tristate "Lattice sysCONFIG SPI FPGA manager"
> +	depends on SPI
> +	help
> +	  FPGA manager driver support for Lattice FPGAs programming over slave
> +	  SPI sysCONFIG interface.
> +
>  endif # FPGA
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index 42ae8b58abce..115dba916024 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -20,6 +20,7 @@ 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
>  obj-$(CONFIG_FPGA_MGR_MICROCHIP_SPI)	+= microchip-spi.o
> +obj-$(CONFIG_FPGA_MGR_LATTICE_SPI)	+= lattice-sysconfig-spi.o
>  obj-$(CONFIG_ALTERA_PR_IP_CORE)		+= altera-pr-ip-core.o
>  obj-$(CONFIG_ALTERA_PR_IP_CORE_PLAT)	+= altera-pr-ip-core-plat.o
>  
> diff --git a/drivers/fpga/lattice-sysconfig-spi.c b/drivers/fpga/lattice-sysconfig-spi.c
> new file mode 100644
> index 000000000000..145b5b27b88d
> --- /dev/null
> +++ b/drivers/fpga/lattice-sysconfig-spi.c
> @@ -0,0 +1,630 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Lattice FPGA programming over slave SPI sysCONFIG interface.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/fpga/fpga-mgr.h>
> +#include <linux/of_device.h>
> +#include <linux/spi/spi.h>
> +
> +#define	SYSCONFIG_ISC_ENABLE		{0xC6, 0x00, 0x00, 0x00}
> +#define	SYSCONFIG_ISC_DISABLE		{0x26, 0x00, 0x00, 0x00}
> +#define	SYSCONFIG_ISC_ERASE		{0x0E, 0x00, 0x00, 0x00}
> +#define	SYSCONFIG_ISC_PROGRAM_DONE	{0x5E, 0x00, 0x00, 0x00}
> +#define	SYSCONFIG_LSC_READ_STATUS	{0x3C, 0x00, 0x00, 0x00}
> +#define	SYSCONFIG_LSC_CHECK_BUSY	{0xF0, 0x00, 0x00, 0x00}
> +#define	SYSCONFIG_LSC_REFRESH		{0x79, 0x00, 0x00, 0x00}
> +#define	SYSCONFIG_LSC_INIT_ADDR		{0x46, 0x00, 0x00, 0x00}
> +#define	SYSCONFIG_LSC_BITSTREAM_BURST	{0x7a, 0x00, 0x00, 0x00}
> +#define	SYSCONFIG_LSC_PROG_INCR_NV	{0x70, 0x00, 0x00, 0x01}
> +
> +#define	SYSCONFIG_STATUS_DONE		BIT(8)
> +#define	SYSCONFIG_STATUS_BUSY		BIT(12)
> +#define	SYSCONFIG_STATUS_FAIL		BIT(13)
> +#define	SYSCONFIG_STATUS_ERR		(BIT(23) | BIT(24) | BIT(25))
> +
> +#define	SYSCONFIG_POLL_RETRIES		100000
> +
> +#define	ECP5_SPI_MAX_SPEED_HZ		60000000
> +#define	ECP5_ISC_ERASE_OPERAND		0x01
> +
> +#define	MACHXO2_SPI_MAX_SPEED_HZ	66000000
> +#define	MACHXO2_PAGE_SIZE		16
> +#define	MACHXO2_ISC_ENABLE_OPERAND	0x08
> +#define	MACHXO2_ISC_ERASE_OPERAND	0x04
> +
> +struct sysconfig_priv {
> +	struct gpio_desc *program;
> +	struct gpio_desc *init;
> +	struct gpio_desc *done;
> +	struct spi_device *spi;
> +	u8 isc_enable_operand;
> +	u8 isc_erase_operand;
> +};

As Johannes Zink working on adding I2C to MachXO2, I am thinking how
about we add to struct sysconfig_priv a callback

int (*sysconfig_transfer)(struct sysconfig_priv,
			  const void *tx_buf, size_t tx_len,
			  void *rx_buf, size_t rx_len)

For SPI it would be defined like this:

static int sysconfig_spi_transfer(struct sysconfig_priv *data,
				  const void *tx_buf, size_t tx_len,
				  void *rx_buf, size_t rx_len)
{
	if (!rx_buf)
		return spi_write(data->spi, tx_buf, tx_size);
	
	return spi_write_then_read(data->spi, tx_buf, tx_size, rx_buf, rx_size);
}

And later in sysconfig_ commands we would call this callback.

Would it be a good enough starting base for Johannes to add I2C interface
for MachXO2?

> +
> +static int sysconfig_poll_busy(struct sysconfig_priv *data)
> +{
> +	const u8 lsc_check_busy[] = SYSCONFIG_LSC_CHECK_BUSY;
> +	int ret, retries = SYSCONFIG_POLL_RETRIES;
> +	u8 busy;
> +
> +	while (retries--) {
> +		ret = spi_write_then_read(data->spi,
> +					  lsc_check_busy, sizeof(lsc_check_busy),
> +					  &busy, sizeof(busy));
> +		if (ret)
> +			return ret;
> +
> +		if (!busy)
> +			return 0;
> +
> +		usleep_range(50, 100);
> +	}
> +
> +	return -EBUSY;
> +}
> +
> +static int sysconfig_read_status(struct sysconfig_priv *data, u32 *status)
> +{
> +	const u8 lsc_read_status[] = SYSCONFIG_LSC_READ_STATUS;
> +	__be32 device_status;
> +	int ret;
> +
> +	ret = spi_write_then_read(data->spi,
> +				  lsc_read_status, sizeof(lsc_read_status),
> +				  &device_status, sizeof(device_status));
> +	if (ret)
> +		return ret;
> +
> +	*status = be32_to_cpu(device_status);
> +
> +	return 0;
> +}
> +
> +static int sysconfig_poll_status(struct sysconfig_priv *data, u32 *status)
> +{
> +	int ret;
> +
> +	ret = sysconfig_poll_busy(data);
> +	if (ret)
> +		return ret;
> +
> +	return sysconfig_read_status(data, status);
> +}
> +
> +static int sysconfig_poll_gpio(struct gpio_desc *gpio, bool is_active)
> +{
> +	int value, retries = SYSCONFIG_POLL_RETRIES;
> +
> +	while (retries--) {
> +		value = gpiod_get_value(gpio);
> +		if (value < 0)
> +			return value;
> +
> +		if ((!is_active && !value) || (is_active && value))
> +			return 0;
> +
> +		ndelay(10);
> +	}
> +
> +	return -ETIMEDOUT;
> +}
> +
> +static int sysconfig_refresh(struct sysconfig_priv *data)
> +{
> +	static const u8 lsc_refresh[] = SYSCONFIG_LSC_REFRESH;
> +	int ret;
> +
> +	ret = spi_write(data->spi, lsc_refresh, sizeof(lsc_refresh));
> +	if (ret)
> +		return ret;
> +
> +	usleep_range(4000, 8000);
> +
> +	return 0;
> +}
> +
> +static int sysconfig_isc_enable(struct sysconfig_priv *data)
> +{
> +	u8 isc_enable[] = SYSCONFIG_ISC_ENABLE;
> +	u32 status;
> +	int ret;
> +
> +	isc_enable[1] = data->isc_enable_operand;
> +
> +	ret = spi_write(data->spi, isc_enable, sizeof(isc_enable));
> +	if (ret)
> +		return ret;
> +
> +	ret = sysconfig_poll_status(data, &status);
> +	if (ret || (status & SYSCONFIG_STATUS_FAIL))
> +		return ret ? : -EFAULT;
> +
> +	return 0;
> +}

Quirks for I2C enable and refresh commands are 3-bytes instead of
4-bytes for SPI can be added here. Just check which of data->spi or
data->i2c is not NULL.

> +
> +static int sysconfig_isc_erase(struct sysconfig_priv *data)
> +{
> +	u8 isc_erase[] = SYSCONFIG_ISC_ERASE;
> +	u32 status;
> +	int ret;
> +
> +	isc_erase[1] = data->isc_erase_operand;
> +
> +	ret = spi_write(data->spi, isc_erase, sizeof(isc_erase));
> +	if (ret)
> +		return ret;
> +
> +	ret = sysconfig_poll_status(data, &status);
> +	if (ret || (status & SYSCONFIG_STATUS_FAIL))
> +		return ret ? : -EFAULT;
> +
> +	return 0;
> +}
> +
> +static int sysconfig_isc_init(struct sysconfig_priv *data)
> +{
> +	int ret;
> +
> +	ret = sysconfig_isc_enable(data);
> +	if (ret)
> +		return ret;
> +
> +	return sysconfig_isc_erase(data);
> +}
> +
> +static int sysconfig_lsc_init_addr(struct sysconfig_priv *data)
> +{
> +	const u8 lsc_init_addr[] = SYSCONFIG_LSC_INIT_ADDR;
> +
> +	return spi_write(data->spi, lsc_init_addr, sizeof(lsc_init_addr));
> +}
> +
> +static int sysconfig_lsc_bitstream_burst(struct sysconfig_priv *data)
> +{
> +	const u8 lsc_bitstream_burst[] = SYSCONFIG_LSC_BITSTREAM_BURST;
> +	struct spi_transfer xfer = {
> +		.tx_buf = lsc_bitstream_burst,
> +		.len = sizeof(lsc_bitstream_burst),
> +		.cs_change = 1,
> +	};
> +	struct spi_message msg;
> +
> +	spi_message_init_with_transfers(&msg, &xfer, 1);
> +
> +	return spi_sync_locked(data->spi, &msg);
> +}
> +
> +static int sysconfig_isc_prog_done(struct sysconfig_priv *data)
> +{
> +	const u8 isc_prog_done[] = SYSCONFIG_ISC_PROGRAM_DONE;
> +	u32 status;
> +	int ret;
> +
> +	ret = spi_write(data->spi, isc_prog_done, sizeof(isc_prog_done));
> +	if (ret)
> +		return ret;
> +
> +	ret = sysconfig_poll_status(data, &status);
> +	if (ret)
> +		return ret;
> +
> +	if (status & SYSCONFIG_STATUS_DONE)
> +		return 0;
> +
> +	return -EFAULT;
> +}
> +
> +static int sysconfig_isc_disable(struct sysconfig_priv *data)
> +{
> +	const u8 isc_disable[] = SYSCONFIG_ISC_DISABLE;
> +
> +	return spi_write(data->spi, isc_disable, sizeof(isc_disable));
> +}
> +
> +static enum fpga_mgr_states ecp5_ops_state(struct fpga_manager *mgr)
> +{
> +	struct sysconfig_priv *priv = mgr->priv;
> +
> +	return (gpiod_get_value(priv->done) > 0) ? FPGA_MGR_STATE_OPERATING :
> +						   FPGA_MGR_STATE_UNKNOWN;
> +}
> +
> +static int ecp5_ops_write_init(struct fpga_manager *mgr,
> +			       struct fpga_image_info *info,
> +			       const char *buf, size_t count)
> +{
> +	struct sysconfig_priv *priv = mgr->priv;
> +	struct spi_device *spi = priv->spi;
> +	struct device *dev = &mgr->dev;
> +	struct gpio_desc *program;
> +	struct gpio_desc *init;
> +	struct gpio_desc *done;
> +	int ret;
> +
> +	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
> +		dev_err(dev, "Partial reconfiguration is not supported\n");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	program = priv->program;
> +	init = priv->init;
> +	done = priv->done;
> +
> +	/* Enter init mode */
> +	gpiod_set_value(program, 1);
> +
> +	ret = sysconfig_poll_gpio(init, true);
> +	if (!ret)
> +		ret = sysconfig_poll_gpio(done, false);
> +
> +	if (ret) {
> +		dev_err(dev, "Failed to go to init mode\n");
> +		return ret;
> +	}
> +
> +	/* Enter program mode */
> +	gpiod_set_value(program, 0);
> +
> +	ret = sysconfig_poll_gpio(init, false);
> +	if (ret) {
> +		dev_err(dev, "Failed to go to program mode\n");
> +		return ret;
> +	}
> +
> +	/* Enter ISC mode */
> +	ret = sysconfig_isc_init(priv);
> +	if (ret) {
> +		dev_err(dev, "Failed to go to ISC mode\n");
> +		return ret;
> +	}
> +
> +	/* Initialize the Address Shift Register */
> +	ret = sysconfig_lsc_init_addr(priv);
> +	if (ret) {
> +		dev_err(dev,
> +			"Failed to initialize the Address Shift Register\n");
> +		return ret;
> +	}
> +
> +	/*
> +	 * Lock SPI bus for exclusive usage until FPGA programming is done.
> +	 * SPI bus will be released in ecp5_ops_write_complete() or on error.
> +	 */
> +	spi_bus_lock(spi->controller);
> +
> +	/* Prepare for bitstream burst write */
> +	ret = sysconfig_lsc_bitstream_burst(priv);
> +	if (ret) {
> +		dev_err(dev, "Failed to prepare for bitstream burst write\n");
> +		spi_bus_unlock(spi->controller);
> +	}
> +
> +	return ret;
> +}
> +
> +static int ecp5_ops_write(struct fpga_manager *mgr, const char *buf, size_t count)
> +{
> +	struct sysconfig_priv *priv = mgr->priv;
> +	struct spi_device *spi = priv->spi;
> +	struct spi_transfer xfer = {
> +		.tx_buf = buf,
> +		.len = count,
> +		.cs_change = 1,
> +	};
> +	struct spi_message msg;
> +	int ret;
> +
> +	spi_message_init_with_transfers(&msg, &xfer, 1);
> +	ret = spi_sync_locked(spi, &msg);
> +	if (ret)
> +		spi_bus_unlock(spi->controller);
> +
> +	return ret;
> +}
> +
> +static int ecp5_ops_write_complete(struct fpga_manager *mgr,
> +				   struct fpga_image_info *info)
> +{
> +	struct sysconfig_priv *priv = mgr->priv;
> +	struct spi_device *spi = priv->spi;
> +	struct device *dev = &mgr->dev;
> +	int ret;
> +
> +	/* Bitstream burst write is done, release SPI bus */
> +	spi_bus_unlock(spi->controller);
> +
> +	/* Toggle CS and wait for bitstream write to finish */
> +	ret = spi_write(spi, NULL, 0);
> +	if (!ret)
> +		ret = sysconfig_poll_busy(priv);
> +
> +	if (ret) {
> +		dev_err(dev, "Error while waiting bitstream write to finish\n");
> +		return ret;
> +	}
> +
> +	/* Exit ISC mode */
> +	ret = sysconfig_isc_disable(priv);
> +	if (!ret)
> +		ret = sysconfig_poll_gpio(priv->done, true);
> +
> +	if (ret)
> +		dev_err(dev, "Failed to finish ISC\n");
> +
> +	return ret;
> +}
> +
> +static const struct fpga_manager_ops ecp5_fpga_ops = {
> +	.state = ecp5_ops_state,
> +	.write_init = ecp5_ops_write_init,
> +	.write = ecp5_ops_write,
> +	.write_complete = ecp5_ops_write_complete,
> +};
> +
> +static int ecp5_probe(struct sysconfig_priv *priv)
> +{
> +	struct spi_device *spi = priv->spi;
> +	struct device *dev = &spi->dev;
> +	struct fpga_manager *mgr;
> +	int ret;
> +
> +	if (spi->max_speed_hz > ECP5_SPI_MAX_SPEED_HZ) {
> +		dev_err(dev, "SPI speed %u is too high, maximum speed is %u\n",
> +			spi->max_speed_hz, ECP5_SPI_MAX_SPEED_HZ);
> +		return -EINVAL;
> +	}
> +
> +	priv->isc_erase_operand = ECP5_ISC_ERASE_OPERAND;
> +
> +	priv->done = devm_gpiod_get(dev, "done", GPIOD_IN);
> +	if (IS_ERR(priv->done)) {
> +		ret = PTR_ERR(priv->done);
> +		dev_err(dev, "Failed to get DONE GPIO: %d\n", ret);
> +		return ret;
> +	}
> +
> +	priv->init = devm_gpiod_get(dev, "init", GPIOD_IN);
> +	if (IS_ERR(priv->init)) {
> +		ret = PTR_ERR(priv->init);
> +		dev_err(dev, "Failed to get INIT GPIO: %d\n", ret);
> +		return ret;
> +	}
> +
> +	priv->program = devm_gpiod_get(dev, "program", GPIOD_OUT_LOW);
> +	if (IS_ERR(priv->program)) {
> +		ret = PTR_ERR(priv->program);
> +		dev_err(dev, "Failed to get PROGRAM GPIO: %d\n", ret);
> +		return ret;
> +	}
> +
> +	mgr = devm_fpga_mgr_register(dev, "Lattice ECP5 SPI FPGA Manager",
> +				     &ecp5_fpga_ops, priv);
> +
> +	return PTR_ERR_OR_ZERO(mgr);
> +}
> +
> +static enum fpga_mgr_states machxo2_ops_state(struct fpga_manager *mgr)
> +{
> +	struct sysconfig_priv *priv = mgr->priv;
> +	u32 status;
> +	int ret;
> +
> +	ret = sysconfig_read_status(priv, &status);
> +	if (ret || !(status & SYSCONFIG_STATUS_DONE))
> +		return FPGA_MGR_STATE_UNKNOWN;
> +
> +	return FPGA_MGR_STATE_OPERATING;
> +}
> +
> +static int machxo2_ops_write_init(struct fpga_manager *mgr,
> +				  struct fpga_image_info *info,
> +				  const char *buf, size_t count)
> +{
> +	struct sysconfig_priv *priv = mgr->priv;
> +	struct device *dev = &mgr->dev;
> +	int ret;
> +
> +	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
> +		dev_err(dev, "Partial reconfiguration is not supported\n");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	/* Enter ISC mode */
> +	ret = sysconfig_isc_init(priv);
> +	if (ret) {
> +		dev_err(dev, "Failed to go to ISC mode\n");
> +		return ret;
> +	}
> +
> +	/* Initialize the Address Shift Register */
> +	ret = sysconfig_lsc_init_addr(priv);
> +	if (ret)
> +		dev_err(dev,
> +			"Failed to initialize the Address Shift Register\n");
> +
> +	return ret;
> +}
> +
> +static int machxo2_ops_write(struct fpga_manager *mgr, const char *buf, size_t count)
> +{
> +	const u8 lsc_progincr[] = SYSCONFIG_LSC_PROG_INCR_NV;
> +	struct sysconfig_priv *priv = mgr->priv;
> +	struct device *dev = &mgr->dev;
> +	struct spi_transfer xfers[2] = {
> +		{
> +			.tx_buf = lsc_progincr,
> +			.len = sizeof(lsc_progincr),
> +		}, {
> +			.len = MACHXO2_PAGE_SIZE,
> +		},
> +	};
> +	size_t i;
> +	int ret;
> +
> +	if (count % MACHXO2_PAGE_SIZE) {
> +		dev_err(dev, "Malformed payload.\n");
> +		return -EINVAL;
> +	}
> +
> +	for (i = 0; i < count; i += MACHXO2_PAGE_SIZE) {
> +		xfers[1].tx_buf = buf + i;
> +
> +		ret = spi_sync_transfer(priv->spi, xfers, 2);
> +		if (!ret)
> +			ret = sysconfig_poll_busy(priv);
> +
> +		if (ret) {
> +			dev_err(dev, "Failed to write frame %zu of %zu\n",
> +				i / MACHXO2_PAGE_SIZE, count / MACHXO2_PAGE_SIZE);
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static void machxo2_cleanup(struct sysconfig_priv *data)
> +{
> +	sysconfig_isc_erase(data);
> +	sysconfig_refresh(data);
> +}
> +
> +static int machxo2_ops_write_complete(struct fpga_manager *mgr,
> +				      struct fpga_image_info *info)
> +{
> +	int ret, retries = SYSCONFIG_POLL_RETRIES;
> +	struct sysconfig_priv *priv = mgr->priv;
> +	struct device *dev = &mgr->dev;
> +	u32 status;
> +
> +	ret = sysconfig_isc_prog_done(priv);
> +	if (ret) {
> +		dev_err(dev, "Failed to enable Self-Download Mode\n");
> +		goto fail;
> +	}
> +
> +	ret = sysconfig_isc_disable(priv);
> +	if (ret) {
> +		dev_err(dev, "Failed to disable Configuration Interface\n");
> +		goto fail;
> +	}
> +
> +	while (retries--) {
> +		ret = sysconfig_refresh(priv);
> +		if (!ret)
> +			ret = sysconfig_read_status(priv, &status);
> +
> +		if (ret) {
> +			dev_err(dev, "Failed to refresh\n");
> +			break;
> +		}
> +
> +		if (status & SYSCONFIG_STATUS_DONE &&
> +		    !(status & SYSCONFIG_STATUS_BUSY) &&
> +		    !(status & SYSCONFIG_STATUS_ERR))
> +			return 0;
> +	}
> +
> +fail:
> +	machxo2_cleanup(priv);
> +
> +	return -EFAULT;
> +}
> +
> +static const struct fpga_manager_ops machxo2_fpga_ops = {
> +	.state = machxo2_ops_state,
> +	.write_init = machxo2_ops_write_init,
> +	.write = machxo2_ops_write,
> +	.write_complete = machxo2_ops_write_complete,
> +};
> +
> +static int machxo2_probe(struct sysconfig_priv *priv)
> +{
> +	struct spi_device *spi = priv->spi;
> +	struct device *dev = &spi->dev;
> +	struct fpga_manager *mgr;
> +
> +	if (spi->max_speed_hz > MACHXO2_SPI_MAX_SPEED_HZ) {
> +		dev_err(dev, "SPI speed %u is too high, maximum speed is %u\n",
> +			spi->max_speed_hz, MACHXO2_SPI_MAX_SPEED_HZ);
> +		return -EINVAL;
> +	}
> +
> +	priv->isc_enable_operand = MACHXO2_ISC_ENABLE_OPERAND;
> +	priv->isc_erase_operand = MACHXO2_ISC_ERASE_OPERAND;
> +
> +	mgr = devm_fpga_mgr_register(dev, "Lattice MachXO2 SPI FPGA Manager",
> +				     &machxo2_fpga_ops, priv);
> +
> +	return PTR_ERR_OR_ZERO(mgr);
> +}
> +
> +typedef int (*lattice_fpga_probe_func)(struct sysconfig_priv *);
> +
> +static int sysconfig_probe(struct spi_device *spi)
> +{
> +	const struct spi_device_id *dev_id;
> +	lattice_fpga_probe_func probe_func;
> +	struct device *dev = &spi->dev;
> +	struct sysconfig_priv *priv;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->spi = spi;
> +
> +	probe_func = of_device_get_match_data(&spi->dev);
> +	if (!probe_func) {
> +		dev_id = spi_get_device_id(spi);
> +		if (!dev_id)
> +			return -ENODEV;
> +
> +		probe_func = (lattice_fpga_probe_func)dev_id->driver_data;
> +	}
> +
> +	if (!probe_func)
> +		return -EINVAL;
> +
> +	return probe_func(priv);
> +}
> +
> +static const struct spi_device_id sysconfig_spi_ids[] = {
> +	{
> +		.name = "ecp5-fpga-mgr",
> +		.driver_data = (kernel_ulong_t)ecp5_probe,
> +	}, {
> +		.name = "machxo2-fpga-mgr",
> +		.driver_data = (kernel_ulong_t)machxo2_probe,
> +	}, {},
> +};
> +MODULE_DEVICE_TABLE(spi, sysconfig_spi_ids);
> +
> +#if IS_ENABLED(CONFIG_OF)
> +static const struct of_device_id sysconfig_of_ids[] = {
> +	{
> +		.compatible = "lattice,ecp5-fpga-mgr",
> +		.data = ecp5_probe,
> +	}, {
> +		.compatible = "lattice,machxo2-fpga-mgr",
> +		.data = machxo2_probe
> +	}, {},
> +};
> +MODULE_DEVICE_TABLE(of, sysconfig_of_ids);
> +#endif /* IS_ENABLED(CONFIG_OF) */
> +
> +static struct spi_driver lattice_sysconfig_driver = {
> +	.probe = sysconfig_probe,
> +	.id_table = sysconfig_spi_ids,
> +	.driver = {
> +		.name = "lattice_sysconfig_spi_fpga_mgr",
> +		.of_match_table = of_match_ptr(sysconfig_of_ids),
> +	},
> +};
> +
> +module_spi_driver(lattice_sysconfig_driver);
> +
> +MODULE_DESCRIPTION("Lattice sysCONFIG Slave SPI FPGA Manager");
> +MODULE_LICENSE("GPL");
> -- 
> 2.37.2
> 

Re: [PATCH v8 1/2] fpga: lattice-sysconfig-spi: add Lattice sysCONFIG FPGA manager

[Ivan Bornyakov]

On Fri, Aug 26, 2022 at 01:57:21PM +0300, Ivan Bornyakov wrote:
> On Thu, Aug 25, 2022 at 02:24:32PM +0300, Ivan Bornyakov wrote:
> > Add support to the FPGA manager for programming Lattice ECP5 and MachXO2
> > FPGAs over slave SPI sysCONFIG interface.
> > 
> > Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
> > ---
> >
> > ... snip ...
> >
> > +
> > +struct sysconfig_priv {
> > +	struct gpio_desc *program;
> > +	struct gpio_desc *init;
> > +	struct gpio_desc *done;
> > +	struct spi_device *spi;
> > +	u8 isc_enable_operand;
> > +	u8 isc_erase_operand;
> > +};
> 
> As Johannes Zink working on adding I2C to MachXO2, I am thinking how
> about we add to struct sysconfig_priv a callback
> 
> int (*sysconfig_transfer)(struct sysconfig_priv,
> 			  const void *tx_buf, size_t tx_len,
> 			  void *rx_buf, size_t rx_len)
> 
> For SPI it would be defined like this:
> 
> static int sysconfig_spi_transfer(struct sysconfig_priv *data,
> 				  const void *tx_buf, size_t tx_len,
> 				  void *rx_buf, size_t rx_len)
> {
> 	if (!rx_buf)
> 		return spi_write(data->spi, tx_buf, tx_size);
> 	
> 	return spi_write_then_read(data->spi, tx_buf, tx_size, rx_buf, rx_size);
> }
> 
> And later in sysconfig_ commands we would call this callback.
> 
> Would it be a good enough starting base for Johannes to add I2C interface
> for MachXO2?
> 
> >
> > ... snip ...
> >
> > +static int sysconfig_isc_enable(struct sysconfig_priv *data)
> > +{
> > +	u8 isc_enable[] = SYSCONFIG_ISC_ENABLE;
> > +	u32 status;
> > +	int ret;
> > +
> > +	isc_enable[1] = data->isc_enable_operand;
> > +
> > +	ret = spi_write(data->spi, isc_enable, sizeof(isc_enable));
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = sysconfig_poll_status(data, &status);
> > +	if (ret || (status & SYSCONFIG_STATUS_FAIL))
> > +		return ret ? : -EFAULT;
> > +
> > +	return 0;
> > +}
> 
> Quirks for I2C enable and refresh commands are 3-bytes instead of
> 4-bytes for SPI can be added here. Just check which of data->spi or
> data->i2c is not NULL.
> 

Actually, here is a patch for your tinkering.

I guess, what you need to do for MachXO2 I2C support is:
1) implement sysconfig_transfer callback for I2C. Something like this:

static int sysconfig_i2c_transfer(struct sysconfig_priv *data,
				  const void *tx_buf, size_t tx_len,
				  void *rx_buf, size_t rx_len)
{
	struct i2c_client *i2c = data->i2c;
	struct i2c_msg msg[] = {
		{
			.addr = i2c->addr,
			.flags = 0,
			.buf = tx_buf,
			.len = tx_len,
		}, {
			.addr = i2c->addr,
			.flags = I2C_M_RD,
			.buf = rx_buf,
			.len = rx_len,
		}
	};

	if (!rx_buf)
		return i2c_master_send(i2c, tx_buf, tx_len);

	return i2c_transfer(i2c->adapter, msg, ARRAY_SIZE(msg));
}

2) add quirks for 3-byte transfer to sysconfig_refresh() and
   sysconfig_isc_enable()
3) add probe routine
4) add separate fpga_manager_ops->write() for I2C
   state(), write_init() and write_complete() can be reused



---
 drivers/fpga/lattice-sysconfig-spi.c | 46 ++++++++++++++++++++--------
 1 file changed, 33 insertions(+), 13 deletions(-)

diff --git a/drivers/fpga/lattice-sysconfig-spi.c b/drivers/fpga/lattice-sysconfig-spi.c
index 145b5b27b88d..c2caf613d2f0 100644
--- a/drivers/fpga/lattice-sysconfig-spi.c
+++ b/drivers/fpga/lattice-sysconfig-spi.c
@@ -41,8 +41,21 @@ struct sysconfig_priv {
 	struct spi_device *spi;
 	u8 isc_enable_operand;
 	u8 isc_erase_operand;
+	int (*sysconfig_transfer)(struct sysconfig_priv *data,
+				  const void *tx_buf, size_t tx_len,
+				  void *rx_buf, size_t rx_len);
 };
 
+static int sysconfig_spi_transfer(struct sysconfig_priv *data,
+				  const void *tx_buf, size_t tx_len,
+				  void *rx_buf, size_t rx_len)
+{
+	if (!rx_buf)
+		return spi_write(data->spi, tx_buf, tx_len);
+
+	return spi_write_then_read(data->spi, tx_buf, tx_len, rx_buf, rx_len);
+}
+
 static int sysconfig_poll_busy(struct sysconfig_priv *data)
 {
 	const u8 lsc_check_busy[] = SYSCONFIG_LSC_CHECK_BUSY;
@@ -50,9 +63,9 @@ static int sysconfig_poll_busy(struct sysconfig_priv *data)
 	u8 busy;
 
 	while (retries--) {
-		ret = spi_write_then_read(data->spi,
-					  lsc_check_busy, sizeof(lsc_check_busy),
-					  &busy, sizeof(busy));
+		ret = data->sysconfig_transfer(data, lsc_check_busy,
+					       sizeof(lsc_check_busy),
+					       &busy, sizeof(busy));
 		if (ret)
 			return ret;
 
@@ -71,9 +84,9 @@ static int sysconfig_read_status(struct sysconfig_priv *data, u32 *status)
 	__be32 device_status;
 	int ret;
 
-	ret = spi_write_then_read(data->spi,
-				  lsc_read_status, sizeof(lsc_read_status),
-				  &device_status, sizeof(device_status));
+	ret = data->sysconfig_transfer(data, lsc_read_status,
+				       sizeof(lsc_read_status),
+				       &device_status, sizeof(device_status));
 	if (ret)
 		return ret;
 
@@ -116,7 +129,8 @@ static int sysconfig_refresh(struct sysconfig_priv *data)
 	static const u8 lsc_refresh[] = SYSCONFIG_LSC_REFRESH;
 	int ret;
 
-	ret = spi_write(data->spi, lsc_refresh, sizeof(lsc_refresh));
+	ret = data->sysconfig_transfer(data, lsc_refresh, sizeof(lsc_refresh),
+				       NULL, 0);
 	if (ret)
 		return ret;
 
@@ -133,7 +147,8 @@ static int sysconfig_isc_enable(struct sysconfig_priv *data)
 
 	isc_enable[1] = data->isc_enable_operand;
 
-	ret = spi_write(data->spi, isc_enable, sizeof(isc_enable));
+	ret = data->sysconfig_transfer(data, isc_enable, sizeof(isc_enable),
+				       NULL, 0);
 	if (ret)
 		return ret;
 
@@ -152,7 +167,8 @@ static int sysconfig_isc_erase(struct sysconfig_priv *data)
 
 	isc_erase[1] = data->isc_erase_operand;
 
-	ret = spi_write(data->spi, isc_erase, sizeof(isc_erase));
+	ret = data->sysconfig_transfer(data, isc_erase, sizeof(isc_erase),
+				       NULL, 0);
 	if (ret)
 		return ret;
 
@@ -178,7 +194,8 @@ static int sysconfig_lsc_init_addr(struct sysconfig_priv *data)
 {
 	const u8 lsc_init_addr[] = SYSCONFIG_LSC_INIT_ADDR;
 
-	return spi_write(data->spi, lsc_init_addr, sizeof(lsc_init_addr));
+	return data->sysconfig_transfer(data, lsc_init_addr,
+					sizeof(lsc_init_addr), NULL, 0);
 }
 
 static int sysconfig_lsc_bitstream_burst(struct sysconfig_priv *data)
@@ -202,7 +219,8 @@ static int sysconfig_isc_prog_done(struct sysconfig_priv *data)
 	u32 status;
 	int ret;
 
-	ret = spi_write(data->spi, isc_prog_done, sizeof(isc_prog_done));
+	ret = data->sysconfig_transfer(data, isc_prog_done,
+				       sizeof(isc_prog_done), NULL, 0);
 	if (ret)
 		return ret;
 
@@ -220,7 +238,8 @@ static int sysconfig_isc_disable(struct sysconfig_priv *data)
 {
 	const u8 isc_disable[] = SYSCONFIG_ISC_DISABLE;
 
-	return spi_write(data->spi, isc_disable, sizeof(isc_disable));
+	return data->sysconfig_transfer(data, isc_disable, sizeof(isc_disable),
+					NULL, 0);
 }
 
 static enum fpga_mgr_states ecp5_ops_state(struct fpga_manager *mgr)
@@ -336,7 +355,7 @@ static int ecp5_ops_write_complete(struct fpga_manager *mgr,
 	spi_bus_unlock(spi->controller);
 
 	/* Toggle CS and wait for bitstream write to finish */
-	ret = spi_write(spi, NULL, 0);
+	ret = priv->sysconfig_transfer(priv, NULL, 0, NULL, 0);
 	if (!ret)
 		ret = sysconfig_poll_busy(priv);
 
@@ -575,6 +594,7 @@ static int sysconfig_probe(struct spi_device *spi)
 		return -ENOMEM;
 
 	priv->spi = spi;
+	priv->sysconfig_transfer = sysconfig_spi_transfer;
 
 	probe_func = of_device_get_match_data(&spi->dev);
 	if (!probe_func) {
-- 
2.37.2

Re: [PATCH v8 1/2] fpga: lattice-sysconfig-spi: add Lattice sysCONFIG FPGA manager

[Xu Yilun]

On 2022-08-25 at 14:24:32 +0300, Ivan Bornyakov wrote:
> Add support to the FPGA manager for programming Lattice ECP5 and MachXO2
> FPGAs over slave SPI sysCONFIG interface.
> 
> Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
> ---
>  drivers/fpga/Kconfig                 |   7 +
>  drivers/fpga/Makefile                |   1 +
>  drivers/fpga/lattice-sysconfig-spi.c | 630 +++++++++++++++++++++++++++
>  3 files changed, 638 insertions(+)
>  create mode 100644 drivers/fpga/lattice-sysconfig-spi.c
> 
> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> index 6c416955da53..991d9d976dca 100644
> --- a/drivers/fpga/Kconfig
> +++ b/drivers/fpga/Kconfig
> @@ -263,4 +263,11 @@ config FPGA_MGR_MICROCHIP_SPI
>  	  programming over slave SPI interface with .dat formatted
>  	  bitstream image.
>  
> +config FPGA_MGR_LATTICE_SPI
> +	tristate "Lattice sysCONFIG SPI FPGA manager"
> +	depends on SPI
> +	help
> +	  FPGA manager driver support for Lattice FPGAs programming over slave
> +	  SPI sysCONFIG interface.
> +
>  endif # FPGA
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index 42ae8b58abce..115dba916024 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -20,6 +20,7 @@ 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
>  obj-$(CONFIG_FPGA_MGR_MICROCHIP_SPI)	+= microchip-spi.o
> +obj-$(CONFIG_FPGA_MGR_LATTICE_SPI)	+= lattice-sysconfig-spi.o
>  obj-$(CONFIG_ALTERA_PR_IP_CORE)		+= altera-pr-ip-core.o
>  obj-$(CONFIG_ALTERA_PR_IP_CORE_PLAT)	+= altera-pr-ip-core-plat.o
>  
> diff --git a/drivers/fpga/lattice-sysconfig-spi.c b/drivers/fpga/lattice-sysconfig-spi.c
> new file mode 100644
> index 000000000000..145b5b27b88d
> --- /dev/null
> +++ b/drivers/fpga/lattice-sysconfig-spi.c
> @@ -0,0 +1,630 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Lattice FPGA programming over slave SPI sysCONFIG interface.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/fpga/fpga-mgr.h>
> +#include <linux/of_device.h>
> +#include <linux/spi/spi.h>
> +
> +#define	SYSCONFIG_ISC_ENABLE		{0xC6, 0x00, 0x00, 0x00}
> +#define	SYSCONFIG_ISC_DISABLE		{0x26, 0x00, 0x00, 0x00}
> +#define	SYSCONFIG_ISC_ERASE		{0x0E, 0x00, 0x00, 0x00}
> +#define	SYSCONFIG_ISC_PROGRAM_DONE	{0x5E, 0x00, 0x00, 0x00}
> +#define	SYSCONFIG_LSC_READ_STATUS	{0x3C, 0x00, 0x00, 0x00}
> +#define	SYSCONFIG_LSC_CHECK_BUSY	{0xF0, 0x00, 0x00, 0x00}
> +#define	SYSCONFIG_LSC_REFRESH		{0x79, 0x00, 0x00, 0x00}
> +#define	SYSCONFIG_LSC_INIT_ADDR		{0x46, 0x00, 0x00, 0x00}
> +#define	SYSCONFIG_LSC_BITSTREAM_BURST	{0x7a, 0x00, 0x00, 0x00}
> +#define	SYSCONFIG_LSC_PROG_INCR_NV	{0x70, 0x00, 0x00, 0x01}
> +
> +#define	SYSCONFIG_STATUS_DONE		BIT(8)
> +#define	SYSCONFIG_STATUS_BUSY		BIT(12)
> +#define	SYSCONFIG_STATUS_FAIL		BIT(13)
> +#define	SYSCONFIG_STATUS_ERR		(BIT(23) | BIT(24) | BIT(25))
> +
> +#define	SYSCONFIG_POLL_RETRIES		100000
> +
> +#define	ECP5_SPI_MAX_SPEED_HZ		60000000
> +#define	ECP5_ISC_ERASE_OPERAND		0x01
> +
> +#define	MACHXO2_SPI_MAX_SPEED_HZ	66000000
> +#define	MACHXO2_PAGE_SIZE		16
> +#define	MACHXO2_ISC_ENABLE_OPERAND	0x08
> +#define	MACHXO2_ISC_ERASE_OPERAND	0x04

You need to deliver the meaning of each operand, rather than just point
out this is for machxo2, that is for esp5. I assume the sysCONFIG will
not deliberately design different commands with the exact same
functionality for different boards.

We should have a set of common configurations with finer granularity that
board specific fpga manager device could select from, rather than create
more and more similar macros for each new board.

Back to the 2 operands, what's the meaning of 0x1 & 0x4 for erase, also
0x0 & 0x8 for enable?

> +
> +struct sysconfig_priv {
> +	struct gpio_desc *program;
> +	struct gpio_desc *init;
> +	struct gpio_desc *done;
> +	struct spi_device *spi;
> +	u8 isc_enable_operand;
> +	u8 isc_erase_operand;
> +};
> +
> +static int sysconfig_poll_busy(struct sysconfig_priv *data)
> +{
> +	const u8 lsc_check_busy[] = SYSCONFIG_LSC_CHECK_BUSY;
> +	int ret, retries = SYSCONFIG_POLL_RETRIES;
> +	u8 busy;
> +
> +	while (retries--) {
> +		ret = spi_write_then_read(data->spi,
> +					  lsc_check_busy, sizeof(lsc_check_busy),
> +					  &busy, sizeof(busy));
> +		if (ret)
> +			return ret;
> +
> +		if (!busy)
> +			return 0;
> +
> +		usleep_range(50, 100);
> +	}
> +
> +	return -EBUSY;
> +}
> +
> +static int sysconfig_read_status(struct sysconfig_priv *data, u32 *status)
> +{
> +	const u8 lsc_read_status[] = SYSCONFIG_LSC_READ_STATUS;
> +	__be32 device_status;
> +	int ret;
> +
> +	ret = spi_write_then_read(data->spi,
> +				  lsc_read_status, sizeof(lsc_read_status),
> +				  &device_status, sizeof(device_status));
> +	if (ret)
> +		return ret;
> +
> +	*status = be32_to_cpu(device_status);
> +
> +	return 0;
> +}
> +
> +static int sysconfig_poll_status(struct sysconfig_priv *data, u32 *status)
> +{
> +	int ret;
> +
> +	ret = sysconfig_poll_busy(data);
> +	if (ret)
> +		return ret;
> +
> +	return sysconfig_read_status(data, status);
> +}
> +
> +static int sysconfig_poll_gpio(struct gpio_desc *gpio, bool is_active)
> +{
> +	int value, retries = SYSCONFIG_POLL_RETRIES;
> +
> +	while (retries--) {
> +		value = gpiod_get_value(gpio);
> +		if (value < 0)
> +			return value;
> +
> +		if ((!is_active && !value) || (is_active && value))
> +			return 0;
> +
> +		ndelay(10);
> +	}
> +
> +	return -ETIMEDOUT;
> +}
> +
> +static int sysconfig_refresh(struct sysconfig_priv *data)
> +{
> +	static const u8 lsc_refresh[] = SYSCONFIG_LSC_REFRESH;
> +	int ret;
> +
> +	ret = spi_write(data->spi, lsc_refresh, sizeof(lsc_refresh));
> +	if (ret)
> +		return ret;
> +
> +	usleep_range(4000, 8000);
> +
> +	return 0;
> +}
> +
> +static int sysconfig_isc_enable(struct sysconfig_priv *data)
> +{
> +	u8 isc_enable[] = SYSCONFIG_ISC_ENABLE;
> +	u32 status;
> +	int ret;
> +
> +	isc_enable[1] = data->isc_enable_operand;
> +
> +	ret = spi_write(data->spi, isc_enable, sizeof(isc_enable));
> +	if (ret)
> +		return ret;
> +
> +	ret = sysconfig_poll_status(data, &status);
> +	if (ret || (status & SYSCONFIG_STATUS_FAIL))
> +		return ret ? : -EFAULT;
> +
> +	return 0;
> +}
> +
> +static int sysconfig_isc_erase(struct sysconfig_priv *data)
> +{
> +	u8 isc_erase[] = SYSCONFIG_ISC_ERASE;
> +	u32 status;
> +	int ret;
> +
> +	isc_erase[1] = data->isc_erase_operand;
> +
> +	ret = spi_write(data->spi, isc_erase, sizeof(isc_erase));
> +	if (ret)
> +		return ret;
> +
> +	ret = sysconfig_poll_status(data, &status);
> +	if (ret || (status & SYSCONFIG_STATUS_FAIL))
> +		return ret ? : -EFAULT;
> +
> +	return 0;
> +}
> +
> +static int sysconfig_isc_init(struct sysconfig_priv *data)
> +{
> +	int ret;
> +
> +	ret = sysconfig_isc_enable(data);
> +	if (ret)
> +		return ret;
> +
> +	return sysconfig_isc_erase(data);
> +}
> +
> +static int sysconfig_lsc_init_addr(struct sysconfig_priv *data)
> +{
> +	const u8 lsc_init_addr[] = SYSCONFIG_LSC_INIT_ADDR;
> +
> +	return spi_write(data->spi, lsc_init_addr, sizeof(lsc_init_addr));
> +}
> +
> +static int sysconfig_lsc_bitstream_burst(struct sysconfig_priv *data)
> +{
> +	const u8 lsc_bitstream_burst[] = SYSCONFIG_LSC_BITSTREAM_BURST;
> +	struct spi_transfer xfer = {
> +		.tx_buf = lsc_bitstream_burst,
> +		.len = sizeof(lsc_bitstream_burst),
> +		.cs_change = 1,
> +	};
> +	struct spi_message msg;
> +
> +	spi_message_init_with_transfers(&msg, &xfer, 1);
> +
> +	return spi_sync_locked(data->spi, &msg);
> +}
> +
> +static int sysconfig_isc_prog_done(struct sysconfig_priv *data)
> +{
> +	const u8 isc_prog_done[] = SYSCONFIG_ISC_PROGRAM_DONE;
> +	u32 status;
> +	int ret;
> +
> +	ret = spi_write(data->spi, isc_prog_done, sizeof(isc_prog_done));
> +	if (ret)
> +		return ret;
> +
> +	ret = sysconfig_poll_status(data, &status);
> +	if (ret)
> +		return ret;
> +
> +	if (status & SYSCONFIG_STATUS_DONE)
> +		return 0;
> +
> +	return -EFAULT;
> +}
> +
> +static int sysconfig_isc_disable(struct sysconfig_priv *data)
> +{
> +	const u8 isc_disable[] = SYSCONFIG_ISC_DISABLE;
> +
> +	return spi_write(data->spi, isc_disable, sizeof(isc_disable));
> +}
> +
> +static enum fpga_mgr_states ecp5_ops_state(struct fpga_manager *mgr)
> +{
> +	struct sysconfig_priv *priv = mgr->priv;
> +
> +	return (gpiod_get_value(priv->done) > 0) ? FPGA_MGR_STATE_OPERATING :
> +						   FPGA_MGR_STATE_UNKNOWN;
> +}
> +
> +static int ecp5_ops_write_init(struct fpga_manager *mgr,
> +			       struct fpga_image_info *info,
> +			       const char *buf, size_t count)
> +{
> +	struct sysconfig_priv *priv = mgr->priv;
> +	struct spi_device *spi = priv->spi;
> +	struct device *dev = &mgr->dev;
> +	struct gpio_desc *program;
> +	struct gpio_desc *init;
> +	struct gpio_desc *done;
> +	int ret;
> +
> +	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
> +		dev_err(dev, "Partial reconfiguration is not supported\n");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	program = priv->program;
> +	init = priv->init;
> +	done = priv->done;
> +
> +	/* Enter init mode */
> +	gpiod_set_value(program, 1);

Same concern, provide gpio or command options for all board specific
fpga manager to select. 

I'll not list each features one by one below.

some more comments below.

> +
> +	ret = sysconfig_poll_gpio(init, true);
> +	if (!ret)
> +		ret = sysconfig_poll_gpio(done, false);
> +
> +	if (ret) {
> +		dev_err(dev, "Failed to go to init mode\n");
> +		return ret;
> +	}
> +
> +	/* Enter program mode */
> +	gpiod_set_value(program, 0);
> +
> +	ret = sysconfig_poll_gpio(init, false);
> +	if (ret) {
> +		dev_err(dev, "Failed to go to program mode\n");
> +		return ret;
> +	}
> +
> +	/* Enter ISC mode */
> +	ret = sysconfig_isc_init(priv);
> +	if (ret) {
> +		dev_err(dev, "Failed to go to ISC mode\n");
> +		return ret;
> +	}
> +
> +	/* Initialize the Address Shift Register */
> +	ret = sysconfig_lsc_init_addr(priv);
> +	if (ret) {
> +		dev_err(dev,
> +			"Failed to initialize the Address Shift Register\n");
> +		return ret;
> +	}
> +
> +	/*
> +	 * Lock SPI bus for exclusive usage until FPGA programming is done.
> +	 * SPI bus will be released in ecp5_ops_write_complete() or on error.
> +	 */
> +	spi_bus_lock(spi->controller);
> +
> +	/* Prepare for bitstream burst write */
> +	ret = sysconfig_lsc_bitstream_burst(priv);
> +	if (ret) {
> +		dev_err(dev, "Failed to prepare for bitstream burst write\n");
> +		spi_bus_unlock(spi->controller);
> +	}
> +
> +	return ret;
> +}
> +
> +static int ecp5_ops_write(struct fpga_manager *mgr, const char *buf, size_t count)
> +{
> +	struct sysconfig_priv *priv = mgr->priv;
> +	struct spi_device *spi = priv->spi;
> +	struct spi_transfer xfer = {
> +		.tx_buf = buf,
> +		.len = count,
> +		.cs_change = 1,
> +	};
> +	struct spi_message msg;
> +	int ret;
> +
> +	spi_message_init_with_transfers(&msg, &xfer, 1);
> +	ret = spi_sync_locked(spi, &msg);
> +	if (ret)
> +		spi_bus_unlock(spi->controller);
> +
> +	return ret;
> +}
> +
> +static int ecp5_ops_write_complete(struct fpga_manager *mgr,
> +				   struct fpga_image_info *info)
> +{
> +	struct sysconfig_priv *priv = mgr->priv;
> +	struct spi_device *spi = priv->spi;
> +	struct device *dev = &mgr->dev;
> +	int ret;
> +
> +	/* Bitstream burst write is done, release SPI bus */
> +	spi_bus_unlock(spi->controller);
> +
> +	/* Toggle CS and wait for bitstream write to finish */
> +	ret = spi_write(spi, NULL, 0);
> +	if (!ret)
> +		ret = sysconfig_poll_busy(priv);
> +
> +	if (ret) {
> +		dev_err(dev, "Error while waiting bitstream write to finish\n");
> +		return ret;
> +	}
> +
> +	/* Exit ISC mode */
> +	ret = sysconfig_isc_disable(priv);
> +	if (!ret)
> +		ret = sysconfig_poll_gpio(priv->done, true);
> +
> +	if (ret)
> +		dev_err(dev, "Failed to finish ISC\n");
> +
> +	return ret;
> +}
> +
> +static const struct fpga_manager_ops ecp5_fpga_ops = {
> +	.state = ecp5_ops_state,
> +	.write_init = ecp5_ops_write_init,
> +	.write = ecp5_ops_write,
> +	.write_complete = ecp5_ops_write_complete,
> +};
> +
> +static int ecp5_probe(struct sysconfig_priv *priv)
> +{
> +	struct spi_device *spi = priv->spi;
> +	struct device *dev = &spi->dev;
> +	struct fpga_manager *mgr;
> +	int ret;
> +
> +	if (spi->max_speed_hz > ECP5_SPI_MAX_SPEED_HZ) {
> +		dev_err(dev, "SPI speed %u is too high, maximum speed is %u\n",
> +			spi->max_speed_hz, ECP5_SPI_MAX_SPEED_HZ);
> +		return -EINVAL;
> +	}
> +
> +	priv->isc_erase_operand = ECP5_ISC_ERASE_OPERAND;
> +
> +	priv->done = devm_gpiod_get(dev, "done", GPIOD_IN);
> +	if (IS_ERR(priv->done)) {
> +		ret = PTR_ERR(priv->done);
> +		dev_err(dev, "Failed to get DONE GPIO: %d\n", ret);
> +		return ret;
> +	}
> +
> +	priv->init = devm_gpiod_get(dev, "init", GPIOD_IN);
> +	if (IS_ERR(priv->init)) {
> +		ret = PTR_ERR(priv->init);
> +		dev_err(dev, "Failed to get INIT GPIO: %d\n", ret);
> +		return ret;
> +	}
> +
> +	priv->program = devm_gpiod_get(dev, "program", GPIOD_OUT_LOW);
> +	if (IS_ERR(priv->program)) {
> +		ret = PTR_ERR(priv->program);
> +		dev_err(dev, "Failed to get PROGRAM GPIO: %d\n", ret);
> +		return ret;
> +	}
> +
> +	mgr = devm_fpga_mgr_register(dev, "Lattice ECP5 SPI FPGA Manager",
> +				     &ecp5_fpga_ops, priv);
> +
> +	return PTR_ERR_OR_ZERO(mgr);
> +}
> +
> +static enum fpga_mgr_states machxo2_ops_state(struct fpga_manager *mgr)
> +{
> +	struct sysconfig_priv *priv = mgr->priv;
> +	u32 status;
> +	int ret;
> +
> +	ret = sysconfig_read_status(priv, &status);
> +	if (ret || !(status & SYSCONFIG_STATUS_DONE))
> +		return FPGA_MGR_STATE_UNKNOWN;
> +
> +	return FPGA_MGR_STATE_OPERATING;
> +}
> +
> +static int machxo2_ops_write_init(struct fpga_manager *mgr,
> +				  struct fpga_image_info *info,
> +				  const char *buf, size_t count)
> +{
> +	struct sysconfig_priv *priv = mgr->priv;
> +	struct device *dev = &mgr->dev;
> +	int ret;
> +
> +	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
> +		dev_err(dev, "Partial reconfiguration is not supported\n");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	/* Enter ISC mode */
> +	ret = sysconfig_isc_init(priv);
> +	if (ret) {
> +		dev_err(dev, "Failed to go to ISC mode\n");
> +		return ret;
> +	}
> +
> +	/* Initialize the Address Shift Register */
> +	ret = sysconfig_lsc_init_addr(priv);
> +	if (ret)
> +		dev_err(dev,
> +			"Failed to initialize the Address Shift Register\n");
> +
> +	return ret;
> +}
> +
> +static int machxo2_ops_write(struct fpga_manager *mgr, const char *buf, size_t count)
> +{
> +	const u8 lsc_progincr[] = SYSCONFIG_LSC_PROG_INCR_NV;
> +	struct sysconfig_priv *priv = mgr->priv;
> +	struct device *dev = &mgr->dev;
> +	struct spi_transfer xfers[2] = {
> +		{
> +			.tx_buf = lsc_progincr,
> +			.len = sizeof(lsc_progincr),
> +		}, {
> +			.len = MACHXO2_PAGE_SIZE,
> +		},
> +	};
> +	size_t i;
> +	int ret;
> +
> +	if (count % MACHXO2_PAGE_SIZE) {
> +		dev_err(dev, "Malformed payload.\n");
> +		return -EINVAL;
> +	}
> +
> +	for (i = 0; i < count; i += MACHXO2_PAGE_SIZE) {
> +		xfers[1].tx_buf = buf + i;
> +
> +		ret = spi_sync_transfer(priv->spi, xfers, 2);
> +		if (!ret)
> +			ret = sysconfig_poll_busy(priv);
> +
> +		if (ret) {
> +			dev_err(dev, "Failed to write frame %zu of %zu\n",
> +				i / MACHXO2_PAGE_SIZE, count / MACHXO2_PAGE_SIZE);
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static void machxo2_cleanup(struct sysconfig_priv *data)
> +{
> +	sysconfig_isc_erase(data);
> +	sysconfig_refresh(data);
> +}
> +
> +static int machxo2_ops_write_complete(struct fpga_manager *mgr,
> +				      struct fpga_image_info *info)
> +{
> +	int ret, retries = SYSCONFIG_POLL_RETRIES;
> +	struct sysconfig_priv *priv = mgr->priv;
> +	struct device *dev = &mgr->dev;
> +	u32 status;
> +
> +	ret = sysconfig_isc_prog_done(priv);
> +	if (ret) {
> +		dev_err(dev, "Failed to enable Self-Download Mode\n");
> +		goto fail;
> +	}
> +
> +	ret = sysconfig_isc_disable(priv);
> +	if (ret) {
> +		dev_err(dev, "Failed to disable Configuration Interface\n");
> +		goto fail;
> +	}
> +
> +	while (retries--) {
> +		ret = sysconfig_refresh(priv);
> +		if (!ret)
> +			ret = sysconfig_read_status(priv, &status);
> +
> +		if (ret) {
> +			dev_err(dev, "Failed to refresh\n");
> +			break;
> +		}
> +
> +		if (status & SYSCONFIG_STATUS_DONE &&
> +		    !(status & SYSCONFIG_STATUS_BUSY) &&
> +		    !(status & SYSCONFIG_STATUS_ERR))
> +			return 0;
> +	}
> +
> +fail:
> +	machxo2_cleanup(priv);
> +
> +	return -EFAULT;
> +}
> +
> +static const struct fpga_manager_ops machxo2_fpga_ops = {
> +	.state = machxo2_ops_state,
> +	.write_init = machxo2_ops_write_init,
> +	.write = machxo2_ops_write,
> +	.write_complete = machxo2_ops_write_complete,
> +};
> +
> +static int machxo2_probe(struct sysconfig_priv *priv)
> +{
> +	struct spi_device *spi = priv->spi;
> +	struct device *dev = &spi->dev;
> +	struct fpga_manager *mgr;
> +
> +	if (spi->max_speed_hz > MACHXO2_SPI_MAX_SPEED_HZ) {
> +		dev_err(dev, "SPI speed %u is too high, maximum speed is %u\n",
> +			spi->max_speed_hz, MACHXO2_SPI_MAX_SPEED_HZ);
> +		return -EINVAL;
> +	}
> +
> +	priv->isc_enable_operand = MACHXO2_ISC_ENABLE_OPERAND;
> +	priv->isc_erase_operand = MACHXO2_ISC_ERASE_OPERAND;
> +
> +	mgr = devm_fpga_mgr_register(dev, "Lattice MachXO2 SPI FPGA Manager",
> +				     &machxo2_fpga_ops, priv);
> +
> +	return PTR_ERR_OR_ZERO(mgr);
> +}
> +
> +typedef int (*lattice_fpga_probe_func)(struct sysconfig_priv *);
> +
> +static int sysconfig_probe(struct spi_device *spi)
> +{
> +	const struct spi_device_id *dev_id;
> +	lattice_fpga_probe_func probe_func;
> +	struct device *dev = &spi->dev;
> +	struct sysconfig_priv *priv;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->spi = spi;
> +
> +	probe_func = of_device_get_match_data(&spi->dev);
> +	if (!probe_func) {
> +		dev_id = spi_get_device_id(spi);
> +		if (!dev_id)
> +			return -ENODEV;
> +
> +		probe_func = (lattice_fpga_probe_func)dev_id->driver_data;
> +	}
> +
> +	if (!probe_func)
> +		return -EINVAL;
> +
> +	return probe_func(priv);
> +}
> +
> +static const struct spi_device_id sysconfig_spi_ids[] = {
> +	{
> +		.name = "ecp5-fpga-mgr",
> +		.driver_data = (kernel_ulong_t)ecp5_probe,
> +	}, {
> +		.name = "machxo2-fpga-mgr",
> +		.driver_data = (kernel_ulong_t)machxo2_probe,

Putting the whole probe flow in driver_data is the same as providing 2
drivers. The purpose is not to put all the code in one file.

Thanks,
Yilun

> +	}, {},
> +};
> +MODULE_DEVICE_TABLE(spi, sysconfig_spi_ids);
> +
> +#if IS_ENABLED(CONFIG_OF)
> +static const struct of_device_id sysconfig_of_ids[] = {
> +	{
> +		.compatible = "lattice,ecp5-fpga-mgr",
> +		.data = ecp5_probe,
> +	}, {
> +		.compatible = "lattice,machxo2-fpga-mgr",
> +		.data = machxo2_probe
> +	}, {},
> +};
> +MODULE_DEVICE_TABLE(of, sysconfig_of_ids);
> +#endif /* IS_ENABLED(CONFIG_OF) */
> +
> +static struct spi_driver lattice_sysconfig_driver = {
> +	.probe = sysconfig_probe,
> +	.id_table = sysconfig_spi_ids,
> +	.driver = {
> +		.name = "lattice_sysconfig_spi_fpga_mgr",
> +		.of_match_table = of_match_ptr(sysconfig_of_ids),
> +	},
> +};
> +
> +module_spi_driver(lattice_sysconfig_driver);
> +
> +MODULE_DESCRIPTION("Lattice sysCONFIG Slave SPI FPGA Manager");
> +MODULE_LICENSE("GPL");
> -- 
> 2.37.2
> 
> 

Re: [PATCH v8 1/2] fpga: lattice-sysconfig-spi: add Lattice sysCONFIG FPGA manager

[Ivan Bornyakov]

On Mon, Aug 29, 2022 at 03:25:41PM +0800, Xu Yilun wrote:
> On 2022-08-25 at 14:24:32 +0300, Ivan Bornyakov wrote:
> > Add support to the FPGA manager for programming Lattice ECP5 and MachXO2
> > FPGAs over slave SPI sysCONFIG interface.
> > 
> > Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
> > ---
> >  drivers/fpga/Kconfig                 |   7 +
> >  drivers/fpga/Makefile                |   1 +
> >  drivers/fpga/lattice-sysconfig-spi.c | 630 +++++++++++++++++++++++++++
> >  3 files changed, 638 insertions(+)
> >  create mode 100644 drivers/fpga/lattice-sysconfig-spi.c
> > 
> > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> > index 6c416955da53..991d9d976dca 100644
> > --- a/drivers/fpga/Kconfig
> > +++ b/drivers/fpga/Kconfig
> > @@ -263,4 +263,11 @@ config FPGA_MGR_MICROCHIP_SPI
> >  	  programming over slave SPI interface with .dat formatted
> >  	  bitstream image.
> >  
> > +config FPGA_MGR_LATTICE_SPI
> > +	tristate "Lattice sysCONFIG SPI FPGA manager"
> > +	depends on SPI
> > +	help
> > +	  FPGA manager driver support for Lattice FPGAs programming over slave
> > +	  SPI sysCONFIG interface.
> > +
> >  endif # FPGA
> > diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> > index 42ae8b58abce..115dba916024 100644
> > --- a/drivers/fpga/Makefile
> > +++ b/drivers/fpga/Makefile
> > @@ -20,6 +20,7 @@ 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
> >  obj-$(CONFIG_FPGA_MGR_MICROCHIP_SPI)	+= microchip-spi.o
> > +obj-$(CONFIG_FPGA_MGR_LATTICE_SPI)	+= lattice-sysconfig-spi.o
> >  obj-$(CONFIG_ALTERA_PR_IP_CORE)		+= altera-pr-ip-core.o
> >  obj-$(CONFIG_ALTERA_PR_IP_CORE_PLAT)	+= altera-pr-ip-core-plat.o
> >  
> > diff --git a/drivers/fpga/lattice-sysconfig-spi.c b/drivers/fpga/lattice-sysconfig-spi.c
> > new file mode 100644
> > index 000000000000..145b5b27b88d
> > --- /dev/null
> > +++ b/drivers/fpga/lattice-sysconfig-spi.c
> > @@ -0,0 +1,630 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Lattice FPGA programming over slave SPI sysCONFIG interface.
> > + */
> > +
> > +#include <linux/delay.h>
> > +#include <linux/fpga/fpga-mgr.h>
> > +#include <linux/of_device.h>
> > +#include <linux/spi/spi.h>
> > +
> > +#define	SYSCONFIG_ISC_ENABLE		{0xC6, 0x00, 0x00, 0x00}
> > +#define	SYSCONFIG_ISC_DISABLE		{0x26, 0x00, 0x00, 0x00}
> > +#define	SYSCONFIG_ISC_ERASE		{0x0E, 0x00, 0x00, 0x00}
> > +#define	SYSCONFIG_ISC_PROGRAM_DONE	{0x5E, 0x00, 0x00, 0x00}
> > +#define	SYSCONFIG_LSC_READ_STATUS	{0x3C, 0x00, 0x00, 0x00}
> > +#define	SYSCONFIG_LSC_CHECK_BUSY	{0xF0, 0x00, 0x00, 0x00}
> > +#define	SYSCONFIG_LSC_REFRESH		{0x79, 0x00, 0x00, 0x00}
> > +#define	SYSCONFIG_LSC_INIT_ADDR		{0x46, 0x00, 0x00, 0x00}
> > +#define	SYSCONFIG_LSC_BITSTREAM_BURST	{0x7a, 0x00, 0x00, 0x00}
> > +#define	SYSCONFIG_LSC_PROG_INCR_NV	{0x70, 0x00, 0x00, 0x01}
> > +
> > +#define	SYSCONFIG_STATUS_DONE		BIT(8)
> > +#define	SYSCONFIG_STATUS_BUSY		BIT(12)
> > +#define	SYSCONFIG_STATUS_FAIL		BIT(13)
> > +#define	SYSCONFIG_STATUS_ERR		(BIT(23) | BIT(24) | BIT(25))
> > +
> > +#define	SYSCONFIG_POLL_RETRIES		100000
> > +
> > +#define	ECP5_SPI_MAX_SPEED_HZ		60000000
> > +#define	ECP5_ISC_ERASE_OPERAND		0x01
> > +
> > +#define	MACHXO2_SPI_MAX_SPEED_HZ	66000000
> > +#define	MACHXO2_PAGE_SIZE		16
> > +#define	MACHXO2_ISC_ENABLE_OPERAND	0x08
> > +#define	MACHXO2_ISC_ERASE_OPERAND	0x04
> 
> You need to deliver the meaning of each operand, rather than just point
> out this is for machxo2, that is for esp5. I assume the sysCONFIG will
> not deliberately design different commands with the exact same
> functionality for different boards.
> 
> We should have a set of common configurations with finer granularity that
> board specific fpga manager device could select from, rather than create
> more and more similar macros for each new board.
> 
> Back to the 2 operands, what's the meaning of 0x1 & 0x4 for erase,

0x1 - erase SRAM, 0x4 - erase FLASH. I'll redefine them properly in v9

> also 0x0 & 0x8 for enable?
> 

Their meaning is not documented. It's just 0x08 for MachXO2 and 0x00 for
ECP5.

> > +
> > +struct sysconfig_priv {
> > +	struct gpio_desc *program;
> > +	struct gpio_desc *init;
> > +	struct gpio_desc *done;
> > +	struct spi_device *spi;
> > +	u8 isc_enable_operand;
> > +	u8 isc_erase_operand;
> > +};
> > +
> > +static int sysconfig_poll_busy(struct sysconfig_priv *data)
> > +{
> > +	const u8 lsc_check_busy[] = SYSCONFIG_LSC_CHECK_BUSY;
> > +	int ret, retries = SYSCONFIG_POLL_RETRIES;
> > +	u8 busy;
> > +
> > +	while (retries--) {
> > +		ret = spi_write_then_read(data->spi,
> > +					  lsc_check_busy, sizeof(lsc_check_busy),
> > +					  &busy, sizeof(busy));
> > +		if (ret)
> > +			return ret;
> > +
> > +		if (!busy)
> > +			return 0;
> > +
> > +		usleep_range(50, 100);
> > +	}
> > +
> > +	return -EBUSY;
> > +}
> > +
> > +static int sysconfig_read_status(struct sysconfig_priv *data, u32 *status)
> > +{
> > +	const u8 lsc_read_status[] = SYSCONFIG_LSC_READ_STATUS;
> > +	__be32 device_status;
> > +	int ret;
> > +
> > +	ret = spi_write_then_read(data->spi,
> > +				  lsc_read_status, sizeof(lsc_read_status),
> > +				  &device_status, sizeof(device_status));
> > +	if (ret)
> > +		return ret;
> > +
> > +	*status = be32_to_cpu(device_status);
> > +
> > +	return 0;
> > +}
> > +
> > +static int sysconfig_poll_status(struct sysconfig_priv *data, u32 *status)
> > +{
> > +	int ret;
> > +
> > +	ret = sysconfig_poll_busy(data);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return sysconfig_read_status(data, status);
> > +}
> > +
> > +static int sysconfig_poll_gpio(struct gpio_desc *gpio, bool is_active)
> > +{
> > +	int value, retries = SYSCONFIG_POLL_RETRIES;
> > +
> > +	while (retries--) {
> > +		value = gpiod_get_value(gpio);
> > +		if (value < 0)
> > +			return value;
> > +
> > +		if ((!is_active && !value) || (is_active && value))
> > +			return 0;
> > +
> > +		ndelay(10);
> > +	}
> > +
> > +	return -ETIMEDOUT;
> > +}
> > +
> > +static int sysconfig_refresh(struct sysconfig_priv *data)
> > +{
> > +	static const u8 lsc_refresh[] = SYSCONFIG_LSC_REFRESH;
> > +	int ret;
> > +
> > +	ret = spi_write(data->spi, lsc_refresh, sizeof(lsc_refresh));
> > +	if (ret)
> > +		return ret;
> > +
> > +	usleep_range(4000, 8000);
> > +
> > +	return 0;
> > +}
> > +
> > +static int sysconfig_isc_enable(struct sysconfig_priv *data)
> > +{
> > +	u8 isc_enable[] = SYSCONFIG_ISC_ENABLE;
> > +	u32 status;
> > +	int ret;
> > +
> > +	isc_enable[1] = data->isc_enable_operand;
> > +
> > +	ret = spi_write(data->spi, isc_enable, sizeof(isc_enable));
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = sysconfig_poll_status(data, &status);
> > +	if (ret || (status & SYSCONFIG_STATUS_FAIL))
> > +		return ret ? : -EFAULT;
> > +
> > +	return 0;
> > +}
> > +
> > +static int sysconfig_isc_erase(struct sysconfig_priv *data)
> > +{
> > +	u8 isc_erase[] = SYSCONFIG_ISC_ERASE;
> > +	u32 status;
> > +	int ret;
> > +
> > +	isc_erase[1] = data->isc_erase_operand;
> > +
> > +	ret = spi_write(data->spi, isc_erase, sizeof(isc_erase));
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = sysconfig_poll_status(data, &status);
> > +	if (ret || (status & SYSCONFIG_STATUS_FAIL))
> > +		return ret ? : -EFAULT;
> > +
> > +	return 0;
> > +}
> > +
> > +static int sysconfig_isc_init(struct sysconfig_priv *data)
> > +{
> > +	int ret;
> > +
> > +	ret = sysconfig_isc_enable(data);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return sysconfig_isc_erase(data);
> > +}
> > +
> > +static int sysconfig_lsc_init_addr(struct sysconfig_priv *data)
> > +{
> > +	const u8 lsc_init_addr[] = SYSCONFIG_LSC_INIT_ADDR;
> > +
> > +	return spi_write(data->spi, lsc_init_addr, sizeof(lsc_init_addr));
> > +}
> > +
> > +static int sysconfig_lsc_bitstream_burst(struct sysconfig_priv *data)
> > +{
> > +	const u8 lsc_bitstream_burst[] = SYSCONFIG_LSC_BITSTREAM_BURST;
> > +	struct spi_transfer xfer = {
> > +		.tx_buf = lsc_bitstream_burst,
> > +		.len = sizeof(lsc_bitstream_burst),
> > +		.cs_change = 1,
> > +	};
> > +	struct spi_message msg;
> > +
> > +	spi_message_init_with_transfers(&msg, &xfer, 1);
> > +
> > +	return spi_sync_locked(data->spi, &msg);
> > +}
> > +
> > +static int sysconfig_isc_prog_done(struct sysconfig_priv *data)
> > +{
> > +	const u8 isc_prog_done[] = SYSCONFIG_ISC_PROGRAM_DONE;
> > +	u32 status;
> > +	int ret;
> > +
> > +	ret = spi_write(data->spi, isc_prog_done, sizeof(isc_prog_done));
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = sysconfig_poll_status(data, &status);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (status & SYSCONFIG_STATUS_DONE)
> > +		return 0;
> > +
> > +	return -EFAULT;
> > +}
> > +
> > +static int sysconfig_isc_disable(struct sysconfig_priv *data)
> > +{
> > +	const u8 isc_disable[] = SYSCONFIG_ISC_DISABLE;
> > +
> > +	return spi_write(data->spi, isc_disable, sizeof(isc_disable));
> > +}
> > +
> > +static enum fpga_mgr_states ecp5_ops_state(struct fpga_manager *mgr)
> > +{
> > +	struct sysconfig_priv *priv = mgr->priv;
> > +
> > +	return (gpiod_get_value(priv->done) > 0) ? FPGA_MGR_STATE_OPERATING :
> > +						   FPGA_MGR_STATE_UNKNOWN;
> > +}
> > +
> > +static int ecp5_ops_write_init(struct fpga_manager *mgr,
> > +			       struct fpga_image_info *info,
> > +			       const char *buf, size_t count)
> > +{
> > +	struct sysconfig_priv *priv = mgr->priv;
> > +	struct spi_device *spi = priv->spi;
> > +	struct device *dev = &mgr->dev;
> > +	struct gpio_desc *program;
> > +	struct gpio_desc *init;
> > +	struct gpio_desc *done;
> > +	int ret;
> > +
> > +	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
> > +		dev_err(dev, "Partial reconfiguration is not supported\n");
> > +		return -EOPNOTSUPP;
> > +	}
> > +
> > +	program = priv->program;
> > +	init = priv->init;
> > +	done = priv->done;
> > +
> > +	/* Enter init mode */
> > +	gpiod_set_value(program, 1);
> 
> Same concern, provide gpio or command options for all board specific
> fpga manager to select. 
> 
> I'll not list each features one by one below.
> 
> some more comments below.
> 
> > +
> > +	ret = sysconfig_poll_gpio(init, true);
> > +	if (!ret)
> > +		ret = sysconfig_poll_gpio(done, false);
> > +
> > +	if (ret) {
> > +		dev_err(dev, "Failed to go to init mode\n");
> > +		return ret;
> > +	}
> > +
> > +	/* Enter program mode */
> > +	gpiod_set_value(program, 0);
> > +
> > +	ret = sysconfig_poll_gpio(init, false);
> > +	if (ret) {
> > +		dev_err(dev, "Failed to go to program mode\n");
> > +		return ret;
> > +	}
> > +
> > +	/* Enter ISC mode */
> > +	ret = sysconfig_isc_init(priv);
> > +	if (ret) {
> > +		dev_err(dev, "Failed to go to ISC mode\n");
> > +		return ret;
> > +	}
> > +
> > +	/* Initialize the Address Shift Register */
> > +	ret = sysconfig_lsc_init_addr(priv);
> > +	if (ret) {
> > +		dev_err(dev,
> > +			"Failed to initialize the Address Shift Register\n");
> > +		return ret;
> > +	}
> > +
> > +	/*
> > +	 * Lock SPI bus for exclusive usage until FPGA programming is done.
> > +	 * SPI bus will be released in ecp5_ops_write_complete() or on error.
> > +	 */
> > +	spi_bus_lock(spi->controller);
> > +
> > +	/* Prepare for bitstream burst write */
> > +	ret = sysconfig_lsc_bitstream_burst(priv);
> > +	if (ret) {
> > +		dev_err(dev, "Failed to prepare for bitstream burst write\n");
> > +		spi_bus_unlock(spi->controller);
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> > +static int ecp5_ops_write(struct fpga_manager *mgr, const char *buf, size_t count)
> > +{
> > +	struct sysconfig_priv *priv = mgr->priv;
> > +	struct spi_device *spi = priv->spi;
> > +	struct spi_transfer xfer = {
> > +		.tx_buf = buf,
> > +		.len = count,
> > +		.cs_change = 1,
> > +	};
> > +	struct spi_message msg;
> > +	int ret;
> > +
> > +	spi_message_init_with_transfers(&msg, &xfer, 1);
> > +	ret = spi_sync_locked(spi, &msg);
> > +	if (ret)
> > +		spi_bus_unlock(spi->controller);
> > +
> > +	return ret;
> > +}
> > +
> > +static int ecp5_ops_write_complete(struct fpga_manager *mgr,
> > +				   struct fpga_image_info *info)
> > +{
> > +	struct sysconfig_priv *priv = mgr->priv;
> > +	struct spi_device *spi = priv->spi;
> > +	struct device *dev = &mgr->dev;
> > +	int ret;
> > +
> > +	/* Bitstream burst write is done, release SPI bus */
> > +	spi_bus_unlock(spi->controller);
> > +
> > +	/* Toggle CS and wait for bitstream write to finish */
> > +	ret = spi_write(spi, NULL, 0);
> > +	if (!ret)
> > +		ret = sysconfig_poll_busy(priv);
> > +
> > +	if (ret) {
> > +		dev_err(dev, "Error while waiting bitstream write to finish\n");
> > +		return ret;
> > +	}
> > +
> > +	/* Exit ISC mode */
> > +	ret = sysconfig_isc_disable(priv);
> > +	if (!ret)
> > +		ret = sysconfig_poll_gpio(priv->done, true);
> > +
> > +	if (ret)
> > +		dev_err(dev, "Failed to finish ISC\n");
> > +
> > +	return ret;
> > +}
> > +
> > +static const struct fpga_manager_ops ecp5_fpga_ops = {
> > +	.state = ecp5_ops_state,
> > +	.write_init = ecp5_ops_write_init,
> > +	.write = ecp5_ops_write,
> > +	.write_complete = ecp5_ops_write_complete,
> > +};
> > +
> > +static int ecp5_probe(struct sysconfig_priv *priv)
> > +{
> > +	struct spi_device *spi = priv->spi;
> > +	struct device *dev = &spi->dev;
> > +	struct fpga_manager *mgr;
> > +	int ret;
> > +
> > +	if (spi->max_speed_hz > ECP5_SPI_MAX_SPEED_HZ) {
> > +		dev_err(dev, "SPI speed %u is too high, maximum speed is %u\n",
> > +			spi->max_speed_hz, ECP5_SPI_MAX_SPEED_HZ);
> > +		return -EINVAL;
> > +	}
> > +
> > +	priv->isc_erase_operand = ECP5_ISC_ERASE_OPERAND;
> > +
> > +	priv->done = devm_gpiod_get(dev, "done", GPIOD_IN);
> > +	if (IS_ERR(priv->done)) {
> > +		ret = PTR_ERR(priv->done);
> > +		dev_err(dev, "Failed to get DONE GPIO: %d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	priv->init = devm_gpiod_get(dev, "init", GPIOD_IN);
> > +	if (IS_ERR(priv->init)) {
> > +		ret = PTR_ERR(priv->init);
> > +		dev_err(dev, "Failed to get INIT GPIO: %d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	priv->program = devm_gpiod_get(dev, "program", GPIOD_OUT_LOW);
> > +	if (IS_ERR(priv->program)) {
> > +		ret = PTR_ERR(priv->program);
> > +		dev_err(dev, "Failed to get PROGRAM GPIO: %d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	mgr = devm_fpga_mgr_register(dev, "Lattice ECP5 SPI FPGA Manager",
> > +				     &ecp5_fpga_ops, priv);
> > +
> > +	return PTR_ERR_OR_ZERO(mgr);
> > +}
> > +
> > +static enum fpga_mgr_states machxo2_ops_state(struct fpga_manager *mgr)
> > +{
> > +	struct sysconfig_priv *priv = mgr->priv;
> > +	u32 status;
> > +	int ret;
> > +
> > +	ret = sysconfig_read_status(priv, &status);
> > +	if (ret || !(status & SYSCONFIG_STATUS_DONE))
> > +		return FPGA_MGR_STATE_UNKNOWN;
> > +
> > +	return FPGA_MGR_STATE_OPERATING;
> > +}
> > +
> > +static int machxo2_ops_write_init(struct fpga_manager *mgr,
> > +				  struct fpga_image_info *info,
> > +				  const char *buf, size_t count)
> > +{
> > +	struct sysconfig_priv *priv = mgr->priv;
> > +	struct device *dev = &mgr->dev;
> > +	int ret;
> > +
> > +	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
> > +		dev_err(dev, "Partial reconfiguration is not supported\n");
> > +		return -EOPNOTSUPP;
> > +	}
> > +
> > +	/* Enter ISC mode */
> > +	ret = sysconfig_isc_init(priv);
> > +	if (ret) {
> > +		dev_err(dev, "Failed to go to ISC mode\n");
> > +		return ret;
> > +	}
> > +
> > +	/* Initialize the Address Shift Register */
> > +	ret = sysconfig_lsc_init_addr(priv);
> > +	if (ret)
> > +		dev_err(dev,
> > +			"Failed to initialize the Address Shift Register\n");
> > +
> > +	return ret;
> > +}
> > +
> > +static int machxo2_ops_write(struct fpga_manager *mgr, const char *buf, size_t count)
> > +{
> > +	const u8 lsc_progincr[] = SYSCONFIG_LSC_PROG_INCR_NV;
> > +	struct sysconfig_priv *priv = mgr->priv;
> > +	struct device *dev = &mgr->dev;
> > +	struct spi_transfer xfers[2] = {
> > +		{
> > +			.tx_buf = lsc_progincr,
> > +			.len = sizeof(lsc_progincr),
> > +		}, {
> > +			.len = MACHXO2_PAGE_SIZE,
> > +		},
> > +	};
> > +	size_t i;
> > +	int ret;
> > +
> > +	if (count % MACHXO2_PAGE_SIZE) {
> > +		dev_err(dev, "Malformed payload.\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	for (i = 0; i < count; i += MACHXO2_PAGE_SIZE) {
> > +		xfers[1].tx_buf = buf + i;
> > +
> > +		ret = spi_sync_transfer(priv->spi, xfers, 2);
> > +		if (!ret)
> > +			ret = sysconfig_poll_busy(priv);
> > +
> > +		if (ret) {
> > +			dev_err(dev, "Failed to write frame %zu of %zu\n",
> > +				i / MACHXO2_PAGE_SIZE, count / MACHXO2_PAGE_SIZE);
> > +			return ret;
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static void machxo2_cleanup(struct sysconfig_priv *data)
> > +{
> > +	sysconfig_isc_erase(data);
> > +	sysconfig_refresh(data);
> > +}
> > +
> > +static int machxo2_ops_write_complete(struct fpga_manager *mgr,
> > +				      struct fpga_image_info *info)
> > +{
> > +	int ret, retries = SYSCONFIG_POLL_RETRIES;
> > +	struct sysconfig_priv *priv = mgr->priv;
> > +	struct device *dev = &mgr->dev;
> > +	u32 status;
> > +
> > +	ret = sysconfig_isc_prog_done(priv);
> > +	if (ret) {
> > +		dev_err(dev, "Failed to enable Self-Download Mode\n");
> > +		goto fail;
> > +	}
> > +
> > +	ret = sysconfig_isc_disable(priv);
> > +	if (ret) {
> > +		dev_err(dev, "Failed to disable Configuration Interface\n");
> > +		goto fail;
> > +	}
> > +
> > +	while (retries--) {
> > +		ret = sysconfig_refresh(priv);
> > +		if (!ret)
> > +			ret = sysconfig_read_status(priv, &status);
> > +
> > +		if (ret) {
> > +			dev_err(dev, "Failed to refresh\n");
> > +			break;
> > +		}
> > +
> > +		if (status & SYSCONFIG_STATUS_DONE &&
> > +		    !(status & SYSCONFIG_STATUS_BUSY) &&
> > +		    !(status & SYSCONFIG_STATUS_ERR))
> > +			return 0;
> > +	}
> > +
> > +fail:
> > +	machxo2_cleanup(priv);
> > +
> > +	return -EFAULT;
> > +}
> > +
> > +static const struct fpga_manager_ops machxo2_fpga_ops = {
> > +	.state = machxo2_ops_state,
> > +	.write_init = machxo2_ops_write_init,
> > +	.write = machxo2_ops_write,
> > +	.write_complete = machxo2_ops_write_complete,
> > +};
> > +
> > +static int machxo2_probe(struct sysconfig_priv *priv)
> > +{
> > +	struct spi_device *spi = priv->spi;
> > +	struct device *dev = &spi->dev;
> > +	struct fpga_manager *mgr;
> > +
> > +	if (spi->max_speed_hz > MACHXO2_SPI_MAX_SPEED_HZ) {
> > +		dev_err(dev, "SPI speed %u is too high, maximum speed is %u\n",
> > +			spi->max_speed_hz, MACHXO2_SPI_MAX_SPEED_HZ);
> > +		return -EINVAL;
> > +	}
> > +
> > +	priv->isc_enable_operand = MACHXO2_ISC_ENABLE_OPERAND;
> > +	priv->isc_erase_operand = MACHXO2_ISC_ERASE_OPERAND;
> > +
> > +	mgr = devm_fpga_mgr_register(dev, "Lattice MachXO2 SPI FPGA Manager",
> > +				     &machxo2_fpga_ops, priv);
> > +
> > +	return PTR_ERR_OR_ZERO(mgr);
> > +}
> > +
> > +typedef int (*lattice_fpga_probe_func)(struct sysconfig_priv *);
> > +
> > +static int sysconfig_probe(struct spi_device *spi)
> > +{
> > +	const struct spi_device_id *dev_id;
> > +	lattice_fpga_probe_func probe_func;
> > +	struct device *dev = &spi->dev;
> > +	struct sysconfig_priv *priv;
> > +
> > +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > +	if (!priv)
> > +		return -ENOMEM;
> > +
> > +	priv->spi = spi;
> > +
> > +	probe_func = of_device_get_match_data(&spi->dev);
> > +	if (!probe_func) {
> > +		dev_id = spi_get_device_id(spi);
> > +		if (!dev_id)
> > +			return -ENODEV;
> > +
> > +		probe_func = (lattice_fpga_probe_func)dev_id->driver_data;
> > +	}
> > +
> > +	if (!probe_func)
> > +		return -EINVAL;
> > +
> > +	return probe_func(priv);
> > +}
> > +
> > +static const struct spi_device_id sysconfig_spi_ids[] = {
> > +	{
> > +		.name = "ecp5-fpga-mgr",
> > +		.driver_data = (kernel_ulong_t)ecp5_probe,
> > +	}, {
> > +		.name = "machxo2-fpga-mgr",
> > +		.driver_data = (kernel_ulong_t)machxo2_probe,
> 
> Putting the whole probe flow in driver_data is the same as providing 2
> drivers. The purpose is not to put all the code in one file.
> 

Sorry, I don't understand what you suggest. Separate file for each
specific FPGA?

> Thanks,
> Yilun
> 
> > +	}, {},
> > +};
> > +MODULE_DEVICE_TABLE(spi, sysconfig_spi_ids);
> > +
> > +#if IS_ENABLED(CONFIG_OF)
> > +static const struct of_device_id sysconfig_of_ids[] = {
> > +	{
> > +		.compatible = "lattice,ecp5-fpga-mgr",
> > +		.data = ecp5_probe,
> > +	}, {
> > +		.compatible = "lattice,machxo2-fpga-mgr",
> > +		.data = machxo2_probe
> > +	}, {},
> > +};
> > +MODULE_DEVICE_TABLE(of, sysconfig_of_ids);
> > +#endif /* IS_ENABLED(CONFIG_OF) */
> > +
> > +static struct spi_driver lattice_sysconfig_driver = {
> > +	.probe = sysconfig_probe,
> > +	.id_table = sysconfig_spi_ids,
> > +	.driver = {
> > +		.name = "lattice_sysconfig_spi_fpga_mgr",
> > +		.of_match_table = of_match_ptr(sysconfig_of_ids),
> > +	},
> > +};
> > +
> > +module_spi_driver(lattice_sysconfig_driver);
> > +
> > +MODULE_DESCRIPTION("Lattice sysCONFIG Slave SPI FPGA Manager");
> > +MODULE_LICENSE("GPL");
> > -- 
> > 2.37.2
> > 
> > 

Re: [PATCH v8 1/2] fpga: lattice-sysconfig-spi: add Lattice sysCONFIG FPGA manager

[Ivan Bornyakov]

On Mon, Aug 29, 2022 at 11:27:40AM +0300, Ivan Bornyakov wrote:
> On Mon, Aug 29, 2022 at 03:25:41PM +0800, Xu Yilun wrote:
> > On 2022-08-25 at 14:24:32 +0300, Ivan Bornyakov wrote:
> > > Add support to the FPGA manager for programming Lattice ECP5 and MachXO2
> > > FPGAs over slave SPI sysCONFIG interface.
> > > 
> > > ... snip ...
> > >
> > > +
> > > +static const struct spi_device_id sysconfig_spi_ids[] = {
> > > +	{
> > > +		.name = "ecp5-fpga-mgr",
> > > +		.driver_data = (kernel_ulong_t)ecp5_probe,
> > > +	}, {
> > > +		.name = "machxo2-fpga-mgr",
> > > +		.driver_data = (kernel_ulong_t)machxo2_probe,
> > 
> > Putting the whole probe flow in driver_data is the same as providing 2
> > drivers. The purpose is not to put all the code in one file.
> > 
> 
> Sorry, I don't understand what you suggest. Separate file for each
> specific FPGA?
> 

In v9 I was about to split this to sysconfig.c with sysCONFIG interface
port functions independent of port type + sysconfig-spi.c with functions
specific for SPI port. In this approach there is opportunity for
Johannes to add sysconfig-i2c.c with MachXO2's I2C port while reusing
fpga_manager_ops->state(), fpga_manager_ops->write_init() and
fpga_manager_ops->write_complete().

IDs structs array in sysconfig-spi.c still contains data with FPGA
specific probe, though.

Re: [PATCH v8 1/2] fpga: lattice-sysconfig-spi: add Lattice sysCONFIG FPGA manager

[Xu Yilun]

On 2022-08-29 at 11:27:40 +0300, Ivan Bornyakov wrote:
> On Mon, Aug 29, 2022 at 03:25:41PM +0800, Xu Yilun wrote:
> > On 2022-08-25 at 14:24:32 +0300, Ivan Bornyakov wrote:
> > > Add support to the FPGA manager for programming Lattice ECP5 and MachXO2
> > > FPGAs over slave SPI sysCONFIG interface.
> > > 
> > > Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
> > > ---
> > >  drivers/fpga/Kconfig                 |   7 +
> > >  drivers/fpga/Makefile                |   1 +
> > >  drivers/fpga/lattice-sysconfig-spi.c | 630 +++++++++++++++++++++++++++
> > >  3 files changed, 638 insertions(+)
> > >  create mode 100644 drivers/fpga/lattice-sysconfig-spi.c
> > > 
> > > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> > > index 6c416955da53..991d9d976dca 100644
> > > --- a/drivers/fpga/Kconfig
> > > +++ b/drivers/fpga/Kconfig
> > > @@ -263,4 +263,11 @@ config FPGA_MGR_MICROCHIP_SPI
> > >  	  programming over slave SPI interface with .dat formatted
> > >  	  bitstream image.
> > >  
> > > +config FPGA_MGR_LATTICE_SPI
> > > +	tristate "Lattice sysCONFIG SPI FPGA manager"
> > > +	depends on SPI
> > > +	help
> > > +	  FPGA manager driver support for Lattice FPGAs programming over slave
> > > +	  SPI sysCONFIG interface.
> > > +
> > >  endif # FPGA
> > > diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> > > index 42ae8b58abce..115dba916024 100644
> > > --- a/drivers/fpga/Makefile
> > > +++ b/drivers/fpga/Makefile
> > > @@ -20,6 +20,7 @@ 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
> > >  obj-$(CONFIG_FPGA_MGR_MICROCHIP_SPI)	+= microchip-spi.o
> > > +obj-$(CONFIG_FPGA_MGR_LATTICE_SPI)	+= lattice-sysconfig-spi.o
> > >  obj-$(CONFIG_ALTERA_PR_IP_CORE)		+= altera-pr-ip-core.o
> > >  obj-$(CONFIG_ALTERA_PR_IP_CORE_PLAT)	+= altera-pr-ip-core-plat.o
> > >  
> > > diff --git a/drivers/fpga/lattice-sysconfig-spi.c b/drivers/fpga/lattice-sysconfig-spi.c
> > > new file mode 100644
> > > index 000000000000..145b5b27b88d
> > > --- /dev/null
> > > +++ b/drivers/fpga/lattice-sysconfig-spi.c
> > > @@ -0,0 +1,630 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Lattice FPGA programming over slave SPI sysCONFIG interface.
> > > + */
> > > +
> > > +#include <linux/delay.h>
> > > +#include <linux/fpga/fpga-mgr.h>
> > > +#include <linux/of_device.h>
> > > +#include <linux/spi/spi.h>
> > > +
> > > +#define	SYSCONFIG_ISC_ENABLE		{0xC6, 0x00, 0x00, 0x00}
> > > +#define	SYSCONFIG_ISC_DISABLE		{0x26, 0x00, 0x00, 0x00}
> > > +#define	SYSCONFIG_ISC_ERASE		{0x0E, 0x00, 0x00, 0x00}
> > > +#define	SYSCONFIG_ISC_PROGRAM_DONE	{0x5E, 0x00, 0x00, 0x00}
> > > +#define	SYSCONFIG_LSC_READ_STATUS	{0x3C, 0x00, 0x00, 0x00}
> > > +#define	SYSCONFIG_LSC_CHECK_BUSY	{0xF0, 0x00, 0x00, 0x00}
> > > +#define	SYSCONFIG_LSC_REFRESH		{0x79, 0x00, 0x00, 0x00}
> > > +#define	SYSCONFIG_LSC_INIT_ADDR		{0x46, 0x00, 0x00, 0x00}
> > > +#define	SYSCONFIG_LSC_BITSTREAM_BURST	{0x7a, 0x00, 0x00, 0x00}
> > > +#define	SYSCONFIG_LSC_PROG_INCR_NV	{0x70, 0x00, 0x00, 0x01}
> > > +
> > > +#define	SYSCONFIG_STATUS_DONE		BIT(8)
> > > +#define	SYSCONFIG_STATUS_BUSY		BIT(12)
> > > +#define	SYSCONFIG_STATUS_FAIL		BIT(13)
> > > +#define	SYSCONFIG_STATUS_ERR		(BIT(23) | BIT(24) | BIT(25))
> > > +
> > > +#define	SYSCONFIG_POLL_RETRIES		100000
> > > +
> > > +#define	ECP5_SPI_MAX_SPEED_HZ		60000000
> > > +#define	ECP5_ISC_ERASE_OPERAND		0x01
> > > +
> > > +#define	MACHXO2_SPI_MAX_SPEED_HZ	66000000
> > > +#define	MACHXO2_PAGE_SIZE		16
> > > +#define	MACHXO2_ISC_ENABLE_OPERAND	0x08
> > > +#define	MACHXO2_ISC_ERASE_OPERAND	0x04
> > 
> > You need to deliver the meaning of each operand, rather than just point
> > out this is for machxo2, that is for esp5. I assume the sysCONFIG will
> > not deliberately design different commands with the exact same
> > functionality for different boards.
> > 
> > We should have a set of common configurations with finer granularity that
> > board specific fpga manager device could select from, rather than create
> > more and more similar macros for each new board.
> > 
> > Back to the 2 operands, what's the meaning of 0x1 & 0x4 for erase,
> 
> 0x1 - erase SRAM, 0x4 - erase FLASH. I'll redefine them properly in v9
> 
> > also 0x0 & 0x8 for enable?
> > 
> 
> Their meaning is not documented. It's just 0x08 for MachXO2 and 0x00 for
> ECP5.
> 
> > > +
> > > +struct sysconfig_priv {
> > > +	struct gpio_desc *program;
> > > +	struct gpio_desc *init;
> > > +	struct gpio_desc *done;
> > > +	struct spi_device *spi;
> > > +	u8 isc_enable_operand;
> > > +	u8 isc_erase_operand;
> > > +};
> > > +
> > > +static int sysconfig_poll_busy(struct sysconfig_priv *data)
> > > +{
> > > +	const u8 lsc_check_busy[] = SYSCONFIG_LSC_CHECK_BUSY;
> > > +	int ret, retries = SYSCONFIG_POLL_RETRIES;
> > > +	u8 busy;
> > > +
> > > +	while (retries--) {
> > > +		ret = spi_write_then_read(data->spi,
> > > +					  lsc_check_busy, sizeof(lsc_check_busy),
> > > +					  &busy, sizeof(busy));
> > > +		if (ret)
> > > +			return ret;
> > > +
> > > +		if (!busy)
> > > +			return 0;
> > > +
> > > +		usleep_range(50, 100);
> > > +	}
> > > +
> > > +	return -EBUSY;
> > > +}
> > > +
> > > +static int sysconfig_read_status(struct sysconfig_priv *data, u32 *status)
> > > +{
> > > +	const u8 lsc_read_status[] = SYSCONFIG_LSC_READ_STATUS;
> > > +	__be32 device_status;
> > > +	int ret;
> > > +
> > > +	ret = spi_write_then_read(data->spi,
> > > +				  lsc_read_status, sizeof(lsc_read_status),
> > > +				  &device_status, sizeof(device_status));
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	*status = be32_to_cpu(device_status);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static int sysconfig_poll_status(struct sysconfig_priv *data, u32 *status)
> > > +{
> > > +	int ret;
> > > +
> > > +	ret = sysconfig_poll_busy(data);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	return sysconfig_read_status(data, status);
> > > +}
> > > +
> > > +static int sysconfig_poll_gpio(struct gpio_desc *gpio, bool is_active)
> > > +{
> > > +	int value, retries = SYSCONFIG_POLL_RETRIES;
> > > +
> > > +	while (retries--) {
> > > +		value = gpiod_get_value(gpio);
> > > +		if (value < 0)
> > > +			return value;
> > > +
> > > +		if ((!is_active && !value) || (is_active && value))
> > > +			return 0;
> > > +
> > > +		ndelay(10);
> > > +	}
> > > +
> > > +	return -ETIMEDOUT;
> > > +}
> > > +
> > > +static int sysconfig_refresh(struct sysconfig_priv *data)
> > > +{
> > > +	static const u8 lsc_refresh[] = SYSCONFIG_LSC_REFRESH;
> > > +	int ret;
> > > +
> > > +	ret = spi_write(data->spi, lsc_refresh, sizeof(lsc_refresh));
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	usleep_range(4000, 8000);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static int sysconfig_isc_enable(struct sysconfig_priv *data)
> > > +{
> > > +	u8 isc_enable[] = SYSCONFIG_ISC_ENABLE;
> > > +	u32 status;
> > > +	int ret;
> > > +
> > > +	isc_enable[1] = data->isc_enable_operand;
> > > +
> > > +	ret = spi_write(data->spi, isc_enable, sizeof(isc_enable));
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	ret = sysconfig_poll_status(data, &status);
> > > +	if (ret || (status & SYSCONFIG_STATUS_FAIL))
> > > +		return ret ? : -EFAULT;
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static int sysconfig_isc_erase(struct sysconfig_priv *data)
> > > +{
> > > +	u8 isc_erase[] = SYSCONFIG_ISC_ERASE;
> > > +	u32 status;
> > > +	int ret;
> > > +
> > > +	isc_erase[1] = data->isc_erase_operand;
> > > +
> > > +	ret = spi_write(data->spi, isc_erase, sizeof(isc_erase));
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	ret = sysconfig_poll_status(data, &status);
> > > +	if (ret || (status & SYSCONFIG_STATUS_FAIL))
> > > +		return ret ? : -EFAULT;
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static int sysconfig_isc_init(struct sysconfig_priv *data)
> > > +{
> > > +	int ret;
> > > +
> > > +	ret = sysconfig_isc_enable(data);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	return sysconfig_isc_erase(data);
> > > +}
> > > +
> > > +static int sysconfig_lsc_init_addr(struct sysconfig_priv *data)
> > > +{
> > > +	const u8 lsc_init_addr[] = SYSCONFIG_LSC_INIT_ADDR;
> > > +
> > > +	return spi_write(data->spi, lsc_init_addr, sizeof(lsc_init_addr));
> > > +}
> > > +
> > > +static int sysconfig_lsc_bitstream_burst(struct sysconfig_priv *data)
> > > +{
> > > +	const u8 lsc_bitstream_burst[] = SYSCONFIG_LSC_BITSTREAM_BURST;
> > > +	struct spi_transfer xfer = {
> > > +		.tx_buf = lsc_bitstream_burst,
> > > +		.len = sizeof(lsc_bitstream_burst),
> > > +		.cs_change = 1,
> > > +	};
> > > +	struct spi_message msg;
> > > +
> > > +	spi_message_init_with_transfers(&msg, &xfer, 1);
> > > +
> > > +	return spi_sync_locked(data->spi, &msg);
> > > +}
> > > +
> > > +static int sysconfig_isc_prog_done(struct sysconfig_priv *data)
> > > +{
> > > +	const u8 isc_prog_done[] = SYSCONFIG_ISC_PROGRAM_DONE;
> > > +	u32 status;
> > > +	int ret;
> > > +
> > > +	ret = spi_write(data->spi, isc_prog_done, sizeof(isc_prog_done));
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	ret = sysconfig_poll_status(data, &status);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	if (status & SYSCONFIG_STATUS_DONE)
> > > +		return 0;
> > > +
> > > +	return -EFAULT;
> > > +}
> > > +
> > > +static int sysconfig_isc_disable(struct sysconfig_priv *data)
> > > +{
> > > +	const u8 isc_disable[] = SYSCONFIG_ISC_DISABLE;
> > > +
> > > +	return spi_write(data->spi, isc_disable, sizeof(isc_disable));
> > > +}
> > > +
> > > +static enum fpga_mgr_states ecp5_ops_state(struct fpga_manager *mgr)
> > > +{
> > > +	struct sysconfig_priv *priv = mgr->priv;
> > > +
> > > +	return (gpiod_get_value(priv->done) > 0) ? FPGA_MGR_STATE_OPERATING :
> > > +						   FPGA_MGR_STATE_UNKNOWN;
> > > +}
> > > +
> > > +static int ecp5_ops_write_init(struct fpga_manager *mgr,
> > > +			       struct fpga_image_info *info,
> > > +			       const char *buf, size_t count)
> > > +{
> > > +	struct sysconfig_priv *priv = mgr->priv;
> > > +	struct spi_device *spi = priv->spi;
> > > +	struct device *dev = &mgr->dev;
> > > +	struct gpio_desc *program;
> > > +	struct gpio_desc *init;
> > > +	struct gpio_desc *done;
> > > +	int ret;
> > > +
> > > +	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
> > > +		dev_err(dev, "Partial reconfiguration is not supported\n");
> > > +		return -EOPNOTSUPP;
> > > +	}
> > > +
> > > +	program = priv->program;
> > > +	init = priv->init;
> > > +	done = priv->done;
> > > +
> > > +	/* Enter init mode */
> > > +	gpiod_set_value(program, 1);
> > 
> > Same concern, provide gpio or command options for all board specific
> > fpga manager to select. 
> > 
> > I'll not list each features one by one below.
> > 
> > some more comments below.
> > 
> > > +
> > > +	ret = sysconfig_poll_gpio(init, true);
> > > +	if (!ret)
> > > +		ret = sysconfig_poll_gpio(done, false);
> > > +
> > > +	if (ret) {
> > > +		dev_err(dev, "Failed to go to init mode\n");
> > > +		return ret;
> > > +	}
> > > +
> > > +	/* Enter program mode */
> > > +	gpiod_set_value(program, 0);
> > > +
> > > +	ret = sysconfig_poll_gpio(init, false);
> > > +	if (ret) {
> > > +		dev_err(dev, "Failed to go to program mode\n");
> > > +		return ret;
> > > +	}
> > > +
> > > +	/* Enter ISC mode */
> > > +	ret = sysconfig_isc_init(priv);
> > > +	if (ret) {
> > > +		dev_err(dev, "Failed to go to ISC mode\n");
> > > +		return ret;
> > > +	}
> > > +
> > > +	/* Initialize the Address Shift Register */
> > > +	ret = sysconfig_lsc_init_addr(priv);
> > > +	if (ret) {
> > > +		dev_err(dev,
> > > +			"Failed to initialize the Address Shift Register\n");
> > > +		return ret;
> > > +	}
> > > +
> > > +	/*
> > > +	 * Lock SPI bus for exclusive usage until FPGA programming is done.
> > > +	 * SPI bus will be released in ecp5_ops_write_complete() or on error.
> > > +	 */
> > > +	spi_bus_lock(spi->controller);
> > > +
> > > +	/* Prepare for bitstream burst write */
> > > +	ret = sysconfig_lsc_bitstream_burst(priv);
> > > +	if (ret) {
> > > +		dev_err(dev, "Failed to prepare for bitstream burst write\n");
> > > +		spi_bus_unlock(spi->controller);
> > > +	}
> > > +
> > > +	return ret;
> > > +}
> > > +
> > > +static int ecp5_ops_write(struct fpga_manager *mgr, const char *buf, size_t count)
> > > +{
> > > +	struct sysconfig_priv *priv = mgr->priv;
> > > +	struct spi_device *spi = priv->spi;
> > > +	struct spi_transfer xfer = {
> > > +		.tx_buf = buf,
> > > +		.len = count,
> > > +		.cs_change = 1,
> > > +	};
> > > +	struct spi_message msg;
> > > +	int ret;
> > > +
> > > +	spi_message_init_with_transfers(&msg, &xfer, 1);
> > > +	ret = spi_sync_locked(spi, &msg);
> > > +	if (ret)
> > > +		spi_bus_unlock(spi->controller);
> > > +
> > > +	return ret;
> > > +}
> > > +
> > > +static int ecp5_ops_write_complete(struct fpga_manager *mgr,
> > > +				   struct fpga_image_info *info)
> > > +{
> > > +	struct sysconfig_priv *priv = mgr->priv;
> > > +	struct spi_device *spi = priv->spi;
> > > +	struct device *dev = &mgr->dev;
> > > +	int ret;
> > > +
> > > +	/* Bitstream burst write is done, release SPI bus */
> > > +	spi_bus_unlock(spi->controller);
> > > +
> > > +	/* Toggle CS and wait for bitstream write to finish */
> > > +	ret = spi_write(spi, NULL, 0);
> > > +	if (!ret)
> > > +		ret = sysconfig_poll_busy(priv);
> > > +
> > > +	if (ret) {
> > > +		dev_err(dev, "Error while waiting bitstream write to finish\n");
> > > +		return ret;
> > > +	}
> > > +
> > > +	/* Exit ISC mode */
> > > +	ret = sysconfig_isc_disable(priv);
> > > +	if (!ret)
> > > +		ret = sysconfig_poll_gpio(priv->done, true);
> > > +
> > > +	if (ret)
> > > +		dev_err(dev, "Failed to finish ISC\n");
> > > +
> > > +	return ret;
> > > +}
> > > +
> > > +static const struct fpga_manager_ops ecp5_fpga_ops = {
> > > +	.state = ecp5_ops_state,
> > > +	.write_init = ecp5_ops_write_init,
> > > +	.write = ecp5_ops_write,
> > > +	.write_complete = ecp5_ops_write_complete,
> > > +};
> > > +
> > > +static int ecp5_probe(struct sysconfig_priv *priv)
> > > +{
> > > +	struct spi_device *spi = priv->spi;
> > > +	struct device *dev = &spi->dev;
> > > +	struct fpga_manager *mgr;
> > > +	int ret;
> > > +
> > > +	if (spi->max_speed_hz > ECP5_SPI_MAX_SPEED_HZ) {
> > > +		dev_err(dev, "SPI speed %u is too high, maximum speed is %u\n",
> > > +			spi->max_speed_hz, ECP5_SPI_MAX_SPEED_HZ);
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	priv->isc_erase_operand = ECP5_ISC_ERASE_OPERAND;
> > > +
> > > +	priv->done = devm_gpiod_get(dev, "done", GPIOD_IN);
> > > +	if (IS_ERR(priv->done)) {
> > > +		ret = PTR_ERR(priv->done);
> > > +		dev_err(dev, "Failed to get DONE GPIO: %d\n", ret);
> > > +		return ret;
> > > +	}
> > > +
> > > +	priv->init = devm_gpiod_get(dev, "init", GPIOD_IN);
> > > +	if (IS_ERR(priv->init)) {
> > > +		ret = PTR_ERR(priv->init);
> > > +		dev_err(dev, "Failed to get INIT GPIO: %d\n", ret);
> > > +		return ret;
> > > +	}
> > > +
> > > +	priv->program = devm_gpiod_get(dev, "program", GPIOD_OUT_LOW);
> > > +	if (IS_ERR(priv->program)) {
> > > +		ret = PTR_ERR(priv->program);
> > > +		dev_err(dev, "Failed to get PROGRAM GPIO: %d\n", ret);
> > > +		return ret;
> > > +	}
> > > +
> > > +	mgr = devm_fpga_mgr_register(dev, "Lattice ECP5 SPI FPGA Manager",
> > > +				     &ecp5_fpga_ops, priv);
> > > +
> > > +	return PTR_ERR_OR_ZERO(mgr);
> > > +}
> > > +
> > > +static enum fpga_mgr_states machxo2_ops_state(struct fpga_manager *mgr)
> > > +{
> > > +	struct sysconfig_priv *priv = mgr->priv;
> > > +	u32 status;
> > > +	int ret;
> > > +
> > > +	ret = sysconfig_read_status(priv, &status);
> > > +	if (ret || !(status & SYSCONFIG_STATUS_DONE))
> > > +		return FPGA_MGR_STATE_UNKNOWN;
> > > +
> > > +	return FPGA_MGR_STATE_OPERATING;
> > > +}
> > > +
> > > +static int machxo2_ops_write_init(struct fpga_manager *mgr,
> > > +				  struct fpga_image_info *info,
> > > +				  const char *buf, size_t count)
> > > +{
> > > +	struct sysconfig_priv *priv = mgr->priv;
> > > +	struct device *dev = &mgr->dev;
> > > +	int ret;
> > > +
> > > +	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
> > > +		dev_err(dev, "Partial reconfiguration is not supported\n");
> > > +		return -EOPNOTSUPP;
> > > +	}
> > > +
> > > +	/* Enter ISC mode */
> > > +	ret = sysconfig_isc_init(priv);
> > > +	if (ret) {
> > > +		dev_err(dev, "Failed to go to ISC mode\n");
> > > +		return ret;
> > > +	}
> > > +
> > > +	/* Initialize the Address Shift Register */
> > > +	ret = sysconfig_lsc_init_addr(priv);
> > > +	if (ret)
> > > +		dev_err(dev,
> > > +			"Failed to initialize the Address Shift Register\n");
> > > +
> > > +	return ret;
> > > +}
> > > +
> > > +static int machxo2_ops_write(struct fpga_manager *mgr, const char *buf, size_t count)
> > > +{
> > > +	const u8 lsc_progincr[] = SYSCONFIG_LSC_PROG_INCR_NV;
> > > +	struct sysconfig_priv *priv = mgr->priv;
> > > +	struct device *dev = &mgr->dev;
> > > +	struct spi_transfer xfers[2] = {
> > > +		{
> > > +			.tx_buf = lsc_progincr,
> > > +			.len = sizeof(lsc_progincr),
> > > +		}, {
> > > +			.len = MACHXO2_PAGE_SIZE,
> > > +		},
> > > +	};
> > > +	size_t i;
> > > +	int ret;
> > > +
> > > +	if (count % MACHXO2_PAGE_SIZE) {
> > > +		dev_err(dev, "Malformed payload.\n");
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	for (i = 0; i < count; i += MACHXO2_PAGE_SIZE) {
> > > +		xfers[1].tx_buf = buf + i;
> > > +
> > > +		ret = spi_sync_transfer(priv->spi, xfers, 2);
> > > +		if (!ret)
> > > +			ret = sysconfig_poll_busy(priv);
> > > +
> > > +		if (ret) {
> > > +			dev_err(dev, "Failed to write frame %zu of %zu\n",
> > > +				i / MACHXO2_PAGE_SIZE, count / MACHXO2_PAGE_SIZE);
> > > +			return ret;
> > > +		}
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static void machxo2_cleanup(struct sysconfig_priv *data)
> > > +{
> > > +	sysconfig_isc_erase(data);
> > > +	sysconfig_refresh(data);
> > > +}
> > > +
> > > +static int machxo2_ops_write_complete(struct fpga_manager *mgr,
> > > +				      struct fpga_image_info *info)
> > > +{
> > > +	int ret, retries = SYSCONFIG_POLL_RETRIES;
> > > +	struct sysconfig_priv *priv = mgr->priv;
> > > +	struct device *dev = &mgr->dev;
> > > +	u32 status;
> > > +
> > > +	ret = sysconfig_isc_prog_done(priv);
> > > +	if (ret) {
> > > +		dev_err(dev, "Failed to enable Self-Download Mode\n");
> > > +		goto fail;
> > > +	}
> > > +
> > > +	ret = sysconfig_isc_disable(priv);
> > > +	if (ret) {
> > > +		dev_err(dev, "Failed to disable Configuration Interface\n");
> > > +		goto fail;
> > > +	}
> > > +
> > > +	while (retries--) {
> > > +		ret = sysconfig_refresh(priv);
> > > +		if (!ret)
> > > +			ret = sysconfig_read_status(priv, &status);
> > > +
> > > +		if (ret) {
> > > +			dev_err(dev, "Failed to refresh\n");
> > > +			break;
> > > +		}
> > > +
> > > +		if (status & SYSCONFIG_STATUS_DONE &&
> > > +		    !(status & SYSCONFIG_STATUS_BUSY) &&
> > > +		    !(status & SYSCONFIG_STATUS_ERR))
> > > +			return 0;
> > > +	}
> > > +
> > > +fail:
> > > +	machxo2_cleanup(priv);
> > > +
> > > +	return -EFAULT;
> > > +}
> > > +
> > > +static const struct fpga_manager_ops machxo2_fpga_ops = {
> > > +	.state = machxo2_ops_state,
> > > +	.write_init = machxo2_ops_write_init,
> > > +	.write = machxo2_ops_write,
> > > +	.write_complete = machxo2_ops_write_complete,
> > > +};
> > > +
> > > +static int machxo2_probe(struct sysconfig_priv *priv)
> > > +{
> > > +	struct spi_device *spi = priv->spi;
> > > +	struct device *dev = &spi->dev;
> > > +	struct fpga_manager *mgr;
> > > +
> > > +	if (spi->max_speed_hz > MACHXO2_SPI_MAX_SPEED_HZ) {
> > > +		dev_err(dev, "SPI speed %u is too high, maximum speed is %u\n",
> > > +			spi->max_speed_hz, MACHXO2_SPI_MAX_SPEED_HZ);
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	priv->isc_enable_operand = MACHXO2_ISC_ENABLE_OPERAND;
> > > +	priv->isc_erase_operand = MACHXO2_ISC_ERASE_OPERAND;
> > > +
> > > +	mgr = devm_fpga_mgr_register(dev, "Lattice MachXO2 SPI FPGA Manager",
> > > +				     &machxo2_fpga_ops, priv);
> > > +
> > > +	return PTR_ERR_OR_ZERO(mgr);
> > > +}
> > > +
> > > +typedef int (*lattice_fpga_probe_func)(struct sysconfig_priv *);
> > > +
> > > +static int sysconfig_probe(struct spi_device *spi)
> > > +{
> > > +	const struct spi_device_id *dev_id;
> > > +	lattice_fpga_probe_func probe_func;
> > > +	struct device *dev = &spi->dev;
> > > +	struct sysconfig_priv *priv;
> > > +
> > > +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > > +	if (!priv)
> > > +		return -ENOMEM;
> > > +
> > > +	priv->spi = spi;
> > > +
> > > +	probe_func = of_device_get_match_data(&spi->dev);
> > > +	if (!probe_func) {
> > > +		dev_id = spi_get_device_id(spi);
> > > +		if (!dev_id)
> > > +			return -ENODEV;
> > > +
> > > +		probe_func = (lattice_fpga_probe_func)dev_id->driver_data;
> > > +	}
> > > +
> > > +	if (!probe_func)
> > > +		return -EINVAL;
> > > +
> > > +	return probe_func(priv);
> > > +}
> > > +
> > > +static const struct spi_device_id sysconfig_spi_ids[] = {
> > > +	{
> > > +		.name = "ecp5-fpga-mgr",
> > > +		.driver_data = (kernel_ulong_t)ecp5_probe,
> > > +	}, {
> > > +		.name = "machxo2-fpga-mgr",
> > > +		.driver_data = (kernel_ulong_t)machxo2_probe,
> > 
> > Putting the whole probe flow in driver_data is the same as providing 2
> > drivers. The purpose is not to put all the code in one file.
> > 
> 
> Sorry, I don't understand what you suggest. Separate file for each
> specific FPGA?

Sorry, didn't make it clear. I feel like there are still 2 board
specific drivers although they are now in one file, they have
separate probes, fpga manager ops, different spi APIs ...

What I expect is, the driver operates the HW according to the configured
properties specified in sysCONFIG spec, such as having PROGRAMN gpio pin?
burst? having backup flash? ...

Thanks,
Yilun

> 
> > Thanks,
> > Yilun
> > 
> > > +	}, {},
> > > +};
> > > +MODULE_DEVICE_TABLE(spi, sysconfig_spi_ids);
> > > +
> > > +#if IS_ENABLED(CONFIG_OF)
> > > +static const struct of_device_id sysconfig_of_ids[] = {
> > > +	{
> > > +		.compatible = "lattice,ecp5-fpga-mgr",
> > > +		.data = ecp5_probe,
> > > +	}, {
> > > +		.compatible = "lattice,machxo2-fpga-mgr",
> > > +		.data = machxo2_probe
> > > +	}, {},
> > > +};
> > > +MODULE_DEVICE_TABLE(of, sysconfig_of_ids);
> > > +#endif /* IS_ENABLED(CONFIG_OF) */
> > > +
> > > +static struct spi_driver lattice_sysconfig_driver = {
> > > +	.probe = sysconfig_probe,
> > > +	.id_table = sysconfig_spi_ids,
> > > +	.driver = {
> > > +		.name = "lattice_sysconfig_spi_fpga_mgr",
> > > +		.of_match_table = of_match_ptr(sysconfig_of_ids),
> > > +	},
> > > +};
> > > +
> > > +module_spi_driver(lattice_sysconfig_driver);
> > > +
> > > +MODULE_DESCRIPTION("Lattice sysCONFIG Slave SPI FPGA Manager");
> > > +MODULE_LICENSE("GPL");
> > > -- 
> > > 2.37.2
> > > 
> > > 
> 

Re: [PATCH v8 1/2] fpga: lattice-sysconfig-spi: add Lattice sysCONFIG FPGA manager

[Ivan Bornyakov]

On Mon, Aug 29, 2022 at 06:34:44PM +0800, Xu Yilun wrote:
> On 2022-08-29 at 11:27:40 +0300, Ivan Bornyakov wrote:
> > On Mon, Aug 29, 2022 at 03:25:41PM +0800, Xu Yilun wrote:
> > > On 2022-08-25 at 14:24:32 +0300, Ivan Bornyakov wrote:
> > > > Add support to the FPGA manager for programming Lattice ECP5 and MachXO2
> > > > FPGAs over slave SPI sysCONFIG interface.
> > > > 
> > > > Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
> > > > ---
> > > >  drivers/fpga/Kconfig                 |   7 +
> > > >  drivers/fpga/Makefile                |   1 +
> > > >  drivers/fpga/lattice-sysconfig-spi.c | 630 +++++++++++++++++++++++++++
> > > >  3 files changed, 638 insertions(+)
> > > >  create mode 100644 drivers/fpga/lattice-sysconfig-spi.c
> > > > 
> > > > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> > > > index 6c416955da53..991d9d976dca 100644
> > > > --- a/drivers/fpga/Kconfig
> > > > +++ b/drivers/fpga/Kconfig
> > > > @@ -263,4 +263,11 @@ config FPGA_MGR_MICROCHIP_SPI
> > > >  	  programming over slave SPI interface with .dat formatted
> > > >  	  bitstream image.
> > > >  
> > > > +config FPGA_MGR_LATTICE_SPI
> > > > +	tristate "Lattice sysCONFIG SPI FPGA manager"
> > > > +	depends on SPI
> > > > +	help
> > > > +	  FPGA manager driver support for Lattice FPGAs programming over slave
> > > > +	  SPI sysCONFIG interface.
> > > > +
> > > >  endif # FPGA
> > > > diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> > > > index 42ae8b58abce..115dba916024 100644
> > > > --- a/drivers/fpga/Makefile
> > > > +++ b/drivers/fpga/Makefile
> > > > @@ -20,6 +20,7 @@ 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
> > > >  obj-$(CONFIG_FPGA_MGR_MICROCHIP_SPI)	+= microchip-spi.o
> > > > +obj-$(CONFIG_FPGA_MGR_LATTICE_SPI)	+= lattice-sysconfig-spi.o
> > > >  obj-$(CONFIG_ALTERA_PR_IP_CORE)		+= altera-pr-ip-core.o
> > > >  obj-$(CONFIG_ALTERA_PR_IP_CORE_PLAT)	+= altera-pr-ip-core-plat.o
> > > >  
> > > > diff --git a/drivers/fpga/lattice-sysconfig-spi.c b/drivers/fpga/lattice-sysconfig-spi.c
> > > > new file mode 100644
> > > > index 000000000000..145b5b27b88d
> > > > --- /dev/null
> > > > +++ b/drivers/fpga/lattice-sysconfig-spi.c
> > > > @@ -0,0 +1,630 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * Lattice FPGA programming over slave SPI sysCONFIG interface.
> > > > + */
> > > > +
> > > > +#include <linux/delay.h>
> > > > +#include <linux/fpga/fpga-mgr.h>
> > > > +#include <linux/of_device.h>
> > > > +#include <linux/spi/spi.h>
> > > > +
> > > > +#define	SYSCONFIG_ISC_ENABLE		{0xC6, 0x00, 0x00, 0x00}
> > > > +#define	SYSCONFIG_ISC_DISABLE		{0x26, 0x00, 0x00, 0x00}
> > > > +#define	SYSCONFIG_ISC_ERASE		{0x0E, 0x00, 0x00, 0x00}
> > > > +#define	SYSCONFIG_ISC_PROGRAM_DONE	{0x5E, 0x00, 0x00, 0x00}
> > > > +#define	SYSCONFIG_LSC_READ_STATUS	{0x3C, 0x00, 0x00, 0x00}
> > > > +#define	SYSCONFIG_LSC_CHECK_BUSY	{0xF0, 0x00, 0x00, 0x00}
> > > > +#define	SYSCONFIG_LSC_REFRESH		{0x79, 0x00, 0x00, 0x00}
> > > > +#define	SYSCONFIG_LSC_INIT_ADDR		{0x46, 0x00, 0x00, 0x00}
> > > > +#define	SYSCONFIG_LSC_BITSTREAM_BURST	{0x7a, 0x00, 0x00, 0x00}
> > > > +#define	SYSCONFIG_LSC_PROG_INCR_NV	{0x70, 0x00, 0x00, 0x01}
> > > > +
> > > > +#define	SYSCONFIG_STATUS_DONE		BIT(8)
> > > > +#define	SYSCONFIG_STATUS_BUSY		BIT(12)
> > > > +#define	SYSCONFIG_STATUS_FAIL		BIT(13)
> > > > +#define	SYSCONFIG_STATUS_ERR		(BIT(23) | BIT(24) | BIT(25))
> > > > +
> > > > +#define	SYSCONFIG_POLL_RETRIES		100000
> > > > +
> > > > +#define	ECP5_SPI_MAX_SPEED_HZ		60000000
> > > > +#define	ECP5_ISC_ERASE_OPERAND		0x01
> > > > +
> > > > +#define	MACHXO2_SPI_MAX_SPEED_HZ	66000000
> > > > +#define	MACHXO2_PAGE_SIZE		16
> > > > +#define	MACHXO2_ISC_ENABLE_OPERAND	0x08
> > > > +#define	MACHXO2_ISC_ERASE_OPERAND	0x04
> > > 
> > > You need to deliver the meaning of each operand, rather than just point
> > > out this is for machxo2, that is for esp5. I assume the sysCONFIG will
> > > not deliberately design different commands with the exact same
> > > functionality for different boards.
> > > 
> > > We should have a set of common configurations with finer granularity that
> > > board specific fpga manager device could select from, rather than create
> > > more and more similar macros for each new board.
> > > 
> > > Back to the 2 operands, what's the meaning of 0x1 & 0x4 for erase,
> > 
> > 0x1 - erase SRAM, 0x4 - erase FLASH. I'll redefine them properly in v9
> > 
> > > also 0x0 & 0x8 for enable?
> > > 
> > 
> > Their meaning is not documented. It's just 0x08 for MachXO2 and 0x00 for
> > ECP5.
> > 
> > > > +
> > > > +struct sysconfig_priv {
> > > > +	struct gpio_desc *program;
> > > > +	struct gpio_desc *init;
> > > > +	struct gpio_desc *done;
> > > > +	struct spi_device *spi;
> > > > +	u8 isc_enable_operand;
> > > > +	u8 isc_erase_operand;
> > > > +};
> > > > +
> > > > +static int sysconfig_poll_busy(struct sysconfig_priv *data)
> > > > +{
> > > > +	const u8 lsc_check_busy[] = SYSCONFIG_LSC_CHECK_BUSY;
> > > > +	int ret, retries = SYSCONFIG_POLL_RETRIES;
> > > > +	u8 busy;
> > > > +
> > > > +	while (retries--) {
> > > > +		ret = spi_write_then_read(data->spi,
> > > > +					  lsc_check_busy, sizeof(lsc_check_busy),
> > > > +					  &busy, sizeof(busy));
> > > > +		if (ret)
> > > > +			return ret;
> > > > +
> > > > +		if (!busy)
> > > > +			return 0;
> > > > +
> > > > +		usleep_range(50, 100);
> > > > +	}
> > > > +
> > > > +	return -EBUSY;
> > > > +}
> > > > +
> > > > +static int sysconfig_read_status(struct sysconfig_priv *data, u32 *status)
> > > > +{
> > > > +	const u8 lsc_read_status[] = SYSCONFIG_LSC_READ_STATUS;
> > > > +	__be32 device_status;
> > > > +	int ret;
> > > > +
> > > > +	ret = spi_write_then_read(data->spi,
> > > > +				  lsc_read_status, sizeof(lsc_read_status),
> > > > +				  &device_status, sizeof(device_status));
> > > > +	if (ret)
> > > > +		return ret;
> > > > +
> > > > +	*status = be32_to_cpu(device_status);
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +static int sysconfig_poll_status(struct sysconfig_priv *data, u32 *status)
> > > > +{
> > > > +	int ret;
> > > > +
> > > > +	ret = sysconfig_poll_busy(data);
> > > > +	if (ret)
> > > > +		return ret;
> > > > +
> > > > +	return sysconfig_read_status(data, status);
> > > > +}
> > > > +
> > > > +static int sysconfig_poll_gpio(struct gpio_desc *gpio, bool is_active)
> > > > +{
> > > > +	int value, retries = SYSCONFIG_POLL_RETRIES;
> > > > +
> > > > +	while (retries--) {
> > > > +		value = gpiod_get_value(gpio);
> > > > +		if (value < 0)
> > > > +			return value;
> > > > +
> > > > +		if ((!is_active && !value) || (is_active && value))
> > > > +			return 0;
> > > > +
> > > > +		ndelay(10);
> > > > +	}
> > > > +
> > > > +	return -ETIMEDOUT;
> > > > +}
> > > > +
> > > > +static int sysconfig_refresh(struct sysconfig_priv *data)
> > > > +{
> > > > +	static const u8 lsc_refresh[] = SYSCONFIG_LSC_REFRESH;
> > > > +	int ret;
> > > > +
> > > > +	ret = spi_write(data->spi, lsc_refresh, sizeof(lsc_refresh));
> > > > +	if (ret)
> > > > +		return ret;
> > > > +
> > > > +	usleep_range(4000, 8000);
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +static int sysconfig_isc_enable(struct sysconfig_priv *data)
> > > > +{
> > > > +	u8 isc_enable[] = SYSCONFIG_ISC_ENABLE;
> > > > +	u32 status;
> > > > +	int ret;
> > > > +
> > > > +	isc_enable[1] = data->isc_enable_operand;
> > > > +
> > > > +	ret = spi_write(data->spi, isc_enable, sizeof(isc_enable));
> > > > +	if (ret)
> > > > +		return ret;
> > > > +
> > > > +	ret = sysconfig_poll_status(data, &status);
> > > > +	if (ret || (status & SYSCONFIG_STATUS_FAIL))
> > > > +		return ret ? : -EFAULT;
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +static int sysconfig_isc_erase(struct sysconfig_priv *data)
> > > > +{
> > > > +	u8 isc_erase[] = SYSCONFIG_ISC_ERASE;
> > > > +	u32 status;
> > > > +	int ret;
> > > > +
> > > > +	isc_erase[1] = data->isc_erase_operand;
> > > > +
> > > > +	ret = spi_write(data->spi, isc_erase, sizeof(isc_erase));
> > > > +	if (ret)
> > > > +		return ret;
> > > > +
> > > > +	ret = sysconfig_poll_status(data, &status);
> > > > +	if (ret || (status & SYSCONFIG_STATUS_FAIL))
> > > > +		return ret ? : -EFAULT;
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +static int sysconfig_isc_init(struct sysconfig_priv *data)
> > > > +{
> > > > +	int ret;
> > > > +
> > > > +	ret = sysconfig_isc_enable(data);
> > > > +	if (ret)
> > > > +		return ret;
> > > > +
> > > > +	return sysconfig_isc_erase(data);
> > > > +}
> > > > +
> > > > +static int sysconfig_lsc_init_addr(struct sysconfig_priv *data)
> > > > +{
> > > > +	const u8 lsc_init_addr[] = SYSCONFIG_LSC_INIT_ADDR;
> > > > +
> > > > +	return spi_write(data->spi, lsc_init_addr, sizeof(lsc_init_addr));
> > > > +}
> > > > +
> > > > +static int sysconfig_lsc_bitstream_burst(struct sysconfig_priv *data)
> > > > +{
> > > > +	const u8 lsc_bitstream_burst[] = SYSCONFIG_LSC_BITSTREAM_BURST;
> > > > +	struct spi_transfer xfer = {
> > > > +		.tx_buf = lsc_bitstream_burst,
> > > > +		.len = sizeof(lsc_bitstream_burst),
> > > > +		.cs_change = 1,
> > > > +	};
> > > > +	struct spi_message msg;
> > > > +
> > > > +	spi_message_init_with_transfers(&msg, &xfer, 1);
> > > > +
> > > > +	return spi_sync_locked(data->spi, &msg);
> > > > +}
> > > > +
> > > > +static int sysconfig_isc_prog_done(struct sysconfig_priv *data)
> > > > +{
> > > > +	const u8 isc_prog_done[] = SYSCONFIG_ISC_PROGRAM_DONE;
> > > > +	u32 status;
> > > > +	int ret;
> > > > +
> > > > +	ret = spi_write(data->spi, isc_prog_done, sizeof(isc_prog_done));
> > > > +	if (ret)
> > > > +		return ret;
> > > > +
> > > > +	ret = sysconfig_poll_status(data, &status);
> > > > +	if (ret)
> > > > +		return ret;
> > > > +
> > > > +	if (status & SYSCONFIG_STATUS_DONE)
> > > > +		return 0;
> > > > +
> > > > +	return -EFAULT;
> > > > +}
> > > > +
> > > > +static int sysconfig_isc_disable(struct sysconfig_priv *data)
> > > > +{
> > > > +	const u8 isc_disable[] = SYSCONFIG_ISC_DISABLE;
> > > > +
> > > > +	return spi_write(data->spi, isc_disable, sizeof(isc_disable));
> > > > +}
> > > > +
> > > > +static enum fpga_mgr_states ecp5_ops_state(struct fpga_manager *mgr)
> > > > +{
> > > > +	struct sysconfig_priv *priv = mgr->priv;
> > > > +
> > > > +	return (gpiod_get_value(priv->done) > 0) ? FPGA_MGR_STATE_OPERATING :
> > > > +						   FPGA_MGR_STATE_UNKNOWN;
> > > > +}
> > > > +
> > > > +static int ecp5_ops_write_init(struct fpga_manager *mgr,
> > > > +			       struct fpga_image_info *info,
> > > > +			       const char *buf, size_t count)
> > > > +{
> > > > +	struct sysconfig_priv *priv = mgr->priv;
> > > > +	struct spi_device *spi = priv->spi;
> > > > +	struct device *dev = &mgr->dev;
> > > > +	struct gpio_desc *program;
> > > > +	struct gpio_desc *init;
> > > > +	struct gpio_desc *done;
> > > > +	int ret;
> > > > +
> > > > +	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
> > > > +		dev_err(dev, "Partial reconfiguration is not supported\n");
> > > > +		return -EOPNOTSUPP;
> > > > +	}
> > > > +
> > > > +	program = priv->program;
> > > > +	init = priv->init;
> > > > +	done = priv->done;
> > > > +
> > > > +	/* Enter init mode */
> > > > +	gpiod_set_value(program, 1);
> > > 
> > > Same concern, provide gpio or command options for all board specific
> > > fpga manager to select. 
> > > 
> > > I'll not list each features one by one below.
> > > 
> > > some more comments below.
> > > 
> > > > +
> > > > +	ret = sysconfig_poll_gpio(init, true);
> > > > +	if (!ret)
> > > > +		ret = sysconfig_poll_gpio(done, false);
> > > > +
> > > > +	if (ret) {
> > > > +		dev_err(dev, "Failed to go to init mode\n");
> > > > +		return ret;
> > > > +	}
> > > > +
> > > > +	/* Enter program mode */
> > > > +	gpiod_set_value(program, 0);
> > > > +
> > > > +	ret = sysconfig_poll_gpio(init, false);
> > > > +	if (ret) {
> > > > +		dev_err(dev, "Failed to go to program mode\n");
> > > > +		return ret;
> > > > +	}
> > > > +
> > > > +	/* Enter ISC mode */
> > > > +	ret = sysconfig_isc_init(priv);
> > > > +	if (ret) {
> > > > +		dev_err(dev, "Failed to go to ISC mode\n");
> > > > +		return ret;
> > > > +	}
> > > > +
> > > > +	/* Initialize the Address Shift Register */
> > > > +	ret = sysconfig_lsc_init_addr(priv);
> > > > +	if (ret) {
> > > > +		dev_err(dev,
> > > > +			"Failed to initialize the Address Shift Register\n");
> > > > +		return ret;
> > > > +	}
> > > > +
> > > > +	/*
> > > > +	 * Lock SPI bus for exclusive usage until FPGA programming is done.
> > > > +	 * SPI bus will be released in ecp5_ops_write_complete() or on error.
> > > > +	 */
> > > > +	spi_bus_lock(spi->controller);
> > > > +
> > > > +	/* Prepare for bitstream burst write */
> > > > +	ret = sysconfig_lsc_bitstream_burst(priv);
> > > > +	if (ret) {
> > > > +		dev_err(dev, "Failed to prepare for bitstream burst write\n");
> > > > +		spi_bus_unlock(spi->controller);
> > > > +	}
> > > > +
> > > > +	return ret;
> > > > +}
> > > > +
> > > > +static int ecp5_ops_write(struct fpga_manager *mgr, const char *buf, size_t count)
> > > > +{
> > > > +	struct sysconfig_priv *priv = mgr->priv;
> > > > +	struct spi_device *spi = priv->spi;
> > > > +	struct spi_transfer xfer = {
> > > > +		.tx_buf = buf,
> > > > +		.len = count,
> > > > +		.cs_change = 1,
> > > > +	};
> > > > +	struct spi_message msg;
> > > > +	int ret;
> > > > +
> > > > +	spi_message_init_with_transfers(&msg, &xfer, 1);
> > > > +	ret = spi_sync_locked(spi, &msg);
> > > > +	if (ret)
> > > > +		spi_bus_unlock(spi->controller);
> > > > +
> > > > +	return ret;
> > > > +}
> > > > +
> > > > +static int ecp5_ops_write_complete(struct fpga_manager *mgr,
> > > > +				   struct fpga_image_info *info)
> > > > +{
> > > > +	struct sysconfig_priv *priv = mgr->priv;
> > > > +	struct spi_device *spi = priv->spi;
> > > > +	struct device *dev = &mgr->dev;
> > > > +	int ret;
> > > > +
> > > > +	/* Bitstream burst write is done, release SPI bus */
> > > > +	spi_bus_unlock(spi->controller);
> > > > +
> > > > +	/* Toggle CS and wait for bitstream write to finish */
> > > > +	ret = spi_write(spi, NULL, 0);
> > > > +	if (!ret)
> > > > +		ret = sysconfig_poll_busy(priv);
> > > > +
> > > > +	if (ret) {
> > > > +		dev_err(dev, "Error while waiting bitstream write to finish\n");
> > > > +		return ret;
> > > > +	}
> > > > +
> > > > +	/* Exit ISC mode */
> > > > +	ret = sysconfig_isc_disable(priv);
> > > > +	if (!ret)
> > > > +		ret = sysconfig_poll_gpio(priv->done, true);
> > > > +
> > > > +	if (ret)
> > > > +		dev_err(dev, "Failed to finish ISC\n");
> > > > +
> > > > +	return ret;
> > > > +}
> > > > +
> > > > +static const struct fpga_manager_ops ecp5_fpga_ops = {
> > > > +	.state = ecp5_ops_state,
> > > > +	.write_init = ecp5_ops_write_init,
> > > > +	.write = ecp5_ops_write,
> > > > +	.write_complete = ecp5_ops_write_complete,
> > > > +};
> > > > +
> > > > +static int ecp5_probe(struct sysconfig_priv *priv)
> > > > +{
> > > > +	struct spi_device *spi = priv->spi;
> > > > +	struct device *dev = &spi->dev;
> > > > +	struct fpga_manager *mgr;
> > > > +	int ret;
> > > > +
> > > > +	if (spi->max_speed_hz > ECP5_SPI_MAX_SPEED_HZ) {
> > > > +		dev_err(dev, "SPI speed %u is too high, maximum speed is %u\n",
> > > > +			spi->max_speed_hz, ECP5_SPI_MAX_SPEED_HZ);
> > > > +		return -EINVAL;
> > > > +	}
> > > > +
> > > > +	priv->isc_erase_operand = ECP5_ISC_ERASE_OPERAND;
> > > > +
> > > > +	priv->done = devm_gpiod_get(dev, "done", GPIOD_IN);
> > > > +	if (IS_ERR(priv->done)) {
> > > > +		ret = PTR_ERR(priv->done);
> > > > +		dev_err(dev, "Failed to get DONE GPIO: %d\n", ret);
> > > > +		return ret;
> > > > +	}
> > > > +
> > > > +	priv->init = devm_gpiod_get(dev, "init", GPIOD_IN);
> > > > +	if (IS_ERR(priv->init)) {
> > > > +		ret = PTR_ERR(priv->init);
> > > > +		dev_err(dev, "Failed to get INIT GPIO: %d\n", ret);
> > > > +		return ret;
> > > > +	}
> > > > +
> > > > +	priv->program = devm_gpiod_get(dev, "program", GPIOD_OUT_LOW);
> > > > +	if (IS_ERR(priv->program)) {
> > > > +		ret = PTR_ERR(priv->program);
> > > > +		dev_err(dev, "Failed to get PROGRAM GPIO: %d\n", ret);
> > > > +		return ret;
> > > > +	}
> > > > +
> > > > +	mgr = devm_fpga_mgr_register(dev, "Lattice ECP5 SPI FPGA Manager",
> > > > +				     &ecp5_fpga_ops, priv);
> > > > +
> > > > +	return PTR_ERR_OR_ZERO(mgr);
> > > > +}
> > > > +
> > > > +static enum fpga_mgr_states machxo2_ops_state(struct fpga_manager *mgr)
> > > > +{
> > > > +	struct sysconfig_priv *priv = mgr->priv;
> > > > +	u32 status;
> > > > +	int ret;
> > > > +
> > > > +	ret = sysconfig_read_status(priv, &status);
> > > > +	if (ret || !(status & SYSCONFIG_STATUS_DONE))
> > > > +		return FPGA_MGR_STATE_UNKNOWN;
> > > > +
> > > > +	return FPGA_MGR_STATE_OPERATING;
> > > > +}
> > > > +
> > > > +static int machxo2_ops_write_init(struct fpga_manager *mgr,
> > > > +				  struct fpga_image_info *info,
> > > > +				  const char *buf, size_t count)
> > > > +{
> > > > +	struct sysconfig_priv *priv = mgr->priv;
> > > > +	struct device *dev = &mgr->dev;
> > > > +	int ret;
> > > > +
> > > > +	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
> > > > +		dev_err(dev, "Partial reconfiguration is not supported\n");
> > > > +		return -EOPNOTSUPP;
> > > > +	}
> > > > +
> > > > +	/* Enter ISC mode */
> > > > +	ret = sysconfig_isc_init(priv);
> > > > +	if (ret) {
> > > > +		dev_err(dev, "Failed to go to ISC mode\n");
> > > > +		return ret;
> > > > +	}
> > > > +
> > > > +	/* Initialize the Address Shift Register */
> > > > +	ret = sysconfig_lsc_init_addr(priv);
> > > > +	if (ret)
> > > > +		dev_err(dev,
> > > > +			"Failed to initialize the Address Shift Register\n");
> > > > +
> > > > +	return ret;
> > > > +}
> > > > +
> > > > +static int machxo2_ops_write(struct fpga_manager *mgr, const char *buf, size_t count)
> > > > +{
> > > > +	const u8 lsc_progincr[] = SYSCONFIG_LSC_PROG_INCR_NV;
> > > > +	struct sysconfig_priv *priv = mgr->priv;
> > > > +	struct device *dev = &mgr->dev;
> > > > +	struct spi_transfer xfers[2] = {
> > > > +		{
> > > > +			.tx_buf = lsc_progincr,
> > > > +			.len = sizeof(lsc_progincr),
> > > > +		}, {
> > > > +			.len = MACHXO2_PAGE_SIZE,
> > > > +		},
> > > > +	};
> > > > +	size_t i;
> > > > +	int ret;
> > > > +
> > > > +	if (count % MACHXO2_PAGE_SIZE) {
> > > > +		dev_err(dev, "Malformed payload.\n");
> > > > +		return -EINVAL;
> > > > +	}
> > > > +
> > > > +	for (i = 0; i < count; i += MACHXO2_PAGE_SIZE) {
> > > > +		xfers[1].tx_buf = buf + i;
> > > > +
> > > > +		ret = spi_sync_transfer(priv->spi, xfers, 2);
> > > > +		if (!ret)
> > > > +			ret = sysconfig_poll_busy(priv);
> > > > +
> > > > +		if (ret) {
> > > > +			dev_err(dev, "Failed to write frame %zu of %zu\n",
> > > > +				i / MACHXO2_PAGE_SIZE, count / MACHXO2_PAGE_SIZE);
> > > > +			return ret;
> > > > +		}
> > > > +	}
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > > +static void machxo2_cleanup(struct sysconfig_priv *data)
> > > > +{
> > > > +	sysconfig_isc_erase(data);
> > > > +	sysconfig_refresh(data);
> > > > +}
> > > > +
> > > > +static int machxo2_ops_write_complete(struct fpga_manager *mgr,
> > > > +				      struct fpga_image_info *info)
> > > > +{
> > > > +	int ret, retries = SYSCONFIG_POLL_RETRIES;
> > > > +	struct sysconfig_priv *priv = mgr->priv;
> > > > +	struct device *dev = &mgr->dev;
> > > > +	u32 status;
> > > > +
> > > > +	ret = sysconfig_isc_prog_done(priv);
> > > > +	if (ret) {
> > > > +		dev_err(dev, "Failed to enable Self-Download Mode\n");
> > > > +		goto fail;
> > > > +	}
> > > > +
> > > > +	ret = sysconfig_isc_disable(priv);
> > > > +	if (ret) {
> > > > +		dev_err(dev, "Failed to disable Configuration Interface\n");
> > > > +		goto fail;
> > > > +	}
> > > > +
> > > > +	while (retries--) {
> > > > +		ret = sysconfig_refresh(priv);
> > > > +		if (!ret)
> > > > +			ret = sysconfig_read_status(priv, &status);
> > > > +
> > > > +		if (ret) {
> > > > +			dev_err(dev, "Failed to refresh\n");
> > > > +			break;
> > > > +		}
> > > > +
> > > > +		if (status & SYSCONFIG_STATUS_DONE &&
> > > > +		    !(status & SYSCONFIG_STATUS_BUSY) &&
> > > > +		    !(status & SYSCONFIG_STATUS_ERR))
> > > > +			return 0;
> > > > +	}
> > > > +
> > > > +fail:
> > > > +	machxo2_cleanup(priv);
> > > > +
> > > > +	return -EFAULT;
> > > > +}
> > > > +
> > > > +static const struct fpga_manager_ops machxo2_fpga_ops = {
> > > > +	.state = machxo2_ops_state,
> > > > +	.write_init = machxo2_ops_write_init,
> > > > +	.write = machxo2_ops_write,
> > > > +	.write_complete = machxo2_ops_write_complete,
> > > > +};
> > > > +
> > > > +static int machxo2_probe(struct sysconfig_priv *priv)
> > > > +{
> > > > +	struct spi_device *spi = priv->spi;
> > > > +	struct device *dev = &spi->dev;
> > > > +	struct fpga_manager *mgr;
> > > > +
> > > > +	if (spi->max_speed_hz > MACHXO2_SPI_MAX_SPEED_HZ) {
> > > > +		dev_err(dev, "SPI speed %u is too high, maximum speed is %u\n",
> > > > +			spi->max_speed_hz, MACHXO2_SPI_MAX_SPEED_HZ);
> > > > +		return -EINVAL;
> > > > +	}
> > > > +
> > > > +	priv->isc_enable_operand = MACHXO2_ISC_ENABLE_OPERAND;
> > > > +	priv->isc_erase_operand = MACHXO2_ISC_ERASE_OPERAND;
> > > > +
> > > > +	mgr = devm_fpga_mgr_register(dev, "Lattice MachXO2 SPI FPGA Manager",
> > > > +				     &machxo2_fpga_ops, priv);
> > > > +
> > > > +	return PTR_ERR_OR_ZERO(mgr);
> > > > +}
> > > > +
> > > > +typedef int (*lattice_fpga_probe_func)(struct sysconfig_priv *);
> > > > +
> > > > +static int sysconfig_probe(struct spi_device *spi)
> > > > +{
> > > > +	const struct spi_device_id *dev_id;
> > > > +	lattice_fpga_probe_func probe_func;
> > > > +	struct device *dev = &spi->dev;
> > > > +	struct sysconfig_priv *priv;
> > > > +
> > > > +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > > > +	if (!priv)
> > > > +		return -ENOMEM;
> > > > +
> > > > +	priv->spi = spi;
> > > > +
> > > > +	probe_func = of_device_get_match_data(&spi->dev);
> > > > +	if (!probe_func) {
> > > > +		dev_id = spi_get_device_id(spi);
> > > > +		if (!dev_id)
> > > > +			return -ENODEV;
> > > > +
> > > > +		probe_func = (lattice_fpga_probe_func)dev_id->driver_data;
> > > > +	}
> > > > +
> > > > +	if (!probe_func)
> > > > +		return -EINVAL;
> > > > +
> > > > +	return probe_func(priv);
> > > > +}
> > > > +
> > > > +static const struct spi_device_id sysconfig_spi_ids[] = {
> > > > +	{
> > > > +		.name = "ecp5-fpga-mgr",
> > > > +		.driver_data = (kernel_ulong_t)ecp5_probe,
> > > > +	}, {
> > > > +		.name = "machxo2-fpga-mgr",
> > > > +		.driver_data = (kernel_ulong_t)machxo2_probe,
> > > 
> > > Putting the whole probe flow in driver_data is the same as providing 2
> > > drivers. The purpose is not to put all the code in one file.
> > > 
> > 
> > Sorry, I don't understand what you suggest. Separate file for each
> > specific FPGA?
> 
> Sorry, didn't make it clear. I feel like there are still 2 board
> specific drivers although they are now in one file, they have
> separate probes, fpga manager ops, different spi APIs ...
> 
> What I expect is, the driver operates the HW according to the configured
> properties specified in sysCONFIG spec, such as having PROGRAMN gpio pin?
> burst? having backup flash? ...

Ok, got it. But since I have only ECP5 this will be extremely error
prone for other FPGAs.

> 
> Thanks,
> Yilun
> 
> > 
> > > Thanks,
> > > Yilun
> > > 
> > > > +	}, {},
> > > > +};
> > > > +MODULE_DEVICE_TABLE(spi, sysconfig_spi_ids);
> > > > +
> > > > +#if IS_ENABLED(CONFIG_OF)
> > > > +static const struct of_device_id sysconfig_of_ids[] = {
> > > > +	{
> > > > +		.compatible = "lattice,ecp5-fpga-mgr",
> > > > +		.data = ecp5_probe,
> > > > +	}, {
> > > > +		.compatible = "lattice,machxo2-fpga-mgr",
> > > > +		.data = machxo2_probe
> > > > +	}, {},
> > > > +};
> > > > +MODULE_DEVICE_TABLE(of, sysconfig_of_ids);
> > > > +#endif /* IS_ENABLED(CONFIG_OF) */
> > > > +
> > > > +static struct spi_driver lattice_sysconfig_driver = {
> > > > +	.probe = sysconfig_probe,
> > > > +	.id_table = sysconfig_spi_ids,
> > > > +	.driver = {
> > > > +		.name = "lattice_sysconfig_spi_fpga_mgr",
> > > > +		.of_match_table = of_match_ptr(sysconfig_of_ids),
> > > > +	},
> > > > +};
> > > > +
> > > > +module_spi_driver(lattice_sysconfig_driver);
> > > > +
> > > > +MODULE_DESCRIPTION("Lattice sysCONFIG Slave SPI FPGA Manager");
> > > > +MODULE_LICENSE("GPL");
> > > > -- 
> > > > 2.37.2
> > > > 
> > > > 
> > 

Re: [PATCH v8 1/2] fpga: lattice-sysconfig-spi: add Lattice sysCONFIG FPGA manager

[Ivan Bornyakov]

On Mon, Aug 29, 2022 at 02:04:10PM +0300, Ivan Bornyakov wrote:
> On Mon, Aug 29, 2022 at 06:34:44PM +0800, Xu Yilun wrote:
> > On 2022-08-29 at 11:27:40 +0300, Ivan Bornyakov wrote:
> > > On Mon, Aug 29, 2022 at 03:25:41PM +0800, Xu Yilun wrote:
> > > > On 2022-08-25 at 14:24:32 +0300, Ivan Bornyakov wrote:
> > > > > Add support to the FPGA manager for programming Lattice ECP5 and MachXO2
> > > > > FPGAs over slave SPI sysCONFIG interface.
> > > > > 
> > > > > Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
> > > > > ---
> > > > >  drivers/fpga/Kconfig                 |   7 +
> > > > >  drivers/fpga/Makefile                |   1 +
> > > > >  drivers/fpga/lattice-sysconfig-spi.c | 630 +++++++++++++++++++++++++++
> > > > >  3 files changed, 638 insertions(+)
> > > > >  create mode 100644 drivers/fpga/lattice-sysconfig-spi.c
> > > > > 
> > > > > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> > > > > index 6c416955da53..991d9d976dca 100644
> > > > > --- a/drivers/fpga/Kconfig
> > > > > +++ b/drivers/fpga/Kconfig
> > > > > @@ -263,4 +263,11 @@ config FPGA_MGR_MICROCHIP_SPI
> > > > >  	  programming over slave SPI interface with .dat formatted
> > > > >  	  bitstream image.
> > > > >  
> > > > > +config FPGA_MGR_LATTICE_SPI
> > > > > +	tristate "Lattice sysCONFIG SPI FPGA manager"
> > > > > +	depends on SPI
> > > > > +	help
> > > > > +	  FPGA manager driver support for Lattice FPGAs programming over slave
> > > > > +	  SPI sysCONFIG interface.
> > > > > +
> > > > >  endif # FPGA
> > > > > diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> > > > > index 42ae8b58abce..115dba916024 100644
> > > > > --- a/drivers/fpga/Makefile
> > > > > +++ b/drivers/fpga/Makefile
> > > > > @@ -20,6 +20,7 @@ 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
> > > > >  obj-$(CONFIG_FPGA_MGR_MICROCHIP_SPI)	+= microchip-spi.o
> > > > > +obj-$(CONFIG_FPGA_MGR_LATTICE_SPI)	+= lattice-sysconfig-spi.o
> > > > >  obj-$(CONFIG_ALTERA_PR_IP_CORE)		+= altera-pr-ip-core.o
> > > > >  obj-$(CONFIG_ALTERA_PR_IP_CORE_PLAT)	+= altera-pr-ip-core-plat.o
> > > > >  
> > > > > diff --git a/drivers/fpga/lattice-sysconfig-spi.c b/drivers/fpga/lattice-sysconfig-spi.c
> > > > > new file mode 100644
> > > > > index 000000000000..145b5b27b88d
> > > > > --- /dev/null
> > > > > +++ b/drivers/fpga/lattice-sysconfig-spi.c
> > > > > @@ -0,0 +1,630 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > +/*
> > > > > + * Lattice FPGA programming over slave SPI sysCONFIG interface.
> > > > > + */
> > > > > +
> > > > > +#include <linux/delay.h>
> > > > > +#include <linux/fpga/fpga-mgr.h>
> > > > > +#include <linux/of_device.h>
> > > > > +#include <linux/spi/spi.h>
> > > > > +
> > > > > +#define	SYSCONFIG_ISC_ENABLE		{0xC6, 0x00, 0x00, 0x00}
> > > > > +#define	SYSCONFIG_ISC_DISABLE		{0x26, 0x00, 0x00, 0x00}
> > > > > +#define	SYSCONFIG_ISC_ERASE		{0x0E, 0x00, 0x00, 0x00}
> > > > > +#define	SYSCONFIG_ISC_PROGRAM_DONE	{0x5E, 0x00, 0x00, 0x00}
> > > > > +#define	SYSCONFIG_LSC_READ_STATUS	{0x3C, 0x00, 0x00, 0x00}
> > > > > +#define	SYSCONFIG_LSC_CHECK_BUSY	{0xF0, 0x00, 0x00, 0x00}
> > > > > +#define	SYSCONFIG_LSC_REFRESH		{0x79, 0x00, 0x00, 0x00}
> > > > > +#define	SYSCONFIG_LSC_INIT_ADDR		{0x46, 0x00, 0x00, 0x00}
> > > > > +#define	SYSCONFIG_LSC_BITSTREAM_BURST	{0x7a, 0x00, 0x00, 0x00}
> > > > > +#define	SYSCONFIG_LSC_PROG_INCR_NV	{0x70, 0x00, 0x00, 0x01}
> > > > > +
> > > > > +#define	SYSCONFIG_STATUS_DONE		BIT(8)
> > > > > +#define	SYSCONFIG_STATUS_BUSY		BIT(12)
> > > > > +#define	SYSCONFIG_STATUS_FAIL		BIT(13)
> > > > > +#define	SYSCONFIG_STATUS_ERR		(BIT(23) | BIT(24) | BIT(25))
> > > > > +
> > > > > +#define	SYSCONFIG_POLL_RETRIES		100000
> > > > > +
> > > > > +#define	ECP5_SPI_MAX_SPEED_HZ		60000000
> > > > > +#define	ECP5_ISC_ERASE_OPERAND		0x01
> > > > > +
> > > > > +#define	MACHXO2_SPI_MAX_SPEED_HZ	66000000
> > > > > +#define	MACHXO2_PAGE_SIZE		16
> > > > > +#define	MACHXO2_ISC_ENABLE_OPERAND	0x08
> > > > > +#define	MACHXO2_ISC_ERASE_OPERAND	0x04
> > > > 
> > > > You need to deliver the meaning of each operand, rather than just point
> > > > out this is for machxo2, that is for esp5. I assume the sysCONFIG will
> > > > not deliberately design different commands with the exact same
> > > > functionality for different boards.
> > > > 
> > > > We should have a set of common configurations with finer granularity that
> > > > board specific fpga manager device could select from, rather than create
> > > > more and more similar macros for each new board.
> > > > 
> > > > Back to the 2 operands, what's the meaning of 0x1 & 0x4 for erase,
> > > 
> > > 0x1 - erase SRAM, 0x4 - erase FLASH. I'll redefine them properly in v9
> > > 
> > > > also 0x0 & 0x8 for enable?
> > > > 
> > > 
> > > Their meaning is not documented. It's just 0x08 for MachXO2 and 0x00 for
> > > ECP5.
> > > 
> > > > > +
> > > > > +struct sysconfig_priv {
> > > > > +	struct gpio_desc *program;
> > > > > +	struct gpio_desc *init;
> > > > > +	struct gpio_desc *done;
> > > > > +	struct spi_device *spi;
> > > > > +	u8 isc_enable_operand;
> > > > > +	u8 isc_erase_operand;
> > > > > +};
> > > > > +
> > > > > +static int sysconfig_poll_busy(struct sysconfig_priv *data)
> > > > > +{
> > > > > +	const u8 lsc_check_busy[] = SYSCONFIG_LSC_CHECK_BUSY;
> > > > > +	int ret, retries = SYSCONFIG_POLL_RETRIES;
> > > > > +	u8 busy;
> > > > > +
> > > > > +	while (retries--) {
> > > > > +		ret = spi_write_then_read(data->spi,
> > > > > +					  lsc_check_busy, sizeof(lsc_check_busy),
> > > > > +					  &busy, sizeof(busy));
> > > > > +		if (ret)
> > > > > +			return ret;
> > > > > +
> > > > > +		if (!busy)
> > > > > +			return 0;
> > > > > +
> > > > > +		usleep_range(50, 100);
> > > > > +	}
> > > > > +
> > > > > +	return -EBUSY;
> > > > > +}
> > > > > +
> > > > > +static int sysconfig_read_status(struct sysconfig_priv *data, u32 *status)
> > > > > +{
> > > > > +	const u8 lsc_read_status[] = SYSCONFIG_LSC_READ_STATUS;
> > > > > +	__be32 device_status;
> > > > > +	int ret;
> > > > > +
> > > > > +	ret = spi_write_then_read(data->spi,
> > > > > +				  lsc_read_status, sizeof(lsc_read_status),
> > > > > +				  &device_status, sizeof(device_status));
> > > > > +	if (ret)
> > > > > +		return ret;
> > > > > +
> > > > > +	*status = be32_to_cpu(device_status);
> > > > > +
> > > > > +	return 0;
> > > > > +}
> > > > > +
> > > > > +static int sysconfig_poll_status(struct sysconfig_priv *data, u32 *status)
> > > > > +{
> > > > > +	int ret;
> > > > > +
> > > > > +	ret = sysconfig_poll_busy(data);
> > > > > +	if (ret)
> > > > > +		return ret;
> > > > > +
> > > > > +	return sysconfig_read_status(data, status);
> > > > > +}
> > > > > +
> > > > > +static int sysconfig_poll_gpio(struct gpio_desc *gpio, bool is_active)
> > > > > +{
> > > > > +	int value, retries = SYSCONFIG_POLL_RETRIES;
> > > > > +
> > > > > +	while (retries--) {
> > > > > +		value = gpiod_get_value(gpio);
> > > > > +		if (value < 0)
> > > > > +			return value;
> > > > > +
> > > > > +		if ((!is_active && !value) || (is_active && value))
> > > > > +			return 0;
> > > > > +
> > > > > +		ndelay(10);
> > > > > +	}
> > > > > +
> > > > > +	return -ETIMEDOUT;
> > > > > +}
> > > > > +
> > > > > +static int sysconfig_refresh(struct sysconfig_priv *data)
> > > > > +{
> > > > > +	static const u8 lsc_refresh[] = SYSCONFIG_LSC_REFRESH;
> > > > > +	int ret;
> > > > > +
> > > > > +	ret = spi_write(data->spi, lsc_refresh, sizeof(lsc_refresh));
> > > > > +	if (ret)
> > > > > +		return ret;
> > > > > +
> > > > > +	usleep_range(4000, 8000);
> > > > > +
> > > > > +	return 0;
> > > > > +}
> > > > > +
> > > > > +static int sysconfig_isc_enable(struct sysconfig_priv *data)
> > > > > +{
> > > > > +	u8 isc_enable[] = SYSCONFIG_ISC_ENABLE;
> > > > > +	u32 status;
> > > > > +	int ret;
> > > > > +
> > > > > +	isc_enable[1] = data->isc_enable_operand;
> > > > > +
> > > > > +	ret = spi_write(data->spi, isc_enable, sizeof(isc_enable));
> > > > > +	if (ret)
> > > > > +		return ret;
> > > > > +
> > > > > +	ret = sysconfig_poll_status(data, &status);
> > > > > +	if (ret || (status & SYSCONFIG_STATUS_FAIL))
> > > > > +		return ret ? : -EFAULT;
> > > > > +
> > > > > +	return 0;
> > > > > +}
> > > > > +
> > > > > +static int sysconfig_isc_erase(struct sysconfig_priv *data)
> > > > > +{
> > > > > +	u8 isc_erase[] = SYSCONFIG_ISC_ERASE;
> > > > > +	u32 status;
> > > > > +	int ret;
> > > > > +
> > > > > +	isc_erase[1] = data->isc_erase_operand;
> > > > > +
> > > > > +	ret = spi_write(data->spi, isc_erase, sizeof(isc_erase));
> > > > > +	if (ret)
> > > > > +		return ret;
> > > > > +
> > > > > +	ret = sysconfig_poll_status(data, &status);
> > > > > +	if (ret || (status & SYSCONFIG_STATUS_FAIL))
> > > > > +		return ret ? : -EFAULT;
> > > > > +
> > > > > +	return 0;
> > > > > +}
> > > > > +
> > > > > +static int sysconfig_isc_init(struct sysconfig_priv *data)
> > > > > +{
> > > > > +	int ret;
> > > > > +
> > > > > +	ret = sysconfig_isc_enable(data);
> > > > > +	if (ret)
> > > > > +		return ret;
> > > > > +
> > > > > +	return sysconfig_isc_erase(data);
> > > > > +}
> > > > > +
> > > > > +static int sysconfig_lsc_init_addr(struct sysconfig_priv *data)
> > > > > +{
> > > > > +	const u8 lsc_init_addr[] = SYSCONFIG_LSC_INIT_ADDR;
> > > > > +
> > > > > +	return spi_write(data->spi, lsc_init_addr, sizeof(lsc_init_addr));
> > > > > +}
> > > > > +
> > > > > +static int sysconfig_lsc_bitstream_burst(struct sysconfig_priv *data)
> > > > > +{
> > > > > +	const u8 lsc_bitstream_burst[] = SYSCONFIG_LSC_BITSTREAM_BURST;
> > > > > +	struct spi_transfer xfer = {
> > > > > +		.tx_buf = lsc_bitstream_burst,
> > > > > +		.len = sizeof(lsc_bitstream_burst),
> > > > > +		.cs_change = 1,
> > > > > +	};
> > > > > +	struct spi_message msg;
> > > > > +
> > > > > +	spi_message_init_with_transfers(&msg, &xfer, 1);
> > > > > +
> > > > > +	return spi_sync_locked(data->spi, &msg);
> > > > > +}
> > > > > +
> > > > > +static int sysconfig_isc_prog_done(struct sysconfig_priv *data)
> > > > > +{
> > > > > +	const u8 isc_prog_done[] = SYSCONFIG_ISC_PROGRAM_DONE;
> > > > > +	u32 status;
> > > > > +	int ret;
> > > > > +
> > > > > +	ret = spi_write(data->spi, isc_prog_done, sizeof(isc_prog_done));
> > > > > +	if (ret)
> > > > > +		return ret;
> > > > > +
> > > > > +	ret = sysconfig_poll_status(data, &status);
> > > > > +	if (ret)
> > > > > +		return ret;
> > > > > +
> > > > > +	if (status & SYSCONFIG_STATUS_DONE)
> > > > > +		return 0;
> > > > > +
> > > > > +	return -EFAULT;
> > > > > +}
> > > > > +
> > > > > +static int sysconfig_isc_disable(struct sysconfig_priv *data)
> > > > > +{
> > > > > +	const u8 isc_disable[] = SYSCONFIG_ISC_DISABLE;
> > > > > +
> > > > > +	return spi_write(data->spi, isc_disable, sizeof(isc_disable));
> > > > > +}
> > > > > +
> > > > > +static enum fpga_mgr_states ecp5_ops_state(struct fpga_manager *mgr)
> > > > > +{
> > > > > +	struct sysconfig_priv *priv = mgr->priv;
> > > > > +
> > > > > +	return (gpiod_get_value(priv->done) > 0) ? FPGA_MGR_STATE_OPERATING :
> > > > > +						   FPGA_MGR_STATE_UNKNOWN;
> > > > > +}
> > > > > +
> > > > > +static int ecp5_ops_write_init(struct fpga_manager *mgr,
> > > > > +			       struct fpga_image_info *info,
> > > > > +			       const char *buf, size_t count)
> > > > > +{
> > > > > +	struct sysconfig_priv *priv = mgr->priv;
> > > > > +	struct spi_device *spi = priv->spi;
> > > > > +	struct device *dev = &mgr->dev;
> > > > > +	struct gpio_desc *program;
> > > > > +	struct gpio_desc *init;
> > > > > +	struct gpio_desc *done;
> > > > > +	int ret;
> > > > > +
> > > > > +	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
> > > > > +		dev_err(dev, "Partial reconfiguration is not supported\n");
> > > > > +		return -EOPNOTSUPP;
> > > > > +	}
> > > > > +
> > > > > +	program = priv->program;
> > > > > +	init = priv->init;
> > > > > +	done = priv->done;
> > > > > +
> > > > > +	/* Enter init mode */
> > > > > +	gpiod_set_value(program, 1);
> > > > 
> > > > Same concern, provide gpio or command options for all board specific
> > > > fpga manager to select. 
> > > > 
> > > > I'll not list each features one by one below.
> > > > 
> > > > some more comments below.
> > > > 
> > > > > +
> > > > > +	ret = sysconfig_poll_gpio(init, true);
> > > > > +	if (!ret)
> > > > > +		ret = sysconfig_poll_gpio(done, false);
> > > > > +
> > > > > +	if (ret) {
> > > > > +		dev_err(dev, "Failed to go to init mode\n");
> > > > > +		return ret;
> > > > > +	}
> > > > > +
> > > > > +	/* Enter program mode */
> > > > > +	gpiod_set_value(program, 0);
> > > > > +
> > > > > +	ret = sysconfig_poll_gpio(init, false);
> > > > > +	if (ret) {
> > > > > +		dev_err(dev, "Failed to go to program mode\n");
> > > > > +		return ret;
> > > > > +	}
> > > > > +
> > > > > +	/* Enter ISC mode */
> > > > > +	ret = sysconfig_isc_init(priv);
> > > > > +	if (ret) {
> > > > > +		dev_err(dev, "Failed to go to ISC mode\n");
> > > > > +		return ret;
> > > > > +	}
> > > > > +
> > > > > +	/* Initialize the Address Shift Register */
> > > > > +	ret = sysconfig_lsc_init_addr(priv);
> > > > > +	if (ret) {
> > > > > +		dev_err(dev,
> > > > > +			"Failed to initialize the Address Shift Register\n");
> > > > > +		return ret;
> > > > > +	}
> > > > > +
> > > > > +	/*
> > > > > +	 * Lock SPI bus for exclusive usage until FPGA programming is done.
> > > > > +	 * SPI bus will be released in ecp5_ops_write_complete() or on error.
> > > > > +	 */
> > > > > +	spi_bus_lock(spi->controller);
> > > > > +
> > > > > +	/* Prepare for bitstream burst write */
> > > > > +	ret = sysconfig_lsc_bitstream_burst(priv);
> > > > > +	if (ret) {
> > > > > +		dev_err(dev, "Failed to prepare for bitstream burst write\n");
> > > > > +		spi_bus_unlock(spi->controller);
> > > > > +	}
> > > > > +
> > > > > +	return ret;
> > > > > +}
> > > > > +
> > > > > +static int ecp5_ops_write(struct fpga_manager *mgr, const char *buf, size_t count)
> > > > > +{
> > > > > +	struct sysconfig_priv *priv = mgr->priv;
> > > > > +	struct spi_device *spi = priv->spi;
> > > > > +	struct spi_transfer xfer = {
> > > > > +		.tx_buf = buf,
> > > > > +		.len = count,
> > > > > +		.cs_change = 1,
> > > > > +	};
> > > > > +	struct spi_message msg;
> > > > > +	int ret;
> > > > > +
> > > > > +	spi_message_init_with_transfers(&msg, &xfer, 1);
> > > > > +	ret = spi_sync_locked(spi, &msg);
> > > > > +	if (ret)
> > > > > +		spi_bus_unlock(spi->controller);
> > > > > +
> > > > > +	return ret;
> > > > > +}
> > > > > +
> > > > > +static int ecp5_ops_write_complete(struct fpga_manager *mgr,
> > > > > +				   struct fpga_image_info *info)
> > > > > +{
> > > > > +	struct sysconfig_priv *priv = mgr->priv;
> > > > > +	struct spi_device *spi = priv->spi;
> > > > > +	struct device *dev = &mgr->dev;
> > > > > +	int ret;
> > > > > +
> > > > > +	/* Bitstream burst write is done, release SPI bus */
> > > > > +	spi_bus_unlock(spi->controller);
> > > > > +
> > > > > +	/* Toggle CS and wait for bitstream write to finish */
> > > > > +	ret = spi_write(spi, NULL, 0);
> > > > > +	if (!ret)
> > > > > +		ret = sysconfig_poll_busy(priv);
> > > > > +
> > > > > +	if (ret) {
> > > > > +		dev_err(dev, "Error while waiting bitstream write to finish\n");
> > > > > +		return ret;
> > > > > +	}
> > > > > +
> > > > > +	/* Exit ISC mode */
> > > > > +	ret = sysconfig_isc_disable(priv);
> > > > > +	if (!ret)
> > > > > +		ret = sysconfig_poll_gpio(priv->done, true);
> > > > > +
> > > > > +	if (ret)
> > > > > +		dev_err(dev, "Failed to finish ISC\n");
> > > > > +
> > > > > +	return ret;
> > > > > +}
> > > > > +
> > > > > +static const struct fpga_manager_ops ecp5_fpga_ops = {
> > > > > +	.state = ecp5_ops_state,
> > > > > +	.write_init = ecp5_ops_write_init,
> > > > > +	.write = ecp5_ops_write,
> > > > > +	.write_complete = ecp5_ops_write_complete,
> > > > > +};
> > > > > +
> > > > > +static int ecp5_probe(struct sysconfig_priv *priv)
> > > > > +{
> > > > > +	struct spi_device *spi = priv->spi;
> > > > > +	struct device *dev = &spi->dev;
> > > > > +	struct fpga_manager *mgr;
> > > > > +	int ret;
> > > > > +
> > > > > +	if (spi->max_speed_hz > ECP5_SPI_MAX_SPEED_HZ) {
> > > > > +		dev_err(dev, "SPI speed %u is too high, maximum speed is %u\n",
> > > > > +			spi->max_speed_hz, ECP5_SPI_MAX_SPEED_HZ);
> > > > > +		return -EINVAL;
> > > > > +	}
> > > > > +
> > > > > +	priv->isc_erase_operand = ECP5_ISC_ERASE_OPERAND;
> > > > > +
> > > > > +	priv->done = devm_gpiod_get(dev, "done", GPIOD_IN);
> > > > > +	if (IS_ERR(priv->done)) {
> > > > > +		ret = PTR_ERR(priv->done);
> > > > > +		dev_err(dev, "Failed to get DONE GPIO: %d\n", ret);
> > > > > +		return ret;
> > > > > +	}
> > > > > +
> > > > > +	priv->init = devm_gpiod_get(dev, "init", GPIOD_IN);
> > > > > +	if (IS_ERR(priv->init)) {
> > > > > +		ret = PTR_ERR(priv->init);
> > > > > +		dev_err(dev, "Failed to get INIT GPIO: %d\n", ret);
> > > > > +		return ret;
> > > > > +	}
> > > > > +
> > > > > +	priv->program = devm_gpiod_get(dev, "program", GPIOD_OUT_LOW);
> > > > > +	if (IS_ERR(priv->program)) {
> > > > > +		ret = PTR_ERR(priv->program);
> > > > > +		dev_err(dev, "Failed to get PROGRAM GPIO: %d\n", ret);
> > > > > +		return ret;
> > > > > +	}
> > > > > +
> > > > > +	mgr = devm_fpga_mgr_register(dev, "Lattice ECP5 SPI FPGA Manager",
> > > > > +				     &ecp5_fpga_ops, priv);
> > > > > +
> > > > > +	return PTR_ERR_OR_ZERO(mgr);
> > > > > +}
> > > > > +
> > > > > +static enum fpga_mgr_states machxo2_ops_state(struct fpga_manager *mgr)
> > > > > +{
> > > > > +	struct sysconfig_priv *priv = mgr->priv;
> > > > > +	u32 status;
> > > > > +	int ret;
> > > > > +
> > > > > +	ret = sysconfig_read_status(priv, &status);
> > > > > +	if (ret || !(status & SYSCONFIG_STATUS_DONE))
> > > > > +		return FPGA_MGR_STATE_UNKNOWN;
> > > > > +
> > > > > +	return FPGA_MGR_STATE_OPERATING;
> > > > > +}
> > > > > +
> > > > > +static int machxo2_ops_write_init(struct fpga_manager *mgr,
> > > > > +				  struct fpga_image_info *info,
> > > > > +				  const char *buf, size_t count)
> > > > > +{
> > > > > +	struct sysconfig_priv *priv = mgr->priv;
> > > > > +	struct device *dev = &mgr->dev;
> > > > > +	int ret;
> > > > > +
> > > > > +	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
> > > > > +		dev_err(dev, "Partial reconfiguration is not supported\n");
> > > > > +		return -EOPNOTSUPP;
> > > > > +	}
> > > > > +
> > > > > +	/* Enter ISC mode */
> > > > > +	ret = sysconfig_isc_init(priv);
> > > > > +	if (ret) {
> > > > > +		dev_err(dev, "Failed to go to ISC mode\n");
> > > > > +		return ret;
> > > > > +	}
> > > > > +
> > > > > +	/* Initialize the Address Shift Register */
> > > > > +	ret = sysconfig_lsc_init_addr(priv);
> > > > > +	if (ret)
> > > > > +		dev_err(dev,
> > > > > +			"Failed to initialize the Address Shift Register\n");
> > > > > +
> > > > > +	return ret;
> > > > > +}
> > > > > +
> > > > > +static int machxo2_ops_write(struct fpga_manager *mgr, const char *buf, size_t count)
> > > > > +{
> > > > > +	const u8 lsc_progincr[] = SYSCONFIG_LSC_PROG_INCR_NV;
> > > > > +	struct sysconfig_priv *priv = mgr->priv;
> > > > > +	struct device *dev = &mgr->dev;
> > > > > +	struct spi_transfer xfers[2] = {
> > > > > +		{
> > > > > +			.tx_buf = lsc_progincr,
> > > > > +			.len = sizeof(lsc_progincr),
> > > > > +		}, {
> > > > > +			.len = MACHXO2_PAGE_SIZE,
> > > > > +		},
> > > > > +	};
> > > > > +	size_t i;
> > > > > +	int ret;
> > > > > +
> > > > > +	if (count % MACHXO2_PAGE_SIZE) {
> > > > > +		dev_err(dev, "Malformed payload.\n");
> > > > > +		return -EINVAL;
> > > > > +	}
> > > > > +
> > > > > +	for (i = 0; i < count; i += MACHXO2_PAGE_SIZE) {
> > > > > +		xfers[1].tx_buf = buf + i;
> > > > > +
> > > > > +		ret = spi_sync_transfer(priv->spi, xfers, 2);
> > > > > +		if (!ret)
> > > > > +			ret = sysconfig_poll_busy(priv);
> > > > > +
> > > > > +		if (ret) {
> > > > > +			dev_err(dev, "Failed to write frame %zu of %zu\n",
> > > > > +				i / MACHXO2_PAGE_SIZE, count / MACHXO2_PAGE_SIZE);
> > > > > +			return ret;
> > > > > +		}
> > > > > +	}
> > > > > +
> > > > > +	return 0;
> > > > > +}
> > > > > +
> > > > > +static void machxo2_cleanup(struct sysconfig_priv *data)
> > > > > +{
> > > > > +	sysconfig_isc_erase(data);
> > > > > +	sysconfig_refresh(data);
> > > > > +}
> > > > > +
> > > > > +static int machxo2_ops_write_complete(struct fpga_manager *mgr,
> > > > > +				      struct fpga_image_info *info)
> > > > > +{
> > > > > +	int ret, retries = SYSCONFIG_POLL_RETRIES;
> > > > > +	struct sysconfig_priv *priv = mgr->priv;
> > > > > +	struct device *dev = &mgr->dev;
> > > > > +	u32 status;
> > > > > +
> > > > > +	ret = sysconfig_isc_prog_done(priv);
> > > > > +	if (ret) {
> > > > > +		dev_err(dev, "Failed to enable Self-Download Mode\n");
> > > > > +		goto fail;
> > > > > +	}
> > > > > +
> > > > > +	ret = sysconfig_isc_disable(priv);
> > > > > +	if (ret) {
> > > > > +		dev_err(dev, "Failed to disable Configuration Interface\n");
> > > > > +		goto fail;
> > > > > +	}
> > > > > +
> > > > > +	while (retries--) {
> > > > > +		ret = sysconfig_refresh(priv);
> > > > > +		if (!ret)
> > > > > +			ret = sysconfig_read_status(priv, &status);
> > > > > +
> > > > > +		if (ret) {
> > > > > +			dev_err(dev, "Failed to refresh\n");
> > > > > +			break;
> > > > > +		}
> > > > > +
> > > > > +		if (status & SYSCONFIG_STATUS_DONE &&
> > > > > +		    !(status & SYSCONFIG_STATUS_BUSY) &&
> > > > > +		    !(status & SYSCONFIG_STATUS_ERR))
> > > > > +			return 0;
> > > > > +	}
> > > > > +
> > > > > +fail:
> > > > > +	machxo2_cleanup(priv);
> > > > > +
> > > > > +	return -EFAULT;
> > > > > +}
> > > > > +
> > > > > +static const struct fpga_manager_ops machxo2_fpga_ops = {
> > > > > +	.state = machxo2_ops_state,
> > > > > +	.write_init = machxo2_ops_write_init,
> > > > > +	.write = machxo2_ops_write,
> > > > > +	.write_complete = machxo2_ops_write_complete,
> > > > > +};
> > > > > +
> > > > > +static int machxo2_probe(struct sysconfig_priv *priv)
> > > > > +{
> > > > > +	struct spi_device *spi = priv->spi;
> > > > > +	struct device *dev = &spi->dev;
> > > > > +	struct fpga_manager *mgr;
> > > > > +
> > > > > +	if (spi->max_speed_hz > MACHXO2_SPI_MAX_SPEED_HZ) {
> > > > > +		dev_err(dev, "SPI speed %u is too high, maximum speed is %u\n",
> > > > > +			spi->max_speed_hz, MACHXO2_SPI_MAX_SPEED_HZ);
> > > > > +		return -EINVAL;
> > > > > +	}
> > > > > +
> > > > > +	priv->isc_enable_operand = MACHXO2_ISC_ENABLE_OPERAND;
> > > > > +	priv->isc_erase_operand = MACHXO2_ISC_ERASE_OPERAND;
> > > > > +
> > > > > +	mgr = devm_fpga_mgr_register(dev, "Lattice MachXO2 SPI FPGA Manager",
> > > > > +				     &machxo2_fpga_ops, priv);
> > > > > +
> > > > > +	return PTR_ERR_OR_ZERO(mgr);
> > > > > +}
> > > > > +
> > > > > +typedef int (*lattice_fpga_probe_func)(struct sysconfig_priv *);
> > > > > +
> > > > > +static int sysconfig_probe(struct spi_device *spi)
> > > > > +{
> > > > > +	const struct spi_device_id *dev_id;
> > > > > +	lattice_fpga_probe_func probe_func;
> > > > > +	struct device *dev = &spi->dev;
> > > > > +	struct sysconfig_priv *priv;
> > > > > +
> > > > > +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > > > > +	if (!priv)
> > > > > +		return -ENOMEM;
> > > > > +
> > > > > +	priv->spi = spi;
> > > > > +
> > > > > +	probe_func = of_device_get_match_data(&spi->dev);
> > > > > +	if (!probe_func) {
> > > > > +		dev_id = spi_get_device_id(spi);
> > > > > +		if (!dev_id)
> > > > > +			return -ENODEV;
> > > > > +
> > > > > +		probe_func = (lattice_fpga_probe_func)dev_id->driver_data;
> > > > > +	}
> > > > > +
> > > > > +	if (!probe_func)
> > > > > +		return -EINVAL;
> > > > > +
> > > > > +	return probe_func(priv);
> > > > > +}
> > > > > +
> > > > > +static const struct spi_device_id sysconfig_spi_ids[] = {
> > > > > +	{
> > > > > +		.name = "ecp5-fpga-mgr",
> > > > > +		.driver_data = (kernel_ulong_t)ecp5_probe,
> > > > > +	}, {
> > > > > +		.name = "machxo2-fpga-mgr",
> > > > > +		.driver_data = (kernel_ulong_t)machxo2_probe,
> > > > 
> > > > Putting the whole probe flow in driver_data is the same as providing 2
> > > > drivers. The purpose is not to put all the code in one file.
> > > > 
> > > 
> > > Sorry, I don't understand what you suggest. Separate file for each
> > > specific FPGA?
> > 
> > Sorry, didn't make it clear. I feel like there are still 2 board
> > specific drivers although they are now in one file, they have
> > separate probes, fpga manager ops, different spi APIs ...
> > 
> > What I expect is, the driver operates the HW according to the configured
> > properties specified in sysCONFIG spec, such as having PROGRAMN gpio pin?
> > burst? having backup flash? ...
> 
> Ok, got it. But since I have only ECP5 this will be extremely error
> prone for other FPGAs.
> 

Also there is no single sysCONFIG spec, but each specific FPGA have its
sysCONFIG description in its datasheet. Thus this path might be a
slippery slope.

> > 
> > Thanks,
> > Yilun
> > 
> > > 
> > > > Thanks,
> > > > Yilun
> > > > 
> > > > > +	}, {},
> > > > > +};
> > > > > +MODULE_DEVICE_TABLE(spi, sysconfig_spi_ids);
> > > > > +
> > > > > +#if IS_ENABLED(CONFIG_OF)
> > > > > +static const struct of_device_id sysconfig_of_ids[] = {
> > > > > +	{
> > > > > +		.compatible = "lattice,ecp5-fpga-mgr",
> > > > > +		.data = ecp5_probe,
> > > > > +	}, {
> > > > > +		.compatible = "lattice,machxo2-fpga-mgr",
> > > > > +		.data = machxo2_probe
> > > > > +	}, {},
> > > > > +};
> > > > > +MODULE_DEVICE_TABLE(of, sysconfig_of_ids);
> > > > > +#endif /* IS_ENABLED(CONFIG_OF) */
> > > > > +
> > > > > +static struct spi_driver lattice_sysconfig_driver = {
> > > > > +	.probe = sysconfig_probe,
> > > > > +	.id_table = sysconfig_spi_ids,
> > > > > +	.driver = {
> > > > > +		.name = "lattice_sysconfig_spi_fpga_mgr",
> > > > > +		.of_match_table = of_match_ptr(sysconfig_of_ids),
> > > > > +	},
> > > > > +};
> > > > > +
> > > > > +module_spi_driver(lattice_sysconfig_driver);
> > > > > +
> > > > > +MODULE_DESCRIPTION("Lattice sysCONFIG Slave SPI FPGA Manager");
> > > > > +MODULE_LICENSE("GPL");
> > > > > -- 
> > > > > 2.37.2
> > > > > 
> > > > > 
> > >