diff mbox series

[v8,1/2] fpga: microchip-spi: add Microchip MPF FPGA manager

Message ID 20220322191552.13156-2-i.bornyakov@metrotek.ru (mailing list archive)
State New
Headers show
Series Microchip Polarfire FPGA manager | expand

Commit Message

Ivan Bornyakov March 22, 2022, 7:15 p.m. UTC
Add support to the FPGA manager for programming Microchip Polarfire
FPGAs over slave SPI interface with .dat formatted bitsream image.

Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
---
 drivers/fpga/Kconfig         |  10 +
 drivers/fpga/Makefile        |   1 +
 drivers/fpga/microchip-spi.c | 448 +++++++++++++++++++++++++++++++++++
 3 files changed, 459 insertions(+)
 create mode 100644 drivers/fpga/microchip-spi.c

Comments

Conor Dooley March 30, 2022, 2:37 p.m. UTC | #1
Hey Ivan,
Been testing this and generated a couple questions.
I've put them inline where they were relevant.
Thanks,
Conor.

On 22/03/2022 19:15, Ivan Bornyakov wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Add support to the FPGA manager for programming Microchip Polarfire
> FPGAs over slave SPI interface with .dat formatted bitsream image.
> 
> Signed-off-by: Ivan Bornyakov <i.bornyakov@metrotek.ru>
> ---
>   drivers/fpga/Kconfig         |  10 +
>   drivers/fpga/Makefile        |   1 +
>   drivers/fpga/microchip-spi.c | 448 +++++++++++++++++++++++++++++++++++
>   3 files changed, 459 insertions(+)
>   create mode 100644 drivers/fpga/microchip-spi.c
> 
> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> index 26025dbab353..791ecf48503a 100644
> --- a/drivers/fpga/Kconfig
> +++ b/drivers/fpga/Kconfig
> @@ -248,4 +248,14 @@ config FPGA_MGR_VERSAL_FPGA
>            configure the programmable logic(PL).
> 
>            To compile this as a module, choose M here.
> +
> +config FPGA_MGR_MICROCHIP_SPI
> +       tristate "Microchip Polarfire SPI FPGA manager"
> +       depends on SPI
> +       select CRC_CCITT
> +       help
> +         FPGA manager driver support for Microchip Polarfire FPGAs
> +         programming over slave SPI interface with .dat formatted
> +         bitstream image.
> +
>   endif # FPGA
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index 4da5273948df..fcb389ca4873 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -19,6 +19,7 @@ obj-$(CONFIG_FPGA_MGR_XILINX_SPI)     += xilinx-spi.o
>   obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA)       += zynq-fpga.o
>   obj-$(CONFIG_FPGA_MGR_ZYNQMP_FPGA)     += zynqmp-fpga.o
>   obj-$(CONFIG_FPGA_MGR_VERSAL_FPGA)      += versal-fpga.o
> +obj-$(CONFIG_FPGA_MGR_MICROCHIP_SPI)   += microchip-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/microchip-spi.c b/drivers/fpga/microchip-spi.c
> new file mode 100644
> index 000000000000..5e79677c29f4
> --- /dev/null
> +++ b/drivers/fpga/microchip-spi.c
> @@ -0,0 +1,448 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Microchip Polarfire FPGA programming over slave SPI interface.
> + */
> +
> +#include <linux/crc-ccitt.h>
> +#include <linux/delay.h>
> +#include <linux/fpga/fpga-mgr.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/spi/spi.h>
> +
> +#define        MPF_SPI_ISC_ENABLE      0x0B
> +#define        MPF_SPI_ISC_DISABLE     0x0C
> +#define        MPF_SPI_READ_STATUS     0x00
> +#define        MPF_SPI_READ_DATA       0x01
> +#define        MPF_SPI_FRAME_INIT      0xAE
> +#define        MPF_SPI_FRAME           0xEE
> +#define        MPF_SPI_PRG_MODE        0x01
> +#define        MPF_SPI_RELEASE         0x23
> +
> +#define        MPF_SPI_FRAME_SIZE      16
> +
> +#define        MPF_HEADER_SIZE_OFFSET  24
> +#define        MPF_DATA_SIZE_OFFSET    55
> +
> +#define        MPF_LOOKUP_TABLE_RECORD_SIZE            9
> +#define        MPF_LOOKUP_TABLE_BLOCK_ID_OFFSET        0
> +#define        MPF_LOOKUP_TABLE_BLOCK_START_OFFSET     1
> +
> +#define        MPF_COMPONENTS_SIZE_ID  5
> +#define        MPF_BITSTREAM_ID        8
> +
> +#define        MPF_BITS_PER_COMPONENT_SIZE     22
> +
> +#define        MPF_STATUS_POLL_TIMEOUT 1000
> +#define        MPF_STATUS_BUSY         BIT(0)
> +#define        MPF_STATUS_READY        BIT(1)
> +
> +struct mpf_priv {
> +       struct spi_device *spi;
> +       bool program_mode;
> +};
> +
> +static enum fpga_mgr_states mpf_ops_state(struct fpga_manager *mgr)
> +{
> +       struct mpf_priv *priv = mgr->priv;
> +       struct spi_device *spi;
> +       bool program_mode;
> +       ssize_t status;
> +
> +       program_mode = priv->program_mode;
> +       spi = priv->spi;
> +
> +       status = spi_w8r8(spi, MPF_SPI_READ_STATUS);
> +
> +       if (!program_mode && !status)
> +               return FPGA_MGR_STATE_OPERATING;
> +
> +       return FPGA_MGR_STATE_UNKNOWN;
> +}
> +
> +static int poll_status_not_busy(struct spi_device *spi, u8 mask)
> +{
> +       ssize_t status, timeout = MPF_STATUS_POLL_TIMEOUT;
> +
> +       while (timeout--) {
> +               status = spi_w8r8(spi, MPF_SPI_READ_STATUS);> +               if (status < 0)
> +                       return status;
> +
> +               if (!(status & MPF_STATUS_BUSY) && (!mask || (status & mask)))
> +                       return status;
> +
> +               usleep_range(1000, 2000);
> +       }
> +
> +       return -EBUSY;
> +}
> +
> +static int mpf_spi_write(struct spi_device *spi, const void *buf, size_t buf_size)
> +{
> +       int status = poll_status_not_busy(spi, 0);
> +
> +       if (status < 0)
> +               return status;
> +
> +       return spi_write(spi, buf, buf_size);
> +}
> +
> +static int mpf_spi_write_then_read(struct spi_device *spi,
> +                                  const void *txbuf, size_t txbuf_size,
> +                                  void *rxbuf, size_t rxbuf_size)
> +{
> +       const u8 read_command[] = { MPF_SPI_READ_DATA };
> +       int ret;
> +
> +       ret = mpf_spi_write(spi, txbuf, txbuf_size);
> +       if (ret)
> +               return ret;
> +
> +       ret = poll_status_not_busy(spi, MPF_STATUS_READY);
> +       if (ret < 0)
> +               return ret;
> +
> +       return spi_write_then_read(spi, read_command, sizeof(read_command),
> +                                  rxbuf, rxbuf_size);
> +}
> +
> +static int mpf_ops_write_init(struct fpga_manager *mgr,
> +                             struct fpga_image_info *info, const char *buf,
> +                             size_t count)
> +{
> +       const u8 program_mode[] = { MPF_SPI_FRAME_INIT, MPF_SPI_PRG_MODE };
> +       const u8 isc_en_command[] = { MPF_SPI_ISC_ENABLE };
> +       struct mpf_priv *priv = mgr->priv;
> +       struct device *dev = &mgr->dev;
> +       struct spi_device *spi;
> +       u32 isc_ret;
> +       int ret;
> +
> +       if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
> +               dev_err(dev, "Partial reconfiguration is not supported\n");
> +               return -EOPNOTSUPP;
> +       }
> +
> +       spi = priv->spi;
> +
> +       ret = mpf_spi_write_then_read(spi, isc_en_command, sizeof(isc_en_command),
> +                                     &isc_ret, sizeof(isc_ret));
> +       if (ret || isc_ret) {
> +               dev_err(dev, "Failed to enable ISC: %d\n", ret ? : isc_ret);
> +               return -EFAULT;
> +       }

