Message ID | 20220322191552.13156-2-i.bornyakov@metrotek.ru (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | Microchip Polarfire FPGA manager | expand |
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 > >
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.
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 --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");
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