lkml.org 
[lkml]   [2022]   [Jun]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH v14 2/3] fpga: microchip-spi: add Microchip MPF FPGA manager
    On Wed, Jun 01, 2022 at 07:49:48AM +0000, Conor.Dooley@microchip.com wrote:
    > On 31/05/2022 20:23, 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>
    >
    > Hey Ivan,
    > Could've kept at least the R-b tag from v13, but either way:
    > Tested-by: Conor Dooley <conor.dooley@microchip.com>
    > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
    >
    > I did have another look at Yiluns two questions.
    > The reason for the double status read effectively boils down
    > to the system controller potentially not having had enough
    > time to process the previous command/frame and update its
    > status yet.
    >
    > And see below about the memcpy()...
    >
    > > ---
    > > drivers/fpga/Kconfig | 9 +
    > > drivers/fpga/Makefile | 1 +
    > > drivers/fpga/microchip-spi.c | 384 +++++++++++++++++++++++++++++++++++
    > > 3 files changed, 394 insertions(+)
    > > create mode 100644 drivers/fpga/microchip-spi.c
    > >
    > > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
    > > index 26025dbab353..75806ef5c9ea 100644
    > > --- a/drivers/fpga/Kconfig
    > > +++ b/drivers/fpga/Kconfig
    > > @@ -248,4 +248,13 @@ 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
    > > + 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 e32bfa90f968..5425a15892df 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..413e5c364f68
    > ---8<---
    > > +
    > > +static int mpf_ops_write(struct fpga_manager *mgr, const char *buf, size_t count)
    > > +{
    > > + u8 tmp_buf[MPF_SPI_FRAME_SIZE + 1] = { MPF_SPI_FRAME, };
    > > + struct mpf_priv *priv = mgr->priv;
    > > + struct device *dev = &mgr->dev;
    > > + struct spi_device *spi;
    > > + int ret, i;
    > > +
    > > + if (count % MPF_SPI_FRAME_SIZE) {
    > > + dev_err(dev, "Bitstream size is not a multiple of %d\n",
    > > + MPF_SPI_FRAME_SIZE);
    > > + return -EINVAL;
    > > + }
    > > +
    > > + spi = priv->spi;
    > > +
    > > + for (i = 0; i < count / MPF_SPI_FRAME_SIZE; i++) {
    > > + memcpy(tmp_buf + 1, buf + i * MPF_SPI_FRAME_SIZE,
    > > + MPF_SPI_FRAME_SIZE);
    > > +
    > > + ret = mpf_spi_write(spi, tmp_buf, sizeof(tmp_buf));
    > > + if (ret) {
    > > + dev_err(dev, "Failed to write bitstream frame %d/%zu\n",
    > > + i, count / MPF_SPI_FRAME_SIZE);
    > > + return ret;
    > > + }
    > > + }
    > > +
    > > + return 0;
    > > +}
    >
    > I was able to program with the following diff, which is effectively
    > the same as the one you sent (which also worked for me):
    >
    > diff --git a/drivers/fpga/microchip-spi.c b/drivers/fpga/microchip-spi.c
    > index 413e5c364f68..fa6220e8056a 100644
    > --- a/drivers/fpga/microchip-spi.c
    > +++ b/drivers/fpga/microchip-spi.c
    > @@ -268,7 +268,8 @@ static int mpf_ops_write_init(struct fpga_manager *mgr,
    >
    > static int mpf_ops_write(struct fpga_manager *mgr, const char *buf, size_t count)
    > {
    > - u8 tmp_buf[MPF_SPI_FRAME_SIZE + 1] = { MPF_SPI_FRAME, };
    > + u8 spi_frame_command = MPF_SPI_FRAME;
    > + struct spi_transfer xfers[2] = { 0 };
    > struct mpf_priv *priv = mgr->priv;
    > struct device *dev = &mgr->dev;
    > struct spi_device *spi;
    > @@ -282,11 +283,16 @@ static int mpf_ops_write(struct fpga_manager *mgr, const char *buf, size_t count
    >
    > spi = priv->spi;
    >
    > + xfers[0].tx_buf = &spi_frame_command;
    > + xfers[0].len = 1;
    > +
    > for (i = 0; i < count / MPF_SPI_FRAME_SIZE; i++) {
    > - memcpy(tmp_buf + 1, buf + i * MPF_SPI_FRAME_SIZE,
    > - MPF_SPI_FRAME_SIZE);
    > + xfers[1].tx_buf = buf + i * MPF_SPI_FRAME_SIZE;
    > + xfers[1].len = MPF_SPI_FRAME_SIZE;
    >
    > - ret = mpf_spi_write(spi, tmp_buf, sizeof(tmp_buf));
    > + ret = mpf_poll_status(spi, 0);
    > + if (ret >= 0)
    > + ret = spi_sync_transfer(spi, xfers, 2);
    > if (ret) {
    > dev_err(dev, "Failed to write bitstream frame %d/%zu\n",
    > i, count / MPF_SPI_FRAME_SIZE);
    >
    > Have you been able to look at a waveform of programming like this
    > on your hardware? Off the top of my head, I was wondering if your
    > spi controller releases CS between every xfer?
    > Mine is perfectly happy not to release it - which is why I noticed
    > the extra cs_change in mpf_read_status().
    >
    > Thanks,
    > Conor.
    >

    Thanks for the hint. I'll ask our HW folks if it's possible.

    \
     
     \ /
      Last update: 2022-06-01 10:28    [W:2.609 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site