So, my test board for this has had a PolarFire SoC, not a standard
PolarFire. I ran into some problems with the ISC enable code, due to
a sequence error. After sending the SPI_ISC_ENABLE, you then do a
poll_status_not_busy to hold until you see a STATUS_READY.
poll_status_not_busy does a w8r8 to request and then read the status,
and you expect a sequence as below:

op:     w       w   r   w   r
M:	0xB	0x0	0x0
S:   		    0x1	    0x2

I could not get past this check & it would just poll until the
timeout. What I saw on a protocol analyser was more like so:

op:     w       w   r    w   r
M:	0xB	0x0      0x0
S:   		0x1 0x0  0x2 0x0

So the read in that w8r8 would always get a zero back and then time out.
Changing the poll function (just for isc) to only read gave:

op:     w       r       r
M:	0xB	0x0	0x0
S:   		0x1	0x2

For the code after the ISC enable, I reverted to your implementation
of the poll function & the rest of the programming sequence ran.

I spoke to the guys that wrote the HW about this, and they said that
reading the status back *as* the 0x0 the poll command is clocked in is
the expected behaviour.
They also said that MPF should work identically to an MPFS and I was unable
to find a documented difference between MPF and MPFS other than the envm,
which is an optional component anyway.

But I can only assume that what you were doing worked for you, so if
you could possibly share some waveforms of the write_init sequence
that'd be great. Or if there is something that you think I am
overlooking, please let me know.

