[PATCH 0/2] FPGA: TS-7300 FPGA manager

[PATCH 0/2] FPGA: TS-7300 FPGA manager

[Florian Fainelli]

Hi all,

This patch series adds support for loading bitstreams into the Altera Cyclone II
connected to an EP9302 on a TS-7300 board.

Florian Fainelli (1):
  ARM: ep93xx: Register ts73xx-fpga manager driver for TS-7300
  FPGA: Add TS-7300 FPGA manager

 drivers/fpga/Kconfig       |   7 ++
 drivers/fpga/Makefile      |   1 +
 drivers/fpga/ts73xx-fpga.c | 165 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/mfd/Kconfig        |   7 ++
 4 files changed, 180 insertions(+)
 create mode 100644 drivers/fpga/ts73xx-fpga.c

-- 
2.9.3

[PATCH 1/2] ARM: ep93xx: Register ts73xx-fpga manager driver for TS-7300

[Florian Fainelli]

Register the TS-7300 FPGA manager device drivers which allows us to load
bitstreams into the on-board Altera Cyclone II FPGA.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 arch/arm/mach-ep93xx/ts72xx.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/arch/arm/mach-ep93xx/ts72xx.c b/arch/arm/mach-ep93xx/ts72xx.c
index 3b39ea353d30..acf72ea670ef 100644
--- a/arch/arm/mach-ep93xx/ts72xx.c
+++ b/arch/arm/mach-ep93xx/ts72xx.c
@@ -230,6 +230,28 @@ static struct ep93xx_eth_data __initdata ts72xx_eth_data = {
 	.phy_id		= 1,
 };
 
+#if IS_ENABLED(CONFIG_FPGA_MGR_TS73XX)
+
+/* Relative to EP93XX_CS1_PHYS_BASE */
+#define TS73XX_FPGA_LOADER_BASE		0x03c00000
+
+static struct resource ts73xx_fpga_resources[] = {
+	{
+		.start	= EP93XX_CS1_PHYS_BASE + TS73XX_FPGA_LOADER_BASE,
+		.end	= EP93XX_CS1_PHYS_BASE + TS73XX_FPGA_LOADER_BASE + 1,
+		.flags	= IORESOURCE_MEM,
+	},
+};
+
+static struct platform_device ts73xx_fpga_device = {
+	.name	= "ts73xx-fpga-mgr",
+	.id	= -1,
+	.resource = ts73xx_fpga_resources,
+	.num_resources = ARRAY_SIZE(ts73xx_fpga_resources),
+};
+
+#endif
+
 static void __init ts72xx_init_machine(void)
 {
 	ep93xx_init_devices();
@@ -238,6 +260,10 @@ static void __init ts72xx_init_machine(void)
 	platform_device_register(&ts72xx_wdt_device);
 
 	ep93xx_register_eth(&ts72xx_eth_data, 1);
+#if IS_ENABLED(CONFIG_FPGA_MGR_TS73XX)
+	if (board_is_ts7300())
+		platform_device_register(&ts73xx_fpga_device);
+#endif
 }
 
 MACHINE_START(TS72XX, "Technologic Systems TS-72xx SBC")
-- 
2.9.3

[PATCH 2/2] FPGA: Add TS-7300 FPGA manager

[Florian Fainelli]

Add support for loading bitstreams on the Altera Cyclone II FPGA
populated on the TS-7300 board. This is done through the configuration
and data registers offered through a memory interface between the EP93xx
SoC and the FPGA.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/fpga/Kconfig       |   7 ++
 drivers/fpga/Makefile      |   1 +
 drivers/fpga/ts73xx-fpga.c | 165 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 173 insertions(+)
 create mode 100644 drivers/fpga/ts73xx-fpga.c

diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index cd84934774cc..109625707ef0 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -26,6 +26,13 @@ config FPGA_MGR_ZYNQ_FPGA
 	help
 	  FPGA manager driver support for Xilinx Zynq FPGAs.
 
+config FPGA_MGR_TS73XX
+	tristate "Technologic Systems TS-73xx SBC FPGA Manager"
+	depends on ARCH_EP93XX && MACH_TS72XX
+	help
+	  FPGA manager driver support for the Altera Cyclone II FPGA
+	  present on the TS-73xx SBC boards.
+
 endif # FPGA
 
 endmenu
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index 8d83fc6b1613..5d51265cc1b4 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -8,3 +8,4 @@ obj-$(CONFIG_FPGA)			+= fpga-mgr.o
 # FPGA Manager Drivers
 obj-$(CONFIG_FPGA_MGR_SOCFPGA)		+= socfpga.o
 obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA)	+= zynq-fpga.o
