[v2,2/2] fpga manager: Add Xilinx slave serial SPI driver

Commit Message

Anatolij Gustschin Feb. 20, 2017, 2:25 p.m. UTC

The driver loads FPGA firmware over SPI, using the "slave serial"
configuration interface on Xilinx FPGAs.

Signed-off-by: Anatolij Gustschin <[e-mail]>
---
This patch requires patch https://lkml.org/lkml/2017/2/15/667
for building

Changes in v2:

 - rebased on v4.10

 drivers/fpga/Kconfig      |   7 ++
 drivers/fpga/Makefile     |   1 +
 drivers/fpga/xilinx-spi.c | 181 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 189 insertions(+)
 create mode 100644 drivers/fpga/xilinx-spi.c

Comments

Moritz Fischer Feb. 20, 2017, 5:27 p.m. UTC | #1

Anatolij,

small nits inline.

On Mon, Feb 20, 2017 at 6:25 AM, Anatolij Gustschin <[e-mail]> wrote:
> The driver loads FPGA firmware over SPI, using the "slave serial"
> configuration interface on Xilinx FPGAs.
>
> Signed-off-by: Anatolij Gustschin <[e-mail]>

Acked-by: Moritz Fischer <[e-mail]>

> ---
> This patch requires patch https://lkml.org/lkml/2017/2/15/667
> for building
>
> Changes in v2:
>
>  - rebased on v4.10
>
>  drivers/fpga/Kconfig      |   7 ++
>  drivers/fpga/Makefile     |   1 +
>  drivers/fpga/xilinx-spi.c | 181 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 189 insertions(+)
>  create mode 100644 drivers/fpga/xilinx-spi.c
>
> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> index ce861a2..6b42786 100644
> --- a/drivers/fpga/Kconfig
> +++ b/drivers/fpga/Kconfig
> @@ -33,6 +33,13 @@ config FPGA_MGR_SOCFPGA_A10
>         help
>           FPGA manager driver support for Altera Arria10 SoCFPGA.
>
> +config FPGA_MGR_XILINX_SPI
> +       tristate "Xilinx Configuration over Slave Serial (SPI)"
> +       depends on SPI
> +       help
> +         FPGA manager driver support for Xilinx FPGA configuration
> +         over slave serial interface.
> +
>  config FPGA_MGR_ZYNQ_FPGA
>         tristate "Xilinx Zynq FPGA"
>         depends on ARCH_ZYNQ || COMPILE_TEST
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index 8df07bc..7582c5a 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -8,6 +8,7 @@ obj-$(CONFIG_FPGA)                      += fpga-mgr.o
>  # FPGA Manager Drivers
>  obj-$(CONFIG_FPGA_MGR_SOCFPGA)         += socfpga.o
>  obj-$(CONFIG_FPGA_MGR_SOCFPGA_A10)     += socfpga-a10.o
> +obj-$(CONFIG_FPGA_MGR_XILINX_SPI)      += xilinx-spi.o
>  obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA)       += zynq-fpga.o
>
>  # FPGA Bridge Drivers
> diff --git a/drivers/fpga/xilinx-spi.c b/drivers/fpga/xilinx-spi.c
> new file mode 100644
> index 0000000..16b3fa2
> --- /dev/null
> +++ b/drivers/fpga/xilinx-spi.c
> @@ -0,0 +1,181 @@
> +/*
> + * Xilinx Spartan6 Slave Serial SPI Driver
> + *
> + * Copyright (C) 2017 DENX Software Engineering
> + *
> + * Anatolij Gustschin <[e-mail]>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * Manage Xilinx FPGA firmware that is loaded over SPI using
> + * the slave serial configuration interface.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/fpga/fpga-mgr.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/of.h>
> +#include <linux/spi/spi.h>
> +#include <linux/sizes.h>
> +
> +#define FPGA_MIN_CFG_WAIT_US   10
> +
> +struct xilinx_spi_conf {
> +       struct spi_device *spi;
> +       struct gpio_desc *prog_b;
> +       struct gpio_desc *done;
> +};
> +
> +static enum fpga_mgr_states xilinx_spi_state(struct fpga_manager *mgr)
> +{
> +       struct xilinx_spi_conf *conf = mgr->priv;
> +
> +       if (!gpiod_get_value(conf->done))
> +               return FPGA_MGR_STATE_RESET;
> +
> +       return FPGA_MGR_STATE_UNKNOWN;
> +}
> +
> +static int xilinx_spi_write_init(struct fpga_manager *mgr,
> +                                struct fpga_image_info *info,
> +                                const char *buf, size_t count)
> +{
> +       struct xilinx_spi_conf *conf = mgr->priv;
> +
> +       if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
> +               dev_err(&mgr->dev, "Partial reconfiguration not supported.\n");
> +               return -EINVAL;
> +       }
> +
> +       gpiod_set_value(conf->prog_b, 1);
> +
> +       udelay(1); /* min is 500 ns */

