[BUG] microchip-spi programming issue

* [BUG] microchip-spi programming issue
@ 2023-06-06 11:30 Conor Dooley
  2023-06-06 11:55 ` Mark Brown
  0 siblings, 1 reply; 5+ messages in thread
From: Conor Dooley @ 2023-06-06 11:30 UTC (permalink / raw)
  To: linux-spi, linux-fpga
  Cc: Mark Brown, conor, Vladimir Georgiev, Moritz Fischer, Wu Hao,
	Xu Yilun, Tom Rix, valentina.fernandezalanis

[-- 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 --]

^ permalink raw reply	[flat|nested] 5+ messages in thread