> +
> +       ret = mpf_spi_write(spi, program_mode, sizeof(program_mode));
> +       if (ret) {
> +               dev_err(dev, "Failed to enter program mode: %d\n", ret);
> +               return ret;
> +       }
> +
> +       priv->program_mode = true;
> +
> +       return 0;
> +}
> +
> +static int get_sgt_data(struct sg_table *sgt, size_t offset, void *data,
> +                       size_t data_size)
> +{
> +       struct sg_mapping_iter miter;
> +       size_t remainder;
> +
> +       sg_miter_start(&miter, sgt->sgl, sgt->nents, SG_MITER_FROM_SG);
> +       while (sg_miter_next(&miter)) {
> +               if (offset < miter.length)
> +                       break;
> +
> +               offset -= miter.length;
> +       }
> +
> +       if (!miter.addr) {
> +               sg_miter_stop(&miter);
> +               return -EFAULT;
> +       }
> +
> +       remainder = max((ssize_t)0, (ssize_t)offset + (ssize_t)data_size -
> +                       (ssize_t)miter.length);
> +
> +       memcpy(data, miter.addr + offset, data_size - remainder);
> +
> +       while (remainder && sg_miter_next(&miter)) {
> +               memcpy(data + data_size - remainder, miter.addr,
> +                      min(remainder, miter.length));
> +               remainder = max((ssize_t)0,
> +                               (ssize_t)remainder - (ssize_t)miter.length);
> +       }
> +       sg_miter_stop(&miter);
> +
> +       if (remainder)
> +               return -EFAULT;
> +
> +       return 0;
> +}
> +
> +static ssize_t lookup_block_start(struct sg_table *sgt, int id)
> +{
> +       size_t block_start_offset, block_id_offset, i;
> +       u8 header_size, blocks_num, block_id;
> +       u32 block_start;
> +       int ret;
> +
> +       ret = get_sgt_data(sgt, MPF_HEADER_SIZE_OFFSET, &header_size,
> +                          sizeof(header_size));
> +       if (ret)
> +               return ret;
> +
> +       ret = get_sgt_data(sgt, header_size - 1, &blocks_num,
> +                          sizeof(blocks_num));
> +       if (ret)
> +               return ret;
> +
> +       for (i = 0; i < blocks_num; i++) {
> +               block_id_offset = header_size +
> +                                 i * MPF_LOOKUP_TABLE_RECORD_SIZE +
> +                                 MPF_LOOKUP_TABLE_BLOCK_ID_OFFSET;
> +
> +               ret = get_sgt_data(sgt, block_id_offset, &block_id,
> +                                  sizeof(block_id));
> +               if (ret)
> +                       return ret;
> +
> +               if (block_id == id) {
> +                       block_start_offset = header_size +
> +                                            i * MPF_LOOKUP_TABLE_RECORD_SIZE +
> +                                            MPF_LOOKUP_TABLE_BLOCK_START_OFFSET;
> +
> +                       ret = get_sgt_data(sgt, block_start_offset, &block_start,
> +                                          sizeof(block_start));
> +                       if (ret)
> +                               return ret;
> +
> +                       return le32_to_cpu(block_start);
> +               }
> +       }
> +
> +       return -EFAULT;
> +}
> +
> +static ssize_t parse_bitstream_size(struct sg_table *sgt)
> +{
> +       size_t component_size_byte_num, component_size_byte_off,
> +              i, bitstream_size = 0;
> +       ssize_t components_size_start;
> +       u16 components_num;
> +       u32 component_size;
> +       int ret;
> +
> +       ret = get_sgt_data(sgt, MPF_DATA_SIZE_OFFSET, &components_num,
> +                          sizeof(components_num));
> +       if (ret)
> +               return ret;
> +
> +       components_num = le16_to_cpu(components_num);

Not expecting you to do this at all since you don't have one, but this
will need to be extended for polarfire soc since components_* doesn't
include the envm which has its own parameter.

> +
> +       components_size_start = lookup_block_start(sgt, MPF_COMPONENTS_SIZE_ID);
> +       if (components_size_start < 0)
> +               return components_size_start;
> +
> +       for (i = 0; i < components_num; i++) {
> +               component_size_byte_num =
> +                       (i * MPF_BITS_PER_COMPONENT_SIZE) / BITS_PER_BYTE;
> +               component_size_byte_off =
> +                       (i * MPF_BITS_PER_COMPONENT_SIZE) % BITS_PER_BYTE;
> +
> +               ret = get_sgt_data(sgt, components_size_start +
> +                                  component_size_byte_num,
> +                                  &component_size, sizeof(component_size));
> +               if (ret)
> +                       return ret;
> +
> +               component_size = le32_to_cpu(component_size);
> +               component_size >>= component_size_byte_off;
> +               component_size &= GENMASK(MPF_BITS_PER_COMPONENT_SIZE - 1, 0);
> +
> +               bitstream_size += component_size;
> +       }
> +
> +       return bitstream_size;
> +}
> +
> +static u16 crc_ccitt_sg(struct sg_table *sgt)
> +{
> +       struct sg_mapping_iter miter;
> +       u16 crc = 0;
> +
> +       sg_miter_start(&miter, sgt->sgl, sgt->nents, SG_MITER_FROM_SG);
> +       while (sg_miter_next(&miter))
> +               crc = crc_ccitt(crc, miter.addr, miter.length);
> +       sg_miter_stop(&miter);
> +
> +       return crc;
> +}
> +
> +static int mpf_ops_write_sg(struct fpga_manager *mgr, struct sg_table *sgt)
> +{
> +       u8 tmp_buf[MPF_SPI_FRAME_SIZE + 1] = { MPF_SPI_FRAME, };
> +       ssize_t bitstream_start, bitstream_size;
> +       struct mpf_priv *priv = mgr->priv;
> +       struct device *dev = &mgr->dev;
> +       struct sg_mapping_iter miter;
> +       struct spi_device *spi;
> +       size_t remainder;
> +       int ret, i;
> +
> +       if (crc_ccitt_sg(sgt)) {
> +               dev_err(dev, "CRC error\n");
> +               return -EINVAL;
> +       }
> +
> +       bitstream_start = lookup_block_start(sgt, MPF_BITSTREAM_ID);
> +       if (bitstream_start < 0) {
> +               dev_err(dev, "Failed to find bitstream start %zd\n",
> +                       bitstream_start);
> +               return bitstream_start;
> +       }
> +
> +       bitstream_size = parse_bitstream_size(sgt);
> +       if (bitstream_size < 0) {
> +               dev_err(dev, "Failed to parse bitstream size %zd\n",
> +                       bitstream_size);
> +               return bitstream_size;
> +       }
> +
> +       spi = priv->spi;
> +
> +       sg_miter_start(&miter, sgt->sgl, sgt->nents, SG_MITER_FROM_SG);> +       for (i = 0; i < bitstream_size; i++) {
> +               do {
> +                       if (bitstream_start < miter.length)
> +                               break;
> +
> +                       bitstream_start -= miter.length;
> +               } while (sg_miter_next(&miter));
> +
> +               remainder = max((ssize_t)0, bitstream_start +
> +                               (ssize_t)MPF_SPI_FRAME_SIZE -
> +                               (ssize_t)miter.length);
> +
> +               memcpy(tmp_buf + 1, miter.addr + bitstream_start,
> +                      MPF_SPI_FRAME_SIZE - remainder);
> +
> +               while (remainder) {
> +                       bitstream_start -= miter.length;
> +
> +                       if (sg_miter_next(&miter)) {
> +                               memcpy(tmp_buf + 1 + MPF_SPI_FRAME_SIZE -
> +                                      remainder, miter.addr,
> +                                      min(remainder, miter.length));
> +                               remainder = max((ssize_t)0, (ssize_t)remainder -
> +                                               (ssize_t)miter.length);
> +                       } else {
> +                               dev_err(dev,
> +                                       "Bitstream outruns firmware image. Frame %d of %zd\n",
> +                                       i, bitstream_size);
> +                               sg_miter_stop(&miter);
> +                               return -EFAULT;
> +                       }
> +               }
> +
> +               bitstream_start += MPF_SPI_FRAME_SIZE;
> +
> +               ret = mpf_spi_write(spi, tmp_buf, sizeof(tmp_buf));
> +               if (ret) {
> +                       dev_err(dev,
> +                               "Failed to write bitstream frame number %d of %zd\n",
> +                               i, bitstream_size);
> +                       sg_miter_stop(&miter);
> +                       return ret;
> +               }
> +       }
> +       sg_miter_stop(&miter);
> +
> +       return 0;
> +}
> +
> +static int mpf_ops_write_complete(struct fpga_manager *mgr,
> +                                 struct fpga_image_info *info)
> +{
> +       const u8 isc_dis_command[] = { MPF_SPI_ISC_DISABLE };
> +       const u8 release_command[] = { MPF_SPI_RELEASE };
> +       struct mpf_priv *priv = mgr->priv;
> +       struct device *dev = &mgr->dev;
> +       struct spi_device *spi;
> +       int ret;
> +
> +       spi = priv->spi;
> +
> +       ret = mpf_spi_write(spi, isc_dis_command, sizeof(isc_dis_command));
> +       if (ret) {
> +               dev_err(dev, "Failed to disable ISC: %d\n", ret);
> +               return ret;
> +       }
> +
> +       usleep_range(1000, 2000);
> +
> +       ret = mpf_spi_write(spi, release_command, sizeof(release_command));
> +       if (ret) {
> +               dev_err(dev, "Failed to exit program mode: %d\n", ret);
> +               return ret;
> +       }

While I was debugging the sequence issue, it got to this stage of the
transfer several times, but hadn't actually programmed the device.
It seems that poll_status_not_busy covers checking ERR/VIOLATION from
polling the status, but again that check is avoided due to the
sequence issues I mentioned above.

It might be nice to check the device certificate, but I am not sure
if that fits the flow for fpga managers.

> +
> +       priv->program_mode = false;
> +
> +       return 0;
> +}
> +
> +static const struct fpga_manager_ops mpf_ops = {
> +       .state = mpf_ops_state,
> +       .write_init = mpf_ops_write_init,
> +       .write_sg = mpf_ops_write_sg,
> +       .write_complete = mpf_ops_write_complete,
> +};
> +
> +static int mpf_probe(struct spi_device *spi)
> +{
> +       struct device *dev = &spi->dev;
> +       struct fpga_manager *mgr;
> +       struct mpf_priv *priv;
> +
> +       priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +       if (!priv)
> +               return -ENOMEM;
> +
> +       priv->spi = spi;
> +
> +       mgr = devm_fpga_mgr_register(dev, "Microchip Polarfire SPI FPGA Manager",
> +                                    &mpf_ops, priv);
> +
> +       return PTR_ERR_OR_ZERO(mgr);
> +}
> +
> +static const struct spi_device_id mpf_spi_ids[] = {
> +       { .name = "mpf-spi-fpga-mgr", },
> +       {},
> +};
> +MODULE_DEVICE_TABLE(spi, mpf_spi_ids);
> +
> +#if IS_ENABLED(CONFIG_OF)
> +static const struct of_device_id mpf_of_ids[] = {
> +       { .compatible = "microchip,mpf-spi-fpga-mgr" },
> +       {},
> +};
> +MODULE_DEVICE_TABLE(of, mpf_of_ids);
> +#endif /* IS_ENABLED(CONFIG_OF) */
> +
> +static struct spi_driver mpf_driver = {
> +       .probe = mpf_probe,
> +       .id_table = mpf_spi_ids,
> +       .driver = {
> +               .name = "microchip_mpf_spi_fpga_mgr",
> +               .of_match_table = of_match_ptr(mpf_of_ids),
> +       },
> +};
> +
> +module_spi_driver(mpf_driver);
> +
> +MODULE_DESCRIPTION("Microchip Polarfire SPI FPGA Manager");
> +MODULE_LICENSE("GPL");
> --
> 2.34.1
> 
>
Ivan Bornyakov March 30, 2022, 3:48 p.m. UTC | #2
Hi, Conor!