Maybe make this a named constant.
> +
> +       gpiod_set_value(conf->prog_b, 0);
> +
> +       if (gpiod_get_value(conf->done)) {
> +               dev_err(&mgr->dev, "Unexpected DONE pin state...\n");
> +               return -EIO;
> +       }
> +
> +       /* program latency */
> +       usleep_range(1000, 1500);

Same here.
> +       return 0;
> +}
> +
> +static int xilinx_spi_write(struct fpga_manager *mgr, const char *buf,
> +                           size_t count)
> +{
> +       struct xilinx_spi_conf *conf = mgr->priv;
> +       const char *fw_data = buf;
> +       const char *fw_data_end = fw_data + count;
> +
> +       while (fw_data < fw_data_end) {
> +               int ret;
> +               size_t stride = min(fw_data_end - fw_data, SZ_4K);
> +
> +               ret = spi_write(conf->spi, fw_data, stride);
> +               if (ret) {
> +                       dev_err(&mgr->dev, "SPI error in firmware write: %d\n",
> +                               ret);
> +                       return ret;
> +               }
> +               fw_data += stride;
> +       }
> +
> +       return 0;
> +}
> +
> +static int xilinx_spi_write_complete(struct fpga_manager *mgr,
> +                                    struct fpga_image_info *info)
> +{
> +       struct xilinx_spi_conf *conf = mgr->priv;
> +       int wait;
> +
> +       if (gpiod_get_value(conf->done))
> +               return 0;
> +
> +       wait = info->config_complete_timeout_us / FPGA_MIN_CFG_WAIT_US;
> +       if (info->config_complete_timeout_us % FPGA_MIN_CFG_WAIT_US)
> +               wait += 1;
> +
> +       while (wait--) {
> +               usleep_range(FPGA_MIN_CFG_WAIT_US, FPGA_MIN_CFG_WAIT_US + 5);
> +
> +               if (gpiod_get_value(conf->done))
> +                       return 0;
> +       }
> +
> +       dev_err(&mgr->dev, "Timeout after config data transfer.\n");
> +       return -ETIMEDOUT;
> +}
> +
> +static const struct fpga_manager_ops xilinx_spi_ops = {
> +       .state = xilinx_spi_state,
> +       .write_init = xilinx_spi_write_init,
> +       .write = xilinx_spi_write,
> +       .write_complete = xilinx_spi_write_complete,
> +};
> +
> +static int xilinx_spi_probe(struct spi_device *spi)
> +{
> +       struct xilinx_spi_conf *conf;
> +
> +       conf = devm_kzalloc(&spi->dev, sizeof(*conf), GFP_KERNEL);
> +       if (!conf)
> +               return -ENOMEM;
> +
> +       conf->spi = spi;
> +
> +       /* PROGRAM_B is active low */
> +       conf->prog_b = devm_gpiod_get(&spi->dev, "prog_b", GPIOD_OUT_LOW);
> +       if (IS_ERR(conf->prog_b)) {
> +               dev_err(&spi->dev, "Failed to get PROGRAM_B gpio: %ld\n",
> +                       PTR_ERR(conf->prog_b));
> +               return PTR_ERR(conf->prog_b);
> +       }
> +
> +       conf->done = devm_gpiod_get(&spi->dev, "done", GPIOD_IN);
> +       if (IS_ERR(conf->done)) {
> +               dev_err(&spi->dev, "Failed to get DONE gpio: %ld\n",
> +                       PTR_ERR(conf->done));
> +               return PTR_ERR(conf->done);
> +       }
> +
> +       return fpga_mgr_register(&spi->dev, "Xilinx Slave Serial FPGA Manager",
> +                                &xilinx_spi_ops, conf);
> +}
> +
> +static int xilinx_spi_remove(struct spi_device *spi)
> +{
> +       fpga_mgr_unregister(&spi->dev);
> +
> +       return 0;
> +}
> +
> +static const struct of_device_id xlnx_spi_of_match[] = {
> +       { .compatible = "xlnx,fpga-slave-serial", },
> +       {}
> +};
> +MODULE_DEVICE_TABLE(of, xlnx_spi_of_match);
> +
> +static struct spi_driver xilinx_slave_spi_driver = {
> +       .driver = {
> +               .name = "xlnx-slave-spi",
> +               .owner = THIS_MODULE,
> +               .of_match_table = of_match_ptr(xlnx_spi_of_match),
> +       },
> +       .probe = xilinx_spi_probe,
> +       .remove = xilinx_spi_remove,
> +};
> +
> +module_spi_driver(xilinx_slave_spi_driver)
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Anatolij Gustschin <[e-mail]>");
> +MODULE_DESCRIPTION("Load Xilinx FPGA firmware over SPI");
> --
> 2.7.4
>

