On Mon, May 09, 2022 at 11:41:18AM +0000, Conor.Dooley@microchip.com wrote:

Hey Ivan, one comment below.
Thanks,
Conor.

On 07/05/2022 08:43, Ivan Bornyakov wrote:

quoted

... snip ...
+static int mpf_read_status(struct spi_device *spi)
+{
+       u8 status, status_command = MPF_SPI_READ_STATUS;
+       struct spi_transfer xfer = {
+               .tx_buf = &status_command,
+               .rx_buf = &status,
+               .len = 1,
+       };
+       int ret = spi_sync_transfer(spi, &xfer, 1);
+
+       if ((status & MPF_STATUS_SPI_VIOLATION) ||
+           (status & MPF_STATUS_SPI_ERROR))
+               ret = -EIO;
+
+       return ret ? : status;
+}
+
... snip ...
+
+static int poll_status_not_busy(struct spi_device *spi, u8 mask)
+{
+       int status, timeout = MPF_STATUS_POLL_TIMEOUT;
+
+       while (timeout--) {
+               status = mpf_read_status(spi);
+               if (status < 0 ||
+                   (!(status & MPF_STATUS_BUSY) && (!mask || (status & mask))))
+                       return status;
+
+               usleep_range(1000, 2000);
+       }
+
+       return -EBUSY;
+}

Is there a reason you changed this from the snippet you sent me
in the responses to version 8:
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;
}

With the current version, I hit the "Failed to write bitstream
frame" check in mpf_ops_write at random points in the transfer.
Replacing poll_status_not_busy with the above allows it to run
to completion.

In my eyes they are equivalent, aren't they?