On Wed, Mar 30, 2022 at 02:37:05PM +0000, Conor.Dooley@microchip.com wrote:
> Hey Ivan,
> Been testing this and generated a couple questions.
> I've put them inline where they were relevant.
> Thanks,
> Conor.
> 
> On 22/03/2022 19:15, Ivan Bornyakov wrote:
> > 
> > ... snip ...
> > 
> > +static int mpf_ops_write_init(struct fpga_manager *mgr,
> > +                             struct fpga_image_info *info, const char *buf,
> > +                             size_t count)
> > +{
> > +       const u8 program_mode[] = { MPF_SPI_FRAME_INIT, MPF_SPI_PRG_MODE };
> > +       const u8 isc_en_command[] = { MPF_SPI_ISC_ENABLE };
> > +       struct mpf_priv *priv = mgr->priv;
> > +       struct device *dev = &mgr->dev;
> > +       struct spi_device *spi;
> > +       u32 isc_ret;
> > +       int ret;
> > +
> > +       if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
> > +               dev_err(dev, "Partial reconfiguration is not supported\n");
> > +               return -EOPNOTSUPP;
> > +       }
> > +
> > +       spi = priv->spi;
> > +
> > +       ret = mpf_spi_write_then_read(spi, isc_en_command, sizeof(isc_en_command),
> > +                                     &isc_ret, sizeof(isc_ret));
> > +       if (ret || isc_ret) {
> > +               dev_err(dev, "Failed to enable ISC: %d\n", ret ? : isc_ret);
> > +               return -EFAULT;
> > +       }
> 
> So, my test board for this has had a PolarFire SoC, not a standard
> PolarFire. I ran into some problems with the ISC enable code, due to
> a sequence error. After sending the SPI_ISC_ENABLE, you then do a
> poll_status_not_busy to hold until you see a STATUS_READY.
> poll_status_not_busy does a w8r8 to request and then read the status,
> and you expect a sequence as below:
> 
> op:     w       w   r   w   r
> M:	0xB	0x0	0x0
> S:   		    0x1	    0x2
> 
> I could not get past this check & it would just poll until the
> timeout. What I saw on a protocol analyser was more like so:
> 
> op:     w       w   r    w   r
> M:	0xB	0x0      0x0
> S:   		0x1 0x0  0x2 0x0
> 
> So the read in that w8r8 would always get a zero back and then time out.
> Changing the poll function (just for isc) to only read gave:
> 
> op:     w       r       r
> M:	0xB	0x0	0x0
> S:   		0x1	0x2
> 
> For the code after the ISC enable, I reverted to your implementation
> of the poll function & the rest of the programming sequence ran.
> 
> I spoke to the guys that wrote the HW about this, and they said that
> reading the status back *as* the 0x0 the poll command is clocked in is
> the expected behaviour.
> They also said that MPF should work identically to an MPFS and I was unable
> to find a documented difference between MPF and MPFS other than the envm,
> which is an optional component anyway.
> 
> But I can only assume that what you were doing worked for you, so if
> you could possibly share some waveforms of the write_init sequence
> that'd be great. Or if there is something that you think I am
> overlooking, please let me know.
> 

If you replace poll_status_not_busy() function with this code:

static int poll_status_not_busy(struct spi_device *spi, u8 mask)
{
	u8 status, status_command = MPF_SPI_READ_STATUS;
	int ret, timeout = MPF_STATUS_POLL_TIMEOUT;
	struct spi_transfer xfer = {
		.tx_buf = &status_command,
		.rx_buf = &status,
		.len = 1,
	};

	while (timeout--) {
		ret = spi_sync_transfer(spi, &xfer, 1);
		if (ret < 0)
			return ret;

		if (!(status & MPF_STATUS_BUSY) && (!mask || (status & mask)))
			return status;

		usleep_range(1000, 2000);
	}

	return -EBUSY;
}

Will it work for you? It is still works in my case.

> > 
> > ... snip ...
> > 
> > +static ssize_t parse_bitstream_size(struct sg_table *sgt)
> > +{
> > +       size_t component_size_byte_num, component_size_byte_off,
> > +              i, bitstream_size = 0;
> > +       ssize_t components_size_start;
> > +       u16 components_num;
> > +       u32 component_size;
> > +       int ret;
> > +
> > +       ret = get_sgt_data(sgt, MPF_DATA_SIZE_OFFSET, &components_num,
> > +                          sizeof(components_num));
> > +       if (ret)
> > +               return ret;
> > +
> > +       components_num = le16_to_cpu(components_num);
> 
> Not expecting you to do this at all since you don't have one, but this
> will need to be extended for polarfire soc since components_* doesn't
> include the envm which has its own parameter.
> 
> > +
> > +       components_size_start = lookup_block_start(sgt, MPF_COMPONENTS_SIZE_ID);
> > +       if (components_size_start < 0)
> > +               return components_size_start;
> > +
> > +       for (i = 0; i < components_num; i++) {
> > +               component_size_byte_num =
> > +                       (i * MPF_BITS_PER_COMPONENT_SIZE) / BITS_PER_BYTE;
> > +               component_size_byte_off =
> > +                       (i * MPF_BITS_PER_COMPONENT_SIZE) % BITS_PER_BYTE;
> > +
> > +               ret = get_sgt_data(sgt, components_size_start +
> > +                                  component_size_byte_num,
> > +                                  &component_size, sizeof(component_size));
> > +               if (ret)
> > +                       return ret;
> > +
> > +               component_size = le32_to_cpu(component_size);
> > +               component_size >>= component_size_byte_off;
> > +               component_size &= GENMASK(MPF_BITS_PER_COMPONENT_SIZE - 1, 0);
> > +
> > +               bitstream_size += component_size;
> > +       }
> > +
> > +       return bitstream_size;
> > +}
> > 
> > ... snip ...
> > 
> > +static int mpf_ops_write_complete(struct fpga_manager *mgr,
> > +                                 struct fpga_image_info *info)
> > +{
> > +       const u8 isc_dis_command[] = { MPF_SPI_ISC_DISABLE };
> > +       const u8 release_command[] = { MPF_SPI_RELEASE };
> > +       struct mpf_priv *priv = mgr->priv;
> > +       struct device *dev = &mgr->dev;
> > +       struct spi_device *spi;
> > +       int ret;
> > +
> > +       spi = priv->spi;
> > +
> > +       ret = mpf_spi_write(spi, isc_dis_command, sizeof(isc_dis_command));
> > +       if (ret) {
> > +               dev_err(dev, "Failed to disable ISC: %d\n", ret);
> > +               return ret;
> > +       }
> > +
> > +       usleep_range(1000, 2000);
> > +
> > +       ret = mpf_spi_write(spi, release_command, sizeof(release_command));
> > +       if (ret) {
> > +               dev_err(dev, "Failed to exit program mode: %d\n", ret);
> > +               return ret;
> > +       }
> 
> While I was debugging the sequence issue, it got to this stage of the
> transfer several times, but hadn't actually programmed the device.
> It seems that poll_status_not_busy covers checking ERR/VIOLATION from
> polling the status, but again that check is avoided due to the
> sequence issues I mentioned above.
> 
> It might be nice to check the device certificate, but I am not sure
> if that fits the flow for fpga managers.
> 

Noted.
Conor Dooley March 31, 2022, 9:13 a.m. UTC | #3
On 30/03/2022 15:48, Ivan Bornyakov wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi, Conor!
> 
> On Wed, Mar 30, 2022 at 02:37:05PM +0000, Conor.Dooley@microchip.com wrote:
>> Hey Ivan,
>> Been testing this and generated a couple questions.
>> I've put them inline where they were relevant.
>> Thanks,
>> Conor.
>>
>> On 22/03/2022 19:15, Ivan Bornyakov wrote:
>>>
>>> ... snip ...
>>>
>>> +static int mpf_ops_write_init(struct fpga_manager *mgr,
>>> +                             struct fpga_image_info *info, const char *buf,
>>> +                             size_t count)
>>> +{
>>> +       const u8 program_mode[] = { MPF_SPI_FRAME_INIT, MPF_SPI_PRG_MODE };
>>> +       const u8 isc_en_command[] = { MPF_SPI_ISC_ENABLE };
>>> +       struct mpf_priv *priv = mgr->priv;
>>> +       struct device *dev = &mgr->dev;
>>> +       struct spi_device *spi;
>>> +       u32 isc_ret;
>>> +       int ret;
>>> +
>>> +       if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
>>> +               dev_err(dev, "Partial reconfiguration is not supported\n");
>>> +               return -EOPNOTSUPP;
>>> +       }
>>> +
>>> +       spi = priv->spi;
>>> +
>>> +       ret = mpf_spi_write_then_read(spi, isc_en_command, sizeof(isc_en_command),
>>> +                                     &isc_ret, sizeof(isc_ret));
>>> +       if (ret || isc_ret) {
>>> +               dev_err(dev, "Failed to enable ISC: %d\n", ret ? : isc_ret);
>>> +               return -EFAULT;
>>> +       }
>>
>> So, my test board for this has had a PolarFire SoC, not a standard
>> PolarFire. I ran into some problems with the ISC enable code, due to
>> a sequence error. After sending the SPI_ISC_ENABLE, you then do a
>> poll_status_not_busy to hold until you see a STATUS_READY.
>> poll_status_not_busy does a w8r8 to request and then read the status,
>> and you expect a sequence as below:
>>
>> op:     w       w   r   w   r
>> M:    0xB     0x0     0x0
>> S:                0x1     0x2
>>
>> I could not get past this check & it would just poll until the
>> timeout. What I saw on a protocol analyser was more like so:
>>
>> op:     w       w   r    w   r
>> M:    0xB     0x0      0x0
>> S:            0x1 0x0  0x2 0x0
>>
>> So the read in that w8r8 would always get a zero back and then time out.
>> Changing the poll function (just for isc) to only read gave:
>>
>> op:     w       r       r
>> M:    0xB     0x0     0x0
>> S:            0x1     0x2
>>
>> For the code after the ISC enable, I reverted to your implementation
>> of the poll function & the rest of the programming sequence ran.
>>
>> I spoke to the guys that wrote the HW about this, and they said that
>> reading the status back *as* the 0x0 the poll command is clocked in is
>> the expected behaviour.
>> They also said that MPF should work identically to an MPFS and I was unable
>> to find a documented difference between MPF and MPFS other than the envm,
>> which is an optional component anyway.
>>
>> But I can only assume that what you were doing worked for you, so if
>> you could possibly share some waveforms of the write_init sequence
>> that'd be great. Or if there is something that you think I am
>> overlooking, please let me know.
>>
> 
> If you replace poll_status_not_busy() function with this code:
> 
> static int poll_status_not_busy(struct spi_device *spi, u8 mask)
> {
>          u8 status, status_command = MPF_SPI_READ_STATUS;
>          int ret, timeout = MPF_STATUS_POLL_TIMEOUT;
>          struct spi_transfer xfer = {
>                  .tx_buf = &status_command,
>                  .rx_buf = &status,
>                  .len = 1,
>          };
> 
>          while (timeout--) {
>                  ret = spi_sync_transfer(spi, &xfer, 1);
>                  if (ret < 0)
>                          return ret;
> 
>                  if (!(status & MPF_STATUS_BUSY) && (!mask || (status & mask)))
>                          return status;
> 
>                  usleep_range(1000, 2000);
>          }
> 
>          return -EBUSY;
> }
> 
> Will it work for you? It is still works in my case.

Yeah, status checking after the ISC enable works for me with
this changed. However, the mpf_ops_state still uses w8r8 &
will need to be changed too.

Thanks,
Conor.
diff mbox series

Patch

diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index 26025dbab353..791ecf48503a 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -248,4 +248,14 @@  config FPGA_MGR_VERSAL_FPGA
 	  configure the programmable logic(PL).
 
 	  To compile this as a module, choose M here.
+
+config FPGA_MGR_MICROCHIP_SPI
+	tristate "Microchip Polarfire SPI FPGA manager"
+	depends on SPI
+	select CRC_CCITT
+	help
+	  FPGA manager driver support for Microchip Polarfire FPGAs
+	  programming over slave SPI interface with .dat formatted
+	  bitstream image.
+
 endif # FPGA
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index 4da5273948df..fcb389ca4873 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -19,6 +19,7 @@  obj-$(CONFIG_FPGA_MGR_XILINX_SPI)	+= xilinx-spi.o
 obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA)	+= zynq-fpga.o
 obj-$(CONFIG_FPGA_MGR_ZYNQMP_FPGA)	+= zynqmp-fpga.o
 obj-$(CONFIG_FPGA_MGR_VERSAL_FPGA)      += versal-fpga.o