+obj-$(CONFIG_FPGA_MGR_TS73XX)		+= ts73xx-fpga.o
diff --git a/drivers/fpga/ts73xx-fpga.c b/drivers/fpga/ts73xx-fpga.c
new file mode 100644
index 000000000000..2b3d5d668dfc
--- /dev/null
+++ b/drivers/fpga/ts73xx-fpga.c
@@ -0,0 +1,165 @@
+/*
+ * Technologic Systems TS-73xx SBC FPGA loader
+ *
+ * Copyright (C) 2016 Florian Fainelli <f.fainelli@gmail.com>
+ *
+ * FPGA Manager Driver for the on-board Altera Cyclone II FPGA found on
+ * TS-7300, heavily based on load_fpga.c in their vendor tree.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/string.h>
+#include <linux/bitrev.h>
+#include <linux/fpga/fpga-mgr.h>
+
+#define TS73XX_FPGA_DATA_REG	0
+#define TS73XX_FPGA_CONFIG_REG	1
+
+struct ts73xx_fpga_priv {
+	void __iomem	*io_base;
+	struct device	*dev;
+};
+
+static enum fpga_mgr_states ts73xx_fpga_state(struct fpga_manager *mgr)
+{
+	return FPGA_MGR_STATE_UNKNOWN;
+}
+
+static int ts73xx_fpga_write_init(struct fpga_manager *mgr, u32 flags,
+				  const char *buf, size_t count)
+{
+	struct ts73xx_fpga_priv *priv = mgr->priv;
+
+	/* Reset the FPGA */
+	writeb(0, priv->io_base + TS73XX_FPGA_CONFIG_REG);
+	udelay(30);
+	writeb(0x2, priv->io_base + TS73XX_FPGA_CONFIG_REG);
+	udelay(80);
+
+	return 0;
+}
+
+static inline int ts73xx_fpga_can_write(struct ts73xx_fpga_priv *priv)
+{
+	unsigned int timeout = 1000;
+	u8 reg;
+
+	while (timeout--) {
+		reg = readb(priv->io_base + TS73XX_FPGA_CONFIG_REG);
+		if (!(reg & 0x1))
+			return 0;
+		cpu_relax();
+	}
+
+	return -ETIMEDOUT;
+}
+
+static int ts73xx_fpga_write(struct fpga_manager *mgr, const char *buf,
+			     size_t count)
+{
+	struct ts73xx_fpga_priv *priv = mgr->priv;
+	size_t i = 0;
+	int ret;
+	u8 reg;
+
+	while (count--) {
+		ret = ts73xx_fpga_can_write(priv);
+		if (ret < 0)
+			return ret;
+
+		writeb(buf[i], priv->io_base + TS73XX_FPGA_DATA_REG);
+		i++;
+	}
+
+	usleep_range(1000, 2000);
+	reg = readb(priv->io_base + TS73XX_FPGA_CONFIG_REG);
+	reg |= 0x8;
+	writeb(reg, priv->io_base + TS73XX_FPGA_CONFIG_REG);
+	usleep_range(1000, 2000);
+
+	reg = readb(priv->io_base + TS73XX_FPGA_CONFIG_REG);
+	reg &= ~0x8;
+	writeb(reg, priv->io_base + TS73XX_FPGA_CONFIG_REG);
+
+	return 0;
+}
+
+static int ts73xx_fpga_write_complete(struct fpga_manager *mgr, u32 flags)
+{
+	struct ts73xx_fpga_priv *priv = mgr->priv;
+	u8 reg;
+
+	reg = readb(priv->io_base + TS73XX_FPGA_CONFIG_REG);
+	if ((reg & 0x4) != 0x4)
+		return -ETIMEDOUT;
+
+	return 0;
+}
+
+static const struct fpga_manager_ops ts73xx_fpga_ops = {
+	.state		= ts73xx_fpga_state,
+	.write_init	= ts73xx_fpga_write_init,
+	.write		= ts73xx_fpga_write,
+	.write_complete	= ts73xx_fpga_write_complete,
+};
+
+static int ts73xx_fpga_probe(struct platform_device *pdev)
+{
+	struct device *kdev = &pdev->dev;
+	struct ts73xx_fpga_priv *priv;
+	struct fpga_manager *mgr;
+	struct resource *res;
+	int err;
+
+	priv = devm_kzalloc(kdev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = kdev;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	priv->io_base = devm_ioremap_resource(kdev, res);
+	if (IS_ERR(priv->io_base))
+		return PTR_ERR(priv->io_base);
+
+	err = fpga_mgr_register(kdev, "TS-73xx FPGA Manager",
+				&ts73xx_fpga_ops, priv);
+	if (err) {
+		dev_err(kdev, "failed to register FPGA manager\n");
+		return err;
+	}
+
+	return err;
+}
+
+static int ts73xx_fpga_remove(struct platform_device *pdev)
+{
+	fpga_mgr_unregister(&pdev->dev);
+
+	return 0;
+}
+
+static struct platform_driver ts73xx_fpga_driver = {
+	.driver	= {
+		.name	= "ts73xx-fpga-mgr",
+	},
+	.probe	= ts73xx_fpga_probe,
+	.remove	= ts73xx_fpga_remove,
+};
+module_platform_driver(ts73xx_fpga_driver);
+
+MODULE_AUTHOR("Florian Fainelli <f.fainelli@gmail.com>");
+MODULE_DESCRIPTION("TS-73xx FPGA Manager driver");
+MODULE_LICENSE("GPL v2");
-- 
2.9.3

Re: [PATCH 2/2] FPGA: Add TS-7300 FPGA manager

[Moritz Fischer]

Hi Florian,

can you Cc: linxu-fpga@vger.kernel.org for your next round?

On Sun, Dec 11, 2016 at 2:17 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> Add support for loading bitstreams on the Altera Cyclone II FPGA
> populated on the TS-7300 board. This is done through the configuration
> and data registers offered through a memory interface between the EP93xx
> SoC and the FPGA.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  drivers/fpga/Kconfig       |   7 ++
>  drivers/fpga/Makefile      |   1 +
>  drivers/fpga/ts73xx-fpga.c | 165 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 173 insertions(+)
>  create mode 100644 drivers/fpga/ts73xx-fpga.c
>
> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> index cd84934774cc..109625707ef0 100644
> --- a/drivers/fpga/Kconfig
> +++ b/drivers/fpga/Kconfig
> @@ -26,6 +26,13 @@ config FPGA_MGR_ZYNQ_FPGA
>         help
>           FPGA manager driver support for Xilinx Zynq FPGAs.
>
> +config FPGA_MGR_TS73XX
> +       tristate "Technologic Systems TS-73xx SBC FPGA Manager"
> +       depends on ARCH_EP93XX && MACH_TS72XX
> +       help
> +         FPGA manager driver support for the Altera Cyclone II FPGA
> +         present on the TS-73xx SBC boards.
> +
>  endif # FPGA
>
>  endmenu
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index 8d83fc6b1613..5d51265cc1b4 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -8,3 +8,4 @@ obj-$(CONFIG_FPGA)                      += fpga-mgr.o
>  # FPGA Manager Drivers
>  obj-$(CONFIG_FPGA_MGR_SOCFPGA)         += socfpga.o
>  obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA)       += zynq-fpga.o
> +obj-$(CONFIG_FPGA_MGR_TS73XX)          += ts73xx-fpga.o
> diff --git a/drivers/fpga/ts73xx-fpga.c b/drivers/fpga/ts73xx-fpga.c
> new file mode 100644
> index 000000000000..2b3d5d668dfc
> --- /dev/null
> +++ b/drivers/fpga/ts73xx-fpga.c
> @@ -0,0 +1,165 @@
> +/*
> + * Technologic Systems TS-73xx SBC FPGA loader
> + *
> + * Copyright (C) 2016 Florian Fainelli <f.fainelli@gmail.com>
> + *
> + * FPGA Manager Driver for the on-board Altera Cyclone II FPGA found on
> + * TS-7300, heavily based on load_fpga.c in their vendor tree.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/string.h>
> +#include <linux/bitrev.h>
> +#include <linux/fpga/fpga-mgr.h>
> +
> +#define TS73XX_FPGA_DATA_REG   0
> +#define TS73XX_FPGA_CONFIG_REG 1
> +
> +struct ts73xx_fpga_priv {
> +       void __iomem    *io_base;
> +       struct device   *dev;
> +};
> +
> +static enum fpga_mgr_states ts73xx_fpga_state(struct fpga_manager *mgr)
> +{
> +       return FPGA_MGR_STATE_UNKNOWN;
> +}
> +
> +static int ts73xx_fpga_write_init(struct fpga_manager *mgr, u32 flags,
> +                                 const char *buf, size_t count)
> +{
> +       struct ts73xx_fpga_priv *priv = mgr->priv;
> +
> +       /* Reset the FPGA */
> +       writeb(0, priv->io_base + TS73XX_FPGA_CONFIG_REG);
> +       udelay(30);
> +       writeb(0x2, priv->io_base + TS73XX_FPGA_CONFIG_REG);
> +       udelay(80);
> +
> +       return 0;
> +}
> +
> +static inline int ts73xx_fpga_can_write(struct ts73xx_fpga_priv *priv)
> +{
> +       unsigned int timeout = 1000;
> +       u8 reg;
> +
> +       while (timeout--) {
> +               reg = readb(priv->io_base + TS73XX_FPGA_CONFIG_REG);
> +               if (!(reg & 0x1))
> +                       return 0;
> +               cpu_relax();
> +       }

You can use readb_poll_timeout() for this I think.
> +
> +       return -ETIMEDOUT;
> +}
> +
> +static int ts73xx_fpga_write(struct fpga_manager *mgr, const char *buf,
> +                            size_t count)
> +{
> +       struct ts73xx_fpga_priv *priv = mgr->priv;
> +       size_t i = 0;
> +       int ret;
> +       u8 reg;
> +
> +       while (count--) {
> +               ret = ts73xx_fpga_can_write(priv);
> +               if (ret < 0)
> +                       return ret;
> +
> +               writeb(buf[i], priv->io_base + TS73XX_FPGA_DATA_REG);
> +               i++;
> +       }
> +
> +       usleep_range(1000, 2000);
> +       reg = readb(priv->io_base + TS73XX_FPGA_CONFIG_REG);
> +       reg |= 0x8;

What's happening here?

> +       writeb(reg, priv->io_base + TS73XX_FPGA_CONFIG_REG);
> +       usleep_range(1000, 2000);
> +
> +       reg = readb(priv->io_base + TS73XX_FPGA_CONFIG_REG);
> +       reg &= ~0x8;

and here? Can we have a named constant for these, or a comment to
explain what happens?
> +       writeb(reg, priv->io_base + TS73XX_FPGA_CONFIG_REG);
> +
> +       return 0;
> +}
> +
> +static int ts73xx_fpga_write_complete(struct fpga_manager *mgr, u32 flags)
> +{
> +       struct ts73xx_fpga_priv *priv = mgr->priv;
> +       u8 reg;
> +
> +       reg = readb(priv->io_base + TS73XX_FPGA_CONFIG_REG);
> +       if ((reg & 0x4) != 0x4)

Named constants, please

> +               return -ETIMEDOUT;
> +
> +       return 0;
> +}
> +
> +static const struct fpga_manager_ops ts73xx_fpga_ops = {
> +       .state          = ts73xx_fpga_state,
> +       .write_init     = ts73xx_fpga_write_init,
> +       .write          = ts73xx_fpga_write,
> +       .write_complete = ts73xx_fpga_write_complete,
> +};
> +
> +static int ts73xx_fpga_probe(struct platform_device *pdev)
> +{
> +       struct device *kdev = &pdev->dev;
> +       struct ts73xx_fpga_priv *priv;
> +       struct fpga_manager *mgr;
> +       struct resource *res;
> +       int err;
> +
> +       priv = devm_kzalloc(kdev, sizeof(*priv), GFP_KERNEL);
> +       if (!priv)
> +               return -ENOMEM;
> +
> +       priv->dev = kdev;
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       priv->io_base = devm_ioremap_resource(kdev, res);
> +       if (IS_ERR(priv->io_base))
> +               return PTR_ERR(priv->io_base);

Maybe an error message here?
> +
> +       err = fpga_mgr_register(kdev, "TS-73xx FPGA Manager",
> +                               &ts73xx_fpga_ops, priv);
> +       if (err) {
> +               dev_err(kdev, "failed to register FPGA manager\n");
> +               return err;
> +       }
> +
> +       return err;
> +}
> +
> +static int ts73xx_fpga_remove(struct platform_device *pdev)
> +{
> +       fpga_mgr_unregister(&pdev->dev);
> +
> +       return 0;
> +}
> +
> +static struct platform_driver ts73xx_fpga_driver = {
> +       .driver = {
> +               .name   = "ts73xx-fpga-mgr",
> +       },
> +       .probe  = ts73xx_fpga_probe,
> +       .remove = ts73xx_fpga_remove,
> +};
> +module_platform_driver(ts73xx_fpga_driver);
> +
> +MODULE_AUTHOR("Florian Fainelli <f.fainelli@gmail.com>");
> +MODULE_DESCRIPTION("TS-73xx FPGA Manager driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.9.3
>

Thanks,

Moritz

Re: [PATCH 2/2] FPGA: Add TS-7300 FPGA manager

[Alan Tull]

On Sun, 11 Dec 2016, Florian Fainelli wrote:

> Add support for loading bitstreams on the Altera Cyclone II FPGA
> populated on the TS-7300 board. This is done through the configuration
> and data registers offered through a memory interface between the EP93xx
> SoC and the FPGA.
> 
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Hi Florain,

Thanks for submitting!

How specific is this to the tx7300 board?

I'm unclear about the programming method here.  Are these registers
exposed by the EP93xx?  Is it possible that another cpu could access
these two registers to configure the cyclone ii?  Is this passive
serial?

Please cc linux-fpga@vger.kernel.org for the next version.

Other comments below...

> ---
>  drivers/fpga/Kconfig       |   7 ++
>  drivers/fpga/Makefile      |   1 +
>  drivers/fpga/ts73xx-fpga.c | 165 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 173 insertions(+)
>  create mode 100644 drivers/fpga/ts73xx-fpga.c
> 
> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> index cd84934774cc..109625707ef0 100644
> --- a/drivers/fpga/Kconfig
> +++ b/drivers/fpga/Kconfig
> @@ -26,6 +26,13 @@ config FPGA_MGR_ZYNQ_FPGA
>  	help
>  	  FPGA manager driver support for Xilinx Zynq FPGAs.
>  
> +config FPGA_MGR_TS73XX
> +	tristate "Technologic Systems TS-73xx SBC FPGA Manager"
> +	depends on ARCH_EP93XX && MACH_TS72XX
> +	help
> +	  FPGA manager driver support for the Altera Cyclone II FPGA
> +	  present on the TS-73xx SBC boards.
> +
>  endif # FPGA
>  
>  endmenu
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index 8d83fc6b1613..5d51265cc1b4 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -8,3 +8,4 @@ obj-$(CONFIG_FPGA)			+= fpga-mgr.o
>  # FPGA Manager Drivers
>  obj-$(CONFIG_FPGA_MGR_SOCFPGA)		+= socfpga.o
>  obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA)	+= zynq-fpga.o
> +obj-$(CONFIG_FPGA_MGR_TS73XX)		+= ts73xx-fpga.o
> diff --git a/drivers/fpga/ts73xx-fpga.c b/drivers/fpga/ts73xx-fpga.c
> new file mode 100644
> index 000000000000..2b3d5d668dfc
> --- /dev/null
> +++ b/drivers/fpga/ts73xx-fpga.c
> @@ -0,0 +1,165 @@
> +/*
> + * Technologic Systems TS-73xx SBC FPGA loader
> + *
> + * Copyright (C) 2016 Florian Fainelli <f.fainelli@gmail.com>
> + *
> + * FPGA Manager Driver for the on-board Altera Cyclone II FPGA found on
> + * TS-7300, heavily based on load_fpga.c in their vendor tree.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/string.h>
> +#include <linux/bitrev.h>
> +#include <linux/fpga/fpga-mgr.h>
> +
> +#define TS73XX_FPGA_DATA_REG	0
> +#define TS73XX_FPGA_CONFIG_REG	1
> +
> +struct ts73xx_fpga_priv {
> +	void __iomem	*io_base;
> +	struct device	*dev;
> +};
> +
> +static enum fpga_mgr_states ts73xx_fpga_state(struct fpga_manager *mgr)
> +{
> +	return FPGA_MGR_STATE_UNKNOWN;
> +}
> +
> +static int ts73xx_fpga_write_init(struct fpga_manager *mgr, u32 flags,
> +				  const char *buf, size_t count)
> +{
> +	struct ts73xx_fpga_priv *priv = mgr->priv;
> +
> +	/* Reset the FPGA */
> +	writeb(0, priv->io_base + TS73XX_FPGA_CONFIG_REG);
> +	udelay(30);
> +	writeb(0x2, priv->io_base + TS73XX_FPGA_CONFIG_REG);
> +	udelay(80);

