From: Ivan Bornyakov <i.bornyakov@metrotek.ru>
To: Conor.Dooley@microchip.com
Cc: yilun.xu@intel.com, mdf@kernel.org, hao.wu@intel.com,
trix@redhat.com, robh+dt@kernel.org,
krzysztof.kozlowski+dt@linaro.org, linux-fpga@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
system@metrotek.ru
Subject: Re: [PATCH v13 2/3] fpga: microchip-spi: add Microchip MPF FPGA manager
Date: Sun, 29 May 2022 21:51:58 +0300 [thread overview]
Message-ID: <20220529185158.gjoxrb65gpnh4g3k@x260> (raw)
In-Reply-To: <3d44115c-08c8-07ee-e707-617cfbe0352f@microchip.com>
On Sun, May 29, 2022 at 01:03:10PM +0000, Conor.Dooley@microchip.com wrote:
> On 29/05/2022 13:39, Xu Yilun wrote:
> > On Thu, May 26, 2022 at 09:13:43PM +0300, Ivan Bornyakov wrote:
> >> Add support to the FPGA manager for programming Microchip Polarfire
> >> FPGAs over slave SPI interface with .dat formatted bitsream image.
> >
> > From previous mail thread, there are still some hardware operations yet
> > to be clarified, so I may need a Reviewed-by from Conor.Dooley@microchip.com.
>
> Yeah, was really busy last week. Planning on giving this version a go
> tomorrow. I *think* I explained the reason the status check needed to be a
> sync_transfer() but it hasn't been reflected in a comment yet.
>
> I didn't know the answers to the two other questions & passed them on to the
> designers of the hardware blocks - but both are traveling so not got a
> response yet. There's one bit of clarification I'd like from your:
>
> >>> +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));
> >>
> >> As I mentioned before, is it possible we use spi_sync_transfer to avoid
> >> memcpy the whole bitstream?
> >
> > Unfortunately, I didn't succeed with spi_sunc_transfer here. May be
> > Conor or other folks with more insight on Microchip's HW would be able
> > to eliminate this memcpy...
>
> I understood this as being a question about why it was required to check
> the status of the hardware between frames of the bitstream rather than
> using spi_sync_transfer() to copy each frame back to back.
>
> Is that correct?
No.
The issue here is memcpy() a bitstream data frame to temporary buffer
before sending it to the device.
The reason for memcpy() is that we need to send to the device 17 bytes:
1st byte 0xEE and next 16 bytes - bitstream data.
Possible solution to eliminate memcpy() is to use spi_sync_transfer()
instead of spi_write() for sending bitstream frames, like so:
diff --git a/drivers/fpga/microchip-spi.c b/drivers/fpga/microchip-spi.c
index 7579b0de119f..bf62ee7fd630 100644
--- a/drivers/fpga/microchip-spi.c
+++ b/drivers/fpga/microchip-spi.c
@@ -270,7 +270,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;
@@ -285,10 +286,15 @@ static int mpf_ops_write(struct fpga_manager *mgr, const char *buf, size_t count
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);
+ xfers[0].tx_buf = &spi_frame_command;
+ xfers[0].len = 1;
+ xfers[1].tx_buf = buf + i * MPF_SPI_FRAME_SIZE;
+ xfers[1].len = MPF_SPI_FRAME_SIZE;
+
+ ret = mpf_poll_status(spi, 0);
+ if (ret >= 0)
+ ret = spi_sync_transfer(spi, xfers, 2);
- 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);
Note that this is not a working solution.
next prev parent reply other threads:[~2022-05-29 19:14 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-26 18:13 [PATCH v13 0/3] Microchip Polarfire FPGA manager Ivan Bornyakov
2022-05-26 18:13 ` [PATCH v13 1/3] fpga: fpga-mgr: support bitstream offset in image buffer Ivan Bornyakov
2022-05-29 12:27 ` Xu Yilun
2022-05-26 18:13 ` [PATCH v13 2/3] fpga: microchip-spi: add Microchip MPF FPGA manager Ivan Bornyakov
2022-05-29 12:39 ` Xu Yilun
2022-05-29 13:03 ` Conor.Dooley
2022-05-29 18:51 ` Ivan Bornyakov [this message]
2022-05-29 19:27 ` Conor.Dooley
2022-05-30 11:22 ` Conor.Dooley
2022-05-30 12:07 ` Ivan Bornyakov
2022-05-30 14:26 ` Conor.Dooley
2022-05-30 14:26 ` Ivan Bornyakov
2022-05-30 15:36 ` Conor.Dooley
2022-05-31 10:53 ` Conor.Dooley
2022-05-31 14:01 ` Ivan Bornyakov
2022-05-26 18:13 ` [PATCH v13 3/3] dt-bindings: fpga: add binding doc for microchip-spi fpga mgr Ivan Bornyakov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220529185158.gjoxrb65gpnh4g3k@x260 \
--to=i.bornyakov@metrotek.ru \
--cc=Conor.Dooley@microchip.com \
--cc=devicetree@vger.kernel.org \
--cc=hao.wu@intel.com \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=linux-fpga@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mdf@kernel.org \
--cc=robh+dt@kernel.org \
--cc=system@metrotek.ru \
--cc=trix@redhat.com \
--cc=yilun.xu@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).