+obj-$(CONFIG_FPGA_MGR_MICROCHIP_SPI)	+= microchip-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/microchip-spi.c b/drivers/fpga/microchip-spi.c
new file mode 100644
index 000000000000..5e79677c29f4
--- /dev/null
+++ b/drivers/fpga/microchip-spi.c
@@ -0,0 +1,448 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Microchip Polarfire FPGA programming over slave SPI interface.
+ */
+
+#include <linux/crc-ccitt.h>
+#include <linux/delay.h>
+#include <linux/fpga/fpga-mgr.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/spi/spi.h>
+
+#define	MPF_SPI_ISC_ENABLE	0x0B
+#define	MPF_SPI_ISC_DISABLE	0x0C
+#define	MPF_SPI_READ_STATUS	0x00
+#define	MPF_SPI_READ_DATA	0x01
+#define	MPF_SPI_FRAME_INIT	0xAE
+#define	MPF_SPI_FRAME		0xEE
+#define	MPF_SPI_PRG_MODE	0x01
+#define	MPF_SPI_RELEASE		0x23
+
+#define	MPF_SPI_FRAME_SIZE	16
+
+#define	MPF_HEADER_SIZE_OFFSET	24
+#define	MPF_DATA_SIZE_OFFSET	55
+
+#define	MPF_LOOKUP_TABLE_RECORD_SIZE		9
+#define	MPF_LOOKUP_TABLE_BLOCK_ID_OFFSET	0
+#define	MPF_LOOKUP_TABLE_BLOCK_START_OFFSET	1
+
+#define	MPF_COMPONENTS_SIZE_ID	5
+#define	MPF_BITSTREAM_ID	8
+
+#define	MPF_BITS_PER_COMPONENT_SIZE	22
+
+#define	MPF_STATUS_POLL_TIMEOUT	1000
+#define	MPF_STATUS_BUSY		BIT(0)
+#define	MPF_STATUS_READY	BIT(1)
+
+struct mpf_priv {
+	struct spi_device *spi;
+	bool program_mode;
+};
+
+static enum fpga_mgr_states mpf_ops_state(struct fpga_manager *mgr)
+{
+	struct mpf_priv *priv = mgr->priv;
+	struct spi_device *spi;
+	bool program_mode;
+	ssize_t status;
+
+	program_mode = priv->program_mode;
+	spi = priv->spi;
+
+	status = spi_w8r8(spi, MPF_SPI_READ_STATUS);
+
+	if (!program_mode && !status)
+		return FPGA_MGR_STATE_OPERATING;
+
+	return FPGA_MGR_STATE_UNKNOWN;
+}
+
+static int poll_status_not_busy(struct spi_device *spi, u8 mask)
+{
+	ssize_t status, timeout = MPF_STATUS_POLL_TIMEOUT;
+
+	while (timeout--) {
+		status = spi_w8r8(spi, MPF_SPI_READ_STATUS);
+		if (status < 0)
+			return status;
+
+		if (!(status & MPF_STATUS_BUSY) && (!mask || (status & mask)))
+			return status;
+
+		usleep_range(1000, 2000);
+	}
+
+	return -EBUSY;
+}
+
+static int mpf_spi_write(struct spi_device *spi, const void *buf, size_t buf_size)
+{
+	int status = poll_status_not_busy(spi, 0);
+
+	if (status < 0)
+		return status;
+
+	return spi_write(spi, buf, buf_size);
+}
+
+static int mpf_spi_write_then_read(struct spi_device *spi,
+				   const void *txbuf, size_t txbuf_size,
+				   void *rxbuf, size_t rxbuf_size)
+{
+	const u8 read_command[] = { MPF_SPI_READ_DATA };
+	int ret;
+
+	ret = mpf_spi_write(spi, txbuf, txbuf_size);
+	if (ret)
+		return ret;
+
+	ret = poll_status_not_busy(spi, MPF_STATUS_READY);
+	if (ret < 0)
+		return ret;
+
+	return spi_write_then_read(spi, read_command, sizeof(read_command),
+				   rxbuf, rxbuf_size);
+}
+
+static int mpf_ops_write_init(struct fpga_manager *mgr,
+			      struct fpga_image_info *info, const char *buf,
+			      size_t count)
+{
+	const u8 program_mode[] = { MPF_SPI_FRAME_INIT, MPF_SPI_PRG_MODE };
+	const u8 isc_en_command[] = { MPF_SPI_ISC_ENABLE };
+	struct mpf_priv *priv = mgr->priv;
+	struct device *dev = &mgr->dev;
+	struct spi_device *spi;
+	u32 isc_ret;
+	int ret;
+
+	if (info->flags & FPGA_MGR_PARTIAL_RECONFIG) {
+		dev_err(dev, "Partial reconfiguration is not supported\n");
+		return -EOPNOTSUPP;
+	}
+
+	spi = priv->spi;
+
+	ret = mpf_spi_write_then_read(spi, isc_en_command, sizeof(isc_en_command),
+				      &isc_ret, sizeof(isc_ret));
+	if (ret || isc_ret) {
+		dev_err(dev, "Failed to enable ISC: %d\n", ret ? : isc_ret);
+		return -EFAULT;
+	}
+
+	ret = mpf_spi_write(spi, program_mode, sizeof(program_mode));
+	if (ret) {
+		dev_err(dev, "Failed to enter program mode: %d\n", ret);
+		return ret;
+	}
+
+	priv->program_mode = true;
+
+	return 0;
+}
+
+static int get_sgt_data(struct sg_table *sgt, size_t offset, void *data,
+			size_t data_size)
+{
+	struct sg_mapping_iter miter;
+	size_t remainder;
+
+	sg_miter_start(&miter, sgt->sgl, sgt->nents, SG_MITER_FROM_SG);
+	while (sg_miter_next(&miter)) {
+		if (offset < miter.length)
+			break;
+
+		offset -= miter.length;
+	}
+
+	if (!miter.addr) {
+		sg_miter_stop(&miter);
+		return -EFAULT;
+	}
+
+	remainder = max((ssize_t)0, (ssize_t)offset + (ssize_t)data_size -
+			(ssize_t)miter.length);
+
+	memcpy(data, miter.addr + offset, data_size - remainder);
+
+	while (remainder && sg_miter_next(&miter)) {
+		memcpy(data + data_size - remainder, miter.addr,
+		       min(remainder, miter.length));
+		remainder = max((ssize_t)0,
+				(ssize_t)remainder - (ssize_t)miter.length);
+	}
+	sg_miter_stop(&miter);
+
+	if (remainder)
+		return -EFAULT;
+
+	return 0;
+}
+
+static ssize_t lookup_block_start(struct sg_table *sgt, int id)
+{
+	size_t block_start_offset, block_id_offset, i;
+	u8 header_size, blocks_num, block_id;
+	u32 block_start;
+	int ret;
+
+	ret = get_sgt_data(sgt, MPF_HEADER_SIZE_OFFSET, &header_size,
+			   sizeof(header_size));
+	if (ret)
+		return ret;
+
+	ret = get_sgt_data(sgt, header_size - 1, &blocks_num,
+			   sizeof(blocks_num));
+	if (ret)
+		return ret;
+
+	for (i = 0; i < blocks_num; i++) {
+		block_id_offset = header_size +
+				  i * MPF_LOOKUP_TABLE_RECORD_SIZE +
+				  MPF_LOOKUP_TABLE_BLOCK_ID_OFFSET;
+
+		ret = get_sgt_data(sgt, block_id_offset, &block_id,
+				   sizeof(block_id));
+		if (ret)
+			return ret;
+
+		if (block_id == id) {
+			block_start_offset = header_size +
+					     i * MPF_LOOKUP_TABLE_RECORD_SIZE +
+					     MPF_LOOKUP_TABLE_BLOCK_START_OFFSET;
+
+			ret = get_sgt_data(sgt, block_start_offset, &block_start,
+					   sizeof(block_start));
+			if (ret)
+				return ret;
+
+			return le32_to_cpu(block_start);
+		}
+	}
+
+	return -EFAULT;
+}
+
+static ssize_t parse_bitstream_size(struct sg_table *sgt)
+{
+	size_t component_size_byte_num, component_size_byte_off,
+	       i, bitstream_size = 0;
+	ssize_t components_size_start;
+	u16 components_num;
+	u32 component_size;
+	int ret;
+
+	ret = get_sgt_data(sgt, MPF_DATA_SIZE_OFFSET, &components_num,
+			   sizeof(components_num));
+	if (ret)
+		return ret;
+
+	components_num = le16_to_cpu(components_num);
+
+	components_size_start = lookup_block_start(sgt, MPF_COMPONENTS_SIZE_ID);
+	if (components_size_start < 0)
+		return components_size_start;
+
+	for (i = 0; i < components_num; i++) {
+		component_size_byte_num =
+			(i * MPF_BITS_PER_COMPONENT_SIZE) / BITS_PER_BYTE;
+		component_size_byte_off =
+			(i * MPF_BITS_PER_COMPONENT_SIZE) % BITS_PER_BYTE;
+
+		ret = get_sgt_data(sgt, components_size_start +
+				   component_size_byte_num,
+				   &component_size, sizeof(component_size));
+		if (ret)
+			return ret;
+
+		component_size = le32_to_cpu(component_size);
+		component_size >>= component_size_byte_off;
+		component_size &= GENMASK(MPF_BITS_PER_COMPONENT_SIZE - 1, 0);
+
+		bitstream_size += component_size;
+	}
+
+	return bitstream_size;
+}
+
+static u16 crc_ccitt_sg(struct sg_table *sgt)
+{
+	struct sg_mapping_iter miter;
+	u16 crc = 0;
+
+	sg_miter_start(&miter, sgt->sgl, sgt->nents, SG_MITER_FROM_SG);
+	while (sg_miter_next(&miter))
+		crc = crc_ccitt(crc, miter.addr, miter.length);
+	sg_miter_stop(&miter);
+
+	return crc;
+}
+
+static int mpf_ops_write_sg(struct fpga_manager *mgr, struct sg_table *sgt)
+{
+	u8 tmp_buf[MPF_SPI_FRAME_SIZE + 1] = { MPF_SPI_FRAME, };
+	ssize_t bitstream_start, bitstream_size;
+	struct mpf_priv *priv = mgr->priv;
+	struct device *dev = &mgr->dev;
+	struct sg_mapping_iter miter;
+	struct spi_device *spi;
+	size_t remainder;
+	int ret, i;
+
+	if (crc_ccitt_sg(sgt)) {
+		dev_err(dev, "CRC error\n");
+		return -EINVAL;
+	}
+
+	bitstream_start = lookup_block_start(sgt, MPF_BITSTREAM_ID);
+	if (bitstream_start < 0) {
+		dev_err(dev, "Failed to find bitstream start %zd\n",
+			bitstream_start);
+		return bitstream_start;
+	}
+
+	bitstream_size = parse_bitstream_size(sgt);
+	if (bitstream_size < 0) {
+		dev_err(dev, "Failed to parse bitstream size %zd\n",
+			bitstream_size);
+		return bitstream_size;
+	}
+
+	spi = priv->spi;
+
+	sg_miter_start(&miter, sgt->sgl, sgt->nents, SG_MITER_FROM_SG);
+	for (i = 0; i < bitstream_size; i++) {
+		do {
+			if (bitstream_start < miter.length)
+				break;
+
+			bitstream_start -= miter.length;
+		} while (sg_miter_next(&miter));
+
+		remainder = max((ssize_t)0, bitstream_start +
+				(ssize_t)MPF_SPI_FRAME_SIZE -
+				(ssize_t)miter.length);
+
+		memcpy(tmp_buf + 1, miter.addr + bitstream_start,
+		       MPF_SPI_FRAME_SIZE - remainder);
+
+		while (remainder) {
+			bitstream_start -= miter.length;
+
+			if (sg_miter_next(&miter)) {
+				memcpy(tmp_buf + 1 + MPF_SPI_FRAME_SIZE -
+				       remainder, miter.addr,
+				       min(remainder, miter.length));
+				remainder = max((ssize_t)0, (ssize_t)remainder -
+						(ssize_t)miter.length);
+			} else {
+				dev_err(dev,
+					"Bitstream outruns firmware image. Frame %d of %zd\n",
+					i, bitstream_size);
+				sg_miter_stop(&miter);
+				return -EFAULT;
+			}
+		}
+
+		bitstream_start += MPF_SPI_FRAME_SIZE;
+
+		ret = mpf_spi_write(spi, tmp_buf, sizeof(tmp_buf));
+		if (ret) {
+			dev_err(dev,
+				"Failed to write bitstream frame number %d of %zd\n",
+				i, bitstream_size);
+			sg_miter_stop(&miter);
+			return ret;
+		}
+	}
+	sg_miter_stop(&miter);
+
+	return 0;
+}
+
+static int mpf_ops_write_complete(struct fpga_manager *mgr,
+				  struct fpga_image_info *info)
+{
+	const u8 isc_dis_command[] = { MPF_SPI_ISC_DISABLE };
+	const u8 release_command[] = { MPF_SPI_RELEASE };
+	struct mpf_priv *priv = mgr->priv;
+	struct device *dev = &mgr->dev;
+	struct spi_device *spi;
+	int ret;
+
+	spi = priv->spi;
+
+	ret = mpf_spi_write(spi, isc_dis_command, sizeof(isc_dis_command));
+	if (ret) {
+		dev_err(dev, "Failed to disable ISC: %d\n", ret);
+		return ret;
+	}
+
+	usleep_range(1000, 2000);
+
+	ret = mpf_spi_write(spi, release_command, sizeof(release_command));
+	if (ret) {
+		dev_err(dev, "Failed to exit program mode: %d\n", ret);
+		return ret;
+	}
+
+	priv->program_mode = false;
+
+	return 0;
+}
+
+static const struct fpga_manager_ops mpf_ops = {
+	.state = mpf_ops_state,
+	.write_init = mpf_ops_write_init,
+	.write_sg = mpf_ops_write_sg,
+	.write_complete = mpf_ops_write_complete,
+};
+
+static int mpf_probe(struct spi_device *spi)
+{
+	struct device *dev = &spi->dev;
+	struct fpga_manager *mgr;
+	struct mpf_priv *priv;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->spi = spi;
+
+	mgr = devm_fpga_mgr_register(dev, "Microchip Polarfire SPI FPGA Manager",
+				     &mpf_ops, priv);
+
+	return PTR_ERR_OR_ZERO(mgr);
+}
+
+static const struct spi_device_id mpf_spi_ids[] = {
+	{ .name = "mpf-spi-fpga-mgr", },
+	{},
+};
+MODULE_DEVICE_TABLE(spi, mpf_spi_ids);
+
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id mpf_of_ids[] = {
+	{ .compatible = "microchip,mpf-spi-fpga-mgr" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, mpf_of_ids);
+#endif /* IS_ENABLED(CONFIG_OF) */
+
+static struct spi_driver mpf_driver = {
+	.probe = mpf_probe,
+	.id_table = mpf_spi_ids,
+	.driver = {
+		.name = "microchip_mpf_spi_fpga_mgr",
+		.of_match_table = of_match_ptr(mpf_of_ids),
+	},
+};
+
+module_spi_driver(mpf_driver);
+
+MODULE_DESCRIPTION("Microchip Polarfire SPI FPGA Manager");
+MODULE_LICENSE("GPL");