Could these udelay values be macros?

> +
> +	return 0;
> +}
> +
> +static inline int ts73xx_fpga_can_write(struct ts73xx_fpga_priv *priv)
> +{
> +	unsigned int timeout = 1000;

Another macro?

> +	u8 reg;
> +
> +	while (timeout--) {
> +		reg = readb(priv->io_base + TS73XX_FPGA_CONFIG_REG);
> +		if (!(reg & 0x1))

Macro to tell us what the bit name of this bit.

> +			return 0;
> +		cpu_relax();
> +	}
> +
> +	return -ETIMEDOUT;
> +}
> +
> +static int ts73xx_fpga_write(struct fpga_manager *mgr, const char *buf,
> +			     size_t count)
> +{
> +	struct ts73xx_fpga_priv *priv = mgr->priv;
> +	size_t i = 0;
> +	int ret;
> +	u8 reg;
> +
> +	while (count--) {
> +		ret = ts73xx_fpga_can_write(priv);
> +		if (ret < 0)
> +			return ret;
> +
> +		writeb(buf[i], priv->io_base + TS73XX_FPGA_DATA_REG);
> +		i++;
> +	}
> +
> +	usleep_range(1000, 2000);
> +	reg = readb(priv->io_base + TS73XX_FPGA_CONFIG_REG);
> +	reg |= 0x8;

A macro for this bit.

> +	writeb(reg, priv->io_base + TS73XX_FPGA_CONFIG_REG);
> +	usleep_range(1000, 2000);
> +
> +	reg = readb(priv->io_base + TS73XX_FPGA_CONFIG_REG);
> +	reg &= ~0x8;
> +	writeb(reg, priv->io_base + TS73XX_FPGA_CONFIG_REG);
> +
> +	return 0;
> +}
> +
> +static int ts73xx_fpga_write_complete(struct fpga_manager *mgr, u32 flags)
> +{
> +	struct ts73xx_fpga_priv *priv = mgr->priv;
> +	u8 reg;
> +
> +	reg = readb(priv->io_base + TS73XX_FPGA_CONFIG_REG);
> +	if ((reg & 0x4) != 0x4)

Another macro...

> +		return -ETIMEDOUT;
> +
> +	return 0;
> +}
> +
> +static const struct fpga_manager_ops ts73xx_fpga_ops = {
> +	.state		= ts73xx_fpga_state,
> +	.write_init	= ts73xx_fpga_write_init,
> +	.write		= ts73xx_fpga_write,
> +	.write_complete	= ts73xx_fpga_write_complete,
> +};
> +
> +static int ts73xx_fpga_probe(struct platform_device *pdev)
> +{
> +	struct device *kdev = &pdev->dev;
> +	struct ts73xx_fpga_priv *priv;
> +	struct fpga_manager *mgr;
> +	struct resource *res;
> +	int err;
> +
> +	priv = devm_kzalloc(kdev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->dev = kdev;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	priv->io_base = devm_ioremap_resource(kdev, res);
> +	if (IS_ERR(priv->io_base))
> +		return PTR_ERR(priv->io_base);
> +
> +	err = fpga_mgr_register(kdev, "TS-73xx FPGA Manager",
> +				&ts73xx_fpga_ops, priv);
> +	if (err) {
> +		dev_err(kdev, "failed to register FPGA manager\n");
> +		return err;
> +	}
> +
> +	return err;
> +}
> +
> +static int ts73xx_fpga_remove(struct platform_device *pdev)
> +{
> +	fpga_mgr_unregister(&pdev->dev);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver ts73xx_fpga_driver = {
> +	.driver	= {
> +		.name	= "ts73xx-fpga-mgr",
> +	},
> +	.probe	= ts73xx_fpga_probe,
> +	.remove	= ts73xx_fpga_remove,
> +};
> +module_platform_driver(ts73xx_fpga_driver);
> +
> +MODULE_AUTHOR("Florian Fainelli <f.fainelli@gmail.com>");
> +MODULE_DESCRIPTION("TS-73xx FPGA Manager driver");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.9.3
> 
> 

Re: [PATCH 2/2] FPGA: Add TS-7300 FPGA manager

[Florian Fainelli]

On 12/12/2016 08:01 AM, Alan Tull wrote:
> On Sun, 11 Dec 2016, Florian Fainelli wrote:
> 
>> Add support for loading bitstreams on the Altera Cyclone II FPGA
>> populated on the TS-7300 board. This is done through the configuration
>> and data registers offered through a memory interface between the EP93xx
>> SoC and the FPGA.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> 
> Hi Florain,
> 
> Thanks for submitting!
> 
> How specific is this to the tx7300 board?
> 
> I'm unclear about the programming method here.  Are these registers
> exposed by the EP93xx?  Is it possible that another cpu could access
> these two registers to configure the cyclone ii?  Is this passive
> serial?

So here is my understanding, from glancing at the TS-7300 board manual:

- there is an on-board CPLD which does a variety of services and I/O for
the EP9302 SoC, one of these services is the configuration of the
on-board FPGA

- the programming interface here is some kind of abstraction around a
Cyclone II FPGA, and is by no means standard, nor directly exposed to
the CPU

- unless you go through the CPLD, there is no other way that you could
configure the FPGA

Does that help answer your questions?

> 
> Please cc linux-fpga@vger.kernel.org for the next version.
> 
> Other comments below...

OK, I will fix those,

> 
>> ---
>>  drivers/fpga/Kconfig       |   7 ++
>>  drivers/fpga/Makefile      |   1 +
>>  drivers/fpga/ts73xx-fpga.c | 165 +++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 173 insertions(+)
>>  create mode 100644 drivers/fpga/ts73xx-fpga.c
>>
>> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
>> index cd84934774cc..109625707ef0 100644
>> --- a/drivers/fpga/Kconfig
>> +++ b/drivers/fpga/Kconfig
>> @@ -26,6 +26,13 @@ config FPGA_MGR_ZYNQ_FPGA
>>  	help
>>  	  FPGA manager driver support for Xilinx Zynq FPGAs.
>>  
>> +config FPGA_MGR_TS73XX
>> +	tristate "Technologic Systems TS-73xx SBC FPGA Manager"
>> +	depends on ARCH_EP93XX && MACH_TS72XX
>> +	help
>> +	  FPGA manager driver support for the Altera Cyclone II FPGA
>> +	  present on the TS-73xx SBC boards.
>> +
>>  endif # FPGA
>>  
>>  endmenu
>> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
>> index 8d83fc6b1613..5d51265cc1b4 100644
>> --- a/drivers/fpga/Makefile
>> +++ b/drivers/fpga/Makefile
>> @@ -8,3 +8,4 @@ obj-$(CONFIG_FPGA)			+= fpga-mgr.o
>>  # FPGA Manager Drivers
>>  obj-$(CONFIG_FPGA_MGR_SOCFPGA)		+= socfpga.o
>>  obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA)	+= zynq-fpga.o
>> +obj-$(CONFIG_FPGA_MGR_TS73XX)		+= ts73xx-fpga.o
>> diff --git a/drivers/fpga/ts73xx-fpga.c b/drivers/fpga/ts73xx-fpga.c
>> new file mode 100644
>> index 000000000000..2b3d5d668dfc
>> --- /dev/null
>> +++ b/drivers/fpga/ts73xx-fpga.c
>> @@ -0,0 +1,165 @@
>> +/*
>> + * Technologic Systems TS-73xx SBC FPGA loader
>> + *
>> + * Copyright (C) 2016 Florian Fainelli <f.fainelli@gmail.com>
>> + *
>> + * FPGA Manager Driver for the on-board Altera Cyclone II FPGA found on
>> + * TS-7300, heavily based on load_fpga.c in their vendor tree.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; version 2 of the License.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/delay.h>
>> +#include <linux/io.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/string.h>
>> +#include <linux/bitrev.h>
>> +#include <linux/fpga/fpga-mgr.h>
>> +
>> +#define TS73XX_FPGA_DATA_REG	0
>> +#define TS73XX_FPGA_CONFIG_REG	1
>> +
>> +struct ts73xx_fpga_priv {
>> +	void __iomem	*io_base;
>> +	struct device	*dev;
>> +};
>> +
>> +static enum fpga_mgr_states ts73xx_fpga_state(struct fpga_manager *mgr)
>> +{
>> +	return FPGA_MGR_STATE_UNKNOWN;
>> +}
>> +
>> +static int ts73xx_fpga_write_init(struct fpga_manager *mgr, u32 flags,
>> +				  const char *buf, size_t count)
>> +{
>> +	struct ts73xx_fpga_priv *priv = mgr->priv;
>> +
>> +	/* Reset the FPGA */
>> +	writeb(0, priv->io_base + TS73XX_FPGA_CONFIG_REG);
>> +	udelay(30);
>> +	writeb(0x2, priv->io_base + TS73XX_FPGA_CONFIG_REG);
>> +	udelay(80);
> 
> Could these udelay values be macros?

The bit definitions could be defined, but the delays, why would that be
useful?

> 
>> +
>> +	return 0;
>> +}
>> +
>> +static inline int ts73xx_fpga_can_write(struct ts73xx_fpga_priv *priv)
>> +{
>> +	unsigned int timeout = 1000;
> 
> Another macro?

The delay is just an arbitrary good timeout.
-- 
Florian

Re: [PATCH 2/2] FPGA: Add TS-7300 FPGA manager

[Alan Tull]

On Mon, 12 Dec 2016, Florian Fainelli wrote:

> On 12/12/2016 08:01 AM, Alan Tull wrote:
> > On Sun, 11 Dec 2016, Florian Fainelli wrote:
> > 
> >> Add support for loading bitstreams on the Altera Cyclone II FPGA
> >> populated on the TS-7300 board. This is done through the configuration
> >> and data registers offered through a memory interface between the EP93xx
> >> SoC and the FPGA.
> >>
> >> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> > 
> > Hi Florain,
> > 
> > Thanks for submitting!
> > 
> > How specific is this to the tx7300 board?
> > 
> > I'm unclear about the programming method here.  Are these registers
> > exposed by the EP93xx?  Is it possible that another cpu could access
> > these two registers to configure the cyclone ii?  Is this passive
> > serial?
> 
> So here is my understanding, from glancing at the TS-7300 board manual:
> 
> - there is an on-board CPLD which does a variety of services and I/O for
> the EP9302 SoC, one of these services is the configuration of the
> on-board FPGA
> 
> - the programming interface here is some kind of abstraction around a
> Cyclone II FPGA, and is by no means standard, nor directly exposed to
> the CPU
> 
> - unless you go through the CPLD, there is no other way that you could
> configure the FPGA
> 
> Does that help answer your questions?

Yes it does.  Maybe a brief comment explaining that similar to what
you just said.

> >> +static int ts73xx_fpga_write_init(struct fpga_manager *mgr, u32 flags,
> >> +				  const char *buf, size_t count)
> >> +{
> >> +	struct ts73xx_fpga_priv *priv = mgr->priv;
> >> +
> >> +	/* Reset the FPGA */
> >> +	writeb(0, priv->io_base + TS73XX_FPGA_CONFIG_REG);
> >> +	udelay(30);
> >> +	writeb(0x2, priv->io_base + TS73XX_FPGA_CONFIG_REG);
> >> +	udelay(80);
> > 
> > Could these udelay values be macros?
> 
> The bit definitions could be defined, but the delays, why would that be
> useful?

If it is helpful for someone reading the code to know what the delays
are, if some future generation of the board/cpld uses this same
driver.  So when this driver is broken for the next generation
board/cpld, people trying to fix this know what the delay is there for
and can have a better chance at adjusting the right delay.

> 
> > 
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static inline int ts73xx_fpga_can_write(struct ts73xx_fpga_priv *priv)
> >> +{
> >> +	unsigned int timeout = 1000;
> > 
> > Another macro?
> 
> The delay is just an arbitrary good timeout.
> -- 
> Florian
> 

RE: [PATCH 2/2] FPGA: Add TS-7300 FPGA manager

[Hartley Sweeten]

On Monday, December 12, 2016 9:02 AM, Alan Tull wrote:
> On Sun, 11 Dec 2016, Florian Fainelli wrote:
>> Add support for loading bitstreams on the Altera Cyclone II FPGA
>> populated on the TS-7300 board. This is done through the configuration
>> and data registers offered through a memory interface between the EP93xx
>> SoC and the FPGA.
>> 
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>
> Hi Florain,
>
> Thanks for submitting!
>
> How specific is this to the tx7300 board?
>
> I'm unclear about the programming method here.  Are these registers
> exposed by the EP93xx?  Is it possible that another cpu could access
> these two registers to configure the cyclone ii?  Is this passive
> serial?

Alan,

>From the schematic, it appears that the Cyclone II FPGA is configured for
Fast AS programming. The glue chip (MAXII CLPD) on the board appears to
implement some kind of state machine to handle the FPGA programming
using two registers in the glue chip.

Hartley