Thanks for the patch,

Moritz
--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Anatolij Gustschin Feb. 20, 2017, 8:05 p.m. UTC | #2

Hi Moritz,

On Mon, 20 Feb 2017 09:27:35 -0800
Moritz Fischer [e-mail] wrote:

>Anatolij,
>
>small nits inline.
...

>> +       gpiod_set_value(conf->prog_b, 1);
>> +
>> +       udelay(1); /* min is 500 ns */  
>
>Maybe make this a named constant.

Okay, I can add

  const size_t prog_pulse_us = 1;
  ...
  udelay(prog_pulse_us);

...
>> +       gpiod_set_value(conf->prog_b, 0);
>> +
>> +       if (gpiod_get_value(conf->done)) {
>> +               dev_err(&mgr->dev, "Unexpected DONE pin state...\n");
>> +               return -EIO;
>> +       }
>> +
>> +       /* program latency */
>> +       usleep_range(1000, 1500);  
>
>Same here.

Okay, will change here as well.

Thanks for review! I'll wait some time for more comments and
will submit v3 then.

Anatolij
--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Michal Simek Feb. 21, 2017, 2:01 p.m. UTC | #3

On 20.2.2017 15:25, Anatolij Gustschin wrote:
> The driver loads FPGA firmware over SPI, using the "slave serial"
> configuration interface on Xilinx FPGAs.
> 
> Signed-off-by: Anatolij Gustschin <[e-mail]>
> ---
> This patch requires patch https://lkml.org/lkml/2017/2/15/667
> for building
> 
> Changes in v2:
> 
>  - rebased on v4.10
> 
>  drivers/fpga/Kconfig      |   7 ++
>  drivers/fpga/Makefile     |   1 +
>  drivers/fpga/xilinx-spi.c | 181 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 189 insertions(+)
>  create mode 100644 drivers/fpga/xilinx-spi.c
> 
> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> index ce861a2..6b42786 100644
> --- a/drivers/fpga/Kconfig
> +++ b/drivers/fpga/Kconfig
> @@ -33,6 +33,13 @@ config FPGA_MGR_SOCFPGA_A10
>  	help
>  	  FPGA manager driver support for Altera Arria10 SoCFPGA.
>  
> +config FPGA_MGR_XILINX_SPI
> +	tristate "Xilinx Configuration over Slave Serial (SPI)"
> +	depends on SPI
> +	help
> +	  FPGA manager driver support for Xilinx FPGA configuration
> +	  over slave serial interface.
> +
>  config FPGA_MGR_ZYNQ_FPGA
>  	tristate "Xilinx Zynq FPGA"
>  	depends on ARCH_ZYNQ || COMPILE_TEST
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index 8df07bc..7582c5a 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -8,6 +8,7 @@ obj-$(CONFIG_FPGA)			+= fpga-mgr.o
>  # FPGA Manager Drivers
>  obj-$(CONFIG_FPGA_MGR_SOCFPGA)		+= socfpga.o
>  obj-$(CONFIG_FPGA_MGR_SOCFPGA_A10)	+= socfpga-a10.o
> +obj-$(CONFIG_FPGA_MGR_XILINX_SPI)	+= xilinx-spi.o
>  obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA)	+= zynq-fpga.o
>  
>  # FPGA Bridge Drivers
> diff --git a/drivers/fpga/xilinx-spi.c b/drivers/fpga/xilinx-spi.c
> new file mode 100644
> index 0000000..16b3fa2
> --- /dev/null
> +++ b/drivers/fpga/xilinx-spi.c
> @@ -0,0 +1,181 @@
> +/*
> + * Xilinx Spartan6 Slave Serial SPI Driver
> + *
> + * Copyright (C) 2017 DENX Software Engineering
> + *
> + * Anatolij Gustschin <[e-mail]>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * Manage Xilinx FPGA firmware that is loaded over SPI using
> + * the slave serial configuration interface.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/fpga/fpga-mgr.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/of.h>
> +#include <linux/spi/spi.h>
> +#include <linux/sizes.h>
> +
> +#define FPGA_MIN_CFG_WAIT_US	10
> +
> +struct xilinx_spi_conf {
> +	struct spi_device *spi;
> +	struct gpio_desc *prog_b;
> +	struct gpio_desc *done;
> +};
> +
> +static enum fpga_mgr_states xilinx_spi_state(struct fpga_manager *mgr)
> +{
> +	struct xilinx_spi_conf *conf = mgr->priv;
> +
> +	if (!gpiod_get_value(conf->done))
> +		return FPGA_MGR_STATE_RESET;
> +
> +	return FPGA_MGR_STATE_UNKNOWN;
> +}
> +
> +static int xilinx_spi_write_init(struct fpga_manager *mgr,
> +				 struct fpga_image_info *info,
> +				 const char *buf, size_t count)
> +{
> +	struct xilinx_spi_conf *conf = mgr->priv;
> +
> +	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
> +		dev_err(&mgr->dev, "Partial reconfiguration not supported.\n");
> +		return -EINVAL;
> +	}
> +
> +	gpiod_set_value(conf->prog_b, 1);
> +
> +	udelay(1); /* min is 500 ns */
> +
> +	gpiod_set_value(conf->prog_b, 0);
> +
> +	if (gpiod_get_value(conf->done)) {
> +		dev_err(&mgr->dev, "Unexpected DONE pin state...\n");
> +		return -EIO;
> +	}
> +
> +	/* program latency */
> +	usleep_range(1000, 1500);
> +	return 0;
> +}
> +
> +static int xilinx_spi_write(struct fpga_manager *mgr, const char *buf,
> +			    size_t count)
> +{
> +	struct xilinx_spi_conf *conf = mgr->priv;
> +	const char *fw_data = buf;
> +	const char *fw_data_end = fw_data + count;
> +
> +	while (fw_data < fw_data_end) {
> +		int ret;
> +		size_t stride = min(fw_data_end - fw_data, SZ_4K);
> +
> +		ret = spi_write(conf->spi, fw_data, stride);
> +		if (ret) {
> +			dev_err(&mgr->dev, "SPI error in firmware write: %d\n",
> +				ret);
> +			return ret;
> +		}
> +		fw_data += stride;
> +	}
> +
> +	return 0;
> +}
> +
> +static int xilinx_spi_write_complete(struct fpga_manager *mgr,
> +				     struct fpga_image_info *info)
> +{
> +	struct xilinx_spi_conf *conf = mgr->priv;
> +	int wait;
> +
> +	if (gpiod_get_value(conf->done))
> +		return 0;
> +
> +	wait = info->config_complete_timeout_us / FPGA_MIN_CFG_WAIT_US;
> +	if (info->config_complete_timeout_us % FPGA_MIN_CFG_WAIT_US)
> +		wait += 1;
> +
> +	while (wait--) {
> +		usleep_range(FPGA_MIN_CFG_WAIT_US, FPGA_MIN_CFG_WAIT_US + 5);
> +
> +		if (gpiod_get_value(conf->done))
> +			return 0;
> +	}
> +
> +	dev_err(&mgr->dev, "Timeout after config data transfer.\n");
> +	return -ETIMEDOUT;
> +}
> +
> +static const struct fpga_manager_ops xilinx_spi_ops = {
> +	.state = xilinx_spi_state,
> +	.write_init = xilinx_spi_write_init,
> +	.write = xilinx_spi_write,
> +	.write_complete = xilinx_spi_write_complete,
> +};
> +
> +static int xilinx_spi_probe(struct spi_device *spi)
> +{
> +	struct xilinx_spi_conf *conf;
> +
> +	conf = devm_kzalloc(&spi->dev, sizeof(*conf), GFP_KERNEL);
> +	if (!conf)
> +		return -ENOMEM;
> +
> +	conf->spi = spi;
> +
> +	/* PROGRAM_B is active low */
> +	conf->prog_b = devm_gpiod_get(&spi->dev, "prog_b", GPIOD_OUT_LOW);
> +	if (IS_ERR(conf->prog_b)) {
> +		dev_err(&spi->dev, "Failed to get PROGRAM_B gpio: %ld\n",
> +			PTR_ERR(conf->prog_b));
> +		return PTR_ERR(conf->prog_b);
> +	}
> +
> +	conf->done = devm_gpiod_get(&spi->dev, "done", GPIOD_IN);
> +	if (IS_ERR(conf->done)) {
> +		dev_err(&spi->dev, "Failed to get DONE gpio: %ld\n",
> +			PTR_ERR(conf->done));
> +		return PTR_ERR(conf->done);
> +	}
> +
> +	return fpga_mgr_register(&spi->dev, "Xilinx Slave Serial FPGA Manager",
> +				 &xilinx_spi_ops, conf);
> +}
> +
> +static int xilinx_spi_remove(struct spi_device *spi)
> +{
> +	fpga_mgr_unregister(&spi->dev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id xlnx_spi_of_match[] = {
> +	{ .compatible = "xlnx,fpga-slave-serial", },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, xlnx_spi_of_match);
> +
> +static struct spi_driver xilinx_slave_spi_driver = {
> +	.driver = {
> +		.name = "xlnx-slave-spi",
> +		.owner = THIS_MODULE,

This should be done by core and it is checked by testing system.

> +		.of_match_table = of_match_ptr(xlnx_spi_of_match),
> +	},
> +	.probe = xilinx_spi_probe,
> +	.remove = xilinx_spi_remove,
> +};
> +
> +module_spi_driver(xilinx_slave_spi_driver)
> +
> +MODULE_LICENSE("GPL");

Above you have GPL v2 and here v2 or later.

include/linux/module.h
 *      "GPL"                           [GNU Public License v2 or later]
 *      "GPL v2"                        [GNU Public License v2]

What's the license then?


> +MODULE_AUTHOR("Anatolij Gustschin <[e-mail]>");
> +MODULE_DESCRIPTION("Load Xilinx FPGA firmware over SPI");
> 

What's the hw configuration?

Thanks,
Michal

--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Anatolij Gustschin Feb. 21, 2017, 2:24 p.m. UTC | #4

On Tue, 21 Feb 2017 15:01:32 +0100
Michal Simek [e-mail] wrote:
...
>> +static struct spi_driver xilinx_slave_spi_driver = {
>> +	.driver = {
>> +		.name = "xlnx-slave-spi",
>> +		.owner = THIS_MODULE,  
>
>This should be done by core and it is checked by testing system.

Okay, will remove it.

...
>> +MODULE_LICENSE("GPL");  
>
>Above you have GPL v2 and here v2 or later.
>
>include/linux/module.h
> *      "GPL"                           [GNU Public License v2 or later]
> *      "GPL v2"                        [GNU Public License v2]
>
>What's the license then?

should be "GPL v2", I'll fix it next patch version.

...
>> +MODULE_DESCRIPTION("Load Xilinx FPGA firmware over SPI");
>>   
>
>What's the hw configuration?

I used Spartan-6 for testing. But the driver might work with other
Xilinx FPGAs, so I didn't add exact hw description.

Thanks for review and comments!

Anatolij
--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Michal Simek Feb. 21, 2017, 2:51 p.m. UTC | #5

On 21.2.2017 15:24, Anatolij Gustschin wrote:
> On Tue, 21 Feb 2017 15:01:32 +0100
> Michal Simek [e-mail] wrote:
> ...
>>> +static struct spi_driver xilinx_slave_spi_driver = {
>>> +	.driver = {
>>> +		.name = "xlnx-slave-spi",
>>> +		.owner = THIS_MODULE,  
>>
>> This should be done by core and it is checked by testing system.
> 
> Okay, will remove it.
> 
> ...
>>> +MODULE_LICENSE("GPL");  
>>
>> Above you have GPL v2 and here v2 or later.
>>
>> include/linux/module.h
>> *      "GPL"                           [GNU Public License v2 or later]
>> *      "GPL v2"                        [GNU Public License v2]
>>
>> What's the license then?
> 
> should be "GPL v2", I'll fix it next patch version.
> 
> ...
>>> +MODULE_DESCRIPTION("Load Xilinx FPGA firmware over SPI");
>>>   
>>
>> What's the hw configuration?
> 
> I used Spartan-6 for testing. But the driver might work with other
> Xilinx FPGAs, so I didn't add exact hw description.

ok. I expect this is spartan-6 next to another SoC and you use spi for
that another SoC and you connect.

I think it will be good to extend that example and add also spi device
node to see where this should be placed.

Thanks,
Michal





--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Anatolij Gustschin Feb. 21, 2017, 3:17 p.m. UTC | #6

On Tue, 21 Feb 2017 15:51:18 +0100
Michal Simek [e-mail] wrote:
...
>>> What's the hw configuration?  
>> 
>> I used Spartan-6 for testing. But the driver might work with other
>> Xilinx FPGAs, so I didn't add exact hw description.  
>
>ok. I expect this is spartan-6 next to another SoC and you use spi for
>that another SoC and you connect.

yes, it is MVEBU SoC, I'm using orion_spi driver. The connection is
unidirectional SPI.

>I think it will be good to extend that example and add also spi device
>node to see where this should be placed.

You mean the example in the binding description? I can add there
something like:

Usage example for full FPGA configuration:

fpga-region0 {
	compatible = "fpga-region";
	fpga-mgr = <&fpga_mgr_spi>;
	#address-cells = <0x1>;
	#size-cells = <0x1>;
};

&spi1 {
	status = "okay";

	fpga_mgr_spi: fpga-mgr@0 {
		compatible = "xlnx,fpga-slave-serial";
		reg = <0>;
		spi-max-frequency = <60000000>;
		spi-cpha;
		done-gpios = <&gpio0 7 GPIO_ACTIVE_HIGH>;
		prog_b-gpios = <&gpio0 26 GPIO_ACTIVE_LOW>;
	};
}; 

Thanks,
Anatolij
--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Michal Simek Feb. 21, 2017, 3:27 p.m. UTC | #7

On 21.2.2017 16:17, Anatolij Gustschin wrote:
> On Tue, 21 Feb 2017 15:51:18 +0100
> Michal Simek [e-mail] wrote:
> ...
>>>> What's the hw configuration?  
>>>
>>> I used Spartan-6 for testing. But the driver might work with other
>>> Xilinx FPGAs, so I didn't add exact hw description.  
>>
>> ok. I expect this is spartan-6 next to another SoC and you use spi for
>> that another SoC and you connect.
> 
> yes, it is MVEBU SoC, I'm using orion_spi driver. The connection is
> unidirectional SPI.

ok. That means only dout (din fpga) and clk out (fpga clk in) and gpio
handling for others signals.

> 
>> I think it will be good to extend that example and add also spi device
>> node to see where this should be placed.
> 
> You mean the example in the binding description? I can add there
> something like:

yes in binding doc.

> 
> Usage example for full FPGA configuration:
> 
> fpga-region0 {
> 	compatible = "fpga-region";
> 	fpga-mgr = <&fpga_mgr_spi>;
> 	#address-cells = <0x1>;
> 	#size-cells = <0x1>;
> };
> 
> &spi1 {
> 	status = "okay";
> 
> 	fpga_mgr_spi: fpga-mgr@0 {
> 		compatible = "xlnx,fpga-slave-serial";
> 		reg = <0>;
> 		spi-max-frequency = <60000000>;
> 		spi-cpha;
> 		done-gpios = <&gpio0 7 GPIO_ACTIVE_HIGH>;
> 		prog_b-gpios = <&gpio0 26 GPIO_ACTIVE_LOW>;
> 	};
> }; 

Right I would even replace &spi1 with full node to see content but this
is also good.

Thanks,
Michal


--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Anatolij Gustschin Feb. 21, 2017, 3:39 p.m. UTC | #8

On Tue, 21 Feb 2017 16:27:34 +0100
Michal Simek [e-mail] wrote:
...
>ok. That means only dout (din fpga) and clk out (fpga clk in) and gpio
>handling for others signals.

yes, exactly.

...
>> Usage example for full FPGA configuration:
>> 
>> fpga-region0 {
>> 	compatible = "fpga-region";
>> 	fpga-mgr = <&fpga_mgr_spi>;
>> 	#address-cells = <0x1>;
>> 	#size-cells = <0x1>;
>> };
>> 
>> &spi1 {
>> 	status = "okay";
>> 
>> 	fpga_mgr_spi: fpga-mgr@0 {
>> 		compatible = "xlnx,fpga-slave-serial";
>> 		reg = <0>;
>> 		spi-max-frequency = <60000000>;
>> 		spi-cpha;
>> 		done-gpios = <&gpio0 7 GPIO_ACTIVE_HIGH>;
>> 		prog_b-gpios = <&gpio0 26 GPIO_ACTIVE_LOW>;
>> 	};
>> };   
>
>Right I would even replace &spi1 with full node to see content but this
>is also good.

Okay, will add in patch v3.

Thanks,
Anatolij
--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Michal Simek Feb. 21, 2017, 6:53 p.m. UTC | #9

On 21.2.2017 16:39, Anatolij Gustschin wrote:
> On Tue, 21 Feb 2017 16:27:34 +0100
> Michal Simek [e-mail] wrote:
> ...
>> ok. That means only dout (din fpga) and clk out (fpga clk in) and gpio
>> handling for others signals.
> 
> yes, exactly.
> 
> ...
>>> Usage example for full FPGA configuration:
>>>
>>> fpga-region0 {
>>> 	compatible = "fpga-region";
>>> 	fpga-mgr = <&fpga_mgr_spi>;
>>> 	#address-cells = <0x1>;
>>> 	#size-cells = <0x1>;
>>> };
>>>
>>> &spi1 {
>>> 	status = "okay";
>>>
>>> 	fpga_mgr_spi: fpga-mgr@0 {
>>> 		compatible = "xlnx,fpga-slave-serial";
>>> 		reg = <0>;
>>> 		spi-max-frequency = <60000000>;
>>> 		spi-cpha;
>>> 		done-gpios = <&gpio0 7 GPIO_ACTIVE_HIGH>;
>>> 		prog_b-gpios = <&gpio0 26 GPIO_ACTIVE_LOW>;
>>> 	};
>>> };   
>>
>> Right I would even replace &spi1 with full node to see content but this
>> is also good.
> 
> Okay, will add in patch v3.

There is one thing I have checked ml505 - old virtex 5 board which has
this interface.
http://www-inst.eecs.berkeley.edu/~cs150/fa13/resources/ml50x_schematics.pdf

And I was chatting with one board guy and I think that done pin should
be optional not required because not all boards export it.

Then there is a question how to find out that programming was done and
that's something what I don't know. But will try to find out if you can
send any sequence over spi to find it out instead of gpio done pin.

Thanks,
Michal


--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Anatolij Gustschin Feb. 21, 2017, 8:24 p.m. UTC | #10

On Tue, 21 Feb 2017 19:53:40 +0100
Michal Simek [e-mail] wrote:
...
>There is one thing I have checked ml505 - old virtex 5 board which has
>this interface.
>http://www-inst.eecs.berkeley.edu/~cs150/fa13/resources/ml50x_schematics.pdf
>
>And I was chatting with one board guy and I think that done pin should
>be optional not required because not all boards export it.
>
>Then there is a question how to find out that programming was done and
>that's something what I don't know. But will try to find out if you can
>send any sequence over spi to find it out instead of gpio done pin.

maybe the boards without DONE pin use INIT_B pin with external pull-up
for CRC error reporting after configuration? Not sure if this is a
reliable way to ensure the configuration has completed.

Thanks,
Anatolij
--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index ce861a2..6b42786 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -33,6 +33,13 @@  config FPGA_MGR_SOCFPGA_A10
 	help
 	  FPGA manager driver support for Altera Arria10 SoCFPGA.
 
+config FPGA_MGR_XILINX_SPI
+	tristate "Xilinx Configuration over Slave Serial (SPI)"
+	depends on SPI
+	help
+	  FPGA manager driver support for Xilinx FPGA configuration
+	  over slave serial interface.
+
 config FPGA_MGR_ZYNQ_FPGA
 	tristate "Xilinx Zynq FPGA"
 	depends on ARCH_ZYNQ || COMPILE_TEST
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index 8df07bc..7582c5a 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -8,6 +8,7 @@  obj-$(CONFIG_FPGA)			+= fpga-mgr.o
 # FPGA Manager Drivers
 obj-$(CONFIG_FPGA_MGR_SOCFPGA)		+= socfpga.o
 obj-$(CONFIG_FPGA_MGR_SOCFPGA_A10)	+= socfpga-a10.o
+obj-$(CONFIG_FPGA_MGR_XILINX_SPI)	+= xilinx-spi.o
 obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA)	+= zynq-fpga.o
 
 # FPGA Bridge Drivers
diff --git a/drivers/fpga/xilinx-spi.c b/drivers/fpga/xilinx-spi.c
new file mode 100644
index 0000000..16b3fa2
--- /dev/null
+++ b/drivers/fpga/xilinx-spi.c
@@ -0,0 +1,181 @@ 
+/*
+ * Xilinx Spartan6 Slave Serial SPI Driver
+ *
+ * Copyright (C) 2017 DENX Software Engineering
+ *
+ * Anatolij Gustschin <[e-mail]>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * Manage Xilinx FPGA firmware that is loaded over SPI using
+ * the slave serial configuration interface.
+ */
+
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/fpga/fpga-mgr.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/of.h>
+#include <linux/spi/spi.h>
+#include <linux/sizes.h>
+
+#define FPGA_MIN_CFG_WAIT_US	10
+
+struct xilinx_spi_conf {
+	struct spi_device *spi;
+	struct gpio_desc *prog_b;
+	struct gpio_desc *done;
+};
+
+static enum fpga_mgr_states xilinx_spi_state(struct fpga_manager *mgr)
+{
+	struct xilinx_spi_conf *conf = mgr->priv;
+
+	if (!gpiod_get_value(conf->done))
+		return FPGA_MGR_STATE_RESET;
+
+	return FPGA_MGR_STATE_UNKNOWN;
+}
+
+static int xilinx_spi_write_init(struct fpga_manager *mgr,
+				 struct fpga_image_info *info,
+				 const char *buf, size_t count)
+{
+	struct xilinx_spi_conf *conf = mgr->priv;
+
+	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
+		dev_err(&mgr->dev, "Partial reconfiguration not supported.\n");
+		return -EINVAL;
+	}
+
+	gpiod_set_value(conf->prog_b, 1);
+
+	udelay(1); /* min is 500 ns */
+
+	gpiod_set_value(conf->prog_b, 0);
+
+	if (gpiod_get_value(conf->done)) {
+		dev_err(&mgr->dev, "Unexpected DONE pin state...\n");
+		return -EIO;
+	}
+
+	/* program latency */
+	usleep_range(1000, 1500);
+	return 0;
+}
+
+static int xilinx_spi_write(struct fpga_manager *mgr, const char *buf,
+			    size_t count)
+{
+	struct xilinx_spi_conf *conf = mgr->priv;
+	const char *fw_data = buf;
+	const char *fw_data_end = fw_data + count;
+
+	while (fw_data < fw_data_end) {
+		int ret;
+		size_t stride = min(fw_data_end - fw_data, SZ_4K);
+
+		ret = spi_write(conf->spi, fw_data, stride);
+		if (ret) {
+			dev_err(&mgr->dev, "SPI error in firmware write: %d\n",
+				ret);
+			return ret;
+		}
+		fw_data += stride;
+	}
+
+	return 0;
+}
+
+static int xilinx_spi_write_complete(struct fpga_manager *mgr,
+				     struct fpga_image_info *info)
+{
+	struct xilinx_spi_conf *conf = mgr->priv;
+	int wait;
+
+	if (gpiod_get_value(conf->done))
+		return 0;
+
+	wait = info->config_complete_timeout_us / FPGA_MIN_CFG_WAIT_US;
+	if (info->config_complete_timeout_us % FPGA_MIN_CFG_WAIT_US)
+		wait += 1;
+
+	while (wait--) {
+		usleep_range(FPGA_MIN_CFG_WAIT_US, FPGA_MIN_CFG_WAIT_US + 5);
+
+		if (gpiod_get_value(conf->done))
+			return 0;
+	}
+
+	dev_err(&mgr->dev, "Timeout after config data transfer.\n");
+	return -ETIMEDOUT;
+}
+
+static const struct fpga_manager_ops xilinx_spi_ops = {
+	.state = xilinx_spi_state,
+	.write_init = xilinx_spi_write_init,
+	.write = xilinx_spi_write,
+	.write_complete = xilinx_spi_write_complete,
+};
+
+static int xilinx_spi_probe(struct spi_device *spi)
+{
+	struct xilinx_spi_conf *conf;
+
+	conf = devm_kzalloc(&spi->dev, sizeof(*conf), GFP_KERNEL);
+	if (!conf)
+		return -ENOMEM;
+
+	conf->spi = spi;
+
+	/* PROGRAM_B is active low */
+	conf->prog_b = devm_gpiod_get(&spi->dev, "prog_b", GPIOD_OUT_LOW);
+	if (IS_ERR(conf->prog_b)) {
+		dev_err(&spi->dev, "Failed to get PROGRAM_B gpio: %ld\n",
+			PTR_ERR(conf->prog_b));
+		return PTR_ERR(conf->prog_b);
+	}
+
+	conf->done = devm_gpiod_get(&spi->dev, "done", GPIOD_IN);
+	if (IS_ERR(conf->done)) {
+		dev_err(&spi->dev, "Failed to get DONE gpio: %ld\n",
+			PTR_ERR(conf->done));
+		return PTR_ERR(conf->done);
+	}
+
+	return fpga_mgr_register(&spi->dev, "Xilinx Slave Serial FPGA Manager",
+				 &xilinx_spi_ops, conf);
+}
+
+static int xilinx_spi_remove(struct spi_device *spi)
+{
+	fpga_mgr_unregister(&spi->dev);
+
+	return 0;
+}
+
+static const struct of_device_id xlnx_spi_of_match[] = {
+	{ .compatible = "xlnx,fpga-slave-serial", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, xlnx_spi_of_match);
+
+static struct spi_driver xilinx_slave_spi_driver = {
+	.driver = {
+		.name = "xlnx-slave-spi",
+		.owner = THIS_MODULE,
+		.of_match_table = of_match_ptr(xlnx_spi_of_match),
+	},
+	.probe = xilinx_spi_probe,
+	.remove = xilinx_spi_remove,
+};
+
+module_spi_driver(xilinx_slave_spi_driver)
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Anatolij Gustschin <[e-mail]>");
+MODULE_DESCRIPTION("Load Xilinx FPGA firmware over SPI");