Linux-FPGA Archive mirror
 help / color / mirror / Atom feed
From: Conor Dooley <conor.dooley@microchip.com>
To: <linux-spi@vger.kernel.org>, <linux-fpga@vger.kernel.org>
Cc: Mark Brown <broonie@kernel.org>, <conor@kernel.org>,
	Vladimir Georgiev <v.georgiev@metrotek.ru>,
	Moritz Fischer <mdf@kernel.org>, Wu Hao <hao.wu@intel.com>,
	Xu Yilun <yilun.xu@intel.com>, Tom Rix <trix@redhat.com>,
	<valentina.fernandezalanis@microchip.com>
Subject: [BUG] microchip-spi programming issue
Date: Tue, 6 Jun 2023 12:30:02 +0100	[thread overview]
Message-ID: <20230606-reentry-undusted-3c44fd7b4325@wendy> (raw)

[-- Attachment #1: Type: text/plain, Size: 2548 bytes --]

Hey folks,

A customer has reported an issue with the microchip-spi FPGA manager
driver that's causing programming of the FPGA to fail.
The culprit has been identified as the CS being deasserted between
the two transfers in mpf_spi_frame_write()

static int mpf_spi_frame_write(struct mpf_priv *priv, const char *buf)
{
	struct spi_transfer xfers[2] = {
		{
			.tx_buf = &priv->tx,
			.len = 1,
		}, {
			.tx_buf = buf,
			.len = MPF_SPI_FRAME_SIZE,
		},
	};
	int ret;

	ret = mpf_poll_status(priv, 0);
	if (ret < 0)
		return ret;

	priv->tx = MPF_SPI_FRAME;

	return spi_sync_transfer(priv->spi, xfers, ARRAY_SIZE(xfers));
}

which the system controller on the FPGA does not like & returns an
error.

I went poking around to see if this might've been another instance of
controller-specific behaviour like we'd seen fixes for during the
initial upstreaming of the driver, but I am not so sure this time
around that the fault is in the FPGA manager driver.

In spi.h, the kerneldoc for struct spi_transfer reads:
 * All SPI transfers start with the relevant chipselect active.  Normally
 * it stays selected until after the last transfer in a message.  Drivers
 * can affect the chipselect signal using cs_change

If I am not misunderstanding the SPI core, spi_sync_transfer() converts
the array of transfers into a message containing several transfers and
the controller should keep CS asserted between both transfers in this
message, since cs_change has not been set.

Following on from that, how strong is "normally" in the comment above?
Is it valid for a controller to deassert CS even if cs_change is not
set? Or have I totally misunderstood things and there's something
invalid about how the transfers are being set up in the driver?

The issue was reported against v6.1.20, but there have been no changes
to the SPI core or FPGA manager drivers in 6.1 kernels since v6.1.20.
The programming is being done with an i.MX8MP so I assume that
"nxp,imx8mp-fspi" or "fsl,imx8mp-ecspi" are the compatibles for the
controller in question.
The driver for the former doesn't appear to have been changed since
v6.1.20 & for the latter there is a single change, which would seem
unrelated.
If this isn't just me misunderstanding the SPI core, I'll go and request
the exact configuration.

The obvious/interim fix is to make sure that only one transfer is done,
but that seems like a hack to me. Either telling me I misunderstood the
SPI core and/or any suggestions about the correct flags for the
transfers would be really appreciated.

Cheers,
Conor.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

             reply	other threads:[~2023-06-06 11:32 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-06 11:30 Conor Dooley [this message]
2023-06-06 11:55 ` [BUG] microchip-spi programming issue Mark Brown
2023-06-06 12:01   ` Conor Dooley
2023-06-06 12:04     ` Mark Brown
2023-06-07 16:48       ` Conor Dooley

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=20230606-reentry-undusted-3c44fd7b4325@wendy \
    --to=conor.dooley@microchip.com \
    --cc=broonie@kernel.org \
    --cc=conor@kernel.org \
    --cc=hao.wu@intel.com \
    --cc=linux-fpga@vger.kernel.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=mdf@kernel.org \
    --cc=trix@redhat.com \
    --cc=v.georgiev@metrotek.ru \
    --cc=valentina.fernandezalanis@microchip.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 read-only IMAP folder(s) and NNTP newsgroup(s).